Hi,

* Russell King - ARM Linux <linux@arm.linux.org.uk> [081207 03:35]:
> On Fri, Dec 05, 2008 at 12:06:27PM -0800, Tony Lindgren wrote:
> > +#include <linux/clk.h>
> 
> Not required.
> 
> > +#include <linux/delay.h>
> 
> Not required.
> 
> > +#include <linux/err.h>
> 
> Not required.
> 
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> 
> May not be required.
> 

Fixed in the attached patch.

<snip>

> > +static void __init omap3pandora_flash_init(void)
> > +{
> > +	u8 cs = 0;
> > +	u8 nandcs = GPMC_CS_NUM + 1;
> > +
> > +	u32 gpmc_base_add = OMAP34XX_GPMC_VIRT;
> 
> If it's a virtual address why is it an integer?

Will be fixed in a separate patch, see below.

> > +
> > +	/* find out the chip-select on which NAND exists */
> > +	while (cs < GPMC_CS_NUM) {
> > +		u32 ret = 0;
> > +		ret = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
> > +
> > +		if ((ret & 0xC00) == 0x800) {
> > +			printk(KERN_INFO "Found NAND on CS%d\n", cs);
> > +			if (nandcs > GPMC_CS_NUM)
> > +				nandcs = cs;
> > +		}
> > +		cs++;
> > +	}
> 
> Shouldn't this be some generic bit of code?

Good point. I'll put together a separate patch against
gpmc.c that does this for other boards too.

I've also updated this patch to drop the NAND support, that will
be added in a separate patch.

> 
> > +
> > +	if (nandcs > GPMC_CS_NUM) {
> > +		printk(KERN_INFO "NAND: Unable to find configuration "
> > +				 "in GPMC\n ");
> > +		return;
> > +	}
> > +
> > +	if (nandcs < GPMC_CS_NUM) {
> > +		omap3pandora_nand_data.cs = nandcs;
> > +		omap3pandora_nand_data.gpmc_cs_baseaddr = (void *)
> > +			(gpmc_base_add + GPMC_CS0_BASE + nandcs * GPMC_CS_SIZE);
> > +		omap3pandora_nand_data.gpmc_baseaddr = (void *) (gpmc_base_add);
> 
> Unnecessary casts with proper typing of gpmc_base_add.
> 
> > +
> > +		printk(KERN_INFO "Registering NAND on CS%d\n", nandcs);
> 
> Shouldn't something also update omap3pandora_nand_resource?

Dropped in the attached patch.

Regards,

Tony