On Fri, May 29, 2020 at 12:46:48PM +0300, Andy Shevchenko wrote:

On Fri, May 29, 2020 at 06:59:03AM +0300, Serge Semin wrote:

quoted

Having any data left in the Rx FIFO after the DMA engine claimed it has
finished all DMA transactions is an abnormal situation, since the DW SPI
controller driver expects to have all the data being fetched and placed
into the SPI Rx buffer at that moment. In case if this has happened we
assume that DMA engine still may be doing the data fetching, thus we give
it sometime to finish. If after a short period of time the data is still
left in the Rx FIFO, the driver will give up waiting and return an error
indicating that the SPI controller/DMA engine must have hung up or failed
at some point of doing their duties.

...

quoted

+static int dw_spi_dma_wait_rx_done(struct dw_spi *dws)
+{
+	int retry = WAIT_RETRIES;
+	struct spi_delay delay;
+	unsigned long ns, us;
+	u32 nents;
+
+	/*
+	 * It's unlikely that DMA engine is still doing the data fetching, but
+	 * if it's let's give it some reasonable time. The timeout calculation
+	 * is based on the synchronous APB/SSI reference clock rate, on a
+	 * number of data entries left in the Rx FIFO, times a number of clock
+	 * periods normally needed for a single APB read/write transaction
+	 * without PREADY signal utilized (which is true for the DW APB SSI
+	 * controller).
+	 */
+	nents = dw_readl(dws, DW_SPI_RXFLR);

quoted

+	ns = NSEC_PER_SEC / dws->max_freq * 4 * nents;

I think we may slightly increase precision by writing this like

	ns = 4 * NSEC_PER_SEC / dws->max_freq * nents;

Good point. Although both 4 and NSEC_PER_SEC are signed. The later is
1000000000L. Formally speaking on x32 systems (4 * 1000 000 000L) equals
to a negative value. Though overflow still won't happen so the result will
be correct. Anyway to be on a safe side it would be better to use an explicit
unsigned literal:

+       ns = 4U * NSEC_PER_SEC / dws->max_freq * nents;

-Sergey

quoted

+	if (ns <= NSEC_PER_USEC) {
+		delay.unit = SPI_DELAY_UNIT_NSECS;
+		delay.value = ns;
+	} else {
+		us = DIV_ROUND_UP(ns, NSEC_PER_USEC);
+		delay.unit = SPI_DELAY_UNIT_USECS;
+		delay.value = clamp_val(us, 0, USHRT_MAX);
+	}
+
+	while (dw_spi_dma_rx_busy(dws) && retry--)
+		spi_delay_exec(&delay, NULL);
+
+	if (retry < 0) {
+		dev_err(&dws->master->dev, "Rx hanged up\n");
+		return -EIO;
+	}
+
+	return 0;
+}

-- 
With Best Regards,
Andy Shevchenko