[PATCH 0/5] serial: 8250: omap: Add am62 wakeup support

[PATCH 0/5] serial: 8250: omap: Add am62 wakeup support

[Markus Schneider-Pargmann]

Hi,

to support wakeup from Partial-IO using serial pins of mcu_uart0 and
wkup_uart0, this series adds a new pinctrl state 'wakeup' in which
specific wakeup flags are set on these pins. Partial-IO is a poweroff
state of the SoC in which only a few pingroups are partly powered.

If the user enabled wakeup from the serial port, the pinctrl state is
selected on shutdown. For another deep sleep state, which is comparable
with suspend to memory, the same pinctrl state is selected on suspend as
well.

The series is based on v6.9-rc1.

This series is part of a bigger topic to support Partial-IO on am62,
am62a and am62p. Partial-IO is a poweroff state in which some pins are
able to wakeup the SoC. In detail MCU m_can and two serial port pins can
trigger the wakeup.

These two other series are relevant for the support of Partial-IO:

 - firmware: ti_sci: Partial-IO support
 - can: m_can: Add am62 wakeup support

A test branch is available that includes all patches required to test
Partial-IO:

https://gitlab.baylibre.com/msp8/linux/-/tree/integration/am62-lp-sk-partialio/v6.9?ref_type=heads

After enabling Wake-on-LAN the system can be powered off and will enter
the Partial-IO state in which it can be woken up by activity on the
specific pins:
    ethtool -s can0 wol p
    ethtool -s can1 wol p
    poweroff

I tested these patches on am62-lp-sk.

Best,
Markus

Markus Schneider-Pargmann (5):
  dt-bindings: serial: 8250_omap: Add wakeup pinctrl state
  serial: 8250: omap: Remove unused wakeups_enabled
  serial: 8250: omap: Set wakeup capable, do not enable
  serial: 8250: omap: Support wakeup pinctrl state
  serial: 8250: omap: Set wakeup pinctrl on suspend

 .../devicetree/bindings/serial/8250_omap.yaml | 16 ++++++
 drivers/tty/serial/8250/8250_omap.c           | 56 ++++++++++++++++++-
 2 files changed, 70 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH 1/5] dt-bindings: serial: 8250_omap: Add wakeup pinctrl state

[Markus Schneider-Pargmann]

Pins associated with the 8250 omap unit can be the source of a wakeup in
deep sleep states. To be able to wakeup, these pins have to be
configured in a special way. To support this configuration add the
default and wakeup pinctrl states.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 .../devicetree/bindings/serial/8250_omap.yaml    | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/8250_omap.yaml b/Documentation/devicetree/bindings/serial/8250_omap.yaml
index 6a7be42da523..402e2ca81017 100644
--- a/Documentation/devicetree/bindings/serial/8250_omap.yaml
+++ b/Documentation/devicetree/bindings/serial/8250_omap.yaml
@@ -77,6 +77,22 @@ properties:
   current-speed: true
   overrun-throttle-ms: true
 
+  pinctrl-0:
+    description: Default pinctrl state
+
+  pinctrl-1:
+    description: Wakeup pinctrl state
+
+  pinctrl-names:
+    description:
+      When present should contain at least "default" describing the default pin
+      states. The second state called "wakeup" describes the pins in their
+      wakeup configuration required to exit sleep states.
+    minItems: 1
+    items:
+      - const: default
+      - const: wakeup
+
 required:
   - compatible
   - reg
-- 
2.43.0

Re: [PATCH 1/5] dt-bindings: serial: 8250_omap: Add wakeup pinctrl state

[Rob Herring (Arm)]

On Thu, 23 May 2024 09:58:15 +0200, Markus Schneider-Pargmann wrote:
> Pins associated with the 8250 omap unit can be the source of a wakeup in
> deep sleep states. To be able to wakeup, these pins have to be
> configured in a special way. To support this configuration add the
> default and wakeup pinctrl states.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  .../devicetree/bindings/serial/8250_omap.yaml    | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH 2/5] serial: 8250: omap: Remove unused wakeups_enabled

[Markus Schneider-Pargmann]

This field seems to be unused for quite some time already. Remove it.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/tty/serial/8250/8250_omap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 66901d93089a..ca456ea23317 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -141,7 +141,6 @@ struct omap8250_priv {
 	atomic_t active;
 	bool is_suspending;
 	int wakeirq;
-	int wakeups_enabled;
 	u32 latency;
 	u32 calc_latency;
 	struct pm_qos_request pm_qos_request;
-- 
2.43.0

[PATCH 3/5] serial: 8250: omap: Set wakeup capable, do not enable

[Markus Schneider-Pargmann]

The driver sets wakeup enable by default. But not all uarts are meant to
be wakeup enabled. Change the default to be wakeup capable but not
enabled. The user can enable wakeup when needed.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/tty/serial/8250/8250_omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index ca456ea23317..5b7508dfb5d8 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1496,7 +1496,7 @@ static int omap8250_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, priv);
 
-	device_init_wakeup(&pdev->dev, true);
+	device_set_wakeup_capable(&pdev->dev, true);
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_use_autosuspend(&pdev->dev);
 
-- 
2.43.0

Re: [PATCH 3/5] serial: 8250: omap: Set wakeup capable, do not enable

[Andy Shevchenko]

On Thu, May 23, 2024 at 09:58:17AM +0200, Markus Schneider-Pargmann wrote:
> The driver sets wakeup enable by default. But not all uarts are meant to

UARTs

> be wakeup enabled. Change the default to be wakeup capable but not
> enabled. The user can enable wakeup when needed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/5] serial: 8250: omap: Set wakeup capable, do not enable

[Kevin Hilman]

Markus Schneider-Pargmann <msp@baylibre.com> writes:

> The driver sets wakeup enable by default. But not all uarts are meant to
> be wakeup enabled. Change the default to be wakeup capable but not
> enabled. The user can enable wakeup when needed.
>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/tty/serial/8250/8250_omap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index ca456ea23317..5b7508dfb5d8 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -1496,7 +1496,7 @@ static int omap8250_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, priv);
>  
> -	device_init_wakeup(&pdev->dev, true);
> +	device_set_wakeup_capable(&pdev->dev, true);
>  	pm_runtime_enable(&pdev->dev);
>  	pm_runtime_use_autosuspend(&pdev->dev);
>  
> -- 
> 2.43.0

Re: [PATCH 3/5] serial: 8250: omap: Set wakeup capable, do not enable

[Kevin Hilman]

Markus Schneider-Pargmann <msp@baylibre.com> writes:

> The driver sets wakeup enable by default. But not all uarts are meant to
> be wakeup enabled. Change the default to be wakeup capable but not
> enabled. The user can enable wakeup when needed.

In addition to the user enabling via sysfs, this driver should also look
for the `wakeup-source` DT property, and enable when that property is set. 

> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

[PATCH 4/5] serial: 8250: omap: Support wakeup pinctrl state

[Markus Schneider-Pargmann]

UART can be used as a wakeup source for am62 from a powered-off SoC
state. To enable wakeup from UART am62 requires a wakeup flag being set
in the pinctrl.

If the device is marked as wakeup enabled, select the 'wakeup' pinctrl
state on sys_off.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/tty/serial/8250/8250_omap.c | 39 +++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 5b7508dfb5d8..617a421a1396 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -27,8 +27,10 @@
 #include <linux/pm_qos.h>
 #include <linux/pm_wakeirq.h>
 #include <linux/dma-mapping.h>
+#include <linux/reboot.h>
 #include <linux/sys_soc.h>
 #include <linux/pm_domain.h>
+#include <linux/pinctrl/consumer.h>
 
 #include "8250.h"
 
@@ -149,6 +151,9 @@ struct omap8250_priv {
 	spinlock_t rx_dma_lock;
 	bool rx_dma_broken;
 	bool throttled;
+
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pinctrl_wakeup;
 };
 
 struct omap8250_dma_params {
@@ -1345,6 +1350,30 @@ static int omap8250_no_handle_irq(struct uart_port *port)
 	return 0;
 }
 
+static int omap8250_select_wakeup_pinctrl(struct device *dev,
+					  struct omap8250_priv *priv)
+{
+	if (IS_ERR_OR_NULL(priv->pinctrl_wakeup))
+		return 0;
+
+	if (!device_may_wakeup(dev))
+		return 0;
+
+	return pinctrl_select_state(priv->pinctrl, priv->pinctrl_wakeup);
+}
+
+static int omap8250_sysoff_handler(struct sys_off_data *data)
+{
+	struct omap8250_priv *priv = dev_get_drvdata(data->dev);
+	int ret;
+
+	ret = omap8250_select_wakeup_pinctrl(data->dev, priv);
+	if (ret)
+		dev_err(data->dev, "Failed to select pinctrl state 'wakeup', continuing poweroff\n");
+
+	return NOTIFY_DONE;
+}
+
 static struct omap8250_dma_params am654_dma = {
 	.rx_size = SZ_2K,
 	.rx_trigger = 1,
@@ -1566,6 +1595,16 @@ static int omap8250_probe(struct platform_device *pdev)
 	priv->line = ret;
 	pm_runtime_mark_last_busy(&pdev->dev);
 	pm_runtime_put_autosuspend(&pdev->dev);
+
+	priv->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (!IS_ERR_OR_NULL(priv->pinctrl))
+		priv->pinctrl_wakeup = pinctrl_lookup_state(priv->pinctrl, "wakeup");
+
+	devm_register_sys_off_handler(&pdev->dev,
+				      SYS_OFF_MODE_POWER_OFF_PREPARE,
+				      SYS_OFF_PRIO_DEFAULT,
+				      omap8250_sysoff_handler, NULL);
+
 	return 0;
 err:
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
-- 
2.43.0

Re: [PATCH 4/5] serial: 8250: omap: Support wakeup pinctrl state

[Andy Shevchenko]

On Thu, May 23, 2024 at 09:58:18AM +0200, Markus Schneider-Pargmann wrote:
> UART can be used as a wakeup source for am62 from a powered-off SoC
> state. To enable wakeup from UART am62 requires a wakeup flag being set
> in the pinctrl.
> 
> If the device is marked as wakeup enabled, select the 'wakeup' pinctrl
> state on sys_off.

...

>  #include <linux/pm_qos.h>
>  #include <linux/pm_wakeirq.h>
>  #include <linux/dma-mapping.h>

> +#include <linux/reboot.h>

See below.

>  #include <linux/sys_soc.h>
>  #include <linux/pm_domain.h>

> +#include <linux/pinctrl/consumer.h>

Can we make some order here and put this before above pm_*.h or even earlier
according to the alphabet?

...

> +	priv->pinctrl = devm_pinctrl_get(&pdev->dev);
> +	if (!IS_ERR_OR_NULL(priv->pinctrl))

Shouldn't we report an error to the user? I assume that NULL is for optional
pin control, otherwise it's an error state which at bare minimum has to be
reported.

> +		priv->pinctrl_wakeup = pinctrl_lookup_state(priv->pinctrl, "wakeup");

> +	devm_register_sys_off_handler(&pdev->dev,

No error check?

> +				      SYS_OFF_MODE_POWER_OFF_PREPARE,
> +				      SYS_OFF_PRIO_DEFAULT,
> +				      omap8250_sysoff_handler, NULL);
> +

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] serial: 8250: omap: Support wakeup pinctrl state

[Kevin Hilman]

Markus Schneider-Pargmann <msp@baylibre.com> writes:

> UART can be used as a wakeup source for am62 from a powered-off SoC

nit: To be a bit more precise, instead of saying UART can be used as a
wakeup source, I think you should say:

Certain UART pins can be used...

> state. To enable wakeup from UART am62 requires a wakeup flag being set
> in the pinctrl.
>
> If the device is marked as wakeup enabled, select the 'wakeup' pinctrl
> state on sys_off.
>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/tty/serial/8250/8250_omap.c | 39 +++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 5b7508dfb5d8..617a421a1396 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -27,8 +27,10 @@
>  #include <linux/pm_qos.h>
>  #include <linux/pm_wakeirq.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/reboot.h>
>  #include <linux/sys_soc.h>
>  #include <linux/pm_domain.h>
> +#include <linux/pinctrl/consumer.h>
>  
>  #include "8250.h"
>  
> @@ -149,6 +151,9 @@ struct omap8250_priv {
>  	spinlock_t rx_dma_lock;
>  	bool rx_dma_broken;
>  	bool throttled;
> +
> +	struct pinctrl *pinctrl;
> +	struct pinctrl_state *pinctrl_wakeup;
>  };
>  
>  struct omap8250_dma_params {
> @@ -1345,6 +1350,30 @@ static int omap8250_no_handle_irq(struct uart_port *port)
>  	return 0;
>  }
>  
> +static int omap8250_select_wakeup_pinctrl(struct device *dev,
> +					  struct omap8250_priv *priv)
> +{
> +	if (IS_ERR_OR_NULL(priv->pinctrl_wakeup))
> +		return 0;
> +
> +	if (!device_may_wakeup(dev))
> +		return 0;
> +
> +	return pinctrl_select_state(priv->pinctrl, priv->pinctrl_wakeup);
> +}
> +
> +static int omap8250_sysoff_handler(struct sys_off_data *data)
> +{
> +	struct omap8250_priv *priv = dev_get_drvdata(data->dev);
> +	int ret;
> +
> +	ret = omap8250_select_wakeup_pinctrl(data->dev, priv);
> +	if (ret)
> +		dev_err(data->dev, "Failed to select pinctrl state 'wakeup', continuing poweroff\n");
> +
> +	return NOTIFY_DONE;
> +}
> +
>  static struct omap8250_dma_params am654_dma = {
>  	.rx_size = SZ_2K,
>  	.rx_trigger = 1,
> @@ -1566,6 +1595,16 @@ static int omap8250_probe(struct platform_device *pdev)
>  	priv->line = ret;
>  	pm_runtime_mark_last_busy(&pdev->dev);
>  	pm_runtime_put_autosuspend(&pdev->dev);
> +
> +	priv->pinctrl = devm_pinctrl_get(&pdev->dev);
> +	if (!IS_ERR_OR_NULL(priv->pinctrl))
> +		priv->pinctrl_wakeup = pinctrl_lookup_state(priv->pinctrl, "wakeup");
> +
> +	devm_register_sys_off_handler(&pdev->dev,
> +				      SYS_OFF_MODE_POWER_OFF_PREPARE,
> +				      SYS_OFF_PRIO_DEFAULT,
> +				      omap8250_sysoff_handler, NULL);
> +
>  	return 0;
>  err:
>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
> -- 
> 2.43.0

[PATCH 5/5] serial: 8250: omap: Set wakeup pinctrl on suspend

[Markus Schneider-Pargmann]

To enable the serial driver and it's pin to be a wakeup source in
suspend to ram states, select the wakeup pinctrl state on suspend and
restore the default pinctrl state on resume.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/tty/serial/8250/8250_omap.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 617a421a1396..61f056c4e78e 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1663,6 +1663,13 @@ static int omap8250_suspend(struct device *dev)
 	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
 	int err = 0;
 
+	err = omap8250_select_wakeup_pinctrl(dev, priv);
+	if (err) {
+		dev_err(dev, "Failed to select wakeup pinctrl, aborting suspend %pe\n",
+			ERR_PTR(err));
+		return err;
+	}
+
 	serial8250_suspend_port(priv->line);
 
 	err = pm_runtime_resume_and_get(dev);
@@ -1696,6 +1703,13 @@ static int omap8250_resume(struct device *dev)
 	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
 	int err;
 
+	err = pinctrl_select_default_state(dev);
+	if (err) {
+		dev_err(dev, "Failed to select default pinctrl state on resume: %pe\n",
+			ERR_PTR(err));
+		return err;
+	}
+
 	if (uart_console(&up->port) && console_suspend_enabled) {
 		if (console_suspend_enabled) {
 			err = pm_runtime_force_resume(dev);
-- 
2.43.0

Re: [PATCH 5/5] serial: 8250: omap: Set wakeup pinctrl on suspend

[Kevin Hilman]

Markus Schneider-Pargmann <msp@baylibre.com> writes:

> To enable the serial driver and it's pin to be a wakeup source in
> suspend to ram states, select the wakeup pinctrl state on suspend and
> restore the default pinctrl state on resume.
>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

but with a minor nit below...

> ---
>  drivers/tty/serial/8250/8250_omap.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 617a421a1396..61f056c4e78e 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -1663,6 +1663,13 @@ static int omap8250_suspend(struct device *dev)
>  	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>  	int err = 0;
>  
> +	err = omap8250_select_wakeup_pinctrl(dev, priv);
> +	if (err) {
> +		dev_err(dev, "Failed to select wakeup pinctrl, aborting suspend %pe\n",
> +			ERR_PTR(err));
> +		return err;
> +	}
> +
>  	serial8250_suspend_port(priv->line);
>  
>  	err = pm_runtime_resume_and_get(dev);
> @@ -1696,6 +1703,13 @@ static int omap8250_resume(struct device *dev)
>  	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>  	int err;
>  
> +	err = pinctrl_select_default_state(dev);
> +	if (err) {
> +		dev_err(dev, "Failed to select default pinctrl state on resume: %pe\n",
> +			ERR_PTR(err));
> +		return err;
> +	}

Shouldn't this bit should be at the end of this resume function?
Otherwise, if this fails, the UART could be left unusable, and if it's
the console UART, you wouldn't get any logs to know why.

>  	if (uart_console(&up->port) && console_suspend_enabled) {
>  		if (console_suspend_enabled) {
>  			err = pm_runtime_force_resume(dev);
> -- 
> 2.43.0

Kevin

Re: [PATCH 0/5] serial: 8250: omap: Add am62 wakeup support

[Francesco Dolcini]

Hello Markus,

On Thu, May 23, 2024 at 09:58:14AM +0200, Markus Schneider-Pargmann wrote:
> Hi,
> 
> to support wakeup from Partial-IO using serial pins of mcu_uart0 and
> wkup_uart0, this series adds a new pinctrl state 'wakeup' in which
> specific wakeup flags are set on these pins. Partial-IO is a poweroff
> state of the SoC in which only a few pingroups are partly powered.
> 
> If the user enabled wakeup from the serial port, the pinctrl state is
> selected on shutdown. For another deep sleep state, which is comparable
> with suspend to memory, the same pinctrl state is selected on suspend as
> well.
> 

...

> 
> After enabling Wake-on-LAN the system can be powered off and will enter
> the Partial-IO state in which it can be woken up by activity on the
> specific pins:
>     ethtool -s can0 wol p
>     ethtool -s can1 wol p
>     poweroff

Confused ... Why the CAN? This is about the UART.

Francesco