[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4cd2b42d-f49d-4f21-8677-928cb3d75fd7@tuxon.dev>
Date: Mon, 26 Jan 2026 10:53:13 +0200
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,
bmasney@...hat.com, alexander.sverdlin@...il.com,
varshini.rajendran@...rochip.com
Cc: cristian.birsan@...rochip.com, balamanikandan.gunasundar@...rochip.com,
linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 04/31] clk: at91: clk-sam9x60-pll: use clk_parent_data
On 1/16/26 22:06, 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. As clk-sam9x60-pll need to know
> parent's rate at initialization we pass it now from SoC specific drivers.
> This will lead in the end at removing __clk_get_hw() in SoC specific
> drivers (that will be solved by subsequent commits).
>
> Get the main_xtal name via of_clk_get_parent_name() to consistently get
> the correct name for the main_xtal.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@...on.dev>
> [ryan.wanner@...rochip.com: Add SAMA7D65 and SAM9X75 SoCs to the change set.]
> Signed-off-by: Ryan Wanner <Ryan.Wanner@...rochip.com>
> ---
[ ... ]
> @@ -1137,10 +1141,8 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
>
> bypass = of_property_read_bool(np, "atmel,osc-bypass");
>
> - parent_data.name = main_xtal_name;
> - parent_data.fw_name = main_xtal_name;
> main_osc_hw = at91_clk_register_main_osc(regmap, "main_osc", NULL,
> - &parent_data, bypass);
> + &AT91_CLK_PD_NAME(main_xtal_name), bypass);
> if (IS_ERR(main_osc_hw))
> goto err_free;
>
> @@ -1154,7 +1156,7 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
>
> for (i = 0; i < PLL_ID_MAX; i++) {
> for (j = 0; j < PLL_COMPID_MAX; j++) {
> - struct clk_hw *parent_hw;
> + unsigned long parent_rate;
>
> if (!sama7d65_plls[i][j].n)
> continue;
> @@ -1163,20 +1165,24 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
> case PLL_TYPE_FRAC:
> switch (sama7d65_plls[i][j].p) {
> case SAMA7D65_PLL_PARENT_MAINCK:
> - parent_hw = sama7d65_pmc->chws[PMC_MAIN];
> + parent_data = AT91_CLK_PD_NAME("mainck");
> + hw = sama7d65_pmc->chws[PMC_MAIN];
> break;
> case SAMA7D65_PLL_PARENT_MAIN_XTAL:
> - parent_hw = main_xtal_hw;
> + parent_data = AT91_CLK_PD_NAME(main_xtal_name);
> + hw = main_xtal_hw;
main_xtal_hw is not initialized. You should be able to do something similar to
what was done in patch 7/31 for sama7g5.
> break;
> default:
> /* Should not happen. */
> - parent_hw = NULL;
> break;
> }
> + parent_rate = clk_hw_get_rate(hw);
> + if (!parent_rate)
> + return;
>
> hw = sam9x60_clk_register_frac_pll(regmap,
> &pmc_pll_lock, sama7d65_plls[i][j].n,
> - NULL, parent_hw, i,
> + &parent_data, parent_rate, i,
> sama7d65_plls[i][j].c,
> sama7d65_plls[i][j].l,
> sama7d65_plls[i][j].f);
> diff --git a/drivers/clk/at91/sama7g5.c b/drivers/clk/at91/sama7g5.c
> index 713f5dfe7be2..c6550044cba1 100644
> --- a/drivers/clk/at91/sama7g5.c
> +++ b/drivers/clk/at91/sama7g5.c
> @@ -971,7 +971,7 @@ static const struct clk_pcr_layout sama7g5_pcr_layout = {
>
> static void __init sama7g5_pmc_setup(struct device_node *np)
> {
> - const char *main_xtal_name = "main_xtal";
> + const char *main_xtal_name;
> struct pmc_data *sama7g5_pmc;
> void **alloc_mem = NULL;
> int alloc_mem_size = 0;
> @@ -983,11 +983,15 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
> bool bypass;
> int i, j;
>
> + i = of_property_match_string(np, "clock-names", "main_xtal");
> + if (i < 0)
> + return;
> + main_xtal_name = of_clk_get_parent_name(np, i);
> +
> 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 || !main_xtal_hw)
> + if (!td_slck_hw || !md_slck_hw)
> return;
>
> regmap = device_node_to_regmap(np);
> @@ -1014,10 +1018,8 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
>
> bypass = of_property_read_bool(np, "atmel,osc-bypass");
>
> - parent_data.name = main_xtal_name;
> - parent_data.fw_name = main_xtal_name;
> main_osc_hw = at91_clk_register_main_osc(regmap, "main_osc", NULL,
> - &parent_data, bypass);
> + &AT91_CLK_PD_NAME(main_xtal_name), bypass);
> if (IS_ERR(main_osc_hw))
> goto err_free;
>
> @@ -1031,7 +1033,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
>
> for (i = 0; i < PLL_ID_MAX; i++) {
> for (j = 0; j < PLL_COMPID_MAX; j++) {
> - struct clk_hw *parent_hw;
> + unsigned long parent_rate;
>
> if (!sama7g5_plls[i][j].n)
> continue;
> @@ -1040,20 +1042,25 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
> case PLL_TYPE_FRAC:
> switch (sama7g5_plls[i][j].p) {
> case SAMA7G5_PLL_PARENT_MAINCK:
> - parent_hw = sama7g5_pmc->chws[PMC_MAIN];
> + parent_data = AT91_CLK_PD_NAME("mainck");
> + hw = sama7g5_pmc->chws[PMC_MAIN];
> break;
> case SAMA7G5_PLL_PARENT_MAIN_XTAL:
> - parent_hw = main_xtal_hw;
> + parent_data = AT91_CLK_PD_NAME(main_xtal_name);
> + hw = main_xtal_hw;
main_xtal_hw is not initialized. You should be able to do something similar to
what was done in patch 7/31 for sama7g5.
The rest LGTM.
Thank you,
Claudiu
Powered by blists - more mailing lists