diff mbox series

[net-next,13/13] net: ibm: emac: mal: use devm for request_irq

Message ID 20240930180036.87598-14-rosenp@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ibm: emac: more cleanups | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 9 this patch: 9
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 12 this patch: 12
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 81 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Rosen Penev Sept. 30, 2024, 6 p.m. UTC 
Avoids manual frees. Also replaced irq_of_parse_and_map with
platform_get_irq since it's simpler and does the same thing.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/ibm/emac/mal.c | 47 ++++++++++++-----------------
 1 file changed, 19 insertions(+), 28 deletions(-)

Comments

Simon Horman Oct. 1, 2024, 1:32 p.m. UTC | #1 
On Mon, Sep 30, 2024 at 11:00:36AM -0700, Rosen Penev wrote:
> Avoids manual frees. Also replaced irq_of_parse_and_map with
> platform_get_irq since it's simpler and does the same thing.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/net/ethernet/ibm/emac/mal.c | 47 ++++++++++++-----------------
>  1 file changed, 19 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/emac/mal.c b/drivers/net/ethernet/ibm/emac/mal.c
> index 70019ced47ff..449f0abe6e9e 100644
> --- a/drivers/net/ethernet/ibm/emac/mal.c
> +++ b/drivers/net/ethernet/ibm/emac/mal.c
> @@ -578,15 +578,15 @@ static int mal_probe(struct platform_device *ofdev)
>  #endif
>  	}
>  
> -	mal->txeob_irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
> -	mal->rxeob_irq = irq_of_parse_and_map(ofdev->dev.of_node, 1);
> -	mal->serr_irq = irq_of_parse_and_map(ofdev->dev.of_node, 2);
> +	mal->txeob_irq = platform_get_irq(ofdev, 0);
> +	mal->rxeob_irq = platform_get_irq(ofdev, 1);
> +	mal->serr_irq = platform_get_irq(ofdev, 2);
>  
>  	if (mal_has_feature(mal, MAL_FTR_COMMON_ERR_INT)) {
>  		mal->txde_irq = mal->rxde_irq = mal->serr_irq;
>  	} else {
> -		mal->txde_irq = irq_of_parse_and_map(ofdev->dev.of_node, 3);
> -		mal->rxde_irq = irq_of_parse_and_map(ofdev->dev.of_node, 4);
> +		mal->txde_irq = platform_get_irq(ofdev, 3);
> +		mal->rxde_irq = platform_get_irq(ofdev, 4);
>  	}
>  
>  	if (!mal->txeob_irq || !mal->rxeob_irq || !mal->serr_irq ||

I think this error handing needs to be updated.
platform_get_irq() returns either a non-zero IRQ number or
a negative error value. It does not return zero.

Flagged by Smatch.

...
Rosen Penev Oct. 1, 2024, 7:35 p.m. UTC | #2 
On Tue, Oct 1, 2024 at 6:32 AM Simon Horman <horms@kernel.org> wrote:
>
> On Mon, Sep 30, 2024 at 11:00:36AM -0700, Rosen Penev wrote:
> > Avoids manual frees. Also replaced irq_of_parse_and_map with
> > platform_get_irq since it's simpler and does the same thing.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  drivers/net/ethernet/ibm/emac/mal.c | 47 ++++++++++++-----------------
> >  1 file changed, 19 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/ibm/emac/mal.c b/drivers/net/ethernet/ibm/emac/mal.c
> > index 70019ced47ff..449f0abe6e9e 100644
> > --- a/drivers/net/ethernet/ibm/emac/mal.c
> > +++ b/drivers/net/ethernet/ibm/emac/mal.c
> > @@ -578,15 +578,15 @@ static int mal_probe(struct platform_device *ofdev)
> >  #endif
> >       }
> >
> > -     mal->txeob_irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
> > -     mal->rxeob_irq = irq_of_parse_and_map(ofdev->dev.of_node, 1);
> > -     mal->serr_irq = irq_of_parse_and_map(ofdev->dev.of_node, 2);
> > +     mal->txeob_irq = platform_get_irq(ofdev, 0);
> > +     mal->rxeob_irq = platform_get_irq(ofdev, 1);
> > +     mal->serr_irq = platform_get_irq(ofdev, 2);
> >
> >       if (mal_has_feature(mal, MAL_FTR_COMMON_ERR_INT)) {
> >               mal->txde_irq = mal->rxde_irq = mal->serr_irq;
> >       } else {
> > -             mal->txde_irq = irq_of_parse_and_map(ofdev->dev.of_node, 3);
> > -             mal->rxde_irq = irq_of_parse_and_map(ofdev->dev.of_node, 4);
> > +             mal->txde_irq = platform_get_irq(ofdev, 3);
> > +             mal->rxde_irq = platform_get_irq(ofdev, 4);
> >       }
> >
> >       if (!mal->txeob_irq || !mal->rxeob_irq || !mal->serr_irq ||
>
> I think this error handing needs to be updated.
> platform_get_irq() returns either a non-zero IRQ number or
> a negative error value. It does not return zero.
I wonder if error handling here is needed. devm_request_irq will error
if something is wrong.
>
> Flagged by Smatch.
>
> ...
diff mbox series

Patch

diff --git a/drivers/net/ethernet/ibm/emac/mal.c b/drivers/net/ethernet/ibm/emac/mal.c
index 70019ced47ff..449f0abe6e9e 100644
--- a/drivers/net/ethernet/ibm/emac/mal.c
+++ b/drivers/net/ethernet/ibm/emac/mal.c
@@ -578,15 +578,15 @@  static int mal_probe(struct platform_device *ofdev)
 #endif
 	}
 
-	mal->txeob_irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
-	mal->rxeob_irq = irq_of_parse_and_map(ofdev->dev.of_node, 1);
-	mal->serr_irq = irq_of_parse_and_map(ofdev->dev.of_node, 2);
+	mal->txeob_irq = platform_get_irq(ofdev, 0);
+	mal->rxeob_irq = platform_get_irq(ofdev, 1);
+	mal->serr_irq = platform_get_irq(ofdev, 2);
 
 	if (mal_has_feature(mal, MAL_FTR_COMMON_ERR_INT)) {
 		mal->txde_irq = mal->rxde_irq = mal->serr_irq;
 	} else {
-		mal->txde_irq = irq_of_parse_and_map(ofdev->dev.of_node, 3);
-		mal->rxde_irq = irq_of_parse_and_map(ofdev->dev.of_node, 4);
+		mal->txde_irq = platform_get_irq(ofdev, 3);
+		mal->rxde_irq = platform_get_irq(ofdev, 4);
 	}
 
 	if (!mal->txeob_irq || !mal->rxeob_irq || !mal->serr_irq ||
@@ -660,21 +660,26 @@  static int mal_probe(struct platform_device *ofdev)
 		hdlr_rxde = mal_rxde;
 	}
 
-	err = request_irq(mal->serr_irq, hdlr_serr, irqflags, "MAL SERR", mal);
+	err = devm_request_irq(&ofdev->dev, mal->serr_irq, hdlr_serr, irqflags,
+			       "MAL SERR", mal);
 	if (err)
 		goto fail2;
-	err = request_irq(mal->txde_irq, hdlr_txde, irqflags, "MAL TX DE", mal);
+	err = devm_request_irq(&ofdev->dev, mal->txde_irq, hdlr_txde, irqflags,
+			       "MAL TX DE", mal);
 	if (err)
-		goto fail3;
-	err = request_irq(mal->txeob_irq, mal_txeob, 0, "MAL TX EOB", mal);
+		goto fail2;
+	err = devm_request_irq(&ofdev->dev, mal->txeob_irq, mal_txeob, 0,
+			       "MAL TX EOB", mal);
 	if (err)
-		goto fail4;
-	err = request_irq(mal->rxde_irq, hdlr_rxde, irqflags, "MAL RX DE", mal);
+		goto fail2;
+	err = devm_request_irq(&ofdev->dev, mal->rxde_irq, hdlr_rxde, irqflags,
+			       "MAL RX DE", mal);
 	if (err)
-		goto fail5;
-	err = request_irq(mal->rxeob_irq, mal_rxeob, 0, "MAL RX EOB", mal);
+		goto fail2;
+	err = devm_request_irq(&ofdev->dev, mal->rxeob_irq, mal_rxeob, 0,
+			       "MAL RX EOB", mal);
 	if (err)
-		goto fail6;
+		goto fail2;
 
 	/* Enable all MAL SERR interrupt sources */
 	set_mal_dcrn(mal, MAL_IER, MAL_IER_EVENTS);
@@ -693,14 +698,6 @@  static int mal_probe(struct platform_device *ofdev)
 
 	return 0;
 
- fail6:
-	free_irq(mal->rxde_irq, mal);
- fail5:
-	free_irq(mal->txeob_irq, mal);
- fail4:
-	free_irq(mal->txde_irq, mal);
- fail3:
-	free_irq(mal->serr_irq, mal);
  fail2:
 	dma_free_coherent(&ofdev->dev, bd_size, mal->bd_virt, mal->bd_dma);
  fail_dummy:
@@ -725,12 +722,6 @@  static void mal_remove(struct platform_device *ofdev)
 		       "mal%d: commac list is not empty on remove!\n",
 		       mal->index);
 
-	free_irq(mal->serr_irq, mal);
-	free_irq(mal->txde_irq, mal);
-	free_irq(mal->txeob_irq, mal);
-	free_irq(mal->rxde_irq, mal);
-	free_irq(mal->rxeob_irq, mal);
-
 	mal_reset(mal);
 
 	free_netdev(mal->dummy_dev);