diff for duplicates of <20150927053320.3201.92585@quantum> diff --git a/a/1.txt b/N1/1.txt index 877340e..3b0e8ae 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,37 +1,29 @@ Quoting Magnus Damm (2015-09-07 09:54:03) > Hi Geert, Mike, Stephen, all, -> = - -> On Thu, Sep 3, 2015 at 5:22 PM, Geert Uytterhoeven <geert@linux-m68k.org>= - wrote: +> +> On Thu, Sep 3, 2015 at 5:22 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Ping? -> = - +> > Btw, I've just sent off the V7 series which still has this issue: > [PATCH v7 00/05] Renesas R-Car Gen3 CPG support V7 -> = - +> > From my point of view it looks like some core CCF changes are needed > to allow us to drop "clock-output-names" for the CPG node. I've > written some ugly prototype code to patch things together and I've > pasted it in as a proof-of-concent patch below. My apologies in > advance if gmail mangled the inline patch somehow... -> = - +> > > On Mon, Aug 31, 2015 at 5:56 PM, Geert Uytterhoeven > > <geert@linux-m68k.org> wrote: -> >> On Mon, Aug 31, 2015 at 2:49 PM, Magnus Damm <magnus.damm@gmail.com> w= -rote: +> >> On Mon, Aug 31, 2015 at 2:49 PM, Magnus Damm <magnus.damm@gmail.com> wrote: > >>> From: Gaku Inami <gaku.inami.xw@bp.renesas.com> > >>> > >>> This V5 patch adds initial CPG support for R-Car Generation 3 and in > >>> particular the R8A7795 SoC. > >>> -> >>> The R-Car Gen3 clock hardware has a register write protection feature= - that +> >>> The R-Car Gen3 clock hardware has a register write protection feature that > >>> needs to be shared between the CPG function needs to be shared between -> >>> the CPG and MSTP hardware somehow. So far this feature is simply igno= -red. +> >>> the CPG and MSTP hardware somehow. So far this feature is simply ignored. > >>> > >>> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com> > >>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> @@ -44,8 +36,7 @@ red. > >>> - of_property_count_strings() -> of_property_count_u32_elems() fix > >>> > >>> Changes since V3: (Magnus Damm <damm+renesas@opensource.se>) -> >>> - Reworked driver to incorporate most feedback from Stephen Boyd - t= -hanks!! +> >>> - Reworked driver to incorporate most feedback from Stephen Boyd - thanks!! > >>> - Major things like syscon and driver model require more discussion. > >>> - Added hunk to build drivers/clk/shmobile if ARCH_RENESAS is set. > >>> @@ -53,18 +44,15 @@ hanks!! > >>> - Reworked driver to rely on clock index instead of strings. > >>> - Dropped use of "clock-output-names". > >> -> >> Unfortunately dropping "clock-output-names" causes a regression: it tu= -rns -> >> out all fixed-factor clocks that have <&cpg_clocks R8A7795_CLK_*> as a= - parent +> >> Unfortunately dropping "clock-output-names" causes a regression: it turns +> >> out all fixed-factor clocks that have <&cpg_clocks R8A7795_CLK_*> as a parent > >> have lost their parent clock, and have rate zero. > >> > >> E.g. instead of > >> > >> clock enable_cnt prepare_cnt rate > >> accuracy phase -> >> ----------------------------------------------------------------------= ------------------- +> >> ---------------------------------------------------------------------------------------- > >> extal 1 1 16666600 > >> 50000 0 > >> main 1 1 8333300 @@ -84,8 +72,7 @@ rns > >> > >> clock enable_cnt prepare_cnt rate > >> accuracy phase -> >> ----------------------------------------------------------------------= ------------------- +> >> ---------------------------------------------------------------------------------------- > >> extal 0 0 16666600 > >> 50000 0 > >> main 0 0 8333300 @@ -103,59 +90,44 @@ rns > >> > >> Surprisingly, the system still works. > >> -> >> Several clock drivers (e.g. fixed-factor-clock) use of_clk_get_parent_= -name() +> >> Several clock drivers (e.g. fixed-factor-clock) use of_clk_get_parent_name() > >> to find the name of the parent clock. -> >> In the absence of "clock-output-names", this returns the name of the d= -evice -> >> node of the parent's clock. Which is always "cpg_clocks", and no longe= -r the +> >> In the absence of "clock-output-names", this returns the name of the device +> >> node of the parent's clock. Which is always "cpg_clocks", and no longer the > >> name of the clock matching the index. > >> -> >> I believe the same issue is present for MSTP clocks, but there we don'= -t have +> >> I believe the same issue is present for MSTP clocks, but there we don't have > >> children clocks, so it doesn't manifest itself (yet). > >> -> >> A quick workaround is to just re-add the clock-output-names to r8a7795= -.dtsi: +> >> A quick workaround is to just re-add the clock-output-names to r8a7795.dtsi: > >> -> >> clock-output-names =3D -> >> "main", "pll0", "pll1","pll2", +> >> clock-output-names > >> "main", "pll0", "pll1","pll2", > >> "pll3", "pll4"; > >> -> >> Hence it seems like dropping "clock-output-names" for clocks with mult= -iple +> >> Hence it seems like dropping "clock-output-names" for clocks with multiple > >> outputs is not such a good idea? > >> -> >> See also my question in "Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add i= -nitial -> >> r8a7795 SoC support" (http://www.spinics.net/lists/linux-sh/msg44609.h= -tml): +> >> See also my question in "Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial +> >> r8a7795 SoC support" (http://www.spinics.net/lists/linux-sh/msg44609.html): > >> -> >> | BTW, was "dropping clock-output-names" a general comment for all clo= -ck +> >> | BTW, was "dropping clock-output-names" a general comment for all clock > >> | drivers, or specific to the SoC's Clock Pulse Generator? > >> | -> >> | For e.g. "fixed-factor-clock", the driver falls back to using the no= -de's +> >> | For e.g. "fixed-factor-clock", the driver falls back to using the node's > >> | name if "clock-output-names" is not present. > >> | -> >> | But e.g. for MSTP clocks, that can't be done, as the clocks wouldn't= - have -> >> | names in that case (single node with multiple clocks). Unless we sta= -rt +> >> | But e.g. for MSTP clocks, that can't be done, as the clocks wouldn't have +> >> | names in that case (single node with multiple clocks). Unless we start > >> | fabricating them from the node name and the indices." > >> > >> For MSTP, we started fabricating them, but that doesn't solve the > >> of_clk_get_parent_name() issue. > >> > >> Thanks for your comments! -> = - +> > Thanks for making this very useful summary. It made it relatively easy > for me to understand what was going on. -> = - +> > From my side I've noticed exactly the same thing as you describe. So > in case we have more than one clock provided by a single node the > of_clk_get_parent_name() function will not work as expected unless @@ -163,15 +135,13 @@ rt > drivers for renesas hardware is built using a single node with > multiple indices, and today we use clock-output-names so we have no > issues for existing platforms. -> = - +> > For the upcoming R-Car Gen3 SoC we were asked to get rid of > clock-output-names if possible, but it seems that is difficult to do > without changing core CCF code. The patch below is my proporsal how to > fix the issue. It needs to be reworked to handle memory allocation > properly, but hopefully there are better ways to fix this. -> = - +> > The main portion of the patch below is based around a new function > called of_clk_get_name(). The idea is that it will check a certain > node if the clock-indices property is present together with @@ -179,19 +149,16 @@ rt > of that. If no clock-output-names is provided and no clock-indices > then the node name is used as-is, but if clock-indices is set then a > "%s.%u" name is generated based on node name and clock-index. -> = - +> > Included in the patch is also some debug code and minor changes to > make use of of_clk_get_name() from of_clk_get_parent_name() and a > couple of Renesas-specific drivers. With this patch the debugfs clock > frequency propagation is unbroken again, but unfortunately we now have -> memory leaks instead. =3D) -> = - +> memory leaks instead. =) +> > The prototype is only written to highlight what that the problem is. > Not for upstream merge. -> = - +> > Not-yet-Signed-off-by: Magnus Damm <damm+renesas@opensource.se> Magnus, @@ -216,38 +183,32 @@ Regards, Mike > --- -> = - +> > drivers/clk/clk-fixed-factor.c | 3 ++ -> drivers/clk/clk.c | 46 ++++++++++++++++++++++------= ------- +> drivers/clk/clk.c | 46 ++++++++++++++++++++++------------ > drivers/clk/shmobile/clk-mstp.c | 3 -- > drivers/clk/shmobile/clk-rcar-gen3.c | 13 +-------- > include/linux/clk-provider.h | 2 + > 5 files changed, 39 insertions(+), 28 deletions(-) -> = - +> > --- 0001/drivers/clk/clk-fixed-factor.c -> +++ work/drivers/clk/clk-fixed-factor.c 2015-09-07 21:49:36.472366518 = -+0900 +> +++ work/drivers/clk/clk-fixed-factor.c 2015-09-07 21:49:36.472366518 +0900 > @@ -82,6 +82,9 @@ struct clk *clk_register_fixed_factor(st > if (!fix) > return ERR_PTR(-ENOMEM); -> = - +> > + printk("xxxx name %s parent name %s %i %i\n", name, parent_name, > + mult, div); > + > /* struct clk_fixed_factor assignments */ -> fix->mult =3D mult; -> fix->div =3D div; +> fix->mult = mult; +> fix->div = div; > --- 0001/drivers/clk/clk.c > +++ work/drivers/clk/clk.c 2015-09-07 22:08:32.472366518 +0900 > @@ -3045,31 +3045,25 @@ int of_clk_get_parent_count(struct devic > } > EXPORT_SYMBOL_GPL(of_clk_get_parent_count); -> = - +> > -const char *of_clk_get_parent_name(struct device_node *np, int index) > +const char *of_clk_get_name(struct device_node *np, int index) > { @@ -258,42 +219,36 @@ Mike > u32 pv; > - int rc; > int count; -> + bool has_indices =3D false; -> = - +> + bool has_indices = false; +> > if (index < 0) > return NULL; -> = - -> - rc =3D of_parse_phandle_with_args(np, "clocks", "#clock-cells", inde= -x, +> +> - rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index, > - &clkspec); > - if (rc) > - return NULL; > - -> - index =3D clkspec.args_count ? clkspec.args[0] : 0; -> count =3D 0; -> = - +> - index = clkspec.args_count ? clkspec.args[0] : 0; +> count = 0; +> > /* if there is an indices property, use it to transfer the index -> * specified into an array offset for the clock-output-names propert= -y. +> * specified into an array offset for the clock-output-names property. > */ > - of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) { > + of_property_for_each_u32(np, "clock-indices", prop, vp, pv) { -> + has_indices =3D true; -> if (index =3D=3D pv) { -> index =3D count; +> + has_indices = true; +> if (index = pv) { +> index = count; > break; > @@ -3077,12 +3071,34 @@ const char *of_clk_get_parent_name(struc > count++; > } -> = - +> > - if (of_property_read_string_index(clkspec.np, "clock-output-names", > - index, > - &clk_name) < 0) -> - clk_name =3D clkspec.np->name; +> - clk_name = clkspec.np->name; > + if (of_property_read_string_index(np, "clock-output-names", index, > + &clk_name) < 0) { > + if (has_indices) @@ -309,19 +264,17 @@ y. > +const char *of_clk_get_parent_name(struct device_node *np, int index) > +{ > + struct of_phandle_args clkspec; -> + const char *clk_name =3D NULL; +> + const char *clk_name = NULL; > + int rc; > + > + if (index < 0) > + return NULL; -> = - +> > - of_node_put(clkspec.np); -> + rc =3D of_parse_phandle_with_args(np, "clocks", "#clock-cells", inde= -x, +> + rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index, > + &clkspec); > + if (!rc) { -> + clk_name =3D of_clk_get_name(clkspec.np, clkspec.args_count ? +> + clk_name = of_clk_get_name(clkspec.np, clkspec.args_count ? > + clkspec.args[0] : 0); > + of_node_put(clkspec.np); > + } @@ -329,19 +282,16 @@ x, > } > EXPORT_SYMBOL_GPL(of_clk_get_parent_name); > --- 0017/drivers/clk/shmobile/clk-mstp.c -> +++ work/drivers/clk/shmobile/clk-mstp.c 2015-09-07 22:04:50.942366518= - +0900 +> +++ work/drivers/clk/shmobile/clk-mstp.c 2015-09-07 22:04:50.942366518 +0900 > @@ -216,8 +216,7 @@ static void __init cpg_mstp_clocks_init( -> = - +> > if (of_property_read_string_index(np, "clock-output-names", > i, &name) < 0) -> - allocated_name =3D name =3D kasprintf(GFP_KERNEL, "%s.%u", +> - allocated_name = name = kasprintf(GFP_KERNEL, "%s.%u", > - np->name, clkidx); -> + allocated_name =3D name =3D of_clk_get_name(np, clkidx); -> = - -> clks[clkidx] =3D cpg_mstp_clock_register(name, parent_name, +> + allocated_name = name = of_clk_get_name(np, clkidx); +> +> clks[clkidx] = cpg_mstp_clock_register(name, parent_name, > clkidx, group); > --- 0020/drivers/clk/shmobile/clk-rcar-gen3.c > +++ work/drivers/clk/shmobile/clk-rcar-gen3.c 2015-09-07 @@ -349,15 +299,14 @@ x, > @@ -26,15 +26,6 @@ > #define RCAR_GEN3_CLK_PLL4 5 > #define RCAR_GEN3_CLK_NR 6 -> = - -> -static const char * const rcar_gen3_clk_names[RCAR_GEN3_CLK_NR] =3D { -> - [RCAR_GEN3_CLK_MAIN] =3D "main", -> - [RCAR_GEN3_CLK_PLL0] =3D "pll0", -> - [RCAR_GEN3_CLK_PLL1] =3D "pll1", -> - [RCAR_GEN3_CLK_PLL2] =3D "pll2", -> - [RCAR_GEN3_CLK_PLL3] =3D "pll3", -> - [RCAR_GEN3_CLK_PLL4] =3D "pll4", +> +> -static const char * const rcar_gen3_clk_names[RCAR_GEN3_CLK_NR] = { +> - [RCAR_GEN3_CLK_MAIN] = "main", +> - [RCAR_GEN3_CLK_PLL0] = "pll0", +> - [RCAR_GEN3_CLK_PLL1] = "pll1", +> - [RCAR_GEN3_CLK_PLL2] = "pll2", +> - [RCAR_GEN3_CLK_PLL3] = "pll3", +> - [RCAR_GEN3_CLK_PLL4] = "pll4", > -}; > - > struct rcar_gen3_cpg { @@ -367,25 +316,22 @@ x, > const struct cpg_pll_config *config, > unsigned int gen3_clk) > { -> - const char *parent_name =3D rcar_gen3_clk_names[RCAR_GEN3_CLK_MAIN]; -> + const char *parent_name =3D of_clk_get_name(np, RCAR_GEN3_CLK_MAIN); -> unsigned int mult =3D 1; -> unsigned int div =3D 1; +> - const char *parent_name = rcar_gen3_clk_names[RCAR_GEN3_CLK_MAIN]; +> + const char *parent_name = of_clk_get_name(np, RCAR_GEN3_CLK_MAIN); +> unsigned int mult = 1; +> unsigned int div = 1; > u32 value; > @@ -177,7 +168,7 @@ rcar_gen3_cpg_register_clk(struct device > return ERR_PTR(-EINVAL); > } -> = - +> > - return clk_register_fixed_factor(NULL, rcar_gen3_clk_names[gen3_clk], > + return clk_register_fixed_factor(NULL, of_clk_get_name(np, gen3_clk), > parent_name, 0, mult, div); > } -> = - +> > --- 0001/include/linux/clk-provider.h -> +++ work/include/linux/clk-provider.h 2015-09-07 21:35:54.332366518 +0= -900 +> +++ work/include/linux/clk-provider.h 2015-09-07 21:35:54.332366518 +0900 > @@ -665,6 +665,8 @@ struct clk *of_clk_src_onecell_get(struc > int of_clk_get_parent_count(struct device_node *np); > int of_clk_parent_fill(struct device_node *np, const char **parents, @@ -393,6 +339,5 @@ x, > + > +const char *of_clk_get_name(struct device_node *np, int index); > const char *of_clk_get_parent_name(struct device_node *np, int index); -> = - +> > void of_clk_init(const struct of_device_id *matches); diff --git a/a/content_digest b/N1/content_digest index 5daab9b..371a8b0 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -2,8 +2,8 @@ "ref\0CAMuHMdWScUcBuAMcYzkTi1wV3OnkDJ7Vb86iuD_OJfzDmTKTQg@mail.gmail.com\0" "ref\0CANqRtoSmr_7SGxRtvYY-d3pObuU+NPjFqTiSym2NZMWGqdAyFA@mail.gmail.com\0" "From\0Michael Turquette <mturquette@baylibre.com>\0" - "Subject\0Re: Regression due to dropping \"clock-output-names\" (was: Re: [PATCH v5 02/05] clk: shmobile: Add Renesas R-Car Gen3 CPG support)\0" - "Date\0Sat, 26 Sep 2015 22:33:20 -0700\0" + "Subject\0Re: Regression due to dropping \"clock-output-names\" (was: Re: [PATCH v5 02/05] clk: shmobile: Add Re\0" + "Date\0Sun, 27 Sep 2015 05:33:20 +0000\0" "To\0Magnus Damm <magnus.damm@gmail.com>" " Geert Uytterhoeven <geert@linux-m68k.org>\0" "Cc\0Stephen Boyd <sboyd@codeaurora.org>" @@ -17,38 +17,30 @@ "b\0" "Quoting Magnus Damm (2015-09-07 09:54:03)\n" "> Hi Geert, Mike, Stephen, all,\n" - "> =\n" - "\n" - "> On Thu, Sep 3, 2015 at 5:22 PM, Geert Uytterhoeven <geert@linux-m68k.org>=\n" - " wrote:\n" + "> \n" + "> On Thu, Sep 3, 2015 at 5:22 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:\n" "> > Ping?\n" - "> =\n" - "\n" + "> \n" "> Btw, I've just sent off the V7 series which still has this issue:\n" "> [PATCH v7 00/05] Renesas R-Car Gen3 CPG support V7\n" - "> =\n" - "\n" + "> \n" "> From my point of view it looks like some core CCF changes are needed\n" "> to allow us to drop \"clock-output-names\" for the CPG node. I've\n" "> written some ugly prototype code to patch things together and I've\n" "> pasted it in as a proof-of-concent patch below. My apologies in\n" "> advance if gmail mangled the inline patch somehow...\n" - "> =\n" - "\n" + "> \n" "> > On Mon, Aug 31, 2015 at 5:56 PM, Geert Uytterhoeven\n" "> > <geert@linux-m68k.org> wrote:\n" - "> >> On Mon, Aug 31, 2015 at 2:49 PM, Magnus Damm <magnus.damm@gmail.com> w=\n" - "rote:\n" + "> >> On Mon, Aug 31, 2015 at 2:49 PM, Magnus Damm <magnus.damm@gmail.com> wrote:\n" "> >>> From: Gaku Inami <gaku.inami.xw@bp.renesas.com>\n" "> >>>\n" "> >>> This V5 patch adds initial CPG support for R-Car Generation 3 and in\n" "> >>> particular the R8A7795 SoC.\n" "> >>>\n" - "> >>> The R-Car Gen3 clock hardware has a register write protection feature=\n" - " that\n" + "> >>> The R-Car Gen3 clock hardware has a register write protection feature that\n" "> >>> needs to be shared between the CPG function needs to be shared between\n" - "> >>> the CPG and MSTP hardware somehow. So far this feature is simply igno=\n" - "red.\n" + "> >>> the CPG and MSTP hardware somehow. So far this feature is simply ignored.\n" "> >>>\n" "> >>> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>\n" "> >>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>\n" @@ -61,8 +53,7 @@ "> >>> - of_property_count_strings() -> of_property_count_u32_elems() fix\n" "> >>>\n" "> >>> Changes since V3: (Magnus Damm <damm+renesas@opensource.se>)\n" - "> >>> - Reworked driver to incorporate most feedback from Stephen Boyd - t=\n" - "hanks!!\n" + "> >>> - Reworked driver to incorporate most feedback from Stephen Boyd - thanks!!\n" "> >>> - Major things like syscon and driver model require more discussion.\n" "> >>> - Added hunk to build drivers/clk/shmobile if ARCH_RENESAS is set.\n" "> >>>\n" @@ -70,18 +61,15 @@ "> >>> - Reworked driver to rely on clock index instead of strings.\n" "> >>> - Dropped use of \"clock-output-names\".\n" "> >>\n" - "> >> Unfortunately dropping \"clock-output-names\" causes a regression: it tu=\n" - "rns\n" - "> >> out all fixed-factor clocks that have <&cpg_clocks R8A7795_CLK_*> as a=\n" - " parent\n" + "> >> Unfortunately dropping \"clock-output-names\" causes a regression: it turns\n" + "> >> out all fixed-factor clocks that have <&cpg_clocks R8A7795_CLK_*> as a parent\n" "> >> have lost their parent clock, and have rate zero.\n" "> >>\n" "> >> E.g. instead of\n" "> >>\n" "> >> clock enable_cnt prepare_cnt rate\n" "> >> accuracy phase\n" - "> >> ----------------------------------------------------------------------=\n" - "------------------\n" + "> >> ----------------------------------------------------------------------------------------\n" "> >> extal 1 1 16666600\n" "> >> 50000 0\n" "> >> main 1 1 8333300\n" @@ -101,8 +89,7 @@ "> >>\n" "> >> clock enable_cnt prepare_cnt rate\n" "> >> accuracy phase\n" - "> >> ----------------------------------------------------------------------=\n" - "------------------\n" + "> >> ----------------------------------------------------------------------------------------\n" "> >> extal 0 0 16666600\n" "> >> 50000 0\n" "> >> main 0 0 8333300\n" @@ -120,59 +107,44 @@ "> >>\n" "> >> Surprisingly, the system still works.\n" "> >>\n" - "> >> Several clock drivers (e.g. fixed-factor-clock) use of_clk_get_parent_=\n" - "name()\n" + "> >> Several clock drivers (e.g. fixed-factor-clock) use of_clk_get_parent_name()\n" "> >> to find the name of the parent clock.\n" - "> >> In the absence of \"clock-output-names\", this returns the name of the d=\n" - "evice\n" - "> >> node of the parent's clock. Which is always \"cpg_clocks\", and no longe=\n" - "r the\n" + "> >> In the absence of \"clock-output-names\", this returns the name of the device\n" + "> >> node of the parent's clock. Which is always \"cpg_clocks\", and no longer the\n" "> >> name of the clock matching the index.\n" "> >>\n" - "> >> I believe the same issue is present for MSTP clocks, but there we don'=\n" - "t have\n" + "> >> I believe the same issue is present for MSTP clocks, but there we don't have\n" "> >> children clocks, so it doesn't manifest itself (yet).\n" "> >>\n" - "> >> A quick workaround is to just re-add the clock-output-names to r8a7795=\n" - ".dtsi:\n" + "> >> A quick workaround is to just re-add the clock-output-names to r8a7795.dtsi:\n" "> >>\n" - "> >> clock-output-names =3D\n" - "> >> \"main\", \"pll0\", \"pll1\",\"pll2\",\n" + "> >> clock-output-names > >> \"main\", \"pll0\", \"pll1\",\"pll2\",\n" "> >> \"pll3\", \"pll4\";\n" "> >>\n" - "> >> Hence it seems like dropping \"clock-output-names\" for clocks with mult=\n" - "iple\n" + "> >> Hence it seems like dropping \"clock-output-names\" for clocks with multiple\n" "> >> outputs is not such a good idea?\n" "> >>\n" - "> >> See also my question in \"Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add i=\n" - "nitial\n" - "> >> r8a7795 SoC support\" (http://www.spinics.net/lists/linux-sh/msg44609.h=\n" - "tml):\n" + "> >> See also my question in \"Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial\n" + "> >> r8a7795 SoC support\" (http://www.spinics.net/lists/linux-sh/msg44609.html):\n" "> >>\n" - "> >> | BTW, was \"dropping clock-output-names\" a general comment for all clo=\n" - "ck\n" + "> >> | BTW, was \"dropping clock-output-names\" a general comment for all clock\n" "> >> | drivers, or specific to the SoC's Clock Pulse Generator?\n" "> >> |\n" - "> >> | For e.g. \"fixed-factor-clock\", the driver falls back to using the no=\n" - "de's\n" + "> >> | For e.g. \"fixed-factor-clock\", the driver falls back to using the node's\n" "> >> | name if \"clock-output-names\" is not present.\n" "> >> |\n" - "> >> | But e.g. for MSTP clocks, that can't be done, as the clocks wouldn't=\n" - " have\n" - "> >> | names in that case (single node with multiple clocks). Unless we sta=\n" - "rt\n" + "> >> | But e.g. for MSTP clocks, that can't be done, as the clocks wouldn't have\n" + "> >> | names in that case (single node with multiple clocks). Unless we start\n" "> >> | fabricating them from the node name and the indices.\"\n" "> >>\n" "> >> For MSTP, we started fabricating them, but that doesn't solve the\n" "> >> of_clk_get_parent_name() issue.\n" "> >>\n" "> >> Thanks for your comments!\n" - "> =\n" - "\n" + "> \n" "> Thanks for making this very useful summary. It made it relatively easy\n" "> for me to understand what was going on.\n" - "> =\n" - "\n" + "> \n" "> From my side I've noticed exactly the same thing as you describe. So\n" "> in case we have more than one clock provided by a single node the\n" "> of_clk_get_parent_name() function will not work as expected unless\n" @@ -180,15 +152,13 @@ "> drivers for renesas hardware is built using a single node with\n" "> multiple indices, and today we use clock-output-names so we have no\n" "> issues for existing platforms.\n" - "> =\n" - "\n" + "> \n" "> For the upcoming R-Car Gen3 SoC we were asked to get rid of\n" "> clock-output-names if possible, but it seems that is difficult to do\n" "> without changing core CCF code. The patch below is my proporsal how to\n" "> fix the issue. It needs to be reworked to handle memory allocation\n" "> properly, but hopefully there are better ways to fix this.\n" - "> =\n" - "\n" + "> \n" "> The main portion of the patch below is based around a new function\n" "> called of_clk_get_name(). The idea is that it will check a certain\n" "> node if the clock-indices property is present together with\n" @@ -196,19 +166,16 @@ "> of that. If no clock-output-names is provided and no clock-indices\n" "> then the node name is used as-is, but if clock-indices is set then a\n" "> \"%s.%u\" name is generated based on node name and clock-index.\n" - "> =\n" - "\n" + "> \n" "> Included in the patch is also some debug code and minor changes to\n" "> make use of of_clk_get_name() from of_clk_get_parent_name() and a\n" "> couple of Renesas-specific drivers. With this patch the debugfs clock\n" "> frequency propagation is unbroken again, but unfortunately we now have\n" - "> memory leaks instead. =3D)\n" - "> =\n" - "\n" + "> memory leaks instead. =)\n" + "> \n" "> The prototype is only written to highlight what that the problem is.\n" "> Not for upstream merge.\n" - "> =\n" - "\n" + "> \n" "> Not-yet-Signed-off-by: Magnus Damm <damm+renesas@opensource.se>\n" "\n" "Magnus,\n" @@ -233,38 +200,32 @@ "Mike\n" "\n" "> ---\n" - "> =\n" - "\n" + "> \n" "> drivers/clk/clk-fixed-factor.c | 3 ++\n" - "> drivers/clk/clk.c | 46 ++++++++++++++++++++++------=\n" - "------\n" + "> drivers/clk/clk.c | 46 ++++++++++++++++++++++------------\n" "> drivers/clk/shmobile/clk-mstp.c | 3 --\n" "> drivers/clk/shmobile/clk-rcar-gen3.c | 13 +--------\n" "> include/linux/clk-provider.h | 2 +\n" "> 5 files changed, 39 insertions(+), 28 deletions(-)\n" - "> =\n" - "\n" + "> \n" "> --- 0001/drivers/clk/clk-fixed-factor.c\n" - "> +++ work/drivers/clk/clk-fixed-factor.c 2015-09-07 21:49:36.472366518 =\n" - "+0900\n" + "> +++ work/drivers/clk/clk-fixed-factor.c 2015-09-07 21:49:36.472366518 +0900\n" "> @@ -82,6 +82,9 @@ struct clk *clk_register_fixed_factor(st\n" "> if (!fix)\n" "> return ERR_PTR(-ENOMEM);\n" - "> =\n" - "\n" + "> \n" "> + printk(\"xxxx name %s parent name %s %i %i\\n\", name, parent_name,\n" "> + mult, div);\n" "> +\n" "> /* struct clk_fixed_factor assignments */\n" - "> fix->mult =3D mult;\n" - "> fix->div =3D div;\n" + "> fix->mult = mult;\n" + "> fix->div = div;\n" "> --- 0001/drivers/clk/clk.c\n" "> +++ work/drivers/clk/clk.c 2015-09-07 22:08:32.472366518 +0900\n" "> @@ -3045,31 +3045,25 @@ int of_clk_get_parent_count(struct devic\n" "> }\n" "> EXPORT_SYMBOL_GPL(of_clk_get_parent_count);\n" - "> =\n" - "\n" + "> \n" "> -const char *of_clk_get_parent_name(struct device_node *np, int index)\n" "> +const char *of_clk_get_name(struct device_node *np, int index)\n" "> {\n" @@ -275,42 +236,36 @@ "> u32 pv;\n" "> - int rc;\n" "> int count;\n" - "> + bool has_indices =3D false;\n" - "> =\n" - "\n" + "> + bool has_indices = false;\n" + "> \n" "> if (index < 0)\n" "> return NULL;\n" - "> =\n" - "\n" - "> - rc =3D of_parse_phandle_with_args(np, \"clocks\", \"#clock-cells\", inde=\n" - "x,\n" + "> \n" + "> - rc = of_parse_phandle_with_args(np, \"clocks\", \"#clock-cells\", index,\n" "> - &clkspec);\n" "> - if (rc)\n" "> - return NULL;\n" "> -\n" - "> - index =3D clkspec.args_count ? clkspec.args[0] : 0;\n" - "> count =3D 0;\n" - "> =\n" - "\n" + "> - index = clkspec.args_count ? clkspec.args[0] : 0;\n" + "> count = 0;\n" + "> \n" "> /* if there is an indices property, use it to transfer the index\n" - "> * specified into an array offset for the clock-output-names propert=\n" - "y.\n" + "> * specified into an array offset for the clock-output-names property.\n" "> */\n" "> - of_property_for_each_u32(clkspec.np, \"clock-indices\", prop, vp, pv) {\n" "> + of_property_for_each_u32(np, \"clock-indices\", prop, vp, pv) {\n" - "> + has_indices =3D true;\n" - "> if (index =3D=3D pv) {\n" - "> index =3D count;\n" + "> + has_indices = true;\n" + "> if (index = pv) {\n" + "> index = count;\n" "> break;\n" "> @@ -3077,12 +3071,34 @@ const char *of_clk_get_parent_name(struc\n" "> count++;\n" "> }\n" - "> =\n" - "\n" + "> \n" "> - if (of_property_read_string_index(clkspec.np, \"clock-output-names\",\n" "> - index,\n" "> - &clk_name) < 0)\n" - "> - clk_name =3D clkspec.np->name;\n" + "> - clk_name = clkspec.np->name;\n" "> + if (of_property_read_string_index(np, \"clock-output-names\", index,\n" "> + &clk_name) < 0) {\n" "> + if (has_indices)\n" @@ -326,19 +281,17 @@ "> +const char *of_clk_get_parent_name(struct device_node *np, int index)\n" "> +{\n" "> + struct of_phandle_args clkspec;\n" - "> + const char *clk_name =3D NULL;\n" + "> + const char *clk_name = NULL;\n" "> + int rc;\n" "> +\n" "> + if (index < 0)\n" "> + return NULL;\n" - "> =\n" - "\n" + "> \n" "> - of_node_put(clkspec.np);\n" - "> + rc =3D of_parse_phandle_with_args(np, \"clocks\", \"#clock-cells\", inde=\n" - "x,\n" + "> + rc = of_parse_phandle_with_args(np, \"clocks\", \"#clock-cells\", index,\n" "> + &clkspec);\n" "> + if (!rc) {\n" - "> + clk_name =3D of_clk_get_name(clkspec.np, clkspec.args_count ?\n" + "> + clk_name = of_clk_get_name(clkspec.np, clkspec.args_count ?\n" "> + clkspec.args[0] : 0);\n" "> + of_node_put(clkspec.np);\n" "> + }\n" @@ -346,19 +299,16 @@ "> }\n" "> EXPORT_SYMBOL_GPL(of_clk_get_parent_name);\n" "> --- 0017/drivers/clk/shmobile/clk-mstp.c\n" - "> +++ work/drivers/clk/shmobile/clk-mstp.c 2015-09-07 22:04:50.942366518=\n" - " +0900\n" + "> +++ work/drivers/clk/shmobile/clk-mstp.c 2015-09-07 22:04:50.942366518 +0900\n" "> @@ -216,8 +216,7 @@ static void __init cpg_mstp_clocks_init(\n" - "> =\n" - "\n" + "> \n" "> if (of_property_read_string_index(np, \"clock-output-names\",\n" "> i, &name) < 0)\n" - "> - allocated_name =3D name =3D kasprintf(GFP_KERNEL, \"%s.%u\",\n" + "> - allocated_name = name = kasprintf(GFP_KERNEL, \"%s.%u\",\n" "> - np->name, clkidx);\n" - "> + allocated_name =3D name =3D of_clk_get_name(np, clkidx);\n" - "> =\n" - "\n" - "> clks[clkidx] =3D cpg_mstp_clock_register(name, parent_name,\n" + "> + allocated_name = name = of_clk_get_name(np, clkidx);\n" + "> \n" + "> clks[clkidx] = cpg_mstp_clock_register(name, parent_name,\n" "> clkidx, group);\n" "> --- 0020/drivers/clk/shmobile/clk-rcar-gen3.c\n" "> +++ work/drivers/clk/shmobile/clk-rcar-gen3.c 2015-09-07\n" @@ -366,15 +316,14 @@ "> @@ -26,15 +26,6 @@\n" "> #define RCAR_GEN3_CLK_PLL4 5\n" "> #define RCAR_GEN3_CLK_NR 6\n" - "> =\n" - "\n" - "> -static const char * const rcar_gen3_clk_names[RCAR_GEN3_CLK_NR] =3D {\n" - "> - [RCAR_GEN3_CLK_MAIN] =3D \"main\",\n" - "> - [RCAR_GEN3_CLK_PLL0] =3D \"pll0\",\n" - "> - [RCAR_GEN3_CLK_PLL1] =3D \"pll1\",\n" - "> - [RCAR_GEN3_CLK_PLL2] =3D \"pll2\",\n" - "> - [RCAR_GEN3_CLK_PLL3] =3D \"pll3\",\n" - "> - [RCAR_GEN3_CLK_PLL4] =3D \"pll4\",\n" + "> \n" + "> -static const char * const rcar_gen3_clk_names[RCAR_GEN3_CLK_NR] = {\n" + "> - [RCAR_GEN3_CLK_MAIN] = \"main\",\n" + "> - [RCAR_GEN3_CLK_PLL0] = \"pll0\",\n" + "> - [RCAR_GEN3_CLK_PLL1] = \"pll1\",\n" + "> - [RCAR_GEN3_CLK_PLL2] = \"pll2\",\n" + "> - [RCAR_GEN3_CLK_PLL3] = \"pll3\",\n" + "> - [RCAR_GEN3_CLK_PLL4] = \"pll4\",\n" "> -};\n" "> -\n" "> struct rcar_gen3_cpg {\n" @@ -384,25 +333,22 @@ "> const struct cpg_pll_config *config,\n" "> unsigned int gen3_clk)\n" "> {\n" - "> - const char *parent_name =3D rcar_gen3_clk_names[RCAR_GEN3_CLK_MAIN];\n" - "> + const char *parent_name =3D of_clk_get_name(np, RCAR_GEN3_CLK_MAIN);\n" - "> unsigned int mult =3D 1;\n" - "> unsigned int div =3D 1;\n" + "> - const char *parent_name = rcar_gen3_clk_names[RCAR_GEN3_CLK_MAIN];\n" + "> + const char *parent_name = of_clk_get_name(np, RCAR_GEN3_CLK_MAIN);\n" + "> unsigned int mult = 1;\n" + "> unsigned int div = 1;\n" "> u32 value;\n" "> @@ -177,7 +168,7 @@ rcar_gen3_cpg_register_clk(struct device\n" "> return ERR_PTR(-EINVAL);\n" "> }\n" - "> =\n" - "\n" + "> \n" "> - return clk_register_fixed_factor(NULL, rcar_gen3_clk_names[gen3_clk],\n" "> + return clk_register_fixed_factor(NULL, of_clk_get_name(np, gen3_clk),\n" "> parent_name, 0, mult, div);\n" "> }\n" - "> =\n" - "\n" + "> \n" "> --- 0001/include/linux/clk-provider.h\n" - "> +++ work/include/linux/clk-provider.h 2015-09-07 21:35:54.332366518 +0=\n" - "900\n" + "> +++ work/include/linux/clk-provider.h 2015-09-07 21:35:54.332366518 +0900\n" "> @@ -665,6 +665,8 @@ struct clk *of_clk_src_onecell_get(struc\n" "> int of_clk_get_parent_count(struct device_node *np);\n" "> int of_clk_parent_fill(struct device_node *np, const char **parents,\n" @@ -410,8 +356,7 @@ "> +\n" "> +const char *of_clk_get_name(struct device_node *np, int index);\n" "> const char *of_clk_get_parent_name(struct device_node *np, int index);\n" - "> =\n" - "\n" + "> \n" > void of_clk_init(const struct of_device_id *matches); -525d8225a413ecf3232be03240a15f45d4e3967a3a882e1eb5e3a98ade0c0523 +ade0ee3732042a34f2a51dc66a7410b5857a7d1e6a1c713606e7a5b347d03b4b
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.