[PATCH net-next v3] net: ipa: Remove redundant comparison with zero

[PATCH net-next v3] net: ipa: Remove redundant comparison with zero

[Kang Minchul]

platform_get_irq_byname() returns non-zero IRQ number on success,
and negative error number on failure.

So comparing return value with zero is unnecessary.

Signed-off-by: Kang Minchul <tegongkang@gmail.com>
---
 drivers/net/ipa/gsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index bea2da1c4c51..e05e94bd9ba0 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1302,7 +1302,7 @@ static int gsi_irq_init(struct gsi *gsi, struct platform_device *pdev)
 	int ret;
 
 	ret = platform_get_irq_byname(pdev, "gsi");
-	if (ret <= 0)
+	if (ret < 0)
 		return ret ? : -EINVAL;
 
 	gsi->irq = ret;
-- 
2.34.1

Re: [PATCH net-next v3] net: ipa: Remove redundant comparison with zero

[Alex Elder]

On 12/16/22 3:11 PM, Kang Minchul wrote:
> platform_get_irq_byname() returns non-zero IRQ number on success,
> and negative error number on failure.
> 
> So comparing return value with zero is unnecessary.
> 
> Signed-off-by: Kang Minchul <tegongkang@gmail.com>
> ---
>   drivers/net/ipa/gsi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
> index bea2da1c4c51..e05e94bd9ba0 100644
> --- a/drivers/net/ipa/gsi.c
> +++ b/drivers/net/ipa/gsi.c
> @@ -1302,7 +1302,7 @@ static int gsi_irq_init(struct gsi *gsi, struct platform_device *pdev)
>   	int ret;
>   
>   	ret = platform_get_irq_byname(pdev, "gsi");
> -	if (ret <= 0)
> +	if (ret < 0)
>   		return ret ? : -EINVAL;

In doing this, you assume platform_get_irq_byname() never
returns 0.  I accept that assumption now.  But in that case,
ret will *always* be non-zero if the branch is taken, so the
next line should simply be:

		return ret;

There are two other places in the IPA driver that follow exactly
the same pattern of getting the IRQ number and handling the
possibility that the value returned is (erroneously) 0.  If
you're making this change in one place, please do so in all
of them.

Frankly though, I think this change adds little value, and I
would be content to just leave everything as-is...

					-Alex

>   
>   	gsi->irq = ret;

Re: [PATCH net-next v3] net: ipa: Remove redundant comparison with zero

[Kang Minchul]

2022년 12월 17일 (토) 오전 6:34, Alex Elder <elder@ieee.org>님이 작성:
>
> On 12/16/22 3:11 PM, Kang Minchul wrote:
> > platform_get_irq_byname() returns non-zero IRQ number on success,
> > and negative error number on failure.
> >
> > So comparing return value with zero is unnecessary.
> >
> > Signed-off-by: Kang Minchul <tegongkang@gmail.com>
> > ---
> >   drivers/net/ipa/gsi.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
> > index bea2da1c4c51..e05e94bd9ba0 100644
> > --- a/drivers/net/ipa/gsi.c
> > +++ b/drivers/net/ipa/gsi.c
> > @@ -1302,7 +1302,7 @@ static int gsi_irq_init(struct gsi *gsi, struct platform_device *pdev)
> >       int ret;
> >
> >       ret = platform_get_irq_byname(pdev, "gsi");
> > -     if (ret <= 0)
> > +     if (ret < 0)
> >               return ret ? : -EINVAL;
>
> In doing this, you assume platform_get_irq_byname() never
> returns 0.  I accept that assumption now.  But in that case,
> ret will *always* be non-zero if the branch is taken, so the
> next line should simply be:
>
>                 return ret;
>
> There are two other places in the IPA driver that follow exactly
> the same pattern of getting the IRQ number and handling the
> possibility that the value returned is (erroneously) 0.  If
> you're making this change in one place, please do so in all
> of them.
>
> Frankly though, I think this change adds little value, and I
> would be content to just leave everything as-is...
>
>                                         -Alex
>
> >
> >       gsi->irq = ret;
>

Hmm, yes.
I think you're right.
I don't think this code is worth much either.
Please ignore this patch and leave everything as-is.

Thanks for your feedback though :)

Kang Minchul