Re: [PATCH v2 2/2] pinctrl: add NXP S32 SoC family support

[Chester Lin <clin@xxxxxxxx>]

Hi Andreas,

On Tue, Nov 29, 2022 at 02:40:34PM +0100, Andreas Färber wrote:
> Hi Chester,
> 
> Am 28.11.22 um 06:48 schrieb Chester Lin:
> > Add the pinctrl driver for NXP S32 SoC family. This driver is mainly based
> > on NXP's downstream implementation on CodeAurora[1].
> > 
> > [1] https://source.codeaurora.org/external/autobsps32/linux/tree/drivers/pinctrl/freescale?h=bsp34.0-5.10.120-rt
> > 
> > Signed-off-by: Matthew Nunez <matthew.nunez@xxxxxxx>
> > Signed-off-by: Phu Luu An <phu.luuan@xxxxxxx>
> > Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@xxxxxxx>
> > Signed-off-by: Larisa Grigore <larisa.grigore@xxxxxxx>
> > Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@xxxxxxx>
> > Signed-off-by: Andrei Stefanescu <andrei.stefanescu@xxxxxxx>
> > Signed-off-by: Radu Pirea <radu-nicolae.pirea@xxxxxxx>
> > Signed-off-by: Chester Lin <clin@xxxxxxxx>
> > ---
> > 
> > Changes in v2:
> > - Create a s32_pin_range matrix in the driver for replacing the "nxp,pins"
> >    property in DT.
> > - Refine the compatible name to "nxp,s32g2-siul2-pinctrl".
> 
> Thanks.
> 
> > - Fix the copyright requested by NXP.
> > - Remove a few recipients from the Cc list since these email addresses are no
> >    longer available.
> > 
> >   drivers/pinctrl/freescale/Kconfig         |   16 +
> >   drivers/pinctrl/freescale/Makefile        |    2 +
> >   drivers/pinctrl/freescale/pinctrl-s32.h   |   77 ++
> >   drivers/pinctrl/freescale/pinctrl-s32cc.c | 1003 +++++++++++++++++++++
> >   drivers/pinctrl/freescale/pinctrl-s32g.c  |  773 ++++++++++++++++
> >   5 files changed, 1871 insertions(+)
> >   create mode 100644 drivers/pinctrl/freescale/pinctrl-s32.h
> >   create mode 100644 drivers/pinctrl/freescale/pinctrl-s32cc.c
> >   create mode 100644 drivers/pinctrl/freescale/pinctrl-s32g.c
> > 
> > diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
> > index 7a32f77792d9..fdd8f5492830 100644
> > --- a/drivers/pinctrl/freescale/Kconfig
> > +++ b/drivers/pinctrl/freescale/Kconfig
> > @@ -217,3 +217,19 @@ config PINCTRL_IMXRT1170
> >   	select PINCTRL_IMX
> >   	help
> >   	  Say Y here to enable the imxrt1170 pinctrl driver
> > +
> > +config PINCTRL_S32CC
> > +	bool "NXP S32 Common Chassis pinctrl driver core"
> > +	depends on ARCH_S32 && OF
> > +	select GENERIC_PINCTRL_GROUPS
> > +	select GENERIC_PINMUX_FUNCTIONS
> > +	select GENERIC_PINCONF
> > +	help
> > +	  Say Y here to enable the NXP S32CC pinctrl driver core
> 
> Does this driver core make any sense without a specific driver?
> 

No, it does not. This driver core must be associated with a specific platform
driver, such as PINCTL_S32G.

> I.e., could this just be a menu-invisible internal option if S32G is the one
> the user needs to select anyway?

If you mean making the option invisible in menuconfig, it can be done by simply
removing the prompt message "NXP S32 Common .." from the bool type line. Although
it will disappear from menuconfig but it can be automatically selected once the
config PINCTRL_S32G is set to "y" due to its "select" option.

> The alternative would be to leave it and have S32G depend on it, creating a
> submenu structure, but that would then still allow to build the driver core
> without any users.
> 
> > +
> > +config PINCTRL_S32G
> > +	depends on ARCH_S32 && OF
> > +	bool "NXP S32G pinctrl driver"
> > +	select PINCTRL_S32CC
> > +	help
> > +	  Say Y here to enable the pinctrl driver for NXP 32G family SoCs
> 
> s/32G/S32G/
> 

Will fix it in v3. Thanks!

> > diff --git a/drivers/pinctrl/freescale/Makefile b/drivers/pinctrl/freescale/Makefile
> > index 647dff060477..ceb0f61c6215 100644
> > --- a/drivers/pinctrl/freescale/Makefile
> > +++ b/drivers/pinctrl/freescale/Makefile
> > @@ -33,3 +33,5 @@ obj-$(CONFIG_PINCTRL_IMX25)	+= pinctrl-imx25.o
> >   obj-$(CONFIG_PINCTRL_IMX28)	+= pinctrl-imx28.o
> >   obj-$(CONFIG_PINCTRL_IMXRT1050)	+= pinctrl-imxrt1050.o
> >   obj-$(CONFIG_PINCTRL_IMXRT1170)	+= pinctrl-imxrt1170.o
> > +obj-$(CONFIG_PINCTRL_S32CC)	+= pinctrl-s32cc.o
> > +obj-$(CONFIG_PINCTRL_S32G)	+= pinctrl-s32g.o
> [snip]
> 
> Regards,
> Andreas
> 
> -- 
> SUSE Software Solutions Germany GmbH
> Frankenstraße 146, 90461 Nürnberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nürnberg)