Re: [PATCH v2 2/2] soc: renesas: rcar-mfis: add hwspinlock support

Hi Wolfram,

Thanks for your patch!

Any specific reason this is not CCed to the hwspinlock maintainers/list?

On Sat, 18 Jul 2026 at 20:24, Wolfram Sang
<[email protected]> wrote:
> Every MFIS instance has a block of hardware spinlocks. Add support for
> them. Gen4 has only one instance, so the base_id is always 0. Gen5 has
> multiple instances, so the base_id has to be encoded in the info
> description. Also being a provider, a hwspinlock-private header needs
> to be included. Work to refactor the headers is on-going, but will need
> many preparational steps. Until then, we need to live with the special
> include.
>
> Signed-off-by: Wolfram Sang <[email protected]>

> --- a/drivers/soc/renesas/rcar-mfis.c
> +++ b/drivers/soc/renesas/rcar-mfis.c
> @@ -59,6 +67,9 @@ struct mfis_priv {
>         /* mailbox private data */
>         struct mbox_controller mbox;
>         struct mfis_chan_priv *chan_privs;
> +
> +       /* hwspinlock private data */
> +       struct hwspinlock_device bank;

Please document this member must be last, as it contains a flexible
array, which is not obvious to the casual reader.

>  };
>
>  static u32 mfis_read(struct mfis_reg *mreg, unsigned int reg)
> @@ -86,6 +97,39 @@ static void mfis_write(struct mfis_reg *mreg, u32 reg, u32 val)
>         raw_spin_unlock_irqrestore(&priv->unprotect_lock, flags);
>  }
>
> +/********************************************************
> + *                     HW Spinlocks                    *
> + ********************************************************/
> +
> +#define MFISLCKR8_CH_OFS (MFISLCKR8 - 8 * sizeof(u32))
> +#define hwlock_to_local_id(hwlock) ((hwlock) - &(hwlock)->bank->lock[0])

Perhaps move this to drivers/hwspinlock/hwspinlock_internal.h
for general use, and use it in the existing hwlock_to_id()?

Or else, open-code "hwlock_to_id(lock) - hwlock->bank->base_id" below?

> +
> +static int rcar_mfis_hwsp_trylock(struct hwspinlock *lock)
> +{
> +       struct mfis_priv *priv = lock->priv;
> +       int id = hwlock_to_local_id(lock);
> +       u32 val, reg;
> +
> +       reg = id * sizeof(u32) + (id < 8 ? MFISLCKR0 : MFISLCKR8_CH_OFS);
> +       val = mfis_read(&priv->common_reg, reg);
> +       return !val;
> +}
> +
> +static void rcar_mfis_hwsp_unlock(struct hwspinlock *lock)
> +{
> +       struct mfis_priv *priv = lock->priv;
> +       int id = hwlock_to_local_id(lock);
> +       u32 reg;
> +
> +       reg = id * sizeof(u32) + (id < 8 ? MFISLCKR0 : MFISLCKR8_CH_OFS);
> +       mfis_write(&priv->common_reg, reg, 0);
> +}

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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