[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aec7e5c30802201834h7eae8892n7b9434168944eace@mail.gmail.com>
Date: Thu, 21 Feb 2008 11:34:20 +0900
From: "Magnus Damm" <magnus.damm@...il.com>
To: "Nicolas Pitre" <nico@....org>
Cc: netdev@...r.kernel.org, lethal@...ux-sh.org,
akpm@...ux-foundation.org
Subject: Re: [PATCH 02/04] smc91x: introduce platform data flags
On Thu, Feb 21, 2008 at 12:58 AM, Nicolas Pitre <nico@....org> wrote:
> On Wed, 20 Feb 2008, Magnus Damm wrote:
>
> > This patch introduces struct smc91x_platdata and modifies the driver so
> > bus width is checked during run time using SMC_nBIT() instead of
> > SMC_CAN_USE_nBIT.
> >
> > Signed-off-by: Magnus Damm <damm@...l.co.jp>
> > ---
>
> NAK.
>
> The SMC91C111 (for example) is often used on devices which have a CPU
> clock barely higher than the network throughput, hence it is crutial for
> those devices to have the most efficient access possible to the chip or
> performance will suffer. This is the main reason behind the heavily
> macroized register access so things are always optimized for the data
> bus capabilities at compile time.
I understand that you want to keep the code as fast as possible. So do
I. But I want to remove the need to modify smc91x.h for each board I
want to support. It is much better in my opinion to have a wide range
of boards supported but a little bit lower throughput compared to only
a few boards supported and excellent performance... People can always
modify the header file by themselves if they want to squeeze out that
extra percent of performance. But maybe that's just me wanting to
support more boards than I have time for. =)
> This patch introduces a runtimes test on lp->cfg.flags for every
> register access even on those platforms not using the platform data
> based bus configuration at all.
Right. It's kind of hard to avoid the check if you want to select the
bus width using platform data though... =) I understand you want to
keep the compile time optimization working well, and this patch is not
so good in that regard.
> I think you should add a SMC_DYNAMIC_BUS_CONFIG and redefine SMC_nBITS()
> so they dereference cfg.flags only when it is defined.
Sure, good idea. I'll fix that. Do you mind if I make that the default
behavior? So boards that doesn't have any hard coded configuration in
smc91x.h can use the platform data flags method for configuration
instead.
Thanks for your feedback!
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists