[U-Boot] [PATCH 2/2] KB9202: Add NAND support

Scott Wood scottwood at freescale.com
Sat May 16 00:30:48 CEST 2009


Matthias Kaehlcke wrote:
> +/*
> + * Board-specific function to access the device ready signal.
> + */
> +static int kb9202_nand_ready(struct mtd_info *mtd)
> +{
> +	return (((AT91C_BASE_PIOC->PIO_PDSR) & KB9202_NAND_BUSY) != 0);
> +}

Use I/O accessors.

> +int board_nand_init(struct nand_chip *nand)
> +{
> +	unsigned	value;
[snip]
>> +	/* enable internal NAND controller */
>> +	value = *(AT91C_EBI_CSA);
>> +	value |= AT91C_EBI_CS3A_SMC_SmartMedia;
>> +	*(AT91C_EBI_CSA) = value;

This is a hardware register.  Surely it has a defined width?

> +	/* setup nand flash access (allow ample margin) */
> +	/* 4 wait states, 1 setup, 1 hold, 1 float for 8-bit device */
> +	((AT91PS_SMC2)AT91C_BASE_SMC2)->SMC2_CSR[3] =
> +		AT91C_SMC2_WSEN |
> +		(4 & AT91C_SMC2_NWS) |
> +		((1 << 8) & AT91C_SMC2_TDF) |
> +		AT91C_SMC2_DBW_8 |
> +		((1 << 24) & AT91C_SMC2_RWSETUP) |
> +		((1 << 29) & AT91C_SMC2_RWHOLD);

What is AT91PS_SMC2?  Please don't hide pointers inside typedefs. 
Please define symbols to have the proper type in the first place, rather 
than casting at the point of use.

> +#ifdef CONFIG_KB9202B_ATL
> +#define CFG_MAX_FLASH_BANKS    0
> +#else

Is this really the recommended way of turning off flash support?

-Scott


More information about the U-Boot mailing list