linux-kernel - RE: [PATCH] clk: clk-fixed-factor: Use new macro CLK_OF_DECLARE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SN4PR0201MB3502AC253C8B1C9E57A83B7FB7DE0@SN4PR0201MB3502.namprd02.prod.outlook.com>
Date:   Fri, 9 Mar 2018 19:27:40 +0000
From:   Rajan Vaja <RAJANV@...inx.com>
To:     'Stephen Boyd' <sboyd@...nel.org>
CC:     "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jolly Shah <JOLLYS@...inx.com>,
        Michal Simek <michals@...inx.com>,
        "mturquette@...libre.com" <mturquette@...libre.com>
Subject: RE: [PATCH] clk: clk-fixed-factor: Use new macro
 CLK_OF_DECLARE_DRIVER

Hi Stephen,

Thanks for the review.

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@...nel.org]
> Sent: Friday, March 09, 2018 10:25 AM
> To: Rajan Vaja <RAJANV@...inx.com>; mturquette@...libre.com
> Cc: linux-clk@...r.kernel.org; linux-kernel@...r.kernel.org; Jolly Shah
> <JOLLYS@...inx.com>; Michal Simek <michals@...inx.com>; Rajan Vaja
> <RAJANV@...inx.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@...inx.com>
> > Suggested-by: Michal Simek <michal.simek@...inx.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 .