Hi,
Thank you for your comments.
On Wen, 04 Apr 2012 21:53, Anatolij Gustschin wrote:
> Hi!
>
> Sorry for not looking at this earlier. The patch looks pretty good, but
> there are some style issues, please see some comments below.
>
> Also the patch subject should reflect on which subsystem the patch is
> doing the changes, so I would suggest using the patch subject like
> "LCD: add data structure for EXYNOS display driver"
>
Ok, I will change the title.
> On Mon, 02 Apr 2012 17:39:24 +0900
> Donghwa Lee <dh09....@samsung.com> wrote:
>
>> add vidinfo data structure for EXYNOS display driver
>>
>> Signed-off-by: Donghwa Lee <dh09....@samsung.com>
>> Signed-off-by: Inki Dae <inki....@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
>> ---
>> include/lcd.h | 63
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 63 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/lcd.h b/include/lcd.h
>> index d95feeb..651ff42 100644
>> --- a/include/lcd.h
>> +++ b/include/lcd.h
>> @@ -56,6 +56,11 @@ extern void lcd_initcolregs (void);
>> /* gunzip_bmp used if CONFIG_VIDEO_BMP_GZIP */
>> extern struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long
>> *lenp);
>>
>> +enum {
>> + FIMD_RGB_INTERFACE = 1,
>> + FIMD_CPU_INTERFACE = 2,
>> +};
>
> This is EXYNOS specific so please move it under
> #elif defined(CONFIG_EXYNOS_FB) line.
>
> ...
Ok, I will move it.
>> + /* LCD configuration register */
>> + u_char vl_freq; /* Frequency */
>> + u_char vl_clkp; /* Clock polarity */
>> + u_char vl_oep; /* Output Enable polarity */
>> + u_char vl_hsp; /* Horizontal Sync polarity */
>> + u_char vl_vsp; /* Vertical Sync polarity */
>> + u_char vl_dp; /* Data polarity */
>> + u_char vl_bpix; /* Bits per pixel, 0 = 1, 1 = 2, 2 = 4, 3 = 8,
>> 4 = 16 */
>
> The above line is too long, we have 80 chars/line limit. I know
> there are other bad examples in this file, but please change to
>
> u_char vl_bpix; /* Bits per pixel, bpp = pow(2, vl_bpix) */
>
> ...
>> @@ -213,6 +275,7 @@ void lcd_puts (const char *s);
>> void lcd_printf (const char *fmt, ...);
>> void lcd_clear(void);
>> int lcd_display_bitmap(ulong bmp_image, int x, int y);
>> +void init_panel_info (vidinfo_t *vid);
>
> Please remove space between function name and open parenthesis '('.
>
> There are other examples with spaces between function names and '('
> in this file, I'll fix them later.
>
> Btw, where is init_panel_info() defined? I only see that it is used
> in drivers/video/exynos_fb.c, but do not see the function code. So the
> driver cannot be build at all. How did you test it?
>
> Also try to check your patches with 'tools/checkpatch.pl'. It will
> warn you about such style issues. Some of the warnings will be Linux
> style specific, these can be disabled using "--ignore=" option, e.g.
> using --ignore=NEW_TYPEDEFS would be appropriate for checking this
> patch.
>
This function is used in TRATS board file to get vidinfo data that sets up in
board file,
Although I had already check by using checkpatch.pl, maybe some file was missed.
I will send patch next version.
Thank you,
Donghwa Lee
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
