| Message ID | 1357415148-9243-1-git-send-email-wd@denx.de |
|---|---|
| State | Accepted |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
Dear Anatolij, In message <1357415148-9243-1-git-send-email-wd@denx.de> you wrote: > lcd_color_fg and lcd_color_bg had to be declared in board specific > code, but were not actually used there; in addition, we have getter / > setter functions for these, which were not used either. > > Get rid of the global variables, and use the getter function where > needed (so far no setter calls are needed). A similar change could (and probably should?) be implemented for lcd_base, too - but thjis requires a little more code changes, plus the introduction of both getter and setter functions. Do you think this should be done? If so, I would prepare a patch... [Otherwise I'd save the effort...] Best regards, Wolfgang Denk
On 01/05/2013 08:50 PM, Wolfgang Denk wrote: > Dear Anatolij, > > In message <1357415148-9243-1-git-send-email-wd@denx.de> you wrote: >> lcd_color_fg and lcd_color_bg had to be declared in board specific >> code, but were not actually used there; in addition, we have getter / >> setter functions for these, which were not used either. >> >> Get rid of the global variables, and use the getter function where >> needed (so far no setter calls are needed). > A similar change could (and probably should?) be implemented for > lcd_base, too - but thjis requires a little more code changes, plus > the introduction of both getter and setter functions. > > Do you think this should be done? If so, I would prepare a patch... > [Otherwise I'd save the effort...] yes I think it should be done, unless Anatolij has a really good reason these variables are defined all over the place. I have patches ready to remove allot more of these globals. Yet I have to test them. (and as this is cross arch / several patches this gona take some time) Simon: I intend to remove you cursors stuff from tegra since it is not used anywhere. What is the intention of the cursor stuff? The amba driver is scheduled for removal as well since it is not used. Regards, Jeroen
Hello Wolfgang, On Sat, 05 Jan 2013 20:50:01 +0100 Wolfgang Denk <wd@denx.de> wrote: > Dear Anatolij, > > In message <1357415148-9243-1-git-send-email-wd@denx.de> you wrote: > > lcd_color_fg and lcd_color_bg had to be declared in board specific > > code, but were not actually used there; in addition, we have getter / > > setter functions for these, which were not used either. > > > > Get rid of the global variables, and use the getter function where > > needed (so far no setter calls are needed). > > A similar change could (and probably should?) be implemented for > lcd_base, too - but thjis requires a little more code changes, plus > the introduction of both getter and setter functions. > > Do you think this should be done? If so, I would prepare a patch... it should be done. And we probably do not need getter and setter functions, we can use gd->fb_addr directly. lcd_base is set to the value of gd->fb_base. Thanks, Anatolij
On Sat, Jan 5, 2013 at 11:45 AM, Wolfgang Denk <wd@denx.de> wrote: > lcd_color_fg and lcd_color_bg had to be declared in board specific > code, but were not actually used there; in addition, we have getter / > setter functions for these, which were not used either. > > Get rid of the global variables, and use the getter function where > needed (so far no setter calls are needed). > > Signed-off-by: Wolfgang Denk <wd@denx.de> > Cc: Alessandro Rubini <rubini@unipv.it> > Cc: Anatolij Gustschin <agust@denx.de> > Cc: Bo Shen <voice.shen@atmel.com> > Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Cc: Marek Vasut <marek.vasut@gmail.com> > Cc: Minkyu Kang <mk7.kang@samsung.com> > Cc: Nikita Kiryanov <nikita@compulab.co.il> > Cc: Simon Glass <sjg@chromium.org> > Cc: Stelian Pop <stelian@popies.net> > Cc: Tom Warren <twarren@nvidia.com> Acked-by: Simon Glass <sjg@chromium.org> > --- > arch/arm/cpu/pxa/pxafb.c | 2 -- > arch/powerpc/cpu/mpc8xx/lcd.c | 3 --- > board/mcc200/lcd.c | 3 --- > common/lcd.c | 7 ++++--- > drivers/video/amba.c | 2 -- > drivers/video/atmel_hlcdfb.c | 2 -- > drivers/video/atmel_lcdfb.c | 2 -- > drivers/video/exynos_fb.c | 2 -- > drivers/video/tegra.c | 4 +--- > include/lcd.h | 6 +++--- > 10 files changed, 8 insertions(+), 25 deletions(-)
Hi Jeroen, On Sun, Jan 6, 2013 at 2:43 PM, Jeroen Hofstee <jeroen@myspectrum.nl> wrote: > On 01/05/2013 08:50 PM, Wolfgang Denk wrote: >> >> Dear Anatolij, >> >> In message <1357415148-9243-1-git-send-email-wd@denx.de> you wrote: >>> >>> lcd_color_fg and lcd_color_bg had to be declared in board specific >>> code, but were not actually used there; in addition, we have getter / >>> setter functions for these, which were not used either. >>> >>> Get rid of the global variables, and use the getter function where >>> needed (so far no setter calls are needed). >> >> A similar change could (and probably should?) be implemented for >> lcd_base, too - but thjis requires a little more code changes, plus >> the introduction of both getter and setter functions. >> >> Do you think this should be done? If so, I would prepare a patch... >> [Otherwise I'd save the effort...] > > yes I think it should be done, unless Anatolij has a really good reason > these variables are defined all over the place. > > I have patches ready to remove allot more of these globals. Yet I have > to test them. (and as this is cross arch / several patches this gona take > some time) > > Simon: I intend to remove you cursors stuff from tegra since it is not > used anywhere. What is the intention of the cursor stuff? It can't be important - let's remove it. > > The amba driver is scheduled for removal as well since it is not used. > > Regards, > Jeroen > Regards, Simon
On 01/05/2013 08:45 PM, Wolfgang Denk wrote: > lcd_color_fg and lcd_color_bg had to be declared in board specific > code, but were not actually used there; in addition, we have getter / > setter functions for these, which were not used either. > > Get rid of the global variables, and use the getter function where > needed (so far no setter calls are needed). > > Signed-off-by: Wolfgang Denk <wd@denx.de> > Cc: Alessandro Rubini <rubini@unipv.it> > Cc: Anatolij Gustschin <agust@denx.de> > Cc: Bo Shen <voice.shen@atmel.com> > Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Cc: Marek Vasut <marek.vasut@gmail.com> > Cc: Minkyu Kang <mk7.kang@samsung.com> > Cc: Nikita Kiryanov <nikita@compulab.co.il> > Cc: Simon Glass <sjg@chromium.org> > Cc: Stelian Pop <stelian@popies.net> > Cc: Tom Warren <twarren@nvidia.com> > --- > Acked-by: Jeroen Hofstee <jeroen@myspectrum.nl>
Hello Wolfgang, On Sat, 5 Jan 2013 20:45:48 +0100 Wolfgang Denk <wd@denx.de> wrote: > lcd_color_fg and lcd_color_bg had to be declared in board specific > code, but were not actually used there; in addition, we have getter / > setter functions for these, which were not used either. > > Get rid of the global variables, and use the getter function where > needed (so far no setter calls are needed). > > Signed-off-by: Wolfgang Denk <wd@denx.de> > Cc: Alessandro Rubini <rubini@unipv.it> > Cc: Anatolij Gustschin <agust@denx.de> > Cc: Bo Shen <voice.shen@atmel.com> > Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Cc: Marek Vasut <marek.vasut@gmail.com> > Cc: Minkyu Kang <mk7.kang@samsung.com> > Cc: Nikita Kiryanov <nikita@compulab.co.il> > Cc: Simon Glass <sjg@chromium.org> > Cc: Stelian Pop <stelian@popies.net> > Cc: Tom Warren <twarren@nvidia.com> > --- > arch/arm/cpu/pxa/pxafb.c | 2 -- > arch/powerpc/cpu/mpc8xx/lcd.c | 3 --- > board/mcc200/lcd.c | 3 --- > common/lcd.c | 7 ++++--- > drivers/video/amba.c | 2 -- > drivers/video/atmel_hlcdfb.c | 2 -- > drivers/video/atmel_lcdfb.c | 2 -- > drivers/video/exynos_fb.c | 2 -- > drivers/video/tegra.c | 4 +--- > include/lcd.h | 6 +++--- > 10 files changed, 8 insertions(+), 25 deletions(-) Patch rebased and merged, thanks! Anatolij
diff --git a/arch/arm/cpu/pxa/pxafb.c b/arch/arm/cpu/pxa/pxafb.c index 987fa06..25747b1 100644 --- a/arch/arm/cpu/pxa/pxafb.c +++ b/arch/arm/cpu/pxa/pxafb.c @@ -333,8 +333,6 @@ void lcd_ctrl_init (void *lcdbase); void lcd_enable (void); int lcd_line_length; -int lcd_color_fg; -int lcd_color_bg; void *lcd_base; /* Start of framebuffer memory */ void *lcd_console_address; /* Start of console buffer */ diff --git a/arch/powerpc/cpu/mpc8xx/lcd.c b/arch/powerpc/cpu/mpc8xx/lcd.c index 4b88b21..4fd44ac 100644 --- a/arch/powerpc/cpu/mpc8xx/lcd.c +++ b/arch/powerpc/cpu/mpc8xx/lcd.c @@ -258,9 +258,6 @@ vidinfo_t panel_info = { int lcd_line_length; -int lcd_color_fg; -int lcd_color_bg; - /* * Frame buffer memory information */ diff --git a/board/mcc200/lcd.c b/board/mcc200/lcd.c index 893f4b7..0f3f585 100644 --- a/board/mcc200/lcd.c +++ b/board/mcc200/lcd.c @@ -70,9 +70,6 @@ vidinfo_t panel_info = { int lcd_line_length; -int lcd_color_fg; -int lcd_color_bg; - /* * Frame buffer memory information */ diff --git a/common/lcd.c b/common/lcd.c index 4778655..b67724e 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -97,6 +97,9 @@ static int lcd_getbgcolor(void); static void lcd_setfgcolor(int color); static void lcd_setbgcolor(int color); +static int lcd_color_fg; +static int lcd_color_bg; + char lcd_is_enabled = 0; static char lcd_flush_dcache; /* 1 to flush dcache after each lcd update */ @@ -532,12 +535,10 @@ static void lcd_setbgcolor(int color) /*----------------------------------------------------------------------*/ -#ifdef NOT_USED_SO_FAR -static int lcd_getfgcolor(void) +int lcd_getfgcolor(void) { return lcd_color_fg; } -#endif /* NOT_USED_SO_FAR */ /*----------------------------------------------------------------------*/ diff --git a/drivers/video/amba.c b/drivers/video/amba.c index ffa1c39..b4fb47d 100644 --- a/drivers/video/amba.c +++ b/drivers/video/amba.c @@ -29,8 +29,6 @@ /* These variables are required by lcd.c -- although it sets them by itself */ int lcd_line_length; -int lcd_color_fg; -int lcd_color_bg; void *lcd_base; void *lcd_console_address; short console_col; diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c index b10ca4b..e74eb65 100644 --- a/drivers/video/atmel_hlcdfb.c +++ b/drivers/video/atmel_hlcdfb.c @@ -30,8 +30,6 @@ #include <atmel_hlcdc.h> int lcd_line_length; -int lcd_color_fg; -int lcd_color_bg; void *lcd_base; /* Start of framebuffer memory */ void *lcd_console_address; /* Start of console buffer */ diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c index c02ffd8..d96f175 100644 --- a/drivers/video/atmel_lcdfb.c +++ b/drivers/video/atmel_lcdfb.c @@ -30,8 +30,6 @@ #include <atmel_lcdc.h> int lcd_line_length; -int lcd_color_fg; -int lcd_color_bg; void *lcd_base; /* Start of framebuffer memory */ void *lcd_console_address; /* Start of console buffer */ diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c index d9a3f9a..3dd6100 100644 --- a/drivers/video/exynos_fb.c +++ b/drivers/video/exynos_fb.c @@ -34,8 +34,6 @@ #include "exynos_fb.h" int lcd_line_length; -int lcd_color_fg; -int lcd_color_bg; void *lcd_base; void *lcd_console_address; diff --git a/drivers/video/tegra.c b/drivers/video/tegra.c index 750a283..3709d0b 100644 --- a/drivers/video/tegra.c +++ b/drivers/video/tegra.c @@ -61,8 +61,6 @@ enum { }; int lcd_line_length; -int lcd_color_fg; -int lcd_color_bg; void *lcd_base; /* Start of framebuffer memory */ void *lcd_console_address; /* Start of console buffer */ @@ -108,7 +106,7 @@ void lcd_toggle_cursor(void) for (i = 0; i < lcd_cursor_width; ++i) { color = *d; - color ^= lcd_color_fg; + color ^= lcd_getfgcolor(); *d = color; ++d; } diff --git a/include/lcd.h b/include/lcd.h index c24164a..7d8c41f 100644 --- a/include/lcd.h +++ b/include/lcd.h @@ -32,13 +32,11 @@ extern char lcd_is_enabled; extern int lcd_line_length; -extern int lcd_color_fg; -extern int lcd_color_bg; /* * Frame buffer memory information */ -extern void *lcd_base; /* Start of framebuffer memory */ +extern void *lcd_base; /* Start of framebuffer memory */ extern void *lcd_console_address; /* Start of console buffer */ extern short console_col; @@ -53,6 +51,8 @@ extern void lcd_setcolreg (ushort regno, ushort red, ushort green, ushort blue); extern void lcd_initcolregs (void); +extern int lcd_getfgcolor(void); + /* gunzip_bmp used if CONFIG_VIDEO_BMP_GZIP */ extern struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp); extern int bmp_display(ulong addr, int x, int y);
lcd_color_fg and lcd_color_bg had to be declared in board specific code, but were not actually used there; in addition, we have getter / setter functions for these, which were not used either. Get rid of the global variables, and use the getter function where needed (so far no setter calls are needed). Signed-off-by: Wolfgang Denk <wd@denx.de> Cc: Alessandro Rubini <rubini@unipv.it> Cc: Anatolij Gustschin <agust@denx.de> Cc: Bo Shen <voice.shen@atmel.com> Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com> Cc: Kyungmin Park <kyungmin.park@samsung.com> Cc: Marek Vasut <marek.vasut@gmail.com> Cc: Minkyu Kang <mk7.kang@samsung.com> Cc: Nikita Kiryanov <nikita@compulab.co.il> Cc: Simon Glass <sjg@chromium.org> Cc: Stelian Pop <stelian@popies.net> Cc: Tom Warren <twarren@nvidia.com> --- arch/arm/cpu/pxa/pxafb.c | 2 -- arch/powerpc/cpu/mpc8xx/lcd.c | 3 --- board/mcc200/lcd.c | 3 --- common/lcd.c | 7 ++++--- drivers/video/amba.c | 2 -- drivers/video/atmel_hlcdfb.c | 2 -- drivers/video/atmel_lcdfb.c | 2 -- drivers/video/exynos_fb.c | 2 -- drivers/video/tegra.c | 4 +--- include/lcd.h | 6 +++--- 10 files changed, 8 insertions(+), 25 deletions(-)