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

From: Xianwei Zhao
Date: Mon Jan 20 2025 - 00:58:08 EST


Hi Krzysztof,
Thanks for reply.

On 2025/1/17 16:48, Krzysztof Kozlowski wrote:
[ EXTERNAL EMAIL ]

On Wed, Jan 15, 2025 at 02:41:59PM +0800, Xianwei Zhao wrote:
+properties:
+ compatible:
+ const: amlogic,pinctrl-a4
+
+ "#address-cells":
+ const: 2
+
+ "#size-cells":
+ const: 2

Why this moved to '2'? 32-bit was not enough?

Previously only represented a register address offset value, now directly represents a register, through 'ranges" which to represent. 32-bit is enough, but since APB bus is defined as 64-bit, here is following. I increase support for 32-bit. Like the following values:
"#address-cells":
enum: [1, 2]

"#size-cells":
enum: [1, 2]

+
+ ranges: true
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"

Keep required after patternProperties


Will do.

+
+patternProperties:
+ "^gpio@[0-9a-f]+$":
+ type: object
+
+ properties:
+ reg:
+ minItems: 1
+ items:
+ - description: pin config register
+ - description: pin mux setting register (some special pin fixed function)
+ - description: pin drive strength register (optionanl)

typo: optional

Will fix.
+
+ reg-names:
+ minItems: 1
+ items:
+ - const: gpio
+ - const: mux
+ - const: ds
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+
+ gpio-ranges:
+ maxItems: 1
+
+ bank-number:
+ description: |
+ bank-number are provided by the pin controller header file at:
+ <include/dt-bindings/pinctrl/amlogic,pinctrl.h>
+ $ref: /schemas/types.yaml#/definitions/uint32

gpio-ranges tell you that, don't they?

Yes. Get it from gpio-ranges.

Anyway, you would need here minimum and maximum.

Will drop bank-number property.
+
+ required:
+ - reg
+ - reg-names
+ - gpio-controller
+ - "#gpio-cells"
+ - gpio-ranges
+ - bank-number
+
+ additionalProperties: false
+
+ "^func-[0-9a-z-]+$":
+ type: object
+ patternProperties:
+ "^group-[0-9a-z-]+$":
+ type: object
+ allOf:
+ - $ref: /schemas/pinctrl/pincfg-node.yaml
+ - $ref: /schemas/pinctrl/pinmux-node.yaml

Missing required pinmux and maybe other properties.


Will add required.

+ additionalProperties: false
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/pinctrl/amlogic,pinctrl.h>
+ apb {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ periphs_pinctrl: pinctrl {
+ compatible = "amlogic,pinctrl-a4";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ gpio@14 {
+ reg = <0 0x14 0 0x10>,
+ <0 0x14 0 0x10>;

One line.


Will do.

+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ bank-number = <AMLOGIC_GPIO_B>;
+ gpio-ranges = <&periphs_pinctrl 0 8 10>;
+ };

Best regards,
Krzysztof