Re: [PATCH v2] pinctrl: nxp: s32g2: Enable by default on ARCH_S32

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] pinctrl: nxp: s32g2: broken COMPILE_TEST prompt with strict ARCH_S32 dependency
- [Low] pinctrl: nxp: s32g2: missing Signed-off-by and revision notes in commit log
--

commit d48093c09173ce7575fa22c4980bf4ece96a9965
Author: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>

pinctrl: nxp: s32g2: Enable by default on ARCH_S32

This commit defaults PINCTRL_S32G2 to ARCH_S32, restricts its Kconfig prompt
to COMPILE_TEST, and expands its help text.

>     pinctrl: nxp: s32g2: Enable by default on ARCH_S32
>     
>     The SIUL2 pin controller provides the pin muxing used by peripherals on
>     the supported S32G boards: the S32G2-based S32G274A-EVB and
>     S32G274A-RDB2, and the S32G3-based S32G399A-RDB3. Without the driver
>     some peripherals never finish probing and a defconfig kernel cannot
>     boot to rootfs on these boards.
>     
>     As the driver is required by every supported S32G board rather than
>     being optional, make PINCTRL_S32G2 default to ARCH_S32 and keep the
>     user prompt only under COMPILE_TEST.
>     
>     v2 -> v1:
>     - list the S32G boards that need the pinctrl driver
>     in the commit message
>     - drop the v1 arm64 defconfig change
>     - default to ARCH_S32 in Kconfig instead

[Severity: Low]
Should this commit include a Signed-off-by tag from the author, and should the
"v2 -> v1:" changelog notes be moved below the "---" tear line so they are not
recorded in the permanent commit history?

> diff --git a/drivers/pinctrl/nxp/Kconfig b/drivers/pinctrl/nxp/Kconfig
> index abca7ef97003d..d6d97c944b94e 100644
> --- a/drivers/pinctrl/nxp/Kconfig
> +++ b/drivers/pinctrl/nxp/Kconfig
> @@ -8,8 +8,14 @@ config PINCTRL_S32CC
>  	select REGMAP_MMIO
>  
>  config PINCTRL_S32G2
> +	bool "NXP S32G2 pinctrl driver" if COMPILE_TEST
>  	depends on ARCH_S32 && OF
> -	bool "NXP S32G2 pinctrl driver"
> +	default ARCH_S32
>  	select PINCTRL_S32CC

[Severity: Medium]
Does gating the PINCTRL_S32G2 prompt on COMPILE_TEST while keeping the strict
ARCH_S32 dependency work as intended?

In drivers/pinctrl/nxp/Kconfig, both PINCTRL_S32CC and PINCTRL_S32G2 depend on
ARCH_S32 && OF without "|| COMPILE_TEST":

config PINCTRL_S32CC
	bool
	depends on ARCH_S32 && OF
	...

config PINCTRL_S32G2
	bool "NXP S32G2 pinctrl driver" if COMPILE_TEST
	depends on ARCH_S32 && OF
	default ARCH_S32
	select PINCTRL_S32CC

Because the "depends on" expression bounds prompt visibility in Kconfig, when
ARCH_S32=n and COMPILE_TEST=y, PINCTRL_S32G2 still cannot be enabled for
compile-testing.

Conversely, when ARCH_S32=y and COMPILE_TEST=n, the prompt and newly expanded
help text are hidden, and PINCTRL_S32G2 is unconditionally forced to y.
However, ARCH_S32 covers non-S32G platforms in
arch/arm64/boot/dts/freescale/Makefile that do not use the S32G2 SIUL2 pin
controller:

dtb-$(CONFIG_ARCH_S32) += s32g274a-evb.dtb
dtb-$(CONFIG_ARCH_S32) += s32g274a-rdb2.dtb
dtb-$(CONFIG_ARCH_S32) += s32g399a-rdb3.dtb
dtb-$(CONFIG_ARCH_S32) += s32n79-rdb.dtb
dtb-$(CONFIG_ARCH_S32) += s32v234-evb.dtb

Should PINCTRL_S32G2 keep its prompt unconditionally visible so it can be
disabled when building for other ARCH_S32 SoCs like S32N79 or S32V234, and
should PINCTRL_S32CC and PINCTRL_S32G2 depend on "(ARCH_S32 || COMPILE_TEST)
&& OF" if compile-test coverage is desired?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104113905.96582-1-khristineandreea.barbulescu@oss.nxp.com?part=1