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

Joe Hershberger
2012-07-10T21:41:24Z

Hi Marek,

On Fri, Dec 16, 2011 at 2:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On Friday 16 December 2011 13:13:33 Marek Vasut wrote:
>> > On Friday 16 December 2011 12:33:53 Mike Frysinger wrote:
>> > > 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
>> > ?
>>
>> do you actually have this issue ? there are plenty of theoretical
>> situations like this which would break a significant number (majority?) of
>> drivers in the tree. so unless this is a real case, i'd ignore it for now
>> and stick with what optimizes away to no overhead.
>
> Sadly, I almost do. Not now of course, but eventually, I'll be there :-(

Are you planning to improve this patch?

I think at the very least you should have a compile option to enable
this "functional interface" presumably behind a macro, but every user
of NE2000 should not have to pay the price. However, like Mike
suggested, you should only add this complexity if you actually have
this problem on a board.

Thanks,
-Joe