Re: [PATCH 1/3] usb: dwc3: Add reference clock properties

[Thinh Nguyen <thinh.nguyen@synopsys.com>]

Hi Felipe,

On 11/6/2018 10:37 PM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>>> Add two new device properties to program the reference clock period and
>>>> to enable low power management using the reference clock. This allows a
>>>> higher demand to go in low power for Audio Device Class devices. This
>>>> feature is currently only valid for DWC_usb31 peripheral controller
>>>> v1.80a and higher.
>>>>
>>>> Set "snps,refclk-period-ns" 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            |
>>>>  +-------------+-----------------+
>>>>
>>>> Set "snps,refclk-lpm" to enable low power scheduling of isochronous
>>>> transfers by running SOF/ITP counters using the reference clock. Both
>>>> "snps,dis_u2_susphy_quirk" and "snps,dis_enblslpm_quirk" must not be
>>>> set for this feature to be enabled.
>>>>
>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/dwc3.txt | 18 ++++++++++++++++++
>>>>  1 file changed, 18 insertions(+)
>>>>
>>>> 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?

Thinh