diff mbox series

[v2,1/4] usb: dwc3: Add property snps,refclk-period-ns

Message ID 83adc98adc1760a0fad87d81d171e1dac783e7e5.1544235317.git.thinhn@synopsys.com (mailing list archive)
State New, archived
Headers show
Series usb: dwc3: Introduce refclk lpm | expand

Commit Message

Thinh Nguyen Dec. 8, 2018, 2:27 a.m. UTC
This patch introduces property "snps,refclk-period-ns" to inform the
controller of the reference clock period. If the reference clock period
is different from the default Core Consultant setting, then this
property can be set to the reference clock period.

This property does not control the reference clock rate. The controller
uses this value to perform internal timing calculations that are based
on the reference clock.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
Changes in v2:
- Split from "usb: dwc3: Add reference clock properties"
- Revise commit message and property description

 Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
 1 file changed, 2 insertions(+)

Comments

Rob Herring (Arm) Dec. 18, 2018, 4:38 p.m. UTC | #1
On Fri, Dec 07, 2018 at 06:27:30PM -0800, Thinh Nguyen wrote:
> This patch introduces property "snps,refclk-period-ns" to inform the
> controller of the reference clock period. If the reference clock period
> is different from the default Core Consultant setting, then this
> property can be set to the reference clock period.
> 
> This property does not control the reference clock rate. The controller
> uses this value to perform internal timing calculations that are based
> on the reference clock.
> 
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
> Changes in v2:
> - Split from "usb: dwc3: Add reference clock properties"
> - Revise commit message and property description
> 
>  Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index 8e5265e9f658..b7e67edff9b2 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -99,6 +99,8 @@ 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: if set, this value informs the controller of the
> +			reference clock period in nanoseconds.

Shouldn't you be able to retrieve the refclk frequency and then 
calculate the period?

Rob
Thinh Nguyen Dec. 19, 2018, 12:22 a.m. UTC | #2
Hi Rob,

On 12/18/2018 8:39 AM, Rob Herring wrote:
> On Fri, Dec 07, 2018 at 06:27:30PM -0800, Thinh Nguyen wrote:
>> This patch introduces property "snps,refclk-period-ns" to inform the
>> controller of the reference clock period. If the reference clock period
>> is different from the default Core Consultant setting, then this
>> property can be set to the reference clock period.
>>
>> This property does not control the reference clock rate. The controller
>> uses this value to perform internal timing calculations that are based
>> on the reference clock.
>>
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>> Changes in v2:
>> - Split from "usb: dwc3: Add reference clock properties"
>> - Revise commit message and property description
>>
>>  Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index 8e5265e9f658..b7e67edff9b2 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -99,6 +99,8 @@ 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: if set, this value informs the controller of the
>> +			reference clock period in nanoseconds.
> Shouldn't you be able to retrieve the refclk frequency and then 
> calculate the period?

The thing is we cannot determine the ref_clk frequency for some devices
that don't specify their clocks. So I think we should have an option to
inform the controller of the ref_clk period for those devices.

Thanks for the the review,
Thinh
Rob Herring (Arm) Dec. 19, 2018, 1:18 p.m. UTC | #3
On Tue, Dec 18, 2018 at 6:22 PM Thinh Nguyen <thinh.nguyen@synopsys.com> wrote:
>
> Hi Rob,
>
> On 12/18/2018 8:39 AM, Rob Herring wrote:
> > On Fri, Dec 07, 2018 at 06:27:30PM -0800, Thinh Nguyen wrote:
> >> This patch introduces property "snps,refclk-period-ns" to inform the
> >> controller of the reference clock period. If the reference clock period
> >> is different from the default Core Consultant setting, then this
> >> property can be set to the reference clock period.
> >>
> >> This property does not control the reference clock rate. The controller
> >> uses this value to perform internal timing calculations that are based
> >> on the reference clock.
> >>
> >> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> >> ---
> >> Changes in v2:
> >> - Split from "usb: dwc3: Add reference clock properties"
> >> - Revise commit message and property description
> >>
> >>  Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> >> index 8e5265e9f658..b7e67edff9b2 100644
> >> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> >> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> >> @@ -99,6 +99,8 @@ 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: if set, this value informs the controller of the
> >> +                    reference clock period in nanoseconds.
> > Shouldn't you be able to retrieve the refclk frequency and then
> > calculate the period?
>
> The thing is we cannot determine the ref_clk frequency for some devices
> that don't specify their clocks. So I think we should have an option to
> inform the controller of the ref_clk period for those devices.

Specifying the clock should be mandatory (if you want/need this
feature). It just requires a fixed-clock node at a minimum.

Rob
Thinh Nguyen Dec. 19, 2018, 9:31 p.m. UTC | #4
Hi Rob,

On 12/19/2018 5:18 AM, Rob Herring wrote:
> On Tue, Dec 18, 2018 at 6:22 PM Thinh Nguyen <thinh.nguyen@synopsys.com> wrote:
>> Hi Rob,
>>
>> On 12/18/2018 8:39 AM, Rob Herring wrote:
>>> On Fri, Dec 07, 2018 at 06:27:30PM -0800, Thinh Nguyen wrote:
>>>> This patch introduces property "snps,refclk-period-ns" to inform the
>>>> controller of the reference clock period. If the reference clock period
>>>> is different from the default Core Consultant setting, then this
>>>> property can be set to the reference clock period.
>>>>
>>>> This property does not control the reference clock rate. The controller
>>>> uses this value to perform internal timing calculations that are based
>>>> on the reference clock.
>>>>
>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>> ---
>>>> Changes in v2:
>>>> - Split from "usb: dwc3: Add reference clock properties"
>>>> - Revise commit message and property description
>>>>
>>>>  Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>> index 8e5265e9f658..b7e67edff9b2 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>> @@ -99,6 +99,8 @@ 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: if set, this value informs the controller of the
>>>> +                    reference clock period in nanoseconds.
>>> Shouldn't you be able to retrieve the refclk frequency and then
>>> calculate the period?
>> The thing is we cannot determine the ref_clk frequency for some devices
>> that don't specify their clocks. So I think we should have an option to
>> inform the controller of the ref_clk period for those devices.
> Specifying the clock should be mandatory (if you want/need this
> feature). It just requires a fixed-clock node at a minimum.

Depending on the design of the controller, the ref_clk frequency is not
something that the OS can read/control. So we cannot make it mandatory
for every device to have a clock node.

Thinh
Felipe Balbi Dec. 20, 2018, 6:48 a.m. UTC | #5
Hi,

Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>> On 12/18/2018 8:39 AM, Rob Herring wrote:
>>>> On Fri, Dec 07, 2018 at 06:27:30PM -0800, Thinh Nguyen wrote:
>>>>> This patch introduces property "snps,refclk-period-ns" to inform the
>>>>> controller of the reference clock period. If the reference clock period
>>>>> is different from the default Core Consultant setting, then this
>>>>> property can be set to the reference clock period.
>>>>>
>>>>> This property does not control the reference clock rate. The controller
>>>>> uses this value to perform internal timing calculations that are based
>>>>> on the reference clock.
>>>>>
>>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>>> ---
>>>>> Changes in v2:
>>>>> - Split from "usb: dwc3: Add reference clock properties"
>>>>> - Revise commit message and property description
>>>>>
>>>>>  Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>> index 8e5265e9f658..b7e67edff9b2 100644
>>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>> @@ -99,6 +99,8 @@ 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: if set, this value informs the controller of the
>>>>> +                    reference clock period in nanoseconds.
>>>> Shouldn't you be able to retrieve the refclk frequency and then
>>>> calculate the period?
>>> The thing is we cannot determine the ref_clk frequency for some devices
>>> that don't specify their clocks. So I think we should have an option to
>>> inform the controller of the ref_clk period for those devices.
>> Specifying the clock should be mandatory (if you want/need this
>> feature). It just requires a fixed-clock node at a minimum.
>
> Depending on the design of the controller, the ref_clk frequency is not
> something that the OS can read/control. So we cannot make it mandatory
> for every device to have a clock node.

We can make it mandatory to everyone who wants to use the feature. It's
no different than making snps,refclk-period-ns mandatory to everyone who
wants to use the feature you're introducing.
Thinh Nguyen Dec. 21, 2018, 12:21 a.m. UTC | #6
Hi,

On 12/19/2018 10:48 PM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>>> On 12/18/2018 8:39 AM, Rob Herring wrote:
>>>>> On Fri, Dec 07, 2018 at 06:27:30PM -0800, Thinh Nguyen wrote:
>>>>>> This patch introduces property "snps,refclk-period-ns" to inform the
>>>>>> controller of the reference clock period. If the reference clock period
>>>>>> is different from the default Core Consultant setting, then this
>>>>>> property can be set to the reference clock period.
>>>>>>
>>>>>> This property does not control the reference clock rate. The controller
>>>>>> uses this value to perform internal timing calculations that are based
>>>>>> on the reference clock.
>>>>>>
>>>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>> - Split from "usb: dwc3: Add reference clock properties"
>>>>>> - Revise commit message and property description
>>>>>>
>>>>>>  Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>>>>>  1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>> index 8e5265e9f658..b7e67edff9b2 100644
>>>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>> @@ -99,6 +99,8 @@ 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: if set, this value informs the controller of the
>>>>>> +                    reference clock period in nanoseconds.
>>>>> Shouldn't you be able to retrieve the refclk frequency and then
>>>>> calculate the period?
>>>> The thing is we cannot determine the ref_clk frequency for some devices
>>>> that don't specify their clocks. So I think we should have an option to
>>>> inform the controller of the ref_clk period for those devices.
>>> Specifying the clock should be mandatory (if you want/need this
>>> feature). It just requires a fixed-clock node at a minimum.
>> Depending on the design of the controller, the ref_clk frequency is not
>> something that the OS can read/control. So we cannot make it mandatory
>> for every device to have a clock node.
> We can make it mandatory to everyone who wants to use the feature. It's
> no different than making snps,refclk-period-ns mandatory to everyone who
> wants to use the feature you're introducing.
>

But not every design has access to the clock with an actual address for
the OS to read. Only the developers will know the frequency of the
ref_clk, and they can inform the controller via this property.

We cannot force the developers to change their design requirement simply
to inform the controller of the ref_clk period.

Thinh
Rob Herring (Arm) Dec. 21, 2018, 5:11 p.m. UTC | #7
On Thu, Dec 20, 2018 at 6:22 PM Thinh Nguyen <thinh.nguyen@synopsys.com> wrote:
>
> Hi,
>
> On 12/19/2018 10:48 PM, Felipe Balbi wrote:
> > Hi,
> >
> > Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
> >>>> On 12/18/2018 8:39 AM, Rob Herring wrote:
> >>>>> On Fri, Dec 07, 2018 at 06:27:30PM -0800, Thinh Nguyen wrote:
> >>>>>> This patch introduces property "snps,refclk-period-ns" to inform the
> >>>>>> controller of the reference clock period. If the reference clock period
> >>>>>> is different from the default Core Consultant setting, then this
> >>>>>> property can be set to the reference clock period.
> >>>>>>
> >>>>>> This property does not control the reference clock rate. The controller
> >>>>>> uses this value to perform internal timing calculations that are based
> >>>>>> on the reference clock.
> >>>>>>
> >>>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> >>>>>> ---
> >>>>>> Changes in v2:
> >>>>>> - Split from "usb: dwc3: Add reference clock properties"
> >>>>>> - Revise commit message and property description
> >>>>>>
> >>>>>>  Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
> >>>>>>  1 file changed, 2 insertions(+)
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>>> index 8e5265e9f658..b7e67edff9b2 100644
> >>>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>>> @@ -99,6 +99,8 @@ 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: if set, this value informs the controller of the
> >>>>>> +                    reference clock period in nanoseconds.
> >>>>> Shouldn't you be able to retrieve the refclk frequency and then
> >>>>> calculate the period?
> >>>> The thing is we cannot determine the ref_clk frequency for some devices
> >>>> that don't specify their clocks. So I think we should have an option to
> >>>> inform the controller of the ref_clk period for those devices.
> >>> Specifying the clock should be mandatory (if you want/need this
> >>> feature). It just requires a fixed-clock node at a minimum.
> >> Depending on the design of the controller, the ref_clk frequency is not
> >> something that the OS can read/control. So we cannot make it mandatory
> >> for every device to have a clock node.
> > We can make it mandatory to everyone who wants to use the feature. It's
> > no different than making snps,refclk-period-ns mandatory to everyone who
> > wants to use the feature you're introducing.
> >
>
> But not every design has access to the clock with an actual address for
> the OS to read. Only the developers will know the frequency of the
> ref_clk, and they can inform the controller via this property.

Then you use the flxed-clock binding.

Rob
Thinh Nguyen Dec. 21, 2018, 7:30 p.m. UTC | #8
Hi,

On 12/21/2018 9:12 AM, Rob Herring wrote:
> On Thu, Dec 20, 2018 at 6:22 PM Thinh Nguyen <thinh.nguyen@synopsys.com> wrote:
>> Hi,
>>
>> On 12/19/2018 10:48 PM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>>>>> On 12/18/2018 8:39 AM, Rob Herring wrote:
>>>>>>> On Fri, Dec 07, 2018 at 06:27:30PM -0800, Thinh Nguyen wrote:
>>>>>>>> This patch introduces property "snps,refclk-period-ns" to inform the
>>>>>>>> controller of the reference clock period. If the reference clock period
>>>>>>>> is different from the default Core Consultant setting, then this
>>>>>>>> property can be set to the reference clock period.
>>>>>>>>
>>>>>>>> This property does not control the reference clock rate. The controller
>>>>>>>> uses this value to perform internal timing calculations that are based
>>>>>>>> on the reference clock.
>>>>>>>>
>>>>>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>>>>>>>> ---
>>>>>>>> Changes in v2:
>>>>>>>> - Split from "usb: dwc3: Add reference clock properties"
>>>>>>>> - Revise commit message and property description
>>>>>>>>
>>>>>>>>  Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>>>>>>>>  1 file changed, 2 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>>>> index 8e5265e9f658..b7e67edff9b2 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>>>> @@ -99,6 +99,8 @@ 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: if set, this value informs the controller of the
>>>>>>>> +                    reference clock period in nanoseconds.
>>>>>>> Shouldn't you be able to retrieve the refclk frequency and then
>>>>>>> calculate the period?
>>>>>> The thing is we cannot determine the ref_clk frequency for some devices
>>>>>> that don't specify their clocks. So I think we should have an option to
>>>>>> inform the controller of the ref_clk period for those devices.
>>>>> Specifying the clock should be mandatory (if you want/need this
>>>>> feature). It just requires a fixed-clock node at a minimum.
>>>> Depending on the design of the controller, the ref_clk frequency is not
>>>> something that the OS can read/control. So we cannot make it mandatory
>>>> for every device to have a clock node.
>>> We can make it mandatory to everyone who wants to use the feature. It's
>>> no different than making snps,refclk-period-ns mandatory to everyone who
>>> wants to use the feature you're introducing.
>>>
>> But not every design has access to the clock with an actual address for
>> the OS to read. Only the developers will know the frequency of the
>> ref_clk, and they can inform the controller via this property.
> Then you use the flxed-clock binding.

Ah.. I didn't know that. Sure, I'll check and revise the change so it
can be done that way.

Thanks,
Thinh
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index 8e5265e9f658..b7e67edff9b2 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -99,6 +99,8 @@  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: if set, this value informs the controller of the
+			reference clock period in nanoseconds.
 
  - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
  - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0