Re: [PATCH v5 03/31] clk: at91: sam9x75: switch to parent_hw and parent_data
From: Claudiu Beznea
Date: Mon Jan 26 2026 - 03:52:58 EST
Hi, Ryan,
On 1/16/26 22:06, ryan.wanner@xxxxxxxxxxxxx wrote:
From: Ryan Wanner <Ryan.Wanner@xxxxxxxxxxxxx>
Switch SAM9X75 clocks to use parent_hw and parent_data. Having
parent_hw instead of parent names improves to clock registration
speed and re-parenting.
The USBCLK will be updated in subsequent patches that update the clock
registration functions to use parent_hw and parent_data.
__clk_get_hw() will be removed in subsequent patches in this series.
Signed-off-by: Ryan Wanner <Ryan.Wanner@xxxxxxxxxxxxx>
---
drivers/clk/at91/sam9x7.c | 305 ++++++++++++++++++++++----------------
1 file changed, 174 insertions(+), 131 deletions(-)
diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
index 89868a0aeaba..c48c91da914e 100644
--- a/drivers/clk/at91/sam9x7.c
+++ b/drivers/clk/at91/sam9x7.c
[ ...]
/*
@@ -426,7 +450,8 @@ static const struct {
/*
* Generic clock description
* @n: clock name
- * @pp: PLL parents
+ * @pp: PLL parents (entry formed by PLL components identifiers
+ * (see enum pll_component_id))
* @pp_mux_table: PLL parents mux table
* @r: clock output range
* @pp_chg_id: id in parent array of changeable PLL parent
@@ -435,7 +460,10 @@ static const struct {
*/
static const struct {
const char *n;
- const char *pp[8];
+ struct {
+ int pll_id;
+ int pll_compid;
Still having more than 2 tabs here.
[ ... ]
static void __init sam9x7_pmc_setup(struct device_node *np)
{
struct clk_range range = CLK_RANGE(0, 0);
- const char *td_slck_name, *md_slck_name, *mainxtal_name;
+ const char *main_xtal_name;
struct pmc_data *sam9x7_pmc;
const char *parent_names[9];
void **clk_mux_buffer = NULL;
int clk_mux_buffer_size = 0;
- struct clk_hw *main_osc_hw;
struct regmap *regmap;
- struct clk_hw *hw;
+ 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_hw *parent_hws[9];
int i, j;
- i = of_property_match_string(np, "clock-names", "td_slck");
- if (i < 0)
- return;
-
- td_slck_name = of_clk_get_parent_name(np, i);
+ i = of_property_match_string(np, "clock-names", "main_xtal");
- i = of_property_match_string(np, "clock-names", "md_slck");
if (i < 0)
return;
+ main_xtal_name = of_clk_get_parent_name(np, i);
- md_slck_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"));
- i = of_property_match_string(np, "clock-names", "main_xtal");
- if (i < 0)
+ if (!td_slck_hw || !md_slck_hw)
return;
- mainxtal_name = of_clk_get_parent_name(np, i);
I would have been prefered that the code here to look more organized, something like:
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"));
if (!td_slck_hw || !md_slck_hw)
return;
but it is OK for now as the final form looks alike.
regmap = device_node_to_regmap(np);
if (IS_ERR(regmap))
@@ -760,26 +784,26 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
if (!clk_mux_buffer)
goto err_free;
- hw = at91_clk_register_main_rc_osc(regmap, "main_rc_osc", 12000000,
- 50000000);
- if (IS_ERR(hw))
+ main_rc_hw = at91_clk_register_main_rc_osc(regmap, "main_rc_osc", 12000000,
+ 50000000);
+ if (IS_ERR(main_rc_hw))
goto err_free;
- hw = at91_clk_register_main_osc(regmap, "main_osc", mainxtal_name, NULL, 0);
- if (IS_ERR(hw))
+ main_osc_hw = at91_clk_register_main_osc(regmap, "main_osc", NULL,
+ &AT91_CLK_PD_NAME(main_xtal_name), 0);
+ if (IS_ERR(main_osc_hw))
goto err_free;
- main_osc_hw = hw;
- parent_names[0] = "main_rc_osc";
- parent_names[1] = "main_osc";
- hw = at91_clk_register_sam9x5_main(regmap, "mainck", parent_names, NULL, 2);
+ parent_hws[0] = main_rc_hw;
+ parent_hws[1] = main_osc_hw;
+ hw = at91_clk_register_sam9x5_main(regmap, "mainck", NULL, parent_hws, 2);
if (IS_ERR(hw))
goto err_free;
sam9x7_pmc->chws[PMC_MAIN] = hw;
for (i = 0; i < PLL_ID_MAX; i++) {
- for (j = 0; j < 3; j++) {
+ for (j = 0; j < PLL_COMPID_MAX; j++) {
struct clk_hw *parent_hw;
if (!sam9x7_plls[i][j].n)
@@ -787,19 +811,23 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
switch (sam9x7_plls[i][j].t) {
case PLL_TYPE_FRAC:
- if (!strcmp(sam9x7_plls[i][j].p, "mainck"))
+ switch (sam9x7_plls[i][j].p) {
+ case SAM9X7_PLL_PARENT_MAINCK:
parent_hw = sam9x7_pmc->chws[PMC_MAIN];
- else if (!strcmp(sam9x7_plls[i][j].p, "main_osc"))
- parent_hw = main_osc_hw;
- else
- parent_hw = __clk_get_hw(of_clk_get_by_name
- (np, sam9x7_plls[i][j].p));
+ break;
+ case SAM9X7_PLL_PARENT_MAIN_XTAL:
+ parent_hw = main_xtal_hw;
main_xtal_hw is not initialized. You should be able to do here as you did on patch 7/31.
+ break;
+ default:
+ /* Should not happen. */
+ parent_hw = NULL;
+ break;
+ }
hw = sam9x60_clk_register_frac_pll(regmap,
&pmc_pll_lock,
sam9x7_plls[i][j].n,
- sam9x7_plls[i][j].p,
- parent_hw, i,
+ NULL, parent_hw, i,
sam9x7_plls[i][j].c,
sam9x7_plls[i][j].l,
sam9x7_plls[i][j].f);
@@ -809,7 +837,7 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
hw = sam9x60_clk_register_div_pll(regmap,
&pmc_pll_lock,
sam9x7_plls[i][j].n,
- sam9x7_plls[i][j].p, NULL, i,
+ NULL, sam9x7_plls[i][0].hw, i,
sam9x7_plls[i][j].c,
sam9x7_plls[i][j].l,
sam9x7_plls[i][j].f, 0);
@@ -822,23 +850,24 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
if (IS_ERR(hw))
goto err_free;
+ sam9x7_plls[i][j].hw = hw;
if (sam9x7_plls[i][j].eid)
sam9x7_pmc->chws[sam9x7_plls[i][j].eid] = hw;
}
}
- parent_names[0] = md_slck_name;
- parent_names[1] = "mainck";
- parent_names[2] = "plla_divpmcck";
- parent_names[3] = "upll_divpmcck";
+ 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;
hw = at91_clk_register_master_pres(regmap, "masterck_pres", 4,
- parent_names, NULL, &sam9x7_master_layout,
+ NULL, parent_hws, &sam9x7_master_layout,
&mck_characteristics, &mck_lock);
if (IS_ERR(hw))
goto err_free;
hw = at91_clk_register_master_div(regmap, "masterck_div",
- "masterck_pres", NULL, &sam9x7_master_layout,
+ NULL, hw, &sam9x7_master_layout,
&mck_characteristics, &mck_lock,
CLK_SET_RATE_GATE, 0);
if (IS_ERR(hw))
@@ -849,24 +878,24 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
parent_names[0] = "plla_divpmcck";
parent_names[1] = "upll_divpmcck";
parent_names[2] = "main_osc";
- hw = sam9x60_clk_register_usb(regmap, "usbck", parent_names, 3);
- if (IS_ERR(hw))
+ usbck_hw = sam9x60_clk_register_usb(regmap, "usbck", parent_names, 3);
+ if (IS_ERR(usbck_hw))
goto err_free;
- parent_names[0] = md_slck_name;
- parent_names[1] = td_slck_name;
- parent_names[2] = "mainck";
- parent_names[3] = "masterck_div";
- parent_names[4] = "plla_divpmcck";
- parent_names[5] = "upll_divpmcck";
- parent_names[6] = "audiopll_divpmcck";
+ parent_hws[0] = md_slck_hw;
+ parent_hws[1] = td_slck_hw;
+ parent_hws[2] = sam9x7_pmc->chws[PMC_MAIN];
+ parent_hws[3] = sam9x7_pmc->chws[PMC_MCK];
+ parent_hws[4] = sam9x7_plls[PLL_ID_PLLA][PLL_COMPID_DIV0].hw;
+ parent_hws[5] = sam9x7_plls[PLL_ID_UPLL][PLL_COMPID_DIV0].hw;
+ parent_hws[6] = sam9x7_plls[PLL_ID_AUDIO][PLL_COMPID_DIV0].hw;
for (i = 0; i < 2; i++) {
char name[6];
snprintf(name, sizeof(name), "prog%d", i);
hw = at91_clk_register_programmable(regmap, name,
- parent_names, NULL, 7, i,
+ NULL, parent_hws, 7, i,
&sam9x7_programmable_layout,
NULL);
if (IS_ERR(hw))
@@ -875,9 +904,14 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
sam9x7_pmc->pchws[i] = hw;
}
+ /* Set systemck parent hws. */
+ sam9x7_systemck[0].parent_hw = sam9x7_pmc->chws[PMC_MCK];
+ sam9x7_systemck[1].parent_hw = usbck_hw;
+ sam9x7_systemck[2].parent_hw = sam9x7_pmc->pchws[0];
+ sam9x7_systemck[3].parent_hw = sam9x7_pmc->pchws[1];
for (i = 0; i < ARRAY_SIZE(sam9x7_systemck); i++) {
hw = at91_clk_register_system(regmap, sam9x7_systemck[i].n,
- sam9x7_systemck[i].p, NULL,
+ NULL, sam9x7_systemck[i].parent_hw,
sam9x7_systemck[i].id,
sam9x7_systemck[i].flags);
if (IS_ERR(hw))
@@ -890,7 +924,7 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
hw = at91_clk_register_sam9x5_peripheral(regmap, &pmc_pcr_lock,
&sam9x7_pcr_layout,
sam9x7_periphck[i].n,
- "masterck_div", NULL,
+ NULL, sam9x7_pmc->chws[PMC_MCK],
sam9x7_periphck[i].id,
&range, INT_MIN,
sam9x7_periphck[i].f);
@@ -900,12 +934,13 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
sam9x7_pmc->phws[sam9x7_periphck[i].id] = hw;
}
- parent_names[0] = md_slck_name;
- parent_names[1] = td_slck_name;
- parent_names[2] = "mainck";
- parent_names[3] = "masterck_div";
+ parent_hws[0] = md_slck_hw;
+ parent_hws[1] = td_slck_hw;
+ parent_hws[2] = sam9x7_pmc->chws[PMC_MAIN];
+ parent_hws[3] = sam9x7_pmc->chws[PMC_MCK];
for (i = 0; i < ARRAY_SIZE(sam9x7_gck); i++) {
u8 num_parents = 4 + sam9x7_gck[i].pp_count;
+ struct clk_hw *tmp_parent_hws[6];
I see maximum pp_count is 2, thus the size of this array could be reduced.
u32 *mux_table;
mux_table = kmalloc_array(num_parents, sizeof(*mux_table),
@@ -916,13 +951,21 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
PMC_INIT_TABLE(mux_table, 4);
PMC_FILL_TABLE(&mux_table[4], sam9x7_gck[i].pp_mux_table,
sam9x7_gck[i].pp_count);
- PMC_FILL_TABLE(&parent_names[4], sam9x7_gck[i].pp,
+
+ for (j = 0; j < sam9x7_gck[i].pp_count; j++) {
+ u8 pll_id = sam9x7_gck[i].pp[j].pll_id;
+ u8 pll_compid = sam9x7_gck[i].pp[j].pll_compid;
+
+ tmp_parent_hws[j] = sam9x7_plls[pll_id][pll_compid].hw;
You can get rid of tmp_parent_hws[] and the above PMC_FILL_TABLE() and just use here something like:
parent_hws[4 + j] = sam9x7_plls[pll_id][pll_compid].hw;
+ }
+
+ PMC_FILL_TABLE(&parent_hws[4], tmp_parent_hws,
sam9x7_gck[i].pp_count);
hw = at91_clk_register_generated(regmap, &pmc_pcr_lock,
&sam9x7_pcr_layout,
sam9x7_gck[i].n,
- parent_names, NULL, mux_table,
+ NULL, parent_hws, mux_table,
num_parents,
sam9x7_gck[i].id,
&sam9x7_gck[i].r,