[U-Boot] [PATCH] mxc_nand: add nand driver for MX2/MX3

[Scott Wood]

On Mon, Aug 03, 2009 at 05:45:14AM +0400, Ilya Yanok wrote:
> +	if (this->options & NAND_BUSWIDTH_16) {
> +		void __iomem *main_buf = host->regs->main_area0;
> +		/* compress the ID info */
> +		writeb(readb(main_buf + 2), main_buf + 1);
> +		writeb(readb(main_buf + 4), main_buf + 2);
> +		writeb(readb(main_buf + 6), main_buf + 3);
> +		writeb(readb(main_buf + 8), main_buf + 4);
> +		writeb(readb(main_buf + 10), main_buf + 5);
> +	}

This indicates that read_byte isn't doing what the generic NAND code is
expecting.  It should advance by a word in the buffer for each byte read
-- the implementation of nand_read_byte16 confirms this.

In that case, though, why have separate read_byte and read_word?  Just
to provide an endian-broken alternative, that is only used where
endianness doesn't matter (checking bad block markers)? :-(

> +	if (col & 1) {
> +		union {
> +			uint16_t word;
> +			uint8_t bytes[2];
> +		} nfc_word;
> +
> +		nfc_word.word = readw(p);
> +		ret = nfc_word.bytes[1];
> +		nfc_word.word = readw(&p[1]);
> +		ret |= nfc_word.bytes[0] << 16;

This still has an endian assumption (and shouldn't << 16 be << 8, and
require a cast to avoid shifting beyond the type width?) -- it's moot
given the nature of read_byte and read_word, but if this were necessary
it should be something like:

union {
	...
} nfc_word[3];

nfc_word[0] = readw(p);
nfc_word[1] = readw(p + 1);

nfc_word[2].bytes[0] = nfc_word[0].bytes[1];
nfc_word[2].bytes[1] = nfc_word[1].bytes[0];

ret = nfc_word[2].word;

Alternatively, you could apply le16_to_cpu to the result of something
like your version.

-Scott