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

[Jonas Gorski]

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;

Best regards,
Jonas