FAILED: patch "[PATCH] pinctrl: sunxi: keep a shadow copy of the data register" failed to apply to 6.1-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x a13f7f5d14af9baf34eb12c25962f8d3542b281d
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026092930-ethics-jawless-929e@gregkh' --subject-prefix 'PATCH 6.1.y' 'HEAD^..'
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From a13f7f5d14af9baf34eb12c25962f8d3542b281d Mon Sep 17 00:00:00 2001
From: Ilya Titov <[email protected]>
Date: Thu, 3 Sep 2026 12:17:18 +0300
Subject: [PATCH] pinctrl: sunxi: keep a shadow copy of the data register
output latches
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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").
drivers/pinctrl/sunxi/pinctrl-sunxi.c +61 -15
@@ -837,6 +837,21 @@ static void sunxi_pmx_set(struct pinctrl_dev *pctldev,writel ((readl (pctl->membase + reg) & ~mask) | config << shift, pctl->membase + reg); +/* +* A pin muxed to gpio_out directly through a pinmux node bypasses +* sunxi_pinctrl_gpio_set() and drives whatever its output latch +* holds. Now that the pin is in output mode the data register +* reads back the latch, so refresh the shadow to keep such pins +* driving their pre-existing level. +*/ +if (config ==SUN4I_FUNC_OUTPUT ) { +u32 *shadow = &pctl->dat_shadow[pin /PINS_PER_BANK ]; + +sunxi_data_reg (pctl, pin, ®, &shift, &mask); + *shadow = (*shadow & ~mask) | + (readl (pctl->membase + reg) & mask); + } +raw_spin_unlock_irqrestore (&pctl->lock, flags); }
@@ -1017,21 +1032,29 @@ static int sunxi_pinctrl_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) {struct sunxi_pinctrl *pctl =gpiochip_get_data (chip); -u32 reg, shift, mask, val; +u32 *shadow = &pctl->dat_shadow[offset /PINS_PER_BANK ]; +u32 reg, shift, mask;unsigned long flags;sunxi_data_reg (pctl, offset, ®, &shift, &mask);raw_spin_lock_irqsave (&pctl->lock, flags); - val =readl (pctl->membase + reg); - +/* +* Reading the data register returns the pin level, not the output +* latch, for pins muxed as inputs. A read-modify-write based on +* the register would therefore corrupt the latches of input-muxed +* pins in the same bank (e.g. an emulated open-drain I2C line +* released high), making them drive the wrong level once switched +* to output. Base the read-modify-write on a shadow copy of the +* latches instead. +*/ if (value) - val |= mask; + *shadow |= mask;else - val &= ~mask; + *shadow &= ~mask; -writel (val, pctl->membase + reg); +writel (*shadow, pctl->membase + reg);raw_spin_unlock_irqrestore (&pctl->lock, flags);
@@ -1572,7 +1595,7 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,struct pinctrl_pin_desc *pins;struct sunxi_pinctrl *pctl;struct pinmux_ops *pmxops; -int i, ret, last_pin, pin_idx; +int i, ret, last_pin, pin_idx, nbanks;struct clk *clk; pctl =devm_kzalloc (&pdev->dev,sizeof (*pctl),GFP_KERNEL );
@@ -1610,6 +1633,37 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,if (!pctl->irq_array)return -ENOMEM ; +/* +* The bus clock has to be enabled before the pinctrl device +* registers, as the pin hogs claimed from there access registers. +*/ + ret =of_clk_get_parent_count (node); + clk =devm_clk_get_enabled (&pdev->dev, ret ==1 ?NULL :"apb" ); +if (IS_ERR (clk)) +return PTR_ERR (clk); + +/* +* Seed the output latch shadow from the hardware so pins the +* bootloader left in output mode keep their state; see +* sunxi_pinctrl_gpio_set() for why a shadow is needed. This must +* happen before the pinctrl device registers, as pin hogs can mux +* pins to gpio_out and thereby update the shadow. +*/ + last_pin = pctl->desc->pins[pctl->desc->npins -1 ].pin.number; + nbanks =DIV_ROUND_UP (last_pin +1 - pctl->desc->pin_base, +PINS_PER_BANK ); + pctl->dat_shadow =devm_kcalloc (&pdev->dev, nbanks, +sizeof (*pctl->dat_shadow),GFP_KERNEL ); +if (!pctl->dat_shadow) +return -ENOMEM ; + +for (i =0 ; i < nbanks; i++) { +u32 reg, shift, mask; + +sunxi_data_reg (pctl, i *PINS_PER_BANK , ®, &shift, &mask); + pctl->dat_shadow[i] =readl (pctl->membase + reg); + } + ret =sunxi_pinctrl_build_state (pdev);if (ret) {dev_err (&pdev->dev,"dt probe failed: %d\n" , ret);
@@ -1665,7 +1719,6 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,if (!pctl->chip)return -ENOMEM ; - last_pin = pctl->desc->pins[pctl->desc->npins -1 ].pin.number; pctl->chip->owner =THIS_MODULE ; pctl->chip->request = gpiochip_generic_request; pctl->chip->free = gpiochip_generic_free;
@@ -1699,13 +1752,6 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev, goto gpiochip_error; } - ret =of_clk_get_parent_count (node); - clk =devm_clk_get_enabled (&pdev->dev, ret ==1 ?NULL :"apb" ); -if (IS_ERR (clk)) { - ret =PTR_ERR (clk); - goto gpiochip_error; - } - pctl->irq = devm_kcalloc(&pdev->dev, pctl->desc->irq_banks, sizeof(*pctl->irq),
drivers/pinctrl/sunxi/pinctrl-sunxi.h +7 -0
@@ -85,6 +85,7 @@#define IO_BIAS_MASK GENMASK(3, 0)#define SUN4I_FUNC_INPUT 0 +#define SUN4I_FUNC_OUTPUT 1#define SUN4I_FUNC_IRQ 6#define SUN4I_FUNC_DISABLED_OLD 7#define SUN4I_FUNC_DISABLED_NEW 15
@@ -175,6 +176,12 @@ struct sunxi_pinctrl {int *irq;unsigned *irq_array;raw_spinlock_t lock; +/* +* Output latch shadow, one word per bank. Seeded lockless at +* probe before the pinctrl device registers, protected by @lock +* afterwards. +*/ +u32 *dat_shadow;struct pinctrl_dev *pctl_dev;unsigned long flags;u32 bank_mem_size;
PATCH 1/1 pinctrl: sunxi: keep a shadow copy of the data register output latches
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").
---
Backport of a13f7f5d14af to 6.1.y. The only conflict is in sunxi_pinctrl_init_with_variant(): upstream had already converted the "apb" clock to devm_clk_get_enabled() by the time this commit moved the request ahead of devm_pinctrl_register(), while 6.1.y still does devm_clk_get() + clk_prepare_enable() and unwinds through a clk_error label. Resolved by requesting the clock with devm_clk_get_enabled() at the new location -- present in 6.1.y -- so the manual unprepare goes away and the three former clk_error gotos land on gpiochip_error instead. The rest applies as-is, and the resulting patch is identical to the one sent for 6.6.y.
Tested: the bug was found and the fix verified on a Wiren Board 8.5.1 (Allwinner T507) running a 6.18-based kernel, where a bit-banged I2C bus on port E survives 200 probe rounds while another PE line toggles every 100 us in the background. This backport itself is build tested only:
arm64 allmodconfig, all 26 PINCTRL_SUN* variants, W=1, no new warnings.
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 88 +++++++++++++++++++-------- drivers/pinctrl/sunxi/pinctrl-sunxi.h | 7 +++ 2 files changed, 71 insertions(+), 24 deletions(-)
drivers/pinctrl/sunxi/pinctrl-sunxi.c +64 -24
@@ -793,6 +793,21 @@ static void sunxi_pmx_set(struct pinctrl_dev *pctldev,writel ((readl (pctl->membase + reg) & ~mask) | config << shift, pctl->membase + reg); +/* +* A pin muxed to gpio_out directly through a pinmux node bypasses +* sunxi_pinctrl_gpio_set() and drives whatever its output latch +* holds. Now that the pin is in output mode the data register +* reads back the latch, so refresh the shadow to keep such pins +* driving their pre-existing level. +*/ +if (config ==SUN4I_FUNC_OUTPUT ) { +u32 *shadow = &pctl->dat_shadow[pin /PINS_PER_BANK ]; + +sunxi_data_reg (pctl, pin, ®, &shift, &mask); + *shadow = (*shadow & ~mask) | + (readl (pctl->membase + reg) & mask); + } +raw_spin_unlock_irqrestore (&pctl->lock, flags); }
@@ -946,21 +961,29 @@ static void sunxi_pinctrl_gpio_set(struct gpio_chip *chip, unsigned offset, int value) {struct sunxi_pinctrl *pctl =gpiochip_get_data (chip); -u32 reg, shift, mask, val; +u32 *shadow = &pctl->dat_shadow[offset /PINS_PER_BANK ]; +u32 reg, shift, mask;unsigned long flags;sunxi_data_reg (pctl, offset, ®, &shift, &mask);raw_spin_lock_irqsave (&pctl->lock, flags); - val =readl (pctl->membase + reg); - +/* +* Reading the data register returns the pin level, not the output +* latch, for pins muxed as inputs. A read-modify-write based on +* the register would therefore corrupt the latches of input-muxed +* pins in the same bank (e.g. an emulated open-drain I2C line +* released high), making them drive the wrong level once switched +* to output. Base the read-modify-write on a shadow copy of the +* latches instead. +*/ if (value) - val |= mask; + *shadow |= mask;else - val &= ~mask; + *shadow &= ~mask; -writel (val, pctl->membase + reg); +writel (*shadow, pctl->membase + reg);raw_spin_unlock_irqrestore (&pctl->lock, flags); }
@@ -1484,7 +1507,7 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,struct pinctrl_pin_desc *pins;struct sunxi_pinctrl *pctl;struct pinmux_ops *pmxops; -int i, ret, last_pin, pin_idx; +int i, ret, last_pin, pin_idx, nbanks;struct clk *clk; pctl =devm_kzalloc (&pdev->dev,sizeof (*pctl),GFP_KERNEL );
@@ -1518,6 +1541,37 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,if (!pctl->irq_array)return -ENOMEM ; +/* +* The bus clock has to be enabled before the pinctrl device +* registers, as the pin hogs claimed from there access registers. +*/ + ret =of_clk_get_parent_count (node); + clk =devm_clk_get_enabled (&pdev->dev, ret ==1 ?NULL :"apb" ); +if (IS_ERR (clk)) +return PTR_ERR (clk); + +/* +* Seed the output latch shadow from the hardware so pins the +* bootloader left in output mode keep their state; see +* sunxi_pinctrl_gpio_set() for why a shadow is needed. This must +* happen before the pinctrl device registers, as pin hogs can mux +* pins to gpio_out and thereby update the shadow. +*/ + last_pin = pctl->desc->pins[pctl->desc->npins -1 ].pin.number; + nbanks =DIV_ROUND_UP (last_pin +1 - pctl->desc->pin_base, +PINS_PER_BANK ); + pctl->dat_shadow =devm_kcalloc (&pdev->dev, nbanks, +sizeof (*pctl->dat_shadow),GFP_KERNEL ); +if (!pctl->dat_shadow) +return -ENOMEM ; + +for (i =0 ; i < nbanks; i++) { +u32 reg, shift, mask; + +sunxi_data_reg (pctl, i *PINS_PER_BANK , ®, &shift, &mask); + pctl->dat_shadow[i] =readl (pctl->membase + reg); + } + ret =sunxi_pinctrl_build_state (pdev);if (ret) {dev_err (&pdev->dev,"dt probe failed: %d\n" , ret);
@@ -1572,7 +1626,6 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,if (!pctl->chip)return -ENOMEM ; - last_pin = pctl->desc->pins[pctl->desc->npins -1 ].pin.number; pctl->chip->owner =THIS_MODULE ; pctl->chip->request = gpiochip_generic_request; pctl->chip->free = gpiochip_generic_free;
@@ -1605,31 +1658,20 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev, goto gpiochip_error; } - ret =of_clk_get_parent_count (node); - clk =devm_clk_get (&pdev->dev, ret ==1 ?NULL :"apb" ); -if (IS_ERR (clk)) { - ret =PTR_ERR (clk); - goto gpiochip_error; - } - - ret =clk_prepare_enable (clk); -if (ret) - goto gpiochip_error; - pctl->irq =devm_kcalloc (&pdev->dev, pctl->desc->irq_banks,sizeof (*pctl->irq),GFP_KERNEL );if (!pctl->irq) { ret = -ENOMEM ; - goto clk_error; + goto gpiochip_error; }for (i =0 ; i < pctl->desc->irq_banks; i++) { pctl->irq[i] =platform_get_irq (pdev, i);if (pctl->irq[i] <0 ) { ret = pctl->irq[i]; - goto clk_error; + goto gpiochip_error; } }
@@ -1640,7 +1682,7 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,if (!pctl->domain) {dev_err (&pdev->dev,"Couldn't register IRQ domain\n" ); ret = -ENOMEM ; - goto clk_error; + goto gpiochip_error; }for (i =0 ; i < (pctl->desc->irq_banks *IRQ_PER_BANK ); i++) {
@@ -1672,8 +1714,6 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,return 0 ; -clk_error: -clk_disable_unprepare (clk); gpiochip_error:gpiochip_remove (pctl->chip);return ret;
drivers/pinctrl/sunxi/pinctrl-sunxi.h +7 -0
@@ -80,6 +80,7 @@#define IO_BIAS_MASK GENMASK(3, 0)#define SUN4I_FUNC_INPUT 0 +#define SUN4I_FUNC_OUTPUT 1#define SUN4I_FUNC_IRQ 6#define PINCTRL_SUN5I_A10S BIT(1)
@@ -173,6 +174,12 @@ struct sunxi_pinctrl {int *irq;unsigned *irq_array;raw_spinlock_t lock; +/* +* Output latch shadow, one word per bank. Seeded lockless at +* probe before the pinctrl device registers, protected by @lock +* afterwards. +*/ +u32 *dat_shadow;struct pinctrl_dev *pctl_dev;unsigned long variant;u32 bank_mem_size;
Signature · 1 line
2.54.0 (Apple Git-157)
Re: [PATCH 6.1.y] pinctrl: sunxi: keep a shadow copy of the data register output latches
Quoted 5 lines
Backport of
a13f7f5d14afto 6.1.y. The only conflict is insunxi_pinctrl_init_with_variant(): upstream had already converted the "apb" clock todevm_clk_get_enabled()by the time this commit moved the request ahead ofdevm_pinctrl_register(), while 6.1.y still doesdevm_clk_get()+clk_prepare_enable()and unwinds through aclk_errorlabel.
Queued for 6.1, thanks.
Signature · 2 lines
Thanks, Sasha