[PATCH] nvme: apple: remove some dead code

[PATCH] nvme: apple: remove some dead code

[Dan Carpenter]

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(-)

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

Re: [PATCH] nvme: apple: remove some dead code

[Eric Curtin]

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
>
>

Re: [PATCH] nvme: apple: remove some dead code

[Dan Carpenter]

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