Re: [PATCH net-next v2 2/2] net: ethernet: oa_tc6: add reset-gpios support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: oa_tc6: probe deferral broken by swallowed devm_gpiod_get_optional error
--

commit 68c336e363623d8a5b0d1022273415e9f9d3b4a9
Author: Alessandro Zini <alessandro.zini@siemens.com>

net: ethernet: oa_tc6: add reset-gpios support

Adds support for an optional reset GPIO in oa_tc6_init(). Asserts the hardware
reset line if the GPIO is specified in the device tree.

> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index 6fcc5f561d560..a697e2cb99cd8 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c

[ ... ]

> @@ -1503,6 +1506,24 @@ struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev,
>  	if (!tc6->spi_data_rx_buf)
>  		return NULL;
>  
> +	tc6->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
> +						  GPIOD_OUT_LOW);
> +	if (IS_ERR(tc6->reset_gpio)) {
> +		dev_err_probe(&spi->dev, PTR_ERR(tc6->reset_gpio),
> +			      "Failed to get reset GPIO\n");
> +		return NULL;

[Severity: High]
Does returning NULL here break probe deferral?

If devm_gpiod_get_optional() returns ERR_PTR(-EPROBE_DEFER) because the GPIO
provider is not yet ready, dev_err_probe() catches and logs it. However, this
function then ignores the error code and unconditionally returns NULL.

Callers of oa_tc6_init() (like lan865x_probe()) treat a NULL return as a
permanent failure and return -ENODEV:

drivers/net/ethernet/microchip/lan865x/lan865x.c:lan865x_probe() {
    ...
	priv->tc6 = oa_tc6_init(spi, netdev, NULL);
	if (!priv->tc6) {
		ret = -ENODEV;
		goto free_netdev;
	}
    ...
}

This converts a retryable -EPROBE_DEFER into a fatal -ENODEV, causing a
permanent initialization failure if the GPIO driver loads after the SPI driver.

Should oa_tc6_init() return an ERR_PTR() instead of NULL on failure, so that
callers can propagate the exact error code?

> +	}
> +
> +	if (tc6->reset_gpio) {
> +		/* Assert hardware reset for 10 us (datasheet specifies min 5 us)
> +		 * and allow 1 ms settle time for crystal oscillator startup.
> +		 */
> +		gpiod_set_value_cansleep(tc6->reset_gpio, 1);
> +		fsleep(10);
> +		gpiod_set_value_cansleep(tc6->reset_gpio, 0);
> +		fsleep(1000);
> +	}
> +
>  	/* Check the PROTE bit status so that we can reset the device */
>  	ret = oa_tc6_check_ctrl_protection(tc6);
>  	if (ret) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916075349.21822-1-alessandro.zini@siemens.com?part=2