All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()
@ 2022-03-10  6:20 cgel.zte
  2022-03-11 11:10 ` patchwork-bot+netdevbpf
  2022-03-11 16:20 ` Jakub Kicinski
  0 siblings, 2 replies; 6+ messages in thread
From: cgel.zte @ 2022-03-10  6:20 UTC (permalink / raw)
  To: sebastian.hesselbarth
  Cc: davem, kuba, netdev, linux-kernel, Minghao Chi, Zeal Robot

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()
  2022-03-10  6:20 [PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource() cgel.zte
@ 2022-03-11 11:10 ` patchwork-bot+netdevbpf
  2022-03-11 16:20 ` Jakub Kicinski
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-03-11 11:10 UTC (permalink / raw)
  To: Lv Ruyi
  Cc: sebastian.hesselbarth, davem, kuba, netdev, linux-kernel,
	chi.minghao, zealci

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()
  2022-03-10  6:20 [PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource() cgel.zte
  2022-03-11 11:10 ` patchwork-bot+netdevbpf
@ 2022-03-11 16:20 ` Jakub Kicinski
  2022-03-14  2:41   ` [PATCH V3] " cgel.zte
  1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2022-03-11 16:20 UTC (permalink / raw)
  To: cgel.zte
  Cc: sebastian.hesselbarth, davem, netdev, linux-kernel, Minghao Chi,
	Zeal Robot

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;
>  

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V3] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()
  2022-03-11 16:20 ` Jakub Kicinski
@ 2022-03-14  2:41   ` cgel.zte
  2022-03-14 13:04     ` Andrew Lunn
  2022-03-14 16:40     ` Jakub Kicinski
  0 siblings, 2 replies; 6+ messages in thread
From: cgel.zte @ 2022-03-14  2:41 UTC (permalink / raw)
  To: kuba
  Cc: cgel.zte, chi.minghao, davem, linux-kernel, netdev,
	sebastian.hesselbarth, zealci

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V3] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()
  2022-03-14  2:41   ` [PATCH V3] " cgel.zte
@ 2022-03-14 13:04     ` Andrew Lunn
  2022-03-14 16:40     ` Jakub Kicinski
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2022-03-14 13:04 UTC (permalink / raw)
  To: cgel.zte
  Cc: kuba, chi.minghao, davem, linux-kernel, netdev,
	sebastian.hesselbarth, zealci

> -	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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V3] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource()
  2022-03-14  2:41   ` [PATCH V3] " cgel.zte
  2022-03-14 13:04     ` Andrew Lunn
@ 2022-03-14 16:40     ` Jakub Kicinski
  1 sibling, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2022-03-14 16:40 UTC (permalink / raw)
  To: cgel.zte
  Cc: chi.minghao, davem, linux-kernel, netdev, sebastian.hesselbarth, zealci

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/

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-03-14 16:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10  6:20 [PATCH V2] net: mv643xx_eth: use platform_get_irq() instead of platform_get_resource() cgel.zte
2022-03-11 11:10 ` patchwork-bot+netdevbpf
2022-03-11 16:20 ` Jakub Kicinski
2022-03-14  2:41   ` [PATCH V3] " cgel.zte
2022-03-14 13:04     ` Andrew Lunn
2022-03-14 16:40     ` Jakub Kicinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.