Re: [PATCH v2 11/12] soc: starfive: Add socinfo driver for JHB100 SoC

From: Conor Dooley <conor@kernel.org>
To: Changhuang Liang <changhuang.liang@starfivetech.com>
Cc: Michael Turquette <mturquette@baylibre.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Brian Masney <bmasney@redhat.com>, Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Emil Renner Berthing <kernel@esmil.dk>,
	Chen Wang <unicorn_wang@outlook.com>,
	Inochi Amaoto <inochiama@gmail.com>,
	Alexey Charkov <alchark@gmail.com>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Keguang Zhang <keguang.zhang@gmail.com>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>
Subject: Re: [PATCH v2 11/12] soc: starfive: Add socinfo driver for JHB100 SoC
Date: Wed, 13 May 2026 20:16:44 +0100	[thread overview]
Message-ID: <20260513-headstand-hertz-bf31fe6c1938@spud> (raw)
In-Reply-To: <ZQ4PR01MB120205CCC0A3229824701640F2062@ZQ4PR01MB1202.CHNPR01.prod.partner.outlook.cn>

[-- Attachment #1.1: Type: text/plain, Size: 3254 bytes --]

On Wed, May 13, 2026 at 01:01:46AM +0000, Changhuang Liang wrote:
> Hi, Conor
> 
> Thanks for the review.
> 
> > On Tue, May 12, 2026 at 01:35:20AM -0700, Changhuang Liang wrote:
> > > Add socinfo driver for JHB100 SoC. Currently available for
> > > distinguishing between the two reversions, A0 and A1.
> > >
> > > Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> > > ---
> > >  MAINTAINERS                                   |  5 ++
> > >  drivers/soc/Kconfig                           |  1 +
> > >  drivers/soc/Makefile                          |  1 +
> > >  drivers/soc/starfive/Kconfig                  |  6 ++
> > >  drivers/soc/starfive/Makefile                 |  2 +
> > >  drivers/soc/starfive/socinfo/Kconfig          | 11 +++
> > >  drivers/soc/starfive/socinfo/Makefile         |  2 +
> > >  drivers/soc/starfive/socinfo/jhb100-socinfo.c | 80
> > > +++++++++++++++++++
> > >  8 files changed, 108 insertions(+)
> > >  create mode 100644 drivers/soc/starfive/Kconfig  create mode 100644
> > > drivers/soc/starfive/Makefile  create mode 100644
> > > drivers/soc/starfive/socinfo/Kconfig
> > >  create mode 100644 drivers/soc/starfive/socinfo/Makefile
> > >  create mode 100644 drivers/soc/starfive/socinfo/jhb100-socinfo.c
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS index
> > > 6f6aac7cea95..66175f453d20 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -25613,6 +25613,11 @@ S:	Maintained
> > >  F:	drivers/reset/starfive/reset-starfive-jhb1*
> > >  F:	include/dt-bindings/reset/starfive,jhb1*.h
> > >
> > > +STARFIVE JHB100 SOCINFO DRIVER
> > > +M:	Changhuang Liang <changhuang.liang@starfivetech.com>
> > > +S:	Maintained
> > > +F:	drivers/soc/starfive/
> > 
> > FWIW, this is not what I asked for in v1, this should remain the full path to the
> > driver. I wanted you to add this directory back to the "STARFIVE SOC
> > DRIVERS" entry, since it's me that'll be applying patches for this driver.
> 
> Sorry, I misunderstood what you meant.
> 
> > 
> > > +
> > >  STARFIVE JHB100 SYSCON
> > >  M:	Changhuang Liang <changhuang.liang@starfivetech.com>
> > >  S:	Maintained
> > > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index
> > > a2d65adffb80..b3b01fc38139 100644
> > > --- a/drivers/soc/Kconfig
> > > +++ b/drivers/soc/Kconfig
> > > @@ -24,6 +24,7 @@ source "drivers/soc/renesas/Kconfig"
> > >  source "drivers/soc/rockchip/Kconfig"
> > >  source "drivers/soc/samsung/Kconfig"
> > >  source "drivers/soc/sophgo/Kconfig"
> > > +source "drivers/soc/starfive/Kconfig"
> > >  source "drivers/soc/sunxi/Kconfig"
> > >  source "drivers/soc/tegra/Kconfig"
> 
> [...]
> 
> > > +	pr_info("StarFive %s SoC rev(%s)\n", attrs->soc_id,
> > > +attrs->revision);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +early_initcall(starfive_socinfo_init);
> > 
> > Does this really need to be an early initcall btw?
> 
> Yes, there are some drivers that need to use soc_device_match() to match different 
> revisions, in order to handle the processes caused by differences between revisions. 
> So there's no harm in initializing this driver earlier.

Other socinfo drivers use device_initcall, can you use that instead?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv