Re: [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic

[Philippe Mathieu-Daudé]

On 12/17/20 6:28 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> For the ECSPIx_CONREG register BURST_LENGTH field, the manual says:
> 
> 0x020 A SPI burst contains the 1 LSB in first word and all 32 bits in second 
> word.
> 0x021 A SPI burst contains the 2 LSB in first word and all 32 bits in second 
> word.
> 
> Current logic uses either s->burst_length or 32, whichever smaller,
> to determine how many bits it should read from the tx fifo each time.
> For example, for a 48 bit burst length, current logic transfers the
> first 32 bit from the first word in the tx fifo, followed by a 16
> bit from the second word in the tx fifo, which is wrong. The correct
> logic should be: transfer the first 16 bit from the first word in
> the tx fifo, followed by a 32 bit from the second word in the tx fifo.
> 
> With this change, SPI flash can be successfully probed by U-Boot on
> imx6 sabrelite board.
> 
>   => sf probe
>   SF: Detected sst25vf016b with page size 256 Bytes, erase size 4 KiB, total 
> 2 MiB
> 
> Fixes: c906a3a01582 ("i.MX: Add the Freescale SPI Controller")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>  hw/ssi/imx_spi.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
> index 85c172e..509fb9f 100644
> --- a/hw/ssi/imx_spi.c
> +++ b/hw/ssi/imx_spi.c
> @@ -178,7 +178,10 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>  
>          DPRINTF("data tx:0x%08x\n", tx);
>  
> -        tx_burst = MIN(s->burst_length, 32);
> +        tx_burst = s->burst_length % 32;
> +        if (tx_burst == 0) {
> +            tx_burst = 32;
> +        }

Or alternatively using ternary operator:

           tx_burst = (s->burst_length % 32) ?: 32;

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>  
>          rx = 0;
>  
>