Re: [PATCH 1/3] scsi: ufs: spacemit: dt-bindings: Add UFS controller for K3 SoC

hi sashiko,

On 02:38 Thu 02 Jul     , [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] The example device tree node is missing the required `interrupts` property, causing schema validation (`dt_binding_check`) to fail.
> - [Low] The `freq-table-hz` property in the example uses an invalid flat array syntax for a matrix property and relies on a deprecated binding.
> --
> 
> commit 1206e23c10bdd35379971f48c29e246d5b43200d
> Author: Yixun Lan <[email protected]>
> 
> scsi: ufs: spacemit: dt-bindings: Add UFS controller for K3 SoC
> 
> Document the compatible for UFS (Universal Flash Storage) Host Controller
> which found in SpacemiT K3 SoC chip. This adds the YAML schema defining 
> features like MIPI UniPro v1.61 and M-PHY v3.0 compliance.
> 
> > diff --git a/Documentation/devicetree/bindings/ufs/spacemit,k3-ufshc.yaml b/Documentation/devicetree/bindings/ufs/spacemit,k3-ufshc.yaml
> > new file mode 100644
> > index 0000000000000..e6cb6fb2496c8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ufs/spacemit,k3-ufshc.yaml
> 
> [ ... ]
> 
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/spacemit,k3-clocks.h>
> > +    #include <dt-bindings/reset/spacemit,k3-resets.h>
> > +    ufs@c0e00000 {
> > +        compatible = "spacemit,k3-ufshc";
> > +        reg = <0xc0e00000 0x40000>;
> 
> [Severity: Low]
> Does this example node miss the interrupts property? The included
> ufs-common.yaml schema requires an interrupts property, which might cause
> make dt_binding_check to fail validation here.
> 
right, I got this warning during dts checking, but messed up in final commit,
I will fix it in next version

> > +        clocks = <&syscon_apmu CLK_APMU_UFS_ACLK>,
> > +        <&syscon_apmu CLK_APMU_UFS_REFCLK>;
> > +        clock-names = "aclk", "ref_clk";
> > +        resets = <&syscon_apmu RESET_APMU_UFS_ACLK>;
> > +        freq-table-hz = <491520000 491520000 19200000 19200000>;
> 
> [Severity: Low]
> Is this the correct formatting for freq-table-hz? This property expects a
> multi-dimensional array, so writing it as a flat array (rather than
> <A B>, <C D>) will likely cause dt-schema validation to fail.
> 
right, should be multi-dimensional array

> Also, does this introduce a deprecated property to a new binding?
> 
seems prefer to use operating-points-v2 as new way, I will check it..

-- 
Yixun Lan (dlan)