Re: [PATCH v6 4/5] pinctrl: meson: Add driver support for Amlogic A4 SoCs
From: Neil Armstrong
Date: Thu Nov 14 2024 - 04:25:06 EST
On 13/11/2024 19:04, Rob Herring wrote:
On Wed, Nov 13, 2024 at 03:29:42PM +0800, Xianwei Zhao wrote:
Add a new pinctrl driver for Amlogic A4 SoCs which share
the same register layout as the previous Amlogic S4.
Signed-off-by: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
---
drivers/pinctrl/meson/Kconfig | 6 +
drivers/pinctrl/meson/Makefile | 1 +
drivers/pinctrl/meson/pinctrl-amlogic-a4.c | 1324 ++++++++++++++++++++++++++++
3 files changed, 1331 insertions(+)
diff --git a/drivers/pinctrl/meson/Kconfig b/drivers/pinctrl/meson/Kconfig
index cc397896762c..3e90bb5ec442 100644
--- a/drivers/pinctrl/meson/Kconfig
+++ b/drivers/pinctrl/meson/Kconfig
@@ -67,6 +67,12 @@ config PINCTRL_MESON_S4
select PINCTRL_MESON_AXG_PMX
default y
+config PINCTRL_AMLOGIC_A4
+ tristate "Amlogic A4 SoC pinctrl driver"
+ depends on ARM64
+ select PINCTRL_MESON_AXG_PMX
+ default y
+
config PINCTRL_AMLOGIC_C3
tristate "Amlogic C3 SoC pinctrl driver"
depends on ARM64
diff --git a/drivers/pinctrl/meson/Makefile b/drivers/pinctrl/meson/Makefile
index 9e538b9ffb9b..c92a65a83344 100644
--- a/drivers/pinctrl/meson/Makefile
+++ b/drivers/pinctrl/meson/Makefile
@@ -10,5 +10,6 @@ obj-$(CONFIG_PINCTRL_MESON_AXG) += pinctrl-meson-axg.o
obj-$(CONFIG_PINCTRL_MESON_G12A) += pinctrl-meson-g12a.o
obj-$(CONFIG_PINCTRL_MESON_A1) += pinctrl-meson-a1.o
obj-$(CONFIG_PINCTRL_MESON_S4) += pinctrl-meson-s4.o
+obj-$(CONFIG_PINCTRL_AMLOGIC_A4) += pinctrl-amlogic-a4.o
obj-$(CONFIG_PINCTRL_AMLOGIC_C3) += pinctrl-amlogic-c3.o
obj-$(CONFIG_PINCTRL_AMLOGIC_T7) += pinctrl-amlogic-t7.o
diff --git a/drivers/pinctrl/meson/pinctrl-amlogic-a4.c b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
new file mode 100644
index 000000000000..edc5f2ba2c8a
--- /dev/null
+++ b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
@@ -0,0 +1,1324 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
+/*
+ * Pin controller and GPIO driver for Amlogic A4 SoC.
+ *
+ * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
+ * Author: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
+ * Huqiang Qin <huqiang.qin@xxxxxxxxxxx>
+ */
+
+#include "pinctrl-meson.h"
+#include "pinctrl-meson-axg-pmx.h"
+#include <dt-bindings/gpio/amlogic-gpio.h>
+
+/* Standard port */
+
+#define GPIOE_0 0
+#define GPIOE_1 1
+
+#define GPIOD_0 2
+#define GPIOD_1 3
+#define GPIOD_2 4
+#define GPIOD_3 5
+#define GPIOD_4 6
+#define GPIOD_5 7
+#define GPIOD_6 8
+#define GPIOD_7 9
+#define GPIOD_8 10
+#define GPIOD_9 11
+#define GPIOD_10 12
+#define GPIOD_11 13
+#define GPIOD_12 14
+#define GPIOD_13 15
+#define GPIOD_14 16
+#define GPIOD_15 17
The conversion from bank+index to a single index space seems less than
ideal, and looks like a work-around to fit into the existing driver from
a brief look at it.
Not really, it simply adds a custom xlate per SoC, nothing particulary hacky.
I was relunctant at first, but since Xianwei added the plumbing for a per-SoC
xlate, then it was easy to add 3-cells support.
If there's not really banks of GPIOs here, then DT shouldn't have them
either. The question is does anything need to know the bank number
and/or index? If it's only for human readability (and matching to
datasheet), then just something like this can be done:
#define GPIOD(n) (2 + (n))
There's no linear mapping possible, each set of gpios is grouped into logical
"banks" per group of functions, and this grouping is also in the gpio controller
register space.
So it makes sense to split this in 2 with banks and offset, it maps the architecture
of the SoC, and with this scheme we only need to add the bindings for the
"banks" once since all the SoCs shares the same names.
It simplifies bindings, has minimal cost due to the 3 cells, and only requires slightly
more larger xlate function per SoC family.
Neil
Rob