lkml.org 
[lkml]   [2018]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] clk: clk-fixed-factor: Use new macro CLK_OF_DECLARE_DRIVER
Date
Hi Stephen,

Thanks for the review.

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@kernel.org]
> Sent: Friday, March 09, 2018 10:25 AM
> To: Rajan Vaja <RAJANV@xilinx.com>; mturquette@baylibre.com
> Cc: linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; Jolly Shah
> <JOLLYS@xilinx.com>; Michal Simek <michals@xilinx.com>; Rajan Vaja
> <RAJANV@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 <rajanv@xilinx.com>
> > Suggested-by: Michal Simek <michal.simek@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 .
\
 
 \ /
  Last update: 2018-03-09 20:28    [W:0.100 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site