[PATCH v2 1/7] console: Introduce console_start() and console_stop()

In the future we would like to stop unused consoles and also add a reference counting to avoid imbalanced calls to ->start() and ->stop() in some cases.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- v2: new patch common/console.c | 30 +++++++++++++++++++++++------- include/console.h | 3 +++ 2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/common/console.c b/common/console.c index 3348436da6f7..1efbcc7672ce 100644 --- a/common/console.c +++ b/common/console.c @@ -114,13 +114,9 @@ static int console_setfile(int file, struct stdio_dev * dev) case stdin: case stdout: case stderr: - /* Start new device */ - if (dev->start) { - error = dev->start(dev); - /* If it's not started dont use it */ - if (error < 0) - break; - } + error = console_start(file, dev); + if (error) + break;
/* Assign the new device (leaving the existing one started) */ stdio_devices[file] = dev; @@ -310,6 +306,26 @@ static inline void console_doenv(int file, struct stdio_dev *dev) #endif #endif /* CONIFIG_IS_ENABLED(CONSOLE_MUX) */
+int console_start(int file, struct stdio_dev *dev) +{ + int error; + + /* Start new device */ + if (dev->start) { + error = dev->start(dev); + /* If it's not started don't use it */ + if (error < 0) + return error; + } + return 0; +} + +void console_stop(int file, struct stdio_dev *dev) +{ + if (dev->stop) + dev->stop(dev); +} + /** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/
int serial_printf(const char *fmt, ...) diff --git a/include/console.h b/include/console.h index 432f892b6cce..233ff323e1ee 100644 --- a/include/console.h +++ b/include/console.h @@ -8,6 +8,7 @@ #define __CONSOLE_H
#include <stdbool.h> +#include <stdio_dev.h> #include <linux/errno.h>
extern char console_buffer[]; @@ -15,6 +16,8 @@ extern char console_buffer[]; /* common/console.c */ int console_init_f(void); /* Before relocation; uses the serial stuff */ int console_init_r(void); /* After relocation; uses the console stuff */ +int console_start(int file, struct stdio_dev *dev); +void console_stop(int file, struct stdio_dev *dev); int console_assign(int file, const char *devname); /* Assign the console */ int ctrlc(void); int had_ctrlc(void); /* have we had a Control-C since last clear? */

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 CONSOLE_MUX case.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- v2: new patch common/console.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/common/console.c b/common/console.c index 1efbcc7672ce..b051c24e7389 100644 --- a/common/console.c +++ b/common/console.c @@ -174,6 +174,16 @@ static struct stdio_dev *tstcdev; struct stdio_dev **console_devices[MAX_FILES]; int cd_count[MAX_FILES];
+static bool console_needs_handle(int file, struct stdio_dev *dev) +{ + int i; + + for (i = 0; i < cd_count[file]; i++) + if (console_devices[file][i] == dev) + 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 +282,11 @@ static inline void console_doenv(int file, struct stdio_dev *dev) } #endif #else +static inline bool console_needs_handle(int file, struct stdio_dev *dev) +{ + return true; +} + static inline int console_getc(int file) { return stdio_devices[file]->getc(stdio_devices[file]); @@ -310,6 +325,9 @@ int console_start(int file, struct stdio_dev *dev) { int error;
+ if (!console_needs_handle(file, dev)) + return 0; + /* Start new device */ if (dev->start) { error = dev->start(dev); @@ -322,6 +340,9 @@ int console_start(int file, struct stdio_dev *dev)
void console_stop(int file, struct stdio_dev *dev) { + if (!console_needs_handle(file, dev)) + return; + if (dev->stop) dev->stop(dev); }

On Thu, Dec 17, 2020 at 1:16 AM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
There is no need to call ->start() for already started device. All the same,
an already
there is no need to call ->stop() for devices still in use.
For now enforce this only for CONSOLE_MUX case.
now, enforce the CONSOLE_MUX
...
+static bool console_needs_handle(int file, struct stdio_dev *dev) +{
int i;
for (i = 0; i < cd_count[file]; i++)
if (console_devices[file][i] == dev)
return false;
Actually this is no-op. I realized it later on. It misses the iteration over all files. And file argument to exclude iteration over that specific file.
I will update this, but will also wait a couple of days for other comments against the series.
return true;
+}

Hi Andy,
On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
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 CONSOLE_MUX case.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
v2: new patch common/console.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/common/console.c b/common/console.c index 1efbcc7672ce..b051c24e7389 100644 --- a/common/console.c +++ b/common/console.c @@ -174,6 +174,16 @@ static struct stdio_dev *tstcdev; struct stdio_dev **console_devices[MAX_FILES]; int cd_count[MAX_FILES];
+static bool console_needs_handle(int file, struct stdio_dev *dev)
function comment - what does this do?
+{
int i;
for (i = 0; i < cd_count[file]; i++)
if (console_devices[file][i] == dev)
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 +282,11 @@ static inline void console_doenv(int file, struct stdio_dev *dev) } #endif #else +static inline bool console_needs_handle(int file, struct stdio_dev *dev) +{
return true;
+}
static inline int console_getc(int file) { return stdio_devices[file]->getc(stdio_devices[file]); @@ -310,6 +325,9 @@ int console_start(int file, struct stdio_dev *dev) { int error;
if (!console_needs_handle(file, dev))
return 0;
/* Start new device */ if (dev->start) { error = dev->start(dev);
@@ -322,6 +340,9 @@ int console_start(int file, struct stdio_dev *dev)
void console_stop(int file, struct stdio_dev *dev) {
if (!console_needs_handle(file, dev))
return;
if (dev->stop) dev->stop(dev);
}
2.29.2
Regards, Simon

search_device() is defined in console.c. Move its declaration to an appropriate header file.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- v2: new patch include/console.h | 2 ++ include/iomux.h | 1 - 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/console.h b/include/console.h index 233ff323e1ee..79f9534a9535 100644 --- a/include/console.h +++ b/include/console.h @@ -25,6 +25,8 @@ 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" */
+struct stdio_dev *search_device(int flags, const char *name); + #ifdef CONFIG_CONSOLE_RECORD /** * console_record_init() - set up the console recording buffers diff --git a/include/iomux.h b/include/iomux.h index e6e1097db5b2..da7ff697d218 100644 --- a/include/iomux.h +++ b/include/iomux.h @@ -26,6 +26,5 @@ extern int cd_count[MAX_FILES];
int iomux_doenv(const int, const char *); void iomux_printdevs(const int); -struct stdio_dev *search_device(int, const char *);
#endif /* _IO_MUX_H */

On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
search_device() is defined in console.c. Move its declaration to an appropriate header file.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
v2: new patch include/console.h | 2 ++ include/iomux.h | 1 - 2 files changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/include/console.h b/include/console.h index 233ff323e1ee..79f9534a9535 100644 --- a/include/console.h +++ b/include/console.h @@ -25,6 +25,8 @@ 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" */
+struct stdio_dev *search_device(int flags, const char *name);
#ifdef CONFIG_CONSOLE_RECORD /**
- console_record_init() - set up the console recording buffers
diff --git a/include/iomux.h b/include/iomux.h index e6e1097db5b2..da7ff697d218 100644 --- a/include/iomux.h +++ b/include/iomux.h @@ -26,6 +26,5 @@ extern int cd_count[MAX_FILES];
int iomux_doenv(const int, const char *); void iomux_printdevs(const int); -struct stdio_dev *search_device(int, const char *);
Please add a full function comment
Also consider renaming it to console_search_dev() or similar, since it is in console.h
#endif /* _IO_MUX_H */
2.29.2
Regards, Simon

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 --- v2: no changes common/iomux.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/common/iomux.c b/common/iomux.c index 7cfd9f2e9162..cee5f266c86e 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 */

On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
It's realloc() 101 to avoid `foo = realloc(foo, ...);` call due to getting a memory leak.
Hmm I don't think I knew that...
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
v2: no changes common/iomux.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/common/iomux.c b/common/iomux.c index 7cfd9f2e9162..cee5f266c86e 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 */
2.29.2

On Fri, Dec 18, 2020 at 07:29:19PM -0700, Simon Glass wrote:
On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
It's realloc() 101 to avoid `foo = realloc(foo, ...);` call due to getting a memory leak.
Hmm I don't think I knew that...
When you use the same variable for the source and destination in case of NULL the source gone.
It's okay to have
foo = bar; bar = realloc(bar, ...); if (bar == NULL) ...do something with foo if needed...
But it seems it's not the case here.
Reviewed-by: Simon Glass sjg@chromium.org
Thanks!

Hi Andy,
On Mon, 21 Dec 2020 at 05:00, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Fri, Dec 18, 2020 at 07:29:19PM -0700, Simon Glass wrote:
On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
It's realloc() 101 to avoid `foo = realloc(foo, ...);` call due to getting a memory leak.
Hmm I don't think I knew that...
When you use the same variable for the source and destination in case of NULL the source gone.
It's okay to have
foo = bar; bar = realloc(bar, ...); if (bar == NULL) ...do something with foo if needed...
Here is man malloc on this point:
If ptr is NULL, then the call is equivalent to mal‐ loc(size), for all values of size; if size is equal to zero, and ptr is not NULL, then the call is equivalent to free(ptr).
But it seems it's not the case here.
Reviewed-by: Simon Glass sjg@chromium.org
Thanks!
Regards, Simon

On Mon, Dec 21, 2020 at 09:47:03AM -0700, Simon Glass wrote:
On Mon, 21 Dec 2020 at 05:00, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Fri, Dec 18, 2020 at 07:29:19PM -0700, Simon Glass wrote:
On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
It's realloc() 101 to avoid `foo = realloc(foo, ...);` call due to getting a memory leak.
Hmm I don't think I knew that...
When you use the same variable for the source and destination in case of NULL the source gone.
It's okay to have
foo = bar; bar = realloc(bar, ...); if (bar == NULL) ...do something with foo if needed...
Here is man malloc on this point:
If ptr is NULL, then the call is equivalent to mal‐ loc(size), for all values of size; if size is equal to zero, and ptr is not NULL, then the call is equivalent to free(ptr).
But it's about another case. I'm talking about realloc() to fail.
foo = realloc(foo, ...);
will effectively leak memory if foo is not saved previously somewhere. And this is the case here.
For instance [1] is telling about the same: "Of course if you will write
p = realloc(p, 2 * sizeof(int));
... if the function was unable to reallocate memory. In this case a memory leak will occur provided that initial value of the pointer p was not equal to NULL."
Really, it's 101 of realloc() usage.
[1]: https://stackoverflow.com/questions/57498538/does-realloc-mutate-its-argumen...

Hi Andy,
On Mon, 21 Dec 2020 at 10:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Mon, Dec 21, 2020 at 09:47:03AM -0700, Simon Glass wrote:
On Mon, 21 Dec 2020 at 05:00, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Fri, Dec 18, 2020 at 07:29:19PM -0700, Simon Glass wrote:
On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
It's realloc() 101 to avoid `foo = realloc(foo, ...);` call due to getting a memory leak.
Hmm I don't think I knew that...
When you use the same variable for the source and destination in case of NULL the source gone.
It's okay to have
foo = bar; bar = realloc(bar, ...); if (bar == NULL) ...do something with foo if needed...
Here is man malloc on this point:
If ptr is NULL, then the call is equivalent to mal‐ loc(size), for all values of size; if size is equal to zero, and ptr is not NULL, then the call is equivalent to free(ptr).
But it's about another case. I'm talking about realloc() to fail.
foo = realloc(foo, ...);
will effectively leak memory if foo is not saved previously somewhere. And this is the case here.
For instance [1] is telling about the same: "Of course if you will write
p = realloc(p, 2 * sizeof(int));
... if the function was unable to reallocate memory. In this case a memory leak will occur provided that initial value of the pointer p was not equal to NULL."
Really, it's 101 of realloc() usage.
Oh I missed that it failed...OK
Regards, Simon

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 --- v2: no changes common/iomux.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/common/iomux.c b/common/iomux.c index cee5f266c86e..51557d028029 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) {

On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
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
v2: no changes common/iomux.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/common/iomux.c b/common/iomux.c index cee5f266c86e..51557d028029 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) {
event better:
if (temp)
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) {
-- 2.29.2

On Fri, Dec 18, 2020 at 07:29:21PM -0700, Simon Glass wrote:
On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
Refactor iomux_doenv() a bit in order to increase readability. There is no change in code generation on x86.
...
Reviewed-by: Simon Glass sjg@chromium.org
Thanks!
...
event better:
if (temp)
I didn't get this. Can you elaborate what exactly you had in mind? Because...
temp = strchr(temp, ',');
if (temp == NULL)
...here is a new code.
break;
temp++;
Since you gave Rb tag I'll leave as is for now.

Hi Andy,
On Mon, 21 Dec 2020 at 05:03, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Fri, Dec 18, 2020 at 07:29:21PM -0700, Simon Glass wrote:
On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
Refactor iomux_doenv() a bit in order to increase readability. There is no change in code generation on x86.
...
Reviewed-by: Simon Glass sjg@chromium.org
Thanks!
...
event better:
if (temp)
I didn't get this. Can you elaborate what exactly you had in mind? Because...
temp = strchr(temp, ',');
if (temp == NULL)
...here is a new code.
It would help if I wrote it for the new code. I just mean:
if (temp == NULL)
is better written in U-Boot as
if (!temp)
break;
temp++;
Since you gave Rb tag I'll leave as is for now.
Yes that's fine.
Regards, Simon

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 --- v2: no changes common/iomux.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/iomux.c b/common/iomux.c index 51557d028029..8a063563aa27 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 */

On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
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
v2: no changes common/iomux.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

When at some point environment shrinks we need to stop dropped devices.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- v2: rebased to use console_stop() common/iomux.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/common/iomux.c b/common/iomux.c index 8a063563aa27..4fe73c8cba8a 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 */

On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
When at some point environment shrinks we need to stop dropped devices.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
v2: rebased to use console_stop() common/iomux.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Andy,
On Wed, 16 Dec 2020 at 16:16, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
In the future we would like to stop unused consoles and also add a reference counting to avoid imbalanced calls to ->start() and ->stop() in some cases.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
v2: new patch common/console.c | 30 +++++++++++++++++++++++------- include/console.h | 3 +++ 2 files changed, 26 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
But please see below
diff --git a/common/console.c b/common/console.c index 3348436da6f7..1efbcc7672ce 100644 --- a/common/console.c +++ b/common/console.c @@ -114,13 +114,9 @@ static int console_setfile(int file, struct stdio_dev * dev) case stdin: case stdout: case stderr:
/* Start new device */
if (dev->start) {
error = dev->start(dev);
/* If it's not started dont use it */
if (error < 0)
break;
}
error = console_start(file, dev);
if (error)
break; /* Assign the new device (leaving the existing one started) */ stdio_devices[file] = dev;
@@ -310,6 +306,26 @@ static inline void console_doenv(int file, struct stdio_dev *dev) #endif #endif /* CONIFIG_IS_ENABLED(CONSOLE_MUX) */
+int console_start(int file, struct stdio_dev *dev) +{
int error;
/* Start new device */
if (dev->start) {
error = dev->start(dev);
/* If it's not started don't use it */
if (error < 0)
return error;
}
return 0;
+}
+void console_stop(int file, struct stdio_dev *dev) +{
if (dev->stop)
dev->stop(dev);
+}
/** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/
int serial_printf(const char *fmt, ...) diff --git a/include/console.h b/include/console.h index 432f892b6cce..233ff323e1ee 100644 --- a/include/console.h +++ b/include/console.h @@ -8,6 +8,7 @@ #define __CONSOLE_H
#include <stdbool.h> +#include <stdio_dev.h> #include <linux/errno.h>
extern char console_buffer[]; @@ -15,6 +16,8 @@ extern char console_buffer[]; /* common/console.c */ int console_init_f(void); /* Before relocation; uses the serial stuff */ int console_init_r(void); /* After relocation; uses the console stuff */ +int console_start(int file, struct stdio_dev *dev); +void console_stop(int file, struct stdio_dev *dev);
These two need comments
Also I'd prefer to use sdev for the args, since we use dev for a driver model device.
int console_assign(int file, const char *devname); /* Assign the console */ int ctrlc(void); int had_ctrlc(void); /* have we had a Control-C since last clear? */ -- 2.29.2
Regards, Simon
participants (3)
-
Andy Shevchenko
-
Andy Shevchenko
-
Simon Glass