Current status of SuperH soc-camera/CEU driver

Current status of SuperH soc-camera/CEU driver

[Kuninori Morimoto]

Hi Guennadi

Now I'm checking Soc-Camera/CEU driver, and I noticed that
platform settings (maybe) weren't cared even though driver side was updated.
I'm not sure for details, but it makes me misunderstand.
Could you please update these ?

1) e1db704326c9a5164da4e24b01e487c0be687fa2
([media] V4L: sh_mobile_ceu_camera: convert to the new mbus-config subdev operations)

	This patch removed SH_CEU_FLAG_USE_xxBIT_BUS flags from CEU driver,
	but below platform still has this flags.

	arch/sh/boards/mach-ap325rxa/
	arch/sh/boards/mach-ecovec24/
	arch/sh/boards/mach-kfr2r09/
	arch/sh/boards/mach-migor/
	arch/sh/boards/mach-se/7724/
	arch/arm/mach-shmobile/board-ap4evb.c
	arch/arm/mach-shmobile/board-mackerel.c

2) ff51345832628eb641805a01213aeae0bb4a23c1
([media] V4L: mt9t112: remove superfluous soc-camera client operations)

	this patch remoded MT9T112_FLAG_DATAWIDTH_xx flags from mt9t112 driver,
	but Ecovec platform still has it.

	arch/sh/boards/mach-ecovec24/


Best regards
---
Kuninori Morimoto

Re: Current status of SuperH soc-camera/CEU driver

[Guennadi Liakhovetski]

Hi Morimoto-san

Thanks for spotting these.

On Sun, 22 Apr 2012, Kuninori Morimoto wrote:

> Hi Guennadi
> 
> Now I'm checking Soc-Camera/CEU driver, and I noticed that
> platform settings (maybe) weren't cared even though driver side was updated.
> I'm not sure for details, but it makes me misunderstand.
> Could you please update these ?
> 
> 1) e1db704326c9a5164da4e24b01e487c0be687fa2
> ([media] V4L: sh_mobile_ceu_camera: convert to the new mbus-config subdev operations)
> 
> 	This patch removed SH_CEU_FLAG_USE_xxBIT_BUS flags from CEU driver,
> 	but below platform still has this flags.
> 
> 	arch/sh/boards/mach-ap325rxa/
> 	arch/sh/boards/mach-ecovec24/
> 	arch/sh/boards/mach-kfr2r09/
> 	arch/sh/boards/mach-migor/
> 	arch/sh/boards/mach-se/7724/
> 	arch/arm/mach-shmobile/board-ap4evb.c
> 	arch/arm/mach-shmobile/board-mackerel.c

AFAICS, all these platforms only use 8 bits, so, none of them is broken. 
OTOH, I'm not sure any more, what was the motivation behind that removal. 
Maybe exactly because we didn't have any platforms with 16-bit camera 
connections and maybe I saw a problem with it, so, I decided to remove 
them until we get a chance to properly implement and test 16-bits? Do you 
have such a board?

> 2) ff51345832628eb641805a01213aeae0bb4a23c1
> ([media] V4L: mt9t112: remove superfluous soc-camera client operations)
> 
> 	this patch remoded MT9T112_FLAG_DATAWIDTH_xx flags from mt9t112 driver,
> 	but Ecovec platform still has it.
> 
> 	arch/sh/boards/mach-ecovec24/

Perhaps, the reason was more or less the same - no users, untested code.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: Current status of SuperH soc-camera/CEU driver

[Kuninori Morimoto]

Hi Guennadi

Thanks for reply.

> AFAICS, all these platforms only use 8 bits, so, none of them is broken. 
> OTOH, I'm not sure any more, what was the motivation behind that removal. 
> Maybe exactly because we didn't have any platforms with 16-bit camera 
> connections and maybe I saw a problem with it, so, I decided to remove 
> them until we get a chance to properly implement and test 16-bits? Do you 
> have such a board?

about 16bit camera, one guy has it, but he is using v3.0 kernel,
so, it is not in trouble at this point.
(it is working)

The motivation was just "misunderstand-able", not super important at this point.
So please keep considering about it.

> > 2) ff51345832628eb641805a01213aeae0bb4a23c1
> > ([media] V4L: mt9t112: remove superfluous soc-camera client operations)
> > 
> > 	this patch remoded MT9T112_FLAG_DATAWIDTH_xx flags from mt9t112 driver,
> > 	but Ecovec platform still has it.
> > 
> > 	arch/sh/boards/mach-ecovec24/
> 
> Perhaps, the reason was more or less the same - no users, untested code.

About this, now, I'm trying to use this camera on new board (but 8bit),
and the platform code was copied from ecovec.
But the camera image was not correct.

So, I checked the platform/ceu/mt9t112, 
and noticed that the driver is using new mbus style,
but platform is still using old style.
And I got confusion.

Now, this code is no users, untested.
So, the reason why image was broken is 50% new board specific issue,
but 50% original code issue.

Best regards
---
Kuninori Morimoto

[PATCH] V4L: sh-mobile-ceu-camera: restore the bus-width test

[Guennadi Liakhovetski]

An earlier commit "[media] V4L: sh_mobile_ceu_camera: convert to the new 
mbus-config subdev operations" has inadvertantly removed the check in the 
sh-mobile-ceu-camera driver, whether a specific bus-width is supported. 
This patch restores the check for formats, requiring wider than 8-bit 
video bus. The other check from the above commit - whether 8-bits per 
sample are supported - is, however, not restored. All currently known set 
ups support 8 bits per sample, hence, this check so far seems redundant. 
The respective SH_CEU_FLAG_USE_8BIT_BUS flag will be kept for now, but may 
be removed in the future.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

On Mon, 23 Apr 2012, Kuninori Morimoto wrote:

> 
> Hi Guennadi
> 
> Thanks for reply.
> 
> > AFAICS, all these platforms only use 8 bits, so, none of them is broken. 
> > OTOH, I'm not sure any more, what was the motivation behind that removal. 
> > Maybe exactly because we didn't have any platforms with 16-bit camera 
> > connections and maybe I saw a problem with it, so, I decided to remove 
> > them until we get a chance to properly implement and test 16-bits? Do you 
> > have such a board?
> 
> about 16bit camera, one guy has it, but he is using v3.0 kernel,
> so, it is not in trouble at this point.
> (it is working)
> 
> The motivation was just "misunderstand-able", not super important at this point.
> So please keep considering about it.

Would be nice if the below patch could be tested with a 16-bit set up. But 
it should be tested "negatively." This means: I think, also now 16-bit set 
ups work. The only problem is, that even if your board only connects 8 
data lines, an attempt to set a 16-bit format wouldn't fail and would, 
probably, deliver corrupt data. So, the test would be:
- take a 16-bit set up and choose a 16-bit format - it should work
- remove the 16-bit flag from the platform data - it would, presumably, 
  still work, which is a bug
- apply the patch
- now verify that 16-bits formats can only be used, if the board specifies 
  the respective flag in platform data

Thanks
Guennadi

diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 424dfac..3e7b794 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -951,7 +951,8 @@ static int sh_mobile_ceu_try_bus_param(struct soc_camera_device *icd,
 	else if (ret != -ENOIOCTLCMD)
 		return ret;
 
-	if (!common_flags || buswidth > 16)
+	if (!common_flags || buswidth > 16 ||
+	    (buswidth > 8 && !(pcdev->pdata->flags & SH_CEU_FLAG_USE_16BIT_BUS)))
 		return -EINVAL;
 
 	return 0;

Re: [PATCH] V4L: sh-mobile-ceu-camera: restore the bus-width test

[Kuninori Morimoto]

Hi Guennadi

> Would be nice if the below patch could be tested with a 16-bit set up. But 
> it should be tested "negatively." This means: I think, also now 16-bit set 
> ups work. The only problem is, that even if your board only connects 8 
> data lines, an attempt to set a 16-bit format wouldn't fail and would, 
> probably, deliver corrupt data. So, the test would be:
> - take a 16-bit set up and choose a 16-bit format - it should work
> - remove the 16-bit flag from the platform data - it would, presumably, 
>   still work, which is a bug
> - apply the patch
> - now verify that 16-bits formats can only be used, if the board specifies 
>   the respective flag in platform data

Thank you about this patch, but we are busy now. Sorry.
We would like to test this, but 16bit camera is using v3.0 kernel now.
And it is difficult to update kernel because of time issue.
(The board itself is not upstreamed)
Of course we will test when we have free time in the future.

Best regards
---
Kuninori Morimoto