Re: [PATCH 14/20] pinctrl: berlin: support Synaptics SL261X SoCs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] pinctrl: sl261x: incorrect multiplexer register base offset for SM_GPIO20
- [High] pinctrl: sl261x: incorrect configuration register offset for GPIO54
- [Low] pinctrl: sl261x: mismatched comments for uart1 pin functions
--

commit fd69572f1bec3df913befb121d2d8d563a04f10c
Author: Jisheng Zhang <jszhang@kernel.org>

pinctrl: berlin: support Synaptics SL261X SoCs

This commit adds a new pinctrl driver for the Synaptics SL261X SoCs. It
defines the pin configuration groups, register offsets, and multiplexer
functions for both the main SoC and the system manager pin controller blocks.

> diff --git a/drivers/pinctrl/berlin/pinctrl-sl261x.c b/drivers/pinctrl/berlin/pinctrl-sl261x.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/pinctrl/berlin/pinctrl-sl261x.c

[ ... ]

> +	BERLIN_PINCTRLCONF_GROUP("GPIO53", 0x800c, 0x3, 0x00,
> +			0x8878, 10,
> +			BERLIN_PINCTRL_FUNCTION(0x0, "gpio"), /* GPIO53 */
> +			BERLIN_PINCTRL_FUNCTION(0x1, "rgmii2"), /* RD1 */
> +			BERLIN_PINCTRL_FUNCTION(0x6, "spi4"), /* SS3n */
> +			BERLIN_PINCTRL_FUNCTION(0x7, "spi3")), /* SCLK */
> +	BERLIN_PINCTRLCONF_GROUP("GPIO54", 0x800c, 0x3, 0x03,
> +			0x87c, 10,

[Severity: High]
Is the configuration register offset for GPIO54 correct here? 

The surrounding GPIO configurations in sl261x_soc_pinctrl_groups use a 0x8800
base (for example, GPIO53 uses 0x8878 and GPIO55 uses 0x8880). Does this code
incorrectly hardcode 0x87c instead of 0x887c, potentially causing writes to
the wrong hardware address when configuring pins?

[ ... ]

> +	BERLIN_PINCTRLCONF_GROUP("SM_GPIO21", 0xb08, 0x3, 0x0c,
> +			0xc68, 10,
> +			BERLIN_PINCTRL_FUNCTION(0x0, "gpio"), /* SM GPIO21 */
> +			BERLIN_PINCTRL_FUNCTION(0x1, "xspi")), /* SM XSPI DATA2 */
> +	BERLIN_PINCTRLCONF_GROUP("SM_GPIO20", 0x8, 0x3, 0x0f,
> +			0xc6c, 10,

[Severity: High]
Does this code use the wrong multiplexer register base offset for SM_GPIO20?

Surrounding SM_GPIO groups in sl261x_sysmgr_pinctrl_groups (like SM_GPIO21 and
SM_GPIO19) correctly use 0xb08. Using 0x8 here could cause the driver to
write to the wrong hardware address when applying pinmux states.

[ ... ]

> +	BERLIN_PINCTRLCONF_GROUP("SM_GPIO8", 0xb0c, 0x3, 0x0c,
> +			0xc90, 10,
> +			BERLIN_PINCTRL_FUNCTION(0x0, "gpio"), /* SM GPIO8 */
> +			BERLIN_PINCTRL_FUNCTION(0x1, "uart0"), /* SM UART0 TXD */
> +			BERLIN_PINCTRL_FUNCTION(0x2, "can0"), /* SM CAN0 TX */
> +			BERLIN_PINCTRL_FUNCTION(0x3, "sm_clkout"),
> +			BERLIN_PINCTRL_FUNCTION(0x6, "uart1")), /* SM UART0 TXD */

[Severity: Low]
This isn't a bug, but the trailing comment for uart1 incorrectly documents
the pin functionality as SM UART0 TXD. Should this be SM UART1 TXD?

> +	BERLIN_PINCTRLCONF_GROUP("SM_GPIO7", 0xb0c, 0x3, 0x0f,
> +			0xc94, 10,
> +			BERLIN_PINCTRL_FUNCTION(0x0, "gpio"), /* SM GPIO7 */
> +			BERLIN_PINCTRL_FUNCTION(0x1, "uart0"), /* SM UART0 RXD */
> +			BERLIN_PINCTRL_FUNCTION(0x2, "can0"), /* SM CAN0 RX */
> +			BERLIN_PINCTRL_FUNCTION(0x3, "key_row6"),
> +			BERLIN_PINCTRL_FUNCTION(0x4, "gpio_trig"), /* TRIG2 */
> +			BERLIN_PINCTRL_FUNCTION(0x5, "pwm"), /* SM PWM9 */
> +			BERLIN_PINCTRL_FUNCTION(0x6, "uart1")), /* SM UART0 RXD */

[Severity: Low]
This isn't a bug, but there appears to be a similar copy-paste error here.
Should this trailing comment be labeled as SM UART1 RXD?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929061417.20733-1-jszhang@kernel.org?part=14