Hi Stephen, Thanks for the review.
> -----Original Message----- > From: Stephen Boyd [mailto:sb...@kernel.org] > Sent: Friday, March 09, 2018 10:25 AM > To: Rajan Vaja <raj...@xilinx.com>; mturque...@baylibre.com > Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Jolly Shah > <jol...@xilinx.com>; Michal Simek <mich...@xilinx.com>; Rajan Vaja > <raj...@xilinx.com> > Subject: Re: [PATCH] clk: clk-fixed-factor: Use new macro > CLK_OF_DECLARE_DRIVER > > Quoting Rajan Vaja (2018-03-08 06:15:00) > > Fixed factor clock has two initialization at of_clk_init() time and > > also during platform driver probe. So declare the fixed factor clock > > with CLK_OF_DECLARE_DRIVER instead of CLK_OF_DECLARE. > > > > See below commit for reference: > > "clk: sunxi: apb0: Use new macro CLK_OF_DECLARE_DRIVER" > > (sha1: 915128b621a05c63fa58ca9e4cbdf394bbe592f3) > > > > Signed-off-by: Rajan Vaja <raj...@xilinx.com> > > Suggested-by: Michal Simek <michal.si...@xilinx.com> > > --- > > drivers/clk/clk-fixed-factor.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/clk/clk-fixed-factor.c > > b/drivers/clk/clk-fixed-factor.c index a5d402d..d72ef2d 100644 > > --- a/drivers/clk/clk-fixed-factor.c > > +++ b/drivers/clk/clk-fixed-factor.c > > @@ -196,8 +196,9 @@ void __init of_fixed_factor_clk_setup(struct > > device_node *node) { > > _of_fixed_factor_clk_setup(node); } > > -CLK_OF_DECLARE(fixed_factor_clk, "fixed-factor-clock", > > - of_fixed_factor_clk_setup); > > + > > +CLK_OF_DECLARE_DRIVER(fixed_factor_clk, "fixed-factor-clock", > > + of_fixed_factor_clk_setup); > > > > Is the intent to register the clk twice? I believe things are working as > intended without this patch, so maybe you can explain a little more what > you're trying to fix. [Rajan] Yes. During of_clk_init() if some DT fixed factor clock has parent which is neither mentioned in output-clock-names of clock controller nor registered as clock provider, of_clk_init() will try to forcefully register in second loop. if (force || parent_ready(clk_provider->np)) { /* Don't populate platform devices */ of_node_set_flag(clk_provider->np, OF_POPULATED); So registration of this DT fixed-factor clock would fail as parent would be NULL as below (called from _of_fixed_factor_clk_setup()): parent_name = of_clk_get_parent_name(node, 0); On the other hand, even if registration failed, that node will be marked as OF_POPULATED, so probe of clk-fixed-factor.c will also not be called and that DT fixed-factor clock would never be registered. Same thing is discussed at https://lkml.org/lkml/2017/6/5/681 .
