Hi Rob,
Thanks for your reply.
On 2024/12/17 22:20, Rob Herring wrote:
[ EXTERNAL EMAIL ]
On Wed, Dec 11, 2024 at 02:47:49PM +0800, Xianwei Zhao wrote:
Add the dt-bindings for Amlogic pin controller, and add a new
dt-binding header file which document the GPIO bank names and
alternative func value of all Amlogic subsequent SoCs.
Signed-off-by: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
---
.../bindings/pinctrl/amlogic,pinctrl.yaml | 150 +++++++++++++++++++++
include/dt-bindings/pinctrl/amlogic,pinctrl.h | 68 ++++++++++
2 files changed, 218 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl.yaml
new file mode 100644
index 000000000000..b0c2ae585d7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl.yaml
@@ -0,0 +1,150 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/amlogic,pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic pinmux controller
+
+maintainers:
+ - Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
+
+allOf:
+ - $ref: pinctrl.yaml#
+
+properties:
+ compatible:
+ const: amlogic,pinctrl
Only one Amlogic pinctrl block? Pretty sure there's more than 1 SoC and
pinctrl is unlikely identical for all. This must be SoC specific.
Will do, "amlogic,pinctrl" -> "amlogic,pinctrl-a4".
+
+ reg:
+ minItems: 2
+
+ reg-names:
+ items:
+ - const: mux
+ - const: gpio
+
+ "#address-cells":
+ const: 2
+
+ "#size-cells":
+ const: 2
Why do you need these? You don't have any child nodes with addresses.
But maybe you should?
I will modify "reg_mux_offset" to "reg", So
#address-cells = <1>
#size-cells = <0>
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - "#address-cells"
+ - "#size-cells"
+
+patternProperties:
+ "^gpio[a-z]":
This allows "gpioanythingyouwant" for example. If there is no
unit-address, then it should be '^gpio-[0-9]+$'.
Will do.
+ type: object
+
+ properties:
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+
+ bank-name:
+ $ref: /schemas/types.yaml#/definitions/string
You must define values, but why do you need this? No other GPIO
controller needs something like this.
We define the bank name(GPIOA,GPIOB) on the chip specification, so it is
more readable to describe the source code with such a name in the
pin(GPIOA-0,GPIOA-1).
Bank indexes are defined primarily for use in pinmux implicit queries.
Bank-name and bank-index, they both describe the characteristics of the
Bank, they just use it differently. I can keep bank-index, I can query
the bank-name through bank-index.
I will delete this property, keep bank-index property.
+
+ npins:
We have a standard property for this: ngpios
Will do.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1
+ maximum: 32
+
+ bank-index:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 29
Consider if 'bank' should be part of the gpio cells rather than
separate controller nodes. That works better if each bank is not its own
separate h/w block. If banks share a register, then that's not separate
blocks.
Each bank is its own separate h/w block(With TEST_N bank exception,it
share mux reg with AO bank).
Change the name of this property to "bank-identity" make it more
appropriate.
+
+ reg-mux-offset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ bit-mux-offset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ reg-gpio-offset:
+ $ref: /schemas/types.yaml#/definitions/uint32
Put these into 'reg'. We even have a way to deal with bit offsets. See
leds/register-bit-led.yaml for example.
Will do.
>> +
+ required:
+ - gpio-controller
+ - "#gpio-cells"
+ - npins
+ - bank-index
+ - reg-mux-offset
+ - reg-gpio-offset
+ - bank-name
+
+ additionalProperties: false
+ unevaluatedProperties: false
Don't need both. 'additionalProperties' is more restrictive, so drop
unevaluatedProperties.
Will drop unevaluatedProperties.
+
+ "^func-[0-9a-z]":
"^func-[0-9a-z-]+$"
Will do.
+ type: object
+ additionalProperties:
+ type: object
+ allOf:
+ - $ref: pincfg-node.yaml#
+ - $ref: pinmux-node.yaml#
+
+additionalProperties: false
+
+examples:
+ - |
+ apb {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ periphs_pinctrl: pinctrl@8e700 {
+ compatible = "amlogic,pinctrl";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ reg = <0x0 0x8e700 0x0 0x04>,
+ <0x0 0x8e704 0x0 0x60>;
+ reg-names = "mux", "gpio";
+
+ gpiob {
+ gpio-controller;
+ #gpio-cells = <2>;
+ npins = <10>;
+ bank-index = <1>;
+ reg-mux-offset = <0x14>;
+ bit-mux-offset = <0x14>;
+ reg-gpio-offset = <0x30>;
+ bank-name = "GPIOB";
+ };
+
+ gpioe {
+ gpio-controller;
+ #gpio-cells = <2>;
+ npins = <10>;
+ bank-index = <5>;
+ reg-mux-offset = <0x14>;
+ reg-gpio-offset = <0x30>;
+ bank-name = "GPIOE";
+ };
+
+ func-uart-b {
+ uart-b-default{
+ pinmux = <3>;
+ bias-pull-up;
+ drive-strength-microamp = <4000>;
+ };
+
+ uart-c-default{
+ pinmux = <4>;
+ bias-pull-up;
+ drive-strength-microamp = <4000>;
+ };
+ };
+
+ func-uart-c {
+ uart-c-default{
+ pinmux = <3>;
+ bias-pull-up;
+ drive-strength-microamp = <4000>;
+ };
+ };
+ };
+ };
diff --git a/include/dt-bindings/pinctrl/amlogic,pinctrl.h b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
new file mode 100644
index 000000000000..03db0a730e8b
--- /dev/null
+++ b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
@@ -0,0 +1,68 @@
+/* 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_PINCTRL_H
+#define _DT_BINDINGS_AMLOGIC_PINCTRL_H
+
+/* define PIN modes */
+#define AF0 0x0
+#define AF1 0x1
+#define AF2 0x2
+#define AF3 0x3
+#define AF4 0x4
+#define AF5 0x5
+#define AF6 0x6
+#define AF7 0x7
+#define AF8 0x8
+#define AF9 0x9
+#define AF10 0xa
+#define AF11 0xb
+#define AF12 0xc
+#define AF13 0xd
+#define AF14 0xe
+#define AF15 0xf
+
+#define AML_PIN_ALT_FUNC_MASK 0xf
+
+/* Normal PIN bank */
+#define AMLOGIC_GPIO_A 0
+#define AMLOGIC_GPIO_B 1
+#define AMLOGIC_GPIO_C 2
+#define AMLOGIC_GPIO_D 3
+#define AMLOGIC_GPIO_E 4
+#define AMLOGIC_GPIO_F 5
+#define AMLOGIC_GPIO_G 6
+#define AMLOGIC_GPIO_H 7
+#define AMLOGIC_GPIO_I 8
+#define AMLOGIC_GPIO_J 9
+#define AMLOGIC_GPIO_K 10
+#define AMLOGIC_GPIO_L 11
+#define AMLOGIC_GPIO_M 12
+#define AMLOGIC_GPIO_N 13
+#define AMLOGIC_GPIO_O 14
+#define AMLOGIC_GPIO_P 15
+#define AMLOGIC_GPIO_Q 16
+#define AMLOGIC_GPIO_R 17
+#define AMLOGIC_GPIO_S 18
+#define AMLOGIC_GPIO_T 19
+#define AMLOGIC_GPIO_U 20
+#define AMLOGIC_GPIO_V 21
+#define AMLOGIC_GPIO_W 22
+#define AMLOGIC_GPIO_X 23
+#define AMLOGIC_GPIO_Y 24
+#define AMLOGIC_GPIO_Z 25
+
+/* Special PIN bank */
+#define AMLOGIC_GPIO_DV 26
+#define AMLOGIC_GPIO_AO 27
+#define AMLOGIC_GPIO_CC 28
+#define AMLOGIC_GPIO_TEST_N 29
+
+#define AML_PINMUX(bank, offset, mode) (((((bank) << 8) + (offset)) << 8) | (mode))
+#define AML_PINMUX_TO_BANK(pinmux) (((pinmux) >> 16) & 0xff)
+#define AML_PINMUX_TO_OFFSET(pinmux) (((pinmux) >> 8) & 0xff)
+
+#endif /* _DT_BINDINGS_AMLOGIC_PINCTRL_H */
--
2.37.1
