Re: [PATCH v2] pinctrl: sunxi: keep a shadow copy of the data register output latches

[Linus Walleij]

On Thu, Sep 3, 2026 at 11:17 AM Ilya Titov <ilya.titov@xxxxxxxxxxxxxx> wrote:

> On Allwinner SoCs, reading a bank's data register returns the pin level,
> not the output latch, for pins that are muxed as inputs.  Writing a GPIO
> therefore corrupts the output latches of all input-muxed pins in the
> same bank: the read-modify-write in sunxi_pinctrl_gpio_set() reads back
> their pin levels and writes those into their latches.
>
> This breaks emulated open-drain lines (e.g. a bit-banged I2C bus from
> i2c-gpio).  Such a line is released high by muxing it as input and
> letting the pull-up raise it, so any concurrent GPIO write in the same
> bank stores 1 into its latch.  Driving the line low afterwards is a
> non-atomic data-then-mux sequence in sunxi_pinctrl_gpio_direction_output();
> if the poisoning write lands between the two steps, the pin actively
> drives high (push-pull) instead of low.
>
> Observed in practice as sporadic glitches on a T507 board bit-banging
> I2C on port E while other PE GPIOs are toggled.  On a scope the failure
> is unmistakable: on a clock pulse where SCL should fall to GND, the line
> instead steps *above* its idle high level for the whole low phase — the
> pad drives a strong push-pull 3.3 V high, higher than the level the
> pull-up sustains on the loaded bus — before the next transition recovers
> it.  The same can hit SDA, corrupting data instead of clocks.
>
> Steps to reproduce on any sunxi board with a bit-banged (i2c-gpio) bus:
>
>   # background: toggle any other GPIO of the same bank, e.g. line 21
>   gpioset -c <chip> --toggle 100us 21=0 &
>
>   # foreground: keep the bit-banged bus busy
>   while :; do i2cdetect -y <bus> 0x50 0x57; done
>
>   # watch SCL/SDA with a scope or logic analyzer: sporadic clock-low
>   # phases driven high (above the pull-up level) instead of low
>
> The bank spinlock cannot help: the racing write is a perfectly valid
> whole-register RMW that faithfully writes back what the hardware
> returned.  There are no set/clear registers on this IP to write a single
> bit atomically.
>
> Fix it the same way gpio-mmio handles hardware whose data register read
> does not return the output latch: keep a shadow copy of each bank's
> latches, base the read-modify-write on the shadow, and only write the
> register.  The shadow is seeded from the hardware at probe time so pins
> left in output mode by the bootloader keep their state.  Pins that reach
> output mode through the gpiolib paths write their value (and thereby
> their shadow bit) before the mux switch in
> sunxi_pinctrl_gpio_direction_output(); pins muxed to gpio_out directly
> through a pinmux node bypass that path, so sunxi_pmx_set() refreshes
> their shadow bit from the latch (readable once the pin is in output
> mode) to keep them driving their pre-existing level.
>
> Seeding the shadow reads the PIO registers at probe time, which requires
> the bus clock to be enabled.  The clock was only requested at the very
> end of probe, after devm_pinctrl_register() had already claimed the pin
> hogs described in the device tree - which mux pins, and thus access
> registers, with the clock still gated.  Move the request ahead of both.
> Boards whose bootloader leaves the PIO clock running are unaffected,
> which is why the pre-existing hog problem has gone unnoticed since
> commit 950707c0eb5c ("pinctrl: sunxi: add clock support").
>
> Fixes: df7b34f4c3d2 ("pinctrl: sunxi: Fix gpio_set behaviour")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Ilya Titov <ilya.titov@xxxxxxxxxxxxxx>

Patch applied for fixes.

Yours,
Linus Walleij