public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/5] cleanup and refactor lcd.c
@ 2014-11-20 16:13 Nikita Kiryanov
  2014-11-20 16:13 ` [U-Boot] [PATCH 1/5] lcd: remove CONFIG_SYS_INVERT_COLORS Nikita Kiryanov
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Nikita Kiryanov @ 2014-11-20 16:13 UTC (permalink / raw)
  To: u-boot

This series is a first step towards an end goal of merging all CONFIG_LCD
related functionality into CONFIG_VIDEO code. My plan is to start by refactoring
lcd.c into something cleaner (less ifdefs) and more modular (split code into
multiple files), then possibly refactor CONFIG_VIDEO code if needed, and then
finally: move CONFIG_LCD related functionality over to CONFIG_VIDEO code,
replacing as much CONFIG_LCD related code with CONFIG_VIDEO related code as
possible.

This specific step eliminates some unused code and refactors lcd console stuff
into its own file. These changes were compile tested on arm and powerpc.

Cc: Anatolij Gustschin <agust@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Simon Glass <sjg@chromium.org>

Nikita Kiryanov (5):
  lcd: remove CONFIG_SYS_INVERT_COLORS
  lcd: cleanup lcd_drawchars
  mpc8xx_lcd: get rid of CONFIG_EDT32F10
  lcd: remove LCD_MONOCHROME
  lcd: refactor lcd console stuff into its own file

 common/Makefile            |   2 +-
 common/lcd.c               | 314 +++++----------------------------------------
 common/lcd_console.c       | 199 ++++++++++++++++++++++++++++
 drivers/video/mpc8xx_lcd.c |  49 +------
 drivers/video/pxa_lcd.c    |  15 ---
 include/configs/R360MPI.h  |   1 -
 include/lcd.h              |  13 +-
 include/lcd_console.h      |  14 ++
 8 files changed, 251 insertions(+), 356 deletions(-)
 create mode 100644 common/lcd_console.c
 create mode 100644 include/lcd_console.h

-- 
1.9.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 1/5] lcd: remove CONFIG_SYS_INVERT_COLORS
  2014-11-20 16:13 [U-Boot] [PATCH 0/5] cleanup and refactor lcd.c Nikita Kiryanov
@ 2014-11-20 16:13 ` Nikita Kiryanov
  2014-11-20 18:24   ` Simon Glass
  2014-11-20 16:13 ` [U-Boot] [PATCH 2/5] lcd: cleanup lcd_drawchars Nikita Kiryanov
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Nikita Kiryanov @ 2014-11-20 16:13 UTC (permalink / raw)
  To: u-boot

No one is using CONFIG_SYS_INVERT_COLORS; remove related code.

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Cc: Simon Glass <sjg@chromium.org>
Cc: Anatolij Gustschin <agust@denx.de>
---
 common/lcd.c               | 8 --------
 drivers/video/mpc8xx_lcd.c | 4 +---
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index 37147af..2f711c6 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -684,11 +684,7 @@ void bitmap_plot(int x, int y)
 			*(cmap + BMP_LOGO_OFFSET) = lut_entry;
 			cmap++;
 #else /* !CONFIG_ATMEL_LCD */
-#ifdef  CONFIG_SYS_INVERT_COLORS
-			*cmap++ = 0xffff - colreg;
-#else
 			*cmap++ = colreg;
-#endif
 #endif /* CONFIG_ATMEL_LCD */
 		}
 
@@ -966,11 +962,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 				( ((cte.red)   << 8) & 0xf800) |
 				( ((cte.green) << 3) & 0x07e0) |
 				( ((cte.blue)  >> 3) & 0x001f) ;
-#ifdef CONFIG_SYS_INVERT_COLORS
-			*cmap = 0xffff - colreg;
-#else
 			*cmap = colreg;
-#endif
 #if defined(CONFIG_MPC823)
 			cmap--;
 #else
diff --git a/drivers/video/mpc8xx_lcd.c b/drivers/video/mpc8xx_lcd.c
index 2bc3ceb..98b9f5e 100644
--- a/drivers/video/mpc8xx_lcd.c
+++ b/drivers/video/mpc8xx_lcd.c
@@ -373,9 +373,7 @@ lcd_setcolreg (ushort regno, ushort red, ushort green, ushort blue)
 	colreg = ((red   & 0x0F) << 8) |
 		 ((green & 0x0F) << 4) |
 		  (blue  & 0x0F) ;
-#ifdef	CONFIG_SYS_INVERT_COLORS
-	colreg ^= 0x0FFF;
-#endif
+
 	*cmap_ptr = colreg;
 
 	debug ("setcolreg: reg %2d @ %p: R=%02X G=%02X B=%02X => %02X%02X\n",
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 2/5] lcd: cleanup lcd_drawchars
  2014-11-20 16:13 [U-Boot] [PATCH 0/5] cleanup and refactor lcd.c Nikita Kiryanov
  2014-11-20 16:13 ` [U-Boot] [PATCH 1/5] lcd: remove CONFIG_SYS_INVERT_COLORS Nikita Kiryanov
@ 2014-11-20 16:13 ` Nikita Kiryanov
  2014-11-20 18:30   ` Simon Glass
  2014-11-20 16:13 ` [U-Boot] [PATCH 3/5] mpc8xx_lcd: get rid of CONFIG_EDT32F10 Nikita Kiryanov
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Nikita Kiryanov @ 2014-11-20 16:13 UTC (permalink / raw)
  To: u-boot

Remove code duplication from lcd_drawchars().

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Cc: Anatolij Gustschin <agust@denx.de>
---
 common/lcd.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index 2f711c6..ff53cf1 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -345,19 +345,7 @@ static void lcd_drawchars(ushort x, ushort y, uchar *str, int count)
 
 			*d++ = rest | (sym >> off);
 			rest = sym << (8-off);
-#elif LCD_BPP == LCD_COLOR8
-			for (c = 0; c < 8; ++c) {
-				*d++ = (bits & 0x80) ?
-						lcd_color_fg : lcd_color_bg;
-				bits <<= 1;
-			}
-#elif LCD_BPP == LCD_COLOR16
-			for (c = 0; c < 8; ++c) {
-				*d++ = (bits & 0x80) ?
-						lcd_color_fg : lcd_color_bg;
-				bits <<= 1;
-			}
-#elif LCD_BPP == LCD_COLOR32
+#else /* LCD_BPP == LCD_COLOR8 or LCD_COLOR16 or LCD_COLOR32 */
 			for (c = 0; c < 8; ++c) {
 				*d++ = (bits & 0x80) ?
 						lcd_color_fg : lcd_color_bg;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 3/5] mpc8xx_lcd: get rid of CONFIG_EDT32F10
  2014-11-20 16:13 [U-Boot] [PATCH 0/5] cleanup and refactor lcd.c Nikita Kiryanov
  2014-11-20 16:13 ` [U-Boot] [PATCH 1/5] lcd: remove CONFIG_SYS_INVERT_COLORS Nikita Kiryanov
  2014-11-20 16:13 ` [U-Boot] [PATCH 2/5] lcd: cleanup lcd_drawchars Nikita Kiryanov
@ 2014-11-20 16:13 ` Nikita Kiryanov
  2014-11-20 18:31   ` Simon Glass
  2014-11-20 16:13 ` [U-Boot] [PATCH 4/5] lcd: remove LCD_MONOCHROME Nikita Kiryanov
  2014-11-20 16:13 ` [U-Boot] [PATCH 5/5] lcd: refactor lcd console stuff into its own file Nikita Kiryanov
  4 siblings, 1 reply; 12+ messages in thread
From: Nikita Kiryanov @ 2014-11-20 16:13 UTC (permalink / raw)
  To: u-boot

No one is using CONFIG_EDT32F10; remove related code.

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Anatolij Gustschin <agust@denx.de>
---
 drivers/video/mpc8xx_lcd.c | 28 ----------------------------
 include/configs/R360MPI.h  |  1 -
 2 files changed, 29 deletions(-)

diff --git a/drivers/video/mpc8xx_lcd.c b/drivers/video/mpc8xx_lcd.c
index 98b9f5e..3ea240d 100644
--- a/drivers/video/mpc8xx_lcd.c
+++ b/drivers/video/mpc8xx_lcd.c
@@ -34,11 +34,6 @@
 #define CONFIG_LCD_INFO		/* Display Logo, (C) and system info	*/
 #endif
 
-#if defined(CONFIG_EDT32F10)
-#undef CONFIG_LCD_LOGO
-#undef CONFIG_LCD_INFO
-#endif
-
 /*----------------------------------------------------------------------*/
 #ifdef CONFIG_KYOCERA_KCS057QV1AJ
 /*
@@ -224,20 +219,6 @@ vidinfo_t panel_info = {
 };
 #endif /* CONFIG_OPTREX_BW */
 
-/*-----------------------------------------------------------------*/
-#ifdef CONFIG_EDT32F10
-/*
- * Emerging Display Technologies 320x240. Passive, monochrome, single scan.
- */
-#define LCD_BPP		LCD_MONOCHROME
-#define LCD_DF		10
-
-vidinfo_t panel_info = {
-    320, 240, 0, 0, CONFIG_SYS_HIGH, CONFIG_SYS_HIGH, CONFIG_SYS_HIGH, CONFIG_SYS_HIGH, CONFIG_SYS_LOW,
-    LCD_BPP,  0, 0, 0, 0, 33, 0, 0, 0
-};
-#endif
-
 /************************************************************************/
 /* ----------------- chipset specific functions ----------------------- */
 /************************************************************************/
@@ -305,7 +286,6 @@ void lcd_ctrl_init (void *lcdbase)
 	immr->im_clkrst.car_sccr &= ~0x1F;
 	immr->im_clkrst.car_sccr |= LCD_DF;	/* was 8 */
 
-#if !defined(CONFIG_EDT32F10)
 	/* Enable LCD on port D.
 	 */
 	immr->im_ioport.iop_pdpar |= 0x1FFF;
@@ -315,14 +295,6 @@ void lcd_ctrl_init (void *lcdbase)
 	 */
 	immr->im_cpm.cp_pbpar |= 0x00005001;
 	immr->im_cpm.cp_pbdir |= 0x00005001;
-#else
-	/* Enable LCD on port D.
-	 */
-	immr->im_ioport.iop_pdpar |= 0x1DFF;
-	immr->im_ioport.iop_pdpar &= ~0x0200;
-	immr->im_ioport.iop_pddir |= 0x1FFF;
-	immr->im_ioport.iop_pddat |= 0x0200;
-#endif
 
 	/* Load the physical address of the linear frame buffer
 	 * into the LCD controller.
diff --git a/include/configs/R360MPI.h b/include/configs/R360MPI.h
index 009d1cf..fbaf6a5 100644
--- a/include/configs/R360MPI.h
+++ b/include/configs/R360MPI.h
@@ -24,7 +24,6 @@
 
 #define CONFIG_LCD
 #define CONFIG_MPC8XX_LCD
-#undef  CONFIG_EDT32F10
 #define CONFIG_SHARP_LQ057Q3DC02
 
 #define	CONFIG_SPLASH_SCREEN
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 4/5] lcd: remove LCD_MONOCHROME
  2014-11-20 16:13 [U-Boot] [PATCH 0/5] cleanup and refactor lcd.c Nikita Kiryanov
                   ` (2 preceding siblings ...)
  2014-11-20 16:13 ` [U-Boot] [PATCH 3/5] mpc8xx_lcd: get rid of CONFIG_EDT32F10 Nikita Kiryanov
@ 2014-11-20 16:13 ` Nikita Kiryanov
  2014-11-20 18:32   ` Simon Glass
  2014-11-20 16:13 ` [U-Boot] [PATCH 5/5] lcd: refactor lcd console stuff into its own file Nikita Kiryanov
  4 siblings, 1 reply; 12+ messages in thread
From: Nikita Kiryanov @ 2014-11-20 16:13 UTC (permalink / raw)
  To: u-boot

No one is using LCD_MONOCHROME; remove related code.

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Anatolij Gustschin <agust@denx.de>
---
 common/lcd.c               | 30 ++----------------------------
 drivers/video/mpc8xx_lcd.c | 17 -----------------
 drivers/video/pxa_lcd.c    | 15 ---------------
 include/lcd.h              | 10 +---------
 4 files changed, 3 insertions(+), 69 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index ff53cf1..4c86f6c 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -96,10 +96,7 @@
 #define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * CONSOLE_ROWS)
 #define CONSOLE_SCROLL_SIZE	(CONSOLE_SIZE - CONSOLE_ROW_SIZE)
 
-#if LCD_BPP == LCD_MONOCHROME
-# define COLOR_MASK(c)		((c)	  | (c) << 1 | (c) << 2 | (c) << 3 | \
-				 (c) << 4 | (c) << 5 | (c) << 6 | (c) << 7)
-#elif (LCD_BPP == LCD_COLOR8) || (LCD_BPP == LCD_COLOR16) || \
+#if (LCD_BPP == LCD_COLOR8) || (LCD_BPP == LCD_COLOR16) || \
 	(LCD_BPP == LCD_COLOR32)
 # define COLOR_MASK(c)		(c)
 #else
@@ -312,10 +309,6 @@ static void lcd_drawchars(ushort x, ushort y, uchar *str, int count)
 	y += BMP_LOGO_HEIGHT;
 #endif
 
-#if LCD_BPP == LCD_MONOCHROME
-	ushort off  = x * (1 << LCD_BPP) % 8;
-#endif
-
 	dest = (uchar *)(lcd_base + y * lcd_line_length + x * NBITS(LCD_BPP)/8);
 
 	for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
@@ -329,33 +322,18 @@ static void lcd_drawchars(ushort x, ushort y, uchar *str, int count)
 		uchar *d = dest;
 #endif
 
-#if LCD_BPP == LCD_MONOCHROME
-		uchar rest = *d & -(1 << (8 - off));
-		uchar sym;
-#endif
 		for (i = 0; i < count; ++i) {
 			uchar c, bits;
 
 			c = *s++;
 			bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
 
-#if LCD_BPP == LCD_MONOCHROME
-			sym  = (COLOR_MASK(lcd_color_fg) & bits) |
-				(COLOR_MASK(lcd_color_bg) & ~bits);
-
-			*d++ = rest | (sym >> off);
-			rest = sym << (8-off);
-#else /* LCD_BPP == LCD_COLOR8 or LCD_COLOR16 or LCD_COLOR32 */
 			for (c = 0; c < 8; ++c) {
 				*d++ = (bits & 0x80) ?
 						lcd_color_fg : lcd_color_bg;
 				bits <<= 1;
 			}
-#endif
 		}
-#if LCD_BPP == LCD_MONOCHROME
-		*d  = rest | (*d & ((1 << (8 - off)) - 1));
-#endif
 	}
 }
 
@@ -442,11 +420,7 @@ int drv_lcd_init(void)
 /*----------------------------------------------------------------------*/
 void lcd_clear(void)
 {
-#if LCD_BPP == LCD_MONOCHROME
-	/* Setting the palette */
-	lcd_initcolregs();
-
-#elif LCD_BPP == LCD_COLOR8
+#if LCD_BPP == LCD_COLOR8
 	/* Setting the palette */
 	lcd_setcolreg(CONSOLE_COLOR_BLACK, 0, 0, 0);
 	lcd_setcolreg(CONSOLE_COLOR_RED, 0xFF, 0, 0);
diff --git a/drivers/video/mpc8xx_lcd.c b/drivers/video/mpc8xx_lcd.c
index 3ea240d..3c16bf6 100644
--- a/drivers/video/mpc8xx_lcd.c
+++ b/drivers/video/mpc8xx_lcd.c
@@ -357,23 +357,6 @@ lcd_setcolreg (ushort regno, ushort red, ushort green, ushort blue)
 
 /*----------------------------------------------------------------------*/
 
-#if LCD_BPP == LCD_MONOCHROME
-static
-void lcd_initcolregs (void)
-{
-	volatile immap_t *immr = (immap_t *) CONFIG_SYS_IMMR;
-	volatile cpm8xx_t *cp = &(immr->im_cpm);
-	ushort regno;
-
-	for (regno = 0; regno < 16; regno++) {
-		cp->lcd_cmap[regno * 2] = 0;
-		cp->lcd_cmap[(regno * 2) + 1] = regno & 0x0f;
-	}
-}
-#endif
-
-/*----------------------------------------------------------------------*/
-
 void lcd_enable (void)
 {
 	volatile immap_t *immr = (immap_t *) CONFIG_SYS_IMMR;
diff --git a/drivers/video/pxa_lcd.c b/drivers/video/pxa_lcd.c
index e19f6ac..f66f615 100644
--- a/drivers/video/pxa_lcd.c
+++ b/drivers/video/pxa_lcd.c
@@ -379,21 +379,6 @@ lcd_setcolreg (ushort regno, ushort red, ushort green, ushort blue)
 #endif /* LCD_COLOR8 */
 
 /*----------------------------------------------------------------------*/
-#if LCD_BPP == LCD_MONOCHROME
-void lcd_initcolregs (void)
-{
-	struct pxafb_info *fbi = &panel_info.pxa;
-	cmap = (ushort *)fbi->palette;
-	ushort regno;
-
-	for (regno = 0; regno < 16; regno++) {
-		cmap[regno * 2] = 0;
-		cmap[(regno * 2) + 1] = regno & 0x0f;
-	}
-}
-#endif /* LCD_MONOCHROME */
-
-/*----------------------------------------------------------------------*/
 __weak void lcd_enable(void)
 {
 }
diff --git a/include/lcd.h b/include/lcd.h
index 020d880..01609ac 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -359,15 +359,7 @@ void lcd_sync(void);
 /************************************************************************/
 /* ** CONSOLE CONSTANTS							*/
 /************************************************************************/
-#if LCD_BPP == LCD_MONOCHROME
-
-/*
- * Simple black/white definitions
- */
-# define CONSOLE_COLOR_BLACK	0
-# define CONSOLE_COLOR_WHITE	1	/* Must remain last / highest	*/
-
-#elif LCD_BPP == LCD_COLOR8
+#if LCD_BPP == LCD_COLOR8
 
 /*
  * 8bpp color definitions
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 5/5] lcd: refactor lcd console stuff into its own file
  2014-11-20 16:13 [U-Boot] [PATCH 0/5] cleanup and refactor lcd.c Nikita Kiryanov
                   ` (3 preceding siblings ...)
  2014-11-20 16:13 ` [U-Boot] [PATCH 4/5] lcd: remove LCD_MONOCHROME Nikita Kiryanov
@ 2014-11-20 16:13 ` Nikita Kiryanov
  2014-11-20 18:35   ` Simon Glass
  4 siblings, 1 reply; 12+ messages in thread
From: Nikita Kiryanov @ 2014-11-20 16:13 UTC (permalink / raw)
  To: u-boot

common/lcd.c is a mix of code portions that do different but related
things. To improve modularity, the various code portions should be split
into their own modules. Separate lcd console code into its own file.

In the process of making this move, some minor changes are introduced:
CONSOLE_ROWS and CONSOLE_COLS macros are replaced with variables which
are assigned using a new function lcd_init_console().

Minor changes were done to facilitate communication between lcd code
and lcd_console code, specifically in the introduction of lcd_console
functions set_console_col(), set_console_row(), lcd_get_screen_rows(),
lcd_get_screen_columns(), and lcd funcitons lcd_getbgcolor(),
lcd_getfgcolor().

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Simon Glass <sjg@chromium.org>
---
 common/Makefile       |   2 +-
 common/lcd.c          | 268 ++++++--------------------------------------------
 common/lcd_console.c  | 199 +++++++++++++++++++++++++++++++++++++
 include/lcd.h         |   3 +
 include/lcd_console.h |  14 +++
 5 files changed, 249 insertions(+), 237 deletions(-)
 create mode 100644 common/lcd_console.c
 create mode 100644 include/lcd_console.h

diff --git a/common/Makefile b/common/Makefile
index 6cc4de8..5f2350b 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -205,7 +205,7 @@ obj-$(CONFIG_CMD_KGDB) += kgdb.o kgdb_stubs.o
 obj-$(CONFIG_I2C_EDID) += edid.o
 obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-y += splash.o
-obj-$(CONFIG_LCD) += lcd.o
+obj-$(CONFIG_LCD) += lcd.o lcd_console.o
 obj-$(CONFIG_LYNXKDI) += lynxkdi.o
 obj-$(CONFIG_MENU) += menu.o
 obj-$(CONFIG_MODEM_SUPPORT) += modem.o
diff --git a/common/lcd.c b/common/lcd.c
index 4c86f6c..ea41ac2 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -72,42 +72,13 @@
 #define CONFIG_LCD_ALIGNMENT PAGE_SIZE
 #endif
 
-/* By default we scroll by a single line */
-#ifndef CONFIG_CONSOLE_SCROLL_LINES
-#define CONFIG_CONSOLE_SCROLL_LINES 1
-#endif
-
-/************************************************************************/
-/* ** CONSOLE DEFINITIONS & FUNCTIONS					*/
-/************************************************************************/
-#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
-# define CONSOLE_ROWS		((panel_info.vl_row-BMP_LOGO_HEIGHT) \
-					/ VIDEO_FONT_HEIGHT)
-#else
-# define CONSOLE_ROWS		(panel_info.vl_row / VIDEO_FONT_HEIGHT)
-#endif
-
-#define CONSOLE_COLS		(panel_info.vl_col / VIDEO_FONT_WIDTH)
-#define CONSOLE_ROW_SIZE	(VIDEO_FONT_HEIGHT * lcd_line_length)
-#define CONSOLE_ROW_FIRST	lcd_console_address
-#define CONSOLE_ROW_SECOND	(lcd_console_address + CONSOLE_ROW_SIZE)
-#define CONSOLE_ROW_LAST	(lcd_console_address + CONSOLE_SIZE \
-					- CONSOLE_ROW_SIZE)
-#define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * CONSOLE_ROWS)
-#define CONSOLE_SCROLL_SIZE	(CONSOLE_SIZE - CONSOLE_ROW_SIZE)
-
-#if (LCD_BPP == LCD_COLOR8) || (LCD_BPP == LCD_COLOR16) || \
-	(LCD_BPP == LCD_COLOR32)
-# define COLOR_MASK(c)		(c)
-#else
+#if (LCD_BPP != LCD_COLOR8) && (LCD_BPP != LCD_COLOR16) && \
+	(LCD_BPP != LCD_COLOR32)
 # error Unsupported LCD BPP.
 #endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static void lcd_drawchars(ushort x, ushort y, uchar *str, int count);
-static inline void lcd_putc_xy(ushort x, ushort y, uchar  c);
-
 static int lcd_init(void *lcdbase);
 
 static void *lcd_logo(void);
@@ -121,16 +92,12 @@ int lcd_line_length;
 
 char lcd_is_enabled = 0;
 
-static short console_col;
-static short console_row;
-
-static void *lcd_console_address;
 static void *lcd_base;			/* Start of framebuffer memory	*/
 
 static char lcd_flush_dcache;	/* 1 to flush dcache after each lcd update */
 
-/************************************************************************/
 
+/************************************************************************/
 /* Flush LCD activity to the caches */
 void lcd_sync(void)
 {
@@ -160,186 +127,14 @@ void lcd_set_flush_dcache(int flush)
 	lcd_flush_dcache = (flush != 0);
 }
 
-/*----------------------------------------------------------------------*/
-
-static void console_scrollup(void)
-{
-	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
-
-	/* Copy up rows ignoring those that will be overwritten */
-	memcpy(CONSOLE_ROW_FIRST,
-	       lcd_console_address + CONSOLE_ROW_SIZE * rows,
-	       CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows);
-
-	/* Clear the last rows */
-#if (LCD_BPP != LCD_COLOR32)
-	memset(lcd_console_address + CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows,
-		COLOR_MASK(lcd_color_bg),
-		CONSOLE_ROW_SIZE * rows);
-#else
-	u32 *ppix = lcd_console_address +
-		    CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows;
-	u32 i;
-	for (i = 0;
-	    i < (CONSOLE_ROW_SIZE * rows) / NBYTES(panel_info.vl_bpix);
-	    i++) {
-		*ppix++ = COLOR_MASK(lcd_color_bg);
-	}
-#endif
-	lcd_sync();
-	console_row -= rows;
-}
-
-/*----------------------------------------------------------------------*/
-
-static inline void console_back(void)
-{
-	if (--console_col < 0) {
-		console_col = CONSOLE_COLS-1 ;
-		if (--console_row < 0)
-			console_row = 0;
-	}
-
-	lcd_putc_xy(console_col * VIDEO_FONT_WIDTH,
-		console_row * VIDEO_FONT_HEIGHT, ' ');
-}
-
-/*----------------------------------------------------------------------*/
-
-static inline void console_newline(void)
-{
-	console_col = 0;
-
-	/* Check if we need to scroll the terminal */
-	if (++console_row >= CONSOLE_ROWS)
-		console_scrollup();
-	else
-		lcd_sync();
-}
-
-/*----------------------------------------------------------------------*/
-
-static void lcd_stub_putc(struct stdio_dev *dev, const char c)
-{
-	lcd_putc(c);
-}
-
-void lcd_putc(const char c)
-{
-	if (!lcd_is_enabled) {
-		serial_putc(c);
-
-		return;
-	}
-
-	switch (c) {
-	case '\r':
-		console_col = 0;
-
-		return;
-	case '\n':
-		console_newline();
-
-		return;
-	case '\t':	/* Tab (8 chars alignment) */
-		console_col +=  8;
-		console_col &= ~7;
-
-		if (console_col >= CONSOLE_COLS)
-			console_newline();
-
-		return;
-	case '\b':
-		console_back();
-
-		return;
-	default:
-		lcd_putc_xy(console_col * VIDEO_FONT_WIDTH,
-			console_row * VIDEO_FONT_HEIGHT, c);
-		if (++console_col >= CONSOLE_COLS)
-			console_newline();
-	}
-}
-
-/*----------------------------------------------------------------------*/
-
 static void lcd_stub_puts(struct stdio_dev *dev, const char *s)
 {
 	lcd_puts(s);
 }
 
-void lcd_puts(const char *s)
-{
-	if (!lcd_is_enabled) {
-		serial_puts(s);
-
-		return;
-	}
-
-	while (*s)
-		lcd_putc(*s++);
-
-	lcd_sync();
-}
-
-/*----------------------------------------------------------------------*/
-
-void lcd_printf(const char *fmt, ...)
-{
-	va_list args;
-	char buf[CONFIG_SYS_PBSIZE];
-
-	va_start(args, fmt);
-	vsprintf(buf, fmt, args);
-	va_end(args);
-
-	lcd_puts(buf);
-}
-
-/************************************************************************/
-/* ** Low-Level Graphics Routines					*/
-/************************************************************************/
-
-static void lcd_drawchars(ushort x, ushort y, uchar *str, int count)
-{
-	uchar *dest;
-	ushort row;
-
-#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
-	y += BMP_LOGO_HEIGHT;
-#endif
-
-	dest = (uchar *)(lcd_base + y * lcd_line_length + x * NBITS(LCD_BPP)/8);
-
-	for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
-		uchar *s = str;
-		int i;
-#if LCD_BPP == LCD_COLOR16
-		ushort *d = (ushort *)dest;
-#elif LCD_BPP == LCD_COLOR32
-		u32 *d = (u32 *)dest;
-#else
-		uchar *d = dest;
-#endif
-
-		for (i = 0; i < count; ++i) {
-			uchar c, bits;
-
-			c = *s++;
-			bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
-
-			for (c = 0; c < 8; ++c) {
-				*d++ = (bits & 0x80) ?
-						lcd_color_fg : lcd_color_bg;
-				bits <<= 1;
-			}
-		}
-	}
-}
-
-static inline void lcd_putc_xy(ushort x, ushort y, uchar c)
+static void lcd_stub_putc(struct stdio_dev *dev, const char c)
 {
-	lcd_drawchars(x, y, &c, 1);
+	lcd_putc(c);
 }
 
 /************************************************************************/
@@ -420,6 +215,8 @@ int drv_lcd_init(void)
 /*----------------------------------------------------------------------*/
 void lcd_clear(void)
 {
+	short console_rows, console_cols;
+
 #if LCD_BPP == LCD_COLOR8
 	/* Setting the palette */
 	lcd_setcolreg(CONSOLE_COLOR_BLACK, 0, 0, 0);
@@ -447,7 +244,7 @@ void lcd_clear(void)
 	/* set framebuffer to background color */
 #if (LCD_BPP != LCD_COLOR32)
 	memset((char *)lcd_base,
-		COLOR_MASK(lcd_color_bg),
+		lcd_color_bg,
 		lcd_line_length * panel_info.vl_row);
 #else
 	u32 *ppix = lcd_base;
@@ -455,16 +252,21 @@ void lcd_clear(void)
 	for (i = 0;
 	   i < (lcd_line_length * panel_info.vl_row)/NBYTES(panel_info.vl_bpix);
 	   i++) {
-		*ppix++ = COLOR_MASK(lcd_color_bg);
+		*ppix++ = lcd_color_bg;
 	}
 #endif
 #endif
 	/* Paint the logo and retrieve LCD base address */
 	debug("[LCD] Drawing the logo...\n");
-	lcd_console_address = lcd_logo();
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
+	console_rows = (panel_info.vl_row - BMP_LOGO_HEIGHT);
+	console_rows /= VIDEO_FONT_HEIGHT;
+#else
+	console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT;
+#endif
+	console_cols = panel_info.vl_col / VIDEO_FONT_WIDTH;
 
-	console_col = 0;
-	console_row = 0;
+	lcd_init_console(lcd_logo(), console_rows, console_cols);
 	lcd_sync();
 }
 
@@ -507,11 +309,11 @@ static int lcd_init(void *lcdbase)
 	lcd_enable();
 
 	/* Initialize the console */
-	console_col = 0;
+	set_console_col(0);
 #ifdef CONFIG_LCD_INFO_BELOW_LOGO
-	console_row = 7 + BMP_LOGO_HEIGHT / VIDEO_FONT_HEIGHT;
+	set_console_row(7 + BMP_LOGO_HEIGHT / VIDEO_FONT_HEIGHT);
 #else
-	console_row = 1;	/* leave 1 blank line below logo */
+	set_console_col(1);	/* leave 1 blank line below logo */
 #endif
 
 	return 0;
@@ -558,6 +360,11 @@ static void lcd_setfgcolor(int color)
 	lcd_color_fg = color;
 }
 
+int lcd_getfgcolor(void)
+{
+	return lcd_color_fg;
+}
+
 /*----------------------------------------------------------------------*/
 
 static void lcd_setbgcolor(int color)
@@ -565,6 +372,11 @@ static void lcd_setbgcolor(int color)
 	lcd_color_bg = color;
 }
 
+int lcd_getbgcolor(void)
+{
+	return lcd_color_bg;
+}
+
 /************************************************************************/
 /* ** Chipset depending Bitmap / Logo stuff...                          */
 /************************************************************************/
@@ -1061,8 +873,8 @@ static void *lcd_logo(void)
 	bitmap_plot(0, 0);
 
 #ifdef CONFIG_LCD_INFO
-	console_col = LCD_INFO_X / VIDEO_FONT_WIDTH;
-	console_row = LCD_INFO_Y / VIDEO_FONT_HEIGHT;
+	set_console_col(LCD_INFO_X / VIDEO_FONT_WIDTH);
+	set_console_row(LCD_INFO_Y / VIDEO_FONT_HEIGHT);
 	lcd_show_board_info();
 #endif /* CONFIG_LCD_INFO */
 
@@ -1097,12 +909,6 @@ static int on_splashimage(const char *name, const char *value, enum env_op op,
 U_BOOT_ENV_CALLBACK(splashimage, on_splashimage);
 #endif
 
-void lcd_position_cursor(unsigned col, unsigned row)
-{
-	console_col = min(col, CONSOLE_COLS - 1);
-	console_row = min(row, CONSOLE_ROWS - 1);
-}
-
 int lcd_get_pixel_width(void)
 {
 	return panel_info.vl_col;
@@ -1113,16 +919,6 @@ int lcd_get_pixel_height(void)
 	return panel_info.vl_row;
 }
 
-int lcd_get_screen_rows(void)
-{
-	return CONSOLE_ROWS;
-}
-
-int lcd_get_screen_columns(void)
-{
-	return CONSOLE_COLS;
-}
-
 #if defined(CONFIG_LCD_DT_SIMPLEFB)
 static int lcd_dt_simplefb_configure_node(void *blob, int off)
 {
diff --git a/common/lcd_console.c b/common/lcd_console.c
new file mode 100644
index 0000000..e6e1887
--- /dev/null
+++ b/common/lcd_console.c
@@ -0,0 +1,199 @@
+#include <common.h>
+#include <lcd.h>
+#include <video_font.h>		/* Get font data, width and height	*/
+
+static short console_cols;
+static short console_rows;
+static short console_curr_col;
+static short console_curr_row;
+static void *lcd_console_address;
+
+#define CONSOLE_ROW_FIRST	lcd_console_address
+#define CONSOLE_ROW_SIZE	(VIDEO_FONT_HEIGHT * lcd_line_length)
+#define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * console_rows)
+
+static void lcd_drawchars(ushort x, ushort y, uchar *str, int count)
+{
+	uchar *dest;
+	ushort row;
+
+	dest = (uchar *)(lcd_console_address +
+			 y * lcd_line_length + x * NBITS(LCD_BPP) / 8);
+	for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
+		uchar *s = str;
+		int i;
+#if LCD_BPP == LCD_COLOR16
+		ushort *d = (ushort *)dest;
+#elif LCD_BPP == LCD_COLOR32
+		u32 *d = (u32 *)dest;
+#else
+		uchar *d = dest;
+#endif
+
+		for (i = 0; i < count; ++i) {
+			uchar c, bits;
+
+			c = *s++;
+			bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
+
+			for (c = 0; c < 8; ++c) {
+				*d++ = (bits & 0x80) ?
+					lcd_getfgcolor() : lcd_getbgcolor();
+				bits <<= 1;
+			}
+		}
+	}
+}
+
+static inline void lcd_putc_xy(ushort x, ushort y, uchar c)
+{
+	lcd_drawchars(x, y, &c, 1);
+}
+
+static void console_scrollup(void)
+{
+	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
+
+	/* Copy up rows ignoring those that will be overwritten */
+	memcpy(CONSOLE_ROW_FIRST,
+	       lcd_console_address + CONSOLE_ROW_SIZE * rows,
+	       CONSOLE_ROW_SIZE - CONSOLE_ROW_SIZE * rows);
+
+	/* Clear the last rows */
+#if (LCD_BPP != LCD_COLOR32)
+	memset(lcd_console_address + CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows,
+	       lcd_getbgcolor(), CONSOLE_ROW_SIZE * rows);
+#else
+	u32 *ppix = lcd_console_address +
+		    CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows;
+	u32 i;
+	for (i = 0;
+	    i < (CONSOLE_ROW_SIZE * rows) / NBYTES(panel_info.vl_bpix);
+	    i++) {
+		*ppix++ = lcd_getbgcolor();
+	}
+#endif
+	lcd_sync();
+	console_curr_row -= rows;
+}
+
+static inline void console_back(void)
+{
+	if (--console_curr_col < 0) {
+		console_curr_col = console_cols - 1;
+		if (--console_curr_row < 0)
+			console_curr_row = 0;
+	}
+
+	lcd_putc_xy(console_curr_col * VIDEO_FONT_WIDTH,
+		    console_curr_row * VIDEO_FONT_HEIGHT, ' ');
+}
+
+static inline void console_newline(void)
+{
+	console_curr_col = 0;
+
+	/* Check if we need to scroll the terminal */
+	if (++console_curr_row >= console_rows)
+		console_scrollup();
+	else
+		lcd_sync();
+}
+
+void lcd_init_console(void *address, int rows, int cols)
+{
+	console_curr_col = 0;
+	console_curr_row = 0;
+	console_cols = cols;
+	console_rows = rows;
+	lcd_console_address = address;
+}
+
+void set_console_col(short col)
+{
+	console_curr_col = col;
+}
+
+void set_console_row(short row)
+{
+	console_curr_row = row;
+}
+
+void lcd_position_cursor(unsigned col, unsigned row)
+{
+	set_console_col(min(col, console_cols - 1));
+	set_console_row(min(row, console_rows - 1));
+}
+
+int lcd_get_screen_rows(void)
+{
+	return console_rows;
+}
+
+int lcd_get_screen_columns(void)
+{
+	return console_cols;
+}
+
+void lcd_putc(const char c)
+{
+	if (!lcd_is_enabled) {
+		serial_putc(c);
+
+		return;
+	}
+
+	switch (c) {
+	case '\r':
+		console_curr_col = 0;
+
+		return;
+	case '\n':
+		console_newline();
+
+		return;
+	case '\t':	/* Tab (8 chars alignment) */
+		console_curr_col +=  8;
+		console_curr_col &= ~7;
+
+		if (console_curr_col >= console_cols)
+			console_newline();
+
+		return;
+	case '\b':
+		console_back();
+
+		return;
+	default:
+		lcd_putc_xy(console_curr_col * VIDEO_FONT_WIDTH,
+			    console_curr_row * VIDEO_FONT_HEIGHT, c);
+		if (++console_curr_col >= console_cols)
+			console_newline();
+	}
+}
+
+void lcd_puts(const char *s)
+{
+	if (!lcd_is_enabled) {
+		serial_puts(s);
+
+		return;
+	}
+
+	while (*s)
+		lcd_putc(*s++);
+
+	lcd_sync();
+}
+
+void lcd_printf(const char *fmt, ...)
+{
+	va_list args;
+	char buf[CONFIG_SYS_PBSIZE];
+
+	va_start(args, fmt);
+	vsprintf(buf, fmt, args);
+	va_end(args);
+
+	lcd_puts(buf);
+}
diff --git a/include/lcd.h b/include/lcd.h
index 01609ac..8960b7a 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -12,6 +12,7 @@
 
 #ifndef _LCD_H_
 #define _LCD_H_
+#include <lcd_console.h>
 
 extern char lcd_is_enabled;
 
@@ -290,6 +291,8 @@ int lcd_get_screen_rows(void);
  */
 int lcd_get_screen_columns(void);
 
+int lcd_getbgcolor(void);
+int lcd_getfgcolor(void);
 /**
  * Set the position of the text cursor
  *
diff --git a/include/lcd_console.h b/include/lcd_console.h
new file mode 100644
index 0000000..928173d
--- /dev/null
+++ b/include/lcd_console.h
@@ -0,0 +1,14 @@
+/* By default we scroll by a single line */
+#ifndef CONFIG_CONSOLE_SCROLL_LINES
+#define CONFIG_CONSOLE_SCROLL_LINES 1
+#endif
+
+void lcd_init_console(void *address, int rows, int cols);
+void set_console_col(short col);
+void set_console_row(short row);
+void lcd_position_cursor(unsigned col, unsigned row);
+int lcd_get_screen_rows(void);
+int lcd_get_screen_columns(void);
+void lcd_putc(const char c);
+void lcd_puts(const char *s);
+void lcd_printf(const char *fmt, ...);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 1/5] lcd: remove CONFIG_SYS_INVERT_COLORS
  2014-11-20 16:13 ` [U-Boot] [PATCH 1/5] lcd: remove CONFIG_SYS_INVERT_COLORS Nikita Kiryanov
@ 2014-11-20 18:24   ` Simon Glass
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-11-20 18:24 UTC (permalink / raw)
  To: u-boot

On 20 November 2014 16:13, Nikita Kiryanov <nikita@compulab.co.il> wrote:
> No one is using CONFIG_SYS_INVERT_COLORS; remove related code.
>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Anatolij Gustschin <agust@denx.de>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 2/5] lcd: cleanup lcd_drawchars
  2014-11-20 16:13 ` [U-Boot] [PATCH 2/5] lcd: cleanup lcd_drawchars Nikita Kiryanov
@ 2014-11-20 18:30   ` Simon Glass
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-11-20 18:30 UTC (permalink / raw)
  To: u-boot

Hi Nikita,

On 20 November 2014 16:13, Nikita Kiryanov <nikita@compulab.co.il> wrote:
> Remove code duplication from lcd_drawchars().
>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Cc: Anatolij Gustschin <agust@denx.de>

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

Although I'm not sure it is an improvement - using a different type
for the variable based on an #ifdef is pretty horrible. Another option
would be to move the declarations down to above each loop.

> ---
>  common/lcd.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/common/lcd.c b/common/lcd.c
> index 2f711c6..ff53cf1 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -345,19 +345,7 @@ static void lcd_drawchars(ushort x, ushort y, uchar *str, int count)
>
>                         *d++ = rest | (sym >> off);
>                         rest = sym << (8-off);
> -#elif LCD_BPP == LCD_COLOR8
> -                       for (c = 0; c < 8; ++c) {
> -                               *d++ = (bits & 0x80) ?
> -                                               lcd_color_fg : lcd_color_bg;
> -                               bits <<= 1;
> -                       }
> -#elif LCD_BPP == LCD_COLOR16
> -                       for (c = 0; c < 8; ++c) {
> -                               *d++ = (bits & 0x80) ?
> -                                               lcd_color_fg : lcd_color_bg;
> -                               bits <<= 1;
> -                       }
> -#elif LCD_BPP == LCD_COLOR32
> +#else /* LCD_BPP == LCD_COLOR8 or LCD_COLOR16 or LCD_COLOR32 */
>                         for (c = 0; c < 8; ++c) {
>                                 *d++ = (bits & 0x80) ?
>                                                 lcd_color_fg : lcd_color_bg;
> --
> 1.9.1

Regards,
Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 3/5] mpc8xx_lcd: get rid of CONFIG_EDT32F10
  2014-11-20 16:13 ` [U-Boot] [PATCH 3/5] mpc8xx_lcd: get rid of CONFIG_EDT32F10 Nikita Kiryanov
@ 2014-11-20 18:31   ` Simon Glass
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-11-20 18:31 UTC (permalink / raw)
  To: u-boot

On 20 November 2014 16:13, Nikita Kiryanov <nikita@compulab.co.il> wrote:
> No one is using CONFIG_EDT32F10; remove related code.
>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Anatolij Gustschin <agust@denx.de>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 4/5] lcd: remove LCD_MONOCHROME
  2014-11-20 16:13 ` [U-Boot] [PATCH 4/5] lcd: remove LCD_MONOCHROME Nikita Kiryanov
@ 2014-11-20 18:32   ` Simon Glass
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-11-20 18:32 UTC (permalink / raw)
  To: u-boot

On 20 November 2014 16:13, Nikita Kiryanov <nikita@compulab.co.il> wrote:
> No one is using LCD_MONOCHROME; remove related code.
>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Anatolij Gustschin <agust@denx.de>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 5/5] lcd: refactor lcd console stuff into its own file
  2014-11-20 16:13 ` [U-Boot] [PATCH 5/5] lcd: refactor lcd console stuff into its own file Nikita Kiryanov
@ 2014-11-20 18:35   ` Simon Glass
  2014-11-21 10:42     ` Nikita Kiryanov
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Glass @ 2014-11-20 18:35 UTC (permalink / raw)
  To: u-boot

Hi Nikita,

On 20 November 2014 16:13, Nikita Kiryanov <nikita@compulab.co.il> wrote:
> common/lcd.c is a mix of code portions that do different but related
> things. To improve modularity, the various code portions should be split
> into their own modules. Separate lcd console code into its own file.
>
> In the process of making this move, some minor changes are introduced:
> CONSOLE_ROWS and CONSOLE_COLS macros are replaced with variables which
> are assigned using a new function lcd_init_console().
>
> Minor changes were done to facilitate communication between lcd code
> and lcd_console code, specifically in the introduction of lcd_console
> functions set_console_col(), set_console_row(), lcd_get_screen_rows(),
> lcd_get_screen_columns(), and lcd funcitons lcd_getbgcolor(),
> lcd_getfgcolor().
>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Cc: Anatolij Gustschin <agust@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Simon Glass <sjg@chromium.org>

To my mind this patch should be split - one that changes the code and
another to move part of it into a separate file. Also how about adding
function comments to the header file (lcd_console.h and lcd.h)?

Regards,
Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 5/5] lcd: refactor lcd console stuff into its own file
  2014-11-20 18:35   ` Simon Glass
@ 2014-11-21 10:42     ` Nikita Kiryanov
  0 siblings, 0 replies; 12+ messages in thread
From: Nikita Kiryanov @ 2014-11-21 10:42 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 11/20/2014 08:35 PM, Simon Glass wrote:
> Hi Nikita,
>
> On 20 November 2014 16:13, Nikita Kiryanov <nikita@compulab.co.il> wrote:
>> common/lcd.c is a mix of code portions that do different but related
>> things. To improve modularity, the various code portions should be split
>> into their own modules. Separate lcd console code into its own file.
>>
>> In the process of making this move, some minor changes are introduced:
>> CONSOLE_ROWS and CONSOLE_COLS macros are replaced with variables which
>> are assigned using a new function lcd_init_console().
>>
>> Minor changes were done to facilitate communication between lcd code
>> and lcd_console code, specifically in the introduction of lcd_console
>> functions set_console_col(), set_console_row(), lcd_get_screen_rows(),
>> lcd_get_screen_columns(), and lcd funcitons lcd_getbgcolor(),
>> lcd_getfgcolor().
>>
>> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
>> Cc: Anatolij Gustschin <agust@denx.de>
>> Cc: Wolfgang Denk <wd@denx.de>
>> Cc: Simon Glass <sjg@chromium.org>
>
> To my mind this patch should be split - one that changes the code and
> another to move part of it into a separate file. Also how about adding
> function comments to the header file (lcd_console.h and lcd.h)?

Sure, I can do that.
I also just noticed that I forgot the license headers in the new files,
so V2 coming up.

>
> Regards,
> Simon
>

-- 
Regards,
Nikita Kiryanov

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-11-21 10:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-20 16:13 [U-Boot] [PATCH 0/5] cleanup and refactor lcd.c Nikita Kiryanov
2014-11-20 16:13 ` [U-Boot] [PATCH 1/5] lcd: remove CONFIG_SYS_INVERT_COLORS Nikita Kiryanov
2014-11-20 18:24   ` Simon Glass
2014-11-20 16:13 ` [U-Boot] [PATCH 2/5] lcd: cleanup lcd_drawchars Nikita Kiryanov
2014-11-20 18:30   ` Simon Glass
2014-11-20 16:13 ` [U-Boot] [PATCH 3/5] mpc8xx_lcd: get rid of CONFIG_EDT32F10 Nikita Kiryanov
2014-11-20 18:31   ` Simon Glass
2014-11-20 16:13 ` [U-Boot] [PATCH 4/5] lcd: remove LCD_MONOCHROME Nikita Kiryanov
2014-11-20 18:32   ` Simon Glass
2014-11-20 16:13 ` [U-Boot] [PATCH 5/5] lcd: refactor lcd console stuff into its own file Nikita Kiryanov
2014-11-20 18:35   ` Simon Glass
2014-11-21 10:42     ` Nikita Kiryanov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox