* [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user
@ 2020-11-11 6:39 Bingbu Cao
2020-11-11 6:39 ` [PATCH v3 2/2] media: ov2740: allow OTP data access during streaming Bingbu Cao
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Bingbu Cao @ 2020-11-11 6:39 UTC (permalink / raw)
To: linux-media, sakari.ailus
Cc: tfiga, senozhatsky, bingbu.cao, bingbu.cao, qingwu.zhang
OTP data access of ov2740 in probe need power up, it may cause
the camera flash LED blink during probe if the LED use same power
rail with camera, this patch move the OTP data access out of
probe, it will only occur on demand from user by nvmem sysfs.
Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
Signed-off-by: Qingwu Zhang <qingwu.zhang@intel.com>
---
drivers/media/i2c/ov2740.c | 111 ++++++++++++++++++++++++++++++++-------------
1 file changed, 79 insertions(+), 32 deletions(-)
diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 64ecb6917dd3..41c17df46f47 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -71,9 +71,10 @@
#define OV2740_REG_OTP_CUSTOMER 0x7010
struct nvm_data {
- char *nvm_buffer;
+ struct i2c_client *client;
struct nvmem_device *nvmem;
struct regmap *regmap;
+ char *nvm_buffer;
};
enum {
@@ -335,6 +336,9 @@ struct ov2740 {
/* Streaming on/off */
bool streaming;
+
+ /* NVM data inforamtion */
+ struct nvm_data *nvm;
};
static inline struct ov2740 *to_ov2740(struct v4l2_subdev *subdev)
@@ -930,45 +934,57 @@ static int ov2740_remove(struct i2c_client *client)
return 0;
}
-static int ov2740_load_otp_data(struct i2c_client *client, struct nvm_data *nvm)
+static int ov2740_load_otp_data(struct nvm_data *nvm)
{
+ struct i2c_client *client = nvm->client;
struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
u32 isp_ctrl00 = 0;
u32 isp_ctrl01 = 0;
int ret;
+ if (!nvm)
+ return -EINVAL;
+
+ if (nvm->nvm_buffer)
+ return 0;
+
+ nvm->nvm_buffer = kzalloc(CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
+ if (!nvm->nvm_buffer)
+ return -ENOMEM;
+
ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, &isp_ctrl00);
if (ret) {
dev_err(&client->dev, "failed to read ISP CTRL00\n");
- goto exit;
+ goto err;
}
+
ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, &isp_ctrl01);
if (ret) {
dev_err(&client->dev, "failed to read ISP CTRL01\n");
- goto exit;
+ goto err;
}
/* Clear bit 5 of ISP CTRL00 */
ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1,
isp_ctrl00 & ~BIT(5));
if (ret) {
- dev_err(&client->dev, "failed to write ISP CTRL00\n");
- goto exit;
+ dev_err(&client->dev, "failed to set ISP CTRL00\n");
+ goto err;
}
/* Clear bit 7 of ISP CTRL01 */
ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1,
isp_ctrl01 & ~BIT(7));
if (ret) {
- dev_err(&client->dev, "failed to write ISP CTRL01\n");
- goto exit;
+ dev_err(&client->dev, "failed to set ISP CTRL01\n");
+ goto err;
}
ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
OV2740_MODE_STREAMING);
if (ret) {
- dev_err(&client->dev, "failed to start streaming\n");
- goto exit;
+ dev_err(&client->dev, "failed to set streaming mode\n");
+ goto err;
}
/*
@@ -981,15 +997,33 @@ static int ov2740_load_otp_data(struct i2c_client *client, struct nvm_data *nvm)
nvm->nvm_buffer, CUSTOMER_USE_OTP_SIZE);
if (ret) {
dev_err(&client->dev, "failed to read OTP data, ret %d\n", ret);
- goto exit;
+ goto err;
}
- ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
- OV2740_MODE_STANDBY);
- ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
- ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
+ ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
+ OV2740_MODE_STANDBY);
+ if (ret) {
+ dev_err(&client->dev, "failed to set streaming mode\n");
+ goto err;
+ }
+
+ ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
+ if (ret) {
+ dev_err(&client->dev, "failed to set ISP CTRL01\n");
+ goto err;
+ }
+
+ ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
+ if (ret) {
+ dev_err(&client->dev, "failed to set ISP CTRL00\n");
+ goto err;
+ }
+
+ return 0;
+err:
+ kfree(nvm->nvm_buffer);
+ nvm->nvm_buffer = NULL;
-exit:
return ret;
}
@@ -997,29 +1031,43 @@ static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
size_t count)
{
struct nvm_data *nvm = priv;
+ struct v4l2_subdev *sd = i2c_get_clientdata(nvm->client);
+ struct device *dev = &nvm->client->dev;
+ struct ov2740 *ov2740 = to_ov2740(sd);
+ int ret = 0;
+
+ mutex_lock(&ov2740->mutex);
- memcpy(val, nvm->nvm_buffer + off, count);
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(dev);
+ goto exit;
+ }
- return 0;
+ ret = ov2740_load_otp_data(nvm);
+ if (!ret)
+ memcpy(val, nvm->nvm_buffer + off, count);
+
+ pm_runtime_put(dev);
+exit:
+ mutex_unlock(&ov2740->mutex);
+ return ret;
}
-static int ov2740_register_nvmem(struct i2c_client *client)
+static int ov2740_register_nvmem(struct i2c_client *client,
+ struct ov2740 *ov2740)
{
struct nvm_data *nvm;
struct regmap_config regmap_config = { };
struct nvmem_config nvmem_config = { };
struct regmap *regmap;
struct device *dev = &client->dev;
- int ret = 0;
+ int ret;
nvm = devm_kzalloc(dev, sizeof(*nvm), GFP_KERNEL);
if (!nvm)
return -ENOMEM;
- nvm->nvm_buffer = devm_kzalloc(dev, CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
- if (!nvm->nvm_buffer)
- return -ENOMEM;
-
regmap_config.val_bits = 8;
regmap_config.reg_bits = 16;
regmap_config.disable_locking = true;
@@ -1028,12 +1076,7 @@ static int ov2740_register_nvmem(struct i2c_client *client)
return PTR_ERR(regmap);
nvm->regmap = regmap;
-
- ret = ov2740_load_otp_data(client, nvm);
- if (ret) {
- dev_err(dev, "failed to load OTP data, ret %d\n", ret);
- return ret;
- }
+ nvm->client = client;
nvmem_config.name = dev_name(dev);
nvmem_config.dev = dev;
@@ -1051,7 +1094,11 @@ static int ov2740_register_nvmem(struct i2c_client *client)
nvm->nvmem = devm_nvmem_register(dev, &nvmem_config);
- return PTR_ERR_OR_ZERO(nvm->nvmem);
+ ret = PTR_ERR_OR_ZERO(nvm->nvmem);
+ if (!ret)
+ ov2740->nvm = nvm;
+
+ return ret;
}
static int ov2740_probe(struct i2c_client *client)
@@ -1103,7 +1150,7 @@ static int ov2740_probe(struct i2c_client *client)
goto probe_error_media_entity_cleanup;
}
- ret = ov2740_register_nvmem(client);
+ ret = ov2740_register_nvmem(client, ov2740);
if (ret)
dev_warn(&client->dev, "register nvmem failed, ret %d\n", ret);
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] media: ov2740: allow OTP data access during streaming
2020-11-11 6:39 [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user Bingbu Cao
@ 2020-11-11 6:39 ` Bingbu Cao
2020-11-12 5:11 ` Tomasz Figa
2020-11-11 10:36 ` [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user Sakari Ailus
2020-11-12 5:05 ` Tomasz Figa
2 siblings, 1 reply; 6+ messages in thread
From: Bingbu Cao @ 2020-11-11 6:39 UTC (permalink / raw)
To: linux-media, sakari.ailus
Cc: tfiga, senozhatsky, bingbu.cao, bingbu.cao, qingwu.zhang
OTP data access of ov2740 need enable the streaming mode to load
and it could be done in any time, so driver need allow the OTP
data access during streaming instead of return EBUSY, this patch
try to read the OTP data out in STREAMON if OTP data is not ready
before first time streaming start.
Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
drivers/media/i2c/ov2740.c | 189 +++++++++++++++++++++++----------------------
1 file changed, 96 insertions(+), 93 deletions(-)
diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 41c17df46f47..d723e0e7adf7 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -598,13 +598,109 @@ static void ov2740_update_pad_format(const struct ov2740_mode *mode,
fmt->field = V4L2_FIELD_NONE;
}
+static int ov2740_load_otp_data(struct nvm_data *nvm)
+{
+ struct i2c_client *client = nvm->client;
+ struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
+ u32 isp_ctrl00 = 0;
+ u32 isp_ctrl01 = 0;
+ int ret;
+
+ if (!nvm)
+ return -EINVAL;
+
+ if (nvm->nvm_buffer)
+ return 0;
+
+ nvm->nvm_buffer = kzalloc(CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
+ if (!nvm->nvm_buffer)
+ return -ENOMEM;
+
+ ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, &isp_ctrl00);
+ if (ret) {
+ dev_err(&client->dev, "failed to read ISP CTRL00\n");
+ goto err;
+ }
+
+ ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, &isp_ctrl01);
+ if (ret) {
+ dev_err(&client->dev, "failed to read ISP CTRL01\n");
+ goto err;
+ }
+
+ /* Clear bit 5 of ISP CTRL00 */
+ ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1,
+ isp_ctrl00 & ~BIT(5));
+ if (ret) {
+ dev_err(&client->dev, "failed to set ISP CTRL00\n");
+ goto err;
+ }
+
+ /* Clear bit 7 of ISP CTRL01 */
+ ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1,
+ isp_ctrl01 & ~BIT(7));
+ if (ret) {
+ dev_err(&client->dev, "failed to set ISP CTRL01\n");
+ goto err;
+ }
+
+ ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
+ OV2740_MODE_STREAMING);
+ if (ret) {
+ dev_err(&client->dev, "failed to set streaming mode\n");
+ goto err;
+ }
+
+ /*
+ * Users are not allowed to access OTP-related registers and memory
+ * during the 20 ms period after streaming starts (0x100 = 0x01).
+ */
+ msleep(20);
+
+ ret = regmap_bulk_read(nvm->regmap, OV2740_REG_OTP_CUSTOMER,
+ nvm->nvm_buffer, CUSTOMER_USE_OTP_SIZE);
+ if (ret) {
+ dev_err(&client->dev, "failed to read OTP data, ret %d\n", ret);
+ goto err;
+ }
+
+ ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
+ OV2740_MODE_STANDBY);
+ if (ret) {
+ dev_err(&client->dev, "failed to set streaming mode\n");
+ goto err;
+ }
+
+ ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
+ if (ret) {
+ dev_err(&client->dev, "failed to set ISP CTRL01\n");
+ goto err;
+ }
+
+ ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
+ if (ret) {
+ dev_err(&client->dev, "failed to set ISP CTRL00\n");
+ goto err;
+ }
+
+ return 0;
+err:
+ kfree(nvm->nvm_buffer);
+ nvm->nvm_buffer = NULL;
+
+ return ret;
+}
+
static int ov2740_start_streaming(struct ov2740 *ov2740)
{
struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd);
+ struct nvm_data *nvm = ov2740->nvm;
const struct ov2740_reg_list *reg_list;
int link_freq_index;
int ret = 0;
+ ov2740_load_otp_data(nvm);
+
link_freq_index = ov2740->cur_mode->link_freq_index;
reg_list = &link_freq_configs[link_freq_index].reg_list;
ret = ov2740_write_reg_list(ov2740, reg_list);
@@ -934,99 +1030,6 @@ static int ov2740_remove(struct i2c_client *client)
return 0;
}
-static int ov2740_load_otp_data(struct nvm_data *nvm)
-{
- struct i2c_client *client = nvm->client;
- struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
- u32 isp_ctrl00 = 0;
- u32 isp_ctrl01 = 0;
- int ret;
-
- if (!nvm)
- return -EINVAL;
-
- if (nvm->nvm_buffer)
- return 0;
-
- nvm->nvm_buffer = kzalloc(CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
- if (!nvm->nvm_buffer)
- return -ENOMEM;
-
- ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, &isp_ctrl00);
- if (ret) {
- dev_err(&client->dev, "failed to read ISP CTRL00\n");
- goto err;
- }
-
- ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, &isp_ctrl01);
- if (ret) {
- dev_err(&client->dev, "failed to read ISP CTRL01\n");
- goto err;
- }
-
- /* Clear bit 5 of ISP CTRL00 */
- ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1,
- isp_ctrl00 & ~BIT(5));
- if (ret) {
- dev_err(&client->dev, "failed to set ISP CTRL00\n");
- goto err;
- }
-
- /* Clear bit 7 of ISP CTRL01 */
- ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1,
- isp_ctrl01 & ~BIT(7));
- if (ret) {
- dev_err(&client->dev, "failed to set ISP CTRL01\n");
- goto err;
- }
-
- ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
- OV2740_MODE_STREAMING);
- if (ret) {
- dev_err(&client->dev, "failed to set streaming mode\n");
- goto err;
- }
-
- /*
- * Users are not allowed to access OTP-related registers and memory
- * during the 20 ms period after streaming starts (0x100 = 0x01).
- */
- msleep(20);
-
- ret = regmap_bulk_read(nvm->regmap, OV2740_REG_OTP_CUSTOMER,
- nvm->nvm_buffer, CUSTOMER_USE_OTP_SIZE);
- if (ret) {
- dev_err(&client->dev, "failed to read OTP data, ret %d\n", ret);
- goto err;
- }
-
- ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
- OV2740_MODE_STANDBY);
- if (ret) {
- dev_err(&client->dev, "failed to set streaming mode\n");
- goto err;
- }
-
- ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
- if (ret) {
- dev_err(&client->dev, "failed to set ISP CTRL01\n");
- goto err;
- }
-
- ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
- if (ret) {
- dev_err(&client->dev, "failed to set ISP CTRL00\n");
- goto err;
- }
-
- return 0;
-err:
- kfree(nvm->nvm_buffer);
- nvm->nvm_buffer = NULL;
-
- return ret;
-}
-
static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
size_t count)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user
2020-11-11 6:39 [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user Bingbu Cao
2020-11-11 6:39 ` [PATCH v3 2/2] media: ov2740: allow OTP data access during streaming Bingbu Cao
@ 2020-11-11 10:36 ` Sakari Ailus
2020-11-11 11:53 ` Cao, Bingbu
2020-11-12 5:05 ` Tomasz Figa
2 siblings, 1 reply; 6+ messages in thread
From: Sakari Ailus @ 2020-11-11 10:36 UTC (permalink / raw)
To: Bingbu Cao; +Cc: linux-media, tfiga, senozhatsky, bingbu.cao, qingwu.zhang
Hi Bingbu,
On Wed, Nov 11, 2020 at 02:39:56PM +0800, Bingbu Cao wrote:
> OTP data access of ov2740 in probe need power up, it may cause
> the camera flash LED blink during probe if the LED use same power
> rail with camera, this patch move the OTP data access out of
> probe, it will only occur on demand from user by nvmem sysfs.
>
> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> Signed-off-by: Qingwu Zhang <qingwu.zhang@intel.com>
> ---
> drivers/media/i2c/ov2740.c | 111 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 79 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 64ecb6917dd3..41c17df46f47 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -71,9 +71,10 @@
> #define OV2740_REG_OTP_CUSTOMER 0x7010
>
> struct nvm_data {
> - char *nvm_buffer;
> + struct i2c_client *client;
> struct nvmem_device *nvmem;
> struct regmap *regmap;
> + char *nvm_buffer;
> };
>
> enum {
> @@ -335,6 +336,9 @@ struct ov2740 {
>
> /* Streaming on/off */
> bool streaming;
> +
> + /* NVM data inforamtion */
> + struct nvm_data *nvm;
> };
>
> static inline struct ov2740 *to_ov2740(struct v4l2_subdev *subdev)
> @@ -930,45 +934,57 @@ static int ov2740_remove(struct i2c_client *client)
> return 0;
> }
>
> -static int ov2740_load_otp_data(struct i2c_client *client, struct nvm_data *nvm)
> +static int ov2740_load_otp_data(struct nvm_data *nvm)
> {
> + struct i2c_client *client = nvm->client;
> struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
> u32 isp_ctrl00 = 0;
> u32 isp_ctrl01 = 0;
> int ret;
>
> + if (!nvm)
> + return -EINVAL;
> +
> + if (nvm->nvm_buffer)
> + return 0;
> +
> + nvm->nvm_buffer = kzalloc(CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
> + if (!nvm->nvm_buffer)
> + return -ENOMEM;
> +
> ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, &isp_ctrl00);
> if (ret) {
> dev_err(&client->dev, "failed to read ISP CTRL00\n");
> - goto exit;
> + goto err;
> }
> +
> ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, &isp_ctrl01);
> if (ret) {
> dev_err(&client->dev, "failed to read ISP CTRL01\n");
> - goto exit;
> + goto err;
> }
>
> /* Clear bit 5 of ISP CTRL00 */
> ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1,
> isp_ctrl00 & ~BIT(5));
> if (ret) {
> - dev_err(&client->dev, "failed to write ISP CTRL00\n");
> - goto exit;
> + dev_err(&client->dev, "failed to set ISP CTRL00\n");
> + goto err;
> }
>
> /* Clear bit 7 of ISP CTRL01 */
> ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1,
> isp_ctrl01 & ~BIT(7));
> if (ret) {
> - dev_err(&client->dev, "failed to write ISP CTRL01\n");
> - goto exit;
> + dev_err(&client->dev, "failed to set ISP CTRL01\n");
> + goto err;
> }
>
> ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> OV2740_MODE_STREAMING);
> if (ret) {
> - dev_err(&client->dev, "failed to start streaming\n");
> - goto exit;
> + dev_err(&client->dev, "failed to set streaming mode\n");
> + goto err;
> }
>
> /*
> @@ -981,15 +997,33 @@ static int ov2740_load_otp_data(struct i2c_client *client, struct nvm_data *nvm)
> nvm->nvm_buffer, CUSTOMER_USE_OTP_SIZE);
> if (ret) {
> dev_err(&client->dev, "failed to read OTP data, ret %d\n", ret);
> - goto exit;
> + goto err;
> }
>
> - ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> - OV2740_MODE_STANDBY);
> - ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
> - ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
> + ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> + OV2740_MODE_STANDBY);
> + if (ret) {
> + dev_err(&client->dev, "failed to set streaming mode\n");
> + goto err;
> + }
> +
> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
> + if (ret) {
> + dev_err(&client->dev, "failed to set ISP CTRL01\n");
> + goto err;
> + }
> +
> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
> + if (ret) {
> + dev_err(&client->dev, "failed to set ISP CTRL00\n");
> + goto err;
> + }
> +
> + return 0;
> +err:
> + kfree(nvm->nvm_buffer);
> + nvm->nvm_buffer = NULL;
>
> -exit:
> return ret;
> }
>
> @@ -997,29 +1031,43 @@ static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
> size_t count)
> {
> struct nvm_data *nvm = priv;
> + struct v4l2_subdev *sd = i2c_get_clientdata(nvm->client);
> + struct device *dev = &nvm->client->dev;
> + struct ov2740 *ov2740 = to_ov2740(sd);
> + int ret = 0;
> +
> + mutex_lock(&ov2740->mutex);
>
> - memcpy(val, nvm->nvm_buffer + off, count);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(dev);
> + goto exit;
> + }
>
> - return 0;
> + ret = ov2740_load_otp_data(nvm);
ov2740_load_otp_data() only needs to access the device on the first time.
Could you move resuming the device where it's really needed, please?
Can be a separate patch. Up to you.
> + if (!ret)
> + memcpy(val, nvm->nvm_buffer + off, count);
> +
> + pm_runtime_put(dev);
> +exit:
> + mutex_unlock(&ov2740->mutex);
> + return ret;
> }
>
> -static int ov2740_register_nvmem(struct i2c_client *client)
> +static int ov2740_register_nvmem(struct i2c_client *client,
> + struct ov2740 *ov2740)
> {
> struct nvm_data *nvm;
> struct regmap_config regmap_config = { };
> struct nvmem_config nvmem_config = { };
> struct regmap *regmap;
> struct device *dev = &client->dev;
> - int ret = 0;
> + int ret;
>
> nvm = devm_kzalloc(dev, sizeof(*nvm), GFP_KERNEL);
> if (!nvm)
> return -ENOMEM;
>
> - nvm->nvm_buffer = devm_kzalloc(dev, CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
> - if (!nvm->nvm_buffer)
> - return -ENOMEM;
> -
> regmap_config.val_bits = 8;
> regmap_config.reg_bits = 16;
> regmap_config.disable_locking = true;
> @@ -1028,12 +1076,7 @@ static int ov2740_register_nvmem(struct i2c_client *client)
> return PTR_ERR(regmap);
>
> nvm->regmap = regmap;
> -
> - ret = ov2740_load_otp_data(client, nvm);
> - if (ret) {
> - dev_err(dev, "failed to load OTP data, ret %d\n", ret);
> - return ret;
> - }
> + nvm->client = client;
>
> nvmem_config.name = dev_name(dev);
> nvmem_config.dev = dev;
> @@ -1051,7 +1094,11 @@ static int ov2740_register_nvmem(struct i2c_client *client)
>
> nvm->nvmem = devm_nvmem_register(dev, &nvmem_config);
>
> - return PTR_ERR_OR_ZERO(nvm->nvmem);
> + ret = PTR_ERR_OR_ZERO(nvm->nvmem);
> + if (!ret)
> + ov2740->nvm = nvm;
> +
> + return ret;
> }
>
> static int ov2740_probe(struct i2c_client *client)
> @@ -1103,7 +1150,7 @@ static int ov2740_probe(struct i2c_client *client)
> goto probe_error_media_entity_cleanup;
> }
>
> - ret = ov2740_register_nvmem(client);
> + ret = ov2740_register_nvmem(client, ov2740);
> if (ret)
> dev_warn(&client->dev, "register nvmem failed, ret %d\n", ret);
>
> --
> 2.7.4
>
--
Sakari Ailus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user
2020-11-11 10:36 ` [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user Sakari Ailus
@ 2020-11-11 11:53 ` Cao, Bingbu
0 siblings, 0 replies; 6+ messages in thread
From: Cao, Bingbu @ 2020-11-11 11:53 UTC (permalink / raw)
To: Sakari Ailus, Bingbu Cao; +Cc: linux-media, tfiga, senozhatsky, qingwu.zhang
On 11/11/20 6:36 PM, Sakari Ailus wrote:
> Hi Bingbu,
>
> On Wed, Nov 11, 2020 at 02:39:56PM +0800, Bingbu Cao wrote:
>> OTP data access of ov2740 in probe need power up, it may cause
>> the camera flash LED blink during probe if the LED use same power
>> rail with camera, this patch move the OTP data access out of
>> probe, it will only occur on demand from user by nvmem sysfs.
>>
>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>> Signed-off-by: Qingwu Zhang <qingwu.zhang@intel.com>
>> ---
>> drivers/media/i2c/ov2740.c | 111 ++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 79 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
>> index 64ecb6917dd3..41c17df46f47 100644
>> --- a/drivers/media/i2c/ov2740.c
>> +++ b/drivers/media/i2c/ov2740.c
>> @@ -71,9 +71,10 @@
>> #define OV2740_REG_OTP_CUSTOMER 0x7010
>>
>> struct nvm_data {
>> - char *nvm_buffer;
>> + struct i2c_client *client;
>> struct nvmem_device *nvmem;
>> struct regmap *regmap;
>> + char *nvm_buffer;
>> };
>>
>> enum {
>> @@ -335,6 +336,9 @@ struct ov2740 {
>>
>> /* Streaming on/off */
>> bool streaming;
>> +
>> + /* NVM data inforamtion */
>> + struct nvm_data *nvm;
>> };
>>
>> static inline struct ov2740 *to_ov2740(struct v4l2_subdev *subdev)
>> @@ -930,45 +934,57 @@ static int ov2740_remove(struct i2c_client *client)
>> return 0;
>> }
>>
>> -static int ov2740_load_otp_data(struct i2c_client *client, struct nvm_data *nvm)
>> +static int ov2740_load_otp_data(struct nvm_data *nvm)
>> {
>> + struct i2c_client *client = nvm->client;
>> struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
>> u32 isp_ctrl00 = 0;
>> u32 isp_ctrl01 = 0;
>> int ret;
>>
>> + if (!nvm)
>> + return -EINVAL;
>> +
>> + if (nvm->nvm_buffer)
>> + return 0;
>> +
>> + nvm->nvm_buffer = kzalloc(CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
>> + if (!nvm->nvm_buffer)
>> + return -ENOMEM;
>> +
>> ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, &isp_ctrl00);
>> if (ret) {
>> dev_err(&client->dev, "failed to read ISP CTRL00\n");
>> - goto exit;
>> + goto err;
>> }
>> +
>> ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, &isp_ctrl01);
>> if (ret) {
>> dev_err(&client->dev, "failed to read ISP CTRL01\n");
>> - goto exit;
>> + goto err;
>> }
>>
>> /* Clear bit 5 of ISP CTRL00 */
>> ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1,
>> isp_ctrl00 & ~BIT(5));
>> if (ret) {
>> - dev_err(&client->dev, "failed to write ISP CTRL00\n");
>> - goto exit;
>> + dev_err(&client->dev, "failed to set ISP CTRL00\n");
>> + goto err;
>> }
>>
>> /* Clear bit 7 of ISP CTRL01 */
>> ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1,
>> isp_ctrl01 & ~BIT(7));
>> if (ret) {
>> - dev_err(&client->dev, "failed to write ISP CTRL01\n");
>> - goto exit;
>> + dev_err(&client->dev, "failed to set ISP CTRL01\n");
>> + goto err;
>> }
>>
>> ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
>> OV2740_MODE_STREAMING);
>> if (ret) {
>> - dev_err(&client->dev, "failed to start streaming\n");
>> - goto exit;
>> + dev_err(&client->dev, "failed to set streaming mode\n");
>> + goto err;
>> }
>>
>> /*
>> @@ -981,15 +997,33 @@ static int ov2740_load_otp_data(struct i2c_client *client, struct nvm_data *nvm)
>> nvm->nvm_buffer, CUSTOMER_USE_OTP_SIZE);
>> if (ret) {
>> dev_err(&client->dev, "failed to read OTP data, ret %d\n", ret);
>> - goto exit;
>> + goto err;
>> }
>>
>> - ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
>> - OV2740_MODE_STANDBY);
>> - ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
>> - ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
>> + ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
>> + OV2740_MODE_STANDBY);
>> + if (ret) {
>> + dev_err(&client->dev, "failed to set streaming mode\n");
>> + goto err;
>> + }
>> +
>> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
>> + if (ret) {
>> + dev_err(&client->dev, "failed to set ISP CTRL01\n");
>> + goto err;
>> + }
>> +
>> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
>> + if (ret) {
>> + dev_err(&client->dev, "failed to set ISP CTRL00\n");
>> + goto err;
>> + }
>> +
>> + return 0;
>> +err:
>> + kfree(nvm->nvm_buffer);
>> + nvm->nvm_buffer = NULL;
>>
>> -exit:
>> return ret;
>> }
>>
>> @@ -997,29 +1031,43 @@ static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
>> size_t count)
>> {
>> struct nvm_data *nvm = priv;
>> + struct v4l2_subdev *sd = i2c_get_clientdata(nvm->client);
>> + struct device *dev = &nvm->client->dev;
>> + struct ov2740 *ov2740 = to_ov2740(sd);
>> + int ret = 0;
>> +
>> + mutex_lock(&ov2740->mutex);
>>
>> - memcpy(val, nvm->nvm_buffer + off, count);
>> + ret = pm_runtime_get_sync(dev);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(dev);
>> + goto exit;
>> + }
>>
>> - return 0;
>> + ret = ov2740_load_otp_data(nvm);
>
> ov2740_load_otp_data() only needs to access the device on the first time.
> Could you move resuming the device where it's really needed, please?
>
> Can be a separate patch. Up to you.
Good point, will address in v4.
......
mutex_lock(&ov2740->mutex);
+ if (nvm->nvm_buffer) {
+ memcpy(val, nvm->nvm_buffer + off, count);
+ goto exit;
+ }
+
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
pm_runtime_put_noidle(dev);
....
>
>> + if (!ret)
>> + memcpy(val, nvm->nvm_buffer + off, count);
>> +
>> + pm_runtime_put(dev);
>> +exit:
>> + mutex_unlock(&ov2740->mutex);
>> + return ret;
>> }
>>
>> -static int ov2740_register_nvmem(struct i2c_client *client)
>> +static int ov2740_register_nvmem(struct i2c_client *client,
>> + struct ov2740 *ov2740)
>> {
>> struct nvm_data *nvm;
>> struct regmap_config regmap_config = { };
>> struct nvmem_config nvmem_config = { };
>> struct regmap *regmap;
>> struct device *dev = &client->dev;
>> - int ret = 0;
>> + int ret;
>>
>> nvm = devm_kzalloc(dev, sizeof(*nvm), GFP_KERNEL);
>> if (!nvm)
>> return -ENOMEM;
>>
>> - nvm->nvm_buffer = devm_kzalloc(dev, CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
>> - if (!nvm->nvm_buffer)
>> - return -ENOMEM;
>> -
>> regmap_config.val_bits = 8;
>> regmap_config.reg_bits = 16;
>> regmap_config.disable_locking = true;
>> @@ -1028,12 +1076,7 @@ static int ov2740_register_nvmem(struct i2c_client *client)
>> return PTR_ERR(regmap);
>>
>> nvm->regmap = regmap;
>> -
>> - ret = ov2740_load_otp_data(client, nvm);
>> - if (ret) {
>> - dev_err(dev, "failed to load OTP data, ret %d\n", ret);
>> - return ret;
>> - }
>> + nvm->client = client;
>>
>> nvmem_config.name = dev_name(dev);
>> nvmem_config.dev = dev;
>> @@ -1051,7 +1094,11 @@ static int ov2740_register_nvmem(struct i2c_client *client)
>>
>> nvm->nvmem = devm_nvmem_register(dev, &nvmem_config);
>>
>> - return PTR_ERR_OR_ZERO(nvm->nvmem);
>> + ret = PTR_ERR_OR_ZERO(nvm->nvmem);
>> + if (!ret)
>> + ov2740->nvm = nvm;
>> +
>> + return ret;
>> }
>>
>> static int ov2740_probe(struct i2c_client *client)
>> @@ -1103,7 +1150,7 @@ static int ov2740_probe(struct i2c_client *client)
>> goto probe_error_media_entity_cleanup;
>> }
>>
>> - ret = ov2740_register_nvmem(client);
>> + ret = ov2740_register_nvmem(client, ov2740);
>> if (ret)
>> dev_warn(&client->dev, "register nvmem failed, ret %d\n", ret);
>>
>> --
>> 2.7.4
>>
>
--
Best regards,
Bingbu Cao
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user
2020-11-11 6:39 [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user Bingbu Cao
2020-11-11 6:39 ` [PATCH v3 2/2] media: ov2740: allow OTP data access during streaming Bingbu Cao
2020-11-11 10:36 ` [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user Sakari Ailus
@ 2020-11-12 5:05 ` Tomasz Figa
2 siblings, 0 replies; 6+ messages in thread
From: Tomasz Figa @ 2020-11-12 5:05 UTC (permalink / raw)
To: Bingbu Cao
Cc: linux-media, sakari.ailus, senozhatsky, bingbu.cao, qingwu.zhang
Hi Bingbu,
On Wed, Nov 11, 2020 at 02:39:56PM +0800, Bingbu Cao wrote:
> OTP data access of ov2740 in probe need power up, it may cause
> the camera flash LED blink during probe if the LED use same power
> rail with camera, this patch move the OTP data access out of
> probe, it will only occur on demand from user by nvmem sysfs.
>
> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> Signed-off-by: Qingwu Zhang <qingwu.zhang@intel.com>
> ---
> drivers/media/i2c/ov2740.c | 111 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 79 insertions(+), 32 deletions(-)
>
With Sakari's comment addressed:
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Best regards,
Tomasz
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 64ecb6917dd3..41c17df46f47 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -71,9 +71,10 @@
> #define OV2740_REG_OTP_CUSTOMER 0x7010
>
> struct nvm_data {
> - char *nvm_buffer;
> + struct i2c_client *client;
> struct nvmem_device *nvmem;
> struct regmap *regmap;
> + char *nvm_buffer;
> };
>
> enum {
> @@ -335,6 +336,9 @@ struct ov2740 {
>
> /* Streaming on/off */
> bool streaming;
> +
> + /* NVM data inforamtion */
> + struct nvm_data *nvm;
> };
>
> static inline struct ov2740 *to_ov2740(struct v4l2_subdev *subdev)
> @@ -930,45 +934,57 @@ static int ov2740_remove(struct i2c_client *client)
> return 0;
> }
>
> -static int ov2740_load_otp_data(struct i2c_client *client, struct nvm_data *nvm)
> +static int ov2740_load_otp_data(struct nvm_data *nvm)
> {
> + struct i2c_client *client = nvm->client;
> struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
> u32 isp_ctrl00 = 0;
> u32 isp_ctrl01 = 0;
> int ret;
>
> + if (!nvm)
> + return -EINVAL;
> +
> + if (nvm->nvm_buffer)
> + return 0;
> +
> + nvm->nvm_buffer = kzalloc(CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
> + if (!nvm->nvm_buffer)
> + return -ENOMEM;
> +
> ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, &isp_ctrl00);
> if (ret) {
> dev_err(&client->dev, "failed to read ISP CTRL00\n");
> - goto exit;
> + goto err;
> }
> +
> ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, &isp_ctrl01);
> if (ret) {
> dev_err(&client->dev, "failed to read ISP CTRL01\n");
> - goto exit;
> + goto err;
> }
>
> /* Clear bit 5 of ISP CTRL00 */
> ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1,
> isp_ctrl00 & ~BIT(5));
> if (ret) {
> - dev_err(&client->dev, "failed to write ISP CTRL00\n");
> - goto exit;
> + dev_err(&client->dev, "failed to set ISP CTRL00\n");
> + goto err;
> }
>
> /* Clear bit 7 of ISP CTRL01 */
> ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1,
> isp_ctrl01 & ~BIT(7));
> if (ret) {
> - dev_err(&client->dev, "failed to write ISP CTRL01\n");
> - goto exit;
> + dev_err(&client->dev, "failed to set ISP CTRL01\n");
> + goto err;
> }
>
> ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> OV2740_MODE_STREAMING);
> if (ret) {
> - dev_err(&client->dev, "failed to start streaming\n");
> - goto exit;
> + dev_err(&client->dev, "failed to set streaming mode\n");
> + goto err;
> }
>
> /*
> @@ -981,15 +997,33 @@ static int ov2740_load_otp_data(struct i2c_client *client, struct nvm_data *nvm)
> nvm->nvm_buffer, CUSTOMER_USE_OTP_SIZE);
> if (ret) {
> dev_err(&client->dev, "failed to read OTP data, ret %d\n", ret);
> - goto exit;
> + goto err;
> }
>
> - ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> - OV2740_MODE_STANDBY);
> - ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
> - ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
> + ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> + OV2740_MODE_STANDBY);
> + if (ret) {
> + dev_err(&client->dev, "failed to set streaming mode\n");
> + goto err;
> + }
> +
> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
> + if (ret) {
> + dev_err(&client->dev, "failed to set ISP CTRL01\n");
> + goto err;
> + }
> +
> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
> + if (ret) {
> + dev_err(&client->dev, "failed to set ISP CTRL00\n");
> + goto err;
> + }
> +
> + return 0;
> +err:
> + kfree(nvm->nvm_buffer);
> + nvm->nvm_buffer = NULL;
>
> -exit:
> return ret;
> }
>
> @@ -997,29 +1031,43 @@ static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
> size_t count)
> {
> struct nvm_data *nvm = priv;
> + struct v4l2_subdev *sd = i2c_get_clientdata(nvm->client);
> + struct device *dev = &nvm->client->dev;
> + struct ov2740 *ov2740 = to_ov2740(sd);
> + int ret = 0;
> +
> + mutex_lock(&ov2740->mutex);
>
> - memcpy(val, nvm->nvm_buffer + off, count);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(dev);
> + goto exit;
> + }
>
> - return 0;
> + ret = ov2740_load_otp_data(nvm);
> + if (!ret)
> + memcpy(val, nvm->nvm_buffer + off, count);
> +
> + pm_runtime_put(dev);
> +exit:
> + mutex_unlock(&ov2740->mutex);
> + return ret;
> }
>
> -static int ov2740_register_nvmem(struct i2c_client *client)
> +static int ov2740_register_nvmem(struct i2c_client *client,
> + struct ov2740 *ov2740)
> {
> struct nvm_data *nvm;
> struct regmap_config regmap_config = { };
> struct nvmem_config nvmem_config = { };
> struct regmap *regmap;
> struct device *dev = &client->dev;
> - int ret = 0;
> + int ret;
>
> nvm = devm_kzalloc(dev, sizeof(*nvm), GFP_KERNEL);
> if (!nvm)
> return -ENOMEM;
>
> - nvm->nvm_buffer = devm_kzalloc(dev, CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
> - if (!nvm->nvm_buffer)
> - return -ENOMEM;
> -
> regmap_config.val_bits = 8;
> regmap_config.reg_bits = 16;
> regmap_config.disable_locking = true;
> @@ -1028,12 +1076,7 @@ static int ov2740_register_nvmem(struct i2c_client *client)
> return PTR_ERR(regmap);
>
> nvm->regmap = regmap;
> -
> - ret = ov2740_load_otp_data(client, nvm);
> - if (ret) {
> - dev_err(dev, "failed to load OTP data, ret %d\n", ret);
> - return ret;
> - }
> + nvm->client = client;
>
> nvmem_config.name = dev_name(dev);
> nvmem_config.dev = dev;
> @@ -1051,7 +1094,11 @@ static int ov2740_register_nvmem(struct i2c_client *client)
>
> nvm->nvmem = devm_nvmem_register(dev, &nvmem_config);
>
> - return PTR_ERR_OR_ZERO(nvm->nvmem);
> + ret = PTR_ERR_OR_ZERO(nvm->nvmem);
> + if (!ret)
> + ov2740->nvm = nvm;
> +
> + return ret;
> }
>
> static int ov2740_probe(struct i2c_client *client)
> @@ -1103,7 +1150,7 @@ static int ov2740_probe(struct i2c_client *client)
> goto probe_error_media_entity_cleanup;
> }
>
> - ret = ov2740_register_nvmem(client);
> + ret = ov2740_register_nvmem(client, ov2740);
> if (ret)
> dev_warn(&client->dev, "register nvmem failed, ret %d\n", ret);
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] media: ov2740: allow OTP data access during streaming
2020-11-11 6:39 ` [PATCH v3 2/2] media: ov2740: allow OTP data access during streaming Bingbu Cao
@ 2020-11-12 5:11 ` Tomasz Figa
0 siblings, 0 replies; 6+ messages in thread
From: Tomasz Figa @ 2020-11-12 5:11 UTC (permalink / raw)
To: Bingbu Cao
Cc: linux-media, sakari.ailus, senozhatsky, bingbu.cao, qingwu.zhang
On Wed, Nov 11, 2020 at 02:39:57PM +0800, Bingbu Cao wrote:
> OTP data access of ov2740 need enable the streaming mode to load
> and it could be done in any time, so driver need allow the OTP
> data access during streaming instead of return EBUSY, this patch
> try to read the OTP data out in STREAMON if OTP data is not ready
> before first time streaming start.
>
> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> ---
> drivers/media/i2c/ov2740.c | 189 +++++++++++++++++++++++----------------------
> 1 file changed, 96 insertions(+), 93 deletions(-)
>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Best regards,
Tomasz
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 41c17df46f47..d723e0e7adf7 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -598,13 +598,109 @@ static void ov2740_update_pad_format(const struct ov2740_mode *mode,
> fmt->field = V4L2_FIELD_NONE;
> }
>
> +static int ov2740_load_otp_data(struct nvm_data *nvm)
> +{
> + struct i2c_client *client = nvm->client;
> + struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
> + u32 isp_ctrl00 = 0;
> + u32 isp_ctrl01 = 0;
> + int ret;
> +
> + if (!nvm)
> + return -EINVAL;
> +
> + if (nvm->nvm_buffer)
> + return 0;
> +
> + nvm->nvm_buffer = kzalloc(CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
> + if (!nvm->nvm_buffer)
> + return -ENOMEM;
> +
> + ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, &isp_ctrl00);
> + if (ret) {
> + dev_err(&client->dev, "failed to read ISP CTRL00\n");
> + goto err;
> + }
> +
> + ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, &isp_ctrl01);
> + if (ret) {
> + dev_err(&client->dev, "failed to read ISP CTRL01\n");
> + goto err;
> + }
> +
> + /* Clear bit 5 of ISP CTRL00 */
> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1,
> + isp_ctrl00 & ~BIT(5));
> + if (ret) {
> + dev_err(&client->dev, "failed to set ISP CTRL00\n");
> + goto err;
> + }
> +
> + /* Clear bit 7 of ISP CTRL01 */
> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1,
> + isp_ctrl01 & ~BIT(7));
> + if (ret) {
> + dev_err(&client->dev, "failed to set ISP CTRL01\n");
> + goto err;
> + }
> +
> + ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> + OV2740_MODE_STREAMING);
> + if (ret) {
> + dev_err(&client->dev, "failed to set streaming mode\n");
> + goto err;
> + }
> +
> + /*
> + * Users are not allowed to access OTP-related registers and memory
> + * during the 20 ms period after streaming starts (0x100 = 0x01).
> + */
> + msleep(20);
> +
> + ret = regmap_bulk_read(nvm->regmap, OV2740_REG_OTP_CUSTOMER,
> + nvm->nvm_buffer, CUSTOMER_USE_OTP_SIZE);
> + if (ret) {
> + dev_err(&client->dev, "failed to read OTP data, ret %d\n", ret);
> + goto err;
> + }
> +
> + ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> + OV2740_MODE_STANDBY);
> + if (ret) {
> + dev_err(&client->dev, "failed to set streaming mode\n");
> + goto err;
> + }
> +
> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
> + if (ret) {
> + dev_err(&client->dev, "failed to set ISP CTRL01\n");
> + goto err;
> + }
> +
> + ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
> + if (ret) {
> + dev_err(&client->dev, "failed to set ISP CTRL00\n");
> + goto err;
> + }
> +
> + return 0;
> +err:
> + kfree(nvm->nvm_buffer);
> + nvm->nvm_buffer = NULL;
> +
> + return ret;
> +}
> +
> static int ov2740_start_streaming(struct ov2740 *ov2740)
> {
> struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd);
> + struct nvm_data *nvm = ov2740->nvm;
> const struct ov2740_reg_list *reg_list;
> int link_freq_index;
> int ret = 0;
>
> + ov2740_load_otp_data(nvm);
> +
> link_freq_index = ov2740->cur_mode->link_freq_index;
> reg_list = &link_freq_configs[link_freq_index].reg_list;
> ret = ov2740_write_reg_list(ov2740, reg_list);
> @@ -934,99 +1030,6 @@ static int ov2740_remove(struct i2c_client *client)
> return 0;
> }
>
> -static int ov2740_load_otp_data(struct nvm_data *nvm)
> -{
> - struct i2c_client *client = nvm->client;
> - struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
> - u32 isp_ctrl00 = 0;
> - u32 isp_ctrl01 = 0;
> - int ret;
> -
> - if (!nvm)
> - return -EINVAL;
> -
> - if (nvm->nvm_buffer)
> - return 0;
> -
> - nvm->nvm_buffer = kzalloc(CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
> - if (!nvm->nvm_buffer)
> - return -ENOMEM;
> -
> - ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, &isp_ctrl00);
> - if (ret) {
> - dev_err(&client->dev, "failed to read ISP CTRL00\n");
> - goto err;
> - }
> -
> - ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, &isp_ctrl01);
> - if (ret) {
> - dev_err(&client->dev, "failed to read ISP CTRL01\n");
> - goto err;
> - }
> -
> - /* Clear bit 5 of ISP CTRL00 */
> - ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1,
> - isp_ctrl00 & ~BIT(5));
> - if (ret) {
> - dev_err(&client->dev, "failed to set ISP CTRL00\n");
> - goto err;
> - }
> -
> - /* Clear bit 7 of ISP CTRL01 */
> - ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1,
> - isp_ctrl01 & ~BIT(7));
> - if (ret) {
> - dev_err(&client->dev, "failed to set ISP CTRL01\n");
> - goto err;
> - }
> -
> - ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> - OV2740_MODE_STREAMING);
> - if (ret) {
> - dev_err(&client->dev, "failed to set streaming mode\n");
> - goto err;
> - }
> -
> - /*
> - * Users are not allowed to access OTP-related registers and memory
> - * during the 20 ms period after streaming starts (0x100 = 0x01).
> - */
> - msleep(20);
> -
> - ret = regmap_bulk_read(nvm->regmap, OV2740_REG_OTP_CUSTOMER,
> - nvm->nvm_buffer, CUSTOMER_USE_OTP_SIZE);
> - if (ret) {
> - dev_err(&client->dev, "failed to read OTP data, ret %d\n", ret);
> - goto err;
> - }
> -
> - ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
> - OV2740_MODE_STANDBY);
> - if (ret) {
> - dev_err(&client->dev, "failed to set streaming mode\n");
> - goto err;
> - }
> -
> - ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
> - if (ret) {
> - dev_err(&client->dev, "failed to set ISP CTRL01\n");
> - goto err;
> - }
> -
> - ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
> - if (ret) {
> - dev_err(&client->dev, "failed to set ISP CTRL00\n");
> - goto err;
> - }
> -
> - return 0;
> -err:
> - kfree(nvm->nvm_buffer);
> - nvm->nvm_buffer = NULL;
> -
> - return ret;
> -}
> -
> static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
> size_t count)
> {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-12 5:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 6:39 [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user Bingbu Cao
2020-11-11 6:39 ` [PATCH v3 2/2] media: ov2740: allow OTP data access during streaming Bingbu Cao
2020-11-12 5:11 ` Tomasz Figa
2020-11-11 10:36 ` [PATCH v3 1/2] media: ov2740: only do OTP data read on demand from user Sakari Ailus
2020-11-11 11:53 ` Cao, Bingbu
2020-11-12 5:05 ` Tomasz Figa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.