[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d35401da-7db8-4bfe-90cd-042da909c940@tuxon.dev>
Date: Sat, 6 Sep 2025 21:36:04 +0300
From: claudiu beznea <claudiu.beznea@...on.dev>
To: Ryan.Wanner@...rochip.com, mturquette@...libre.com, sboyd@...nel.org,
nicolas.ferre@...rochip.com, alexandre.belloni@...tlin.com
Cc: varshini.rajendran@...rochip.com, linux-clk@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
robh@...nel.org
Subject: Re: [PATCH v3 07/32] clk: at91: clk-utmi: use clk_parent_data
Hi, Ryan,
On 7/10/25 23:07, Ryan.Wanner@...rochip.com wrote:
> From: Claudiu Beznea <claudiu.beznea@...on.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.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@...on.dev>
> Signed-off-by: Ryan Wanner <Ryan.Wanner@...rochip.com>
> ---
> drivers/clk/at91/clk-utmi.c | 16 ++++++++--------
> drivers/clk/at91/pmc.h | 4 ++--
> drivers/clk/at91/sama7g5.c | 25 +++++++++++++++----------
> 3 files changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/clk/at91/clk-utmi.c b/drivers/clk/at91/clk-utmi.c
> index b991180beea1..38ffe4d712a5 100644
> --- a/drivers/clk/at91/clk-utmi.c
> +++ b/drivers/clk/at91/clk-utmi.c
> @@ -144,7 +144,7 @@ static struct clk_hw * __init
> at91_clk_register_utmi_internal(struct regmap *regmap_pmc,
> struct regmap *regmap_sfr,
> const char *name, const char *parent_name,
> - struct clk_hw *parent_hw,
> + struct clk_parent_data *parent_data,
> const struct clk_ops *ops, unsigned long flags)
> {
> struct clk_utmi *utmi;
> @@ -152,7 +152,7 @@ at91_clk_register_utmi_internal(struct regmap *regmap_pmc,
> struct clk_init_data init = {};
> int ret;
>
> - if (!(parent_name || parent_hw))
> + if (!(parent_name || parent_data))
> return ERR_PTR(-EINVAL);
>
> utmi = kzalloc(sizeof(*utmi), GFP_KERNEL);
> @@ -161,8 +161,8 @@ at91_clk_register_utmi_internal(struct regmap *regmap_pmc,
>
> init.name = name;
> init.ops = ops;
> - if (parent_hw)
> - init.parent_hws = (const struct clk_hw **)&parent_hw;
> + if (parent_data)
> + init.parent_data = (const struct clk_parent_data *)parent_data;
> else
> init.parent_names = &parent_name;
> init.num_parents = 1;
> @@ -185,10 +185,10 @@ at91_clk_register_utmi_internal(struct regmap *regmap_pmc,
> struct clk_hw * __init
> at91_clk_register_utmi(struct regmap *regmap_pmc, struct regmap *regmap_sfr,
> const char *name, const char *parent_name,
> - struct clk_hw *parent_hw)
> + struct clk_parent_data *parent_data)
> {
> return at91_clk_register_utmi_internal(regmap_pmc, regmap_sfr, name,
> - parent_name, parent_hw, &utmi_ops, CLK_SET_RATE_GATE);
> + parent_name, parent_data, &utmi_ops, CLK_SET_RATE_GATE);
> }
>
> static int clk_utmi_sama7g5_prepare(struct clk_hw *hw)
> @@ -287,8 +287,8 @@ static const struct clk_ops sama7g5_utmi_ops = {
>
> struct clk_hw * __init
> at91_clk_sama7g5_register_utmi(struct regmap *regmap_pmc, const char *name,
> - const char *parent_name, struct clk_hw *parent_hw)
> + const char *parent_name, struct clk_parent_data *parent_data)
> {
> return at91_clk_register_utmi_internal(regmap_pmc, NULL, name,
> - parent_name, parent_hw, &sama7g5_utmi_ops, 0);
> + parent_name, parent_data, &sama7g5_utmi_ops, 0);
> }
> diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
> index e32a5e85d08f..d9a04fddb0b1 100644
> --- a/drivers/clk/at91/pmc.h
> +++ b/drivers/clk/at91/pmc.h
> @@ -299,10 +299,10 @@ at91rm9200_clk_register_usb(struct regmap *regmap, const char *name,
> struct clk_hw * __init
> at91_clk_register_utmi(struct regmap *regmap_pmc, struct regmap *regmap_sfr,
> const char *name, const char *parent_name,
> - struct clk_hw *parent_hw);
> + struct clk_parent_data *parent_data);
>
> struct clk_hw * __init
> at91_clk_sama7g5_register_utmi(struct regmap *regmap, const char *name,
> - const char *parent_name, struct clk_hw *parent_hw);
> + const char *parent_name, struct clk_parent_data *parent_data);
>
> #endif /* __PMC_H_ */
> diff --git a/drivers/clk/at91/sama7g5.c b/drivers/clk/at91/sama7g5.c
> index f816a5551277..c4723b875a1d 100644
> --- a/drivers/clk/at91/sama7g5.c
> +++ b/drivers/clk/at91/sama7g5.c
> @@ -975,30 +975,33 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
> void **alloc_mem = NULL;
> int alloc_mem_size = 0;
> struct regmap *regmap;
> - struct clk_hw *hw, *main_rc_hw, *main_osc_hw, *main_xtal_hw;
> + struct clk_hw *hw, *main_rc_hw, *main_osc_hw;
> struct clk_hw *td_slck_hw, *md_slck_hw;
> struct clk_parent_data parent_data[2];
> struct clk_hw *parent_hws[10];
> + struct clk *main_xtal;
> bool bypass;
> 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"));
> - main_xtal_hw = __clk_get_hw(of_clk_get_by_name(np, main_xtal_name));
> + if (!td_slck_hw || !md_slck_hw)
> + return;
>
> - if (!td_slck_hw || !md_slck_hw || !main_xtal_hw)
> + main_xtal = of_clk_get(np, main_xtal_index);
Maybe:
i = of_property_match_string(np, "clock-names", "main_xtal");
main_xtal_name = of_clk_get_parent_name(np, i);
And move the:
main_xtal = of_clk_get()
above in the switch/case where the main_xtal clock is needed to get its rate.
> + if (IS_ERR(main_xtal))
> return;
>
> regmap = device_node_to_regmap(np);
> if (IS_ERR(regmap))
> - return;
> + goto put_main_xtal;
>
> sama7g5_pmc = pmc_data_allocate(PMC_MCK1 + 1,
> nck(sama7g5_systemck),
> nck(sama7g5_periphck),
> nck(sama7g5_gck), 8);
> if (!sama7g5_pmc)
> - return;
> + goto put_main_xtal;
>
> alloc_mem = kmalloc(sizeof(void *) *
> (ARRAY_SIZE(sama7g5_mckx) + ARRAY_SIZE(sama7g5_gck)),
> @@ -1039,19 +1042,18 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
> switch (sama7g5_plls[i][j].p) {
> case SAMA7G5_PLL_PARENT_MAINCK:
> parent_data[0] = AT91_CLK_PD_NAME("mainck", -1);
> - hw = sama7g5_pmc->chws[PMC_MAIN];
> + parent_rate = clk_hw_get_rate(sama7g5_pmc->chws[PMC_MAIN]);
> break;
> case SAMA7G5_PLL_PARENT_MAIN_XTAL:
> parent_data[0] = AT91_CLK_PD_NAME(main_xtal_name,
> main_xtal_index);
> - hw = main_xtal_hw;
here:
main_xtal = of_clk_get_by_index(np, i);
> + parent_rate = clk_get_rate(main_xtal);
clk_put(main_xtal);
> break;
> default:
> /* Should not happen. */
> break;
> }
>
> - parent_rate = clk_hw_get_rate(hw);
> if (!parent_rate)
> return;
>
> @@ -1135,7 +1137,8 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
> sama7g5_pmc->chws[sama7g5_mckx[i].eid] = hw;
> }
>
> - hw = at91_clk_sama7g5_register_utmi(regmap, "utmick", NULL, main_xtal_hw);
> + hw = at91_clk_sama7g5_register_utmi(regmap, "utmick", NULL,
> + &AT91_CLK_PD_NAME(main_xtal_name, main_xtal_index));
> if (IS_ERR(hw))
> goto err_free;
>
> @@ -1233,7 +1236,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
>
> of_clk_add_hw_provider(np, of_clk_hw_pmc_get, sama7g5_pmc);
>
> - return;
> + goto put_main_xtal;
>
> err_free:
> if (alloc_mem) {
> @@ -1243,6 +1246,8 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
> }
>
> kfree(sama7g5_pmc);
> +put_main_xtal:
> + clk_put(main_xtal);
> }
>
> /* Some clks are used for a clocksource */
Powered by blists - more mailing lists