non-PC parport config change

PATCH In review Adrian Bunk 2 participants 3 messages Jan 17, 2005 – Jan 20, 2005

Subsystems (2)

USB Parallel port

lore

Share & tools

lore

Adrian Bunk

author

2005-01-17 10:33 UTC #0 lore raw

PATCH 1/1 non-PC parport config change

This patch adds a config option PARPORT_NOT_PC (and removes the PARPORT_OTHER option) and lets all non-PC parallel ports options depend on it.

Advantages:
- the config structure is IMHO a bit more logical
- the mega #if in parport.h is gone now

Additionally, it removes the unneeded PARPORT_NEED_GENERIC_OPS #define.

Signed-off-by Adrian Bunk <[email protected]>

---

diffstat output:

 drivers/parport/Kconfig |   28 +++++++++++++---------------
 drivers/usb/Kconfig     |    1 +
 include/linux/parport.h |   12 +++++-------
 3 files changed, 19 insertions(+), 22 deletions(-)

This patch was already sent on:
- 9 Dec 2004

include/linux/parport.h renamed from include/linux/parport.h.old +5 -7

@@ -105,8 +105,6 @@
 #include <asm/ptrace.h>
 #include <asm/semaphore.h>
 
-#define PARPORT_NEED_GENERIC_OPS
-
 /* Define this later. */
 struct parport;
 struct pardevice;

@@ -520,9 +518,8 @@
 extern int parport_device_proc_unregister(struct pardevice *device);
 
 /* If PC hardware is the only type supported, we can optimise a bit.  */
-#if (defined(CONFIG_PARPORT_PC) || defined(CONFIG_PARPORT_PC_MODULE)) && !(defined(CONFIG_PARPORT_ARC) || defined(CONFIG_PARPORT_ARC_MODULE)) && !(defined(CONFIG_PARPORT_AMIGA) || defined(CONFIG_PARPORT_AMIGA_MODULE)) && !(defined(CONFIG_PARPORT_MFC3) || defined(CONFIG_PARPORT_MFC3_MODULE)) && !(defined(CONFIG_PARPORT_ATARI) || defined(CONFIG_PARPORT_ATARI_MODULE)) && !(defined(CONFIG_USB_USS720) || defined(CONFIG_USB_USS720_MODULE)) && !(defined(CONFIG_PARPORT_SUNBPP) || defined(CONFIG_PARPORT_SUNBPP_MODULE)) && !defined(CONFIG_PARPORT_OTHER)
+#if !defined(CONFIG_PARPORT_NOT_PC)
 
-#undef PARPORT_NEED_GENERIC_OPS
 #include <linux/parport_pc.h>
 #define parport_write_data(p,x)            parport_pc_write_data(p,x)
 #define parport_read_data(p)               parport_pc_read_data(p)

@@ -534,9 +531,9 @@
 #define parport_disable_irq(p)             parport_pc_disable_irq(p)
 #define parport_data_forward(p)            parport_pc_data_forward(p)
 #define parport_data_reverse(p)            parport_pc_data_reverse(p)
-#endif
 
-#ifdef PARPORT_NEED_GENERIC_OPS
+#else  /*  !CONFIG_PARPORT_NOT_PC  */
+
 /* Generic operations vector through the dispatch table. */
 #define parport_write_data(p,x)            (p)->ops->write_data(p,x)
 #define parport_read_data(p)               (p)->ops->read_data(p)

@@ -548,7 +545,8 @@
 #define parport_disable_irq(p)             (p)->ops->disable_irq(p)
 #define parport_data_forward(p)            (p)->ops->data_forward(p)
 #define parport_data_reverse(p)            (p)->ops->data_reverse(p)
-#endif
+
+#endif /*  !CONFIG_PARPORT_NOT_PC  */
 
 #endif /* __KERNEL__ */
 #endif /* _PARPORT_H_ */

drivers/parport/Kconfig renamed from drivers/parport/Kconfig.old +13 ~~-15~~

@@ -88,13 +88,21 @@
 	  Say Y here if you need PCMCIA support for your PC-style parallel
 	  ports. If unsure, say N.
 
+config PARPORT_NOT_PC
+	bool "Support non-PC-style parallel ports"
+	depends on PARPORT
+	help
+	  Say Y here if you want to use non-PC-style parallel ports.
+
+	  This causes a performance loss, so most people say N.
+
 config PARPORT_ARC
 	tristate "Archimedes hardware"
-	depends on ARM && PARPORT
+	depends on ARM && PARPORT_NOT_PC
 
 config PARPORT_AMIGA
 	tristate "Amiga builtin port"
-	depends on AMIGA && PARPORT
+	depends on AMIGA && PARPORT_NOT_PC
 	help
 	  Say Y here if you need support for the parallel port hardware on
 	  Amiga machines. This code is also available as a module (say M),

@@ -102,7 +110,7 @@
 
 config PARPORT_MFC3
 	tristate "Multiface III parallel port"
-	depends on ZORRO && PARPORT
+	depends on ZORRO && PARPORT_NOT_PC
 	help
 	  Say Y here if you need parallel port support for the MFC3 card.
 	  This code is also available as a module (say M), called

@@ -110,7 +118,7 @@
 
 config PARPORT_ATARI
 	tristate "Atari hardware"
-	depends on ATARI && PARPORT
+	depends on ATARI && PARPORT_NOT_PC
 	help
 	  Say Y here if you need support for the parallel port hardware on
 	  Atari machines. This code is also available as a module (say M),

@@ -123,22 +131,12 @@
 
 config PARPORT_SUNBPP
 	tristate "Sparc hardware (EXPERIMENTAL)"
-	depends on SBUS && EXPERIMENTAL && PARPORT
+	depends on SBUS && EXPERIMENTAL && PARPORT_NOT_PC
 	help
 	  This driver provides support for the bidirectional parallel port
 	  found on many Sun machines. Note that many of the newer Ultras
 	  actually have pc style hardware instead.
 
-# If exactly one hardware type is selected then parport will optimise away
-# support for loading any others.  Defeat this if the user is keen.
-config PARPORT_OTHER
-	bool "Support foreign hardware"
-	depends on PARPORT
-	help
-	  Say Y here if you want to be able to load driver modules to support
-	  other non-standard types of parallel ports. This causes a
-	  performance loss, so most people say N.
-
 config PARPORT_1284
 	bool "IEEE 1284 transfer modes"
 	depends on PARPORT

drivers/usb/Kconfig renamed from drivers/usb/Kconfig.old +1 -0

@@ -62,6 +62,7 @@
 config USB_USS720
 	tristate "USS720 parport driver"
 	depends on USB && PARPORT
+	select PARPORT_NOT_PC
 	---help---
 	  This driver is for USB parallel port adapters that use the Lucent
 	  Technologies USS-720 chip. These cables are plugged into your USB

Russell King

2005-01-17 10:44 UTC #1 lore raw

Quoted 11 lines · Adrian Bunk

This patch adds a config option PARPORT_NOT_PC (and removes the PARPORT_OTHER option) and lets all non-PC parallel ports options depend on it.
Advantages:
- the config structure is IMHO a bit more logical
- the mega #if in parport.h is gone now
Additionally, it removes the unneeded PARPORT_NEED_GENERIC_OPS #define.
Signed-off-by Adrian Bunk <[email protected]>

This actually looks like a nice example where the use of select would work nicely. IOW:

config PARPORT_ARC

	tristate "Archimedes hardware"
	depends on ARM && PARPORT
	select PARPORT_NOT_PC

config PARPORT_AMIGA

	tristate "Amiga builtin port"
	depends on AMIGA && PARPORT
	select PARPORT_NOT_PC

config PARPORT_NOT_PC
bool

And I notice in passing that you've already used select on this new configuration option in USB, and this new configuration option is user visible. Bad.

The golden rule (if you don't wish _me_ to complain) is:

Don't use select on user-visible options. Either make them non-user visibile or don't use select at all.

In this case, the former is far more preferable because it means that the existing defconfig files don't need to be fixed for their parports work again.

Signature · 4 lines

Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Adrian Bunk

author

2005-01-20 17:39 UTC #2 lore raw

PATCH 1/1 non-PC parport config change

Quoted 21 lines · Russell King

This actually looks like a nice example where the use of select would work nicely. IOW:
config PARPORT_ARC
	tristate "Archimedes hardware"
	depends on ARM && PARPORT
	select PARPORT_NOT_PC
config PARPORT_AMIGA
	tristate "Amiga builtin port"
	depends on AMIGA && PARPORT
	select PARPORT_NOT_PC
config PARPORT_NOT_PC
bool
And I notice in passing that you've already used select on this new configuration option in USB, and this new configuration option is user visible. Bad.

Quoted 8 lines

The golden rule (if you don't wish _me_ to complain) is:
Don't use select on user-visible options. Either make them non-user visibile or don't use select at all.
In this case, the former is far more preferable because it means that the existing defconfig files don't need to be fixed for their parports work again.

Is the patch below OK?

cu
Adrian

<-- snip -->

This patch adds a config option PARPORT_NOT_PC (and removes the PARPORT_OTHER option) that get's selected if any non-PC hardware was chosen.

This way, the mega #if in parport.h is gone now.

Additionally, it removes the unneeded PARPORT_NEED_GENERIC_OPS #define.

Signed-off-by Adrian Bunk <[email protected]>

---

diffstat output:

 include/linux/parport.h |   12 +++++-------
 drivers/parport/Kconfig |   20 +++++++++-----------
 drivers/usb/Kconfig     |    1 +
 3 files changed, 15 insertions(+), 18 deletions(-)

include/linux/parport.h renamed from include/linux/parport.h.old +5 -7

@@ -105,8 +105,6 @@
 #include <asm/ptrace.h>
 #include <asm/semaphore.h>
 
-#define PARPORT_NEED_GENERIC_OPS
-
 /* Define this later. */
 struct parport;
 struct pardevice;

@@ -520,9 +518,8 @@
 extern int parport_device_proc_unregister(struct pardevice *device);
 
 /* If PC hardware is the only type supported, we can optimise a bit.  */
-#if (defined(CONFIG_PARPORT_PC) || defined(CONFIG_PARPORT_PC_MODULE)) && !(defined(CONFIG_PARPORT_ARC) || defined(CONFIG_PARPORT_ARC_MODULE)) && !(defined(CONFIG_PARPORT_AMIGA) || defined(CONFIG_PARPORT_AMIGA_MODULE)) && !(defined(CONFIG_PARPORT_MFC3) || defined(CONFIG_PARPORT_MFC3_MODULE)) && !(defined(CONFIG_PARPORT_ATARI) || defined(CONFIG_PARPORT_ATARI_MODULE)) && !(defined(CONFIG_USB_USS720) || defined(CONFIG_USB_USS720_MODULE)) && !(defined(CONFIG_PARPORT_SUNBPP) || defined(CONFIG_PARPORT_SUNBPP_MODULE)) && !defined(CONFIG_PARPORT_OTHER)
+#if !defined(CONFIG_PARPORT_NOT_PC)
 
-#undef PARPORT_NEED_GENERIC_OPS
 #include <linux/parport_pc.h>
 #define parport_write_data(p,x)            parport_pc_write_data(p,x)
 #define parport_read_data(p)               parport_pc_read_data(p)

@@ -534,9 +531,9 @@
 #define parport_disable_irq(p)             parport_pc_disable_irq(p)
 #define parport_data_forward(p)            parport_pc_data_forward(p)
 #define parport_data_reverse(p)            parport_pc_data_reverse(p)
-#endif
 
-#ifdef PARPORT_NEED_GENERIC_OPS
+#else  /*  !CONFIG_PARPORT_NOT_PC  */
+
 /* Generic operations vector through the dispatch table. */
 #define parport_write_data(p,x)            (p)->ops->write_data(p,x)
 #define parport_read_data(p)               (p)->ops->read_data(p)

@@ -548,7 +545,8 @@
 #define parport_disable_irq(p)             (p)->ops->disable_irq(p)
 #define parport_data_forward(p)            (p)->ops->data_forward(p)
 #define parport_data_reverse(p)            (p)->ops->data_reverse(p)
-#endif
+
+#endif /*  !CONFIG_PARPORT_NOT_PC  */
 
 #endif /* __KERNEL__ */
 #endif /* _PARPORT_H_ */

drivers/parport/Kconfig renamed from drivers/parport/Kconfig.old +9 ~~-11~~

@@ -88,13 +88,18 @@
 	  Say Y here if you need PCMCIA support for your PC-style parallel
 	  ports. If unsure, say N.
 
+config PARPORT_NOT_PC
+	bool
+
 config PARPORT_ARC
 	tristate "Archimedes hardware"
 	depends on ARM && PARPORT
+	select PARPORT_NOT_PC
 
 config PARPORT_AMIGA
 	tristate "Amiga builtin port"
 	depends on AMIGA && PARPORT
+	select PARPORT_NOT_PC
 	help
 	  Say Y here if you need support for the parallel port hardware on
 	  Amiga machines. This code is also available as a module (say M),

@@ -103,6 +108,7 @@
 config PARPORT_MFC3
 	tristate "Multiface III parallel port"
 	depends on ZORRO && PARPORT
+	select PARPORT_NOT_PC
 	help
 	  Say Y here if you need parallel port support for the MFC3 card.
 	  This code is also available as a module (say M), called

@@ -111,6 +117,7 @@
 config PARPORT_ATARI
 	tristate "Atari hardware"
 	depends on ATARI && PARPORT
+	select PARPORT_NOT_PC
 	help
 	  Say Y here if you need support for the parallel port hardware on
 	  Atari machines. This code is also available as a module (say M),

@@ -123,22 +130,13 @@
 
 config PARPORT_SUNBPP
 	tristate "Sparc hardware (EXPERIMENTAL)"
-	depends on SBUS && EXPERIMENTAL && PARPORT
+	depends on SBUS && PARPORT && EXPERIMENTAL
+	select PARPORT_NOT_PC
 	help
 	  This driver provides support for the bidirectional parallel port
 	  found on many Sun machines. Note that many of the newer Ultras
 	  actually have pc style hardware instead.
 
-# If exactly one hardware type is selected then parport will optimise away
-# support for loading any others.  Defeat this if the user is keen.
-config PARPORT_OTHER
-	bool "Support foreign hardware"
-	depends on PARPORT
-	help
-	  Say Y here if you want to be able to load driver modules to support
-	  other non-standard types of parallel ports. This causes a
-	  performance loss, so most people say N.
-
 config PARPORT_1284
 	bool "IEEE 1284 transfer modes"
 	depends on PARPORT

drivers/usb/Kconfig renamed from drivers/usb/Kconfig.old +1 -0

@@ -62,6 +62,7 @@
 config USB_USS720
 	tristate "USS720 parport driver"
 	depends on USB && PARPORT
+	select PARPORT_NOT_PC
 	---help---
 	  This driver is for USB parallel port adapters that use the Lucent
 	  Technologies USS-720 chip. These cables are plugged into your USB