All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] usb: dwc3: Introduce refclk lpm
@ 2018-11-03  1:35 Thinh Nguyen
  2018-11-03  1:35   ` [1/3] " Thinh Nguyen
  0 siblings, 1 reply; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-03  1:35 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb, devicetree, Rob Herring, Mark Rutland; +Cc: John Youn

This patch series introduce a new feature in DWC_usb31 with more
aggressive low power management using reference clock.


Thinh Nguyen (3):
  usb: dwc3: Add reference clock properties
  usb: dwc3: Set reference clock period
  usb: dwc3: Enable low power management using refclk

 Documentation/devicetree/bindings/usb/dwc3.txt | 18 ++++++++++++++
 drivers/usb/dwc3/core.c                        | 34 ++++++++++++++++++++++++++
 drivers/usb/dwc3/core.h                        | 14 +++++++++++
 3 files changed, 66 insertions(+)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-03  1:35   ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-03  1:35 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb, devicetree, Rob Herring, Mark Rutland; +Cc: John Youn

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.
 
  - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
  - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0
-- 
2.11.0

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-03  1:35   ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-03  1:35 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb, devicetree, Rob Herring, Mark Rutland; +Cc: John Youn

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.
 
  - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
  - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-06 11:26     ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-06 11:26 UTC (permalink / raw)
  To: Thinh Nguyen, linux-usb, devicetree, Rob Herring, Mark Rutland; +Cc: John Youn

[-- Attachment #1: Type: text/plain, Size: 2776 bytes --]


hi,

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.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-06 11:26     ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-06 11:26 UTC (permalink / raw)
  To: Thinh Nguyen, linux-usb, devicetree, Rob Herring, Mark Rutland; +Cc: John Youn

hi,

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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-07  4:11       ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-07  4:11 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

Hi Felipe,

On 11/6/2018 3:26 AM, Felipe Balbi wrote:
> hi,
>
> 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.

Thinh


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-07  4:11       ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-07  4:11 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

Hi Felipe,

On 11/6/2018 3:26 AM, Felipe Balbi wrote:
> hi,
>
> 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.

Thinh

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-07  6:37         ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-07  6:37 UTC (permalink / raw)
  To: Thinh Nguyen; +Cc: John Youn

[-- Attachment #1: Type: text/plain, Size: 3626 bytes --]


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.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-07  6:37         ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-07  6:37 UTC (permalink / raw)
  To: Thinh Nguyen; +Cc: John Youn

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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-07 22:49           ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-07 22:49 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-07 22:49           ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-07 22:49 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-08  7:17             ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-08  7:17 UTC (permalink / raw)
  To: Thinh Nguyen; +Cc: John Youn

[-- Attachment #1: Type: text/plain, Size: 2774 bytes --]


Hi,

Thinh Nguyen <thinh.nguyen@synopsys.com> 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?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-08  7:17             ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-08  7:17 UTC (permalink / raw)
  To: Thinh Nguyen; +Cc: John Youn

Hi,

Thinh Nguyen <thinh.nguyen@synopsys.com> 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?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-08 19:51               ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-08 19:51 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

Hi Felipe,

On 11/7/2018 11:17 PM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@synopsys.com> 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.

Thinh

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-08 19:51               ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-08 19:51 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

Hi Felipe,

On 11/7/2018 11:17 PM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@synopsys.com> 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.

Thinh

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-09  7:14                 ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-09  7:14 UTC (permalink / raw)
  To: Thinh Nguyen; +Cc: John Youn

[-- Attachment #1: Type: text/plain, Size: 3191 bytes --]


Hi,

Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>> Thinh Nguyen <thinh.nguyen@synopsys.com> 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.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-09  7:14                 ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-09  7:14 UTC (permalink / raw)
  To: Thinh Nguyen; +Cc: John Youn

Hi,

Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>> Thinh Nguyen <thinh.nguyen@synopsys.com> 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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-09  7:45                   ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-09  7:45 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

Hi Felipe,

On 11/8/2018 11:14 PM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>> Thinh Nguyen <thinh.nguyen@synopsys.com> 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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-09  7:45                   ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-11-09  7:45 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

Hi Felipe,

On 11/8/2018 11:14 PM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>> Thinh Nguyen <thinh.nguyen@synopsys.com> 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-11-09  8:54                     ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-09  8:54 UTC (permalink / raw)
  To: Thinh Nguyen; +Cc: John Youn

[-- Attachment #1: Type: text/plain, Size: 3649 bytes --]


Hi,

Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>>> Thinh Nguyen <thinh.nguyen@synopsys.com> 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.

now I understand. There's no special LPM clock, this is just the regular
old reference clock being used for LPM. I agree with you, only
refclk-period-ns will be replaced.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-11-09  8:54                     ` Felipe Balbi
  0 siblings, 0 replies; 23+ messages in thread
From: Felipe Balbi @ 2018-11-09  8:54 UTC (permalink / raw)
  To: Thinh Nguyen; +Cc: John Youn

Hi,

Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>>> Thinh Nguyen <thinh.nguyen@synopsys.com> 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.

now I understand. There's no special LPM clock, this is just the regular
old reference clock being used for LPM. I agree with you, only
refclk-period-ns will be replaced.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/3] usb: dwc3: Add reference clock properties
@ 2018-12-08  2:25                       ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-12-08  2:25 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

Hi Felipe,

On 11/9/2018 12:54 AM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>>>> Thinh Nguyen <thinh.nguyen@synopsys.com> 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.
> now I understand. There's no special LPM clock, this is just the regular
> old reference clock being used for LPM. I agree with you, only
> refclk-period-ns will be replaced.
>

I had some misunderstanding with the purpose of GUCTL.REFCLKPER. After a
discussion with the RTL engineers, it's not to control the reference
clock. Setting this register field does not control the reference clock
rate. The controller uses this value to perform internal timing
calculations that are based on the reference clock. So, we will still
need this property to set this value. I will resend the patch series
with some changes and correct the commit messages with the proper
definition of this feature.

Thanks,
Thinh


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [1/3] usb: dwc3: Add reference clock properties
@ 2018-12-08  2:25                       ` Thinh Nguyen
  0 siblings, 0 replies; 23+ messages in thread
From: Thinh Nguyen @ 2018-12-08  2:25 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, linux-usb, devicetree, Rob Herring,
	Mark Rutland
  Cc: John Youn

Hi Felipe,

On 11/9/2018 12:54 AM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>>>>> Thinh Nguyen <thinh.nguyen@synopsys.com> 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.
> now I understand. There's no special LPM clock, this is just the regular
> old reference clock being used for LPM. I agree with you, only
> refclk-period-ns will be replaced.
>

I had some misunderstanding with the purpose of GUCTL.REFCLKPER. After a
discussion with the RTL engineers, it's not to control the reference
clock. Setting this register field does not control the reference clock
rate. The controller uses this value to perform internal timing
calculations that are based on the reference clock. So, we will still
need this property to set this value. I will resend the patch series
with some changes and correct the commit messages with the proper
definition of this feature.

Thanks,
Thinh

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2018-12-08  2:25 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-03  1:35 [PATCH 0/3] usb: dwc3: Introduce refclk lpm Thinh Nguyen
2018-11-03  1:35 ` [PATCH 1/3] usb: dwc3: Add reference clock properties Thinh Nguyen
2018-11-03  1:35   ` [1/3] " Thinh Nguyen
2018-11-06 11:26   ` [PATCH 1/3] " Felipe Balbi
2018-11-06 11:26     ` [1/3] " Felipe Balbi
2018-11-07  4:11     ` [PATCH 1/3] " Thinh Nguyen
2018-11-07  4:11       ` [1/3] " Thinh Nguyen
2018-11-07  6:37       ` [PATCH 1/3] " Felipe Balbi
2018-11-07  6:37         ` [1/3] " Felipe Balbi
2018-11-07 22:49         ` [PATCH 1/3] " Thinh Nguyen
2018-11-07 22:49           ` [1/3] " Thinh Nguyen
2018-11-08  7:17           ` [PATCH 1/3] " Felipe Balbi
2018-11-08  7:17             ` [1/3] " Felipe Balbi
2018-11-08 19:51             ` [PATCH 1/3] " Thinh Nguyen
2018-11-08 19:51               ` [1/3] " Thinh Nguyen
2018-11-09  7:14               ` [PATCH 1/3] " Felipe Balbi
2018-11-09  7:14                 ` [1/3] " Felipe Balbi
2018-11-09  7:45                 ` [PATCH 1/3] " Thinh Nguyen
2018-11-09  7:45                   ` [1/3] " Thinh Nguyen
2018-11-09  8:54                   ` [PATCH 1/3] " Felipe Balbi
2018-11-09  8:54                     ` [1/3] " Felipe Balbi
2018-12-08  2:25                     ` [PATCH 1/3] " Thinh Nguyen
2018-12-08  2:25                       ` [1/3] " Thinh Nguyen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.