RE: ixp4xx stuff

RE: ixp4xx stuff

[John Bowler]

David originally posted this to linux-arm-kernel@lists.arm.linux.org.uk, but the
l-a-k maintainers want the patches on linux-mtd.  The patches are effectively
a rewrite of the NSLU2 fixes to ixp4xx.c to solve an alignment problem and get
it working on an LE ixp4xx system.

When we tested these changes against 2.6.15-rc1 we
found that, while LE systems work correctly, BE systems half-word-byte-swap
the flash - i.e. the flash ends up with PDP endianness.  We expect existing
systems (i.e existing flash) to work with an appropriately configured kernel,
and this isn't the case because of the change in byte sex (from big endian
to pdp endian.)

More details:

2.6.15-rc1 seems to have some of the fixes already applied, we applied just:

mtd-unaligned-read-fix
mtd-little-endian-support

along with the other NSLU2 patches.  We found that it is now necessary to
set CONFIG_MTD_LE_BYTE_SWAP - with the NSLU2 patches CONFIG_MTD_BE_BYTE_SWAP
must be set.  If this is not done (i.e. if BE is set) the flash probe fails.

After setting this the LE system worked fine, but BE fails reading the FIS
directory because of the half-word swapping - reflashing with a
half-word-byte-swapped FIS directory (and JFFS2) makes the system bootable.

Unfortunately it is not possible for me to examine the flash contents from
the booted BE system because the IXP ethernet drivers are also not working
in 2.6.15-rc1 (at least, I can't get an ethernet connection to my NSLU2).
Nevertheless, I am certain that examination would show other MTD partitions
are also half-word-byte-swapped on BE.  (We have confirmed that this does
not happen on LE).

John Bowler <jbowler@acm.org>

Re: ixp4xx stuff

[David Vrabel]

John Bowler wrote:
> 
> along with the other NSLU2 patches.  We found that it is now necessary to
> set CONFIG_MTD_LE_BYTE_SWAP - with the NSLU2 patches CONFIG_MTD_BE_BYTE_SWAP
> must be set.  If this is not done (i.e. if BE is set) the flash probe fails.

Neither of these options should be enabled.  The flash data is returned
in the same order regardless of the endianness of the CPU.

David Vrabel
-- 
David Vrabel, Design Engineer

Arcom, Clifton Road           Tel: +44 (0)1223 411200 ext. 3233
Cambridge CB1 7EA, UK         Web: http://www.arcom.com/

RE: ixp4xx stuff

[John Bowler]

From: David Vrabel
>> along with the other NSLU2 patches.  We found that it is now necessary to
>> set CONFIG_MTD_LE_BYTE_SWAP - with the NSLU2 patches CONFIG_MTD_BE_BYTE_SWAP
>> must be set.  If this is not done (i.e. if BE is set) the flash probe fails.
>
>Neither of these options should be enabled.  The flash data is returned
>in the same order regardless of the endianness of the CPU.

What is the difference between CONFIG_MTD_BE_BYTE_SWAP and ..._NO_SWAP on a
BE machine?

John Bowler <jbowler@acm.org>

Re: ixp4xx stuff

[David Vrabel]

John Bowler wrote:
> From: David Vrabel
> 
>>>along with the other NSLU2 patches.  We found that it is now necessary to
>>>set CONFIG_MTD_LE_BYTE_SWAP - with the NSLU2 patches CONFIG_MTD_BE_BYTE_SWAP
>>>must be set.  If this is not done (i.e. if BE is set) the flash probe fails.
>>
>>Neither of these options should be enabled.  The flash data is returned
>>in the same order regardless of the endianness of the CPU.
> 
> 
> What is the difference between CONFIG_MTD_BE_BYTE_SWAP and ..._NO_SWAP on a
> BE machine?

They're both no-ops on BE CPUs.

To conclude: With CONFIG_MTD_NOSWAP set the current patches:

a) Work on BE systems as they have always done (tested by myself and
it's fairly obvious it's right since it doesn't do anything different).
b) Work on LE systems (as you have verified -- CONFIG_MTD_LE_BYTE_SWAP
is a no-op on LE CPUs).

So it looks like everything is good to go, yes?

David Vrabel
-- 
David Vrabel, Design Engineer

Arcom, Clifton Road           Tel: +44 (0)1223 411200 ext. 3233
Cambridge CB1 7EA, UK         Web: http://www.arcom.com/

RE: ixp4xx stuff

[John Bowler]

Just to be very clear about this.

These patches do not work on BE systems.

John Bowler <jbowler@acm.org>

[PATCH] [MTD] MAPS/ixp4xx.c: fix BE system break in 2.6.15-rc1, enable LE operation in 2.6.15

[John Bowler]

[MTD] MAPS/ixp4xx.c: fix BE system break in 2.6.15-rc1, enable LE operation in 2.6.15

Linux 2.6.14.2 with NSLU2 patches excluding patches to ixp4xx.c
correctly boots on a BE (default) NSLU2 system from a JFFS2 rootfs
in the on-board flash.

Linux 2.6.15-rc1 with NSLU2 patches (excluding ixp4xx.c) and with
or without the additional patches from DVrabel:

mtd-unaligned-read-fix1.txt
mtd-little-endian-support.txt

fails during boot on NSLU2 BE because the probe of the on board flash
fails.

This patch applies to 2.6.15-rc1 (without any additional patches) and
fixes the problems so that 2.6.15-rc1+NSLU2 patches(excluding ixp4xx.c)+
this patch works in all cases where 2.6.14.2+NSLU2 patches (including,
in this case, the NSLU2 submitted ixp4xx.c patch).  That is to say,
the patched system will function correctly in both BE and LE modes using
(reading *and* writing) a JFFS2 from the on-board flash.

The comments in the patch explain the problems being fixed and how the
fix is achieved.  The fix is equivalent to the previously submitted
NSLU2 patch.

The full problem set is as follows, where '2.6.14' means 2.6.14 without
the NSLU2 patches, '2.6.15' means 2.6.15-rc1 without NSLU2 patches
and 'Vrabel' means 2.6.15-rc1 with the two patches mentioned above.

[2.6.14 BE]: boots, writeable JFFS2 but alignment problems for some reads
[2.6.14 LE]: does not work, flash probe fails, flash not accessible
[2.6.15, Vrabel BE]: does not work, flash probe fails, flash not accessible
[Vrabel LE]: boots, flash readable, flash fails on write (because the write
  byte swaps the bytes written, whereas the copy-from API used for the read
  does not).

[From: John Bowler <jbowler@acm.org>]
Signed-off-by: John Bowler <jbowler@acm.org>
Signed-off-by: Alessandro Zummo <a.zummo@towertech.it>

--- linux-2.6.15/drivers/mtd/maps/ixp4xx.c	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.15/drivers/mtd/maps/ixp4xx.c	1970-01-01 00:00:00.000000000 +0000
@@ -28,24 +28,45 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/map.h>
 #include <linux/mtd/partitions.h>
+#include <linux/mtd/cfi_endian.h>
 
 #include <asm/io.h>
 #include <asm/mach/flash.h>
 
 #include <linux/reboot.h>
 
-#ifndef __ARMEB__
-#define	BYTE0(h)	((h) & 0xFF)
-#define	BYTE1(h)	(((h) >> 8) & 0xFF)
+/*
+ * Read a 16 bit word from flash address 'addr'.
+ * This code ensures that the ixp4xx flash behaves as a consistent byte
+ * stream (same bytes in same order) regardless of whether the CPU is
+ * running in BE or LE mode, and regardless of whether the read16 or
+ * copyfrom method is used to read the flash data.
+ *
+ * When the cpu is in little-endian mode the hardware inverts the low
+ * address lines between the MMU (including the cache) and the flash chip
+ * ('address coherency') so we need to undo the swizzling to ensure commands
+ * and the like end up on the correct flash address.  This requirement
+ * only applies to command operations, however this code cannot distinguish
+ * a command read from a data read, therefore the fix applies to the data
+ * as well as the command and must be implemented consistently throughout
+ * the map driver (here).
+ */
+static inline u16 flash_read16(const void __iomem *virt, unsigned long offset)
+{
+#ifdef __ARMEB__
+	return *(const volatile u16 __iomem *)(virt + offset);
 #else
-#define	BYTE0(h)	(((h) >> 8) & 0xFF)
-#define	BYTE1(h)	((h) & 0xFF)
+	/* Cancel the address coherency invert of address line 1 in the
+	 * hardware on LE systems.
+	 */
+	return *(const volatile u16 __iomem *)(virt + (offset ^ 2));
 #endif
+}
 
 static map_word ixp4xx_read16(struct map_info *map, unsigned long ofs)
 {
 	map_word val;
-	val.x[0] = le16_to_cpu(readw(map->virt + ofs));
+	val.x[0] = cfi16_to_cpu(flash_read16(map->virt, ofs));
 	return val;
 }
 
@@ -53,41 +74,70 @@ static map_word ixp4xx_read16(struct map
  * The IXP4xx expansion bus only allows 16-bit wide acceses
  * when attached to a 16-bit wide device (such as the 28F128J3A),
  * so we can't just memcpy_fromio().
+ *
+ * This code must obtain the correct bytes in the correct order
+ * according to the configuration of the CFI subsystem.
  */
+#if (defined CFI_BIG_ENDIAN) || ((defined CFI_HOST_ENDIAN) && (defined __ARMEB__))
+	/* BE flash */
+#define	BYTE0(h)	(((h) >> 8) & 0xFF)
+#define	BYTE1(h)	((h) & 0xFF)
+#else
+	/* LE flash */
+#define	BYTE0(h)	((h) & 0xFF)
+#define	BYTE1(h)	(((h) >> 8) & 0xFF)
+#endif
+
 static void ixp4xx_copy_from(struct map_info *map, void *to,
 			     unsigned long from, ssize_t len)
 {
-	int i;
-	u8 *dest = (u8 *) to;
-	void __iomem *src = map->virt + from;
-	u16 data;
-
-	for (i = 0; i < (len / 2); i++) {
-		data = le16_to_cpu(readw(src + 2*i));
-		dest[i * 2] = BYTE0(data);
-		dest[i * 2 + 1] = BYTE1(data);
+	u8 *dest = to;
+	const void __iomem * const virt = map->virt;
+
+	if (len <= 0)
+		return;
+
+	if (from & 1) {
+		*dest++ = BYTE1(flash_read16(virt, from-1));
+                ++from;
+		--len;
+	}
+
+	while (len >= 2) {
+		u16 data = flash_read16(virt, from);
+		*dest++ = BYTE0(data);
+		*dest++ = BYTE1(data);
+		from += 2;
+		len -= 2;
 	}
 
-	if (len & 1)
-		dest[len - 1] = BYTE0(le16_to_cpu(readw(src + 2*i)));
+	if (len > 0)
+		*dest++ = BYTE0(flash_read16(virt, from));
 }
 
 /*
- * Unaligned writes are ignored, causing the 8-bit
- * probe to fail and proceed to the 16-bit probe (which succeeds).
+ * Fast write16 function without the probing check below.
  */
-static void ixp4xx_probe_write16(struct map_info *map, map_word d, unsigned long adr)
+static void ixp4xx_write16(struct map_info *map, map_word d, unsigned long adr)
 {
-	if (!(adr & 1))
-		writew(cpu_to_le16(d.x[0]), map->virt + adr);
+#ifdef __ARMEB__
+	*(volatile u16 __iomem*)(map->virt + adr) = cpu_to_cfi16(d.x[0]);
+#else
+	/* Cancel the address coherency invert of address line 1 in the
+	 * hardware on LE systems.
+	 */
+	*(volatile u16 __iomem*)(map->virt + (adr ^ 2)) = cpu_to_cfi16(d.x[0]);
+#endif
 }
 
 /*
- * Fast write16 function without the probing check above
+ * Unaligned writes are ignored, causing the 8-bit
+ * probe to fail and proceed to the 16-bit probe (which succeeds).
  */
-static void ixp4xx_write16(struct map_info *map, map_word d, unsigned long adr)
+static void ixp4xx_probe_write16(struct map_info *map, map_word d, unsigned long adr)
 {
-	writew(cpu_to_le16(d.x[0]), map->virt + adr);
+	if (!(adr & 1))
+		ixp4xx_write16(map, d, adr);
 }
 
 struct ixp4xx_flash_info {

Re: [PATCH] [MTD] MAPS/ixp4xx.c: fix BE system break in 2.6.15-rc1, enable LE operation in 2.6.15

[David Vrabel]

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

Commited with some cleanups.

FYI, I've attached what was committed.

David Vrabel
-- 
David Vrabel, Design Engineer

Arcom, Clifton Road           Tel: +44 (0)1223 411200 ext. 3233
Cambridge CB1 7EA, UK         Web: http://www.arcom.com/

[-- Attachment #2: mtd-ixp4xx-updates --]
[-- Type: text/plain, Size: 3572 bytes --]

ixp4xx updates:
  - Handle reads that don't start on a half-word boundary.
  - Make it work when CPU is in little-endian mode.
    
Signed-off-by: John Bowler <jbowler@acm.org>
Signed-off-by: Alessandro Zummo <a.zummo@towertech.it>
Signed-off-by: David Vrabel <dvrabel@arcom.com>

Index: linux-2.6-working/drivers/mtd/maps/ixp4xx.c
===================================================================
--- linux-2.6-working.orig/drivers/mtd/maps/ixp4xx.c	2005-11-16 15:19:34.000000000 +0000
+++ linux-2.6-working/drivers/mtd/maps/ixp4xx.c	2005-11-16 16:06:54.000000000 +0000
@@ -34,10 +34,55 @@
 
 #include <linux/reboot.h>
 
+/*
+ * Read/write a 16 bit word from flash address 'addr'.
+ *
+ * When the cpu is in little-endian mode it swizzles the address lines
+ * ('address coherency') so we need to undo the swizzling to ensure commands
+ * and the like end up on the correct flash address.
+ *
+ * To further complicate matters, due to the way the expansion bus controller
+ * handles 32 bit reads, the byte stream ABCD is stored on the flash as:
+ *     D15    D0
+ *     +---+---+
+ *     | A | B | 0
+ *     +---+---+
+ *     | C | D | 2
+ *     +---+---+
+ * This means that on LE systems each 16 bit word must be swapped. Note that
+ * this requires CONFIG_MTD_CFI_BE_BYTE_SWAP to be enabled to 'unswap' the CFI
+ * data and other flash commands which are always in D7-D0.
+ */
 #ifndef __ARMEB__
+#ifndef CONFIG_MTD_CFI_BE_BYTE_SWAP
+#  error CONFIG_MTD_CFI_BE_BYTE_SWAP required
+#endif
+
+static inline u16 flash_read16(void __iomem *addr)
+{
+	return be16_to_cpu(__raw_readw((void __iomem *)((unsigned long)addr ^ 0x2)));
+}
+
+static inline void flash_write16(u16 d, void __iomem *addr)
+{
+	__raw_writew(cpu_to_be16(d), (void __iomem *)((unsigned long)addr ^ 0x2));
+}
+
 #define	BYTE0(h)	((h) & 0xFF)
 #define	BYTE1(h)	(((h) >> 8) & 0xFF)
+
 #else
+
+static inline u16 flash_read16(const void __iomem *addr)
+{
+	return __raw_readw(addr);
+}
+
+static inline void flash_write16(u16 d, void __iomem *addr)
+{
+	__raw_writew(d, addr);
+}
+
 #define	BYTE0(h)	(((h) >> 8) & 0xFF)
 #define	BYTE1(h)	((h) & 0xFF)
 #endif
@@ -45,7 +90,7 @@
 static map_word ixp4xx_read16(struct map_info *map, unsigned long ofs)
 {
 	map_word val;
-	val.x[0] = le16_to_cpu(readw(map->virt + ofs));
+	val.x[0] = flash_read16(map->virt + ofs);
 	return val;
 }
 
@@ -57,19 +102,28 @@
 static void ixp4xx_copy_from(struct map_info *map, void *to,
 			     unsigned long from, ssize_t len)
 {
-	int i;
 	u8 *dest = (u8 *) to;
 	void __iomem *src = map->virt + from;
-	u16 data;
 
-	for (i = 0; i < (len / 2); i++) {
-		data = le16_to_cpu(readw(src + 2*i));
-		dest[i * 2] = BYTE0(data);
-		dest[i * 2 + 1] = BYTE1(data);
+	if (len <= 0)
+		return;
+
+	if (from & 1) {
+		*dest++ = BYTE1(flash_read16(src));
+                src++;
+		--len;
 	}
 
-	if (len & 1)
-		dest[len - 1] = BYTE0(le16_to_cpu(readw(src + 2*i)));
+	while (len >= 2) {
+		u16 data = flash_read16(src);
+		*dest++ = BYTE0(data);
+		*dest++ = BYTE1(data);
+		src += 2;
+		len -= 2;
+        }
+
+	if (len > 0)
+		*dest++ = BYTE0(flash_read16(src));
 }
 
 /*
@@ -79,7 +133,7 @@
 static void ixp4xx_probe_write16(struct map_info *map, map_word d, unsigned long adr)
 {
 	if (!(adr & 1))
-		writew(cpu_to_le16(d.x[0]), map->virt + adr);
+		flash_write16(d.x[0], map->virt + adr);
 }
 
 /*
@@ -87,7 +141,7 @@
  */
 static void ixp4xx_write16(struct map_info *map, map_word d, unsigned long adr)
 {
-	writew(cpu_to_le16(d.x[0]), map->virt + adr);
+	flash_write16(d.x[0], map->virt + adr);
 }
 
 struct ixp4xx_flash_info {

RE: [PATCH] [MTD] MAPS/ixp4xx.c: fix BE system break in 2.6.15-rc1, enable LE operation in 2.6.15

[John Bowler]

So far we've verified the *BE* part pretty much completely, but only
verified LE by code examination (it looks fine to me).  Anyway, I don't
anticipate any problems.

John Bowler <jbowler@acm.org>