[U-Boot] [PATCHv3 3/3] 83xx, kmeter1: added NAND support

[Scott Wood]

On Mon, Jul 13, 2009 at 12:15:12PM +0200, Heiko Schocher wrote:
> +#define read_mode()	in_8((volatile unsigned char __iomem *) \
> +				CONFIG_NAND_MODE_REG)
> +#define write_mode(val)	out_8((volatile unsigned char __iomem *) \
> +				CONFIG_NAND_MODE_REG, val)
> +#define read_data()	in_8((volatile unsigned char __iomem *) \
> +				CONFIG_NAND_DATA_REG)
> +#define write_data(val)	out_8((volatile unsigned char __iomem *) \
> +				CONFIG_NAND_DATA_REG, val)

No need for volatile when using accessors.  If you kept a pointer around
instead of casting here, you could reasonably use the accessors directly
without needing wrappers...

If this is purely for U-Boot and not shared with Linux we can drop the
__iomem.

> +static void kpn_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
> +{
> +	u8	reg_val = read_mode();

No tab after "u8".

> +	if (ctrl & NAND_CTRL_CHANGE) {
> +		if ( ctrl & NAND_NCE)

No space after "(".

> +			reg_val = reg_val & ~KPN_CE1N;
> +		else
> +			reg_val = reg_val | KPN_CE1N;
> +		write_mode(reg_val);
> +	}
> +	if (cmd == NAND_CMD_NONE)
> +		return;
> +
> +	reg_val = reg_val & ~(KPN_ALE + KPN_CLE);
> +	if (ctrl & NAND_CLE)
> +		reg_val = reg_val | KPN_CLE;
> +	if (ctrl & NAND_ALE)
> +		reg_val = reg_val | KPN_ALE;

If ALE/CLE is sticky in the hardware mode register, you could probably
move this under NAND_CTRL_CHANGE and simplify things a little.

> diff --git a/include/configs/kmeter1.h b/include/configs/kmeter1.h
> index 811ba88..4de0dfc 100644
> --- a/include/configs/kmeter1.h
> +++ b/include/configs/kmeter1.h
> @@ -324,6 +324,12 @@
>  #define CONFIG_SYS_DTT_HYSTERESIS	3
>  #define CONFIG_SYS_DTT_BUS_NUM		(CONFIG_SYS_MAX_I2C_BUS)
> 
> +#if defined(CONFIG_CMD_NAND)
> +#define	CONFIG_NAND_KMETER1

No tab after #define.

> +#define CONFIG_SYS_MAX_NAND_DEVICE	1
> +#define CONFIG_SYS_NAND_BASE		CONFIG_SYS_PIGGY_BASE
> +#endif
> +
>  #if defined(CONFIG_PCI)
>  #define CONFIG_CMD_PCI
>  #endif

This file looks a little different in the current tree (2 rather than
CONFIG_SYS_MAX_I2C_BUS), so it wouldn't apply cleanly.

-Scott