[U-Boot] [PATCH 1/2] NET: NE2000: Cleanup IO accessors

[Mike Frysinger]

On Monday 12 December 2011 05:17:49 Marek Vasut wrote:
> > On Sunday 11 December 2011 08:42:07 Marek Vasut wrote:
> > > > On Saturday 10 December 2011 20:09:30 Marek Vasut wrote:
> > > > > Introduce ne2k_register_io(in, out), which allows user to supply
> > > > > two functions. One for reading data from the card, the other for
> > > > > writing data to the card. Then introduce drivers' private data,
> > > > > which carry pointers to these functions and are passed throughout
> > > > > the driver.
> > > > 
> > > > where are the users of this new API ?  as it stands, i just see
> > > > bloat. every register access is now an indirect function call ? 
> > > > what's the point
> > > 
> > > Go to ... drivers/net/ax88796.h ... and check how it's done now. It's
> > > just wrong. Now for .03 release I have pxa3xx support ready which uses
> > > just this chip and adding more sh^Htuff to that fill would be even
> > > worse bloat.
> > 
> > i agree, that code is terrible.  however, those code paths can be
> > trivially merged without the proposed bloat yours brings in.
> 
> So what's your suggested awesome clean solution?

there's no need to get snarky

rename ISA_OFFSET to CONFIG_NE2000_IO_OFFSET, then move the "2" to 
CONFIG_NE2000_IO_STRIDE, and move them both to the board config header.
then you get one unified set:

#define DP_IN(_b_, _o_, _d_) \
	(_d_) = readw((void *)((_b_) + ((_o_) * CONFIG_NE2000_IO_STRIDE) + \
		CONFIG_NE2000_IO_OFFSET));
etc...

if you really wanted to clean up the driver, the DP_XXX funcs would get turned 
into C code as static inline helpers, and the base + register offset would get 
turned into a C struct.

> > further, that code base
> > isn't even used by the ne2000 driver.
> 
> What are you talking about, did you even bother to look?

might want to cut the attitude.  it's really not adding anything.

of course i looked and i saw ne2000.h defining DP_OUT/etc...  but i missed the 
ugly ifdef logic with CONFIG_DRIVER_AX88796L in ne2000_base.c.

> > so again, the question stands: what exactly do you need to do different ?
> > looks to me like the DP_* macros should get punted in favor of io.h
> > accessors, and the register offsets rewritten into C structs.
> 
> Sure, but not every hardware accesses the registers the same way.

which is why we have asm/io.h in the first place.

on a semi-related note, these vu_{short,char,etc...} types need to get culled 
from the code base.  they're volatile markings in disguise ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20111216/4dd3f0a9/attachment.pgp>