Hi Simon,

On 01/31/2015 02:24 AM, Simon Glass wrote:
Hi Nikita,

On 29 January 2015 at 04:21, Nikita Kiryanov <nik...@compulab.co.il> wrote:
configuration_get_cmap() is multiple platform specific functions stuffed into
one function. Split it into multiple versions, and move each version to the
appropriate driver to reduce the #ifdef complexity.

Signed-off-by: Nikita Kiryanov <nik...@compulab.co.il>
Cc: Bo Shen <voice.s...@atmel.com>
Cc: Simon Glass <s...@chromium.org>
Cc: Anatolij Gustschin <ag...@denx.de>
---
  common/lcd.c                 | 19 -------------------
  drivers/video/atmel_hlcdfb.c | 13 +++++++++++++
  drivers/video/atmel_lcdfb.c  |  5 +++++
  drivers/video/exynos_fb.c    |  9 +++++++++
  drivers/video/mpc8xx_lcd.c   |  7 +++++++
  drivers/video/pxa_lcd.c      |  6 ++++++
  include/lcd.h                |  9 +++++++++
  7 files changed, 49 insertions(+), 19 deletions(-)

Reviewed-by: Simon Glass <s...@chromium.org>

See suggestion below.

[snip]
diff --git a/include/lcd.h b/include/lcd.h
index fbba6a2..838f645 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -42,13 +42,17 @@ void lcd_set_flush_dcache(int flush);

  #if defined CONFIG_MPC823
  #include <mpc823_lcd.h>
+ushort *configuration_get_cmap(void);
  #elif defined(CONFIG_CPU_PXA25X) || defined(CONFIG_CPU_PXA27X) || \
         defined CONFIG_CPU_MONAHANS
  #include <pxa_lcd.h>
+ushort *configuration_get_cmap(void);
  #elif defined(CONFIG_ATMEL_LCD) || defined(CONFIG_ATMEL_HLCD)
  #include <atmel_lcd.h>
+ushort *configuration_get_cmap(void);
  #elif defined(CONFIG_EXYNOS_FB)
  #include <exynos_lcd.h>
+ushort *configuration_get_cmap(void);
  #else
  typedef struct vidinfo {
         ushort  vl_col;         /* Number of columns (i.e. 160) */
@@ -60,6 +64,11 @@ typedef struct vidinfo {

         void    *priv;          /* Pointer to driver-specific data */
  } vidinfo_t;
+
+static inline ushort *configuration_get_cmap(void)
+{
+       return panel_info.cmap;
+}
  #endif

I'd argue for dropping the inline version and just having the same
prototype in each case.

Are you suggesting something along the lines of:

static ushort *configuration_get_cmap(void)
{
       return panel_info.cmap;
}
#endif

ushort *configuration_get_cmap(void);

?

This is something I considered while preparing this, but I was under the 
impression
that this isn't well defined by the standard. However, I actually have 
confirmation
now that it is in fact defined behavior[1], so I will submit a V2 for this 
(even though
I expect all of the above to disappear from lcd.h in the next refactor series).

[1] 
http://stackoverflow.com/questions/28264874/non-static-function-prototype-follows-static-function-declaration


Regards,
Simon


--
Regards,
Nikita Kiryanov
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to