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

[Chris Packham]

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@xxxxxxxxxxxxxxxxxxx>
> > > ---
> > >  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@xxxxxxx>
> > 
> > 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