Michael-Luke Jones [off-list ref] writes:
Code placement:
Queue Manager & NPE code => arch/arm/mach-ixp4xx
WAN driver code => drivers/net/wan
Eth code => drivers/net/arm
Why would you want such placement?
Potential problems: header files would have to be moved to
include/asm-arm = headers pollution.
All 4 drivers are, in fact, network (related) drivers.
drivers/net/arm would probably make (some) sense if it was
a single (or not so single) Ethernet driver.
Kconfig:
I'm not convinced about 'config IXP4XX_NETDEVICES'. I'd lose it
together with the drivers/net/ixp4xx directory
It wouldn't make sense without the directory, no doubt.
Ethernet & HSS code should probably select NPE and QMGR (rather than
depend)
Actually, that's exactly what this patch do.
but these options should still be exposed in arch/arm/mach-
ixp4xx/Kconfig
Why exactly? They are network devices, who would expect them there?
How about the dependency mess (NET_ETHERNET etc.) that would be
created?
--
Krzysztof Halasa
On Mon, May 07, 2007 at 07:12:49PM +0200, Krzysztof Halasa wrote:
Michael-Luke Jones [off-list ref] writes:
quoted
Code placement:
Queue Manager & NPE code => arch/arm/mach-ixp4xx
WAN driver code => drivers/net/wan
Eth code => drivers/net/arm
Why would you want such placement?
Potential problems: header files would have to be moved to
include/asm-arm = headers pollution.
All 4 drivers are, in fact, network (related) drivers.
No.
- qmgr is a versatile hardware fifo stack, that is currently
used to exchange data with the NPE.
- the NPE can also be used as DMA engine and for crypto operations.
Both are not network related.
Additionally, the NPE is not only ixp4xx related, but is
also used in IXP23xx CPUs, so it could be placed in
arch/arm/common or arch/arm/xscale ?
- The MAC is used on IXP23xx, too. So the drivers for
both CPU familys only differ in the way they exchange
network packets between the NPE and the kernel.
drivers/net/arm would probably make (some) sense if it was
a single (or not so single) Ethernet driver.
If Queue Manager & NPE move to arch/.... , it can be a single file.
Christian Hohnstaedt
--
Christian Hohnstaedt
Software Engineer
Innominate Security Technologies AG /protecting industrial networks/
tel: +49.30.6392-3285 fax: +49.30.6392-3307
Albert-Einstein-Strasse 14, D-12489 Berlin, Germany
http://www.innominate.com
Register Court: AG Charlottenburg, HR B 81603
Management Board: Joachim Fietz, Dirk Seewald
Chairman of the Supervisory Board: Edward M. Stadum
-------------------------------------------------------------------
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
[Added Lennert Buytenhek to CC list]
Hey again,
quoted
Code placement:
Queue Manager & NPE code => arch/arm/mach-ixp4xx
WAN driver code => drivers/net/wan
Eth code => drivers/net/arm
Why would you want such placement?
Potential problems: header files would have to be moved to
include/asm-arm = headers pollution.
Headers for ixp4xx-specific hardware can surely live in the include/
asm-arm/arch-ixp4xx/ quite happily.
All 4 drivers are, in fact, network (related) drivers.
Despite their name, Network Processing Engines are independent
coprocessors which are only coincidentally attached to MACs for
ethernet / WAN purposes. If Intel would allow us to compile code for
these coprocessors, we could get them to do lots of things other than
networking.
In fact, we already kind of can. Crypto is not networking, and if the
kernel gains ixp4xx crypto support, that should be possible to enable
independently of networking. They can also function as DMA engines,
which should also be independent of networking functionality.
So, the NPE driver (which is basically ixp4xx specific) should be,
for practical purposes, networking-code agnostic. As it is a lump of
code talking to an architecture specific piece of hardware, it should
live in arch/arm/ rather than arch-independent drivers/
(NB: the publically reviewed version of Christian's ixp4xx_net driver
had exactly this file layout, see below)
quoted
Ethernet & HSS code should probably select NPE and QMGR (rather than
depend)
Actually, that's exactly what this patch do.
quoted
but these options should still be exposed in arch/arm/mach-
ixp4xx/Kconfig
Sorry, unclear. That sentence was meant as a coherent whole -
agreeing with you that the NPE dependency should use select but then
pointing out that you should still be able to turn NPE support on in
arch/arm/mach/ixp4xx/Kconfig even without selecting any of the
network drivers.
Why exactly? They are network devices, who would expect them there?
How about the dependency mess (NET_ETHERNET etc.) that would be
created?
For networking devices point, see above.
I don't fully understand the specifics, but Christian appeared to
avoid any dependency mess in the publically reviewed version of his
driver (as below).
As I understand it, functions to talk to the NPE should appear in the
NPE driver. The NPE driver should then be called by ethernet/wan/
crypto/dma(?) drivers to carry out the specific firmware-dependent
tasks. I haven't reviewed your code in detail, so I can't comment on
whether this is what you actually do or not.
==Links to the review of Christian's driver==
[1/7] Register & NPE definitions:
http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2007-January/
038082.html
[2/7] Platform devices (thought unnecessary by Lennert in his review):
http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2007-January/
038086.html
[3/7] Stub for Data/Address-Coherent mode setup:
http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2007-January/
038083.html
[4/7] QMGR driver:
http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2007-January/
038278.html
[5/7] NPE driver:
http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2007-January/
038085.html
[6/7] Ethernet driver:
http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2007-January/
038087.html
[7/7] Documentation:
http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2007-January/
038088.html
Sorry if I'm stating the obvious, but this is a public discussion and
I want to make sure everyone who reads this can see what I mean. If
they disagree with me despite this, so be it :)
Mike-Luke