Patchwork [U-Boot,2/2] serial: Make nulldev a serial device

login
register
mail settings
Submitter Joe Hershberger
Date Nov. 1, 2012, 9:46 p.m.
Message ID <1351806388-27322-2-git-send-email-joe.hershberger@ni.com>
Download mbox | patch
Permalink /patch/196388/
State Under Review
Delegated to: Marek Vasut
Headers show

Comments

Joe Hershberger - Nov. 1, 2012, 9:46 p.m.
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 <joe.hershberger@ni.com>
---
 common/stdio.c          | 37 -----------------------------------
 drivers/serial/serial.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/serial.h        |  4 ++++
 3 files changed, 56 insertions(+), 37 deletions(-)
Marek Vasut - Nov. 3, 2012, 1:37 a.m.
Dear Joe Hershberger,

> 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 <joe.hershberger@ni.com>


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 - Nov. 5, 2012, 12:24 a.m.
Hi Marek,

On Fri, Nov 2, 2012 at 8:37 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Joe Hershberger,
>
>> 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 <joe.hershberger@ni.com>
>
>
> 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 - Nov. 5, 2012, 11:10 p.m.
Dear Joe Hershberger,

> Hi Marek,
> 
> On Fri, Nov 2, 2012 at 8:37 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Joe Hershberger,
> > 
> >> 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 <joe.hershberger@ni.com>
> > 
> > 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?

> 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 - Nov. 6, 2012, 12:01 a.m.
Hi Marek.

On Mon, Nov 5, 2012 at 5:10 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Joe Hershberger,
>
>> Hi Marek,
>>
>> On Fri, Nov 2, 2012 at 8:37 PM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Joe Hershberger,
>> >
>> >> 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 <joe.hershberger@ni.com>
>> >
>> > 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.

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 - Nov. 6, 2012, 12:47 a.m.
Dear Joe Hershberger,

> Hi Marek.
> 
> On Mon, Nov 5, 2012 at 5:10 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Joe Hershberger,
> > 
> >> Hi Marek,
> >> 
> >> On Fri, Nov 2, 2012 at 8:37 PM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Joe Hershberger,
> >> > 
> >> >> 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 <joe.hershberger@ni.com>
> >> > 
> >> > 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.
> 
> 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 - Nov. 6, 2012, 1 a.m.
Hi Marek,

On Mon, Nov 5, 2012 at 6:47 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Joe Hershberger,
>
>> Hi Marek.
>>
>> On Mon, Nov 5, 2012 at 5:10 PM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Joe Hershberger,
>> >
>> >> Hi Marek,
>> >>
>> >> On Fri, Nov 2, 2012 at 8:37 PM, Marek Vasut <marex@denx.de> wrote:
>> >> > Dear Joe Hershberger,
>> >> >
>> >> >> 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 <joe.hershberger@ni.com>
>> >> >
>> >> > 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.
>>
>> 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;

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 - Nov. 6, 2012, 1:11 a.m.
Dear Joe Hershberger,

> Hi Marek,
> 
> On Mon, Nov 5, 2012 at 6:47 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Joe Hershberger,
> > 
> >> Hi Marek.
> >> 
> >> On Mon, Nov 5, 2012 at 5:10 PM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Joe Hershberger,
> >> > 
> >> >> Hi Marek,
> >> >> 
> >> >> On Fri, Nov 2, 2012 at 8:37 PM, Marek Vasut <marex@denx.de> wrote:
> >> >> > Dear Joe Hershberger,
> >> >> > 
> >> >> >> 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 <joe.hershberger@ni.com>
> >> >> > 
> >> >> > 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.
> >> 
> >> 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;
> 
> 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

Patch

diff --git a/common/stdio.c b/common/stdio.c
index e9bdc0e..37d36cb 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -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
 }
 
 /**************************************************************************
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c
index f5f43a6..5031b00 100644
--- a/drivers/serial/serial.c
+++ b/drivers/serial/serial.c
@@ -110,6 +110,57 @@  serial_initfunc(s3c44b0_serial_initialize);
 serial_initfunc(sa1100_serial_initialize);
 serial_initfunc(sh_serial_initialize);
 
+#if defined(CONFIG_SPLASH_SCREEN) && !defined(CONFIG_SYS_DEVICE_NULLDEV)
+#define	CONFIG_SYS_DEVICE_NULLDEV
+#endif
+
+#if defined(CONFIG_SYS_DEVICE_NULLDEV)
+int nulldev_init(void)
+{
+	/* nulldev is empty! */
+	return 0;
+}
+
+void nulldev_setbrg(void)
+{
+	/* nulldev is empty! */
+}
+
+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;
+}
+
+struct serial_device nulldev_serial_device = {
+	"nulldev",
+	nulldev_init,
+	NULL,
+	nulldev_setbrg,
+	nulldev_input,
+	nulldev_input,
+	nulldev_putc,
+	nulldev_puts,
+};
+
+void nulldev_serial_initalize(void)
+{
+	serial_register(&nulldev_serial_device);
+}
+#else
+serial_initfunc(nulldev_serial_initalize);
+#endif
+
 /**
  * serial_register() - Register serial driver with serial driver core
  * @dev:	Pointer to the serial driver structure
@@ -154,6 +205,7 @@  void serial_register(struct serial_device *dev)
  */
 void serial_initialize(void)
 {
+	nulldev_serial_initalize();
 	mpc8xx_serial_initialize();
 	ns16550_serial_initialize();
 	pxa_serial_initialize();
diff --git a/include/serial.h b/include/serial.h
index 14f863e..3d404da 100644
--- a/include/serial.h
+++ b/include/serial.h
@@ -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);