Messages in this thread | | | Date | Mon, 20 Mar 2023 10:38:35 +0800 | Subject | Re: [PATCH 4/4] usb: typec: tcpm: fix source caps may lost after soft reset | From | Frank Wang <> |
| |
Hi Guenter,
On 2023/3/17 20:58, Guenter Roeck wrote: > On 3/12/23 19:58, Frank Wang wrote: >> Invoke set_pd_rx() may flush the RX FIFO of PD controller, so do >> set_pd_rx() before sending Soft Reset in case Source caps may be flushed >> at debounce time between SOFT_RESET_SEND and SNK_WAIT_CAPABILITIES >> state. >> > > Isn't that a problem of the fusb302 driver that it flushes its buffers > unconditionally when its set_pd_rx() callback is called ? > > Guenter >
The story goes like this, the fusb302 notified SOFT_RESET completion to TCPM and began to receive the Source Caps automatically, TCPM got completion from fusb302 and changed state to SNK_WAIT_CAPABILITIES and invoked set_pd_rx() callback. However, the fusb302 or TCPM's worker may be not scheduled in time, set_pd_rx() would be performed after the Source Caps packets had received into fusb302's FIFO, even after the GoodCRC (Source Caps) had be replied.
So make forward set_pd_rx() process before PD_CTRL_SOFT_RESET sent at SOFT_RESET_SEND state can cleanup the context in our side and ensure the right PD commucation. I am not sure whether it is sensible?
BR. Frank
>> Without this patch, in PD charger stress test, the FUSB302 driver may >> occur the following exceptions in power negotiation stage. >> >> [ ...] >> [ 4.512252] fusb302_irq_intn >> [ 4.512260] AMS SOFT_RESET_AMS finished >> [ 4.512269] state change SOFT_RESET_SEND ->SNK_WAIT_CAPABILITIES >> [rev3 NONE_AMS] >> [ 4.514511] pd := on >> [ 4.514516] pending state change SNK_WAIT_CAPABILITIES >> ->HARD_RESET_SEND @ 310 ms [rev3 NONE_AMS] >> [ 4.515428] IRQ: 0x51, a: 0x00, b: 0x01, status0: 0x93 >> [ 4.515431] IRQ: BC_LVL, handler pending >> [ 4.515435] IRQ: PD sent good CRC >> [ 4.516434] PD message header: 0 >> [ 4.516437] PD message len: 0 >> [ 4.516444] PD RX, header: 0x0 [1] >> >> Signed-off-by: Frank Wang <frank.wang@rock-chips.com> >> --- >> drivers/usb/typec/tcpm/tcpm.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/usb/typec/tcpm/tcpm.c >> b/drivers/usb/typec/tcpm/tcpm.c >> index 9e583060e64fc..ba6bf71838eed 100644 >> --- a/drivers/usb/typec/tcpm/tcpm.c >> +++ b/drivers/usb/typec/tcpm/tcpm.c >> @@ -4321,10 +4321,12 @@ static void run_state_machine(struct >> tcpm_port *port) >> tcpm_set_state(port, unattached_state(port), 0); >> break; >> case SNK_WAIT_CAPABILITIES: >> - ret = port->tcpc->set_pd_rx(port->tcpc, true); >> - if (ret < 0) { >> - tcpm_set_state(port, SNK_READY, 0); >> - break; >> + if (port->prev_state != SOFT_RESET_SEND) { >> + ret = port->tcpc->set_pd_rx(port->tcpc, true); >> + if (ret < 0) { >> + tcpm_set_state(port, SNK_READY, 0); >> + break; >> + } >> } >> /* >> * If VBUS has never been low, and we time out waiting >> @@ -4603,6 +4605,7 @@ static void run_state_machine(struct tcpm_port >> *port) >> case SOFT_RESET_SEND: >> port->message_id = 0; >> port->rx_msgid = -1; >> + port->tcpc->set_pd_rx(port->tcpc, true); >> if (tcpm_pd_send_control(port, PD_CTRL_SOFT_RESET)) >> tcpm_set_state_cond(port, hard_reset_state(port), 0); >> else >
| |