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

[Changhuang Liang]

Hi, Jerome

Thanks for the review.

> 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@xxxxxxxxxxxxxxxx>
> > Reviewed-by: Xingyu Wu <xingyu.wu@xxxxxxxxxxxxxxxx>
> > ---
> >  .../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.
> 

Got it.

Best Regards,
Changhuang