Re: [PATCH v2 07/14] mfd: zl3073x: Add components versions register defs

From: Andrew Lunn
Date: Tue Apr 15 2025 - 08:58:34 EST


> Hi Andrew,
> the idea looks interesting but there are some caveats and disadvantages.
> I thought about it but the idea with two regmaps (one for simple registers
> and one for mailboxes) where the simple one uses implicit locking and
> mailbox one has locking disabled with explicit locking requirement. There
> are two main problems:
>
> 1) Regmap cache has to be disabled as it cannot be shared between multiple
> regmaps... so also page selector cannot be cached.
>
> 2) You cannot mix access to mailbox registers and to simple registers. This
> means that mailbox accesses have to be wrapped e.g. inside scoped_guard()
>
> The first problem is really pain as I would like to extend later the driver
> with proper caching (page selector for now).
> The second one brings only confusions for a developer how to properly access
> different types of registers.
>
> I think the best approach would be to use just single regmap for all
> registers with implicit locking enabled and have extra mailbox mutex to
> protect mailbox registers and ensure atomic operations with them.
> This will allow to use regmap cache and also intermixing mailbox and simple
> registers' accesses won't be an issue.

As i said, it was just an idea, i had no idea if it was a good idea.

What is important is that the scope of the locking becomes clear,
unlike what the first version had. So locking has to be pushed down to
the lower levels so you lock a single register access, or you lock an
mailbox access.

Also, you say this is an MFD partially because GPIOs could be added
later. I assume that GPIO code would have the same locking issue,
which suggests the locking should be in the MFD core, not the
individual drivers stacked on top of it.

Andrew