All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3 v2] net: smsc911x: augment device tree bindings
@ 2016-08-24 12:59 Linus Walleij
  2016-08-24 12:59 ` [PATCH 2/3 v2] net: smsc911x: request and deassert optional RESET GPIO Linus Walleij
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Linus Walleij @ 2016-08-24 12:59 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA, David S . Miller, Steve Glendinning
  Cc: Guenter Roeck, Jeremy Linton, Kamlakant Patel, Pavel Fedin,
	Linus Walleij, devicetree-u79uwXL29TY76Z2rM5mHXA

This adds device tree bindings for:

- An optional GPIO line for releasing the RESET signal to the
  SMSC911x devices

- An optional PME (power management event) interrupt line that
  can be utilized to wake up the system on network activity.
  This signal exist on all the SMSC911x devices, it is just not
  very often routed.

Both these lines are routed to the SoC on the Qualcomm APQ8060
Dragonboard and thus needs to be bound in the device tree.

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
ChangeLog v1->v2:
- Document for "interrupts", skip mentioning "interrupts-extended"
  as both are always supported.
---
 Documentation/devicetree/bindings/net/smsc911x.txt | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
index 3fed3c124411..11c4a46ba682 100644
--- a/Documentation/devicetree/bindings/net/smsc911x.txt
+++ b/Documentation/devicetree/bindings/net/smsc911x.txt
@@ -3,9 +3,12 @@
 Required properties:
 - compatible : Should be "smsc,lan<model>", "smsc,lan9115"
 - reg : Address and length of the io space for SMSC LAN
-- interrupts : Should contain SMSC LAN interrupt line
-- interrupt-parent : Should be the phandle for the interrupt controller
-  that services interrupts for this device
+- interrupts : Should contain the SMSC LAN
+  interrupt line as cell 0, cell 1 is an OPTIONAL PME (power
+  management event) interrupt that is able to wake up the host
+  system with a 50ms pulse on network activity
+  For generic bindings for interrupt controller parents, refer to
+  interrupt-controller/interrupts.txt
 - phy-mode : See ethernet.txt file in the same directory
 
 Optional properties:
@@ -21,6 +24,10 @@ Optional properties:
   external PHY
 - smsc,save-mac-address : Indicates that mac address needs to be saved
   before resetting the controller
+- reset-gpios : a GPIO line connected to the RESET (active low) signal
+  of the device. On many systems this is wired high so the device goes
+  out of reset at power-on, but if it is under program control, this
+  optional GPIO can wake up in response to it.
 
 Examples:
 
@@ -29,7 +36,8 @@ lan9220@f4000000 {
 	reg = <0xf4000000 0x2000000>;
 	phy-mode = "mii";
 	interrupt-parent = <&gpio1>;
-	interrupts = <31>;
+	interrupts = <31>, <32>;
+	reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
 	reg-io-width = <4>;
 	smsc,irq-push-pull;
 };
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3 v2] net: smsc911x: request and deassert optional RESET GPIO
  2016-08-24 12:59 [PATCH 1/3 v2] net: smsc911x: augment device tree bindings Linus Walleij
@ 2016-08-24 12:59 ` Linus Walleij
  2016-08-31 15:08   ` Jeremy Linton
  2016-08-24 12:59 ` [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support Linus Walleij
  2016-08-24 15:55 ` [PATCH 1/3 v2] net: smsc911x: augment device tree bindings Arnd Bergmann
  2 siblings, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2016-08-24 12:59 UTC (permalink / raw)
  To: netdev, David S . Miller, Steve Glendinning
  Cc: Guenter Roeck, Jeremy Linton, Kamlakant Patel, Pavel Fedin,
	Linus Walleij

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>
---
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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support
  2016-08-24 12:59 [PATCH 1/3 v2] net: smsc911x: augment device tree bindings Linus Walleij
  2016-08-24 12:59 ` [PATCH 2/3 v2] net: smsc911x: request and deassert optional RESET GPIO Linus Walleij
@ 2016-08-24 12:59 ` Linus Walleij
  2016-08-26 14:40   ` Tony Lindgren
                     ` (2 more replies)
  2016-08-24 15:55 ` [PATCH 1/3 v2] net: smsc911x: augment device tree bindings Arnd Bergmann
  2 siblings, 3 replies; 10+ messages in thread
From: Linus Walleij @ 2016-08-24 12:59 UTC (permalink / raw)
  To: netdev, David S . Miller, Steve Glendinning
  Cc: Guenter Roeck, Jeremy Linton, Kamlakant Patel, Pavel Fedin,
	Linus Walleij, Sudeep Holla, Tony Lindgren, Florian Fainelli

The SMSC911x have a line out of the chip called "PME",
Power Management Event. When connected to an asynchronous
interrupt controller this is able to wake the system up
from sleep in response to certain network events.

This is the first attempt to support this in the Linux
driver: the Qualcomm APQ8060 Dragonboard has this line
routed to a GPIO line on the primary SoC padring, and as
such it can be armed as a wakeup interrupt.

The patch is inspired by the wakeup code in the RTC
subsystem.

The code looks for an additional interrupt - apart from the
ordinary device interrupt - and in case that is present,
we register an interrupt handler to respons to this,
and flag the device and this interrupt as a wakeup.

Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Call pm_wakeup_event() in the wakeup IRQ thread to
  account for the wakeup event.
- Drop the enable/disable_irq_wake() calls from suspend/resume:
  this is handled from the irq core when you call
  dev_pm_set_wake_irq() as we do.
---
 drivers/net/ethernet/smsc/smsc911x.c | 42 ++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 8ab8d4b9614b..8fffc1dc2bdd 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -63,6 +63,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
 #include <linux/gpio/consumer.h>
+#include <linux/pm_wakeirq.h>
 
 #include "smsc911x.h"
 
@@ -151,6 +152,9 @@ struct smsc911x_data {
 	/* Reset GPIO */
 	struct gpio_desc *reset_gpiod;
 
+	/* PME interrupt */
+	int pme_irq;
+
 	/* clock */
 	struct clk *clk;
 };
@@ -1881,6 +1885,19 @@ static irqreturn_t smsc911x_irqhandler(int irq, void *dev_id)
 	return serviced;
 }
 
+static irqreturn_t smsc911x_pme_irq_thread(int irq, void *dev_id)
+{
+	struct net_device *dev = dev_id;
+	struct smsc911x_data *pdata __maybe_unused = netdev_priv(dev);
+
+	SMSC_TRACE(pdata, pm, "wakeup event");
+	pm_wakeup_event(&dev->dev, 50);
+	/* This signal is active for 50 ms, wait for it to deassert */
+	usleep_range(50000, 100000);
+
+	return IRQ_HANDLED;
+}
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void smsc911x_poll_controller(struct net_device *dev)
 {
@@ -2501,6 +2518,31 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 		goto out_disable_resources;
 	}
 
+	irq = platform_get_irq(pdev, 1);
+	if (irq == -EPROBE_DEFER) {
+		retval = -EPROBE_DEFER;
+		goto out_disable_resources;
+	/* It's perfectly fine to not have a PME IRQ */
+	} else if (irq > 0) {
+		/*
+		 * The Power Management Event (PME) IRQ appears as
+		 * a pulse waking up the system from sleep in response to  a
+		 * network event.
+		 */
+		retval = request_threaded_irq(irq, NULL,
+					      smsc911x_pme_irq_thread,
+					      IRQF_ONESHOT, "smsc911x-pme",
+					      dev);
+		if (retval) {
+			SMSC_WARN(pdata, probe,
+			"Unable to claim requested PME irq: %d", irq);
+			goto out_disable_resources;
+		}
+		pdata->pme_irq = irq;
+		device_init_wakeup(&pdev->dev, true);
+		dev_pm_set_wake_irq(&pdev->dev, irq);
+	}
+
 	netif_carrier_off(dev);
 
 	retval = register_netdev(dev);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3 v2] net: smsc911x: augment device tree bindings
  2016-08-24 12:59 [PATCH 1/3 v2] net: smsc911x: augment device tree bindings Linus Walleij
  2016-08-24 12:59 ` [PATCH 2/3 v2] net: smsc911x: request and deassert optional RESET GPIO Linus Walleij
  2016-08-24 12:59 ` [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support Linus Walleij
@ 2016-08-24 15:55 ` Arnd Bergmann
  2 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2016-08-24 15:55 UTC (permalink / raw)
  To: Linus Walleij
  Cc: netdev, David S . Miller, Steve Glendinning, Guenter Roeck,
	Jeremy Linton, Kamlakant Patel, Pavel Fedin, devicetree

On Wednesday, August 24, 2016 2:59:40 PM CEST Linus Walleij wrote:
> +- interrupts : Should contain the SMSC LAN
> +  interrupt line as cell 0, cell 1 is an OPTIONAL PME (power
> +  management event) interrupt that is able to wake up the host
> +  system with a 50ms pulse on network activity
> +  For generic bindings for interrupt controller parents, refer to
> +  interrupt-controller/interrupts.txt

I think you should (slightly) reword this to avoid using the
term "cell", which refers to a 32-bit word in the property,
not the interrupt specifier that is often made up of two or
three cells.

Maybe something like

- interrupts: one or two interrupt specifiers:
	- The first interrupt is the SMSC LAN interrupt line.
	- The second interrupt (if present) is the power management
	  event ...

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support
  2016-08-24 12:59 ` [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support Linus Walleij
@ 2016-08-26 14:40   ` Tony Lindgren
  2016-08-31 15:27   ` Jeremy Linton
  2016-08-31 15:32   ` Jeremy Linton
  2 siblings, 0 replies; 10+ messages in thread
From: Tony Lindgren @ 2016-08-26 14:40 UTC (permalink / raw)
  To: Linus Walleij
  Cc: netdev, David S . Miller, Steve Glendinning, Guenter Roeck,
	Jeremy Linton, Kamlakant Patel, Pavel Fedin, Sudeep Holla,
	Florian Fainelli

* Linus Walleij <linus.walleij@linaro.org> [160824 06:00]:
> The SMSC911x have a line out of the chip called "PME",
> Power Management Event. When connected to an asynchronous
> interrupt controller this is able to wake the system up
> from sleep in response to certain network events.
> 
> This is the first attempt to support this in the Linux
> driver: the Qualcomm APQ8060 Dragonboard has this line
> routed to a GPIO line on the primary SoC padring, and as
> such it can be armed as a wakeup interrupt.
> 
> The patch is inspired by the wakeup code in the RTC
> subsystem.
> 
> The code looks for an additional interrupt - apart from the
> ordinary device interrupt - and in case that is present,
> we register an interrupt handler to respons to this,
> and flag the device and this interrupt as a wakeup.
> 
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Call pm_wakeup_event() in the wakeup IRQ thread to
>   account for the wakeup event.
> - Drop the enable/disable_irq_wake() calls from suspend/resume:
>   this is handled from the irq core when you call
>   dev_pm_set_wake_irq() as we do.

Looks OK to me:

Acked-by: Tony Lindgren <tony@atomide.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3 v2] net: smsc911x: request and deassert optional RESET GPIO
  2016-08-24 12:59 ` [PATCH 2/3 v2] net: smsc911x: request and deassert optional RESET GPIO Linus Walleij
@ 2016-08-31 15:08   ` Jeremy Linton
  2016-09-07  9:59     ` Linus Walleij
  0 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2016-08-31 15:08 UTC (permalink / raw)
  To: Linus Walleij, netdev, David S . Miller, Steve Glendinning
  Cc: Guenter Roeck, Kamlakant Patel, Pavel Fedin

Hi Linus,

On 08/24/2016 07:59 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.

Hmm, at least in our hardware case AFAIK (juno/lan9118) the hardware 
reset line on the lan9118 is active low, but the chip itself is 
documented as having internal pullups so that it may be left unconnected.

Which microchip/smsc chip are we talking about? because it seems that 
you probably want the GPIO pin to be in any state (hi-Z, or just high) 
besides the one you selected here.

Beyond that, is it not possible for the firmware to get the reset pin in 
the correct configuration, so that linux doesn't have to mess with it?



>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> 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))
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support
  2016-08-24 12:59 ` [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support Linus Walleij
  2016-08-26 14:40   ` Tony Lindgren
@ 2016-08-31 15:27   ` Jeremy Linton
  2016-09-07 13:51     ` Linus Walleij
  2016-08-31 15:32   ` Jeremy Linton
  2 siblings, 1 reply; 10+ messages in thread
From: Jeremy Linton @ 2016-08-31 15:27 UTC (permalink / raw)
  To: Linus Walleij, netdev, David S . Miller, Steve Glendinning
  Cc: Guenter Roeck, Kamlakant Patel, Pavel Fedin, Sudeep Holla,
	Tony Lindgren, Florian Fainelli

Hi Linus,


On 08/24/2016 07:59 AM, Linus Walleij wrote:
> The SMSC911x have a line out of the chip called "PME",
> Power Management Event. When connected to an asynchronous
> interrupt controller this is able to wake the system up
> from sleep in response to certain network events.
>
> This is the first attempt to support this in the Linux
> driver: the Qualcomm APQ8060 Dragonboard has this line
> routed to a GPIO line on the primary SoC padring, and as
> such it can be armed as a wakeup interrupt.
>
> The patch is inspired by the wakeup code in the RTC
> subsystem.
>
> The code looks for an additional interrupt - apart from the
> ordinary device interrupt - and in case that is present,
> we register an interrupt handler to respons to this,
> and flag the device and this interrupt as a wakeup.
>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Call pm_wakeup_event() in the wakeup IRQ thread to
>   account for the wakeup event.
> - Drop the enable/disable_irq_wake() calls from suspend/resume:
>   this is handled from the irq core when you call
>   dev_pm_set_wake_irq() as we do.
> ---
>  drivers/net/ethernet/smsc/smsc911x.c | 42 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index 8ab8d4b9614b..8fffc1dc2bdd 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -63,6 +63,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/property.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/pm_wakeirq.h>
>
>  #include "smsc911x.h"
>
> @@ -151,6 +152,9 @@ struct smsc911x_data {
>  	/* Reset GPIO */
>  	struct gpio_desc *reset_gpiod;
>
> +	/* PME interrupt */
> +	int pme_irq;
> +
>  	/* clock */
>  	struct clk *clk;
>  };
> @@ -1881,6 +1885,19 @@ static irqreturn_t smsc911x_irqhandler(int irq, void *dev_id)
>  	return serviced;
>  }
>
> +static irqreturn_t smsc911x_pme_irq_thread(int irq, void *dev_id)
> +{
> +	struct net_device *dev = dev_id;
> +	struct smsc911x_data *pdata __maybe_unused = netdev_priv(dev);
> +
> +	SMSC_TRACE(pdata, pm, "wakeup event");
> +	pm_wakeup_event(&dev->dev, 50);
> +	/* This signal is active for 50 ms, wait for it to deassert */
> +	usleep_range(50000, 100000);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  static void smsc911x_poll_controller(struct net_device *dev)
>  {
> @@ -2501,6 +2518,31 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
>  		goto out_disable_resources;
>  	}
>
> +	irq = platform_get_irq(pdev, 1);
> +	if (irq == -EPROBE_DEFER) {
> +		retval = -EPROBE_DEFER;
> +		goto out_disable_resources;
> +	/* It's perfectly fine to not have a PME IRQ */
> +	} else if (irq > 0) {
> +		/*
> +		 * The Power Management Event (PME) IRQ appears as
> +		 * a pulse waking up the system from sleep in response to  a
> +		 * network event.
> +		 */
> +		retval = request_threaded_irq(irq, NULL,
> +					      smsc911x_pme_irq_thread,
> +					      IRQF_ONESHOT, "smsc911x-pme",
> +					      dev);
> +		if (retval) {
> +			SMSC_WARN(pdata, probe,
> +			"Unable to claim requested PME irq: %d", irq);
> +			goto out_disable_resources;
> +		}
> +		pdata->pme_irq = irq;
> +		device_init_wakeup(&pdev->dev, true);
> +		dev_pm_set_wake_irq(&pdev->dev, irq);
> +	}
> +
>  	netif_carrier_off(dev);
>
>  	retval = register_netdev(dev);
>


The cleanup code in drv_remove seems to be missing, am I missing something?

Also, do you want the wake-up to be active if the interface is downed?


Thanks,

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support
  2016-08-24 12:59 ` [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support Linus Walleij
  2016-08-26 14:40   ` Tony Lindgren
  2016-08-31 15:27   ` Jeremy Linton
@ 2016-08-31 15:32   ` Jeremy Linton
  2 siblings, 0 replies; 10+ messages in thread
From: Jeremy Linton @ 2016-08-31 15:32 UTC (permalink / raw)
  To: Linus Walleij, netdev, David S . Miller, Steve Glendinning
  Cc: Guenter Roeck, Kamlakant Patel, Pavel Fedin, Sudeep Holla,
	Tony Lindgren, Florian Fainelli

Hi,

On 08/24/2016 07:59 AM, Linus Walleij wrote:
> The SMSC911x have a line out of the chip called "PME",
> Power Management Event. When connected to an asynchronous
> interrupt controller this is able to wake the system up
> from sleep in response to certain network events.
>
> This is the first attempt to support this in the Linux
> driver: the Qualcomm APQ8060 Dragonboard has this line
> routed to a GPIO line on the primary SoC padring, and as
> such it can be armed as a wakeup interrupt.
>
> The patch is inspired by the wakeup code in the RTC
> subsystem.
>
> The code looks for an additional interrupt - apart from the
> ordinary device interrupt - and in case that is present,
> we register an interrupt handler to respons to this,
> and flag the device and this interrupt as a wakeup.


Having looked at a couple of the supported smsc chips, it seems they can 
route the wakeup through the chip's interrupt as well. If you add code 
to support this, it should work on a lot of the smsc911x devices rather 
than just the dragonboard.


Thanks,




>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Call pm_wakeup_event() in the wakeup IRQ thread to
>   account for the wakeup event.
> - Drop the enable/disable_irq_wake() calls from suspend/resume:
>   this is handled from the irq core when you call
>   dev_pm_set_wake_irq() as we do.
> ---
>  drivers/net/ethernet/smsc/smsc911x.c | 42 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index 8ab8d4b9614b..8fffc1dc2bdd 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -63,6 +63,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/property.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/pm_wakeirq.h>
>
>  #include "smsc911x.h"
>
> @@ -151,6 +152,9 @@ struct smsc911x_data {
>  	/* Reset GPIO */
>  	struct gpio_desc *reset_gpiod;
>
> +	/* PME interrupt */
> +	int pme_irq;
> +
>  	/* clock */
>  	struct clk *clk;
>  };
> @@ -1881,6 +1885,19 @@ static irqreturn_t smsc911x_irqhandler(int irq, void *dev_id)
>  	return serviced;
>  }
>
> +static irqreturn_t smsc911x_pme_irq_thread(int irq, void *dev_id)
> +{
> +	struct net_device *dev = dev_id;
> +	struct smsc911x_data *pdata __maybe_unused = netdev_priv(dev);
> +
> +	SMSC_TRACE(pdata, pm, "wakeup event");
> +	pm_wakeup_event(&dev->dev, 50);
> +	/* This signal is active for 50 ms, wait for it to deassert */
> +	usleep_range(50000, 100000);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  static void smsc911x_poll_controller(struct net_device *dev)
>  {
> @@ -2501,6 +2518,31 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
>  		goto out_disable_resources;
>  	}
>
> +	irq = platform_get_irq(pdev, 1);
> +	if (irq == -EPROBE_DEFER) {
> +		retval = -EPROBE_DEFER;
> +		goto out_disable_resources;
> +	/* It's perfectly fine to not have a PME IRQ */
> +	} else if (irq > 0) {
> +		/*
> +		 * The Power Management Event (PME) IRQ appears as
> +		 * a pulse waking up the system from sleep in response to  a
> +		 * network event.
> +		 */
> +		retval = request_threaded_irq(irq, NULL,
> +					      smsc911x_pme_irq_thread,
> +					      IRQF_ONESHOT, "smsc911x-pme",
> +					      dev);
> +		if (retval) {
> +			SMSC_WARN(pdata, probe,
> +			"Unable to claim requested PME irq: %d", irq);
> +			goto out_disable_resources;
> +		}
> +		pdata->pme_irq = irq;
> +		device_init_wakeup(&pdev->dev, true);
> +		dev_pm_set_wake_irq(&pdev->dev, irq);
> +	}
> +
>  	netif_carrier_off(dev);
>
>  	retval = register_netdev(dev);
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3 v2] net: smsc911x: request and deassert optional RESET GPIO
  2016-08-31 15:08   ` Jeremy Linton
@ 2016-09-07  9:59     ` Linus Walleij
  0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2016-09-07  9:59 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: netdev, David S . Miller, Steve Glendinning, Guenter Roeck,
	Kamlakant Patel, Pavel Fedin

On Wed, Aug 31, 2016 at 5:08 PM, Jeremy Linton <jeremy.linton@arm.com> wrote:
> On 08/24/2016 07:59 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.
>
> Hmm, at least in our hardware case AFAIK (juno/lan9118) the hardware reset
> line on the lan9118 is active low, but the chip itself is documented as
> having internal pullups so that it may be left unconnected.

I guess this is only a comment about the contents of the commit message,
I'll check it up and augment.

This:

+       /* Request optional RESET GPIO */
+       pdata->reset_gpiod = devm_gpiod_get_optional(&pdev->dev,
+                                                    "reset",
+                                                    GPIOD_OUT_LOW);

Is Linux-internal way of saying "deassert the RESET" line, it does
*not* mean "drive it low".

GPIO lines are configured in the device tree for the platform in
question, and what you say about it being active low is true, and
that is why the device tree for the DragonBoard looks like so:

+ reset-gpios = <&tlmm 30 GPIO_ACTIVE_LOW>;

Flagging it active low in the device tree make sure it is driven
high by the statement in the code.

> Beyond that, is it not possible for the firmware to get the reset pin in the
> correct configuration, so that linux doesn't have to mess with it?

That is always possible, and in that case you simply do not specify
the GPIO line for RESET. But this firmware for the APQ8060 DragonBoard
does not do it and Qualcomm is not going to update it, and I need
to deal with it.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support
  2016-08-31 15:27   ` Jeremy Linton
@ 2016-09-07 13:51     ` Linus Walleij
  0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2016-09-07 13:51 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: netdev, David S . Miller, Steve Glendinning, Guenter Roeck,
	Kamlakant Patel, Pavel Fedin, Sudeep Holla, Tony Lindgren,
	Florian Fainelli

On Wed, Aug 31, 2016 at 5:27 PM, Jeremy Linton <jeremy.linton@arm.com> wrote:

> The cleanup code in drv_remove seems to be missing, am I missing something?

No you're right it's just my bad coding. Fixing it and thanks for noticing.

> Also, do you want the wake-up to be active if the interface is downed?

Hm! Good point.

It seems most other drivers call
device_set_wakeup_enable() on the device inside a
foo_set_wol() (wake-on-LAN) from the
.set_wol() callback in struct ethtool_ops.

This is in response to the ethtool calls from userspace.

So we need to implement this too.

I don't know whether that has anything to do with whether
the interface is up or not, it seems orthogonal actually.

I will hold this patch back until I can investigate and test
and just resend patches 1+2 right now. (Bindings should
still be OK to merge I guess, and I think the RESET patch
is sorted out.)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-09-07 13:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-24 12:59 [PATCH 1/3 v2] net: smsc911x: augment device tree bindings Linus Walleij
2016-08-24 12:59 ` [PATCH 2/3 v2] net: smsc911x: request and deassert optional RESET GPIO Linus Walleij
2016-08-31 15:08   ` Jeremy Linton
2016-09-07  9:59     ` Linus Walleij
2016-08-24 12:59 ` [PATCH 3/3 v2] net: smsc911x: add wake-up event interrupt support Linus Walleij
2016-08-26 14:40   ` Tony Lindgren
2016-08-31 15:27   ` Jeremy Linton
2016-09-07 13:51     ` Linus Walleij
2016-08-31 15:32   ` Jeremy Linton
2016-08-24 15:55 ` [PATCH 1/3 v2] net: smsc911x: augment device tree bindings Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.