Re: [PATCH 2/2] mtd: add switch to support NAND flash on big endian bus

[Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>]

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?

-- 
Hans-Christian Egtvedt