[PATCH 0/2] pinctrl: sunxi: A733: use the two-bit I/O voltage fields

[PATCH 0/2] pinctrl: sunxi: A733: use the two-bit I/O voltage fields

[Vinicius Pedrosa]

Hi,

this follows my question in [1]. Andre asked why not extend the A733 I/O
bias code to two bits instead of switching it off, so patch 1 does that
for the main PIO and patch 2 gives the A733-R its own descriptor, as it
cannot share the A523-R encoding.

I have not seen the board fail with the current code, and no A733 board
DT upstream uses these supplies yet. What I have is the old and the new
helper, copied from the driver, run on the host with register values
read (read-only) from a Cubie A7Z, silicon revision B:

  PIO SEL 0x01155550: a PJ request at 1.8 V gives 0x01155750 with the
  old code (PE goes from automatic to fixed 1.8 V), unchanged with the
  new one.

  R-PIO SEL 0x000aaaaa: a PL request at 3.3 V gives 0x000aaaab with the
  old code (fixed 1.8 V), plus a read-modify-write of +0x344 that the
  manual does not list. The new code leaves both alone.

With both patches the kernel boots on the A7Z (RAM boot of an rc6-based
kernel): UART0 and MMC0 work and the voltage selection registers are
unchanged.

PF and PI are not programmed, and neither is any silicon revision other
than B, as reported by SMCCC. The A523 variants behave as before.

I found nothing to change in the DT binding: the a733-r compatible has
no a523-r fallback and the schema checks pass for both controllers.
Andre, if you meant something else there, please tell me.

[1] https://lore.kernel.org/r/20261008194719.328146-1-vinicius.eduardo.pedrosa@gmail.com

Vinicius Pedrosa (2):
  pinctrl: sunxi: a733: program the two-bit I/O voltage fields
  pinctrl: sunxi: a733-r: use the A733 I/O voltage fields

 drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c | 26 ++++++++-
 drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c   | 15 ++++-
 drivers/pinctrl/sunxi/pinctrl-sunxi.c         | 57 +++++++++++++++++++
 drivers/pinctrl/sunxi/pinctrl-sunxi.h         |  6 ++
 4 files changed, 101 insertions(+), 3 deletions(-)


base-commit: bdacd2542f3c1b2b411fa86ee1f39de19cce6562
-- 
2.43.0

[PATCH 1/2] pinctrl: sunxi: a733: program the two-bit I/O voltage fields

[Vinicius Pedrosa]

The A733 GPIO_POW_MOD_SEL register has a two-bit field per bank, but
the A733 uses the helper written for one bit per bank, so a request
for one bank can change the field of another one. With the register
as firmware leaves it on a Radxa Cubie A7Z (the fields of PB, PC, PD,
PE, PG, PH, PJ and PK in automatic mode, 0x01155550), a PJ request at
1.8 V turns the PE field into fixed 1.8 V (0x01155750).

Add a variant that takes the field of each bank from a mask table. It
keeps automatic mode and a fixed 3.3 V mode that already fits, and
writes only the field of the requested bank. PF is left out: its field
(11:10) is known, but it is tied to the MMC vqmmc voltage switching.
PI is left out as well, since its supply domain is not documented.
Only silicon revision B, as reported through SMCCC, is programmed,
since the encoding is documented for that revision only.

Computed with both helpers on that register value, the old one changes
it and the new one leaves it as it is.

Fixes: 0535bfbfa97b ("pinctrl: sunxi: add support for the Allwinner A733")
Signed-off-by: Vinicius Pedrosa <vinicius.eduardo.pedrosa@gmail.com>
---
 drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c | 15 +++++-
 drivers/pinctrl/sunxi/pinctrl-sunxi.c       | 57 +++++++++++++++++++++
 drivers/pinctrl/sunxi/pinctrl-sunxi.h       |  6 +++
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c b/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
index ca82f08a3a65..6f5757b8adea 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
@@ -21,10 +21,23 @@ static const u8 a733_irq_bank_muxes[SUNXI_PINCTRL_MAX_BANKS] =
 /*	  PA  PB  PC  PD  PE  PF  PG  PH  PI  PJ  PK */
 	{  0, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14};
 
+/* PA is not implemented. PF and PI are left to the firmware, so are not listed. */
+static const u32 a733_io_bias_masks[SUNXI_PINCTRL_MAX_BANKS] = {
+	[1] = GENMASK(25, 24),	/* PB, VCC-IO */
+	[2] = GENMASK(5, 4),	/* PC */
+	[3] = GENMASK(7, 6),	/* PD */
+	[4] = GENMASK(9, 8),	/* PE */
+	[6] = GENMASK(13, 12),	/* PG */
+	[7] = GENMASK(15, 14),	/* PH */
+	[9] = GENMASK(19, 18),	/* PJ */
+	[10] = GENMASK(21, 20),	/* PK */
+};
+
 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,
+	.io_bias_masks = a733_io_bias_masks,
 };
 
 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 3ee5012463da..8e62111d3409 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -10,6 +10,8 @@
  * warranty of any kind, whether express or implied.
  */
 
+#include <linux/arm-smccc.h>
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/export.h>
 #include <linux/gpio/driver.h>
@@ -722,6 +724,58 @@ static const struct pinconf_ops sunxi_pconf_ops = {
 	.pin_config_group_set	= sunxi_pconf_group_set,
 };
 
+/*
+ * The two-bit encoding is documented for A733 silicon revision B, which
+ * firmware reports through the SMCCC SOC_ID call. Do not assume it for any
+ * other revision, or when the firmware does not tell.
+ */
+#define SUNXI_A733_SOC_ID		0x091e1903
+#define SUNXI_A733_REV_B		1
+
+static bool sunxi_pinctrl_a733_rev_b(void)
+{
+#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
+	s32 id = arm_smccc_get_soc_id_version();
+
+	/* Bit 23 is the JEP106 parity bit. */
+	return id >= 0 && (id & ~BIT(23)) == SUNXI_A733_SOC_ID &&
+	       arm_smccc_get_soc_id_revision() == SUNXI_A733_REV_B;
+#else
+	return false;
+#endif
+}
+
+/*
+ * Fields: 0 and 2 fixed 3.3V, 1 automatic, 3 fixed 1.8V. Leave automatic
+ * mode and any other supply voltage to the firmware, and touch only the
+ * field of this bank.
+ */
+static void sunxi_pinctrl_set_io_bias_2bit(struct sunxi_pinctrl *pctl,
+					   unsigned int bank, int uV)
+{
+	u32 mask = pctl->desc->io_bias_masks[bank];
+	unsigned long flags;
+	u32 val, cur, reg;
+
+	if (!mask || !sunxi_pinctrl_a733_rev_b())
+		return;
+
+	if (uV >= 1620000 && uV <= 1980000)
+		val = 3;
+	else if (uV >= 2970000 && uV <= 3630000)
+		val = 2;
+	else
+		return;
+
+	raw_spin_lock_irqsave(&pctl->lock, flags);
+	reg = readl(pctl->membase + pctl->pow_mod_sel_offset);
+	cur = field_get(mask, reg);
+	if (cur != 1 && cur != val && !(val == 2 && cur == 0))
+		writel((reg & ~mask) | field_prep(mask, val),
+		       pctl->membase + pctl->pow_mod_sel_offset);
+	raw_spin_unlock_irqrestore(&pctl->lock, flags);
+}
+
 static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
 					 unsigned pin,
 					 struct regulator *supply)
@@ -795,6 +849,9 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
 		       pctl->membase + pctl->pow_mod_sel_offset);
 		raw_spin_unlock_irqrestore(&pctl->lock, flags);
 		return 0;
+	case BIAS_VOLTAGE_PIO_POW_MODE_SEL_2BIT:
+		sunxi_pinctrl_set_io_bias_2bit(pctl, bank, uV);
+		return 0;
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index 7c2f29e75519..c37170b2e4f4 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -130,6 +130,11 @@ enum sunxi_desc_bias_voltage {
 	 */
 	BIAS_VOLTAGE_PIO_POW_MODE_CTL,
 	BIAS_VOLTAGE_PIO_POW_MODE_CTL_INV,
+	/*
+	 * Bias voltage is set through two-bit fields in PIO_POW_MOD_SEL_REG,
+	 * whose positions are given by io_bias_masks[], as seen on A733.
+	 */
+	BIAS_VOLTAGE_PIO_POW_MODE_SEL_2BIT,
 };
 
 struct sunxi_desc_function {
@@ -155,6 +160,7 @@ struct sunxi_pinctrl_desc {
 	bool				irq_read_needs_mux;
 	bool				disable_strict_mode;
 	enum sunxi_desc_bias_voltage	io_bias_cfg_variant;
+	const u32			*io_bias_masks;
 };
 
 struct sunxi_pinctrl_function {
-- 
2.43.0

Re: [PATCH 1/2] pinctrl: sunxi: a733: program the two-bit I/O voltage fields

[Andre Przywara]

Hi,

On 10/12/26 20:41, Vinicius Pedrosa wrote:
> The A733 GPIO_POW_MOD_SEL register has a two-bit field per bank, but
> the A733 uses the helper written for one bit per bank, so a request
> for one bank can change the field of another one. With the register
> as firmware leaves it on a Radxa Cubie A7Z (the fields of PB, PC, PD,
> PE, PG, PH, PJ and PK in automatic mode, 0x01155550), a PJ request at
> 1.8 V turns the PE field into fixed 1.8 V (0x01155750).
> 
> Add a variant that takes the field of each bank from a mask table. It

Why do we need a table? The bit assignment is a linear
[bank * 2 + 1:bank * 2], no?

> keeps automatic mode and a fixed 3.3 V mode that already fits, and
> writes only the field of the requested bank. PF is left out: its field
> (11:10) is known, but it is tied to the MMC vqmmc voltage switching.

I don't think this redirection is legit. Yes, PortF could be supplied by 
VCC-IO, but surely that port always has to stay at 3.3V, as the 
datasheet requires.
So I would not treat PortF special here.

> PI is left out as well, since its supply domain is not documented.

Mmmh, but isn't that just because it's not exposed on the A733 package? 
Similar to what we saw on the A523: the registers for the banks are 
clearly there, it's just the manual doesn't mention them, because they 
are not used in the particular SoC version. If I read 0x2000480, I see 
at least 8 pins there (all bits set), and 0x2000488 reads 0xf, so there 
are 17 pins on PortI.
And if we don't use PI pins or the PI supply in the DT, we will never 
try to adjust the PI voltage withstand, so there is no problem.

> Only silicon revision B, as reported through SMCCC, is programmed,
> since the encoding is documented for that revision only.

Where does this come from? Where is it documented to be applicable for 
only a single revision?

I will send what I came up with in a minute, feel free to poke holes in 
there (it's just compile tested).

Cheers,
Andre

> Computed with both helpers on that register value, the old one changes
> it and the new one leaves it as it is.
> 
> Fixes: 0535bfbfa97b ("pinctrl: sunxi: add support for the Allwinner A733")
> Signed-off-by: Vinicius Pedrosa <vinicius.eduardo.pedrosa@gmail.com>
> ---
>   drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c | 15 +++++-
>   drivers/pinctrl/sunxi/pinctrl-sunxi.c       | 57 +++++++++++++++++++++
>   drivers/pinctrl/sunxi/pinctrl-sunxi.h       |  6 +++
>   3 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c b/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
> index ca82f08a3a65..6f5757b8adea 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
> @@ -21,10 +21,23 @@ static const u8 a733_irq_bank_muxes[SUNXI_PINCTRL_MAX_BANKS] =
>   /*	  PA  PB  PC  PD  PE  PF  PG  PH  PI  PJ  PK */
>   	{  0, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14};
>   
> +/* PA is not implemented. PF and PI are left to the firmware, so are not listed. */
> +static const u32 a733_io_bias_masks[SUNXI_PINCTRL_MAX_BANKS] = {
> +	[1] = GENMASK(25, 24),	/* PB, VCC-IO */
> +	[2] = GENMASK(5, 4),	/* PC */
> +	[3] = GENMASK(7, 6),	/* PD */
> +	[4] = GENMASK(9, 8),	/* PE */
> +	[6] = GENMASK(13, 12),	/* PG */
> +	[7] = GENMASK(15, 14),	/* PH */
> +	[9] = GENMASK(19, 18),	/* PJ */
> +	[10] = GENMASK(21, 20),	/* PK */
> +};
> +
>   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,
> +	.io_bias_masks = a733_io_bias_masks,
>   };
>   
>   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 3ee5012463da..8e62111d3409 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -10,6 +10,8 @@
>    * warranty of any kind, whether express or implied.
>    */
>   
> +#include <linux/arm-smccc.h>
> +#include <linux/bitfield.h>
>   #include <linux/clk.h>
>   #include <linux/export.h>
>   #include <linux/gpio/driver.h>
> @@ -722,6 +724,58 @@ static const struct pinconf_ops sunxi_pconf_ops = {
>   	.pin_config_group_set	= sunxi_pconf_group_set,
>   };
>   
> +/*
> + * The two-bit encoding is documented for A733 silicon revision B, which
> + * firmware reports through the SMCCC SOC_ID call. Do not assume it for any
> + * other revision, or when the firmware does not tell.
> + */
> +#define SUNXI_A733_SOC_ID		0x091e1903
> +#define SUNXI_A733_REV_B		1
> +
> +static bool sunxi_pinctrl_a733_rev_b(void)
> +{
> +#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
> +	s32 id = arm_smccc_get_soc_id_version();
> +
> +	/* Bit 23 is the JEP106 parity bit. */
> +	return id >= 0 && (id & ~BIT(23)) == SUNXI_A733_SOC_ID &&
> +	       arm_smccc_get_soc_id_revision() == SUNXI_A733_REV_B;
> +#else
> +	return false;
> +#endif
> +}
> +
> +/*
> + * Fields: 0 and 2 fixed 3.3V, 1 automatic, 3 fixed 1.8V. Leave automatic
> + * mode and any other supply voltage to the firmware, and touch only the
> + * field of this bank.
> + */
> +static void sunxi_pinctrl_set_io_bias_2bit(struct sunxi_pinctrl *pctl,
> +					   unsigned int bank, int uV)
> +{
> +	u32 mask = pctl->desc->io_bias_masks[bank];
> +	unsigned long flags;
> +	u32 val, cur, reg;
> +
> +	if (!mask || !sunxi_pinctrl_a733_rev_b())
> +		return;
> +
> +	if (uV >= 1620000 && uV <= 1980000)
> +		val = 3;
> +	else if (uV >= 2970000 && uV <= 3630000)
> +		val = 2;
> +	else
> +		return;
> +
> +	raw_spin_lock_irqsave(&pctl->lock, flags);
> +	reg = readl(pctl->membase + pctl->pow_mod_sel_offset);
> +	cur = field_get(mask, reg);
> +	if (cur != 1 && cur != val && !(val == 2 && cur == 0))
> +		writel((reg & ~mask) | field_prep(mask, val),
> +		       pctl->membase + pctl->pow_mod_sel_offset);
> +	raw_spin_unlock_irqrestore(&pctl->lock, flags);
> +}
> +
>   static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
>   					 unsigned pin,
>   					 struct regulator *supply)
> @@ -795,6 +849,9 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
>   		       pctl->membase + pctl->pow_mod_sel_offset);
>   		raw_spin_unlock_irqrestore(&pctl->lock, flags);
>   		return 0;
> +	case BIAS_VOLTAGE_PIO_POW_MODE_SEL_2BIT:
> +		sunxi_pinctrl_set_io_bias_2bit(pctl, bank, uV);
> +		return 0;
>   	default:
>   		return -EINVAL;
>   	}
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> index 7c2f29e75519..c37170b2e4f4 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> @@ -130,6 +130,11 @@ enum sunxi_desc_bias_voltage {
>   	 */
>   	BIAS_VOLTAGE_PIO_POW_MODE_CTL,
>   	BIAS_VOLTAGE_PIO_POW_MODE_CTL_INV,
> +	/*
> +	 * Bias voltage is set through two-bit fields in PIO_POW_MOD_SEL_REG,
> +	 * whose positions are given by io_bias_masks[], as seen on A733.
> +	 */
> +	BIAS_VOLTAGE_PIO_POW_MODE_SEL_2BIT,
>   };
>   
>   struct sunxi_desc_function {
> @@ -155,6 +160,7 @@ struct sunxi_pinctrl_desc {
>   	bool				irq_read_needs_mux;
>   	bool				disable_strict_mode;
>   	enum sunxi_desc_bias_voltage	io_bias_cfg_variant;
> +	const u32			*io_bias_masks;
>   };
>   
>   struct sunxi_pinctrl_function {

[PATCH 2/2] pinctrl: sunxi: a733-r: use the A733 I/O voltage fields

[Vinicius Pedrosa]

The A733 R-PIO shares the A523-R descriptor, and with it the A523
inverted one-bit encoding and the access to PIO_POW_MOD_CTL. Its
SGPIO_POW_MOD_SEL has two-bit fields instead, and the manual documents
no control register at offset 0x344. With PL in fixed 3.3 V mode, as
read on a Radxa Cubie A7Z (0x000aaaaa), running the old helper on the
host with a 3.3 V request for PL gives 0x000aaaab, which is fixed
1.8 V. This was not run on the board.

Give the A733-R its own descriptor with the PL and PM fields, and
leave the A523-R as it is.

Fixes: 31d68e10381c ("pinctrl: sunxi: a523-r: add a733-r compatible string")
Signed-off-by: Vinicius Pedrosa <vinicius.eduardo.pedrosa@gmail.com>
---
 drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c | 26 +++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c b/drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c
index cffc1e53eef1..1aa7f32035e1 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c
@@ -30,11 +30,33 @@ static struct sunxi_pinctrl_desc a523_r_pinctrl_data = {
 	.pin_base = PL_BASE,
 };
 
+/*
+ * The A733 R-PIO has the A523 one's layout, but its SGPIO_POW_MOD_SEL uses
+ * two-bit fields, and the manual documents no control register at offset 0x344.
+ */
+static const u32 a733_r_io_bias_masks[SUNXI_PINCTRL_MAX_BANKS] = {
+	[0] = GENMASK(1, 0),	/* PL */
+	[1] = GENMASK(3, 2),	/* PM */
+};
+
+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,
+	.io_bias_cfg_variant = BIAS_VOLTAGE_PIO_POW_MODE_SEL_2BIT,
+	.io_bias_masks = a733_r_io_bias_masks,
+	.pin_base = PL_BASE,
+};
+
 static int a523_r_pinctrl_probe(struct platform_device *pdev)
 {
+	struct sunxi_pinctrl_desc *desc = &a523_r_pinctrl_data;
+
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "allwinner,sun60i-a733-r-pinctrl"))
+		desc = &a733_r_pinctrl_data;
+
 	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);
 }
 
-- 
2.43.0