[PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()

[PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()

[cgel.zte]

From: Minghao Chi <chi.minghao@zte.com.cn>

It is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ)
for requesting IRQ's resources any more, as they can be not ready yet in
case of DT-booting.

platform_get_irq() instead is a recommended way for getting IRQ even if
it was not retrieved earlier.

It also makes code simpler because we're getting "int" value right away
and no conversion from resource to int is required.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
---
v1->v2:
  - Add a space after "net:".
  - Use WARN_ON instead of BUG_ON
 drivers/net/ethernet/marvell/mv643xx_eth.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index c31cbbae0eca..34fa5ab21d62 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -3092,8 +3092,7 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	struct mv643xx_eth_private *mp;
 	struct net_device *dev;
 	struct phy_device *phydev = NULL;
-	struct resource *res;
-	int err;
+	int err, irq;
 
 	pd = dev_get_platdata(&pdev->dev);
 	if (pd == NULL) {
@@ -3189,9 +3188,10 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	timer_setup(&mp->rx_oom, oom_timer_wrapper, 0);
 
 
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	BUG_ON(!res);
-	dev->irq = res->start;
+	irq = platform_get_irq(pdev, 0);
+	if (WARN_ON(irq < 0))
+		return irq;
+	dev->irq = irq;
 
 	dev->netdev_ops = &mv643xx_eth_netdev_ops;
 
-- 
2.25.1

Re: [PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu, 10 Mar 2022 06:20:35 +0000 you wrote:
> From: Minghao Chi <chi.minghao@zte.com.cn>
> 
> It is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ)
> for requesting IRQ's resources any more, as they can be not ready yet in
> case of DT-booting.
> 
> platform_get_irq() instead is a recommended way for getting IRQ even if
> it was not retrieved earlier.
> 
> [...]

Here is the summary with links:
  - [V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()
    https://git.kernel.org/netdev/net-next/c/bf2b83425b59

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()

[Jakub Kicinski]

On Thu, 10 Mar 2022 06:20:35 +0000 cgel.zte@gmail.com wrote:
> @@ -3189,9 +3188,10 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
>  	timer_setup(&mp->rx_oom, oom_timer_wrapper, 0);
>  
>  
> -	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -	BUG_ON(!res);
> -	dev->irq = res->start;
> +	irq = platform_get_irq(pdev, 0);
> +	if (WARN_ON(irq < 0))
> +		return irq;

You can't just return from here, there are operations that need 
to be undone, look at the end of this function :/ Please follow 
up with an incremental fix ASAP.

> +	dev->irq = irq;
>  

[PATCH V3] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()

[cgel.zte]

From: Minghao Chi <chi.minghao@zte.com.cn>

It is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ)
for requesting IRQ's resources any more, as they can be not ready yet in
case of DT-booting.

platform_get_irq() instead is a recommended way for getting IRQ even if
it was not retrieved earlier.

It also makes code simpler because we're getting "int" value right away
and no conversion from resource to int is required.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
---
v1->v2:
  - Add a space after "net:".
  - Use WARN_ON instead of BUG_ON.
v2->v3:
  - Release some operations.

 drivers/net/ethernet/marvell/mv643xx_eth.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index c31cbbae0eca..d75cf9097c7a 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -3092,8 +3092,7 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	struct mv643xx_eth_private *mp;
 	struct net_device *dev;
 	struct phy_device *phydev = NULL;
-	struct resource *res;
-	int err;
+	int err, irq;
 
 	pd = dev_get_platdata(&pdev->dev);
 	if (pd == NULL) {
@@ -3189,9 +3188,14 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
 	timer_setup(&mp->rx_oom, oom_timer_wrapper, 0);
 
 
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	BUG_ON(!res);
-	dev->irq = res->start;
+	irq = platform_get_irq(pdev, 0);
+	if (WARN_ON(irq < 0)) {
+		if (!IS_ERR(mp->clk))
+			clk_disable_unprepare(mp->clk);
+		free_netdev(dev);
+		return irq;
+	}
+	dev->irq = irq;
 
 	dev->netdev_ops = &mv643xx_eth_netdev_ops;
 
-- 
2.25.1

Re: [PATCH V3] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()

[Andrew Lunn]

> -	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -	BUG_ON(!res);
> -	dev->irq = res->start;
> +	irq = platform_get_irq(pdev, 0);
> +	if (WARN_ON(irq < 0)) {
> +		if (!IS_ERR(mp->clk))
> +			clk_disable_unprepare(mp->clk);
> +		free_netdev(dev);
> +		return irq;
> +	}

Why not
		goto out;

?

And FYI: You can pass an error code to clk_disable_unprepare() and it
will not care and do the right thing. So if you were to keep this
code, which you should not, you don't need the if !IS_ERR(mp->clk))


      Andrew

Re: [PATCH V3] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()

[Jakub Kicinski]

On Mon, 14 Mar 2022 02:41:44 +0000 cgel.zte@gmail.com wrote:
> From: Minghao Chi <chi.minghao@zte.com.cn>
> 
> It is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ)
> for requesting IRQ's resources any more, as they can be not ready yet in
> case of DT-booting.
> 
> platform_get_irq() instead is a recommended way for getting IRQ even if
> it was not retrieved earlier.
> 
> It also makes code simpler because we're getting "int" value right away
> and no conversion from resource to int is required.
> 
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>

Your previous patch was applied, you must send a incremental fix on top
of this tree:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/