lkml.org 
[lkml]   [2017]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v2] typec: tcpm: fusb302: Resolve out of order messaging events
On Thu, Nov 16, 2017 at 04:28:11PM +0000, Adam Thomson wrote:
> The expectation in the FUSB302 driver is that a TX_SUCCESS event
> should occur after a message has been sent, but before a GCRCSENT
> event is raised to indicate successful receipt of a message from
> the partner. However in some circumstances it is possible to see
> the hardware raise a GCRCSENT event before a TX_SUCCESS event
> is raised. The upshot of this is that the GCRCSENT handling portion
> of code ends up reporting the GoodCRC message to TCPM because the
> TX_SUCCESS event hasn't yet arrived to trigger a consumption of it.
> When TX_SUCCESS is then raised by the chip it ends up consuming the
> actual message that was meant for TCPM, and this incorrect sequence
> results in a hard reset from TCPM.
>
> To avoid this problem, this commit moves all FIFO reading to be
> done based on a GCRCSENT event, and when reading from the FIFO
> any GoodCRC messages read in are discarded so only valid messages
> are reported to TCPM.
>
> Changes in v2:
> - Remove erroneous extended header check
>
> Patch is based on Linux next-20171114 to include move out of staging.
>
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> ---
> drivers/usb/typec/fusb302/fusb302.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/typec/fusb302/fusb302.c b/drivers/usb/typec/fusb302/fusb302.c
> index 72cb060..ddf88f0 100644
> --- a/drivers/usb/typec/fusb302/fusb302.c
> +++ b/drivers/usb/typec/fusb302/fusb302.c
> @@ -1650,12 +1650,6 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id)
>
> if (interrupta & FUSB_REG_INTERRUPTA_TX_SUCCESS) {
> fusb302_log(chip, "IRQ: PD tx success");
> - /* read out the received good CRC */
> - ret = fusb302_pd_read_message(chip, &pd_msg);
> - if (ret < 0) {
> - fusb302_log(chip, "cannot read in GCRC, ret=%d", ret);
> - goto done;
> - }

If multiple "Good CRC" messages are received in a row, they won't be read from
the chip, which might result in a buffer overflow.

It might be better to always read all pending messages and handle it depending
on the message type. Something along the line of

while (interrupts & (FUSB_REG_INTERRUPTA_TX_SUCCESS |
FUSB_REG_INTERRUPTB_GCRCSENT)) {
ret = fusb302_pd_read_message(chip, &pd_msg);
if (ret < 0)
goto done;
if (msg_type == good CRC) {
tcpm_pd_transmit_complete(chip->tcpm_port, TCPC_TX_SUCCESS);
interrupts &= ~FUSB_REG_INTERRUPTA_TX_SUCCESS;
} else {
tcpm_pd_receive(chip->tcpm_port, &pd_msg);
interrupts &= ~FUSB_REG_INTERRUPTB_GCRCSENT;
}
}

Guenter

> tcpm_pd_transmit_complete(chip->tcpm_port, TCPC_TX_SUCCESS);
> }
>
> @@ -1671,12 +1665,22 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id)
>
> if (interruptb & FUSB_REG_INTERRUPTB_GCRCSENT) {
> fusb302_log(chip, "IRQ: PD sent good CRC");
> +retry:
> ret = fusb302_pd_read_message(chip, &pd_msg);
> if (ret < 0) {
> fusb302_log(chip,
> "cannot read in PD message, ret=%d", ret);
> goto done;
> }
> +
> + /*
> + * Check to make sure we've not read off a GoodCRC message.
> + * If so then read again to retrieve expected message
> + */
> + if ((!pd_header_cnt_le(pd_msg.header)) &&
> + (pd_header_type_le(pd_msg.header) == PD_CTRL_GOOD_CRC))
> + goto retry;
> +
> tcpm_pd_receive(chip->tcpm_port, &pd_msg);
> }
> done:
> --
> 1.9.1
>

\
 
 \ /
  Last update: 2017-11-17 18:20    [W:0.070 / U:9.672 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site