Re: [PATCH v2 3/3] pinctrl: renesas: rza2: Add lock around pinctrl_generic{{add,remove}_group,{add,remove}_function}

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Biju,

On Tue, Aug 15, 2023 at 9:56 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> The pinctrl group and function creation/remove calls expect
> caller to take care of locking. Add lock around these functions.
>
> Fixes: b59d0e782706 ("pinctrl: Add RZ/A2 pin and gpio controller")
> Cc: stable@xxxxxxxxxx
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v1->v2:
>  * No change.

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rza2.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rza2.c

> @@ -359,10 +361,13 @@ static int rza2_dt_node_to_map(struct pinctrl_dev *pctldev,
>                 psel_val[i] = MUX_FUNC(value);
>         }
>
> +       mutex_lock(&priv->mutex);
>         /* Register a single pin group listing all the pins we read from DT */
>         gsel = pinctrl_generic_add_group(pctldev, np->name, pins, npins, NULL);
> -       if (gsel < 0)
> +       if (gsel < 0) {
> +               mutex_unlock(&priv->mutex);
>                 return gsel;

Please do not mix "cleanup + return" and "goto cleanup" style in the same
function.  I.e. goto new label below.

> +       }
>
>         /*
>          * Register a single group function where the 'data' is an array PSEL

> @@ -398,6 +404,7 @@ static int rza2_dt_node_to_map(struct pinctrl_dev *pctldev,
>
>  remove_group:
>         pinctrl_generic_remove_group(pctldev, gsel);

^^ new label here.

> +       mutex_unlock(&priv->mutex);
>
>         dev_err(priv->dev, "Unable to parse DT node %s\n", np->name);
>

The rest LGTM (unless I'm missing something critical in the real root
cause of the issue?).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds