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

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

[Linus Walleij]

On some systems (such as the Qualcomm APQ8060 Dragonboard) the
RESET signal of the SMSC911x is not pulled up by a resistor (or
the internal pull-up that will pull it up if the pin is not
even connected) but instead 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.

Notice that we request the line to go logical low (deassert)
whilst the line on the actual component is active low. This
is managed in the respective hardware description when
specifying the GPIO line with e.g. device tree or ACPI. With
device tree it looks like this in one case:

  reset-gpios = <&tlmm 30 GPIO_ACTIVE_LOW>;

Which means that logically requesting the RESET line to be
deasserted will result in the line being driven high, taking
the device out of reset.

Cc: Jeremy Linton <jeremy.linton@arm.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- Specify in the commit message that the component has an internal
  pull-up that will act if the RESET line is not connected.
- Specify in the commit message that the hardware description
  such as device tree or ACPI is responsible of flagging the line
  as active low.
ChangeLog v1->v2:
- Use devm_gpiod_request_optiona() and request the line with
  GPIOD_OUT_LOW so it is deasserted immediately if active.
---
 drivers/net/ethernet/smsc/smsc911x.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index ca3134540d2d..8ab8d4b9614b 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"
 
@@ -147,6 +148,9 @@ struct smsc911x_data {
 	/* regulators */
 	struct regulator_bulk_data supplies[SMSC911X_NUM_SUPPLIES];
 
+	/* Reset GPIO */
+	struct gpio_desc *reset_gpiod;
+
 	/* clock */
 	struct clk *clk;
 };
@@ -438,6 +442,11 @@ 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_optional(&pdev->dev,
+						     "reset",
+						     GPIOD_OUT_LOW);
+
 	/* Request clock */
 	pdata->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(pdata->clk))
-- 
2.7.4

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

[Jeremy Linton]

Hi,

On 09/07/2016 08:53 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 (or
> the internal pull-up that will pull it up if the pin is not
> even connected) but instead 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.
>
> Notice that we request the line to go logical low (deassert)
> whilst the line on the actual component is active low. This
> is managed in the respective hardware description when
> specifying the GPIO line with e.g. device tree or ACPI. With
> device tree it looks like this in one case:
>
>   reset-gpios = <&tlmm 30 GPIO_ACTIVE_LOW>;
>
> Which means that logically requesting the RESET line to be
> deasserted will result in the line being driven high, taking
> the device out of reset.
>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Thanks for clarifying the GPIO_ACTIVE_LOW state on this pin. I've 
reviewed this patch, and tested that it doesn't have any affect on a 
JunoR2 (ACPI or DT) system. I've got some personal reservations about 
making changes for a single board that might better be handled in 
firmware. But, I don't think those should be an excuse to keep that 
hardware from working. Particularly since this is a fairly trivial change.

So FWIW,

Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>


Thanks,

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

[David Miller]

From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed,  7 Sep 2016 15:53:42 +0200

> On some systems (such as the Qualcomm APQ8060 Dragonboard) the
> RESET signal of the SMSC911x is not pulled up by a resistor (or
> the internal pull-up that will pull it up if the pin is not
> even connected) but instead 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.
> 
> Notice that we request the line to go logical low (deassert)
> whilst the line on the actual component is active low. This
> is managed in the respective hardware description when
> specifying the GPIO line with e.g. device tree or ACPI. With
> device tree it looks like this in one case:
> 
>   reset-gpios = <&tlmm 30 GPIO_ACTIVE_LOW>;
> 
> Which means that logically requesting the RESET line to be
> deasserted will result in the line being driven high, taking
> the device out of reset.
> 
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Applied.