Hi Walter
On 02/09/2017 04:59 PM, Tony Lindgren wrote:
> Hi,
>
> * Walter Wagner <walter.wagner@xxxxxxxxxx> [170209 00:40]:
>> Dear Tony Lindgren,
>>
>> I hope I'm right here and You are responsible for drivers/spi/spi-omap2-mcspi.c too.
>
> Well I don't know much about the spi-omap2-mcspi.c driver.. But as
> Franklin, Peter and Micheal have been patching it, they might have a
> better idea what's going on. I've added them to Cc.
>
>> To the history:
>>
>> We use TI AM33xx/AM43xx SOCs, especially SPI interface under kernel 4.1.20.
>> Testing a device with very low baudrate (1,5 KHz) I made following experience:
>>
>> 1. The last byte was not always sure received --> sometimes I got timeout
So the only time your missing the last byte is when a timeout occurs?
>>
>> 2. The telegrams we have contain < 160 bytes, but we need DMA mode and not PIO (because CPU load)
>>
>> Therefore I've changed the driver.
>> I hope these changes help other developer.
>>
>> I've attached the patch and the files itself for simple comparing.
>
> Please check Documentation/SubmittingPatches and CodingStyle etc,
> also note that with git you can easily generate patches :)
>
> I've attached your diff below for easier reading.
>
> Regards,
>
> Tony
>
> 8< --------------------
> --- a/drivers/spi/spi-omap2-mcspi.c 2016-03-17 19:11:03.000000000 +0100
> +++ b/drivers/spi/spi-omap2-mcspi.c 2017-02-06 10:35:02.518061500 +0100
> @@ -116,6 +116,11 @@
> */
> #define DMA_MIN_BYTES 160
>
> +/* Default value of delay at waiting for a bit in a register.
> + * Needed to avoid latency at waiting.
> + */
> +#define BUSY_WAIT_DELAY 1 /* in ms */
> +
>
> /*
> * Used for context save and restore, structure members to be updated whenever
> @@ -138,6 +143,8 @@
> struct omap2_mcspi_regs ctx;
> int fifo_depth;
> unsigned int pin_dir:1;
> + unsigned long dma_min_bytes;
> + unsigned long busy_wait_delay;
Can't you calculate a value for this based on the SPI transaction speed
instead of making this user select able?
> };
>
> struct omap2_mcspi_cs {
> @@ -363,6 +370,23 @@
> return 0;
> }
>
> +static int mcspi_wait_for_reg_bit_interruptible(void __iomem *reg, unsigned long bit, unsigned long delay)
> +{
> + unsigned long timeout;
> +
> + timeout = jiffies + msecs_to_jiffies(1000);
> + while (!(readl_relaxed(reg) & bit)) {
> + if (time_after(jiffies, timeout)) {
> + if (!(readl_relaxed(reg) & bit))
> + return -ETIMEDOUT;
> + else
> + return 0;
> + }
> + msleep_interruptible(delay);
> + }
> + return 0;
> +}
> +
> static void omap2_mcspi_rx_callback(void *data)
> {
> struct spi_device *spi = data;
> @@ -496,8 +520,9 @@
> if (l & OMAP2_MCSPI_CHCONF_TURBO) {
> elements--;
>
> - if (likely(mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHSTAT0)
> - & OMAP2_MCSPI_CHSTAT_RXS)) {
> + if (mcspi_wait_for_reg_bit_interruptible(cs->base + OMAP2_MCSPI_CHSTAT0,
> + OMAP2_MCSPI_CHSTAT_RXS, mcspi->busy_wait_delay) == 0) {
> + /* reading completed */
> u32 w;
>
> w = mcspi_read_cs_reg(spi, OMAP2_MCSPI_RX0);
> @@ -508,6 +533,7 @@
> else /* word_len <= 32 */
> ((u32 *)xfer->rx_buf)[elements++] = w;
> } else {
> + /* timeout */
> int bytes_per_word = mcspi_bytes_per_word(word_len);
> dev_err(&spi->dev, "DMA RX penultimate word empty\n");
> count -= (bytes_per_word << 1);
> @@ -515,8 +541,9 @@
> return count;
> }
> }
> - if (likely(q(spi, OMAP2_MCSPI_CHSTAT0)
> - & OMAP2_MCSPI_CHSTAT_RXS)) {
> + if (mcspi_wait_for_reg_bit_interruptible(cs->base + OMAP2_MCSPI_CHSTAT0,
> + OMAP2_MCSPI_CHSTAT_RXS, mcspi->busy_wait_delay) == 0) {
> + /* reading completed */
> u32 w;
>
> w = mcspi_read_cs_reg(spi, OMAP2_MCSPI_RX0);
> @@ -527,6 +554,7 @@
> else /* word_len <= 32 */
> ((u32 *)xfer->rx_buf)[elements] = w;
> } else {
> + /* timeout */
> dev_err(&spi->dev, "DMA RX last word empty\n");
> count -= mcspi_bytes_per_word(word_len);
> }
> @@ -1141,7 +1169,7 @@
> unsigned count;
>
> if ((mcspi_dma->dma_rx && mcspi_dma->dma_tx) &&
> - (m->is_dma_mapped || t->len >= DMA_MIN_BYTES))
> + (m->is_dma_mapped || t->len >= mcspi->dma_min_bytes))
> omap2_mcspi_set_fifo(spi, t, 1);
>
> omap2_mcspi_set_enable(spi, 1);
> @@ -1152,7 +1180,7 @@
> + OMAP2_MCSPI_TX0);
>
> if ((mcspi_dma->dma_rx && mcspi_dma->dma_tx) &&
> - (m->is_dma_mapped || t->len >= DMA_MIN_BYTES))
> + (m->is_dma_mapped || t->len >= mcspi->dma_min_bytes))
> count = omap2_mcspi_txrx_dma(spi, t);
> else
> count = omap2_mcspi_txrx_pio(spi, t);
> @@ -1234,7 +1262,7 @@
> goto out;
> }
>
> - if (m->is_dma_mapped || len < DMA_MIN_BYTES)
> + if (m->is_dma_mapped || len < mcspi->dma_min_bytes)
> continue;
>
> if (mcspi_dma->dma_tx && tx_buf != NULL) {
> @@ -1375,6 +1403,11 @@
> master->bus_num = pdev->id;
> mcspi->pin_dir = pdata->pin_dir;
> }
> + mcspi->dma_min_bytes = DMA_MIN_BYTES; /* default value */
> + of_property_read_u32(node, "ti,dma-min-bytes", &mcspi->dma_min_bytes);
> + mcspi->busy_wait_delay = BUSY_WAIT_DELAY; /* default value */
> + of_property_read_u32(node, "ti,busy-wait-delay", &mcspi->busy_wait_delay);
> +
> regs_offset = pdata->regs_offset;
>
> r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
