| Message ID | 20211224142917.6966-4-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | serial: Use platform_get_irq*() variants to fetch IRQ's | expand |
On 12/24/2021 6:29 AM, Lad Prabhakar wrote: > In case of failures brcmuart_probe() always returned -ENODEV, this > isn't correct for example platform_get_irq_byname() may return > -EPROBE_DEFER to handle such cases propagate error codes in > brcmuart_probe() in case of failures. > > Fixes: 41a469482de25 ("serial: 8250: Add new 8250-core based Broadcom STB driver") > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Hi Andy, Thank you for the review. On Sat, Dec 25, 2021 at 11:20 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > > On Friday, December 24, 2021, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: >> >> In case of failures brcmuart_probe() always returned -ENODEV, this >> isn't correct for example platform_get_irq_byname() may return >> -EPROBE_DEFER to handle such cases propagate error codes in >> brcmuart_probe() in case of failures. >> >> Fixes: 41a469482de25 ("serial: 8250: Add new 8250-core based Broadcom STB driver") >> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >> --- >> drivers/tty/serial/8250/8250_bcm7271.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c >> index 7ecfcc650d28..cc60a7874e8b 100644 >> --- a/drivers/tty/serial/8250/8250_bcm7271.c >> +++ b/drivers/tty/serial/8250/8250_bcm7271.c >> @@ -1074,14 +1074,18 @@ static int brcmuart_probe(struct platform_device *pdev) >> priv->rx_bufs = dma_alloc_coherent(dev, >> priv->rx_size, >> &priv->rx_addr, GFP_KERNEL); >> - if (!priv->rx_bufs) >> + if (!priv->rx_bufs) { >> + ret = -EINVAL; > > > > For memory allocation we usually return -ENOMEM. > Agreed, will fix that. Cheers, Prabhakar >> >> goto err; >> + } >> priv->tx_size = UART_XMIT_SIZE; >> priv->tx_buf = dma_alloc_coherent(dev, >> priv->tx_size, >> &priv->tx_addr, GFP_KERNEL); >> - if (!priv->tx_buf) >> + if (!priv->tx_buf) { >> + ret = -EINVAL; >> goto err; >> + } >> } >> >> ret = serial8250_register_8250_port(&up); >> @@ -1095,6 +1099,7 @@ static int brcmuart_probe(struct platform_device *pdev) >> if (priv->dma_enabled) { >> dma_irq = platform_get_irq_byname(pdev, "dma"); >> if (dma_irq < 0) { >> + ret = dma_irq; >> dev_err(dev, "no IRQ resource info\n"); >> goto err1; >> } >> @@ -1114,7 +1119,7 @@ static int brcmuart_probe(struct platform_device *pdev) >> err: >> brcmuart_free_bufs(dev, priv); >> brcmuart_arbitration(priv, 0); >> - return -ENODEV; >> + return ret; >> } >> >> static int brcmuart_remove(struct platform_device *pdev) >> -- >> 2.17.1 >> > > > -- > With Best Regards, > Andy Shevchenko > >
On Sat, Dec 25, 2021 at 12:20:02PM +0000, Lad, Prabhakar wrote: > Hi Andy, > > Thank you for the review. > > On Sat, Dec 25, 2021 at 11:20 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > > > > > > > On Friday, December 24, 2021, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > >> > >> In case of failures brcmuart_probe() always returned -ENODEV, this > >> isn't correct for example platform_get_irq_byname() may return > >> -EPROBE_DEFER to handle such cases propagate error codes in > >> brcmuart_probe() in case of failures. > >> > >> Fixes: 41a469482de25 ("serial: 8250: Add new 8250-core based Broadcom STB driver") > >> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > >> --- > >> drivers/tty/serial/8250/8250_bcm7271.c | 11 ++++++++--- > >> 1 file changed, 8 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c > >> index 7ecfcc650d28..cc60a7874e8b 100644 > >> --- a/drivers/tty/serial/8250/8250_bcm7271.c > >> +++ b/drivers/tty/serial/8250/8250_bcm7271.c > >> @@ -1074,14 +1074,18 @@ static int brcmuart_probe(struct platform_device *pdev) > >> priv->rx_bufs = dma_alloc_coherent(dev, > >> priv->rx_size, > >> &priv->rx_addr, GFP_KERNEL); > >> - if (!priv->rx_bufs) > >> + if (!priv->rx_bufs) { > >> + ret = -EINVAL; > > > > > > > > For memory allocation we usually return -ENOMEM. > > > Agreed, will fix that. Just send a follow-on patch for that, thanks. greg k-h
diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c index 7ecfcc650d28..cc60a7874e8b 100644 --- a/drivers/tty/serial/8250/8250_bcm7271.c +++ b/drivers/tty/serial/8250/8250_bcm7271.c @@ -1074,14 +1074,18 @@ static int brcmuart_probe(struct platform_device *pdev) priv->rx_bufs = dma_alloc_coherent(dev, priv->rx_size, &priv->rx_addr, GFP_KERNEL); - if (!priv->rx_bufs) + if (!priv->rx_bufs) { + ret = -EINVAL; goto err; + } priv->tx_size = UART_XMIT_SIZE; priv->tx_buf = dma_alloc_coherent(dev, priv->tx_size, &priv->tx_addr, GFP_KERNEL); - if (!priv->tx_buf) + if (!priv->tx_buf) { + ret = -EINVAL; goto err; + } } ret = serial8250_register_8250_port(&up); @@ -1095,6 +1099,7 @@ static int brcmuart_probe(struct platform_device *pdev) if (priv->dma_enabled) { dma_irq = platform_get_irq_byname(pdev, "dma"); if (dma_irq < 0) { + ret = dma_irq; dev_err(dev, "no IRQ resource info\n"); goto err1; } @@ -1114,7 +1119,7 @@ static int brcmuart_probe(struct platform_device *pdev) err: brcmuart_free_bufs(dev, priv); brcmuart_arbitration(priv, 0); - return -ENODEV; + return ret; } static int brcmuart_remove(struct platform_device *pdev)
In case of failures brcmuart_probe() always returned -ENODEV, this isn't correct for example platform_get_irq_byname() may return -EPROBE_DEFER to handle such cases propagate error codes in brcmuart_probe() in case of failures. Fixes: 41a469482de25 ("serial: 8250: Add new 8250-core based Broadcom STB driver") Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- drivers/tty/serial/8250/8250_bcm7271.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)