Hi > -----Original Message----- > From: Jun Li > Sent: 2018年6月13日 19:07 > To: Guenter Roeck <linux@xxxxxxxxxxxx>; Heikki Krogerus > <heikki.krogerus@xxxxxxxxxxxxxxx>; shufan_lee@xxxxxxxxxxx > Cc: robh+dt@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; cw00.choi@xxxxxxxxxxx; > a.hajda@xxxxxxxxxxx; Peter Chen <peter.chen@xxxxxxx>; > garsilva@xxxxxxxxxxxxxx; gsomlo@xxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > Subject: RE: [PATCH v6 12/15] usb: typec: tcpm: set cc for drp toggling attach > > Hi, > > -----Original Message----- > > From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter > > Roeck > > Sent: 2018年6月11日 21:35 > > To: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>; Jun Li > > <jun.li@xxxxxxx> > > Cc: robh+dt@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; > > cw00.choi@xxxxxxxxxxx; a.hajda@xxxxxxxxxxx; shufan_lee@xxxxxxxxxxx; > > Peter Chen <peter.chen@xxxxxxx>; garsilva@xxxxxxxxxxxxxx; > > gsomlo@xxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > > devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > > Subject: Re: [PATCH v6 12/15] usb: typec: tcpm: set cc for drp > > toggling attach > > > > On 06/11/2018 05:29 AM, Heikki Krogerus wrote: > > > On Mon, May 28, 2018 at 10:52:44AM +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. > > > > > > Is this something that should be considered as a fix? > > > > > > > The problem with this patch is that it hides a problem. CC should have > > been set by the time a port reaches the attached state. The patch > > means that there is a state machine path where this does not happen. > > I'd rather understand that path and fix the problem where it happens. > > Here is my previous explanation about this state machine path[1] > > [1]https://www.spinics.net/lists/linux-usb/msg167467.html > > The physical CC line state is correct, just the Role Control register value may > mismatch on some(e.g. shufan's) HW, if the common change in tcpm is not the > right way, I have to ask shufan to try resolve this in his custom rt1711h driver. > > I understand the concern of setting cc at attached state is late, should be done > before that, how about I move this in cc_change like below: > > @@ -3492,10 +3492,13 @@ static void _tcpm_cc_change(struct tcpm_port *port, > enum typec_cc_status cc1, > switch (port->state) { > case DRP_TOGGLING: > if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) || > - tcpm_port_is_source(port)) > + tcpm_port_is_source(port)) { > + tcpm_set_cc(port, tcpm_rp_cc(port)); > tcpm_set_state(port, SRC_ATTACH_WAIT, 0); > - else if (tcpm_port_is_sink(port)) > + } else if (tcpm_port_is_sink(port)) { > + if (port->cc_req == TYPEC_CC_OPEN) Sorry, here should be: + } else if (tcpm_port_is_sink(port)) { + tcpm_set_cc(port, TYPEC_CC_RD); tcpm_set_state(port, SNK_ATTACH_WAIT, 0); + } > tcpm_set_state(port, SNK_ATTACH_WAIT, 0); > + } > break; > case SRC_UNATTACHED: > case ACC_UNATTACHED: > > thanks > Li Jun > > > > > Guenter > > > > >> Signed-off-by: Li Jun <jun.li@xxxxxxx> > > >> --- > > >> 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 d885bff..98ea916 100644 > > >> --- a/drivers/usb/typec/tcpm.c > > >> +++ b/drivers/usb/typec/tcpm.c > > >> @@ -2599,6 +2599,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 = TYPEC_CC_OPEN; > > >> port->supply_voltage = 0; > > >> port->current_limit = 0; > > >> port->usb_type = POWER_SUPPLY_USB_TYPE_C; @@ -2831,6 +2832,8 > > @@ > > >> static void run_state_machine(struct tcpm_port *port) > > >> break; > > >> > > >> case SRC_ATTACHED: > > >> + 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); @@ -3004,6 > > +3007,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 == TYPEC_CC_OPEN) > > >> + tcpm_set_cc(port, TYPEC_CC_RD); > > >> ret = tcpm_snk_attach(port); > > >> if (ret < 0) > > >> tcpm_set_state(port, SNK_UNATTACHED, 0); > > > ��.n��������+%������w��{.n�����{���)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
