tw9910: color format check is added on set_fmt

Message ID uwscdm9t7.wl%morimoto.kuninori@renesas.com (mailing list archive)
State Rejected, archived
Headers

Commit Message

Kuninori Morimoto Jan. 27, 2009, 6:02 a.m. UTC
  Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
 drivers/media/video/tw9910.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
  

Comments

Guennadi Liakhovetski Feb. 1, 2009, 6:07 p.m. UTC | #1
On Tue, 27 Jan 2009, Kuninori Morimoto wrote:

> 
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>

Why is this needed? Do you see any possibility for tw9910 to be called 
with an unsupported format?

Thanks
Guennadi

> ---
>  drivers/media/video/tw9910.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/tw9910.c b/drivers/media/video/tw9910.c
> index 1a9c6fd..57027c0 100644
> --- a/drivers/media/video/tw9910.c
> +++ b/drivers/media/video/tw9910.c
> @@ -647,6 +647,19 @@ static int tw9910_set_fmt(struct soc_camera_device *icd, __u32 pixfmt,
>  	struct tw9910_priv *priv = container_of(icd, struct tw9910_priv, icd);
>  	int                 ret  = -EINVAL;
>  	u8                  val;
> +	int                 i;
> +
> +	/*
> +	 * check color format
> +	 */
> +	for (i = 0 ; i < ARRAY_SIZE(tw9910_color_fmt) ; i++) {
> +		if (pixfmt == tw9910_color_fmt[i].fourcc) {
> +			ret = 0;
> +			break;
> +		}
> +	}
> +	if (ret < 0)
> +		goto tw9910_set_fmt_error;
>  
>  	/*
>  	 * select suitable norm
> -- 
> 1.5.6.3
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Kuninori Morimoto Feb. 2, 2009, 12:20 a.m. UTC | #2
Dear Guennadi

> > Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
> 
> Why is this needed? Do you see any possibility for tw9910 to be called 
> with an unsupported format?

for example,
capture_example -f use V4L2_PIX_FMT_YUYV.
but tw9910 support only V4L2_PIX_FMT_VYUY now.

If you think this patch is unnecessary,
please ignore it.

Best regards
--
Kuninori Morimoto
 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Guennadi Liakhovetski Feb. 2, 2009, 7:43 a.m. UTC | #3
On Mon, 2 Feb 2009, morimoto.kuninori@renesas.com wrote:

> > > Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
> > 
> > Why is this needed? Do you see any possibility for tw9910 to be called 
> > with an unsupported format?
> 
> for example,
> capture_example -f use V4L2_PIX_FMT_YUYV.
> but tw9910 support only V4L2_PIX_FMT_VYUY now.

But are you actually getting this set_fmt(V4L2_PIX_FMT_YUYV) in your 
tw9910 driver? If yes, then this is a bug elsewhere. It shouldn't get this 
far. It should be caught earlier along the path

soc_camera_s_fmt_vid_cap()
	soc_camera_try_fmt_vid_cap()
		sh_mobile_ceu_try_fmt()
			soc_camera_xlate_by_fourcc()
				<error>

> If you think this patch is unnecessary,
> please ignore it.

Could you please test whether you indeed can get an unsupported format in 
your driver. If so, this is a bug at a higher level and we'll have to fix 
it there. I'll drop this patch for now.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Kuninori Morimoto Feb. 2, 2009, 7:52 a.m. UTC | #4
Dear Guennadi

> > If you think this patch is unnecessary,
> > please ignore it.
> 
> Could you please test whether you indeed can get an unsupported format in 
> your driver. If so, this is a bug at a higher level and we'll have to fix 
> it there. I'll drop this patch for now.

tw9910 driver can not get an unsupported format.
host driver (sh_mobile_ceu) check it and return error.

I just thought double check is important. sorry.

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

Patch

diff --git a/drivers/media/video/tw9910.c b/drivers/media/video/tw9910.c
index 1a9c6fd..57027c0 100644
--- a/drivers/media/video/tw9910.c
+++ b/drivers/media/video/tw9910.c
@@ -647,6 +647,19 @@  static int tw9910_set_fmt(struct soc_camera_device *icd, __u32 pixfmt,
 	struct tw9910_priv *priv = container_of(icd, struct tw9910_priv, icd);
 	int                 ret  = -EINVAL;
 	u8                  val;
+	int                 i;
+
+	/*
+	 * check color format
+	 */
+	for (i = 0 ; i < ARRAY_SIZE(tw9910_color_fmt) ; i++) {
+		if (pixfmt == tw9910_color_fmt[i].fourcc) {
+			ret = 0;
+			break;
+		}
+	}
+	if (ret < 0)
+		goto tw9910_set_fmt_error;
 
 	/*
 	 * select suitable norm