mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Kconfig dependancy update for drivers/misc/ibmasm
@ 2004-04-20 21:01 Tony Breeds
  2004-04-20 21:34 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Tony Breeds @ 2004-04-20 21:01 UTC (permalink / raw)
  To: Linux Kernel ML, Andrew Morton


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

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

################################################################################
--- 2.6.4.clean/drivers/misc/Kconfig	2004-03-11 17:57:23.000000000 +1100
+++ 2.6.4.noconfig/drivers/misc/Kconfig	2004-03-30 09:32:07.000000000 +1000
@@ -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 majordomo@vger.kernel.org
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!


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Kconfig dependancy update for drivers/misc/ibmasm
  2004-04-20 21:01 [PATCH] Kconfig dependancy update for drivers/misc/ibmasm Tony Breeds
@ 2004-04-20 21:34 ` Andrew Morton
  2004-04-20 21:41   ` Tony Breeds
  2004-04-20 22:49   ` Max Asbock
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2004-04-20 21:34 UTC (permalink / raw)
  To: Tony Breeds; +Cc: linux-kernel, Max Asbock

Tony Breeds <tony@bakeyournoodle.com> wrote:
>
> 
> Hello,
> 	Some weeks ago I saw this compile error posted to lkml:
> 
> ---
> >   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.

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?


> ################################################################################
> --- 2.6.4.clean/drivers/misc/Kconfig	2004-03-11 17:57:23.000000000 +1100
> +++ 2.6.4.noconfig/drivers/misc/Kconfig	2004-03-30 09:32:07.000000000 +1000
> @@ -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 majordomo@vger.kernel.org
> 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!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Kconfig dependancy update for drivers/misc/ibmasm
  2004-04-20 21:34 ` Andrew Morton
@ 2004-04-20 21:41   ` Tony Breeds
  2004-04-20 22:49   ` Max Asbock
  1 sibling, 0 replies; 6+ messages in thread
From: Tony Breeds @ 2004-04-20 21:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, Max Asbock

On Tue, Apr 20, 2004 at 02:34:18PM -0700, Andrew Morton wrote:
 
> 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?

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!


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Kconfig dependancy update for drivers/misc/ibmasm
  2004-04-20 21:34 ` Andrew Morton
  2004-04-20 21:41   ` Tony Breeds
@ 2004-04-20 22:49   ` Max Asbock
  2004-04-20 23:42     ` Andrew Morton
  1 sibling, 1 reply; 6+ messages in thread
From: Max Asbock @ 2004-04-20 22:49 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Tony Breeds, linux-kernel

On Tue, 2004-04-20 at 14:34, Andrew Morton wrote:
> Tony Breeds <tony@bakeyournoodle.com> wrote:
> 
> 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?
> 

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


> 
> > ################################################################################
> > --- 2.6.4.clean/drivers/misc/Kconfig	2004-03-11 17:57:23.000000000 +1100
> > +++ 2.6.4.noconfig/drivers/misc/Kconfig	2004-03-30 09:32:07.000000000 +1000
> > @@ -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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Kconfig dependancy update for drivers/misc/ibmasm
  2004-04-20 22:49   ` Max Asbock
@ 2004-04-20 23:42     ` Andrew Morton
  2004-04-21  0:17       ` Max Asbock
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2004-04-20 23:42 UTC (permalink / raw)
  To: Max Asbock; +Cc: tony, linux-kernel

Max Asbock <masbock@us.ibm.com> wrote:
>
> On Tue, 2004-04-20 at 14:34, Andrew Morton wrote:
> > Tony Breeds <tony@bakeyournoodle.com> wrote:
> > 
> > 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?
> > 
> 
> 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(+)

diff -puN drivers/misc/ibmasm/uart.c~a drivers/misc/ibmasm/uart.c
--- 25/drivers/misc/ibmasm/uart.c~a	Tue Apr 20 16:41:32 2004
+++ 25-akpm/drivers/misc/ibmasm/uart.c	Tue Apr 20 16:41:56 2004
@@ -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
 }

_


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Kconfig dependancy update for drivers/misc/ibmasm
  2004-04-20 23:42     ` Andrew Morton
@ 2004-04-21  0:17       ` Max Asbock
  0 siblings, 0 replies; 6+ messages in thread
From: Max Asbock @ 2004-04-21  0:17 UTC (permalink / raw)
  To: Andrew Morton; +Cc: tony, linux-kernel

On Tue, 2004-04-20 at 16:42, Andrew Morton wrote:

> Max Asbock <masbock@us.ibm.com> wrote:

> > ... 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
> --- 25/drivers/misc/ibmasm/uart.c~a	Tue Apr 20 16:41:32 2004
> +++ 25-akpm/drivers/misc/ibmasm/uart.c	Tue Apr 20 16:41:56 2004
> @@ -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
>  }
> 

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

diff -urN linux-2.6.5/drivers/misc/ibmasm/ibmasm.h linux-2.6.5-ibmasm/drivers/misc/ibmasm/ibmasm.h
--- linux-2.6.5/drivers/misc/ibmasm/ibmasm.h	2004-04-03 19:36:18.000000000 -0800
+++ linux-2.6.5-ibmasm/drivers/misc/ibmasm/ibmasm.h	2004-04-06 10:56:31.000000000 -0700
@@ -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
diff -urN linux-2.6.5/drivers/misc/ibmasm/Makefile linux-2.6.5-ibmasm/drivers/misc/ibmasm/Makefile
--- linux-2.6.5/drivers/misc/ibmasm/Makefile	2004-04-03 19:37:37.000000000 -0800
+++ linux-2.6.5-ibmasm/drivers/misc/ibmasm/Makefile	2004-04-06 13:07:54.000000000 -0700
@@ -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
+
diff -urN linux-2.6.5/drivers/misc/Kconfig linux-2.6.5-ibmasm/drivers/misc/Kconfig
--- linux-2.6.5/drivers/misc/Kconfig	2004-04-03 19:36:26.000000000 -0800
+++ linux-2.6.5-ibmasm/drivers/misc/Kconfig	2004-04-06 13:50:49.924254952 -0700
@@ -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.




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2004-04-21  0:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-04-20 21:01 [PATCH] Kconfig dependancy update for drivers/misc/ibmasm Tony Breeds
2004-04-20 21:34 ` Andrew Morton
2004-04-20 21:41   ` Tony Breeds
2004-04-20 22:49   ` Max Asbock
2004-04-20 23:42     ` Andrew Morton
2004-04-21  0:17       ` Max Asbock

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®