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

[Wolfgang Denk]

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?

> -#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.

> -static inline byte SMC_inb(dword offset)
> +static inline byte SMC_io_inb(struct smc91111_device *smc, dword offset)
>  {
>  	word  _w;

...and here.

> @@ -264,18 +234,22 @@ static inline byte SMC_inb(dword offset)
>  	return (offset & 1) ? (byte)(_w >> 8) : (byte)(_w);
>  }
>  
> -static inline void SMC_outb(byte value, dword offset)
> +static inline void SMC_io_outb(struct smc91111_device *smc, byte value,
> +			       dword offset)
>  {
>  	word  _w;
>  
>  	_w = SMC_inw(offset & ~((dword)1));
>  	if (offset & 1)
> -			*((volatile word*)(SMC_BASE_ADDRESS+(offset & ~((dword)1)))) = (value<<8) | (_w & 0x00ff);
> +			*((volatile word*)(smc->regs+(offset & ~((dword)1)))) =
> +				(value<<8) | (_w & 0x00ff);
>  	else
> -			*((volatile word*)(SMC_BASE_ADDRESS+offset)) = value | (_w & 0xff00);
> +			*((volatile word*)(smc->regs+offset)) =
> +				value | (_w & 0xff00);
>  }

...and here.

> -static inline void SMC_insw(dword offset, volatile uchar* buf, dword len)
> +static inline void SMC_io_insw(struct smc91111_device *smc, dword offset,
> +			       volatile uchar* buf, dword len)
>  {
>  	volatile word *p = (volatile word *)buf;

...and here.

> @@ -286,7 +260,8 @@ static inline void SMC_insw(dword offset, volatile uchar* buf, dword len)
>  	}
>  }
>  
> -static inline void SMC_outsw(dword offset, uchar* buf, dword len)
> +static inline void SMC_io_outsw(struct smc91111_device *smc, dword offset,
> +				uchar* buf, dword len)
>  {
>  	volatile word *p = (volatile word *)buf;

...and here.

> -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?


> diff --git a/drivers/net/smc91111.h b/drivers/net/smc91111.h
> index 967addd..b2ed6a5 100644
> --- a/drivers/net/smc91111.h
> +++ b/drivers/net/smc91111.h
> @@ -72,6 +72,10 @@ typedef unsigned long int		dword;
>  
>  /* Because of bank switching, the LAN91xxx uses only 16 I/O ports */
>  
> +#ifndef SMC_BASE_ADDRESS
> +#define SMC_BASE_ADDRESS smc->regs
> +#endif
> +
>  #define	SMC_IO_EXTENT	16
>  
>  #ifdef CONFIG_PXA250
> @@ -301,8 +305,6 @@ typedef unsigned long int		dword;
>  
>  #endif  /* CONFIG_SMC_USE_IOFUNCS */
>  
> -#if defined(CONFIG_SMC_USE_32_BIT)
> -
>  #ifdef CONFIG_XSENGINE
>  #define	SMC_inl(r)	(*((volatile dword *)(SMC_BASE_ADDRESS+(r<<1))))

This should be fixed to use I/O accessors, too.



>  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?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Lack of skill dictates economy of style.                - Joey Ramone