Hello Scott
Scott Wood wrote:
> 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.
OK, I fix it.
>> +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 "(".
Yep, you are right. Fixed.
>> + 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.
I try this out.
>> 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.
fixed.
>> +#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.
Hmm... this is the third patch of a patchset, so it apply cleanly, if
the other 2 patches are first applied ... or should I base this patch
against current nand-flash tree, because this patch goes through your
tree?
thanks for reviewing
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
