| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <05e16211-7521-452d-9ef3-988dfe70618e@tuxon.dev>
Date: Sat, 6 Sep 2025 21:33:56 +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 03/32] clk: at91: sam9x75: switch to parent_hw and
parent_data
Hi, Ryan,
On 7/10/25 23:06, Ryan.Wanner@...rochip.com wrote:
> From: Ryan Wanner <Ryan.Wanner@...rochip.com>
>
> 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.
>
> Signed-off-by: Ryan Wanner <Ryan.Wanner@...rochip.com>
> ---
> drivers/clk/at91/sam9x7.c | 308 +++++++++++++++++++++-----------------
> 1 file changed, 173 insertions(+), 135 deletions(-)
>
> diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
> index cbb8b220f16b..31184e11165a 100644
> --- a/drivers/clk/at91/sam9x7.c
> +++ b/drivers/clk/at91/sam9x7.c
> @@ -33,10 +33,22 @@ enum pll_ids {
> PLL_ID_UPLL,
> PLL_ID_AUDIO,
> PLL_ID_LVDS,
> - PLL_ID_PLLA_DIV2,
> PLL_ID_MAX,
> };
>
> +/*
> + * PLL component identifier
> + * @PLL_COMPID_FRAC: Fractional PLL component identifier
> + * @PLL_COMPID_DIV0: 1st PLL divider component identifier
> + * @PLL_COMPID_DIV1: 2nd PLL divider component identifier
> + */
> +enum pll_component_id {
> + PLL_COMPID_FRAC,
> + PLL_COMPID_DIV0,
> + PLL_COMPID_DIV1,
> + PLL_COMPID_MAX,
> +};
> +
> /**
> * enum pll_type - PLL type identifiers
> * @PLL_TYPE_FRAC: fractional PLL identifier
> @@ -180,6 +192,18 @@ static const struct clk_pll_layout pll_divio_layout = {
> .endiv_shift = 30,
> };
>
> +/*
> + * SAM9X7 PLL possible parents
> + * @SAM9X7_PLL_PARENT_MAINCK: MAINCK is PLL a parent
> + * @SAM9X7_PLL_PARENT_MAIN_XTAL: MAIN XTAL is a PLL parent
> + * @SAM9X7_PLL_PARENT_FRACCK: Frac PLL is a PLL parent (for PLL dividers)
> + */
> +enum sam9x7_pll_parent {
> + SAM9X7_PLL_PARENT_MAINCK,
> + SAM9X7_PLL_PARENT_MAIN_XTAL,
> + SAM9X7_PLL_PARENT_FRACCK
> +};
> +
> /*
> * PLL clocks description
> * @n: clock name
> @@ -187,22 +211,24 @@ static const struct clk_pll_layout pll_divio_layout = {
> * @l: clock layout
> * @t: clock type
> * @c: pll characteristics
> + * @hw: pointer to clk_hw
> * @f: clock flags
> * @eid: export index in sam9x7->chws[] array
> */
> -static const struct {
> +static struct {
> const char *n;
> - const char *p;
> const struct clk_pll_layout *l;
> u8 t;
> const struct clk_pll_characteristics *c;
> + struct clk_hw *hw;
> unsigned long f;
> + enum sam9x7_pll_parent p;
> u8 eid;
> -} sam9x7_plls[][3] = {
> +} sam9x7_plls[][PLL_COMPID_MAX] = {
> [PLL_ID_PLLA] = {
> - {
> + [PLL_COMPID_FRAC] = {
> .n = "plla_fracck",
> - .p = "mainck",
> + .p = SAM9X7_PLL_PARENT_MAINCK,
> .l = &plla_frac_layout,
> .t = PLL_TYPE_FRAC,
> /*
> @@ -213,9 +239,9 @@ static const struct {
> .c = &plla_characteristics,
> },
>
> - {
> + [PLL_COMPID_DIV0] = {
> .n = "plla_divpmcck",
> - .p = "plla_fracck",
> + .p = SAM9X7_PLL_PARENT_FRACCK,
> .l = &pll_divpmc_layout,
> .t = PLL_TYPE_DIV,
> /* This feeds CPU. It should not be disabled */
> @@ -223,21 +249,35 @@ static const struct {
> .eid = PMC_PLLACK,
> .c = &plla_characteristics,
> },
> +
> + [PLL_COMPID_DIV1] = {
> + .n = "plla_div2pmcck",
> + .p = SAM9X7_PLL_PARENT_FRACCK,
> + .l = &plladiv2_divpmc_layout,
> + /*
> + * This may feed critical parts of the system like timers.
> + * It should not be disabled.
> + */
> + .f = CLK_IS_CRITICAL | CLK_SET_RATE_GATE,
> + .c = &plladiv2_characteristics,
> + .eid = PMC_PLLADIV2,
> + .t = PLL_TYPE_DIV,
> + },
According to manual at [1] this looks correct. Maybe mention this move in
commit description.
[1]
https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X7-Series-Data-Sheet-DS60001813.pdf
[...]
> 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 = "main_xtal";
> 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;
> + static struct clk_parent_data parent_data;
> + 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", "md_slck");
> - if (i < 0)
> - return;
> -
> - 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"));
> + main_xtal_hw = __clk_get_hw(of_clk_get_by_name(np, main_xtal_name));
>
> - i = of_property_match_string(np, "clock-names", "main_xtal");
> - if (i < 0)
> + if (!td_slck_hw || !md_slck_hw || !main_xtal_hw)
> return;
> - mainxtal_name = of_clk_get_parent_name(np, i);
>
> regmap = device_node_to_regmap(np);
> if (IS_ERR(regmap))
> @@ -754,26 +774,27 @@ 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))
> + parent_data.name = main_xtal_name;
> + parent_data.fw_name = main_xtal_name;
You should be able to use AT91_CLK_PD_NAME() directly in the above call
where main_xtal_name should be retrieved with of_clk_get_parent_name()
Powered by blists - more mailing lists