diff mbox series

serdev: Add support for wakeup-source

Message ID 20250213143430.3893651-1-loic.poulain@linaro.org
State New
Headers show
Series serdev: Add support for wakeup-source | expand

Commit Message

Loic Poulain Feb. 13, 2025, 2:34 p.m. UTC
This brings support for dedicated interrupt as wakeup source into the
serdev core, similarly to the I2C bus, and aligned with the documentation:
Documentation/devicetree/bindings/power/wakeup-source.txt

As an example, this can be used for bluetooth serial devices with dedicated
controller-to-host wakeup pin.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/tty/serdev/core.c | 48 +++++++++++++++++++++++++++++++++++++--
 include/linux/serdev.h    |  1 +
 2 files changed, 47 insertions(+), 2 deletions(-)

Comments

Neeraj Sanjay Kale March 21, 2025, 1:24 p.m. UTC | #1
Hi Loic,

Thank you for the patch.

I have verified this patch with btnxpuart driver and I am able to wakeup the host with a GPIO interrupt signal from BT controller.

Tested-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>

Thanks,
Neeraj

On Thu, Feb 13, 2025 at 03:34:30PM +0100, Loic Poulain wrote:
> This brings support for dedicated interrupt as wakeup source into the
> serdev core, similarly to the I2C bus, and aligned with the documentation:
> Documentation/devicetree/bindings/power/wakeup-source.txt
> 
> As an example, this can be used for bluetooth serial devices with dedicated
> controller-to-host wakeup pin.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  drivers/tty/serdev/core.c | 48 +++++++++++++++++++++++++++++++++++++--
>  include/linux/serdev.h    |  1 +
>  2 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index ebf0bbc2cff2..2d016fa546ca 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -13,8 +13,10 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/of_irq.h>
>  #include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pm_wakeirq.h>
>  #include <linux/property.h>
>  #include <linux/sched.h>
>  #include <linux/serdev.h>
> @@ -164,6 +166,9 @@ int serdev_device_open(struct serdev_device *serdev)
>  		goto err_close;
>  	}
>  
> +	if (serdev->wakeup_source)
> +		device_wakeup_enable(&serdev->dev);
> +
>  	return 0;
>  
>  err_close:
> @@ -181,6 +186,9 @@ void serdev_device_close(struct serdev_device *serdev)
>  	if (!ctrl || !ctrl->ops->close)
>  		return;
>  
> +	if (serdev->wakeup_source)
> +		device_wakeup_disable(&serdev->dev);
> +
>  	pm_runtime_put(&ctrl->dev);
>  
>  	ctrl->ops->close(ctrl);
> @@ -406,18 +414,52 @@ int serdev_device_break_ctl(struct serdev_device *serdev, int break_state)
>  }
>  EXPORT_SYMBOL_GPL(serdev_device_break_ctl);
>  
> +static int serdev_wakeup_attach(struct device *dev)
> +{
> +	int wakeirq;
> +
> +	if (!of_property_read_bool(dev->of_node, "wakeup-source"))
> +		return 0;
> +
> +	to_serdev_device(dev)->wakeup_source = true;
> +
> +	device_set_wakeup_capable(dev, true);
> +
> +	wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
> +	if (wakeirq == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +	else if (wakeirq > 0)
> +		return dev_pm_set_dedicated_wake_irq(dev, wakeirq);
> +
> +	return 0;
> +}
> +
> +static void serdev_wakeup_detach(struct device *dev)
> +{
> +	device_init_wakeup(dev, false);
> +	dev_pm_clear_wake_irq(dev);
> +}
> +
>  static int serdev_drv_probe(struct device *dev)
>  {
>  	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
>  	int ret;
>  
> -	ret = dev_pm_domain_attach(dev, true);
> +	ret = serdev_wakeup_attach(dev);
>  	if (ret)
>  		return ret;
>  
> +	ret = dev_pm_domain_attach(dev, true);
> +	if (ret) {
> +		serdev_wakeup_detach(dev);
> +		return ret;
> +	}
> +
>  	ret = sdrv->probe(to_serdev_device(dev));
> -	if (ret)
> +	if (ret) {
>  		dev_pm_domain_detach(dev, true);
> +		serdev_wakeup_detach(dev);
> +	}
>  
>  	return ret;
>  }
> @@ -429,6 +471,8 @@ static void serdev_drv_remove(struct device *dev)
>  		sdrv->remove(to_serdev_device(dev));
>  
>  	dev_pm_domain_detach(dev, true);
> +
> +	serdev_wakeup_detach(dev);
>  }
>  
>  static const struct bus_type serdev_bus_type = {
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index ff78efc1f60d..2b3ee7b2c141 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -47,6 +47,7 @@ struct serdev_device {
>  	const struct serdev_device_ops *ops;
>  	struct completion write_comp;
>  	struct mutex write_lock;
> +	bool wakeup_source;
>  };
>  
>  static inline struct serdev_device *to_serdev_device(struct device *d)
> -- 
> 2.34.1
>
Neeraj Sanjay Kale April 7, 2025, 1:29 p.m. UTC | #2
Hello maintainers,

This patch is in review for quite some time, and it's an important feature which we would like to have support for, in the NXP BT UART driver.

 If there are no further review comments, can you please help approve this?

Thanks,
Neeraj

> Hi Loic,
> 
> Thank you for the patch.
> 
> I have verified this patch with btnxpuart driver and I am able to wakeup the
> host with a GPIO interrupt signal from BT controller.
> 
> Tested-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> 
> Thanks,
> Neeraj
> 
> On Thu, Feb 13, 2025 at 03:34:30PM +0100, Loic Poulain wrote:
> > This brings support for dedicated interrupt as wakeup source into the
> > serdev core, similarly to the I2C bus, and aligned with the documentation:
> > Documentation/devicetree/bindings/power/wakeup-source.txt
> >
> > As an example, this can be used for bluetooth serial devices with
> > dedicated controller-to-host wakeup pin.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > ---
> >  drivers/tty/serdev/core.c | 48
> +++++++++++++++++++++++++++++++++++++--
> >  include/linux/serdev.h    |  1 +
> >  2 files changed, 47 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index ebf0bbc2cff2..2d016fa546ca 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -13,8 +13,10 @@
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> > +#include <linux/of_irq.h>
> >  #include <linux/pm_domain.h>
> >  #include <linux/pm_runtime.h>
> > +#include <linux/pm_wakeirq.h>
> >  #include <linux/property.h>
> >  #include <linux/sched.h>
> >  #include <linux/serdev.h>
> > @@ -164,6 +166,9 @@ int serdev_device_open(struct serdev_device
> *serdev)
> >  		goto err_close;
> >  	}
> >
> > +	if (serdev->wakeup_source)
> > +		device_wakeup_enable(&serdev->dev);
> > +
> >  	return 0;
> >
> >  err_close:
> > @@ -181,6 +186,9 @@ void serdev_device_close(struct serdev_device
> *serdev)
> >  	if (!ctrl || !ctrl->ops->close)
> >  		return;
> >
> > +	if (serdev->wakeup_source)
> > +		device_wakeup_disable(&serdev->dev);
> > +
> >  	pm_runtime_put(&ctrl->dev);
> >
> >  	ctrl->ops->close(ctrl);
> > @@ -406,18 +414,52 @@ int serdev_device_break_ctl(struct serdev_device
> > *serdev, int break_state)  }
> > EXPORT_SYMBOL_GPL(serdev_device_break_ctl);
> >
> > +static int serdev_wakeup_attach(struct device *dev) {
> > +	int wakeirq;
> > +
> > +	if (!of_property_read_bool(dev->of_node, "wakeup-source"))
> > +		return 0;
> > +
> > +	to_serdev_device(dev)->wakeup_source = true;
> > +
> > +	device_set_wakeup_capable(dev, true);
> > +
> > +	wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
> > +	if (wakeirq == -EPROBE_DEFER)
> > +		return -EPROBE_DEFER;
> > +	else if (wakeirq > 0)
> > +		return dev_pm_set_dedicated_wake_irq(dev, wakeirq);
> > +
> > +	return 0;
> > +}
> > +
> > +static void serdev_wakeup_detach(struct device *dev) {
> > +	device_init_wakeup(dev, false);
> > +	dev_pm_clear_wake_irq(dev);
> > +}
> > +
> >  static int serdev_drv_probe(struct device *dev)  {
> >  	const struct serdev_device_driver *sdrv =
> to_serdev_device_driver(dev->driver);
> >  	int ret;
> >
> > -	ret = dev_pm_domain_attach(dev, true);
> > +	ret = serdev_wakeup_attach(dev);
> >  	if (ret)
> >  		return ret;
> >
> > +	ret = dev_pm_domain_attach(dev, true);
> > +	if (ret) {
> > +		serdev_wakeup_detach(dev);
> > +		return ret;
> > +	}
> > +
> >  	ret = sdrv->probe(to_serdev_device(dev));
> > -	if (ret)
> > +	if (ret) {
> >  		dev_pm_domain_detach(dev, true);
> > +		serdev_wakeup_detach(dev);
> > +	}
> >
> >  	return ret;
> >  }
> > @@ -429,6 +471,8 @@ static void serdev_drv_remove(struct device *dev)
> >  		sdrv->remove(to_serdev_device(dev));
> >
> >  	dev_pm_domain_detach(dev, true);
> > +
> > +	serdev_wakeup_detach(dev);
> >  }
> >
> >  static const struct bus_type serdev_bus_type = { diff --git
> > a/include/linux/serdev.h b/include/linux/serdev.h index
> > ff78efc1f60d..2b3ee7b2c141 100644
> > --- a/include/linux/serdev.h
> > +++ b/include/linux/serdev.h
> > @@ -47,6 +47,7 @@ struct serdev_device {
> >  	const struct serdev_device_ops *ops;
> >  	struct completion write_comp;
> >  	struct mutex write_lock;
> > +	bool wakeup_source;
> >  };
> >
> >  static inline struct serdev_device *to_serdev_device(struct device
> > *d)
> > --
> > 2.34.1
> >
Rob Herring (Arm) April 8, 2025, 1:09 p.m. UTC | #3
On Thu, Feb 13, 2025 at 8:34 AM Loic Poulain <loic.poulain@linaro.org> wrote:
>
> This brings support for dedicated interrupt as wakeup source into the
> serdev core, similarly to the I2C bus, and aligned with the documentation:
> Documentation/devicetree/bindings/power/wakeup-source.txt

Don't add new references to old documentation.

The difference I think is I2C has some defined mechanisms for wakeup.
We don't have anything in platform devices, SPI, etc., so why serdev?
A "wakeup" IRQ is also a suggestion more than a hard requirement of
how wakeup is implemented.

Rob
diff mbox series

Patch

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index ebf0bbc2cff2..2d016fa546ca 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -13,8 +13,10 @@ 
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_irq.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/property.h>
 #include <linux/sched.h>
 #include <linux/serdev.h>
@@ -164,6 +166,9 @@  int serdev_device_open(struct serdev_device *serdev)
 		goto err_close;
 	}
 
+	if (serdev->wakeup_source)
+		device_wakeup_enable(&serdev->dev);
+
 	return 0;
 
 err_close:
@@ -181,6 +186,9 @@  void serdev_device_close(struct serdev_device *serdev)
 	if (!ctrl || !ctrl->ops->close)
 		return;
 
+	if (serdev->wakeup_source)
+		device_wakeup_disable(&serdev->dev);
+
 	pm_runtime_put(&ctrl->dev);
 
 	ctrl->ops->close(ctrl);
@@ -406,18 +414,52 @@  int serdev_device_break_ctl(struct serdev_device *serdev, int break_state)
 }
 EXPORT_SYMBOL_GPL(serdev_device_break_ctl);
 
+static int serdev_wakeup_attach(struct device *dev)
+{
+	int wakeirq;
+
+	if (!of_property_read_bool(dev->of_node, "wakeup-source"))
+		return 0;
+
+	to_serdev_device(dev)->wakeup_source = true;
+
+	device_set_wakeup_capable(dev, true);
+
+	wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
+	if (wakeirq == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+	else if (wakeirq > 0)
+		return dev_pm_set_dedicated_wake_irq(dev, wakeirq);
+
+	return 0;
+}
+
+static void serdev_wakeup_detach(struct device *dev)
+{
+	device_init_wakeup(dev, false);
+	dev_pm_clear_wake_irq(dev);
+}
+
 static int serdev_drv_probe(struct device *dev)
 {
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
 	int ret;
 
-	ret = dev_pm_domain_attach(dev, true);
+	ret = serdev_wakeup_attach(dev);
 	if (ret)
 		return ret;
 
+	ret = dev_pm_domain_attach(dev, true);
+	if (ret) {
+		serdev_wakeup_detach(dev);
+		return ret;
+	}
+
 	ret = sdrv->probe(to_serdev_device(dev));
-	if (ret)
+	if (ret) {
 		dev_pm_domain_detach(dev, true);
+		serdev_wakeup_detach(dev);
+	}
 
 	return ret;
 }
@@ -429,6 +471,8 @@  static void serdev_drv_remove(struct device *dev)
 		sdrv->remove(to_serdev_device(dev));
 
 	dev_pm_domain_detach(dev, true);
+
+	serdev_wakeup_detach(dev);
 }
 
 static const struct bus_type serdev_bus_type = {
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index ff78efc1f60d..2b3ee7b2c141 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -47,6 +47,7 @@  struct serdev_device {
 	const struct serdev_device_ops *ops;
 	struct completion write_comp;
 	struct mutex write_lock;
+	bool wakeup_source;
 };
 
 static inline struct serdev_device *to_serdev_device(struct device *d)