[U-Boot] [PATCH 3/3] smc91111: switch to MULTI_NET api

[Jean-Christophe PLAGNIOL-VILLARD]

On 21:12 Wed 12 Aug     , Wolfgang Denk wrote:
> Dear Jean-Christophe PLAGNIOL-VILLARD,
> 
> In message <1250023747-20224-3-git-send-email-plagnioj at jcrosoft.com> you wrote:
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj at jcrosoft.com>
> ...
> >  #ifdef	CONFIG_SMC_USE_32_BIT
> > -#define USE_32_BIT  1
> > +#define is_use_32bit(x) (x->use_32bit)
> 
> Does switching from a compile time check to a run time check not cause
> an avoidable growth of the memory footprint?
> 
> How much bigger is the new code?
none for the non 32 bit support it will only impact you if you do enable the
32bits support but the impact depend on the arch you use I've seen different
impact on arm and sh
but on the board I've test less than 300 bytes for dual support

with dual support
... configured to boot from Nand Flash
Configuring for nhk8815 board...
   text    data     bss     dec     hex filename
 189330    8132   24984  222446   364ee u-boot

with 32bit only
... configured to boot from Nand Flash
Configuring for nhk8815 board...
   text    data     bss     dec     hex filename
 189082    8132   24984  222198   363f6 u-boot

with non 32bit
... configured to boot from Nand Flash
Configuring for nhk8815 board...
   text    data     bss     dec     hex filename
 189050    8132   24984  222166   363d6 u-boot

> 
> > -#undef USE_32_BIT
> > +#define is_use_32bit(x) (0)
> >  #endif
> 
> 
> ...
> > -static inline word SMC_inw(dword offset)
> > +static inline word SMC_io_inw(struct smc91111_device *smc, dword offset)
> >  {
> >  	word v;
> > -	v = *((volatile word*)(SMC_BASE_ADDRESS+offset));
> > +	v = *((volatile word*)(smc->regs+offset));
> >  	barrier(); *(volatile u32*)(0xc0000000);
> >  	return v;
> >  }
> >  
> > -static inline void SMC_outw(word value, dword offset)
> > +static inline void SMC_io_outw(struct smc91111_device *smc, word value,
> > +			       dword offset)
> >  {
> > -	*((volatile word*)(SMC_BASE_ADDRESS+offset)) = value;
> > +	*((volatile word*)(smc->regs+offset)) = value;
> >  	barrier(); *(volatile u32*)(0xc0000000);
> >  }
> 
> Please use proper I/O accessor functions here.
this will be for an other patch I do not want to mix Net multi api update and I/O cleanup
> 
> > -static int smc_close()
> > +static void smc_halt(struct eth_device *netdev)
> >  {
> > +	struct smc91111_device *smc = to_smc91111(netdev);
> >  	PRINTK2("%s: smc_close\n", SMC_DEV_NAME);
> 
> You should also adapt the debug messages to the changed function
> names.
> 
> > +#ifdef CONFIG_SMC_USE_32_BIT
> > +#define USE_32BIT 1
> > +#else
> > +#define USE_32BIT 0
> > +#endif
> 
> Above you get rid of the USE_32BIT stuff; here you re-introduce it.
> Why?
yes it's different here it's to declare that you want to activate the 32bit for the default
driver init
> >  include/configs/ADNPESC1.h       |    1 +
> >  include/configs/DK1C20.h         |    1 +
> >  include/configs/DK1S10.h         |    1 +
> >  include/configs/EP1C20.h         |    1 +
> >  include/configs/EP1S10.h         |    1 +
> >  include/configs/EP1S40.h         |    1 +
> >  include/configs/MigoR.h          |    1 +
> >  include/configs/PK1C20.h         |    1 +
> >  include/configs/bf533-ezkit.h    |    1 +
> >  include/configs/bf533-stamp.h    |    1 +
> >  include/configs/bf538f-ezkit.h   |    1 +
> >  include/configs/bf561-ezkit.h    |    1 +
> >  include/configs/blackstamp.h     |    1 +
> >  include/configs/cerf250.h        |    1 +
> >  include/configs/cm-bf533.h       |    1 +
> >  include/configs/cm-bf561.h       |    1 +
> >  include/configs/cradle.h         |    1 +
> >  include/configs/delta.h          |    1 +
> >  include/configs/dnp1110.h        |    1 +
> >  include/configs/gr_cpci_ax2000.h |    1 +
> >  include/configs/gr_ep2s60.h      |    1 +
> >  include/configs/innokom.h        |    1 +
> >  include/configs/integratorcp.h   |    1 +
> >  include/configs/logodl.h         |    1 +
> >  include/configs/lpd7a400-10.h    |    1 +
> >  include/configs/lpd7a404-10.h    |    1 +
> >  include/configs/ms7722se.h       |    1 +
> >  include/configs/netstar.h        |    1 +
> >  include/configs/nhk8815.h        |    1 +
> >  include/configs/pxa255_idp.h     |    1 +
> >  include/configs/versatile.h      |    1 +
> >  include/configs/voiceblue.h      |    1 +
> >  include/configs/xaeniax.h        |    1 +
> >  include/configs/xm250.h          |    1 +
> >  include/configs/xsengine.h       |    1 +
> >  include/configs/zylonite.h       |    1 +
> 
> This is a pretty long list of boards which is affected. How many of
> these have actuaaly been tested with this patch, and which ones, and
> to what extent?
none will be impact as the share the same init and I've not modify any config
just active the multi support
and the code is tested on custom boards (4 differents) + nhk8815

and compile on arm & sh

Best Regards,
J.