Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Steve.Glendinning]

Hi all,

> > Of course, this is even more interesting given that there's a new, 
smsc911x
> > driver floating around on netdev that's supposed to replace the 
> existing one.
> > 
> > It's submitted by SMSC themselves, and has the support of the 
> smc911x author too...
> 
> I think that's been going on for more than a year. We used (and
> contributed to) the new driver in ARM Ltd from the early stages and we
> are happy with it. However, I'd like the mainline kernel to have a
> working configuration for the RealView boards without additional patches
> (we'll probably still use the new driver for our stable, mostly internal
> kernel releases).

The smsc911x driver is still maintained (out of tree), and is used by many
of our customers.  Mainline inclusion is still very much the intention!

Almost all the feedback I've had has been positive, but I've had a few
"can't you just fix the in-tree driver?" responses.  The trouble is it's
not a *small* piece of work!

Both drivers have their technical advantages:

smsc911x
- uses functions instead of macros, making it more readable
- uses napi (and manages significantly higher throughput because of it)
- uses phylib
- passes checkpatch.pl
- has workarounds for older LAN911x variants

smc911x
- is already in-tree (not really technical, but still important)
- has support for pxa dma*
- has dynamic bus width config

We could start from either driver to incorporate the extra features of
the other, but I think it would be significantly less work to start from
smsc911x.  What does everyone else think?

* I do have a patch to add DMA support to smsc911x, but it's
platform-specific to sh.  I haven't added pxa dma support only because I
don't have a suitable pxa platform available.

> Regarding the merging of the new driver, the last version posted was in
> June 2008 and the author stated that he's now focusing on the LAN9500
> USB Ethernet driver before re-posting the LAN911x one:
> 
> http://marc.info/?l=linux-netdev&m=122097407603180&w=2

The LAN9500 driver is in-tree now, so I'll be updating smsc911x for 
changes
in current git and reposting for review later today.

Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: steve.glendinning@smsc.com

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Catalin Marinas]

On Tue, 2008-10-21 at 11:26 +0100, Steve.Glendinning@smsc.com wrote:
> The smsc911x driver is still maintained (out of tree), and is used by many
> of our customers.  Mainline inclusion is still very much the intention!
> 
> Almost all the feedback I've had has been positive, but I've had a few
> "can't you just fix the in-tree driver?" responses.  The trouble is it's
> not a *small* piece of work!

Can we not have both drivers in the kernel until the transition to the
new one has finished? We seem to still have both eepro100.c and e100.c
in the kernel targeting similar chips.

-- 
Catalin

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Bill Gatliff]

Catalin Marinas wrote:
> On Tue, 2008-10-21 at 11:26 +0100, Steve.Glendinning@smsc.com wrote:
>> The smsc911x driver is still maintained (out of tree), and is used by many
>> of our customers.  Mainline inclusion is still very much the intention!
>>
>> Almost all the feedback I've had has been positive, but I've had a few
>> "can't you just fix the in-tree driver?" responses.  The trouble is it's
>> not a *small* piece of work!
> 
> Can we not have both drivers in the kernel until the transition to the
> new one has finished? We seem to still have both eepro100.c and e100.c
> in the kernel targeting similar chips.

I don't have a problem with that.  I have standardized my kernels on Steve's
driver, though, in anticipation of the transition.  So you know which one I'll
advocate for.  :)


b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Bill Gatliff]

Steve.Glendinning@smsc.com wrote:

> * I do have a patch to add DMA support to smsc911x, but it's
> platform-specific to sh.  I haven't added pxa dma support only because I
> don't have a suitable pxa platform available.

I have several PXA platforms with SMSC chips.  Let me know off-list if I can
help in any way.


b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Russell King - ARM Linux]

On Tue, Oct 21, 2008 at 11:26:42AM +0100, Steve.Glendinning@smsc.com wrote:
> Almost all the feedback I've had has been positive, but I've had a few
> "can't you just fix the in-tree driver?" responses.  The trouble is it's
> not a *small* piece of work!

Which is reasonable given your summary, since...

> smsc911x
> - uses functions instead of macros, making it more readable
> - uses napi (and manages significantly higher throughput because of it)
> - uses phylib
> - passes checkpatch.pl
> - has workarounds for older LAN911x variants
> 
> smc911x
> - is already in-tree (not really technical, but still important)
> - has support for pxa dma*
> - has dynamic bus width config

.. we want to avoid having two drivers for the same device, but not
supporting these features would be seen as a regression.

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Russell King - ARM Linux]

On Tue, Oct 21, 2008 at 11:42:06AM +0100, Catalin Marinas wrote:
> Can we not have both drivers in the kernel until the transition to the
> new one has finished? We seem to still have both eepro100.c and e100.c
> in the kernel targeting similar chips.

There has been several concerted efforts to eliminating one or other.
The problem is that eepro100.c works on some platform/chip combinations
that e100.c doesn't and, probably, vice versa.  So, removing either
causes people regressions.

That's the problem with merging two drivers, and then trying to remove
one later - the hardware they support diverges and then you need to
keep and maintain both.

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Steve.Glendinning]

Russell King - ARM Linux <linux@arm.linux.org.uk> wrote on 21/10/2008 
19:46:53:

> On Tue, Oct 21, 2008 at 11:26:42AM +0100, Steve.Glendinning@smsc.com 
wrote:
> > Almost all the feedback I've had has been positive, but I've had a few
> > "can't you just fix the in-tree driver?" responses.  The trouble is 
it's
> > not a *small* piece of work!
> 
> Which is reasonable given your summary, since...
> 
> > smsc911x
> > - uses functions instead of macros, making it more readable
> > - uses napi (and manages significantly higher throughput because of 
it)
> > - uses phylib
> > - passes checkpatch.pl
> > - has workarounds for older LAN911x variants
> > 
> > smc911x
> > - is already in-tree (not really technical, but still important)
> > - has support for pxa dma*
> > - has dynamic bus width config
>
> .. we want to avoid having two drivers for the same device, but not
> supporting these features would be seen as a regression.

Exactly, we already have two drivers.  Only one is currently in-tree, but
both have users.  The question is where we go from here?

I believe it's much easier to bring smsc911x up to feature parity with
smc911x than vice-versa.  The dynamic bus configuration is simple enough
to add, and Bill Gatliff has offered to help out with PXA DMA support.
I'd like us to reach a consensus first though, so we all know the plan
moving forward!

Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: steve.glendinning@smsc.com

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Steve.Glendinning]

Hi Jeff,

Russell's was the last response on this subject, so I guess there are no 
strong objections?

If I add the missing features (to bring the smsc911x driver up to feature 
parity with smc911x) before the next merge window, would you accept a 
patchset to add it and deprecate the existing driver?

Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: steve.glendinning@smsc.com



__________________

Russell King - ARM Linux <linux@arm.linux.org.uk> wrote on 21/10/2008 
19:46:53:

> On Tue, Oct 21, 2008 at 11:26:42AM +0100, Steve.Glendinning@smsc.com 
wrote:
> > Almost all the feedback I've had has been positive, but I've had a few
> > "can't you just fix the in-tree driver?" responses.  The trouble is 
it's
> > not a *small* piece of work!
> 
> Which is reasonable given your summary, since...
> 
> > smsc911x
> > - uses functions instead of macros, making it more readable
> > - uses napi (and manages significantly higher throughput because of 
it)
> > - uses phylib
> > - passes checkpatch.pl
> > - has workarounds for older LAN911x variants
> > 
> > smc911x
> > - is already in-tree (not really technical, but still important)
> > - has support for pxa dma*
> > - has dynamic bus width config
>
> .. we want to avoid having two drivers for the same device, but not
> supporting these features would be seen as a regression.

Exactly, we already have two drivers.  Only one is currently in-tree, but
both have users.  The question is where we go from here?

I believe it's much easier to bring smsc911x up to feature parity with
smc911x than vice-versa.  The dynamic bus configuration is simple enough
to add, and Bill Gatliff has offered to help out with PXA DMA support.
I'd like us to reach a consensus first though, so we all know the plan
moving forward!

Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: steve.glendinning@smsc.com

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Bill Gatliff]

Steve.Glendinning@smsc.com wrote:
> Hi Jeff,
> 
> Russell's was the last response on this subject, so I guess there are no 
> strong objections?
> 
> If I add the missing features (to bring the smsc911x driver up to feature 
> parity with smc911x) before the next merge window, would you accept a 
> patchset to add it and deprecate the existing driver?

By "deprecate" you mean "switch users over to the new driver"?


b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [PATCH 14/16] RealView: Use the in-kernel smc911x.c driver on RealView

[Steve.Glendinning]

Hi Bill,

Yes, I'm working now on a patchset to move all the in-tree users over. 
I'll post it for review/testing once it's complete.

Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: steve.glendinning@smsc.com



Bill Gatliff <bgat@billgatliff.com> wrote on 04/11/2008 20:25:31:

> Steve.Glendinning@smsc.com wrote:
> > Hi Jeff,
> > 
> > Russell's was the last response on this subject, so I guess there are 
no 
> > strong objections?
> > 
> > If I add the missing features (to bring the smsc911x driver up to 
feature 
> > parity with smc911x) before the next merge window, would you accept a 
> > patchset to add it and deprecate the existing driver?
> 
> By "deprecate" you mean "switch users over to the new driver"?
> 
> 
> b.g.
> -- 
> Bill Gatliff
> bgat@billgatliff.com