Re: [U-Boot] [PATCH 09/28] video: Drop unused console functions

Hi Bin,

On 15 September 2015 at 00:11, Bin Meng bmeng.cn@gmail.com wrote:

Hi Simon,

On Wed, Sep 9, 2015 at 12:32 PM, Simon Glass sjg@chromium.org wrote:

...

CONFIG_CONSOLE_CURSOR, CONFIG_SYS_CONSOLE_BLINK_COUNT and CONFIG_CONSOLE_TIME are not used by any board. The implementation is not great and stands in the way of a refactor of i8042. Drop these for now. They can be re-introduced quite easily later, perhaps with driver model RTC support.

RTC?

Yes - they use the real-time clock for this feature.

...

When reintroducing, it might be useful to make a few changes:

• Blink time would be more useful than blink count
• The confusing #ifdefs should be avoided
• The time functions should support driver model
• It would be best keyed off console_tstc() or some similar idle loop rather than a particular input driver (i8042 in this case)

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

README | 7 ----- drivers/input/i8042.c | 23 ---------------- drivers/video/cfb_console.c | 62 +++++++------------------------------------ include/configs/MPC8536DS.h | 1 - include/configs/MPC8544DS.h | 1 - include/configs/MPC8572DS.h | 1 - include/configs/MPC8641HPCN.h | 1 - 7 files changed, 9 insertions(+), 87 deletions(-)

diff --git a/README b/README index 08f2f70..54c1d08 100644 --- a/README +++ b/README @@ -890,13 +890,6 @@ The following options need to be configured: (i.e. i8042_tstc) VIDEO_GETC_FCT get char fct (i.e. i8042_getc)

•                   CONFIG_CONSOLE_CURSOR   cursor drawing on/off
  

•                                           (requires blink timer
  

•                                           cf. i8042.c)
  

•                   CONFIG_SYS_CONSOLE_BLINK_COUNT blink interval (cf. i8042.c)
  

•                   CONFIG_CONSOLE_TIME     display time/date info in
  

•                                           upper right corner
  

•                                           (requires CONFIG_CMD_DATE)
                    CONFIG_VIDEO_LOGO       display Linux logo in
                                            upper left corner
                    CONFIG_VIDEO_BMP_LOGO   use bmp_logo.h instead of
  

diff --git a/drivers/input/i8042.c b/drivers/input/i8042.c index 9b5fa32..7b95b21 100644 --- a/drivers/input/i8042.c +++ b/drivers/input/i8042.c @@ -17,12 +17,6 @@ #define in8(p) inb(p) #define out8(p, v) outb(v, p)

-#ifdef CONFIG_CONSOLE_CURSOR -extern void console_cursor(int state); -static int blink_count = CONFIG_SYS_CONSOLE_BLINK_COUNT; -static int cursor_state; -#endif

/* locals */

static int kbd_input = -1; /* no input yet */ @@ -598,15 +592,6 @@ int i8042_tstc(struct stdio_dev *dev) { unsigned char scan_code = 0;

-#ifdef CONFIG_CONSOLE_CURSOR

•   if (--blink_count == 0) {
  

•           cursor_state ^= 1;
  

•           console_cursor(cursor_state);
  

•           blink_count = CONFIG_SYS_CONSOLE_BLINK_COUNT;
  

•           udelay(10);
  

•   }
  

-#endif

•   if ((in8(I8042_STS_REG) & STATUS_OBF) == 0) {
            return 0;
    } else {
  

@@ -635,14 +620,6 @@ int i8042_getc(struct stdio_dev *dev)

    while (kbd_input == -1) {
            while ((in8(I8042_STS_REG) & STATUS_OBF) == 0) {

-#ifdef CONFIG_CONSOLE_CURSOR

•                   if (--blink_count == 0) {
  

•                           cursor_state ^= 1;
  

•                           console_cursor(cursor_state);
  

•                           blink_count = CONFIG_SYS_CONSOLE_BLINK_COUNT;
  

•                   }
  

•                   udelay(10);
  

-#endif } scan_code = in8(I8042_DATA_REG); if (scan_code != 0xfa) diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c index aa7ca86..1b5c3e0 100644 --- a/drivers/video/cfb_console.c +++ b/drivers/video/cfb_console.c @@ -43,13 +43,6 @@

• VIDEO_TSTC_FCT - keyboard_tstc function
• VIDEO_GETC_FCT - keyboard_getc function

• • CONFIG_CONSOLE_CURSOR - on/off drawing cursor is done with

• •                         delay loop in VIDEO_TSTC_FCT (i8042)
    

• • CONFIG_SYS_CONSOLE_BLINK_COUNT - value for delay loop - blink rate
• • CONFIG_CONSOLE_TIME - display time/date in upper right

• •                         corner, needs CONFIG_CMD_DATE and
    

• •                         CONFIG_CONSOLE_CURSOR
    

  • CONFIG_VIDEO_LOGO - display Linux Logo in upper left corner.

  •                         Use CONFIG_SPLASH_SCREEN_ALIGN with
    

  •                         environment variable "splashpos" to place
    

@@ -198,9 +191,6 @@

/*

• Cursor definition:

• • CONFIG_CONSOLE_CURSOR: Uses a timer function (see drivers/input/i8042.c)

• •                    to let the cursor blink. Uses the macros
    

• •                    CURSOR_OFF and CURSOR_ON.
    

  • CONFIG_VIDEO_SW_CURSOR: Draws a cursor after the last character. No

  •                    blinking is provided. Uses the macros CURSOR_SET
    

  •                    and CURSOR_OFF.
    

@@ -210,42 +200,29 @@

•                    must disable the hardware register of the graphic
  

•                    chip. Otherwise a blinking field is displayed
  

*/ -#if !defined(CONFIG_CONSOLE_CURSOR) && \

• !defined(CONFIG_VIDEO_SW_CURSOR) && \
• !defined(CONFIG_VIDEO_HW_CURSOR)

+#if !defined(CONFIG_VIDEO_SW_CURSOR) && !defined(CONFIG_VIDEO_HW_CURSOR) /* no Cursor defined */ #define CURSOR_ON #define CURSOR_OFF #define CURSOR_SET #endif

-#if defined(CONFIG_CONSOLE_CURSOR) || defined(CONFIG_VIDEO_SW_CURSOR) -#if defined(CURSOR_ON) || \

•   (defined(CONFIG_CONSOLE_CURSOR) && defined(CONFIG_VIDEO_SW_CURSOR))
  

-#error only one of CONFIG_CONSOLE_CURSOR, CONFIG_VIDEO_SW_CURSOR, \

•   or CONFIG_VIDEO_HW_CURSOR can be defined
  

+#if defined(CONFIG_VIDEO_SW_CURSOR) +#if defined(CONFIG_VIDEO_HW_CURSOR)

#if defined(CURSOR_ON) || defined(CONFIG_VIDEO_HW_CURSOR) ?

I don't think so, but the whole thing is quite confusing.

Regards, Simon