On Tue, Nov 21, 2017 at 02:12:12PM +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 updates the message reading
> code to check whether a GoodCRC message was received or not. Based
> on this check it will either report that the previous transmission
> has completed or it will pass the msg data to TCPM for futher
> processing. This way the incorrect ordering of the events no longer
> matters.
>
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx>
FWIW:
Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
>
> Changes in v3:
> - Always read from FIFO on TX_SUCCES and GCRCSENT events, but decision on how
> to report to TCPM is no longer based on these event types but instead on type
> of message read in from FIFO.
>
> Changes in v2:
> - Remove erroneous extended header check
>
> Patch is based on Linux next-20171114 to include move out of staging.
>
> drivers/usb/typec/fusb302/fusb302.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/typec/fusb302/fusb302.c b/drivers/usb/typec/fusb302/fusb302.c
> index 72cb060..d200085 100644
> --- a/drivers/usb/typec/fusb302/fusb302.c
> +++ b/drivers/usb/typec/fusb302/fusb302.c
> @@ -1543,6 +1543,21 @@ static int fusb302_pd_read_message(struct fusb302_chip *chip,
> fusb302_log(chip, "PD message header: %x", msg->header);
> fusb302_log(chip, "PD message len: %d", len);
>
> + /*
> + * Check if we've read off a GoodCRC message. If so then indicate to
> + * TCPM that the previous transmission has completed. Otherwise we pass
> + * the received message over to TCPM for processing.
> + *
> + * We make this check here instead of basing the reporting decision on
> + * the IRQ event type, as it's possible for the chip to report the
> + * TX_SUCCESS and GCRCSENT events out of order on occasion, so we need
> + * to check the message type to ensure correct reporting to TCPM.
> + */
> + if ((!len) && (pd_header_type_le(msg->header) == PD_CTRL_GOOD_CRC))
> + tcpm_pd_transmit_complete(chip->tcpm_port, TCPC_TX_SUCCESS);
> + else
> + tcpm_pd_receive(chip->tcpm_port, msg);
> +
> return ret;
> }
>
> @@ -1650,13 +1665,12 @@ 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);
> + fusb302_log(chip,
> + "cannot read in PD message, ret=%d", ret);
> goto done;
> }
> - tcpm_pd_transmit_complete(chip->tcpm_port, TCPC_TX_SUCCESS);
> }
>
> if (interrupta & FUSB_REG_INTERRUPTA_HARDRESET) {
> @@ -1677,7 +1691,6 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id)
> "cannot read in PD message, ret=%d", ret);
> goto done;
> }
> - tcpm_pd_receive(chip->tcpm_port, &pd_msg);
> }
> done:
> mutex_unlock(&chip->lock);
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
