[PATCH 0/6] stdio: Clean up stdio.c

[PATCH 0/6] stdio: Clean up stdio.c

[Simon Glass]

This file is quite old and has some crufty code. This series converts a
few things to Kconfig and updates the file.


Simon Glass (6):
  Convert CONFIG_SYS_DEVICE_NULLDEV to Kconfig
  stdio: Tidy up use of CONFIG_SYS_DEVICE_NULLDEV
  stdio: Drop #ifdefs in the header file
  stdio: Update to use compiler for Kconfig checks
  stdio: Drop brackets around &devs.list
  stdio: Tidy up the coding style

 common/Kconfig                   |  17 +++
 common/stdio.c                   | 251 ++++++++++++++-----------------
 configs/M5249EVB_defconfig       |   3 +-
 configs/colibri_pxa270_defconfig |   2 +-
 doc/README.silent                |   2 +-
 include/configs/M5249EVB.h       |   2 -
 include/configs/colibri_pxa270.h |   2 -
 include/stdio_dev.h              |  49 +++---
 scripts/config_whitelist.txt     |   1 -
 9 files changed, 157 insertions(+), 172 deletions(-)

-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH 1/6] Convert CONFIG_SYS_DEVICE_NULLDEV to Kconfig

[Simon Glass]

This converts the following to Kconfig:
   CONFIG_SYS_DEVICE_NULLDEV

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/Kconfig                   | 16 ++++++++++++++++
 configs/M5249EVB_defconfig       |  3 ++-
 configs/colibri_pxa270_defconfig |  2 +-
 doc/README.silent                |  2 +-
 include/configs/M5249EVB.h       |  2 --
 include/configs/colibri_pxa270.h |  2 --
 scripts/config_whitelist.txt     |  1 -
 7 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 67b3818fde..4d5b3a9cfb 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -634,6 +634,22 @@ config SYS_STDIO_DEREGISTER
 	  removed (for example a USB keyboard) then this option can be
 	  enabled to ensure this is handled correctly.
 
+config SPL_SYS_STDIO_DEREGISTER
+	bool "Allow deregistering stdio devices in SPL"
+	help
+	  Generally there is no need to deregister stdio devices since they
+	  are never deactivated. But if a stdio device is used which can be
+	  removed (for example a USB keyboard) then this option can be
+	  enabled to ensure this is handled correctly. This is very rarely
+	  needed in SPL.
+
+config SYS_DEVICE_NULLDEV
+	bool "Enable a null device for stdio"
+	help
+	  Enable creation of a "nulldev" stdio device. This allows silent
+	  operation of the console by setting stdout to "nulldev". Enable
+	  this to use a serial console under board control.
+
 endmenu
 
 menu "Logging"
diff --git a/configs/M5249EVB_defconfig b/configs/M5249EVB_defconfig
index 12db389b69..8f8a4a6bad 100644
--- a/configs/M5249EVB_defconfig
+++ b/configs/M5249EVB_defconfig
@@ -3,7 +3,9 @@ CONFIG_SYS_TEXT_BASE=0xFFE00000
 CONFIG_ENV_SIZE=0x2000
 CONFIG_ENV_SECT_SIZE=0x2000
 CONFIG_TARGET_M5249EVB=y
+CONFIG_DEFAULT_DEVICE_TREE="M5249EVB"
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
+CONFIG_SYS_DEVICE_NULLDEV=y
 # CONFIG_DISPLAY_BOARDINFO is not set
 # CONFIG_CMDLINE_EDITING is not set
 # CONFIG_AUTOBOOT is not set
@@ -12,7 +14,6 @@ CONFIG_LOOPW=y
 CONFIG_CMD_MX_CYCLIC=y
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_CMD_CACHE=y
-CONFIG_DEFAULT_DEVICE_TREE="M5249EVB"
 CONFIG_ENV_ADDR=0xFFE04000
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 # CONFIG_NET is not set
diff --git a/configs/colibri_pxa270_defconfig b/configs/colibri_pxa270_defconfig
index 669b9dfe58..aff7b62639 100644
--- a/configs/colibri_pxa270_defconfig
+++ b/configs/colibri_pxa270_defconfig
@@ -10,6 +10,7 @@ CONFIG_NR_DRAM_BANKS=1
 CONFIG_ENV_VARS_UBOOT_CONFIG=y
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=tty0 console=ttyS0,115200"
+CONFIG_SYS_DEVICE_NULLDEV=y
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 # CONFIG_CMDLINE_EDITING is not set
@@ -43,6 +44,5 @@ CONFIG_SYS_FLASH_CFI=y
 CONFIG_DM_SERIAL=y
 CONFIG_PXA_SERIAL=y
 CONFIG_USB=y
-CONFIG_USB_STORAGE=y
 # CONFIG_REGEX is not set
 CONFIG_OF_LIBFDT=y
diff --git a/doc/README.silent b/doc/README.silent
index 6d90a0ec40..00288e03b0 100644
--- a/doc/README.silent
+++ b/doc/README.silent
@@ -19,7 +19,7 @@ The following actions are taken if "silent" is set at boot time:
  - When the console devices have been initialized, "stdout" and
    "stderr" are set to "nulldev", so subsequent messages are
    suppressed automatically. Make sure to enable "nulldev" by
-   #defining CONFIG_SYS_DEVICE_NULLDEV in your board config file.
+   enabling CONFIG_SYS_DEVICE_NULLDEV in your board defconfig file.
 
  - When booting a linux kernel, the "bootargs" are fixed up so that
    the argument "console=" will be in the command line, no matter how
diff --git a/include/configs/M5249EVB.h b/include/configs/M5249EVB.h
index de7132940c..1a1a110765 100644
--- a/include/configs/M5249EVB.h
+++ b/include/configs/M5249EVB.h
@@ -31,8 +31,6 @@
  */
 #undef CONFIG_BOOTP_BOOTFILESIZE
 
-#define CONFIG_SYS_DEVICE_NULLDEV	1	/* include nulldev device	*/
-
 #define CONFIG_SYS_LOAD_ADDR		0x200000	/* default load address */
 
 /*
diff --git a/include/configs/colibri_pxa270.h b/include/configs/colibri_pxa270.h
index 29827f1ee8..3bbef55ec3 100644
--- a/include/configs/colibri_pxa270.h
+++ b/include/configs/colibri_pxa270.h
@@ -70,8 +70,6 @@
 #define	CONFIG_BOOTP_BOOTFILESIZE
 #endif
 
-#define	CONFIG_SYS_DEVICE_NULLDEV	1
-
 /*
  * Clock Configuration
  */
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index 1c7946fb65..2da34a5d23 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -2178,7 +2178,6 @@ CONFIG_SYS_DEBUG_SERVER_FW_IN_NOR
 CONFIG_SYS_DEFAULT_LPDDR2_TIMINGS
 CONFIG_SYS_DEFAULT_VIDEO_MODE
 CONFIG_SYS_DEF_EEPROM_ADDR
-CONFIG_SYS_DEVICE_NULLDEV
 CONFIG_SYS_DFU_DATA_BUF_SIZE
 CONFIG_SYS_DFU_MAX_FILE_SIZE
 CONFIG_SYS_DIAG_ADDR
-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH 2/6] stdio: Tidy up use of CONFIG_SYS_DEVICE_NULLDEV

[Simon Glass]

Now that this is in Kconfig we can move the logic at the top of the file
to Kconfig, and use if() instead of #if. Update the file with these
changes.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/Kconfig |  1 +
 common/stdio.c | 30 ++++++++++--------------------
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 4d5b3a9cfb..c9abd400ee 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -645,6 +645,7 @@ config SPL_SYS_STDIO_DEREGISTER
 
 config SYS_DEVICE_NULLDEV
 	bool "Enable a null device for stdio"
+	default y if SPLASH_SCREEN || SYS_STDIO_DEREGISTER
 	help
 	  Enable creation of a "nulldev" stdio device. This allows silent
 	  operation of the console by setting stdout to "nulldev". Enable
diff --git a/common/stdio.c b/common/stdio.c
index 2119204b98..33a795e7bb 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -31,15 +31,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
-
-#if CONFIG_IS_ENABLED(SYS_STDIO_DEREGISTER)
-#define	CONFIG_SYS_DEVICE_NULLDEV	1
-#endif
-
-#ifdef CONFIG_SYS_DEVICE_NULLDEV
 static void nulldev_putc(struct stdio_dev *dev, const char c)
 {
 	/* nulldev is empty! */
@@ -55,7 +46,6 @@ static int nulldev_input(struct stdio_dev *dev)
 	/* nulldev is empty! */
 	return 0;
 }
-#endif
 
 static void stdio_serial_putc(struct stdio_dev *dev, const char c)
 {
@@ -96,18 +86,18 @@ static void drv_system_init (void)
 	dev.tstc = stdio_serial_tstc;
 	stdio_register (&dev);
 
-#ifdef CONFIG_SYS_DEVICE_NULLDEV
-	memset (&dev, 0, sizeof (dev));
+	if (CONFIG_IS_ENABLED(SYS_DEVICE_NULLDEV)) {
+		memset(&dev, '\0', sizeof(dev));
 
-	strcpy (dev.name, "nulldev");
-	dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT;
-	dev.putc = nulldev_putc;
-	dev.puts = nulldev_puts;
-	dev.getc = nulldev_input;
-	dev.tstc = nulldev_input;
+		strcpy(dev.name, "nulldev");
+		dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT;
+		dev.putc = nulldev_putc;
+		dev.puts = nulldev_puts;
+		dev.getc = nulldev_input;
+		dev.tstc = nulldev_input;
 
-	stdio_register (&dev);
-#endif
+		stdio_register(&dev);
+	}
 }
 
 /**************************************************************************
-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH 3/6] stdio: Drop #ifdefs in the header file

[Simon Glass]

These prevent the use of IS_ENABLED() and are unnecessary. Drop them and
fix a few code-style nits nearby.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/stdio_dev.h | 40 ++++++++++++----------------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/include/stdio_dev.h b/include/stdio_dev.h
index cd0cd601bf..b61c0c6cef 100644
--- a/include/stdio_dev.h
+++ b/include/stdio_dev.h
@@ -57,7 +57,7 @@ extern char *stdio_names[MAX_FILES];
 /*
  * PROTOTYPES
  */
-int	stdio_register (struct stdio_dev * dev);
+int stdio_register(struct stdio_dev *dev);
 int stdio_register_dev(struct stdio_dev *dev, struct stdio_dev **devp);
 
 /**
@@ -82,35 +82,19 @@ int stdio_add_devices(void);
  */
 int stdio_init(void);
 
-void	stdio_print_current_devices(void);
-#if CONFIG_IS_ENABLED(SYS_STDIO_DEREGISTER)
+void stdio_print_current_devices(void);
 int stdio_deregister(const char *devname, int force);
 int stdio_deregister_dev(struct stdio_dev *dev, int force);
-#endif
-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_LCD
-int	drv_lcd_init (void);
-#endif
-#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
-int	drv_video_init (void);
-#endif
-#ifdef CONFIG_KEYBOARD
-int	drv_keyboard_init (void);
-#endif
-#ifdef CONFIG_USB_TTY
-int	drv_usbtty_init (void);
-#endif
-#ifdef CONFIG_NETCONSOLE
-int	drv_nc_init (void);
-#endif
-#ifdef CONFIG_JTAG_CONSOLE
-int drv_jtag_console_init (void);
-#endif
-#ifdef CONFIG_CBMEM_CONSOLE
+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);
+
+int drv_lcd_init(void);
+int drv_video_init(void);
+int drv_keyboard_init(void);
+int drv_usbtty_init(void);
+int drv_nc_init(void);
+int drv_jtag_console_init(void);
 int cbmemc_init(void);
-#endif
 
 #endif
-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH 4/6] stdio: Update to use compiler for Kconfig checks

[Simon Glass]

Drop use of the preprocessor where possible.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/stdio.c | 172 ++++++++++++++++++++++++-------------------------
 1 file changed, 83 insertions(+), 89 deletions(-)

diff --git a/common/stdio.c b/common/stdio.c
index 33a795e7bb..1921dc6719 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -18,10 +18,7 @@
 #include <stdio_dev.h>
 #include <serial.h>
 #include <splash.h>
-
-#if defined(CONFIG_SYS_I2C)
 #include <i2c.h>
-#endif
 
 #include <dm/device-internal.h>
 
@@ -109,7 +106,6 @@ struct list_head* stdio_get_list(void)
 	return &(devs.list);
 }
 
-#ifdef CONFIG_DM_VIDEO
 /**
  * stdio_probe_device() - Find a device which provides the given stdio device
  *
@@ -160,7 +156,6 @@ static int stdio_probe_device(const char *name, enum uclass_id id,
 
 	return 0;
 }
-#endif
 
 struct stdio_dev *stdio_get_by_name(const char *name)
 {
@@ -175,22 +170,23 @@ struct stdio_dev *stdio_get_by_name(const char *name)
 		if (strcmp(sdev->name, name) == 0)
 			return sdev;
 	}
-#ifdef CONFIG_DM_VIDEO
-	/*
-	 * We did not find a suitable stdio device. If there is a video
-	 * driver with a name starting with 'vidconsole', we can try probing
-	 * that in the hope that it will produce the required stdio device.
-	 *
-	 * This function is sometimes called with the entire value of
-	 * 'stdout', which may include a list of devices separate by commas.
-	 * Obviously this is not going to work, so we ignore that case. The
-	 * call path in that case is console_init_r() -> search_device() ->
-	 * stdio_get_by_name().
-	 */
-	if (!strncmp(name, "vidconsole", 10) && !strchr(name, ',') &&
-	    !stdio_probe_device(name, UCLASS_VIDEO, &sdev))
-		return sdev;
-#endif
+	if (IS_ENABLED(CONFIG_DM_VIDEO)) {
+		/*
+		 * We did not find a suitable stdio device. If there is a video
+		 * driver with a name starting with 'vidconsole', we can try
+		 * probing that in the hope that it will produce the required
+		 * stdio device.
+		 *
+		 * This function is sometimes called with the entire value of
+		 * 'stdout', which may include a list of devices separate by
+		 * commas. Obviously this is not going to work, so we ignore
+		 * that case. The call path in that case is
+		 * console_init_r() -> search_device() -> stdio_get_by_name()
+		 */
+		if (!strncmp(name, "vidconsole", 10) && !strchr(name, ',') &&
+		    !stdio_probe_device(name, UCLASS_VIDEO, &sdev))
+			return sdev;
+	}
 
 	return NULL;
 }
@@ -234,7 +230,6 @@ int stdio_register(struct stdio_dev *dev)
 /* deregister the device "devname".
  * returns 0 if success, -1 if device is assigned and 1 if devname not found
  */
-#if CONFIG_IS_ENABLED(SYS_STDIO_DEREGISTER)
 int stdio_deregister_dev(struct stdio_dev *dev, int force)
 {
 	int l;
@@ -281,7 +276,6 @@ int stdio_deregister(const char *devname, int force)
 
 	return stdio_deregister_dev(dev, force);
 }
-#endif /* CONFIG_IS_ENABLED(SYS_STDIO_DEREGISTER) */
 
 int stdio_init_tables(void)
 {
@@ -305,87 +299,87 @@ int stdio_init_tables(void)
 
 int stdio_add_devices(void)
 {
-#ifdef CONFIG_DM_KEYBOARD
 	struct udevice *dev;
 	struct uclass *uc;
 	int ret;
 
-	/*
-	 * For now we probe all the devices here. At some point this should be
-	 * done only when the devices are required - e.g. we have a list of
-	 * input devices to start up in the stdin environment variable. That
-	 * work probably makes more sense when stdio itself is converted to
-	 * driver model.
-	 *
-	 * TODO(sjg at chromium.org): Convert changing uclass_first_device() etc.
-	 * to return the device even on error. Then we could use that here.
-	 */
-	ret = uclass_get(UCLASS_KEYBOARD, &uc);
-	if (ret)
-		return ret;
-
-	/* Don't report errors to the caller - assume that they are non-fatal */
-	uclass_foreach_dev(dev, uc) {
-		ret = device_probe(dev);
+	if (IS_ENABLED(CONFIG_DM_KEYBOARD)) {
+		/*
+		 * For now we probe all the devices here. At some point this
+		 * should be done only when the devices are required - e.g. we
+		 * have a list of input devices to start up in the stdin
+		 * environment variable. That work probably makes more sense
+		 * when stdio itself is converted to driver model.
+		 *
+		 * TODO(sjg at chromium.org): Convert changing
+		 * uclass_first_device() etc. to return the device even on
+		 * error. Then we could use that here.
+		 */
+		ret = uclass_get(UCLASS_KEYBOARD, &uc);
 		if (ret)
-			printf("Failed to probe keyboard '%s'\n", dev->name);
+			return ret;
+
+		/*
+		 * Don't report errors to the caller - assume that they are
+		 * non-fatal
+		 */
+		uclass_foreach_dev(dev, uc) {
+			ret = device_probe(dev);
+			if (ret)
+				printf("Failed to probe keyboard '%s'\n",
+				       dev->name);
+		}
 	}
-#endif
 #ifdef CONFIG_SYS_I2C
 	i2c_init_all();
-#else
 #endif
-#ifdef CONFIG_DM_VIDEO
-	/*
-	 * If the console setting is not in environment variables then
-	 * console_init_r() will not be calling iomux_doenv() (which calls
-	 * search_device()). So we will not dynamically add devices by
-	 * calling stdio_probe_device().
-	 *
-	 * So just probe all video devices now so that whichever one is
-	 * required will be available.
-	 */
-#ifndef CONFIG_SYS_CONSOLE_IS_IN_ENV
-	struct udevice *vdev;
-# ifndef CONFIG_DM_KEYBOARD
-	int ret;
-# endif
-
-	for (ret = uclass_first_device(UCLASS_VIDEO, &vdev);
-	     vdev;
-	     ret = uclass_next_device(&vdev))
-		;
-	if (ret)
-		printf("%s: Video device failed (ret=%d)\n", __func__, ret);
-#endif /* !CONFIG_SYS_CONSOLE_IS_IN_ENV */
-#if defined(CONFIG_SPLASH_SCREEN) && defined(CONFIG_CMD_BMP)
-	splash_display();
-#endif /* CONFIG_SPLASH_SCREEN && CONFIG_CMD_BMP */
-#else
-# if defined(CONFIG_LCD)
-	drv_lcd_init ();
-# endif
-# if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
-	drv_video_init ();
-# endif
-#endif /* CONFIG_DM_VIDEO */
+	if (IS_ENABLED(CONFIG_DM_VIDEO)) {
+		/*
+		 * If the console setting is not in environment variables then
+		 * console_init_r() will not be calling iomux_doenv() (which
+		 * calls search_device()). So we will not dynamically add
+		 * devices by calling stdio_probe_device().
+		 *
+		 * So just probe all video devices now so that whichever one is
+		 * required will be available.
+		 */
+		struct udevice *vdev;
+		int ret;
+
+		if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
+			for (ret = uclass_first_device(UCLASS_VIDEO, &vdev);
+			     vdev;
+			     ret = uclass_next_device(&vdev))
+				;
+			if (ret)
+				printf("%s: Video device failed (ret=%d)\n",
+				       __func__, ret);
+		}
+		if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
+		    IS_ENABLED(CONFIG_CMD_BMP))
+			splash_display();
+	} else {
+		if (IS_ENABLED(CONFIG_LCD))
+			drv_lcd_init();
+		if (IS_ENABLED(CONFIG_VIDEO) || IS_ENABLED(CONFIG_CFB_CONSOLE))
+			drv_video_init();
+	}
+
 #if defined(CONFIG_KEYBOARD) && !defined(CONFIG_DM_KEYBOARD)
-	drv_keyboard_init ();
+	drv_keyboard_init();
 #endif
-	drv_system_init ();
-	serial_stdio_init ();
+	drv_system_init();
+	serial_stdio_init();
 #ifdef CONFIG_USB_TTY
-	drv_usbtty_init ();
-#endif
-#ifdef CONFIG_NETCONSOLE
-	drv_nc_init ();
+	drv_usbtty_init();
 #endif
+	if (IS_ENABLED(CONFIG_NETCONSOLE))
+		drv_nc_init();
 #ifdef CONFIG_JTAG_CONSOLE
-	drv_jtag_console_init ();
-#endif
-#ifdef CONFIG_CBMEM_CONSOLE
-	cbmemc_init();
+	drv_jtag_console_init();
 #endif
+	if (IS_ENABLED(CONFIG_CBMEM_CONSOLE))
+		cbmemc_init();
 
 	return 0;
 }
-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH 5/6] stdio: Drop brackets around &devs.list

[Simon Glass]

These brackets are not needed. Drop them.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/stdio.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/common/stdio.c b/common/stdio.c
index 1921dc6719..d67ea60d6c 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -103,7 +103,7 @@ static void drv_system_init (void)
  */
 struct list_head* stdio_get_list(void)
 {
-	return &(devs.list);
+	return &devs.list;
 }
 
 /**
@@ -165,7 +165,7 @@ struct stdio_dev *stdio_get_by_name(const char *name)
 	if (!name)
 		return NULL;
 
-	list_for_each(pos, &(devs.list)) {
+	list_for_each(pos, &devs.list) {
 		sdev = list_entry(pos, struct stdio_dev, list);
 		if (strcmp(sdev->name, name) == 0)
 			return sdev;
@@ -215,7 +215,7 @@ int stdio_register_dev(struct stdio_dev *dev, struct stdio_dev **devp)
 	_dev = stdio_clone(dev);
 	if(!_dev)
 		return -ENODEV;
-	list_add_tail(&(_dev->list), &(devs.list));
+	list_add_tail(&_dev->list, &devs.list);
 	if (devp)
 		*devp = _dev;
 
@@ -251,11 +251,11 @@ int stdio_deregister_dev(struct stdio_dev *dev, int force)
 			sizeof(temp_names[l]));
 	}
 
-	list_del(&(dev->list));
+	list_del(&dev->list);
 	free(dev);
 
 	/* reassign Device list */
-	list_for_each(pos, &(devs.list)) {
+	list_for_each(pos, &devs.list) {
 		dev = list_entry(pos, struct stdio_dev, list);
 		for (l=0 ; l< MAX_FILES; l++) {
 			if(strcmp(dev->name, temp_names[l]) == 0)
@@ -292,7 +292,7 @@ int stdio_init_tables(void)
 #endif /* CONFIG_NEEDS_MANUAL_RELOC */
 
 	/* Initialize the list */
-	INIT_LIST_HEAD(&(devs.list));
+	INIT_LIST_HEAD(&devs.list);
 
 	return 0;
 }
-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH 6/6] stdio: Tidy up the coding style

[Simon Glass]

Bring the coding style in this file up to the current level.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/stdio.c      | 37 ++++++++++++++++---------------------
 include/stdio_dev.h |  9 +++++++++
 2 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/common/stdio.c b/common/stdio.c
index d67ea60d6c..84c36a735c 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -191,16 +191,15 @@ struct stdio_dev *stdio_get_by_name(const char *name)
 	return NULL;
 }
 
-struct stdio_dev* stdio_clone(struct stdio_dev *dev)
+struct stdio_dev *stdio_clone(struct stdio_dev *dev)
 {
 	struct stdio_dev *_dev;
 
-	if(!dev)
+	if (!dev)
 		return NULL;
 
 	_dev = calloc(1, sizeof(struct stdio_dev));
-
-	if(!_dev)
+	if (!_dev)
 		return NULL;
 
 	memcpy(_dev, dev, sizeof(struct stdio_dev));
@@ -213,7 +212,7 @@ int stdio_register_dev(struct stdio_dev *dev, struct stdio_dev **devp)
 	struct stdio_dev *_dev;
 
 	_dev = stdio_clone(dev);
-	if(!_dev)
+	if (!_dev)
 		return -ENODEV;
 	list_add_tail(&_dev->list, &devs.list);
 	if (devp)
@@ -227,41 +226,38 @@ int stdio_register(struct stdio_dev *dev)
 	return stdio_register_dev(dev, NULL);
 }
 
-/* deregister the device "devname".
- * returns 0 if success, -1 if device is assigned and 1 if devname not found
- */
 int stdio_deregister_dev(struct stdio_dev *dev, int force)
 {
-	int l;
 	struct list_head *pos;
 	char temp_names[3][16];
+	int i;
 
 	/* get stdio devices (ListRemoveItem changes the dev list) */
-	for (l=0 ; l< MAX_FILES; l++) {
-		if (stdio_devices[l] == dev) {
+	for (i = 0 ; i < MAX_FILES; i++) {
+		if (stdio_devices[i] == dev) {
 			if (force) {
-				strcpy(temp_names[l], "nulldev");
+				strcpy(temp_names[i], "nulldev");
 				continue;
 			}
 			/* Device is assigned -> report error */
-			return -1;
+			return -EBUSY;
 		}
-		memcpy (&temp_names[l][0],
-			stdio_devices[l]->name,
-			sizeof(temp_names[l]));
+		memcpy(&temp_names[i][0], stdio_devices[i]->name,
+		       sizeof(temp_names[i]));
 	}
 
 	list_del(&dev->list);
 	free(dev);
 
-	/* reassign Device list */
+	/* reassign device list */
 	list_for_each(pos, &devs.list) {
 		dev = list_entry(pos, struct stdio_dev, list);
-		for (l=0 ; l< MAX_FILES; l++) {
-			if(strcmp(dev->name, temp_names[l]) == 0)
-				stdio_devices[l] = dev;
+		for (i = 0 ; i < MAX_FILES; i++) {
+			if (strcmp(dev->name, temp_names[i]) == 0)
+				stdio_devices[i] = dev;
 		}
 	}
+
 	return 0;
 }
 
@@ -270,7 +266,6 @@ int stdio_deregister(const char *devname, int force)
 	struct stdio_dev *dev;
 
 	dev = stdio_get_by_name(devname);
-
 	if (!dev) /* device not found */
 		return -ENODEV;
 
diff --git a/include/stdio_dev.h b/include/stdio_dev.h
index b61c0c6cef..48871a6a22 100644
--- a/include/stdio_dev.h
+++ b/include/stdio_dev.h
@@ -84,6 +84,15 @@ int stdio_init(void);
 
 void stdio_print_current_devices(void);
 int stdio_deregister(const char *devname, int force);
+
+/**
+ * stdio_deregister_dev() - deregister the device "devname".
+ *
+ * @dev: Stdio device to deregister
+ * @force: true to force deregistration even if in use
+ *
+ * returns 0 on success, -EBUSY if device is assigned
+ */
 int stdio_deregister_dev(struct stdio_dev *dev, int force);
 struct list_head *stdio_get_list(void);
 struct stdio_dev *stdio_get_by_name(const char *name);
-- 
2.28.0.rc0.105.gf9edc3c819-goog

[PATCH 1/6] Convert CONFIG_SYS_DEVICE_NULLDEV to Kconfig

[Ovidiu Panait]

Hi Simon,

On 18.07.2020 06:03, Simon Glass wrote:
> This converts the following to Kconfig:
>     CONFIG_SYS_DEVICE_NULLDEV
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   common/Kconfig                   | 16 ++++++++++++++++
>   configs/M5249EVB_defconfig       |  3 ++-
>   configs/colibri_pxa270_defconfig |  2 +-
>   doc/README.silent                |  2 +-
>   include/configs/M5249EVB.h       |  2 --
>   include/configs/colibri_pxa270.h |  2 --
>   scripts/config_whitelist.txt     |  1 -
>   7 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 67b3818fde..4d5b3a9cfb 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -634,6 +634,22 @@ config SYS_STDIO_DEREGISTER
>   	  removed (for example a USB keyboard) then this option can be
>   	  enabled to ensure this is handled correctly.
>   
> +config SPL_SYS_STDIO_DEREGISTER
> +	bool "Allow deregistering stdio devices in SPL"
> +	help
> +	  Generally there is no need to deregister stdio devices since they
> +	  are never deactivated. But if a stdio device is used which can be
> +	  removed (for example a USB keyboard) then this option can be
> +	  enabled to ensure this is handled correctly. This is very rarely
> +	  needed in SPL.
> +
> +config SYS_DEVICE_NULLDEV
> +	bool "Enable a null device for stdio"
> +	help
> +	  Enable creation of a "nulldev" stdio device. This allows silent
> +	  operation of the console by setting stdout to "nulldev". Enable
> +	  this to use a serial console under board control.
> +
>   endmenu
>   
>   menu "Logging"
> diff --git a/configs/M5249EVB_defconfig b/configs/M5249EVB_defconfig
> index 12db389b69..8f8a4a6bad 100644
> --- a/configs/M5249EVB_defconfig
> +++ b/configs/M5249EVB_defconfig
> @@ -3,7 +3,9 @@ CONFIG_SYS_TEXT_BASE=0xFFE00000
>   CONFIG_ENV_SIZE=0x2000
>   CONFIG_ENV_SECT_SIZE=0x2000
>   CONFIG_TARGET_M5249EVB=y
> +CONFIG_DEFAULT_DEVICE_TREE="M5249EVB"
>   CONFIG_SYS_CONSOLE_INFO_QUIET=y
> +CONFIG_SYS_DEVICE_NULLDEV=y
>   # CONFIG_DISPLAY_BOARDINFO is not set
>   # CONFIG_CMDLINE_EDITING is not set
>   # CONFIG_AUTOBOOT is not set
> @@ -12,7 +14,6 @@ CONFIG_LOOPW=y
>   CONFIG_CMD_MX_CYCLIC=y
>   # CONFIG_CMD_SETEXPR is not set
>   CONFIG_CMD_CACHE=y
> -CONFIG_DEFAULT_DEVICE_TREE="M5249EVB"
>   CONFIG_ENV_ADDR=0xFFE04000
>   CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>   # CONFIG_NET is not set
> diff --git a/configs/colibri_pxa270_defconfig b/configs/colibri_pxa270_defconfig
> index 669b9dfe58..aff7b62639 100644
> --- a/configs/colibri_pxa270_defconfig
> +++ b/configs/colibri_pxa270_defconfig
> @@ -10,6 +10,7 @@ CONFIG_NR_DRAM_BANKS=1
>   CONFIG_ENV_VARS_UBOOT_CONFIG=y
>   CONFIG_USE_BOOTARGS=y
>   CONFIG_BOOTARGS="console=tty0 console=ttyS0,115200"
> +CONFIG_SYS_DEVICE_NULLDEV=y
>   # CONFIG_DISPLAY_BOARDINFO is not set
>   CONFIG_DISPLAY_BOARDINFO_LATE=y
>   # CONFIG_CMDLINE_EDITING is not set
> @@ -43,6 +44,5 @@ CONFIG_SYS_FLASH_CFI=y
>   CONFIG_DM_SERIAL=y
>   CONFIG_PXA_SERIAL=y
>   CONFIG_USB=y
> -CONFIG_USB_STORAGE=y

Is this deletion intentional? It is not immediately obvious to me how it 
relates to

CONFIG_SYS_DEVICE_NULLDEV getting converted to Kconfig.


Reviewed-by: Ovidiu Panait <ovidiu.panait@windriver.com>


Thanks!

Ovidiu

>   # CONFIG_REGEX is not set
>   CONFIG_OF_LIBFDT=y
> diff --git a/doc/README.silent b/doc/README.silent
> index 6d90a0ec40..00288e03b0 100644
> --- a/doc/README.silent
> +++ b/doc/README.silent
> @@ -19,7 +19,7 @@ The following actions are taken if "silent" is set at boot time:
>    - When the console devices have been initialized, "stdout" and
>      "stderr" are set to "nulldev", so subsequent messages are
>      suppressed automatically. Make sure to enable "nulldev" by
> -   #defining CONFIG_SYS_DEVICE_NULLDEV in your board config file.
> +   enabling CONFIG_SYS_DEVICE_NULLDEV in your board defconfig file.
>   
>    - When booting a linux kernel, the "bootargs" are fixed up so that
>      the argument "console=" will be in the command line, no matter how
> diff --git a/include/configs/M5249EVB.h b/include/configs/M5249EVB.h
> index de7132940c..1a1a110765 100644
> --- a/include/configs/M5249EVB.h
> +++ b/include/configs/M5249EVB.h
> @@ -31,8 +31,6 @@
>    */
>   #undef CONFIG_BOOTP_BOOTFILESIZE
>   
> -#define CONFIG_SYS_DEVICE_NULLDEV	1	/* include nulldev device	*/
> -
>   #define CONFIG_SYS_LOAD_ADDR		0x200000	/* default load address */
>   
>   /*
> diff --git a/include/configs/colibri_pxa270.h b/include/configs/colibri_pxa270.h
> index 29827f1ee8..3bbef55ec3 100644
> --- a/include/configs/colibri_pxa270.h
> +++ b/include/configs/colibri_pxa270.h
> @@ -70,8 +70,6 @@
>   #define	CONFIG_BOOTP_BOOTFILESIZE
>   #endif
>   
> -#define	CONFIG_SYS_DEVICE_NULLDEV	1
> -
>   /*
>    * Clock Configuration
>    */
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index 1c7946fb65..2da34a5d23 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -2178,7 +2178,6 @@ CONFIG_SYS_DEBUG_SERVER_FW_IN_NOR
>   CONFIG_SYS_DEFAULT_LPDDR2_TIMINGS
>   CONFIG_SYS_DEFAULT_VIDEO_MODE
>   CONFIG_SYS_DEF_EEPROM_ADDR
> -CONFIG_SYS_DEVICE_NULLDEV
>   CONFIG_SYS_DFU_DATA_BUF_SIZE
>   CONFIG_SYS_DFU_MAX_FILE_SIZE
>   CONFIG_SYS_DIAG_ADDR

[PATCH 1/6] Convert CONFIG_SYS_DEVICE_NULLDEV to Kconfig

[Simon Glass]

Hi Ovidiu,

On Sat, 18 Jul 2020 at 12:04, Ovidiu Panait <ovidiu.panait@windriver.com> wrote:
>
> Hi Simon,
>
> On 18.07.2020 06:03, Simon Glass wrote:
>
> This converts the following to Kconfig:
>    CONFIG_SYS_DEVICE_NULLDEV
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  common/Kconfig                   | 16 ++++++++++++++++
>  configs/M5249EVB_defconfig       |  3 ++-
>  configs/colibri_pxa270_defconfig |  2 +-
>  doc/README.silent                |  2 +-
>  include/configs/M5249EVB.h       |  2 --
>  include/configs/colibri_pxa270.h |  2 --
>  scripts/config_whitelist.txt     |  1 -
>  7 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 67b3818fde..4d5b3a9cfb 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -634,6 +634,22 @@ config SYS_STDIO_DEREGISTER
>    removed (for example a USB keyboard) then this option can be
>    enabled to ensure this is handled correctly.
>
> +config SPL_SYS_STDIO_DEREGISTER
> + bool "Allow deregistering stdio devices in SPL"
> + help
> +  Generally there is no need to deregister stdio devices since they
> +  are never deactivated. But if a stdio device is used which can be
> +  removed (for example a USB keyboard) then this option can be
> +  enabled to ensure this is handled correctly. This is very rarely
> +  needed in SPL.
> +
> +config SYS_DEVICE_NULLDEV
> + bool "Enable a null device for stdio"
> + help
> +  Enable creation of a "nulldev" stdio device. This allows silent
> +  operation of the console by setting stdout to "nulldev". Enable
> +  this to use a serial console under board control.
> +
>  endmenu
>
>  menu "Logging"
> diff --git a/configs/M5249EVB_defconfig b/configs/M5249EVB_defconfig
> index 12db389b69..8f8a4a6bad 100644
> --- a/configs/M5249EVB_defconfig
> +++ b/configs/M5249EVB_defconfig
> @@ -3,7 +3,9 @@ CONFIG_SYS_TEXT_BASE=0xFFE00000
>  CONFIG_ENV_SIZE=0x2000
>  CONFIG_ENV_SECT_SIZE=0x2000
>  CONFIG_TARGET_M5249EVB=y
> +CONFIG_DEFAULT_DEVICE_TREE="M5249EVB"
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> +CONFIG_SYS_DEVICE_NULLDEV=y
>  # CONFIG_DISPLAY_BOARDINFO is not set
>  # CONFIG_CMDLINE_EDITING is not set
>  # CONFIG_AUTOBOOT is not set
> @@ -12,7 +14,6 @@ CONFIG_LOOPW=y
>  CONFIG_CMD_MX_CYCLIC=y
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_CMD_CACHE=y
> -CONFIG_DEFAULT_DEVICE_TREE="M5249EVB"
>  CONFIG_ENV_ADDR=0xFFE04000
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  # CONFIG_NET is not set
> diff --git a/configs/colibri_pxa270_defconfig b/configs/colibri_pxa270_defconfig
> index 669b9dfe58..aff7b62639 100644
> --- a/configs/colibri_pxa270_defconfig
> +++ b/configs/colibri_pxa270_defconfig
> @@ -10,6 +10,7 @@ CONFIG_NR_DRAM_BANKS=1
>  CONFIG_ENV_VARS_UBOOT_CONFIG=y
>  CONFIG_USE_BOOTARGS=y
>  CONFIG_BOOTARGS="console=tty0 console=ttyS0,115200"
> +CONFIG_SYS_DEVICE_NULLDEV=y
>  # CONFIG_DISPLAY_BOARDINFO is not set
>  CONFIG_DISPLAY_BOARDINFO_LATE=y
>  # CONFIG_CMDLINE_EDITING is not set
> @@ -43,6 +44,5 @@ CONFIG_SYS_FLASH_CFI=y
>  CONFIG_DM_SERIAL=y
>  CONFIG_PXA_SERIAL=y
>  CONFIG_USB=y
> -CONFIG_USB_STORAGE=y
>
> Is this deletion intentional? It is not immediately obvious to me how it relates to
>
> CONFIG_SYS_DEVICE_NULLDEV getting converted to Kconfig.

I suppose this is caused by a resync of the defconfig.


>
>
> Reviewed-by: Ovidiu Panait <ovidiu.panait@windriver.com>
>
>
> Thanks!
>
> Ovidiu
>
>  # CONFIG_REGEX is not set
>  CONFIG_OF_LIBFDT=y
> diff --git a/doc/README.silent b/doc/README.silent
> index 6d90a0ec40..00288e03b0 100644
> --- a/doc/README.silent
> +++ b/doc/README.silent
> @@ -19,7 +19,7 @@ The following actions are taken if "silent" is set at boot time:
>   - When the console devices have been initialized, "stdout" and
>     "stderr" are set to "nulldev", so subsequent messages are
>     suppressed automatically. Make sure to enable "nulldev" by
> -   #defining CONFIG_SYS_DEVICE_NULLDEV in your board config file.
> +   enabling CONFIG_SYS_DEVICE_NULLDEV in your board defconfig file.
>
>   - When booting a linux kernel, the "bootargs" are fixed up so that
>     the argument "console=" will be in the command line, no matter how
> diff --git a/include/configs/M5249EVB.h b/include/configs/M5249EVB.h
> index de7132940c..1a1a110765 100644
> --- a/include/configs/M5249EVB.h
> +++ b/include/configs/M5249EVB.h
> @@ -31,8 +31,6 @@
>   */
>  #undef CONFIG_BOOTP_BOOTFILESIZE
>
> -#define CONFIG_SYS_DEVICE_NULLDEV 1 /* include nulldev device */
> -
>  #define CONFIG_SYS_LOAD_ADDR 0x200000 /* default load address */
>
>  /*
> diff --git a/include/configs/colibri_pxa270.h b/include/configs/colibri_pxa270.h
> index 29827f1ee8..3bbef55ec3 100644
> --- a/include/configs/colibri_pxa270.h
> +++ b/include/configs/colibri_pxa270.h
> @@ -70,8 +70,6 @@
>  #define CONFIG_BOOTP_BOOTFILESIZE
>  #endif
>
> -#define CONFIG_SYS_DEVICE_NULLDEV 1
> -
>  /*
>   * Clock Configuration
>   */
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index 1c7946fb65..2da34a5d23 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -2178,7 +2178,6 @@ CONFIG_SYS_DEBUG_SERVER_FW_IN_NOR
>  CONFIG_SYS_DEFAULT_LPDDR2_TIMINGS
>  CONFIG_SYS_DEFAULT_VIDEO_MODE
>  CONFIG_SYS_DEF_EEPROM_ADDR
> -CONFIG_SYS_DEVICE_NULLDEV
>  CONFIG_SYS_DFU_DATA_BUF_SIZE
>  CONFIG_SYS_DFU_MAX_FILE_SIZE
>  CONFIG_SYS_DIAG_ADDR

[PATCH 4/6] stdio: Update to use compiler for Kconfig checks

[Tom Rini]

On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:

> Drop use of the preprocessor where possible.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
[snip]
> +	if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> +		/*
> +		 * If the console setting is not in environment variables then
> +		 * console_init_r() will not be calling iomux_doenv() (which
> +		 * calls search_device()). So we will not dynamically add
> +		 * devices by calling stdio_probe_device().
> +		 *
> +		 * So just probe all video devices now so that whichever one is
> +		 * required will be available.
> +		 */
> +		struct udevice *vdev;
> +		int ret;
> +
> +		if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> +			for (ret = uclass_first_device(UCLASS_VIDEO, &vdev);
> +			     vdev;
> +			     ret = uclass_next_device(&vdev))
> +				;
> +			if (ret)
> +				printf("%s: Video device failed (ret=%d)\n",
> +				       __func__, ret);
> +		}
> +		if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> +		    IS_ENABLED(CONFIG_CMD_BMP))
> +			splash_display();

We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
test fails and we drop the bmp logo.  I got this run-time tested and
confirmed on colibri imx6 by my colleague Matt Porter.  So I'm holding
off on _this_ patch (and then the style clean up patch) until I can push
SPLASH_SCREEN migration through.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200803/5ae639ca/attachment.sig>

[PATCH 4/6] stdio: Update to use compiler for Kconfig checks

[Tom Rini]

On Mon, Aug 03, 2020 at 06:57:05PM -0400, Tom Rini wrote:
> On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:
> 
> > Drop use of the preprocessor where possible.
> > 
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> [snip]
> > +	if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> > +		/*
> > +		 * If the console setting is not in environment variables then
> > +		 * console_init_r() will not be calling iomux_doenv() (which
> > +		 * calls search_device()). So we will not dynamically add
> > +		 * devices by calling stdio_probe_device().
> > +		 *
> > +		 * So just probe all video devices now so that whichever one is
> > +		 * required will be available.
> > +		 */
> > +		struct udevice *vdev;
> > +		int ret;
> > +
> > +		if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> > +			for (ret = uclass_first_device(UCLASS_VIDEO, &vdev);
> > +			     vdev;
> > +			     ret = uclass_next_device(&vdev))
> > +				;
> > +			if (ret)
> > +				printf("%s: Video device failed (ret=%d)\n",
> > +				       __func__, ret);
> > +		}
> > +		if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> > +		    IS_ENABLED(CONFIG_CMD_BMP))
> > +			splash_display();
> 
> We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
> test fails and we drop the bmp logo.  I got this run-time tested and
> confirmed on colibri imx6 by my colleague Matt Porter.  So I'm holding
> off on _this_ patch (and then the style clean up patch) until I can push
> SPLASH_SCREEN migration through.

This is also an issue for "stdio: Tidy up use of
CONFIG_SYS_DEVICE_NULLDEV" as well, so I'm going to put aside the whole
series until I can convert SPLASH_SCREEN.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200803/82738ed2/attachment.sig>

[PATCH 4/6] stdio: Update to use compiler for Kconfig checks

[Simon Glass]

Hi Tom,

On Mon, 3 Aug 2020 at 20:18, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Aug 03, 2020 at 06:57:05PM -0400, Tom Rini wrote:
> > On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:
> >
> > > Drop use of the preprocessor where possible.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > [snip]
> > > +   if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> > > +           /*
> > > +            * If the console setting is not in environment variables then
> > > +            * console_init_r() will not be calling iomux_doenv() (which
> > > +            * calls search_device()). So we will not dynamically add
> > > +            * devices by calling stdio_probe_device().
> > > +            *
> > > +            * So just probe all video devices now so that whichever one is
> > > +            * required will be available.
> > > +            */
> > > +           struct udevice *vdev;
> > > +           int ret;
> > > +
> > > +           if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> > > +                   for (ret = uclass_first_device(UCLASS_VIDEO, &vdev);
> > > +                        vdev;
> > > +                        ret = uclass_next_device(&vdev))
> > > +                           ;
> > > +                   if (ret)
> > > +                           printf("%s: Video device failed (ret=%d)\n",
> > > +                                  __func__, ret);
> > > +           }
> > > +           if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> > > +               IS_ENABLED(CONFIG_CMD_BMP))
> > > +                   splash_display();
> >
> > We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
> > test fails and we drop the bmp logo.  I got this run-time tested and
> > confirmed on colibri imx6 by my colleague Matt Porter.  So I'm holding
> > off on _this_ patch (and then the style clean up patch) until I can push
> > SPLASH_SCREEN migration through.
>
> This is also an issue for "stdio: Tidy up use of
> CONFIG_SYS_DEVICE_NULLDEV" as well, so I'm going to put aside the whole
> series until I can convert SPLASH_SCREEN.

OK, let me know if you'd like me to do something here. My patch 2 was
supposed to migrate CONFIG_SPLASH_SCREEN, etc. Did that not work?

Regards,
SImon

[PATCH 4/6] stdio: Update to use compiler for Kconfig checks

[Tom Rini]

On Tue, Aug 04, 2020 at 07:37:02AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 3 Aug 2020 at 20:18, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Aug 03, 2020 at 06:57:05PM -0400, Tom Rini wrote:
> > > On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:
> > >
> > > > Drop use of the preprocessor where possible.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > [snip]
> > > > +   if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> > > > +           /*
> > > > +            * If the console setting is not in environment variables then
> > > > +            * console_init_r() will not be calling iomux_doenv() (which
> > > > +            * calls search_device()). So we will not dynamically add
> > > > +            * devices by calling stdio_probe_device().
> > > > +            *
> > > > +            * So just probe all video devices now so that whichever one is
> > > > +            * required will be available.
> > > > +            */
> > > > +           struct udevice *vdev;
> > > > +           int ret;
> > > > +
> > > > +           if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> > > > +                   for (ret = uclass_first_device(UCLASS_VIDEO, &vdev);
> > > > +                        vdev;
> > > > +                        ret = uclass_next_device(&vdev))
> > > > +                           ;
> > > > +                   if (ret)
> > > > +                           printf("%s: Video device failed (ret=%d)\n",
> > > > +                                  __func__, ret);
> > > > +           }
> > > > +           if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> > > > +               IS_ENABLED(CONFIG_CMD_BMP))
> > > > +                   splash_display();
> > >
> > > We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
> > > test fails and we drop the bmp logo.  I got this run-time tested and
> > > confirmed on colibri imx6 by my colleague Matt Porter.  So I'm holding
> > > off on _this_ patch (and then the style clean up patch) until I can push
> > > SPLASH_SCREEN migration through.
> >
> > This is also an issue for "stdio: Tidy up use of
> > CONFIG_SYS_DEVICE_NULLDEV" as well, so I'm going to put aside the whole
> > series until I can convert SPLASH_SCREEN.
> 
> OK, let me know if you'd like me to do something here. My patch 2 was
> supposed to migrate CONFIG_SPLASH_SCREEN, etc. Did that not work?

I don't see that patch:
http://patchwork.ozlabs.org/user/todo/uboot/?series=190587

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200804/d17512a0/attachment.sig>

[PATCH 4/6] stdio: Update to use compiler for Kconfig checks

[Simon Glass]

Hi Tom,

I can't see anything other than my patches at that link. But it looks
like I didn't send it, sadly. I'll resend that patch.

Regards,
SImon

On Tue, 4 Aug 2020 at 07:46, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Aug 04, 2020 at 07:37:02AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 3 Aug 2020 at 20:18, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Aug 03, 2020 at 06:57:05PM -0400, Tom Rini wrote:
> > > > On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:
> > > >
> > > > > Drop use of the preprocessor where possible.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > [snip]
> > > > > +   if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> > > > > +           /*
> > > > > +            * If the console setting is not in environment variables then
> > > > > +            * console_init_r() will not be calling iomux_doenv() (which
> > > > > +            * calls search_device()). So we will not dynamically add
> > > > > +            * devices by calling stdio_probe_device().
> > > > > +            *
> > > > > +            * So just probe all video devices now so that whichever one is
> > > > > +            * required will be available.
> > > > > +            */
> > > > > +           struct udevice *vdev;
> > > > > +           int ret;
> > > > > +
> > > > > +           if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> > > > > +                   for (ret = uclass_first_device(UCLASS_VIDEO, &vdev);
> > > > > +                        vdev;
> > > > > +                        ret = uclass_next_device(&vdev))
> > > > > +                           ;
> > > > > +                   if (ret)
> > > > > +                           printf("%s: Video device failed (ret=%d)\n",
> > > > > +                                  __func__, ret);
> > > > > +           }
> > > > > +           if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> > > > > +               IS_ENABLED(CONFIG_CMD_BMP))
> > > > > +                   splash_display();
> > > >
> > > > We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
> > > > test fails and we drop the bmp logo.  I got this run-time tested and
> > > > confirmed on colibri imx6 by my colleague Matt Porter.  So I'm holding
> > > > off on _this_ patch (and then the style clean up patch) until I can push
> > > > SPLASH_SCREEN migration through.
> > >
> > > This is also an issue for "stdio: Tidy up use of
> > > CONFIG_SYS_DEVICE_NULLDEV" as well, so I'm going to put aside the whole
> > > series until I can convert SPLASH_SCREEN.
> >
> > OK, let me know if you'd like me to do something here. My patch 2 was
> > supposed to migrate CONFIG_SPLASH_SCREEN, etc. Did that not work?
>
> I don't see that patch:
> http://patchwork.ozlabs.org/user/todo/uboot/?series=190587
>
> --
> Tom