Re: [PATCH v13 01/13] dt-bindings: gpio: pl061: add Ambarella CV75 variant

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: gpio: pl061: split incompatible ambarella,cv75-gpio schema
- [Low] dt-bindings: gpio: pl061: undocumented ambarella vendor prefix
--

commit 698f86de70b79c39ee77a80fb98df0823e4d3989
Author: Long Zhao <longzhao@ambarella.com>

dt-bindings: gpio: pl061: add Ambarella CV75 variant

This commit adds the ambarella,cv75-gpio compatible string and variant
constraints to the ARM PL061 GPIO controller Device Tree binding schema,
requiring gpio-ranges and a fixed arm,primecell-periphid of 0x000e8061.

> diff --git a/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml b/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
> index 4d970e55104bb..c22dd8a1d2c18 100644
> --- a/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
> +++ b/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
> @@ -15,7 +15,9 @@ select:
>    properties:
>      compatible:
>        contains:
> -        const: arm,pl061
> +        enum:
> +          - arm,pl061
> +          - ambarella,cv75-gpio

[Severity: Low]
As reported by Krzysztof Kozlowski on v1 [1], does the ambarella vendor
prefix in ambarella,cv75-gpio need to be documented in
Documentation/devicetree/bindings/vendor-prefixes.yaml first so that
checkpatch.pl and make dt_binding_check do not report an undocumented vendor
prefix?

>    required:
>      - compatible
>  
> @@ -25,7 +27,9 @@ properties:
>  
>    compatible:
>      items:
> -      - const: arm,pl061
> +      - enum:
> +          - ambarella,cv75-gpio
> +          - arm,pl061
>        - const: arm,primecell
>  
>    reg:
> @@ -64,6 +68,21 @@ required:
>    - "#gpio-cells"
>    - gpio-controller
>  
> -additionalProperties: false
> +allOf:
> +  - $ref: /schemas/arm/primecell.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ambarella,cv75-gpio
> +    then:
> +      properties:
> +        arm,primecell-periphid:
> +          const: 0x000e8061
> +      required:
> +        - arm,primecell-periphid
> +        - gpio-ranges
> +
> +unevaluatedProperties: false

[Severity: Medium]
Should ambarella,cv75-gpio be split into its own binding schema document
instead of adding conditional rules to
Documentation/devicetree/bindings/gpio/pl061-gpio.yaml?

As noted in the commit description, the Ambarella CV75 controller has 32-bit
registers and 32 lines per bank, adds MASK and ENABLE registers, is not
register-compatible with the 8-line ARM PL061, and does not implement a
standard PrimeCell ID in its hardware ID registers.

In Documentation/devicetree/bindings/gpio/pl061-gpio.yaml, the existing
properties are constrained for an 8-line PL061 block:

  interrupts:
    oneOf:
      - maxItems: 1
      - maxItems: 8
  ...
  gpio-ranges:
    minItems: 1
    maxItems: 8

Requiring gpio-ranges for ambarella,cv75-gpio in this schema leaves the
8-item limit in place for a 32-line bank, and arm,primecell-periphid is only
defined inside the conditional then branch rather than top-level properties.
Given these hardware and schema differences, would a separate schema file for
ambarella,cv75-gpio be cleaner?

[1] https://lore.kernel.org/r/a8b0729c-d6ff-4fa0-955e-deb0af0e726b@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-cv75-gpio-v13-0-4f93114ea204@ambarella.com?part=1