diff mbox series

[v2,1/1] cmd: add serial console support for the cls command

Message ID 20220211171105.26189-1-heinrich.schuchardt@canonical.com
State Accepted, archived
Delegated to: Tom Rini
Headers show
Series [v2,1/1] cmd: add serial console support for the cls command | expand

Commit Message

Heinrich Schuchardt Feb. 11, 2022, 5:11 p.m. UTC
Currently the cls command does not support the serial console

The screen can be cleared in the video uclass, the colored frame buffer
console, and the serial console by sending the same escape sequence.
This reduces the cls command to a single printf() statement on most
boards.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	support cls with CONFIG_DM_VIDEO=y and CONFIG_VIDEO_ANSI=n
---
 cmd/cls.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Simon Glass Feb. 11, 2022, 8:29 p.m. UTC | #1
On Fri, 11 Feb 2022 at 10:11, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> Currently the cls command does not support the serial console
>
> The screen can be cleared in the video uclass, the colored frame buffer
> console, and the serial console by sending the same escape sequence.
> This reduces the cls command to a single printf() statement on most
> boards.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
>         support cls with CONFIG_DM_VIDEO=y and CONFIG_VIDEO_ANSI=n
> ---
>  cmd/cls.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

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

(would be better with if() instead of #if)


>
> diff --git a/cmd/cls.c b/cmd/cls.c
> index eab4e6993b..b411fc1eb1 100644
> --- a/cmd/cls.c
> +++ b/cmd/cls.c
> @@ -11,12 +11,16 @@
>  #include <lcd.h>
>  #include <video.h>
>
> +#define CSI "\x1b["
> +
>  static int do_video_clear(struct cmd_tbl *cmdtp, int flag, int argc,
>                           char *const argv[])
>  {
> -#if defined(CONFIG_DM_VIDEO)
> -       struct udevice *dev;
> +       __maybe_unused struct udevice *dev;
>
> +       /*  Send clear screen and home */
> +       printf(CSI "2J" CSI "1;1H");
> +#if defined(CONFIG_DM_VIDEO) && !defined(CONFIG_VIDEO_ANSI)
>         if (uclass_first_device_err(UCLASS_VIDEO, &dev))
>                 return CMD_RET_FAILURE;
>
> --
> 2.34.1
>
Heinrich Schuchardt March 11, 2022, 7:06 a.m. UTC | #2
On 2/11/22 21:29, Simon Glass wrote:
> On Fri, 11 Feb 2022 at 10:11, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> Currently the cls command does not support the serial console
>>
>> The screen can be cleared in the video uclass, the colored frame buffer
>> console, and the serial console by sending the same escape sequence.
>> This reduces the cls command to a single printf() statement on most
>> boards.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> v2:
>>          support cls with CONFIG_DM_VIDEO=y and CONFIG_VIDEO_ANSI=n
>> ---
>>   cmd/cls.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> (would be better with if() instead of #if)

This is not possible because you chose to give two functions with a 
different number of parameters the same name (video_clear()).

Best regards

Heinrich

> 
> 
>>
>> diff --git a/cmd/cls.c b/cmd/cls.c
>> index eab4e6993b..b411fc1eb1 100644
>> --- a/cmd/cls.c
>> +++ b/cmd/cls.c
>> @@ -11,12 +11,16 @@
>>   #include <lcd.h>
>>   #include <video.h>
>>
>> +#define CSI "\x1b["
>> +
>>   static int do_video_clear(struct cmd_tbl *cmdtp, int flag, int argc,
>>                            char *const argv[])
>>   {
>> -#if defined(CONFIG_DM_VIDEO)
>> -       struct udevice *dev;
>> +       __maybe_unused struct udevice *dev;
>>
>> +       /*  Send clear screen and home */
>> +       printf(CSI "2J" CSI "1;1H");
>> +#if defined(CONFIG_DM_VIDEO) && !defined(CONFIG_VIDEO_ANSI)
>>          if (uclass_first_device_err(UCLASS_VIDEO, &dev))
>>                  return CMD_RET_FAILURE;
>>
>> --
>> 2.34.1
>>
Simon Glass March 12, 2022, 5:02 a.m. UTC | #3
Hi Heinrich,

On Fri, 11 Mar 2022 at 00:06, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 2/11/22 21:29, Simon Glass wrote:
> > On Fri, 11 Feb 2022 at 10:11, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> Currently the cls command does not support the serial console
> >>
> >> The screen can be cleared in the video uclass, the colored frame buffer
> >> console, and the serial console by sending the same escape sequence.
> >> This reduces the cls command to a single printf() statement on most
> >> boards.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> ---
> >> v2:
> >>          support cls with CONFIG_DM_VIDEO=y and CONFIG_VIDEO_ANSI=n
> >> ---
> >>   cmd/cls.c | 8 ++++++--
> >>   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > (would be better with if() instead of #if)
>
> This is not possible because you chose to give two functions with a
> different number of parameters the same name (video_clear()).

Yes that is bad, but I sent a series to remove cfb_console:

https://patchwork.ozlabs.org/project/uboot/list/?series=282367

Regards,
Simon
Heinrich Schuchardt March 12, 2022, 12:05 p.m. UTC | #4
On 3/12/22 06:02, Simon Glass wrote:
> Hi Heinrich,
> 
> On Fri, 11 Mar 2022 at 00:06, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> On 2/11/22 21:29, Simon Glass wrote:
>>> On Fri, 11 Feb 2022 at 10:11, Heinrich Schuchardt
>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>
>>>> Currently the cls command does not support the serial console
>>>>
>>>> The screen can be cleared in the video uclass, the colored frame buffer
>>>> console, and the serial console by sending the same escape sequence.
>>>> This reduces the cls command to a single printf() statement on most
>>>> boards.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>>> ---
>>>> v2:
>>>>           support cls with CONFIG_DM_VIDEO=y and CONFIG_VIDEO_ANSI=n
>>>> ---
>>>>    cmd/cls.c | 8 ++++++--
>>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>
>>> (would be better with if() instead of #if)
>>
>> This is not possible because you chose to give two functions with a
>> different number of parameters the same name (video_clear()).
> 
> Yes that is bad, but I sent a series to remove cfb_console:
> 
> https://patchwork.ozlabs.org/project/uboot/list/?series=282367

There still remains:

In lcd.h there is an #ifndef CONFIG_DM_VIDEO hiding the definition of 
lcd_clear().

Best regards

Heinrich

> 
> Regards,
> Simon
Simon Glass March 12, 2022, 5:59 p.m. UTC | #5
Hi Heinrich,

On Sat, 12 Mar 2022 at 05:05, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
>
>
> On 3/12/22 06:02, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 11 Mar 2022 at 00:06, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> On 2/11/22 21:29, Simon Glass wrote:
> >>> On Fri, 11 Feb 2022 at 10:11, Heinrich Schuchardt
> >>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>
> >>>> Currently the cls command does not support the serial console
> >>>>
> >>>> The screen can be cleared in the video uclass, the colored frame buffer
> >>>> console, and the serial console by sending the same escape sequence.
> >>>> This reduces the cls command to a single printf() statement on most
> >>>> boards.
> >>>>
> >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >>>> ---
> >>>> v2:
> >>>>           support cls with CONFIG_DM_VIDEO=y and CONFIG_VIDEO_ANSI=n
> >>>> ---
> >>>>    cmd/cls.c | 8 ++++++--
> >>>>    1 file changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> Reviewed-by: Simon Glass <sjg@chromium.org>
> >>>
> >>> (would be better with if() instead of #if)
> >>
> >> This is not possible because you chose to give two functions with a
> >> different number of parameters the same name (video_clear()).
> >
> > Yes that is bad, but I sent a series to remove cfb_console:
> >
> > https://patchwork.ozlabs.org/project/uboot/list/?series=282367
>
> There still remains:
>
> In lcd.h there is an #ifndef CONFIG_DM_VIDEO hiding the definition of
> lcd_clear().

OK, well you can drop that if you like. I cannot see why the #iifdef
is needed there.

I am waiting for the video series to go in but can do a series to
remove the LCD stuff after that.

Regards,
Simon
diff mbox series

Patch

diff --git a/cmd/cls.c b/cmd/cls.c
index eab4e6993b..b411fc1eb1 100644
--- a/cmd/cls.c
+++ b/cmd/cls.c
@@ -11,12 +11,16 @@ 
 #include <lcd.h>
 #include <video.h>
 
+#define CSI "\x1b["
+
 static int do_video_clear(struct cmd_tbl *cmdtp, int flag, int argc,
 			  char *const argv[])
 {
-#if defined(CONFIG_DM_VIDEO)
-	struct udevice *dev;
+	__maybe_unused struct udevice *dev;
 
+	/*  Send clear screen and home */
+	printf(CSI "2J" CSI "1;1H");
+#if defined(CONFIG_DM_VIDEO) && !defined(CONFIG_VIDEO_ANSI)
 	if (uclass_first_device_err(UCLASS_VIDEO, &dev))
 		return CMD_RET_FAILURE;