display and keypad support for nhk8815
Subsystems (3)
v2 cover display and keypad support for nhk8815
This patch set implements display and keypad support. To turn the display on I need the STMPE2401 port expander, set up as a separate driver in this patch set.
It's the same set I sent for RFC on Oct 9 2009, with the requested fixes. I also added commit messages to each of them, and an initial nhk-specific patch that was not submitted for RFC.
Detailed list of changes from V1 (as well as fixed message-id):
1/9 (nhk8815: fix i2c and other gpios at board_init time, not later):
new patch, needed to apply the other ones.
Please note that there is a block with local variables;
I know it is generally not advisable but they are really of strictly local use, I think they deserve their limited scope.
2/9 (video: add amba-clcd prime-cell):
include/amba_clcd.h: fixed white space (spaces before tab) better comments on the data structures
3/9 (include/nomadik.h: add physical address for cldc):
unchanged
4/9 (drivers/misc: add stmpe2401 port extender and keypad controller):
drivers/misc/stmpe2401.c: newlined all statements of if/else include/stmpe2401.h: white space fixes
====> Note: I did not turn it into a C structure, as those registers
are I2C registers, so they are plain numbers, not offsets in an I/O memory region where I use readl/writel
5/9 (nhk8815.h: define we need stmpe):
unchanged
6/9 (lcd: make 16bpp work):
declared the new "d" pointer in new ifdef block. I dislike it, but
my previous approach to avoid the ifdef was nacked. common/lcd.c is a mess, unfortunately.
I made "./MAKEALL arm" and "./MAKEALL ppc" with no adverse effects.
7/9 (nhk8815: added keypad):
layout (space, if blocks)
removed extra __LINE__ temporary printouts
added an header for local prototypes
8/9 (nhk8815: start lower in RAM, so the 800x480 frame buffer fits):
changed the comment in config.mk.
9/9 (nhk8815: added lcd support):
white space changes (tabified the structures) removed the __LINE__ temporary printouts
used the header introduced in 7/9 for local prototypes
Alessandro Rubini (9):
nhk8815: change the order of initialization
video: add amba-clcd prime-cell
include/nomadik.h: add physical address for cldc drivers/misc: add stmpe2401 port extender and keypad controller nhk8815.h: define we need stmpe
lcd: make 16bpp work
nhk8815: added keypad
nhk8815: start lower in RAM, so the 800x480 frame buffer fits
nhk8815: added lcd support
board/st/nhk8815/Makefile | 6 +- board/st/nhk8815/config.mk | 8 +- board/st/nhk8815/keypad.c | 99 +++++++++++++++++++ board/st/nhk8815/lcd.c | 87 ++++++++++++++++ board/st/nhk8815/nhk8815-devices.h | 8 ++ board/st/nhk8815/nhk8815.c | 38 +++++-- common/lcd.c | 27 ++++- drivers/misc/Makefile | 1 + drivers/misc/stmpe2401.c | 191 ++++++++++++++++++++++++++++++++++++ drivers/video/Makefile | 1 + drivers/video/amba.c | 86 ++++++++++++++++ include/amba_clcd.h | 85 ++++++++++++++++ include/configs/nhk8815.h | 20 ++++- include/nomadik.h | 1 + include/stmpe2401.h | 66 ++++++++++++ 15 files changed, 701 insertions(+), 23 deletions(-) create mode 100644 board/st/nhk8815/keypad.c create mode 100644 board/st/nhk8815/lcd.c create mode 100644 board/st/nhk8815/nhk8815-devices.h create mode 100644 drivers/misc/stmpe2401.c create mode 100644 drivers/video/amba.c create mode 100644 include/amba_clcd.h create mode 100644 include/stmpe2401.h
PATCH v2 1/9 nhk8815: change the order of initialization
From: Alessandro Rubini <[email protected]>
Some inizialization was in board_late_init(), but to satisfy drivers added in the next patches must be performed in normal board_init. This patch leaves board_late_init() empty, but later patches fill it.
--- board/st/nhk8815/nhk8815.c | 31 +++++++++++++++++++++---------- 1 files changed, 21 insertions(+), 10 deletions(-)
board/st/nhk8815/nhk8815.c +21 -10
@@ -60,22 +60,26 @@ int board_init(void)writel (0x02100551 ,NOMADIK_FSMC_BASE +0x04 );/* FSMC_BTR0 */ icache_enable (); -return 0 ; -} -int board_late_init (void ) -{ +/* +* Configure I2C pins, as we will use I2C in a later commit +*/ +/* Set the two I2C gpio lines to be gpio high */ nmk_gpio_set (__SCL,1 );nmk_gpio_set (__SDA,1 );nmk_gpio_dir (__SCL,1 );nmk_gpio_dir (__SDA,1 );nmk_gpio_af (__SCL,GPIO_GPIO );nmk_gpio_af (__SDA,GPIO_GPIO ); -/* Reset the I2C port expander, on GPIO77 */ -nmk_gpio_af (77 ,GPIO_GPIO ); -nmk_gpio_dir (77 ,1 ); -nmk_gpio_set (77 ,0 ); -udelay (10 ); -nmk_gpio_set (77 ,1 ); +/* Put the two I2C port expanders out of reset, on GPIO77 and 79 */ + { +int n[2 ]={77 ,79 }; +int i; +for (i =0 ; i <ARRAY_SIZE (n); i++) { +nmk_gpio_af (n[i],GPIO_GPIO ); +nmk_gpio_dir (n[i],1 ); +nmk_gpio_set (n[i],1 ); + } + }return 0 ; }
@@ -101,3 +105,10 @@ int board_eth_init(bd_t *bis) return rc; }#endif + +/* Initialization callback, from lib_arm/board.c */ +int board_late_init (void ) +{ +return 0 ; +} +
Signature · 1 line
1.6.0.2
PATCH v2 2/9 video: add amba-clcd prime-cell
In reply to Alessandro RubiniFrom: Alessandro Rubini <[email protected]>
This adds support for the CLCD logic cell. It accepts precompiled register values for specific configuration through a board-supplied data structure. It is used by the Nomadik nhk8815, added by a later patch in this series.
--- drivers/video/Makefile | 1 + drivers/video/amba.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++ include/amba_clcd.h | 85 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+), 0 deletions(-) create mode 100644 drivers/video/amba.c create mode 100644 include/amba_clcd.h
drivers/video/Makefile +1 -0
@@ -29,6 +29,7 @@ COBJS-$(CONFIG_ATI_RADEON_FB) += ati_radeon_fb.o COBJS-$(CONFIG_ATMEL_LCD) += atmel_lcdfb.o COBJS-$(CONFIG_CFB_CONSOLE) += cfb_console.o COBJS-$(CONFIG_S6E63D6) += s6e63d6.o +COBJS-$(CONFIG_VIDEO_AMBA) += amba.o COBJS-$(CONFIG_VIDEO_CT69000) += ct69000.o COBJS-$(CONFIG_VIDEO_MB862xx) += mb862xx.o COBJS-$(CONFIG_VIDEO_MX3) += mx3fb.o
drivers/video/amba.c new file +86 -0
86 lines not on this page · raw message
include/amba_clcd.h new file +85 -0
85 lines not on this page · raw message
Signature · 1 line
1.6.0.2
PATCH v2 3/9 include/nomadik.h: add physical address for cldc
In reply to Alessandro RubiniFrom: Alessandro Rubini <[email protected]>
--- include/nomadik.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
include/nomadik.h +1 -0
@@ -4,6 +4,7 @@#define __NOMADIK_H__/* Base addresses of our peripherals */ +#define NOMADIK_CLCDC_BASE 0x10120000/* CLCD Controller */ #define NOMADIK_SRC_BASE 0x101E0000/* System and Reset Cnt */ #define NOMADIK_PMU_BASE 0x101E9000/* Power Management Unit */ #define NOMADIK_MPMC_BASE 0x10110000/* SDRAM Controller */
Signature · 1 line
1.6.0.2
PATCH v2 4/9 drivers/misc: add stmpe2401 port extender and keypad controller
In reply to Alessandro RubiniFrom: Alessandro Rubini <[email protected]>
This driver is an i2c device acting as a port extender. Since the keypad can be configured to act on specific row and column lines, the specific setup is passed by the board file. This is used by the Nomadik nhk8815, through a later patch in this series.
--- drivers/misc/Makefile | 1 + drivers/misc/stmpe2401.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++ include/stmpe2401.h | 66 ++++++++++++++++ 3 files changed, 258 insertions(+), 0 deletions(-) create mode 100644 drivers/misc/stmpe2401.c create mode 100644 include/stmpe2401.h
drivers/misc/Makefile +1 -0
@@ -30,6 +30,7 @@ COBJS-$(CONFIG_DS4510) += ds4510.o COBJS-$(CONFIG_FSL_LAW) += fsl_law.o COBJS-$(CONFIG_NS87308) += ns87308.o COBJS-$(CONFIG_STATUS_LED) += status_led.o +COBJS-$(CONFIG_STMPE2401) += stmpe2401.o COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o COBJS := $(COBJS-y)
drivers/misc/stmpe2401.c new file +191 -0
191 lines not on this page · raw message
include/stmpe2401.h new file +66 -0
66 lines not on this page · raw message
Signature · 1 line
1.6.0.2
PATCH v2 5/9 nhk8815.h: define we need stmpe
In reply to Alessandro RubiniFrom: Alessandro Rubini <[email protected]>
--- include/configs/nhk8815.h | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
include/configs/nhk8815.h +6 -1
@@ -110,7 +110,7 @@#define CONFIG_PL01x_PORTS { (void *)CFG_SERIAL0, (void *)CFG_SERIAL1 }#define CONFIG_PL011_CLOCK 48000000 -/* i2c, for the port extenders (uses gpio.c in board directory) */ +/* i2c, for the stmpe2401 port extenders (uses gpio.c in board directory) */ #ifndef __ASSEMBLY__#include <asm/arch/gpio.h> #define CONFIG_CMD_I2C
@@ -126,6 +126,11 @@#define I2C_DELAY (udelay(2))#endif /* __ASSEMBLY__ */ +/* Activate port extenders and define their i2c address */ +#define CONFIG_STMPE2401 +#define STMPE0 0x43 +#define STMPE1 0x44 +/* Ethernet */ #define PCI_MEMORY_VADDR 0xe8000000#define PCI_IO_VADDR 0xee000000
Signature · 1 line
1.6.0.2
PATCH v2 6/9 lcd: make 16bpp work
In reply to Alessandro RubiniFrom: Alessandro Rubini <[email protected]>
Support for 16bpp was supposed to be in the code but was not working. This makes it work and has been tested in the nhk8815 board.
--- common/lcd.c | 27 +++++++++++++++++++++------ 1 files changed, 21 insertions(+), 6 deletions(-)
common/lcd.c +21 -6
@@ -63,7 +63,7 @@/************************************************************************/ #ifdef CONFIG_LCD_LOGO # include <bmp_logo.h> /* Get logo data, width and height */ -# if (CONSOLE_COLOR_WHITE >=BMP_LOGO_OFFSET ) +# if (CONSOLE_COLOR_WHITE >=BMP_LOGO_OFFSET ) && (LCD_BPP !=LCD_COLOR16 )# error Default Color Map overlaps with Logo Color Map# endif #endif
@@ -218,8 +218,12 @@ static void lcd_drawchars (ushort x, ushort y, uchar *str, int count)for (row=0 ; row <VIDEO_FONT_HEIGHT ; ++row, dest += lcd_line_length) {uchar *s = str; -uchar *d = dest;int i; +#if LCD_BPP ==LCD_COLOR16 +ushort *d = (ushort *)dest; +#else +uchar *d = dest; +#endif #if LCD_BPP ==LCD_MONOCHROME uchar rest = *d & -(1 << (8 -off));
@@ -244,9 +248,8 @@ static void lcd_drawchars (ushort x, ushort y, uchar *str, int count) bits <<= 1; }#elif LCD_BPP == LCD_COLOR16 -for (c=0 ; c<16 ; ++c) { - *d++ = (bits &0x80 ) ? - lcd_color_fg : lcd_color_bg; +for (c=0 ; c<8 ; ++c) { + *d++ = (bits &0x80 ) ?0xffff :0 ; bits <<=1 ; }#endif
@@ -529,6 +532,13 @@ void bitmap_plot (int x, int y) cmap = (ushort *)&(cp->lcd_cmap[BMP_LOGO_OFFSET *sizeof (ushort)]);#elif defined(CONFIG_ATMEL_LCD ) cmap = (uint *) (panel_info.mmio +ATMEL_LCDC_LUT (0 )); +#else +/* +* default case: generic system with no cmap (most likely 16bpp) +* We set cmap to the source palette, so no change is done. +* This avoids even more ifdef in the next stanza +*/ + cmap = bmp_logo_palette;#endif WATCHDOG_RESET ();
@@ -567,10 +577,15 @@ void bitmap_plot (int x, int y) } }else {/* true color mode */ +u16 col16; fb16 = (ushort *)(lcd_base + y * lcd_line_length + x);for (i=0 ; i<BMP_LOGO_HEIGHT ; ++i) {for (j=0 ; j<BMP_LOGO_WIDTH ; j++) { - fb16[j] = bmp_logo_palette[(bmap[j])]; + col16 = bmp_logo_palette[(bmap[j]-16 )]; + fb16[j] = + ((col16 &0x000F ) <<1 ) | + ((col16 &0x00F0 ) <<3 ) | + ((col16 &0x0F00 ) <<4 ); } bmap +=BMP_LOGO_WIDTH ; fb16 += panel_info.vl_col;
Signature · 1 line
1.6.0.2
PATCH v2 7/9 nhk8815: added keypad
In reply to Alessandro RubiniFrom: Alessandro Rubini <[email protected]>
This patch adds keypad support for the nhk8815 board, based on the stmpe2401 driver. The keypad hosts 16 keys, so each of them sends a string instead of a single key. The provided keymap is only an example and must be customized according to the use case.
--- board/st/nhk8815/Makefile | 5 ++- board/st/nhk8815/keypad.c | 99 ++++++++++++++++++++++++++++++++++++ board/st/nhk8815/nhk8815-devices.h | 7 +++ board/st/nhk8815/nhk8815.c | 4 ++ include/configs/nhk8815.h | 3 + 5 files changed, 117 insertions(+), 1 deletions(-) create mode 100644 board/st/nhk8815/keypad.c create mode 100644 board/st/nhk8815/nhk8815-devices.h
board/st/nhk8815/Makefile +4 -1
@@ -29,7 +29,10 @@ include $(TOPDIR)/config.mk LIB = $(obj)lib$(BOARD).a -COBJS := nhk8815.o +COBJS-y := nhk8815.o +COBJS-$(CONFIG_NHK8815_KEYPAD) += keypad.o + +COBJS := $(COBJS-y) SOBJS := platform.o SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
board/st/nhk8815/keypad.c new file +99 -0
99 lines not on this page · raw message
board/st/nhk8815/nhk8815-devices.h new file +7 -0
@@ -0,0 +1,7 @@ +#ifndef __NHK8815_DEVICES__ +#define __NHK8815_DEVICES__ + +/* Prototypes for functions exported by device files in this directory */ +extern int nhk8815_keypad_init (void );/* ./keypad.c */ + +#endif /* __NHK8815_DEVICES__ */
board/st/nhk8815/nhk8815.c +4 -0
@@ -29,6 +29,7 @@#include <netdev.h> #include <asm/io.h> #include <asm/arch/gpio.h> +#include "nhk8815-devices.h" DECLARE_GLOBAL_DATA_PTR ;
@@ -109,6 +110,9 @@ int board_eth_init(bd_t *bis)/* Initialization callback, from lib_arm/board.c */ int board_late_init (void ) { +#ifdef CONFIG_NHK8815_KEYPAD +nhk8815_keypad_init (); +#endif return 0 ; }
include/configs/nhk8815.h +3 -0
@@ -131,6 +131,9 @@#define STMPE0 0x43#define STMPE1 0x44 +/* Keypad using stmpe2401 */ +#define CONFIG_NHK8815_KEYPAD +/* Ethernet */ #define PCI_MEMORY_VADDR 0xe8000000#define PCI_IO_VADDR 0xee000000
Signature · 1 line
1.6.0.2
PATCH v2 8/9 nhk8815: start lower in RAM, so the 800x480 frame buffer fits
In reply to Alessandro RubiniFrom: Alessandro Rubini <[email protected]>
This simply moves u-boot to a lower address, as the frame buffer is allocated after u-boot itself in memory.
--- board/st/nhk8815/config.mk | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)
board/st/nhk8815/config.mk +3 -5
@@ -18,9 +18,7 @@ # along with this program; if not, write to the Free Software # Foundation, Inc., 59 Temple Place, Suite 330, Boston, # MA 02111-1307 USA -# -# -# image should be loaded at 0x01000000 -# -TEXT_BASE = 0x03F80000 +# Start 4MB before the end, as the frame buffer is allocate after +# u-boot. 800x480 @ 32bpp takes 1.5MB alone, so let's play safe. +TEXT_BASE = 0x03c00000
Signature · 1 line
1.6.0.2
PATCH v2 9/9 nhk8815: added lcd support
In reply to Alessandro RubiniFrom: Alessandro Rubini <[email protected]>
This adds lcd support for the board. It includes defines for 32-bit parameter as well, although support for LCD_COLOR32 is not yet in u-boot. This uses the stmpe2401 to turn on display backlight.
--- board/st/nhk8815/Makefile | 1 + board/st/nhk8815/lcd.c | 87 ++++++++++++++++++++++++++++++++++++ board/st/nhk8815/nhk8815-devices.h | 1 + board/st/nhk8815/nhk8815.c | 3 + include/configs/nhk8815.h | 10 ++++ 5 files changed, 102 insertions(+), 0 deletions(-) create mode 100644 board/st/nhk8815/lcd.c
board/st/nhk8815/Makefile +1 -0
@@ -31,6 +31,7 @@ LIB = $(obj)lib$(BOARD).a COBJS-y := nhk8815.o COBJS-$(CONFIG_NHK8815_KEYPAD) += keypad.o +COBJS-$(CONFIG_LCD) += lcd.o COBJS := $(COBJS-y) SOBJS := platform.o
board/st/nhk8815/lcd.c new file +87 -0
87 lines not on this page · raw message
board/st/nhk8815/nhk8815-devices.h +1 -0
@@ -3,5 +3,6 @@/* Prototypes for functions exported by device files in this directory */ extern int nhk8815_keypad_init (void );/* ./keypad.c */ +extern int nhk8815_backlight_on (void );/* in ./lcd.c */ #endif /* __NHK8815_DEVICES__ */
board/st/nhk8815/nhk8815.c +3 -0
@@ -113,6 +113,9 @@ int board_late_init(void)#ifdef CONFIG_NHK8815_KEYPAD nhk8815_keypad_init ();#endif +#ifdef CONFIG_LCD +nhk8815_backlight_on (); +#endif return 0 ; }
include/configs/nhk8815.h +10 -0
@@ -134,6 +134,16 @@/* Keypad using stmpe2401 */ #define CONFIG_NHK8815_KEYPAD +/* Display support */ +#define CONFIG_LCD +#define CONFIG_LCD_LOGO +#define CONFIG_LCD_INFO_BELOW_LOGO +#define CONFIG_SYS_WHITE_ON_BLACK +#define LCD_BPP LCD_COLOR16 +#define CONFIG_VIDEO_AMBA +#define CONFIG_SYS_CONSOLE_IS_IN_ENV 1 +#define CONFIG_CONSOLE_MUX +/* Ethernet */ #define PCI_MEMORY_VADDR 0xe8000000#define PCI_IO_VADDR 0xee000000
Signature · 1 line
1.6.0.2
PATCH v2 2/9 video: add amba-clcd prime-cell
In reply to a message not in the archiveI'm sorry I forgot a few extra prints in there:
Review
Quoted 4 lines
(quoted diff) +3
-0+ printf("%s:%s - %i %i %i %i\n", __FILE__, __func__, + regno, red, green, blue); + printf("%s:%s\n", __FILE__, __func__);
[and more]
If there are no other issues and the patch set is acceptable I'll put them on a public git to avoid more list traffic without the prints. If there are other issues, I'll fix this as well in V3.
Sorry for that
/alessandro
PATCH v2 2/9 video: add amba-clcd prime-cell
In reply to Alessandro RubiniHello Alessandro,
Alessandro Rubini <[email protected]> wrote:
<snip>
Quoted 6 lines
include/amba_clcd.h new file +0
-0@@ -0,0 +1,85 @@
...
include/amba_clcd.h line 22
Quoted 9 lines
include/amba_clcd.h new file +9
-0+/* +* CLCD Controller Internal Register addresses +*/ +#define CLCD_TIM0 0x00000000 +#define CLCD_TIM1 0x00000004 +#define CLCD_TIM2 0x00000008 +#define CLCD_TIM3 0x0000000c +#define CLCD_UBAS 0x00000010 +#define CLCD_LBAS 0x00000014
CLCD_LBAS isn't used in the code, please remove it.
...
include/amba_clcd.h line 48
Quoted 13 lines
include/amba_clcd.h new file +13
-0+#define CLCD_STAT 0x00000020 +#define CLCD_INTR 0x00000024 +#define CLCD_UCUR 0x00000028 +#define CLCD_LCUR 0x0000002C +#define CLCD_PALL 0x00000200 +#define CLCD_PALETTE 0x00000200 + +#define TIM2_CLKSEL (1 << 5) +#define TIM2_IVS (1 << 11) +#define TIM2_IHS (1 << 12) +#define TIM2_IPC (1 << 13) +#define TIM2_IOE (1 << 14) +#define TIM2_BCD (1 << 26)
all these defines above are not used in the code,
please remove them.
Quoted 9 lines
include/amba_clcd.h new file +9
-0+#define CNTL_LCDEN (1 << 0) +#define CNTL_LCDBPP1 (0 << 1) +#define CNTL_LCDBPP2 (1 << 1) +#define CNTL_LCDBPP4 (2 << 1) +#define CNTL_LCDBPP8 (3 << 1) +#define CNTL_LCDBPP16 (4 << 1) +#define CNTL_LCDBPP16_565 (6 << 1) +#define CNTL_LCDBPP24 (5 << 1) +#define CNTL_LCDBW (1 << 4)
include/amba_clcd.h line 68
Quoted 10 lines
include/amba_clcd.h new file +10
-0+#define CNTL_LCDTFT (1 << 5) +#define CNTL_LCDMONO8 (1 << 6) +#define CNTL_LCDDUAL (1 << 7) +#define CNTL_BGR (1 << 8) +#define CNTL_BEBO (1 << 9) +#define CNTL_BEPO (1 << 10) +#define CNTL_LCDPWR (1 << 11) +#define CNTL_LCDVCOMP (x) ((x) << 12) +#define CNTL_LDMAFIFOTIME (1 << 15) +#define CNTL_WATERMARK (1 << 16)
It seems that only CNTL_LCDEN is uses in the code, please remove other unused defines, too.
...
include/amba_clcd.h line 85
Quoted 6 lines
include/amba_clcd.h new file +6
-0+/* u-boot specific: cooked information (not used at this point) */ +struct clcd_params { +int left_margin, right_margin, upper_margin, lower_margin; +int hsync_len, vsync_len; +int sync, vmode; +};
this is also not used, please do not add unused code. I couldn't comment on this in the first patch version, sorry.
Best regards,
Anatolij
PATCH v2 2/9 video: add amba-clcd prime-cell
Hello
Quoted 3 lines
Quoted 1 line
+#define
CLCD_LBAS0x00000014
CLCD_LBASisn't used in the code, please remove it.
I disagree for two reasons: the register exists in the logic block, and the code is copied from a linux header (as noted in the file itself).
Since the clcd block is not something designed for the nomadik but the standard primecell, I don't think trimming its definition is useful; other boards can benefit from the header being complete.
Quoted 8 lines
include/amba_clcd.h line 85
Quoted 6 lines
include/amba_clcd.h new file +6
-0+/* u-boot specific: cooked information (not used at this point) */ +struct clcd_params { +int left_margin, right_margin, upper_margin, lower_margin; +int hsync_len, vsync_len; +int sync, vmode; +};this is also not used, please do not add unused code.
I planned to used it, and this is the complete information that's needed, and I plan to implement it sooner or later. Nonetheless, I understand your request to remove it and I have no problems here.
Quoted 1 line
I couldn't comment on this in the first patch version, sorry.
Ok, no problem.
/alessandro
PATCH v2 6/9 lcd: make 16bpp work
In reply to Alessandro RubiniAlessandro Rubini <[email protected]> wrote:
<snip>
common/lcd.c static void lcd_drawchars (ushort x, ushort y, uchar *str, int count) · line 252
Quoted 6 lines
#elif
LCD_BPP==LCD_COLOR16common/lcd.c +2
-3-for (c=0 ; c<16 ; ++c) { - *d++ = (bits &0x80 ) ? - lcd_color_fg : lcd_color_bg; +for (c=0 ; c<8 ; ++c) { + *d++ = (bits &0x80 ) ?0xffff :0 ;
I think we should use 'lcd_color_fg' and 'lcd_color_bg' here.
Best regards,
Anatolij
PATCH v2 2/9 video: add amba-clcd prime-cell
In reply to Anatolij GustschinDear Alessandro,
Quoted 24 lines · In message <20091129222444.2e454083@wker> Anatolij Gustschin wrote:
Hello Alessandro,
Alessandro Rubini <[email protected]> wrote:
<snip>
Quoted 6 lines
include/amba_clcd.h new file +0
-0@@ -0,0 +1,85 @@...
include/amba_clcd.h line 22
Quoted 9 lines
include/amba_clcd.h new file +9
-0+/* +* CLCD Controller Internal Register addresses +*/ +#define CLCD_TIM0 0x00000000 +#define CLCD_TIM1 0x00000004 +#define CLCD_TIM2 0x00000008 +#define CLCD_TIM3 0x0000000c +#define CLCD_UBAS 0x00000010 +#define CLCD_LBAS 0x00000014
CLCD_LBASisn't used in the code, please remove it.
Quoted 5 lines
...
include/amba_clcd.h line 39
Quoted 4 lines
include/amba_clcd.h new file +4
-0+#define CLCD_STAT 0x00000020 +#define CLCD_INTR 0x00000024 +#define CLCD_UCUR 0x00000028 +#define CLCD_LCUR 0x0000002C
Actually such a register descriptiion must not be implemented by offset definitions at all. Please define a C struct, and access it using appropriate accessor functions only.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de It is necessary to have purpose. -- Alice #1, "I, Mudd", stardate 4513.3
PATCH v2 2/9 video: add amba-clcd prime-cell
In reply to Alessandro RubiniAlessandro Rubini <[email protected]> wrote:
...
Quoted 10 lines
Quoted 3 lines
Quoted 1 line
+#define
CLCD_LBAS0x00000014
CLCD_LBASisn't used in the code, please remove it.I disagree for two reasons: the register exists in the logic block, and the code is copied from a linux header (as noted in the file itself).
Since the clcd block is not something designed for the nomadik but the standard primecell, I don't think trimming its definition is useful; other boards can benefit from the header being complete.
I understand your arguments. If Wolfgang will not reject my pull request, I'm fine with it. Could you please use something like:
struct clcd_regs {
u32 tim0;
u32 tim1;
u32 tim2;
u32 tim3;
u32 ubas;
u32 lbas;
#if !defined(CONFIG_ARCH_VERSATILE) && !defined(CONFIG_ARCH_REALVIEW)
u32 ienb;
u32 cntl;#else
/*
* Someone rearranged these two registers on the Versatile
* platform...
*/
u32 cntl; u32 ienb; #endif u32 stat; u32 intr; u32 ucur; u32 lcur; u32 res[0x74]; u32 pal; };
in the amba_clcd header?
And then in the code something like:
struct clcd_regs *regs; ... ... regs = (struct clcd_regs *)NOMADIK_CLCDC_BASE; ... writel(regval->tim0, ®s->tim0); ...
Thanks,
Anatolij
PATCH v2 2/9 video: add amba-clcd prime-cell
Quoted 2 lines
I understand your arguments. If Wolfgang will not reject my pull request, I'm fine with it. Could you please use something like:
Ok, I'll rearrange as a data structure as you both suggest.
[I noted you already did the work for me in your message, thanks for that]
/alessandro
PATCH v2 2/9 video: add amba-clcd prime-cell
In reply to Alessandro RubiniDear Alessandro Rubini,
Quoted 5 lines · In message <[email protected]> you wrote:
I'm sorry I forgot a few extra prints in there:
Quoted 2 lines
(quoted diff) +2
-0+ printf("%s:%s - %i %i %i %i\n", __FILE__, __func__, + regno, red, green, blue);
Quoted 7 lines
Review
Quoted 1 line
(quoted diff) +1
-0+ printf("%s:%s\n", __FILE__, __func__);[and more]
If there are no other issues and the patch set is acceptable I'll put them on a public git to avoid more list traffic without the prints. If there are other issues, I'll fix this as well in V3.
If you want this included into mainline, you have to post here. I will not accept any patches that have not been posted on this list (and thus found their way into the list archive).
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de There's no sense in being precise when you don't even know what you're talking about. -- John von Neumann