2021-08-17 02:56:30

by Yang Yingliang

Subject: [PATCH -next] ASoC: sh: rz-ssi: Fix return value check in rz_ssi_dma_request()

In case of error, the function dma_request_chan() returns ERR_PTR()
and never returns NULL. Set 'dma_ch' to NULL, if dma_request_chan()
returns error, so the code using 'dma_ch' can work correctly.

Fixes: 26ac471c5354 ("ASoC: sh: rz-ssi: Add SSI DMAC support")
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Yang Yingliang <[email protected]>
---
sound/soc/sh/rz-ssi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index ea8d33ede5d2..5ec78fd94d94 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -676,11 +676,17 @@ static void rz_ssi_release_dma_channels(struct rz_ssi_priv *ssi)
static int rz_ssi_dma_request(struct rz_ssi_priv *ssi, struct device *dev)
{
ssi->playback.dma_ch = dma_request_chan(dev, "tx");
+ if (IS_ERR(ssi->playback.dma_ch))
+ ssi->playback.dma_ch = NULL;
ssi->capture.dma_ch = dma_request_chan(dev, "rx");
+ if (IS_ERR(ssi->capture.dma_ch))
+ ssi->capture.dma_ch = NULL;
if (!ssi->playback.dma_ch && !ssi->capture.dma_ch) {
ssi->playback.dma_ch = dma_request_chan(dev, "rt");
- if (!ssi->playback.dma_ch)
+ if (IS_ERR(ssi->playback.dma_ch)) {
+ ssi->playback.dma_ch = NULL;
goto no_dma;
+ }

ssi->dma_rt = true;
}
--
2.25.1

2021-08-17 06:43:20

by Biju Das

Subject: RE: [PATCH -next] ASoC: sh: rz-ssi: Fix return value check in rz_ssi_dma_request()

Hi Yang Yingliang,

Thanks for your patch.

Already I have posted a patch for fixing it.

https://patchwork.kernel.org/project/alsa-devel/patch/[email protected]/

Since you posted a similar patch as Reported-by: Hulk Robot <[email protected]>,
I am adding my Rb tag for this patch.

Regards,
Biju


> -----Original Message-----
> From: Yang Yingliang <[email protected]>
> Sent: 17 August 2021 04:00
> To: [email protected]; [email protected]
> Cc: [email protected]; Biju Das <[email protected]>; Prabhakar
> Mahadev Lad <[email protected]>; [email protected]
> Subject: [PATCH -next] ASoC: sh: rz-ssi: Fix return value check in
> rz_ssi_dma_request()
>
> In case of error, the function dma_request_chan() returns ERR_PTR() and
> never returns NULL. Set 'dma_ch' to NULL, if dma_request_chan() returns
> error, so the code using 'dma_ch' can work correctly.
>
> Fixes: 26ac471c5354 ("ASoC: sh: rz-ssi: Add SSI DMAC support")
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Yang Yingliang <[email protected]>

Reviewed-by: Biju Das <[email protected]>

> ---
> sound/soc/sh/rz-ssi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c index
> ea8d33ede5d2..5ec78fd94d94 100644
> --- a/sound/soc/sh/rz-ssi.c
> +++ b/sound/soc/sh/rz-ssi.c
> @@ -676,11 +676,17 @@ static void rz_ssi_release_dma_channels(struct
> rz_ssi_priv *ssi) static int rz_ssi_dma_request(struct rz_ssi_priv *ssi,
> struct device *dev) {
> ssi->playback.dma_ch = dma_request_chan(dev, "tx");
> + if (IS_ERR(ssi->playback.dma_ch))
> + ssi->playback.dma_ch = NULL;
> ssi->capture.dma_ch = dma_request_chan(dev, "rx");
> + if (IS_ERR(ssi->capture.dma_ch))
> + ssi->capture.dma_ch = NULL;
> if (!ssi->playback.dma_ch && !ssi->capture.dma_ch) {
> ssi->playback.dma_ch = dma_request_chan(dev, "rt");
> - if (!ssi->playback.dma_ch)
> + if (IS_ERR(ssi->playback.dma_ch)) {
> + ssi->playback.dma_ch = NULL;
> goto no_dma;
> + }
>
> ssi->dma_rt = true;
> }
> --
> 2.25.1

2021-08-18 06:45:26

by Biju Das

Subject: RE: [PATCH -next] ASoC: sh: rz-ssi: Fix return value check in rz_ssi_dma_request()

Hi Yang Yingliang,

Still some improvements to be done on this patch.
If one of the dma channel is failed, we should fallback that channel to PIO.

Something like below. I will post a new patch after testing this.

ssi->playback.transfer = rz_ssi_pio_send;
ssi->capture.transfer = rz_ssi_pio_recv;

/* Detect DMA support */
ret = rz_ssi_dma_request(ssi, &pdev->dev);
if (ret < 0) {
dev_warn(&pdev->dev, "DMA not available, using PIO\n");
} else {
dev_info(&pdev->dev, "DMA enabled");
if (ssi->playback.dma_ch)
ssi->playback.transfer = rz_ssi_dma_transfer;
if (ssi->capture.dma_ch)
ssi->capture.transfer = rz_ssi_dma_transfer;
}

Regards,
Biju


> Subject: RE: [PATCH -next] ASoC: sh: rz-ssi: Fix return value check in
> rz_ssi_dma_request()
>
> Hi Yang Yingliang,
>
> Thanks for your patch.
>
> Already I have posted a patch for fixing it.
>
> https://patchwork.kernel.org/project/alsa-
> devel/patch/[email protected]/
>
> Since you posted a similar patch as Reported-by: Hulk Robot
> <[email protected]>, I am adding my Rb tag for this patch.
>
> Regards,
> Biju
>
>
> > -----Original Message-----
> > From: Yang Yingliang <[email protected]>
> > Sent: 17 August 2021 04:00
> > To: [email protected]; [email protected]
> > Cc: [email protected]; Biju Das <[email protected]>;
> > Prabhakar Mahadev Lad <[email protected]>;
> > [email protected]
> > Subject: [PATCH -next] ASoC: sh: rz-ssi: Fix return value check in
> > rz_ssi_dma_request()
> >
> > In case of error, the function dma_request_chan() returns ERR_PTR()
> > and never returns NULL. Set 'dma_ch' to NULL, if dma_request_chan()
> > returns error, so the code using 'dma_ch' can work correctly.
> >
> > Fixes: 26ac471c5354 ("ASoC: sh: rz-ssi: Add SSI DMAC support")
> > Reported-by: Hulk Robot <[email protected]>
> > Signed-off-by: Yang Yingliang <[email protected]>
>
> Reviewed-by: Biju Das <[email protected]>
>
> > ---
> > sound/soc/sh/rz-ssi.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c index
> > ea8d33ede5d2..5ec78fd94d94 100644
> > --- a/sound/soc/sh/rz-ssi.c
> > +++ b/sound/soc/sh/rz-ssi.c
> > @@ -676,11 +676,17 @@ static void rz_ssi_release_dma_channels(struct
> > rz_ssi_priv *ssi) static int rz_ssi_dma_request(struct rz_ssi_priv
> > *ssi, struct device *dev) {
> > ssi->playback.dma_ch = dma_request_chan(dev, "tx");
> > + if (IS_ERR(ssi->playback.dma_ch))
> > + ssi->playback.dma_ch = NULL;
> > ssi->capture.dma_ch = dma_request_chan(dev, "rx");
> > + if (IS_ERR(ssi->capture.dma_ch))
> > + ssi->capture.dma_ch = NULL;
> > if (!ssi->playback.dma_ch && !ssi->capture.dma_ch) {
> > ssi->playback.dma_ch = dma_request_chan(dev, "rt");
> > - if (!ssi->playback.dma_ch)
> > + if (IS_ERR(ssi->playback.dma_ch)) {
> > + ssi->playback.dma_ch = NULL;
> > goto no_dma;
> > + }
> >
> > ssi->dma_rt = true;
> > }
> > --
> > 2.25.1