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

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

[Chris Packham]

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@alliedtelesis.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,
-- 
2.25.1

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

[Andrew Lunn]

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@alliedtelesis.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@lunn.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;
        }

    Andrew

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

[Chris Packham]

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@alliedtelesis.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@lunn.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.

> 
>     Andrew

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@alliedtelesis.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@lunn.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