[PATCH v4 7/8] soc: starfive: Add socinfo driver for JHB100 SoC

Changhuang Liang changhuang.liang at starfivetech.com
Sun Aug 16 18:27:08 PDT 2026


Hi, Conor

Thanks for the review.

> On Fri, Aug 14, 2026 at 01:13:38AM +0000, Changhuang Liang wrote:
> > Hi,Conor
> >
> > Thanks for the review.
> >
> > > On Thu, Aug 13, 2026 at 08:10:12AM +0000, Changhuang Liang wrote:
> > > > Hi ,Conor
> > > >
> > > > Thanks for the review.
> > > >
> > > > > On Wed, Aug 12, 2026 at 10:37:16AM +0000, Changhuang Liang
> wrote:
> > > > > > Hi, Conor
> > > > > >
> > > > > > Thanks for the review.
> > > > > >
> > > > > > > On Tue, Aug 11, 2026 at 05:36:19AM +0000, Changhuang Liang
> wrote:
> > > > > > >
> > > > > > > > > On Sat, Aug 08, 2026 at 06:50:53PM -0700, Changhuang
> > > > > > > > > Liang
> > > wrote:
> > > > > > >
> > > > > > > > > > +static int __init starfive_socinfo_init(void) {
> > > > > > > > > > +	struct soc_device_attribute *attrs;
> > > > > > > > > > +	struct soc_device *soc_dev;
> > > > > > > > > > +	const char *machine = NULL;
> > > > > > > > > > +	struct device_node *np;
> > > > > > > > > > +	struct regmap *regmap;
> > > > > > > > > > +	char rev_char;
> > > > > > > > > > +	u32 rev_id;
> > > > > > > > > > +	int ret;
> > > > > > > > > > +
> > > > > > > > > > +	np = of_find_compatible_node(NULL, NULL,
> > > > > > > > > > +"starfive,jhb100-sys0-syscon");
> > > > > > > > >
> > > > > > > > > Remind me again why this is not just probed as an mfd
> > > > > > > > > cell from the syscon driver?
> > > > > > > > >
> > > > > > > > > (Hint: please put this info in the commit message).
> > > > > > > >
> > > > > > > > The discussion result with Krzysztof at that time is recorded here:
> > > > > > > > https://lore.kernel.org/all/20260405-strong-watchful-marmo
> > > > > > > > t-fd
> > > > > > > > fad6
> > > > > > > > @quo
> > > > > > > > ll/
> > > > > > > >
> > > > > > > > One register should not be treated as a separate device node.
> > > > > > >
> > > > > > > That's a different question. In the case of an mfd cell
> > > > > > > probed from the syscon driver there would be no devicetree
> > > > > > > modifications required compared to what you have now. e.g.
> > > > > > > sg2044-topsys.c
> > > > > >
> > > > > > This seems feasible, and I can introduce this modification in
> > > > > > the next
> > > > > version.
> > > > >
> > > > >
> > > > > In fact, you don't even need the mfd cell, because your driver
> > > > > is going to be in drivers/soc/starfive anyway? You can just bind
> > > > > directly to the
> > > > > sys0 syscon I think.
> > > >
> > > > I suddenly realized that I missed one issue:
> > > > starfive,jhb100-sys0-syscon is already bound to the PLL driver by
> > > > default, I
> > > can't use it to bind another device anymore.
> > > >
> > > > which is why I used
> > > >
> > > > np = of_find_compatible_node(NULL, NULL,
> > > > "starfive,jhb100-sys0-syscon");
> > > >
> > > > in jhb100-socinfo.c.
> > > >
> > > > So it seems the MFD cell approach isn't really feasible either?
> > >
> > > You could, in that case, probe the clock driver using the mfd_cell
> > > like sg2044-topsys. What else other than the pll controls and the
> > > soc info register is in this register region?
> >
> > We don't have any other drivers right now in this register region.
> 
> I'm not asking about what's got a driver right now though, I am wondering
> what else the registers in this region are for.

Here are some registers related to debug functionality.

> >
> > So does that mean I can revise it to the format below:
> >
> > static const struct mfd_cell jhb100_sys0_subdev[] = {
> > 	{
> > 		.name = "jhb100-sys0-pll",
> > 	},
> 
> > 	{
> > 		.name = "jhb100-socinfo",
> 
> Why would this be needed? The driver with the mfd_cell would be the same
> one that provides the soc info, no?

My understanding is that one driver can only bind to one device, so the PLL driver 
and the socinfo driver need these two devices respectively, and thus cannot share
one?

> 
> > 	},
> > };
> >
> > static const struct mfd_cell jhb100_per0_subdev[] = {
> > 	{
> > 		.name = "jhb100-per0-pll",
> 
> Why do these other ones need to be modified?

I was thinking, since sys0-pll has been adjusted, would it be better to also adjust per0-pll and per1-pll together in a unified way?

> 
> > 	},
> > };
> >
> > static const struct mfd_cell jhb100_per1_subdev[] = {
> > 	{
> > 		.name = "jhb100-per1-pll",
> > 	},
> > };
> >
> > static const struct mfd_cell jhb100_ pcierp _subdev[] = {
> > 	{
> > 		.name = "jhb100-pcierp-reset",
> > 	},
> > };

Best Regards,
Changhuang




More information about the linux-riscv mailing list