serial: Remove the "serial" console device

PATCH New Joe Hershberger 3 participants 14 messages Nov 1, 2012 – Nov 6, 2012

Share & tools

Joe Hershberger

author

2012-11-01 21:46 UTC #0 lore raw

PATCH 1/2 serial: Remove the "serial" console device

Each serial device is added as a console device. There was also the "serial" device that points to the most-recently-assigned-to-some- console-handle device. This can be confusing. Instead, only show the actual serial devices.

Signed-off-by Joe Hershberger <[email protected]>

---
 common/console.c | 37 +++++++++++++++++++------------------
 common/stdio.c   | 12 +-----------
 2 files changed, 20 insertions(+), 29 deletions(-)

common/console.c +19 ~~-18~~

index 1177f7d..9cc8197 100644
@@ -26,6 +26,7 @@
 #include <malloc.h>
 #include <stdio_dev.h>
 #include <exports.h>
+#include <serial.h>
 
 DECLARE_GLOBAL_DATA_PTR;

@@ -550,6 +551,7 @@ int console_assign(int file, const char *devname)
 {
 	int flag;
 	struct stdio_dev *dev;
+	const char *realdevname;
 
 	/* Check for valid file */
 	switch (file) {

@@ -565,8 +567,12 @@ int console_assign(int file, const char *devname)
 	}
 
 	/* Check for valid device name */
+	if (strcmp(devname, "serial") == 0)
+		realdevname = default_serial_console()->name;
+	else
+		realdevname = devname;
 
-	dev = search_device(flag, devname);
+	dev = search_device(flag, realdevname);
 
 	if (dev)
 		return console_setfile(file, dev);

@@ -657,13 +663,16 @@ int console_init_r(void)
 	}
 	/* if the devices are overwritten or not found, use default device */
 	if (inputdev == NULL) {
-		inputdev  = search_device(DEV_FLAGS_INPUT,  "serial");
+		inputdev  = search_device(DEV_FLAGS_INPUT,
+			default_serial_console()->name);
 	}
 	if (outputdev == NULL) {
-		outputdev = search_device(DEV_FLAGS_OUTPUT, "serial");
+		outputdev = search_device(DEV_FLAGS_OUTPUT,
+			default_serial_console()->name);
 	}
 	if (errdev == NULL) {
-		errdev    = search_device(DEV_FLAGS_OUTPUT, "serial");
+		errdev = search_device(DEV_FLAGS_OUTPUT,
+			default_serial_console()->name);
 	}
 	/* Initializes output console first */
 	if (outputdev != NULL) {

@@ -713,18 +722,10 @@ int console_init_r(void)
 	struct list_head *pos;
 	struct stdio_dev *dev;
 
-#ifdef CONFIG_SPLASH_SCREEN
-	/*
-	 * suppress all output if splash screen is enabled and we have
-	 * a bmp to display. We redirect the output from frame buffer
-	 * console to serial console in this case or suppress it if
-	 * "silent" mode was requested.
-	 */
-	if (getenv("splashimage") != NULL) {
-		if (!(gd->flags & GD_FLG_SILENT))
-			outputdev = search_device (DEV_FLAGS_OUTPUT, "serial");
-	}
-#endif
+	outputdev = search_device(DEV_FLAGS_OUTPUT,
+		default_serial_console()->name);
+	inputdev = search_device(DEV_FLAGS_INPUT,
+		default_serial_console()->name);
 
 	/* Scan devices looking for input and output devices */
 	list_for_each(pos, list) {

@@ -760,13 +761,13 @@ int console_init_r(void)
 
 	gd->flags |= GD_FLG_DEVINIT;	/* device initialization completed */
 
-	stdio_print_current_devices();
-
 	/* Setting environment variables */
 	for (i = 0; i < 3; i++) {
 		setenv(stdio_names[i], stdio_devices[i]->name);
 	}
 
+	stdio_print_current_devices();
+
 #if 0
 	/* If nothing usable installed, use only the initial console */
 	if ((stdio_devices[stdin] == NULL) && (stdio_devices[stdout] == NULL))

common/stdio.c +1 ~~-11~~

index 605ff3f..e9bdc0e 100644
@@ -70,21 +70,11 @@ int nulldev_input(void)
 
 static void drv_system_init (void)
 {
+#ifdef CONFIG_SYS_DEVICE_NULLDEV
 	struct stdio_dev dev;
 
 	memset (&dev, 0, sizeof (dev));
 
-	strcpy (dev.name, "serial");
-	dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM;
-	dev.putc = serial_putc;
-	dev.puts = serial_puts;
-	dev.getc = serial_getc;
-	dev.tstc = serial_tstc;
-	stdio_register (&dev);
-
-#ifdef CONFIG_SYS_DEVICE_NULLDEV
-	memset (&dev, 0, sizeof (dev));
-
 	strcpy (dev.name, "nulldev");
 	dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM;
 	dev.putc = nulldev_putc;

Signature · 1 line

1.7.11.5

Joe Hershberger

author

2012-11-01 21:46 UTC #1 lore raw

PATCH 2/2 serial: Make nulldev a serial device

This allows the default console to be specified as the nulldev. This is specifically helpful when the real serial console's init() cannot run early in the boot process. When the init can be run, then the console can be switched to the real device using the std* env vars.

Signed-off-by Joe Hershberger <[email protected]>

---
 common/stdio.c          | 37 -----------------------------------
 drivers/serial/serial.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/serial.h        |  4 ++++
 3 files changed, 56 insertions(+), 37 deletions(-)

common/stdio.c +0 ~~-37~~

index e9bdc0e..37d36cb 100644
@@ -40,29 +40,6 @@ static struct stdio_dev devs;
 struct stdio_dev *stdio_devices[] = { NULL, NULL, NULL };
 char *stdio_names[MAX_FILES] = { "stdin", "stdout", "stderr" };
 
-#if defined(CONFIG_SPLASH_SCREEN) && !defined(CONFIG_SYS_DEVICE_NULLDEV)
-#define	CONFIG_SYS_DEVICE_NULLDEV	1
-#endif
-
-
-#ifdef CONFIG_SYS_DEVICE_NULLDEV
-void nulldev_putc(const char c)
-{
-	/* nulldev is empty! */
-}
-
-void nulldev_puts(const char *s)
-{
-	/* nulldev is empty! */
-}
-
-int nulldev_input(void)
-{
-	/* nulldev is empty! */
-	return 0;
-}
-#endif
-
 /**************************************************************************
  * SYSTEM DRIVERS
  **************************************************************************

@@ -70,20 +47,6 @@ int nulldev_input(void)
 
 static void drv_system_init (void)
 {
-#ifdef CONFIG_SYS_DEVICE_NULLDEV
-	struct stdio_dev dev;
-
-	memset (&dev, 0, sizeof (dev));
-
-	strcpy (dev.name, "nulldev");
-	dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM;
-	dev.putc = nulldev_putc;
-	dev.puts = nulldev_puts;
-	dev.getc = nulldev_input;
-	dev.tstc = nulldev_input;
-
-	stdio_register (&dev);
-#endif
 }
 
 /**************************************************************************

drivers/serial/serial.c +52 -0

64 lines not on this page · raw message

include/serial.h +4 -0

index 14f863e..3d404da 100644
@@ -22,6 +22,10 @@ struct serial_device {
 
 void default_serial_puts(const char *s);
 
+#if defined(CONFIG_SYS_DEVICE_NULLDEV)
+extern struct serial_device nulldev_serial_device;
+#endif
+
 extern struct serial_device serial_smc_device;
 extern struct serial_device serial_scc_device;
 extern struct serial_device *default_serial_console(void);

Signature · 1 line

1.7.11.5

Marek Vasut

2012-11-03 01:37 UTC #2 lore raw

PATCH 2/2 serial: Make nulldev a serial device

Dear Joe Hershberger,

Quoted 6 lines

This allows the default console to be specified as the nulldev. This is specifically helpful when the real serial console's init() cannot run early in the boot process. When the init can be run, then the console can be switched to the real device using the std* env vars.
Signed-off-by Joe Hershberger <[email protected]>

Isn't it actually better to have null stdio device? Some systems might not even use serial port (and so null serial will be useless on these systems)!

[...]

Best regards,
Marek Vasut

Joe Hershberger

2012-11-05 00:24 UTC #3 lore raw

PATCH 2/2 serial: Make nulldev a serial device

Hi Marek,

Quoted 7 lines · Marek Vasut

Dear Joe Hershberger,
Quoted 5 lines
This allows the default console to be specified as the nulldev. This is specifically helpful when the real serial console's init() cannot run early in the boot process. When the init can be run, then the console can be switched to the real device using the std* env vars.

Quoted 5 lines

Quoted 1 line
Signed-off-by Joe Hershberger <[email protected]>
Isn't it actually better to have null stdio device? Some systems might not even use serial port (and so null serial will be useless on these systems)!

As I described in my commit log, this is for the case where the serial and console init must not touch the hardware, (since it doesn't exist yet if it's in an FPGA or on a PCI or USB connection). Making the default serial port be the nulldev avoids this issue.

As for systems with no real serial port, there can still be the nulldev serial device... or not. Why is that a problem?

-Joe

Marek Vasut

2012-11-05 23:10 UTC #4 lore raw

PATCH 2/2 serial: Make nulldev a serial device

Dear Joe Hershberger,

Quoted 12 lines

Hi Marek,
Quoted 9 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 6 lines
This allows the default console to be specified as the nulldev. This is specifically helpful when the real serial console's init() cannot run early in the boot process. When the init can be run, then the console can be switched to the real device using the std* env vars.
Signed-off-by Joe Hershberger <[email protected]>

Quoted 8 lines

Quoted 3 lines
Isn't it actually better to have null stdio device? Some systems might not even use serial port (and so null serial will be useless on these systems)!
As I described in my commit log, this is for the case where the serial and console init must not touch the hardware, (since it doesn't exist yet if it's in an FPGA or on a PCI or USB connection). Making the default serial port be the nulldev avoids this issue.

So add nulldev serial device for this stupid case. Even though fixing iomux such that it'd not send anything to serial port at all if nulldev is selected would be even better idea.

That won't work?

Quoted 4 lines

As for systems with no real serial port, there can still be the nulldev serial device... or not. Why is that a problem?
-Joe

Best regards,
Marek Vasut

Joe Hershberger

2012-11-06 00:01 UTC #5 lore raw

PATCH 2/2 serial: Make nulldev a serial device

Hi Marek.

Quoted 18 lines · Marek Vasut

Dear Joe Hershberger,
Quoted 16 lines
Hi Marek,
Quoted 12 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 6 lines
This allows the default console to be specified as the nulldev. This is specifically helpful when the real serial console's init() cannot run early in the boot process. When the init can be run, then the console can be switched to the real device using the std* env vars.
Signed-off-by Joe Hershberger <[email protected]>
Isn't it actually better to have null stdio device? Some systems might not even use serial port (and so null serial will be useless on these systems)!

Quoted 8 lines

Quoted 4 lines
As I described in my commit log, this is for the case where the serial and console init must not touch the hardware, (since it doesn't exist yet if it's in an FPGA or on a PCI or USB connection). Making the default serial port be the nulldev avoids this issue.
So add nulldev serial device for this stupid case. Even though fixing iomux such that it'd not send anything to serial port at all if nulldev is selected would be even better idea.

It's not a problem of something being sent to the serial port. It's the call to serial_init() in arch/*/lib/board.c that kills it. That call needs to be no-op-able.

-Joe

Marek Vasut

2012-11-06 00:47 UTC #6 lore raw

PATCH 2/2 serial: Make nulldev a serial device

Dear Joe Hershberger,

Quoted 26 lines

Hi Marek.
Quoted 23 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 20 lines
Hi Marek,
Quoted 7 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 5 lines
This allows the default console to be specified as the nulldev. This is specifically helpful when the real serial console's init() cannot run early in the boot process. When the init can be run, then the console can be switched to the real device using the std* env vars.
Quoted 5 lines
Quoted 1 line
Signed-off-by Joe Hershberger <[email protected]>
Isn't it actually better to have null stdio device? Some systems might not even use serial port (and so null serial will be useless on these systems)!
As I described in my commit log, this is for the case where the serial and console init must not touch the hardware, (since it doesn't exist yet if it's in an FPGA or on a PCI or USB connection). Making the default serial port be the nulldev avoids this issue.

Quoted 7 lines

Quoted 3 lines
So add nulldev serial device for this stupid case. Even though fixing iomux such that it'd not send anything to serial port at all if nulldev is selected would be even better idea.
It's not a problem of something being sent to the serial port. It's the call to serial_init() in arch/*/lib/board.c that kills it. That call needs to be no-op-able.

Why?

What about fixing serial_init like this:

struct serial_device *dev = get_current();
int ret = 0;
if (dev)
ret = dev->start();

return ret;

Best regards,
Marek Vasut

Joe Hershberger

2012-11-06 01:00 UTC #7 lore raw

PATCH 2/2 serial: Make nulldev a serial device

Hi Marek,

Quoted 38 lines · Marek Vasut

Dear Joe Hershberger,
Quoted 26 lines
Hi Marek.
Quoted 23 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 20 lines
Hi Marek,
Quoted 7 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 5 lines
This allows the default console to be specified as the nulldev. This is specifically helpful when the real serial console's init() cannot run early in the boot process. When the init can be run, then the console can be switched to the real device using the std* env vars.
Quoted 5 lines
Quoted 1 line
Signed-off-by Joe Hershberger <[email protected]>
Isn't it actually better to have null stdio device? Some systems might not even use serial port (and so null serial will be useless on these systems)!
As I described in my commit log, this is for the case where the serial and console init must not touch the hardware, (since it doesn't exist yet if it's in an FPGA or on a PCI or USB connection). Making the default serial port be the nulldev avoids this issue.
Quoted 7 lines
Quoted 3 lines
So add nulldev serial device for this stupid case. Even though fixing iomux such that it'd not send anything to serial port at all if nulldev is selected would be even better idea.
It's not a problem of something being sent to the serial port. It's the call to serial_init() in arch/*/lib/board.c that kills it. That call needs to be no-op-able.
Why?

Quoted 8 lines

What about fixing serial_init like this:
struct serial_device *dev = get_current();
int ret = 0;
if (dev)
ret = dev->start();
return ret;

How does this help? Are you suggesting that default_serial_console() return "NULL"? It seems that you would then have to add a ton of checks for NULL coming back from default_serial_console() all the places that just use it assuming it's actually a device pointer. Sounds ugly. If you aren't talking about having default_serial_console() return NULL, then I don't see how this helps.

-Joe

Marek Vasut

2012-11-06 01:11 UTC #8 lore raw

PATCH 2/2 serial: Make nulldev a serial device

Dear Joe Hershberger,

Quoted 50 lines

Hi Marek,
Quoted 47 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 27 lines
Hi Marek.
Quoted 24 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 21 lines
Hi Marek,
Quoted 8 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 6 lines
This allows the default console to be specified as the nulldev. This is specifically helpful when the real serial console's init() cannot run early in the boot process. When the init can be run, then the console can be switched to the real device using the std* env vars.
Quoted 5 lines
Quoted 1 line
Signed-off-by Joe Hershberger <[email protected]>
Isn't it actually better to have null stdio device? Some systems might not even use serial port (and so null serial will be useless on these systems)!
As I described in my commit log, this is for the case where the serial and console init must not touch the hardware, (since it doesn't exist yet if it's in an FPGA or on a PCI or USB connection).
Making the default serial port be the nulldev avoids this issue.
Quoted 7 lines
Quoted 3 lines
So add nulldev serial device for this stupid case. Even though fixing iomux such that it'd not send anything to serial port at all if nulldev is selected would be even better idea.
It's not a problem of something being sent to the serial port. It's the call to serial_init() in arch/*/lib/board.c that kills it. That call needs to be no-op-able.
Why?
What about fixing serial_init like this:
struct serial_device *dev = get_current();
int ret = 0;
if (dev)
  ret = dev->start();

Quoted 8 lines

Quoted 1 line
return ret;
How does this help? Are you suggesting that default_serial_console() return "NULL"? It seems that you would then have to add a ton of checks for NULL coming back from default_serial_console() all the places that just use it assuming it's actually a device pointer. Sounds ugly. If you aren't talking about having default_serial_console() return NULL, then I don't see how this helps.

Well what will default_serial_console() return in your case? You said the serial_init() is the problematic spot.

Best regards,
Marek Vasut

Marek Vasut

2012-11-03 01:35 UTC #9 lore raw

PATCH 1/2 serial: Remove the "serial" console device

In reply to Joe Hershberger

Dear Joe Hershberger,

Quoted 11 lines

Each serial device is added as a console device. There was also the "serial" device that points to the most-recently-assigned-to-some- console-handle device. This can be confusing. Instead, only show the actual serial devices.
Signed-off-by Joe Hershberger <[email protected]>
---
 common/console.c | 37 +++++++++++++++++++------------------
 common/stdio.c   | 12 +-----------
 2 files changed, 20 insertions(+), 29 deletions(-)

[...]

You also remove the nulldev, shouldn't it go via separate patch? ... but otherwise it's good.

Best regards,
Marek Vasut

Joe Hershberger

2012-11-05 00:20 UTC #10 lore raw

PATCH 1/2 serial: Remove the "serial" console device

Hi Marek,

Quoted 13 lines · Marek Vasut

Dear Joe Hershberger,
Quoted 11 lines
Each serial device is added as a console device. There was also the "serial" device that points to the most-recently-assigned-to-some- console-handle device. This can be confusing. Instead, only show the actual serial devices.
Signed-off-by Joe Hershberger <[email protected]>
---
 common/console.c | 37 +++++++++++++++++++------------------
 common/stdio.c   | 12 +-----------
 2 files changed, 20 insertions(+), 29 deletions(-)

Quoted 4 lines

[...]
You also remove the nulldev, shouldn't it go via separate patch? ... but otherwise it's good.

I'm not sure what you mean. Where in this patch do I remove nulldev? That's the next patch.

Marek Vasut

2012-11-05 23:11 UTC #11 lore raw

PATCH 1/2 serial: Remove the "serial" console device

Dear Joe Hershberger,

Quoted 17 lines

Hi Marek,
Quoted 14 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 11 lines
Each serial device is added as a console device. There was also the "serial" device that points to the most-recently-assigned-to-some- console-handle device. This can be confusing. Instead, only show the actual serial devices.
Signed-off-by Joe Hershberger <[email protected]>
---
 common/console.c | 37 +++++++++++++++++++------------------
 common/stdio.c   | 12 +-----------
 2 files changed, 20 insertions(+), 29 deletions(-)

Quoted 7 lines

Quoted 4 lines
[...]
You also remove the nulldev, shouldn't it go via separate patch? ... but otherwise it's good.
I'm not sure what you mean. Where in this patch do I remove nulldev? That's the next patch.

The adjustment in common/stdio.c

Best regards,
Marek Vasut

Joe Hershberger

2012-11-05 23:45 UTC #12 lore raw

PATCH 1/2 serial: Remove the "serial" console device

Hi Marek,

Quoted 21 lines · Marek Vasut

Dear Joe Hershberger,
Quoted 19 lines
Hi Marek,
Quoted 16 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 11 lines
Each serial device is added as a console device. There was also the "serial" device that points to the most-recently-assigned-to-some- console-handle device. This can be confusing. Instead, only show the actual serial devices.
Signed-off-by Joe Hershberger <[email protected]>
---
 common/console.c | 37 +++++++++++++++++++------------------
 common/stdio.c   | 12 +-----------
 2 files changed, 20 insertions(+), 29 deletions(-)
[...]

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
You also remove the nulldev, shouldn't it go via separate patch? ... but otherwise it's good.
I'm not sure what you mean. Where in this patch do I remove nulldev? That's the next patch.
The adjustment in common/stdio.c

I think you are reading the diff wrong. The only nulldev related "change" is to not get warnings due to a (now) unused variable. It simply guards the definition of the variable with the NULLDEV #ifdef.

-Joe

Marek Vasut

2012-11-05 23:55 UTC #13 lore raw

PATCH 1/2 serial: Remove the "serial" console device

Dear Joe Hershberger,

Quoted 27 lines

Hi Marek,
Quoted 24 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 22 lines
Hi Marek,
Quoted 18 lines · Marek Vasut
Dear Joe Hershberger,
Quoted 11 lines
Each serial device is added as a console device. There was also the "serial" device that points to the most-recently-assigned-to-some- console-handle device. This can be confusing. Instead, only show the actual serial devices.
Signed-off-by Joe Hershberger <[email protected]>
---
 common/console.c | 37 +++++++++++++++++++------------------
 common/stdio.c   | 12 +-----------
 2 files changed, 20 insertions(+), 29 deletions(-)
[...]
You also remove the nulldev, shouldn't it go via separate patch? ... but otherwise it's good.

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
I'm not sure what you mean. Where in this patch do I remove nulldev? That's the next patch.
The adjustment in common/stdio.c
I think you are reading the diff wrong. The only nulldev related "change" is to not get warnings due to a (now) unused variable. It simply guards the definition of the variable with the NULLDEV #ifdef.

Best regards,
Marek Vasut