[v4,10/13] usb: typec: tcpm: set cc for drp toggling attach

[v4,10/13] usb: typec: tcpm: set cc for drp toggling attach

[Jun Li]

In case of drp toggling, we may need set correct cc value for role control
after attach as it may never been set.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/tcpm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 218c230..72d4232 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
 	tcpm_set_attached_state(port, false);
 	port->try_src_count = 0;
 	port->try_snk_count = 0;
+	port->cc_req = 0;
 }
 
 static void tcpm_detach(struct tcpm_port *port)
@@ -2361,6 +2362,8 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 
 	case SRC_ATTACHED:
+		if (!port->cc_req)
+			tcpm_set_cc(port, tcpm_rp_cc(port));
 		ret = tcpm_src_attach(port);
 		tcpm_set_state(port, SRC_UNATTACHED,
 			       ret < 0 ? 0 : PD_T_PS_SOURCE_ON);
@@ -2531,6 +2534,8 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
 		break;
 	case SNK_ATTACHED:
+		if (!port->cc_req)
+			tcpm_set_cc(port, TYPEC_CC_RD);
 		ret = tcpm_snk_attach(port);
 		if (ret < 0)
 			tcpm_set_state(port, SNK_UNATTACHED, 0);

[v4,10/13] usb: typec: tcpm: set cc for drp toggling attach

[Mats Karrman]

Hi Li,

On 03/28/2018 06:06 PM, Li Jun wrote:

> In case of drp toggling, we may need set correct cc value for role control
> after attach as it may never been set.
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  drivers/usb/typec/tcpm.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 218c230..72d4232 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
>  	tcpm_set_attached_state(port, false);
>  	port->try_src_count = 0;
>  	port->try_snk_count = 0;
> +	port->cc_req = 0;

I don't think it's OK to use "0" here. cc_req is an enum so why not use "|TYPEC_CC_OPEN"?|

>  }
>  
>  static void tcpm_detach(struct tcpm_port *port)
> @@ -2361,6 +2362,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		break;
>  
>  	case SRC_ATTACHED:
> +		if (!port->cc_req)

        	if (port->cc_req == |TYPEC_CC_OPEN)|

> +			tcpm_set_cc(port, tcpm_rp_cc(port));
>  		ret = tcpm_src_attach(port);
>  		tcpm_set_state(port, SRC_UNATTACHED,
>  			       ret < 0 ? 0 : PD_T_PS_SOURCE_ON);
> @@ -2531,6 +2534,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
>  		break;
>  	case SNK_ATTACHED:
> +		if (!port->cc_req)

Ditto.

> +			tcpm_set_cc(port, TYPEC_CC_RD);
>  		ret = tcpm_snk_attach(port);
>  		if (ret < 0)
>  			tcpm_set_state(port, SNK_UNATTACHED, 0);

// Mats
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[v4,10/13] usb: typec: tcpm: set cc for drp toggling attach

[Guenter Roeck]

On Thu, Mar 29, 2018 at 12:06:15AM +0800, Li Jun wrote:
> In case of drp toggling, we may need set correct cc value for role control
> after attach as it may never been set.
> 

Isn't CC set by the lower level driver in this case ? In other words, is it ever
necessary to call back into the low level driver to set CC again ? Doing that in
attached state seems a bit late.

It may make more sense to update port->cc_req when the state machine leaves
DRP_TOGGLING state, ie in _tcpm_cc_change(), and to do it without callback
into the low level driver (it should not be necessary).

Guenter

> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  drivers/usb/typec/tcpm.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 218c230..72d4232 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
>  	tcpm_set_attached_state(port, false);
>  	port->try_src_count = 0;
>  	port->try_snk_count = 0;
> +	port->cc_req = 0;
>  }
>  
>  static void tcpm_detach(struct tcpm_port *port)
> @@ -2361,6 +2362,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		break;
>  
>  	case SRC_ATTACHED:
> +		if (!port->cc_req)
> +			tcpm_set_cc(port, tcpm_rp_cc(port));
>  		ret = tcpm_src_attach(port);
>  		tcpm_set_state(port, SRC_UNATTACHED,
>  			       ret < 0 ? 0 : PD_T_PS_SOURCE_ON);
> @@ -2531,6 +2534,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
>  		break;
>  	case SNK_ATTACHED:
> +		if (!port->cc_req)
> +			tcpm_set_cc(port, TYPEC_CC_RD);
>  		ret = tcpm_snk_attach(port);
>  		if (ret < 0)
>  			tcpm_set_state(port, SNK_UNATTACHED, 0);
> -- 
> 2.7.4
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[v4,10/13] usb: typec: tcpm: set cc for drp toggling attach

[Jun Li]

Hi
> -----Original Message-----
> From: Mats Karrman [mailto:mats.dev.list@gmail.com]
> Sent: 2018年3月30日 5:19
> To: Jun Li <jun.li@nxp.com>; robh+dt@kernel.org; gregkh@linuxfoundation.org;
> heikki.krogerus@linux.intel.com; linux@roeck-us.net
> Cc: a.hajda@samsung.com; shufan_lee@richtek.com; Peter Chen
> <peter.chen@nxp.com>; devicetree@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach
> 
> Hi Li,
> 
> On 03/28/2018 06:06 PM, Li Jun wrote:
> 
> > In case of drp toggling, we may need set correct cc value for role
> > control after attach as it may never been set.
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  drivers/usb/typec/tcpm.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index
> > 218c230..72d4232 100644
> > --- a/drivers/usb/typec/tcpm.c
> > +++ b/drivers/usb/typec/tcpm.c
> > @@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
> >  	tcpm_set_attached_state(port, false);
> >  	port->try_src_count = 0;
> >  	port->try_snk_count = 0;
> > +	port->cc_req = 0;
> 
> I don't think it's OK to use "0" here. cc_req is an enum so why not use
> "|TYPEC_CC_OPEN"?|
> 

I will change to be TYPEC_CC_OPEN, also other place.

Li Jun
> >  }
> >
> >  static void tcpm_detach(struct tcpm_port *port) @@ -2361,6 +2362,8 @@
> > static void run_state_machine(struct tcpm_port *port)
> >  		break;
> >
> >  	case SRC_ATTACHED:
> > +		if (!port->cc_req)
> 
>         	if (port->cc_req == |TYPEC_CC_OPEN)|
> 
> > +			tcpm_set_cc(port, tcpm_rp_cc(port));
> >  		ret = tcpm_src_attach(port);
> >  		tcpm_set_state(port, SRC_UNATTACHED,
> >  			       ret < 0 ? 0 : PD_T_PS_SOURCE_ON); @@ -2531,6 +2534,8
> @@
> > static void run_state_machine(struct tcpm_port *port)
> >  		tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
> >  		break;
> >  	case SNK_ATTACHED:
> > +		if (!port->cc_req)
> 
> Ditto.
> 
> > +			tcpm_set_cc(port, TYPEC_CC_RD);
> >  		ret = tcpm_snk_attach(port);
> >  		if (ret < 0)
> >  			tcpm_set_state(port, SNK_UNATTACHED, 0);
> 
> // Mats
>

[v4,10/13] usb: typec: tcpm: set cc for drp toggling attach

[Jun Li]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogR3VlbnRlciBSb2VjayBb
bWFpbHRvOmdyb2VjazdAZ21haWwuY29tXSBPbiBCZWhhbGYgT2YgR3VlbnRlciBSb2Vjaw0KPiBT
ZW50OiAyMDE4xOoz1MIzMMjVIDY6NDkNCj4gVG86IEp1biBMaSA8anVuLmxpQG54cC5jb20+DQo+
IENjOiByb2JoK2R0QGtlcm5lbC5vcmc7IGdyZWdraEBsaW51eGZvdW5kYXRpb24ub3JnOw0KPiBo
ZWlra2kua3JvZ2VydXNAbGludXguaW50ZWwuY29tOyBhLmhhamRhQHNhbXN1bmcuY29tOw0KPiBz
aHVmYW5fbGVlQHJpY2h0ZWsuY29tOyBQZXRlciBDaGVuIDxwZXRlci5jaGVuQG54cC5jb20+Ow0K
PiBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgbGludXgtdXNiQHZnZXIua2VybmVsLm9yZzsg
ZGwtbGludXgtaW14DQo+IDxsaW51eC1pbXhAbnhwLmNvbT47IGRldmVsQGRyaXZlcmRldi5vc3Vv
c2wub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjQgMTAvMTNdIHVzYjogdHlwZWM6IHRjcG06
IHNldCBjYyBmb3IgZHJwIHRvZ2dsaW5nIGF0dGFjaA0KPiANCj4gT24gVGh1LCBNYXIgMjksIDIw
MTggYXQgMTI6MDY6MTVBTSArMDgwMCwgTGkgSnVuIHdyb3RlOg0KPiA+IEluIGNhc2Ugb2YgZHJw
IHRvZ2dsaW5nLCB3ZSBtYXkgbmVlZCBzZXQgY29ycmVjdCBjYyB2YWx1ZSBmb3Igcm9sZQ0KPiA+
IGNvbnRyb2wgYWZ0ZXIgYXR0YWNoIGFzIGl0IG1heSBuZXZlciBiZWVuIHNldC4NCj4gPg0KPiAN
Cj4gSXNuJ3QgQ0Mgc2V0IGJ5IHRoZSBsb3dlciBsZXZlbCBkcml2ZXIgaW4gdGhpcyBjYXNlID8g
SW4gb3RoZXIgd29yZHMsIGlzIGl0IGV2ZXINCj4gbmVjZXNzYXJ5IHRvIGNhbGwgYmFjayBpbnRv
IHRoZSBsb3cgbGV2ZWwgZHJpdmVyIHRvIHNldCBDQyBhZ2FpbiA/IERvaW5nIHRoYXQgaW4NCj4g
YXR0YWNoZWQgc3RhdGUgc2VlbXMgYSBiaXQgbGF0ZS4NCg0KSW4gcXVlc3Rpb24gY2FzZShkcnAg
d2l0aG91dCB0cnkuc3JjIG9yIHRyeS5zbmspLCB5b3UgY2FuIHNlZSB0Y3BtIG5ldmVyIGNhbGwN
CnRjcG1fc2V0X2NjKCkgZnJvbSBkcnAgdG9nZ2xpbmcgdG8gYXR0YWNoZWQgc3RhdGUsIHN0YXJ0
X2RycF90b2dnbGluZw0Kc2V0IGNjIGF0IHRoZSBiZWdpbm5pbmcgaW4gdGhpcyBjYXNlLCBidXQg
dGhlIHZhbHVlIGlzIHRoZSAqc3RhcnQqIHZhbHVlLCBtYXkgbm90DQp0aGUgcmlnaHQgdmFsdWUg
dG8gbWF0Y2ggaXRzIGZpbmFsIHJvbGUuDQoNClBlciB0Y3BjaSBzcGVjDQpGaWd1cmUgNC0xNi4g
RFJQIEluaXRpYWxpemF0aW9uIGFuZCBDb25uZWN0aW9uIERldGVjdGlvbg0KQWZ0ZXIgZGVib3Vu
Y2UsIHRjcG0gc2hvdWxkIHNldCBjYyhhZ2FpbiwgYnV0IG1heSBkaWZmZXJlbnQgdmFsdWUpIGFu
ZCBwb2xhcml0eS4NCg0KU2h1RmFuIGVuY291bnRlcmVkIHRoaXMgY2FzZSBhcyBJIHVuZGVyc3Rv
b2Qgb24gaGlzIHNldHVwIGxpa2UgYmVsb3c6DQotIFRjcGMgc3RhcnQgZHJwIHRvZ2dsaW5nIHdp
dGggUnAvUnAsDQotIENvbm5lY3QgdG8gYWRhcHRlciB3aXRoIGFsc28gUnAvUnANCi0gQWZ0ZXIg
Y29ubmVjdGlvbiByZXNvbHZlZCwgdGhlIHRjcGMgYmVjb21lcyBhIHNpbmsoSFcgcHJlc2VudCBS
ZCksIGJ1dA0Kcm9sZSBjb250cm9sIHJlZ2lzdGVyIHZhbHVlIGlzIHN0aWxsIFJwL1JwLg0KLSBU
aGlzIHNob3VsZCBiZSBjb3JyZWN0ZWQgdG8gc2V0IGNjIGZvciBrZWVwIHRoZSB1bi1jb250YWN0
ZWQgY2MgbGluZSBvcGVuLg0KDQo+IA0KPiBJdCBtYXkgbWFrZSBtb3JlIHNlbnNlIHRvIHVwZGF0
ZSBwb3J0LT5jY19yZXEgd2hlbiB0aGUgc3RhdGUgbWFjaGluZSBsZWF2ZXMNCj4gRFJQX1RPR0dM
SU5HIHN0YXRlLCBpZSBpbiBfdGNwbV9jY19jaGFuZ2UoKSwgYW5kIHRvIGRvIGl0IHdpdGhvdXQg
Y2FsbGJhY2sgaW50bw0KPiB0aGUgbG93IGxldmVsIGRyaXZlciAoaXQgc2hvdWxkIG5vdCBiZSBu
ZWNlc3NhcnkpLg0KDQpDdXJyZW50bHkgdGhlcmUgaXMgbm8gdXNlciBvZiBjY19yZXEgZmxhZywg
c28gSSBjYW4ndCBjYXRjaCB0aGUgcmVhbCBpbnRlbnRpb24NCm9mIHRoaXMgZmxhZywgYXMgb2Yg
bm93IGl0J3Mgb25seSBzZXQgaW4gdGNwbV9zZXRfY2MoKSwgc28gSSB1bmRlcnN0b29kIGl0J3MN
CmZvciBpZiB0Y3BtIGFscmVhZHkgc2V0IHRjcGMgYSBzcGVjaWZpYyByb2xlLg0KDQpMaSBKdW4N
Cj4gDQo+IEd1ZW50ZXINCj4gDQo+ID4gU2lnbmVkLW9mZi1ieTogTGkgSnVuIDxqdW4ubGlAbnhw
LmNvbT4NCj4gPiAtLS0NCj4gPiAgZHJpdmVycy91c2IvdHlwZWMvdGNwbS5jIHwgNSArKysrKw0K
PiA+ICAxIGZpbGUgY2hhbmdlZCwgNSBpbnNlcnRpb25zKCspDQo+ID4NCj4gPiBkaWZmIC0tZ2l0
IGEvZHJpdmVycy91c2IvdHlwZWMvdGNwbS5jIGIvZHJpdmVycy91c2IvdHlwZWMvdGNwbS5jIGlu
ZGV4DQo+ID4gMjE4YzIzMC4uNzJkNDIzMiAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL3VzYi90
eXBlYy90Y3BtLmMNCj4gPiArKysgYi9kcml2ZXJzL3VzYi90eXBlYy90Y3BtLmMNCj4gPiBAQCAt
MjEyNiw2ICsyMTI2LDcgQEAgc3RhdGljIHZvaWQgdGNwbV9yZXNldF9wb3J0KHN0cnVjdCB0Y3Bt
X3BvcnQgKnBvcnQpDQo+ID4gIAl0Y3BtX3NldF9hdHRhY2hlZF9zdGF0ZShwb3J0LCBmYWxzZSk7
DQo+ID4gIAlwb3J0LT50cnlfc3JjX2NvdW50ID0gMDsNCj4gPiAgCXBvcnQtPnRyeV9zbmtfY291
bnQgPSAwOw0KPiA+ICsJcG9ydC0+Y2NfcmVxID0gMDsNCj4gPiAgfQ0KPiA+DQo+ID4gIHN0YXRp
YyB2b2lkIHRjcG1fZGV0YWNoKHN0cnVjdCB0Y3BtX3BvcnQgKnBvcnQpIEBAIC0yMzYxLDYgKzIz
NjIsOCBAQA0KPiA+IHN0YXRpYyB2b2lkIHJ1bl9zdGF0ZV9tYWNoaW5lKHN0cnVjdCB0Y3BtX3Bv
cnQgKnBvcnQpDQo+ID4gIAkJYnJlYWs7DQo+ID4NCj4gPiAgCWNhc2UgU1JDX0FUVEFDSEVEOg0K
PiA+ICsJCWlmICghcG9ydC0+Y2NfcmVxKQ0KPiA+ICsJCQl0Y3BtX3NldF9jYyhwb3J0LCB0Y3Bt
X3JwX2NjKHBvcnQpKTsNCj4gPiAgCQlyZXQgPSB0Y3BtX3NyY19hdHRhY2gocG9ydCk7DQo+ID4g
IAkJdGNwbV9zZXRfc3RhdGUocG9ydCwgU1JDX1VOQVRUQUNIRUQsDQo+ID4gIAkJCSAgICAgICBy
ZXQgPCAwID8gMCA6IFBEX1RfUFNfU09VUkNFX09OKTsgQEAgLTI1MzEsNiArMjUzNCw4DQo+IEBA
DQo+ID4gc3RhdGljIHZvaWQgcnVuX3N0YXRlX21hY2hpbmUoc3RydWN0IHRjcG1fcG9ydCAqcG9y
dCkNCj4gPiAgCQl0Y3BtX3NldF9zdGF0ZShwb3J0LCBTTktfVU5BVFRBQ0hFRCwgUERfVF9QRF9E
RUJPVU5DRSk7DQo+ID4gIAkJYnJlYWs7DQo+ID4gIAljYXNlIFNOS19BVFRBQ0hFRDoNCj4gPiAr
CQlpZiAoIXBvcnQtPmNjX3JlcSkNCj4gPiArCQkJdGNwbV9zZXRfY2MocG9ydCwgVFlQRUNfQ0Nf
UkQpOw0KPiA+ICAJCXJldCA9IHRjcG1fc25rX2F0dGFjaChwb3J0KTsNCj4gPiAgCQlpZiAocmV0
IDwgMCkNCj4gPiAgCQkJdGNwbV9zZXRfc3RhdGUocG9ydCwgU05LX1VOQVRUQUNIRUQsIDApOw0K
PiA+IC0tDQo+ID4gMi43LjQNCj4gPg0K
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html