Please check SPI_MODE1 and SPI_MODE2 - they appear to be swapped

[PaulZC]

Hi gang,
I'm testing some existing SPI code on v2.0.6 of the core. It uses SPI_MODE1 (CPOL=0 and CPHA=1) and is failing... I can see on the 'scope that CPOL is 1, not 0.
If I use SPI_MODE2 instead, it works.
I suspect modes 1 and 2 are swapped?
Looking at am_hal_iom.h, the mode ordering (0,2,1,3) matches the ordering of the SPHA and SPOL bits in the IOM.MSPICFG register.
I'm wondering if we need to swap modes 1 and 2 when using the modes as defined in HardwareSPI.h?
Thanks,
Paul

[paulvha]

I think there is more change needed. The value of SPI_MODEx is loaded in the IOM. MSPICFG and it not setting for the right CPOL and CPHA as PaulZC has pointed out. In HardwareSPI.h there needs to change. It now shows:

typedef enum {
  SPI_MODE0 = 0,
  SPI_MODE1 = 1,
  SPI_MODE2 = 2,
  SPI_MODE3 = 3,
} SPIMode;

But it should show

typedef enum {
  SPI_MODE0 = 0,
  SPI_MODE3,
  SPI_MODE1,
  SPI_MODE2
} SPIMode;

As this setting will lead to :

typedef enum {          CPOL        CPHA       SPI standard
  SPI_MODE0 = 0,         0             0               0
  SPI_MODE3,             1             0               3
  SPI_MODE1,             0             1               1   
  SPI_MODE2              1             1               2 
} SPIMode;

In am_hal_iom.h the definition is also INCORRECT :

typedef enum
{
    AM_HAL_IOM_SPI_MODE_0,                  // CPOL = 0; CPHA = 0
    AM_HAL_IOM_SPI_MODE_2,                  // CPOL = 1; CPHA = 0
    AM_HAL_IOM_SPI_MODE_1,                  // CPOL = 0; CPHA = 1
    AM_HAL_IOM_SPI_MODE_3,                  // CPOL = 1; CPHA = 1
} am_hal_iom_spi_mode_e;

Here they are mixing the SPI- Standard MOD2 and MOD3 (see the definition on https://www.analog.com/en/analog-dialogue/articles/introduction-to-spi-interface.html)

I have checked the setting with external monitor to confirm
regards,
Paulvha

[Wenn0101]

Hey @paulvha
There appears to be some disagreement on mode 2 and 3.
To clarify the work I have done on this issue so far, since I was bad and didn't post and update here. In this commit I added a translation between the HAL layer and the layers above it. As you noted the enum in the hal states.

typedef enum
{
    AM_HAL_IOM_SPI_MODE_0,                  // CPOL = 0; CPHA = 0
    AM_HAL_IOM_SPI_MODE_2,                  // CPOL = 1; CPHA = 0
    AM_HAL_IOM_SPI_MODE_1,                  // CPOL = 0; CPHA = 1
    AM_HAL_IOM_SPI_MODE_3,                  // CPOL = 1; CPHA = 1
} am_hal_iom_spi_mode_e;

This results in MODE_0 = 0, MODE_2 = 1, MODE_1=2, and MODE_3=3, such that the bits in the mode correspond with the bits in the register it will be stored in.
However the function in the mbed that would change the mode would just accept an integer and set this as the mode

obj->spi.iom_obj.iom.cfg.eSpiMode = (am_hal_iom_spi_mode_e)mode;

So an input of 1 as mode would lead to the HAL setting the mode to AM_HAL_IOM_SPI_MODE_2. I fixed this in the mbed driver with:

switch(mode){
        case 0:
            obj->spi.iom_obj.iom.cfg.eSpiMode = AM_HAL_IOM_SPI_MODE_0;
            break;
        case 1:
            obj->spi.iom_obj.iom.cfg.eSpiMode = AM_HAL_IOM_SPI_MODE_1;
            break;
        case 2:
            obj->spi.iom_obj.iom.cfg.eSpiMode = AM_HAL_IOM_SPI_MODE_2;
            break;
        case 3:
            obj->spi.iom_obj.iom.cfg.eSpiMode = AM_HAL_IOM_SPI_MODE_3;
            break;

    }

This fixes it such that there is no longer discrepancy between what is listed in the HAL as the SPI modes and what the spi modes actually are after calling this function.

Now to the issue that you have brought up. Your source (which is also the first result in a google search for me), has this:

• SPI_MODE_0 - CPOL = 0, CPHA = 0
• SPI_MODE_1 - CPOL = 0, CPHA = 1
• SPI_MODE_2 - CPOL = 1, CPHA = 1
• SPI_MODE_3 - CPOL = 1, CPHA = 0

But the HAL and the datasheet have:

• SPI_MODE_0 - CPOL = 0, CPHA = 0
• SPI_MODE_1 - CPOL = 0, CPHA = 1
• SPI_MODE_2 - CPOL = 1, CPHA = 0
• SPI_MODE_3 - CPOL = 1, CPHA = 1

Right now I mostly finding other sources that back up the datasheet, but I am open to more discussion on this topic

[paulvha]

Thanks. You are correct. I have found a number of other articles that confirm you findings. https://www.corelis.com/education/tutorials/spi-tutorial/ and http://dlnware.com/theory/SPI-Transfer-Modes.

Seems next time I need to double-check whether the first found information is correct :-)
regards,
Paul

[Wenn0101]

To be fair, you at least checked. I just saw what was in the datasheet and the code comments and stopped there. I didn't look for other sources until after you commented. :D