On Sat, 15 Dec 2012 17:57:55 +0800
Albert Wang <twan...@marvell.com> wrote:

> From: Libin Yang <lby...@marvell.com>
> 
> This patch adds the new formats support for marvell-ccic.

Once again, just one second-order comment:

> +static bool mcam_fmt_is_planar(__u32 pfmt)
> +{
> +     switch (pfmt) {
> +     case V4L2_PIX_FMT_YUV422P:
> +     case V4L2_PIX_FMT_YUV420:
> +     case V4L2_PIX_FMT_YVU420:
> +             return true;
> +     }
> +     return false;
> +}

This seems like the kind of thing that would be useful in a number of
places; I'd be tempted to push it up a level and make it available to all
V4L2 drivers.  Of course, that means making it work for *all* formats,
which would be a pain.  

But, then, I can see some potential future pain if somebody adds a new
format and forgets to tweak this function here.  Rather than adding a new
switch, could you put a "planar" flag into struct mcam_format_struct
instead?  That would help to keep all this information together.

jon
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to