[PATCH] ep93xx: Add support for Snapper CL15 module

[H Hartley Sweeten]

On Monday, February 08, 2010 1:12 PM, Ryan Mallon wrote:
>> If I read this correctly, you are making the NAND writeable when
>> starting (WPn is high).  Is there any way to write-protect the
>> device?
>>   
> Yes, that is correct. I have added comments to each of the NAND address
> bit defines and removed the unused SNAPPERCL15_NAND_CTRL one. I have
> also rewritten the above to assert SNAPPERCL15_NAND_WPN on each write,
> which gets rid of the first test. I have tested this and reading/writing
> to the device still works. The WPN line does allow the NAND to be made
> read-only, but we don't use this feature.
>
> Updated patch below. Can I still use your Acked-by?
>
> Signed-off-by: Ryan Mallon <ryan at bluewatersys.com>

Based on your changes below I have one more comment below.

[snip]

+#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.

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.

Regards,
Hartley