Re: [PATCH 2/3] net: smsc911x: request and deassert optional RESET GPIO

[Guenter Roeck <linux@roeck-us.net>]

On 07/08/2016 02:07 AM, Linus Walleij wrote:
> On some systems (such as the Qualcomm APQ8060 Dragonboard) the
> RESET signal of the SMSC911x is not pulled up by a resistor but
> connected to a GPIO line, so that the operating system must
> explicitly deassert RESET before use.
>
> Support this in the SMSC911x driver so this ethernet connector
> can be used on such targets.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>   drivers/net/ethernet/smsc/smsc911x.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index 8af25563f627..125d58ac22bd 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -62,6 +62,7 @@
>   #include <linux/acpi.h>
>   #include <linux/pm_runtime.h>
>   #include <linux/property.h>
> +#include <linux/gpio/consumer.h>
>
>   #include "smsc911x.h"
>
> @@ -149,6 +150,9 @@ struct smsc911x_data {
>   	/* regulators */
>   	struct regulator_bulk_data supplies[SMSC911X_NUM_SUPPLIES];
>
> +	/* Reset GPIO */
> +	struct gpio_desc *reset_gpiod;
> +
>   	/* clock */
>   	struct clk *clk;
>   };
> @@ -440,6 +444,15 @@ static int smsc911x_request_resources(struct platform_device *pdev)
>   		netdev_err(ndev, "couldn't get regulators %d\n",
>   				ret);
>
> +	/* Request optional RESET GPIO */
> +	pdata->reset_gpiod = devm_gpiod_get(&pdev->dev, "reset",
> +					    GPIOD_OUT_HIGH);
> +	/* Deassert the signal */
> +	if (!IS_ERR(pdata->reset_gpiod)) {

This could return -EPROBE_DEFER, which should probably not be ignored.

It might be better to use devm_gpiod_get_optional(). If you pass GPIOD_OUT_LOW
as parameter to that function, it should take the chip out of reset immediately.
See drivers/i2c/muxes/i2c-mux-pca954x.c for an example with a similar use case.

Guenter

> +		dev_info(&pdev->dev, "release reset\n");
> +		gpiod_set_value(pdata->reset_gpiod, 0);
> +	}
> +
>   	/* Request clock */
>   	pdata->clk = clk_get(&pdev->dev, NULL);
>   	if (IS_ERR(pdata->clk))
>