Re: [PATCH v3 08/32] clk: at91: clk-master: use clk_parent_data

[Ryan Wanner <ryan.wanner@microchip.com>]

On 9/6/25 11:36, claudiu beznea wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 7/10/25 23:07, Ryan.Wanner@microchip.com wrote:
>> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>
>> Use struct clk_parent_data instead of struct parent_hw as this leads
>> to less usage of __clk_get_hw() in SoC specific clock drivers and simpler
>> conversion of existing SoC specific clock drivers from parent_names to
>> modern clk_parent_data structures.
>>
>> The md_slck name and index are added for the SAM9X75 SoC so the
>> clk-master can properly use parent_data.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>> [ryan.wanner@microchip.com: Add clk-master changes to SAM9X75 and
>> SAMA7D65 SoCs. As well as add md_slck commit message.]
>> Signed-off-by: Ryan Wanner <Ryan.Wanner@microchip.com>
>> ---
>>   drivers/clk/at91/clk-master.c | 24 ++++++++++++------------
>>   drivers/clk/at91/pmc.h        |  6 +++---
>>   drivers/clk/at91/sam9x7.c     | 19 ++++++++++---------
>>   drivers/clk/at91/sama7d65.c   | 23 ++++++++++-------------
>>   drivers/clk/at91/sama7g5.c    | 29 +++++++++++++----------------
>>   5 files changed, 48 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/clk/at91/clk-master.c b/drivers/clk/at91/clk-master.c
>> index 7a544e429d34..cc4f3beb51e5 100644
>> --- a/drivers/clk/at91/clk-master.c
>> +++ b/drivers/clk/at91/clk-master.c
>> @@ -473,7 +473,7 @@ static struct clk_hw * __init
>>   at91_clk_register_master_internal(struct regmap *regmap,
>>               const char *name, int num_parents,
>>               const char **parent_names,
>> -             struct clk_hw **parent_hws,
>> +             struct clk_parent_data *parent_data,
>>               const struct clk_master_layout *layout,
>>               const struct clk_master_characteristics *characteristics,
>>               const struct clk_ops *ops, spinlock_t *lock, u32 flags)
>> @@ -485,7 +485,7 @@ at91_clk_register_master_internal(struct regmap *regmap,
>>       unsigned long irqflags;
>>       int ret;
>>
>> -     if (!name || !num_parents || !(parent_names || parent_hws) || !lock)
>> +     if (!name || !num_parents || !(parent_names || parent_data) || !lock)
>>               return ERR_PTR(-EINVAL);
>>
>>       master = kzalloc(sizeof(*master), GFP_KERNEL);
>> @@ -494,8 +494,8 @@ at91_clk_register_master_internal(struct regmap *regmap,
>>
>>       init.name = name;
>>       init.ops = ops;
>> -     if (parent_hws)
>> -             init.parent_hws = (const struct clk_hw **)parent_hws;
>> +     if (parent_data)
>> +             init.parent_data = (const struct clk_parent_data *)parent_data;
>>       else
>>               init.parent_names = parent_names;
>>       init.num_parents = num_parents;
>> @@ -531,13 +531,13 @@ struct clk_hw * __init
>>   at91_clk_register_master_pres(struct regmap *regmap,
>>               const char *name, int num_parents,
>>               const char **parent_names,
>> -             struct clk_hw **parent_hws,
>> +             struct clk_parent_data *parent_data,
>>               const struct clk_master_layout *layout,
>>               const struct clk_master_characteristics *characteristics,
>>               spinlock_t *lock)
>>   {
>>       return at91_clk_register_master_internal(regmap, name, num_parents,
>> -                                              parent_names, parent_hws, layout,
>> +                                              parent_names, parent_data, layout,
>>                                                characteristics,
>>                                                &master_pres_ops,
>>                                                lock, CLK_SET_RATE_GATE);
>> @@ -546,7 +546,7 @@ at91_clk_register_master_pres(struct regmap *regmap,
>>   struct clk_hw * __init
>>   at91_clk_register_master_div(struct regmap *regmap,
>>               const char *name, const char *parent_name,
>> -             struct clk_hw *parent_hw, const struct clk_master_layout *layout,
>> +             struct clk_parent_data *parent_data, const struct clk_master_layout *layout,
>>               const struct clk_master_characteristics *characteristics,
>>               spinlock_t *lock, u32 flags, u32 safe_div)
>>   {
>> @@ -560,7 +560,7 @@ at91_clk_register_master_div(struct regmap *regmap,
>>
>>       hw = at91_clk_register_master_internal(regmap, name, 1,
>>                                              parent_name ? &parent_name : NULL,
>> -                                            parent_hw ? &parent_hw : NULL, layout,
>> +                                            parent_data, layout,
>>                                              characteristics, ops,
>>                                              lock, flags);
>>
>> @@ -812,7 +812,7 @@ struct clk_hw * __init
>>   at91_clk_sama7g5_register_master(struct regmap *regmap,
>>                                const char *name, int num_parents,
>>                                const char **parent_names,
>> -                              struct clk_hw **parent_hws,
>> +                              struct clk_parent_data *parent_data,
>>                                u32 *mux_table,
>>                                spinlock_t *lock, u8 id,
>>                                bool critical, int chg_pid)
>> @@ -824,7 +824,7 @@ at91_clk_sama7g5_register_master(struct regmap *regmap,
>>       unsigned int val;
>>       int ret;
>>
>> -     if (!name || !num_parents || !(parent_names || parent_hws) || !mux_table ||
>> +     if (!name || !num_parents || !(parent_names || parent_data) || !mux_table ||
>>           !lock || id > MASTER_MAX_ID)
>>               return ERR_PTR(-EINVAL);
>>
>> @@ -834,8 +834,8 @@ at91_clk_sama7g5_register_master(struct regmap *regmap,
>>
>>       init.name = name;
>>       init.ops = &sama7g5_master_ops;
>> -     if (parent_hws)
>> -             init.parent_hws = (const struct clk_hw **)parent_hws;
>> +     if (parent_data)
>> +             init.parent_data = (const struct clk_parent_data *)parent_data;
>>       else
>>               init.parent_names = parent_names;
>>       init.num_parents = num_parents;
>> diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
>> index d9a04fddb0b1..54d472276fc9 100644
>> --- a/drivers/clk/at91/pmc.h
>> +++ b/drivers/clk/at91/pmc.h
>> @@ -204,14 +204,14 @@ at91_clk_register_sam9x5_main(struct regmap *regmap, const char *name,
>>   struct clk_hw * __init
>>   at91_clk_register_master_pres(struct regmap *regmap, const char *name,
>>                             int num_parents, const char **parent_names,
>> -                           struct clk_hw **parent_hws,
>> +                           struct clk_parent_data *parent_data,
>>                             const struct clk_master_layout *layout,
>>                             const struct clk_master_characteristics *characteristics,
>>                             spinlock_t *lock);
>>
>>   struct clk_hw * __init
>>   at91_clk_register_master_div(struct regmap *regmap, const char *name,
>> -                          const char *parent_names, struct clk_hw *parent_hw,
>> +                          const char *parent_names, struct clk_parent_data *parent_data,
>>                            const struct clk_master_layout *layout,
>>                            const struct clk_master_characteristics *characteristics,
>>                            spinlock_t *lock, u32 flags, u32 safe_div);
>> @@ -220,7 +220,7 @@ struct clk_hw * __init
>>   at91_clk_sama7g5_register_master(struct regmap *regmap,
>>                                const char *name, int num_parents,
>>                                const char **parent_names,
>> -                              struct clk_hw **parent_hws, u32 *mux_table,
>> +                              struct clk_parent_data *parent_data, u32 *mux_table,
>>                                spinlock_t *lock, u8 id, bool critical,
>>                                int chg_pid);
>>
>> diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
>> index eaae05ba21ad..945983f72140 100644
>> --- a/drivers/clk/at91/sam9x7.c
>> +++ b/drivers/clk/at91/sam9x7.c
>> @@ -739,7 +739,8 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
>>   {
>>       struct clk_range range = CLK_RANGE(0, 0);
>>       const char *main_xtal_name = "main_xtal";
>> -     u8 main_xtal_index = 2;
>> +     const char *const md_slck_name = "md_slck";
>> +     u8 md_slck_index = 1, main_xtal_index = 2;
>>       struct pmc_data *sam9x7_pmc;
>>       const char *parent_names[9];
>>       void **clk_mux_buffer = NULL;
>> @@ -747,12 +748,12 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
>>       struct regmap *regmap;
>>       struct clk_hw *hw, *main_rc_hw, *main_osc_hw, *main_xtal_hw;
>>       struct clk_hw *td_slck_hw, *md_slck_hw, *usbck_hw;
>> -     struct clk_parent_data parent_data[2];
>> +     struct clk_parent_data parent_data[9];
>>       struct clk_hw *parent_hws[9];
>>       int i, j;
>>
>>       td_slck_hw = __clk_get_hw(of_clk_get_by_name(np, "td_slck"));
>> -     md_slck_hw = __clk_get_hw(of_clk_get_by_name(np, "md_slck"));
>> +     md_slck_hw = __clk_get_hw(of_clk_get_by_name(np, md_slck_name));
> 
> Please use:
> 
> i = of_property_match_string(np, "clock-names", "md_slck");
> if (i < 0)
>     return;
> 
> md_slck_name = of_clk_get_parent_name(np, i);
> 
> Same sama7d65, sama7g5.

For these SoCs the clk_hw struct is still needed since it is used later
in the driver and not changed until a subsquent patch later in this
series. Would it be better to hold this change untill then?

> 
>>       main_xtal_hw = __clk_get_hw(of_clk_get_by_name(np, main_xtal_name));
>>
>>       if (!td_slck_hw || !md_slck_hw || !main_xtal_hw)
>> @@ -853,18 +854,18 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
>>               }
>>       }
>>
>> -     parent_hws[0] = md_slck_hw;
>> -     parent_hws[1] = sam9x7_pmc->chws[PMC_MAIN];
>> -     parent_hws[2] = sam9x7_plls[PLL_ID_PLLA][PLL_COMPID_DIV0].hw;
>> -     parent_hws[3] = sam9x7_plls[PLL_ID_UPLL][PLL_COMPID_DIV0].hw;
>> +     parent_data[0] = AT91_CLK_PD_NAME(md_slck_name, md_slck_index);
> 
> AT91_CLK_PD_NAME(md_slck_name);
> 
> Same sama7d65, sama7g5.
>