Hi,
The next 3 patches:

[1/3] changes "depends on HDLC" to "select HDLC" for WAN/generic HDLC

      network drivers

[2/3] adds "fuse" functions to help determine installed IXP4xx CPU

      components and to reset/disable/enable them.

[3/3] adds IXP4xx drivers for: hardware queue manager, NPE (on-chip

      network coprocessors), built-in Ethernet ports, built-in HSS

      (sync serial) ports (currently only non-channelized HDLC).
Patch [3/3] requires patches [1/3] and [2/3]
The code is based on publicly available information:

- Intel IXP4xx Developer's Manual and others e-papers

- Intel IXP400 Access Library Software (BSD license)

- previous works by Christian Hohnstaedt <chohnstaedt@innominate.com>

  While I have decided to rewrite most things from scratch, his patch

  was a great help in understanding what's going on within the IXP400

  code (I took some fragments of his code as well).

  Thanks, Christian.
The code is tested with IXP425 CPU.
[1/3]

 drivers/net/wan/Kconfig                   |   31 +-
[2/3]

 include/asm-arm/arch-ixp4xx/ixp4xx-regs.h |   47 ++
[3/3]

 arch/arm/mach-ixp4xx/ixdp425-setup.c      |   27 +

 drivers/net/Kconfig                       |   34 +

 drivers/net/Makefile                      |    1

 drivers/net/ixp4xx/Makefile               |    4

 drivers/net/ixp4xx/ixp4xx_eth.c           | 1002 +++++++++++++++++++++++++++

 drivers/net/ixp4xx/ixp4xx_hss.c           | 1048 ++++++++++++++++++++++++++++

 drivers/net/ixp4xx/ixp4xx_npe.c           |  731 +++++++++++++++++++++

 drivers/net/ixp4xx/ixp4xx_qmgr.c          |  273 +++++++

 drivers/net/ixp4xx/npe.h                  |   41 +

 drivers/net/ixp4xx/qmgr.h                 |  124 +++

 drivers/net/wan/Kconfig                   |   10

 include/asm-arm/arch-ixp4xx/platform.h    |   19

--

Krzysztof Halasa

-

Ok, the status of my patches is as follows:

[PATCH] Use menuconfig objects II - netdev/wan

        the "menuconfig" WAN patch by Jan Engelhardt
[PATCH 1a/3] WAN Kconfig: change "depends on HDLC" to "select"

        the Kconfig changes for WAN (HDLC) drivers
[PATCH 2a/3] Intel IXP4xx network drivers

	IXP4xx "fuses" (installed on-chip CPU components) patch
[PATCH] Intel IXP4xx network drivers v.3 - QMGR

	IXP4xx hardware Queue Manager driver
[PATCH] Intel IXP4xx network drivers v.2 - NPE

	IXP4xx Network Processor Engine driver, needs "fuses" patch
[PATCH] Intel IXP4xx network drivers v.2 - Ethernet and HSS

	You guessed it, needs all of the above.
The:

[PATCH] Intel IXP4xx network drivers v.2 - QMGR

was faulty, thus v.3.
Fire again :-)

--

Krzysztof Halasa

-

Hello Krzysztof,
...

drivers/net/ixp4xx/ixp4xx_eth.c

drivers/net/ixp4xx/ixp4xx_npe.c

drivers/net/ixp4xx/ixp4xx_qmgr.c

...
suggestion: credit Christian for his work by putting his copyright in

the header as well?
Thanks to both of you!
Leon.

-

Hello,
Obviously, as I wrote, his work was a great help for me.

However, a copyright is a different thing, this is a complete

rewrite with maybe two small fragments adapted from his code

(including the "fuses" thing). I can put info in the sources,

though.

--

Krzysztof Halasa

-

Adds "fuse" functions to help determine installed IXP4xx CPU

components and to reset/disable/enable them (only NPE - network

coprocessors - can be reset and reenabled).
Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
diff --git a/include/asm-arm/arch-ixp4xx/ixp4xx-regs.h b/include/asm-arm/arch-ixp4xx/ixp4xx-regs.h

index 5d949d7..5652c41 100644

--- a/include/asm-arm/arch-ixp4xx/ixp4xx-regs.h

+++ b/include/asm-arm/arch-ixp4xx/ixp4xx-regs.h

@@ -607,4 +607,55 @@
 #define DCMD_LENGTH	0x01fff		/* length mask (max = 8K - 1) */
+/* Fuse Bits of IXP_EXP_CFG2 */

+#define IXP4XX_FUSE_RCOMP		(1 << 0)

+#define IXP4XX_FUSE_USB_DEVICE		(1 << 1)

+#define IXP4XX_FUSE_HASH		(1 << 2)

+#define IXP4XX_FUSE_AES			(1 << 3)

+#define IXP4XX_FUSE_DES			(1 << 4)

+#define IXP4XX_FUSE_HDLC		(1 << 5)

+#define IXP4XX_FUSE_AAL			(1 << 6)

+#define IXP4XX_FUSE_HSS			(1 << 7)

+#define IXP4XX_FUSE_UTOPIA		(1 << 8)

+#define IXP4XX_FUSE_NPEB_ETH0		(1 << 9)

+#define IXP4XX_FUSE_NPEC_ETH		(1 << 10)

+#define IXP4XX_FUSE_RESET_NPEA		(1 << 11)

+#define IXP4XX_FUSE_RESET_NPEB		(1 << 12)

+#define IXP4XX_FUSE_RESET_NPEC		(1 << 13)

+#define IXP4XX_FUSE_PCI			(1 << 14)

+#define IXP4XX_FUSE_ECC_TIMESYNC	(1 << 15)

+#define IXP4XX_FUSE_UTOPIA_PHY_LIMIT	(3 << 16)

+#define IXP4XX_FUSE_USB_HOST		(1 << 18)

+#define IXP4XX_FUSE_NPEA_ETH		(1 << 19)

+#define IXP4XX_FUSE_NPEB_ETH_1_TO_3	(1 << 20)

+#define IXP4XX_FUSE_RSA			(1 << 21)

+#define IXP4XX_FUSE_XSCALE_MAX_FREQ	(3 << 22)

+#define IXP4XX_FUSE_RESERVED		(0xFF << 24)

+

+#define IXP4XX_FUSE_IXP46X_ONLY (IXP4XX_FUSE_ECC_TIMESYNC |		\

+				 IXP4XX_FUSE_USB_HOST |			\

+				 IXP4XX_FUSE_NPEA_ETH |			\

+				 IXP4XX_FUSE_NPEB_ETH_1_TO_3 |		\

+				 IXP4XX_FUSE_RSA |			\

+				 IXP4XX_FUSE_XSCALE_MAX_FREQ)

+

+#ifndef __ASSEMBLY__

+

+static inline u32 ixp4xx_read_fuses(void)

+{

+	unsigned int fuses = ~*IXP4XX_EXP_CFG2;

+	fuses &a...
[ message continues ]

Adds IXP4xx drivers for built-in CPU components:

- hardware queue manager

- NPE (network coprocessors),

- Ethernet ports,

- HSS (sync serial) ports (currently only non-channelized HDLC).
Both Ethernet and HSS drivers use queue manager and NPE driver and

require external firmware file(s) available from www.intel.com.
"Platform device" definitions for Ethernet ports on IXDP425 development

platform are provided (though it has been tested on not yet available

IXP425-based hardware only)
Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
diff --git a/arch/arm/mach-ixp4xx/ixdp425-setup.c b/arch/arm/mach-ixp4xx/ixdp425-setup.c

index 04b1d56..0dc497f 100644

--- a/arch/arm/mach-ixp4xx/ixdp425-setup.c

+++ b/arch/arm/mach-ixp4xx/ixdp425-setup.c

@@ -101,10 +101,35 @@ static struct platform_device ixdp425_uart = {

 	.resource		= ixdp425_uart_resources

 };
+/* Built-in 10/100 Ethernet MAC interfaces */

+static struct mac_plat_info ixdp425_plat_mac[] = {

+	{

+		.phy		= 0,

+		.rxq		= 3,

+	}, {

+		.phy		= 1,

+		.rxq		= 4,

+	}

+};

+

+static struct platform_device ixdp425_mac[] = {

+	{

+		.name			= "ixp4xx_eth",

+		.id			= IXP4XX_ETH_NPEB,

+		.dev.platform_data	= ixdp425_plat_mac,

+	}, {

+		.name			= "ixp4xx_eth",

+		.id			= IXP4XX_ETH_NPEC,

+		.dev.platform_data	= ixdp425_plat_mac + 1,

+	}

+};

+

 static struct platform_device *ixdp425_devices[] __initdata = {

 	&ixdp425_i2c_controller,

 	&ixdp425_flash,

-	&ixdp425_uart

+	&ixdp425_uart,

+	&ixdp425_mac[0],

+	&ixdp425_mac[1],

 };
 static void __init ixdp425_init(void)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig

index a3d46ea..94dbfec 100644

--- a/drivers/net/Kconfig

+++ b/drivers/net/Kconfig

@@ -1891,6 +1891,16 @@ config NE_H8300

 source "drivers/net/fec_8xx/Kconfig"

 source "drivers/net/fs_enet/Kconfig"
+config IXP4XX_ETH

+	tristate "IXP4xx Ethernet support"

+	depends on ARCH_IXP4XX

+	select IXP4XX_NPE

+	select IXP4XX_QMGR

+	select MII

+	help

+	  Say Y here if you w...
[ message continues ]

(Note that this assignment depends on the firmware, and different

firmware versions use different queues -- you might want to add a

note about which firmware version this holds for.)

-

Immediate comments as follows:
(Krzysztof has already seen them in a private email but I'm putting

them out so people can publically disagree with me if I have got this

wrong.)
Code placement:

Queue Manager & NPE code => arch/arm/mach-ixp4xx

WAN driver code => drivers/net/wan

Eth code => drivers/net/arm
Kconfig:

I'm not convinced about 'config IXP4XX_NETDEVICES'. I'd lose it

together with the drivers/net/ixp4xx directory

Ethernet & HSS code should probably select NPE and QMGR (rather than

depend) but these options should still be exposed in arch/arm/mach-

ixp4xx/Kconfig
Michael-Luke Jones
PS: Please cc me on replies as I only subscribe to l-a-k.

-

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
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

-

[Added Lennert Buytenhek to CC list]
Headers for ixp4xx-specific hardware can surely live in the include/ 
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  
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  
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 ...
[ message continues ]

Having thought about it a bit more, a layout similar to the one

proposed by you may make sense.
Not sure about that. Intel doesn't say much about it, but I think

one can safely assume that while NPEs can probably be programmed

to do other things, their performance comes not from NPE firmware

but from specialized network coprocessors (not NPEs) which can only

do (in hardware) things like Ethernet, HDLC, bit sync, CRC16/32,

and MD5/SHA-1/DES/AES.
I think you can even use MD5 and SHA-1 without any firmware (but

would have to check this info).
Note that while certain CPUs have the same set of NPEs, they are

missing some network coprocessors and can't do, for example, crypto.
That's what the docs say. Not sure about real-life purpose of
Well, I'm told that (compatible) NPEs are present on other IXP CPUs.
Actually, the NPE code does two things:

a) initialized NPEs and downloades the firmware

b) exchanges control messages with NPEs.

--

Krzysztof Halasa

-

Adds a driver for built-in IXP4xx Ethernet MAC and HSS ports
Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
diff --git a/arch/arm/mach-ixp4xx/ixdp425-setup.c b/arch/arm/mach-ixp4xx/ixdp425-setup.c

index ec4f079..f20d39d 100644

--- a/arch/arm/mach-ixp4xx/ixdp425-setup.c

+++ b/arch/arm/mach-ixp4xx/ixdp425-setup.c

@@ -101,10 +101,35 @@ static struct platform_device ixdp425_uart = {

 	.resource		= ixdp425_uart_resources

 };
+/* Built-in 10/100 Ethernet MAC interfaces */

+static struct mac_plat_info ixdp425_plat_mac[] = {

+	{

+		.phy		= 0,

+		.rxq		= 3,

+	}, {

+		.phy		= 1,

+		.rxq		= 4,

+	}

+};

+

+static struct platform_device ixdp425_mac[] = {

+	{

+		.name			= "ixp4xx_eth",

+		.id			= IXP4XX_ETH_NPEB,

+		.dev.platform_data	= ixdp425_plat_mac,

+	}, {

+		.name			= "ixp4xx_eth",

+		.id			= IXP4XX_ETH_NPEC,

+		.dev.platform_data	= ixdp425_plat_mac + 1,

+	}

+};

+

 static struct platform_device *ixdp425_devices[] __initdata = {

 	&ixdp425_i2c_controller,

 	&ixdp425_flash,

-	&ixdp425_uart

+	&ixdp425_uart,

+	&ixdp425_mac[0],

+	&ixdp425_mac[1],

 };
 static void __init ixdp425_init(void)

diff --git a/drivers/net/arm/Kconfig b/drivers/net/arm/Kconfig

index 678e4f4..5e2acb6 100644

--- a/drivers/net/arm/Kconfig

+++ b/drivers/net/arm/Kconfig

@@ -46,3 +46,13 @@ config EP93XX_ETH

 	help

 	  This is a driver for the ethernet hardware included in EP93xx CPUs.

 	  Say Y if you are building a kernel for EP93xx based devices.

+

+config IXP4XX_ETH

+	tristate "IXP4xx Ethernet support"

+	depends on NET_ETHERNET && ARM && ARCH_IXP4XX

+	select IXP4XX_NPE

+	select IXP4XX_QMGR

+	select MII

+	help

+	  Say Y here if you want to use built-in Ethernet ports

+	  on IXP4xx processor.

diff --git a/drivers/net/arm/Makefile b/drivers/net/arm/Makefile

index a4c8682..7c812ac 100644

--- a/drivers/net/arm/Makefile

+++ b/drivers/net/arm/Makefile

@@ -9,3 +9,4 @@ obj-$(CONFIG_ARM_ETHER3)	+= ether3.o

 obj-$(CONFIG_ARM_ETHER1)	+= ether1.o

 o...
[ message continues ]

As with Christian's driver (I'm feeling like a bit of a broken record

here :-), putting knowledge of which queue to use (which is firmware-

specific) in the _board_ support file is almost certainly wrong.
I would just put the port number in there, and let the ethernet

driver map the port number to the hardware queue number.  After all,

the ethernet driver knows which queues the firmware uses, while the
I generally discourage the use of such wrappers, as it often makes

people forget that the set and clear operations are not atomic, and

it ignores the fact that some of the other bits in the register you

are modifying might have side-effects.
Didn't review the rest -- not sure whether I'm looking at the latest

version.

-

No, quite the opposite. The board code knows its set of hardware

interfaces etc. and can let Ethernet driver use, say, HSS queues.

The driver can't know that.
It would make sense if we had many queues, but it doesn't seem

the case (perhaps the upper queues could be used for some

purposes, but Intel's code doesn't use them either and they
Without them the code in question is hardly readable, I pick the need

to remember about non-atomicity and possible side effects instead :-)
I've outlined the current versions in a separate mail, generally

2 patches are marked "v.2" and one "v.3".

--

Krzysztof Halasa

-

You are attacking a point that I did not make.
The board support code knows such things as that the "front ethernet

port" on the board is connected to the CPU's MII port number #2, but

the board support code does _not_ know that MII port number #2

corresponds to "ixp4xx hardware queue #5."
If Intel puts out a firmware update next month, and your ethernet

driver is modified to take advantage of the new features in that

firmware and starts depending on the newer version of that firmware,

we will have to modify every ixp4xx board support file in case the

firmware update modifies the ixp4xx queue numbers in use.  The

mapping from hardware ports (MII port #0, MII port #6, HSS port

#42, whatever) to ixp4xx hardware queue numbers (0-63) should _not_

be put in every single ixp4xx board support file.
Even if you only change the
	(in board support file)

	.rxq		= 4,
line to something like this instead:
	(in some ixp4xx-specific or driver-specific header file)

	#define IXP4XX_MII_PORT_1_RX_QUEUE		4
	(in board support file)

	.rxq		= IXP4XX_MII_PORT_1_RX_QUEUE,
then you have remved this dependency, and then you only have to update
You can read Polish, how can you complain about code readability. :-))
Sure, point taken, it's just that the person after you might not

remember..

-

Sure. And I don't want it to know.
It has to pick up any available queue for RX, that is. If the

board code knows it uses ETH connected to NPE-B and NPE-C, and

HSS-0 connected (obviously) to NPE-A, and it wants some crypto

functions etc., it can pick a queue which normally belongs to

HSS-1. If the code knows the board has both HSS and only NPE-B

Ethernet, it can use one of NPE-C Ethernet's queues. It's that

simple.
The Ethernet (and HSS etc.) driver knows it has to use queue 24 for

NPE-B Ethernet's TX and 27 for TX and so on, this is fixed in the

firmware so I don't let the board code mess with that. The

Ethernet RX queue is different, we can just make something up and

tell NPE about that.
That's BTW the same thing you would want to do with SRAM - except

that the SRAM allocator is technically possible, while making
Nope, we just modify Ethernet driver:

drivers/net/arm/ixp4xx_eth.c:

#define TX_QUEUE(plat)          (NPE_ID(port) + 23)

#define RXFREE_QUEUE(plat)      (NPE_ID(port) + 26)
I've never considered doing that :-)
drivers/net/wan/ixp4xx_hss.c:

/* Queue IDs */

#define HSS0_CHL_RXTRIG_QUEUE   12      /* orig size = 32 dwords */

#define HSS0_PKT_RX_QUEUE       13      /* orig size = 32 dwords */

#define HSS0_PKT_TX0_QUEUE      14      /* orig size = 16 dwords */

#define HSS0_PKT_TX1_QUEUE      15

#define HSS0_PKT_TX2_QUEUE      16

#define HSS0_PKT_TX3_QUEUE      17

#define HSS0_PKT_RXFREE0_QUEUE  18      /* orig size = 16 dwords */

#define HSS0_PKT_RXFREE1_QUEUE  19

#define HSS0_PKT_RXFREE2_QUEUE  20

#define HSS0_PKT_RXFREE3_QUEUE  21

#define HSS0_PKT_TXDONE_QUEUE   22      /* orig size = 64 dwords */
#define HSS1_CHL_RXTRIG_QUEUE   10

#define HSS1_PKT_RX_QUEUE       0

#define HSS1_PKT_TX0_QUEUE      5

#define HSS1_PKT_TX1_QUEUE      6

#define HSS1_PKT_TX2_QUEUE      7

#define HSS1_PKT_TX3_QUEUE      8

#define HSS1_PKT_RXFREE0_QUEUE  1

#define HSS1_PKT_RXFREE1_QUEUE  2

#define HSS1_PKT_RXFREE2_QUEUE  3

#define HSS1_PKT_RXFREE3_QUEUE  4
Well, you may ...
[ message continues ]

A final submission should probably have this platform data separated

from the net driver and sent upstream via Russell's patch tracking  
This has gone from error to warning - fair play but if are planning

to put this upstream this cycle (anything's possible :) ) you'll want

to declare this driver broken on ARMEB in Kconfig please.
I think my comment about adding randomised MAC addresses in case of

no hwaddr stands - it's really not that complex.
Christian's driver did this:
	/* The place of the MAC address is very system dependent.

	 * Here we use a random one to be replaced by one of the

	 * following commands:

	 * "ip link set address 02:03:04:04:04:01 dev eth0"

	 * "ifconfig eth0 hw ether 02:03:04:04:04:07"

	*/
	if (is_zero_ether_addr(plat->hwaddr)) {

		random_ether_addr(dev->dev_addr);

		dev->dev_addr[5] = plat->phy_id;

	}

	else
For our flash and eeprom notifiers to work, we need this converted to

a late_initcall:

http://trac.nslu2-linux.org/kernel/browser/trunk/patches/2.6.21/37-

ixp4xx-net-driver-fix-mac-handling.patch
akpm suggested this fix, but we don't absolutely know if it's  
Personally I'm less fussed about WAN / LE support. Anyone with any

sense will run ixp4xx boards doing such a specialised network

operation as BE. Also, NSLU2-Linux can't test this functionality with

our LE setup as we don't have this hardware on-board. You may just

want to declare a depends on ARMEB in Kconfig (with or without OR  
Greater-than-one-line comments not conforming to Kernel coding style  
[snip]
Again, looking good.
Michael-Luke Jones
-

ACK.
I shall presume that the ARM folks will apply these patches.  You may

tack on an "Acked-by: Jeff Garzik <jeff@garzik.org>" onto the ethernet

driver itself.
I'll let the ARM folks review the rest.
I do agree with the comments in the thread that -- as in your most

recent revision -- the non-eth code belongs in arch/arm.
	Jeff
-

Adds a driver for built-in IXP4xx hardware Queue Manager.
Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
diff --git a/arch/arm/mach-ixp4xx/Kconfig b/arch/arm/mach-ixp4xx/Kconfig

index 9715ef5..71ef55f 100644

--- a/arch/arm/mach-ixp4xx/Kconfig

+++ b/arch/arm/mach-ixp4xx/Kconfig

@@ -176,6 +176,12 @@ config IXP4XX_INDIRECT_PCI

 	  need to use the indirect method instead. If you don't know

 	  what you need, leave this option unselected.
+config IXP4XX_QMGR

+	tristate "IXP4xx Queue Manager support"

+	help

+	  This driver supports IXP4xx built-in hardware queue manager

+	  and is automatically selected by Ethernet and HSS drivers.

+

 endmenu
 endif

diff --git a/arch/arm/mach-ixp4xx/Makefile b/arch/arm/mach-ixp4xx/Makefile

index 3b87c47..f8e1afc 100644

--- a/arch/arm/mach-ixp4xx/Makefile

+++ b/arch/arm/mach-ixp4xx/Makefile

@@ -26,3 +26,4 @@ obj-$(CONFIG_MACH_NAS100D)	+= nas100d-setup.o nas100d-power.o

 obj-$(CONFIG_MACH_DSMG600)      += dsmg600-setup.o dsmg600-power.o
 obj-$(CONFIG_PCI)		+= $(obj-pci-$(CONFIG_PCI)) common-pci.o

+obj-$(CONFIG_IXP4XX_QMGR)	+= ixp4xx_qmgr.o

diff --git a/arch/arm/mach-ixp4xx/ixp4xx_qmgr.c b/arch/arm/mach-ixp4xx/ixp4xx_qmgr.c

new file mode 100644

index 0000000..b9e9bd6

--- /dev/null

+++ b/arch/arm/mach-ixp4xx/ixp4xx_qmgr.c

@@ -0,0 +1,273 @@

+/*

+ * Intel IXP4xx Queue Manager driver for Linux

+ *

+ * Copyright (C) 2007 Krzysztof Halasa <khc@pm.waw.pl>

+ *

+ * This program is free software; you can redistribute it and/or modify it

+ * under the terms of version 2 of the GNU General Public License

+ * as published by the Free Software Foundation.

+ */

+

+#include <linux/interrupt.h>

+#include <linux/kernel.h>

+#include <asm/io.h>

+#include <asm/arch/qmgr.h>

+

+#define DEBUG		0

+

+struct qmgr_regs __iomem *qmgr_regs;

+static struct resource *mem_res;

+static spinlock_t qmgr_lock;

+static u32 used_sram_bitmap[4]; /* 128 16-dword pages */

+static void (*irq_handlers[HALF_QUEUES])(void *pdev);

+s...
[ message continues ]

I'm not sure what the latest versions are, so I'm not sure which

patches to review and which patches are obsolete.
The queue manager interrupts should probably be implemented as an

irqchip, in the same way that GPIO interrupts are implemented.  (I.e.

allocate 'real' interrupt numbers for them, and use the interrupt

cascade mechanism.)  You probably want to have separate irqchips for

the upper and lower halves, too.  This way, drivers can just use

request_irq() instead of having to bother with platform-specific

qmgr_set_irq() methods.  I think I also made this review comment
As with Christian's driver, I don't know whether an SRAM allocator

makes much sense.  We can just set up a static allocation map for the

in-tree drivers and leave out the allocator altogether.  I.e. I don't

think it's worth the complexity (and just because the butt-ugly Intel

code has an allocator isn't a very good reason. :-)
I.e. an API a la:
	ixp4xx_qmgr_config_queue(int queue_nr, int sram_base_address, int queue_size, ...);
might simply suffice.

-

It's a very simple allocator. I don't whink we have enough SRAM

without it. For now it would work but it's probably too small

for all potential users at a time.
There may be up to 6 Ethernet ports (not sure about hardware

status, not yet supported even by Intel) - 7 queues * 128 entries

each = ~ 3.5 KB. Add 2 long queues (RX) for HSS and something

for TX, and then crypto, and maybe other things.
Current allocator have its potential problems, but they can be

solved internally (fragmentation, be we tend to use only

128-entry queues (RX and TX-ready Ethernet pool) and short,

16-entry ones (TX) - easy to deal with).

--

Krzysztof Halasa

-

See for example arch/arm/mach-ep93xx/core.c, handling of the A/B/F

port GPIO interrupts.
In a nutshell, it goes like this.
1) Allocate a set of IRQ numbers.  E.g. in include/asm-arm/arch-ixp4xx/irqs.h:
	#define IRQ_IXP4XX_QUEUE_0	64

	#define IRQ_IXP4XX_QUEUE_1	65

	[...]
   Adjust NR_IRQS, too.
2) Implement interrupt chip functions:
	static void ixp4xx_queue_low_irq_mask_ack(unsigned int irq)

	{

		[...]

	}
	static void ixp4xx_queue_low_irq_mask(unsigned int irq)

	{

		[...]

	}
	static void ixp4xx_queue_low_irq_unmask(unsigned int irq)

	{

		[...]

	}
	static void ixp4xx_queue_low_irq_set_type(unsigned int irq)

	{

		[...]

	}
	static struct irq_chip ixp4xx_queue_low_irq_chip = {

		.name		= "QMGR low",

		.ack		= ixp4xx_queue_low_irq_mask_ack,

		.mask		= ixp4xx_queue_low_irq_mask,

		.unmask		= ixp4xx_queue_low_irq_unmask,

		.set_type	= ixp4xx_queue_low_irq_set_type,

	};
3) Hook up the queue interrupts:
	for (i = IRQ_IXP4XX_QUEUE_0; i <= IRQ_IXP4XX_QUEUE_31; i++) {

		set_irq_chip(i, &ixp4xx_queue_low_irq_chip);

		set_irq_handler(i, handle_level_irq);

		set_irq_flags(i, IRQF_VALID);

	}
4) Implement an interrupt handler for the parent interrupt:
	static void ixp4xx_qmgr_low_irq_handler(unsigned int irq, struct irq_des c *desc)

	{

		u32 status;

		int i;
		status = __raw_readl(IXP4XX_WHATEVER_QMGR_LOW_STATUS_REGISTER);

		for (i = 0; i < 32; i++) {

			if (status & (1 << i)) {

				desc = irq_desc + IRQ_IXP4XX_QUEUE_0 + i;

				desc_handle_irq(IRQ_IXP4XX_QUEUE_0 + i, desc);

			}

		}

	}
5) Hook up the parent interrupt:
	set_irq_chained_handler(IRQ_IXP4XX_QM1, ixp4xx_qmgr_low_irq_handler);
You're unlikely to be using all of those at the same time, though.
And what do you do if the user does compile all of these features into

his kernel and then tries to use them all at the same time?  Return

-ENOMEM?
Shouldn't we make sure that at least the features that are compiled in

can be used at the same time?  If you wa...
[ message continues ]

If he is able to do so, yes - there is nothing we can do. But

I suspect a single machine would not have all possible hardware.

The problem is, we don't know what would it have, so it must be
We can't - hardware capabilities limit that. A general purpose

distribution would probably want to compile in everything (perhaps
That would be most limiting with IMHO no visible advantage.

--

Krzysztof Halasa

-

Well, you _would_ like to have a way to make sure that all the

capabilities on the board can be used.  If you have a future ixp4xx

based board with 16 ethernet ports, you don't want 'ifconfig eth7 up'

to give you -ENOMEM just because we ran out of SRAM.
The way I see it, that means that you do want to scale back your

other SRAM allocations if you know that you're going to need a lot

of SRAM (say, for ethernet RX/TX queues.)
Either you can do this with an ugly hack a la:
	/*

	 * The FOO board has many ethernet ports, and runs out of

	 * SRAM prematurely if we use the default TX/RX ring sizes.

	 */

	#ifdef CONFIG_MACH_IXP483_FOO_BOARD

	#define IXP4XX_ETH_RXTX_QUEUE_SIZE	32

	#else

	#define IXP4XX_ETH_RXTX_QUEUE_SIZE	256

	#endif
Or you can put this knowledge in the board support code (cleaner, IMHO.)
E.g. let arch/arm/mach-ixp4xx/nslu2.c decide, at platform device

instantiation time, which region of queue SRAM can be used by which

queue, and take static allocations for things like the crypto unit

into account.  (This is just one form of that idea, there are many

different variations.)
That way, you can _guarantee_ that you'll always have enough SRAM

to be able to use the functionality that is exposed on the board you

are running on (which is a desirable property, IMHO), which is

something that you can't achieve with an allocator, as far as I can

see.
I'm not per se against the allocator, I just think that there are

problems (running out of SRAM, fragmentation) that can't be solved

by the allocator alone (SRAM users have to be aware which other SRAM

users there are in the system, while the idea of the allocator is to

insulate these users from each other), and any solution that solves

those two problems IMHO also automatically solves the problem that

the allocator is trying to solve.

-

Yep, I will then add "queue_size" parameter to the platform data.
I'd have to put SRAM address in the board code instead. Certainly

not required at this point, and perhaps it will be never needed.

--

Krzysztof Halasa

-

On 5/8/07, Lennert Buytenhek <buytenh@wantstofly.org> wrote:
Is the qmgr used when the NPEs are utilized as DMA engines? And is the

allocator needed in this case? If yes, I beg you not to drop it,

because we use one NPE for this purpose, and if we are going to adopt

this driver instead of the intel's one, you will receive a patch

adding DMA functionality very soon. ;)

-

If you statically partition the available queue SRAM, no.

-

Here, unlike ixp4xx_npe.c defines are in qmgr.h - that seems a bit  
Great work,
Michael-Luke
-

Because they are public interface :-)

--

Krzysztof Halasa

-

Adds a driver for built-in IXP4xx Network Processor Engines.

This patch requires IXP4xx Queue Manager driver and the "fuses" patch.
Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
diff --git a/arch/arm/mach-ixp4xx/Kconfig b/arch/arm/mach-ixp4xx/Kconfig

index 71ef55f..25f8994 100644

--- a/arch/arm/mach-ixp4xx/Kconfig

+++ b/arch/arm/mach-ixp4xx/Kconfig

@@ -182,6 +182,14 @@ config IXP4XX_QMGR

 	  This driver supports IXP4xx built-in hardware queue manager

 	  and is automatically selected by Ethernet and HSS drivers.
+config IXP4XX_NPE

+	tristate "IXP4xx Network Processor Engine support"

+	select HOTPLUG

+	select FW_LOADER

+	help

+	  This driver supports IXP4xx built-in network coprocessors

+	  and is automatically selected by Ethernet and HSS drivers.

+

 endmenu
 endif

diff --git a/arch/arm/mach-ixp4xx/Makefile b/arch/arm/mach-ixp4xx/Makefile

index f8e1afc..33d4b88 100644

--- a/arch/arm/mach-ixp4xx/Makefile

+++ b/arch/arm/mach-ixp4xx/Makefile

@@ -27,3 +27,4 @@ obj-$(CONFIG_MACH_DSMG600)      += dsmg600-setup.o dsmg600-power.o
 obj-$(CONFIG_PCI)		+= $(obj-pci-$(CONFIG_PCI)) common-pci.o

 obj-$(CONFIG_IXP4XX_QMGR)	+= ixp4xx_qmgr.o

+obj-$(CONFIG_IXP4XX_NPE)	+= ixp4xx_npe.o

diff --git a/arch/arm/mach-ixp4xx/ixp4xx_npe.c b/arch/arm/mach-ixp4xx/ixp4xx_npe.c

new file mode 100644

index 0000000..4c77d8a

--- /dev/null

+++ b/arch/arm/mach-ixp4xx/ixp4xx_npe.c

@@ -0,0 +1,737 @@

+/*

+ * Intel IXP4xx Network Processor Engine driver for Linux

+ *

+ * Copyright (C) 2007 Krzysztof Halasa <khc@pm.waw.pl>

+ *

+ * This program is free software; you can redistribute it and/or modify it

+ * under the terms of version 2 of the GNU General Public License

+ * as published by the Free Software Foundation.

+ *

+ * The code is based on publicly available information:

+ * - Intel IXP4xx Developer's Manual and other e-papers

+ * - Intel IXP400 Access Library Software (BSD license)

+ * - previous works by Christian Hohnstaedt <chohnstaedt@innominate.com>

+ *   Thanks, C...
[ message continues ]

It may be a matter of taste, but could some of the many definitions

at the top of ixp4xx_npe.c go in the header file here?
Michael-Luke
-

I want ixp4xx_ prefix in module name, otherwise I'd call it npe.c,
Why? Especially with code having to work with third party binary-only

firmware? Suicide. They are eliminated at build time = performance
It's actually not only a matter of taste, they are private

to the .c file and I don't want to make them available to the

public (but sure, I don't like them in .c either, I think nobody

likes such definitions anywhere but they have to exist somewhere).

--

Krzysztof Halasa

-

Adds a driver for IXP4xx built-in hardware queue manager.
Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
diff --git a/arch/arm/mach-ixp4xx/Kconfig b/arch/arm/mach-ixp4xx/Kconfig

index 9715ef5..71ef55f 100644

--- a/arch/arm/mach-ixp4xx/Kconfig

+++ b/arch/arm/mach-ixp4xx/Kconfig

@@ -176,6 +176,12 @@ config IXP4XX_INDIRECT_PCI

 	  need to use the indirect method instead. If you don't know

 	  what you need, leave this option unselected.
+config IXP4XX_QMGR

+	tristate "IXP4xx Queue Manager support"

+	help

+	  This driver supports IXP4xx built-in hardware queue manager

+	  and is automatically selected by Ethernet and HSS drivers.

+

 endmenu
 endif

diff --git a/arch/arm/mach-ixp4xx/Makefile b/arch/arm/mach-ixp4xx/Makefile

index 3b87c47..f8e1afc 100644

--- a/arch/arm/mach-ixp4xx/Makefile

+++ b/arch/arm/mach-ixp4xx/Makefile

@@ -26,3 +26,4 @@ obj-$(CONFIG_MACH_NAS100D)	+= nas100d-setup.o nas100d-power.o

 obj-$(CONFIG_MACH_DSMG600)      += dsmg600-setup.o dsmg600-power.o
 obj-$(CONFIG_PCI)		+= $(obj-pci-$(CONFIG_PCI)) common-pci.o

+obj-$(CONFIG_IXP4XX_QMGR)	+= ixp4xx_qmgr.o

diff --git a/arch/arm/mach-ixp4xx/ixp4xx_qmgr.c b/arch/arm/mach-ixp4xx/ixp4xx_qmgr.c

new file mode 100644

index 0000000..b9e9bd6

--- /dev/null

+++ b/arch/arm/mach-ixp4xx/ixp4xx_qmgr.c

@@ -0,0 +1,273 @@

+/*

+ * Intel IXP4xx Queue Manager driver for Linux

+ *

+ * Copyright (C) 2007 Krzysztof Halasa <khc@pm.waw.pl>

+ *

+ * This program is free software; you can redistribute it and/or modify it

+ * under the terms of version 2 of the GNU General Public License

+ * as published by the Free Software Foundation.

+ */

+

+#include <linux/interrupt.h>

+#include <linux/kernel.h>

+#include <asm/io.h>

+#include <asm/arch/qmgr.h>

+

+#define DEBUG		0

+

+struct qmgr_regs __iomem *qmgr_regs;

+static struct resource *mem_res;

+static spinlock_t qmgr_lock;

+static u32 used_sram_bitmap[4]; /* 128 16-dword pages */

+static void (*irq_handlers[HALF_QUEUES])(void *pdev);

+s...
[ message continues ]

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

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

-

Hmm... perhaps someone have a spare device with such IXP23xx

and wants to make it a donation for science? :-)

--

Krzysztof Halasa

-

I have a couple of ixp23xx boards at home, but I'm not sure whether I

can give them away.  I can give you remote access to them, though.

-

Hmm, may be interesting some day.

--

Krzysztof Halasa

-

Adds "fuse" functions to help determine installed IXP4xx CPU

components and to reset/disable/enable them (only NPE - network

coprocessors - can be reset and reenabled).
Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
diff --git a/include/asm-arm/arch-ixp4xx/ixp4xx-regs.h b/include/asm-arm/arch-ixp4xx/ixp4xx-regs.h

index 5d949d7..780c851 100644

--- a/include/asm-arm/arch-ixp4xx/ixp4xx-regs.h

+++ b/include/asm-arm/arch-ixp4xx/ixp4xx-regs.h

@@ -607,4 +607,51 @@
 #define DCMD_LENGTH	0x01fff		/* length mask (max = 8K - 1) */
+/* Fuse Bits of IXP_EXP_CFG2 */

+#define IXP4XX_FUSE_RCOMP		(1 << 0)

+#define IXP4XX_FUSE_USB_DEVICE		(1 << 1)

+#define IXP4XX_FUSE_HASH		(1 << 2)

+#define IXP4XX_FUSE_AES			(1 << 3)

+#define IXP4XX_FUSE_DES			(1 << 4)

+#define IXP4XX_FUSE_HDLC		(1 << 5)

+#define IXP4XX_FUSE_AAL			(1 << 6)

+#define IXP4XX_FUSE_HSS			(1 << 7)

+#define IXP4XX_FUSE_UTOPIA		(1 << 8)

+#define IXP4XX_FUSE_NPEB_ETH0		(1 << 9)

+#define IXP4XX_FUSE_NPEC_ETH		(1 << 10)

+#define IXP4XX_FUSE_RESET_NPEA		(1 << 11)

+#define IXP4XX_FUSE_RESET_NPEB		(1 << 12)

+#define IXP4XX_FUSE_RESET_NPEC		(1 << 13)

+#define IXP4XX_FUSE_PCI			(1 << 14)

+#define IXP4XX_FUSE_ECC_TIMESYNC	(1 << 15)

+#define IXP4XX_FUSE_UTOPIA_PHY_LIMIT	(3 << 16)

+#define IXP4XX_FUSE_USB_HOST		(1 << 18)

+#define IXP4XX_FUSE_NPEA_ETH		(1 << 19)

+#define IXP4XX_FUSE_NPEB_ETH_1_TO_3	(1 << 20)

+#define IXP4XX_FUSE_RSA			(1 << 21)

+#define IXP4XX_FUSE_XSCALE_MAX_FREQ	(3 << 22)

+#define IXP4XX_FUSE_RESERVED		(0xFF << 24)

+

+#define IXP4XX_FUSE_IXP46X_ONLY (IXP4XX_FUSE_ECC_TIMESYNC |		\

+				 IXP4XX_FUSE_USB_HOST |			\

+				 IXP4XX_FUSE_NPEA_ETH |			\

+				 IXP4XX_FUSE_NPEB_ETH_1_TO_3 |		\

+				 IXP4XX_FUSE_RSA |			\

+				 IXP4XX_FUSE_XSCALE_MAX_FREQ)

+

+static inline u32 ixp4xx_read_fuses(void)

+{

+	unsigned int fuses = ~*IXP4XX_EXP_CFG2;

+	fuses &= ~IXP4XX_FUSE_RESERVED...
[ message continues ]

Hello, Krzysztof.
On 5/7/07, Krzysztof Halasa <khc@pm.waw.pl> wrote:
Are you sure this is the version you wanted to send? I don't see how this could

compile without this #ifndef. And also there is some problem with undefined

processor_id, but this is not your fault, but a flaw in the pre-rc1

kernel, which

I hope is now noticed. So maybe you should just send the patches made for

the 2.6.21 kernel?
Otherwise, when applied on top of my 2.6.20 kernel, the driver just

works (at least

I see the pings, haven't considered any other tests yet), thank you a lot!

-

Hello,
Oops. You're right, of course.

--

Krzysztof Halasa

-

Allow enabling WAN drivers without selecting generic HDLC first,

HDLC will be selected automatically.
Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig

index 8897f53..3a2fe82 100644

--- a/drivers/net/wan/Kconfig

+++ b/drivers/net/wan/Kconfig

@@ -171,7 +171,8 @@ comment "X.25/LAPB support is disabled"
 config PCI200SYN

 	tristate "Goramo PCI200SYN support"

-	depends on HDLC && PCI

+	depends on PCI

+	select HDLC

 	help

 	  Driver for PCI200SYN cards by Goramo sp. j.
@@ -185,7 +186,8 @@ config PCI200SYN
 config WANXL

 	tristate "SBE Inc. wanXL support"

-	depends on HDLC && PCI

+	depends on PCI

+	select HDLC

 	help

 	  Driver for wanXL PCI cards by SBE Inc.
@@ -208,7 +210,8 @@ config WANXL_BUILD_FIRMWARE
 config PC300

 	tristate "Cyclades-PC300 support (RS-232/V.35, X.21, T1/E1 boards)"

-	depends on HDLC && PCI

+	depends on PCI

+	select HDLC

 	---help---

 	  Driver for the Cyclades-PC300 synchronous communication boards.
@@ -225,19 +228,21 @@ config PC300
 config PC300_MLPPP

 	bool "Cyclades-PC300 MLPPP support"

-	depends on PC300 && PPP_MULTILINK && PPP_SYNC_TTY && HDLC_PPP

+	depends on PC300 && PPP_MULTILINK && PPP_SYNC_TTY

+	select HDLC_PPP

 	help

 	  Multilink PPP over the PC300 synchronous communication boards.
 comment "Cyclades-PC300 MLPPP support is disabled."

-	depends on WAN && HDLC && PC300 && (PPP=n || !PPP_MULTILINK || PPP_SYNC_TTY=n || !HDLC_PPP)

+	depends on WAN && PC300 && (PPP=n || !PPP_MULTILINK || PPP_SYNC_TTY=n)
 comment "Refer to the file README.mlppp, provided by PC300 package."

-	depends on WAN && HDLC && PC300 && (PPP=n || !PPP_MULTILINK || PPP_SYNC_TTY=n || !HDLC_PPP)

+	depends on WAN && PC300 && (PPP=n || !PPP_MULTILINK || PPP_SYNC_TTY=n)
 config PC300TOO

 	tristate "Cyclades PC300 RSV/X21 alternative supp...
[ message continues ]

Hi,
What's the advantage? The HDLC option is directly before this?
bye, Roman

-

You don't have to know it's required, you can just select a driver

for your hardware, without enabling HDLC first.

--

Krzysztof Halasa

-

Hi,
Is this a real problem?

Using select you should also consider removing HDLC as visible option and

use only select. Mixing depends and selects is generally a bad idea.
bye, Roman

-

It has to stay there for external modules.
It's similar to MII - drivers select MII automatically but you can

turn it on (Y or M) by hand as well.
And you can have HDLC=y and driver=m (and it makes perfect sense).
Actually I can't see any bad idea here.

The original dependency was certainly, uhm, not the best one.

--

Krzysztof Halasa

-

Hi,
select seriously screws with the dependencies, it's especially problematic

if the selected symbol has other dependencies as HDLC in this case, it

makes it only more complicated to get the dependencies correct again.

Please use it only if it solves a real problem.
bye, Roman

-

What he's doing is the standard way to deal with library-style code.

Nothing wrong with the patch, it's continuing established methods.
	Jeff
-

Hi,
select was never meant as autoconfiguration tool. It can't be said often

enough: select seriously screws with the dependencies, _please_ don't use

it as a simple depends replacement.

HDLC doesn't really look like simple library code, what's up with all the

HDLC_* options?
bye, Roman

-

Sub-modules. Anyway, what does the patch "screw" exactly?

--

Krzysztof Halasa

-

Hi,
Normal dependencies, you basically have to manually make sure they are

correct (and it seems with your patch they aren't). Again, _please_ (with

sugar on top) don't use select unless you have a good reason for it.
bye, Roman

-

Allow enabling WAN drivers without selecting generic HDLC first,

HDLC will be selected automatically.
Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
--- linux/drivers/net/wan/Kconfig	2007-05-07 22:46:06.000000000 +0200

+++ linux/drivers/net/wan/Kconfig	2007-05-07 22:45:13.000000000 +0200

@@ -169,7 +169,8 @@
 config PCI200SYN

 	tristate "Goramo PCI200SYN support"

-	depends on HDLC && PCI

+	depends on PCI

+	select HDLC

 	help

 	  Driver for PCI200SYN cards by Goramo sp. j.
@@ -183,7 +184,8 @@
 config WANXL

 	tristate "SBE Inc. wanXL support"

-	depends on HDLC && PCI

+	depends on PCI

+	select HDLC

 	help

 	  Driver for wanXL PCI cards by SBE Inc.
@@ -206,7 +208,8 @@
 config PC300

 	tristate "Cyclades-PC300 support (RS-232/V.35, X.21, T1/E1 boards)"

-	depends on HDLC && PCI

+	depends on PCI

+	select HDLC

 	---help---

 	  Driver for the Cyclades-PC300 synchronous communication boards.
@@ -228,14 +231,15 @@

 	  Multilink PPP over the PC300 synchronous communication boards.
 comment "Cyclades-PC300 MLPPP support is disabled."

-	depends on HDLC && PC300 && (PPP=n || !PPP_MULTILINK || PPP_SYNC_TTY=n || !HDLC_PPP)

+	depends on PC300 && (PPP=n || !PPP_MULTILINK || PPP_SYNC_TTY=n || !HDLC_PPP)
 comment "Refer to the file README.mlppp, provided by PC300 package."

-	depends on HDLC && PC300 && (PPP=n || !PPP_MULTILINK || PPP_SYNC_TTY=n || !HDLC_PPP)

+	depends on PC300 && (PPP=n || !PPP_MULTILINK || PPP_SYNC_TTY=n || !HDLC_PPP)
 config PC300TOO

 	tristate "Cyclades PC300 RSV/X21 alternative support"

-	depends on HDLC && PCI

+	depends on PCI

+	select HDLC

 	help

 	  Alternative driver for PC300 RSV/X21 PCI cards made by

 	  Cyclades, Inc. If you have such a card, say Y here and see

@@ -248,7 +252,8 @@
 config N2

 	tristate "SDL RISCom/N2 support"

-	depends on HDLC && ISA

+	depends on ISA

+	select HDLC

 	help

 	  Driver for RISCom/N2 single or dual cha...
[ message continues ]

From: Jan Engelhardt <jengelh@linux01.gwdg.de>
Change Kconfig objects from "menu, config" into "menuconfig" so

that the user can disable the whole feature without having to

enter the menu first.
Signed-off-by: Jan Engelhardt <jengelh@gmx.de>

Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>
---

 drivers/net/wan/Kconfig |   34 +++++++++++++++-------------------

 1 file changed, 15 insertions(+), 19 deletions(-)
--- linux-2.6.21-mm_20070428.orig/drivers/net/wan/Kconfig

+++ linux-2.6.21-mm_20070428/drivers/net/wan/Kconfig

@@ -2,10 +2,7 @@

 # wan devices configuration

 #
-menu "Wan interfaces"

-	depends on NETDEVICES

-

-config WAN

+menuconfig WAN

 	bool "Wan interfaces support"

 	---help---

 	  Wide Area Networks (WANs), such as X.25, Frame Relay and leased

@@ -23,10 +20,12 @@ config WAN
 	  If unsure, say N.
+if WAN

+

 # There is no way to detect a comtrol sv11 - force it modular for now.

 config HOSTESS_SV11

 	tristate "Comtrol Hostess SV-11 support"

-	depends on WAN && ISA && m && ISA_DMA_API && INET

+	depends on ISA && m && ISA_DMA_API && INET

 	help

 	  Driver for Comtrol Hostess SV-11 network card which

 	  operates on low speed synchronous serial links at up to

@@ -38,7 +37,7 @@ config HOSTESS_SV11

 # The COSA/SRP driver has not been tested as non-modular yet.

 config COSA

 	tristate "COSA/SRP sync serial boards support"

-	depends on WAN && ISA && m && ISA_DMA_API

+	depends on ISA && m && ISA_DMA_API

 	---help---

 	  Driver for COSA and SRP synchronous serial boards.
@@ -62,7 +61,7 @@ config COSA

 #

 config LANMEDIA

 	tristate "LanMedia Corp. SSI/V.35, T1/E1, HSSI, T3 boards"

-	depends on WAN && PCI

+	depends on PCI

 	---help---

 	  Driver for the following Lan Media family of serial boards:
@@ -89,7 +88,7 @@ config LANMEDIA

 # There is no way to detect a Sealevel board. Force it modular

 config SEALEVEL_4021

 	trista...
[ message continues ]

You perhaps mean WAN dependency, don't you? I was under impression

that the "menu" patches have already been merged so the WAN

dependency would be automatic.
Anything other than that? Sure, I can see now. I can only say,

in my defense, that I was (and still am) very tired. I'm going to

get some coffee.
There is still a very good reason for the select.

CONFIG_HDLC _is_ a simple library, though probably not the most

simple one. I really feel it's an improvement.
Attaching two patches, hopefully the double check is enough.

--

Krzysztof Halasa

-

Yes, mixing select and depends is a recipe for build disasters. Call

me a rabid fanatic, but I would in fact go as far as to say that this

whole "select" thing in the Kconfig process is one big BUG, and not a

feature. People are lazy by nature and would rather just "select" a

dependency for their config option than burden users with several

"depends".
The following rant doesn't apply only to the select above, but

unfortunately, that's precisely what happens when such stuff is

introduced ... they seem like a good idea to the introducer for his

special / rarest-of-rare case, but then others tend to {ab-,mis-}use

it and the use of such primitives soon proliferates even to cases

where they are clearly inapplicable / avoidable.

-

Tough, the kernel community has voted against you.
It makes far more sense to include a driver during kernel configuration,

and have that driver pull in its libraries via 'select'.  The lame

alternative requires developers to know which libraries they need BEFORE

picking their drivers, which is backwards and requires legwork on the

part of the kernel developer.
	Jeff
-

Hi,
Jeff, there was never anything to "vote" about! There is no

autoconfiguration, kernel configuration isn't ready for Aunt Tilly and

select is no substitute for it...
bye, Roman

-

Heh ... I guess I appeared a trifle _too_ fanatic there, but I'm not.

That rant against it was actually more about controlling its

{ab-,mis-}use and rampant proliferation (even to cases where it is

inapplicable / avoidable) than removing it altogether. Especially,

when a developer is lazy / wrong and mixes select and depends in the
(That bit about a developer _knowing_ which libraries to select

manually before selecting a driver is not really true, you can "?" on

a config item and get all its "depends" and "select" dependencies

anyway -- and it requires more leg work, yes, but then you'll never

have build failures either)
Still, I don't really disagree w.r.t. "select"ing libraries, as long

as we're careful to limit them to _only_ libraries.

-

Andrew (usually) implores people not to use "select" and I agree
Developers?  If you had said "users," I might agree, but IMO it's

OK (or even Good) for developers to know what libraries their code

uses/requires.  Yes, that's a good thing.
---

~Randy

*** Remember to use Documentation/SubmitChecklist when testing your code ***

-

You're absolutely right, but to give Jeff the benefit of the doubt I'm

sure he _meant_ "users" there although he said "developers". Stating

the obvious, the developer _has_ to know what stuff his code uses

anyway, otherwise what does he "select"s or "depends" his config

option on.
As for users, we _can_ avoid pitfalls by building a complete

dependency tree and just selecting _everything_ that we require for a

particular config option to be selected, but some users could

conceivably prefer only being _told_ about what else they need to

successfully pick a config option (than everything just getting in

behind their backs). Actually (correct me if I'm wrong), this is not

presently possible: an option is not visible unless dependencies are

already picked. Just a suggestion, though.

-

That's correct for menuconfig.  For xconfig, there are GUI options to

	Show Name

	Show Range

	Show Data

	Show All Options

	Show Debug Info
I often have all of them enabled.
--

~Randy

*** Remember to use Documentation/SubmitChecklist when testing your code ***

-