[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 5/9] hw/ssi: imx_spi: Rework imx_spi_write() to handle blo
From: |
Bin Meng |
Subject: |
Re: [PATCH v7 5/9] hw/ssi: imx_spi: Rework imx_spi_write() to handle block disabled |
Date: |
Sat, 16 Jan 2021 21:57:19 +0800 |
On Fri, Jan 15, 2021 at 11:37 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> When the block is disabled, only the ECSPI_CONREG register can
> be modified. Setting the EN bit enabled the device, clearing it
I don't know how this conclusion came out. The manual only says the
following 2 registers ignore the write when the block is disabled.
ECSPI_TXDATA, ECSPI_INTREG
> "disables the block and resets the internal logic with the
> exception of the ECSPI_CONREG" register.
>
> Move the imx_spi_is_enabled() check earlier.
>
> Ref: i.MX 6DQ Applications Processor Reference Manual (IMX6DQRM),
> chapter 21.7.3: Control Register (ECSPIx_CONREG)
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/ssi/imx_spi.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
> index ba7d3438d87..f06bbf317e2 100644
> --- a/hw/ssi/imx_spi.c
> +++ b/hw/ssi/imx_spi.c
> @@ -322,6 +322,21 @@ 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;
> + }
> + s->regs[ECSPI_CONREG] = value;
> + if (!(value & ECSPI_CONREG_EN)) {
> + /* Keep disabled */
So other bits except ECSPI_CONREG_EN are discarded? The manual does
not explicitly mention this but this looks suspicious.
> + return;
> + }
> + /* Enable the block */
> + imx_spi_reset(DEVICE(s));
> + }
> +
> change_mask = s->regs[index] ^ value;
>
> switch (index) {
> @@ -330,10 +345,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;
> }
> @@ -359,12 +371,6 @@ static void imx_spi_write(void *opaque, hwaddr offset,
> uint64_t value,
> case ECSPI_CONREG:
> s->regs[ECSPI_CONREG] = value;
>
> - if (!imx_spi_is_enabled(s)) {
> - /* device is disabled, so this is a reset */
> - imx_spi_reset(DEVICE(s));
> - return;
> - }
> -
> if (imx_spi_channel_is_master(s)) {
> int i;
>
Regards,
Bin
- [PATCH v7 0/9] hw/ssi: imx_spi: Fix various bugs in the imx_spi model, Philippe Mathieu-Daudé, 2021/01/15
- [PATCH v7 1/9] hw/ssi: imx_spi: Use a macro for number of chip selects supported, Philippe Mathieu-Daudé, 2021/01/15
- [PATCH v7 2/9] hw/ssi: imx_spi: Remove pointless variable initialization, Philippe Mathieu-Daudé, 2021/01/15
- [PATCH v7 3/9] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value, Philippe Mathieu-Daudé, 2021/01/15
- [PATCH v7 4/9] hw/ssi: imx_spi: Rework imx_spi_read() to handle block disabled, Philippe Mathieu-Daudé, 2021/01/15
- [PATCH v7 5/9] hw/ssi: imx_spi: Rework imx_spi_write() to handle block disabled, Philippe Mathieu-Daudé, 2021/01/15
- Re: [PATCH v7 5/9] hw/ssi: imx_spi: Rework imx_spi_write() to handle block disabled,
Bin Meng <=
[PATCH v7 6/9] hw/ssi: imx_spi: Disable chip selects when controller is disabled, Philippe Mathieu-Daudé, 2021/01/15
[PATCH v7 7/9] hw/ssi: imx_spi: Round up the burst length to be multiple of 8, Philippe Mathieu-Daudé, 2021/01/15
[PATCH v7 8/9] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic, Philippe Mathieu-Daudé, 2021/01/15
[PATCH v7 9/9] hw/ssi: imx_spi: Correct tx and rx fifo endianness, Philippe Mathieu-Daudé, 2021/01/15
Re: [PATCH v7 0/9] hw/ssi: imx_spi: Fix various bugs in the imx_spi model, Bin Meng, 2021/01/16