dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support
@ 2025-09-05 14:44 Tommaso Merciai
  2025-09-05 14:44 ` [PATCH 1/4] dmaengine: sh: rz-dmac: Use devm_pm_runtime_enable() Tommaso Merciai
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Tommaso Merciai @ 2025-09-05 14:44 UTC (permalink / raw)
  To: tomm.merciai
  Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Vinod Koul,
	Geert Uytterhoeven, Wolfram Sang, Fabrizio Castro,
	Uwe Kleine-König, dmaengine, linux-kernel

Dear All,

This patch series improves runtime PM support and adds system sleep PM ops for
supporting deep sleep in the rz-dmac driver.

It also refactors the driver to use the newly added devm_pm_runtime_enable()
and rz_dmac_reset_control_assert() functions for reset cleanup handling.

Thanks & Regards,
Tommaso

Tommaso Merciai (4):
  dmaengine: sh: rz-dmac: Use devm_pm_runtime_enable()
  dmaengine: sh: rz-dmac: Use devm_add_action_or_reset()
  dmaengine: sh: rz-dmac: Refactor runtime PM handling
  dmaengine: sh: rz-dmac: Add system sleep power management

 drivers/dma/sh/rz-dmac.c | 82 ++++++++++++++++++++++++++++++++++------
 1 file changed, 70 insertions(+), 12 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] dmaengine: sh: rz-dmac: Use devm_pm_runtime_enable()
  2025-09-05 14:44 [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support Tommaso Merciai
@ 2025-09-05 14:44 ` Tommaso Merciai
  2025-09-05 14:44 ` [PATCH 2/4] dmaengine: sh: rz-dmac: Use devm_add_action_or_reset() Tommaso Merciai
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Tommaso Merciai @ 2025-09-05 14:44 UTC (permalink / raw)
  To: tomm.merciai
  Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Vinod Koul,
	Geert Uytterhoeven, Fabrizio Castro, Lad Prabhakar, Wolfram Sang,
	Uwe Kleine-König, dmaengine, linux-kernel

Use devm_pm_runtime_enable() into rz_dmac_probe() and drop unnecessary
pm_runtime_disable() from rz_dmac_probe() and rz_dmac_remove().

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/dma/sh/rz-dmac.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 1f687b08d6b86..0b526cc4d24be 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -963,12 +963,15 @@ static int rz_dmac_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(dmac->rstc),
 				     "failed to get resets\n");
 
-	pm_runtime_enable(&pdev->dev);
+	ret = devm_pm_runtime_enable(&pdev->dev);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to enable runtime PM\n");
+
 	ret = pm_runtime_resume_and_get(&pdev->dev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "pm_runtime_resume_and_get failed\n");
-		goto err_pm_disable;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "pm_runtime_resume_and_get failed\n");
 
 	ret = reset_control_deassert(dmac->rstc);
 	if (ret)
@@ -1031,8 +1034,6 @@ static int rz_dmac_probe(struct platform_device *pdev)
 	reset_control_assert(dmac->rstc);
 err_pm_runtime_put:
 	pm_runtime_put(&pdev->dev);
-err_pm_disable:
-	pm_runtime_disable(&pdev->dev);
 
 	return ret;
 }
@@ -1054,7 +1055,6 @@ static void rz_dmac_remove(struct platform_device *pdev)
 	}
 	reset_control_assert(dmac->rstc);
 	pm_runtime_put(&pdev->dev);
-	pm_runtime_disable(&pdev->dev);
 
 	platform_device_put(dmac->icu.pdev);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] dmaengine: sh: rz-dmac: Use devm_add_action_or_reset()
  2025-09-05 14:44 [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support Tommaso Merciai
  2025-09-05 14:44 ` [PATCH 1/4] dmaengine: sh: rz-dmac: Use devm_pm_runtime_enable() Tommaso Merciai
@ 2025-09-05 14:44 ` Tommaso Merciai
  2025-09-05 14:53   ` Philipp Zabel
  2025-09-05 14:44 ` [PATCH 3/4] dmaengine: sh: rz-dmac: Refactor runtime PM handling Tommaso Merciai
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Tommaso Merciai @ 2025-09-05 14:44 UTC (permalink / raw)
  To: tomm.merciai
  Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Vinod Koul,
	Philipp Zabel, Geert Uytterhoeven, Fabrizio Castro, Lad Prabhakar,
	Wolfram Sang, Uwe Kleine-König, dmaengine, linux-kernel

Slightly simplify rz_dmac_probe() by using devm_add_action_or_reset()
for reset cleanup.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/dma/sh/rz-dmac.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 0b526cc4d24be..0bc11a6038383 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -905,6 +905,11 @@ static int rz_dmac_parse_of(struct device *dev, struct rz_dmac *dmac)
 	return rz_dmac_parse_of_icu(dev, dmac);
 }
 
+static void rz_dmac_reset_control_assert(void *data)
+{
+	reset_control_assert(data);
+}
+
 static int rz_dmac_probe(struct platform_device *pdev)
 {
 	const char *irqname = "error";
@@ -977,6 +982,12 @@ static int rz_dmac_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_pm_runtime_put;
 
+	ret = devm_add_action_or_reset(&pdev->dev,
+				       rz_dmac_reset_control_assert,
+				       dmac->rstc);
+	if (ret)
+		goto err_pm_runtime_put;
+
 	for (i = 0; i < dmac->n_channels; i++) {
 		ret = rz_dmac_chan_probe(dmac, &dmac->channels[i], i);
 		if (ret < 0)
@@ -1031,7 +1042,6 @@ static int rz_dmac_probe(struct platform_device *pdev)
 				  channel->lmdesc.base_dma);
 	}
 
-	reset_control_assert(dmac->rstc);
 err_pm_runtime_put:
 	pm_runtime_put(&pdev->dev);
 
@@ -1053,7 +1063,6 @@ static void rz_dmac_remove(struct platform_device *pdev)
 				  channel->lmdesc.base,
 				  channel->lmdesc.base_dma);
 	}
-	reset_control_assert(dmac->rstc);
 	pm_runtime_put(&pdev->dev);
 
 	platform_device_put(dmac->icu.pdev);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] dmaengine: sh: rz-dmac: Refactor runtime PM handling
  2025-09-05 14:44 [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support Tommaso Merciai
  2025-09-05 14:44 ` [PATCH 1/4] dmaengine: sh: rz-dmac: Use devm_pm_runtime_enable() Tommaso Merciai
  2025-09-05 14:44 ` [PATCH 2/4] dmaengine: sh: rz-dmac: Use devm_add_action_or_reset() Tommaso Merciai
@ 2025-09-05 14:44 ` Tommaso Merciai
  2025-09-05 16:17   ` Biju Das
  2025-09-05 14:44 ` [PATCH 4/4] dmaengine: sh: rz-dmac: Add system sleep power management Tommaso Merciai
  2025-09-05 15:36 ` [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support Tommaso Merciai
  4 siblings, 1 reply; 10+ messages in thread
From: Tommaso Merciai @ 2025-09-05 14:44 UTC (permalink / raw)
  To: tomm.merciai
  Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Vinod Koul,
	Geert Uytterhoeven, Wolfram Sang, Fabrizio Castro,
	Uwe Kleine-König, dmaengine, linux-kernel

Refactor runtime PM handling to ensure correct power management and prevent
resource leaks.  Invoke pm_runtime_get_sync() when allocating DMA channel
resources and pm_runtime_put() when freeing them.  Add pm_runtime_put() in
rz_dmac_probe() to balance the usage count during device initialization,
and remove the unnecessary pm_runtime_put() from rz_dmac_remove() to avoid
PM inconsistencies.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/dma/sh/rz-dmac.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 0bc11a6038383..4ab6076f5499e 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -455,7 +455,7 @@ static int rz_dmac_alloc_chan_resources(struct dma_chan *chan)
 	if (!channel->descs_allocated)
 		return -ENOMEM;
 
-	return channel->descs_allocated;
+	return pm_runtime_get_sync(chan->device->dev);
 }
 
 static void rz_dmac_free_chan_resources(struct dma_chan *chan)
@@ -490,6 +490,8 @@ static void rz_dmac_free_chan_resources(struct dma_chan *chan)
 
 	INIT_LIST_HEAD(&channel->ld_free);
 	vchan_free_chan_resources(&channel->vc);
+
+	pm_runtime_put(chan->device->dev);
 }
 
 static struct dma_async_tx_descriptor *
@@ -1027,6 +1029,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "unable to register\n");
 		goto dma_register_err;
 	}
+	pm_runtime_put(&pdev->dev);
 	return 0;
 
 dma_register_err:
@@ -1063,7 +1066,6 @@ static void rz_dmac_remove(struct platform_device *pdev)
 				  channel->lmdesc.base,
 				  channel->lmdesc.base_dma);
 	}
-	pm_runtime_put(&pdev->dev);
 
 	platform_device_put(dmac->icu.pdev);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] dmaengine: sh: rz-dmac: Add system sleep power management
  2025-09-05 14:44 [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support Tommaso Merciai
                   ` (2 preceding siblings ...)
  2025-09-05 14:44 ` [PATCH 3/4] dmaengine: sh: rz-dmac: Refactor runtime PM handling Tommaso Merciai
@ 2025-09-05 14:44 ` Tommaso Merciai
  2025-09-05 15:36 ` [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support Tommaso Merciai
  4 siblings, 0 replies; 10+ messages in thread
From: Tommaso Merciai @ 2025-09-05 14:44 UTC (permalink / raw)
  To: tomm.merciai
  Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Vinod Koul,
	Philipp Zabel, Geert Uytterhoeven, Wolfram Sang, Lad Prabhakar,
	Fabrizio Castro, Uwe Kleine-König, dmaengine, linux-kernel

Add runtime and system sleep power management operations to the RZ DMAC
driver. This enables proper handling of suspend and resume sequences,
including device reset and channel re-initialization, preparing the driver
for power state transitions.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/dma/sh/rz-dmac.c | 47 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 4ab6076f5499e..d849e313a7f79 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -437,6 +437,24 @@ static int rz_dmac_xfer_desc(struct rz_dmac_chan *chan)
  * DMA engine operations
  */
 
+static int rz_dmac_chan_init_all(struct rz_dmac *dmac)
+{
+	unsigned int i;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dmac->dev);
+	if (ret < 0)
+		return ret;
+
+	rz_dmac_writel(dmac, DCTRL_DEFAULT, CHANNEL_0_7_COMMON_BASE + DCTRL);
+	rz_dmac_writel(dmac, DCTRL_DEFAULT, CHANNEL_8_15_COMMON_BASE + DCTRL);
+
+	for (i = 0; i < dmac->n_channels; i++)
+		rz_dmac_ch_writel(&dmac->channels[i], CHCTRL_DEFAULT, CHCTRL, 1);
+
+	return pm_runtime_put_sync(dmac->dev);
+}
+
 static int rz_dmac_alloc_chan_resources(struct dma_chan *chan)
 {
 	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
@@ -1070,6 +1088,34 @@ static void rz_dmac_remove(struct platform_device *pdev)
 	platform_device_put(dmac->icu.pdev);
 }
 
+static int rz_dmac_suspend(struct device *dev)
+{
+	struct rz_dmac *dmac = dev_get_drvdata(dev);
+
+	return reset_control_assert(dmac->rstc);
+}
+
+static int rz_dmac_resume(struct device *dev)
+{
+	struct rz_dmac *dmac = dev_get_drvdata(dev);
+	int ret;
+
+	ret = reset_control_deassert(dmac->rstc);
+	if (ret)
+		return ret;
+
+	return rz_dmac_chan_init_all(dmac);
+}
+
+static const struct dev_pm_ops rz_dmac_pm_ops = {
+	/*
+	 * TODO for system sleep/resume:
+	 *   - Wait for the current transfer to complete and stop the device,
+	 *   - Resume transfers, if any.
+	 */
+	SYSTEM_SLEEP_PM_OPS(rz_dmac_suspend, rz_dmac_resume)
+};
+
 static const struct of_device_id of_rz_dmac_match[] = {
 	{ .compatible = "renesas,r9a09g057-dmac", },
 	{ .compatible = "renesas,rz-dmac", },
@@ -1079,6 +1125,7 @@ MODULE_DEVICE_TABLE(of, of_rz_dmac_match);
 
 static struct platform_driver rz_dmac_driver = {
 	.driver		= {
+		.pm	= pm_sleep_ptr(&rz_dmac_pm_ops),
 		.name	= "rz-dmac",
 		.of_match_table = of_rz_dmac_match,
 	},
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] dmaengine: sh: rz-dmac: Use devm_add_action_or_reset()
  2025-09-05 14:44 ` [PATCH 2/4] dmaengine: sh: rz-dmac: Use devm_add_action_or_reset() Tommaso Merciai
@ 2025-09-05 14:53   ` Philipp Zabel
  2025-09-05 15:22     ` Tommaso Merciai
  0 siblings, 1 reply; 10+ messages in thread
From: Philipp Zabel @ 2025-09-05 14:53 UTC (permalink / raw)
  To: Tommaso Merciai, tomm.merciai
  Cc: linux-renesas-soc, biju.das.jz, Vinod Koul, Geert Uytterhoeven,
	Fabrizio Castro, Lad Prabhakar, Wolfram Sang,
	Uwe Kleine-König, dmaengine, linux-kernel

On Fr, 2025-09-05 at 16:44 +0200, Tommaso Merciai wrote:
> Slightly simplify rz_dmac_probe() by using devm_add_action_or_reset()
> for reset cleanup.
> 
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
>  drivers/dma/sh/rz-dmac.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 0b526cc4d24be..0bc11a6038383 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -905,6 +905,11 @@ static int rz_dmac_parse_of(struct device *dev, struct rz_dmac *dmac)
>  	return rz_dmac_parse_of_icu(dev, dmac);
>  }
>  
> +static void rz_dmac_reset_control_assert(void *data)
> +{
> +	reset_control_assert(data);
> +}
> +
>  static int rz_dmac_probe(struct platform_device *pdev)
>  {
>  	const char *irqname = "error";
> @@ -977,6 +982,12 @@ static int rz_dmac_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_pm_runtime_put;
>  
> +	ret = devm_add_action_or_reset(&pdev->dev,
> +				       rz_dmac_reset_control_assert,
> +				       dmac->rstc);
> +	if (ret)
> +		goto err_pm_runtime_put;
> +
>  	for (i = 0; i < dmac->n_channels; i++) {
>  		ret = rz_dmac_chan_probe(dmac, &dmac->channels[i], i);
>  		if (ret < 0)
> @@ -1031,7 +1042,6 @@ static int rz_dmac_probe(struct platform_device *pdev)
>  				  channel->lmdesc.base_dma);
>  	}
>  
> -	reset_control_assert(dmac->rstc);
>  err_pm_runtime_put:
>  	pm_runtime_put(&pdev->dev);
>  
> @@ -1053,7 +1063,6 @@ static void rz_dmac_remove(struct platform_device *pdev)
>  				  channel->lmdesc.base,
>  				  channel->lmdesc.base_dma);
>  	}
> -	reset_control_assert(dmac->rstc);

This patch changes cleanup order by effectively moving the
reset_control_assert() after pm_runtime_put(). The commit message does
not explain that this is safe to do.

If this is ok, I'd move the reset_control_assert() up before
pm_runtime_enable/resume_and_get().

regards
Philipp

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] dmaengine: sh: rz-dmac: Use devm_add_action_or_reset()
  2025-09-05 14:53   ` Philipp Zabel
@ 2025-09-05 15:22     ` Tommaso Merciai
  2025-09-05 15:30       ` Philipp Zabel
  0 siblings, 1 reply; 10+ messages in thread
From: Tommaso Merciai @ 2025-09-05 15:22 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Vinod Koul,
	Geert Uytterhoeven, Fabrizio Castro, Lad Prabhakar, Wolfram Sang,
	Uwe Kleine-König, dmaengine, linux-kernel

Hi Philipp,
Thank you for your review!

On Fri, Sep 05, 2025 at 04:53:54PM +0200, Philipp Zabel wrote:
> On Fr, 2025-09-05 at 16:44 +0200, Tommaso Merciai wrote:
> > Slightly simplify rz_dmac_probe() by using devm_add_action_or_reset()
> > for reset cleanup.
> > 
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> >  drivers/dma/sh/rz-dmac.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > index 0b526cc4d24be..0bc11a6038383 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> > @@ -905,6 +905,11 @@ static int rz_dmac_parse_of(struct device *dev, struct rz_dmac *dmac)
> >  	return rz_dmac_parse_of_icu(dev, dmac);
> >  }
> >  
> > +static void rz_dmac_reset_control_assert(void *data)
> > +{
> > +	reset_control_assert(data);
> > +}
> > +
> >  static int rz_dmac_probe(struct platform_device *pdev)
> >  {
> >  	const char *irqname = "error";
> > @@ -977,6 +982,12 @@ static int rz_dmac_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		goto err_pm_runtime_put;
> >  
> > +	ret = devm_add_action_or_reset(&pdev->dev,
> > +				       rz_dmac_reset_control_assert,
> > +				       dmac->rstc);
> > +	if (ret)
> > +		goto err_pm_runtime_put;
> > +
> >  	for (i = 0; i < dmac->n_channels; i++) {
> >  		ret = rz_dmac_chan_probe(dmac, &dmac->channels[i], i);
> >  		if (ret < 0)
> > @@ -1031,7 +1042,6 @@ static int rz_dmac_probe(struct platform_device *pdev)
> >  				  channel->lmdesc.base_dma);
> >  	}
> >  
> > -	reset_control_assert(dmac->rstc);
> >  err_pm_runtime_put:
> >  	pm_runtime_put(&pdev->dev);
> >  
> > @@ -1053,7 +1063,6 @@ static void rz_dmac_remove(struct platform_device *pdev)
> >  				  channel->lmdesc.base,
> >  				  channel->lmdesc.base_dma);
> >  	}
> > -	reset_control_assert(dmac->rstc);
> 
> This patch changes cleanup order by effectively moving the
> reset_control_assert() after pm_runtime_put(). The commit message does
> not explain that this is safe to do.

Agreed. Thanks.

> 
> If this is ok, I'd move the reset_control_assert() up before
> pm_runtime_enable/resume_and_get().

You mean having in the end the following calls:

...
	dmac->rstc = devm_reset_control_array_get_optional_exclusive(&pdev->dev);
	if (IS_ERR(dmac->rstc))
		return dev_err_probe(&pdev->dev, PTR_ERR(dmac->rstc),
				     "failed to get resets\n");

	ret = reset_control_deassert(dmac->rstc);
	if (ret)
		return dev_err_probe(&pdev->dev, ret,
				     "failed to deassert resets\n");

	ret = devm_add_action_or_reset(&pdev->dev,
				       rz_dmac_reset_control_assert,
				       dmac->rstc);
	if (ret)
		return dev_err_probe(&pdev->dev, ret,
				     "failed to register reset cleanup action\n");

	ret = devm_pm_runtime_enable(&pdev->dev);
	if (ret < 0)
		return dev_err_probe(&pdev->dev, ret,
				     "Failed to enable runtime PM\n");
...

Right?
Thanks in advance.


Kind Regards,
Tommaso

> 
> regards
> Philipp

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] dmaengine: sh: rz-dmac: Use devm_add_action_or_reset()
  2025-09-05 15:22     ` Tommaso Merciai
@ 2025-09-05 15:30       ` Philipp Zabel
  0 siblings, 0 replies; 10+ messages in thread
From: Philipp Zabel @ 2025-09-05 15:30 UTC (permalink / raw)
  To: Tommaso Merciai
  Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Vinod Koul,
	Geert Uytterhoeven, Fabrizio Castro, Lad Prabhakar, Wolfram Sang,
	Uwe Kleine-König, dmaengine, linux-kernel

On Fr, 2025-09-05 at 17:22 +0200, Tommaso Merciai wrote:
> Hi Philipp,
> Thank you for your review!
> 
> On Fri, Sep 05, 2025 at 04:53:54PM +0200, Philipp Zabel wrote:
> > On Fr, 2025-09-05 at 16:44 +0200, Tommaso Merciai wrote:
> > > Slightly simplify rz_dmac_probe() by using devm_add_action_or_reset()
> > > for reset cleanup.
> > > 
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > >  drivers/dma/sh/rz-dmac.c | 13 +++++++++++--
> > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > > index 0b526cc4d24be..0bc11a6038383 100644
> > > --- a/drivers/dma/sh/rz-dmac.c
> > > +++ b/drivers/dma/sh/rz-dmac.c
> > > @@ -905,6 +905,11 @@ static int rz_dmac_parse_of(struct device *dev, struct rz_dmac *dmac)
> > >  	return rz_dmac_parse_of_icu(dev, dmac);
> > >  }
> > >  
> > > +static void rz_dmac_reset_control_assert(void *data)
> > > +{
> > > +	reset_control_assert(data);
> > > +}
> > > +
> > >  static int rz_dmac_probe(struct platform_device *pdev)
> > >  {
> > >  	const char *irqname = "error";
> > > @@ -977,6 +982,12 @@ static int rz_dmac_probe(struct platform_device *pdev)
> > >  	if (ret)
> > >  		goto err_pm_runtime_put;
> > >  
> > > +	ret = devm_add_action_or_reset(&pdev->dev,
> > > +				       rz_dmac_reset_control_assert,
> > > +				       dmac->rstc);
> > > +	if (ret)
> > > +		goto err_pm_runtime_put;
> > > +
> > >  	for (i = 0; i < dmac->n_channels; i++) {
> > >  		ret = rz_dmac_chan_probe(dmac, &dmac->channels[i], i);
> > >  		if (ret < 0)
> > > @@ -1031,7 +1042,6 @@ static int rz_dmac_probe(struct platform_device *pdev)
> > >  				  channel->lmdesc.base_dma);
> > >  	}
> > >  
> > > -	reset_control_assert(dmac->rstc);
> > >  err_pm_runtime_put:
> > >  	pm_runtime_put(&pdev->dev);
> > >  
> > > @@ -1053,7 +1063,6 @@ static void rz_dmac_remove(struct platform_device *pdev)
> > >  				  channel->lmdesc.base,
> > >  				  channel->lmdesc.base_dma);
> > >  	}
> > > -	reset_control_assert(dmac->rstc);
> > 
> > This patch changes cleanup order by effectively moving the
> > reset_control_assert() after pm_runtime_put(). The commit message does
> > not explain that this is safe to do.
> 
> Agreed. Thanks.
> 
> > 
> > If this is ok, I'd move the reset_control_assert() up before
> > pm_runtime_enable/resume_and_get().
> 
> You mean having in the end the following calls:
> 
> ...
> 	dmac->rstc = devm_reset_control_array_get_optional_exclusive(&pdev->dev);
> 	if (IS_ERR(dmac->rstc))
> 		return dev_err_probe(&pdev->dev, PTR_ERR(dmac->rstc),
> 				     "failed to get resets\n");
> 
> 	ret = reset_control_deassert(dmac->rstc);
> 	if (ret)
> 		return dev_err_probe(&pdev->dev, ret,
> 				     "failed to deassert resets\n");
> 
> 	ret = devm_add_action_or_reset(&pdev->dev,
> 				       rz_dmac_reset_control_assert,
> 				       dmac->rstc);
> 	if (ret)
> 		return dev_err_probe(&pdev->dev, ret,
> 				     "failed to register reset cleanup action\n");
> 
> 	ret = devm_pm_runtime_enable(&pdev->dev);
> 	if (ret < 0)
> 		return dev_err_probe(&pdev->dev, ret,
> 				     "Failed to enable runtime PM\n");
> ...
> 
> Right?

Right.

regards
Philipp

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support
  2025-09-05 14:44 [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support Tommaso Merciai
                   ` (3 preceding siblings ...)
  2025-09-05 14:44 ` [PATCH 4/4] dmaengine: sh: rz-dmac: Add system sleep power management Tommaso Merciai
@ 2025-09-05 15:36 ` Tommaso Merciai
  4 siblings, 0 replies; 10+ messages in thread
From: Tommaso Merciai @ 2025-09-05 15:36 UTC (permalink / raw)
  To: tomm.merciai
  Cc: linux-renesas-soc, biju.das.jz, Vinod Koul, Geert Uytterhoeven,
	Wolfram Sang, Fabrizio Castro, Uwe Kleine-König, dmaengine,
	linux-kernel

On Fri, Sep 05, 2025 at 04:44:16PM +0200, Tommaso Merciai wrote:
> Dear All,
> 
> This patch series improves runtime PM support and adds system sleep PM ops for
> supporting deep sleep in the rz-dmac driver.
> 
> It also refactors the driver to use the newly added devm_pm_runtime_enable()
> and rz_dmac_reset_control_assert() functions for reset cleanup handling.

Series not tested yet on RZ/G2L or any stress test.

Thanks,
Tommaso

> 
> Thanks & Regards,
> Tommaso
> 
> Tommaso Merciai (4):
>   dmaengine: sh: rz-dmac: Use devm_pm_runtime_enable()
>   dmaengine: sh: rz-dmac: Use devm_add_action_or_reset()
>   dmaengine: sh: rz-dmac: Refactor runtime PM handling
>   dmaengine: sh: rz-dmac: Add system sleep power management
> 
>  drivers/dma/sh/rz-dmac.c | 82 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 70 insertions(+), 12 deletions(-)
> 
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 3/4] dmaengine: sh: rz-dmac: Refactor runtime PM handling
  2025-09-05 14:44 ` [PATCH 3/4] dmaengine: sh: rz-dmac: Refactor runtime PM handling Tommaso Merciai
@ 2025-09-05 16:17   ` Biju Das
  0 siblings, 0 replies; 10+ messages in thread
From: Biju Das @ 2025-09-05 16:17 UTC (permalink / raw)
  To: Tommaso Merciai, Tommaso Merciai
  Cc: linux-renesas-soc@vger.kernel.org, Vinod Koul, Geert Uytterhoeven,
	wsa+renesas, Fabrizio Castro, Uwe Kleine-König,
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org

Hi Tommaso,

> -----Original Message-----
> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> Sent: 05 September 2025 15:44
> Subject: [PATCH 3/4] dmaengine: sh: rz-dmac: Refactor runtime PM handling
> 
> Refactor runtime PM handling to ensure correct power management and prevent resource leaks.  Invoke
> pm_runtime_get_sync() when allocating DMA channel resources and pm_runtime_put() when freeing them.
> Add pm_runtime_put() in
> rz_dmac_probe() to balance the usage count during device initialization, and remove the unnecessary
> pm_runtime_put() from rz_dmac_remove() to avoid PM inconsistencies.
> 
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
>  drivers/dma/sh/rz-dmac.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 0bc11a6038383..4ab6076f5499e
> 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -455,7 +455,7 @@ static int rz_dmac_alloc_chan_resources(struct dma_chan *chan)
>  	if (!channel->descs_allocated)
>  		return -ENOMEM;
> 
> -	return channel->descs_allocated;
> +	return pm_runtime_get_sync(chan->device->dev);

I would check for pm_runtime_resume_and_get(chan->device->dev) first
If successful, still will return channel->descs_allocated to comply with API documentation
rather than returning "runtime PM status"

@device_alloc_chan_resources: allocate resources and return the
number of allocated descriptors.

Cheers,
Biju


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-09-05 16:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-05 14:44 [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support Tommaso Merciai
2025-09-05 14:44 ` [PATCH 1/4] dmaengine: sh: rz-dmac: Use devm_pm_runtime_enable() Tommaso Merciai
2025-09-05 14:44 ` [PATCH 2/4] dmaengine: sh: rz-dmac: Use devm_add_action_or_reset() Tommaso Merciai
2025-09-05 14:53   ` Philipp Zabel
2025-09-05 15:22     ` Tommaso Merciai
2025-09-05 15:30       ` Philipp Zabel
2025-09-05 14:44 ` [PATCH 3/4] dmaengine: sh: rz-dmac: Refactor runtime PM handling Tommaso Merciai
2025-09-05 16:17   ` Biju Das
2025-09-05 14:44 ` [PATCH 4/4] dmaengine: sh: rz-dmac: Add system sleep power management Tommaso Merciai
2025-09-05 15:36 ` [PATCH 0/4] dmaengine: sh: rz-dmac: Add system sleep PM support Tommaso Merciai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).