Hi Krzysztof, On Fri, Nov 11, 2022 at 5:51 PM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 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@xxxxxxxxxx> 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 >