[PATCH 0/7] pinctrl: sunxi: Add Allwinner A523 support

[PATCH 0/7] pinctrl: sunxi: Add Allwinner A523 support

[Andre Przywara]

Hi,

this series introduces pinctrl support for the Allwinner A523 family
of SoCs (comprising A523, A527, T527). [1]
The first three patches extend the sunxi pinctrl core code to deal with
some specialities of the new SoC: it uses every of the 11 possible banks
except the first one, which required some register remapping. The first
patch here is actually some cleanup, which we should take regardless, I
think, since it fixes some hack we introduced with the D1 support.

The main feature is actually patch 4, which introduces a new way to
express the required pinmux values for each function/pin pair. 
Traditionally, we dumped a rather large table of data into the (single
image!) kernel for that, but this approach now puts that value into
the DT, and builds the table at runtime. This patch was posted twice
before [1][2], the last time LinusW seemed to be fine with the idea,
just complained about the abuse of the generic pinmux property. I changed
that to allwinner,pinmux now. For yet another alternative, see below.

The rest of the patches are the usual suspects: the two files for the
two pinctrl instances of the new SoC (now very small), and the DT
binding.

Based on v6.12-rc1. Please have a look, review and test!

Cheers,
Andre

[1] https://linux-sunxi.org/A523#Family_of_sun55iw3
[2] https://patchwork.ozlabs.org/project/linux-gpio/cover/20171113012523.2328-1-andre.przywara@arm.com/
[3] https://lore.kernel.org/linux-arm-kernel/20221110014255.20711-1-andre.przywara@arm.com/

P.S. LinusW's comment about "pinmux" being something different made me
think about whether we should adopt an even different approach, and
follow the Apple silicon GPIO driver. That conflates the existing "pins"
and "allwinner,pinmux" properties into the standard "pinmux" one, like
this:
	uart0_pb_pins: uart0-pb-pins {
		pinmux = <SUNXI_PIN(PB, 9, 2)>,
			 <SUNXI_PIN(PB, 10, 2)>;
		function = "uart0";
	};
That looks like a neat solution to me, with the huge drawback of
requiring a completely different of_xlate function, which I guess means
a more or less completely separate pinctrl driver.
Let me know if you think that's worthwhile.

Andre Przywara (7):
  pinctrl: sunxi: refactor pinctrl variants into flags
  pinctrl: sunxi: move bank K register offset
  pinctrl: sunxi: support moved power configuration registers
  pinctrl: sunxi: allow reading mux values from DT
  dt-bindings: pinctrl: add compatible for Allwinner A523/T527
  pinctrl: sunxi: Add support for the Allwinner A523
  pinctrl: sunxi: Add support for the secondary A523 GPIO ports

 .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  |  23 +-
 drivers/pinctrl/sunxi/Kconfig                 |  10 +
 drivers/pinctrl/sunxi/Makefile                |   3 +
 drivers/pinctrl/sunxi/pinctrl-sun20i-d1.c     |   6 +-
 drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c     |   8 +-
 drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c |  54 +++
 drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c   |  54 +++
 drivers/pinctrl/sunxi/pinctrl-sun5i.c         |   8 +-
 drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c     |   8 +-
 drivers/pinctrl/sunxi/pinctrl-sun8i-v3s.c     |   7 +-
 drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c      | 357 ++++++++++++++++++
 drivers/pinctrl/sunxi/pinctrl-sunxi.c         |  54 ++-
 drivers/pinctrl/sunxi/pinctrl-sunxi.h         |  45 ++-
 13 files changed, 586 insertions(+), 51 deletions(-)
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c

-- 
2.46.2

[PATCH 1/7] pinctrl: sunxi: refactor pinctrl variants into flags

[Andre Przywara]

For some Allwinner SoCs we have one pinctrl driver caring for multiple
very similar chips, and are tagging certain pins with a variant bitmask.
The Allwinner D1 introduced a slightly extended register layout, and we
were abusing this variant mask to convey this bit of information into
the common code part.
Now there will be more pinctrl device properties to consider (has PortF
voltage switch, for instance), so shoehorning this into the variant
bitmask will not fly anymore.

Refactor the "variant" field into a more generic "flags" field. It turns
out that we don't need the variant bits to be unique across all SoCs,
but only among those SoCs that share one driver (table), of which there
are at most three variants at the moment. So the actual variant field can
be limited to say 8 bits, and the other bits in the flag register can be
re-purposed to hold other information, like this extended register
layout.
As a side effect we can move the variant definition into the per-SoC
pinctrl driver file, which makes it more obvious that this is just a
private definition, only relevant for this particular table.
This also changes the artificial sun20i-d1 "variant" into the actual
flag bit that we are after.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pinctrl/sunxi/pinctrl-sun20i-d1.c |  6 ++----
 drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c |  8 ++++++--
 drivers/pinctrl/sunxi/pinctrl-sun5i.c     |  8 ++++++--
 drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c |  8 +++++---
 drivers/pinctrl/sunxi/pinctrl-sun8i-v3s.c |  7 +++++--
 drivers/pinctrl/sunxi/pinctrl-sunxi.c     | 10 +++++-----
 drivers/pinctrl/sunxi/pinctrl-sunxi.h     | 23 +++++++----------------
 7 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sun20i-d1.c b/drivers/pinctrl/sunxi/pinctrl-sun20i-d1.c
index 8e2aab542fcfe..8efe35b77af4d 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun20i-d1.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun20i-d1.c
@@ -820,15 +820,13 @@ static const struct sunxi_pinctrl_desc d1_pinctrl_data = {
 
 static int d1_pinctrl_probe(struct platform_device *pdev)
 {
-	unsigned long variant = (unsigned long)of_device_get_match_data(&pdev->dev);
-
-	return sunxi_pinctrl_init_with_variant(pdev, &d1_pinctrl_data, variant);
+	return sunxi_pinctrl_init_with_flags(pdev, &d1_pinctrl_data,
+					     SUNXI_PINCTRL_NEW_REG_LAYOUT);
 }
 
 static const struct of_device_id d1_pinctrl_match[] = {
 	{
 		.compatible = "allwinner,sun20i-d1-pinctrl",
-		.data = (void *)PINCTRL_SUN20I_D1
 	},
 	{}
 };
diff --git a/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c b/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c
index fa47fe36ee5bc..b2e82bf927b3c 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c
@@ -17,6 +17,10 @@
 
 #include "pinctrl-sunxi.h"
 
+#define PINCTRL_SUN4I_A10	BIT(0)
+#define PINCTRL_SUN7I_A20	BIT(1)
+#define PINCTRL_SUN8I_R40	BIT(2)
+
 static const struct sunxi_desc_pin sun4i_a10_pins[] = {
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 0),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
@@ -1295,8 +1299,8 @@ static int sun4i_a10_pinctrl_probe(struct platform_device *pdev)
 {
 	unsigned long variant = (unsigned long)of_device_get_match_data(&pdev->dev);
 
-	return sunxi_pinctrl_init_with_variant(pdev, &sun4i_a10_pinctrl_data,
-					       variant);
+	return sunxi_pinctrl_init_with_flags(pdev, &sun4i_a10_pinctrl_data,
+					     variant);
 }
 
 static const struct of_device_id sun4i_a10_pinctrl_match[] = {
diff --git a/drivers/pinctrl/sunxi/pinctrl-sun5i.c b/drivers/pinctrl/sunxi/pinctrl-sun5i.c
index 06ecb121c8274..6eef314c93775 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun5i.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun5i.c
@@ -16,6 +16,10 @@
 
 #include "pinctrl-sunxi.h"
 
+#define PINCTRL_SUN5I_A10S	BIT(0)
+#define PINCTRL_SUN5I_A13	BIT(1)
+#define PINCTRL_SUN5I_GR8	BIT(2)
+
 static const struct sunxi_desc_pin sun5i_pins[] = {
 	SUNXI_PIN_VARIANT(SUNXI_PINCTRL_PIN(A, 0),
 		  PINCTRL_SUN5I_A10S,
@@ -719,8 +723,8 @@ static int sun5i_pinctrl_probe(struct platform_device *pdev)
 {
 	unsigned long variant = (unsigned long)of_device_get_match_data(&pdev->dev);
 
-	return sunxi_pinctrl_init_with_variant(pdev, &sun5i_pinctrl_data,
-					       variant);
+	return sunxi_pinctrl_init_with_flags(pdev, &sun5i_pinctrl_data,
+					     variant);
 }
 
 static const struct of_device_id sun5i_pinctrl_match[] = {
diff --git a/drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c b/drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c
index 82ac064931df3..8d8c92ce41cff 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun6i-a31.c
@@ -17,6 +17,9 @@
 
 #include "pinctrl-sunxi.h"
 
+#define PINCTRL_SUN6I_A31	BIT(0)
+#define PINCTRL_SUN6I_A31S	BIT(1)
+
 static const struct sunxi_desc_pin sun6i_a31_pins[] = {
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 0),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
@@ -972,9 +975,8 @@ static int sun6i_a31_pinctrl_probe(struct platform_device *pdev)
 	unsigned long variant =
 		(unsigned long)of_device_get_match_data(&pdev->dev);
 
-	return sunxi_pinctrl_init_with_variant(pdev,
-					       &sun6i_a31_pinctrl_data,
-					       variant);
+	return sunxi_pinctrl_init_with_flags(pdev, &sun6i_a31_pinctrl_data,
+					     variant);
 }
 
 static const struct of_device_id sun6i_a31_pinctrl_match[] = {
diff --git a/drivers/pinctrl/sunxi/pinctrl-sun8i-v3s.c b/drivers/pinctrl/sunxi/pinctrl-sun8i-v3s.c
index 49c9a0b6a0eb4..696d7dd8d87ba 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun8i-v3s.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun8i-v3s.c
@@ -22,6 +22,9 @@
 
 #include "pinctrl-sunxi.h"
 
+#define PINCTRL_SUN8I_V3	BIT(0)
+#define PINCTRL_SUN8I_V3S	BIT(1)
+
 static const struct sunxi_desc_pin sun8i_v3s_pins[] = {
 	/* Hole */
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 0),
@@ -552,8 +555,8 @@ static int sun8i_v3s_pinctrl_probe(struct platform_device *pdev)
 {
 	unsigned long variant = (unsigned long)of_device_get_match_data(&pdev->dev);
 
-	return sunxi_pinctrl_init_with_variant(pdev, &sun8i_v3s_pinctrl_data,
-					       variant);
+	return sunxi_pinctrl_init_with_flags(pdev, &sun8i_v3s_pinctrl_data,
+					     variant);
 }
 
 static const struct of_device_id sun8i_v3s_pinctrl_match[] = {
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index bde67ee31417f..ae281a3c2ed34 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -1472,9 +1472,9 @@ static int sunxi_pinctrl_setup_debounce(struct sunxi_pinctrl *pctl,
 	return 0;
 }
 
-int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
-				    const struct sunxi_pinctrl_desc *desc,
-				    unsigned long variant)
+int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
+				  const struct sunxi_pinctrl_desc *desc,
+				  unsigned long flags)
 {
 	struct device_node *node = pdev->dev.of_node;
 	struct pinctrl_desc *pctrl_desc;
@@ -1497,8 +1497,8 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
 
 	pctl->dev = &pdev->dev;
 	pctl->desc = desc;
-	pctl->variant = variant;
-	if (pctl->variant >= PINCTRL_SUN20I_D1) {
+	pctl->variant = flags & SUNXI_PINCTRL_VARIANT_MASK;
+	if (flags & SUNXI_PINCTRL_NEW_REG_LAYOUT) {
 		pctl->bank_mem_size = D1_BANK_MEM_SIZE;
 		pctl->pull_regs_offset = D1_PULL_REGS_OFFSET;
 		pctl->dlevel_field_width = D1_DLEVEL_FIELD_WIDTH;
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index a87a2f944d609..8e2eca45b57f8 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -82,18 +82,9 @@
 #define SUN4I_FUNC_INPUT	0
 #define SUN4I_FUNC_IRQ		6
 
-#define PINCTRL_SUN5I_A10S	BIT(1)
-#define PINCTRL_SUN5I_A13	BIT(2)
-#define PINCTRL_SUN5I_GR8	BIT(3)
-#define PINCTRL_SUN6I_A31	BIT(4)
-#define PINCTRL_SUN6I_A31S	BIT(5)
-#define PINCTRL_SUN4I_A10	BIT(6)
-#define PINCTRL_SUN7I_A20	BIT(7)
-#define PINCTRL_SUN8I_R40	BIT(8)
-#define PINCTRL_SUN8I_V3	BIT(9)
-#define PINCTRL_SUN8I_V3S	BIT(10)
-/* Variants below here have an updated register layout. */
-#define PINCTRL_SUN20I_D1	BIT(11)
+#define SUNXI_PINCTRL_VARIANT_MASK	GENMASK(7, 0)
+#define SUNXI_PINCTRL_NEW_REG_LAYOUT	BIT(8)
+#define SUNXI_PINCTRL_PORTF_SWITCH	BIT(9)
 
 #define PIO_POW_MOD_SEL_REG	0x340
 #define PIO_POW_MOD_CTL_REG	0x344
@@ -299,11 +290,11 @@ static inline u32 sunxi_grp_config_reg(u16 pin)
 	return GRP_CFG_REG + bank * 0x4;
 }
 
-int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
-				    const struct sunxi_pinctrl_desc *desc,
-				    unsigned long variant);
+int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
+				  const struct sunxi_pinctrl_desc *desc,
+				  unsigned long flags);
 
 #define sunxi_pinctrl_init(_dev, _desc) \
-	sunxi_pinctrl_init_with_variant(_dev, _desc, 0)
+	sunxi_pinctrl_init_with_flags(_dev, _desc, 0)
 
 #endif /* __PINCTRL_SUNXI_H */
-- 
2.46.2

[PATCH 2/7] pinctrl: sunxi: move bank K register offset

[Andre Przywara]

The Allwinner pincontroller register layout used to allow for at least
11 banks per controller, any more banks would reside at a second
controller instance.
When the per-bank register map size was increased with the D1, it turned
out that the last bank (port K) of those maximum 11 banks actually would
not fit anymore in the 512 bytes reserved for the pincontroller registers.
On new SoCs Allwinner thus moved the last bank beyond the existing
registers, at offset 0x500.

So far SoCs never used more than 9 banks per controller, but the new
Allwinner A523 actually uses all 11 banks. Since that SoC also uses the
extended layout, its PortK needs to be programmed at offset 0x500.

Factor out the bank offset calculation into a new function, and handle
the case for the last bank separately. Since none of the older SoCs ever
used PortK, we can ignore this case, and just always use offset 0x500
for the last bank.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 29 +++++++++++++++++++--------
 drivers/pinctrl/sunxi/pinctrl-sunxi.h |  4 ++++
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index ae281a3c2ed34..83a031ceb29f2 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -58,13 +58,29 @@ static struct irq_chip sunxi_pinctrl_level_irq_chip;
  * The following functions calculate the register and the bit offset to access.
  * They take a pin number which is relative to the start of the current device.
  */
+
+/*
+ * When using the extended register layout, Bank K does not fit into the
+ * space used for the other banks. Instead it lives at offset 0x500.
+ */
+static u32 sunxi_bank_offset(const struct sunxi_pinctrl *pctl, u32 pin)
+{
+	u32 offset = 0;
+
+	if (pin >= PK_BASE) {
+		pin -= PK_BASE;
+		offset = PIO_BANK_K_OFFSET;
+	}
+
+	return offset + (pin / PINS_PER_BANK) * pctl->bank_mem_size;
+}
+
 static void sunxi_mux_reg(const struct sunxi_pinctrl *pctl,
 			  u32 pin, u32 *reg, u32 *shift, u32 *mask)
 {
-	u32 bank   = pin / PINS_PER_BANK;
 	u32 offset = pin % PINS_PER_BANK * MUX_FIELD_WIDTH;
 
-	*reg   = bank * pctl->bank_mem_size + MUX_REGS_OFFSET +
+	*reg   = sunxi_bank_offset(pctl, pin) + MUX_REGS_OFFSET +
 		 offset / BITS_PER_TYPE(u32) * sizeof(u32);
 	*shift = offset % BITS_PER_TYPE(u32);
 	*mask  = (BIT(MUX_FIELD_WIDTH) - 1) << *shift;
@@ -73,10 +89,9 @@ static void sunxi_mux_reg(const struct sunxi_pinctrl *pctl,
 static void sunxi_data_reg(const struct sunxi_pinctrl *pctl,
 			   u32 pin, u32 *reg, u32 *shift, u32 *mask)
 {
-	u32 bank   = pin / PINS_PER_BANK;
 	u32 offset = pin % PINS_PER_BANK * DATA_FIELD_WIDTH;
 
-	*reg   = bank * pctl->bank_mem_size + DATA_REGS_OFFSET +
+	*reg   = sunxi_bank_offset(pctl, pin) + DATA_REGS_OFFSET +
 		 offset / BITS_PER_TYPE(u32) * sizeof(u32);
 	*shift = offset % BITS_PER_TYPE(u32);
 	*mask  = (BIT(DATA_FIELD_WIDTH) - 1) << *shift;
@@ -85,10 +100,9 @@ static void sunxi_data_reg(const struct sunxi_pinctrl *pctl,
 static void sunxi_dlevel_reg(const struct sunxi_pinctrl *pctl,
 			     u32 pin, u32 *reg, u32 *shift, u32 *mask)
 {
-	u32 bank   = pin / PINS_PER_BANK;
 	u32 offset = pin % PINS_PER_BANK * pctl->dlevel_field_width;
 
-	*reg   = bank * pctl->bank_mem_size + DLEVEL_REGS_OFFSET +
+	*reg   = sunxi_bank_offset(pctl, pin) + DLEVEL_REGS_OFFSET +
 		 offset / BITS_PER_TYPE(u32) * sizeof(u32);
 	*shift = offset % BITS_PER_TYPE(u32);
 	*mask  = (BIT(pctl->dlevel_field_width) - 1) << *shift;
@@ -97,10 +111,9 @@ static void sunxi_dlevel_reg(const struct sunxi_pinctrl *pctl,
 static void sunxi_pull_reg(const struct sunxi_pinctrl *pctl,
 			   u32 pin, u32 *reg, u32 *shift, u32 *mask)
 {
-	u32 bank   = pin / PINS_PER_BANK;
 	u32 offset = pin % PINS_PER_BANK * PULL_FIELD_WIDTH;
 
-	*reg   = bank * pctl->bank_mem_size + pctl->pull_regs_offset +
+	*reg   = sunxi_bank_offset(pctl, pin) + pctl->pull_regs_offset +
 		 offset / BITS_PER_TYPE(u32) * sizeof(u32);
 	*shift = offset % BITS_PER_TYPE(u32);
 	*mask  = (BIT(PULL_FIELD_WIDTH) - 1) << *shift;
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index 8e2eca45b57f8..37a64624142b6 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -25,6 +25,8 @@
 #define PG_BASE	192
 #define PH_BASE	224
 #define PI_BASE	256
+#define PJ_BASE	288
+#define PK_BASE	320
 #define PL_BASE	352
 #define PM_BASE	384
 #define PN_BASE	416
@@ -89,6 +91,8 @@
 #define PIO_POW_MOD_SEL_REG	0x340
 #define PIO_POW_MOD_CTL_REG	0x344
 
+#define PIO_BANK_K_OFFSET		0x500
+
 enum sunxi_desc_bias_voltage {
 	BIAS_VOLTAGE_NONE,
 	/*
-- 
2.46.2

[PATCH 3/7] pinctrl: sunxi: support moved power configuration registers

[Andre Przywara]

The Allwinner pincontroller IP features some registers to control the
withstand voltage of each pin group. So far those registers were always
located at the same offset, but the A523 SoC has moved them (probably to
accommodate all eleven pin banks).

Add a flag to note this feature, and use that to program the registers
either at offset 0x340 or 0x380. So far no pincontroller driver uses
this flag, but we need it for the upcoming A523 support.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 15 +++++++++++----
 drivers/pinctrl/sunxi/pinctrl-sunxi.h |  7 +++++--
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 83a031ceb29f2..a1057122272bd 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -736,9 +736,9 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
 		val = uV > 1800000 && uV <= 2500000 ? BIT(bank) : 0;
 
 		raw_spin_lock_irqsave(&pctl->lock, flags);
-		reg = readl(pctl->membase + PIO_POW_MOD_CTL_REG);
+		reg = readl(pctl->membase + pctl->pow_mod_sel_offset);
 		reg &= ~BIT(bank);
-		writel(reg | val, pctl->membase + PIO_POW_MOD_CTL_REG);
+		writel(reg | val, pctl->membase + pctl->pow_mod_sel_offset);
 		raw_spin_unlock_irqrestore(&pctl->lock, flags);
 
 		fallthrough;
@@ -746,9 +746,12 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
 		val = uV <= 1800000 ? 1 : 0;
 
 		raw_spin_lock_irqsave(&pctl->lock, flags);
-		reg = readl(pctl->membase + PIO_POW_MOD_SEL_REG);
+		reg = readl(pctl->membase + pctl->pow_mod_sel_offset +
+			    PIO_POW_MOD_SEL_OFS);
 		reg &= ~(1 << bank);
-		writel(reg | val << bank, pctl->membase + PIO_POW_MOD_SEL_REG);
+		writel(reg | val << bank,
+		       pctl->membase + pctl->pow_mod_sel_offset +
+		       PIO_POW_MOD_SEL_OFS);
 		raw_spin_unlock_irqrestore(&pctl->lock, flags);
 		return 0;
 	default:
@@ -1520,6 +1523,10 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
 		pctl->pull_regs_offset = PULL_REGS_OFFSET;
 		pctl->dlevel_field_width = DLEVEL_FIELD_WIDTH;
 	}
+	if (flags & SUNXI_PINCTRL_ELEVEN_BANKS)
+		pctl->pow_mod_sel_offset = PIO_11B_POW_MOD_SEL_REG;
+	else
+		pctl->pow_mod_sel_offset = PIO_POW_MOD_SEL_REG;
 
 	pctl->irq_array = devm_kcalloc(&pdev->dev,
 				       IRQ_PER_BANK * pctl->desc->irq_banks,
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index 37a64624142b6..5b4b01fca3274 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -87,9 +87,11 @@
 #define SUNXI_PINCTRL_VARIANT_MASK	GENMASK(7, 0)
 #define SUNXI_PINCTRL_NEW_REG_LAYOUT	BIT(8)
 #define SUNXI_PINCTRL_PORTF_SWITCH	BIT(9)
+#define SUNXI_PINCTRL_ELEVEN_BANKS	BIT(10)
 
-#define PIO_POW_MOD_SEL_REG	0x340
-#define PIO_POW_MOD_CTL_REG	0x344
+#define PIO_POW_MOD_SEL_REG		0x340
+#define PIO_11B_POW_MOD_SEL_REG		0x380
+#define PIO_POW_MOD_SEL_OFS		0x004
 
 #define PIO_BANK_K_OFFSET		0x500
 
@@ -173,6 +175,7 @@ struct sunxi_pinctrl {
 	u32				bank_mem_size;
 	u32				pull_regs_offset;
 	u32				dlevel_field_width;
+	u32				pow_mod_sel_offset;
 };
 
 #define SUNXI_PIN(_pin, ...)					\
-- 
2.46.2

[PATCH 4/7] pinctrl: sunxi: allow reading mux values from DT

[Andre Przywara]

So far every Allwinner SoC needs a large table in the kernel code, to
describe the mapping between the pinctrl function names ("uart") and
the actual pincontroller mux value to be written into the registers.
This adds a lot of data into a single image kernel, and also looks
somewhat weird, as the DT can easily store the mux value.

Add some code that allows to avoid that table: the struct that describes
the existing pins will be build at *runtime*, based on very basic
information provided by the respective SoC's pinctrl driver. This
consists of the number of pins per bank, plus information which bank
provides IRQ support, along with the mux value to use for that.
The code will then iterate over all children of the pincontroller DT
node (which describe each pin group), and populate that struct with the
mapping between function names and mux values. The only thing that needs
adding in the DT is a property with that value, per pin group.

When this table is built, it will be handed over to the existing sunxi
pinctrl driver, which cannot tell a difference between a hardcoded
struct and this new one built at runtime. It will take care of
registering the pinctrl device with the pinctrl subsystem.

All a new SoC driver would need to do is to provide two arrays, and then
call the sunxi_pinctrl_dt_table_init() function.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pinctrl/sunxi/Makefile           |   1 +
 drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c | 357 +++++++++++++++++++++++
 drivers/pinctrl/sunxi/pinctrl-sunxi.h    |   9 +
 3 files changed, 367 insertions(+)
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c

diff --git a/drivers/pinctrl/sunxi/Makefile b/drivers/pinctrl/sunxi/Makefile
index 2ff5a55927ad6..f5bad7a529519 100644
--- a/drivers/pinctrl/sunxi/Makefile
+++ b/drivers/pinctrl/sunxi/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 # Core
 obj-y					+= pinctrl-sunxi.o
+obj-y					+= pinctrl-sunxi-dt.o
 
 # SoC Drivers
 obj-$(CONFIG_PINCTRL_SUNIV_F1C100S)	+= pinctrl-suniv-f1c100s.o
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c b/drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c
new file mode 100644
index 0000000000000..939c191f5b616
--- /dev/null
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi-dt.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017-2024 Arm Ltd.
+ *
+ * Generic DT driven Allwinner pinctrl driver routines.
+ * Builds the pin tables from minimal driver information and pin groups
+ * described in the DT. Then hands those tables of to the traditional
+ * sunxi pinctrl driver.
+ */
+
+#include <linux/export.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "pinctrl-sunxi.h"
+
+#define INVALID_MUX	0xff
+
+/*
+ * Return the "index"th element from the "allwinner,pinmux" property. If the
+ * property does not hold enough entries, return the last one instead.
+ * For almost every group the pinmux value is actually the same, so this
+ * allows to just list one value in the property.
+ */
+static u8 sunxi_pinctrl_dt_read_pinmux(const struct device_node *node,
+				       int index)
+{
+	int ret, num_elems;
+	u32 value;
+
+	num_elems = of_property_count_u32_elems(node, "allwinner,pinmux");
+	if (num_elems <= 0)
+		return INVALID_MUX;
+
+	if (index >= num_elems)
+		index = num_elems - 1;
+
+	ret = of_property_read_u32_index(node, "allwinner,pinmux", index,
+					 &value);
+	if (ret)
+		return INVALID_MUX;
+
+	return value;
+}
+
+/*
+ * Allocate a table with a sunxi_desc_pin structure for every pin needed.
+ * Fills in the respective pin names ("PA0") and their pin numbers.
+ * Returns the pins array. We cannot use the member in *desc yet, as this
+ * is marked as const, and we will need to change the array still.
+ */
+static struct sunxi_desc_pin *init_pins_table(struct device *dev,
+					      const u8 *pins_per_bank,
+					      struct sunxi_pinctrl_desc *desc)
+{
+	struct sunxi_desc_pin *pins, *cur_pin;
+	int name_size = 0;
+	int port_base = desc->pin_base / PINS_PER_BANK;
+	char *pin_names, *cur_name;
+	int i, j;
+
+	/*
+	 * Find the total number of pins.
+	 * Also work out how much memory we need to store all the pin names.
+	 */
+	for (i = 0; i < SUNXI_PINCTRL_MAX_BANKS; i++) {
+		desc->npins += pins_per_bank[i];
+		if (pins_per_bank[i] < 10) {
+			/* 4 bytes for "PXy\0" */
+			name_size += pins_per_bank[i] * 4;
+		} else {
+			/* 4 bytes for each "PXy\0" */
+			name_size += 10 * 4;
+
+			/* 5 bytes for each "PXyy\0" */
+			name_size += (pins_per_bank[i] - 10) * 5;
+		}
+	}
+
+	if (desc->npins == 0) {
+		dev_err(dev, "no ports defined\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pins = devm_kzalloc(dev, desc->npins * sizeof(*pins), GFP_KERNEL);
+	if (!pins)
+		return ERR_PTR(-ENOMEM);
+
+	/* Allocate memory to store the name for every pin. */
+	pin_names = devm_kmalloc(dev, name_size, GFP_KERNEL);
+	if (!pin_names)
+		return ERR_PTR(-ENOMEM);
+
+	/* Fill the pins array with the name and the number for each pin. */
+	cur_name = pin_names;
+	cur_pin = pins;
+	for (i = 0; i < SUNXI_PINCTRL_MAX_BANKS; i++) {
+		for (j = 0; j < pins_per_bank[i]; j++, cur_pin++) {
+			int nchars = sprintf(cur_name, "P%c%d",
+					     port_base + 'A' + i, j);
+
+			cur_pin->pin.number = (port_base + i) * PINS_PER_BANK + j;
+			cur_pin->pin.name = cur_name;
+			cur_name += nchars + 1;
+		}
+	}
+
+	return pins;
+}
+
+/*
+ * Work out the number of functions for each pin. This will visit every
+ * child node of the pinctrl DT node to find all advertised functions.
+ * Provide memory to hold the per-function information and assign it to
+ * the pin table.
+ * Fill in the GPIO in/out functions already (that every pin has), also add
+ * an "irq" function at the end, for those pins in IRQ-capable ports.
+ * We do not fill in the extra functions (those describe in DT nodes) yet.
+ * We (ab)use the "variant" member in each pin to keep track of the number of
+ * extra functions needed. At the end this will get reset to 2, so that we
+ * can add extra function later, after the two GPIO functions.
+ */
+static int prepare_function_table(struct device *dev, struct device_node *pnode,
+				  struct sunxi_desc_pin *pins, int npins,
+				  const u8 *irq_bank_muxes)
+{
+	struct device_node *node;
+	struct property *prop;
+	struct sunxi_desc_function *func;
+	int num_funcs, irq_bank, last_bank, i;
+
+	/*
+	 * We need at least three functions per pin:
+	 * - one for GPIO in
+	 * - one for GPIO out
+	 * - one for the sentinel signalling the end of the list
+	 */
+	num_funcs = 3 * npins;
+
+	/*
+	 * Add a function for each pin in a bank supporting interrupts.
+	 * We temporarily (ab)use the variant field to store the number of
+	 * functions per pin. This will be cleaned back to 0 before we hand
+	 * over the whole structure to the generic sunxi pinctrl setup code.
+	 */
+	for (i = 0; i < npins; i++) {
+		struct sunxi_desc_pin *pin = &pins[i];
+		int bank = pin->pin.number / PINS_PER_BANK;
+
+		if (irq_bank_muxes[bank]) {
+			pin->variant++;
+			num_funcs++;
+		}
+	}
+
+	/*
+	 * Go over each pin group (every child of the pinctrl DT node) and
+	 * add the number of special functions each pins has. Also update the
+	 * total number of functions required.
+	 * We might slightly overshoot here in case of double definitions.
+	 */
+	for_each_child_of_node(pnode, node) {
+		const char *name;
+
+		of_property_for_each_string(node, "pins", prop, name) {
+			for (i = 0; i < npins; i++) {
+				if (strcmp(pins[i].pin.name, name))
+					continue;
+
+				pins[i].variant++;
+				num_funcs++;
+				break;
+			}
+		}
+	}
+
+	/*
+	 * Allocate the memory needed for the functions in one table.
+	 * We later use pointers into this table to mark each pin.
+	 */
+	func = devm_kzalloc(dev, num_funcs * sizeof(*func), GFP_KERNEL);
+	if (!func)
+		return -ENOMEM;
+
+	/*
+	 * Assign the function's memory and fill in GPIOs, IRQ and a sentinel.
+	 * The extra functions will be filled in later.
+	 */
+	irq_bank = 0;
+	last_bank = 0;
+	for (i = 0; i < npins; i++) {
+		struct sunxi_desc_pin *pin = &pins[i];
+		int bank = pin->pin.number / PINS_PER_BANK;
+		int lastfunc = pin->variant + 1;
+		int irq_mux = irq_bank_muxes[bank];
+
+		func[0].name = "gpio_in";
+		func[0].muxval = 0;
+		func[1].name = "gpio_out";
+		func[1].muxval = 1;
+
+		if (irq_mux) {
+			if (bank > last_bank)
+				irq_bank++;
+			func[lastfunc].muxval = irq_mux;
+			func[lastfunc].irqbank = irq_bank;
+			func[lastfunc].irqnum = pin->pin.number % PINS_PER_BANK;
+			func[lastfunc].name = "irq";
+		}
+
+		if (bank > last_bank)
+			last_bank = bank;
+
+		pin->functions = func;
+
+		/* Skip over the other needed functions and the sentinel. */
+		func += pin->variant + 3;
+
+		/*
+		 * Reset the value for filling in the remaining functions
+		 * behind the GPIOs later.
+		 */
+		pin->variant = 2;
+	}
+
+	return 0;
+}
+
+/*
+ * Iterate over all pins in a single group and add the function name and its
+ * mux value to the respective pin.
+ * The "variant" member is again used to temporarily track the number of
+ * already added functions.
+ */
+static void fill_pin_function(struct device *dev, struct device_node *node,
+			      struct sunxi_desc_pin *pins, int npins)
+{
+	const char *name, *funcname;
+	struct sunxi_desc_function *func;
+	struct property *prop;
+	int pin, i, index;
+	u8 muxval;
+
+	if (of_property_read_string(node, "function", &funcname)) {
+		dev_warn(dev, "missing \"function\" property\n");
+		return;
+	}
+
+	index = 0;
+	of_property_for_each_string(node, "pins", prop, name) {
+		/* Find the index of this pin in our table. */
+		for (pin = 0; pin < npins; pin++)
+			if (!strcmp(pins[pin].pin.name, name))
+				break;
+		if (pin == npins) {
+			dev_warn(dev, "%s: cannot find pin %s\n",
+				 of_node_full_name(node), name);
+			index++;
+			continue;
+		}
+
+		/* Read the associated mux value. */
+		muxval = sunxi_pinctrl_dt_read_pinmux(node, index);
+		if (muxval == INVALID_MUX) {
+			dev_warn(dev, "%s: invalid mux value for pin %s\n",
+				 of_node_full_name(node), name);
+			index++;
+			continue;
+		}
+
+		/*
+		 * Check for double definitions by comparing the to-be-added
+		 * function with already assigned ones.
+		 * Ignore identical pairs (function name and mux value the
+		 * same), but warn about conflicting assignments.
+		 */
+		for (i = 2; i < pins[pin].variant; i++) {
+			func = &pins[pin].functions[i];
+
+			/* Skip over totally unrelated functions. */
+			if (strcmp(func->name, funcname) &&
+			    func->muxval != muxval)
+				continue;
+
+			/* Ignore (but skip below) any identical functions. */
+			if (!strcmp(func->name, funcname) &&
+			    muxval == func->muxval)
+				break;
+
+			dev_warn(dev,
+				 "pin %s: function %s redefined to mux %d\n",
+				 name, funcname, muxval);
+			break;
+		}
+
+		/* Skip any pins with that function already assigned. */
+		if (i < pins[pin].variant) {
+			index++;
+			continue;
+		}
+
+		/* Assign function and muxval to the next free slot. */
+		func = &pins[pin].functions[pins[pin].variant];
+		func->muxval = muxval;
+		func->name = funcname;
+
+		pins[pin].variant++;
+		index++;
+	}
+}
+
+/*
+ * Initialise the pinctrl table, by building it from driver provided
+ * information: the number of pins per bank, the IRQ capable banks and their
+ * IRQ mux value.
+ * Then iterate over all pinctrl DT node children to enter the function name
+ * and mux values for each mentioned pin.
+ * At the end hand over this structure to the actual sunxi pinctrl driver.
+ */
+int sunxi_pinctrl_dt_table_init(struct platform_device *pdev,
+				const u8 *pins_per_bank,
+				const u8 *irq_bank_muxes,
+				struct sunxi_pinctrl_desc *desc,
+				unsigned long flags)
+{
+	struct device_node *pnode = pdev->dev.of_node, *node;
+	struct sunxi_desc_pin *pins;
+	int ret, i;
+
+	pins = init_pins_table(&pdev->dev, pins_per_bank, desc);
+	if (IS_ERR(pins))
+		return PTR_ERR(pins);
+
+	ret = prepare_function_table(&pdev->dev, pnode, pins, desc->npins,
+				     irq_bank_muxes);
+	if (ret)
+		return ret;
+
+	/*
+	 * Now iterate over all groups and add the respective function name
+	 * and mux values to each pin listed within.
+	 */
+	for_each_child_of_node(pnode, node)
+		fill_pin_function(&pdev->dev, node, pins, desc->npins);
+
+	/* Clear the temporary storage. */
+	for (i = 0; i < desc->npins; i++)
+		pins[i].variant = 0;
+
+	desc->pins = pins;
+
+	return sunxi_pinctrl_init_with_flags(pdev, desc, flags);
+}
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index 5b4b01fca3274..89bed627c9cd2 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -31,6 +31,9 @@
 #define PM_BASE	384
 #define PN_BASE	416
 
+/* maximum number of banks per controller (PA -> PK) */
+#define SUNXI_PINCTRL_MAX_BANKS	11
+
 #define SUNXI_PINCTRL_PIN(bank, pin)		\
 	PINCTRL_PIN(P ## bank ## _BASE + (pin), "P" #bank #pin)
 
@@ -304,4 +307,10 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
 #define sunxi_pinctrl_init(_dev, _desc) \
 	sunxi_pinctrl_init_with_flags(_dev, _desc, 0)
 
+int sunxi_pinctrl_dt_table_init(struct platform_device *pdev,
+				const u8 *pins_per_bank,
+				const u8 *irq_bank_muxes,
+				struct sunxi_pinctrl_desc *desc,
+				unsigned long flags);
+
 #endif /* __PINCTRL_SUNXI_H */
-- 
2.46.2

[PATCH 5/7] dt-bindings: pinctrl: add compatible for Allwinner A523/T527

[Andre Przywara]

The A523 contains a pin controller similar to previous SoCs, although
using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
This introduces a new style of binding, where the pinmux values for each
pin group is stored in the new "allwinner,pinmux" property in the DT
node, instead of requiring every driver to store a mapping between the
function names and the required pinmux.

Add the new name to the list of compatible strings, and required it to
have 10 interrupts described. Also add the new pinmux property.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
index 4502405703145..6fc18e92e1e94 100644
--- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
@@ -56,6 +56,8 @@ properties:
       - allwinner,sun50i-h6-r-pinctrl
       - allwinner,sun50i-h616-pinctrl
       - allwinner,sun50i-h616-r-pinctrl
+      - allwinner,sun55i-a523-pinctrl
+      - allwinner,sun55i-a523-r-pinctrl
       - allwinner,suniv-f1c100s-pinctrl
       - nextthing,gr8-pinctrl
 
@@ -64,7 +66,7 @@ properties:
 
   interrupts:
     minItems: 1
-    maxItems: 8
+    maxItems: 10
     description:
       One interrupt per external interrupt bank supported on the
       controller, sorted by bank number ascending order.
@@ -119,13 +121,17 @@ patternProperties:
         $ref: /schemas/types.yaml#/definitions/uint32
         enum: [10, 20, 30, 40]
 
+      allwinner,pinmux:
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        description: pinmux selector for each pin
+
     required:
       - pins
       - function
 
     additionalProperties: false
 
-  "^vcc-p[a-ilm]-supply$":
+  "^vcc-p[a-klm]-supply$":
     description:
       Power supplies for pin banks.
 
@@ -156,6 +162,17 @@ allOf:
         - interrupts
         - interrupt-controller
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - allwinner,sun55i-a523-pinctrl
+
+    then:
+      properties:
+        interrupts:
+          minItems: 10
+
   - if:
       properties:
         compatible:
@@ -166,6 +183,7 @@ allOf:
       properties:
         interrupts:
           minItems: 8
+          maxItems: 8
 
   - if:
       properties:
@@ -244,6 +262,7 @@ allOf:
             - allwinner,sun8i-v3s-pinctrl
             - allwinner,sun9i-a80-r-pinctrl
             - allwinner,sun50i-h6-r-pinctrl
+            - allwinner,sun55i-a523-r-pinctrl
 
     then:
       properties:
-- 
2.46.2

[PATCH 6/7] pinctrl: sunxi: Add support for the Allwinner A523

[Andre Przywara]

The Allwinner A523 contains pins in 10 out of the 11 possible pin banks;
it just skips port A.
Use the newly introduced DT based pinctrl driver to describe just the
generic pinctrl properties, so advertise the number of pins per bank
and the interrupt capabilities. The actual function/mux assignment is
taken from the devicetree.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pinctrl/sunxi/Kconfig               |  5 ++
 drivers/pinctrl/sunxi/Makefile              |  1 +
 drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c | 54 +++++++++++++++++++++
 3 files changed, 60 insertions(+)
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c

diff --git a/drivers/pinctrl/sunxi/Kconfig b/drivers/pinctrl/sunxi/Kconfig
index a78fdbbdfc0c7..0cbe466683650 100644
--- a/drivers/pinctrl/sunxi/Kconfig
+++ b/drivers/pinctrl/sunxi/Kconfig
@@ -131,4 +131,9 @@ config PINCTRL_SUN50I_H616_R
 	default ARM64 && ARCH_SUNXI
 	select PINCTRL_SUNXI
 
+config PINCTRL_SUN55I_A523
+	bool "Support for the Allwinner A523 PIO"
+	default ARM64 && ARCH_SUNXI
+	select PINCTRL_SUNXI
+
 endif
diff --git a/drivers/pinctrl/sunxi/Makefile b/drivers/pinctrl/sunxi/Makefile
index f5bad7a529519..4e55508ff7f76 100644
--- a/drivers/pinctrl/sunxi/Makefile
+++ b/drivers/pinctrl/sunxi/Makefile
@@ -27,5 +27,6 @@ obj-$(CONFIG_PINCTRL_SUN50I_H6)		+= pinctrl-sun50i-h6.o
 obj-$(CONFIG_PINCTRL_SUN50I_H6_R)	+= pinctrl-sun50i-h6-r.o
 obj-$(CONFIG_PINCTRL_SUN50I_H616)	+= pinctrl-sun50i-h616.o
 obj-$(CONFIG_PINCTRL_SUN50I_H616_R)	+= pinctrl-sun50i-h616-r.o
+obj-$(CONFIG_PINCTRL_SUN55I_A523)	+= pinctrl-sun55i-a523.o
 obj-$(CONFIG_PINCTRL_SUN9I_A80)		+= pinctrl-sun9i-a80.o
 obj-$(CONFIG_PINCTRL_SUN9I_A80_R)	+= pinctrl-sun9i-a80-r.o
diff --git a/drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c b/drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c
new file mode 100644
index 0000000000000..8f6bb3fc315c5
--- /dev/null
+++ b/drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Allwinner A523 SoC pinctrl driver.
+ *
+ * Copyright (C) 2023 Arm Ltd.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "pinctrl-sunxi.h"
+
+static const u8 a523_nr_bank_pins[SUNXI_PINCTRL_MAX_BANKS] =
+/*	  PA  PB  PC  PD  PE  PF  PG  PH  PI  PJ  PK */
+	{  0, 15, 17, 24, 16,  7, 15, 20, 17, 18, 24 };
+
+static const unsigned int a523_irq_bank_map[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+
+static const u8 a523_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};
+
+static struct sunxi_pinctrl_desc a523_pinctrl_data = {
+	.irq_banks = ARRAY_SIZE(a523_irq_bank_map),
+	.irq_bank_map = a523_irq_bank_map,
+	.irq_read_needs_mux = true,
+	.io_bias_cfg_variant = BIAS_VOLTAGE_PIO_POW_MODE_SEL,
+};
+
+static int a523_pinctrl_probe(struct platform_device *pdev)
+{
+	return sunxi_pinctrl_dt_table_init(pdev, a523_nr_bank_pins,
+					   a523_irq_bank_muxes,
+					   &a523_pinctrl_data,
+					   SUNXI_PINCTRL_NEW_REG_LAYOUT |
+					   SUNXI_PINCTRL_ELEVEN_BANKS);
+}
+
+static const struct of_device_id a523_pinctrl_match[] = {
+	{ .compatible = "allwinner,sun55i-a523-pinctrl", },
+	{}
+};
+
+static struct platform_driver a523_pinctrl_driver = {
+	.probe	= a523_pinctrl_probe,
+	.driver	= {
+		.name		= "sun55i-a523-pinctrl",
+		.of_match_table	= a523_pinctrl_match,
+	},
+};
+builtin_platform_driver(a523_pinctrl_driver);
-- 
2.46.2

[PATCH 7/7] pinctrl: sunxi: Add support for the secondary A523 GPIO ports

[Andre Przywara]

As most other Allwinner SoCs before, the A523 chip contains a second
GPIO controller, managing banks PL and PM.
Use the newly introduced DT based pinctrl driver to describe just the
generic pinctrl properties, so advertise the number of pins per bank
and the interrupt capabilities. The actual function/mux assignment is
taken from the devicetree.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pinctrl/sunxi/Kconfig                 |  5 ++
 drivers/pinctrl/sunxi/Makefile                |  1 +
 drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c | 54 +++++++++++++++++++
 3 files changed, 60 insertions(+)
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c

diff --git a/drivers/pinctrl/sunxi/Kconfig b/drivers/pinctrl/sunxi/Kconfig
index 0cbe466683650..dc62eba96348e 100644
--- a/drivers/pinctrl/sunxi/Kconfig
+++ b/drivers/pinctrl/sunxi/Kconfig
@@ -136,4 +136,9 @@ config PINCTRL_SUN55I_A523
 	default ARM64 && ARCH_SUNXI
 	select PINCTRL_SUNXI
 
+config PINCTRL_SUN55I_A523_R
+	bool "Support for the Allwinner A523 R-PIO"
+	default ARM64 && ARCH_SUNXI
+	select PINCTRL_SUNXI
+
 endif
diff --git a/drivers/pinctrl/sunxi/Makefile b/drivers/pinctrl/sunxi/Makefile
index 4e55508ff7f76..951b3f1e4b4f1 100644
--- a/drivers/pinctrl/sunxi/Makefile
+++ b/drivers/pinctrl/sunxi/Makefile
@@ -28,5 +28,6 @@ obj-$(CONFIG_PINCTRL_SUN50I_H6_R)	+= pinctrl-sun50i-h6-r.o
 obj-$(CONFIG_PINCTRL_SUN50I_H616)	+= pinctrl-sun50i-h616.o
 obj-$(CONFIG_PINCTRL_SUN50I_H616_R)	+= pinctrl-sun50i-h616-r.o
 obj-$(CONFIG_PINCTRL_SUN55I_A523)	+= pinctrl-sun55i-a523.o
+obj-$(CONFIG_PINCTRL_SUN55I_A523_R)	+= pinctrl-sun55i-a523-r.o
 obj-$(CONFIG_PINCTRL_SUN9I_A80)		+= pinctrl-sun9i-a80.o
 obj-$(CONFIG_PINCTRL_SUN9I_A80_R)	+= pinctrl-sun9i-a80-r.o
diff --git a/drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c b/drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c
new file mode 100644
index 0000000000000..69cd2b4ebd7d7
--- /dev/null
+++ b/drivers/pinctrl/sunxi/pinctrl-sun55i-a523-r.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Allwinner A523 SoC r-pinctrl driver.
+ *
+ * Copyright (C) 2024 Arm Ltd.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "pinctrl-sunxi.h"
+
+static const u8 a523_r_nr_bank_pins[SUNXI_PINCTRL_MAX_BANKS] =
+/*	  PL  PM */
+	{ 14,  6 };
+
+static const unsigned int a523_r_irq_bank_map[] = { 0, 1 };
+
+static const u8 a523_r_irq_bank_muxes[SUNXI_PINCTRL_MAX_BANKS] =
+/*	  PL  PM */
+	{ 14, 14 };
+
+static struct sunxi_pinctrl_desc a523_r_pinctrl_data = {
+	.irq_banks = ARRAY_SIZE(a523_r_irq_bank_map),
+	.irq_bank_map = a523_r_irq_bank_map,
+	.irq_read_needs_mux = true,
+	.io_bias_cfg_variant = BIAS_VOLTAGE_PIO_POW_MODE_SEL,
+	.pin_base = PL_BASE,
+};
+
+static int a523_r_pinctrl_probe(struct platform_device *pdev)
+{
+	return sunxi_pinctrl_dt_table_init(pdev, a523_r_nr_bank_pins,
+					   a523_r_irq_bank_muxes,
+					   &a523_r_pinctrl_data,
+					   SUNXI_PINCTRL_NEW_REG_LAYOUT);
+}
+
+static const struct of_device_id a523_r_pinctrl_match[] = {
+	{ .compatible = "allwinner,sun55i-a523-r-pinctrl", },
+	{}
+};
+
+static struct platform_driver a523_r_pinctrl_driver = {
+	.probe	= a523_r_pinctrl_probe,
+	.driver	= {
+		.name		= "sun55i-a523-r-pinctrl",
+		.of_match_table	= a523_r_pinctrl_match,
+	},
+};
+builtin_platform_driver(a523_r_pinctrl_driver);
-- 
2.46.2

Re: [PATCH 5/7] dt-bindings: pinctrl: add compatible for Allwinner A523/T527

[Rob Herring]

On Mon, Nov 11, 2024 at 12:57:48AM +0000, Andre Przywara wrote:
> The A523 contains a pin controller similar to previous SoCs, although
> using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
> This introduces a new style of binding, where the pinmux values for each
> pin group is stored in the new "allwinner,pinmux" property in the DT
> node, instead of requiring every driver to store a mapping between the
> function names and the required pinmux.
> 
> Add the new name to the list of compatible strings, and required it to
> have 10 interrupts described. Also add the new pinmux property.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> index 4502405703145..6fc18e92e1e94 100644
> --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> @@ -56,6 +56,8 @@ properties:
>        - allwinner,sun50i-h6-r-pinctrl
>        - allwinner,sun50i-h616-pinctrl
>        - allwinner,sun50i-h616-r-pinctrl
> +      - allwinner,sun55i-a523-pinctrl
> +      - allwinner,sun55i-a523-r-pinctrl
>        - allwinner,suniv-f1c100s-pinctrl
>        - nextthing,gr8-pinctrl
>  
> @@ -64,7 +66,7 @@ properties:
>  
>    interrupts:
>      minItems: 1
> -    maxItems: 8
> +    maxItems: 10
>      description:
>        One interrupt per external interrupt bank supported on the
>        controller, sorted by bank number ascending order.
> @@ -119,13 +121,17 @@ patternProperties:
>          $ref: /schemas/types.yaml#/definitions/uint32
>          enum: [10, 20, 30, 40]
>  
> +      allwinner,pinmux:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: pinmux selector for each pin

Please add some constraints and/or description about what goes in the 
array.

> +
>      required:
>        - pins
>        - function
>  
>      additionalProperties: false
>  
> -  "^vcc-p[a-ilm]-supply$":
> +  "^vcc-p[a-klm]-supply$":
>      description:
>        Power supplies for pin banks.
>  
> @@ -156,6 +162,17 @@ allOf:
>          - interrupts
>          - interrupt-controller
>  
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun55i-a523-pinctrl
> +
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 10
> +
>    - if:
>        properties:
>          compatible:
> @@ -166,6 +183,7 @@ allOf:
>        properties:
>          interrupts:
>            minItems: 8
> +          maxItems: 8
>  
>    - if:
>        properties:
> @@ -244,6 +262,7 @@ allOf:
>              - allwinner,sun8i-v3s-pinctrl
>              - allwinner,sun9i-a80-r-pinctrl
>              - allwinner,sun50i-h6-r-pinctrl
> +            - allwinner,sun55i-a523-r-pinctrl
>  
>      then:
>        properties:
> -- 
> 2.46.2
> 

Re: [PATCH 5/7] dt-bindings: pinctrl: add compatible for Allwinner A523/T527

[Chen-Yu Tsai]

On Mon, Nov 11, 2024 at 8:58 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> The A523 contains a pin controller similar to previous SoCs, although
> using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
> This introduces a new style of binding, where the pinmux values for each
> pin group is stored in the new "allwinner,pinmux" property in the DT
> node, instead of requiring every driver to store a mapping between the
> function names and the required pinmux.
>
> Add the new name to the list of compatible strings, and required it to
> have 10 interrupts described. Also add the new pinmux property.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> index 4502405703145..6fc18e92e1e94 100644
> --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> @@ -56,6 +56,8 @@ properties:
>        - allwinner,sun50i-h6-r-pinctrl
>        - allwinner,sun50i-h616-pinctrl
>        - allwinner,sun50i-h616-r-pinctrl
> +      - allwinner,sun55i-a523-pinctrl
> +      - allwinner,sun55i-a523-r-pinctrl
>        - allwinner,suniv-f1c100s-pinctrl
>        - nextthing,gr8-pinctrl
>
> @@ -64,7 +66,7 @@ properties:
>
>    interrupts:
>      minItems: 1
> -    maxItems: 8
> +    maxItems: 10
>      description:
>        One interrupt per external interrupt bank supported on the
>        controller, sorted by bank number ascending order.
> @@ -119,13 +121,17 @@ patternProperties:
>          $ref: /schemas/types.yaml#/definitions/uint32
>          enum: [10, 20, 30, 40]
>
> +      allwinner,pinmux:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: pinmux selector for each pin
> +

Why not just the standard "pinmux" property, as given in
Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml

>      required:
>        - pins
>        - function

This section should be made to apply only to the existing
compatibles? Maybe we could just split the files and have
a clean slate for sun55i?

ChenYu

>      additionalProperties: false
>
> -  "^vcc-p[a-ilm]-supply$":
> +  "^vcc-p[a-klm]-supply$":
>      description:
>        Power supplies for pin banks.
>
> @@ -156,6 +162,17 @@ allOf:
>          - interrupts
>          - interrupt-controller
>
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun55i-a523-pinctrl
> +
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 10
> +
>    - if:
>        properties:
>          compatible:
> @@ -166,6 +183,7 @@ allOf:
>        properties:
>          interrupts:
>            minItems: 8
> +          maxItems: 8
>
>    - if:
>        properties:
> @@ -244,6 +262,7 @@ allOf:
>              - allwinner,sun8i-v3s-pinctrl
>              - allwinner,sun9i-a80-r-pinctrl
>              - allwinner,sun50i-h6-r-pinctrl
> +            - allwinner,sun55i-a523-r-pinctrl
>
>      then:
>        properties:
> --
> 2.46.2
>

Re: [PATCH 5/7] dt-bindings: pinctrl: add compatible for Allwinner A523/T527

[Andre Przywara]

On Wed, 13 Nov 2024 16:50:19 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

Hi Chen-Yu,

sorry for the late reply, I was away for a week.

> On Mon, Nov 11, 2024 at 8:58 AM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > The A523 contains a pin controller similar to previous SoCs, although
> > using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
> > This introduces a new style of binding, where the pinmux values for each
> > pin group is stored in the new "allwinner,pinmux" property in the DT
> > node, instead of requiring every driver to store a mapping between the
> > function names and the required pinmux.
> >
> > Add the new name to the list of compatible strings, and required it to
> > have 10 interrupts described. Also add the new pinmux property.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
> >  1 file changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > index 4502405703145..6fc18e92e1e94 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > @@ -56,6 +56,8 @@ properties:
> >        - allwinner,sun50i-h6-r-pinctrl
> >        - allwinner,sun50i-h616-pinctrl
> >        - allwinner,sun50i-h616-r-pinctrl
> > +      - allwinner,sun55i-a523-pinctrl
> > +      - allwinner,sun55i-a523-r-pinctrl
> >        - allwinner,suniv-f1c100s-pinctrl
> >        - nextthing,gr8-pinctrl
> >
> > @@ -64,7 +66,7 @@ properties:
> >
> >    interrupts:
> >      minItems: 1
> > -    maxItems: 8
> > +    maxItems: 10
> >      description:
> >        One interrupt per external interrupt bank supported on the
> >        controller, sorted by bank number ascending order.
> > @@ -119,13 +121,17 @@ patternProperties:
> >          $ref: /schemas/types.yaml#/definitions/uint32
> >          enum: [10, 20, 30, 40]
> >
> > +      allwinner,pinmux:
> > +        $ref: /schemas/types.yaml#/definitions/uint32-array
> > +        description: pinmux selector for each pin
> > +  
> 
> Why not just the standard "pinmux" property, as given in
> Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml

I had it like this in my last post two years ago, but learned from
LinusW [1] that the generic pinmux property has a slightly different
meaning, and abusing it for just the pinmux index values would not match
the generic definition.
We *could* use the generic definition, but then this would include what's
in the "pins" property, like I sketched out in the cover letter, as an
alternative to this approach:
	pinmux = <SUNXI_PIN(PB, 9, 2)>, <SUNXI_PIN(PB, 10, 2)>;
Where the SUNXI_PIN macro would combine the pin number and the pinmux into
one 32-bit cell. See the Apple GPIO DT nodes for an example.
This looks indeed nicer, but requires quite some rewrite of the existing
pinctrl driver, AFAICS.

[1] Previous reply from LinusW:
https://lore.kernel.org/linux-sunxi/CACRpkdbMc-Q6wjgsiddu6-tWC1dt2uFk+4LyerMdgFk2KRGK4w@mail.gmail.com/

> 
> >      required:
> >        - pins
> >        - function  
> 
> This section should be made to apply only to the existing
> compatibles? Maybe we could just split the files and have
> a clean slate for sun55i?

Yeah, I couldn't find a good example how to make it *required* for one
compatible and *not allowed* for all the others. But creating a whole new
file is actually a good idea, as this also avoids adding another case to
the already quite indented if-else cascade.

Cheers,
Andre

> ChenYu
> 
> >      additionalProperties: false
> >
> > -  "^vcc-p[a-ilm]-supply$":
> > +  "^vcc-p[a-klm]-supply$":
> >      description:
> >        Power supplies for pin banks.
> >
> > @@ -156,6 +162,17 @@ allOf:
> >          - interrupts
> >          - interrupt-controller
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - allwinner,sun55i-a523-pinctrl
> > +
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          minItems: 10
> > +
> >    - if:
> >        properties:
> >          compatible:
> > @@ -166,6 +183,7 @@ allOf:
> >        properties:
> >          interrupts:
> >            minItems: 8
> > +          maxItems: 8
> >
> >    - if:
> >        properties:
> > @@ -244,6 +262,7 @@ allOf:
> >              - allwinner,sun8i-v3s-pinctrl
> >              - allwinner,sun9i-a80-r-pinctrl
> >              - allwinner,sun50i-h6-r-pinctrl
> > +            - allwinner,sun55i-a523-r-pinctrl
> >
> >      then:
> >        properties:
> > --
> > 2.46.2
> >  

Re: [PATCH 5/7] dt-bindings: pinctrl: add compatible for Allwinner A523/T527

[Chen-Yu Tsai]

On Wed, Nov 20, 2024 at 6:12 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Wed, 13 Nov 2024 16:50:19 +0800
> Chen-Yu Tsai <wens@csie.org> wrote:
>
> Hi Chen-Yu,
>
> sorry for the late reply, I was away for a week.
>
> > On Mon, Nov 11, 2024 at 8:58 AM Andre Przywara <andre.przywara@arm.com> wrote:
> > >
> > > The A523 contains a pin controller similar to previous SoCs, although
> > > using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
> > > This introduces a new style of binding, where the pinmux values for each
> > > pin group is stored in the new "allwinner,pinmux" property in the DT
> > > node, instead of requiring every driver to store a mapping between the
> > > function names and the required pinmux.
> > >
> > > Add the new name to the list of compatible strings, and required it to
> > > have 10 interrupts described. Also add the new pinmux property.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > ---
> > >  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
> > >  1 file changed, 21 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > index 4502405703145..6fc18e92e1e94 100644
> > > --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > @@ -56,6 +56,8 @@ properties:
> > >        - allwinner,sun50i-h6-r-pinctrl
> > >        - allwinner,sun50i-h616-pinctrl
> > >        - allwinner,sun50i-h616-r-pinctrl
> > > +      - allwinner,sun55i-a523-pinctrl
> > > +      - allwinner,sun55i-a523-r-pinctrl
> > >        - allwinner,suniv-f1c100s-pinctrl
> > >        - nextthing,gr8-pinctrl
> > >
> > > @@ -64,7 +66,7 @@ properties:
> > >
> > >    interrupts:
> > >      minItems: 1
> > > -    maxItems: 8
> > > +    maxItems: 10
> > >      description:
> > >        One interrupt per external interrupt bank supported on the
> > >        controller, sorted by bank number ascending order.
> > > @@ -119,13 +121,17 @@ patternProperties:
> > >          $ref: /schemas/types.yaml#/definitions/uint32
> > >          enum: [10, 20, 30, 40]
> > >
> > > +      allwinner,pinmux:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +        description: pinmux selector for each pin
> > > +
> >
> > Why not just the standard "pinmux" property, as given in
> > Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml
>
> I had it like this in my last post two years ago, but learned from
> LinusW [1] that the generic pinmux property has a slightly different
> meaning, and abusing it for just the pinmux index values would not match
> the generic definition.
> We *could* use the generic definition, but then this would include what's
> in the "pins" property, like I sketched out in the cover letter, as an
> alternative to this approach:
>         pinmux = <SUNXI_PIN(PB, 9, 2)>, <SUNXI_PIN(PB, 10, 2)>;
> Where the SUNXI_PIN macro would combine the pin number and the pinmux into
> one 32-bit cell. See the Apple GPIO DT nodes for an example.
> This looks indeed nicer, but requires quite some rewrite of the existing
> pinctrl driver, AFAICS.

Sorry for taking so long to get back to this.

Could we maybe add a generic replacement of the existing "function"
property, which takes a string? Like "function-id" or "function-selector"
that takes u32 (or u8). Then it could be one or the other. Not sure
if the binding maintainers would accept this or not.

I understand that we probably don't want the mux value combined with
the pin number.

ChenYu

> [1] Previous reply from LinusW:
> https://lore.kernel.org/linux-sunxi/CACRpkdbMc-Q6wjgsiddu6-tWC1dt2uFk+4LyerMdgFk2KRGK4w@mail.gmail.com/
>
> >
> > >      required:
> > >        - pins
> > >        - function
> >
> > This section should be made to apply only to the existing
> > compatibles? Maybe we could just split the files and have
> > a clean slate for sun55i?
>
> Yeah, I couldn't find a good example how to make it *required* for one
> compatible and *not allowed* for all the others. But creating a whole new
> file is actually a good idea, as this also avoids adding another case to
> the already quite indented if-else cascade.
>
> Cheers,
> Andre
>
> > ChenYu
> >
> > >      additionalProperties: false
> > >
> > > -  "^vcc-p[a-ilm]-supply$":
> > > +  "^vcc-p[a-klm]-supply$":
> > >      description:
> > >        Power supplies for pin banks.
> > >
> > > @@ -156,6 +162,17 @@ allOf:
> > >          - interrupts
> > >          - interrupt-controller
> > >
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          enum:
> > > +            - allwinner,sun55i-a523-pinctrl
> > > +
> > > +    then:
> > > +      properties:
> > > +        interrupts:
> > > +          minItems: 10
> > > +
> > >    - if:
> > >        properties:
> > >          compatible:
> > > @@ -166,6 +183,7 @@ allOf:
> > >        properties:
> > >          interrupts:
> > >            minItems: 8
> > > +          maxItems: 8
> > >
> > >    - if:
> > >        properties:
> > > @@ -244,6 +262,7 @@ allOf:
> > >              - allwinner,sun8i-v3s-pinctrl
> > >              - allwinner,sun9i-a80-r-pinctrl
> > >              - allwinner,sun50i-h6-r-pinctrl
> > > +            - allwinner,sun55i-a523-r-pinctrl
> > >
> > >      then:
> > >        properties:
> > > --
> > > 2.46.2
> > >
>

Re: [PATCH 5/7] dt-bindings: pinctrl: add compatible for Allwinner A523/T527

[Andre Przywara]

On Tue, 14 Jan 2025 15:01:31 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

Hi Chen-Yu,

before I get to your specific question below: what do you think in
general of the idea of getting rid of that table based approach we use so
far? Is that something worthwhile? I definitely think yes, but wanted to
hear the maintainers' opinion about this. Happy to present some arguments
if need be.

...

> On Wed, Nov 20, 2024 at 6:12 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > On Wed, 13 Nov 2024 16:50:19 +0800
> > Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > Hi Chen-Yu,
> >
> > sorry for the late reply, I was away for a week.
> >  
> > > On Mon, Nov 11, 2024 at 8:58 AM Andre Przywara <andre.przywara@arm.com> wrote:  
> > > >
> > > > The A523 contains a pin controller similar to previous SoCs, although
> > > > using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
> > > > This introduces a new style of binding, where the pinmux values for each
> > > > pin group is stored in the new "allwinner,pinmux" property in the DT
> > > > node, instead of requiring every driver to store a mapping between the
> > > > function names and the required pinmux.
> > > >
> > > > Add the new name to the list of compatible strings, and required it to
> > > > have 10 interrupts described. Also add the new pinmux property.
> > > >
> > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > > ---
> > > >  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
> > > >  1 file changed, 21 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > > index 4502405703145..6fc18e92e1e94 100644
> > > > --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > > +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > > @@ -56,6 +56,8 @@ properties:
> > > >        - allwinner,sun50i-h6-r-pinctrl
> > > >        - allwinner,sun50i-h616-pinctrl
> > > >        - allwinner,sun50i-h616-r-pinctrl
> > > > +      - allwinner,sun55i-a523-pinctrl
> > > > +      - allwinner,sun55i-a523-r-pinctrl
> > > >        - allwinner,suniv-f1c100s-pinctrl
> > > >        - nextthing,gr8-pinctrl
> > > >
> > > > @@ -64,7 +66,7 @@ properties:
> > > >
> > > >    interrupts:
> > > >      minItems: 1
> > > > -    maxItems: 8
> > > > +    maxItems: 10
> > > >      description:
> > > >        One interrupt per external interrupt bank supported on the
> > > >        controller, sorted by bank number ascending order.
> > > > @@ -119,13 +121,17 @@ patternProperties:
> > > >          $ref: /schemas/types.yaml#/definitions/uint32
> > > >          enum: [10, 20, 30, 40]
> > > >
> > > > +      allwinner,pinmux:
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +        description: pinmux selector for each pin
> > > > +  
> > >
> > > Why not just the standard "pinmux" property, as given in
> > > Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml  
> >
> > I had it like this in my last post two years ago, but learned from
> > LinusW [1] that the generic pinmux property has a slightly different
> > meaning, and abusing it for just the pinmux index values would not match
> > the generic definition.
> > We *could* use the generic definition, but then this would include what's
> > in the "pins" property, like I sketched out in the cover letter, as an
> > alternative to this approach:
> >         pinmux = <SUNXI_PIN(PB, 9, 2)>, <SUNXI_PIN(PB, 10, 2)>;
> > Where the SUNXI_PIN macro would combine the pin number and the pinmux into
> > one 32-bit cell. See the Apple GPIO DT nodes for an example.
> > This looks indeed nicer, but requires quite some rewrite of the existing
> > pinctrl driver, AFAICS.  
> 
> Sorry for taking so long to get back to this.
> 
> Could we maybe add a generic replacement of the existing "function"
> property, which takes a string? Like "function-id" or "function-selector"
> that takes u32 (or u8). Then it could be one or the other. Not sure
> if the binding maintainers would accept this or not.

Do you mean specifically a *generic* property, as opposed to something
prefixed with a vendor string, and coded up in just the sunxi driver?

Because otherwise "allwinner,pinmux" is that numeric equivalent to
"function". I kept "function", as a string, because the GPIO framework
still needs a string at places, for instance for sysfs. We could create
those strings, based on the node name, by sprintf'ing something, but I
figured we might as well keep "function".
In my U-Boot patches [1] I actually ignore the new pinmux property, and
still use the function name, as it was easier to integrate into the
existing code. U-Boot uses a very limited subset of our current table,
so each new SoC doesn't add much to the code.

[1] https://github.com/apritzel/u-boot/commit/ab4f7ed0879022357646

Code-wise I think we would still need our own driver code, so whether we
use "function-id" or "allwinner,pinmux" in there doesn't make much of a
difference, I think. 

> I understand that we probably don't want the mux value combined with
> the pin number.

You mean like the Apple GPIO does? I wonder why not, actually? I find this
actually quite clever and compact. Again the "pins" property atm is quite
string-heavy, so the code has to translate this back into a bank and pin
number, then lookup the function string in our table to get the pinmux
value. We could fit all of this information easily into this new
generic "pinmux" property, and the code just needs to mask and shift that
number. Each pin occupies a cell, I don't think we can get much better
than that?

Cheers,
Andre

> ChenYu
> 
> > [1] Previous reply from LinusW:
> > https://lore.kernel.org/linux-sunxi/CACRpkdbMc-Q6wjgsiddu6-tWC1dt2uFk+4LyerMdgFk2KRGK4w@mail.gmail.com/
> >  
> > >  
> > > >      required:
> > > >        - pins
> > > >        - function  
> > >
> > > This section should be made to apply only to the existing
> > > compatibles? Maybe we could just split the files and have
> > > a clean slate for sun55i?  
> >
> > Yeah, I couldn't find a good example how to make it *required* for one
> > compatible and *not allowed* for all the others. But creating a whole new
> > file is actually a good idea, as this also avoids adding another case to
> > the already quite indented if-else cascade.
> >
> > Cheers,
> > Andre
> >  
> > > ChenYu
> > >  
> > > >      additionalProperties: false
> > > >
> > > > -  "^vcc-p[a-ilm]-supply$":
> > > > +  "^vcc-p[a-klm]-supply$":
> > > >      description:
> > > >        Power supplies for pin banks.
> > > >
> > > > @@ -156,6 +162,17 @@ allOf:
> > > >          - interrupts
> > > >          - interrupt-controller
> > > >
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          enum:
> > > > +            - allwinner,sun55i-a523-pinctrl
> > > > +
> > > > +    then:
> > > > +      properties:
> > > > +        interrupts:
> > > > +          minItems: 10
> > > > +
> > > >    - if:
> > > >        properties:
> > > >          compatible:
> > > > @@ -166,6 +183,7 @@ allOf:
> > > >        properties:
> > > >          interrupts:
> > > >            minItems: 8
> > > > +          maxItems: 8
> > > >
> > > >    - if:
> > > >        properties:
> > > > @@ -244,6 +262,7 @@ allOf:
> > > >              - allwinner,sun8i-v3s-pinctrl
> > > >              - allwinner,sun9i-a80-r-pinctrl
> > > >              - allwinner,sun50i-h6-r-pinctrl
> > > > +            - allwinner,sun55i-a523-r-pinctrl
> > > >
> > > >      then:
> > > >        properties:
> > > > --
> > > > 2.46.2
> > > >  
> >  

Re: [PATCH 5/7] dt-bindings: pinctrl: add compatible for Allwinner A523/T527

[Chen-Yu Tsai]

On Tue, Jan 14, 2025 at 7:21 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Tue, 14 Jan 2025 15:01:31 +0800
> Chen-Yu Tsai <wens@csie.org> wrote:
>
> Hi Chen-Yu,
>
> before I get to your specific question below: what do you think in
> general of the idea of getting rid of that table based approach we use so
> far? Is that something worthwhile? I definitely think yes, but wanted to
> hear the maintainers' opinion about this. Happy to present some arguments
> if need be.

Seems OK to me. The other platforms I've worked on don't have the tables.

OOTH, would we have headers alongside the dts files for macros that
give the function IDs a bit more meaning? I mean sure we can always
go back to the datasheets, it's just a bit of a hassle.

> ...
>
> > On Wed, Nov 20, 2024 at 6:12 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > >
> > > On Wed, 13 Nov 2024 16:50:19 +0800
> > > Chen-Yu Tsai <wens@csie.org> wrote:
> > >
> > > Hi Chen-Yu,
> > >
> > > sorry for the late reply, I was away for a week.
> > >
> > > > On Mon, Nov 11, 2024 at 8:58 AM Andre Przywara <andre.przywara@arm.com> wrote:
> > > > >
> > > > > The A523 contains a pin controller similar to previous SoCs, although
> > > > > using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
> > > > > This introduces a new style of binding, where the pinmux values for each
> > > > > pin group is stored in the new "allwinner,pinmux" property in the DT
> > > > > node, instead of requiring every driver to store a mapping between the
> > > > > function names and the required pinmux.
> > > > >
> > > > > Add the new name to the list of compatible strings, and required it to
> > > > > have 10 interrupts described. Also add the new pinmux property.
> > > > >
> > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > > > ---
> > > > >  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
> > > > >  1 file changed, 21 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > > > index 4502405703145..6fc18e92e1e94 100644
> > > > > --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > > > +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > > > @@ -56,6 +56,8 @@ properties:
> > > > >        - allwinner,sun50i-h6-r-pinctrl
> > > > >        - allwinner,sun50i-h616-pinctrl
> > > > >        - allwinner,sun50i-h616-r-pinctrl
> > > > > +      - allwinner,sun55i-a523-pinctrl
> > > > > +      - allwinner,sun55i-a523-r-pinctrl
> > > > >        - allwinner,suniv-f1c100s-pinctrl
> > > > >        - nextthing,gr8-pinctrl
> > > > >
> > > > > @@ -64,7 +66,7 @@ properties:
> > > > >
> > > > >    interrupts:
> > > > >      minItems: 1
> > > > > -    maxItems: 8
> > > > > +    maxItems: 10
> > > > >      description:
> > > > >        One interrupt per external interrupt bank supported on the
> > > > >        controller, sorted by bank number ascending order.
> > > > > @@ -119,13 +121,17 @@ patternProperties:
> > > > >          $ref: /schemas/types.yaml#/definitions/uint32
> > > > >          enum: [10, 20, 30, 40]
> > > > >
> > > > > +      allwinner,pinmux:
> > > > > +        $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > +        description: pinmux selector for each pin
> > > > > +
> > > >
> > > > Why not just the standard "pinmux" property, as given in
> > > > Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml
> > >
> > > I had it like this in my last post two years ago, but learned from
> > > LinusW [1] that the generic pinmux property has a slightly different
> > > meaning, and abusing it for just the pinmux index values would not match
> > > the generic definition.
> > > We *could* use the generic definition, but then this would include what's
> > > in the "pins" property, like I sketched out in the cover letter, as an
> > > alternative to this approach:
> > >         pinmux = <SUNXI_PIN(PB, 9, 2)>, <SUNXI_PIN(PB, 10, 2)>;
> > > Where the SUNXI_PIN macro would combine the pin number and the pinmux into
> > > one 32-bit cell. See the Apple GPIO DT nodes for an example.
> > > This looks indeed nicer, but requires quite some rewrite of the existing
> > > pinctrl driver, AFAICS.
> >
> > Sorry for taking so long to get back to this.
> >
> > Could we maybe add a generic replacement of the existing "function"
> > property, which takes a string? Like "function-id" or "function-selector"
> > that takes u32 (or u8). Then it could be one or the other. Not sure
> > if the binding maintainers would accept this or not.
>
> Do you mean specifically a *generic* property, as opposed to something
> prefixed with a vendor string, and coded up in just the sunxi driver?
>
> Because otherwise "allwinner,pinmux" is that numeric equivalent to
> "function". I kept "function", as a string, because the GPIO framework
> still needs a string at places, for instance for sysfs. We could create
> those strings, based on the node name, by sprintf'ing something, but I
> figured we might as well keep "function".

That seems to be what other drivers are doing.

> In my U-Boot patches [1] I actually ignore the new pinmux property, and
> still use the function name, as it was easier to integrate into the
> existing code. U-Boot uses a very limited subset of our current table,
> so each new SoC doesn't add much to the code.
>
> [1] https://github.com/apritzel/u-boot/commit/ab4f7ed0879022357646
>
> Code-wise I think we would still need our own driver code, so whether we
> use "function-id" or "allwinner,pinmux" in there doesn't make much of a
> difference, I think.

I guess it simply affects whether the bindings and subsequent helper
code can be generalized or not.

> > I understand that we probably don't want the mux value combined with
> > the pin number.
>
> You mean like the Apple GPIO does? I wonder why not, actually? I find this
> actually quite clever and compact. Again the "pins" property atm is quite
> string-heavy, so the code has to translate this back into a bank and pin
> number, then lookup the function string in our table to get the pinmux
> value. We could fit all of this information easily into this new
> generic "pinmux" property, and the code just needs to mask and shift that
> number. Each pin occupies a cell, I don't think we can get much better
> than that?

Mostly because it prevents one from having pin config options without
muxing, for example set drive strength without setting the mux to GPIO,
the latter on sunxi is done exclusively using GPIO bindings.

I talked about this at my talk at ELCE last year, and afterwards was
talking to Heiko and Quentin, and it seems at least Rockchip can't
separate this. Rockchip goes even further with their "rockchip,pins"
property by also including a reference to the pin config node phandle.

I believe MediaTek, which uses "pinmux" as you described, can't either.

That's my main concern.

Also, the "pins" property can be integers. The question then becomes
how do you want to handle the numbering, i.e. what to do with the
holes in between the different banks.


ChenYu


> Cheers,
> Andre
>
> > ChenYu
> >
> > > [1] Previous reply from LinusW:
> > > https://lore.kernel.org/linux-sunxi/CACRpkdbMc-Q6wjgsiddu6-tWC1dt2uFk+4LyerMdgFk2KRGK4w@mail.gmail.com/
> > >
> > > >
> > > > >      required:
> > > > >        - pins
> > > > >        - function
> > > >
> > > > This section should be made to apply only to the existing
> > > > compatibles? Maybe we could just split the files and have
> > > > a clean slate for sun55i?
> > >
> > > Yeah, I couldn't find a good example how to make it *required* for one
> > > compatible and *not allowed* for all the others. But creating a whole new
> > > file is actually a good idea, as this also avoids adding another case to
> > > the already quite indented if-else cascade.
> > >
> > > Cheers,
> > > Andre
> > >
> > > > ChenYu
> > > >
> > > > >      additionalProperties: false
> > > > >
> > > > > -  "^vcc-p[a-ilm]-supply$":
> > > > > +  "^vcc-p[a-klm]-supply$":
> > > > >      description:
> > > > >        Power supplies for pin banks.
> > > > >
> > > > > @@ -156,6 +162,17 @@ allOf:
> > > > >          - interrupts
> > > > >          - interrupt-controller
> > > > >
> > > > > +  - if:
> > > > > +      properties:
> > > > > +        compatible:
> > > > > +          enum:
> > > > > +            - allwinner,sun55i-a523-pinctrl
> > > > > +
> > > > > +    then:
> > > > > +      properties:
> > > > > +        interrupts:
> > > > > +          minItems: 10
> > > > > +
> > > > >    - if:
> > > > >        properties:
> > > > >          compatible:
> > > > > @@ -166,6 +183,7 @@ allOf:
> > > > >        properties:
> > > > >          interrupts:
> > > > >            minItems: 8
> > > > > +          maxItems: 8
> > > > >
> > > > >    - if:
> > > > >        properties:
> > > > > @@ -244,6 +262,7 @@ allOf:
> > > > >              - allwinner,sun8i-v3s-pinctrl
> > > > >              - allwinner,sun9i-a80-r-pinctrl
> > > > >              - allwinner,sun50i-h6-r-pinctrl
> > > > > +            - allwinner,sun55i-a523-r-pinctrl
> > > > >
> > > > >      then:
> > > > >        properties:
> > > > > --
> > > > > 2.46.2
> > > > >
> > >
>
>

Re: [PATCH 1/7] pinctrl: sunxi: refactor pinctrl variants into flags

[Jernej Škrabec]

Dne ponedeljek, 11. november 2024 ob 01:57:44 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> For some Allwinner SoCs we have one pinctrl driver caring for multiple
> very similar chips, and are tagging certain pins with a variant bitmask.
> The Allwinner D1 introduced a slightly extended register layout, and we
> were abusing this variant mask to convey this bit of information into
> the common code part.
> Now there will be more pinctrl device properties to consider (has PortF
> voltage switch, for instance), so shoehorning this into the variant
> bitmask will not fly anymore.
> 
> Refactor the "variant" field into a more generic "flags" field. It turns
> out that we don't need the variant bits to be unique across all SoCs,
> but only among those SoCs that share one driver (table), of which there
> are at most three variants at the moment. So the actual variant field can
> be limited to say 8 bits, and the other bits in the flag register can be
> re-purposed to hold other information, like this extended register
> layout.
> As a side effect we can move the variant definition into the per-SoC
> pinctrl driver file, which makes it more obvious that this is just a
> private definition, only relevant for this particular table.
> This also changes the artificial sun20i-d1 "variant" into the actual
> flag bit that we are after.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

That looks pretty neat cleanup.

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej