[PATCH] ep93xx: Add support for Snapper CL15 module

[Ryan Mallon]

H Hartley Sweeten wrote:
> On Monday, February 08, 2010 1:27 PM, H Hartley Sweeten wrote:
>> +#define SNAPPERCL15_NAND_BASE	(EP93XX_CS7_PHYS_BASE + SZ_16M)
>> +
>> +#define SNAPPERCL15_NAND_WPN	(1 << 8)  /* Write protect (active low) */
>> +#define SNAPPERCL15_NAND_ALE	(1 << 9)  /* Address latch */
>> +#define SNAPPERCL15_NAND_CLE	(1 << 10) /* Command latch */
>> +#define SNAPPERCL15_NAND_CEN	(1 << 11) /* Chip enable (active low) */
>> +#define SNAPPERCL15_NAND_RDY	(1 << 14) /* Device ready */
>> +
>> +#define NAND_CTRL_ADDR(chip) 	(chip->IO_ADDR_W + 0x40)
>> +
>> +static unsigned long nand_state;
>> +
>> +static void snappercl15_nand_cmd_ctrl(struct mtd_info *mtd, int cmd,
>> +				      unsigned int ctrl)
>> +{
>> +	struct nand_chip *chip = mtd->priv;
>> +	unsigned set;
>> +
>> +	if (ctrl & NAND_CTRL_CHANGE) {
>> +		set = SNAPPERCL15_NAND_CEN | SNAPPERCL15_NAND_WPN;
>> +
>> +		if (ctrl & NAND_NCE)
>> +			set &= ~SNAPPERCL15_NAND_CEN;
>> +		if (ctrl & NAND_CLE)
>> +			set |= SNAPPERCL15_NAND_CLE;
>> +		if (ctrl & NAND_ALE)
>> +			set |= SNAPPERCL15_NAND_ALE;
>> +
>> +		nand_state &= ~(SNAPPERCL15_NAND_CEN |
>> +				SNAPPERCL15_NAND_CLE |
>> +				SNAPPERCL15_NAND_ALE);
>> +		nand_state |= set;
>> +		__raw_writew(nand_state, NAND_CTRL_ADDR(chip));
>> +	}
>> +
>> +	if (cmd != NAND_CMD_NONE)
>> +		__raw_writew((cmd & 0xff) | nand_state, chip->IO_ADDR_W);
>> +}
>>
>> It's possible (though unlikely) for 'nand_state' to start off with an
>> unknown value.  The upper layers could call the function without 
>> NAND_CTRL_CHANGE set the first time.

I think the NAND sub-system guarantees this wont' happen.

>> You can use the gen_nand .probe callback to set the initial value.
>> This keeps you from having to keep the static 'first' variable that
>> used to be in the patch.

I could just change the initialisation of nand_state to:

  static unsigned long nand_state = SNAPPERCL15_NAND_WPN;

Rather than having a probe function. However, I don't think even this is
necessary.

> Actually, do you even need to save a cached value for 'nand_state'?
> 
> 1) The write to NAND_CTRL_ADDR sets all the control lines.
> 2) The write to IO_ADDR_W 'writes' the command to the chip.

I'm working based on a driver written for our 2.6.20 kernel (not by me),
and it uses the cached value there too. I tried making nand_state local
to the cmd_ctrl function but it doesn't work correctly. It correctly
identifies the chip, but then marks every block on the NAND as bad.

> Do you really need to OR nand_state with the cmd?

It appears so. Removing nand_state, or even replacing with just
SNAPPERCL15_NAND_WPN does not work.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934