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

[PATCH v2 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

v1->v2:
* Updated patch description for patch 1/3
* Patch 2/3 unchanged
* For patch 3/3 dropped devers action and instead called
  rz_ssi_release_dma_channels() in the error path.

v1:
https://patchwork.kernel.org/project/linux-renesas-soc/cover/
20220421203555.29011-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Lad Prabhakar (3):
  ASoC: sh: rz-ssi: Drop SSIFSR_TDC and SSIFSR_RDC macros
  ASoC: sh: rz-ssi: Propagate error codes returned from
    platform_get_irq_byname()
  ASoC: sh: rz-ssi: Release the DMA channels in rz_ssi_probe() error
    path

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

-- 
2.25.1

[PATCH v2 1/3] ASoC: sh: rz-ssi: Drop SSIFSR_TDC and SSIFSR_RDC macros

[Lad Prabhakar]

The mask values of SSIFSR_TDC and SSIFSR_RDC macros are incorrect and
they are unused in the file so just drop them.

Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2
* Updated commit message
---
 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.25.1

[PATCH v2 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>
---
v1->v2
* No change
---
 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.25.1

[PATCH v2 3/3] ASoC: sh: rz-ssi: Release the DMA channels in rz_ssi_probe() error path

[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 calling rz_ssi_release_dma_channels() in the error path.

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>
---
v1->v2
* Dropped devers action and instead called rz_ssi_release_dma_channels()
  in the error path.
---
 sound/soc/sh/rz-ssi.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index d9a684e71ec3..e392de7a262e 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -976,14 +976,18 @@ 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)
+	if (ssi->irq_int < 0) {
+		rz_ssi_release_dma_channels(ssi);
 		return ssi->irq_int;
+	}
 
 	ret = devm_request_irq(&pdev->dev, ssi->irq_int, &rz_ssi_interrupt,
 			       0, dev_name(&pdev->dev), ssi);
-	if (ret < 0)
+	if (ret < 0) {
+		rz_ssi_release_dma_channels(ssi);
 		return dev_err_probe(&pdev->dev, ret,
 				     "irq request error (int_req)\n");
+	}
 
 	if (!rz_ssi_is_dma_enabled(ssi)) {
 		/* Tx and Rx interrupts (pio only) */
@@ -1011,13 +1015,16 @@ static int rz_ssi_probe(struct platform_device *pdev)
 	}
 
 	ssi->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
-	if (IS_ERR(ssi->rstc))
+	if (IS_ERR(ssi->rstc)) {
+		rz_ssi_release_dma_channels(ssi);
 		return PTR_ERR(ssi->rstc);
+	}
 
 	reset_control_deassert(ssi->rstc);
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_resume_and_get(&pdev->dev);
 	if (ret < 0) {
+		rz_ssi_release_dma_channels(ssi);
 		pm_runtime_disable(ssi->dev);
 		reset_control_assert(ssi->rstc);
 		return dev_err_probe(ssi->dev, ret, "pm_runtime_resume_and_get failed\n");
-- 
2.25.1

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

[Sergey Shtylyov]

Hello!

On 4/26/22 10:49 AM, Lad Prabhakar wrote:

> 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>
> ---
> v1->v2
> * No change
> ---
>  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;

   Why not:

		return dev_err_probe(&pdev->dev, ssi->irq_int,
				     "Unable to get SSI int_req IRQ\n");

[...]
> @@ -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;

   Same here...

>  
>  		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;

   And here...

[...]

MBR, Sergey

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

[Lad, Prabhakar]

Hi Sergey,

Thank you for the review.

On Tue, Apr 26, 2022 at 10:47 AM Sergey Shtylyov <s.shtylyov@omp.ru> wrote:
>
> Hello!
>
> On 4/26/22 10:49 AM, Lad Prabhakar wrote:
>
> > 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>
> > ---
> > v1->v2
> > * No change
> > ---
> >  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;
>
>    Why not:
>
>                 return dev_err_probe(&pdev->dev, ssi->irq_int,
>                                      "Unable to get SSI int_req IRQ\n");
>
That is because platform_get_irq_byname() already does this for us [0]
(also mentioned in the commit message). In case I keep the
dev_err_probe() I'll get two prints for each error.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/platform.c?h=v5.18-rc4#n471

Cheers,
Prabhakar

> [...]
> > @@ -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;
>
>    Same here...
>
> >
> >               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;
>
>    And here...
>
> [...]
>
> MBR, Sergey

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

[Sergey Shtylyov]

On 4/26/22 12:55 PM, Lad, Prabhakar wrote:

[...]
>>> 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>
>>> ---
>>> v1->v2
>>> * No change
>>> ---
>>>  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;
>>
>>    Why not:
>>
>>                 return dev_err_probe(&pdev->dev, ssi->irq_int,
>>                                      "Unable to get SSI int_req IRQ\n");
>>
> That is because platform_get_irq_byname() already does this for us [0]
> (also mentioned in the commit message). In case I keep the
> dev_err_probe() I'll get two prints for each error.
> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/platform.c?h=v5.18-rc4#n471

   Ah! Sorry, didn't read your commit log... :-/
   More shame on me as it was me who added dev_err_probe() call there! :-) 

> Cheers,
> Prabhakar

[...]

MBR, Sergey

Re: [PATCH v2 1/3] ASoC: sh: rz-ssi: Drop SSIFSR_TDC and SSIFSR_RDC macros

[Geert Uytterhoeven]

On Tue, Apr 26, 2022 at 9:49 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> The mask values of SSIFSR_TDC and SSIFSR_RDC macros are incorrect and
> they are unused in the file so just drop them.
>
> Reported-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v1->v2
> * Updated commit message

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 3/3] ASoC: sh: rz-ssi: Release the DMA channels in rz_ssi_probe() error path

[Biju Das]

Hi Lad Prabhakar,

Thanks for the patch.

> Subject: [PATCH v2 3/3] ASoC: sh: rz-ssi: Release the DMA channels in
> rz_ssi_probe() error path
> 
> 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 calling rz_ssi_release_dma_channels() in the error path.
> 
> 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>
> ---
> v1->v2
> * Dropped devers action and instead called rz_ssi_release_dma_channels()
>   in the error path.
> ---
>  sound/soc/sh/rz-ssi.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c index
> d9a684e71ec3..e392de7a262e 100644
> --- a/sound/soc/sh/rz-ssi.c
> +++ b/sound/soc/sh/rz-ssi.c
> @@ -976,14 +976,18 @@ 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)
> +	if (ssi->irq_int < 0) {
> +		rz_ssi_release_dma_channels(ssi);
>  		return ssi->irq_int;
> +	}
> 
>  	ret = devm_request_irq(&pdev->dev, ssi->irq_int, &rz_ssi_interrupt,
>  			       0, dev_name(&pdev->dev), ssi);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		rz_ssi_release_dma_channels(ssi);
>  		return dev_err_probe(&pdev->dev, ret,
>  				     "irq request error (int_req)\n");
> +	}
> 
>  	if (!rz_ssi_is_dma_enabled(ssi)) {
>  		/* Tx and Rx interrupts (pio only) */ @@ -1011,13 +1015,16 @@
> static int rz_ssi_probe(struct platform_device *pdev)
>  	}
> 
>  	ssi->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> -	if (IS_ERR(ssi->rstc))
> +	if (IS_ERR(ssi->rstc)) {
> +		rz_ssi_release_dma_channels(ssi);
>  		return PTR_ERR(ssi->rstc);
> +	}

May be we could move reset handle get above DMA channel request??

Cheers,
Biju

> 
>  	reset_control_deassert(ssi->rstc);
>  	pm_runtime_enable(&pdev->dev);
>  	ret = pm_runtime_resume_and_get(&pdev->dev);
>  	if (ret < 0) {
> +		rz_ssi_release_dma_channels(ssi);
>  		pm_runtime_disable(ssi->dev);
>  		reset_control_assert(ssi->rstc);
>  		return dev_err_probe(ssi->dev, ret, "pm_runtime_resume_and_get
> failed\n");
> --
> 2.25.1

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

[Mark Brown]

On Tue, 26 Apr 2022 08:49:19 +0100, Lad Prabhakar wrote:
> This patch series aims to fix trivial issues found in rz-ssi driver.
> 
> Cheers,
> Prabhakar
> 
> v1->v2:
> * Updated patch description for patch 1/3
> * Patch 2/3 unchanged
> * For patch 3/3 dropped devers action and instead called
>   rz_ssi_release_dma_channels() in the error path.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/3] ASoC: sh: rz-ssi: Drop SSIFSR_TDC and SSIFSR_RDC macros
      commit: 17a1fef58c65ec9c9a15dd60386712567ff28d45
[2/3] ASoC: sh: rz-ssi: Propagate error codes returned from platform_get_irq_byname()
      commit: 91686a3984f34df0ab844cdbaa7e4d9621129f5d
[3/3] ASoC: sh: rz-ssi: Release the DMA channels in rz_ssi_probe() error path
      commit: 767e6f26204d3f5406630e86b720d01818b8616d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark