| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<PN3P287MB182970BB2FE2A28D5515A0428BBDA@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM>
Date: Wed, 31 Dec 2025 10:26:08 +0000
From: Tarang Raval <tarang.raval@...iconsignals.io>
To: Xiaolei Wang <xiaolei.wang@...driver.com>,
"laurent.pinchart@...asonboard.com" <laurent.pinchart@...asonboard.com>,
"sakari.ailus@...ux.intel.com" <sakari.ailus@...ux.intel.com>,
"dave.stevenson@...pberrypi.com" <dave.stevenson@...pberrypi.com>,
"jacopo@...ndi.org" <jacopo@...ndi.org>, "mchehab@...nel.org"
<mchehab@...nel.org>, "prabhakar.mahadev-lad.rj@...renesas.com"
<prabhakar.mahadev-lad.rj@...renesas.com>, "hverkuil+cisco@...nel.org"
<hverkuil+cisco@...nel.org>, "johannes.goede@....qualcomm.com"
<johannes.goede@....qualcomm.com>, "hverkuil-cisco@...all.nl"
<hverkuil-cisco@...all.nl>, "jai.luthra@...asonboard.com"
<jai.luthra@...asonboard.com>
CC: "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/3] media: i2c: ov5647: Convert to CCI register access
helpers
Hi Xiaolei,
> Use the new common CCI register access helpers to replace the private
> register access helpers in the ov5647 driver. This simplifies the driver
> by reducing the amount of code.
>
> Signed-off-by: Xiaolei Wang <xiaolei.wang@...driver.com>
> ---
> drivers/media/i2c/Kconfig | 1 +
> drivers/media/i2c/ov5647.c | 289 +++++++++++++------------------------
> 2 files changed, 99 insertions(+), 191 deletions(-)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 4b4db8c4f496..cce63349e71e 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -529,6 +529,7 @@ config VIDEO_OV5645
>
> config VIDEO_OV5647
> tristate "OmniVision OV5647 sensor support"
> + select V4L2_CCI_I2C
> help
> This is a Video4Linux2 sensor driver for the OmniVision
> OV5647 camera.
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> index e193fef4fced..cbcb760ba5cd 100644
> --- a/drivers/media/i2c/ov5647.c
> +++ b/drivers/media/i2c/ov5647.c
> @@ -20,8 +20,10 @@
> #include <linux/module.h>
> #include <linux/of_graph.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> #include <linux/slab.h>
> #include <linux/videodev2.h>
> +#include <media/v4l2-cci.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-event.h>
> @@ -41,24 +43,19 @@
> #define MIPI_CTRL00_BUS_IDLE BIT(2)
> #define MIPI_CTRL00_CLOCK_LANE_DISABLE BIT(0)
>
> -#define OV5647_SW_STANDBY 0x0100
> -#define OV5647_SW_RESET 0x0103
> -#define OV5647_REG_CHIPID_H 0x300a
> -#define OV5647_REG_CHIPID_L 0x300b
> -#define OV5640_REG_PAD_OUT 0x300d
> -#define OV5647_REG_EXP_HI 0x3500
> -#define OV5647_REG_EXP_MID 0x3501
> -#define OV5647_REG_EXP_LO 0x3502
> -#define OV5647_REG_AEC_AGC 0x3503
> -#define OV5647_REG_GAIN_HI 0x350a
> -#define OV5647_REG_GAIN_LO 0x350b
> -#define OV5647_REG_VTS_HI 0x380e
> -#define OV5647_REG_VTS_LO 0x380f
> -#define OV5647_REG_FRAME_OFF_NUMBER 0x4202
> -#define OV5647_REG_MIPI_CTRL00 0x4800
> -#define OV5647_REG_MIPI_CTRL14 0x4814
> -#define OV5647_REG_AWB 0x5001
> -#define OV5647_REG_ISPCTRL3D 0x503d
> +#define OV5647_SW_STANDBY CCI_REG8(0x0100)
> +#define OV5647_SW_RESET CCI_REG8(0x0103)
> +#define OV5647_REG_CHIPID CCI_REG16(0x300a)
> +#define OV5640_REG_PAD_OUT CCI_REG8(0x300d)
> +#define OV5647_REG_EXPOSURE CCI_REG24(0x3500)
> +#define OV5647_REG_AEC_AGC CCI_REG8(0x3503)
> +#define OV5647_REG_GAIN CCI_REG16(0x350a)
> +#define OV5647_REG_VTS CCI_REG16(0x380e)
> +#define OV5647_REG_FRAME_OFF_NUMBER CCI_REG8(0x4202)
> +#define OV5647_REG_MIPI_CTRL00 CCI_REG8(0x4800)
> +#define OV5647_REG_MIPI_CTRL14 CCI_REG8(0x4814)
> +#define OV5647_REG_AWB CCI_REG8(0x5001)
> +#define OV5647_REG_ISPCTRL3D CCI_REG8(0x503d)
>
> #define REG_TERM 0xfffe
> #define VAL_TERM 0xfe
> @@ -81,23 +78,19 @@
> #define OV5647_EXPOSURE_DEFAULT 1000
> #define OV5647_EXPOSURE_MAX 65535
>
> -struct regval_list {
> - u16 addr;
> - u8 data;
> -};
> -
> struct ov5647_mode {
> struct v4l2_mbus_framefmt format;
> struct v4l2_rect crop;
> u64 pixel_rate;
> int hts;
> int vts;
> - const struct regval_list *reg_list;
> + const struct reg_sequence *reg_list;
> unsigned int num_regs;
> };
>
> struct ov5647 {
> struct v4l2_subdev sd;
> + struct regmap *regmap;
> struct media_pad pad;
> struct mutex lock;
> struct clk *xclk;
> @@ -130,19 +123,19 @@ static const u8 ov5647_test_pattern_val[] = {
> 0x81, /* Random Data */
> };
>
> -static const struct regval_list sensor_oe_disable_regs[] = {
> +static const struct reg_sequence sensor_oe_disable_regs[] = {
> {0x3000, 0x00},
> {0x3001, 0x00},
> {0x3002, 0x00},
> };
>
> -static const struct regval_list sensor_oe_enable_regs[] = {
> +static const struct reg_sequence sensor_oe_enable_regs[] = {
> {0x3000, 0x0f},
> {0x3001, 0xff},
> {0x3002, 0xe4},
> };
>
> -static struct regval_list ov5647_2592x1944_10bpp[] = {
> +static const struct reg_sequence ov5647_2592x1944_10bpp[] = {
> {0x0100, 0x00},
> {0x0103, 0x01},
> {0x3034, 0x1a},
> @@ -230,8 +223,7 @@ static struct regval_list ov5647_2592x1944_10bpp[] = {
> {0x3503, 0x03},
> {0x0100, 0x01},
> };
> -
Please keep one blank line here. do not remove.
> -static struct regval_list ov5647_1080p30_10bpp[] = {
> +static const struct reg_sequence ov5647_1080p30_10bpp[] = {
> {0x0100, 0x00},
> {0x0103, 0x01},
> {0x3034, 0x1a},
> @@ -320,7 +312,7 @@ static struct regval_list ov5647_1080p30_10bpp[] = {
> {0x0100, 0x01},
> };
>
> -static struct regval_list ov5647_2x2binned_10bpp[] = {
> +static const struct reg_sequence ov5647_2x2binned_10bpp[] = {
> {0x0100, 0x00},
> {0x0103, 0x01},
> {0x3034, 0x1a},
> @@ -413,7 +405,7 @@ static struct regval_list ov5647_2x2binned_10bpp[] = {
> {0x0100, 0x01},
> };
>
> -static struct regval_list ov5647_640x480_10bpp[] = {
> +static const struct reg_sequence ov5647_640x480_10bpp[] = {
> {0x0100, 0x00},
> {0x0103, 0x01},
> {0x3035, 0x11},
> @@ -594,109 +586,35 @@ static const struct ov5647_mode ov5647_modes[] = {
> #define OV5647_DEFAULT_MODE (&ov5647_modes[3])
> #define OV5647_DEFAULT_FORMAT (ov5647_modes[3].format)
>
> -static int ov5647_write16(struct v4l2_subdev *sd, u16 reg, u16 val)
> -{
> - unsigned char data[4] = { reg >> 8, reg & 0xff, val >> 8, val & 0xff};
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
> - int ret;
> -
> - ret = i2c_master_send(client, data, 4);
> - if (ret < 0) {
> - dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
> - __func__, reg);
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
> -{
> - unsigned char data[3] = { reg >> 8, reg & 0xff, val};
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
> - int ret;
> -
> - ret = i2c_master_send(client, data, 3);
> - if (ret < 0) {
> - dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
> - __func__, reg);
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
> -{
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
> - u8 buf[2] = { reg >> 8, reg & 0xff };
> - struct i2c_msg msg[2];
> - int ret;
> -
> - msg[0].addr = client->addr;
> - msg[0].flags = client->flags;
> - msg[0].buf = buf;
> - msg[0].len = sizeof(buf);
> -
> - msg[1].addr = client->addr;
> - msg[1].flags = client->flags | I2C_M_RD;
> - msg[1].buf = buf;
> - msg[1].len = 1;
> -
> - ret = i2c_transfer(client->adapter, msg, 2);
> - if (ret != 2) {
> - dev_err(&client->dev, "%s: i2c read error, reg: %x = %d\n",
> - __func__, reg, ret);
> - return ret >= 0 ? -EINVAL : ret;
> - }
> -
> - *val = buf[0];
> -
> - return 0;
> -}
> -
> -static int ov5647_write_array(struct v4l2_subdev *sd,
> - const struct regval_list *regs, int array_size)
> -{
> - int i, ret;
> -
> - for (i = 0; i < array_size; i++) {
> - ret = ov5647_write(sd, regs[i].addr, regs[i].data);
> - if (ret < 0)
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> static int ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
> {
> - u8 channel_id;
> + struct ov5647 *sensor = to_sensor(sd);
> + u64 channel_id;
> int ret;
>
> - ret = ov5647_read(sd, OV5647_REG_MIPI_CTRL14, &channel_id);
> + ret = cci_read(sensor->regmap, OV5647_REG_MIPI_CTRL14, &channel_id, NULL);
> if (ret < 0)
> return ret;
>
> channel_id &= ~(3 << 6);
>
> - return ov5647_write(sd, OV5647_REG_MIPI_CTRL14,
> - channel_id | (channel << 6));
> + return cci_write(sensor->regmap, OV5647_REG_MIPI_CTRL14,
> + channel_id | (channel << 6), NULL);
> }
>
> static int ov5647_set_mode(struct v4l2_subdev *sd)
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> struct ov5647 *sensor = to_sensor(sd);
> - u8 resetval, rdval;
> + u64 resetval, rdval;
> int ret;
>
> - ret = ov5647_read(sd, OV5647_SW_STANDBY, &rdval);
> + ret = cci_read(sensor->regmap, OV5647_SW_STANDBY, &rdval, NULL);
> if (ret < 0)
> return ret;
>
> - ret = ov5647_write_array(sd, sensor->mode->reg_list,
> - sensor->mode->num_regs);
> + ret = regmap_multi_reg_write(sensor->regmap, sensor->mode->reg_list,
> + sensor->mode->num_regs);
> if (ret < 0) {
> dev_err(&client->dev, "write sensor default regs error\n");
> return ret;
> @@ -706,13 +624,13 @@ static int ov5647_set_mode(struct v4l2_subdev *sd)
> if (ret < 0)
> return ret;
>
> - ret = ov5647_read(sd, OV5647_SW_STANDBY, &resetval);
> + ret = cci_read(sensor->regmap, OV5647_SW_STANDBY, &resetval, NULL);
> if (ret < 0)
> return ret;
>
> if (!(resetval & 0x01)) {
> dev_err(&client->dev, "Device was in SW standby");
> - ret = ov5647_write(sd, OV5647_SW_STANDBY, 0x01);
> + ret = cci_write(sensor->regmap, OV5647_SW_STANDBY, 0x01, NULL);
> if (ret < 0)
> return ret;
This feels wrong to me but anyway this is not related to your patch.
> }
> @@ -725,7 +643,7 @@ static int ov5647_stream_on(struct v4l2_subdev *sd)
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> struct ov5647 *sensor = to_sensor(sd);
> u8 val = MIPI_CTRL00_BUS_IDLE;
> - int ret;
> + int ret = 0;
No need for zero initialization.
>
> ret = ov5647_set_mode(sd);
> if (ret) {
> @@ -742,32 +660,25 @@ static int ov5647_stream_on(struct v4l2_subdev *sd)
> val |= MIPI_CTRL00_CLOCK_LANE_GATE |
> MIPI_CTRL00_LINE_SYNC_ENABLE;
>
> - ret = ov5647_write(sd, OV5647_REG_MIPI_CTRL00, val);
> - if (ret < 0)
> - return ret;
> -
> - ret = ov5647_write(sd, OV5647_REG_FRAME_OFF_NUMBER, 0x00);
> - if (ret < 0)
> - return ret;
> + cci_write(sensor->regmap, OV5647_REG_MIPI_CTRL00, val, &ret);
> + cci_write(sensor->regmap, OV5647_REG_FRAME_OFF_NUMBER, 0x00, &ret);
> + cci_write(sensor->regmap, OV5640_REG_PAD_OUT, 0x00, &ret);
>
> - return ov5647_write(sd, OV5640_REG_PAD_OUT, 0x00);
> + return ret;
> }
>
> static int ov5647_stream_off(struct v4l2_subdev *sd)
> {
> - int ret;
> + struct ov5647 *sensor = to_sensor(sd);
> + int ret = 0;
>
> - ret = ov5647_write(sd, OV5647_REG_MIPI_CTRL00,
> - MIPI_CTRL00_CLOCK_LANE_GATE | MIPI_CTRL00_BUS_IDLE |
> - MIPI_CTRL00_CLOCK_LANE_DISABLE);
> - if (ret < 0)
> - return ret;
> + cci_write(sensor->regmap, OV5647_REG_MIPI_CTRL00,
> + MIPI_CTRL00_CLOCK_LANE_GATE | MIPI_CTRL00_BUS_IDLE |
> + MIPI_CTRL00_CLOCK_LANE_DISABLE, &ret);
> + cci_write(sensor->regmap, OV5647_REG_FRAME_OFF_NUMBER, 0x0f, &ret);
> + cci_write(sensor->regmap, OV5640_REG_PAD_OUT, 0x01, &ret);
>
> - ret = ov5647_write(sd, OV5647_REG_FRAME_OFF_NUMBER, 0x0f);
> - if (ret < 0)
> - return ret;
> -
> - return ov5647_write(sd, OV5640_REG_PAD_OUT, 0x01);
> + return ret;
> }
>
> static int ov5647_power_on(struct device *dev)
> @@ -788,8 +699,8 @@ static int ov5647_power_on(struct device *dev)
> goto error_pwdn;
> }
>
> - ret = ov5647_write_array(&sensor->sd, sensor_oe_enable_regs,
> - ARRAY_SIZE(sensor_oe_enable_regs));
> + ret = regmap_multi_reg_write(sensor->regmap, sensor_oe_enable_regs,
> + ARRAY_SIZE(sensor_oe_enable_regs));
> if (ret < 0) {
> dev_err(dev, "write sensor_oe_enable_regs error\n");
> goto error_clk_disable;
> @@ -815,23 +726,23 @@ static int ov5647_power_on(struct device *dev)
> static int ov5647_power_off(struct device *dev)
> {
> struct ov5647 *sensor = dev_get_drvdata(dev);
> - u8 rdval;
> + u64 rdval;
> int ret;
>
> dev_dbg(dev, "OV5647 power off\n");
>
> - ret = ov5647_write_array(&sensor->sd, sensor_oe_disable_regs,
> - ARRAY_SIZE(sensor_oe_disable_regs));
> + ret = regmap_multi_reg_write(sensor->regmap, sensor_oe_disable_regs,
> + ARRAY_SIZE(sensor_oe_disable_regs));
> if (ret < 0)
> dev_dbg(dev, "disable oe failed\n");
>
> /* Enter software standby */
> - ret = ov5647_read(&sensor->sd, OV5647_SW_STANDBY, &rdval);
> + ret = cci_read(sensor->regmap, OV5647_SW_STANDBY, &rdval, NULL);
> if (ret < 0)
> dev_dbg(dev, "software standby failed\n");
>
> rdval &= ~0x01;
> - ret = ov5647_write(&sensor->sd, OV5647_SW_STANDBY, rdval);
> + ret = cci_write(sensor->regmap, OV5647_SW_STANDBY, rdval, NULL);
> if (ret < 0)
> dev_dbg(dev, "software standby failed\n");
>
> @@ -845,10 +756,11 @@ static int ov5647_power_off(struct device *dev)
> static int ov5647_sensor_get_register(struct v4l2_subdev *sd,
> struct v4l2_dbg_register *reg)
> {
> + struct ov5647 *sensor = to_sensor(sd);
> int ret;
> - u8 val;
> + u64 val;
>
> - ret = ov5647_read(sd, reg->reg & 0xff, &val);
> + ret = cci_read(sensor->regmap, reg->reg & 0xff, &val, NULL);
> if (ret < 0)
> return ret;
>
> @@ -861,7 +773,9 @@ static int ov5647_sensor_get_register(struct v4l2_subdev *sd,
> static int ov5647_sensor_set_register(struct v4l2_subdev *sd,
> const struct v4l2_dbg_register *reg)
> {
> - return ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
> + struct ov5647 *sensor = to_sensor(sd);
> +
> + return cci_write(sensor->regmap, reg->reg & 0xff, reg->val & 0xff, NULL);
> }
> #endif
>
> @@ -1089,33 +1003,27 @@ static const struct v4l2_subdev_ops ov5647_subdev_ops = {
>
> static int ov5647_detect(struct v4l2_subdev *sd)
> {
> + struct ov5647 *sensor = to_sensor(sd);
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> - u8 read;
> + u64 read;
> int ret;
>
> - ret = ov5647_write(sd, OV5647_SW_RESET, 0x01);
> + ret = cci_write(sensor->regmap, OV5647_SW_RESET, 0x01, NULL);
> if (ret < 0)
> return ret;
>
> - ret = ov5647_read(sd, OV5647_REG_CHIPID_H, &read);
> - if (ret < 0)
> - return ret;
> -
> - if (read != 0x56) {
> - dev_err(&client->dev, "ID High expected 0x56 got %x", read);
> - return -ENODEV;
> - }
> -
> - ret = ov5647_read(sd, OV5647_REG_CHIPID_L, &read);
> + ret = cci_read(sensor->regmap, OV5647_REG_CHIPID, &read, NULL);
> if (ret < 0)
> - return ret;
> + return dev_err_probe(&client->dev, ret,
> + "failed to read chip id %x\n",
> + OV5647_REG_CHIPID);
>
> - if (read != 0x47) {
> - dev_err(&client->dev, "ID Low expected 0x47 got %x", read);
> + if (read != 0x5647) {
We should define a macro for the chip ID and use it here.
> + dev_err(&client->dev, "Chip ID expected 0x5647 got 0x%llx", read);
> return -ENODEV;
> }
>
> - return ov5647_write(sd, OV5647_SW_RESET, 0x00);
> + return cci_write(sensor->regmap, OV5647_SW_RESET, 0x00, NULL);
> }
>
> static int ov5647_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> @@ -1140,70 +1048,62 @@ static const struct v4l2_subdev_internal_ops ov5647_subdev_internal_ops = {
>
> static int ov5647_s_auto_white_balance(struct v4l2_subdev *sd, u32 val)
> {
> - return ov5647_write(sd, OV5647_REG_AWB, val ? 1 : 0);
> + struct ov5647 *sensor = to_sensor(sd);
> +
> + return cci_write(sensor->regmap, OV5647_REG_AWB, val ? 1 : 0, NULL);
> }
>
> static int ov5647_s_autogain(struct v4l2_subdev *sd, u32 val)
> {
> + struct ov5647 *sensor = to_sensor(sd);
> int ret;
> - u8 reg;
> + u64 reg;
>
> /* Non-zero turns on AGC by clearing bit 1.*/
> - ret = ov5647_read(sd, OV5647_REG_AEC_AGC, ®);
> + ret = cci_read(sensor->regmap, OV5647_REG_AEC_AGC, ®, NULL);
> if (ret)
> return ret;
>
> - return ov5647_write(sd, OV5647_REG_AEC_AGC, val ? reg & ~BIT(1)
> - : reg | BIT(1));
> + return cci_write(sensor->regmap, OV5647_REG_AEC_AGC, val ? reg & ~BIT(1)
> + : reg | BIT(1), NULL);
> }
>
> static int ov5647_s_exposure_auto(struct v4l2_subdev *sd, u32 val)
> {
> + struct ov5647 *sensor = to_sensor(sd);
> int ret;
> - u8 reg;
> + u64 reg;
>
> /*
> * Everything except V4L2_EXPOSURE_MANUAL turns on AEC by
> * clearing bit 0.
> */
> - ret = ov5647_read(sd, OV5647_REG_AEC_AGC, ®);
> + ret = cci_read(sensor->regmap, OV5647_REG_AEC_AGC, ®, NULL);
> if (ret)
> return ret;
>
> - return ov5647_write(sd, OV5647_REG_AEC_AGC,
> + return cci_write(sensor->regmap, OV5647_REG_AEC_AGC,
> val == V4L2_EXPOSURE_MANUAL ? reg | BIT(0)
> - : reg & ~BIT(0));
> + : reg & ~BIT(0), NULL);
> }
>
> static int ov5647_s_analogue_gain(struct v4l2_subdev *sd, u32 val)
> {
> - int ret;
> + struct ov5647 *sensor = to_sensor(sd);
>
> /* 10 bits of gain, 2 in the high register. */
> - ret = ov5647_write(sd, OV5647_REG_GAIN_HI, (val >> 8) & 3);
> - if (ret)
> - return ret;
> -
> - return ov5647_write(sd, OV5647_REG_GAIN_LO, val & 0xff);
> + return cci_write(sensor->regmap, OV5647_REG_GAIN, val & 0x3ff, NULL);
> }
>
> static int ov5647_s_exposure(struct v4l2_subdev *sd, u32 val)
> {
> - int ret;
> + struct ov5647 *sensor = to_sensor(sd);
>
> /*
> * Sensor has 20 bits, but the bottom 4 bits are fractions of a line
> * which we leave as zero (and don't receive in "val").
> */
> - ret = ov5647_write(sd, OV5647_REG_EXP_HI, (val >> 12) & 0xf);
> - if (ret)
> - return ret;
> -
> - ret = ov5647_write(sd, OV5647_REG_EXP_MID, (val >> 4) & 0xff);
> - if (ret)
> - return ret;
> -
> - return ov5647_write(sd, OV5647_REG_EXP_LO, (val & 0xf) << 4);
> + return cci_write(sensor->regmap, OV5647_REG_EXPOSURE, val << 4, NULL);
> }
We can now drop all contorls functions (except ov5647_s_exposure_auto / ov5647_s_autogain).
Since there is only a single register write, we can write the register directly
in the set control(like vblank).
>
> static int ov5647_s_ctrl(struct v4l2_ctrl *ctrl)
> @@ -1254,12 +1154,12 @@ static int ov5647_s_ctrl(struct v4l2_ctrl *ctrl)
> ret = ov5647_s_exposure(sd, ctrl->val);
> break;
> case V4L2_CID_VBLANK:
> - ret = ov5647_write16(sd, OV5647_REG_VTS_HI,
> - sensor->mode->format.height + ctrl->val);
> + ret = cci_write(sensor->regmap, OV5647_REG_VTS,
> + sensor->mode->format.height + ctrl->val, NULL);
> break;
> case V4L2_CID_TEST_PATTERN:
> - ret = ov5647_write(sd, OV5647_REG_ISPCTRL3D,
> - ov5647_test_pattern_val[ctrl->val]);
> + ret = cci_write(sensor->regmap, OV5647_REG_ISPCTRL3D,
> + ov5647_test_pattern_val[ctrl->val], NULL);
> break;
>
> /* Read-only, but we adjust it based on mode. */
> @@ -1435,6 +1335,13 @@ static int ov5647_probe(struct i2c_client *client)
> if (ret < 0)
> goto ctrl_handler_free;
>
> + sensor->regmap = devm_cci_regmap_init_i2c(client, 16);
> + if (IS_ERR(sensor->regmap)) {
> + ret = dev_err_probe(dev, PTR_ERR(sensor->regmap),
> + "Failed to init CCI\n");
> + goto entity_cleanup;
> + }
> +
> ret = ov5647_power_on(dev);
> if (ret)
> goto entity_cleanup;
> --
> 2.43.0
with above changes
Reviewed-by: Tarang Raval <tarang.raval@...iconsignals.io>
Best Regards,
Tarang
Powered by blists - more mailing lists