| Message ID | 124bbda3-7ccf-4e6b-b30d-7115e1c2620f@stanley.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nvme: apple: remove some dead code | expand |
On Fri, 12 Jul 2024 at 15:13, Dan Carpenter <dan.carpenter@linaro.org> wrote: > > platform_get_irq() never returns zero so we can remove his dead code. > Checking for zero is a historical artifact from over ten years ago. > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> There's quite a few return paths in platform_get_irq_optional, are we sure it can never be zero? Not calling out a specific case here, but it's not so clear to me how we can guarantee platform_get_irq() is never zero, Is mise le meas/Regards, Eric Curtin > --- > drivers/nvme/host/apple.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c > index b1387dc459a3..f5a3a4e8b1e5 100644 > --- a/drivers/nvme/host/apple.c > +++ b/drivers/nvme/host/apple.c > @@ -1417,10 +1417,6 @@ static struct apple_nvme *apple_nvme_alloc(struct platform_device *pdev) > ret = anv->irq; > goto put_dev; > } > - if (!anv->irq) { > - ret = -ENXIO; > - goto put_dev; > - } > > anv->mmio_coproc = devm_platform_ioremap_resource_byname(pdev, "ans"); > if (IS_ERR(anv->mmio_coproc)) { > -- > 2.43.0 > >
On Fri, Jul 12, 2024 at 03:29:21PM +0100, Eric Curtin wrote: > On Fri, 12 Jul 2024 at 15:13, Dan Carpenter <dan.carpenter@linaro.org> wrote: > > > > platform_get_irq() never returns zero so we can remove his dead code. > > Checking for zero is a historical artifact from over ten years ago. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > > There's quite a few return paths in platform_get_irq_optional, are we > sure it can never be zero? > > Not calling out a specific case here, but it's not so clear to me how > we can guarantee platform_get_irq() is never zero, > The platform_get_irq() function has a comment which describes how the error handling should work. I wrote a blog about this: https://staticthinking.wordpress.com/2023/08/07/writing-a-check-for-zero-irq-error-codes/ TLDR; platform_get_irq() used to return zero on error but it changed in 2006. I believe someone told me the historical situation was actually worse than I described where the error return wasn't always zero but depended on the arch so sometimes it was -1... Then after 2006 zero was success for a while because there was some hardware where zero was a valid IRQ. But now zero is not a valid IRQ. I think Linus has said that zero is a stupid IRQ number and support for that hardware was removed. So now it never returns zero and never will again. There are still some xxxxxxx_get_irq() which return zero on error, and those cause quite a bit of mixups. Last year there was even one which had a comment similar to platform_get_irq() that said to check for negatives but it returned zero on failure sometimes. :P regards, dan carpenter
diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c index b1387dc459a3..f5a3a4e8b1e5 100644 --- a/drivers/nvme/host/apple.c +++ b/drivers/nvme/host/apple.c @@ -1417,10 +1417,6 @@ static struct apple_nvme *apple_nvme_alloc(struct platform_device *pdev) ret = anv->irq; goto put_dev; } - if (!anv->irq) { - ret = -ENXIO; - goto put_dev; - } anv->mmio_coproc = devm_platform_ioremap_resource_byname(pdev, "ans"); if (IS_ERR(anv->mmio_coproc)) {
platform_get_irq() never returns zero so we can remove his dead code. Checking for zero is a historical artifact from over ten years ago. Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/nvme/host/apple.c | 4 ---- 1 file changed, 4 deletions(-)