Dear Jagan, Thanks for the feedback. I will check the patch and split it up in two. This fix is derived from the igloocommunity github u-boot repository, but their implementation was a bit too ad-hoc for my taste, using #ifdef CONFIG_SNOWBALL to override the alignment of the smc911x driver. I added the delay based on trial-and-error, and without the delay the device would not be detected correctly. Perhaps 25ms is a little bit too long, but since I have no documentation to back the exact value up I guess this will just do. If you please I can experiment with shorter periods, but I personally feel it's not worth the effort and risks having a u-boot that doesn't work based on physical properties of the device (and thus appears to fail "randomly"). Yours,
Roy --- Ursprüngliche Nachricht --- Von: Jagan Teki <jagannadh.t...@gmail.com> Datum: 18:07:30 14-12-2013 An: Roy Spliet <rspl...@eclipso.eu> Betreff: Re: [U-Boot] [PATCH 1/2] snowball: Add networking support > Try to fix - checkpatch.pl issues. > total: 2 errors, 4 warnings, 0 checks, 74 lines checked > > On Sat, Dec 14, 2013 at 10:09 PM, Roy Spliet <rspl...@eclipso.eu> wrote: > > > Signed-off-by: Roy Spliet <rspl...@eclipso.eu> > > --- > > board/st-ericsson/snowball/snowball.c | 11 +++++++++++ > > drivers/net/smc911x.h | 14 ++++++++++---- > > include/configs/snowball.h | 14 ++++++++++++++ > > 3 files changed, 35 insertions(+), 4 deletions(-) > > > > diff --git a/board/st-ericsson/snowball/snowball.c > > b/board/st-ericsson/snowball/snowball.c > > > index c3061e2..356beb1 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); > > + > > 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); > > +} > > TAG++ > > diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h > > index acae0cf..7fe55f1 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) > > + > > #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; > > } > TAG-- > > TAG++ ..TAG-- as this area of code is part of smc911x driver, better to create > > another patch for it. > > By the way does this change comes from bcz of snowball or a generic one? > > > -- > Thanks, > Jagan. > -------- > Jagannadha Sutradharudu Teki, > E: jagannadh.t...@gmail.com, P: +91-9676773388 > Engineer - System Software Hacker > U-boot - SPI Custodian and Zynq APSOC > Ln: http://www.linkedin.com/in/jaganteki > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot