Hi Bo,

On 02/04/2015 09:25 AM, Bo Shen wrote:
Hi Nikita Kiryanov,

On 02/03/2015 07:32 PM, Nikita Kiryanov wrote:

[..]

  #ifdef CONFIG_LCD_LOGO
  void bitmap_plot(int x, int y)
diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
index 935ae42..0ce2370 100644
--- a/drivers/video/atmel_hlcdfb.c
+++ b/drivers/video/atmel_hlcdfb.c
@@ -13,6 +13,10 @@
  #include <lcd.h>
  #include <atmel_hlcdc.h>

+#if defined(CONFIG_LCD_LOGO)
+#include <bmp_logo.h>
+#endif

I think we can include this file directly.

bmp_logo.h does not exist without CONFIG_LCD_LOGO.


  /* configurable parameters */
  #define ATMEL_LCDC_CVAL_DEFAULT        0xc8
  #define ATMEL_LCDC_DMA_BURST_LEN    8
@@ -37,6 +41,15 @@ void lcd_setcolreg(ushort regno, ushort red, ushort green, 
ushort blue)
          panel_info.mmio + ATMEL_LCDC_LUT(regno));
  }

+ushort *configuration_get_cmap(void)
+{
+#if defined(CONFIG_LCD_LOGO)
+    return bmp_logo_palette;
+#else
+    return NULL;
+#endif
+}

Here, I think no need to do the CONFIG_LCD_LOGO check, return bmp_logo_palette 
directly, as it will be used when use bmp command in 8 bit mode. However, no 
hardware to test LCDC work or not in 8 bit
mode :(

This would make CONFIG_LCD_LOGO a prerequisite for this driver 
(bmp_logo_palette does not exist otherwise), and I don't think it's right to 
couple the two.


  void lcd_ctrl_init(void *lcdbase)
  {
      unsigned long value;
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 3cf008c..fa6a82c 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -29,6 +29,11 @@
  #define lcdc_readl(mmio, reg)        __raw_readl((mmio)+(reg))
  #define lcdc_writel(mmio, reg, val)    __raw_writel((val), (mmio)+(reg))


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

Reply via email to