| Message ID | 20240523075819.1285554-1-msp@baylibre.com |
|---|---|
| Headers | show |
| Series | serial: 8250: omap: Add am62 wakeup support | expand |
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
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.
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); > +
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
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
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>
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
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(-)