[PATCH RFT] pinctrl: sunxi: Fix A733 withstand programming

[PATCH RFT] pinctrl: sunxi: Fix A733 withstand programming

[Andre Przywara]

On top of the other pinctrl changes that the Allwinner A733 introduced, it
also uses a different layout for the GPIO_POW_MOD_SEL register, to set up
the GPIO input threshold voltage for each port. For no apparent reason,
it uses now 2 bits per bank, though effectively just always setting the
higher bit to 1 is all that's needed to make it work.

Add a new BIAS_VOLTAGE_PIO_POW_SEL variant, that behaves similar to the
existing one, but uses the 2-bit encoding. Use that variant for the A733
pinctrl driver.

Fixes: 0535bfbfa97b ("pinctrl: sunxi: add support for the Allwinner A733")
Reported-by: Vinicius Pedrosa <vinicius.eduardo.pedrosa@gmail.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
Hi,

this goes on top of LinusW's pinctrl/for-next branch, which contains
both the A523 withstand fix and the A733 pinctrl bits.
Did not test on hardware, please have a look!

Cheers,
Andre

 drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c |  2 +-
 drivers/pinctrl/sunxi/pinctrl-sunxi.c       | 12 +++++++++---
 drivers/pinctrl/sunxi/pinctrl-sunxi.h       |  4 ++++
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c b/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
index ca82f08a3a65b..dcc2254e4089f 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
@@ -24,7 +24,7 @@ static const u8 a733_irq_bank_muxes[SUNXI_PINCTRL_MAX_BANKS] =
 static struct sunxi_pinctrl_desc a733_pinctrl_data = {
 	.irq_banks = 10,
 	.irq_read_needs_mux = true,
-	.io_bias_cfg_variant = BIAS_VOLTAGE_PIO_POW_MODE_SEL,
+	.io_bias_cfg_variant = BIAS_VOLTAGE_PIO_POW_MODE_SEL_2BIT,
 };
 
 static int a733_pinctrl_probe(struct platform_device *pdev)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 3ee5012463da3..7f4ce73bdc670 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -783,6 +783,7 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
 		raw_spin_unlock_irqrestore(&pctl->lock, flags);
 
 		fallthrough;
+	case BIAS_VOLTAGE_PIO_POW_MODE_SEL_2BIT:
 	case BIAS_VOLTAGE_PIO_POW_MODE_SEL:
 		val = uV <= 1800000 ? 1 : 0;
 		if (inverted)
@@ -790,9 +791,14 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
 
 		raw_spin_lock_irqsave(&pctl->lock, flags);
 		reg = readl(pctl->membase + pctl->pow_mod_sel_offset);
-		reg &= ~(1 << bank);
-		writel(reg | val << bank,
-		       pctl->membase + pctl->pow_mod_sel_offset);
+		if (pctl->desc->io_bias_cfg_variant == BIAS_VOLTAGE_PIO_POW_MODE_SEL_2BIT) {
+			reg &= ~(3U << (bank * 2));
+			reg |= (val | 0x2) << (bank * 2);
+		} else {
+			reg &= ~BIT(bank);
+			reg |= val << bank;
+		}
+		writel(reg, pctl->membase + pctl->pow_mod_sel_offset);
 		raw_spin_unlock_irqrestore(&pctl->lock, flags);
 		return 0;
 	default:
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index 7c2f29e75519a..0945890a921b6 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -122,6 +122,10 @@ enum sunxi_desc_bias_voltage {
 	 * register, as seen on H6 SoC, for example.
 	 */
 	BIAS_VOLTAGE_PIO_POW_MODE_SEL,
+	/*
+	 * Same as above, but using a 2-bits-per-bank encoding.
+	 */
+	BIAS_VOLTAGE_PIO_POW_MODE_SEL_2BIT,
 	/*
 	 * Bias voltage is set through PIO_POW_MOD_SEL_REG
 	 * and PIO_POW_MOD_CTL_REG register, as seen on
-- 
2.43.0

Re: [PATCH RFT] pinctrl: sunxi: Fix A733 withstand programming

[Vinicius Pedrosa]

Hi Andre,

Thanks for the RFT. I found two things when checking it against the A733
manual and my Cubie A7Z.

PB is an exception to bank * 2: its VCC-IO field is at bits 25:24, while
bits 3:2 are reserved. With the board's SEL value (0x01155550), a 3.3 V
request produces 0x01155558, leaving VCC-IO untouched.

I also think we should preserve AUTO (01). On my A7Z with vendor 5.15,
PF=10 resulted in 22 SDR104 read timeouts and no rootfs, while PF=01 and
PF=11 reached login. This was one run per setting with one card, not a
mainline before/after test. In my mainline setup, MMC0 already requests PF
with a fixed 3.3 V supply, so the RFT would replace AUTO with 10 even
without voltage switching. Preserving AUTO requires no special case for PF.

I boot-tested your RFT with two small fixups for PB and AUTO on my Cubie
A7Z (rev B). UART0 and MMC0 work, and GPIO_POW_MOD_SEL remains at
0x01155550. No voltage-field writes were expected with this DT. I haven't
tested the RFT as posted.

I agree the full table and PI exclusion aren't needed, and found no
evidence for restricting the encoding to revision B.

A733-R still needs its own descriptor for the two-bit PL/PM fields. I'm
happy for your RFT to supersede my patch 1 once PB and AUTO are addressed,
and I'll rebase the A733-R fix on it.

I have the PB/AUTO fixups and the A733-R patch ready, and can send them if
useful.

Thanks,
Vinicius

Re: [PATCH RFT] pinctrl: sunxi: Fix A733 withstand programming

[Andre Przywara]

On Tue, 13 Oct 2026 14:23:38 -0300
Vinicius Pedrosa <vinicius.eduardo.pedrosa@gmail.com> wrote:

Hi Vinicius,

thanks for having a look and testing it!

> Thanks for the RFT. I found two things when checking it against the A733
> manual and my Cubie A7Z.
> 
> PB is an exception to bank * 2: its VCC-IO field is at bits 25:24, while
> bits 3:2 are reserved.

So PortB has no supply of its own, instead it's always supplied by
VCC-IO. That was even more common on older SoCs, with also PortH and
PortF being supplied by that. Which somewhat means that this is broken
there as well?

> With the board's SEL value (0x01155550), a 3.3 V

What do you mean with "the board's SEL value"? That's not the SoC reset
value, is it? So who programs that value, and why?

> request produces 0x01155558, leaving VCC-IO untouched.

... naturally, because nothing in mainline sets VCC-IO, which is beyond
the SoC's GPIO bank offset.

> I also think we should preserve AUTO (01). On my A7Z with vendor 5.15,
> PF=10 resulted in 22 SDR104 read timeouts and no rootfs, while PF=01 and
> PF=11 reached login.

So this means you hacked the register in the vendor kernel, for the
sake of an experiment? And I guess this just means that the vendor
kernel uses UHS, and uses register 0x70 to switch PortF to 1.8V? So you
need either auto or 1.8V for it to work, the same problem we hit with
PortC on the A523.

> This was one run per setting with one card, not a
> mainline before/after test. In my mainline setup, MMC0 already requests PF
> with a fixed 3.3 V supply, so the RFT would replace AUTO with 10 even
> without voltage switching. Preserving AUTO requires no special case for PF.

So can you say whether you observed a real problem with either the
current (broken) code, or with one the fixes? I am still confused about
your reports, whether they are just experiments, and comparing to the
BSP, or looking at the manual, or whether there is an actually observed
problem.

> I boot-tested your RFT with two small fixups for PB and AUTO on my Cubie
> A7Z (rev B). UART0 and MMC0 work, and GPIO_POW_MOD_SEL remains at
> 0x01155550. No voltage-field writes were expected with this DT. I haven't
> tested the RFT as posted.
> 
> I agree the full table and PI exclusion aren't needed, and found no
> evidence for restricting the encoding to revision B.

Thanks for the confirmation, and just curious where this revB
restriction came from, originally?

> A733-R still needs its own descriptor for the two-bit PL/PM fields. I'm
> happy for your RFT to supersede my patch 1 once PB and AUTO are addressed,
> and I'll rebase the A733-R fix on it.
> 
> I have the PB/AUTO fixups and the A733-R patch ready, and can send them if
> useful.

Well, this is getting really messy, isn't it? I never understood why
the IP even allows manual programming if there is an automatic setting.
Did you find any recommendation? IIRC manuals for other SoCs did
recommend one or the other method. If AUTO works, and is used by the
BSP, then we can just use that, but in a much simpler way: either use 
BIAS_VOLTAGE_NONE, or create a new AUTO name (better), and then skip
the whole action in sunxi_pinctrl_set_io_bias_cfg(). Instead just once
program the GPIO_POW_MOD_SEL to "all auto", once at init time.
Curious what others think about this.

Cheers,
Andre