Patchwork [U-Boot,1/2] serial: arm_dcc: Remove CONFIG_ARM_DCC_MULTI option

login
register
mail settings
Submitter Michal Simek
Date Jan. 23, 2013, 9:40 a.m.
Message ID <1358934007-14928-2-git-send-email-michal.simek@xilinx.com>
Download mbox | patch
Permalink /patch/214865/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Michal Simek - Jan. 23, 2013, 9:40 a.m.
CONFIG_ARM_DCC_MULTI should be also removed in the patch
"serial: Remove CONFIG_SERIAL_MULTI from serial drivers"
(sha1: a3827250606895ec2dd4b8d867342b7cabf3692f)
Because the driver defines serial_* functions
which cause conflict with serial.c (multiple definition of serial_*)

Removing CONFIG_SERIAL_MULTI function also require to define
default_serial_console for cases where another serial driver
is not available in the system.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Error log:
serial.o: In function `serial_init':
/mnt/projects/u-boot/drivers/serial/serial.c:402: multiple definition of `serial_init'
arm_dcc.o:/mnt/projects/u-boot/drivers/serial/arm_dcc.c:104: first defined here
serial.o: In function `serial_setbrg':
/mnt/projects/u-boot/drivers/serial/serial.c:417: multiple definition of `serial_setbrg'
arm_dcc.o:/mnt/projects/u-boot/drivers/serial/arm_dcc.c:94: first defined here
serial.o: In function `serial_getc':
/mnt/projects/u-boot/drivers/serial/serial.c:433: multiple definition of `serial_getc'
arm_dcc.o:/mnt/projects/u-boot/drivers/serial/arm_dcc.c:112: first defined here
serial.o: In function `serial_tstc':
/mnt/projects/u-boot/drivers/serial/serial.c:448: multiple definition of `serial_tstc'
arm_dcc.o:/mnt/projects/u-boot/drivers/serial/arm_dcc.c:145: first defined here
serial.o: In function `serial_putc':
/mnt/projects/u-boot/drivers/serial/serial.c:464: multiple definition of `serial_putc'
arm_dcc.o:/mnt/projects/u-boot/drivers/serial/arm_dcc.c:124: first defined here
serial.o: In function `serial_puts':
/mnt/projects/u-boot/drivers/serial/serial.c:482: multiple definition of `serial_puts'
arm_dcc.o:/mnt/projects/u-boot/drivers/serial/arm_dcc.c:136: first defined here

The second error log:
/mnt/projects/u-boot/drivers/serial/serial.c:374: undefined reference to `default_serial_console'
drivers/serial/libserial.o: In function `serial_initialize':
---
 common/stdio.c           |    2 +-
 drivers/serial/arm_dcc.c |   16 +++++-----------
 include/stdio_dev.h      |    2 +-
 3 files changed, 7 insertions(+), 13 deletions(-)
Wolfgang Denk - Jan. 23, 2013, 10:02 a.m.
Dear Michal Simek,

In message <1358934007-14928-2-git-send-email-michal.simek@xilinx.com> you wrote:
> CONFIG_ARM_DCC_MULTI should be also removed in the patch
> "serial: Remove CONFIG_SERIAL_MULTI from serial drivers"
> (sha1: a3827250606895ec2dd4b8d867342b7cabf3692f)
> Because the driver defines serial_* functions
> which cause conflict with serial.c (multiple definition of serial_*)
> 
> Removing CONFIG_SERIAL_MULTI function also require to define
> default_serial_console for cases where another serial driver
> is not available in the system.

As you pointed out in the cover letter, CONFIG_ARM_DCC and
CONFIG_ARM_DCC_MULTI are nowhere defined in any mainline U-Boot code.
Instead of fixing and keeping the dead code, we should rather remove
the drivers/serial/arm_dcc.c driver and all references to
CONFIG_ARM_DCC*

If you want, I can prepare a cleanup patch.

Best regards,

Wolfgang Denk
Michal Simek - Jan. 23, 2013, 10:20 a.m.
Dear Wolfgang Denk,

2013/1/23 Wolfgang Denk <wd@denx.de>:
> Dear Michal Simek,
>
> In message <1358934007-14928-2-git-send-email-michal.simek@xilinx.com> you wrote:
>> CONFIG_ARM_DCC_MULTI should be also removed in the patch
>> "serial: Remove CONFIG_SERIAL_MULTI from serial drivers"
>> (sha1: a3827250606895ec2dd4b8d867342b7cabf3692f)
>> Because the driver defines serial_* functions
>> which cause conflict with serial.c (multiple definition of serial_*)
>>
>> Removing CONFIG_SERIAL_MULTI function also require to define
>> default_serial_console for cases where another serial driver
>> is not available in the system.
>
> As you pointed out in the cover letter, CONFIG_ARM_DCC and
> CONFIG_ARM_DCC_MULTI are nowhere defined in any mainline U-Boot code.
> Instead of fixing and keeping the dead code, we should rather remove
> the drivers/serial/arm_dcc.c driver and all references to
> CONFIG_ARM_DCC*
>
> If you want, I can prepare a cleanup patch.

I have found that we are using this driver for one board.
That's why I prefer to keep this driver in the mainline and enabling
it for arm zynq.

Thanks,
Michal
Marek Vasut - Jan. 26, 2013, 5:43 a.m.
Dear Michal Simek,

> CONFIG_ARM_DCC_MULTI should be also removed in the patch
> "serial: Remove CONFIG_SERIAL_MULTI from serial drivers"
> (sha1: a3827250606895ec2dd4b8d867342b7cabf3692f)
> Because the driver defines serial_* functions
> which cause conflict with serial.c (multiple definition of serial_*)
> 
> Removing CONFIG_SERIAL_MULTI function also require to define
> default_serial_console for cases where another serial driver
> is not available in the system.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

I think it looks good.

Acked-by: Marek Vasut <marex@denx.de>

btw. did I by any chance became serial subsystem custodian without knowing about 
it recently? ;-)

Best regards,
Marek Vasut
Michal Simek - Jan. 28, 2013, 10:52 a.m.
2013/1/26 Marek Vasut <marex@denx.de>:
> Dear Michal Simek,
>
>> CONFIG_ARM_DCC_MULTI should be also removed in the patch
>> "serial: Remove CONFIG_SERIAL_MULTI from serial drivers"
>> (sha1: a3827250606895ec2dd4b8d867342b7cabf3692f)
>> Because the driver defines serial_* functions
>> which cause conflict with serial.c (multiple definition of serial_*)
>>
>> Removing CONFIG_SERIAL_MULTI function also require to define
>> default_serial_console for cases where another serial driver
>> is not available in the system.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>
> I think it looks good.
>
> Acked-by: Marek Vasut <marex@denx.de>

Thanks,
Michal
Tom Rini - Feb. 4, 2013, 4:40 p.m.
On Tue, Jan 22, 2013 at 11:40:06PM -0000, Michal Simek wrote:

> CONFIG_ARM_DCC_MULTI should be also removed in the patch
> "serial: Remove CONFIG_SERIAL_MULTI from serial drivers"
> (sha1: a3827250606895ec2dd4b8d867342b7cabf3692f)
> Because the driver defines serial_* functions
> which cause conflict with serial.c (multiple definition of serial_*)
> 
> Removing CONFIG_SERIAL_MULTI function also require to define
> default_serial_console for cases where another serial driver
> is not available in the system.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Acked-by: Marek Vasut <marex@denx.de>

Applied to u-boot/master (along with 2/2), thanks!

Patch

diff --git a/common/stdio.c b/common/stdio.c
index 97ff9cf..5d5117c 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -207,7 +207,7 @@  int stdio_init (void)
 	/* Initialize the list */
 	INIT_LIST_HEAD(&(devs.list));
 
-#ifdef CONFIG_ARM_DCC_MULTI
+#ifdef CONFIG_ARM_DCC
 	drv_arm_dcc_init ();
 #endif
 #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
diff --git a/drivers/serial/arm_dcc.c b/drivers/serial/arm_dcc.c
index 7b5ecb5..812dcf0 100644
--- a/drivers/serial/arm_dcc.c
+++ b/drivers/serial/arm_dcc.c
@@ -89,15 +89,6 @@ 
 
 #define TIMEOUT_COUNT 0x4000000
 
-#ifndef CONFIG_ARM_DCC_MULTI
-#define arm_dcc_init serial_init
-void serial_setbrg(void) {}
-#define arm_dcc_getc serial_getc
-#define arm_dcc_putc serial_putc
-#define arm_dcc_puts serial_puts
-#define arm_dcc_tstc serial_tstc
-#endif
-
 int arm_dcc_init(void)
 {
 	return 0;
@@ -147,7 +138,6 @@  int arm_dcc_tstc(void)
 	return reg;
 }
 
-#ifdef CONFIG_ARM_DCC_MULTI
 static struct stdio_dev arm_dcc_dev;
 
 int drv_arm_dcc_init(void)
@@ -167,4 +157,8 @@  int drv_arm_dcc_init(void)
 
 	return stdio_register(&arm_dcc_dev);
 }
-#endif
+
+__weak struct serial_device *default_serial_console(void)
+{
+	return NULL;
+}
diff --git a/include/stdio_dev.h b/include/stdio_dev.h
index 932d093..9451740 100644
--- a/include/stdio_dev.h
+++ b/include/stdio_dev.h
@@ -99,7 +99,7 @@  struct list_head* stdio_get_list(void);
 struct stdio_dev* stdio_get_by_name(const char* name);
 struct stdio_dev* stdio_clone(struct stdio_dev *dev);
 
-#ifdef CONFIG_ARM_DCC_MULTI
+#ifdef CONFIG_ARM_DCC
 int drv_arm_dcc_init(void);
 #endif
 #ifdef CONFIG_LCD