Re: [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
On Thu, Dec 31, 2020 at 6:31 PM Philippe Mathieu-Daudé wrote:
>
> On 12/17/20 6:28 AM, Bin Meng wrote:
> > From: Bin Meng
> >
> > 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
> > ---
> >
> > 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;
Updated this in v2 series:
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=222931
>
> Reviewed-by: Philippe Mathieu-Daudé
>
Regards,
Bin
Re: [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
On 12/17/20 6:28 AM, Bin Meng wrote:
> From: Bin Meng
>
> 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
> ---
>
> 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é
>
> rx = 0;
>
>
Re: [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
On Tue, Dec 22, 2020 at 2:30 PM Bin Meng wrote:
>
> On Thu, Dec 17, 2020 at 1:28 PM Bin Meng wrote:
> >
> > From: Bin Meng
> >
> > 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
> > ---
> >
> > hw/ssi/imx_spi.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
>
> Ping?
Ping?
Re: [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
On Thu, Dec 17, 2020 at 1:28 PM Bin Meng wrote:
>
> From: Bin Meng
>
> 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
> ---
>
> hw/ssi/imx_spi.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
Ping?
[PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
From: Bin Meng
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
---
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;
+}
rx = 0;
--
2.7.4
