[U-Boot] [PATCH V4 3/3] api: export LCD device to external apps

Mike Frysinger
2011-10-20T13:25:23Z

On Thursday 20 October 2011 01:38:46 Che-Liang Chiou wrote:
> --- a/api/api.c
> +++ b/api/api.c
>
> +/*
> + * pseudo signature:
> + *
> + * int API_display_get_info(int type, struct display_info *di)
> + */
> +static int API_display_get_info(va_list ap)
> +{
> + int type;
> + struct display_info *di;
> +
> + type = (int)va_arg(ap, u_int32_t);
> + di = (struct display_info *)va_arg(ap, u_int32_t);

i know you simply copied these warts from the rest of the code, but this
va_arg() usage looks wrong to me. why is u_int32_t always used as the
argument to va_arg() and then the return value is cast ? seems to me this
code should actually read:
type = va_arg(ap, int);
di = va_arg(ap, struct display_info *);

could you see if that change still works for you ?

> +static int API_display_draw_bitmap(va_list ap)
> +{
> + ulong bitmap;
> + int x, y;
> +
> + bitmap = (ulong)va_arg(ap, ulong);
> + if (!bitmap)
> + return API_EINVAL;

seems to me that this NULL pointer check belongs in the lcd core

> --- /dev/null
> +++ b/api/api_display.c
>
> +int display_draw_bitmap(ulong bitmap, int x, int y)
> +{
> + int err = 0;
> +#ifdef CONFIG_LCD
> + err |= lcd_display_bitmap(bitmap, x, y);
> +#endif
> + return err;
> +}

i think this should read:
int display_draw_bitmap(ulong bitmap, int x, int y)
{
#ifdef CONFIG_LCD
return lcd_display_bitmap(bitmap, x, y);
#else
return API_ENODEV;
#endif
}

> --- a/include/video_font.h
> +++ b/include/video_font.h
>
> +/*
> + * Adding unused attribute here so that compiler will not complain
> + * video_fontdata is not used in source files that only use
> + * VIDEO_FONT_* (e.g., api/api_display.c).
> + */
> +__attribute__ ((unused))
> static unsigned char video_fontdata[VIDEO_FONT_SIZE] = {

i don't think this is right. we should split the data from the rest of the
defines/structs.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111020/53c09ffb/attachment.pgp