[PATCH 0/3] ASoC: sh: rz-ssi: Trivial fixes

[PATCH 0/3] ASoC: sh: rz-ssi: Trivial fixes

[Lad Prabhakar]

Hi All,

This patch series aims to fix trivial issues found in rz-ssi driver.

Cheers,
Prabhakar

Lad Prabhakar (3):
  ASoC: sh: rz-ssi: Drop unused macros
  ASoC: sh: rz-ssi: Propagate error codes returned from
    platform_get_irq_byname()
  ASoC: sh: rz-ssi: Add a devres action to release the DMA channels

 sound/soc/sh/rz-ssi.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH 0/3] ASoC: sh: rz-ssi: Trivial fixes

[Lad Prabhakar]

Hi All,

This patch series aims to fix trivial issues found in rz-ssi driver.

Cheers,
Prabhakar

Lad Prabhakar (3):
  ASoC: sh: rz-ssi: Drop unused macros
  ASoC: sh: rz-ssi: Propagate error codes returned from
    platform_get_irq_byname()
  ASoC: sh: rz-ssi: Add a devres action to release the DMA channels

 sound/soc/sh/rz-ssi.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Lad Prabhakar]

Drop unused macros SSIFSR_TDC and SSIFSR_RDC.

Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 sound/soc/sh/rz-ssi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index e8edaed05d4c..cec458b8c507 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -59,9 +59,7 @@
 #define SSIFSR_RDC_MASK		0x3f
 #define SSIFSR_RDC_SHIFT	8
 
-#define SSIFSR_TDC(x)		(((x) & 0x1f) << 24)
 #define SSIFSR_TDE		BIT(16)
-#define SSIFSR_RDC(x)		(((x) & 0x1f) << 8)
 #define SSIFSR_RDF		BIT(0)
 
 #define SSIOFR_LRCONT		BIT(8)
-- 
2.17.1

[PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Lad Prabhakar]

Drop unused macros SSIFSR_TDC and SSIFSR_RDC.

Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 sound/soc/sh/rz-ssi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index e8edaed05d4c..cec458b8c507 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -59,9 +59,7 @@
 #define SSIFSR_RDC_MASK		0x3f
 #define SSIFSR_RDC_SHIFT	8
 
-#define SSIFSR_TDC(x)		(((x) & 0x1f) << 24)
 #define SSIFSR_TDE		BIT(16)
-#define SSIFSR_RDC(x)		(((x) & 0x1f) << 8)
 #define SSIFSR_RDF		BIT(0)
 
 #define SSIOFR_LRCONT		BIT(8)
-- 
2.17.1

[PATCH 2/3] ASoC: sh: rz-ssi: Propagate error codes returned from platform_get_irq_byname()

[Lad Prabhakar]

Propagate error codes returned from platform_get_irq_byname() instead of
returning -ENODEV. platform_get_irq_byname() may return -EPROBE_DEFER, to
handle such cases propagate the error codes.

While at it drop the dev_err_probe() messages as platform_get_irq_byname()
already does this for us in case of error.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 sound/soc/sh/rz-ssi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index cec458b8c507..d9a684e71ec3 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -977,8 +977,7 @@ static int rz_ssi_probe(struct platform_device *pdev)
 	/* Error Interrupt */
 	ssi->irq_int = platform_get_irq_byname(pdev, "int_req");
 	if (ssi->irq_int < 0)
-		return dev_err_probe(&pdev->dev, -ENODEV,
-				     "Unable to get SSI int_req IRQ\n");
+		return ssi->irq_int;
 
 	ret = devm_request_irq(&pdev->dev, ssi->irq_int, &rz_ssi_interrupt,
 			       0, dev_name(&pdev->dev), ssi);
@@ -990,8 +989,7 @@ static int rz_ssi_probe(struct platform_device *pdev)
 		/* Tx and Rx interrupts (pio only) */
 		ssi->irq_tx = platform_get_irq_byname(pdev, "dma_tx");
 		if (ssi->irq_tx < 0)
-			return dev_err_probe(&pdev->dev, -ENODEV,
-					     "Unable to get SSI dma_tx IRQ\n");
+			return ssi->irq_tx;
 
 		ret = devm_request_irq(&pdev->dev, ssi->irq_tx,
 				       &rz_ssi_interrupt, 0,
@@ -1002,8 +1000,7 @@ static int rz_ssi_probe(struct platform_device *pdev)
 
 		ssi->irq_rx = platform_get_irq_byname(pdev, "dma_rx");
 		if (ssi->irq_rx < 0)
-			return dev_err_probe(&pdev->dev, -ENODEV,
-					     "Unable to get SSI dma_rx IRQ\n");
+			return ssi->irq_rx;
 
 		ret = devm_request_irq(&pdev->dev, ssi->irq_rx,
 				       &rz_ssi_interrupt, 0,
-- 
2.17.1

[PATCH 2/3] ASoC: sh: rz-ssi: Propagate error codes returned from platform_get_irq_byname()

[Lad Prabhakar]

Propagate error codes returned from platform_get_irq_byname() instead of
returning -ENODEV. platform_get_irq_byname() may return -EPROBE_DEFER, to
handle such cases propagate the error codes.

While at it drop the dev_err_probe() messages as platform_get_irq_byname()
already does this for us in case of error.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 sound/soc/sh/rz-ssi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index cec458b8c507..d9a684e71ec3 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -977,8 +977,7 @@ static int rz_ssi_probe(struct platform_device *pdev)
 	/* Error Interrupt */
 	ssi->irq_int = platform_get_irq_byname(pdev, "int_req");
 	if (ssi->irq_int < 0)
-		return dev_err_probe(&pdev->dev, -ENODEV,
-				     "Unable to get SSI int_req IRQ\n");
+		return ssi->irq_int;
 
 	ret = devm_request_irq(&pdev->dev, ssi->irq_int, &rz_ssi_interrupt,
 			       0, dev_name(&pdev->dev), ssi);
@@ -990,8 +989,7 @@ static int rz_ssi_probe(struct platform_device *pdev)
 		/* Tx and Rx interrupts (pio only) */
 		ssi->irq_tx = platform_get_irq_byname(pdev, "dma_tx");
 		if (ssi->irq_tx < 0)
-			return dev_err_probe(&pdev->dev, -ENODEV,
-					     "Unable to get SSI dma_tx IRQ\n");
+			return ssi->irq_tx;
 
 		ret = devm_request_irq(&pdev->dev, ssi->irq_tx,
 				       &rz_ssi_interrupt, 0,
@@ -1002,8 +1000,7 @@ static int rz_ssi_probe(struct platform_device *pdev)
 
 		ssi->irq_rx = platform_get_irq_byname(pdev, "dma_rx");
 		if (ssi->irq_rx < 0)
-			return dev_err_probe(&pdev->dev, -ENODEV,
-					     "Unable to get SSI dma_rx IRQ\n");
+			return ssi->irq_rx;
 
 		ret = devm_request_irq(&pdev->dev, ssi->irq_rx,
 				       &rz_ssi_interrupt, 0,
-- 
2.17.1

[PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the DMA channels

[Lad Prabhakar]

DMA channels requested by rz_ssi_dma_request() in rz_ssi_probe() were
never released in the error path apart from one place. This patch fixes
this issue by adding a devres action to release the DMA channels and
dropping the single rz_ssi_release_dma_channels() call which was placed
in the error path in case devm_snd_soc_register_component() failed.

Fixes: 26ac471c5354 ("ASoC: sh: rz-ssi: Add SSI DMAC support")
Reported-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 sound/soc/sh/rz-ssi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index d9a684e71ec3..f04da1bf5680 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -912,6 +912,11 @@ static const struct snd_soc_component_driver rz_ssi_soc_component = {
 	.pcm_construct	= rz_ssi_pcm_new,
 };
 
+static void rz_ssi_release_dma_channels_action(void *data)
+{
+	rz_ssi_release_dma_channels(data);
+}
+
 static int rz_ssi_probe(struct platform_device *pdev)
 {
 	struct rz_ssi_priv *ssi;
@@ -966,6 +971,11 @@ static int rz_ssi_probe(struct platform_device *pdev)
 		dev_info(&pdev->dev, "DMA enabled");
 		ssi->playback.transfer = rz_ssi_dma_transfer;
 		ssi->capture.transfer = rz_ssi_dma_transfer;
+
+		ret = devm_add_action_or_reset(&pdev->dev,
+					       rz_ssi_release_dma_channels_action, ssi);
+		if (ret)
+			return ret;
 	}
 
 	ssi->playback.priv = ssi;
@@ -1027,8 +1037,6 @@ static int rz_ssi_probe(struct platform_device *pdev)
 					      rz_ssi_soc_dai,
 					      ARRAY_SIZE(rz_ssi_soc_dai));
 	if (ret < 0) {
-		rz_ssi_release_dma_channels(ssi);
-
 		pm_runtime_put(ssi->dev);
 		pm_runtime_disable(ssi->dev);
 		reset_control_assert(ssi->rstc);
-- 
2.17.1

[PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the DMA channels

[Lad Prabhakar]

DMA channels requested by rz_ssi_dma_request() in rz_ssi_probe() were
never released in the error path apart from one place. This patch fixes
this issue by adding a devres action to release the DMA channels and
dropping the single rz_ssi_release_dma_channels() call which was placed
in the error path in case devm_snd_soc_register_component() failed.

Fixes: 26ac471c5354 ("ASoC: sh: rz-ssi: Add SSI DMAC support")
Reported-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 sound/soc/sh/rz-ssi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index d9a684e71ec3..f04da1bf5680 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -912,6 +912,11 @@ static const struct snd_soc_component_driver rz_ssi_soc_component = {
 	.pcm_construct	= rz_ssi_pcm_new,
 };
 
+static void rz_ssi_release_dma_channels_action(void *data)
+{
+	rz_ssi_release_dma_channels(data);
+}
+
 static int rz_ssi_probe(struct platform_device *pdev)
 {
 	struct rz_ssi_priv *ssi;
@@ -966,6 +971,11 @@ static int rz_ssi_probe(struct platform_device *pdev)
 		dev_info(&pdev->dev, "DMA enabled");
 		ssi->playback.transfer = rz_ssi_dma_transfer;
 		ssi->capture.transfer = rz_ssi_dma_transfer;
+
+		ret = devm_add_action_or_reset(&pdev->dev,
+					       rz_ssi_release_dma_channels_action, ssi);
+		if (ret)
+			return ret;
 	}
 
 	ssi->playback.priv = ssi;
@@ -1027,8 +1037,6 @@ static int rz_ssi_probe(struct platform_device *pdev)
 					      rz_ssi_soc_dai,
 					      ARRAY_SIZE(rz_ssi_soc_dai));
 	if (ret < 0) {
-		rz_ssi_release_dma_channels(ssi);
-
 		pm_runtime_put(ssi->dev);
 		pm_runtime_disable(ssi->dev);
 		reset_control_assert(ssi->rstc);
-- 
2.17.1

RE: [PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the DMA channels

[Biju Das]

Hi Lad Prabhakar,

Thanks for the patch.

> Subject: [PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the
> DMA channels
> 
> DMA channels requested by rz_ssi_dma_request() in rz_ssi_probe() were never
> released in the error path apart from one place. This patch fixes this
> issue by adding a devres action to release the DMA channels and dropping
> the single rz_ssi_release_dma_channels() call which was placed in the error
> path in case devm_snd_soc_register_component() failed.
> 
> Fixes: 26ac471c5354 ("ASoC: sh: rz-ssi: Add SSI DMAC support")
> Reported-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  sound/soc/sh/rz-ssi.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c index
> d9a684e71ec3..f04da1bf5680 100644
> --- a/sound/soc/sh/rz-ssi.c
> +++ b/sound/soc/sh/rz-ssi.c
> @@ -912,6 +912,11 @@ static const struct snd_soc_component_driver
> rz_ssi_soc_component = {
>  	.pcm_construct	= rz_ssi_pcm_new,
>  };
> 
> +static void rz_ssi_release_dma_channels_action(void *data) {
> +	rz_ssi_release_dma_channels(data);
> +}
> +
>  static int rz_ssi_probe(struct platform_device *pdev)  {
>  	struct rz_ssi_priv *ssi;
> @@ -966,6 +971,11 @@ static int rz_ssi_probe(struct platform_device *pdev)
>  		dev_info(&pdev->dev, "DMA enabled");
>  		ssi->playback.transfer = rz_ssi_dma_transfer;
>  		ssi->capture.transfer = rz_ssi_dma_transfer;
> +
> +		ret = devm_add_action_or_reset(&pdev->dev,
> +					       rz_ssi_release_dma_channels_action,
> ssi);
> +		if (ret)
> +			return ret;
>  	}
> 
>  	ssi->playback.priv = ssi;
> @@ -1027,8 +1037,6 @@ static int rz_ssi_probe(struct platform_device *pdev)
>  					      rz_ssi_soc_dai,
>  					      ARRAY_SIZE(rz_ssi_soc_dai));
>  	if (ret < 0) {
> -		rz_ssi_release_dma_channels(ssi);
> -

Maybe we need to keep this as it is, otherwise DMA channel release will happen
after CLK disable and Reset assert. Ideally release the channel, disable the clock and assert
the reset.

Similarly, you may want to add "rz_ssi_release_dma_channels(ssi)" for error path related to
Pm_runtime_resume_get.


Also with this change there is unbalanced release_dma_channels() one from devres and other from remove.

Regards,
Biju



>  		pm_runtime_put(ssi->dev);
>  		pm_runtime_disable(ssi->dev);
>  		reset_control_assert(ssi->rstc);
> --
> 2.17.1

RE: [PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the DMA channels

[Biju Das]

Hi Lad Prabhakar,

Thanks for the patch.

> Subject: [PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the
> DMA channels
> 
> DMA channels requested by rz_ssi_dma_request() in rz_ssi_probe() were never
> released in the error path apart from one place. This patch fixes this
> issue by adding a devres action to release the DMA channels and dropping
> the single rz_ssi_release_dma_channels() call which was placed in the error
> path in case devm_snd_soc_register_component() failed.
> 
> Fixes: 26ac471c5354 ("ASoC: sh: rz-ssi: Add SSI DMAC support")
> Reported-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  sound/soc/sh/rz-ssi.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c index
> d9a684e71ec3..f04da1bf5680 100644
> --- a/sound/soc/sh/rz-ssi.c
> +++ b/sound/soc/sh/rz-ssi.c
> @@ -912,6 +912,11 @@ static const struct snd_soc_component_driver
> rz_ssi_soc_component = {
>  	.pcm_construct	= rz_ssi_pcm_new,
>  };
> 
> +static void rz_ssi_release_dma_channels_action(void *data) {
> +	rz_ssi_release_dma_channels(data);
> +}
> +
>  static int rz_ssi_probe(struct platform_device *pdev)  {
>  	struct rz_ssi_priv *ssi;
> @@ -966,6 +971,11 @@ static int rz_ssi_probe(struct platform_device *pdev)
>  		dev_info(&pdev->dev, "DMA enabled");
>  		ssi->playback.transfer = rz_ssi_dma_transfer;
>  		ssi->capture.transfer = rz_ssi_dma_transfer;
> +
> +		ret = devm_add_action_or_reset(&pdev->dev,
> +					       rz_ssi_release_dma_channels_action,
> ssi);
> +		if (ret)
> +			return ret;
>  	}
> 
>  	ssi->playback.priv = ssi;
> @@ -1027,8 +1037,6 @@ static int rz_ssi_probe(struct platform_device *pdev)
>  					      rz_ssi_soc_dai,
>  					      ARRAY_SIZE(rz_ssi_soc_dai));
>  	if (ret < 0) {
> -		rz_ssi_release_dma_channels(ssi);
> -

Maybe we need to keep this as it is, otherwise DMA channel release will happen
after CLK disable and Reset assert. Ideally release the channel, disable the clock and assert
the reset.

Similarly, you may want to add "rz_ssi_release_dma_channels(ssi)" for error path related to
Pm_runtime_resume_get.


Also with this change there is unbalanced release_dma_channels() one from devres and other from remove.

Regards,
Biju



>  		pm_runtime_put(ssi->dev);
>  		pm_runtime_disable(ssi->dev);
>  		reset_control_assert(ssi->rstc);
> --
> 2.17.1

Re: [PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the DMA channels

[Lad, Prabhakar]

Hi Biju,

Thank you for the review.

On Fri, Apr 22, 2022 at 7:52 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Lad Prabhakar,
>
> Thanks for the patch.
>
> > Subject: [PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the
> > DMA channels
> >
> > DMA channels requested by rz_ssi_dma_request() in rz_ssi_probe() were never
> > released in the error path apart from one place. This patch fixes this
> > issue by adding a devres action to release the DMA channels and dropping
> > the single rz_ssi_release_dma_channels() call which was placed in the error
> > path in case devm_snd_soc_register_component() failed.
> >
> > Fixes: 26ac471c5354 ("ASoC: sh: rz-ssi: Add SSI DMAC support")
> > Reported-by: Pavel Machek <pavel@denx.de>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  sound/soc/sh/rz-ssi.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c index
> > d9a684e71ec3..f04da1bf5680 100644
> > --- a/sound/soc/sh/rz-ssi.c
> > +++ b/sound/soc/sh/rz-ssi.c
> > @@ -912,6 +912,11 @@ static const struct snd_soc_component_driver
> > rz_ssi_soc_component = {
> >       .pcm_construct  = rz_ssi_pcm_new,
> >  };
> >
> > +static void rz_ssi_release_dma_channels_action(void *data) {
> > +     rz_ssi_release_dma_channels(data);
> > +}
> > +
> >  static int rz_ssi_probe(struct platform_device *pdev)  {
> >       struct rz_ssi_priv *ssi;
> > @@ -966,6 +971,11 @@ static int rz_ssi_probe(struct platform_device *pdev)
> >               dev_info(&pdev->dev, "DMA enabled");
> >               ssi->playback.transfer = rz_ssi_dma_transfer;
> >               ssi->capture.transfer = rz_ssi_dma_transfer;
> > +
> > +             ret = devm_add_action_or_reset(&pdev->dev,
> > +                                            rz_ssi_release_dma_channels_action,
> > ssi);
> > +             if (ret)
> > +                     return ret;
> >       }
> >
> >       ssi->playback.priv = ssi;
> > @@ -1027,8 +1037,6 @@ static int rz_ssi_probe(struct platform_device *pdev)
> >                                             rz_ssi_soc_dai,
> >                                             ARRAY_SIZE(rz_ssi_soc_dai));
> >       if (ret < 0) {
> > -             rz_ssi_release_dma_channels(ssi);
> > -
>
> Maybe we need to keep this as it is, otherwise DMA channel release will happen
> after CLK disable and Reset assert. Ideally release the channel, disable the clock and assert
> the reset.
>
Ok will move this call to individual error paths.

> Similarly, you may want to add "rz_ssi_release_dma_channels(ssi)" for error path related to
> Pm_runtime_resume_get.
>
yes this needs to go under all error paths except the pio chunk.

>
> Also with this change there is unbalanced release_dma_channels() one from devres and other from remove.
>
Agreed.

Cheers,
Prabhakar

> Regards,
> Biju
>
>
>
> >               pm_runtime_put(ssi->dev);
> >               pm_runtime_disable(ssi->dev);
> >               reset_control_assert(ssi->rstc);
> > --
> > 2.17.1
>

Re: [PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the DMA channels

[Lad, Prabhakar]

Hi Biju,

Thank you for the review.

On Fri, Apr 22, 2022 at 7:52 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Lad Prabhakar,
>
> Thanks for the patch.
>
> > Subject: [PATCH 3/3] ASoC: sh: rz-ssi: Add a devres action to release the
> > DMA channels
> >
> > DMA channels requested by rz_ssi_dma_request() in rz_ssi_probe() were never
> > released in the error path apart from one place. This patch fixes this
> > issue by adding a devres action to release the DMA channels and dropping
> > the single rz_ssi_release_dma_channels() call which was placed in the error
> > path in case devm_snd_soc_register_component() failed.
> >
> > Fixes: 26ac471c5354 ("ASoC: sh: rz-ssi: Add SSI DMAC support")
> > Reported-by: Pavel Machek <pavel@denx.de>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  sound/soc/sh/rz-ssi.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c index
> > d9a684e71ec3..f04da1bf5680 100644
> > --- a/sound/soc/sh/rz-ssi.c
> > +++ b/sound/soc/sh/rz-ssi.c
> > @@ -912,6 +912,11 @@ static const struct snd_soc_component_driver
> > rz_ssi_soc_component = {
> >       .pcm_construct  = rz_ssi_pcm_new,
> >  };
> >
> > +static void rz_ssi_release_dma_channels_action(void *data) {
> > +     rz_ssi_release_dma_channels(data);
> > +}
> > +
> >  static int rz_ssi_probe(struct platform_device *pdev)  {
> >       struct rz_ssi_priv *ssi;
> > @@ -966,6 +971,11 @@ static int rz_ssi_probe(struct platform_device *pdev)
> >               dev_info(&pdev->dev, "DMA enabled");
> >               ssi->playback.transfer = rz_ssi_dma_transfer;
> >               ssi->capture.transfer = rz_ssi_dma_transfer;
> > +
> > +             ret = devm_add_action_or_reset(&pdev->dev,
> > +                                            rz_ssi_release_dma_channels_action,
> > ssi);
> > +             if (ret)
> > +                     return ret;
> >       }
> >
> >       ssi->playback.priv = ssi;
> > @@ -1027,8 +1037,6 @@ static int rz_ssi_probe(struct platform_device *pdev)
> >                                             rz_ssi_soc_dai,
> >                                             ARRAY_SIZE(rz_ssi_soc_dai));
> >       if (ret < 0) {
> > -             rz_ssi_release_dma_channels(ssi);
> > -
>
> Maybe we need to keep this as it is, otherwise DMA channel release will happen
> after CLK disable and Reset assert. Ideally release the channel, disable the clock and assert
> the reset.
>
Ok will move this call to individual error paths.

> Similarly, you may want to add "rz_ssi_release_dma_channels(ssi)" for error path related to
> Pm_runtime_resume_get.
>
yes this needs to go under all error paths except the pio chunk.

>
> Also with this change there is unbalanced release_dma_channels() one from devres and other from remove.
>
Agreed.

Cheers,
Prabhakar

> Regards,
> Biju
>
>
>
> >               pm_runtime_put(ssi->dev);
> >               pm_runtime_disable(ssi->dev);
> >               reset_control_assert(ssi->rstc);
> > --
> > 2.17.1
>

Re: [PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Geert Uytterhoeven]

Hi Prabhakar,

On Fri, Apr 22, 2022 at 7:32 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Drop unused macros SSIFSR_TDC and SSIFSR_RDC.
>
> Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

What does this fix?
Is the real issue that there are 32 FIFO entries, and the TDC and RDC
fields are 6 bits wide, while the mask uses 0x1f instead of 0x3f?

> --- a/sound/soc/sh/rz-ssi.c
> +++ b/sound/soc/sh/rz-ssi.c
> @@ -59,9 +59,7 @@
>  #define SSIFSR_RDC_MASK                0x3f
>  #define SSIFSR_RDC_SHIFT       8
>
> -#define SSIFSR_TDC(x)          (((x) & 0x1f) << 24)
>  #define SSIFSR_TDE             BIT(16)
> -#define SSIFSR_RDC(x)          (((x) & 0x1f) << 8)
>  #define SSIFSR_RDF             BIT(0)
>
>  #define SSIOFR_LRCONT          BIT(8)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Geert Uytterhoeven]

Hi Prabhakar,

On Fri, Apr 22, 2022 at 7:32 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Drop unused macros SSIFSR_TDC and SSIFSR_RDC.
>
> Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

What does this fix?
Is the real issue that there are 32 FIFO entries, and the TDC and RDC
fields are 6 bits wide, while the mask uses 0x1f instead of 0x3f?

> --- a/sound/soc/sh/rz-ssi.c
> +++ b/sound/soc/sh/rz-ssi.c
> @@ -59,9 +59,7 @@
>  #define SSIFSR_RDC_MASK                0x3f
>  #define SSIFSR_RDC_SHIFT       8
>
> -#define SSIFSR_TDC(x)          (((x) & 0x1f) << 24)
>  #define SSIFSR_TDE             BIT(16)
> -#define SSIFSR_RDC(x)          (((x) & 0x1f) << 8)
>  #define SSIFSR_RDF             BIT(0)
>
>  #define SSIOFR_LRCONT          BIT(8)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Mon, Apr 25, 2022 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Fri, Apr 22, 2022 at 7:32 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Drop unused macros SSIFSR_TDC and SSIFSR_RDC.
> >
> > Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> What does this fix?
> Is the real issue that there are 32 FIFO entries, and the TDC and RDC
> fields are 6 bits wide, while the mask uses 0x1f instead of 0x3f?
>
I was in two minds here as you have already spotted the masks are
incorrect, instead of fixing the masks I choose to drop the macros
itself as they were not used. Let me know what are your thoughts on
this.

Cheers,
Prabhakar

> > --- a/sound/soc/sh/rz-ssi.c
> > +++ b/sound/soc/sh/rz-ssi.c
> > @@ -59,9 +59,7 @@
> >  #define SSIFSR_RDC_MASK                0x3f
> >  #define SSIFSR_RDC_SHIFT       8
> >
> > -#define SSIFSR_TDC(x)          (((x) & 0x1f) << 24)
> >  #define SSIFSR_TDE             BIT(16)
> > -#define SSIFSR_RDC(x)          (((x) & 0x1f) << 8)
> >  #define SSIFSR_RDF             BIT(0)
> >
> >  #define SSIOFR_LRCONT          BIT(8)
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Mon, Apr 25, 2022 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Fri, Apr 22, 2022 at 7:32 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Drop unused macros SSIFSR_TDC and SSIFSR_RDC.
> >
> > Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> What does this fix?
> Is the real issue that there are 32 FIFO entries, and the TDC and RDC
> fields are 6 bits wide, while the mask uses 0x1f instead of 0x3f?
>
I was in two minds here as you have already spotted the masks are
incorrect, instead of fixing the masks I choose to drop the macros
itself as they were not used. Let me know what are your thoughts on
this.

Cheers,
Prabhakar

> > --- a/sound/soc/sh/rz-ssi.c
> > +++ b/sound/soc/sh/rz-ssi.c
> > @@ -59,9 +59,7 @@
> >  #define SSIFSR_RDC_MASK                0x3f
> >  #define SSIFSR_RDC_SHIFT       8
> >
> > -#define SSIFSR_TDC(x)          (((x) & 0x1f) << 24)
> >  #define SSIFSR_TDE             BIT(16)
> > -#define SSIFSR_RDC(x)          (((x) & 0x1f) << 8)
> >  #define SSIFSR_RDF             BIT(0)
> >
> >  #define SSIOFR_LRCONT          BIT(8)
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Geert Uytterhoeven]

Hi Prabhakar,

On Mon, Apr 25, 2022 at 6:09 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Mon, Apr 25, 2022 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Apr 22, 2022 at 7:32 PM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > Drop unused macros SSIFSR_TDC and SSIFSR_RDC.
> > >
> > > Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > What does this fix?
> > Is the real issue that there are 32 FIFO entries, and the TDC and RDC
> > fields are 6 bits wide, while the mask uses 0x1f instead of 0x3f?
> >
> I was in two minds here as you have already spotted the masks are
> incorrect, instead of fixing the masks I choose to drop the macros
> itself as they were not used. Let me know what are your thoughts on
> this.

IC.

I don't have a preference.
So please either remove them, and make it clear they were wrong,
so no one is tempted to just revert the removal to start using the
definitions, or either keep them, and fix the definitions.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Geert Uytterhoeven]

Hi Prabhakar,

On Mon, Apr 25, 2022 at 6:09 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Mon, Apr 25, 2022 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Apr 22, 2022 at 7:32 PM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > Drop unused macros SSIFSR_TDC and SSIFSR_RDC.
> > >
> > > Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > What does this fix?
> > Is the real issue that there are 32 FIFO entries, and the TDC and RDC
> > fields are 6 bits wide, while the mask uses 0x1f instead of 0x3f?
> >
> I was in two minds here as you have already spotted the masks are
> incorrect, instead of fixing the masks I choose to drop the macros
> itself as they were not used. Let me know what are your thoughts on
> this.

IC.

I don't have a preference.
So please either remove them, and make it clear they were wrong,
so no one is tempted to just revert the removal to start using the
definitions, or either keep them, and fix the definitions.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Lad, Prabhakar]

Hi Geert,

On Mon, Apr 25, 2022 at 5:14 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Apr 25, 2022 at 6:09 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Mon, Apr 25, 2022 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Fri, Apr 22, 2022 at 7:32 PM Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > Drop unused macros SSIFSR_TDC and SSIFSR_RDC.
> > > >
> > > > Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > What does this fix?
> > > Is the real issue that there are 32 FIFO entries, and the TDC and RDC
> > > fields are 6 bits wide, while the mask uses 0x1f instead of 0x3f?
> > >
> > I was in two minds here as you have already spotted the masks are
> > incorrect, instead of fixing the masks I choose to drop the macros
> > itself as they were not used. Let me know what are your thoughts on
> > this.
>
> IC.
>
> I don't have a preference.
> So please either remove them, and make it clear they were wrong,
> so no one is tempted to just revert the removal to start using the
> definitions, or either keep them, and fix the definitions.
>
I'll go with dropping them and make it clear they were wrong.

Cheers,
Prabhakar

> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH 1/3] ASoC: sh: rz-ssi: Drop unused macros

[Lad, Prabhakar]

Hi Geert,

On Mon, Apr 25, 2022 at 5:14 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Apr 25, 2022 at 6:09 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Mon, Apr 25, 2022 at 1:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Fri, Apr 22, 2022 at 7:32 PM Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > Drop unused macros SSIFSR_TDC and SSIFSR_RDC.
> > > >
> > > > Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > What does this fix?
> > > Is the real issue that there are 32 FIFO entries, and the TDC and RDC
> > > fields are 6 bits wide, while the mask uses 0x1f instead of 0x3f?
> > >
> > I was in two minds here as you have already spotted the masks are
> > incorrect, instead of fixing the masks I choose to drop the macros
> > itself as they were not used. Let me know what are your thoughts on
> > this.
>
> IC.
>
> I don't have a preference.
> So please either remove them, and make it clear they were wrong,
> so no one is tempted to just revert the removal to start using the
> definitions, or either keep them, and fix the definitions.
>
I'll go with dropping them and make it clear they were wrong.

Cheers,
Prabhakar

> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds