> 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
Well sometimes it's hard to digest the style of your comments ;-) > > 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. Ok, so if you had two different piece of hardware that had different NE2000_IO_OFFSET and STRIDE, running the same u-boot, how'd you handle it ? > > > > 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. Agreed > > 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. That needs handling in a separate patch. > > > > 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 ... Agreed M _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot