qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] hw/ssi: imx_spi: Improve chip select handling


From: Guenter Roeck
Subject: Re: [PATCH] hw/ssi: imx_spi: Improve chip select handling
Date: Thu, 2 Sep 2021 09:09:13 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

On 9/2/21 8:58 AM, Peter Maydell wrote:
On Sun, 8 Aug 2021 at 02:34, Guenter Roeck <linux@roeck-us.net> wrote:

The control register does not really have a means to deselect
all chip selects directly. As result, CS is effectively never
deselected, and connected flash chips fail to perform read
operations since they don't get the expected chip select signals
to reset their state machine.

Normally and per controller documentation one would assume that
chip select should be set whenever a transfer starts (XCH is
set or the tx fifo is written into), and that it should be disabled
whenever a transfer is complete. However, that does not work in
practice: attempts to implement this approach resulted in failures,
presumably because a single transaction can be split into multiple
transfers.

At the same time, there is no explicit signal from the host indicating
if chip select should be active or not. In the absence of such a direct
signal, use the burst length written into the control register to
determine if an access is ongoing or not. Disable all chip selects
if the burst length field in the configuration register is set to 0,
and (re-)enable chip select if a transfer is started. This is possible
because the Linux driver clears the burst length field whenever it
prepares the controller for the next transfer.
This solution  is less than perfect since it effectively only disables
chip select when initiating the next transfer, but it does work with
Linux and should otherwise do no harm.

Stop complaining if the burst length field is set to a value of 0,
since that is done by Linux for every transfer.

With this patch, a command line parameter such as "-drive
file=flash.sabre,format=raw,if=mtd" can be used to instantiate the
flash chip in the sabrelite emulation. Without this patch, the
flash instantiates, but it only reads zeroes.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
I am not entirely happy with this solution, but it is the best I was
able to come up with. If anyone has a better idea, I'll be happy
to give it a try.

  hw/ssi/imx_spi.c | 17 +++++++----------
  1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index 189423bb3a..7a093156bd 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -167,6 +167,8 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
      DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",
              fifo32_num_used(&s->tx_fifo), fifo32_num_used(&s->rx_fifo));

+    qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)], 0);
+
      while (!fifo32_is_empty(&s->tx_fifo)) {
          int tx_burst = 0;

@@ -385,13 +387,6 @@ 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);
-        }

Why has this log message been removed ?

What I wanted to do is:

"Stop complaining if the burst length field is set to a value of 0,
 since that is done by Linux for every transfer."

What I did instead is to remove the message entirely.

How about the rest of the patch ? Is it worth a resend with the message
restored (except for burst size == 0), or is it not acceptable anyway ?

Thanks,
Guenter


          if (!imx_spi_is_enabled(s)) {
              /* device is disabled, so this is a soft reset */
              imx_spi_soft_reset(s);

thanks
-- PMM





reply via email to

[Prev in Thread] Current Thread [Next in Thread]