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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <577FB037.4070901@roeck-us.net>
Date:	Fri, 8 Jul 2016 06:52:55 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Linus Walleij <linus.walleij@...aro.org>, netdev@...r.kernel.org,
	"David S . Miller" <davem@...emloft.net>,
	Steve Glendinning <steve.glendinning@...c.com>
Cc:	Jeremy Linton <jeremy.linton@....com>,
	Kamlakant Patel <kamlakant.patel@...adcom.com>,
	Pavel Fedin <p.fedin@...sung.com>
Subject: Re: [PATCH 2/3] net: smsc911x: request and deassert optional RESET
 GPIO

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@...aro.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))
>