Hi Kristen, Please review the updates. Thanks a lot.
1, removed the ioremapped memory operation, and leave it to firmware.
2, added the gpio_request and free in the driver probe and remove function, and
also modified ov5640_t_flash(struct v4l2_subdev *sd, int value) using
gpio_set_value(GPIO_FLASH, value)
From: Yong He <yong.m...@intel.com>
Subject: [PATCH] MRST Tablet camera driver ver-0.953, fix 9960
Bug 9960 - Back camera doesn't support auto-flash
Solution,
add Flash switch funtion (GPIO-45, Flash control), so that user app can sync
the flash with the snapshot steps.
add 2 V4L2 CID controls to implement flash trigger function and environment
detection
Signed-off-by: Yong He <yong.m...@intel.com>
Index: linux-2.6.37/drivers/staging/mrstci/mrstov5640/mrstov5640.c
===================================================================
--- linux-2.6.37/drivers/staging/mrstci/mrstov5640/mrstov5640.c (revision 115)
+++ linux-2.6.37/drivers/staging/mrstci/mrstov5640/mrstov5640.c (working copy)
@@ -50,7 +50,7 @@
#include "ci_sensor_common.h"
#include "ov5640.h"
-#define DRIVER_VERSION "0.95"
+#define DRIVER_VERSION "0.953"
#define GPIO_FLASH 45
static int ov5640_flash=0;
@@ -1231,26 +1231,21 @@
return err;
}
-static int ov5640_t_flash(struct i2c_client *c, int value)
+static int ov5640_t_flash(struct v4l2_subdev *sd, int value)
{
int ret;
- if(value!=0&&value!=1)
+
+ if ((value != 0) && (value != 1))
return -EINVAL;
- if(ret == 0)
- {
- ret = gpio_direction_output(GPIO_FLASH,value);
- if(ret == 0)
- ov5640_flash = value;
- gpio_free(GPIO_FLASH);
- }
+
+ gpio_set_value(GPIO_FLASH, value);
+ ov5640_flash = value;
+
return ret;
-
}
-static int ov5640_q_flash(struct i2c_client *c, int *value)
+static int ov5640_q_flash(struct v4l2_subdev *sd, int *value)
{
- if(!value)
- return -EINVAL;
*value=ov5640_flash;
return 0;
}
@@ -1327,6 +1322,37 @@
return ret;
}
+static int ov5640_t_exposure_level_detect(struct v4l2_subdev *sd, int value)
+{
+ int ret = 0;
+ /* this CID is Read Only */
+
+ return ret;
+}
+
+static int ov5640_q_exposure_level_detect(struct v4l2_subdev *sd, int *value)
+{
+ int ret = 0;
+ struct i2c_client *c = v4l2_get_subdevdata(sd);
+ int exposure, gain;
+ u8 v;
+
+ ret += ov5640_read(c, 0x3500, &v);
+ exposure = v;
+ ret += ov5640_read(c, 0x3501, &v);
+ exposure = (exposure<<8) | v;
+ ret += ov5640_read(c, 0x3502, &v);
+ exposure = (exposure<<8) | v;
+
+ ret += ov5640_read(c, 0x350a, &v);
+ gain = v;
+ ret += ov5640_read(c, 0x350b, &v);
+ gain = (gain<<8) | v;
+
+ *value = exposure*gain;
+ return ret;
+}
+
#if 0
static int ov5640_t_awb(struct i2c_client *c, int value)
{
@@ -1455,7 +1481,9 @@
#define CID_FOCUS_POSITION (V4L2_CID_PRIVATE_BASE + 0)
#define CID_FOCUS_MODE_STATUS (V4L2_CID_PRIVATE_BASE + 1)
#define CID_EXPOSURE_MODE_STATUS (V4L2_CID_PRIVATE_BASE + 2)
-#define CID_AUTO_FLASH_DETECT (V4L2_CID_PRIVATE_BASE + 3)
+#define CID_EXPOSURE_WINDOW (V4L2_CID_PRIVATE_BASE + 3)
+#define CID_EXPOSURE_LEVEL_DETECT (V4L2_CID_PRIVATE_BASE + 4)
+#define CID_FLASH_SWITCH (V4L2_CID_PRIVATE_BASE + 5)
{
.qc = {
.id = CID_FOCUS_POSITION,
@@ -1482,12 +1510,24 @@
.tweak = ov5640_t_focus,
.query = ov5640_q_focus,
},
-#if 0 //BUGBUG
{
.qc = {
- .id = V4L2_CID_FLASH_STROBE,
+ .id = CID_EXPOSURE_LEVEL_DETECT,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ .name = "Exposure Level Detect",
+ .minimum = 0,
+ .maximum = 0x7fffffff,
+ .step = 1,
+ .default_value = 0,
+ },
+ .tweak = ov5640_t_exposure_level_detect,
+ .query = ov5640_q_exposure_level_detect,
+ },
+ {
+ .qc = {
+ .id = CID_FLASH_SWITCH ,
.type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "LED flash control",
+ .name = "Flash Switch",
.minimum = 0,
.maximum = 1,
.step = 1,
@@ -1496,7 +1536,6 @@
.tweak = ov5640_t_flash,
.query = ov5640_q_flash,
},
-#endif
#if 0
{
.parm = {
@@ -1886,6 +1925,10 @@
sd = &info->sd;
v4l2_i2c_subdev_init(sd, client, &ov5640_ops);
+ /* Set Flash GPIO direction and init with OFF */
+ gpio_request(GPIO_FLASH, "Camera Flash");
+ gpio_direction_output(GPIO_FLASH, 0);
+
/*
* TODO: Need to check if this can be here.
* Turn into standby mode
@@ -1904,6 +1947,7 @@
{
struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ gpio_free(GPIO_FLASH);
v4l2_device_unregister_subdev(sd);
kfree(to_sensor_config(sd));
return 0;
-- Yong
Phone (+86) 21-61166334
Lab (+86) 21-61167881
-----Original Message-----
From: Kristen Carlson Accardi [mailto:kris...@linux.intel.com]
Sent: Thursday, June 23, 2011 2:17 AM
To: He, Yong M
Cc: MeeGo-kernel@lists.meego.com; Arjan van de Ven
Subject: Re: [PATCH] MRST Tablet camera driver ver-0.953, fix 9960
On Wed, 22 Jun 2011 19:21:41 +0800
"He, Yong M" <yong.m...@intel.com> wrote:
>
> From: Yong He <yong.m...@intel.com>
> Subject: [PATCH] MRST Tablet camera driver ver-0.953, fix 9960
>
> Hi Kristen,
>
> Please help to review the patch. Thanks a lot!
Hi Yong, I have some questions and comments below.
>
> ----
> Bug 9960<https://bugzilla.otcshare.org/show_bug.cgi?id=9960> - Back camera
> doesn't support auto-flash
>
> Solution:
> 1, change GPIO-45 (Flash control) to GPIO function, so that user app can sync
> the flash with the snapshot steps.
> 2, add 2 V4L2 CID controls to implement flash trigger function and
> environment detection
> ---
> Signed-off-by: Yong He <yong.m...@intel.com>
> Index: linux-2.6.37/drivers/staging/mrstci/mrstov5640/mrstov5640.c
> ===================================================================
> --- linux-2.6.37/drivers/staging/mrstci/mrstov5640/mrstov5640.c
> (revision 115)
> +++ linux-2.6.37/drivers/staging/mrstci/mrstov5640/mrstov5640.c (working
> copy)
> @@ -46,11 +46,12 @@
> #include <media/v4l2-device.h>
> #include <media/v4l2-chip-ident.h>
> #include <asm/div64.h>
> +#include <asm/mrst.h>
> #include "ci_sensor_common.h"
> #include "ov5640.h"
> -#define DRIVER_VERSION "0.95"
> +#define DRIVER_VERSION "0.953"
> #define GPIO_FLASH 45
> static int ov5640_flash=0;
> @@ -1231,23 +1232,24 @@
> return err;
> }
> -static int ov5640_t_flash(struct i2c_client *c, int value)
> +static int ov5640_t_flash(struct v4l2_subdev *sd, int value)
> {
> int ret;
> - if(value!=0&&value!=1)
> +
> + if ((value != 0) && (value != 1))
> return -EINVAL;
> +
> + ret = gpio_direction_output(GPIO_FLASH, value);
> +
> if(ret == 0)
> - {
> - ret = gpio_direction_output(GPIO_FLASH,value);
> - if(ret == 0)
> - ov5640_flash = value;
> - gpio_free(GPIO_FLASH);
> - }
> + ov5640_flash = value;
> +
> + gpio_free(GPIO_FLASH);
> +
> return ret;
> -
> }
I think I am not understanding what you are doing here. Normally
I would expect to see you set the gpio direction only one time
during probe, then just set the value of the gpio after that, and
then free the gpio during driver remove. Can you explain what you
are doing in this tweak function?
> -static int ov5640_q_flash(struct i2c_client *c, int *value)
> +static int ov5640_q_flash(struct v4l2_subdev *sd, int *value)
> {
> if(!value)
> return -EINVAL;
> @@ -1327,6 +1329,37 @@
> return ret;
> }
> +static int ov5640_t_exposure_level_detect(struct v4l2_subdev *sd, int value)
> +{
> + int ret = 0;
> + /* this CID is Read Only */
> +
> + return ret;
> +}
> +
> +static int ov5640_q_exposure_level_detect(struct v4l2_subdev *sd, int *value)
> +{
> + int ret = 0;
> + struct i2c_client *c = v4l2_get_subdevdata(sd);
> + int exposure, gain;
> + u8 v;
> +
> + ret += ov5640_read(c, 0x3500, &v);
> + exposure = v;
> + ret += ov5640_read(c, 0x3501, &v);
> + exposure = (exposure<<8) | v;
> + ret += ov5640_read(c, 0x3502, &v);
> + exposure = (exposure<<8) | v;
> +
> + ret += ov5640_read(c, 0x350a, &v);
> + gain = v;
> + ret += ov5640_read(c, 0x350b, &v);
> + gain = (gain<<8) | v;
> +
> + *value = exposure*gain;
> + return ret;
> +}
> +
> #if 0
> static int ov5640_t_awb(struct i2c_client *c, int value)
> {
> @@ -1455,7 +1488,9 @@
> #define CID_FOCUS_POSITION (V4L2_CID_PRIVATE_BASE + 0)
> #define CID_FOCUS_MODE_STATUS (V4L2_CID_PRIVATE_BASE + 1)
> #define CID_EXPOSURE_MODE_STATUS (V4L2_CID_PRIVATE_BASE + 2)
> -#define CID_AUTO_FLASH_DETECT (V4L2_CID_PRIVATE_BASE + 3)
> +#define CID_EXPOSURE_WINDOW (V4L2_CID_PRIVATE_BASE + 3)
> +#define CID_EXPOSURE_LEVEL_DETECT (V4L2_CID_PRIVATE_BASE + 4)
> +#define CID_FLASH_SWITCH (V4L2_CID_PRIVATE_BASE + 5)
> {
> .qc = {
> .id = CID_FOCUS_POSITION,
> @@ -1482,12 +1517,24 @@
> .tweak = ov5640_t_focus,
> .query = ov5640_q_focus,
> },
> -#if 0 //BUGBUG
> {
> .qc = {
> - .id = V4L2_CID_FLASH_STROBE,
> + .id = CID_EXPOSURE_LEVEL_DETECT,
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .name = "Exposure Level Detect",
> + .minimum = 0,
> + .maximum = 0x7fffffff,
> + .step = 1,
> + .default_value = 0,
> + },
> + .tweak = ov5640_t_exposure_level_detect,
> + .query = ov5640_q_exposure_level_detect,
> + },
> + {
> + .qc = {
> + .id = CID_FLASH_SWITCH ,
> .type = V4L2_CTRL_TYPE_BOOLEAN,
> - .name = "LED flash control",
> + .name = "Flash Switch",
> .minimum = 0,
> .maximum = 1,
> .step = 1,
> @@ -1496,7 +1543,6 @@
> .tweak = ov5640_t_flash,
> .query = ov5640_q_flash,
> },
> -#endif
> #if 0
> {
> .parm = {
> @@ -1857,12 +1903,16 @@
> return 0;
> }
> +#define LANGWELL_GPIO_ALTER_FUNC_2 0xff12c040
> +
> static int ov5640_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> struct ci_sensor_config *info;
> struct v4l2_subdev *sd;
> int ret = -1;
> + u32 __iomem *mem = ioremap_nocache(
> + LANGWELL_GPIO_ALTER_FUNC_2, 64);
> DBG_entering;
> @@ -1886,6 +1936,10 @@
> sd = &info->sd;
> v4l2_i2c_subdev_init(sd, client, &ov5640_ops);
> + /* change LANGWELL GPIO-45 mux to GPIO Funtion */
> + gpio_direction_output(GPIO_FLASH, 0);
> + *mem &= 0xf3ffffff; /* Clear bit [27-26]*/
> +
There are a couple issues with the above. Firstly, when accessing
ioremapped memory, you may not accesses it directly, you have to
use readl/writel. Secondly, this *should* be done in Firmware,
and I would prefer to see you make this a pci quirk in arch/x86/pci/mrst.c
(in a separate patch) and then file a bug with the fw team. When they
fix it, we can then easily drop the pci quirk.
Also, as i mentioned above, normally you would do gpio_request before
setting the direction.
Thanks,
Kristen
linux-2.6.37-camera-ov5640-ov9740-version-0.952_to_0.953.patch
Description: linux-2.6.37-camera-ov5640-ov9740-version-0.952_to_0.953.patch
_______________________________________________ MeeGo-kernel mailing list MeeGo-kernel@lists.meego.com http://lists.meego.com/listinfo/meego-kernel
