| Message ID | 1589180996-618-1-git-send-email-Anson.Huang@nxp.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | aa49d8e8b2dfc112f7de9c58698ae06b2101c73c |
| Headers | show |
| Series | tty: serial: imx: Add return value check for platform_get_irq() | expand |
Hello Anson, On Mon, May 11, 2020 at 03:09:56PM +0800, Anson Huang wrote: > RX irq is required, so add return value check for platform_get_irq(). > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > --- > drivers/tty/serial/imx.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index f4d6810..f4023d9 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -2252,6 +2252,8 @@ static int imx_uart_probe(struct platform_device *pdev) > return PTR_ERR(base); > > rxirq = platform_get_irq(pdev, 0); > + if (rxirq < 0) > + return rxirq; > txirq = platform_get_irq_optional(pdev, 1); > rtsirq = platform_get_irq_optional(pdev, 2); I'm not sure we need such a check as devm_request_irq fails if the return value of platform_get_irq() is bogus. But if we decide this construct is good enough, the error reporting needs some love as currently it emits two error messages which is confusing. Best regards Uwe
Hi, Uwe > Subject: Re: [PATCH] tty: serial: imx: Add return value check for > platform_get_irq() > > Hello Anson, > > On Mon, May 11, 2020 at 03:09:56PM +0800, Anson Huang wrote: > > RX irq is required, so add return value check for platform_get_irq(). > > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > > --- > > drivers/tty/serial/imx.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index > > f4d6810..f4023d9 100644 > > --- a/drivers/tty/serial/imx.c > > +++ b/drivers/tty/serial/imx.c > > @@ -2252,6 +2252,8 @@ static int imx_uart_probe(struct platform_device > *pdev) > > return PTR_ERR(base); > > > > rxirq = platform_get_irq(pdev, 0); > > + if (rxirq < 0) > > + return rxirq; > > txirq = platform_get_irq_optional(pdev, 1); > > rtsirq = platform_get_irq_optional(pdev, 2); > > I'm not sure we need such a check as devm_request_irq fails if the return value > of platform_get_irq() is bogus. > > But if we decide this construct is good enough, the error reporting needs some > love as currently it emits two error messages which is confusing. From the driver, the RX IRQ is always required, if it failed in platform_get_irq(), then the rest of the code is NOT necessary to be executed, and also I am NOT sure if platform_get_irq() failed, the devm_request_irq will always failed? Not very understand about your last question, the platform_get_irq() already has error message printed out, so no additional error message is needed in the check. If looking through all other drivers, most of the platform_get_irq() are having the return value check, if it failed in platform_get_irq(), driver can just return error from .probe(). Anson
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index f4d6810..f4023d9 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -2252,6 +2252,8 @@ static int imx_uart_probe(struct platform_device *pdev) return PTR_ERR(base); rxirq = platform_get_irq(pdev, 0); + if (rxirq < 0) + return rxirq; txirq = platform_get_irq_optional(pdev, 1); rtsirq = platform_get_irq_optional(pdev, 2);
RX irq is required, so add return value check for platform_get_irq(). Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- drivers/tty/serial/imx.c | 2 ++ 1 file changed, 2 insertions(+)