Kconfig dependancy update for drivers/misc/ibmasm

PATCH In review Tony Breeds 3 participants 6 messages Apr 20, 2004 – Apr 21, 2004 Char and misc drivers

lore

Share & tools

lore

Tony Breeds

author

2004-04-20 22:20 UTC #0 lore raw

PATCH 1/1 Kconfig dependancy update for drivers/misc/ibmasm

Hello,
Some weeks ago I saw this compile error posted to lkml:

---

Quoted 7 lines

LD .tmp_vmlinux1
drivers/built-in.o(.text+0x435e1): In function `ibmasm_register_uart':
: undefined reference to register_serial' drivers/built-in.o(.text+0x43649): In function ibmasm_unregister_uart':
: undefined reference to `unregister_serial'
make: *** [.tmp_vmlinux1] Error 1 summer@Dolphin:~/pebble/kernel/linux-2.6.4$

---

This was created because ibmasm was set to yes BUT the 8250 was a module. I believe the correct (tested) fix is below.

################################################################################

drivers/misc/Kconfig +1 -1

@@ -6,7 +6,7 @@
 
 config IBM_ASM
 	tristate "Device driver for IBM RSA service processor"
-	depends on X86
+	depends on X86 && SERIAL_8250
 	default n
 	---help---
 	  This option enables device driver support for in-band access to the

################################################################################

Yours Tony

linux.conf.au http://lca2005.linux.org.au/
Apr 18-23 2005 The Australian Linux Technical Conference!

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

----- End forwarded message -----

Yours Tony

linux.conf.au http://lca2005.linux.org.au/
Apr 18-23 2005 The Australian Linux Technical Conference!

Andrew Morton

2004-04-20 22:27 UTC #1 lore raw

Quoted 13 lines · Tony Breeds

Hello,
Some weeks ago I saw this compile error posted to lkml:
---
Quoted 7 lines
LD .tmp_vmlinux1
drivers/built-in.o(.text+0x435e1): In function `ibmasm_register_uart':
: undefined reference to register_serial' drivers/built-in.o(.text+0x43649): In function ibmasm_unregister_uart':
: undefined reference to `unregister_serial'
make: *** [.tmp_vmlinux1] Error 1 summer@Dolphin:~/pebble/kernel/linux-2.6.4$

Quoted 4 lines

---
This was created because ibmasm was set to yes BUT the 8250 was a module. I believe the correct (tested) fix is below.

Seems sane to me, but I'm not sure why this wasn't done originally. ie, this:

+#ifdef CONFIG_SERIAL_8250
extern void ibmasm_register_uart(struct service_processor *sp); extern void ibmasm_unregister_uart(struct service_processor *sp);
+#else
+#define ibmasm_register_uart(sp) do { } while(0)
+#define ibmasm_unregister_uart(sp) do { } while(0)
+#endif

becomes unnecessary with your patch.

Max, any preferences?

Quoted 31 lines

################################################################################
drivers/misc/Kconfig +1 -1
@@ -6,7 +6,7 @@
 
 config IBM_ASM
 	tristate "Device driver for IBM RSA service processor"
-	depends on X86
+	depends on X86 && SERIAL_8250
 	default n
 	---help---
 	  This option enables device driver support for in-band access to the
################################################################################
Yours Tony
linux.conf.au http://lca2005.linux.org.au/
Apr 18-23 2005 The Australian Linux Technical Conference!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
----- End forwarded message -----
Yours Tony
linux.conf.au http://lca2005.linux.org.au/
Apr 18-23 2005 The Australian Linux Technical Conference!

Tony Breeds

author

2004-04-20 22:16 UTC #2 lore raw

Quoted 10 lines · Andrew Morton

Seems sane to me, but I'm not sure why this wasn't done originally. ie, this:

(quoted diff) +5 -0

+#ifdef CONFIG_SERIAL_8250
 extern void ibmasm_register_uart(struct service_processor *sp);
 extern void ibmasm_unregister_uart(struct service_processor *sp);
+#else
+#define ibmasm_register_uart(sp)	do { } while(0)
+#define ibmasm_unregister_uart(sp)	do { } while(0)
+#endif

Quoted 3 lines

becomes unnecessary with your patch.
Max, any preferences?

If I read this correctly the above patch would mean that ibmasm can be built regardless of the value of SERIAL_8250 BUT my patch means it can only be built if SERIAL_8250 is also being built (regardless of state).

Can the device operate correctly without the uart? If so then my patch is bogus.

Yours Tony

linux.conf.au http://lca2005.linux.org.au/
Apr 18-23 2005 The Australian Linux Technical Conference!

Max Asbock

2004-04-20 22:53 UTC #3 lore raw

In reply to Andrew Morton

Quoted 12 lines · Andrew Morton

Tony Breeds <[email protected]> wrote:
Seems sane to me, but I'm not sure why this wasn't done originally. ie, this:
(quoted diff) +5 -0
+#ifdef CONFIG_SERIAL_8250
 extern void ibmasm_register_uart(struct service_processor *sp);
 extern void ibmasm_unregister_uart(struct service_processor *sp);
+#else
+#define ibmasm_register_uart(sp)	do { } while(0)
+#define ibmasm_unregister_uart(sp)	do { } while(0)
+#endif

Quoted 4 lines

becomes unnecessary with your patch.
Max, any preferences?

The above allows the driver to be built without serial line support. It still functions that way. uart support is only part of the driver's functions. Therefore it makes sense to not make the whole driver depend on SERIAL_8250 and instead only configure away the uart support when SERIAL_8250 is not defined.

regards,
max

Quoted 13 lines

################################################################################

drivers/misc/Kconfig +1 -1

@@ -6,7 +6,7 @@
 
 config IBM_ASM
 	tristate "Device driver for IBM RSA service processor"
-	depends on X86
+	depends on X86 && SERIAL_8250
 	default n
 	---help---
 	  This option enables device driver support for in-band access to the

Andrew Morton

2004-04-20 23:41 UTC #4 lore raw

Quoted 16 lines · Max Asbock

Quoted 14 lines · Andrew Morton
Tony Breeds <[email protected]> wrote:
Seems sane to me, but I'm not sure why this wasn't done originally. ie, this:
(quoted diff) +5 -0
+#ifdef CONFIG_SERIAL_8250
 extern void ibmasm_register_uart(struct service_processor *sp);
 extern void ibmasm_unregister_uart(struct service_processor *sp);
+#else
+#define ibmasm_register_uart(sp)	do { } while(0)
+#define ibmasm_unregister_uart(sp)	do { } while(0)
+#endif
becomes unnecessary with your patch.

Quoted 8 lines

Quoted 2 lines
Max, any preferences?
The above allows the driver to be built without serial line support. It still functions that way. uart support is only part of the driver's functions. Therefore it makes sense to not make the whole driver depend on SERIAL_8250 and instead only configure away the uart support when SERIAL_8250 is not defined.

So I think you'll be needing something liek this?

 25-akpm/drivers/misc/ibmasm/uart.c |    4 ++++
 1 files changed, 4 insertions(+)

drivers/misc/ibmasm/uart.c renamed from drivers/misc/ibmasm/uart.c~a +4 -0

diff -puN drivers/misc/ibmasm/uart.c~a drivers/misc/ibmasm/uart.c
@@ -54,12 +54,14 @@ void ibmasm_register_uart(struct service
 	serial.io_type		= UPIO_MEM;
 	serial.iomem_base	= iomem_base;
 
+#ifdef CONFIG_SERIAL_8250
 	sp->serial_line = register_serial(&serial);
 	if (sp->serial_line < 0) {
 		dev_err(sp->dev, "Failed to register serial port\n");
 		return;
 	}
 	enable_uart_interrupts(sp->base_address);
+#endif
 }
 
 void ibmasm_unregister_uart(struct service_processor *sp)

@@ -68,5 +70,7 @@ void ibmasm_unregister_uart(struct servi
 		return;
 
 	disable_uart_interrupts(sp->base_address);
+#ifdef CONFIG_SERIAL_8250
 	unregister_serial(sp->serial_line);
+#endif
 }

Max Asbock

2004-04-21 00:18 UTC #5 lore raw

Quoted 35 lines · Andrew Morton

Quoted 5 lines · Max Asbock
... allows the driver to be built without serial line support. It still functions that way. uart support is only part of the driver's functions. Therefore it makes sense to not make the whole driver depend on SERIAL_8250 and instead only configure away the uart support when SERIAL_8250 is not defined.
So I think you'll be needing something liek this?
 25-akpm/drivers/misc/ibmasm/uart.c |    4 ++++
 1 files changed, 4 insertions(+)
diff -puN drivers/misc/ibmasm/uart.c~a drivers/misc/ibmasm/uart.c
drivers/misc/ibmasm/uart.c renamed from drivers/misc/ibmasm/uart.c~a +2 -0
@@ -54,12 +54,14 @@ void ibmasm_register_uart(struct service
 	serial.io_type		= UPIO_MEM;
 	serial.iomem_base	= iomem_base;
 
+#ifdef CONFIG_SERIAL_8250
 	sp->serial_line = register_serial(&serial);
 	if (sp->serial_line < 0) {
 		dev_err(sp->dev, "Failed to register serial port\n");
 		return;
 	}
 	enable_uart_interrupts(sp->base_address);
+#endif
 }
 
 void ibmasm_unregister_uart(struct service_processor *sp)

drivers/misc/ibmasm/uart.c void ibmasm_unregister_uart(struct servi · line 76

Quoted 7 lines

drivers/misc/ibmasm/uart.c renamed from drivers/misc/ibmasm/uart.c~a +2 -0

@@ -68,5 +70,7 @@ void ibmasm_unregister_uart(struct servi
 		return;
 
 	disable_uart_interrupts(sp->base_address);
+#ifdef CONFIG_SERIAL_8250
 	unregister_serial(sp->serial_line);
+#endif
 }

I posted the following patch a little while ago. I arranged it that way with the intention to avoid #ifdefs in the .c file. This patch has been applied in 2.6.6-rc2. So it is all good.

regards,
max

drivers/misc/ibmasm/ibmasm.h +5 -0

diff -urN linux-2.6.5/drivers/misc/ibmasm/ibmasm.h linux-2.6.5-ibmasm/drivers/misc/ibmasm/ibmasm.h
@@ -220,5 +220,10 @@
 extern void ibmasmfs_add_sp(struct service_processor *sp);
 
 /* uart */
+#ifdef CONFIG_SERIAL_8250
 extern void ibmasm_register_uart(struct service_processor *sp);
 extern void ibmasm_unregister_uart(struct service_processor *sp);
+#else
+#define ibmasm_register_uart(sp)	do { } while(0)
+#define ibmasm_unregister_uart(sp)	do { } while(0)
+#endif

drivers/misc/ibmasm/Makefile +5 -3

diff -urN linux-2.6.5/drivers/misc/ibmasm/Makefile linux-2.6.5-ibmasm/drivers/misc/ibmasm/Makefile
@@ -1,7 +1,7 @@
 
 obj-$(CONFIG_IBM_ASM) := ibmasm.o
 
-ibmasm-objs :=	module.o      \
+ibmasm-y :=	module.o      \
 		ibmasmfs.o    \
 		event.o       \
 		command.o     \

@@ -9,5 +9,7 @@
 		heartbeat.o   \
 		r_heartbeat.o \
 		dot_command.o \
-		lowlevel.o    \
-		uart.o
+		lowlevel.o
+
+ibmasm-$(CONFIG_SERIAL_8250) += uart.o
+

drivers/misc/Kconfig +3 -1

diff -urN linux-2.6.5/drivers/misc/Kconfig linux-2.6.5-ibmasm/drivers/misc/Kconfig
@@ -16,7 +16,9 @@
 	  processor. The driver is meant to be used in conjunction with
 	  a user space API.
 	  The ibmasm driver also enables the OS to use the UART on the
-          service processor board as a regular serial port.
+	  service processor board as a regular serial port. To make use of
+	  this feature serial driver support (CONFIG_SERIAL_8250) must be
+	  enabled.
 	  
 
 	  If unsure, say N.