* [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
@ 2020-12-17 5:28 Bin Meng
2020-12-17 5:28 ` [PATCH 2/2] hw/ssi: imx_spi: Correct tx and rx fifo endianness Bin Meng
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Bin Meng @ 2020-12-17 5:28 UTC (permalink / raw)
To: Peter Maydell, Jean-Christophe Dubois, Alistair Francis,
qemu-arm, qemu-devel
Cc: Bin Meng
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;
+ }
rx = 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] hw/ssi: imx_spi: Correct tx and rx fifo endianness
2020-12-17 5:28 [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic Bin Meng
@ 2020-12-17 5:28 ` Bin Meng
2021-01-08 14:49 ` Peter Maydell
2020-12-22 6:30 ` [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic Bin Meng
2020-12-31 10:31 ` Philippe Mathieu-Daudé
2 siblings, 1 reply; 8+ messages in thread
From: Bin Meng @ 2020-12-17 5:28 UTC (permalink / raw)
To: Peter Maydell, Jean-Christophe Dubois, Alistair Francis,
qemu-arm, qemu-devel
Cc: Bin Meng
From: Bin Meng <bin.meng@windriver.com>
The endianness of data exchange between tx and rx fifo is incorrect.
Earlier bytes are supposed to show up on MSB and later bytes on LSB,
ie: in big endian. The manual does not explicitly say this, but the
U-Boot and Linux driver codes have a swap on the data transferred
to tx fifo and from rx fifo.
With this change, U-Boot read from / write to SPI flash tests pass.
=> sf test 1ff000 1000
SPI flash test:
0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
1 check: 3 ticks, 1333 KiB/s 10.664 Mbps
2 write: 235 ticks, 17 KiB/s 0.136 Mbps
3 read: 2 ticks, 2000 KiB/s 16.000 Mbps
Test passed
0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
1 check: 3 ticks, 1333 KiB/s 10.664 Mbps
2 write: 235 ticks, 17 KiB/s 0.136 Mbps
3 read: 2 ticks, 2000 KiB/s 16.000 Mbps
Fixes: c906a3a01582 ("i.MX: Add the Freescale SPI Controller")
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
hw/ssi/imx_spi.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index 509fb9f..71f0902 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -156,13 +156,14 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
{
uint32_t tx;
uint32_t rx;
+ uint32_t data;
+ uint8_t byte;
DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",
fifo32_num_used(&s->tx_fifo), fifo32_num_used(&s->rx_fifo));
while (!fifo32_is_empty(&s->tx_fifo)) {
int tx_burst = 0;
- int index = 0;
if (s->burst_length <= 0) {
s->burst_length = imx_spi_burst_length(s);
@@ -183,10 +184,18 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
tx_burst = 32;
}
+ data = 0;
+ for (int i = 0; i < tx_burst / 8; i++) {
+ byte = tx & 0xff;
+ tx = tx >> 8;
+ data = (data << 8) | byte;
+ }
+ tx = data;
+
rx = 0;
while (tx_burst > 0) {
- uint8_t byte = tx & 0xff;
+ byte = tx & 0xff;
DPRINTF("writing 0x%02x\n", (uint32_t)byte);
@@ -196,12 +205,11 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
DPRINTF("0x%02x read\n", (uint32_t)byte);
tx = tx >> 8;
- rx |= (byte << (index * 8));
+ rx = (rx << 8) | byte;
/* Remove 8 bits from the actual burst */
tx_burst -= 8;
s->burst_length -= 8;
- index++;
}
DPRINTF("data rx:0x%08x\n", rx);
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
2020-12-17 5:28 [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic Bin Meng
2020-12-17 5:28 ` [PATCH 2/2] hw/ssi: imx_spi: Correct tx and rx fifo endianness Bin Meng
@ 2020-12-22 6:30 ` Bin Meng
2020-12-30 23:51 ` Bin Meng
2020-12-31 10:31 ` Philippe Mathieu-Daudé
2 siblings, 1 reply; 8+ messages in thread
From: Bin Meng @ 2020-12-22 6:30 UTC (permalink / raw)
To: Peter Maydell, Jean-Christophe Dubois, Alistair Francis,
qemu-arm, qemu-devel@nongnu.org Developers
Cc: Bin Meng
On Thu, Dec 17, 2020 at 1:28 PM Bin Meng <bmeng.cn@gmail.com> 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(-)
>
Ping?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
2020-12-22 6:30 ` [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic Bin Meng
@ 2020-12-30 23:51 ` Bin Meng
0 siblings, 0 replies; 8+ messages in thread
From: Bin Meng @ 2020-12-30 23:51 UTC (permalink / raw)
To: Peter Maydell, Jean-Christophe Dubois, Alistair Francis,
qemu-arm, qemu-devel@nongnu.org Developers
Cc: Bin Meng
On Tue, Dec 22, 2020 at 2:30 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Thu, Dec 17, 2020 at 1:28 PM Bin Meng <bmeng.cn@gmail.com> 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(-)
> >
>
> Ping?
Ping?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
2020-12-17 5:28 [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic Bin Meng
2020-12-17 5:28 ` [PATCH 2/2] hw/ssi: imx_spi: Correct tx and rx fifo endianness Bin Meng
2020-12-22 6:30 ` [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic Bin Meng
@ 2020-12-31 10:31 ` Philippe Mathieu-Daudé
2021-01-06 6:06 ` Bin Meng
2 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-31 10:31 UTC (permalink / raw)
To: Bin Meng, Peter Maydell, Jean-Christophe Dubois,
Alistair Francis, qemu-arm, qemu-devel
Cc: Bin Meng
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;
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
2020-12-31 10:31 ` Philippe Mathieu-Daudé
@ 2021-01-06 6:06 ` Bin Meng
0 siblings, 0 replies; 8+ messages in thread
From: Bin Meng @ 2021-01-06 6:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Bin Meng, qemu-devel@nongnu.org Developers,
Jean-Christophe Dubois, qemu-arm, Alistair Francis
On Thu, Dec 31, 2020 at 6:31 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> 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;
Updated this in v2 series:
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=222931
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
Regards,
Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw/ssi: imx_spi: Correct tx and rx fifo endianness
2020-12-17 5:28 ` [PATCH 2/2] hw/ssi: imx_spi: Correct tx and rx fifo endianness Bin Meng
@ 2021-01-08 14:49 ` Peter Maydell
2021-01-09 2:13 ` Bin Meng
0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2021-01-08 14:49 UTC (permalink / raw)
To: Bin Meng
Cc: Bin Meng, qemu-arm, Alistair Francis, QEMU Developers,
Jean-Christophe Dubois
On Thu, 17 Dec 2020 at 05:28, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> The endianness of data exchange between tx and rx fifo is incorrect.
> Earlier bytes are supposed to show up on MSB and later bytes on LSB,
> ie: in big endian. The manual does not explicitly say this, but the
> U-Boot and Linux driver codes have a swap on the data transferred
> to tx fifo and from rx fifo.
To check my understanding, if we have a burst length of 16 bits, say,
when we do the fifo32_pop() of a 32 bit word, where in that
word and which way round are the 2 bytes we are going to transfer ?
> With this change, U-Boot read from / write to SPI flash tests pass.
>
> => sf test 1ff000 1000
> SPI flash test:
> 0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
> 1 check: 3 ticks, 1333 KiB/s 10.664 Mbps
> 2 write: 235 ticks, 17 KiB/s 0.136 Mbps
> 3 read: 2 ticks, 2000 KiB/s 16.000 Mbps
> Test passed
> 0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
> 1 check: 3 ticks, 1333 KiB/s 10.664 Mbps
> 2 write: 235 ticks, 17 KiB/s 0.136 Mbps
> 3 read: 2 ticks, 2000 KiB/s 16.000 Mbps
>
> Fixes: c906a3a01582 ("i.MX: Add the Freescale SPI Controller")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> hw/ssi/imx_spi.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
> index 509fb9f..71f0902 100644
> --- a/hw/ssi/imx_spi.c
> +++ b/hw/ssi/imx_spi.c
> @@ -156,13 +156,14 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
> {
> uint32_t tx;
> uint32_t rx;
> + uint32_t data;
> + uint8_t byte;
>
> DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",
> fifo32_num_used(&s->tx_fifo), fifo32_num_used(&s->rx_fifo));
>
> while (!fifo32_is_empty(&s->tx_fifo)) {
> int tx_burst = 0;
> - int index = 0;
>
> if (s->burst_length <= 0) {
> s->burst_length = imx_spi_burst_length(s);
> @@ -183,10 +184,18 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
> tx_burst = 32;
> }
>
> + data = 0;
> + for (int i = 0; i < tx_burst / 8; i++) {
> + byte = tx & 0xff;
> + tx = tx >> 8;
> + data = (data << 8) | byte;
> + }
> + tx = data;
> +
Why carefully reverse the order of bytes in the word and then
take a byte at a time from the bottom of the word in the loop below,
when you could change the loop to take bytes from the top of the word
instead ?
> rx = 0;
>
> while (tx_burst > 0) {
> - uint8_t byte = tx & 0xff;
> + byte = tx & 0xff;
>
> DPRINTF("writing 0x%02x\n", (uint32_t)byte);
>
> @@ -196,12 +205,11 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
> DPRINTF("0x%02x read\n", (uint32_t)byte);
>
> tx = tx >> 8;
> - rx |= (byte << (index * 8));
> + rx = (rx << 8) | byte;
>
> /* Remove 8 bits from the actual burst */
> tx_burst -= 8;
> s->burst_length -= 8;
> - index++;
> }
>
> DPRINTF("data rx:0x%08x\n", rx);
> --
> 2.7.4
thanks
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw/ssi: imx_spi: Correct tx and rx fifo endianness
2021-01-08 14:49 ` Peter Maydell
@ 2021-01-09 2:13 ` Bin Meng
0 siblings, 0 replies; 8+ messages in thread
From: Bin Meng @ 2021-01-09 2:13 UTC (permalink / raw)
To: Peter Maydell
Cc: Bin Meng, qemu-arm, Alistair Francis, QEMU Developers,
Jean-Christophe Dubois
Hi Peter,
On Fri, Jan 8, 2021 at 10:49 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 17 Dec 2020 at 05:28, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > The endianness of data exchange between tx and rx fifo is incorrect.
> > Earlier bytes are supposed to show up on MSB and later bytes on LSB,
> > ie: in big endian. The manual does not explicitly say this, but the
> > U-Boot and Linux driver codes have a swap on the data transferred
> > to tx fifo and from rx fifo.
>
> To check my understanding, if we have a burst length of 16 bits, say,
> when we do the fifo32_pop() of a 32 bit word, where in that
> word and which way round are the 2 bytes we are going to transfer ?
Say the fifo was written with a value of 0x00001234 when the burst
length is 16 bits, 0x12 will be transferred first then followed by
0x34.
>
> > With this change, U-Boot read from / write to SPI flash tests pass.
> >
> > => sf test 1ff000 1000
> > SPI flash test:
> > 0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
> > 1 check: 3 ticks, 1333 KiB/s 10.664 Mbps
> > 2 write: 235 ticks, 17 KiB/s 0.136 Mbps
> > 3 read: 2 ticks, 2000 KiB/s 16.000 Mbps
> > Test passed
> > 0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
> > 1 check: 3 ticks, 1333 KiB/s 10.664 Mbps
> > 2 write: 235 ticks, 17 KiB/s 0.136 Mbps
> > 3 read: 2 ticks, 2000 KiB/s 16.000 Mbps
> >
> > Fixes: c906a3a01582 ("i.MX: Add the Freescale SPI Controller")
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >
> > ---
> >
> > hw/ssi/imx_spi.c | 16 ++++++++++++----
> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
> > index 509fb9f..71f0902 100644
> > --- a/hw/ssi/imx_spi.c
> > +++ b/hw/ssi/imx_spi.c
> > @@ -156,13 +156,14 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
> > {
> > uint32_t tx;
> > uint32_t rx;
> > + uint32_t data;
> > + uint8_t byte;
> >
> > DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",
> > fifo32_num_used(&s->tx_fifo), fifo32_num_used(&s->rx_fifo));
> >
> > while (!fifo32_is_empty(&s->tx_fifo)) {
> > int tx_burst = 0;
> > - int index = 0;
> >
> > if (s->burst_length <= 0) {
> > s->burst_length = imx_spi_burst_length(s);
> > @@ -183,10 +184,18 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
> > tx_burst = 32;
> > }
> >
> > + data = 0;
> > + for (int i = 0; i < tx_burst / 8; i++) {
> > + byte = tx & 0xff;
> > + tx = tx >> 8;
> > + data = (data << 8) | byte;
> > + }
> > + tx = data;
> > +
>
> Why carefully reverse the order of bytes in the word and then
> take a byte at a time from the bottom of the word in the loop below,
> when you could change the loop to take bytes from the top of the word
> instead ?
Ah, yes, this can be rewritten to simplify a little.
>
> > rx = 0;
> >
> > while (tx_burst > 0) {
> > - uint8_t byte = tx & 0xff;
> > + byte = tx & 0xff;
> >
> > DPRINTF("writing 0x%02x\n", (uint32_t)byte);
> >
> > @@ -196,12 +205,11 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
> > DPRINTF("0x%02x read\n", (uint32_t)byte);
> >
> > tx = tx >> 8;
> > - rx |= (byte << (index * 8));
> > + rx = (rx << 8) | byte;
> >
> > /* Remove 8 bits from the actual burst */
> > tx_burst -= 8;
> > s->burst_length -= 8;
> > - index++;
> > }
> >
> > DPRINTF("data rx:0x%08x\n", rx);
> > --
Regards,
Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-09 2:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-17 5:28 [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic Bin Meng
2020-12-17 5:28 ` [PATCH 2/2] hw/ssi: imx_spi: Correct tx and rx fifo endianness Bin Meng
2021-01-08 14:49 ` Peter Maydell
2021-01-09 2:13 ` Bin Meng
2020-12-22 6:30 ` [PATCH 1/2] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic Bin Meng
2020-12-30 23:51 ` Bin Meng
2020-12-31 10:31 ` Philippe Mathieu-Daudé
2021-01-06 6:06 ` Bin Meng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.