Re: [PATCH v2 1/3] scsi: ufs: dt-bindings: starfive: Add UFS Host Controller for JHB100 soc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Missing `clock-names` in the `required` list.
- [Low] Unconstrained `starfive,syscon` property allows invalid array lengths and arguments.
- [Low] The example section uses the deprecated `freq-table-hz` property.
--

commit dcb3253ccedb17e87bb1f204ca98535c01e219f6
Author: Minda Chen <minda.chen@starfivetech.com>

scsi: ufs: dt-bindings: starfive: Add UFS Host Controller for JHB100 soc

This commit adds the devicetree binding documentation for the StarFive JHB100
SoC UFS Host Controller, based on the Synopsys DesignWare IP.

> diff --git a/Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml b/Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
> new file mode 100644
> index 0000000000000..4773f1c2670f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
[ ... ]
> +  starfive,syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      The phandle to System Register Controller syscon node.

The starfive,syscon property is defined as a phandle-array, but should this
include an items or maxItems constraint? Since the example uses a bare
phandle, would it be better typed as just
/schemas/types.yaml#/definitions/phandle to prevent passing arrays of
arbitrary lengths during schema validation?

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - resets
> +  - reset-names
> +  - interrupts
> +  - starfive,syscon

Is there a missing entry for clock-names in this list? The binding
mandates the clocks property and defines 'ref_clk' and 'ufs', so omitting
clock-names here allows device trees to define clocks without names and
still pass validation, potentially failing during driver lookup.

[ ... ]
> +examples:
> +  - |
> +    ufs@11b10000 {
> +        compatible = "starfive,jhb100-ufs";
> +        reg = <0x11b10000 0x20000>;
> +        interrupts = <105>;
> +        clocks = <&syscrg 4>,
> +                 <&syscrg 5>;
> +        clock-names = "ref_clk", "ufs";
> +        freq-table-hz = <26000000 26000000>,
> +                        <100000000 100000000>;

This isn't a bug, but does this example need to use freq-table-hz? The
common schema ufs-common.yaml included via $ref marks this property as
deprecated in favor of operating-points-v2. Using it in a new example
might encourage copying deprecated patterns into downstream device trees.

> +        resets = <&syscrg 10>,
> +                 <&syscrg 7>;
> +        reset-names = "main", "phy";
> +        starfive,syscon = <&syscon>;
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509062759.125472-1-minda.chen@starfivetech.com?part=1