linux-kernel - Re: [PATCH] net: mvmdio: avoid error message for optional IRQ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e03346a415bc6dafd6f9aab5b5e8bbe2a0c6d2aa.camel@alliedtelesis.co.nz>
Date:   Wed, 11 Mar 2020 19:48:37 +0000
From:   Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To:     "andrew@...n.ch" <andrew@...n.ch>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "josua@...id-run.com" <josua@...id-run.com>
Subject: Re: [PATCH] net: mvmdio: avoid error message for optional IRQ

On Thu, 2020-03-12 at 08:41 +1300, Chris Packham wrote:
> On Wed, 2020-03-11 at 13:10 +0100, Andrew Lunn wrote:
> > On Wed, Mar 11, 2020 at 03:41:30PM +1300, Chris Packham wrote:
> > > Per the dt-binding the interrupt is optional so use
> > > platform_get_irq_optional() instead of platform_get_irq(). Since
> > > commit 7723f4c5ecdb ("driver core: platform: Add an error message to
> > > platform_get_irq*()") platform_get_irq() produces an error message
> > > 
> > >   orion-mdio f1072004.mdio: IRQ index 0 not found
> > > 
> > > which is perfectly normal if one hasn't specified the optional property
> > > in the device tree.
> > > 
> > > Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> > > ---
> > >  drivers/net/ethernet/marvell/mvmdio.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
> > > index 0b9e851f3da4..d14762d93640 100644
> > > --- a/drivers/net/ethernet/marvell/mvmdio.c
> > > +++ b/drivers/net/ethernet/marvell/mvmdio.c
> > > @@ -347,7 +347,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
> > >  	}
> > >  
> > >  
> > > -	dev->err_interrupt = platform_get_irq(pdev, 0);
> > > +	dev->err_interrupt = platform_get_irq_optional(pdev, 0);
> > >  	if (dev->err_interrupt > 0 &&
> > >  	    resource_size(r) < MVMDIO_ERR_INT_MASK + 4) {
> > >  		dev_err(&pdev->dev,
> > 
> > Hi Chris
> > 
> > This is the minimum fix. So:
> > 
> > Reviewed-by: Andrew Lunn <andrew@...n.ch>
> > 
> > However, you could also simplify
> > 
> >         } else if (dev->err_interrupt == -EPROBE_DEFER) {
> >                 ret = -EPROBE_DEFER;
> >                 goto out_mdio;
> >         }
> > 
> > 
> > to just
> > 
> >         } else {
> >                 ret = dev->err_interrupt;
> >                 goto out_mdio;
> >         }
> 
> Makes sense. May as well include that while I'm here.
> 

Actually after looking closer that won't quite work. We still need to
handle the case where dev->err_interrupt == 0 either because it's not
in the dt or because the extra resource is not present (that probably
should be an error but the existing code seems to want to allow it).

I'll post a v2 with } else if (dev->err_interrupt < 0) { and we can go
from there.

> > 
> >     Andrew