| Message ID | 20221110133640.30522-2-zhewang116@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: ufs: Add document for Unisoc UFS host controller | expand |
Hi Krzysztof, On Fri, Nov 11, 2022 at 5:51 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 11/11/2022 10:34, Zhe Wang wrote: > > Hi Krzysztof, > > > > On Fri, Nov 11, 2022 at 3:48 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 11/11/2022 06:34, Zhe Wang wrote: > >>>> > >>> > >>> I'll fix it. > >>> > >>>>> + 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? > >>>> > >>> > >>> freq-table-hz is used to configure the maximum frequency and minimum > >>> frequency of clk, and an empty table means that no scaling up\down > >>> operation is requiredfor the frequency of these clks. > >> > >> No, to indicate lack of scaling you skip freq-table-hz entirely, not > >> provide empty one. > >> > >> > > > > In the ufshcd-pltfrm.c file, the clock information is parsed by > > executing the function ufshcd_parse_clock_info, if the number of > > "freq-table-hz" is zero or if the number of "clock-names" and > > "freq-table-hz" does not match, the UFS CLK information in dts will > > not be obtained. Although we don't need to scaling freq, we also need > > the CLK information for the CLK GATE operations. So we cannot delete > > this freq-table here. > > I did not check the driver implementation, but if that's the case it > does not match bindings. Before adding empty useless tables, please > either fix bindings or driver. I think the latter - the driver - becasue > clocks are not depending logically on freq-table-hz. > OK, I will think about how to solve this problem. > > > >> Best regards, > >> Krzysztof > >> > > > > According to the local test results just now, I would like to ask a > > question about the previous revisions. > >>> + > >>> + 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 > >> > > > > In the syntax of this example, reg is represented by phandle and > > offset, but I only need the information of phandle in this place， > > No. You wrote "analog reg". So one reg. Not regs. > > > So in > > this scenario, whether my original syntax is fine？ just describe the > > pandlle. > > No, because your description said one reg. > This is my mistake, the description is wrong, it's regs. Thank you for your review! Best regards, Zhe Wang > Best regards, > Krzysztof >
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 @@ -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@unisoc.com> + +allOf: + - $ref: ufs-common.yaml + +properties: + compatible: + enum: + - sprd,ums9620-ufs + + clocks: + maxItems: 2 + + clock-names: + items: + - const: hclk + - const: hclk_source + + resets: + maxItems: 2 + + reset-names: + items: + - const: ufs_soft_rst + - const: ufsdev_soft_rst + + sprd,ufs-anly-reg-syscon: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle of syscon used to control ufs analog reg. + + sprd,aon-apb-syscon: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle of syscon used to control always-on reg. + +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"; + 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>; + 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"; + };