Re: [PATCH v5 3/8] clk: starfive: Add peripheral-0 domain PLL clock driver

[Jerome Brunet <jbrunet@baylibre.com>]

On 2026-08-28 02:56 -0700, Changhuang Liang wrote:
> Add peripheral-0 domain PLL clock driver support for StarFive JHB100
> SoC.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> Reviewed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> ---
>  .../clk/starfive/clk-starfive-jhb100-pll.c    | 60 +++++++++++++++----
>  1 file changed, 50 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/clk/starfive/clk-starfive-jhb100-pll.c b/drivers/clk/starfive/clk-starfive-jhb100-pll.c
> index e23365164497..f6ebaaf5aa18 100644
> --- a/drivers/clk/starfive/clk-starfive-jhb100-pll.c
> +++ b/drivers/clk/starfive/clk-starfive-jhb100-pll.c
> @@ -30,6 +30,9 @@
>  #define JHB100_PLL4_OFFSET		0x18
>  #define JHB100_PLL5_OFFSET		0x24
>  
> +/* Peripheral-0 domain PLL */
> +#define JHB100_PLL6_OFFSET		0x00
> +
>  #define JHB100_PLL_CFG0_OFFSET		0x0
>  #define JHB100_PLL_CFG1_OFFSET		0x4
>  #define JHB100_PLL_CFG2_OFFSET		0x8
> @@ -417,16 +420,21 @@ static int jhb100_pll_probe(struct platform_device *pdev)
>  	unsigned int idx;
>  	int ret;
>  
> -	id = platform_get_device_id(pdev);
> -	if (!id)
> -		return dev_err_probe(dev, -EINVAL, "no match data\n");
> -
> -	/*
> -	 * Instantiated as an MFD cell of the sys0 system controller,
> -	 * which owns the DT node describing the PLL registers.
> -	 */
> -	match_data = (const struct jhb100_pll_match_data *)id->driver_data;
> -	np = dev_of_node(dev->parent);
> +	match_data = device_get_match_data(dev);
> +	if (match_data) {
> +		np = dev_of_node(dev);
> +	} else {
> +		id = platform_get_device_id(pdev);
> +		if (!id)
> +			return dev_err_probe(dev, -EINVAL, "no match data\n");
> +
> +		/*
> +		 * Instantiated as an MFD cell of the sys0 system controller,
> +		 * which owns the DT node describing the PLL registers.
> +		 */
> +		match_data = (const struct jhb100_pll_match_data *)id->driver_data;
> +		np = dev_of_node(dev->parent);
> +	}

This is quite ugly.

It looks like your PLL is a building block you'll be re-using.
Just make a module out it and re-use it is different platform drivers rather
than trying to fit all your devices in the same platform driver.

>  
>  	priv = devm_kzalloc(&pdev->dev, struct_size(priv, pll, match_data->num_pll),
>  			    GFP_KERNEL);
> @@ -504,6 +512,28 @@ static const struct jhb100_pll_match_data jhb100_sys0_pll = {
>  	.num_pll = ARRAY_SIZE(jhb100_sys0_pll_info),
>  };
>  
> +static const struct jhb100_pll_preset jhb100_pll6_presets[] = {
> +	{
> +		.freq = 2400000000,
> +		.fbdiv = 192,
> +		.frac = 0,
> +		.refdiv = 1,
> +		.postdiv = 0,
> +		.foutpostdiv_en = 1,
> +		.foutvcop_en = 0,
> +	},
> +};
> +
> +static const struct jhb100_pll_info jhb100_per0_pll_info[] = {
> +	JHB100_PLL(JHB100_PER0PLL_PLL6_OUT, "pll6_out", jhb100_pll6_presets,
> +		   ARRAY_SIZE(jhb100_pll6_presets), JHB100_PLL6_OFFSET, false),
> +};
> +
> +static const struct jhb100_pll_match_data jhb100_per0_pll = {
> +	.pll_info = jhb100_per0_pll_info,
> +	.num_pll = ARRAY_SIZE(jhb100_per0_pll_info),
> +};
> +
>  static const struct platform_device_id jhb100_pll_ids[] = {
>  	{
>  		.name = "jhb100-sys0-pll",
> @@ -513,11 +543,21 @@ static const struct platform_device_id jhb100_pll_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(platform, jhb100_pll_ids);
>  
> +static const struct of_device_id jhb100_pll_match[] = {
> +	{
> +		.compatible = "starfive,jhb100-per0-syscon",
> +		.data = &jhb100_per0_pll,
> +	},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, jhb100_pll_match);
> +
>  static struct platform_driver jhb100_pll_driver = {
>  	.probe = jhb100_pll_probe,
>  	.id_table = jhb100_pll_ids,
>  	.driver = {
>  		.name = "clk-starfive-jhb100-pll",
> +		.of_match_table = jhb100_pll_match,
>  		.suppress_bind_attrs = true,
>  	},
>  };
> -- 
> 2.25.1
> 
>