[PATCH] tty: 8250_dw: fix build error for CONFIG_SERIAL_8250=m

[PATCH] tty: 8250_dw: fix build error for CONFIG_SERIAL_8250=m

[Jamie Iles]

Allow 8250_dw to be built as a module and export serial8250_handle_irq
so that 8250 can still be built as a module.  This prevents the
following build failure:

drivers/built-in.o: In function `dw8250_handle_irq':
8250_dw.c:(.text+0xcad9c): undefined reference to `serial8250_handle_irq'

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Greg KH <greg@kroah.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 drivers/tty/serial/8250.c    |    1 +
 drivers/tty/serial/8250_dw.c |    5 +++++
 drivers/tty/serial/Kconfig   |    2 +-
 3 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index 6f594d2..435ce14 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -1588,6 +1588,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(serial8250_handle_irq);
 
 static int serial8250_default_handle_irq(struct uart_port *port)
 {
diff --git a/drivers/tty/serial/8250_dw.c b/drivers/tty/serial/8250_dw.c
index e25782a..844d3a8 100644
--- a/drivers/tty/serial/8250_dw.c
+++ b/drivers/tty/serial/8250_dw.c
@@ -13,6 +13,7 @@
  * raised, the LCR needs to be rewritten and the uart status register read.
  */
 #include <linux/io.h>
+#include <linux/module.h>
 #include <linux/serial_8250.h>
 #include <linux/serial_core.h>
 #include <linux/serial_reg.h>
@@ -97,3 +98,7 @@ int serial8250_use_designware_io(struct uart_port *up)
 
 	return 0;
 }
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jamie Iles");
+MODULE_DESCRIPTION("Synopsys DesignWare 8250 UART support");
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index d2d1cc2..1002e2e 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -268,7 +268,7 @@ config SERIAL_8250_RM9K
 	  If unsure, say N.
 
 config SERIAL_8250_DW
-	bool "Support for Synopsys DesignWare 8250 quirks"
+	tristate "Support for Synopsys DesignWare 8250 quirks"
 	depends on SERIAL_8250
 	help
 	  Selecting this option will enable handling of the extra features
-- 
1.7.4.4

Re: [PATCH] tty: 8250_dw: fix build error for CONFIG_SERIAL_8250=m

[Arnd Bergmann]

On Wednesday 24 August 2011, Jamie Iles wrote:
> Allow 8250_dw to be built as a module and export serial8250_handle_irq
> so that 8250 can still be built as a module.  This prevents the
> following build failure:
> 
> drivers/built-in.o: In function `dw8250_handle_irq':
> 8250_dw.c:(.text+0xcad9c): undefined reference to `serial8250_handle_irq'

I think this won't fix the bug that Stephen was reporting, it will only
make the error message go away but it won't work in the end: When
CONFIG_SERIAL_8250_DW is set to 'm', the declaration of
serial8250_use_designware_io now gets stubbed out from of_serial.ko,
meaning that the 8250_dw module becomes useless.

At the very least you also need to export the serial8250_use_designware_io
symbol and check for CONFIG_SERIAL_8250_DW_MODULE in the header.

When go go to such length, I think the approach I initially advocated
(making 8250_dw a standalone platform_driver like of_serial) will be
nicer.

Another alternative would be to link 8250_dw.o into 8250.ko, but that
requires renaming the module and will conflict with some of the other
changes I'm still planning to do with the 8250 driver.

	Arnd

Re: [PATCH] tty: 8250_dw: fix build error for CONFIG_SERIAL_8250=m

[Jamie Iles]

On Wed, Aug 24, 2011 at 04:45:16PM +0200, Arnd Bergmann wrote:
> On Wednesday 24 August 2011, Jamie Iles wrote:
> > Allow 8250_dw to be built as a module and export serial8250_handle_irq
> > so that 8250 can still be built as a module.  This prevents the
> > following build failure:
> > 
> > drivers/built-in.o: In function `dw8250_handle_irq':
> > 8250_dw.c:(.text+0xcad9c): undefined reference to `serial8250_handle_irq'
> 
> I think this won't fix the bug that Stephen was reporting, it will only
> make the error message go away but it won't work in the end: When
> CONFIG_SERIAL_8250_DW is set to 'm', the declaration of
> serial8250_use_designware_io now gets stubbed out from of_serial.ko,
> meaning that the 8250_dw module becomes useless.
> 
> At the very least you also need to export the serial8250_use_designware_io
> symbol and check for CONFIG_SERIAL_8250_DW_MODULE in the header.

Ahh, I didn't realise that this would define 
CONFIG_SERIAL_8250_DW_MODULE rather than CONFIG_SERIAL_8250_DW.

> When go go to such length, I think the approach I initially advocated
> (making 8250_dw a standalone platform_driver like of_serial) will be
> nicer.

OK, in hindsight that does seem like a much better option.  I'll spin a 
separate platform_driver instead.

Greg, I'm not sure how to best handle this, do you want patches to 
revert:

	tty: serial8250: add helpers for the DesignWare 8250
	tty: of_serial: add support for the DesignWare 8250

or can these be dropped?

Thanks,

Jamie

Re: [PATCH] tty: 8250_dw: fix build error for CONFIG_SERIAL_8250=m

[Greg KH]

On Wed, Aug 24, 2011 at 03:54:00PM +0100, Jamie Iles wrote:
> On Wed, Aug 24, 2011 at 04:45:16PM +0200, Arnd Bergmann wrote:
> > On Wednesday 24 August 2011, Jamie Iles wrote:
> > > Allow 8250_dw to be built as a module and export serial8250_handle_irq
> > > so that 8250 can still be built as a module.  This prevents the
> > > following build failure:
> > > 
> > > drivers/built-in.o: In function `dw8250_handle_irq':
> > > 8250_dw.c:(.text+0xcad9c): undefined reference to `serial8250_handle_irq'
> > 
> > I think this won't fix the bug that Stephen was reporting, it will only
> > make the error message go away but it won't work in the end: When
> > CONFIG_SERIAL_8250_DW is set to 'm', the declaration of
> > serial8250_use_designware_io now gets stubbed out from of_serial.ko,
> > meaning that the 8250_dw module becomes useless.
> > 
> > At the very least you also need to export the serial8250_use_designware_io
> > symbol and check for CONFIG_SERIAL_8250_DW_MODULE in the header.
> 
> Ahh, I didn't realise that this would define 
> CONFIG_SERIAL_8250_DW_MODULE rather than CONFIG_SERIAL_8250_DW.
> 
> > When go go to such length, I think the approach I initially advocated
> > (making 8250_dw a standalone platform_driver like of_serial) will be
> > nicer.
> 
> OK, in hindsight that does seem like a much better option.  I'll spin a 
> separate platform_driver instead.
> 
> Greg, I'm not sure how to best handle this, do you want patches to 
> revert:
> 
> 	tty: serial8250: add helpers for the DesignWare 8250
> 	tty: of_serial: add support for the DesignWare 8250
> 
> or can these be dropped?

I'll go revert them myself, as that's easiest, right?

thanks,

greg k-h

Re: [PATCH] tty: 8250_dw: fix build error for CONFIG_SERIAL_8250=m

[Jamie Iles]

On Wed, Aug 24, 2011 at 08:09:34AM -0700, Greg KH wrote:
> On Wed, Aug 24, 2011 at 03:54:00PM +0100, Jamie Iles wrote:
> > On Wed, Aug 24, 2011 at 04:45:16PM +0200, Arnd Bergmann wrote:
> > > On Wednesday 24 August 2011, Jamie Iles wrote:
> > > > Allow 8250_dw to be built as a module and export serial8250_handle_irq
> > > > so that 8250 can still be built as a module.  This prevents the
> > > > following build failure:
> > > > 
> > > > drivers/built-in.o: In function `dw8250_handle_irq':
> > > > 8250_dw.c:(.text+0xcad9c): undefined reference to `serial8250_handle_irq'
> > > 
> > > I think this won't fix the bug that Stephen was reporting, it will only
> > > make the error message go away but it won't work in the end: When
> > > CONFIG_SERIAL_8250_DW is set to 'm', the declaration of
> > > serial8250_use_designware_io now gets stubbed out from of_serial.ko,
> > > meaning that the 8250_dw module becomes useless.
> > > 
> > > At the very least you also need to export the serial8250_use_designware_io
> > > symbol and check for CONFIG_SERIAL_8250_DW_MODULE in the header.
> > 
> > Ahh, I didn't realise that this would define 
> > CONFIG_SERIAL_8250_DW_MODULE rather than CONFIG_SERIAL_8250_DW.
> > 
> > > When go go to such length, I think the approach I initially advocated
> > > (making 8250_dw a standalone platform_driver like of_serial) will be
> > > nicer.
> > 
> > OK, in hindsight that does seem like a much better option.  I'll spin a 
> > separate platform_driver instead.
> > 
> > Greg, I'm not sure how to best handle this, do you want patches to 
> > revert:
> > 
> > 	tty: serial8250: add helpers for the DesignWare 8250
> > 	tty: of_serial: add support for the DesignWare 8250
> > 
> > or can these be dropped?
> 
> I'll go revert them myself, as that's easiest, right?

Thanks Greg, that would be great (and sorry for the breakage!).

Jamie

Re: [PATCH] tty: 8250_dw: fix build error for CONFIG_SERIAL_8250=m

[Jamie Iles]

Hi Arnd,

On Wed, Aug 24, 2011 at 04:45:16PM +0200, Arnd Bergmann wrote:
> When go go to such length, I think the approach I initially advocated
> (making 8250_dw a standalone platform_driver like of_serial) will be
> nicer.

For a separate driver, is it okay to make it depend on CONFIG_OF, and 
only have device tree bindings, or should this still support being 
registered as a static platform device with platform data?  I'm not sure 
what the consensus is on creating device tree only platform drivers.

Thanks,

Jamie

Re: [PATCH] tty: 8250_dw: fix build error for CONFIG_SERIAL_8250=m

[Arnd Bergmann]

On Wednesday 24 August 2011, Jamie Iles wrote:
> On Wed, Aug 24, 2011 at 04:45:16PM +0200, Arnd Bergmann wrote:
> > When go go to such length, I think the approach I initially advocated
> > (making 8250_dw a standalone platform_driver like of_serial) will be
> > nicer.
> 
> For a separate driver, is it okay to make it depend on CONFIG_OF, and 
> only have device tree bindings, or should this still support being 
> registered as a static platform device with platform data?  I'm not sure 
> what the consensus is on creating device tree only platform drivers.

Do only what is necessary. If the devices are all probed using device
tree, then support only that. If someone else really needs static
platform_data instead, they can add support for that later.

	Arnd