Re: [PATCH v6 08/12] clk: nuvoton: ma35d1: Retrieve HXT/LXT from DT

[Jerome Brunet <jbrunet@baylibre.com>]

On mer. 30 sept. 2026 at 19:24, Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> HXT and LXT are crystal oscillator inputs of the clock controller, they
> are described in the DT, so retrieve them, in order, and store them in
> their respective HXT/LXT hw table entries.
>
> Since old DTs reference the HXT fixed-clock without naming it and do not
> describe LXT at all, we assume that HXT must be present, and fallback to
> creating a fixed clock for LXT if it is not described (for backward
> compatibility purposes).
>
> The downstream gate clocks can directly use the hw clocks as parents,
> instead of relying on string matching.
>
> Fixes: 691521a367cf ("clk: nuvoton: Add clock driver for ma35d1 clock controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/clk/nuvoton/clk-ma35d1.c | 43 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/nuvoton/clk-ma35d1.c b/drivers/clk/nuvoton/clk-ma35d1.c
> index ceebcbd8c18b..d955d79abdd2 100644
> --- a/drivers/clk/nuvoton/clk-ma35d1.c
> +++ b/drivers/clk/nuvoton/clk-ma35d1.c
> @@ -4,6 +4,7 @@
>   * Author: Chi-Fang Li <cfli0@nuvoton.com>
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
> @@ -191,6 +192,15 @@ static struct clk_hw *ma35d1_clk_gate(struct device *dev, const char *name, cons
>  				    reg, shift, 0, &ma35d1_lock);
>  }
>  
> +static struct clk_hw *ma35d1_clk_gate_parent(struct device *dev, const char *name,
> +					     struct clk_hw *parent,
> +					     void __iomem *reg, u8 shift)
> +{
> +	return devm_clk_hw_register_gate_parent_hw(dev, name, parent,
> +						   CLK_SET_RATE_PARENT,
> +						   reg, shift, 0, &ma35d1_lock);
> +}
> +
>  static int ma35d1_get_pll_setting(struct device_node *clk_node, u32 *pllmode)
>  {
>  	const char *of_str;
> @@ -215,10 +225,12 @@ static int ma35d1_clocks_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct device_node *clk_node = pdev->dev.of_node;
> +	struct clk_bulk_data *clks;
>  	void __iomem *clk_base;
>  	static struct clk_hw **hws;
>  	static struct clk_hw_onecell_data *ma35d1_hw_data;
>  	u32 pllmode[PLL_MAX_NUM];
> +	int num_clks;
>  	int ret;
>  
>  	ma35d1_hw_data = devm_kzalloc(dev,
> @@ -240,12 +252,27 @@ static int ma35d1_clocks_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	hws[HXT] = ma35d1_clk_fixed("hxt", 24000000);
> -	hws[HXT_GATE] = ma35d1_clk_gate(dev, "hxt_gate", "hxt",
> -					clk_base + REG_CLK_PWRCTL, 0);
> -	hws[LXT] = ma35d1_clk_fixed("lxt", 32768);
> -	hws[LXT_GATE] = ma35d1_clk_gate(dev, "lxt_gate", "lxt",
> -					clk_base + REG_CLK_PWRCTL, 1);
> +	num_clks = devm_clk_bulk_get_all(dev, &clks);
> +	if (num_clks < 0)
> +		return num_clks;
> +
> +	if (!num_clks) {
> +		dev_err(dev, "missing crystal input clocks\n");
> +		return -ENODEV;
> +	}
> +
> +	hws[HXT] = __clk_get_hw(clks[0].clk);

Don't open code it. use .fw_name

> +
> +	if (num_clks > 1)
> +		hws[LXT] = __clk_get_hw(clks[1].clk);
> +	else
> +		/* Old DTs do not describe the low-speed crystal */
> +		hws[LXT] = ma35d1_clk_fixed("lxt", 32768);

I'd give it another name so you can clearly see the difference between the
DT one and the manually registered one.

> +

Don't need to open code this either.
provide both .fw_name and .name - CCF will fallback to the name.

When you want to conditionally register the fixed is up to you.

> +	hws[HXT_GATE] = ma35d1_clk_gate_parent(dev, "hxt_gate", hws[HXT],
> +					       clk_base + REG_CLK_PWRCTL, 0);
> +	hws[LXT_GATE] = ma35d1_clk_gate_parent(dev, "lxt_gate", hws[LXT],
> +					       clk_base + REG_CLK_PWRCTL, 1);
>  	hws[HIRC] = ma35d1_clk_fixed("hirc", 12000000);
>  	hws[HIRC_GATE] = ma35d1_clk_gate(dev, "hirc_gate", "hirc",
>  					 clk_base + REG_CLK_PWRCTL, 2);
> @@ -632,8 +659,8 @@ static int ma35d1_clocks_probe(struct platform_device *pdev)
>  	hws[UART16_GATE] = ma35d1_clk_gate(dev, "uart16_gate", "uart16_div",
>  					   clk_base + REG_CLK_APBCLK0, 28);
>  
> -	hws[RTC_GATE] = ma35d1_clk_gate(dev, "rtc_gate", "lxt",
> -					clk_base + REG_CLK_APBCLK0, 29);
> +	hws[RTC_GATE] = ma35d1_clk_gate_parent(dev, "rtc_gate", hws[LXT],
> +					       clk_base + REG_CLK_APBCLK0, 29);
>  	hws[DDR_GATE] = ma35d1_clk_gate(dev, "ddr_gate", "ddrpll",
>  					clk_base + REG_CLK_APBCLK0, 30);
>  
>
> -- 
> 2.55.0
>

-- 
Jerome