[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8361a42d-0c70-4a8c-b0a0-7056ba21b508@tuxon.dev>
Date: Thu, 2 Jan 2025 12:48:25 +0200
From: Claudiu Beznea <claudiu.beznea@...on.dev>
To: Ryan.Wanner@...rochip.com, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, nicolas.ferre@...rochip.com,
alexandre.belloni@...tlin.com, mturquette@...libre.com, sboyd@...nel.org,
arnd@...db.de
Cc: dharma.b@...rochip.com, mihai.sain@...rochip.com,
romain.sioen@...rochip.com, varshini.rajendran@...rochip.com,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
linux-mmc@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-spi@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH v4 08/13] clk: at91: sama7d65: add sama7d65 pmc driver
Hi, Ryan,
On 20.12.2024 23:07, Ryan.Wanner@...rochip.com wrote:
> From: Ryan Wanner <Ryan.Wanner@...rochip.com>
>
> Add clock support for SAMA7D65 SoC.
>
> Increase maximum number of valid master clocks. The PMC for the SAMA7D65
> requires 9 master clocks.
>
> Increase maximum amount of PLLs to 9 to support SAMA7D65 SoC PLL
> requirements.
>
> Signed-off-by: Ryan Wanner <Ryan.Wanner@...rochip.com>
> Reviewed-by: Claudiu Beznea <claudiu.beznea@...on.dev>
> ---
> drivers/clk/at91/Makefile | 1 +
> drivers/clk/at91/clk-master.c | 2 +-
> drivers/clk/at91/clk-sam9x60-pll.c | 2 +-
> drivers/clk/at91/pmc.c | 1 +
> drivers/clk/at91/sama7d65.c | 1375 ++++++++++++++++++++++++++++
> 5 files changed, 1379 insertions(+), 2 deletions(-)
> create mode 100644 drivers/clk/at91/sama7d65.c
>
[ ... ]
> +
> + parent_hws[0] = md_slck_hw;
> + parent_hws[1] = td_slck_hw;
> + parent_hws[2] = sama7d65_pmc->chws[PMC_MAIN];
> + for (i = PCK_PARENT_HW_MCK1; i < ARRAY_SIZE(sama7d65_mckx); i++) {
> + u8 num_parents = 3 + sama7d65_mckx[i].ep_count;
> + struct clk_hw *tmp_parent_hws[8];
> + u32 *mux_table;
> +
> + mux_table = kmalloc_array(num_parents, sizeof(*mux_table),
> + GFP_KERNEL);
> + if (!mux_table)
> + goto err_free;
> +
> + PMC_INIT_TABLE(mux_table, 3);
> + PMC_FILL_TABLE(&mux_table[3], sama7d65_mckx[i].ep_mux_table,
> + sama7d65_mckx[i].ep_count);
> + for (j = 0; j < sama7d65_mckx[i].ep_count; j++) {
> + u8 pll_id = sama7d65_mckx[i].ep[j].pll_id;
> + u8 pll_compid = sama7d65_mckx[i].ep[j].pll_compid;
> +
> + tmp_parent_hws[j] = sama7d65_plls[pll_id][pll_compid].hw;
> + }
> + PMC_FILL_TABLE(&parent_hws[3], tmp_parent_hws,
> + sama7d65_mckx[i].ep_count);
> +
> + hw = at91_clk_sama7g5_register_master(regmap, sama7d65_mckx[i].n,
> + num_parents, NULL, parent_hws,
> + mux_table, &pmc_mckX_lock,
> + sama7d65_mckx[i].id,
> + sama7d65_mckx[i].c,
> + sama7d65_mckx[i].ep_chg_id);
> + alloc_mem[alloc_mem_size++] = mux_table;
> +
> + if (IS_ERR(hw)) {
> + kfree(mux_table);
Now mux_table is freed twice, once here, once in err_free section. Having
mux_table added to alloc_mem[] is enough. I'll do the propoer adjustment
while applying.
> + goto err_free;
> + }
> +
> + sama7d65_mckx[i].hw = hw;
> + if (sama7d65_mckx[i].eid)
> + sama7d65_pmc->chws[sama7d65_mckx[i].eid] = hw;
> + }
> +
> + parent_names[0] = "syspll_divpmcck";
> + parent_names[1] = "usbpll_divpmcck";
> + parent_names[2] = "main_osc";
> + hw = sam9x60_clk_register_usb(regmap, "usbck", parent_names, 3);
> + if (IS_ERR(hw))
> + goto err_free;
> +
> + parent_hws[0] = md_slck_hw;
> + parent_hws[1] = td_slck_hw;
> + parent_hws[2] = sama7d65_pmc->chws[PMC_MAIN];
> + parent_hws[3] = sama7d65_plls[PLL_ID_SYS][PLL_COMPID_DIV0].hw;
> + parent_hws[4] = sama7d65_plls[PLL_ID_DDR][PLL_COMPID_DIV0].hw;
> + parent_hws[5] = sama7d65_plls[PLL_ID_GPU][PLL_COMPID_DIV0].hw;
> + parent_hws[6] = sama7d65_plls[PLL_ID_BAUD][PLL_COMPID_DIV0].hw;
> + parent_hws[7] = sama7d65_plls[PLL_ID_AUDIO][PLL_COMPID_DIV0].hw;
> + parent_hws[8] = sama7d65_plls[PLL_ID_ETH][PLL_COMPID_DIV0].hw;
> +
> + for (i = 0; i < 8; i++) {
> + char name[6];
> +
> + snprintf(name, sizeof(name), "prog%d", i);
> +
> + hw = at91_clk_register_programmable(regmap, name, NULL, parent_hws,
> + 9, i,
> + &programmable_layout,
> + sama7d65_prog_mux_table);
> + if (IS_ERR(hw))
> + goto err_free;
> +
> + sama7d65_pmc->pchws[i] = hw;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(sama7d65_systemck); i++) {
> + hw = at91_clk_register_system(regmap, sama7d65_systemck[i].n,
> + sama7d65_systemck[i].p, NULL,
> + sama7d65_systemck[i].id, 0);
> + if (IS_ERR(hw))
> + goto err_free;
> +
> + sama7d65_pmc->shws[sama7d65_systemck[i].id] = hw;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(sama7d65_periphck); i++) {
> + hw = at91_clk_register_sam9x5_peripheral(regmap, &pmc_pcr_lock,
> + &sama7d65_pcr_layout,
> + sama7d65_periphck[i].n,
> + NULL,
> + sama7d65_mckx[sama7d65_periphck[i].p].hw,
> + sama7d65_periphck[i].id,
> + &sama7d65_periphck[i].r,
> + sama7d65_periphck[i].chgp ? 0 :
> + INT_MIN, 0);
> + if (IS_ERR(hw))
> + goto err_free;
> +
> + sama7d65_pmc->phws[sama7d65_periphck[i].id] = hw;
> + }
> +
> + parent_hws[0] = md_slck_hw;
> + parent_hws[1] = td_slck_hw;
> + parent_hws[2] = sama7d65_pmc->chws[PMC_MAIN];
> + parent_hws[3] = sama7d65_pmc->chws[PMC_MCK1];
> + for (i = 0; i < ARRAY_SIZE(sama7d65_gck); i++) {
> + u8 num_parents = 4 + sama7d65_gck[i].pp_count;
> + struct clk_hw *tmp_parent_hws[8];
> + u32 *mux_table;
> +
> + mux_table = kmalloc_array(num_parents, sizeof(*mux_table),
> + GFP_KERNEL);
> + if (!mux_table)
> + goto err_free;
> +
> + PMC_INIT_TABLE(mux_table, 4);
> + PMC_FILL_TABLE(&mux_table[4], sama7d65_gck[i].pp_mux_table,
> + sama7d65_gck[i].pp_count);
> + for (j = 0; j < sama7d65_gck[i].pp_count; j++) {
> + u8 pll_id = sama7d65_gck[i].pp[j].pll_id;
> + u8 pll_compid = sama7d65_gck[i].pp[j].pll_compid;
> +
> + tmp_parent_hws[j] = sama7d65_plls[pll_id][pll_compid].hw;
> + }
> + PMC_FILL_TABLE(&parent_hws[4], tmp_parent_hws,
> + sama7d65_gck[i].pp_count);
> +
> + hw = at91_clk_register_generated(regmap, &pmc_pcr_lock,
> + &sama7d65_pcr_layout,
> + sama7d65_gck[i].n, NULL,
> + parent_hws, mux_table,
> + num_parents,
> + sama7d65_gck[i].id,
> + &sama7d65_gck[i].r,
> + sama7d65_gck[i].pp_chg_id);
> + if (IS_ERR(hw))
> + goto err_free;
> +
> + sama7d65_pmc->ghws[sama7d65_gck[i].id] = hw;
> + alloc_mem[alloc_mem_size++] = mux_table;
This should have been added just after:
if (!mux_table)
goto err_free;
I'll adjust it while applying.
> + }
> +
> + of_clk_add_hw_provider(np, of_clk_hw_pmc_get, sama7d65_pmc);
> + kfree(alloc_mem);
> +
> + return;
> +
> +err_free:
> + if (alloc_mem) {
> + for (i = 0; i < alloc_mem_size; i++)
> + kfree(alloc_mem[i]);
> + kfree(alloc_mem);
> + }
> +
> + kfree(sama7d65_pmc);
> +}
> +
> +/* Some clks are used for a clocksource */
> +CLK_OF_DECLARE(sama7d65_pmc, "microchip,sama7d65-pmc", sama7d65_pmc_setup);
Powered by blists - more mailing lists