Hi Felipe, On 11/8/2018 11:14 PM, Felipe Balbi wrote: > Hi, > > Thinh Nguyen <thinh.nguyen@xxxxxxxxxxxx> writes: >>> Thinh Nguyen <thinh.nguyen@xxxxxxxxxxxx> writes: >>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt >>>>>>>> index 636630fb92d7..712b344c3a31 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >>>>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >>>>>>>> @@ -95,6 +95,24 @@ Optional properties: >>>>>>>> this and tx-thr-num-pkt-prd to a valid, non-zero value >>>>>>>> 1-16 (DWC_usb31 programming guide section 1.2.3) to >>>>>>>> enable periodic ESS TX threshold. >>>>>>>> + - snps,refclk-period-ns: set to program the reference clock period. The valid >>>>>>>> + input periods are as follow: >>>>>>>> + +-------------+-----------------+ >>>>>>>> + | Period (ns) | Freq (MHz) | >>>>>>>> + +-------------+-----------------+ >>>>>>>> + | 25 | 39.7/40 | >>>>>>>> + | 41 | 24.4 | >>>>>>>> + | 50 | 20 | >>>>>>>> + | 52 | 19.2 | >>>>>>>> + | 58 | 17.2 | >>>>>>>> + | 62 | 16.1 | >>>>>>>> + +-------------+-----------------+ >>>>>>>> + - snps,enable-refclk-lpm: set to enable low power scheduling of isochronous >>>>>>>> + transfers by running SOF/ITP counters using the >>>>>>>> + reference clock. Only valid for DWC_usb31 peripheral >>>>>>>> + controller v1.80a and higher. Both >>>>>>>> + "snps,dis_u2_susphy_quirk" and >>>>>>>> + "snps,dis_enblslpm_quirk" must not be set. >>>>>>> sounds like you should rely on clk API here. Then on driver call >>>>>>> clk_get_rate() to computer whatever you need to compute. >>>>>>> >>>>>> There's nothing to compute here. We can simply enable this feature with >>>>>> "snps, enable-refclk-lpm" and the controller will use the default refclk >>>>>> settings. >>>>> Right, right. What I'm saying, though, is that we have a clock API for >>>>> describing a clock. So why wouldn't we rely on that API for this? I >>>>> think both of these new properties can be replaced with standard clock >>>>> API properties: >>>>> >>>>> clocks = <&clk1>, ..., <&lpm_clk> >>>>> clock-names = "clock1", ...., "lpm"; >>>>> >>>>> Then dwc3 core could, simply, check if we have a clock named "lpm" and >>>>> if there is, use NSECS_PER_SEC / clk_get_rate() to get its period and >>>>> write it to the register that needs the information. >>>> There's no new clock here. We are using the ref_clk for SOF and ITP >>>> counter for this feature. Also, clocks are optional on non-DT platforms. >>>> To use the clock API, then we need to update the driver to allow some >>>> optional clock such as "ref" clock for non-DT platforms. Do you want to >>>> do it like this? >>> I can't think of a problem that would arise from that. Can you? Mark, >>> Rob, what do you think? >>> >> No problem. That can be done. This will remove the >> "snps,refclk-period-ns" property. But we should have >> "snps,enable-refclk-lpm" to enable this feature. > not really. Just check if you have a clock named lpm. If you do, then > you enable the feature. > But this clock name should be "ref". The new name "lpm" would make it seem like it's a different clock. Thinh
