From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bo Shen Date: Fri, 23 Jan 2015 09:20:39 +0800 Subject: [U-Boot] [PATCH] lcd: fix console address is not initialized In-Reply-To: <54C0F6B4.1000208@compulab.co.il> References: <1421815024-24771-1-git-send-email-voice.shen@atmel.com> <54C0F6B4.1000208@compulab.co.il> Message-ID: <54C1A1E7.4090208@atmel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Nikita Kiryanov, On 01/22/2015 09:10 PM, Nikita Kiryanov wrote: > Hi Bo, > > On 01/21/2015 06:37 AM, Bo Shen wrote: >> This commit 904672e (lcd: refactor lcd console stuff into its >> own file), which cause lcd console address is not initialized. > > Based on your fix, I'm certain that the bug was introduced in a > previous patch, perhaps 140beb9 (lcd: expand console api). > > Also, can you provide a more detailed explanation of when this > happens and how? It will cause the system hang. Before this patch, lcd_logo -> lcd_show_board_info (CONFIG_LCD_INFO) -> .. -> lcd_drawchars. It has the following lines: --->8--- dest = (uchar *)(lcd_base + y * lcd_line_length + x * NBITS(LCD_BPP)/8); ---8<--- while with the patch, --->8--- dest = (uchar *)(lcd_console_address + y * lcd_line_length + x * NBITS(LCD_BPP) / 8); ---8<--- As the lcd_console_address is initialized after lcd_logo return, so the lcd_console_address is not initialized, it is 0. When try to write to address 0, the system hang. >> >> This patch split lcd console address initialize and lcd logo >> display into two functions. >> >> Signed-off-by: Bo Shen >> --- >> >> common/lcd.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/common/lcd.c b/common/lcd.c >> index cc34b8a..f435e2a 100644 >> --- a/common/lcd.c >> +++ b/common/lcd.c >> @@ -82,7 +82,8 @@ DECLARE_GLOBAL_DATA_PTR; >> >> static int lcd_init(void *lcdbase); >> >> -static void *lcd_logo(void); >> +static void lcd_logo(void); >> +static void *lcd_console_address(void); >> >> static void lcd_setfgcolor(int color); >> static void lcd_setbgcolor(int color); >> @@ -268,7 +269,8 @@ void lcd_clear(void) >> console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT; >> #endif >> console_cols = panel_info.vl_col / VIDEO_FONT_WIDTH; >> - lcd_init_console(lcd_logo(), console_rows, console_cols); >> + lcd_init_console(lcd_console_address(), console_rows, console_cols); >> + lcd_logo(); >> lcd_sync(); >> } >> >> @@ -849,7 +851,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y) >> } >> #endif >> >> -static void *lcd_logo(void) >> +static void lcd_logo(void) >> { >> #ifdef CONFIG_SPLASH_SCREEN >> char *s; >> @@ -879,7 +881,10 @@ static void *lcd_logo(void) >> lcd_set_row(LCD_INFO_Y / VIDEO_FONT_HEIGHT); >> lcd_show_board_info(); >> #endif /* CONFIG_LCD_INFO */ >> +} >> >> +static void *lcd_console_address(void) >> +{ >> #if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO) >> return (void *)((ulong)lcd_base + BMP_LOGO_HEIGHT * >> lcd_line_length); >> #else >> > > I would like to see some mention of why it's ok to redefine the console > address in such a way. At first glance it looks like there is a slight > change of > behavior (the value no longer depends on whether splash image was loaded > or not), but it seems to be ok if you go over the various cases. The only > instance where I see the difference could manifest itself is if lcd console > would start writing to the frame buffer while the splash screen is still > on, > but that doesn't look good regardless of where the console starts, so > I'm ok > with that. > Thanks for reminder this, I will check whether this will break the splash screen. Best Regards, Bo Shen