[PATCH v2 0/3] ASoC: sh: rz-ssi: Trivial fixes
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(-)
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)
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
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,
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
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/driv...
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
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/driv...
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
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");
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);- }
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);failed\n");
2.25.1
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
participants (6)
-
Biju Das -
Geert Uytterhoeven -
Lad Prabhakar -
Lad, Prabhakar -
Mark Brown -
Sergey Shtylyov