Message ID | 1454280377-25697-1-git-send-email-robert.jarzmik@free.fr |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
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
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.
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
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 :)
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; } /*
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(-)