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

[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