SoC Camera: ov6650: minor cleanups

  This is a followup patch that addresses two minor issues left in the recently 
added ov6650 sensor driver, as I've promised to the subsystem maintainer:
- remove a pair of extra brackets,
- drop useless case for not possible v4l2_mbus_pixelcode enum value of 0.

Created against linux-2.6.37-rc1.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---

 drivers/media/video/ov6650.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--
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
  

Hi Janusz

On Tue, 2 Nov 2010, Janusz Krzysztofik wrote:

> This is a followup patch that addresses two minor issues left in the recently 
> added ov6650 sensor driver, as I've promised to the subsystem maintainer:
> - remove a pair of extra brackets,
> - drop useless case for not possible v4l2_mbus_pixelcode enum value of 0.
> 
> Created against linux-2.6.37-rc1.
> 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

Applied together with other your 3 patches and pushed for 2.6.37-rc2.

Thanks
Guennadi

> ---
> 
>  drivers/media/video/ov6650.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> --- linux-2.6.37-rc1/drivers/media/video/ov6650.c.orig	2010-11-01 22:41:59.000000000 +0100
> +++ linux-2.6.37-rc1/drivers/media/video/ov6650.c	2010-11-02 16:56:49.000000000 +0100
> @@ -754,7 +754,7 @@ static int ov6650_g_fmt(struct v4l2_subd
>  
>  static bool is_unscaled_ok(int width, int height, struct v4l2_rect *rect)
>  {
> -	return (width > rect->width >> 1 || height > rect->height >> 1);
> +	return width > rect->width >> 1 || height > rect->height >> 1;
>  }
>  
>  static u8 to_clkrc(struct v4l2_fract *timeperframe,
> @@ -840,8 +840,6 @@ static int ov6650_s_fmt(struct v4l2_subd
>  		coma_mask |= COMA_BW | COMA_BYTE_SWAP | COMA_WORD_SWAP;
>  		coma_set |= COMA_RAW_RGB | COMA_RGB;
>  		break;
> -	case 0:
> -		break;
>  	default:
>  		dev_err(&client->dev, "Pixel format not handled: 0x%x\n", code);
>  		return -EINVAL;
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

Monday 08 November 2010 22:20:33 Guennadi Liakhovetski wrote:
> On Tue, 2 Nov 2010, Janusz Krzysztofik wrote:
> > This is a followup patch that addresses two minor issues left in the
> > recently added ov6650 sensor driver, as I've promised to the subsystem
> > maintainer:
> > - remove a pair of extra brackets, 
> > - drop useless case for not possible v4l2_mbus_pixelcode enum value of 0.
> >
> > Created against linux-2.6.37-rc1.
> >
> > Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
>
> Applied together with other your 3 patches and pushed for 2.6.37-rc2.

Hi Guennadi,
Thanks for taking my fixes.

Thursday 30 September 2010 13:35:49 Janusz Krzysztofik wrote:
> There are still two SG mode specific corner cases to be corrected,
> previously not detected because of poor sensor driver functionality: 1)
> frame size not exceeding one page, resulting in "unexpected end of frame"
> message and capture restart every frame, and 2) last sgbuf lenght less than
> bytes_per_line, resulting in unstable picture. I'm going to address those
> two with fixes.

Since both issues don't affect typical usage (one of standard resolutions) and 
both are videobuf-sg related, I'm wondering if I should better wait for 
videobuf2 and try to port my driver instead of making things still more 
complicated than they already are. What do you think?

Thanks,
Janusz
--
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
  

On Sat, 13 Nov 2010, Janusz Krzysztofik wrote:

> Thursday 30 September 2010 13:35:49 Janusz Krzysztofik wrote:
> > There are still two SG mode specific corner cases to be corrected,
> > previously not detected because of poor sensor driver functionality: 1)
> > frame size not exceeding one page, resulting in "unexpected end of frame"
> > message and capture restart every frame, and 2) last sgbuf lenght less than
> > bytes_per_line, resulting in unstable picture. I'm going to address those
> > two with fixes.
> 
> Since both issues don't affect typical usage (one of standard resolutions) and 
> both are videobuf-sg related, I'm wondering if I should better wait for 
> videobuf2 and try to port my driver instead of making things still more 
> complicated than they already are. What do you think?

Well, I _would_ say: restrict the driver to avoid those corner cases. 
I.e., add a test to omap1_cam_set_fmt() and / or omap1_cam_set_crop() in 
SG case to verify, that the frame is at least one page large and that the 
lasg sgbuf is large enough. If this is not the case adjust the frame to 
satisfy these restrictions. But the problem is - at S_FMT / S_CROP time 
you don't know yet, whether you're going to use SG.

I haven't studied videobuf2 in enough detail to understand, why and how 
it would help you? Isn't this a principal problem with your SG 
implementation? Maybe we should take this as yet one more argument against 
your "emulated sg" mode and remove it completely from the driver, relying 
on contiguous video buffers, selecting and implementing some boot-time 
memory reservation, and, possibly, adding the omap1 camera driver to the 
list of other drivers, waiting to break down again with 2.6.37, unless the 
"conflicting mapping mode" problem on ARM gets resolved before then?

Also, please, move use_sg inside struct omap1_cam_dev.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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