linux-kernel - Re: [PATCH net-next 13/13] net: ibm: emac: mal: use devm for request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKxU2N-jBRZWN9b2QkHYqO3wnSAX7K_r=afOv-EaOEfyJ8wXoQ@mail.gmail.com>
Date: Tue, 1 Oct 2024 12:35:01 -0700
From: Rosen Penev <rosenp@...il.com>
To: Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org, andrew@...n.ch, davem@...emloft.net, 
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, 
	linux-kernel@...r.kernel.org, jacob.e.keller@...el.com, sd@...asysnail.net, 
	chunkeey@...il.com
Subject: Re: [PATCH net-next 13/13] net: ibm: emac: mal: use devm for request_irq

On Tue, Oct 1, 2024 at 6:32 AM Simon Horman <horms@...nel.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@...il.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.
>
> ...