From: Marek Vasut <marek.vasut@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] NET: NE2000: Cleanup IO accessors
Date: Fri, 13 Jul 2012 12:50:49 +0200 [thread overview]
Message-ID: <201207131250.50047.marek.vasut@gmail.com> (raw)
In-Reply-To: <CANr=Z=aBCqUdTq-oibgX991rpaa7Z9z-uCyjmZKrbwH_CCiWgw@mail.gmail.com>
Dear Joe Hershberger,
> 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?
No, I'll eventually rework this one alongside the whole DM crap.
> 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
Best regards,
Marek Vasut
prev parent reply other threads:[~2012-07-13 10:50 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-11 1:09 [U-Boot] [PATCH 1/2] NET: NE2000: Cleanup IO accessors Marek Vasut
2011-12-11 1:09 ` [U-Boot] [PATCH 2/2] NET: NE2000: Hide dp83902a_priv_data_t into ne2k_private_data Marek Vasut
2011-12-11 7:24 ` [U-Boot] [PATCH 1/2] NET: NE2000: Cleanup IO accessors Mike Frysinger
2011-12-11 13:42 ` Marek Vasut
2011-12-12 5:45 ` Mike Frysinger
2011-12-12 10:17 ` Marek Vasut
2011-12-16 17:33 ` Mike Frysinger
2011-12-16 18:13 ` Marek Vasut
2011-12-16 19:32 ` Mike Frysinger
2011-12-16 20:36 ` Marek Vasut
2012-07-10 21:41 ` Joe Hershberger
2012-07-13 10:50 ` Marek Vasut [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201207131250.50047.marek.vasut@gmail.com \
--to=marek.vasut@gmail.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox