linux-kernel - Re: [PATCH 03/42] clk: at91: sam9x60: switch to parent_hw and parent

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] [day] [month] [year] [list]

Message-ID: <de9c4d5ea85da5614563bee7d3368459.sboyd@kernel.org>
Date:   Fri, 08 Sep 2023 17:06:18 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     alexandre.belloni@...tlin.com,
        claudiu beznea <claudiu.beznea@...on.dev>, mripard@...nel.org,
        mturquette@...libre.com, nicolas.ferre@...rochip.com
Cc:     linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, varshini.rajendran@...rochip.com
Subject: Re: [PATCH 03/42] clk: at91: sam9x60: switch to parent_hw and parent_data

Sorry, I missed this series in my review queue.

Quoting claudiu beznea (2023-08-01 21:24:54)
> On 29.07.2023 06:28, Stephen Boyd wrote:
> > Quoting Claudiu Beznea (2023-07-26 22:31:17)
> >> @@ -177,31 +178,34 @@ static const struct {
> >> -       td_slck_name = of_clk_get_parent_name(np, i);
> >> -
> >> -       i = of_property_match_string(np, "clock-names", "md_slck");
> >> -       if (i < 0)
> >> +       td_slck_hw = __clk_get_hw(clk);
> > 
> > Don't introduce more usage of __clk_get_hw(). The index for "td_slck"
> > should be known, and it can be used as the index member in struct
> > clk_parent_data.  This allows the clk framework to lazily find the
> > parent, instead of requiring the parent to be registered before this
> > code runs. It also reduces the usage of __clk_get_hw().
> 
> If I'll do this I will have to also change the approach that has been done
> for SAMA7G5 (already integrated in v6.5-rc1, maybe I had to let it more on
> the mailing list before taking into the tree) for all the clock drivers
> (basically instead of parent_hws I will have to provide parent_data to AT91
> clock registration APIs AFAICT). No issue with that... just telling...

Ok. Please do that.

> 
> The reason I did it with parent_hws + __clk_get_hw() on PMC parents in
> SAMA7G5 is that SAMA7G5 PLL parent rate need to be known from the
> registration to setup properly the PLL. Otherwise PLL will not lock if not
> properly setup. For this I got the parent_hw for PLL parent to retrieve its
> rate and thus I chose at that time to also get the clk_hw for the other
> parents of PMC just to have the same approach on all parents (and it looked
> to me that code will be simpler).

You can use the regular clk_get() APIs for that if you're trying to
enforce a "this clk must be registered first" sort of thing. If the
clk_get() call fails, then the driver can probe defer, etc. But don't
turn around and take that clk and call __clk_get_hw() on it to express
the parent relationship. Instead, let the parent matching take place
through the normal means. The goal is to get rid of __clk_get_hw() at
some point.