Hi Rob,
Thanks for your review.
On 2024/6/14 01:08, Rob Herring wrote:
[ EXTERNAL EMAIL ]
On Tue, Jun 11, 2024 at 01:10:57PM +0800, Xianwei Zhao wrote:
Add the new compatible name for Amlogic A4 pin controller, and add
a new dt-binding header file which document the detail pin names.
Signed-off-by: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
---
.../bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml | 2 +
.../dt-bindings/gpio/amlogic,a4-aobus-pinctrl.h | 21 +++++
.../dt-bindings/gpio/amlogic,a4-periphs-pinctrl.h | 93 ++++++++++++++++++++++
3 files changed, 116 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml
index d9e0b2c48e84..f5eefa0fab5b 100644
--- a/Documentation/devicetree/bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml
@@ -15,6 +15,8 @@ allOf:
properties:
compatible:
enum:
+ - amlogic,a4-aobus-pinctrl
+ - amlogic,a4-periphs-pinctrl
- amlogic,c3-periphs-pinctrl
- amlogic,t7-periphs-pinctrl
- amlogic,meson-a1-periphs-pinctrl
diff --git a/include/dt-bindings/gpio/amlogic,a4-aobus-pinctrl.h b/include/dt-bindings/gpio/amlogic,a4-aobus-pinctrl.h
new file mode 100644
index 000000000000..7c7e746baed5
--- /dev/null
+++ b/include/dt-bindings/gpio/amlogic,a4-aobus-pinctrl.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
+/*
+ * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
+ * Author: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
+ */
+
+#ifndef _DT_BINDINGS_AMLOGIC_A4_AOBUS_PINCTRL_H
+#define _DT_BINDINGS_AMLOGIC_A4_AOBUS_PINCTRL_H
+
+/* GPIOAO */
+#define GPIOAO_0 0
+#define GPIOAO_1 1
+#define GPIOAO_2 2
+#define GPIOAO_3 3
+#define GPIOAO_4 4
+#define GPIOAO_5 5
+#define GPIOAO_6 6
I find defines with the value of the define in the name pretty
pointless.
In the driver, this macro definition not only uses its value, but also
uses this character, for example as following,
MESON_PIN(GPIOE_0),
#define MESON_PIN(x) PINCTRL_PIN(x, #x)
GPIO_GROUP(GPIOE_0),
#define GPIO_GROUP(gpio) \
{ \
.name = #gpio, \
.pins = (const unsigned int[]){ gpio }, \
.num_pins = 1, \
.data = (const struct meson_pmx_axg_data[]){ \
PMX_DATA(0), \
}, \
}
+
+#define GPIO_TEST_N 7
+
+#endif
diff --git a/include/dt-bindings/gpio/amlogic,a4-periphs-pinctrl.h b/include/dt-bindings/gpio/amlogic,a4-periphs-pinctrl.h
new file mode 100644
index 000000000000..dfabca4b4790
--- /dev/null
+++ b/include/dt-bindings/gpio/amlogic,a4-periphs-pinctrl.h
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
+/*
+ * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
+ * Author: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
+ */
+
+#ifndef _DT_BINDINGS_AMLOGIC_A4_PERIPHS_PINCTRL_H
+#define _DT_BINDINGS_AMLOGIC_A4_PERIPHS_PINCTRL_H
+
+/* GPIOE */
+#define GPIOE_0 0
+#define GPIOE_1 1
+
+/* GPIOD */
+#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
I'm not really much of a fan of using defines for GPIOs, but if you do,
wouldn't be better to split banks and lines up rather than a global
number space. See ASPEED_GPIO() or tegra header.
For the same reasons described above.
I want to keep the same style as the previous drive.
Rob
