Re: [PATCH] pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-10-28 15:30:38 [-0500], Ryan Brue wrote:
> On 10/20/26 2:32 AM, Sebastian Andrzej Siewior wrote:
> > bulk writes could be a concern. The 40us delay is within the lock
> > acquired? If so, could this be done without the lock? I assume this is
> > part of the init sequence so nothing should use the HW so the writes
> > should not be interrupted by other writes to the device.
> 
> Yes, it's under the lock. regmap_multi_reg_write() holds the regmap lock for
> the whole sequence, and since an MMIO syscon's regmap can't sleep, the
> delays are udelay()s. The PLL's enable runs that sequence (13 writes, 40 us
> of delays) whenever the PLL goes from off to on, retrying up to 10 times
> until it locks.

If this is only at boot-up then it is less of a problem. It would be
more of a problem if a driver, such as SPI, would use this to fill its
entire FIFO during a regular transfer. This would then spike up during
the transfer.

> In practice it's init-time, though. The G12A USB3/PCIe PHY takes the PLL
> with devm_clk_get_enabled() at probe and keeps it on, so it runs only once.
> Nothing else writes the PLL then, but the lock is the HHI syscon's, so on RT
> the other clocks in that block would wait it out. Running it without the
> lock would mean changing how the meson PLL code writes its init sequence,
> which I wouldn't want to take on in a pinctrl fix (unless that were
> preferred).

Okay.

> > So it would be safe for now…
> Yes, that's the trade-off with the second option, which relies on the DIN
> never being written in a multi-step sequence. Nothing enforces that though.
> 
> I'd like to send a v2 after -rc1, but I don't know the best option for the
> fix. Still waiting on Lee, Arnd, and Mark to pitch in if they have a
> preference, or any option I've missed.

This is (I think) the second time the syscon mapping becomes a problem.
I would be a bit worried if the same mapping/ regs could be accessed by
different drivers under different locks.
But yeah…

> Best regards,
> Ryan Brue

Sebastian