Message ID | 1800715024.324263.1411508605796.JavaMail.zimbra@xes-inc.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Aaron Sierra <asierra@xes-inc.com> writes: > The .swap member of the map_info structure is provided for situations > where a mapping must always be big or little endian regardless of the > endianness of the system performing the access. I like the idea but the patch doesn't work :-( Without the patch: IXP4XX-Flash.0: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID 0x000089 Chip ID 0x008922 Intel/Sharp Extended Query Table at 0x010A Intel/Sharp Extended Query Table at 0x010A Intel/Sharp Extended Query Table at 0x010A Intel/Sharp Extended Query Table at 0x010A Intel/Sharp Extended Query Table at 0x010A Using buffer write method Using auto-unlock on power-up/resume cfi_cmdset_0001: Erase suspend on write enabled Searching for RedBoot partition table in IXP4XX-Flash.0 at offset 0x1fe0000 5 RedBoot partitions found on MTD device IXP4XX-Flash.0 With the patch applied: IXP4XX-Flash.0: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID 0x000089 Chip ID 0x008922 Intel/Sharp Extended Query Table at 0x010A Intel/Sharp Extended Query Table at 0x010A Intel/Sharp Extended Query Table at 0x010A Intel/Sharp Extended Query Table at 0x010A Intel/Sharp Extended Query Table at 0x010A Using buffer write method Using auto-unlock on power-up/resume cfi_cmdset_0001: Erase suspend on write enabled Searching for RedBoot partition table in IXP4XX-Flash.0 at offset 0x1fe0000 No RedBoot partition table detected in IXP4XX-Flash.0 > +++ b/drivers/mtd/maps/ixp4xx.c > -#ifndef __ARMEB__ > -#ifndef CONFIG_MTD_CFI_BE_BYTE_SWAP > -# error CONFIG_MTD_CFI_BE_BYTE_SWAP required > + > +#ifdef __ARMEB__ > +#define fixup_32(io) (io) > +#else > +#define fixup_32(io) (void __iomem *)((unsigned long)(io) ^ 2) > #endif > > static inline u16 flash_read16(void __iomem *addr) > { > - return be16_to_cpu(__raw_readw((void __iomem *)((unsigned long)addr ^ 0x2))); > + return be16_to_cpu(__raw_readw(fixup_32(addr))); > } > > static inline void flash_write16(u16 d, void __iomem *addr) > { > - __raw_writew(cpu_to_be16(d), (void __iomem *)((unsigned long)addr ^ 0x2)); > + __raw_writew(cpu_to_be16(d), fixup_32(addr)); > } The above change seems fine. > #define BYTE0(h) ((h) & 0xFF) > #define BYTE1(h) (((h) >> 8) & 0xFF) > > -#else ... > -#define BYTE0(h) (((h) >> 8) & 0xFF) > -#define BYTE1(h) ((h) & 0xFF) > -#endif This is used by ixp4xx_copy_from(). I don't exactly know what is MTD layer going to do with map.swap = CFI_BIG_ENDIAN, but I think the natural thing (= big endian on this CPU) is to copy data in big-endian order, and then maybe (in LE mode) swap it. In fact, it detects those RedBoot partitions when I use the original BE BYTE0/BYTE1 macros (this second set, deleted by your patch). Tested in BE mode only for now, can't give it more time at the moment but will do soon. CONFIG_MTD_CFI=y CONFIG_MTD_CFI_ADV_OPTIONS=y CONFIG_MTD_CFI_NOSWAP=y # CONFIG_MTD_CFI_BE_BYTE_SWAP is not set # CONFIG_MTD_CFI_LE_BYTE_SWAP is not set CONFIG_MTD_CFI_GEOMETRY=y CONFIG_MTD_CFI_I1=y # CONFIG_MTD_CFI_I2 is not set # CONFIG_MTD_CFI_I4 is not set # CONFIG_MTD_CFI_I8 is not set CONFIG_MTD_CFI_INTELEXT=y # CONFIG_MTD_CFI_AMDSTD is not set # CONFIG_MTD_CFI_STAA is not set CONFIG_MTD_CFI_UTIL=y
----- Original Message ----- > From: "Krzysztof Ha?asa" <khalasa@piap.pl> > Sent: Monday, September 29, 2014 2:50:08 AM > > Aaron Sierra <asierra@xes-inc.com> writes: > > > The .swap member of the map_info structure is provided for situations > > where a mapping must always be big or little endian regardless of the > > endianness of the system performing the access. > > I like the idea but the patch doesn't work :-( > > Without the patch: > IXP4XX-Flash.0: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID > 0x000089 Chip ID 0x008922 > Intel/Sharp Extended Query Table at 0x010A > Intel/Sharp Extended Query Table at 0x010A > Intel/Sharp Extended Query Table at 0x010A > Intel/Sharp Extended Query Table at 0x010A > Intel/Sharp Extended Query Table at 0x010A > Using buffer write method > Using auto-unlock on power-up/resume > cfi_cmdset_0001: Erase suspend on write enabled > Searching for RedBoot partition table in IXP4XX-Flash.0 at offset 0x1fe0000 > 5 RedBoot partitions found on MTD device IXP4XX-Flash.0 > > With the patch applied: > IXP4XX-Flash.0: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID > 0x000089 Chip ID 0x008922 > Intel/Sharp Extended Query Table at 0x010A > Intel/Sharp Extended Query Table at 0x010A > Intel/Sharp Extended Query Table at 0x010A > Intel/Sharp Extended Query Table at 0x010A > Intel/Sharp Extended Query Table at 0x010A > Using buffer write method > Using auto-unlock on power-up/resume > cfi_cmdset_0001: Erase suspend on write enabled > Searching for RedBoot partition table in IXP4XX-Flash.0 at offset 0x1fe0000 > No RedBoot partition table detected in IXP4XX-Flash.0 > > > +++ b/drivers/mtd/maps/ixp4xx.c > [ snip agreed code ] > > > #define BYTE0(h) ((h) & 0xFF) > > #define BYTE1(h) (((h) >> 8) & 0xFF) > > > > -#else > ... > > -#define BYTE0(h) (((h) >> 8) & 0xFF) > > -#define BYTE1(h) ((h) & 0xFF) > > -#endif > > This is used by ixp4xx_copy_from(). I don't exactly know what is MTD > layer going to do with map.swap = CFI_BIG_ENDIAN, but I think the > natural thing (= big endian on this CPU) is to copy data in big-endian > order, and then maybe (in LE mode) swap it. Your guess regarding .swap is correct. The swapping is performed by inline functions defined in include/linux/mtd/cfi.h via cfiX_to_cpu/cpu_to_cfiX macros. > > In fact, it detects those RedBoot partitions when I use the original BE > BYTE0/BYTE1 macros (this second set, deleted by your patch). > Tested in BE mode only for now, can't give it more time at the moment > but will do soon. Thanks Krzysztof, I see what I did wrong. I will submit a new patch that defines BYTE0/BYTE1 to work for both endian modes. -Aaron
diff --git a/drivers/mtd/maps/ixp4xx.c b/drivers/mtd/maps/ixp4xx.c index 6a589f1..541727e 100644 --- a/drivers/mtd/maps/ixp4xx.c +++ b/drivers/mtd/maps/ixp4xx.c @@ -26,6 +26,7 @@ #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> @@ -48,43 +49,30 @@ * | 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. + * this is accounted for by defining map_info.swap to be CFI_BIG_ENDIAN to + * 'unswap' the CFI commands and using cpu_to_be16/be16_to_cpu to 'unswap' + * the data. */ -#ifndef __ARMEB__ -#ifndef CONFIG_MTD_CFI_BE_BYTE_SWAP -# error CONFIG_MTD_CFI_BE_BYTE_SWAP required + +#ifdef __ARMEB__ +#define fixup_32(io) (io) +#else +#define fixup_32(io) (void __iomem *)((unsigned long)(io) ^ 2) #endif static inline u16 flash_read16(void __iomem *addr) { - return be16_to_cpu(__raw_readw((void __iomem *)((unsigned long)addr ^ 0x2))); + return be16_to_cpu(__raw_readw(fixup_32(addr))); } static inline void flash_write16(u16 d, void __iomem *addr) { - __raw_writew(cpu_to_be16(d), (void __iomem *)((unsigned long)addr ^ 0x2)); + __raw_writew(cpu_to_be16(d), fixup_32(addr)); } #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 - static map_word ixp4xx_read16(struct map_info *map, unsigned long ofs) { map_word val; @@ -200,6 +188,7 @@ static int ixp4xx_flash_probe(struct platform_device *dev) * Tell the MTD layer we're not 1:1 mapped so that it does * not attempt to do a direct access on us. */ + info->map.swap = CFI_BIG_ENDIAN; info->map.phys = NO_XIP; info->map.size = resource_size(dev->resource);
The .swap member of the map_info structure is provided for situations where a mapping must always be big or little endian regardless of the endianness of the system performing the access. Signed-off-by: Aaron Sierra <asierra@xes-inc.com> --- v2: Account for 32-bit swapping drivers/mtd/maps/ixp4xx.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-)