[U-Boot] [RFC PATCH 1/8] NAND: nand_spl/nand_boot.c: add 16-bit and readid support

[Scott Wood]

On Mon, Dec 27, 2010 at 05:47:01PM -0700, John Rigby wrote:
> Some platforms read the nand type to make configuration
> choices.  For example, some versions of OMAP3 Beagle use
> the NAND type as a hint of the DRAM type.

That seems rather hacky... are you sure it makes sense to use a single
U-Boot image across that set of boards?

> diff --git a/nand_spl/nand_boot.c b/nand_spl/nand_boot.c
> index 76b8566..1ae2cd0 100644
> --- a/nand_spl/nand_boot.c
> +++ b/nand_spl/nand_boot.c
> @@ -27,6 +27,15 @@
> 
>  static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS;
> 
> +static uint8_t nand_read_byte(struct nand_chip *chip)
> +{
> +#ifndef CONFIG_SYS_NAND_BUSWIDTH_16
> +       return readb(chip->IO_ADDR_R);
> +#else
> +       return (uint8_t) cpu_to_le16(readw(chip->IO_ADDR_R));
> +#endif

Are the endian assumptions in the above appropriate for all hardware?

Converting something away from CPU endianness and then using it as native
seems wrong in theory, even if in practice the right bytes end up in the
right place.

Unfortunately there seem to be no arch-neutral non-raw native-endian or
big-endian I/O accessors in U-Boot.  For that matter, we don't know if the
NAND controller is native endian or not (e.g.  a PCI-based NAND controller
in a big-endian system).

I suggest defining
CONFIG_SYS_NAND_BUSWIDTH_16LE/CONFIG_SYS_NAND_BUSWIDTH_16BE, and
implementing out_be16/in_be16/out_le16/in_le16 on each architecture that
needs to use this.

> +#ifndef CONFIG_SYS_NAND_BOOT_ECC_SCRATCH
> +#define CONFIG_SYS_NAND_BOOT_ECC_SCRATCH 0x10000
> +#endif

Where is this documented or used?

>  /*
> + * Get ready for booting from NAND.  This is for platforms
> + * that need to read nand data or nand chip id's before initializing
> + * SDRAM.
> + */
> +void nand_boot_init(struct nand_chip *nand_chip, nand_info_t (*nand_info))
> +{
> +       /*
> +        * Init board specific nand support
> +        */
> +       nand_chip->select_chip = NULL;
> +       nand_info->priv = nand_chip;
> +       nand_chip->IO_ADDR_R = nand_chip->IO_ADDR_W = (void  __iomem *)CONFIG_SYS_NAND_BASE;

Line length.

> +       nand_chip->dev_ready = NULL;    /* preset to NULL */
> +       board_nand_init(nand_chip);
> +
> +       if (nand_chip->select_chip)
> +               nand_chip->select_chip(nand_info, 0);
> +
> +}

No newline at end of block.

-Scott