Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add support for Amlogic SoCs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob,
   Thanks for your reply.

On 2025/1/3 05:24, Rob Herring wrote:
[ EXTERNAL EMAIL ]

On Thu, Dec 26, 2024 at 03:57:41PM +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-a4.yaml       | 155 +++++++++++++++++++++
  include/dt-bindings/pinctrl/amlogic,pinctrl.h      |  68 +++++++++
  2 files changed, 223 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
new file mode 100644
index 000000000000..75863d179933
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
@@ -0,0 +1,155 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/amlogic,pinctrl-a4.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-a4
+
+  reg:
+    minItems: 2
+
+  reg-names:
+    items:
+      - const: mux
+      - const: gpio
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0

The addresses in 'reg' below are MMIO, right? If so, #size-cells can't
be 0 and you also need 'ranges' here.


Will add ranges here, reg and reg-names move into gpio object.

+
+required:

Move after 'patternProperties'.

Will do.
+  - compatible
+  - reg
+  - reg-names
+  - "#address-cells"
+  - "#size-cells"
+
+patternProperties:
+  "^gpio@[0-9a-f]+$":
+    type: object
+
+    properties:
+      reg:
+        minItems: 2

You need to describe what each entry is:

items:
   - description: foo
   - description: bar

Will add description in next version
+
+      mask:
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      gpio-controller: true
+
+      "#gpio-cells":
+        const: 2
+
+      ngpios:
+        $ref: /schemas/types.yaml#/definitions/uint32

Don't need a type here.


Will drop ngpios, use gpio_rangs instead.

+        minimum: 1
+        maximum: 32
+
+      identity:

Needs a better name. This is the bank number?


named 'bank-number'.

But shouldn't you be able to use gpio-ranges here:

gpio-ranges = <&periphs_pinctrl 0 (bank# << 8) (value of npgios)>;

And do you really need ngpios if you have the value here?

+        description: |
+          identifier are provided by the pin controller header file at:
+          <include/dt-bindings/pinctrl/amlogic,pinctrl.h>
+
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+    required:
+      - reg
+      - gpio-controller
+      - "#gpio-cells"
+      - ngpios
+      - identity
+
+    additionalProperties: false
+
+  "^func-[0-9a-z-]+$":
+    type: object
+    additionalProperties:

Define a pattern for the node names instead. We only allow anything for
existing bindings.


Will use patternProperies

+      type: object
+      allOf:
+        - $ref: pincfg-node.yaml#
+        - $ref: pinmux-node.yaml#
+
+      description:
+        A pin multiplexing sub-node describes how to configure a set of (or a
+        single) pin in some desired alternate function mode.
+        A single sub-node may define several pin configurations.
+
+      properties:
+        pinmux:
+          description: |
+            Integer array representing pin number and pin multiplexing
+            configuration.
+            When a pin has to be configured in alternate function mode, use
+            this property to identify the pin by its global index, and provide
+            its alternate function configuration number along with it.
+            bank identifier are provided by the pin controller header file at:
+            <include/dt-bindings/pinctrl/amlogic,pinctrl.h>
+            Integers values in "pinmux" argument list are assembled as:
+            (((BANK << 8) + PIN) << 8)  | MUX_FUNC))
+
+      required:
+        - pinmux
+
+      additionalProperties: true

No, not allowed to be true except on common schemas.


This object  I will only save
     allOf:
       - $ref: pincfg-node.yaml#
       - $ref: pinmux-node.yaml#
others(description, properties,required, additionalProperties) will be dropped.

+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/pinctrl/amlogic,pinctrl.h>
+    apb {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      periphs_pinctrl: pinctrl@8e700 {
+        compatible = "amlogic,pinctrl-a4";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <0x0 0x8e700 0x0 0x04>,
+              <0x0 0x8e704 0x0 0x60>;
+        reg-names = "mux", "gpio";
+
+        gpio@14 {
+          reg = <0x14>,<0x30>;
+          gpio-controller;
+          #gpio-cells = <2>;
+          ngpios = <10>;
+          identity = <AMLOGIC_GPIO_B>;
+        };
+
+        func-uart-b {
+          uart-b-default {
+            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 1, AF4)>;
+            bias-pull-up;
+            drive-strength-microamp = <4000>;
+          };
+
+          uart-b-pins1 {
+            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 5, AF2)>;
+            bias-pull-up;
+            drive-strength-microamp = <4000>;
+          };
+        };
+
+        func-uart-c {
+          uart-c-default {
+            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 3, AF1)>,
+                     <AML_PINMUX(AMLOGIC_GPIO_B, 2, AF1)>;
+            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

There's no need for defines in the form "FOOn n".


Will do.

+
+#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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux