| Message ID | 20200618233959.160032-2-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: spi-geni-qcom: Fixes / perf improvements | expand |
Hi, On Thu, Jun 18, 2020 at 4:40 PM Stephen Boyd <swboyd@chromium.org> wrote: > > We only need to test for these counters being non-zero when we see the > end of a transfer. If we're doing a CS change then they will already be > zero. This implies that we don't need to set these to 0 if we're > cancelling an in flight transfer too, because we only care to test these > counters when the 'DONE' bit is set in the hardware and we've set them > to non-zero for a transfer. > > This is a non-functional change, just cleanup to consolidate code. > > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > drivers/spi/spi-geni-qcom.c | 42 ++++++++++++++++++------------------- > 1 file changed, 21 insertions(+), 21 deletions(-) > > diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c > index 670f83793aa4..828cfc988a3f 100644 > --- a/drivers/spi/spi-geni-qcom.c > +++ b/drivers/spi/spi-geni-qcom.c > @@ -126,7 +126,6 @@ static void handle_fifo_timeout(struct spi_master *spi, > * came in while cancelling. > */ > mas->cur_xfer = NULL; > - mas->tx_rem_bytes = mas->rx_rem_bytes = 0; > geni_se_cancel_m_cmd(se); > spin_unlock_irq(&mas->lock); > > @@ -517,29 +516,30 @@ static irqreturn_t geni_spi_isr(int irq, void *data) > if (mas->cur_xfer) { > spi_finalize_current_transfer(spi); > mas->cur_xfer = NULL; > + /* > + * If this happens, then a CMD_DONE came before all the > + * Tx buffer bytes were sent out. This is unusual, log > + * this condition and disable the WM interrupt to > + * prevent the system from stalling due an interrupt > + * storm. > + * > + * If this happens when all Rx bytes haven't been > + * received, log the condition. The only known time > + * this can happen is if bits_per_word != 8 and some > + * registers that expect xfer lengths in num spi_words > + * weren't written correctly. > + */ > + if (mas->tx_rem_bytes) { > + writel(0, se->base + SE_GENI_TX_WATERMARK_REG); > + dev_err(mas->dev, "Premature done. tx_rem = %d bpw%d\n", > + mas->tx_rem_bytes, mas->cur_bits_per_word); > + } > + if (mas->rx_rem_bytes) > + dev_err(mas->dev, "Premature done. rx_rem = %d bpw%d\n", > + mas->rx_rem_bytes, mas->cur_bits_per_word); ...or we just remove these extra error-checks totally. ...but if we want to keep them: Reviewed-by: Douglas Anderson <dianders@chromium.org>
On Thu, Jun 18, 2020 at 04:39:59PM -0700, Stephen Boyd wrote: > We only need to test for these counters being non-zero when we see the > end of a transfer. If we're doing a CS change then they will already be > zero. This implies that we don't need to set these to 0 if we're This doesn't apply against current code, please check and resend.
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 670f83793aa4..828cfc988a3f 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -126,7 +126,6 @@ static void handle_fifo_timeout(struct spi_master *spi, * came in while cancelling. */ mas->cur_xfer = NULL; - mas->tx_rem_bytes = mas->rx_rem_bytes = 0; geni_se_cancel_m_cmd(se); spin_unlock_irq(&mas->lock); @@ -517,29 +516,30 @@ static irqreturn_t geni_spi_isr(int irq, void *data) if (mas->cur_xfer) { spi_finalize_current_transfer(spi); mas->cur_xfer = NULL; + /* + * If this happens, then a CMD_DONE came before all the + * Tx buffer bytes were sent out. This is unusual, log + * this condition and disable the WM interrupt to + * prevent the system from stalling due an interrupt + * storm. + * + * If this happens when all Rx bytes haven't been + * received, log the condition. The only known time + * this can happen is if bits_per_word != 8 and some + * registers that expect xfer lengths in num spi_words + * weren't written correctly. + */ + if (mas->tx_rem_bytes) { + writel(0, se->base + SE_GENI_TX_WATERMARK_REG); + dev_err(mas->dev, "Premature done. tx_rem = %d bpw%d\n", + mas->tx_rem_bytes, mas->cur_bits_per_word); + } + if (mas->rx_rem_bytes) + dev_err(mas->dev, "Premature done. rx_rem = %d bpw%d\n", + mas->rx_rem_bytes, mas->cur_bits_per_word); } else { complete(&mas->cs_done); } - - /* - * If this happens, then a CMD_DONE came before all the Tx - * buffer bytes were sent out. This is unusual, log this - * condition and disable the WM interrupt to prevent the - * system from stalling due an interrupt storm. - * If this happens when all Rx bytes haven't been received, log - * the condition. - * The only known time this can happen is if bits_per_word != 8 - * and some registers that expect xfer lengths in num spi_words - * weren't written correctly. - */ - if (mas->tx_rem_bytes) { - writel(0, se->base + SE_GENI_TX_WATERMARK_REG); - dev_err(mas->dev, "Premature done. tx_rem = %d bpw%d\n", - mas->tx_rem_bytes, mas->cur_bits_per_word); - } - if (mas->rx_rem_bytes) - dev_err(mas->dev, "Premature done. rx_rem = %d bpw%d\n", - mas->rx_rem_bytes, mas->cur_bits_per_word); } if (m_irq & M_CMD_CANCEL_EN)
We only need to test for these counters being non-zero when we see the end of a transfer. If we're doing a CS change then they will already be zero. This implies that we don't need to set these to 0 if we're cancelling an in flight transfer too, because we only care to test these counters when the 'DONE' bit is set in the hardware and we've set them to non-zero for a transfer. This is a non-functional change, just cleanup to consolidate code. Cc: Douglas Anderson <dianders@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/spi/spi-geni-qcom.c | 42 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-)