diff mbox series

[1/3] dt-bindings: pinctrl: Add support for Amlogic A4 SoCs

Message ID 20240611-a4_pinctrl-v1-1-dc487b1977b3@amlogic.com
State New
Headers show
Series Pinctrl: A4: Add pinctrl driver | expand

Commit Message

Xianwei Zhao via B4 Relay June 11, 2024, 5:10 a.m. UTC 
From: Xianwei Zhao <xianwei.zhao@amlogic.com>

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@amlogic.com>
---
 .../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(+)

Comments

Rob Herring June 13, 2024, 5:08 p.m. UTC | #1 
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@amlogic.com>
> ---
>  .../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@amlogic.com>
> + */
> +
> +#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.

> +
> +#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@amlogic.com>
> + */
> +
> +#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.

Rob
Xianwei Zhao June 14, 2024, 8:51 a.m. UTC | #2 
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@amlogic.com>
>> ---
>>   .../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@amlogic.com>
>> + */
>> +
>> +#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@amlogic.com>
>> + */
>> +
>> +#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
diff mbox series

Patch

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@amlogic.com>
+ */
+
+#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
+
+#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@amlogic.com>
+ */
+
+#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
+
+/* GPIOB */
+#define GPIOB_0				18
+#define GPIOB_1				19
+#define GPIOB_2				20
+#define GPIOB_3				21
+#define GPIOB_4				22
+#define GPIOB_5				23
+#define GPIOB_6				24
+#define GPIOB_7				25
+#define GPIOB_8				26
+#define GPIOB_9				27
+#define GPIOB_10			28
+#define GPIOB_11			29
+#define GPIOB_12			30
+#define GPIOB_13			31
+
+/* GPIOX */
+#define GPIOX_0				32
+#define GPIOX_1				33
+#define GPIOX_2				34
+#define GPIOX_3				35
+#define GPIOX_4				36
+#define GPIOX_5				37
+#define GPIOX_6				38
+#define GPIOX_7				39
+#define GPIOX_8				40
+#define GPIOX_9				41
+#define GPIOX_10			42
+#define GPIOX_11			43
+#define GPIOX_12			44
+#define GPIOX_13			45
+#define GPIOX_14			46
+#define GPIOX_15			47
+#define GPIOX_16			48
+#define GPIOX_17			49
+
+/* GPIOT */
+#define GPIOT_0				50
+#define GPIOT_1				51
+#define GPIOT_2				52
+#define GPIOT_3				53
+#define GPIOT_4				54
+#define GPIOT_5				55
+#define GPIOT_6				56
+#define GPIOT_7				57
+#define GPIOT_8				58
+#define GPIOT_9				59
+#define GPIOT_10			60
+#define GPIOT_11			61
+#define GPIOT_12			62
+#define GPIOT_13			63
+#define GPIOT_14			64
+#define GPIOT_15			65
+#define GPIOT_16			66
+#define GPIOT_17			67
+#define GPIOT_18			68
+#define GPIOT_19			69
+#define GPIOT_20			70
+#define GPIOT_21			71
+#define GPIOT_22			72
+
+#endif