Re: [PATCH v5 1/2] dt-bindings: clock: gpio-gate-clock: Add a new compatible string

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: clock: locked-gpios maxItems contradicts commit message
- [Low] dt-bindings: clock: schema does not enforce mutually exclusive GPIO properties
- [Low] dt-bindings: clock: example node name uses invalid underscore character
--

commit 79ea3ad838118b16ee842ed66488a42eefcd7f13
Author: Vyacheslav Yurkov <V.Yurkov.EXT@bruker.com>

dt-bindings: clock: gpio-gate-clock: Add a new compatible string

Extend the binding to support a gpio-locked-fixed-clock. This provides a
fixed-rate clock whose availability depends on one or more GPIO lock-status
signals.

> diff --git a/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml b/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
> index d09d0e3f0c6ec..00cf2ad66dd01 100644
> --- a/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/gpio-gate-clock.yaml

[ ... ]

> @@ -23,10 +25,29 @@ properties:
>      description: GPIO reference for enabling and disabling the clock.
>      maxItems: 1
>  
> +  locked-gpios:
> +    description: GPIO that indicates whether the clock is enabled or disabled.
> +    maxItems: 1

[Severity: Medium]
The commit message states that the clock's availability depends on "one or
more GPIO lock-status signals".

Looking at the schema definition, the locked-gpios property is strictly
restricted to exactly one item via maxItems: 1.

Does this maxItems constraint need to be increased to support multiple signals,
or should the commit message be updated to reflect that only a single GPIO is
supported?

> +
>  required:
>    - compatible
>    - '#clock-cells'
> -  - enable-gpios
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: gpio-gate-clock
> +    then:
> +      required:
> +        - enable-gpios

[Severity: Low]
Since additionalProperties: false only enforces that properties must be
defined in the top-level properties block, both enable-gpios and locked-gpios
are now allowed for any compatible matching this file.

Should this conditional block explicitly reject locked-gpios by setting it
to false? Without this, a device tree node with compatible = "gpio-gate-clock"
can include locked-gpios and inappropriately pass schema validation.

> +  - if:
> +      properties:
> +        compatible:
> +          const: gpio-locked-fixed-clock
> +    then:
> +      required:
> +        - locked-gpios

[ ... ]

> @@ -40,3 +61,10 @@ examples:
>          #clock-cells = <0>;
>          enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
>      };
> +
> +    clk_gpio_locked {
> +        compatible = "gpio-locked-fixed-clock";

[Severity: Low]
Does this example node name need to be updated to use a hyphen instead of an
underscore?

Device tree node names should ideally use hyphens and a generic class name
like clock. Using an underscore violates device tree naming conventions and
will likely trigger a node_name_chars_strict warning during compilation.

> +        #clock-cells = <0>;
> +        clocks = <&pll>;
> +        locked-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>;
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-feature-clock-guard-v5-0-42ab5dc3a6aa@bruker.com?part=1