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

[Rosen Penev]

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