On 10/11/2022 14:36, Zhe Wang wrote: > From: Zhe Wang <zhe.wang1@xxxxxxxxxx> > > Add Unisoc ums9620 ufs host controller devicetree document. > > Signed-off-by: Zhe Wang <zhe.wang1@xxxxxxxxxx> > --- > .../devicetree/bindings/ufs/sprd,ufs.yaml | 72 +++++++++++++++++++ > 1 file changed, 72 insertions(+) > create mode 100644 Documentation/devicetree/bindings/ufs/sprd,ufs.yaml > > diff --git a/Documentation/devicetree/bindings/ufs/sprd,ufs.yaml b/Documentation/devicetree/bindings/ufs/sprd,ufs.yaml > new file mode 100644 > index 000000000000..88f2c670b0a4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/ufs/sprd,ufs.yaml Filename matching the compatible, so sprd,ums9620-ufs.yaml, unless you expect this to grow already? If so, can you post the rest? > @@ -0,0 +1,72 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/ufs/sprd,ufs.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Unisoc Universal Flash Storage (UFS) Controller > + > +maintainers: > + - Zhe Wang <zhe.wang1@xxxxxxxxxx> > + > +allOf: > + - $ref: ufs-common.yaml > + > +properties: > + compatible: > + enum: > + - sprd,ums9620-ufs > + > + clocks: > + maxItems: 2 > + > + clock-names: > + items: > + - const: hclk > + - const: hclk_source Can you make these descriptive? "clk" is redundant, so basically you are saying name is "h" and "h_source"? > + > + resets: > + maxItems: 2 > + > + reset-names: > + items: > + - const: ufs_soft_rst > + - const: ufsdev_soft_rst Drop "_rst" from both. > + > + sprd,ufs-anly-reg-syscon: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle of syscon used to control ufs analog reg. It's a reg? Then such syntax is expected: https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42 > + sprd,aon-apb-syscon: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle of syscon used to control always-on reg. It's a reg? Then such syntax is expected: https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42 > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - resets > + > +unevaluatedProperties: false > + > +examples: > + - | > + ufs: ufs@22000000 { > + compatible = "sprd,ums9620-ufs"; > + reg = <0x22000000 0x3000>; > + interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>; > + vcc-supply = <&vddemmcore>; > + vdd-mphy-supply = <&vddufs1v2>; > + clocks-name = "ufs_eb", "ufs_cfg_eb", > + "ufs_hclk", "ufs_hclk_source"; Align the lines. > + clocks = <&apahb_gate CLK_UFS_EB>, <&apahb_gate CLK_UFS_CFG_EB>, > + <&onapb_clk CLK_UFS_AON>, <&g51_pll CLK_TGPLL_256M>; > + freq-table-hz = <0 0>, <0 0>, <0 0>, <0 0>; Why this is empty? What's the use of empty table? > + reset-names = "ufs_soft_rst", "ufsdev_soft_rst"; > + resets = <&apahb_gate RESET_AP_AHB_UFS_SOFT_RST>, > + <&aonapb_gate RESET_AON_APB_UFSDEV_SOFT_RST>; > + sprd,ufs-anly-reg-syscon = <&anly_phy_g12_regs>; > + sprd,aon-apb-syscon = <&aon_apb_regs>; > + status = "disable"; Drop status. > + }; Best regards, Krzysztof
