[PATCH] mtd: support for bit-reversed CFI cmd/query

[PATCH] mtd: support for bit-reversed CFI cmd/query

[Nikita V. Youshchenko]

[-- Attachment #1: Type: text/plain, Size: 2562 bytes --]

commit 049aa6336a85de3143a3dc9cb3f4c87ecc8896b3
Author: Nikita Youshchenko <yoush@cs.msu.su>
Date:   Wed Feb 27 20:25:22 2008 +0000

    mtd: support for bit-reversed CFI cmd/query
    
    Some embedded boards have flash chips connected with reversed bit order.
    Example for 16-bit width:
     chip D0 is bus D15
     chip D1 is bus D14
     ...
     chip D15 is bus D0
    It works perfectly for data access, however probing and issuing CFI commands
    for such chips require bit reversing. This patch adds support for this.
    
    Signed-off-by: Nikita Youshchenko <yoush@cs.msu.su>

diff --git a/drivers/mtd/chips/Kconfig b/drivers/mtd/chips/Kconfig
index 6d8f30d..0471cd0 100644
--- a/drivers/mtd/chips/Kconfig
+++ b/drivers/mtd/chips/Kconfig
@@ -57,7 +57,8 @@ config MTD_CFI_NOSWAP
 	  enabled, means that the CPU will not do any swapping; the chips
 	  are expected to be wired to the CPU in 'host-endian' form.
 	  Specific arrangements are possible with the BIG_ENDIAN_BYTE and
-	  LITTLE_ENDIAN_BYTE, if the bytes are reversed.
+	  LITTLE_ENDIAN_BYTE, if the bytes are reversed. Also it is possible
+	  to revert bit order in 'magic commands'.
 
 	  If you have a LART, on which the data (and address) lines were
 	  connected in a fashion which ensured that the nets were as short
@@ -72,6 +73,10 @@ config MTD_CFI_BE_BYTE_SWAP
 config MTD_CFI_LE_BYTE_SWAP
 	bool "LITTLE_ENDIAN_BYTE"
 
+config MTD_CFI_BIT_SWAP
+	bool "Reverse bit order"
+	select BITREVERSE
+
 endchoice
 
 config MTD_CFI_GEOMETRY
diff --git a/include/linux/mtd/cfi_endian.h b/include/linux/mtd/cfi_endian.h
index 25724f7..195edae 100644
--- a/include/linux/mtd/cfi_endian.h
+++ b/include/linux/mtd/cfi_endian.h
@@ -52,6 +52,24 @@
 #define cfi16_to_cpu(x) (x)
 #define cfi32_to_cpu(x) (x)
 #define cfi64_to_cpu(x) (x)
+#elif defined(CONFIG_MTD_CFI_BIT_SWAP)
+#include <linux/bitrev.h>
+#define cpu_to_cfi8(x) (x)
+#define cfi8_to_cpu(x) (x)
+#define cpu_to_cfi16(x) ({ u16 y = (x); \
+		           (bitrev8(y & 255) << 8) | bitrev8(y >> 8); \
+			})
+#define cfi16_to_cpu(x) ({ u16 y = (x); \
+		           (bitrev8(y & 255) << 8) | bitrev8(y >> 8); \
+			})
+#define cpu_to_cfi32(x) (x)
+#define cfi32_to_cpu(x) (x)
+#define cpu_to_cfi64(x) ({ u64 y = (x); \
+		           (bitrev32(y & 0xffffffff) << 32) | bitrev32(y >> 32); \
+			})
+#define cfi64_to_cpu(x) ({ u64 y = (x); \
+		           (bitrev32(y & 0xffffffff) << 32) | bitrev32(y >> 32); \
+			})
 #else
 #error No CFI endianness defined
 #endif

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] mtd: support for bit-reversed CFI cmd/query

[David Woodhouse]

On Wed, 2008-02-27 at 23:48 +0300, Nikita V. Youshchenko wrote:
>     mtd: support for bit-reversed CFI cmd/query
>     
>     Some embedded boards have flash chips connected with reversed bit order.
>     Example for 16-bit width:
>      chip D0 is bus D15
>      chip D1 is bus D14
>      ...
>      chip D15 is bus D0
>     It works perfectly for data access, however probing and issuing CFI commands
>     for such chips require bit reversing. This patch adds support for this.
>     
>     Signed-off-by: Nikita Youshchenko <yoush@cs.msu.su>

Am I missing something, or did you only reverse the bits on 16-bit and
64-bit access, and not 8-bit or 32-bit?

-- 
dwmw2

Re: [PATCH] mtd: support for bit-reversed CFI cmd/query

[Nikita V. Youshchenko]

[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]

> On Wed, 2008-02-27 at 23:48 +0300, Nikita V. Youshchenko wrote:
> >     mtd: support for bit-reversed CFI cmd/query
> >
> >     Some embedded boards have flash chips connected with reversed bit
> > order. Example for 16-bit width:
> >      chip D0 is bus D15
> >      chip D1 is bus D14
> >      ...
> >      chip D15 is bus D0
> >     It works perfectly for data access, however probing and issuing
> > CFI commands for such chips require bit reversing. This patch adds
> > support for this.
> >
> >     Signed-off-by: Nikita Youshchenko <yoush@cs.msu.su>
>
> Am I missing something, or did you only reverse the bits on 16-bit and
> 64-bit access, and not 8-bit or 32-bit?

Hmm... Looks so :(.
The board I work with has 16-bit flash, so I could not test others.

Btw, the vendor of the board confirmed that such a strange flash chip 
connection is 'an old hardware bug', however they decided to keep it in 
current and future revisions of the board 'for backward compatibility'.

Is it a good idea to support such strange cases in the official kernel?
If yes, I may send an updated patch.

Nikita

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] mtd: support for bit-reversed CFI cmd/query

[David Woodhouse]

On Tue, 2008-04-22 at 23:08 +0400, Nikita V. Youshchenko wrote:
> > On Wed, 2008-02-27 at 23:48 +0300, Nikita V. Youshchenko wrote:
> > >     mtd: support for bit-reversed CFI cmd/query
> > >
> > >     Some embedded boards have flash chips connected with reversed bit
> > > order. Example for 16-bit width:
> > >      chip D0 is bus D15
> > >      chip D1 is bus D14
> > >      ...
> > >      chip D15 is bus D0
> > >     It works perfectly for data access, however probing and issuing
> > > CFI commands for such chips require bit reversing. This patch adds
> > > support for this.
> > >
> > >     Signed-off-by: Nikita Youshchenko <yoush@cs.msu.su>
> >
> > Am I missing something, or did you only reverse the bits on 16-bit and
> > 64-bit access, and not 8-bit or 32-bit?
> 
> Hmm... Looks so :(.
> The board I work with has 16-bit flash, so I could not test others.
> 
> Btw, the vendor of the board confirmed that such a strange flash chip 
> connection is 'an old hardware bug', however they decided to keep it in 
> current and future revisions of the board 'for backward compatibility'.
> 
> Is it a good idea to support such strange cases in the official kernel?
> If yes, I may send an updated patch.

I don't think it's particularly problematic. I _think_ that hacking it
into the endianness support is going to work, and isn't going to screw
up data access (which we really _don't_ want to byteswap/mangle since we
want the CPU to be able to execute from it).

Perhaps we should even go back to supporting the LART this way?

-- 
dwmw2