| 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: <3c353448-cac7-4519-bc91-baff2ab72206@windriver.com>
Date: Wed, 31 Dec 2025 10:57:52 +0800
From: xiaolei wang <xiaolei.wang@...driver.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Tarang Raval <tarang.raval@...iconsignals.io>
Cc: "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>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] media: i2c: ov5647: Convert to CCI register access
helpers
On 12/29/25 21:01, Laurent Pinchart wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Mon, Dec 29, 2025 at 12:37:41PM +0000, Tarang Raval wrote:
>>> 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 | 997 +++++++++++++++++--------------------
>>> 2 files changed, 453 insertions(+), 545 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..fd69f1616794 100644
>>> --- a/drivers/media/i2c/ov5647.c
>>> +++ b/drivers/media/i2c/ov5647.c
>>> @@ -22,6 +22,7 @@
>>> #include <linux/pm_runtime.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 +42,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(0x350b)
>> It should be 0x350a, not 0x350b.
>>
>>> +#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 +77,19 @@
>>> #define OV5647_EXPOSURE_DEFAULT 1000
>>> #define OV5647_EXPOSURE_MAX 65535
>> ...
>>
>>> @@ -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 = PTR_ERR(sensor->regmap);
>>> + dev_err(dev, "failed to initialize CCI: %d\n", ret);
>> Use return dev_err_probe();
> dev_err_probe() is fine, but goto entity_cleanup is needed.
Hi Tarang and Laurent,
Thank you both for the review feedback.
I'll address both issues in v3:
1. Fix OV5647_REG_GAIN to use CCI_REG16(0x350a) instead of 0x350b
2. Update the error handling to use dev_err_probe() while maintaining
the goto entity_cleanup path for proper resource cleanup
Will send the updated patch shortly.
Best regards,
Xiaolei
>
>>> + goto entity_cleanup;
>>> + }
>>> +
>>> ret = ov5647_power_on(dev);
>>> if (ret)
>>> goto entity_cleanup;
> --
> Regards,
>
> Laurent Pinchart
Powered by blists - more mailing lists