nxp-mcuxpresso / mcux-sdk

MCUXpresso SDK
BSD 3-Clause "New" or "Revised" License
301 stars 136 forks source link

[BUG] ECSPI Burst Length Register field can be more than 8bit #97

Closed kmeinhar closed 1 year ago

kmeinhar commented 1 year ago

Describe the bug

The Reference Manual of i.MX 8M Plus (Link to Product Page) defines in section 10.1.6.3 (Control Register ECSPIx_CONREG) that the BURST_LENGTH field can be 12 bit (Bits 31 to 20). The relevant header that describes this bit field specifies only a maximum field size of uin8_t. See Here: https://github.com/NXPmicro/mcux-sdk/blob/main/drivers/ecspi/fsl_ecspi.h#L182 .

On my i.MX 8M Plus i can create bursts longer than 2^8 words, but only when I change the size of the burst length field to uint16_t.

Additionally the description of the BURST_LENGTH in the RM is a bit unclear. It state: A maximum of 2^12 bits can be transferred in a single SPI burst., but the example below this quote only show value for a maximum of 2^7 words: 0xFFF A SPI burst contains 2^7 words.

Expected behavior

The following line should be changed: https://github.com/NXPmicro/mcux-sdk/blob/main/drivers/ecspi/fsl_ecspi.h#L182

/*! @brief ECSPI slave configure structure.*/
typedef struct _ecspi_slave_config
{
    ecspi_channel_source_t channel;       /*Channel number */
-    uint8_t burstLength;                  /*!< Burst length */
+    uint16_t burstLength;                  /*!< Burst length */
    uint8_t txFifoThreshold;              /*!< TX Threshold */
    uint8_t rxFifoThreshold;              /*!< RX Threshold */
    ecspi_channel_config_t channelConfig; /*!< Channel configuration */
} ecspi_slave_config_t;

Additional context

It might be worth to consider adding a check that burstLength does not exceed the maximum 12 bit.

kmeinhar commented 1 year ago

This also effects other locations of the MCUX SDK. e.g. in the Zephyr RTOS HAL: https://github.com/zephyrproject-rtos/hal_nxp/blob/master/mcux/mcux-sdk/drivers/ecspi/fsl_ecspi.h#L182

mcuxsusan commented 1 year ago

Thanks for reporting this issue. Already forward the issue to the development team for check, reply could be delayed.

mcuxcc commented 1 year ago

This issue going to be closed, as it should be fixed in fb5ddea0fc1fe8a2c8a0a788c921c0a6d5cd2edb