| Message ID | 83adc98adc1760a0fad87d81d171e1dac783e7e5.1544235317.git.thinhn@synopsys.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | usb: dwc3: Introduce refclk lpm | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | "total: 0 errors, 1 warnings, 8 lines checked" |
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
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
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
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
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.
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
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
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 --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
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(+)