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,05/14] usb: typec: add API to get typec basic port power and data config From: Heikki Krogerus Message-Id: <20180517123506.GF11469@kuha.fi.intel.com> Date: Thu, 17 May 2018 15:35:06 +0300 To: Mats Karrman Cc: Jun Li , "robh+dt@kernel.org" , "gregkh@linuxfoundation.org" , "linux@roeck-us.net" , "a.hajda@samsung.com" , "cw00.choi@samsung.com" , "shufan_lee@richtek.com" , Peter Chen , "gsomlo@gmail.com" , "devicetree@vger.kernel.org" , "linux-usb@vger.kernel.org" , dl-linux-imx List-ID: T24gV2VkLCBNYXkgMTYsIDIwMTggYXQgMTA6NTU6MzZQTSArMDIwMCwgTWF0cyBLYXJybWFuIHdy b3RlOgo+IEhpLAo+IAo+IE9uIDA1LzE2LzIwMTggMDI6MjUgUE0sIEhlaWtraSBLcm9nZXJ1cyB3 cm90ZToKPiAKPiA+IEhpIGd1eXMsCj4gPgo+ID4gT24gVHVlLCBNYXkgMTUsIDIwMTggYXQgMTA6 NTI6NTdQTSArMDIwMCwgTWF0cyBLYXJybWFuIHdyb3RlOgo+ID4+IEhpLAo+ID4+Cj4gPj4gT24g MDUvMTQvMjAxOCAxMTozNiBBTSwgSnVuIExpIHdyb3RlOgo+ID4+Cj4gPj4+IEhpCj4gPj4+PiAt LS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQo+ID4+Pj4gRnJvbTogTWF0cyBLYXJybWFuIFttYWls dG86bWF0cy5kZXYubGlzdEBnbWFpbC5jb21dCj4gPj4+PiBTZW50OiAyMDE4Pz8/NT8/PzEyPz8/ IDM6NTYKPiA+Pj4+IFRvOiBKdW4gTGkgPGp1bi5saUBueHAuY29tPjsgcm9iaCtkdEBrZXJuZWwu b3JnOyBncmVna2hAbGludXhmb3VuZGF0aW9uLm9yZzsKPiA+Pj4+IGhlaWtraS5rcm9nZXJ1c0Bs aW51eC5pbnRlbC5jb207IGxpbnV4QHJvZWNrLXVzLm5ldAo+ID4+Pj4gQ2M6IGEuaGFqZGFAc2Ft c3VuZy5jb207IGN3MDAuY2hvaUBzYW1zdW5nLmNvbTsKPiA+Pj4+IHNodWZhbl9sZWVAcmljaHRl ay5jb207IFBldGVyIENoZW4gPHBldGVyLmNoZW5AbnhwLmNvbT47Cj4gPj4+PiBnc29tbG9AZ21h aWwuY29tOyBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgbGludXgtdXNiQHZnZXIua2VybmVs Lm9yZzsKPiA+Pj4+IGRsLWxpbnV4LWlteCA8bGludXgtaW14QG54cC5jb20+Cj4gPj4+PiBTdWJq ZWN0OiBSZTogW1BBVENIIHY1IDA1LzE0XSB1c2I6IHR5cGVjOiBhZGQgQVBJIHRvIGdldCB0eXBl YyBiYXNpYyBwb3J0IHBvd2VyCj4gPj4+PiBhbmQgZGF0YSBjb25maWcKPiA+Pj4+Cj4gPj4+PiBI aSBMaSBKdW4sCj4gPj4+Pgo+ID4+Pj4gT24gMjAxOC0wNS0wMyAwMjoyNCwgTGkgSnVuIHdyb3Rl Ogo+ID4+Pj4KPiA+Pj4+PiBUaGlzIHBhdGNoIGFkZHMgMyBBUElzIHRvIGdldCB0aGUgdHlwZWMg cG9ydCBwb3dlciBhbmQgZGF0YSB0eXBlLCBhbmQKPiA+Pj4+PiBwcmVmZXJyZWQgcG93ZXIgcm9s ZSBieSBpdHMgbmFtZSBzdHJpbmcuCj4gPj4+Pj4KPiA+Pj4+PiBTaWduZWQtb2ZmLWJ5OiBMaSBK dW4gPGp1bi5saUBueHAuY29tPgo+ID4+Pj4+IC0tLQo+ID4+Pj4+ICAgZHJpdmVycy91c2IvdHlw ZWMvY2xhc3MuYyB8IDUyCj4gPj4+PiArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysrKwo+ID4+Pj4+ICAgaW5jbHVkZS9saW51eC91c2IvdHlwZWMuaCB8ICAzICsr Kwo+ID4+Pj4+ICAgMiBmaWxlcyBjaGFuZ2VkLCA1NSBpbnNlcnRpb25zKCspCj4gPj4+Pj4KPiA+ Pj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy91c2IvdHlwZWMvY2xhc3MuYyBiL2RyaXZlcnMvdXNi L3R5cGVjL2NsYXNzLmMKPiA+Pj4+PiBpbmRleCA1M2RmMTBkLi41OTgxZTE4IDEwMDY0NAo+ID4+ Pj4+IC0tLSBhL2RyaXZlcnMvdXNiL3R5cGVjL2NsYXNzLmMKPiA+Pj4+PiArKysgYi9kcml2ZXJz L3VzYi90eXBlYy9jbGFzcy5jCj4gPj4+Pj4gQEAgLTksNiArOSw3IEBACj4gPj4+Pj4gICAjaW5j bHVkZSA8bGludXgvZGV2aWNlLmg+Cj4gPj4+Pj4gICAjaW5jbHVkZSA8bGludXgvbW9kdWxlLmg+ Cj4gPj4+Pj4gICAjaW5jbHVkZSA8bGludXgvbXV0ZXguaD4KPiA+Pj4+PiArI2luY2x1ZGUgPGxp bnV4L3Byb3BlcnR5Lmg+Cj4gPiBJIGRvbid0IHRoaW5rIHlvdSBuZWVkIHRoYXQgYW55bW9yZS4K PiA+Cj4gPj4+Pj4gICAjaW5jbHVkZSA8bGludXgvc2xhYi5oPgo+ID4+Pj4+ICAgI2luY2x1ZGUg PGxpbnV4L3VzYi90eXBlYy5oPgo+ID4+Pj4+ICAgI2luY2x1ZGUgPGxpbnV4L3VzYi90eXBlY19t dXguaD4KPiA+Pj4+PiBAQCAtODAyLDYgKzgwMywxMiBAQCBzdGF0aWMgY29uc3QgY2hhciAqIGNv bnN0IHR5cGVjX3BvcnRfdHlwZXNbXSA9IHsKPiA+Pj4+PiAgIAlbVFlQRUNfUE9SVF9EUlBdID0g ImR1YWwiLAo+ID4+Pj4+ICAgfTsKPiA+Pj4+Pgo+ID4+Pj4+ICtzdGF0aWMgY29uc3QgY2hhciAq IGNvbnN0IHR5cGVjX2RhdGFfdHlwZXNbXSA9IHsKPiA+Pj4+PiArCVtUWVBFQ19QT1JUX0RGUF0g PSAiaG9zdCIsCj4gPj4+Pj4gKwlbVFlQRUNfUE9SVF9VRlBdID0gImRldmljZSIsCj4gPj4+Pj4g KwlbVFlQRUNfUE9SVF9EUkRdID0gImR1YWwiLAo+ID4+Pj4+ICt9Owo+ID4+Pj4+ICsKPiA+Pj4+ PiAgIHN0YXRpYyBjb25zdCBjaGFyICogY29uc3QgdHlwZWNfcG9ydF90eXBlc19kcnBbXSA9IHsK PiA+Pj4+PiAgIAlbVFlQRUNfUE9SVF9TUkNdID0gImR1YWwgW3NvdXJjZV0gc2luayIsCj4gPj4+ Pj4gICAJW1RZUEVDX1BPUlRfU05LXSA9ICJkdWFsIHNvdXJjZSBbc2lua10iLCBAQCAtMTI1Miw2 ICsxMjU5LDUxCj4gPj4+PiBAQAo+ID4+Pj4+IHZvaWQgdHlwZWNfc2V0X3B3cl9vcG1vZGUoc3Ry dWN0IHR5cGVjX3BvcnQgKnBvcnQsCj4gPj4+Pj4gICB9Cj4gPj4+Pj4gICBFWFBPUlRfU1lNQk9M X0dQTCh0eXBlY19zZXRfcHdyX29wbW9kZSk7Cj4gPj4+Pj4KPiA+Pj4+PiArLyoqCj4gPj4+Pj4g KyAqIHR5cGVjX2ZpbmRfcG93ZXJfdHlwZSAtIEdldCB0aGUgdHlwZWMgcG9ydCBwb3dlciB0eXBl Cj4gPj4+PiBXaHkgaXMgdGhpcyBmdW5jdGlvbiBjYWxsZWQgdHlwZWNfZmluZF9wb3dlcl90eXBl KCkgYW5kIG5vdAo+ID4+Pj4gdHlwZWNfZmluZF9wb3J0X3R5cGUoKT8KPiA+Pj4+IEl0J3MgY2Fs bGVkIHBvcnRfdHlwZSBpbiBzeXNmcywgaGF2aW5nIGRpZmZlcmVudCBuYW1lcyBqdXN0IGFkZHMg Y29uZnVzaW9uLgo+ID4+Pj4gKE90aGVyd2lzZSBJIGFncmVlIHBvd2VyX3R5cGUgaXMgYSBiZXR0 ZXIgbmFtZSBidXQuLi4pCj4gPj4+IFdlIGhhdmUgInBvcnQgdHlwZSIgYmVmb3JlIHRoZSBwb3dl ciBhbmQgZGF0YSByb2xlIHNlcGFyYXRpb24sCj4gPj4+IHRoaXMgQVBJIG5hbWUncyBpbnRlbnRp b24gaXMgdG8gcmVmbGVjdCB0aGUgcG93ZXIgY2FwLCBhbnl3YXkgSQo+ID4+PiBsZWF2ZSB0aGlz IHRvIGJlIGRlY2lkZWQgYnkgSGVpa2tpIHRoZW4uCj4gPiBJIHJlYWxseSBoYXRlIHRoZSAiKl90 eXBlIiBuYW1pbmcuIEl0IHdhcyB1bmRlcnN0YW5kYWJsZSB3aGVuIHRoZXJlCj4gPiB3YXMgbm8g c2VwYXJhdGUgcG93ZXIgYW5kIGRhdGEgcm9sZXMgZGVmaW5lZCBpbiB0aGUgc3BlY2lmaWNhdGlv biwgYnV0Cj4gPiBub3cgdGhhdCB0aGVyZSBhcmUsIGl0J3MganVzdCBjb25mdXNpbmcuIElNTyB3 ZSBzaG91bGQgbm90IHVzZSBpdAo+ID4gYW55d2hlcmUuCj4gPgo+ID4gU28gdG8gbWUgdHlwZWNf ZmluZF90eXBlKCkgaXMganVzdCBhcyBiYWQgYXMgdHlwZWNfZmluZF9wb3dlcl90eXBlKCkKPiA+ IGJlY2F1c2UgaXQgaGFzIHRoZSAidHlwZSIgaW4gaXQuIEkgd29uZGVyIGlmIHRoaXMgZnVuY3Rp b24gaXMKPiA+IG5lY2Vzc2FyeSBhdCBhbGw/IElmIGl0IGlzLCB0aGVuIHBlcmhhcHMgd2UgY2Fu IHRoaW5rIG9mIHNvbWUgYmV0dGVyCj4gPiBuYW1lIGZvciBpdCwgbmFtZSB0aGF0IGdpdmVzIGEg YmV0dGVyIGhpbnQgd2hhdCBpdCBpcyB1c2VkIGZvci4KPiAKPiBJIHJlcmVhZCB0aGlzIHBhdGNo IGFuZCB0cmllZCB0byBzZWUgaXQgbW9yZSBpbiB0aGUgY29udGV4dCBvZiB0aGUgb3RoZXIKPiBw YXRjaGVzIGFuZCB0aGUgZXhpc3RpbmcgY29kZS4gVGhlIG5hbWluZyBvZiB0aGUgZXhpc3Rpbmcg c3RyaW5nIHRhYmxlcwo+IGRvZXNuJ3QgaGVscCBpbiBnZXR0aW5nIHRoaXMgcmlnaHQsIGhvd2V2 ZXIgSSBoYXZlIGEgcHJvcG9zYWw6Cj4gCj4gdHlwZWNfZmluZF9wb3J0X3Bvd2VyX3JvbGUoKSB0 byBnZXQgdG8gVFlQRUNfUE9SVF9TUkMvU05LL0RSUAo+IHR5cGVjX2ZpbmRfcG9ydF9kYXRhX3Jv bGUoKSB0byBnZXQgdG8gVFlQRUNfUE9SVF9ERlAvVUZQL0RSRAo+IHR5cGVjX2ZpbmRfcG93ZXJf cm9sZSgpIHRvIGdldCB0byBUWVBFQ19TSU5LL1NPVVJDRQo+IAo+IGFuZCBzb21ldGltZSwgaWYg dGhlIHVzZSBzaG91bGQgYXJpc2UgCj4gCj4gdHlwZWNfZmluZF9kYXRhX3JvbGUoKSB0byBnZXQg dG8gVFlQRUNfREVWSUNFL0hPU1QKPiAKPiBJIHRoaW5rIGl0IGlzIGZhaXJseSBjb21wcmVoZW5z aWJsZSwgKl9wb3J0XyogY29uY2VybnMgYSBjYXBhYmlsaXR5IGFuZAo+IHdpdGhvdXQgKl9wb3J0 XyogaXQgaXMgYW4gYWN0dWFsIHN0YXRlLiBQbHVzIGl0IG1hdGNoZXMgdGhlIG5hbWVzIG9mIHRo ZQo+IGNvbnN0YW50cy4KCldlbGwsIHRoZXJlIGFyZSBub3cgZm91ciB0aGluZ3MgdG8gY29uc2lk ZXI6CgoxKSB0aGUgcm9sZXMgKHBvd2VyIGFuZCBkYXRhKSB0aGUgcG9ydCBpcyBjYXBhYmxlIG9m IHN1cHBvcnRpbmcKMikgVHJ5LlNSQyBhbmQgVHJ5LlNOSywgaS5lLiB0aGUgcHJlZmVycmVkIHJv bGUKMykgdGhlIGN1cnJlbnQgcm9sZXMKNCkgdGhlIHJvbGUocykgdGhlIHVzZXIgd2FudCdzIHRv IGxpbWl0IHRoZSB1c2Ugb2YgYSBwb3J0IHdpdGggRFJQIHBvcnRzCgpUaGUgbGFzdCBvbmUgSSBk b24ndCBrbm93IGlmIGl0J3MgcmVsZXZhbnQgd2l0aCB0aGVzZSBmdW5jdGlvbnMuIEl0J3MKbm90 IGluZm9ybWF0aW9uIHRoYXQgd2Ugd291bGQgZ2V0IGZvciBleGFtcGxlIGZyb20gZmlybXdhcmUu CgpJIGFsc28gZG9uJ3QgdGhpbmsgd2UgbmVlZCB0byB1c2UgdGhlc2UgZnVuY3Rpb25zIHdpdGgg dGhlIGN1cnJlbnQKcm9sZXMgdGhlIHBvcnQgaXMgaW4uCgpJZiB0aGUgcHJlZmVycmVkIHJvbGUg aXMgInNpbmsiIG9yICJzb3VyY2UiLCBzbyBqdXN0IGxpa2UgdGhlIHBvd2VyCnJvbGUsIHdlIGRv bid0IG5lZWQgc2VwYXJhdGUgZnVuY3Rpb24gZm9yIGl0IGhlcmUuCgpTbyBpc24ndCB0d28gZnVu Y3Rpb25zIGFsbCB3ZSBuZWVkIGhlcmU6IG9uZSBmb3IgdGhlIHBvd2VyIGFuZCBvbmUgZm9yCmRh dGEgcm9sZT8KCgpUaGFua3MsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 17 May 2018 15:35:06 +0300 From: Heikki Krogerus Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port power and data config Message-ID: <20180517123506.GF11469@kuha.fi.intel.com> References: <1525307094-27402-1-git-send-email-jun.li@nxp.com> <1525307094-27402-6-git-send-email-jun.li@nxp.com> <74e1c164-1652-75f4-409c-bd1c214bda4d@gmail.com> <20180516122527.GC11469@kuha.fi.intel.com> <33d02e76-bdf2-be50-68ec-9d520acdbe82@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <33d02e76-bdf2-be50-68ec-9d520acdbe82@gmail.com> To: Mats Karrman Cc: Jun Li , "robh+dt@kernel.org" , "gregkh@linuxfoundation.org" , "linux@roeck-us.net" , "a.hajda@samsung.com" , "cw00.choi@samsung.com" , "shufan_lee@richtek.com" , Peter Chen , "gsomlo@gmail.com" , "devicetree@vger.kernel.org" , "linux-usb@vger.kernel.org" , dl-linux-imx List-ID: On Wed, May 16, 2018 at 10:55:36PM +0200, Mats Karrman wrote: > Hi, > > On 05/16/2018 02:25 PM, Heikki Krogerus wrote: > > > Hi guys, > > > > On Tue, May 15, 2018 at 10:52:57PM +0200, Mats Karrman wrote: > >> Hi, > >> > >> On 05/14/2018 11:36 AM, Jun Li wrote: > >> > >>> Hi > >>>> -----Original Message----- > >>>> From: Mats Karrman [mailto:mats.dev.list@gmail.com] > >>>> Sent: 2018???5???12??? 3:56 > >>>> To: Jun Li ; robh+dt@kernel.org; gregkh@linuxfoundation.org; > >>>> heikki.krogerus@linux.intel.com; linux@roeck-us.net > >>>> Cc: a.hajda@samsung.com; cw00.choi@samsung.com; > >>>> shufan_lee@richtek.com; Peter Chen ; > >>>> gsomlo@gmail.com; devicetree@vger.kernel.org; linux-usb@vger.kernel.org; > >>>> dl-linux-imx > >>>> Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port power > >>>> and data config > >>>> > >>>> Hi Li Jun, > >>>> > >>>> On 2018-05-03 02:24, Li Jun wrote: > >>>> > >>>>> This patch adds 3 APIs to get the typec port power and data type, and > >>>>> preferred power role by its name string. > >>>>> > >>>>> Signed-off-by: Li Jun > >>>>> --- > >>>>> drivers/usb/typec/class.c | 52 > >>>> +++++++++++++++++++++++++++++++++++++++++++++++ > >>>>> include/linux/usb/typec.h | 3 +++ > >>>>> 2 files changed, 55 insertions(+) > >>>>> > >>>>> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > >>>>> index 53df10d..5981e18 100644 > >>>>> --- a/drivers/usb/typec/class.c > >>>>> +++ b/drivers/usb/typec/class.c > >>>>> @@ -9,6 +9,7 @@ > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> +#include > > I don't think you need that anymore. > > > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> @@ -802,6 +803,12 @@ static const char * const typec_port_types[] = { > >>>>> [TYPEC_PORT_DRP] = "dual", > >>>>> }; > >>>>> > >>>>> +static const char * const typec_data_types[] = { > >>>>> + [TYPEC_PORT_DFP] = "host", > >>>>> + [TYPEC_PORT_UFP] = "device", > >>>>> + [TYPEC_PORT_DRD] = "dual", > >>>>> +}; > >>>>> + > >>>>> static const char * const typec_port_types_drp[] = { > >>>>> [TYPEC_PORT_SRC] = "dual [source] sink", > >>>>> [TYPEC_PORT_SNK] = "dual source [sink]", @@ -1252,6 +1259,51 > >>>> @@ > >>>>> void typec_set_pwr_opmode(struct typec_port *port, > >>>>> } > >>>>> EXPORT_SYMBOL_GPL(typec_set_pwr_opmode); > >>>>> > >>>>> +/** > >>>>> + * typec_find_power_type - Get the typec port power type > >>>> Why is this function called typec_find_power_type() and not > >>>> typec_find_port_type()? > >>>> It's called port_type in sysfs, having different names just adds confusion. > >>>> (Otherwise I agree power_type is a better name but...) > >>> We have "port type" before the power and data role separation, > >>> this API name's intention is to reflect the power cap, anyway I > >>> leave this to be decided by Heikki then. > > I really hate the "*_type" naming. It was understandable when there > > was no separate power and data roles defined in the specification, but > > now that there are, it's just confusing. IMO we should not use it > > anywhere. > > > > So to me typec_find_type() is just as bad as typec_find_power_type() > > because it has the "type" in it. I wonder if this function is > > necessary at all? If it is, then perhaps we can think of some better > > name for it, name that gives a better hint what it is used for. > > I reread this patch and tried to see it more in the context of the other > patches and the existing code. The naming of the existing string tables > doesn't help in getting this right, however I have a proposal: > > typec_find_port_power_role() to get to TYPEC_PORT_SRC/SNK/DRP > typec_find_port_data_role() to get to TYPEC_PORT_DFP/UFP/DRD > typec_find_power_role() to get to TYPEC_SINK/SOURCE > > and sometime, if the use should arise > > typec_find_data_role() to get to TYPEC_DEVICE/HOST > > I think it is fairly comprehensible, *_port_* concerns a capability and > without *_port_* it is an actual state. Plus it matches the names of the > constants. Well, there are now four things to consider: 1) the roles (power and data) the port is capable of supporting 2) Try.SRC and Try.SNK, i.e. the preferred role 3) the current roles 4) the role(s) the user want's to limit the use of a port with DRP ports The last one I don't know if it's relevant with these functions. It's not information that we would get for example from firmware. I also don't think we need to use these functions with the current roles the port is in. If the preferred role is "sink" or "source", so just like the power role, we don't need separate function for it here. So isn't two functions all we need here: one for the power and one for data role? Thanks, -- heikki