Re: [PATCH 1/2] dt-bindings: ufs: Add document for Unisoc UFS host controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux