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) 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����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
