Re: [PATCHv2 net-next] net: dsa: b53: srab: propagate errors from init helpers

[Rosen Penev]

On Tue, Jul 28, 2026 at 12:59 AM Jonas Gorski <jonas.gorski@xxxxxxxxx> wrote:
>
> Hi,
>
> On Tue, Jul 28, 2026 at 1:40 AM Rosen Penev <rosenp@xxxxxxxxx> wrote:
> >
> > Convert b53_srab_prepare_irq() and b53_srab_mux_init() from void to
> > int-returning functions so probe failures are properly propagated.
> >
> > b53_srab_prepare_irq() now returns -ENOMEM on allocation failure and
> > -EPROBE_DEFER if any port IRQ is not yet available.
> >
> > b53_srab_mux_init() now returns PTR_ERR on ioremap failure instead of
> > silently ignoring it.
> >
> > Check and propagate both return values in b53_srab_probe() so the
> > driver core sees the real error instead of always reaching
> > b53_switch_register().
> >
> > Add error handling in probe as a result of b53_srab_prepare_irq().
> >
> > Assisted-by: Opencode:Big-Pickle
> > Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> > ---
> >  v2: address memory leak in b53_srab_prepare_irq().
> >  drivers/net/dsa/b53/b53_srab.c | 36 ++++++++++++++++++++++++++--------
> >  1 file changed, 28 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
> > index b9939bbd2cd5..289504321f3e 100644
> > --- a/drivers/net/dsa/b53/b53_srab.c
> > +++ b/drivers/net/dsa/b53/b53_srab.c
> > @@ -531,7 +531,7 @@ static void b53_srab_intr_set(struct b53_srab_priv *priv, bool set)
> >         writel(reg, priv->regs + B53_SRAB_CTRLS);
> >  }
> >
> > -static void b53_srab_prepare_irq(struct platform_device *pdev)
> > +static int b53_srab_prepare_irq(struct platform_device *pdev)
> >  {
> >         struct b53_device *dev = platform_get_drvdata(pdev);
> >         struct b53_srab_priv *priv = dev->priv;
> > @@ -551,18 +551,22 @@ static void b53_srab_prepare_irq(struct platform_device *pdev)
> >
> >                 name = kasprintf(GFP_KERNEL, "link_state_p%d", i);
> >                 if (!name)
> > -                       return;
> > +                       return -ENOMEM;
> >
> >                 port->num = i;
> >                 port->dev = dev;
> >                 port->irq = platform_get_irq_byname_optional(pdev, name);
> >                 kfree(name);
> > +               if (port->irq == -EPROBE_DEFER)
> > +                       return port->irq;
> >         }
> >
> >         b53_srab_intr_set(priv, true);
> > +
> > +       return 0;
> >  }
> >
> > -static void b53_srab_mux_init(struct platform_device *pdev)
> > +static int b53_srab_mux_init(struct platform_device *pdev)
> >  {
> >         struct b53_device *dev = platform_get_drvdata(pdev);
> >         struct b53_srab_priv *priv = dev->priv;
> > @@ -572,11 +576,11 @@ static void b53_srab_mux_init(struct platform_device *pdev)
> >         int ret;
> >
> >         if (dev->pdata && dev->pdata->chip_id != BCM58XX_DEVICE_ID)
> > -               return;
> > +               return -ENODEV;
>
> I don't think this is correct. If you look at the b53_srab_of_match[]
> table, only some srab devices have BCM58XX_DEVICE_ID, so returning an
> error code here would break probing all other srab devices.
>
> Just returning 0 here is not enough though ...
>
> >
> >         priv->mux_config = devm_platform_ioremap_resource(pdev, 1);
> >         if (IS_ERR(priv->mux_config))
> > -               return;
> > +               return PTR_ERR(priv->mux_config);
>
> ... since some devices don't have match data, so their pdata is
> unpoulated and the above check would still allow them, so they would
> then be rejected here.
>
> So the above check should be if (!dev->pdata || dev->pdata->chip_id !=
> BCM58XX_DEVICE_ID) return 0;
I thought this as well. That was v1. Guess I'll send a v3 the same as v1...
>
> Best regards,
> Jonas