Hi
> -----Original Message-----
> From: Jun Li
> Sent: 2018年6月13日 19:07
> To: Guenter Roeck <[email protected]>; Heikki Krogerus
> <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Peter Chen <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>
> Subject: RE: [PATCH v6 12/15] usb: typec: tcpm: set cc for drp toggling attach
>
> Hi,
> > -----Original Message-----
> > From: Guenter Roeck [mailto:[email protected]] On Behalf Of Guenter
> > Roeck
> > Sent: 2018年6月11日 21:35
> > To: Heikki Krogerus <[email protected]>; Jun Li
> > <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > Peter Chen <[email protected]>; [email protected];
> > [email protected]; [email protected];
> > [email protected]; dl-linux-imx <[email protected]>
> > 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 <[email protected]>
> > >> ---
> > >> 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);
> > >
