Hi Roy,

Re: the subject: please next time put the version in the subjetc line too. You may benefit from using patman, which helps managing patch series and handles such things as adding version in suject lines and Cc:ing people according to tags, etc. see tools/patman/README.

Also:

On Fri, 20 Dec 2013 13:32:34 +0100, Roy Spliet rspliet@eclipso.eu wrote:

Required for (but potentially not limited to) the snowball board. Implementation is inspired by the linux smsc911x implementation, but by using a (pre-compiler) constant, things should be optimised by the compiler for a shift of 0.

Signed-off-by: Roy Spliet rspliet@eclipso.eu

drivers/net/smc911x.h | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h index acae0cf..7144722 100644 --- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -19,6 +19,12 @@ CONFIG_SMC911X_16_BIT shall be set" #endif

+#ifndef CONFIG_SMC911X_SHIFT +#define CONFIG_SMC911X_SHIFT 0 +#endif

+#define smc911x_shift(i) ((i) << CONFIG_SMC911X_SHIFT)

I find that defining a macro just to shift a field by a constant value is over-engineering. It is not really worse to do the (i << NNN) at the point(s) of use, and the reader will immediately understand what's being done.

#if defined (CONFIG_SMC911X_32_BIT) static inline u32 __smc911x_reg_read(struct eth_device *dev, u32 offset) { @@ -37,14 +43,14 @@ void smc911x_reg_write(struct eth_device *dev, u32 offset, u32 val) #elif defined (CONFIG_SMC911X_16_BIT) static inline u32 smc911x_reg_read(struct eth_device *dev, u32 offset) {

• volatile u16 *addr_16 = (u16 *)(dev->iobase + offset);
• return ((*addr_16 & 0x0000ffff) | (*(addr_16 + 1) << 16));

• volatile u16 *addr_16 = (u16 *)(dev->iobase + smc911x_shift(offset));
• return (*addr_16 & 0x0000FFFF) | (*(addr_16 + smc911x_shift(1)) << 16);

} static inline void smc911x_reg_write(struct eth_device *dev, u32 offset, u32 val) {

• *(volatile u16 *)(dev->iobase + offset) = (u16)val;
• *(volatile u16 *)(dev->iobase + offset + 2) = (u16)(val >> 16);

• *(volatile u16 *)(dev->iobase + smc911x_shift(offset)) = (u16)val;
• *(volatile u16 *)(dev->iobase + smc911x_shift(offset + 2)) = (u16)val;

} #else #error "SMC911X: undefined bus width"

Amicalement,

Hi Roy,

On Fri, 20 Dec 2013 13:32:35 +0100, Roy Spliet rspliet@eclipso.eu wrote:

Signed-off-by: Roy Spliet rspliet@eclipso.eu

board/st-ericsson/snowball/snowball.c | 11 +++++++++++ include/configs/snowball.h | 14 ++++++++++++++ 2 files changed, 25 insertions(+)

diff --git a/board/st-ericsson/snowball/snowball.c b/board/st-ericsson/snowball/snowball.c index c3061e2..c9ab71f 100644 --- a/board/st-ericsson/snowball/snowball.c +++ b/board/st-ericsson/snowball/snowball.c @@ -245,6 +245,8 @@ int board_late_init(void) while (tstc()) (void) getc();

• mdelay(25);

Please explain (here at least, possible also in commit depending on the reason) why a 25 ms delay is needed -- and why it was not needed before.

• return 0;

}

@@ -338,3 +340,12 @@ int board_mmc_init(bd_t *bis) return 0; } #endif /* CONFIG_MMC */

+int board_eth_init(bd_t *bis) +{

• int error = smc911x_initialize(0, CONFIG_SMC911X_BASE);
• if (error)

• return error;
  

• return cpu_eth_init(bis);

+} diff --git a/include/configs/snowball.h b/include/configs/snowball.h index 9a069f3..6201b3f 100644 --- a/include/configs/snowball.h +++ b/include/configs/snowball.h @@ -95,6 +95,10 @@ #define CONFIG_CMD_FAT #define CONFIG_CMD_EXT2 #define CONFIG_CMD_SOURCE +#define CONFIG_CMD_DHCP +#define CONFIG_CMD_NET +#define CONFIG_CMD_NFS +#define CONFIG_CMD_PING

#ifndef CONFIG_BOOTDELAY #define CONFIG_BOOTDELAY 1 @@ -245,4 +249,14 @@

#define CFG_FSMC_BASE 0x80000000 /* FSMC Controller */

+/*

• • Networking
• */

+#define CONFIG_NET +#define CONFIG_NET_MULTI +#define CONFIG_SMC911X +#define CONFIG_SMC911X_16_BIT +#define CONFIG_SMC911X_SHIFT 1 +#define CONFIG_SMC911X_BASE 0x50000000

#endif /* __CONFIG_H */

Amicalement,