Re: [PATCH v1 1/3] dt-bindings: gpio: add Phytium GPIO controller

[Krzysztof Kozlowski]

On Mon, Mar 02, 2026 at 05:51:45PM +0800, Zhu Ling wrote:
> Add the devicetree binding schema for the Phytium platform GPIO
> controller.

What is Phytium platform?

> 
> Register the "phytium" vendor prefix used by the compatible string.
> 
> Use ngpios as the preferred child-node property and keep nr-gpios as
> deprecated for compatibility with existing firmware descriptions.

NAK, you cannot add new deprecated properties. "existing firmware" does
not matter, because this is the first posting, thus the firmware starts
existing from now.

> 
> Signed-off-by: Zhu Ling <1536943441@xxxxxx>
> ---
>  .../bindings/gpio/phytium,gpio.yaml           | 134 ++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  2 files changed, 136 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/phytium,gpio.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/phytium,gpio.yaml b/Documentation/devicetree/bindings/gpio/phytium,gpio.yaml
> new file mode 100644
> index 000000000..1b9200c57
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/phytium,gpio.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/phytium,gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Phytium GPIO controller
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  Phytium GPIO controllers expose one GPIO/interrupt controller and up to
> +  two configurable ports. Child nodes describe per-port configuration.
> +
> +maintainers:
> +  - Chen Baozi <chenbaozi@xxxxxxxxxxxxxx>
> +
> +properties:
> +  $nodename:
> +    pattern: "^gpio@[0-9a-f]+$"

Drop

> +
> +  compatible:
> +    const: phytium,gpio

This is way too generic. Phytium made or will ever make only one controller?

> +
> +  reg:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  '#gpio-cells':
> +    const: 2

Why do you need cells here and in the children?

> +
> +  interrupts:
> +    description: |
> +      The interrupts to the parent controller raised when GPIOs generate
> +      the interrupts. If the controller provides one combined interrupt
> +      for all GPIOs, specify a single interrupt. If the controller provides
> +      one interrupt for each GPIO, provide a list of interrupts that
> +      correspond to each of the GPIO pins.
> +    minItems: 1
> +    maxItems: 32

Why is this flexible? You have one fixed device, no?

> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 2
> +
> +patternProperties:
> +  "^gpio-port@[0-9a-f]+$":
> +    type: object
> +    properties:
> +      compatible:
> +        const: phytium,gpio-port

Drop compatible

> +
> +      reg:
> +        maxItems: 1
> +
> +      gpio-controller: true

So what is the controller exactly?

> +
> +      '#gpio-cells':
> +        const: 2
> +
> +      ngpios:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: The number of GPIO pins exported by the port.
> +        default: 32
> +        minimum: 1
> +        maximum: 32
> +
> +      nr-gpios:

Drop, why do you create duplicated properties for everything?

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: The number of GPIO pins exported by the port.
> +        deprecated: true
> +        default: 32
> +        minimum: 1
> +        maximum: 32
> +
> +    required:
> +      - compatible
> +      - reg
> +      - gpio-controller
> +      - '#gpio-cells'
> +
> +    additionalProperties: false
> +
> +additionalProperties: false

This goes after required: block

> +
> +required:
> +  - compatible
> +  - reg
> +  - gpio-controller
> +  - "#gpio-cells"
> +  - interrupts
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    gpio: gpio@28004000 {

Drop unused label

> +      compatible = "phytium,gpio";
> +      reg = <0x28004000 0x1000>;
> +      gpio-controller;
> +      #gpio-cells = <2>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> +      interrupt-controller;
> +      #interrupt-cells = <2>;
> +
> +      porta: gpio-port@0 {
> +        compatible = "phytium,gpio-port";
> +        reg = <0>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        ngpios = <8>;
> +      };
> +
> +      portb: gpio-port@1 {
> +        compatible = "phytium,gpio-port";
> +        reg = <1>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        ngpios = <8>;
> +      };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index ee7fd3cfe..2c3f9777d 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1265,6 +1265,8 @@ patternProperties:
>      description: PHICOMM Co., Ltd.
>    "^phontech,.*":
>      description: Phontech
> +  "^phytium,.*":

Keep proper sorting.

> +    description: Phytium Technology Co., Ltd.
>    "^phytec,.*":
>      description: PHYTEC Messtechnik GmbH
>    "^picochip,.*":
> -- 
> 2.34.1
>