| Message ID | 1302702918-6050-2-git-send-email-hans-christian.egtvedt@atmel.com |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 13 Apr 2011, Hans-Christian Egtvedt wrote: > This patch adds a new kconfig symbol to the MTD NAND system, MTD_NAND_BE_BUS. > This symbol is used by the nand base to properly convert the data read from the > bus into the format the CPU expects. > > The patch fixes 16-bit NAND flash support on big endian architectures (at least > AVR32) with NAND flash hooked up to a big endian external bus. > > Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> > --- > drivers/mtd/nand/Kconfig | 8 ++++++++ > drivers/mtd/nand/nand_base.c | 8 ++++++++ > 2 files changed, 16 insertions(+), 0 deletions(-) > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > index edec457..bb3a54a 100644 > --- a/drivers/mtd/nand/Kconfig > +++ b/drivers/mtd/nand/Kconfig > @@ -22,6 +22,14 @@ menuconfig MTD_NAND > > if MTD_NAND > > +config MTD_NAND_BE_BUS > + bool "NAND device on big endian bus" > + default n > + help > + This option will assume data read from the bus is in big endian > + format. This is vital when reading command values from the bus, as > + only the lower 8 bit are in use then. > + > config MTD_NAND_VERIFY_WRITE > bool "Verify NAND page writes" > help > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index c54a4cb..bbb0c1d 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -180,7 +180,11 @@ static uint8_t nand_read_byte(struct mtd_info *mtd) > static uint8_t nand_read_byte16(struct mtd_info *mtd) > { > struct nand_chip *chip = mtd->priv; > +#if CONFIG_MTD_NAND_BE_BUS > + return (uint8_t) be16_to_cpu(readw(chip->IO_ADDR_R)); > +#else > return (uint8_t) cpu_to_le16(readw(chip->IO_ADDR_R)); > +#endif } Wouldn't one expect a certain symmetry here, i.e. if one function is cpu_to_le16, the other one would be called cpu_to_be16 ? I guess in most cases cpu_to_le16 would be the same as le16_to_cpu, etc, but it still looks as if someone made a mistake. /Ricard
On Wed, 2011-04-13 at 15:55 +0200, Hans-Christian Egtvedt wrote: > This patch adds a new kconfig symbol to the MTD NAND system, MTD_NAND_BE_BUS. > This symbol is used by the nand base to properly convert the data read from the > bus into the format the CPU expects. > > The patch fixes 16-bit NAND flash support on big endian architectures (at least > AVR32) with NAND flash hooked up to a big endian external bus. > > Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> These kind of config options are bad and lead to confusion. You should try hard to auto-detect the endianness, or take this information from the boot-loader via the device tree.
On Thu, 2011-04-14 at 15:35 +0300, Artem Bityutskiy wrote: > On Wed, 2011-04-13 at 15:55 +0200, Hans-Christian Egtvedt wrote: > > This patch adds a new kconfig symbol to the MTD NAND system, MTD_NAND_BE_BUS. > > This symbol is used by the nand base to properly convert the data read from the > > bus into the format the CPU expects. > > > > The patch fixes 16-bit NAND flash support on big endian architectures (at least > > AVR32) with NAND flash hooked up to a big endian external bus. > > > > Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> > > These kind of config options are bad and lead to confusion. You should > try hard to auto-detect the endianness, or take this information from > the boot-loader via the device tree. AVR32 doesn't support the device tree. For AVR32 assuming big endian wiring on the EBI is sane, as the architecture is big-endian only. I don't know how other big endian architectures behaves, and I am puzzled that I am the only observer of this...
On Wed, 2011-04-13 at 16:11 +0200, Ricard Wanderlof wrote: > On Wed, 13 Apr 2011, Hans-Christian Egtvedt wrote: > > > This patch adds a new kconfig symbol to the MTD NAND system, MTD_NAND_BE_BUS. > > This symbol is used by the nand base to properly convert the data read from the > > bus into the format the CPU expects. > > > > The patch fixes 16-bit NAND flash support on big endian architectures (at least > > AVR32) with NAND flash hooked up to a big endian external bus. > > > > Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> > > --- > > drivers/mtd/nand/Kconfig | 8 ++++++++ > > drivers/mtd/nand/nand_base.c | 8 ++++++++ > > 2 files changed, 16 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > > index edec457..bb3a54a 100644 > > --- a/drivers/mtd/nand/Kconfig > > +++ b/drivers/mtd/nand/Kconfig > > @@ -22,6 +22,14 @@ menuconfig MTD_NAND > > > > if MTD_NAND > > > > +config MTD_NAND_BE_BUS > > + bool "NAND device on big endian bus" > > + default n > > + help > > + This option will assume data read from the bus is in big endian > > + format. This is vital when reading command values from the bus, as > > + only the lower 8 bit are in use then. > > + > > config MTD_NAND_VERIFY_WRITE > > bool "Verify NAND page writes" > > help > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > > index c54a4cb..bbb0c1d 100644 > > --- a/drivers/mtd/nand/nand_base.c > > +++ b/drivers/mtd/nand/nand_base.c > > @@ -180,7 +180,11 @@ static uint8_t nand_read_byte(struct mtd_info *mtd) > > static uint8_t nand_read_byte16(struct mtd_info *mtd) > > { > > struct nand_chip *chip = mtd->priv; > > +#if CONFIG_MTD_NAND_BE_BUS > > + return (uint8_t) be16_to_cpu(readw(chip->IO_ADDR_R)); > > +#else > > return (uint8_t) cpu_to_le16(readw(chip->IO_ADDR_R)); > > +#endif } > > Wouldn't one expect a certain symmetry here, i.e. if one function is > cpu_to_le16, the other one would be called cpu_to_be16 ? I guess in most > cases cpu_to_le16 would be the same as le16_to_cpu, etc, but it still > looks as if someone made a mistake. My other option for this patch is to remove the endianness conversion all together, it looks a bit weird, and AFAICT it indicates that the external NAND device is wired in a certain way to the bus interface. Perhaps someone with knowledge about this specific piece of code can shed some light on this?
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index edec457..bb3a54a 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -22,6 +22,14 @@ menuconfig MTD_NAND if MTD_NAND +config MTD_NAND_BE_BUS + bool "NAND device on big endian bus" + default n + help + This option will assume data read from the bus is in big endian + format. This is vital when reading command values from the bus, as + only the lower 8 bit are in use then. + config MTD_NAND_VERIFY_WRITE bool "Verify NAND page writes" help diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index c54a4cb..bbb0c1d 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -180,7 +180,11 @@ static uint8_t nand_read_byte(struct mtd_info *mtd) static uint8_t nand_read_byte16(struct mtd_info *mtd) { struct nand_chip *chip = mtd->priv; +#if CONFIG_MTD_NAND_BE_BUS + return (uint8_t) be16_to_cpu(readw(chip->IO_ADDR_R)); +#else return (uint8_t) cpu_to_le16(readw(chip->IO_ADDR_R)); +#endif } /** @@ -364,7 +368,11 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip) if (chip->options & NAND_BUSWIDTH_16) { chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos & 0xFE, page); +#if CONFIG_MTD_NAND_BE_BUS + bad = be16_to_cpu(chip->read_word(mtd)); +#else bad = cpu_to_le16(chip->read_word(mtd)); +#endif if (chip->badblockpos & 0x1) bad >>= 8; else
This patch adds a new kconfig symbol to the MTD NAND system, MTD_NAND_BE_BUS. This symbol is used by the nand base to properly convert the data read from the bus into the format the CPU expects. The patch fixes 16-bit NAND flash support on big endian architectures (at least AVR32) with NAND flash hooked up to a big endian external bus. Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> --- drivers/mtd/nand/Kconfig | 8 ++++++++ drivers/mtd/nand/nand_base.c | 8 ++++++++ 2 files changed, 16 insertions(+), 0 deletions(-)