Re: [PATCH net-next v8 1/2] net: dsa: qca8k: let regmap serialize register access

From: Christian Marangi

Date: Fri Nov 06 2026 - 13:11:42 EST


On Fri, Nov 06, 2026 at 02:38:37PM +0100, Andrew Lunn wrote:
> On Wed, Nov 04, 2026 at 01:01:28PM +0100, Yongzhao Chen wrote:
> > From: Stanislaw Pal <kuncy7@xxxxxxxxx>
> >
> > The regmap is created with .disable_locking, on the ground that the
> > qca8k read and write callbacks do their own locking. They only lock the
> > bus transaction, though. Since the driver moved to the regmap
> > read/write API, regmap formats the register and parses the value in the
> > map's shared work_buf, outside of the callbacks, so two concurrent
> > users can swap or corrupt each other's register and value.
> >
> > Most register access runs under rtnl, but not all of it: phylink's
> > resolve work (mac_link_up/down) and the LED class callbacks do not, so
> > a link change can race with an ethtool or LED access and write the
> > wrong switch register or read back the wrong value.
>
> Do any of these operations which don't take RTNL do an open coded
> read/modify/write? Does the regmap read/modify/write operations hold
> the regmap mutex over the entire operation?
>

Maybe I'm just tired from destroying board from my HW project but... I'm a
bit confused of when read/write are not serialized.

Operation of the LED and on the port have their own dedicated register per
port and for shared stuff, there is a dedicated mutex for it.

So I'm not sure what case this patch is fixing...

Both mgmt and mdio operation locks at the start and unlock at the end of
the read/write or rmw operation.

Using the regmap lock seems redundant and this is why I disabled
originally.

I would like to have more explaination for the

"regmap formats the register and parses the value in the
map's shared work_buf, outside of the callbacks, so two concurrent
users can swap or corrupt each other's register and value."

case.

--
Ansuel