[PATCH 1/3] net: smsc911x: augment device tree bindings

[PATCH 1/3] net: smsc911x: augment device tree bindings

[Linus Walleij]

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@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 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..7b01c37272c1 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/extended-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

[PATCH 2/3] 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 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)) {
+		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))
-- 
2.7.4

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

[Guenter Roeck]

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

[PATCH 3/3] RFC: net: smsc911x: add wake-up event interrupt support

[Linus Walleij]

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: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Added some wakeup people at CC who can (hopefully) tell me if
I'm doing this right or not.
---
 drivers/net/ethernet/smsc/smsc911x.c | 48 ++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 125d58ac22bd..e43755ae130a 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"
 
@@ -153,6 +154,9 @@ struct smsc911x_data {
 	/* Reset GPIO */
 	struct gpio_desc *reset_gpiod;
 
+	/* PME interrupt */
+	int pme_irq;
+
 	/* clock */
 	struct clk *clk;
 };
@@ -1882,6 +1886,17 @@ 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");
+	/* 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)
 {
@@ -2525,6 +2540,33 @@ 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) {
+		char *pme_name;
+
+		/*
+		 * 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);
@@ -2613,6 +2655,9 @@ static int smsc911x_suspend(struct device *dev)
 		PMT_CTRL_PM_MODE_D1_ | PMT_CTRL_WOL_EN_ |
 		PMT_CTRL_ED_EN_ | PMT_CTRL_PME_EN_);
 
+	if (pdata->pme_irq && device_may_wakeup(dev))
+		enable_irq_wake(pdata->pme_irq);
+
 	return 0;
 }
 
@@ -2622,6 +2667,9 @@ static int smsc911x_resume(struct device *dev)
 	struct smsc911x_data *pdata = netdev_priv(ndev);
 	unsigned int to = 100;
 
+	if (pdata->pme_irq && device_may_wakeup(dev))
+		disable_irq_wake(pdata->pme_irq);
+
 	/* Note 3.11 from the datasheet:
 	 * 	"When the LAN9220 is in a power saving state, a write of any
 	 * 	 data to the BYTE_TEST register will wake-up the device."
-- 
2.7.4

Re: [PATCH 3/3] RFC: net: smsc911x: add wake-up event interrupt support

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [160708 02:10]:
> 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.

Cool, so far have not found any boards here with that
connected. Should be possible to solder that on at least
omap3-evm at some point though.

> +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");
> +	/* This signal is active for 50 ms, wait for it to deassert */
> +	usleep_range(50000, 100000);
> +	return IRQ_HANDLED;
> +}

This probably adds an extra 50 ms latency before we wake up
the smsc911x driver. Maybe we can eventually come up with
some nice solution to that too. Anyways, no objections on
my side for this patch as it provides a nice GPIO wakeirq test
case that can be automated with just ping -c 1 :)

Regards,

Tony

Re: [PATCH 3/3] RFC: net: smsc911x: add wake-up event interrupt support

[Sudeep Holla]

On 08/07/16 10:07, 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: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Added some wakeup people at CC who can (hopefully) tell me if
> I'm doing this right or not.
> ---
>   drivers/net/ethernet/smsc/smsc911x.c | 48 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)
>
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index 125d58ac22bd..e43755ae130a 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c

[...]

> @@ -2525,6 +2540,33 @@ 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) {
> +		char *pme_name;
> +
> +		/*
> +		 * 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);
> @@ -2613,6 +2655,9 @@ static int smsc911x_suspend(struct device *dev)
>   		PMT_CTRL_PM_MODE_D1_ | PMT_CTRL_WOL_EN_ |
>   		PMT_CTRL_ED_EN_ | PMT_CTRL_PME_EN_);
>
> +	if (pdata->pme_irq && device_may_wakeup(dev))
> +		enable_irq_wake(pdata->pme_irq);
> +

Based on my understanding of dev_pm_{set,clear}_wake_irq, we don't need
this and the below hunk. It's taken care in device_wakeup_arm_wake_irqs
which is called from suspend_enter---->dpm_suspend_noirq path once you
have done the setup with dev_pm_set_wake_irq and is enabled both of
which is true in your case.

>   	return 0;
>   }
>
> @@ -2622,6 +2667,9 @@ static int smsc911x_resume(struct device *dev)
>   	struct smsc911x_data *pdata = netdev_priv(ndev);
>   	unsigned int to = 100;
>
> +	if (pdata->pme_irq && device_may_wakeup(dev))
> +		disable_irq_wake(pdata->pme_irq);
> +

Same as above.

-- 
Regards,
Sudeep

Re: [PATCH 3/3] RFC: net: smsc911x: add wake-up event interrupt support

[Florian Fainelli]

On 07/08/2016 02:07 AM, Linus Walleij wrote:
> +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");
> +	/* This signal is active for 50 ms, wait for it to deassert */
> +	usleep_range(50000, 100000);

Should not you have a call to pm_wakeup_event() such that this probably
gets accounted for as a wake-up event in /sys/*?
-- 
Florian

Re: [PATCH 1/3] net: smsc911x: augment device tree bindings

[Rob Herring]

On Fri, Jul 08, 2016 at 11:07:30AM +0200, Linus Walleij wrote:
> 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@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  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..7b01c37272c1 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/extended-interrupts : Should contain the SMSC LAN

It's interrupts-extended. Documentation-wise both are always supported, 
so we just document 'interrupts' unless interrupts-extended is only ever 
valid which would not be the case here.

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