Re: [PATCH v2 04/10] dt-bindings: pinctrl: add Ambarella CV75 pinctrl

[Krzysztof Kozlowski <krzk@kernel.org>]

On Thu, Aug 06, 2026 at 05:34:12PM +0800, Long Zhao wrote:
> Add the CV75 pin controller binding.
> 
> Signed-off-by: Long Zhao <longzhao@ambarella.com>
> ---
>  .../bindings/pinctrl/ambarella,cv75-pinctrl.yaml   | 175 +++++++++++++++++++++
>  1 file changed, 175 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/ambarella,cv75-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ambarella,cv75-pinctrl.yaml
> new file mode 100644
> index 000000000000..3fa30b275e51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/ambarella,cv75-pinctrl.yaml
> @@ -0,0 +1,175 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/ambarella,cv75-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ambarella CV75 Pin Controller
> +
> +maintainers:
> +  - Long Zhao <longzhao@ambarella.com>
> +
> +description:
> +  The Ambarella CV75 pin controller configures pin multiplexing, bias and
> +  drive strength for the main GPIO banks. Pin groups and their mux settings
> +  are defined by the pin controller driver and selected by function and group
> +  names.
> +
> +allOf:
> +  - $ref: pinctrl.yaml#
> +
> +properties:
> +  compatible:
> +    const: ambarella,cv75-pinctrl
> +
> +  reg:
> +    maxItems: 1
> +    description: IOMUX registers
> +
> +  ambarella,drive-strength-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: RCT syscon containing the drive-strength registers.
> +
> +  ambarella,pull-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Secure scratchpad syscon containing the pull registers.
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  ranges: true
> +
> +patternProperties:
> +  "^gpio@[0-9a-f]+$":
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +
> +      interrupts:
> +        maxItems: 1
> +
> +      gpio-controller: true
> +
> +      "#gpio-cells":
> +        const: 2
> +
> +      gpio-ranges:
> +        maxItems: 1
> +
> +      interrupt-controller: true
> +
> +      "#interrupt-cells":
> +        const: 2
> +
> +    required:
> +      - reg
> +      - interrupts
> +      - gpio-controller
> +      - "#gpio-cells"
> +      - gpio-ranges
> +      - interrupt-controller
> +      - "#interrupt-cells"
> +
> +  "^[a-z0-9-]+-pins$":

Mixing MMIO and non-MMIO nodes in a bus is discouraged. Sloppy code.

Why the pins aren't per gpio-controller?

> +    type: object
> +    allOf:
> +      - $ref: pinmux-node.yaml#
> +      - $ref: pincfg-node.yaml#
> +    additionalProperties: false
> +
> +    properties:
> +      function:
> +        const: uart0

If the function is fixed, then you do not need any of this.

You must post COMPLETE bindings, see writing bindings doc.

> +
> +      groups:
> +        $ref: /schemas/types.yaml#/definitions/string-array
> +        items:
> +          enum: [uart0tx, uart0rx]

Same here, pointless/redunadnt.

> +        minItems: 2
> +        maxItems: 2
> +        uniqueItems: true
> +
> +      bias-disable: true
> +      bias-pull-down: true
> +      bias-pull-up: true
> +
> +      drive-strength:
> +        enum: [3, 4, 5, 6, 7, 8, 9, 12]
> +
> +    required:
> +      - function
> +      - groups
> +
> +required:
> +  - compatible
> +  - reg
> +  - ambarella,drive-strength-syscon
> +  - ambarella,pull-syscon
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    rct: rct {
> +    };
> +
> +    scratchpad: scratchpad {
> +    };

Drop above, irrelevant. Do you see any other bindings doing this?

> +
> +    pinctrl: pinctrl@e4010000 {
> +        compatible = "ambarella,cv75-pinctrl";
> +        #address-cells = <1>;
> +        #size-cells = <1>;

You need to follow DTS coding style.

> +        reg = <0xe4010000 0x1000>;
> +        ranges;
> +        ambarella,drive-strength-syscon = <&rct>;
> +        ambarella,pull-syscon = <&scratchpad>;

Best regards,
Krzysztof