[PATCH] net: smc91x: propagate irq return code

[PATCH] net: smc91x: propagate irq return code

[Robert Jarzmik]

The smc91x driver doesn't honor the probe deferral mechanism when the
interrupt source is not yet available, such as one provided by a gpio
controller not probed.

Fix this by propagating the platform_get_irq() error code as the probe
return value.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/net/ethernet/smsc/smc91x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c
index 0e2fc1a844ab..43ab7aa31a79 100644
--- a/drivers/net/ethernet/smsc/smc91x.c
+++ b/drivers/net/ethernet/smsc/smc91x.c
@@ -2343,7 +2343,7 @@ static int smc_drv_probe(struct platform_device *pdev)
 
 	ndev->irq = platform_get_irq(pdev, 0);
 	if (ndev->irq <= 0) {
-		ret = -ENODEV;
+		ret = ndev->irq;
 		goto out_release_io;
 	}
 	/*
-- 
2.1.4

Re: [PATCH] net: smc91x: propagate irq return code

[Sergei Shtylyov]

Hello.

On 2/1/2016 1:46 AM, Robert Jarzmik wrote:

> The smc91x driver doesn't honor the probe deferral mechanism when the
> interrupt source is not yet available, such as one provided by a gpio
> controller not probed.
>
> Fix this by propagating the platform_get_irq() error code as the probe
> return value.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>   drivers/net/ethernet/smsc/smc91x.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c
> index 0e2fc1a844ab..43ab7aa31a79 100644
> --- a/drivers/net/ethernet/smsc/smc91x.c
> +++ b/drivers/net/ethernet/smsc/smc91x.c
> @@ -2343,7 +2343,7 @@ static int smc_drv_probe(struct platform_device *pdev)
>
>   	ndev->irq = platform_get_irq(pdev, 0);
>   	if (ndev->irq <= 0) {
> -		ret = -ENODEV;
> +		ret = ndev->irq;

    What if 'ndev->irq' does equal 0?

MBR, Sergei

Re: [PATCH] net: smc91x: propagate irq return code

[Robert Jarzmik]

Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes:

> Hello.
>
> On 2/1/2016 1:46 AM, Robert Jarzmik wrote:
>
>> The smc91x driver doesn't honor the probe deferral mechanism when the
>> interrupt source is not yet available, such as one provided by a gpio
>> controller not probed.
>    What if 'ndev->irq' does equal 0?
That's not possible AFAIR.

There was a discussion where Linus had stated that the irq is a cookie, and a 0
value is "no interrupt", expcepting for the single case of a PC and its timer
interrupt.

As we're not in that case, and up to my understanding, platform_get_irq() cannot
return a 0 value, only a strictly negative or positive one.

And yet, that test now looks weird to me. I think I'll respin the patch with a
"if (ndev->irq < 0) {" instead of the "if (ndev->irq <= 0) {".

Cheers.

-- 
Robert

Re: [PATCH] net: smc91x: propagate irq return code

[Sergei Shtylyov]

On 02/01/2016 11:41 PM, Robert Jarzmik wrote:

>>> The smc91x driver doesn't honor the probe deferral mechanism when the
>>> interrupt source is not yet available, such as one provided by a gpio
>>> controller not probed.

>>     What if 'ndev->irq' does equal 0?

> That's not possible AFAIR.

    Possible if of_irq_get() returns 0 (and it will on failure!).

> There was a discussion where Linus had stated that the irq is a cookie, and a 0
> value is "no interrupt", expcepting for the single case of a PC and its timer
> interrupt.

    I know, I know... and even on x86 it was never passed to request_irq(), 
only to setup_irq()...

> As we're not in that case, and up to my understanding, platform_get_irq() cannot
> return a 0 value, only a strictly negative or positive one.

    Wishful thinking...

> And yet, that test now looks weird to me. I think I'll respin the patch with a
> "if (ndev->irq < 0) {" instead of the "if (ndev->irq <= 0) {".

    Defeating Linus' PoV as a result... ;-)

> Cheers.

MBR, Sergei

Re: [PATCH] net: smc91x: propagate irq return code

[Robert Jarzmik]

Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes:

> On 02/01/2016 11:41 PM, Robert Jarzmik wrote:
>
>>>> The smc91x driver doesn't honor the probe deferral mechanism when the
>>>> interrupt source is not yet available, such as one provided by a gpio
>>>> controller not probed.
>
>>>     What if 'ndev->irq' does equal 0?
>
>> That's not possible AFAIR.
>
>    Possible if of_irq_get() returns 0 (and it will on failure!).
Ah good catch, didn't know that one.

>> And yet, that test now looks weird to me. I think I'll respin the patch with a
>> "if (ndev->irq < 0) {" instead of the "if (ndev->irq <= 0) {".
>
>    Defeating Linus' PoV as a result... ;-)
Well, I'd rather face the wrath of others if I'm convinced the code is more
correct. And in this case you convinced me :)

-- 
Robert