| Message ID | 20220426074922.13319-3-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | ASoC: sh: rz-ssi: Trivial fixes | expand |
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/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
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
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,
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(-)