Hi Florian,
Thanks for the review.
On Friday 26 August 2011 19:24:02 Florian Tobias Schandinat wrote:
> On 08/19/2011 09:37 AM, Laurent Pinchart wrote:
[snip]
> > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > b/drivers/video/sh_mobile_lcdcfb.c index 97ab8ba..ea3f619 100644
> > --- a/drivers/video/sh_mobile_lcdcfb.c
> > +++ b/drivers/video/sh_mobile_lcdcfb.c
[snip]
> > @@ -1099,51 +1154,78 @@ static int sh_mobile_check_var(struct
[snip]
> > + if (var->format.fourcc > 1) {
> > + switch (var->format.fourcc) {
> > + case V4L2_PIX_FMT_NV12:
> > + case V4L2_PIX_FMT_NV21:
> > + var->bits_per_pixel = 12;
> > + break;
> > + case V4L2_PIX_FMT_RGB565:
> > + case V4L2_PIX_FMT_NV16:
> > + case V4L2_PIX_FMT_NV61:
> > + var->bits_per_pixel = 16;
> > + break;
> > + case V4L2_PIX_FMT_BGR24:
> > + case V4L2_PIX_FMT_NV24:
> > + case V4L2_PIX_FMT_NV42:
> > + var->bits_per_pixel = 24;
> > + break;
> > + case V4L2_PIX_FMT_BGR32:
> > + var->bits_per_pixel = 32;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + memset(var->format.reserved, 0, sizeof(var->format.reserved));
>
> If we decide to use another of the reserved area this won't have the
> desired behavior as the behavior of this driver will change even if it
> does not support the new field. Probably the best thing is to get the
> desired behavior is zeroing the whole struct and setting the supported
> fields to the actual values. You should check and adjust colorspace here
> as well.
Agreed. I'll fix the patch accordingly.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
