[7/5] spi: spi-geni-qcom: Don't set {tx,rx}_rem_bytes unnecessarily
diff mbox series

Message ID 20200618233959.160032-2-swboyd@chromium.org
State New
Headers show
Series
  • spi: spi-geni-qcom: Fixes / perf improvements
Related show

Commit Message

Stephen Boyd June 18, 2020, 11:39 p.m. UTC
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(-)

Comments

Doug Anderson June 19, 2020, 12:40 a.m. UTC | #1
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>
Mark Brown June 19, 2020, 3:24 p.m. UTC | #2
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.

Patch
diff mbox series

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)