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

[Changhuang Liang]

Hi, Conor

Thanks for the review.

> On Mon, Aug 17, 2026 at 01:27:08AM +0000, Changhuang Liang wrote:
> > 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-m
> > > > > > > > > > armo
> > > > > > > > > > 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 probably nothing else that linux will ever need?

Yes.

> 
> > > > 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?
> 
> Correct, but I am talking about the socinfo driver being the one that calls
> devm_mfd_add_devices() to create a device for the pll so that in the end
> there are a total of two drivers, rather than create something identical to
> sg2044-topsys.c that does nothing other than initialise two mfd cells.
> May as well consolidate the mfd_cell definition and call to
> devm_mfd_add_devices() in one file, since it would be in the same directory
> as the socinfo driver anyway.

Okay, I will use this method.

> 
> >
> > >
> > > > 	},
> > > > };
> > > >
> > > > 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?
> 
> I wouldn't complicate them with things that they don't need, so it depends on
> if these other syscon regions contain more than just PLLs and if there'll ever
> be drivers for the other stuff in those regions.
> 
> >
> > >
> > > > 	},
> > > > };
> > > >
> > > > 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