* [RFT PATCH v2] mtd: ixp4xx: Unrequire CONFIG_MTD_CFI_BE_BYTE_SWAP
[not found] <474471133.324147.1411508523465.JavaMail.zimbra@xes-inc.com>
@ 2014-09-23 21:43 ` Aaron Sierra
2014-09-29 7:50 ` Krzysztof Hałasa
0 siblings, 1 reply; 3+ messages in thread
From: Aaron Sierra @ 2014-09-23 21:43 UTC (permalink / raw)
To: linux-arm-kernel
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(-)
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);
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [RFT PATCH v2] mtd: ixp4xx: Unrequire CONFIG_MTD_CFI_BE_BYTE_SWAP
2014-09-23 21:43 ` [RFT PATCH v2] mtd: ixp4xx: Unrequire CONFIG_MTD_CFI_BE_BYTE_SWAP Aaron Sierra
@ 2014-09-29 7:50 ` Krzysztof Hałasa
2014-09-29 19:05 ` Aaron Sierra
0 siblings, 1 reply; 3+ messages in thread
From: Krzysztof Hałasa @ 2014-09-29 7:50 UTC (permalink / raw)
To: linux-arm-kernel
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
--
Krzysztof Halasa
Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland
^ permalink raw reply [flat|nested] 3+ messages in thread
* [RFT PATCH v2] mtd: ixp4xx: Unrequire CONFIG_MTD_CFI_BE_BYTE_SWAP
2014-09-29 7:50 ` Krzysztof Hałasa
@ 2014-09-29 19:05 ` Aaron Sierra
0 siblings, 0 replies; 3+ messages in thread
From: Aaron Sierra @ 2014-09-29 19:05 UTC (permalink / raw)
To: linux-arm-kernel
----- 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-29 19:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <474471133.324147.1411508523465.JavaMail.zimbra@xes-inc.com>
2014-09-23 21:43 ` [RFT PATCH v2] mtd: ixp4xx: Unrequire CONFIG_MTD_CFI_BE_BYTE_SWAP Aaron Sierra
2014-09-29 7:50 ` Krzysztof Hałasa
2014-09-29 19:05 ` Aaron Sierra
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.