Re: [PATCH] pinctrl: spacemit: fix NULL check in spacemit_pin_set_config

From: Yixun Lan <dlan@kernel.org>
To: Linus Walleij <linusw@kernel.org>, Han Gao <gaohan@iscas.ac.cn>
Cc: Hendrik Hamerlinck <hendrik.hamerlinck@hammernet.be>,
	Anand Moon <linux.amoon@gmail.com>,
	Junhui Liu <junhui.liu@pigmoral.tech>,
	Troy Mitchell <troy.mitchell@linux.spacemit.com>,
	linux-gpio@vger.kernel.org, linux-riscv@lists.infradead.org,
	spacemit@lists.linux.dev, linux-kernel@vger.kernel.org,
	Han Gao <rabenda.cn@gmail.com>
Subject: Re: [PATCH] pinctrl: spacemit: fix NULL check in spacemit_pin_set_config
Date: Tue, 19 May 2026 23:21:54 +0000	[thread overview]
Message-ID: <20260519232154-GKA3708537@kernel.org> (raw)
In-Reply-To: <20260519164007.122574-1-gaohan@iscas.ac.cn>

Hi Linus,

Can you take this as a fix for v7.1 cycle?

On 00:40 Wed 20 May     , Han Gao wrote:
> spacemit_pin_set_config() looks up the per-pin descriptor with
> spacemit_get_pin() then checks the wrong variable for failure:
> 
> 	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
> 	...
> 	if (!pin)
> 		return -EINVAL;
> 
> 	reg = spacemit_pin_to_reg(pctrl, spin->pin);
> 
> pin is an unsigned int pin id, where 0 (GPIO_0 / gmac0_rxdv on K3) is a
> valid pin, so rejecting it here drops the PAD config write for the first
> pin of every group. On K3 Pico-ITX the GMAC RGMII group lists pin 0 as
> its first entry, so its drive-strength / bias configuration was silently
> ignored.
> 
> The intended guard is against spacemit_get_pin() returning NULL when the
> pin id isn't in the SoC's pin table. Check spin instead, which both
> restores PAD setup for pin 0 and prevents a NULL deref on spin->pin.
> 
> Fixes: a83c29e1d145 ("pinctrl: spacemit: add support for SpacemiT K1 SoC")
> Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
LGTM, thanks

Reviewed-by: Yixun Lan <dlan@kernel.org>
> ---
>  drivers/pinctrl/spacemit/pinctrl-k1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c b/drivers/pinctrl/spacemit/pinctrl-k1.c
> index b0be62b1c816..95024e2bb5a5 100644
> --- a/drivers/pinctrl/spacemit/pinctrl-k1.c
> +++ b/drivers/pinctrl/spacemit/pinctrl-k1.c
> @@ -795,7 +795,7 @@ static int spacemit_pin_set_config(struct spacemit_pinctrl *pctrl,
>  	void __iomem *reg;
>  	unsigned int mux;
>  
> -	if (!pin)
> +	if (!spin)
>  		return -EINVAL;
>  
>  	reg = spacemit_pin_to_reg(pctrl, spin->pin);
> -- 
> 2.47.3
> 

-- 
Yixun Lan (dlan)