Re: [PATCH v2] pinctrl: mcp23s08: Propagate reset GPIO errors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pinctrl: mcp23s08: multi-device probe failure due to duplicate reset GPIO request
--

commit 95196894afec909a218e9bd99fc8884c93869fc7
Author: bui duc phuc <phucduc.bui@gmail.com>

pinctrl: mcp23s08: Propagate reset GPIO errors

This commit adds a check for the return value of devm_gpiod_get_optional() in
mcp23s08_probe_one() to propagate errors instead of ignoring them.

> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index cc668f84bf8bc..259ac02134c64 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -613,6 +613,9 @@ int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>  	mcp->chip.owner = THIS_MODULE;
>  
>  	mcp->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(mcp->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(mcp->reset_gpio),
> +				     "failed to get reset GPIO\n");

[Severity: High]
Does this cause a regression for multi-chip SPI configurations by aborting the
probe due to -EBUSY?

In the SPI driver, multiple physical chips can share a single SPI chip select.
This is handled in drivers/pinctrl/pinctrl-mcp23s08_spi.c:mcp23s08_probe() by
iterating over spi_present_mask and calling mcp23s08_probe_one() for each:

drivers/pinctrl/pinctrl-mcp23s08_spi.c:mcp23s08_probe() {
	...
	for_each_set_bit(addr, &spi_present_mask, MCP_MAX_DEV_PER_CS) {
		...
		ret = mcp23s08_probe_one(data->mcp[addr], dev, MCP23S08_SPI_BASE | (addr << 1),
					 info->type, -1);
		if (ret < 0)
			return ret;
	}
	...
}

Because all logical chips share the same SPI device, the same struct device
*dev pointer is passed to every mcp23s08_probe_one() call.

During the first iteration, the reset GPIO is successfully requested
exclusively.

During the second iteration, devm_gpiod_get_optional() is called again on the
same dev pointer for the same "reset" connection ID. The GPIO core will
attempt to request the same descriptor again, which will return -EBUSY because
it is already requested and does not use GPIOD_FLAGS_BIT_NONEXCLUSIVE.

Prior to this patch, the -EBUSY error was ignored, allowing the second and
subsequent chips to probe successfully (which was safe since the reset line
was already asserted and deasserted by the first chip).

>  
>  	/* verify MCP_IOCON.SEQOP = 0, so sequential reads work,
>  	 * and MCP_IOCON.HAEN = 1, so we work with all chips.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012015433.6155-1-phucduc.bui@gmail.com?part=1