[PATCH v2 1/2] can: m_can: allow keeping the transceiver running in suspend

[PATCH v2 1/2] can: m_can: allow keeping the transceiver running in suspend

[Martin Hundebøll]

Add a flag to the suspend class function that leaves the chip in a
running state with rx interrupt enabled, so that m_can device driver can
configure and use the interrupt as a wakeup source.

Signed-off-by: Martin Hundebøll <martin@geanix.com>
---

Changes since v1:
 * Fixed comment formatting
 * Updated m_can_class_{suspend,resume} calls in m_can_pci.c too
 * Skipped calling m_can_start() when resuming a wake-source device

 drivers/net/can/m_can/m_can.c          | 26 +++++++++++++++++++-------
 drivers/net/can/m_can/m_can.h          |  4 ++--
 drivers/net/can/m_can/m_can_pci.c      |  4 ++--
 drivers/net/can/m_can/m_can_platform.c |  4 ++--
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 16ecc11c7f62..ec345f6fd6b0 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2113,7 +2113,7 @@ void m_can_class_unregister(struct m_can_classdev *cdev)
 }
 EXPORT_SYMBOL_GPL(m_can_class_unregister);
 
-int m_can_class_suspend(struct device *dev)
+int m_can_class_suspend(struct device *dev, bool is_wake_source)
 {
 	struct m_can_classdev *cdev = dev_get_drvdata(dev);
 	struct net_device *ndev = cdev->net;
@@ -2121,7 +2121,15 @@ int m_can_class_suspend(struct device *dev)
 	if (netif_running(ndev)) {
 		netif_stop_queue(ndev);
 		netif_device_detach(ndev);
-		m_can_stop(ndev);
+
+		/* leave the chip running with rx interrupt enabled if it used
+		 * as a wake-up source.
+		 */
+		if (is_wake_source)
+			m_can_write(cdev, M_CAN_IE, IR_RF0N);
+		else
+			m_can_stop(ndev);
+
 		m_can_clk_stop(cdev);
 	}
 
@@ -2133,7 +2141,7 @@ int m_can_class_suspend(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(m_can_class_suspend);
 
-int m_can_class_resume(struct device *dev)
+int m_can_class_resume(struct device *dev, bool is_wake_source)
 {
 	struct m_can_classdev *cdev = dev_get_drvdata(dev);
 	struct net_device *ndev = cdev->net;
@@ -2148,11 +2156,15 @@ int m_can_class_resume(struct device *dev)
 		ret = m_can_clk_start(cdev);
 		if (ret)
 			return ret;
-		ret  = m_can_start(ndev);
-		if (ret) {
-			m_can_clk_stop(cdev);
 
-			return ret;
+		if (is_wake_source) {
+			m_can_write(cdev, M_CAN_IE, cdev->active_interrupts);
+		} else {
+			ret  = m_can_start(ndev);
+			if (ret) {
+				m_can_clk_stop(cdev);
+				return ret;
+			}
 		}
 
 		netif_device_attach(ndev);
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 520e14277dff..5d86bcf66e83 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -105,6 +105,6 @@ int m_can_class_get_clocks(struct m_can_classdev *cdev);
 int m_can_init_ram(struct m_can_classdev *priv);
 int m_can_check_mram_cfg(struct m_can_classdev *cdev, u32 mram_max_size);
 
-int m_can_class_suspend(struct device *dev);
-int m_can_class_resume(struct device *dev);
+int m_can_class_suspend(struct device *dev, bool is_wake_source);
+int m_can_class_resume(struct device *dev, bool is_wake_source);
 #endif	/* _CAN_M_H_ */
diff --git a/drivers/net/can/m_can/m_can_pci.c b/drivers/net/can/m_can/m_can_pci.c
index f2219aa2824b..7ffbe06d96d0 100644
--- a/drivers/net/can/m_can/m_can_pci.c
+++ b/drivers/net/can/m_can/m_can_pci.c
@@ -169,12 +169,12 @@ static void m_can_pci_remove(struct pci_dev *pci)
 
 static __maybe_unused int m_can_pci_suspend(struct device *dev)
 {
-	return m_can_class_suspend(dev);
+	return m_can_class_suspend(dev, false);
 }
 
 static __maybe_unused int m_can_pci_resume(struct device *dev)
 {
-	return m_can_class_resume(dev);
+	return m_can_class_resume(dev, false);
 }
 
 static SIMPLE_DEV_PM_OPS(m_can_pci_pm_ops,
diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
index cdb28d6a092c..e49bef50d52c 100644
--- a/drivers/net/can/m_can/m_can_platform.c
+++ b/drivers/net/can/m_can/m_can_platform.c
@@ -169,12 +169,12 @@ static int m_can_plat_probe(struct platform_device *pdev)
 
 static __maybe_unused int m_can_suspend(struct device *dev)
 {
-	return m_can_class_suspend(dev);
+	return m_can_class_suspend(dev, false);
 }
 
 static __maybe_unused int m_can_resume(struct device *dev)
 {
-	return m_can_class_resume(dev);
+	return m_can_class_resume(dev, false);
 }
 
 static void m_can_plat_remove(struct platform_device *pdev)
-- 
2.42.0

[PATCH v2 2/2] can: tcan4x5x: support resuming from rx interrupt signal

[Martin Hundebøll]

Implement the "wakeup-source" device tree property, so the chip is left
running when suspending, and its rx interrupt is used as a wakeup source
to resume operation.

Signed-off-by: Martin Hundebøll <martin@geanix.com>
---

Change since v1:
 * Added `static` keyword to dev_pm_ops sturcture

 drivers/net/can/m_can/tcan4x5x-core.c | 34 ++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index 8a4143809d33..ccb279af22bb 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -459,6 +459,9 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
 		goto out_power;
 	}
 
+	if (device_property_read_bool(&spi->dev, "wakeup-source"))
+		device_init_wakeup(&spi->dev, true);
+
 	ret = m_can_class_register(mcan_class);
 	if (ret) {
 		dev_err(&spi->dev, "Failed registering m_can device %pe\n",
@@ -487,6 +490,31 @@ static void tcan4x5x_can_remove(struct spi_device *spi)
 	m_can_class_free_dev(priv->cdev.net);
 }
 
+static int __maybe_unused tcan4x5x_suspend(struct device *dev)
+{
+	struct spi_device *spi = to_spi_device(dev);
+
+	if (device_may_wakeup(dev)) {
+		enable_irq_wake(spi->irq);
+
+		return m_can_class_suspend(dev, true);
+	}
+
+	return m_can_class_suspend(dev, false);
+}
+
+static int __maybe_unused tcan4x5x_resume(struct device *dev)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	bool wake_source = device_may_wakeup(dev);
+	int ret = m_can_class_resume(dev, wake_source);
+
+	if (wake_source)
+		disable_irq_wake(spi->irq);
+
+	return ret;
+}
+
 static const struct of_device_id tcan4x5x_of_match[] = {
 	{
 		.compatible = "ti,tcan4x5x",
@@ -505,11 +533,15 @@ static const struct spi_device_id tcan4x5x_id_table[] = {
 };
 MODULE_DEVICE_TABLE(spi, tcan4x5x_id_table);
 
+static const struct dev_pm_ops tcan4x5x_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(tcan4x5x_suspend, tcan4x5x_resume)
+};
+
 static struct spi_driver tcan4x5x_can_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.of_match_table = tcan4x5x_of_match,
-		.pm = NULL,
+		.pm = &tcan4x5x_pm_ops,
 	},
 	.id_table = tcan4x5x_id_table,
 	.probe = tcan4x5x_can_probe,
-- 
2.42.0

Re: [PATCH v2 1/2] can: m_can: allow keeping the transceiver running in suspend

[Markus Schneider-Pargmann]

Hi Martin,

On Tue, Sep 19, 2023 at 02:28:32PM +0200, Martin Hundebøll wrote:
> Add a flag to the suspend class function that leaves the chip in a
> running state with rx interrupt enabled, so that m_can device driver can
> configure and use the interrupt as a wakeup source.

As far as I know the wakeup-source property is static, so maybe it would
be possible to store a property in m_can_classdev instead of using a
function parameter?

Looking into m_can_classdev there is already a pm_clock_support field.
Maybe you can put one right next to it for wakeup source?

Best,
Markus

> 
> Signed-off-by: Martin Hundebøll <martin@geanix.com>
> ---
> 
> Changes since v1:
>  * Fixed comment formatting
>  * Updated m_can_class_{suspend,resume} calls in m_can_pci.c too
>  * Skipped calling m_can_start() when resuming a wake-source device
> 
>  drivers/net/can/m_can/m_can.c          | 26 +++++++++++++++++++-------
>  drivers/net/can/m_can/m_can.h          |  4 ++--
>  drivers/net/can/m_can/m_can_pci.c      |  4 ++--
>  drivers/net/can/m_can/m_can_platform.c |  4 ++--
>  4 files changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 16ecc11c7f62..ec345f6fd6b0 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -2113,7 +2113,7 @@ void m_can_class_unregister(struct m_can_classdev *cdev)
>  }
>  EXPORT_SYMBOL_GPL(m_can_class_unregister);
>  
> -int m_can_class_suspend(struct device *dev)
> +int m_can_class_suspend(struct device *dev, bool is_wake_source)
>  {
>  	struct m_can_classdev *cdev = dev_get_drvdata(dev);
>  	struct net_device *ndev = cdev->net;
> @@ -2121,7 +2121,15 @@ int m_can_class_suspend(struct device *dev)
>  	if (netif_running(ndev)) {
>  		netif_stop_queue(ndev);
>  		netif_device_detach(ndev);
> -		m_can_stop(ndev);
> +
> +		/* leave the chip running with rx interrupt enabled if it used
> +		 * as a wake-up source.
> +		 */
> +		if (is_wake_source)
> +			m_can_write(cdev, M_CAN_IE, IR_RF0N);
> +		else
> +			m_can_stop(ndev);
> +
>  		m_can_clk_stop(cdev);
>  	}
>  
> @@ -2133,7 +2141,7 @@ int m_can_class_suspend(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(m_can_class_suspend);
>  
> -int m_can_class_resume(struct device *dev)
> +int m_can_class_resume(struct device *dev, bool is_wake_source)
>  {
>  	struct m_can_classdev *cdev = dev_get_drvdata(dev);
>  	struct net_device *ndev = cdev->net;
> @@ -2148,11 +2156,15 @@ int m_can_class_resume(struct device *dev)
>  		ret = m_can_clk_start(cdev);
>  		if (ret)
>  			return ret;
> -		ret  = m_can_start(ndev);
> -		if (ret) {
> -			m_can_clk_stop(cdev);
>  
> -			return ret;
> +		if (is_wake_source) {
> +			m_can_write(cdev, M_CAN_IE, cdev->active_interrupts);
> +		} else {
> +			ret  = m_can_start(ndev);
> +			if (ret) {
> +				m_can_clk_stop(cdev);
> +				return ret;
> +			}
>  		}
>  
>  		netif_device_attach(ndev);
> diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
> index 520e14277dff..5d86bcf66e83 100644
> --- a/drivers/net/can/m_can/m_can.h
> +++ b/drivers/net/can/m_can/m_can.h
> @@ -105,6 +105,6 @@ int m_can_class_get_clocks(struct m_can_classdev *cdev);
>  int m_can_init_ram(struct m_can_classdev *priv);
>  int m_can_check_mram_cfg(struct m_can_classdev *cdev, u32 mram_max_size);
>  
> -int m_can_class_suspend(struct device *dev);
> -int m_can_class_resume(struct device *dev);
> +int m_can_class_suspend(struct device *dev, bool is_wake_source);
> +int m_can_class_resume(struct device *dev, bool is_wake_source);
>  #endif	/* _CAN_M_H_ */
> diff --git a/drivers/net/can/m_can/m_can_pci.c b/drivers/net/can/m_can/m_can_pci.c
> index f2219aa2824b..7ffbe06d96d0 100644
> --- a/drivers/net/can/m_can/m_can_pci.c
> +++ b/drivers/net/can/m_can/m_can_pci.c
> @@ -169,12 +169,12 @@ static void m_can_pci_remove(struct pci_dev *pci)
>  
>  static __maybe_unused int m_can_pci_suspend(struct device *dev)
>  {
> -	return m_can_class_suspend(dev);
> +	return m_can_class_suspend(dev, false);
>  }
>  
>  static __maybe_unused int m_can_pci_resume(struct device *dev)
>  {
> -	return m_can_class_resume(dev);
> +	return m_can_class_resume(dev, false);
>  }
>  
>  static SIMPLE_DEV_PM_OPS(m_can_pci_pm_ops,
> diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
> index cdb28d6a092c..e49bef50d52c 100644
> --- a/drivers/net/can/m_can/m_can_platform.c
> +++ b/drivers/net/can/m_can/m_can_platform.c
> @@ -169,12 +169,12 @@ static int m_can_plat_probe(struct platform_device *pdev)
>  
>  static __maybe_unused int m_can_suspend(struct device *dev)
>  {
> -	return m_can_class_suspend(dev);
> +	return m_can_class_suspend(dev, false);
>  }
>  
>  static __maybe_unused int m_can_resume(struct device *dev)
>  {
> -	return m_can_class_resume(dev);
> +	return m_can_class_resume(dev, false);
>  }
>  
>  static void m_can_plat_remove(struct platform_device *pdev)
> -- 
> 2.42.0
> 

Re: [PATCH v2 2/2] can: tcan4x5x: support resuming from rx interrupt signal

[Markus Schneider-Pargmann]

Hi Martin,

On Tue, Sep 19, 2023 at 02:28:33PM +0200, Martin Hundebøll wrote:
> Implement the "wakeup-source" device tree property, so the chip is left
> running when suspending, and its rx interrupt is used as a wakeup source
> to resume operation.

I think you need to add a patch in your series to update the binding
documentation in Documentation/devicetree/bindings/net/can/tcan4x5x.txt
with the wakeup-source property.

Best,
Markus

> 
> Signed-off-by: Martin Hundebøll <martin@geanix.com>
> ---
> 
> Change since v1:
>  * Added `static` keyword to dev_pm_ops sturcture
> 
>  drivers/net/can/m_can/tcan4x5x-core.c | 34 ++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
> index 8a4143809d33..ccb279af22bb 100644
> --- a/drivers/net/can/m_can/tcan4x5x-core.c
> +++ b/drivers/net/can/m_can/tcan4x5x-core.c
> @@ -459,6 +459,9 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
>  		goto out_power;
>  	}
>  
> +	if (device_property_read_bool(&spi->dev, "wakeup-source"))
> +		device_init_wakeup(&spi->dev, true);
> +
>  	ret = m_can_class_register(mcan_class);
>  	if (ret) {
>  		dev_err(&spi->dev, "Failed registering m_can device %pe\n",
> @@ -487,6 +490,31 @@ static void tcan4x5x_can_remove(struct spi_device *spi)
>  	m_can_class_free_dev(priv->cdev.net);
>  }
>  
> +static int __maybe_unused tcan4x5x_suspend(struct device *dev)
> +{
> +	struct spi_device *spi = to_spi_device(dev);
> +
> +	if (device_may_wakeup(dev)) {
> +		enable_irq_wake(spi->irq);
> +
> +		return m_can_class_suspend(dev, true);
> +	}
> +
> +	return m_can_class_suspend(dev, false);
> +}
> +
> +static int __maybe_unused tcan4x5x_resume(struct device *dev)
> +{
> +	struct spi_device *spi = to_spi_device(dev);
> +	bool wake_source = device_may_wakeup(dev);
> +	int ret = m_can_class_resume(dev, wake_source);
> +
> +	if (wake_source)
> +		disable_irq_wake(spi->irq);
> +
> +	return ret;
> +}
> +
>  static const struct of_device_id tcan4x5x_of_match[] = {
>  	{
>  		.compatible = "ti,tcan4x5x",
> @@ -505,11 +533,15 @@ static const struct spi_device_id tcan4x5x_id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(spi, tcan4x5x_id_table);
>  
> +static const struct dev_pm_ops tcan4x5x_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(tcan4x5x_suspend, tcan4x5x_resume)
> +};
> +
>  static struct spi_driver tcan4x5x_can_driver = {
>  	.driver = {
>  		.name = KBUILD_MODNAME,
>  		.of_match_table = tcan4x5x_of_match,
> -		.pm = NULL,
> +		.pm = &tcan4x5x_pm_ops,
>  	},
>  	.id_table = tcan4x5x_id_table,
>  	.probe = tcan4x5x_can_probe,
> -- 
> 2.42.0
>