- The platform_get_*_*() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq_byname() error
checking for zero is not correct.
- The change is to call free_netdev(), If of_match_node() will fail.
- Handle return value of platform_get_resource()
Arvind Yadav (10):
[PATCH 01/10] net: bcmgenet: Fix platform_get_irq's error checking
[PATCH 02/10] net: bcmgenet: free netdev on of_match_node() error
[PATCH 03/10] net: ezchip: nps_enet: Fix platform_get_irq's error checking
[PATCH 04/10] can: xilinx: Handle return value of platform_get_irq
[PATCH 05/10] net: ethernet: i825xx: Fix platform_get_irq's error checking
[PATCH 06/10] net: ethernet: natsemi: Handle return value of platform_get_irq
[PATCH 07/10] net: ethernet: smsc: Handle return value of platform_get_irq
[PATCH 08/10] net: fjes: Handle return value of platform_get_irq and platform_get_resource
[PATCH 09/10] net: ethernet: korina: Handle return value of platform_get_irq_byname
[PATCH 10/10] net: ethernet: cpmac: Handle return value of platform_get_irq_byname
drivers/net/can/xilinx_can.c | 4 ++++
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 8 +++++---
drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
drivers/net/ethernet/i825xx/sni_82596.c | 3 ++-
drivers/net/ethernet/korina.c | 9 +++++++++
drivers/net/ethernet/natsemi/jazzsonic.c | 5 +++++
drivers/net/ethernet/smsc/smc911x.c | 5 +++++
drivers/net/ethernet/ti/cpmac.c | 4 ++++
drivers/net/fjes/fjes_main.c | 10 ++++++++++
9 files changed, 47 insertions(+), 8 deletions(-)
--
2.7.4
The platform_get_irq() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq() error checking
for zero is not correct.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 24b4f4c..e2f1268 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3371,7 +3371,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
priv->irq0 = platform_get_irq(pdev, 0);
priv->irq1 = platform_get_irq(pdev, 1);
priv->wol_irq = platform_get_irq(pdev, 2);
- if (!priv->irq0 || !priv->irq1) {
+ if (priv->irq0 <= 0 || priv->irq1 <= 0 || priv->wol_irq <= 0) {
dev_err(&pdev->dev, "can't find IRQs\n");
err = -EINVAL;
goto err;
--
2.7.4
platform_get_irq() can fail here and we must check its return value.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/can/xilinx_can.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 89aec07..e36e2a2 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -1111,6 +1111,10 @@ static int xcan_probe(struct platform_device *pdev)
/* Get IRQ for the device */
ndev->irq = platform_get_irq(pdev, 0);
+ if (ndev->irq <= 0) {
+ ret = ndev->irq ? ndev->irq : -ENODEV;
+ goto err_free;
+ }
ndev->flags |= IFF_ECHO; /* We support local echo */
platform_set_drvdata(pdev, ndev);
--
2.7.4
The change is to call free_netdev(), If of_match_node() will fail.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index e2f1268..e0a8f79 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3363,8 +3363,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
if (dn) {
of_id = of_match_node(bcmgenet_match, dn);
- if (!of_id)
- return -EINVAL;
+ if (!of_id) {
+ err = -EINVAL;
+ goto err;
+ }
}
priv = netdev_priv(dev);
--
2.7.4
platform_get_irq() can fail here and we must check its return value.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/ethernet/natsemi/jazzsonic.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/natsemi/jazzsonic.c b/drivers/net/ethernet/natsemi/jazzsonic.c
index d5b2888..3cf0856 100644
--- a/drivers/net/ethernet/natsemi/jazzsonic.c
+++ b/drivers/net/ethernet/natsemi/jazzsonic.c
@@ -242,6 +242,11 @@ static int jazz_sonic_probe(struct platform_device *pdev)
dev->base_addr = res->start;
dev->irq = platform_get_irq(pdev, 0);
+ if (dev->irq <= 0) {
+ err = dev->irq ? dev->irq : -ENODEV;
+ goto out;
+ }
+
err = sonic_probe1(dev);
if (err)
goto out;
--
2.7.4
platform_get_irq() can fail here and we must check its return value.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/ethernet/smsc/smc911x.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c
index 0515744..a1cf18c 100644
--- a/drivers/net/ethernet/smsc/smc911x.c
+++ b/drivers/net/ethernet/smsc/smc911x.c
@@ -2088,6 +2088,11 @@ static int smc911x_drv_probe(struct platform_device *pdev)
ndev->dma = (unsigned char)-1;
ndev->irq = platform_get_irq(pdev, 0);
+ if (ndev->irq <= 0) {
+ ret = ndev->irq ? ndev->irq : -ENODEV;
+ goto release_both;
+ }
+
lp = netdev_priv(ndev);
lp->netdev = ndev;
#ifdef SMC_DYNAMIC_BUS_CONFIG
--
2.7.4
platform_get_irq_byname() can fail here and we must check its return
value.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/ethernet/korina.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/korina.c b/drivers/net/ethernet/korina.c
index ae195f8..e778504 100644
--- a/drivers/net/ethernet/korina.c
+++ b/drivers/net/ethernet/korina.c
@@ -1039,7 +1039,16 @@ static int korina_probe(struct platform_device *pdev)
memcpy(dev->dev_addr, bif->mac, ETH_ALEN);
lp->rx_irq = platform_get_irq_byname(pdev, "korina_rx");
+ if (lp->rx_irq < 0) {
+ rc = lp->rx_irq;
+ goto probe_err_out;
+ }
+
lp->tx_irq = platform_get_irq_byname(pdev, "korina_tx");
+ if (lp->tx_irq < 0) {
+ rc = lp->tx_irq;
+ goto probe_err_out;
+ }
r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "korina_regs");
dev->base_addr = r->start;
--
2.7.4
platform_get_irq_byname() can fail here and we must check its return
value
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/ethernet/ti/cpmac.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/ti/cpmac.c b/drivers/net/ethernet/ti/cpmac.c
index 9b8a30b..f3acfc0 100644
--- a/drivers/net/ethernet/ti/cpmac.c
+++ b/drivers/net/ethernet/ti/cpmac.c
@@ -1124,6 +1124,10 @@ static int cpmac_probe(struct platform_device *pdev)
}
dev->irq = platform_get_irq_byname(pdev, "irq");
+ if (dev->irq < 0) {
+ rc = dev->irq;
+ goto fail;
+ }
dev->netdev_ops = &cpmac_netdev_ops;
dev->ethtool_ops = &cpmac_ethtool_ops;
--
2.7.4
platform_get_irq() and platform_get_resource() can fail here and
we must check its return value.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/fjes/fjes_main.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
index 750954b..540dd51 100644
--- a/drivers/net/fjes/fjes_main.c
+++ b/drivers/net/fjes/fjes_main.c
@@ -1265,9 +1265,19 @@ static int fjes_probe(struct platform_device *plat_dev)
adapter->interrupt_watch_enable = false;
res = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
+ if (!res) {
+ err = -EINVAL;
+ goto err_free_netdev;
+ }
+
hw->hw_res.start = res->start;
hw->hw_res.size = resource_size(res);
hw->hw_res.irq = platform_get_irq(plat_dev, 0);
+ if (hw->hw_res.irq <= 0) {
+ err = hw->hw_res.irq ? hw->hw_res.irq : -ENODEV;
+ goto err_free_netdev;
+ }
+
err = fjes_hw_init(&adapter->hw);
if (err)
goto err_free_netdev;
--
2.7.4
The platform_get_irq() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq() error checking
for zero is not correct.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/ethernet/i825xx/sni_82596.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/i825xx/sni_82596.c b/drivers/net/ethernet/i825xx/sni_82596.c
index b2c04a7..a6d56f5 100644
--- a/drivers/net/ethernet/i825xx/sni_82596.c
+++ b/drivers/net/ethernet/i825xx/sni_82596.c
@@ -120,9 +120,10 @@ static int sni_82596_probe(struct platform_device *dev)
netdevice->dev_addr[5] = readb(eth_addr + 0x06);
iounmap(eth_addr);
- if (!netdevice->irq) {
+ if (netdevice->irq <= 0) {
printk(KERN_ERR "%s: IRQ not found for i82596 at 0x%lx\n",
__FILE__, netdevice->base_addr);
+ retval = netdevice->irq ? netdevice->irq : -ENODEV;
goto probe_failed;
}
--
2.7.4
The platform_get_irq() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq() error checking
for zero is not correct. And remove unnecessary check for free_netdev().
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 659f1ad..82dc6d0 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -623,9 +623,9 @@ static s32 nps_enet_probe(struct platform_device *pdev)
/* Get IRQ number */
priv->irq = platform_get_irq(pdev, 0);
- if (!priv->irq) {
+ if (priv->irq <= 0) {
dev_err(dev, "failed to retrieve <irq Rx-Tx> value from device tree\n");
- err = -ENODEV;
+ err = priv->irq ? priv->irq : -ENODEV;
goto out_netdev;
}
@@ -646,8 +646,7 @@ static s32 nps_enet_probe(struct platform_device *pdev)
out_netif_api:
netif_napi_del(&priv->napi);
out_netdev:
- if (err)
- free_netdev(ndev);
+ free_netdev(ndev);
return err;
}
--
2.7.4
Hello!
On 12/02/2017 10:26 PM, Arvind Yadav wrote:
> platform_get_irq() and platform_get_resource() can fail here and
> we must check its return value.
>
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> drivers/net/fjes/fjes_main.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
> index 750954b..540dd51 100644
> --- a/drivers/net/fjes/fjes_main.c
> +++ b/drivers/net/fjes/fjes_main.c
> @@ -1265,9 +1265,19 @@ static int fjes_probe(struct platform_device *plat_dev)
> adapter->interrupt_watch_enable = false;
>
> res = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
> + if (!res) {
> + err = -EINVAL;
> + goto err_free_netdev;
> + }
> +
> hw->hw_res.start = res->start;
> hw->hw_res.size = resource_size(res);
> hw->hw_res.irq = platform_get_irq(plat_dev, 0);
> + if (hw->hw_res.irq <= 0) {
This function no longer returns 0 on error, no need to check for <= 0.
> + err = hw->hw_res.irq ? hw->hw_res.irq : -ENODEV;
> + goto err_free_netdev;
gcc allows a shorter way to write that.
err = hw->hw_res.irq ?: -ENODEV;
> + }
> +
> err = fjes_hw_init(&adapter->hw);
> if (err)
> goto err_free_netdev;
MBR, Sergei
On 12/02/2017 10:26 PM, Arvind Yadav wrote:
> platform_get_irq() can fail here and we must check its return value.
>
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> drivers/net/ethernet/smsc/smc911x.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c
> index 0515744..a1cf18c 100644
> --- a/drivers/net/ethernet/smsc/smc911x.c
> +++ b/drivers/net/ethernet/smsc/smc911x.c
> @@ -2088,6 +2088,11 @@ static int smc911x_drv_probe(struct platform_device *pdev)
>
> ndev->dma = (unsigned char)-1;
> ndev->irq = platform_get_irq(pdev, 0);
> + if (ndev->irq <= 0) {
> + ret = ndev->irq ? ndev->irq : -ENODEV;
Same comments as the next patch...
> + goto release_both;
> + }
> +
> lp = netdev_priv(ndev);
> lp->netdev = ndev;
> #ifdef SMC_DYNAMIC_BUS_CONFIG
MBR, Sergei
Hello.
On 12/02/2017 10:26 PM, Arvind Yadav wrote:
> The platform_get_irq() function returns negative if an error occurs.
> zero or positive number on success. platform_get_irq() error checking
> for zero is not correct.
The why you consider returning 0 a sign of failure?
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> drivers/net/ethernet/i825xx/sni_82596.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/i825xx/sni_82596.c b/drivers/net/ethernet/i825xx/sni_82596.c
> index b2c04a7..a6d56f5 100644
> --- a/drivers/net/ethernet/i825xx/sni_82596.c
> +++ b/drivers/net/ethernet/i825xx/sni_82596.c
> @@ -120,9 +120,10 @@ static int sni_82596_probe(struct platform_device *dev)
> netdevice->dev_addr[5] = readb(eth_addr + 0x06);
> iounmap(eth_addr);
>
> - if (!netdevice->irq) {
> + if (netdevice->irq <= 0) {
> printk(KERN_ERR "%s: IRQ not found for i82596 at 0x%lx\n",
> __FILE__, netdevice->base_addr);
> + retval = netdevice->irq ? netdevice->irq : -ENODEV;
> goto probe_failed;
> }
>
MBR, Sergei
Hi Sergei,
On Sunday 03 December 2017 01:38 AM, Sergei Shtylyov wrote:
> Hello.
>
> On 12/02/2017 10:26 PM, Arvind Yadav wrote:
>
>> The platform_get_irq() function returns negative if an error occurs.
>> zero or positive number on success. platform_get_irq() error checking
>> for zero is not correct.
>
> The why you consider returning 0 a sign of failure?
Here, Returning 0 is a problem. Because IRQ0 is always a problem.
This function is for getting an IRQ for a device. So we should check for
0 also.
>
>> Signed-off-by: Arvind Yadav <[email protected]>
>> ---
>> drivers/net/ethernet/i825xx/sni_82596.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/i825xx/sni_82596.c
>> b/drivers/net/ethernet/i825xx/sni_82596.c
>> index b2c04a7..a6d56f5 100644
>> --- a/drivers/net/ethernet/i825xx/sni_82596.c
>> +++ b/drivers/net/ethernet/i825xx/sni_82596.c
>> @@ -120,9 +120,10 @@ static int sni_82596_probe(struct
>> platform_device *dev)
>> netdevice->dev_addr[5] = readb(eth_addr + 0x06);
>> iounmap(eth_addr);
>> - if (!netdevice->irq) {
>> + if (netdevice->irq <= 0) {
>> printk(KERN_ERR "%s: IRQ not found for i82596 at 0x%lx\n",
>> __FILE__, netdevice->base_addr);
>> + retval = netdevice->irq ? netdevice->irq : -ENODEV;
>> goto probe_failed;
>> }
>
> MBR, Sergei
~arvind
Hi Sergei,
On Sunday 03 December 2017 01:36 AM, Sergei Shtylyov wrote:
> Hello!
>
> On 12/02/2017 10:26 PM, Arvind Yadav wrote:
>
>> platform_get_irq() and platform_get_resource() can fail here and
>> we must check its return value.
>>
>> Signed-off-by: Arvind Yadav <[email protected]>
>> ---
>> drivers/net/fjes/fjes_main.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
>> index 750954b..540dd51 100644
>> --- a/drivers/net/fjes/fjes_main.c
>> +++ b/drivers/net/fjes/fjes_main.c
>> @@ -1265,9 +1265,19 @@ static int fjes_probe(struct platform_device
>> *plat_dev)
>> adapter->interrupt_watch_enable = false;
>> res = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
>> + if (!res) {
>> + err = -EINVAL;
>> + goto err_free_netdev;
>> + }
>> +
>> hw->hw_res.start = res->start;
>> hw->hw_res.size = resource_size(res);
>> hw->hw_res.irq = platform_get_irq(plat_dev, 0);
>> + if (hw->hw_res.irq <= 0) {
>
> This function no longer returns 0 on error, no need to check for <= 0.
>
>> + err = hw->hw_res.irq ? hw->hw_res.irq : -ENODEV;
>> + goto err_free_netdev;
>
> gcc allows a shorter way to write that.
>
> err = hw->hw_res.irq ?: -ENODEV;
Yes, you are right, But first is more readable. That's why I have used.
>
>> + }
>> +
>> err = fjes_hw_init(&adapter->hw);
>> if (err)
>> goto err_free_netdev;
>
> MBR, Sergei
Thanks,
~arvind
From: Arvind Yadav <[email protected]>
Date: Sun, 3 Dec 2017 00:56:15 +0530
> The platform_get_irq() function returns negative if an error occurs.
> zero or positive number on success. platform_get_irq() error checking
> for zero is not correct. And remove unnecessary check for free_netdev().
>
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
> index 659f1ad..82dc6d0 100644
> --- a/drivers/net/ethernet/ezchip/nps_enet.c
> +++ b/drivers/net/ethernet/ezchip/nps_enet.c
> @@ -623,9 +623,9 @@ static s32 nps_enet_probe(struct platform_device *pdev)
>
> /* Get IRQ number */
> priv->irq = platform_get_irq(pdev, 0);
> - if (!priv->irq) {
> + if (priv->irq <= 0) {
> dev_err(dev, "failed to retrieve <irq Rx-Tx> value from device tree\n");
> - err = -ENODEV;
> + err = priv->irq ? priv->irq : -ENODEV;
If platform_get_irq() returns "zero or positive number on success" then this
test is wrong and should be "if (priv->irq < 0)"
Also, this series is a mix of different kinds of changes.
Please separate out the platform IRQ error checking and just submit exactly
those changes as a patch series.
The other bug fixes should be submitted outside of those changes since they
are unrelated.
On Mon, Dec 04, 2017 at 11:20:49AM -0500, David Miller wrote:
> From: Arvind Yadav <[email protected]>
> Date: Sun, 3 Dec 2017 00:56:15 +0530
>
> > The platform_get_irq() function returns negative if an error occurs.
> > zero or positive number on success. platform_get_irq() error checking
> > for zero is not correct. And remove unnecessary check for free_netdev().
> >
> > Signed-off-by: Arvind Yadav <[email protected]>
> > ---
> > drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
> > index 659f1ad..82dc6d0 100644
> > --- a/drivers/net/ethernet/ezchip/nps_enet.c
> > +++ b/drivers/net/ethernet/ezchip/nps_enet.c
> > @@ -623,9 +623,9 @@ static s32 nps_enet_probe(struct platform_device *pdev)
> >
> > /* Get IRQ number */
> > priv->irq = platform_get_irq(pdev, 0);
> > - if (!priv->irq) {
> > + if (priv->irq <= 0) {
> > dev_err(dev, "failed to retrieve <irq Rx-Tx> value from device tree\n");
> > - err = -ENODEV;
> > + err = priv->irq ? priv->irq : -ENODEV;
>
> If platform_get_irq() returns "zero or positive number on success" then this
> test is wrong and should be "if (priv->irq < 0)"
>
> Also, this series is a mix of different kinds of changes.
>
> Please separate out the platform IRQ error checking and just submit exactly
> those changes as a patch series.
>
> The other bug fixes should be submitted outside of those changes since they
> are unrelated.
The issue of whether IRQ 0 is valid or not has been covered several times
by Linus, and the result is that it is deemed by Linus that IRQ 0 is not
a valid interrupt.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
From: Russell King - ARM Linux <[email protected]>
Date: Mon, 4 Dec 2017 16:24:47 +0000
> On Mon, Dec 04, 2017 at 11:20:49AM -0500, David Miller wrote:
>> From: Arvind Yadav <[email protected]>
>> Date: Sun, 3 Dec 2017 00:56:15 +0530
>>
>> > The platform_get_irq() function returns negative if an error occurs.
>> > zero or positive number on success. platform_get_irq() error checking
>> > for zero is not correct. And remove unnecessary check for free_netdev().
>> >
>> > Signed-off-by: Arvind Yadav <[email protected]>
>> > ---
>> > drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
>> > 1 file changed, 3 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
>> > index 659f1ad..82dc6d0 100644
>> > --- a/drivers/net/ethernet/ezchip/nps_enet.c
>> > +++ b/drivers/net/ethernet/ezchip/nps_enet.c
>> > @@ -623,9 +623,9 @@ static s32 nps_enet_probe(struct platform_device *pdev)
>> >
>> > /* Get IRQ number */
>> > priv->irq = platform_get_irq(pdev, 0);
>> > - if (!priv->irq) {
>> > + if (priv->irq <= 0) {
>> > dev_err(dev, "failed to retrieve <irq Rx-Tx> value from device tree\n");
>> > - err = -ENODEV;
>> > + err = priv->irq ? priv->irq : -ENODEV;
>>
>> If platform_get_irq() returns "zero or positive number on success" then this
>> test is wrong and should be "if (priv->irq < 0)"
>>
>> Also, this series is a mix of different kinds of changes.
>>
>> Please separate out the platform IRQ error checking and just submit exactly
>> those changes as a patch series.
>>
>> The other bug fixes should be submitted outside of those changes since they
>> are unrelated.
>
> The issue of whether IRQ 0 is valid or not has been covered several times
> by Linus, and the result is that it is deemed by Linus that IRQ 0 is not
> a valid interrupt.
Then either platform_get_irq() as defined or this commit message (or both)
are wrong.
On Mon, Dec 04, 2017 at 11:34:48AM -0500, David Miller wrote:
> From: Russell King - ARM Linux <[email protected]>
> Date: Mon, 4 Dec 2017 16:24:47 +0000
>
> > On Mon, Dec 04, 2017 at 11:20:49AM -0500, David Miller wrote:
> >> From: Arvind Yadav <[email protected]>
> >> Date: Sun, 3 Dec 2017 00:56:15 +0530
> >>
> >> > The platform_get_irq() function returns negative if an error occurs.
> >> > zero or positive number on success. platform_get_irq() error checking
> >> > for zero is not correct. And remove unnecessary check for free_netdev().
> >> >
> >> > Signed-off-by: Arvind Yadav <[email protected]>
> >> > ---
> >> > drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
> >> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
> >> > index 659f1ad..82dc6d0 100644
> >> > --- a/drivers/net/ethernet/ezchip/nps_enet.c
> >> > +++ b/drivers/net/ethernet/ezchip/nps_enet.c
> >> > @@ -623,9 +623,9 @@ static s32 nps_enet_probe(struct platform_device *pdev)
> >> >
> >> > /* Get IRQ number */
> >> > priv->irq = platform_get_irq(pdev, 0);
> >> > - if (!priv->irq) {
> >> > + if (priv->irq <= 0) {
> >> > dev_err(dev, "failed to retrieve <irq Rx-Tx> value from device tree\n");
> >> > - err = -ENODEV;
> >> > + err = priv->irq ? priv->irq : -ENODEV;
> >>
> >> If platform_get_irq() returns "zero or positive number on success" then this
> >> test is wrong and should be "if (priv->irq < 0)"
> >>
> >> Also, this series is a mix of different kinds of changes.
> >>
> >> Please separate out the platform IRQ error checking and just submit exactly
> >> those changes as a patch series.
> >>
> >> The other bug fixes should be submitted outside of those changes since they
> >> are unrelated.
> >
> > The issue of whether IRQ 0 is valid or not has been covered several times
> > by Linus, and the result is that it is deemed by Linus that IRQ 0 is not
> > a valid interrupt.
>
> Then either platform_get_irq() as defined or this commit message (or both)
> are wrong.
Indeed, the commit message is wrong, and that's already been pointed out
in previous patch sets.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
On Monday 04 December 2017 10:12 PM, Russell King - ARM Linux wrote:
> On Mon, Dec 04, 2017 at 11:34:48AM -0500, David Miller wrote:
>> From: Russell King - ARM Linux <[email protected]>
>> Date: Mon, 4 Dec 2017 16:24:47 +0000
>>
>>> On Mon, Dec 04, 2017 at 11:20:49AM -0500, David Miller wrote:
>>>> From: Arvind Yadav <[email protected]>
>>>> Date: Sun, 3 Dec 2017 00:56:15 +0530
>>>>
>>>>> The platform_get_irq() function returns negative if an error occurs.
>>>>> zero or positive number on success. platform_get_irq() error checking
>>>>> for zero is not correct. And remove unnecessary check for free_netdev().
>>>>>
>>>>> Signed-off-by: Arvind Yadav <[email protected]>
>>>>> ---
>>>>> drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
>>>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
>>>>> index 659f1ad..82dc6d0 100644
>>>>> --- a/drivers/net/ethernet/ezchip/nps_enet.c
>>>>> +++ b/drivers/net/ethernet/ezchip/nps_enet.c
>>>>> @@ -623,9 +623,9 @@ static s32 nps_enet_probe(struct platform_device *pdev)
>>>>>
>>>>> /* Get IRQ number */
>>>>> priv->irq = platform_get_irq(pdev, 0);
>>>>> - if (!priv->irq) {
>>>>> + if (priv->irq <= 0) {
>>>>> dev_err(dev, "failed to retrieve <irq Rx-Tx> value from device tree\n");
>>>>> - err = -ENODEV;
>>>>> + err = priv->irq ? priv->irq : -ENODEV;
>>>> If platform_get_irq() returns "zero or positive number on success" then this
>>>> test is wrong and should be "if (priv->irq < 0)"
>>>>
>>>> Also, this series is a mix of different kinds of changes.
>>>>
>>>> Please separate out the platform IRQ error checking and just submit exactly
>>>> those changes as a patch series.
>>>>
>>>> The other bug fixes should be submitted outside of those changes since they
>>>> are unrelated.
>>> The issue of whether IRQ 0 is valid or not has been covered several times
>>> by Linus, and the result is that it is deemed by Linus that IRQ 0 is not
>>> a valid interrupt.
>> Then either platform_get_irq() as defined or this commit message (or both)
>> are wrong.
> Indeed, the commit message is wrong, and that's already been pointed out
> in previous patch sets.
>
I will change commit message. Commit message will be
" The platform_get_irq() function returns negative if an
error occurs, Zero if No irq found and positive number
on get irq successful. platform_get_irq() error checking for
only zero is not correct."
On 12/02/2017 11:26 AM, Arvind Yadav wrote:
> The change is to call free_netdev(), If of_match_node() will fail.
>
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index e2f1268..e0a8f79 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3363,8 +3363,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
>
> if (dn) {
> of_id = of_match_node(bcmgenet_match, dn);
> - if (!of_id)
> - return -EINVAL;
> + if (!of_id) {
> + err = -EINVAL;
> + goto err;
> + }
> }
>
> priv = netdev_priv(dev);
>
I agree with the fix if you want to resubmit separate from this series
and please include a "fixes" tag.
Thanks,
Doug