mbox series

[0/4] spi-mem: Allow specifying the byte order in DTR mode

Message ID 20220218145900.1440045-1-tudor.ambarus@microchip.com
Headers show
Series spi-mem: Allow specifying the byte order in DTR mode | expand

Message

Tudor Ambarus Feb. 18, 2022, 2:58 p.m. UTC 
There are NOR flashes (Macronix) that swap the bytes on a 16-bit boundary
when configured in DTR mode. The byte order of 16-bit words is swapped
when read or written in Double Transfer Rate (DTR) mode compared to
Single Transfer Rate (STR) mode. If one writes D0 D1 D2 D3 bytes using
1-1-1 mode, and uses 8D-8D-8D SPI mode for reading, it will read back
D1 D0 D3 D2. Swapping the bytes may introduce some endianness problems.
It can affect the boot sequence if the entire boot sequence is not handled
in either 8D-8D-8D mode or 1-1-1 mode. Fortunately there are controllers
that can swap back the bytes at runtime, fixing the endiannesses. Provide
a way for the upper layers to specify the byte order in DTR mode.

Tested with atmel-quadspi and mx66lm1g45g.

Tudor Ambarus (4):
  spi: spi-mem: Allow specifying the byte order in DTR mode
  mtd: spi-nor: core: Allow specifying the byte order in DTR mode
  mtd: spi-nor: sfdp: Get the 8D-8D-8D byte order from BFPT
  mtd: spi-nor: core: Introduce SPI_NOR_DTR_BSWAP16 no_sfdp_flag

 drivers/mtd/spi-nor/core.c  | 36 +++++++++++++++++++++++++++++-------
 drivers/mtd/spi-nor/core.h  |  6 +++++-
 drivers/mtd/spi-nor/sfdp.c  |  3 +++
 drivers/mtd/spi-nor/sfdp.h  |  1 +
 include/linux/mtd/spi-nor.h | 17 +++++++++++++++++
 include/linux/spi/spi-mem.h |  3 +++
 6 files changed, 58 insertions(+), 8 deletions(-)

Comments

Michael Walle Feb. 21, 2022, 7:44 a.m. UTC | #1 
Am 2022-02-18 15:58, schrieb Tudor Ambarus:
> Fortunately there are controllers
> that can swap back the bytes at runtime, fixing the endiannesses. 
> Provide
> a way for the upper layers to specify the byte order in DTR mode.

Are there any patches for the atmel-quadspi yet? What happens if
the controller doesn't support it? Will there be a software fallback?

-michael
Tudor Ambarus Feb. 22, 2022, 1:54 p.m. UTC | #2 
On 2/21/22 09:44, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2022-02-18 15:58, schrieb Tudor Ambarus:
>> Fortunately there are controllers
>> that can swap back the bytes at runtime, fixing the endiannesses.
>> Provide
>> a way for the upper layers to specify the byte order in DTR mode.
> 
> Are there any patches for the atmel-quadspi yet? What happens if

not public, but will publish them these days.

> the controller doesn't support it? Will there be a software fallback?

no need for a fallback, the controller can ignore op->data.dtr_bswap16 if
it can't swap bytes.

Here's the changes that enable this on atmel-quadspi:

Author: Tudor Ambarus <tudor.ambarus@microchip.com>
Date:   Thu Feb 17 10:48:10 2022 +0200

    spi: atmel-quadspi: Set endianness on 8D-8D-8D mode according to the flash requirements
    
    Macronix swaps bytes on a 16-bit boundary when configured in Octal DTR.
    The byte order of 16-bit words is swapped when read or write written in
    8D-8D-8D mode compared to STR modes. Set the endianness flash requirements
    to avoid endianness problems during boot stages.
    
    Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index a4ba94ce84f1..c4a3963f7c84 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -697,6 +697,8 @@ static int atmel_qspi_sama7g5_set_cfg(struct atmel_qspi *aq,
                ifr |= QSPI_IFR_DDREN;
                if (op->cmd.dtr)
                        ifr |= QSPI_IFR_DDRCMDEN;
+               if (op->data.dtr_bswap16)
+                       ifr |= QSPI_IFR_END;
 
                ifr |= QSPI_IFR_DQSEN;
        }
Michael Walle Feb. 22, 2022, 2:13 p.m. UTC | #3 
Am 2022-02-22 14:54, schrieb Tudor.Ambarus@microchip.com:
> On 2/21/22 09:44, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> Am 2022-02-18 15:58, schrieb Tudor Ambarus:
>>> Fortunately there are controllers
>>> that can swap back the bytes at runtime, fixing the endiannesses.
>>> Provide
>>> a way for the upper layers to specify the byte order in DTR mode.
>> 
>> Are there any patches for the atmel-quadspi yet? What happens if
> 
> not public, but will publish them these days.
> 
>> the controller doesn't support it? Will there be a software fallback?
> 
> no need for a fallback, the controller can ignore op->data.dtr_bswap16 
> if
> it can't swap bytes.

I don't understand. If the controller doesn't swap the 16bit values,
you will read the wrong content, no?

-michael
Tudor Ambarus Feb. 22, 2022, 2:23 p.m. UTC | #4 
On 2/22/22 16:13, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2022-02-22 14:54, schrieb Tudor.Ambarus@microchip.com:
>> On 2/21/22 09:44, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Am 2022-02-18 15:58, schrieb Tudor Ambarus:
>>>> Fortunately there are controllers
>>>> that can swap back the bytes at runtime, fixing the endiannesses.
>>>> Provide
>>>> a way for the upper layers to specify the byte order in DTR mode.
>>>
>>> Are there any patches for the atmel-quadspi yet? What happens if
>>
>> not public, but will publish them these days.
>>
>>> the controller doesn't support it? Will there be a software fallback?
>>
>> no need for a fallback, the controller can ignore op->data.dtr_bswap16
>> if
>> it can't swap bytes.
> 
> I don't understand. If the controller doesn't swap the 16bit values,
> you will read the wrong content, no?
> 

In linux no, because macronix swaps bytes on a 2 byte boundary both on
reads and on page program. The problem is when you mix 8D-8D-8D mode and
1-1-1 mode along the boot stages. Let's assume you write all boot binaries
in 1-1-1 mode. When reaching u-boot if you enable 8D-8D-8D mode, when u-boot
will try to get the kernel it will fail, as the flash swaps the bytes compared
to what was written with 1-1-1 mode. You write D0 D1 D2 D3 in 1-1-1 mode and
when reaching u-boot you will read D1 D0 D3 D2 and it will mess the kernel image.
Michael Walle Feb. 22, 2022, 2:27 p.m. UTC | #5 
Am 2022-02-22 15:23, schrieb Tudor.Ambarus@microchip.com:
> On 2/22/22 16:13, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> Am 2022-02-22 14:54, schrieb Tudor.Ambarus@microchip.com:
>>> On 2/21/22 09:44, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you 
>>>> know
>>>> the content is safe
>>>> 
>>>> Am 2022-02-18 15:58, schrieb Tudor Ambarus:
>>>>> Fortunately there are controllers
>>>>> that can swap back the bytes at runtime, fixing the endiannesses.
>>>>> Provide
>>>>> a way for the upper layers to specify the byte order in DTR mode.
>>>> 
>>>> Are there any patches for the atmel-quadspi yet? What happens if
>>> 
>>> not public, but will publish them these days.
>>> 
>>>> the controller doesn't support it? Will there be a software 
>>>> fallback?
>>> 
>>> no need for a fallback, the controller can ignore 
>>> op->data.dtr_bswap16
>>> if
>>> it can't swap bytes.
>> 
>> I don't understand. If the controller doesn't swap the 16bit values,
>> you will read the wrong content, no?
>> 
> 
> In linux no, because macronix swaps bytes on a 2 byte boundary both on
> reads and on page program. The problem is when you mix 8D-8D-8D mode 
> and
> 1-1-1 mode along the boot stages. Let's assume you write all boot 
> binaries
> in 1-1-1 mode. When reaching u-boot if you enable 8D-8D-8D mode, when 
> u-boot
> will try to get the kernel it will fail, as the flash swaps the bytes 
> compared
> to what was written with 1-1-1 mode. You write D0 D1 D2 D3 in 1-1-1 
> mode and
> when reaching u-boot you will read D1 D0 D3 D2 and it will mess the
> kernel image.

But you have to consider also 3rd parties, like an external programmer 
or
another OS. So, there has to be *one correct* way of writing/reading 
these
bytes.

-michael
Tudor Ambarus Feb. 22, 2022, 2:43 p.m. UTC | #6 
On 2/22/22 16:27, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2022-02-22 15:23, schrieb Tudor.Ambarus@microchip.com:
>> On 2/22/22 16:13, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Am 2022-02-22 14:54, schrieb Tudor.Ambarus@microchip.com:
>>>> On 2/21/22 09:44, Michael Walle wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>> know
>>>>> the content is safe
>>>>>
>>>>> Am 2022-02-18 15:58, schrieb Tudor Ambarus:
>>>>>> Fortunately there are controllers
>>>>>> that can swap back the bytes at runtime, fixing the endiannesses.
>>>>>> Provide
>>>>>> a way for the upper layers to specify the byte order in DTR mode.
>>>>>
>>>>> Are there any patches for the atmel-quadspi yet? What happens if
>>>>
>>>> not public, but will publish them these days.
>>>>
>>>>> the controller doesn't support it? Will there be a software
>>>>> fallback?
>>>>
>>>> no need for a fallback, the controller can ignore
>>>> op->data.dtr_bswap16
>>>> if
>>>> it can't swap bytes.
>>>
>>> I don't understand. If the controller doesn't swap the 16bit values,
>>> you will read the wrong content, no?
>>>
>>
>> In linux no, because macronix swaps bytes on a 2 byte boundary both on
>> reads and on page program. The problem is when you mix 8D-8D-8D mode
>> and
>> 1-1-1 mode along the boot stages. Let's assume you write all boot
>> binaries
>> in 1-1-1 mode. When reaching u-boot if you enable 8D-8D-8D mode, when
>> u-boot
>> will try to get the kernel it will fail, as the flash swaps the bytes
>> compared
>> to what was written with 1-1-1 mode. You write D0 D1 D2 D3 in 1-1-1
>> mode and
>> when reaching u-boot you will read D1 D0 D3 D2 and it will mess the
>> kernel image.
> 
> But you have to consider also 3rd parties, like an external programmer
> or

Why? If you use the same mode when reading and writing, everything is fine.
I'm not sure what's your suggestion here.

> another OS. So, there has to be *one correct* way of writing/reading
> these
> bytes.
>