From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nikita Kiryanov <nikita@compulab.co.il>
Date: Sun, 08 Feb 2015 13:35:00 +0200
Subject: [U-Boot] [PATCH V2 02/13] lcd: split configuration_get_cmap
In-Reply-To: <54D1C964.6050805@atmel.com>
References: <1422963152-17634-1-git-send-email-nikita@compulab.co.il>
 <1422963152-17634-3-git-send-email-nikita@compulab.co.il>
 <54D1C964.6050805@atmel.com>
Message-ID: <54D749E4.7090102@compulab.co.il>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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