From 1da79ecc7a299a6f3633876c8e49e5418ae37fcf Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Fri, 29 Jan 2021 21:23:14 +0800 Subject: hw/ssi: imx_spi: Use a macro for number of chip selects supported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid using a magic number (4) everywhere for the number of chip selects supported. Signed-off-by: Bin Meng Reviewed-by: Alistair Francis Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela Message-id: 20210129132323.30946-2-bmeng.cn@gmail.com Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index d8885ae454..e605049a21 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -361,7 +361,7 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value, /* We are in master mode */ - for (i = 0; i < 4; i++) { + for (i = 0; i < ECSPI_NUM_CS; i++) { qemu_set_irq(s->cs_lines[i], i == imx_spi_selected_channel(s) ? 0 : 1); } @@ -424,7 +424,7 @@ static void imx_spi_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem); sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); - for (i = 0; i < 4; ++i) { + for (i = 0; i < ECSPI_NUM_CS; ++i) { sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->cs_lines[i]); } -- cgit 1.4.1 From 3c9829e57468f3a53078aa2e10d35afde3208b36 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Fri, 29 Jan 2021 21:23:15 +0800 Subject: hw/ssi: imx_spi: Remove imx_spi_update_irq() in imx_spi_reset() Usually the approach is that the device on the other end of the line is going to reset its state anyway, so there's no need to actively signal an irq line change during the reset hook. Move imx_spi_update_irq() out of imx_spi_reset(), to a new function imx_spi_soft_reset() that is called when the controller is disabled. Signed-off-by: Bin Meng Reviewed-by: Peter Maydell Message-id: 20210129132323.30946-3-bmeng.cn@gmail.com Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index e605049a21..4d488b159a 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -241,11 +241,16 @@ static void imx_spi_reset(DeviceState *dev) imx_spi_rxfifo_reset(s); imx_spi_txfifo_reset(s); - imx_spi_update_irq(s); - s->burst_length = 0; } +static void imx_spi_soft_reset(IMXSPIState *s) +{ + imx_spi_reset(DEVICE(s)); + + imx_spi_update_irq(s); +} + static uint64_t imx_spi_read(void *opaque, hwaddr offset, unsigned size) { uint32_t value = 0; @@ -351,8 +356,9 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value, s->regs[ECSPI_CONREG] = value; if (!imx_spi_is_enabled(s)) { - /* device is disabled, so this is a reset */ - imx_spi_reset(DEVICE(s)); + /* device is disabled, so this is a soft reset */ + imx_spi_soft_reset(s); + return; } -- cgit 1.4.1 From 9c431a43a62255402a6bbe9a01b0464e73b30fe4 Mon Sep 17 00:00:00 2001 From: Philippe Mathieu-Daudé Date: Fri, 29 Jan 2021 21:23:16 +0800 Subject: hw/ssi: imx_spi: Remove pointless variable initialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'burst_length' is cleared in imx_spi_reset(), which is called after imx_spi_realize(). Remove the initialization to simplify. Reviewed-by: Juan Quintela Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Bin Meng Signed-off-by: Bin Meng Message-id: 20210129132323.30946-4-bmeng.cn@gmail.com Message-Id: <20210115153049.3353008-3-f4bug@amsat.org> Reviewed-by: Bin Meng Signed-off-by: Bin Meng Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index 4d488b159a..8fb3c9b6d1 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -434,8 +434,6 @@ static void imx_spi_realize(DeviceState *dev, Error **errp) sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->cs_lines[i]); } - s->burst_length = 0; - fifo32_create(&s->tx_fifo, ECSPI_FIFO_SIZE); fifo32_create(&s->rx_fifo, ECSPI_FIFO_SIZE); } -- cgit 1.4.1 From 93722b6f6a6ef0ab0544f20440a2f6b951103dcb Mon Sep 17 00:00:00 2001 From: Philippe Mathieu-Daudé Date: Fri, 29 Jan 2021 21:23:17 +0800 Subject: hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the block is disabled, all registers are reset with the exception of the ECSPI_CONREG. It is initialized to zero when the instance is created. Ref: i.MX 6DQ Applications Processor Reference Manual (IMX6DQRM), chapter 21.7.3: Control Register (ECSPIx_CONREG) Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-id: 20210129132323.30946-5-bmeng.cn@gmail.com [bmeng: add a 'common_reset' function that does most of reset operation] Signed-off-by: Bin Meng Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index 8fb3c9b6d1..e85be6ae60 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -228,15 +228,23 @@ static void imx_spi_flush_txfifo(IMXSPIState *s) fifo32_num_used(&s->tx_fifo), fifo32_num_used(&s->rx_fifo)); } -static void imx_spi_reset(DeviceState *dev) +static void imx_spi_common_reset(IMXSPIState *s) { - IMXSPIState *s = IMX_SPI(dev); - - DPRINTF("\n"); - - memset(s->regs, 0, sizeof(s->regs)); + int i; - s->regs[ECSPI_STATREG] = 0x00000003; + for (i = 0; i < ARRAY_SIZE(s->regs); i++) { + switch (i) { + case ECSPI_CONREG: + /* CONREG is not updated on soft reset */ + break; + case ECSPI_STATREG: + s->regs[i] = 0x00000003; + break; + default: + s->regs[i] = 0; + break; + } + } imx_spi_rxfifo_reset(s); imx_spi_txfifo_reset(s); @@ -246,11 +254,19 @@ static void imx_spi_reset(DeviceState *dev) static void imx_spi_soft_reset(IMXSPIState *s) { - imx_spi_reset(DEVICE(s)); + imx_spi_common_reset(s); imx_spi_update_irq(s); } +static void imx_spi_reset(DeviceState *dev) +{ + IMXSPIState *s = IMX_SPI(dev); + + imx_spi_common_reset(s); + s->regs[ECSPI_CONREG] = 0; +} + static uint64_t imx_spi_read(void *opaque, hwaddr offset, unsigned size) { uint32_t value = 0; -- cgit 1.4.1 From 7c87bb5333f0fdb17fee7e52acff1d915a68857e Mon Sep 17 00:00:00 2001 From: Philippe Mathieu-Daudé Date: Fri, 29 Jan 2021 21:23:18 +0800 Subject: hw/ssi: imx_spi: Rework imx_spi_read() to handle block disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the block is disabled, it stay it is 'internal reset logic' (internal clocks are gated off). Reading any register returns its reset value. Only update this value if the device is enabled. Ref: i.MX 6DQ Applications Processor Reference Manual (IMX6DQRM), chapter 21.7.3: Control Register (ECSPIx_CONREG) Reviewed-by: Juan Quintela Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Bin Meng Signed-off-by: Bin Meng Message-id: 20210129132323.30946-6-bmeng.cn@gmail.com Message-Id: <20210115153049.3353008-5-f4bug@amsat.org> Reviewed-by: Bin Meng Signed-off-by: Bin Meng Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 60 +++++++++++++++++++++++++++----------------------------- 1 file changed, 29 insertions(+), 31 deletions(-) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index e85be6ae60..21e2c9dea3 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -279,42 +279,40 @@ static uint64_t imx_spi_read(void *opaque, hwaddr offset, unsigned size) return 0; } - switch (index) { - case ECSPI_RXDATA: - if (!imx_spi_is_enabled(s)) { - value = 0; - } else if (fifo32_is_empty(&s->rx_fifo)) { - /* value is undefined */ - value = 0xdeadbeef; - } else { - /* read from the RX FIFO */ - value = fifo32_pop(&s->rx_fifo); - } - - break; - case ECSPI_TXDATA: - qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Trying to read from TX FIFO\n", - TYPE_IMX_SPI, __func__); - - /* Reading from TXDATA gives 0 */ - - break; - case ECSPI_MSGDATA: - qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Trying to read from MSG FIFO\n", - TYPE_IMX_SPI, __func__); + value = s->regs[index]; + + if (imx_spi_is_enabled(s)) { + switch (index) { + case ECSPI_RXDATA: + if (fifo32_is_empty(&s->rx_fifo)) { + /* value is undefined */ + value = 0xdeadbeef; + } else { + /* read from the RX FIFO */ + value = fifo32_pop(&s->rx_fifo); + } + break; + case ECSPI_TXDATA: + qemu_log_mask(LOG_GUEST_ERROR, + "[%s]%s: Trying to read from TX FIFO\n", + TYPE_IMX_SPI, __func__); - /* Reading from MSGDATA gives 0 */ + /* Reading from TXDATA gives 0 */ + break; + case ECSPI_MSGDATA: + qemu_log_mask(LOG_GUEST_ERROR, + "[%s]%s: Trying to read from MSG FIFO\n", + TYPE_IMX_SPI, __func__); + /* Reading from MSGDATA gives 0 */ + break; + default: + break; + } - break; - default: - value = s->regs[index]; - break; + imx_spi_update_irq(s); } - DPRINTF("reg[%s] => 0x%" PRIx32 "\n", imx_spi_reg_name(index), value); - imx_spi_update_irq(s); - return (uint64_t)value; } -- cgit 1.4.1 From fb116b5456c818ae7c3b788adcbc05dfa416c90c Mon Sep 17 00:00:00 2001 From: Philippe Mathieu-Daudé Date: Fri, 29 Jan 2021 21:23:19 +0800 Subject: hw/ssi: imx_spi: Rework imx_spi_write() to handle block disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the block is disabled, only the ECSPI_CONREG register can be modified. Setting the EN bit enabled the device, clearing it "disables the block and resets the internal logic with the exception of the ECSPI_CONREG" register. Ignore all other registers write except ECSPI_CONREG when the block is disabled. Ref: i.MX 6DQ Applications Processor Reference Manual (IMX6DQRM), chapter 21.7.3: Control Register (ECSPIx_CONREG) Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Bin Meng Reviewed-by: Peter Maydell Message-id: 20210129132323.30946-7-bmeng.cn@gmail.com Message-Id: <20210115153049.3353008-6-f4bug@amsat.org> Signed-off-by: Bin Meng Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index 21e2c9dea3..4cfbb73e35 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -332,6 +332,14 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value, DPRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx_spi_reg_name(index), (uint32_t)value); + if (!imx_spi_is_enabled(s)) { + /* Block is disabled */ + if (index != ECSPI_CONREG) { + /* Ignore access */ + return; + } + } + change_mask = s->regs[index] ^ value; switch (index) { @@ -340,10 +348,7 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value, TYPE_IMX_SPI, __func__); break; case ECSPI_TXDATA: - if (!imx_spi_is_enabled(s)) { - /* Ignore writes if device is disabled */ - break; - } else if (fifo32_is_full(&s->tx_fifo)) { + if (fifo32_is_full(&s->tx_fifo)) { /* Ignore writes if queue is full */ break; } -- cgit 1.4.1 From 50dc25932eb31fca15104968e596b7035ce9ece1 Mon Sep 17 00:00:00 2001 From: Xuzhou Cheng Date: Fri, 29 Jan 2021 21:23:20 +0800 Subject: hw/ssi: imx_spi: Disable chip selects when controller is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a write to ECSPI_CONREG register to disable the SPI controller, imx_spi_soft_reset() is called to reset the controller, but chip select lines should have been disabled, otherwise the state machine of any devices (e.g.: SPI flashes) connected to the SPI master is stuck to its last state and responds incorrectly to any follow-up commands. Fixes: c906a3a01582 ("i.MX: Add the Freescale SPI Controller") Signed-off-by: Xuzhou Cheng Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-id: 20210129132323.30946-8-bmeng.cn@gmail.com Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index 4cfbb73e35..2fb65498c3 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -254,9 +254,15 @@ static void imx_spi_common_reset(IMXSPIState *s) static void imx_spi_soft_reset(IMXSPIState *s) { + int i; + imx_spi_common_reset(s); imx_spi_update_irq(s); + + for (i = 0; i < ECSPI_NUM_CS; i++) { + qemu_set_irq(s->cs_lines[i], 1); + } } static void imx_spi_reset(DeviceState *dev) -- cgit 1.4.1 From 24bf8ef3f5300943940fd054763f92808f8481a0 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Fri, 29 Jan 2021 21:23:21 +0800 Subject: hw/ssi: imx_spi: Round up the burst length to be multiple of 8 Current implementation of the imx spi controller expects the burst length to be multiple of 8, which is the most common use case. In case the burst length is not what we expect, log it to give user a chance to notice it, and round it up to be multiple of 8. Signed-off-by: Bin Meng Message-id: 20210129132323.30946-9-bmeng.cn@gmail.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index 2fb65498c3..41fe199c9f 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -128,7 +128,14 @@ static uint8_t imx_spi_selected_channel(IMXSPIState *s) static uint32_t imx_spi_burst_length(IMXSPIState *s) { - return EXTRACT(s->regs[ECSPI_CONREG], ECSPI_CONREG_BURST_LENGTH) + 1; + uint32_t burst; + + burst = EXTRACT(s->regs[ECSPI_CONREG], ECSPI_CONREG_BURST_LENGTH) + 1; + if (burst % 8) { + burst = ROUND_UP(burst, 8); + } + + return burst; } static bool imx_spi_is_enabled(IMXSPIState *s) @@ -328,6 +335,7 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value, IMXSPIState *s = opaque; uint32_t index = offset >> 2; uint32_t change_mask; + uint32_t burst; if (index >= ECSPI_MAX) { qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%" @@ -380,6 +388,13 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value, case ECSPI_CONREG: s->regs[ECSPI_CONREG] = value; + burst = EXTRACT(s->regs[ECSPI_CONREG], ECSPI_CONREG_BURST_LENGTH) + 1; + if (burst % 8) { + qemu_log_mask(LOG_UNIMP, + "[%s]%s: burst length %d not supported: rounding up to next multiple of 8\n", + TYPE_IMX_SPI, __func__, burst); + } + if (!imx_spi_is_enabled(s)) { /* device is disabled, so this is a soft reset */ imx_spi_soft_reset(s); -- cgit 1.4.1 From 6ed924823c87999191776a2bd9a56efd3d83a387 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Fri, 29 Jan 2021 21:23:22 +0800 Subject: hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Philippe Mathieu-Daudé Message-id: 20210129132323.30946-10-bmeng.cn@gmail.com Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index 41fe199c9f..a34194c1b0 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -185,7 +185,7 @@ 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) ? : 32; rx = 0; -- cgit 1.4.1 From 8c495d1379211554208c58be75736e3be5ad60e8 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Fri, 29 Jan 2021 21:23:23 +0800 Subject: hw/ssi: imx_spi: Correct tx and rx fifo endianness 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 Reviewed-by: Peter Maydell Message-id: 20210129132323.30946-11-bmeng.cn@gmail.com Signed-off-by: Peter Maydell --- hw/ssi/imx_spi.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'hw/ssi/imx_spi.c') diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index a34194c1b0..189423bb3a 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -169,7 +169,6 @@ static void imx_spi_flush_txfifo(IMXSPIState *s) 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); @@ -190,7 +189,7 @@ static void imx_spi_flush_txfifo(IMXSPIState *s) rx = 0; while (tx_burst > 0) { - uint8_t byte = tx & 0xff; + uint8_t byte = tx >> (tx_burst - 8); DPRINTF("writing 0x%02x\n", (uint32_t)byte); @@ -199,13 +198,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); -- cgit 1.4.1