From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966052AbdKQRUU (ORCPT ); Fri, 17 Nov 2017 12:20:20 -0500 Received: from mail-pf0-f194.google.com ([209.85.192.194]:45782 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759678AbdKQRUD (ORCPT ); Fri, 17 Nov 2017 12:20:03 -0500 X-Google-Smtp-Source: AGs4zMa9L+9SJ3+jPf9JwvC0cG+YgrazZwX+6aKyC5f9nwnNDo+1y0ssCvNnnpfw4Owr91xn1f10HA== Date: Fri, 17 Nov 2017 09:20:00 -0800 From: Guenter Roeck To: Adam Thomson Cc: Heikki Krogerus , Greg Kroah-Hartman , Hans de Goede , Yueyao Zhu , Rui Miguel Silva , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, support.opensource@diasemi.com Subject: Re: [PATCH v2] typec: tcpm: fusb302: Resolve out of order messaging events Message-ID: <20171117172000.GA4346@roeck-us.net> References: <20171116162811.A5D673FBDF@swsrvapps-01.diasemi.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171116162811.A5D673FBDF@swsrvapps-01.diasemi.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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 >