| Message ID | a55409b0-9839-d9b5-5c90-606c22d0c58b@users.sourceforge.net (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Thu, Nov 2, 2017 at 2:00 PM, SF Markus Elfring <elfring@users.sourceforge.net> wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Thu, 2 Nov 2017 13:35:17 +0100 > > * Adjust a jump target so that a specific error message is stored only once > at the end of this function implementation. > > * Replace two calls of the function "dev_err" by goto statements. > > This issue was detected by using the Coccinelle software. > > Link: https://lkml.org/lkml/2017/11/2/140 > Link: https://lkml.kernel.org/r/<20171102081239.bzaunax3uotejqja@verge.net.au> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > > v2: > An other jump label was requested by Simon Horman. > > drivers/pci/host/pcie-rcar.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > index 12796eccb2be..f3c1ffdf8072 100644 > --- a/drivers/pci/host/pcie-rcar.c > +++ b/drivers/pci/host/pcie-rcar.c > @@ -873,18 +873,14 @@ static int rcar_pcie_enable_msi(struct rcar_pcie *pcie) > err = devm_request_irq(dev, msi->irq1, rcar_pcie_msi_irq, > IRQF_SHARED | IRQF_NO_THREAD, > rcar_msi_irq_chip.name, pcie); > - if (err < 0) { > - dev_err(dev, "failed to request IRQ: %d\n", err); > - goto err; > - } > + if (err < 0) > + goto err_remove_domain; > > err = devm_request_irq(dev, msi->irq2, rcar_pcie_msi_irq, > IRQF_SHARED | IRQF_NO_THREAD, > rcar_msi_irq_chip.name, pcie); > - if (err < 0) { > - dev_err(dev, "failed to request IRQ: %d\n", err); > - goto err; > - } > + if (err < 0) > + goto err_remove_domain; > > /* setup MSI data target */ > msi->pages = __get_free_pages(GFP_KERNEL, 0); > @@ -898,7 +894,8 @@ static int rcar_pcie_enable_msi(struct rcar_pcie *pcie) > > return 0; > > -err: > +err_remove_domain: > + dev_err(dev, "failed to request IRQ: %d\n", err); Moving the error message is confusing and counter-productive: if you need to add another error case + goto err_remove_domain later, you'll have to move it back to all the older cases. Hence NAKed-by: Geert Uytterhoeven <geert@linux-m68k.org> > irq_domain_remove(msi->domain); > return err; > } 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
diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c index 12796eccb2be..f3c1ffdf8072 100644 --- a/drivers/pci/host/pcie-rcar.c +++ b/drivers/pci/host/pcie-rcar.c @@ -873,18 +873,14 @@ static int rcar_pcie_enable_msi(struct rcar_pcie *pcie) err = devm_request_irq(dev, msi->irq1, rcar_pcie_msi_irq, IRQF_SHARED | IRQF_NO_THREAD, rcar_msi_irq_chip.name, pcie); - if (err < 0) { - dev_err(dev, "failed to request IRQ: %d\n", err); - goto err; - } + if (err < 0) + goto err_remove_domain; err = devm_request_irq(dev, msi->irq2, rcar_pcie_msi_irq, IRQF_SHARED | IRQF_NO_THREAD, rcar_msi_irq_chip.name, pcie); - if (err < 0) { - dev_err(dev, "failed to request IRQ: %d\n", err); - goto err; - } + if (err < 0) + goto err_remove_domain; /* setup MSI data target */ msi->pages = __get_free_pages(GFP_KERNEL, 0); @@ -898,7 +894,8 @@ static int rcar_pcie_enable_msi(struct rcar_pcie *pcie) return 0; -err: +err_remove_domain: + dev_err(dev, "failed to request IRQ: %d\n", err); irq_domain_remove(msi->domain); return err; }