From mboxrd@z Thu Jan 1 00:00:00 1970 From: Doug Anderson Subject: Re: [PATCH v5 6/7] dt-bindings: phy-qcom-usb2: Add support to override tuning values Date: Mon, 7 May 2018 08:57:26 -0700 Message-ID: References: <1525295174-15995-1-git-send-email-mgautam@codeaurora.org> <1525295174-15995-7-git-send-email-mgautam@codeaurora.org> <20180507155313.GA9696@rob-hp-laptop> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <20180507155313.GA9696@rob-hp-laptop> Sender: linux-kernel-owner@vger.kernel.org To: Rob Herring Cc: Manu Gautam , Kishon Vijay Abraham I , Stephen Boyd , devicetree@vger.kernel.org, LKML , Evan Green , Vivek Gautam , linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org Rob, On Mon, May 7, 2018 at 8:53 AM, Rob Herring wrote: > On Thu, May 03, 2018 at 02:36:13AM +0530, Manu Gautam wrote: >> To improve eye diagram for PHYs on different boards of same SOC, >> some parameters may need to be changed. Provide device tree >> properties to override these from board specific device tree >> files. While at it, replace "qcom,qusb2-v2-phy" with compatible >> string for USB2 PHY on sdm845 which was earlier added for >> sdm845 only. >> >> Signed-off-by: Manu Gautam >> --- >> .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 23 +++++++++++++- >> include/dt-bindings/phy/phy-qcom-qusb2.h | 37 ++++++++++++++++++++++ >> 2 files changed, 59 insertions(+), 1 deletion(-) >> create mode 100644 include/dt-bindings/phy/phy-qcom-qusb2.h >> >> diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt >> index 42c9742..03025d9 100644 >> --- a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt >> +++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt >> @@ -6,7 +6,7 @@ QUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets. >> Required properties: >> - compatible: compatible list, contains >> "qcom,msm8996-qusb2-phy" for 14nm PHY on msm8996, >> - "qcom,qusb2-v2-phy" for QUSB2 V2 PHY. >> + "qcom,sdm845-qusb2-phy" for 10nm PHY on sdm845. >> >> - reg: offset and length of the PHY register set. >> - #phy-cells: must be 0. >> @@ -27,6 +27,27 @@ Optional properties: >> tuning parameter value for qusb2 phy. >> >> - qcom,tcsr-syscon: Phandle to TCSR syscon register region. >> + - qcom,imp-res-offset-value: It is a 6 bit value that specifies offset to be >> + added to PHY refgen RESCODE via IMP_CTRL1 register. It is a PHY >> + tuning parameter that may vary for different boards of same SOC. >> + This property is applicable to only QUSB2 v2 PHY (sdm845). >> + - qcom,hstx-trim-value: It is a 4 bit value that specifies tuning for HSTX >> + output current. >> + Possible range is - 15mA to 24mA (stepsize of 600 uA). >> + See dt-bindings/phy/phy-qcom-qusb2.h for applicable values. >> + This property is applicable to only QUSB2 v2 PHY (sdm845). >> + Default value is 22.2mA for sdm845. >> + - qcom,preemphasis-level: It is a 2 bit value that specifies pre-emphasis level. >> + Possible range is 0 to 15% (stepsize of 5%). >> + See dt-bindings/phy/phy-qcom-qusb2.h for applicable values. >> + This property is applicable to only QUSB2 v2 PHY (sdm845). >> + Default value is 10% for sdm845. >> +- qcom,preemphasis-width: It is a 1 bit value that specifies how long the HSTX >> + pre-emphasis (specified using qcom,preemphasis-level) must be in >> + effect. Duration could be half-bit of full-bit. > > s/of/or/ > > But I'd just make this a boolean instead: qcom,preemphasis-half-bit I had this same comment in the post of v4. See . Specifically, I said: > Perhaps just make this a boolean property. If it exists then you get > the non-default case. AKA: if the default is full bit width, then > you'd allow a boolean property "qcom,preemphasis-half-width" to > override. If the default is half bit width then you'd allow > "qcom,preemphasis-full-width" to override. Manu replied: > Default property value for an SOC is specified in driver and could vary from > soc to soc. Hence, from board devicetree for different SOCs we might need > to select separate widths overriding default driver values. > Alternative is to have two bool properties each for half and full-width. Did > you actually mean that? IMHO given Manu's argument it seems fine to specify it the way he did. Please advise if you agree or disagree. -Doug 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: [v5,6/7] dt-bindings: phy-qcom-usb2: Add support to override tuning values From: Doug Anderson Message-Id: Date: Mon, 7 May 2018 08:57:26 -0700 To: Rob Herring Cc: Manu Gautam , Kishon Vijay Abraham I , Stephen Boyd , devicetree@vger.kernel.org, LKML , Evan Green , Vivek Gautam , linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org List-ID: Um9iLAoKT24gTW9uLCBNYXkgNywgMjAxOCBhdCA4OjUzIEFNLCBSb2IgSGVycmluZyA8cm9iaEBr ZXJuZWwub3JnPiB3cm90ZToKPiBPbiBUaHUsIE1heSAwMywgMjAxOCBhdCAwMjozNjoxM0FNICsw NTMwLCBNYW51IEdhdXRhbSB3cm90ZToKPj4gVG8gaW1wcm92ZSBleWUgZGlhZ3JhbSBmb3IgUEhZ cyBvbiBkaWZmZXJlbnQgYm9hcmRzIG9mIHNhbWUgU09DLAo+PiBzb21lIHBhcmFtZXRlcnMgbWF5 IG5lZWQgdG8gYmUgY2hhbmdlZC4gUHJvdmlkZSBkZXZpY2UgdHJlZQo+PiBwcm9wZXJ0aWVzIHRv IG92ZXJyaWRlIHRoZXNlIGZyb20gYm9hcmQgc3BlY2lmaWMgZGV2aWNlIHRyZWUKPj4gZmlsZXMu IFdoaWxlIGF0IGl0LCByZXBsYWNlICJxY29tLHF1c2IyLXYyLXBoeSIgd2l0aCBjb21wYXRpYmxl Cj4+IHN0cmluZyBmb3IgVVNCMiBQSFkgb24gc2RtODQ1IHdoaWNoIHdhcyBlYXJsaWVyIGFkZGVk IGZvcgo+PiBzZG04NDUgb25seS4KPj4KPj4gU2lnbmVkLW9mZi1ieTogTWFudSBHYXV0YW0gPG1n YXV0YW1AY29kZWF1cm9yYS5vcmc+Cj4+IC0tLQo+PiAgLi4uL2RldmljZXRyZWUvYmluZGluZ3Mv cGh5L3Fjb20tcXVzYjItcGh5LnR4dCAgICAgfCAyMyArKysrKysrKysrKysrLQo+PiAgaW5jbHVk ZS9kdC1iaW5kaW5ncy9waHkvcGh5LXFjb20tcXVzYjIuaCAgICAgICAgICAgfCAzNyArKysrKysr KysrKysrKysrKysrKysrCj4+ICAyIGZpbGVzIGNoYW5nZWQsIDU5IGluc2VydGlvbnMoKyksIDEg ZGVsZXRpb24oLSkKPj4gIGNyZWF0ZSBtb2RlIDEwMDY0NCBpbmNsdWRlL2R0LWJpbmRpbmdzL3Bo eS9waHktcWNvbS1xdXNiMi5oCj4+Cj4+IGRpZmYgLS1naXQgYS9Eb2N1bWVudGF0aW9uL2Rldmlj ZXRyZWUvYmluZGluZ3MvcGh5L3Fjb20tcXVzYjItcGh5LnR4dCBiL0RvY3VtZW50YXRpb24vZGV2 aWNldHJlZS9iaW5kaW5ncy9waHkvcWNvbS1xdXNiMi1waHkudHh0Cj4+IGluZGV4IDQyYzk3NDIu LjAzMDI1ZDkgMTAwNjQ0Cj4+IC0tLSBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5n cy9waHkvcWNvbS1xdXNiMi1waHkudHh0Cj4+ICsrKyBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJl ZS9iaW5kaW5ncy9waHkvcWNvbS1xdXNiMi1waHkudHh0Cj4+IEBAIC02LDcgKzYsNyBAQCBRVVNC MiBjb250cm9sbGVyIHN1cHBvcnRzIExTL0ZTL0hTIHVzYiBjb25uZWN0aXZpdHkgb24gUXVhbGNv bW0gY2hpcHNldHMuCj4+ICBSZXF1aXJlZCBwcm9wZXJ0aWVzOgo+PiAgIC0gY29tcGF0aWJsZTog Y29tcGF0aWJsZSBsaXN0LCBjb250YWlucwo+PiAgICAgICAgICAgICAgInFjb20sbXNtODk5Ni1x dXNiMi1waHkiIGZvciAxNG5tIFBIWSBvbiBtc204OTk2LAo+PiAtICAgICAgICAgICAgInFjb20s cXVzYjItdjItcGh5IiBmb3IgUVVTQjIgVjIgUEhZLgo+PiArICAgICAgICAgICAgInFjb20sc2Rt ODQ1LXF1c2IyLXBoeSIgZm9yIDEwbm0gUEhZIG9uIHNkbTg0NS4KPj4KPj4gICAtIHJlZzogb2Zm c2V0IGFuZCBsZW5ndGggb2YgdGhlIFBIWSByZWdpc3RlciBzZXQuCj4+ICAgLSAjcGh5LWNlbGxz OiBtdXN0IGJlIDAuCj4+IEBAIC0yNyw2ICsyNywyNyBAQCBPcHRpb25hbCBwcm9wZXJ0aWVzOgo+ PiAgICAgICAgICAgICAgIHR1bmluZyBwYXJhbWV0ZXIgdmFsdWUgZm9yIHF1c2IyIHBoeS4KPj4K Pj4gICAtIHFjb20sdGNzci1zeXNjb246IFBoYW5kbGUgdG8gVENTUiBzeXNjb24gcmVnaXN0ZXIg cmVnaW9uLgo+PiArIC0gcWNvbSxpbXAtcmVzLW9mZnNldC12YWx1ZTogSXQgaXMgYSA2IGJpdCB2 YWx1ZSB0aGF0IHNwZWNpZmllcyBvZmZzZXQgdG8gYmUKPj4gKyAgICAgICAgICAgICBhZGRlZCB0 byBQSFkgcmVmZ2VuIFJFU0NPREUgdmlhIElNUF9DVFJMMSByZWdpc3Rlci4gSXQgaXMgYSBQSFkK Pj4gKyAgICAgICAgICAgICB0dW5pbmcgcGFyYW1ldGVyIHRoYXQgbWF5IHZhcnkgZm9yIGRpZmZl cmVudCBib2FyZHMgb2Ygc2FtZSBTT0MuCj4+ICsgICAgICAgICAgICAgVGhpcyBwcm9wZXJ0eSBp cyBhcHBsaWNhYmxlIHRvIG9ubHkgUVVTQjIgdjIgUEhZIChzZG04NDUpLgo+PiArIC0gcWNvbSxo c3R4LXRyaW0tdmFsdWU6IEl0IGlzIGEgNCBiaXQgdmFsdWUgdGhhdCBzcGVjaWZpZXMgdHVuaW5n IGZvciBIU1RYCj4+ICsgICAgICAgICAgICAgb3V0cHV0IGN1cnJlbnQuCj4+ICsgICAgICAgICAg ICAgUG9zc2libGUgcmFuZ2UgaXMgLSAxNW1BIHRvIDI0bUEgKHN0ZXBzaXplIG9mIDYwMCB1QSku Cj4+ICsgICAgICAgICAgICAgU2VlIGR0LWJpbmRpbmdzL3BoeS9waHktcWNvbS1xdXNiMi5oIGZv ciBhcHBsaWNhYmxlIHZhbHVlcy4KPj4gKyAgICAgICAgICAgICBUaGlzIHByb3BlcnR5IGlzIGFw cGxpY2FibGUgdG8gb25seSBRVVNCMiB2MiBQSFkgKHNkbTg0NSkuCj4+ICsgICAgICAgICAgICAg RGVmYXVsdCB2YWx1ZSBpcyAyMi4ybUEgZm9yIHNkbTg0NS4KPj4gKyAtIHFjb20scHJlZW1waGFz aXMtbGV2ZWw6IEl0IGlzIGEgMiBiaXQgdmFsdWUgdGhhdCBzcGVjaWZpZXMgcHJlLWVtcGhhc2lz IGxldmVsLgo+PiArICAgICAgICAgICAgIFBvc3NpYmxlIHJhbmdlIGlzIDAgdG8gMTUlIChzdGVw c2l6ZSBvZiA1JSkuCj4+ICsgICAgICAgICAgICAgU2VlIGR0LWJpbmRpbmdzL3BoeS9waHktcWNv bS1xdXNiMi5oIGZvciBhcHBsaWNhYmxlIHZhbHVlcy4KPj4gKyAgICAgICAgICAgICBUaGlzIHBy b3BlcnR5IGlzIGFwcGxpY2FibGUgdG8gb25seSBRVVNCMiB2MiBQSFkgKHNkbTg0NSkuCj4+ICsg ICAgICAgICAgICAgRGVmYXVsdCB2YWx1ZSBpcyAxMCUgZm9yIHNkbTg0NS4KPj4gKy0gcWNvbSxw cmVlbXBoYXNpcy13aWR0aDogSXQgaXMgYSAxIGJpdCB2YWx1ZSB0aGF0IHNwZWNpZmllcyBob3cg bG9uZyB0aGUgSFNUWAo+PiArICAgICAgICAgICAgIHByZS1lbXBoYXNpcyAoc3BlY2lmaWVkIHVz aW5nIHFjb20scHJlZW1waGFzaXMtbGV2ZWwpIG11c3QgYmUgaW4KPj4gKyAgICAgICAgICAgICBl ZmZlY3QuIER1cmF0aW9uIGNvdWxkIGJlIGhhbGYtYml0IG9mIGZ1bGwtYml0Lgo+Cj4gcy9vZi9v ci8KPgo+IEJ1dCBJJ2QganVzdCBtYWtlIHRoaXMgYSBib29sZWFuIGluc3RlYWQ6IHFjb20scHJl ZW1waGFzaXMtaGFsZi1iaXQKCkkgaGFkIHRoaXMgc2FtZSBjb21tZW50IGluIHRoZSBwb3N0IG9m IHY0LiAgU2VlCjxodHRwczovL3BhdGNod29yay5rZXJuZWwub3JnL3BhdGNoLzEwMzE0OTIzLz4u ICBTcGVjaWZpY2FsbHksIEkgc2FpZDoKCj4gUGVyaGFwcyBqdXN0IG1ha2UgdGhpcyBhIGJvb2xl YW4gcHJvcGVydHkuICBJZiBpdCBleGlzdHMgdGhlbiB5b3UgZ2V0Cj4gdGhlIG5vbi1kZWZhdWx0 IGNhc2UuICBBS0E6IGlmIHRoZSBkZWZhdWx0IGlzIGZ1bGwgYml0IHdpZHRoLCB0aGVuCj4geW91 J2QgYWxsb3cgYSBib29sZWFuIHByb3BlcnR5ICJxY29tLHByZWVtcGhhc2lzLWhhbGYtd2lkdGgi IHRvCj4gb3ZlcnJpZGUuICBJZiB0aGUgZGVmYXVsdCBpcyBoYWxmIGJpdCB3aWR0aCB0aGVuIHlv dSdkIGFsbG93Cj4gInFjb20scHJlZW1waGFzaXMtZnVsbC13aWR0aCIgdG8gb3ZlcnJpZGUuCgpN YW51IHJlcGxpZWQ6Cgo+IERlZmF1bHQgcHJvcGVydHkgdmFsdWUgZm9yIGFuIFNPQyBpcyBzcGVj aWZpZWQgaW4gZHJpdmVyIGFuZCBjb3VsZCB2YXJ5IGZyb20KPiBzb2MgdG8gc29jLiBIZW5jZSwg ZnJvbSBib2FyZCBkZXZpY2V0cmVlIGZvciBkaWZmZXJlbnQgU09DcyB3ZSBtaWdodCBuZWVkCj4g dG8gc2VsZWN0IHNlcGFyYXRlIHdpZHRocyBvdmVycmlkaW5nIGRlZmF1bHQgZHJpdmVyIHZhbHVl cy4KPiBBbHRlcm5hdGl2ZSBpcyB0byBoYXZlIHR3byBib29sIHByb3BlcnRpZXMgZWFjaCBmb3Ig aGFsZiBhbmQgZnVsbC13aWR0aC4gRGlkCj4geW91IGFjdHVhbGx5IG1lYW4gdGhhdD8KCgpJTUhP IGdpdmVuIE1hbnUncyBhcmd1bWVudCBpdCBzZWVtcyBmaW5lIHRvIHNwZWNpZnkgaXQgdGhlIHdh eSBoZSBkaWQuClBsZWFzZSBhZHZpc2UgaWYgeW91IGFncmVlIG9yIGRpc2FncmVlLgoKLURvdWcK LS0tClRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNj cmliZSBsaW51eC11c2IiIGluCnRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdl ci5rZXJuZWwub3JnCk1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5v cmcvbWFqb3Jkb21vLWluZm8uaHRtbAo=