From mboxrd@z Thu Jan 1 00:00:00 1970 MIME-Version: 1.0 Date: Thu, 20 Dec 2018 09:19:28 -0600 References: <20181218164128.GA14552@bogus> <87sgysu1uc.fsf@linux.intel.com> In-Reply-To: <87sgysu1uc.fsf@linux.intel.com> Message-ID: Subject: Re: [PATCH v2 3/4] usb: dwc3: Add property snps,enable-refclk-sof From: Rob Herring Content-Type: text/plain; charset="UTF-8" To: Felipe Balbi Cc: Thinh Nguyen , Linux USB List , devicetree@vger.kernel.org, Mark Rutland , John Youn List-ID: On Thu, Dec 20, 2018 at 12:52 AM Felipe Balbi wrote: > > > Hi, > > Rob Herring writes: > > On Fri, Dec 07, 2018 at 06:27:43PM -0800, Thinh Nguyen wrote: > >> This patch adds a property to enable the controller to track the > >> frame number based on the reference clock. > >> > >> When operating in USB 2.0 mode, the peripheral controller uses the USB2 > >> PHY clocks to track the frame number. This prevents the controller from > >> suspending the USB2 PHY when the device goes into low power. Version > >> 1.80a of the DWC_usb31 peripheral controller introduces a way to track > >> frame number based on the reference clock instead. This feature allows > >> the controller to suspend the USB2 PHY when the device goes into low > >> power. This improves power saving for devices that have isochronous > >> endpoints. > >> > >> Signed-off-by: Thinh Nguyen > >> --- > >> Changes in v2: > >> - Revise property description > >> - Rename property from snps,enable-refclk-lpm to snps,enable-refclk-sof > >> > >> Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt > >> index b7e67edff9b2..01b948fff0eb 100644 > >> --- a/Documentation/devicetree/bindings/usb/dwc3.txt > >> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt > >> @@ -101,6 +101,9 @@ Optional properties: > >> enable periodic ESS TX threshold. > >> - snps,refclk-period-ns: if set, this value informs the controller of the > >> reference clock period in nanoseconds. > >> + - snps,enable-refclk-sof: set to enable reference clock based frame number > >> + tracking while in low power, allowing the controller to > >> + suspend the PHY during low power states. > > > > This should be implied by the compatible string. > > Two problems with this: > > 1) Won't work for PCI-based systems For PCI, you would decide this based on VID/PID, wouldn't you? Compatible is basically equivalent to that. > 2) If we start having many users of this we will end up with: > > if (of_device_is_compatible("a") || > of_device_is_compatible("b") || > of_device_is_compatible("c") || > of_device_is_compatible("d") || > ...) > foo(); > > Conversely, if we just pass a flag, this branch will never change. We > won't need changes to the kernel because a new platform needing refclk > based frame number tracking is, now, supported upstream. Things are implied based on compatible frequently and this is not how the code ends up looking like. Normally, match data would have the flag setting and that variable will get passed to whatever needs it. USB blocks and their integration quirks are complicated enough that I'm not really worried about needing to update the kernel for a new platform. If a new platform is so similar to an existing one, then a fallback compatible can be used and the kernel doesn't need a change. A property like this makes more sense when it is board-specific, not SoC specific. I may be wrong, but this looked more like a SoC integration decision than board level. Rob 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: [v2,3/4] usb: dwc3: Add property snps,enable-refclk-sof From: Rob Herring Message-Id: Date: Thu, 20 Dec 2018 09:19:28 -0600 To: Felipe Balbi Cc: Thinh Nguyen , Linux USB List , devicetree@vger.kernel.org, Mark Rutland , John Youn List-ID: T24gVGh1LCBEZWMgMjAsIDIwMTggYXQgMTI6NTIgQU0gRmVsaXBlIEJhbGJpIDxiYWxiaUBrZXJu ZWwub3JnPiB3cm90ZToKPgo+Cj4gSGksCj4KPiBSb2IgSGVycmluZyA8cm9iaEBrZXJuZWwub3Jn PiB3cml0ZXM6Cj4gPiBPbiBGcmksIERlYyAwNywgMjAxOCBhdCAwNjoyNzo0M1BNIC0wODAwLCBU aGluaCBOZ3V5ZW4gd3JvdGU6Cj4gPj4gVGhpcyBwYXRjaCBhZGRzIGEgcHJvcGVydHkgdG8gZW5h YmxlIHRoZSBjb250cm9sbGVyIHRvIHRyYWNrIHRoZQo+ID4+IGZyYW1lIG51bWJlciBiYXNlZCBv biB0aGUgcmVmZXJlbmNlIGNsb2NrLgo+ID4+Cj4gPj4gV2hlbiBvcGVyYXRpbmcgaW4gVVNCIDIu MCBtb2RlLCB0aGUgcGVyaXBoZXJhbCBjb250cm9sbGVyIHVzZXMgdGhlIFVTQjIKPiA+PiBQSFkg Y2xvY2tzIHRvIHRyYWNrIHRoZSBmcmFtZSBudW1iZXIuIFRoaXMgcHJldmVudHMgdGhlIGNvbnRy b2xsZXIgZnJvbQo+ID4+IHN1c3BlbmRpbmcgdGhlIFVTQjIgUEhZIHdoZW4gdGhlIGRldmljZSBn b2VzIGludG8gbG93IHBvd2VyLiBWZXJzaW9uCj4gPj4gMS44MGEgb2YgdGhlIERXQ191c2IzMSBw ZXJpcGhlcmFsIGNvbnRyb2xsZXIgaW50cm9kdWNlcyBhIHdheSB0byB0cmFjawo+ID4+IGZyYW1l IG51bWJlciBiYXNlZCBvbiB0aGUgcmVmZXJlbmNlIGNsb2NrIGluc3RlYWQuIFRoaXMgZmVhdHVy ZSBhbGxvd3MKPiA+PiB0aGUgY29udHJvbGxlciB0byBzdXNwZW5kIHRoZSBVU0IyIFBIWSB3aGVu IHRoZSBkZXZpY2UgZ29lcyBpbnRvIGxvdwo+ID4+IHBvd2VyLiBUaGlzIGltcHJvdmVzIHBvd2Vy IHNhdmluZyBmb3IgZGV2aWNlcyB0aGF0IGhhdmUgaXNvY2hyb25vdXMKPiA+PiBlbmRwb2ludHMu Cj4gPj4KPiA+PiBTaWduZWQtb2ZmLWJ5OiBUaGluaCBOZ3V5ZW4gPHRoaW5obkBzeW5vcHN5cy5j b20+Cj4gPj4gLS0tCj4gPj4gQ2hhbmdlcyBpbiB2MjoKPiA+PiAtIFJldmlzZSBwcm9wZXJ0eSBk ZXNjcmlwdGlvbgo+ID4+IC0gUmVuYW1lIHByb3BlcnR5IGZyb20gc25wcyxlbmFibGUtcmVmY2xr LWxwbSB0byBzbnBzLGVuYWJsZS1yZWZjbGstc29mCj4gPj4KPiA+PiAgRG9jdW1lbnRhdGlvbi9k ZXZpY2V0cmVlL2JpbmRpbmdzL3VzYi9kd2MzLnR4dCB8IDMgKysrCj4gPj4gIDEgZmlsZSBjaGFu Z2VkLCAzIGluc2VydGlvbnMoKykKPiA+Pgo+ID4+IGRpZmYgLS1naXQgYS9Eb2N1bWVudGF0aW9u L2RldmljZXRyZWUvYmluZGluZ3MvdXNiL2R3YzMudHh0IGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0 cmVlL2JpbmRpbmdzL3VzYi9kd2MzLnR4dAo+ID4+IGluZGV4IGI3ZTY3ZWRmZjliMi4uMDFiOTQ4 ZmZmMGViIDEwMDY0NAo+ID4+IC0tLSBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5n cy91c2IvZHdjMy50eHQKPiA+PiArKysgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGlu Z3MvdXNiL2R3YzMudHh0Cj4gPj4gQEAgLTEwMSw2ICsxMDEsOSBAQCBPcHRpb25hbCBwcm9wZXJ0 aWVzOgo+ID4+ICAgICAgICAgICAgICAgICAgICAgIGVuYWJsZSBwZXJpb2RpYyBFU1MgVFggdGhy ZXNob2xkLgo+ID4+ICAgLSBzbnBzLHJlZmNsay1wZXJpb2QtbnM6IGlmIHNldCwgdGhpcyB2YWx1 ZSBpbmZvcm1zIHRoZSBjb250cm9sbGVyIG9mIHRoZQo+ID4+ICAgICAgICAgICAgICAgICAgICAg IHJlZmVyZW5jZSBjbG9jayBwZXJpb2QgaW4gbmFub3NlY29uZHMuCj4gPj4gKyAtIHNucHMsZW5h YmxlLXJlZmNsay1zb2Y6IHNldCB0byBlbmFibGUgcmVmZXJlbmNlIGNsb2NrIGJhc2VkIGZyYW1l IG51bWJlcgo+ID4+ICsgICAgICAgICAgICAgICAgICAgIHRyYWNraW5nIHdoaWxlIGluIGxvdyBw b3dlciwgYWxsb3dpbmcgdGhlIGNvbnRyb2xsZXIgdG8KPiA+PiArICAgICAgICAgICAgICAgICAg ICBzdXNwZW5kIHRoZSBQSFkgZHVyaW5nIGxvdyBwb3dlciBzdGF0ZXMuCj4gPgo+ID4gVGhpcyBz aG91bGQgYmUgaW1wbGllZCBieSB0aGUgY29tcGF0aWJsZSBzdHJpbmcuCj4KPiBUd28gcHJvYmxl bXMgd2l0aCB0aGlzOgo+Cj4gMSkgV29uJ3Qgd29yayBmb3IgUENJLWJhc2VkIHN5c3RlbXMKCkZv ciBQQ0ksIHlvdSB3b3VsZCBkZWNpZGUgdGhpcyBiYXNlZCBvbiBWSUQvUElELCB3b3VsZG4ndCB5 b3U/CkNvbXBhdGlibGUgaXMgYmFzaWNhbGx5IGVxdWl2YWxlbnQgdG8gdGhhdC4KCj4gMikgSWYg d2Ugc3RhcnQgaGF2aW5nIG1hbnkgdXNlcnMgb2YgdGhpcyB3ZSB3aWxsIGVuZCB1cCB3aXRoOgo+ Cj4gICAgICAgICBpZiAob2ZfZGV2aWNlX2lzX2NvbXBhdGlibGUoImEiKSB8fAo+ICAgICAgICAg ICAgICAgICBvZl9kZXZpY2VfaXNfY29tcGF0aWJsZSgiYiIpIHx8Cj4gICAgICAgICAgICAgICAg IG9mX2RldmljZV9pc19jb21wYXRpYmxlKCJjIikgfHwKPiAgICAgICAgICAgICAgICAgb2ZfZGV2 aWNlX2lzX2NvbXBhdGlibGUoImQiKSB8fAo+ICAgICAgICAgICAgICAgICAuLi4pCj4gICAgICAg ICAgICAgICAgIGZvbygpOwo+Cj4gQ29udmVyc2VseSwgaWYgd2UganVzdCBwYXNzIGEgZmxhZywg dGhpcyBicmFuY2ggd2lsbCBuZXZlciBjaGFuZ2UuIFdlCj4gd29uJ3QgbmVlZCBjaGFuZ2VzIHRv IHRoZSBrZXJuZWwgYmVjYXVzZSBhIG5ldyBwbGF0Zm9ybSBuZWVkaW5nIHJlZmNsawo+IGJhc2Vk IGZyYW1lIG51bWJlciB0cmFja2luZyBpcywgbm93LCBzdXBwb3J0ZWQgdXBzdHJlYW0uCgpUaGlu Z3MgYXJlIGltcGxpZWQgYmFzZWQgb24gY29tcGF0aWJsZSBmcmVxdWVudGx5IGFuZCB0aGlzIGlz IG5vdCBob3cKdGhlIGNvZGUgZW5kcyB1cCBsb29raW5nIGxpa2UuIE5vcm1hbGx5LCBtYXRjaCBk YXRhIHdvdWxkIGhhdmUgdGhlCmZsYWcgc2V0dGluZyBhbmQgdGhhdCB2YXJpYWJsZSB3aWxsIGdl dCBwYXNzZWQgdG8gd2hhdGV2ZXIgbmVlZHMgaXQuCgpVU0IgYmxvY2tzIGFuZCB0aGVpciBpbnRl Z3JhdGlvbiBxdWlya3MgYXJlIGNvbXBsaWNhdGVkIGVub3VnaCB0aGF0CkknbSBub3QgcmVhbGx5 IHdvcnJpZWQgYWJvdXQgbmVlZGluZyB0byB1cGRhdGUgdGhlIGtlcm5lbCBmb3IgYSBuZXcKcGxh dGZvcm0uIElmIGEgbmV3IHBsYXRmb3JtIGlzIHNvIHNpbWlsYXIgdG8gYW4gZXhpc3Rpbmcgb25l LCB0aGVuIGEKZmFsbGJhY2sgY29tcGF0aWJsZSBjYW4gYmUgdXNlZCBhbmQgdGhlIGtlcm5lbCBk b2Vzbid0IG5lZWQgYSBjaGFuZ2UuCgpBIHByb3BlcnR5IGxpa2UgdGhpcyBtYWtlcyBtb3JlIHNl bnNlIHdoZW4gaXQgaXMgYm9hcmQtc3BlY2lmaWMsIG5vdApTb0Mgc3BlY2lmaWMuIEkgbWF5IGJl IHdyb25nLCBidXQgdGhpcyBsb29rZWQgbW9yZSBsaWtlIGEgU29DCmludGVncmF0aW9uIGRlY2lz aW9uIHRoYW4gYm9hcmQgbGV2ZWwuCgpSb2IK