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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to