There is no need to call ->start() for already started device. All the same, there is no need to call ->stop() for devices still in use.

For now enforce this only for IOMUX case.

Cc: Simon Glass sjg@chromium.org Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- v3: renamed new function and added a documentation for it common/console.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)

diff --git a/common/console.c b/common/console.c index 9973f96e7629..e5fd214ae86f 100644 --- a/common/console.c +++ b/common/console.c @@ -174,6 +174,32 @@ static struct stdio_dev *tstcdev; struct stdio_dev **console_devices[MAX_FILES]; int cd_count[MAX_FILES];

+/** + * console_needs_start_stop() - check if we need to start or stop the STDIO device + * @file: STDIO file + * @sdev: STDIO device in question + * + * This function checks if we need to start or stop the stdio device used for + * a console. For IOMUX case it simply enforces one time start and one time + * stop of the device independently of how many STDIO files are using it. In + * other words, we start console once before first STDIO device wants it and + * stop after the last is gone. + */ +static bool console_needs_start_stop(int file, struct stdio_dev *sdev) +{ + int i, j; + + for (i = 0; i < ARRAY_SIZE(cd_count); i++) { + if (i == file) + continue; + + for (j = 0; j < cd_count[i]; j++) + if (console_devices[i][j] == sdev) + return false; + } + return true; +} + /* * This depends on tstc() always being called before getchar(). * This is guaranteed to be true because this routine is called @@ -272,6 +298,11 @@ static inline void console_doenv(int file, struct stdio_dev *dev) } #endif #else +static inline bool console_needs_start_stop(int file, struct stdio_dev *sdev) +{ + return true; +} + static inline int console_getc(int file) { return stdio_devices[file]->getc(stdio_devices[file]); @@ -310,6 +341,9 @@ int console_start(int file, struct stdio_dev *sdev) { int error;

+ if (!console_needs_start_stop(file, sdev)) + return 0; + /* Start new device */ if (sdev->start) { error = sdev->start(sdev); @@ -322,6 +356,9 @@ int console_start(int file, struct stdio_dev *sdev)

void console_stop(int file, struct stdio_dev *sdev) { + if (!console_needs_start_stop(file, sdev)) + return; + if (sdev->stop) sdev->stop(sdev); }

Provide a documentation for console_search_dev().

Suggested-by: Simon Glass sjg@chromium.org Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- v3: new patch include/console.h | 10 ++++++++++ 1 file changed, 10 insertions(+)

diff --git a/include/console.h b/include/console.h index bb186e7be04d..7e628c0cf836 100644 --- a/include/console.h +++ b/include/console.h @@ -25,6 +25,16 @@ void clear_ctrlc(void); /* clear the Control-C condition */ int disable_ctrlc(int); /* 1 to disable, 0 to enable Control-C detect */ int confirm_yesno(void); /* 1 if input is "y", "Y", "yes" or "YES" */

+/** + * console_search_dev() - search for stdio device with given flags and name + * @flags: device flags as per input/output/system + * @name: device name + * + * Iterates over registered STDIO devices and match them with given @flags + * and @name. + * + * @return pointer to the &struct stdio_dev if found, or NULL otherwise + */ struct stdio_dev *console_search_dev(int flags, const char *name);

#ifdef CONFIG_CONSOLE_RECORD

It's realloc() 101 to avoid `foo = realloc(foo, ...);` call due to getting a memory leak.

Actually it's not clear why realloc() has been used here. If we shrink the array, the memcpy() overwrites it anyway with the contents of a new array. If it becomes bigger, same story.

Drop useless realloc() for good and thus preserve console list in case of failed allocation.

Fixes: 16a28ef219c2 ("IOMUX: Add console multiplexing support.") Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Reviewed-by: Simon Glass sjg@chromium.org --- v3: added tag common/iomux.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/common/iomux.c b/common/iomux.c index e1bd1b48cd03..7b7b063cfc6b 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -129,19 +129,10 @@ int iomux_doenv(const int console, const char *arg) return 1; } else { /* Works even if console_devices[console] is NULL. */ - console_devices[console] = - (struct stdio_dev **)realloc(console_devices[console], - cs_idx * sizeof(struct stdio_dev *)); - if (console_devices[console] == NULL) { - free(cons_set); - return 1; - } - memcpy(console_devices[console], cons_set, cs_idx * - sizeof(struct stdio_dev *)); - + free(console_devices[console]); + console_devices[console] = cons_set; cd_count[console] = cs_idx; } - free(cons_set); return 0; } #endif /* CONSOLE_MUX */

Refactor iomux_doenv() a bit in order to increase readability. There is no change in code generation on x86.

Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Reviewed-by: Simon Glass sjg@chromium.org --- v3: added tag common/iomux.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/common/iomux.c b/common/iomux.c index 7b7b063cfc6b..7991235d1197 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -45,15 +45,14 @@ int iomux_doenv(const int console, const char *arg) i = 0; temp = console_args; for (;;) { - temp = strchr(temp, ','); - if (temp != NULL) { - i++; - temp++; - continue; - } /* There's always one entry more than the number of commas. */ i++; - break; + + temp = strchr(temp, ','); + if (temp == NULL) + break; + + temp++; } start = (char **)malloc(i * sizeof(char *)); if (start == NULL) {

Obviously the following has unnecessary indentation level in 'else' branch.

if (foo) { ... return; } else { ... }

Drop indentation level by removing redundant 'else'.

Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Reviewed-by: Simon Glass sjg@chromium.org --- v3: added tag common/iomux.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/common/iomux.c b/common/iomux.c index 7991235d1197..126d92ce850f 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -126,12 +126,12 @@ int iomux_doenv(const int console, const char *arg) if (cs_idx == 0) { free(cons_set); return 1; - } else { - /* Works even if console_devices[console] is NULL. */ - free(console_devices[console]); - console_devices[console] = cons_set; - cd_count[console] = cs_idx; } + + /* Works even if console_devices[console] is NULL. */ + free(console_devices[console]); + console_devices[console] = cons_set; + cd_count[console] = cs_idx; return 0; } #endif /* CONSOLE_MUX */

When at some point environment shrinks we need to stop dropped devices.

Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Reviewed-by: Simon Glass sjg@chromium.org --- v3: added tag common/iomux.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/common/iomux.c b/common/iomux.c index 126d92ce850f..15bf53388559 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -27,8 +27,8 @@ int iomux_doenv(const int console, const char *arg) { char *console_args, *temp, **start; int i, j, k, io_flag, cs_idx, repeat; + struct stdio_dev **cons_set, **old_set; struct stdio_dev *dev; - struct stdio_dev **cons_set;

console_args = strdup(arg); if (console_args == NULL) @@ -128,10 +128,23 @@ int iomux_doenv(const int console, const char *arg) return 1; }

- /* Works even if console_devices[console] is NULL. */ - free(console_devices[console]); + old_set = console_devices[console]; + repeat = cd_count[console]; + console_devices[console] = cons_set; cd_count[console] = cs_idx; + + /* Stop dropped consoles */ + for (i = 0; i < repeat; i++) { + for (j = 0; j < cs_idx; j++) { + if (old_set[i] == cons_set[j]) + break; + } + if (j == cs_idx) + console_stop(console, old_set[i]); + } + + free(old_set); return 0; } #endif /* CONSOLE_MUX */