Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <79a79ec5-3691-43d2-ca3e-478a94f85e2d@tuxon.dev>
Date:   Wed, 2 Aug 2023 07:24:54 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
From:   claudiu beznea <claudiu.beznea@tuxon.dev>
Subject: Re: [PATCH 03/42] clk: at91: sam9x60: switch to parent_hw and
 parent_data
To:     Stephen Boyd <sboyd@kernel.org>, alexandre.belloni@bootlin.com,
        mripard@kernel.org, mturquette@baylibre.com,
        nicolas.ferre@microchip.com
Cc:     linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, varshini.rajendran@microchip.com
References: <20230727053156.13587-1-claudiu.beznea@tuxon.dev>
 <20230727053156.13587-4-claudiu.beznea@tuxon.dev>
 <412f9209637e6eb861cd07e1d6dbc346.sboyd@kernel.org>
Content-Language: en-US
In-Reply-To: <412f9209637e6eb861cd07e1d6dbc346.sboyd@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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 {
>>  
>>  static void __init sam9x60_pmc_setup(struct device_node *np)
>>  {
>> +       struct clk_hw *td_slck_hw, *md_slck_hw, *main_xtal_hw, *main_rc_hw, *main_osc_hw;
>> +       struct clk_hw *parent_hws[6], *hw, *usbck_hw;
>> +       static struct clk_parent_data parent_data;
>>         struct clk_range range = CLK_RANGE(0, 0);
>> -       const char *td_slck_name, *md_slck_name, *mainxtal_name;
>> +       const char *main_xtal_name = "main_xtal";
>>         struct pmc_data *sam9x60_pmc;
>> -       const char *parent_names[6];
>> -       struct clk_hw *main_osc_hw;
>>         struct regmap *regmap;
>> -       struct clk_hw *hw;
>> +       struct clk *clk;
>>         int i;
>>  
>> -       i = of_property_match_string(np, "clock-names", "td_slck");
>> -       if (i < 0)
>> +       clk = of_clk_get_by_name(np, "td_slck");
>> +       if (IS_ERR(clk))
>>                 return;
>> -
>> -       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...

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).

Thank you for your review,
Claudiu Beznea