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: <20180516122527.GC11469@kuha.fi.intel.com> Date: Wed, 16 May 2018 15:25:27 +0300 To: Jun Li , Mats Karrman Cc: "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: SGkgZ3V5cywKCk9uIFR1ZSwgTWF5IDE1LCAyMDE4IGF0IDEwOjUyOjU3UE0gKzAyMDAsIE1hdHMg S2Fycm1hbiB3cm90ZToKPiBIaSwKPiAKPiBPbiAwNS8xNC8yMDE4IDExOjM2IEFNLCBKdW4gTGkg d3JvdGU6Cj4gCj4gPiBIaQo+ID4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tCj4gPj4gRnJv bTogTWF0cyBLYXJybWFuIFttYWlsdG86bWF0cy5kZXYubGlzdEBnbWFpbC5jb21dCj4gPj4gU2Vu dDogMjAxOD8/PzU/Pz8xMj8/PyAzOjU2Cj4gPj4gVG86IEp1biBMaSA8anVuLmxpQG54cC5jb20+ OyByb2JoK2R0QGtlcm5lbC5vcmc7IGdyZWdraEBsaW51eGZvdW5kYXRpb24ub3JnOwo+ID4+IGhl aWtraS5rcm9nZXJ1c0BsaW51eC5pbnRlbC5jb207IGxpbnV4QHJvZWNrLXVzLm5ldAo+ID4+IENj OiBhLmhhamRhQHNhbXN1bmcuY29tOyBjdzAwLmNob2lAc2Ftc3VuZy5jb207Cj4gPj4gc2h1ZmFu X2xlZUByaWNodGVrLmNvbTsgUGV0ZXIgQ2hlbiA8cGV0ZXIuY2hlbkBueHAuY29tPjsKPiA+PiBn c29tbG9AZ21haWwuY29tOyBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgbGludXgtdXNiQHZn ZXIua2VybmVsLm9yZzsKPiA+PiBkbC1saW51eC1pbXggPGxpbnV4LWlteEBueHAuY29tPgo+ID4+ IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjUgMDUvMTRdIHVzYjogdHlwZWM6IGFkZCBBUEkgdG8gZ2V0 IHR5cGVjIGJhc2ljIHBvcnQgcG93ZXIKPiA+PiBhbmQgZGF0YSBjb25maWcKPiA+Pgo+ID4+IEhp IExpIEp1biwKPiA+Pgo+ID4+IE9uIDIwMTgtMDUtMDMgMDI6MjQsIExpIEp1biB3cm90ZToKPiA+ Pgo+ID4+PiBUaGlzIHBhdGNoIGFkZHMgMyBBUElzIHRvIGdldCB0aGUgdHlwZWMgcG9ydCBwb3dl ciBhbmQgZGF0YSB0eXBlLCBhbmQKPiA+Pj4gcHJlZmVycmVkIHBvd2VyIHJvbGUgYnkgaXRzIG5h bWUgc3RyaW5nLgo+ID4+Pgo+ID4+PiBTaWduZWQtb2ZmLWJ5OiBMaSBKdW4gPGp1bi5saUBueHAu Y29tPgo+ID4+PiAtLS0KPiA+Pj4gICBkcml2ZXJzL3VzYi90eXBlYy9jbGFzcy5jIHwgNTIKPiA+ PiArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKwo+ID4+PiAg IGluY2x1ZGUvbGludXgvdXNiL3R5cGVjLmggfCAgMyArKysKPiA+Pj4gICAyIGZpbGVzIGNoYW5n ZWQsIDU1IGluc2VydGlvbnMoKykKPiA+Pj4KPiA+Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvdXNi L3R5cGVjL2NsYXNzLmMgYi9kcml2ZXJzL3VzYi90eXBlYy9jbGFzcy5jCj4gPj4+IGluZGV4IDUz ZGYxMGQuLjU5ODFlMTggMTAwNjQ0Cj4gPj4+IC0tLSBhL2RyaXZlcnMvdXNiL3R5cGVjL2NsYXNz LmMKPiA+Pj4gKysrIGIvZHJpdmVycy91c2IvdHlwZWMvY2xhc3MuYwo+ID4+PiBAQCAtOSw2ICs5 LDcgQEAKPiA+Pj4gICAjaW5jbHVkZSA8bGludXgvZGV2aWNlLmg+Cj4gPj4+ICAgI2luY2x1ZGUg PGxpbnV4L21vZHVsZS5oPgo+ID4+PiAgICNpbmNsdWRlIDxsaW51eC9tdXRleC5oPgo+ID4+PiAr I2luY2x1ZGUgPGxpbnV4L3Byb3BlcnR5Lmg+CgpJIGRvbid0IHRoaW5rIHlvdSBuZWVkIHRoYXQg YW55bW9yZS4KCj4gPj4+ICAgI2luY2x1ZGUgPGxpbnV4L3NsYWIuaD4KPiA+Pj4gICAjaW5jbHVk ZSA8bGludXgvdXNiL3R5cGVjLmg+Cj4gPj4+ICAgI2luY2x1ZGUgPGxpbnV4L3VzYi90eXBlY19t dXguaD4KPiA+Pj4gQEAgLTgwMiw2ICs4MDMsMTIgQEAgc3RhdGljIGNvbnN0IGNoYXIgKiBjb25z dCB0eXBlY19wb3J0X3R5cGVzW10gPSB7Cj4gPj4+ICAgCVtUWVBFQ19QT1JUX0RSUF0gPSAiZHVh bCIsCj4gPj4+ICAgfTsKPiA+Pj4KPiA+Pj4gK3N0YXRpYyBjb25zdCBjaGFyICogY29uc3QgdHlw ZWNfZGF0YV90eXBlc1tdID0gewo+ID4+PiArCVtUWVBFQ19QT1JUX0RGUF0gPSAiaG9zdCIsCj4g Pj4+ICsJW1RZUEVDX1BPUlRfVUZQXSA9ICJkZXZpY2UiLAo+ID4+PiArCVtUWVBFQ19QT1JUX0RS RF0gPSAiZHVhbCIsCj4gPj4+ICt9Owo+ID4+PiArCj4gPj4+ICAgc3RhdGljIGNvbnN0IGNoYXIg KiBjb25zdCB0eXBlY19wb3J0X3R5cGVzX2RycFtdID0gewo+ID4+PiAgIAlbVFlQRUNfUE9SVF9T UkNdID0gImR1YWwgW3NvdXJjZV0gc2luayIsCj4gPj4+ICAgCVtUWVBFQ19QT1JUX1NOS10gPSAi ZHVhbCBzb3VyY2UgW3NpbmtdIiwgQEAgLTEyNTIsNiArMTI1OSw1MQo+ID4+IEBACj4gPj4+IHZv aWQgdHlwZWNfc2V0X3B3cl9vcG1vZGUoc3RydWN0IHR5cGVjX3BvcnQgKnBvcnQsCj4gPj4+ICAg fQo+ID4+PiAgIEVYUE9SVF9TWU1CT0xfR1BMKHR5cGVjX3NldF9wd3Jfb3Btb2RlKTsKPiA+Pj4K PiA+Pj4gKy8qKgo+ID4+PiArICogdHlwZWNfZmluZF9wb3dlcl90eXBlIC0gR2V0IHRoZSB0eXBl YyBwb3J0IHBvd2VyIHR5cGUKPiA+PiBXaHkgaXMgdGhpcyBmdW5jdGlvbiBjYWxsZWQgdHlwZWNf ZmluZF9wb3dlcl90eXBlKCkgYW5kIG5vdAo+ID4+IHR5cGVjX2ZpbmRfcG9ydF90eXBlKCk/Cj4g Pj4gSXQncyBjYWxsZWQgcG9ydF90eXBlIGluIHN5c2ZzLCBoYXZpbmcgZGlmZmVyZW50IG5hbWVz IGp1c3QgYWRkcyBjb25mdXNpb24uCj4gPj4gKE90aGVyd2lzZSBJIGFncmVlIHBvd2VyX3R5cGUg aXMgYSBiZXR0ZXIgbmFtZSBidXQuLi4pCj4gPiBXZSBoYXZlICJwb3J0IHR5cGUiIGJlZm9yZSB0 aGUgcG93ZXIgYW5kIGRhdGEgcm9sZSBzZXBhcmF0aW9uLAo+ID4gdGhpcyBBUEkgbmFtZSdzIGlu dGVudGlvbiBpcyB0byByZWZsZWN0IHRoZSBwb3dlciBjYXAsIGFueXdheSBJCj4gPiBsZWF2ZSB0 aGlzIHRvIGJlIGRlY2lkZWQgYnkgSGVpa2tpIHRoZW4uCgpJIHJlYWxseSBoYXRlIHRoZSAiKl90 eXBlIiBuYW1pbmcuIEl0IHdhcyB1bmRlcnN0YW5kYWJsZSB3aGVuIHRoZXJlCndhcyBubyBzZXBh cmF0ZSBwb3dlciBhbmQgZGF0YSByb2xlcyBkZWZpbmVkIGluIHRoZSBzcGVjaWZpY2F0aW9uLCBi dXQKbm93IHRoYXQgdGhlcmUgYXJlLCBpdCdzIGp1c3QgY29uZnVzaW5nLiBJTU8gd2Ugc2hvdWxk IG5vdCB1c2UgaXQKYW55d2hlcmUuCgpTbyB0byBtZSB0eXBlY19maW5kX3R5cGUoKSBpcyBqdXN0 IGFzIGJhZCBhcyB0eXBlY19maW5kX3Bvd2VyX3R5cGUoKQpiZWNhdXNlIGl0IGhhcyB0aGUgInR5 cGUiIGluIGl0LiBJIHdvbmRlciBpZiB0aGlzIGZ1bmN0aW9uIGlzCm5lY2Vzc2FyeSBhdCBhbGw/ IElmIGl0IGlzLCB0aGVuIHBlcmhhcHMgd2UgY2FuIHRoaW5rIG9mIHNvbWUgYmV0dGVyCm5hbWUg Zm9yIGl0LCBuYW1lIHRoYXQgZ2l2ZXMgYSBiZXR0ZXIgaGludCB3aGF0IGl0IGlzIHVzZWQgZm9y LgoKPiA+Pj4gKyAqIEBuYW1lOiBwb3J0IHR5cGUgc3RyaW5nCj4gPj4+ICsgKgo+ID4+PiArICog VGhpcyByb3V0aW5lIGlzIHVzZWQgdG8gZmluZCB0aGUgdHlwZWNfcG9ydF90eXBlIGJ5IGl0cyBz dHJpbmcgbmFtZS4KPiA+Pj4gKyAqCj4gPj4+ICsgKiBSZXR1cm5zIHR5cGVjX3BvcnRfdHlwZSBp ZiBzdWNjZXNzLCBvdGhlcndpc2UgbmVnYXRpdmUgZXJyb3IgY29kZS4KPiA+Pj4gKyAqLwo+ID4+ PiAraW50IHR5cGVjX2ZpbmRfcG93ZXJfdHlwZShjb25zdCBjaGFyICpuYW1lKSB7Cj4gPj4+ICsJ cmV0dXJuIG1hdGNoX3N0cmluZyh0eXBlY19wb3J0X3R5cGVzLCBBUlJBWV9TSVpFKHR5cGVjX3Bv cnRfdHlwZXMpLAo+ID4+PiArCQkJICAgIG5hbWUpOwo+ID4+PiArfQo+ID4+PiArRVhQT1JUX1NZ TUJPTF9HUEwodHlwZWNfZmluZF9wb3dlcl90eXBlKTsKPiA+Pj4gKwo+ID4+PiArLyoqCj4gPj4+ ICsgKiB0eXBlY19maW5kX3ByZWZlcnJlZF9yb2xlIC0gRmluZCB0aGUgdHlwZWMgZHJwIHBvcnQg cHJlZmVycmVkCj4gPj4+ICtwb3dlciByb2xlCj4gPj4gV2h5IHR5cGVjX2ZpbmRfcHJlZmVycmVk X3JvbGUoKT8gQ291bGQgYmUgdXNlZCBmb3IgYW55IHBvd2VyX3JvbGUgc28gd2h5IG5vdAo+ID4+ IHR5cGVjX2ZpbmRfcG93ZXJfcm9sZSgpPwo+ID4gSSBhbSBub3Qgc3VyZSBpZiBJIGNhdGNoIHlv dXIgcG9pbnQgb2YgdGhpcyBjb21tZW50Lgo+ID4gRm9yIHByZWZlcnJlZCByb2xlKGlmIHN1cHBv cnQgdHJ5LnNpbmsgb3IgdHJ5LnNyYykgdGhlIG9ubHkgYWxsb3dlZCBwb3dlciByb2xlcyBhcmUg Cj4gPiAic2luayIKPiA+ICJzb3VyY2UiCj4gPiBCdXQgZm9yIHBvd2VyIHJvbGUsIHRoZSBhbGxv d2VkIHR5cGUgYXJlIAo+ID4gInNpbmsiCj4gPiAic291cmNlIgo+ID4gImR1YWwiCj4gCj4gVWht LCB0eXBpbmcgdG9vIGZhc3QgYWdhaW4sIEkgYW0uIEEgYmV0dGVyIG5hbWUgd291bGQgYmUganVz dCB0eXBlY19maW5kX3JvbGUoKS4KPiBXaGF0IEkgbWVhbiBpcyB0aGF0IHRoZSBmdW5jdGlvbiBj b3VsZCBiZSB1c2VkIGZvciBhbnkgc2l0dWF0aW9uIHdoZW4KPiBzb21lb25lIHdhbnRzIHRvIG1h cCBhIHN0cmluZyB0byBhIFRZUEVDX3tTT1VSQ0UsU0lOS30gY29uc3RhbnQgc28gaXQKPiBpcyB1 bm5lY2Vzc2FyeSB0byBsaW1pdCBpdHMgdXNhZ2UgdG8ganVzdCBwcmVmZXJyZWQgcm9sZS4KClRo YXQgc291bmRzIHJlYXNvbmFibGUgdG8gbWUuCgoKVGhhbmtzLAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 16 May 2018 15:25:27 +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: <20180516122527.GC11469@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <74e1c164-1652-75f4-409c-bd1c214bda4d@gmail.com> To: Jun Li , Mats Karrman Cc: "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: 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. > >>> + * @name: port type string > >>> + * > >>> + * This routine is used to find the typec_port_type by its string name. > >>> + * > >>> + * Returns typec_port_type if success, otherwise negative error code. > >>> + */ > >>> +int typec_find_power_type(const char *name) { > >>> + return match_string(typec_port_types, ARRAY_SIZE(typec_port_types), > >>> + name); > >>> +} > >>> +EXPORT_SYMBOL_GPL(typec_find_power_type); > >>> + > >>> +/** > >>> + * typec_find_preferred_role - Find the typec drp port preferred > >>> +power role > >> Why typec_find_preferred_role()? Could be used for any power_role so why not > >> typec_find_power_role()? > > I am not sure if I catch your point of this comment. > > For preferred role(if support try.sink or try.src) the only allowed power roles are > > "sink" > > "source" > > But for power role, the allowed type are > > "sink" > > "source" > > "dual" > > Uhm, typing too fast again, I am. A better name would be just typec_find_role(). > What I mean is that the function could be used for any situation when > someone wants to map a string to a TYPEC_{SOURCE,SINK} constant so it > is unnecessary to limit its usage to just preferred role. That sounds reasonable to me. Thanks, -- heikki