Re: [PATCH v4 3/5] dt-bindings: clock: Move xlnx,zynqmp-clk to its own schema

[Michal Simek <michal.simek@amd.com>]

On 7/8/26 09:37, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The regex pattern for `mio_clk` in `clock-names` contains a logic error that fails to correctly constrain the index to 00-77.
> - [Low] The example in `xlnx,versal-clk.yaml` fails schema validation because it wraps a `versal-clk` clock controller in a `zynqmp-firmware` node.
> --
> 
> --- Patch [3]: [PATCH v4 3/5] dt-bindings: clock: Move xlnx,zynqmp-clk to its own schema ---
> commit 718990212b2e54a25ccb16a23d741b1b1a896e2d
> Author: Michal Simek <michal.simek@amd.com>
> 
> dt-bindings: clock: Move xlnx,zynqmp-clk to its own schema
> 
> The ZynqMP clock controller binding shares only #clock-cells with the
> Versal bindings. Move it to a dedicated xlnx,zynqmp-clk.yaml schema.
> Also remove "(Optional clock)" from clock description because it is visible
> from schema itself.
> 
>> diff --git a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
>> index b1623c2ab0f69..12d060c39bfc4 100644
>> --- a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
>> +++ b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> [ ... ]
>>   examples:
>>     - |
>>       firmware {
>>         zynqmp_firmware: zynqmp-firmware {
>>           compatible = "xlnx,zynqmp-firmware";
>>           method = "smc";
>>           versal_clk: clock-controller {
>>             #clock-cells = <1>;
>>             compatible = "xlnx,versal-clk";
> 
> [Severity: Low]
> Will this example now fail make dt_binding_check validation?
> 
> Because xlnx,zynqmp-firmware.yaml is modified in this patch to add an allOf
> rule requiring the clock-controller child of zynqmp-firmware to use
> compatible = "xlnx,zynqmp-clk", this xlnx,versal-clk node inside a
> zynqmp-firmware example seems like it will trigger a schema validation error.
> 

that fix is in 4/5. I will move it to 3/5.

>>             clocks = <&pss_ref_clk>, <&video_clk>, <&pss_alt_ref_clk>;
>>             clock-names = "pss_ref_clk", "video_clk", "pss_alt_ref_clk";
>>           };
>>         };
>>       };
> [ ... ]
>> diff --git a/Documentation/devicetree/bindings/clock/xlnx,zynqmp-clk.yaml b/Documentation/devicetree/bindings/clock/xlnx,zynqmp-clk.yaml
>> new file mode 100644
>> index 0000000000000..c64550471581c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/xlnx,zynqmp-clk.yaml
> [ ... ]
>> +  clock-names:
>> +    minItems: 5
>> +    items:
>> +      - const: pss_ref_clk
>> +      - const: video_clk
>> +      - const: pss_alt_ref_clk
>> +      - const: aux_ref_clk
>> +      - const: gt_crx_ref_clk
>> +      - pattern: "^mio_clk([0-6][0-9]|7[0-7])+.*$"
> 
> [Severity: Medium]
> Does this pattern correctly constrain the clock index to 00-77?
> 
> By combining the + quantifier with .*, this regex allows strings like
> "mio_clk100" to validate, since the "10" matches the capturing group and the
> remaining "0" matches ".*". Could this be corrected to avoid permitting out of
> bounds values?

That's correct but that's just fix in 2/5 where +.* was there before too.
It means yes it is possible that user will pass mio_clk100 and it won't be 
noticed. But I don't have any reference that anybody is really using it and
I don't want to break them.

Thanks,
Michal