| Message ID | 146506d93c96b842422d31a90b5d23c98b70a111.1668155425.git.christophe.leroy@csgroup.eu |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ata: sata_dwc_460ex: Check !irq instead of irq == NO_IRQ | expand |
On 11/11/22 17:30, Christophe Leroy wrote: > NO_IRQ is a relic from the old days. It is not used anymore in core > functions. By the way, function irq_of_parse_and_map() returns value 0 > on error. > > In some drivers, NO_IRQ is erroneously used to check the return of > irq_of_parse_and_map(). > > It is not a real bug today because the only architectures using the > drivers being fixed by this patch define NO_IRQ as 0, but there are > architectures which define NO_IRQ as -1. If one day those > architectures start using the non fixed drivers, there will be a > problem. > > Long time ago Linus advocated for not using NO_IRQ, see > https://lkml.org/lkml/2005/11/21/221 . He re-iterated the same view > recently in https://lkml.org/lkml/2022/10/12/622 > > So test !irq instead of tesing irq == NO_IRQ. > > And remove the fallback definition of NO_IRQ at the top of the file. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Applied to for-6.2. Thanks ! > --- > v2: Also removed the #ifndef NO_IRQ #define NO_IRQ 0 at the top > --- > drivers/ata/sata_dwc_460ex.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c > index e3263e961045..7b7e1516dbdd 100644 > --- a/drivers/ata/sata_dwc_460ex.c > +++ b/drivers/ata/sata_dwc_460ex.c > @@ -42,10 +42,6 @@ > #define sata_dwc_writel(a, v) writel_relaxed(v, a) > #define sata_dwc_readl(a) readl_relaxed(a) > > -#ifndef NO_IRQ > -#define NO_IRQ 0 > -#endif > - > #define AHB_DMA_BRST_DFLT 64 /* 16 data items burst length */ > > enum { > @@ -242,7 +238,7 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev, > > /* Get SATA DMA interrupt number */ > hsdev->dma->irq = irq_of_parse_and_map(np, 1); > - if (hsdev->dma->irq == NO_IRQ) { > + if (!hsdev->dma->irq) { > dev_err(dev, "no SATA DMA irq\n"); > return -ENODEV; > } > @@ -1180,7 +1176,7 @@ static int sata_dwc_probe(struct platform_device *ofdev) > > /* Get SATA interrupt number */ > irq = irq_of_parse_and_map(np, 0); > - if (irq == NO_IRQ) { > + if (!irq) { > dev_err(dev, "no SATA DMA irq\n"); > return -ENODEV; > }
diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c index e3263e961045..7b7e1516dbdd 100644 --- a/drivers/ata/sata_dwc_460ex.c +++ b/drivers/ata/sata_dwc_460ex.c @@ -42,10 +42,6 @@ #define sata_dwc_writel(a, v) writel_relaxed(v, a) #define sata_dwc_readl(a) readl_relaxed(a) -#ifndef NO_IRQ -#define NO_IRQ 0 -#endif - #define AHB_DMA_BRST_DFLT 64 /* 16 data items burst length */ enum { @@ -242,7 +238,7 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev, /* Get SATA DMA interrupt number */ hsdev->dma->irq = irq_of_parse_and_map(np, 1); - if (hsdev->dma->irq == NO_IRQ) { + if (!hsdev->dma->irq) { dev_err(dev, "no SATA DMA irq\n"); return -ENODEV; } @@ -1180,7 +1176,7 @@ static int sata_dwc_probe(struct platform_device *ofdev) /* Get SATA interrupt number */ irq = irq_of_parse_and_map(np, 0); - if (irq == NO_IRQ) { + if (!irq) { dev_err(dev, "no SATA DMA irq\n"); return -ENODEV; }
NO_IRQ is a relic from the old days. It is not used anymore in core functions. By the way, function irq_of_parse_and_map() returns value 0 on error. In some drivers, NO_IRQ is erroneously used to check the return of irq_of_parse_and_map(). It is not a real bug today because the only architectures using the drivers being fixed by this patch define NO_IRQ as 0, but there are architectures which define NO_IRQ as -1. If one day those architectures start using the non fixed drivers, there will be a problem. Long time ago Linus advocated for not using NO_IRQ, see https://lkml.org/lkml/2005/11/21/221 . He re-iterated the same view recently in https://lkml.org/lkml/2022/10/12/622 So test !irq instead of tesing irq == NO_IRQ. And remove the fallback definition of NO_IRQ at the top of the file. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- v2: Also removed the #ifndef NO_IRQ #define NO_IRQ 0 at the top --- drivers/ata/sata_dwc_460ex.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)