Skip to contentSkip to conversation

pinctrl: sunxi: preserve A733 firmware voltage modes

PATCH In review Enzo Adriano 2 participants 3 messages Nov 4 – Nov 10

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 pinctrl: sunxi: preserve A733 firmware voltage modes

The A733 GPIO_POW_MOD_SEL voltage-mode fields are two bits wide on both controllers (A733 User Manual v1.00, sections 18.14.6.1 and 18.14.7.20). On the main controller, the one-bit BIAS_VOLTAGE_PIO_POW_MODE_SEL handler writes bit N for bank N, which is reserved or half of another bank's field.

Starting from the vendor firmware value 0x01155550, a request for PF with a 1.8 V supply sets bit 5, changing PC from auto (01) to 1.8 V (11). A request for PK with a 3.3 V supply clears bit 10, changing PF from auto to 3.3 V. PB is supplied from VCC-IO (A733 Datasheet v1.01, table 4-5), whose field is at bits 25:24, but a PB request writes reserved bit 1 instead.

A733 R-PIO has SEL at 0x340 and VAL at 0x348 and nothing at 0x344 (section 18.14.5.2), where the BIAS_VOLTAGE_PIO_POW_MODE_CTL_INV handler it shares with A523-R writes the A523 control register. That handler also writes bit N of SEL inverted: for PL with a 3.3 V supply it sets bit 0, changing PL from 3.3 V (10) to 1.8 V (11).

v7.4-rc1 added A733 support with these handlers, so the writes happen on the first request of a pin in a bank with a vcc-pX-supply in the devicetree.

Select BIAS_VOLTAGE_NONE for both A733 controllers and give A733-R its own descriptor, keeping the A523-R descriptor and behaviour. This leaves the firmware voltage modes in place, including any that do not match their supplies. It does not implement voltage switching.

Fixes 0535bfbfa97b ("pinctrl: sunxi: add support for the Allwinner A733")
Fixes 31d68e10381c ("pinctrl: sunxi: a523-r: add a733-r compatible string")

---
Changes in v2:
- Rebased on v7.4-rc1, which added the A733 pinctrl driver. Only the context of the A733 hunk changed.
- Dropped the old base and its prerequisite-patch-id lines.
- Dropped the AI assistance and Assisted-by lines.
- Added Vinicius' Tested-by, from a Cubie A7Z on Linux 7.3-rc6 with his local A733 patches, applied with a context adjustment: warm boots only, no cold boot, 1.8 V switching or A523.
- Added Fixes tags, manual references and examples of the bits the old handlers write.

Andre's RFT [1] fixes the same bug with a two-bit handler and changes the same A733 line, so only one of the two can be applied. This patch uses BIAS_VOLTAGE_NONE, as Vinicius recommended in that thread, rather than an AUTO variant: manual section 18.14.4.6 gives auto as 01, but as 10 for PC/PE/PK, which the 18.14.6.1 table lists as 3.3 V, and there is no evidence that 01 is right on every silicon revision. The cost is that firmware modes that do not match a bank's supply stay as they are. Andre, if you prefer the AUTO variant, I can rework it.

[1] https://lore.kernel.org/r/[email protected]

Tested on v7.4-rc1 on a Cubie A7S with vendor boot firmware (one RAM boot from cold, then a warm reboot), with A733 CCU v6, AXP318W PMIC v8, the sunxi IRQ-bank v3 series, the 8250_dw changes, my A733 firmware-clock patch and A7S DT v2 on top. The main and R-PIO GPIO_POW_MOD_SEL registers (0x02000040, 0x07025340) read 0x01155550 and 0x000aaaaa in both U-Boot and Linux, and the PF selector (0x02000070) read 1. The PMIC regulators registered over R-I2C0 and MMC0 reads worked. The readback was taken before the unused-regulator cleanup.

The main-controller value distinguishes the handlers: MMC0 requests PF, and this DT gives PF a 1.8 V supply, so the old handler would have set bit 5 (0x01155570). An unpatched kernel was not booted. The R-PIO value does not: PL has no vcc-pl-supply in this DT, because PL carries the PMIC bus, so the R-I2C0 request returns before writing, and the old handler's write for PM (3.3 V) would set bit 1, which is already set. Vinicius' test covers R-PIO: he added 3.3 V vcc-pl and vcc-pm supplies, and also read the registers after the unused-regulator cleanup.

Also built arm64 defconfig with W=1 (GCC) and the changed objects with Clang. Not tested: 1.8 V switching, A523.

v1: https://lore.kernel.org/r/[email protected]

 drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c | 20 +++++++++++++++----
 drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c   |  3 ++-
 2 files changed, 18 insertions(+), 5 deletions(-)
drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c +16 -4
index cffc1e53eef1..1cfd81184350 100644
@@ -30,17 +30,29 @@ static struct sunxi_pinctrl_desc a523_r_pinctrl_data = {
 	.pin_base = PL_BASE,
 };
 
+static struct sunxi_pinctrl_desc a733_r_pinctrl_data = {
+	.irq_banks = ARRAY_SIZE(a523_r_irq_bank_map),
+	.irq_bank_map = a523_r_irq_bank_map,
+	/* A733 has two-bit voltage modes, without the A523 control register. */
+	.io_bias_cfg_variant = BIAS_VOLTAGE_NONE,
+	.pin_base = PL_BASE,
+};
+
 static int a523_r_pinctrl_probe(struct platform_device *pdev)
 {
+	struct sunxi_pinctrl_desc *desc;
+
+	desc = (struct sunxi_pinctrl_desc *)of_device_get_match_data(&pdev->dev);
 	return sunxi_pinctrl_dt_table_init(pdev, a523_r_nr_bank_pins,
-					   a523_r_irq_bank_muxes,
-					   &a523_r_pinctrl_data,
+					   a523_r_irq_bank_muxes, desc,
 					   SUNXI_PINCTRL_NCAT2_REG_LAYOUT);
 }
 
 static const struct of_device_id a523_r_pinctrl_match[] = {
-	{ .compatible = "allwinner,sun55i-a523-r-pinctrl", },
-	{ .compatible = "allwinner,sun60i-a733-r-pinctrl", },
+	{ .compatible = "allwinner,sun55i-a523-r-pinctrl",
+	  .data = &a523_r_pinctrl_data },
+	{ .compatible = "allwinner,sun60i-a733-r-pinctrl",
+	  .data = &a733_r_pinctrl_data },
 	{}
 };
 
drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c +2 -1
index ca82f08a3a65..d4b0a40525f9 100644
@@ -24,7 +24,8 @@ 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,
+	/* Preserve the firmware-configured two-bit voltage modes. */
+	.io_bias_cfg_variant = BIAS_VOLTAGE_NONE,
 };
 
 static int a733_pinctrl_probe(struct platform_device *pdev)

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Signature · 1 line
2.53.0
#1 lore raw

On Wed, 4 Nov 2026 15:56:39 -0500
Enzo Adriano <[email protected]> wrote:

Hi,

Quoted 6 lines

The A733 GPIO_POW_MOD_SEL voltage-mode fields are two bits wide on both controllers (A733 User Manual v1.00, sections 18.14.6.1 and 18.14.7.20). On the main controller, the one-bit BIAS_VOLTAGE_PIO_POW_MODE_SEL handler writes bit N for bank N, which is reserved or half of another bank's field.

Quoted 6 lines

Starting from the vendor firmware value 0x01155550, a request for PF with a 1.8 V supply sets bit 5, changing PC from auto (01) to 1.8 V (11). A request for PK with a 3.3 V supply clears bit 10, changing PF from auto to 3.3 V. PB is supplied from VCC-IO (A733 Datasheet v1.01, table 4-5), whose field is at bits 25:24, but a PB request writes reserved bit 1 instead.

I guess this has been written by an agent who like to talk? There is really little point in explaining in multiple lines that two-bits-per-bank is not compatible with one-bit-per-bank.

Quoted 6 lines

A733 R-PIO has SEL at 0x340 and VAL at 0x348 and nothing at 0x344 (section 18.14.5.2), where the BIAS_VOLTAGE_PIO_POW_MODE_CTL_INV handler it shares with A523-R writes the A523 control register. That handler also writes bit N of SEL inverted: for PL with a 3.3 V supply it sets bit 0, changing PL from 3.3 V (10) to 1.8 V (11).

Quoted 8 lines

v7.4-rc1 added A733 support with these handlers, so the writes happen on the first request of a pin in a bank with a vcc-pX-supply in the devicetree.

Select BIAS_VOLTAGE_NONE for both A733 controllers and give A733-R its own descriptor, keeping the A523-R descriptor and behaviour. This leaves the firmware voltage modes in place, including any that do not match their supplies. It does not implement voltage switching.

So I am not really happy with that. Given that we have existing problems with the banks not all lining up (PortB and VCC-IO and friends), just skipping the setup might be a pragmatic solution. But then the kernel should engage the auto setting, as we generally don't want to rely on firmware if we can help it.

So as I suggested before: please introduce a symbol like BIAS_VOLTAGE_PIO_POW_AUTO or so, which also skips the explicit bank setup, like BIAS_VOLTAGE_NONE does, but then triggers setting all banks to auto, somewhere in the generic pinctrl code. Just writing "01" to all banks should do it, don't get hung up on banks that the manual doesn't describe: mostly those bits either don't exist at all, or they belong to ports only exposed on other packages. PortI looks like such a candidate, reading the respective region tells me that there are 17 PortI pins implemented. Same thing we saw on the A523/T527 and also on H616/H700.

Quoted 19 lines
Fixes 0535bfbfa97b ("pinctrl: sunxi: add support for the Allwinner A733")
Fixes 31d68e10381c ("pinctrl: sunxi: a523-r: add a733-r compatible string")

---
Changes in v2:
- Rebased on v7.4-rc1, which added the A733 pinctrl driver. Only the context of the A733 hunk changed.
- Dropped the old base and its prerequisite-patch-id lines.
- Dropped the AI assistance and Assisted-by lines.
- Added Vinicius' Tested-by, from a Cubie A7Z on Linux 7.3-rc6 with his local A733 patches, applied with a context adjustment: warm boots only, no cold boot, 1.8 V switching or A523.
- Added Fixes tags, manual references and examples of the bits the old handlers write.

Quoted 8 lines

Andre's RFT [1] fixes the same bug with a two-bit handler and changes the same A733 line, so only one of the two can be applied. This patch uses BIAS_VOLTAGE_NONE, as Vinicius recommended in that thread, rather than an AUTO variant: manual section 18.14.4.6 gives auto as 01, but as 10 for PC/PE/PK, which the 18.14.6.1 table lists as 3.3 V, and there is no evidence that 01 is right on every silicon revision. The cost is that firmware modes that do not match a bank's supply stay as they are. Andre, if you prefer the AUTO variant, I can rework it.

As mentioned above: Yes, please do. And don't read too much into silicon revisions or the manual not mentioning everything.

Cheers,
Andre

Quoted 86 lines

[1] https://lore.kernel.org/r/[email protected]

Tested on v7.4-rc1 on a Cubie A7S with vendor boot firmware (one RAM boot from cold, then a warm reboot), with A733 CCU v6, AXP318W PMIC v8, the sunxi IRQ-bank v3 series, the 8250_dw changes, my A733 firmware-clock patch and A7S DT v2 on top. The main and R-PIO GPIO_POW_MOD_SEL registers (0x02000040, 0x07025340) read 0x01155550 and 0x000aaaaa in both U-Boot and Linux, and the PF selector (0x02000070) read 1. The PMIC regulators registered over R-I2C0 and MMC0 reads worked. The readback was taken before the unused-regulator cleanup.

The main-controller value distinguishes the handlers: MMC0 requests PF, and this DT gives PF a 1.8 V supply, so the old handler would have set bit 5 (0x01155570). An unpatched kernel was not booted. The R-PIO value does not: PL has no vcc-pl-supply in this DT, because PL carries the PMIC bus, so the R-I2C0 request returns before writing, and the old handler's write for PM (3.3 V) would set bit 1, which is already set. Vinicius' test covers R-PIO: he added 3.3 V vcc-pl and vcc-pm supplies, and also read the registers after the unused-regulator cleanup.

Also built arm64 defconfig with W=1 (GCC) and the changed objects with Clang. Not tested: 1.8 V switching, A523.

v1: https://lore.kernel.org/r/[email protected]

 drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c | 20 +++++++++++++++----
 drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c   |  3 ++-
 2 files changed, 18 insertions(+), 5 deletions(-)
drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c +16 -4
index cffc1e53eef1..1cfd81184350 100644
@@ -30,17 +30,29 @@ static struct sunxi_pinctrl_desc a523_r_pinctrl_data = {
 	.pin_base = PL_BASE,
 };
 
+static struct sunxi_pinctrl_desc a733_r_pinctrl_data = {
+	.irq_banks = ARRAY_SIZE(a523_r_irq_bank_map),
+	.irq_bank_map = a523_r_irq_bank_map,
+	/* A733 has two-bit voltage modes, without the A523 control register. */
+	.io_bias_cfg_variant = BIAS_VOLTAGE_NONE,
+	.pin_base = PL_BASE,
+};
+
 static int a523_r_pinctrl_probe(struct platform_device *pdev)
 {
+	struct sunxi_pinctrl_desc *desc;
+
+	desc = (struct sunxi_pinctrl_desc *)of_device_get_match_data(&pdev->dev);
 	return sunxi_pinctrl_dt_table_init(pdev, a523_r_nr_bank_pins,
-					   a523_r_irq_bank_muxes,
-					   &a523_r_pinctrl_data,
+					   a523_r_irq_bank_muxes, desc,
 					   SUNXI_PINCTRL_NCAT2_REG_LAYOUT);
 }
 
 static const struct of_device_id a523_r_pinctrl_match[] = {
-	{ .compatible = "allwinner,sun55i-a523-r-pinctrl", },
-	{ .compatible = "allwinner,sun60i-a733-r-pinctrl", },
+	{ .compatible = "allwinner,sun55i-a523-r-pinctrl",
+	  .data = &a523_r_pinctrl_data },
+	{ .compatible = "allwinner,sun60i-a733-r-pinctrl",
+	  .data = &a733_r_pinctrl_data },
 	{}
 };
 
drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c +2 -1
index ca82f08a3a65..d4b0a40525f9 100644
@@ -24,7 +24,8 @@ 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,
+	/* Preserve the firmware-configured two-bit voltage modes. */
+	.io_bias_cfg_variant = BIAS_VOLTAGE_NONE,
 };
 
 static int a733_pinctrl_probe(struct platform_device *pdev)

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

#2 lore raw

Hi Andre,

Quoted 1 line

I guess this has been written by an agent who like to talk?

Yes. I use an AI coding agent to help draft, build and test these patches; I review the result and send it myself. The v2 description was its draft and I should have cut it down; v3 is much shorter.

Quoted 2 lines

So as I suggested before: please introduce a symbol like BIAS_VOLTAGE_PIO_POW_AUTO or so,

v3 follows your suggestion with BIAS_VOLTAGE_PIO_POW_AUTO, setting all two-bit fields to 01 once at init in the generic sunxi code for both A733 controllers:
https://lore.kernel.org/r/[email protected]

Thanks,
Enzo