Re: [PATCH v1 1/2] arm64: dts: imx93-var-dart-sonata: enable UART wakeup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: dts: imx93-var-dart-sonata: invalid wakeup-source on lpuart1
--

commit 65667fa7cde1bed5fd9129933907d0e5e0428a2f
Author: Stefano Radaelli <stefano.r@variscite.com>

arm64: dts: imx93-var-dart-sonata: enable UART wakeup

This commit adds the wakeup-source property to the lpuart1 node in
imx93-var-dart-sonata.dts to allow the serial console to wake the system from
suspend.

> diff --git a/arch/arm64/boot/dts/freescale/imx93-var-dart-sonata.dts b/arch/arm64/boot/dts/freescale/imx93-var-dart-sonata.dts
> index 1ca3ec8330d07..d368a09dc9167 100644
> --- a/arch/arm64/boot/dts/freescale/imx93-var-dart-sonata.dts
> +++ b/arch/arm64/boot/dts/freescale/imx93-var-dart-sonata.dts
> @@ -385,6 +385,7 @@ can0: can@0 {
>  &lpuart1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_uart1>;
> +	wakeup-source;

[Severity: Medium]
Does adding wakeup-source to &lpuart1 pass dtbs_check or take effect at
runtime?

The lpuart1 node in arch/arm64/boot/dts/freescale/imx91_93_common.dtsi uses
the "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart"
compatibles, which are matched by
Documentation/devicetree/bindings/serial/fsl-lpuart.yaml:

  required:
    - compatible
    - reg
    - interrupts
    - clocks
    - clock-names

  unevaluatedProperties: false

Neither fsl-lpuart.yaml nor the schemas it references (serial.yaml and
rs485.yaml) define wakeup-source, so this causes an unevaluated property
validation failure on serial@44380000.

In addition, neither lpuart_probe() in drivers/tty/serial/fsl_lpuart.c nor
serial_core_add_one_port() in drivers/tty/serial/serial_core.c reads
wakeup-source from the controller device node. Instead,
serial_core_add_one_port() unconditionally marks the child tty device as
wakeup-capable:

  tty_dev = tty_port_register_device_attr_serdev(port, drv->tty_driver,
                  uport->line, uport->dev, &uport->port_dev->dev, port,
                  uport->tty_groups);
  if (!IS_ERR(tty_dev)) {
          device_set_wakeup_capable(tty_dev, 1);
  }

And lpuart_uport_is_active() in drivers/tty/serial/fsl_lpuart.c checks
wakeup state on that child tty_dev (configured via sysfs) rather than the
platform device:

  tty = tty_port_tty_get(port);
  if (tty) {
          tty_dev = tty->dev;
          may_wake = tty_dev && device_may_wakeup(tty_dev);
          tty_kref_put(tty);
  }

Should fsl-lpuart.yaml and the driver be updated to support wakeup-source
before adding this property to the devicetree?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793645417.git.stefano.r@variscite.com?part=1