| Message ID | 1409649186-1046-10-git-send-email-ulrich.hecht+renesas@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Quoting Ulrich Hecht (2014-09-02 02:13:05) > From: Ulrich Hecht <ulrich.hecht@gmail.com> > > Support for setting the parent at initialization time based on the current > hardware configuration in DIV6 clocks with selectable parents as found in > the r8a73a4, r8a7740, sh73a0, and other SoCs. > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> I still have the same question about switching parents at run-time as I did in the last posting[0]. Is there a use case for modeling this clocks as proper muxes and switching parents dynamically? [0] http://lkml.kernel.org/r/<20140902002329.5251.82127@quantum> Regards, Mike > --- > .../bindings/clock/renesas,cpg-div6-clocks.txt | 12 +++++++- > drivers/clk/shmobile/clk-div6.c | 32 ++++++++++++++++++---- > 2 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt > index 952e373..2633ea1 100644 > --- a/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt > +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt > @@ -7,14 +7,24 @@ to 64. > Required Properties: > > - compatible: Must be one of the following > + - "renesas,r8a73a4-div6-clock" for R8A73A4 (R-Mobile APE6) DIV6 clocks > + - "renesas,r8a7740-div6-clock" for R8A7740 (R-Mobile A1) DIV6 clocks > - "renesas,r8a7790-div6-clock" for R8A7790 (R-Car H2) DIV6 clocks > - "renesas,r8a7791-div6-clock" for R8A7791 (R-Car M2) DIV6 clocks > + - "renesas,sh73a0-div6-clock" for SH73A0 (SH-MobileAG5) DIV6 clocks > - "renesas,cpg-div6-clock" for generic DIV6 clocks > - reg: Base address and length of the memory resource used by the DIV6 clock > - - clocks: Reference to the parent clock > + - clocks: Reference to the parent clock(s) > - #clock-cells: Must be 0 > - clock-output-names: The name of the clock as a free-form string > > +Optional Properties: > + > + - renesas,src-shift: Bit position of the input clock selector (default: > + fixed input clock; requires renesas,src-width) > + - renesas,src-width: Bit width of the input clock selector (default: fixed > + input clock; requires renesas,src-shift) > + > > Example > ------- > diff --git a/drivers/clk/shmobile/clk-div6.c b/drivers/clk/shmobile/clk-div6.c > index f065f69..f8b57bf 100644 > --- a/drivers/clk/shmobile/clk-div6.c > +++ b/drivers/clk/shmobile/clk-div6.c > @@ -39,8 +39,11 @@ struct div6_clock { > static int cpg_div6_clock_enable(struct clk_hw *hw) > { > struct div6_clock *clock = to_div6_clock(hw); > + u32 val; > > - clk_writel(CPG_DIV6_DIV(clock->div - 1), clock->reg); > + val = (clk_readl(clock->reg) & ~(CPG_DIV6_DIV_MASK | CPG_DIV6_CKSTP)) > + | CPG_DIV6_DIV(clock->div - 1); > + clk_writel(val, clock->reg); > > return 0; > } > @@ -52,7 +55,7 @@ static void cpg_div6_clock_disable(struct clk_hw *hw) > /* DIV6 clocks require the divisor field to be non-zero when stopping > * the clock. > */ > - clk_writel(CPG_DIV6_CKSTP | CPG_DIV6_DIV(CPG_DIV6_DIV_MASK), > + clk_writel(clk_readl(clock->reg) | CPG_DIV6_CKSTP | CPG_DIV6_DIV_MASK, > clock->reg); > } > > @@ -94,12 +97,14 @@ static int cpg_div6_clock_set_rate(struct clk_hw *hw, unsigned long rate, > { > struct div6_clock *clock = to_div6_clock(hw); > unsigned int div = cpg_div6_clock_calc_div(rate, parent_rate); > + u32 val; > > clock->div = div; > > + val = clk_readl(clock->reg) & ~CPG_DIV6_DIV_MASK; > /* Only program the new divisor if the clock isn't stopped. */ > - if (!(clk_readl(clock->reg) & CPG_DIV6_CKSTP)) > - clk_writel(CPG_DIV6_DIV(clock->div - 1), clock->reg); > + if (!(val & CPG_DIV6_CKSTP)) > + clk_writel(val | CPG_DIV6_DIV(clock->div - 1), clock->reg); > > return 0; > } > @@ -120,6 +125,8 @@ static void __init cpg_div6_clock_init(struct device_node *np) > const char *parent_name; > const char *name; > struct clk *clk; > + u32 src_shift; > + u32 src_width; > int ret; > > clock = kzalloc(sizeof(*clock), GFP_KERNEL); > @@ -150,7 +157,22 @@ static void __init cpg_div6_clock_init(struct device_node *np) > goto error; > } > > - parent_name = of_clk_get_parent_name(np, 0); > + if (!of_property_read_u32(np, "renesas,src-shift", &src_shift)) { > + if (!of_property_read_u32(np, "renesas,src-width", > + &src_width)) { > + unsigned int parent_idx = > + (clk_readl(clock->reg) >> src_shift) & > + (BIT(src_width) - 1); > + parent_name = of_clk_get_parent_name(np, parent_idx); > + } else { > + pr_err("%s: renesas,src-shift without renesas,src-width in %s\n", > + __func__, np->name); > + goto error; > + } > + } else { > + parent_name = of_clk_get_parent_name(np, 0); > + } > + > if (parent_name == NULL) { > pr_err("%s: failed to get %s DIV6 clock parent name\n", > __func__, np->name); > -- > 1.8.4.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 3, 2014 at 12:45 AM, Mike Turquette <mturquette@linaro.org> wrote: > Quoting Ulrich Hecht (2014-09-02 02:13:05) >> From: Ulrich Hecht <ulrich.hecht@gmail.com> >> >> Support for setting the parent at initialization time based on the current >> hardware configuration in DIV6 clocks with selectable parents as found in >> the r8a73a4, r8a7740, sh73a0, and other SoCs. >> >> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> > > I still have the same question about switching parents at run-time as I > did in the last posting[0]. Is there a use case for modeling this clocks > as proper muxes and switching parents dynamically? I have no idea, frankly. Laurent, can you comment on that? CU Uli -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulrich, Mike, On Wed, Sep 3, 2014 at 9:48 AM, Ulrich Hecht <ulrich.hecht+renesas@gmail.com> wrote: > On Wed, Sep 3, 2014 at 12:45 AM, Mike Turquette <mturquette@linaro.org> wrote: >> Quoting Ulrich Hecht (2014-09-02 02:13:05) >>> From: Ulrich Hecht <ulrich.hecht@gmail.com> >>> >>> Support for setting the parent at initialization time based on the current >>> hardware configuration in DIV6 clocks with selectable parents as found in >>> the r8a73a4, r8a7740, sh73a0, and other SoCs. >>> >>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> >>> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> >> >> I still have the same question about switching parents at run-time as I >> did in the last posting[0]. Is there a use case for modeling this clocks >> as proper muxes and switching parents dynamically? > > I have no idea, frankly. Laurent, can you comment on that? Technically, it seems to be possible to switch parents at run-time. Of course, if we do that, we need a way to set up the parents. It's my understanding this is done explicitly (through clk_set_parent()), or implicitly (through clk_set_rate() and propagation). Is that correct? The default would still be read from the hardware, like is done now, right? It seems we only configure rates explicitly for audio and video. For most drivers, we just use the rates as dictated by following the flow from the main crystal(s) through the clock topology and the divisors present within. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Geert Uytterhoeven (2014-09-03 02:03:06) > Hi Ulrich, Mike, > > On Wed, Sep 3, 2014 at 9:48 AM, Ulrich Hecht > <ulrich.hecht+renesas@gmail.com> wrote: > > On Wed, Sep 3, 2014 at 12:45 AM, Mike Turquette <mturquette@linaro.org> wrote: > >> Quoting Ulrich Hecht (2014-09-02 02:13:05) > >>> From: Ulrich Hecht <ulrich.hecht@gmail.com> > >>> > >>> Support for setting the parent at initialization time based on the current > >>> hardware configuration in DIV6 clocks with selectable parents as found in > >>> the r8a73a4, r8a7740, sh73a0, and other SoCs. > >>> > >>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > >>> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> > >> > >> I still have the same question about switching parents at run-time as I > >> did in the last posting[0]. Is there a use case for modeling this clocks > >> as proper muxes and switching parents dynamically? > > > > I have no idea, frankly. Laurent, can you comment on that? > > Technically, it seems to be possible to switch parents at run-time. Cool. You just need to know if switching parents is glitchless or not. If switching is glitchless then you have nothing to worry about. If not then you might need to set the CLK_SET_PARENT_GATE flag. > > Of course, if we do that, we need a way to set up the parents. > It's my understanding this is done explicitly (through clk_set_parent()), > or implicitly (through clk_set_rate() and propagation). Is that correct? Correct. > The default would still be read from the hardware, like is done now, right? This is done in __clk_init_parent(). This reads the hardware for mux clocks (which MUST implement the .get_parent callback) and updates the clock framework bookkeeping to use the correct parent at clock registration-time. Regards, Mike > > It seems we only configure rates explicitly for audio and video. > For most drivers, we just use the rates as dictated by following the flow > from the main crystal(s) through the clock topology and the divisors > present within. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulrich, On Wednesday 03 September 2014 09:48:28 Ulrich Hecht wrote: > On Wed, Sep 3, 2014 at 12:45 AM, Mike Turquette wrote: > > Quoting Ulrich Hecht (2014-09-02 02:13:05) > > > >> From: Ulrich Hecht <ulrich.hecht@gmail.com> > >> > >> Support for setting the parent at initialization time based on the > >> current hardware configuration in DIV6 clocks with selectable parents as > >> found in the r8a73a4, r8a7740, sh73a0, and other SoCs. > >> > >> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > I still have the same question about switching parents at run-time as I > > did in the last posting[0]. Is there a use case for modeling this clocks > > as proper muxes and switching parents dynamically? > > I have no idea, frankly. Laurent, can you comment on that? Technically it should be possible, but I don't know if we have a use case for it.
Hi Ulrich, Thank you for the patch. On Tuesday 02 September 2014 11:13:05 Ulrich Hecht wrote: > From: Ulrich Hecht <ulrich.hecht@gmail.com> > > Support for setting the parent at initialization time based on the current > hardware configuration in DIV6 clocks with selectable parents as found in > the r8a73a4, r8a7740, sh73a0, and other SoCs. > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > .../bindings/clock/renesas,cpg-div6-clocks.txt | 12 +++++++- Patches touching Documentation/devicetree must have devicetree@vger.kernel.orf CC'ed. > drivers/clk/shmobile/clk-div6.c | 32 +++++++++++++++---- > 2 files changed, 38 insertions(+), 6 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt > b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt index > 952e373..2633ea1 100644 > --- a/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt > +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt > @@ -7,14 +7,24 @@ to 64. > Required Properties: > > - compatible: Must be one of the following > + - "renesas,r8a73a4-div6-clock" for R8A73A4 (R-Mobile APE6) DIV6 clocks > + - "renesas,r8a7740-div6-clock" for R8A7740 (R-Mobile A1) DIV6 clocks > - "renesas,r8a7790-div6-clock" for R8A7790 (R-Car H2) DIV6 clocks > - "renesas,r8a7791-div6-clock" for R8A7791 (R-Car M2) DIV6 clocks > + - "renesas,sh73a0-div6-clock" for SH73A0 (SH-MobileAG5) DIV6 clocks > - "renesas,cpg-div6-clock" for generic DIV6 clocks > - reg: Base address and length of the memory resource used by the DIV6 > clock - - clocks: Reference to the parent clock > + - clocks: Reference to the parent clock(s) > - #clock-cells: Must be 0 > - clock-output-names: The name of the clock as a free-form string > > +Optional Properties: > + > + - renesas,src-shift: Bit position of the input clock selector (default: > + fixed input clock; requires renesas,src-width) > + - renesas,src-width: Bit width of the input clock selector (default: > fixed > + input clock; requires renesas,src-shift) I've proposed an alternate solution in a reply to the previous version of this patch. Please see http://www.spinics.net/lists/linux-sh/msg34936.html Let's discuss it in that mail thread. > + > > Example > ------- > diff --git a/drivers/clk/shmobile/clk-div6.c > b/drivers/clk/shmobile/clk-div6.c index f065f69..f8b57bf 100644 > --- a/drivers/clk/shmobile/clk-div6.c > +++ b/drivers/clk/shmobile/clk-div6.c > @@ -39,8 +39,11 @@ struct div6_clock { > static int cpg_div6_clock_enable(struct clk_hw *hw) > { > struct div6_clock *clock = to_div6_clock(hw); > + u32 val; > > - clk_writel(CPG_DIV6_DIV(clock->div - 1), clock->reg); > + val = (clk_readl(clock->reg) & ~(CPG_DIV6_DIV_MASK | CPG_DIV6_CKSTP)) > + | CPG_DIV6_DIV(clock->div - 1); > + clk_writel(val, clock->reg); > > return 0; > } > @@ -52,7 +55,7 @@ static void cpg_div6_clock_disable(struct clk_hw *hw) > /* DIV6 clocks require the divisor field to be non-zero when stopping > * the clock. > */ > - clk_writel(CPG_DIV6_CKSTP | CPG_DIV6_DIV(CPG_DIV6_DIV_MASK), > + clk_writel(clk_readl(clock->reg) | CPG_DIV6_CKSTP | CPG_DIV6_DIV_MASK, > clock->reg); > } > > @@ -94,12 +97,14 @@ static int cpg_div6_clock_set_rate(struct clk_hw *hw, > unsigned long rate, { > struct div6_clock *clock = to_div6_clock(hw); > unsigned int div = cpg_div6_clock_calc_div(rate, parent_rate); > + u32 val; > > clock->div = div; > > + val = clk_readl(clock->reg) & ~CPG_DIV6_DIV_MASK; > /* Only program the new divisor if the clock isn't stopped. */ > - if (!(clk_readl(clock->reg) & CPG_DIV6_CKSTP)) > - clk_writel(CPG_DIV6_DIV(clock->div - 1), clock->reg); > + if (!(val & CPG_DIV6_CKSTP)) > + clk_writel(val | CPG_DIV6_DIV(clock->div - 1), clock->reg); > > return 0; > } > @@ -120,6 +125,8 @@ static void __init cpg_div6_clock_init(struct > device_node *np) const char *parent_name; > const char *name; > struct clk *clk; > + u32 src_shift; > + u32 src_width; > int ret; > > clock = kzalloc(sizeof(*clock), GFP_KERNEL); > @@ -150,7 +157,22 @@ static void __init cpg_div6_clock_init(struct > device_node *np) goto error; > } > > - parent_name = of_clk_get_parent_name(np, 0); > + if (!of_property_read_u32(np, "renesas,src-shift", &src_shift)) { > + if (!of_property_read_u32(np, "renesas,src-width", > + &src_width)) { > + unsigned int parent_idx = > + (clk_readl(clock->reg) >> src_shift) & > + (BIT(src_width) - 1); > + parent_name = of_clk_get_parent_name(np, parent_idx); > + } else { > + pr_err("%s: renesas,src-shift without renesas,src-width in %s\n", > + __func__, np->name); > + goto error; > + } > + } else { > + parent_name = of_clk_get_parent_name(np, 0); > + } > + > if (parent_name == NULL) { > pr_err("%s: failed to get %s DIV6 clock parent name\n", > __func__, np->name);
diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt index 952e373..2633ea1 100644 --- a/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt @@ -7,14 +7,24 @@ to 64. Required Properties: - compatible: Must be one of the following + - "renesas,r8a73a4-div6-clock" for R8A73A4 (R-Mobile APE6) DIV6 clocks + - "renesas,r8a7740-div6-clock" for R8A7740 (R-Mobile A1) DIV6 clocks - "renesas,r8a7790-div6-clock" for R8A7790 (R-Car H2) DIV6 clocks - "renesas,r8a7791-div6-clock" for R8A7791 (R-Car M2) DIV6 clocks + - "renesas,sh73a0-div6-clock" for SH73A0 (SH-MobileAG5) DIV6 clocks - "renesas,cpg-div6-clock" for generic DIV6 clocks - reg: Base address and length of the memory resource used by the DIV6 clock - - clocks: Reference to the parent clock + - clocks: Reference to the parent clock(s) - #clock-cells: Must be 0 - clock-output-names: The name of the clock as a free-form string +Optional Properties: + + - renesas,src-shift: Bit position of the input clock selector (default: + fixed input clock; requires renesas,src-width) + - renesas,src-width: Bit width of the input clock selector (default: fixed + input clock; requires renesas,src-shift) + Example ------- diff --git a/drivers/clk/shmobile/clk-div6.c b/drivers/clk/shmobile/clk-div6.c index f065f69..f8b57bf 100644 --- a/drivers/clk/shmobile/clk-div6.c +++ b/drivers/clk/shmobile/clk-div6.c @@ -39,8 +39,11 @@ struct div6_clock { static int cpg_div6_clock_enable(struct clk_hw *hw) { struct div6_clock *clock = to_div6_clock(hw); + u32 val; - clk_writel(CPG_DIV6_DIV(clock->div - 1), clock->reg); + val = (clk_readl(clock->reg) & ~(CPG_DIV6_DIV_MASK | CPG_DIV6_CKSTP)) + | CPG_DIV6_DIV(clock->div - 1); + clk_writel(val, clock->reg); return 0; } @@ -52,7 +55,7 @@ static void cpg_div6_clock_disable(struct clk_hw *hw) /* DIV6 clocks require the divisor field to be non-zero when stopping * the clock. */ - clk_writel(CPG_DIV6_CKSTP | CPG_DIV6_DIV(CPG_DIV6_DIV_MASK), + clk_writel(clk_readl(clock->reg) | CPG_DIV6_CKSTP | CPG_DIV6_DIV_MASK, clock->reg); } @@ -94,12 +97,14 @@ static int cpg_div6_clock_set_rate(struct clk_hw *hw, unsigned long rate, { struct div6_clock *clock = to_div6_clock(hw); unsigned int div = cpg_div6_clock_calc_div(rate, parent_rate); + u32 val; clock->div = div; + val = clk_readl(clock->reg) & ~CPG_DIV6_DIV_MASK; /* Only program the new divisor if the clock isn't stopped. */ - if (!(clk_readl(clock->reg) & CPG_DIV6_CKSTP)) - clk_writel(CPG_DIV6_DIV(clock->div - 1), clock->reg); + if (!(val & CPG_DIV6_CKSTP)) + clk_writel(val | CPG_DIV6_DIV(clock->div - 1), clock->reg); return 0; } @@ -120,6 +125,8 @@ static void __init cpg_div6_clock_init(struct device_node *np) const char *parent_name; const char *name; struct clk *clk; + u32 src_shift; + u32 src_width; int ret; clock = kzalloc(sizeof(*clock), GFP_KERNEL); @@ -150,7 +157,22 @@ static void __init cpg_div6_clock_init(struct device_node *np) goto error; } - parent_name = of_clk_get_parent_name(np, 0); + if (!of_property_read_u32(np, "renesas,src-shift", &src_shift)) { + if (!of_property_read_u32(np, "renesas,src-width", + &src_width)) { + unsigned int parent_idx = + (clk_readl(clock->reg) >> src_shift) & + (BIT(src_width) - 1); + parent_name = of_clk_get_parent_name(np, parent_idx); + } else { + pr_err("%s: renesas,src-shift without renesas,src-width in %s\n", + __func__, np->name); + goto error; + } + } else { + parent_name = of_clk_get_parent_name(np, 0); + } + if (parent_name == NULL) { pr_err("%s: failed to get %s DIV6 clock parent name\n", __func__, np->name);