> -----Original Message----- > From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter Roeck > Sent: 2018年3月30日 6:49 > To: Jun Li <jun.li@xxxxxxx> > Cc: robh+dt@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; > heikki.krogerus@xxxxxxxxxxxxxxx; a.hajda@xxxxxxxxxxx; > shufan_lee@xxxxxxxxxxx; Peter Chen <peter.chen@xxxxxxx>; > devicetree@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; dl-linux-imx > <linux-imx@xxxxxxx>; devel@xxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach > > 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. In question case(drp without try.src or try.snk), you can see tcpm never call tcpm_set_cc() from drp toggling to attached state, start_drp_toggling set cc at the beginning in this case, but the value is the *start* value, may not the right value to match its final role. Per tcpci spec Figure 4-16. DRP Initialization and Connection Detection After debounce, tcpm should set cc(again, but may different value) and polarity. ShuFan encountered this case as I understood on his setup like below: - Tcpc start drp toggling with Rp/Rp, - Connect to adapter with also Rp/Rp - After connection resolved, the tcpc becomes a sink(HW present Rd), but role control register value is still Rp/Rp. - This should be corrected to set cc for keep the un-contacted cc line open. > > 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). Currently there is no user of cc_req flag, so I can't catch the real intention of this flag, as of now it's only set in tcpm_set_cc(), so I understood it's for if tcpm already set tcpc a specific role. 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 > > 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 > > ?韬{.n?????%??檩??w?{.n????z谵{???塄}?财??j:+v?? ? ???2??璀??摺?囤??z夸z罐?+?????w棹f
