i.MX6 ecspi race condition with CPOL=1 and CS GPIO

[tbueno]

Hi,

I am having an issue with the imx-spi driver on linux imx branch 4.9.88.

On one of my ecspi channel (i.MX6 quad), I am connecting a SPI slave device that works in mode 3 (CPOL=1, CPHA=1). This device is software controlled by a Linux driver that exchange commands with device during probe.

It seems that just after reset of the ecspi controller, the default SCLK_CTL value (SCLK inactive state) is 0 (stay low) in the CONFIGREG. So by setting the control register CONREG to bring out the ecspi from reset state just before the CONFIGREG, my clock line goes from high to low, then back to high again as the correct SCLK_CTL value propagates in the controller.

As the GPIO chip select is asserted before executing mx51_ecspi_config in spi-imx.c, this counts as a valid clock tick for my SPI device, which sets in an unknown state that prevents its device driver to probe it correctly. This happens whatever device driver the SPI bus is bound to (spidev, custom driver, ...), as can be seen on this capture:

The first clock "tick" is the effect I described, which can be verified by adding an extra delay between in spi-imx driver between CONREG and CONFIGREG writes:

    /* CTRL register always go first to bring out controller from reset */

    writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL);

    udelay(50);

    reg = readl(spi_imx->base + MX51_ECSPI_TESTREG);

    if (spi->mode & SPI_LOOP)

        reg |= MX51_ECSPI_TESTREG_LBC;

    else

        reg &= ~MX51_ECSPI_TESTREG_LBC;

    writel(reg, spi_imx->base + MX51_ECSPI_TESTREG);

    writel(cfg, spi_imx->base + MX51_ECSPI_CONFIG);

Trace with extra delay:

After this first transaction, ecspi controller works as expected and this "ghost" clock tick does not appear as the CONFIGREG already has the correct CPOL configuration. I believe this issue could appear again when dealing with multiple SPI slave devices on the same channel with different chip selects, which is not my case here.

Is this a known issue ? Is were a workaround to prevent this race condition  from happening ? I don't think the GPIO CS line can be asserted after the ecspi configuration in current driver implementation.

I'd also like to add that in linux-imx 4.9.88, the GPIO CS would not even work properly without me backporting these two [1] [2] patches from linux-imx 4.14.78.

Thanks for your help and best regards,

Théo Bueno.

[jian_jiang]

Hi Théo,

There's a temporary solution, add following 2 lines in file drivers/spi/spi-imx.c :

+writel(MX51_ECSPI_CTRL_ENABLE, spi_imx->base + MX51_ECSPI_CTRL);

+writel(0x00x00000, spi_imx->base + MX51_ECSPI_CONFIG);

dev_info(&pdev->dev, "probed\n");

Note: the second 'x' in 0x00x00000 according to the SS the customer use. E.g. If using channel 0, it is 0x00100000.

All the Best

Kane

[ankitr_patel]

Hi Théo Bueno,

After looking into the issue, It looks like that this is related to a known issue that GPIO chip select trigger before clock configure and change the polarity.

This issue is mentioned in the spi-imx.c driver across the most Linux Versions. There is a detail description of this behavior on Linux patch spi: spi-imx: Fix out-of-order CS/SCLK operation at low speeds - Patchwork.

It's a delay in SPI clock configuration before code executes GPIO chip select when SPI have a lower clock. I suspect you are using lower SPI frequency (< 100Khz). And that cause this issue though it's prevention implemented in the spi-imx.c

I assume you had put more udelay(50); in the path is just adding the more delay inline with this 2 tick of SPI clock as mentioned in the above image. Also, It looks like this patch is based on the clock is less than or greater than 100Khz. which is the SPI frequency you are using?

Meanwhile, Could you try with usleep_range(delay, delay + 50); in the else part(on the base of an assumption that SPI clock is < 100Khz at your side)? Your analysis will help to improvise this PATCH.

Regards

Ankit

[tbueno]

Hi,

I am using a frequency of 1 MHz, which is far above the lower 100-200 kHz speeds described in this patch. I tested faster frequencies such as 5 and 10 MHz, and still had this exact issue.

The line in my device tree as reference:

        spi-max-frequency = <1000000>;

Adding a delay after code snippet you mentioned does not help, as the GPIO CS line is asserted before writing into ecspi control/configuration registers:

With vanilla spi-imx code:

After udelay patch:

     * the SPI communication as the device on the other end would consider

     * the change of SCLK polarity as a clock tick already.

     */

    delay = (2 * 1000000) / clk;

    if (likely(delay < 10)) /* SCLK is faster than 100 kHz */

        udelay(delay);

    else            /* SCLK is _very_ slow */

        usleep_range(delay, delay + 10);

    udelay(50);

    /*

     * Configure the DMA register: setup the watermark

I added extra delay of 50 µs regardless of the branch taken by condition you mentionned. On the capture the GPIO CS line is seen asserted before clock polarity change. Extra delay can be seen between change and transaction.

Thanks and best regards,

Théo Bueno.

[jian_jiang]

Hi Théo Bueno,

Have you tried with branch 4.9.88_2.0.0_ga? Seem the patch of "spi-imx: Fix out-of-order CS/SCLK operation at low speeds" is included in it.

All the Best

Kane

[tbueno]

Hi Kane,

Yes, I am running branch 4.9.88_2.0.0_ga.

The patch "spi-imx: Fix out-of-order CS/SCLK operation at low speeds"  you mentioned in the one introducing the feature we've been discussing with Ankit Patel. It seems this patch does not have any effect in my case.

Also, as I said I am running SPI on high frequencies (more than 1MHz).

Thanks and best regards,

Théo.

[jian_jiang]

Hi Théo,

I see now. Thanks!

Could you try the imx_4.14.78_1.0.0_ga release? There's some other commit about the drivers/spi/spi-imx.c file, not sure whether they are useful to this case.