From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Thinh Nguyen Subject: Re: [PATCH 1/3] usb: dwc3: Add reference clock properties Date: Sat, 8 Dec 2018 02:25:35 +0000 Message-ID: <30102591E157244384E984126FC3CB4F639A8805@us01wembx1.internal.synopsys.com> References: <877ehqv44p.fsf@linux.intel.com> <30102591E157244384E984126FC3CB4F639A080E@us01wembx1.internal.synopsys.com> <871s7xv1et.fsf@linux.intel.com> <30102591E157244384E984126FC3CB4F639A0ACE@us01wembx1.internal.synopsys.com> <874lcst4wg.fsf@linux.intel.com> <30102591E157244384E984126FC3CB4F639A0FB0@us01wembx1.internal.synopsys.com> <87muqisoxg.fsf@linux.intel.com> <30102591E157244384E984126FC3CB4F639A114E@us01wembx1.internal.synopsys.com> <87k1lmska2.fsf@linux.intel.com> Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 To: Felipe Balbi , Thinh Nguyen , "linux-usb@vger.kernel.org" , "devicetree@vger.kernel.org" , Rob Herring , Mark Rutland Cc: John Youn List-ID: Hi Felipe,=0A= =0A= On 11/9/2018 12:54 AM, Felipe Balbi wrote:=0A= > Hi,=0A= >=0A= > Thinh Nguyen writes:=0A= >>> Thinh Nguyen writes:=0A= >>>>> Thinh Nguyen writes:=0A= >>>>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Do= cumentation/devicetree/bindings/usb/dwc3.txt=0A= >>>>>>>>>> index 636630fb92d7..712b344c3a31 100644=0A= >>>>>>>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt=0A= >>>>>>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt=0A= >>>>>>>>>> @@ -95,6 +95,24 @@ Optional properties:=0A= >>>>>>>>>> this and tx-thr-num-pkt-prd to a valid, non-zero value=0A= >>>>>>>>>> 1-16 (DWC_usb31 programming guide section 1.2.3) to=0A= >>>>>>>>>> enable periodic ESS TX threshold.=0A= >>>>>>>>>> + - snps,refclk-period-ns: set to program the reference clock pe= riod. The valid=0A= >>>>>>>>>> + input periods are as follow:=0A= >>>>>>>>>> + +-------------+-----------------+=0A= >>>>>>>>>> + | Period (ns) | Freq (MHz) |=0A= >>>>>>>>>> + +-------------+-----------------+=0A= >>>>>>>>>> + | 25 | 39.7/40 |=0A= >>>>>>>>>> + | 41 | 24.4 |=0A= >>>>>>>>>> + | 50 | 20 |=0A= >>>>>>>>>> + | 52 | 19.2 |=0A= >>>>>>>>>> + | 58 | 17.2 |=0A= >>>>>>>>>> + | 62 | 16.1 |=0A= >>>>>>>>>> + +-------------+-----------------+=0A= >>>>>>>>>> + - snps,enable-refclk-lpm: set to enable low power scheduling o= f isochronous=0A= >>>>>>>>>> + transfers by running SOF/ITP counters using the=0A= >>>>>>>>>> + reference clock. Only valid for DWC_usb31 peripheral=0A= >>>>>>>>>> + controller v1.80a and higher. Both=0A= >>>>>>>>>> + "snps,dis_u2_susphy_quirk" and=0A= >>>>>>>>>> + "snps,dis_enblslpm_quirk" must not be set.=0A= >>>>>>>>> sounds like you should rely on clk API here. Then on driver call= =0A= >>>>>>>>> clk_get_rate() to computer whatever you need to compute.=0A= >>>>>>>>>=0A= >>>>>>>> There's nothing to compute here. We can simply enable this feature= with=0A= >>>>>>>> "snps, enable-refclk-lpm" and the controller will use the default = refclk=0A= >>>>>>>> settings.=0A= >>>>>>> Right, right. What I'm saying, though, is that we have a clock API = for=0A= >>>>>>> describing a clock. So why wouldn't we rely on that API for this? I= =0A= >>>>>>> think both of these new properties can be replaced with standard cl= ock=0A= >>>>>>> API properties:=0A= >>>>>>>=0A= >>>>>>> clocks =3D <&clk1>, ..., <&lpm_clk>=0A= >>>>>>> clock-names =3D "clock1", ...., "lpm";=0A= >>>>>>>=0A= >>>>>>> Then dwc3 core could, simply, check if we have a clock named "lpm" = and=0A= >>>>>>> if there is, use NSECS_PER_SEC / clk_get_rate() to get its period a= nd=0A= >>>>>>> write it to the register that needs the information.=0A= >>>>>> There's no new clock here. We are using the ref_clk for SOF and ITP= =0A= >>>>>> counter for this feature. Also, clocks are optional on non-DT platfo= rms.=0A= >>>>>> To use the clock API, then we need to update the driver to allow som= e=0A= >>>>>> optional clock such as "ref" clock for non-DT platforms. Do you want= to=0A= >>>>>> do it like this?=0A= >>>>> I can't think of a problem that would arise from that. Can you? Mark,= =0A= >>>>> Rob, what do you think?=0A= >>>>>=0A= >>>> No problem. That can be done. This will remove the=0A= >>>> "snps,refclk-period-ns" property. But we should have=0A= >>>> "snps,enable-refclk-lpm" to enable this feature.=0A= >>> not really. Just check if you have a clock named lpm. If you do, then= =0A= >>> you enable the feature.=0A= >>>=0A= >> But this clock name should be "ref". The new name "lpm" would make it= =0A= >> seem like it's a different clock.=0A= > now I understand. There's no special LPM clock, this is just the regular= =0A= > old reference clock being used for LPM. I agree with you, only=0A= > refclk-period-ns will be replaced.=0A= >=0A= =0A= I had some misunderstanding with the purpose of GUCTL.REFCLKPER. After a=0A= discussion with the RTL engineers, it's not to control the reference=0A= clock. Setting this register field does not control the reference clock=0A= rate. The controller uses this value to perform internal timing=0A= calculations that are based on the reference clock. So, we will still=0A= need this property to set this value. I will resend the patch series=0A= with some changes and correct the commit messages with the proper=0A= definition of this feature.=0A= =0A= Thanks,=0A= Thinh=0A= =0A= From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [1/3] usb: dwc3: Add reference clock properties From: Thinh Nguyen Message-Id: <30102591E157244384E984126FC3CB4F639A8805@us01wembx1.internal.synopsys.com> Date: Sat, 8 Dec 2018 02:25:35 +0000 To: Felipe Balbi , Thinh Nguyen , "linux-usb@vger.kernel.org" , "devicetree@vger.kernel.org" , Rob Herring , Mark Rutland Cc: John Youn List-ID: SGkgRmVsaXBlLAoKT24gMTEvOS8yMDE4IDEyOjU0IEFNLCBGZWxpcGUgQmFsYmkgd3JvdGU6Cj4g SGksCj4KPiBUaGluaCBOZ3V5ZW4gPHRoaW5oLm5ndXllbkBzeW5vcHN5cy5jb20+IHdyaXRlczoK Pj4+IFRoaW5oIE5ndXllbiA8dGhpbmgubmd1eWVuQHN5bm9wc3lzLmNvbT4gd3JpdGVzOgo+Pj4+ PiBUaGluaCBOZ3V5ZW4gPHRoaW5oLm5ndXllbkBzeW5vcHN5cy5jb20+IHdyaXRlczoKPj4+Pj4+ Pj4+PiBkaWZmIC0tZ2l0IGEvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3VzYi9k d2MzLnR4dCBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy91c2IvZHdjMy50eHQK Pj4+Pj4+Pj4+PiBpbmRleCA2MzY2MzBmYjkyZDcuLjcxMmIzNDRjM2EzMSAxMDA2NDQKPj4+Pj4+ Pj4+PiAtLS0gYS9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvdXNiL2R3YzMudHh0 Cj4+Pj4+Pj4+Pj4gKysrIGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3VzYi9k d2MzLnR4dAo+Pj4+Pj4+Pj4+IEBAIC05NSw2ICs5NSwyNCBAQCBPcHRpb25hbCBwcm9wZXJ0aWVz Ogo+Pj4+Pj4+Pj4+ICAJCQl0aGlzIGFuZCB0eC10aHItbnVtLXBrdC1wcmQgdG8gYSB2YWxpZCwg bm9uLXplcm8gdmFsdWUKPj4+Pj4+Pj4+PiAgCQkJMS0xNiAoRFdDX3VzYjMxIHByb2dyYW1taW5n IGd1aWRlIHNlY3Rpb24gMS4yLjMpIHRvCj4+Pj4+Pj4+Pj4gIAkJCWVuYWJsZSBwZXJpb2RpYyBF U1MgVFggdGhyZXNob2xkLgo+Pj4+Pj4+Pj4+ICsgLSBzbnBzLHJlZmNsay1wZXJpb2QtbnM6IHNl dCB0byBwcm9ncmFtIHRoZSByZWZlcmVuY2UgY2xvY2sgcGVyaW9kLiBUaGUgdmFsaWQKPj4+Pj4+ Pj4+PiArICAgCQkJaW5wdXQgcGVyaW9kcyBhcmUgYXMgZm9sbG93Ogo+Pj4+Pj4+Pj4+ICsJCQkr LS0tLS0tLS0tLS0tLSstLS0tLS0tLS0tLS0tLS0tLSsKPj4+Pj4+Pj4+PiArCQkJfCBQZXJpb2Qg KG5zKSB8IEZyZXEgKE1IeikgICAgICB8Cj4+Pj4+Pj4+Pj4gKwkJCSstLS0tLS0tLS0tLS0tKy0t LS0tLS0tLS0tLS0tLS0tKwo+Pj4+Pj4+Pj4+ICsJCQl8IDI1ICAgICAgICAgIHwgMzkuNy80MCAg ICAgICAgIHwKPj4+Pj4+Pj4+PiArCQkJfCA0MSAgICAgICAgICB8IDI0LjQgICAgICAgICAgICB8 Cj4+Pj4+Pj4+Pj4gKwkJCXwgNTAgICAgICAgICAgfCAyMCAgICAgICAgICAgICAgfAo+Pj4+Pj4+ Pj4+ICsJCQl8IDUyICAgICAgICAgIHwgMTkuMiAgICAgICAgICAgIHwKPj4+Pj4+Pj4+PiArCQkJ fCA1OCAgICAgICAgICB8IDE3LjIgICAgICAgICAgICB8Cj4+Pj4+Pj4+Pj4gKwkJCXwgNjIgICAg ICAgICAgfCAxNi4xICAgICAgICAgICAgfAo+Pj4+Pj4+Pj4+ICsJCQkrLS0tLS0tLS0tLS0tLSst LS0tLS0tLS0tLS0tLS0tLSsKPj4+Pj4+Pj4+PiArIC0gc25wcyxlbmFibGUtcmVmY2xrLWxwbTog c2V0IHRvIGVuYWJsZSBsb3cgcG93ZXIgc2NoZWR1bGluZyBvZiBpc29jaHJvbm91cwo+Pj4+Pj4+ Pj4+ICsJCQl0cmFuc2ZlcnMgYnkgcnVubmluZyBTT0YvSVRQIGNvdW50ZXJzIHVzaW5nIHRoZQo+ Pj4+Pj4+Pj4+ICsJCQlyZWZlcmVuY2UgY2xvY2suIE9ubHkgdmFsaWQgZm9yIERXQ191c2IzMSBw ZXJpcGhlcmFsCj4+Pj4+Pj4+Pj4gKwkJCWNvbnRyb2xsZXIgdjEuODBhIGFuZCBoaWdoZXIuIEJv dGgKPj4+Pj4+Pj4+PiArCQkJInNucHMsZGlzX3UyX3N1c3BoeV9xdWlyayIgYW5kCj4+Pj4+Pj4+ Pj4gKwkJCSJzbnBzLGRpc19lbmJsc2xwbV9xdWlyayIgbXVzdCBub3QgYmUgc2V0Lgo+Pj4+Pj4+ Pj4gc291bmRzIGxpa2UgeW91IHNob3VsZCByZWx5IG9uIGNsayBBUEkgaGVyZS4gVGhlbiBvbiBk cml2ZXIgY2FsbAo+Pj4+Pj4+Pj4gY2xrX2dldF9yYXRlKCkgdG8gY29tcHV0ZXIgd2hhdGV2ZXIg eW91IG5lZWQgdG8gY29tcHV0ZS4KPj4+Pj4+Pj4+Cj4+Pj4+Pj4+IFRoZXJlJ3Mgbm90aGluZyB0 byBjb21wdXRlIGhlcmUuIFdlIGNhbiBzaW1wbHkgZW5hYmxlIHRoaXMgZmVhdHVyZSB3aXRoCj4+ Pj4+Pj4+ICJzbnBzLCBlbmFibGUtcmVmY2xrLWxwbSIgYW5kIHRoZSBjb250cm9sbGVyIHdpbGwg dXNlIHRoZSBkZWZhdWx0IHJlZmNsawo+Pj4+Pj4+PiBzZXR0aW5ncy4KPj4+Pj4+PiBSaWdodCwg cmlnaHQuIFdoYXQgSSdtIHNheWluZywgdGhvdWdoLCBpcyB0aGF0IHdlIGhhdmUgYSBjbG9jayBB UEkgZm9yCj4+Pj4+Pj4gZGVzY3JpYmluZyBhIGNsb2NrLiBTbyB3aHkgd291bGRuJ3Qgd2UgcmVs eSBvbiB0aGF0IEFQSSBmb3IgdGhpcz8gSQo+Pj4+Pj4+IHRoaW5rIGJvdGggb2YgdGhlc2UgbmV3 IHByb3BlcnRpZXMgY2FuIGJlIHJlcGxhY2VkIHdpdGggc3RhbmRhcmQgY2xvY2sKPj4+Pj4+PiBB UEkgcHJvcGVydGllczoKPj4+Pj4+Pgo+Pj4+Pj4+IAljbG9ja3MgPSA8JmNsazE+LCAuLi4sIDwm bHBtX2Nsaz4KPj4+Pj4+PiAgICAgICAgIGNsb2NrLW5hbWVzID0gImNsb2NrMSIsIC4uLi4sICJs cG0iOwo+Pj4+Pj4+Cj4+Pj4+Pj4gVGhlbiBkd2MzIGNvcmUgY291bGQsIHNpbXBseSwgY2hlY2sg aWYgd2UgaGF2ZSBhIGNsb2NrIG5hbWVkICJscG0iIGFuZAo+Pj4+Pj4+IGlmIHRoZXJlIGlzLCB1 c2UgTlNFQ1NfUEVSX1NFQyAvIGNsa19nZXRfcmF0ZSgpIHRvIGdldCBpdHMgcGVyaW9kIGFuZAo+ Pj4+Pj4+IHdyaXRlIGl0IHRvIHRoZSByZWdpc3RlciB0aGF0IG5lZWRzIHRoZSBpbmZvcm1hdGlv bi4KPj4+Pj4+IFRoZXJlJ3Mgbm8gbmV3IGNsb2NrIGhlcmUuIFdlIGFyZSB1c2luZyB0aGUgcmVm X2NsayBmb3IgU09GIGFuZCBJVFAKPj4+Pj4+IGNvdW50ZXIgZm9yIHRoaXMgZmVhdHVyZS4gQWxz bywgY2xvY2tzIGFyZSBvcHRpb25hbCBvbiBub24tRFQgcGxhdGZvcm1zLgo+Pj4+Pj4gVG8gdXNl IHRoZSBjbG9jayBBUEksIHRoZW4gd2UgbmVlZCB0byB1cGRhdGUgdGhlIGRyaXZlciB0byBhbGxv dyBzb21lCj4+Pj4+PiBvcHRpb25hbCBjbG9jayBzdWNoIGFzICJyZWYiIGNsb2NrIGZvciBub24t RFQgcGxhdGZvcm1zLiBEbyB5b3Ugd2FudCB0bwo+Pj4+Pj4gZG8gaXQgbGlrZSB0aGlzPwo+Pj4+ PiBJIGNhbid0IHRoaW5rIG9mIGEgcHJvYmxlbSB0aGF0IHdvdWxkIGFyaXNlIGZyb20gdGhhdC4g Q2FuIHlvdT8gTWFyaywKPj4+Pj4gUm9iLCB3aGF0IGRvIHlvdSB0aGluaz8KPj4+Pj4KPj4+PiBO byBwcm9ibGVtLiBUaGF0IGNhbiBiZSBkb25lLiBUaGlzIHdpbGwgcmVtb3ZlIHRoZQo+Pj4+ICJz bnBzLHJlZmNsay1wZXJpb2QtbnMiIHByb3BlcnR5LiBCdXQgd2Ugc2hvdWxkIGhhdmUKPj4+PiAi c25wcyxlbmFibGUtcmVmY2xrLWxwbSIgdG8gZW5hYmxlIHRoaXMgZmVhdHVyZS4KPj4+IG5vdCBy ZWFsbHkuIEp1c3QgY2hlY2sgaWYgeW91IGhhdmUgYSBjbG9jayBuYW1lZCBscG0uIElmIHlvdSBk bywgdGhlbgo+Pj4geW91IGVuYWJsZSB0aGUgZmVhdHVyZS4KPj4+Cj4+IEJ1dCB0aGlzIGNsb2Nr IG5hbWUgc2hvdWxkIGJlICJyZWYiLiAgVGhlIG5ldyBuYW1lICJscG0iIHdvdWxkIG1ha2UgaXQK Pj4gc2VlbSBsaWtlIGl0J3MgYSBkaWZmZXJlbnQgY2xvY2suCj4gbm93IEkgdW5kZXJzdGFuZC4g VGhlcmUncyBubyBzcGVjaWFsIExQTSBjbG9jaywgdGhpcyBpcyBqdXN0IHRoZSByZWd1bGFyCj4g b2xkIHJlZmVyZW5jZSBjbG9jayBiZWluZyB1c2VkIGZvciBMUE0uIEkgYWdyZWUgd2l0aCB5b3Us IG9ubHkKPiByZWZjbGstcGVyaW9kLW5zIHdpbGwgYmUgcmVwbGFjZWQuCj4KCkkgaGFkIHNvbWUg bWlzdW5kZXJzdGFuZGluZyB3aXRoIHRoZSBwdXJwb3NlIG9mIEdVQ1RMLlJFRkNMS1BFUi4gQWZ0 ZXIgYQpkaXNjdXNzaW9uIHdpdGggdGhlIFJUTCBlbmdpbmVlcnMsIGl0J3Mgbm90IHRvIGNvbnRy b2wgdGhlIHJlZmVyZW5jZQpjbG9jay4gU2V0dGluZyB0aGlzIHJlZ2lzdGVyIGZpZWxkIGRvZXMg bm90IGNvbnRyb2wgdGhlIHJlZmVyZW5jZSBjbG9jawpyYXRlLiBUaGUgY29udHJvbGxlciB1c2Vz IHRoaXMgdmFsdWUgdG8gcGVyZm9ybSBpbnRlcm5hbCB0aW1pbmcKY2FsY3VsYXRpb25zIHRoYXQg YXJlIGJhc2VkIG9uIHRoZSByZWZlcmVuY2UgY2xvY2suIFNvLCB3ZSB3aWxsIHN0aWxsCm5lZWQg dGhpcyBwcm9wZXJ0eSB0byBzZXQgdGhpcyB2YWx1ZS4gSSB3aWxsIHJlc2VuZCB0aGUgcGF0Y2gg c2VyaWVzCndpdGggc29tZSBjaGFuZ2VzIGFuZCBjb3JyZWN0IHRoZSBjb21taXQgbWVzc2FnZXMg d2l0aCB0aGUgcHJvcGVyCmRlZmluaXRpb24gb2YgdGhpcyBmZWF0dXJlLgoKVGhhbmtzLApUaGlu aAo=