* [PATCH v4 0/3] media: i2c: ov9282: Modernize driver with CCI and streams API
@ 2026-03-05 4:33 Xiaolei Wang
2026-03-05 4:33 ` [PATCH v4 1/3] media: i2c: ov9282: Convert to CCI register access helpers Xiaolei Wang
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Xiaolei Wang @ 2026-03-05 4:33 UTC (permalink / raw)
To: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab,
prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede,
hverkuil-cisco, jai.luthra, dave.stevenson, Xiaolei.Wang
Cc: linux-media, linux-kernel
This series modernizes the ov9282 driver by:
1. Converting to the common CCI (Camera Control Interface) register
access helpers, which simplifies the code by removing custom I2C
register access functions.
2. Switching to use the V4L2 sub-device state lock instead of a
private mutex, improving integration with the V4L2 framework.
3. Migrating from the legacy s_stream callback to the modern
enable_streams/disable_streams callbacks, providing better support
for multiplexed streams.
These changes reduce code complexity, improve maintainability, and
align the driver with current V4L2 best practices.
I verified each patch on the Raspberry Pi 5 platform.
Changes in V4:
- patch 1: use cci_multi_reg_write() instead of regmap_multi_reg_write()
- Link to v3: https://patchwork.linuxtv.org/project/linux-media/cover/20260303104942.3111366-1-xiaolei.wang@windriver.com/
Changes in V3:
- Patch 2:
* Remove unnecessary ret assignment in v4l2_subdev_init_finalize() error path
* Move pm_runtime_idle() call after v4l2_async_register_subdev_sensor() to avoid
potential double power-off in error handling path
- Link to v2: https://patchwork.linuxtv.org/project/linux-media/cover/20260301104809.3505257-1-xiaolei.wang@windriver.com/
Changes in V2:
- Patch 1: Fixed group hold release error handling in
ov9282_update_exp_gain()
- Patch 2: Fixed runtime PM cleanup in probe error path
- Patch 3: Improved error handling with clearer err_pm_put: label
- Link to V1: https://patchwork.linuxtv.org/project/linux-media/cover/20260228083401.1007434-1-xiaolei.wang@windriver.com/
Xiaolei Wang (3):
media: i2c: ov9282: Convert to CCI register access helpers
media: i2c: ov9282: Switch to using the sub-device state lock
media: i2c: ov9282: switch to {enable,disable}_streams
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/ov9282.c | 677 +++++++++++++------------------------
2 files changed, 236 insertions(+), 442 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/3] media: i2c: ov9282: Convert to CCI register access helpers
2026-03-05 4:33 [PATCH v4 0/3] media: i2c: ov9282: Modernize driver with CCI and streams API Xiaolei Wang
@ 2026-03-05 4:33 ` Xiaolei Wang
2026-03-05 10:57 ` Dave Stevenson
2026-03-05 4:33 ` [PATCH v4 2/3] media: i2c: ov9282: Switch to using the sub-device state lock Xiaolei Wang
2026-03-05 4:33 ` [PATCH v4 3/3] media: i2c: ov9282: switch to {enable,disable}_streams Xiaolei Wang
2 siblings, 1 reply; 7+ messages in thread
From: Xiaolei Wang @ 2026-03-05 4:33 UTC (permalink / raw)
To: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab,
prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede,
hverkuil-cisco, jai.luthra, dave.stevenson, Xiaolei.Wang
Cc: linux-media, linux-kernel
Use the new common CCI register access helpers to replace the private
register access helpers in the ov9282 driver. This simplifies the driver
by reducing the amount of code.
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
---
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/ov9282.c | 561 +++++++++++++------------------------
2 files changed, 198 insertions(+), 364 deletions(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 5eb1e0e0a87a..3027e71fd8fb 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -690,6 +690,7 @@ config VIDEO_OV8865
config VIDEO_OV9282
tristate "OmniVision OV9282 sensor support"
depends on OF_GPIO
+ select V4L2_CCI_I2C
help
This is a Video4Linux2 sensor driver for the OmniVision
OV9282 camera sensor.
diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index ded9b2044ff8..56f854a4d04f 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -12,38 +12,40 @@
#include <linux/math.h>
#include <linux/module.h>
#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
+#include <media/v4l2-cci.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-event.h>
#include <media/v4l2-fwnode.h>
#include <media/v4l2-subdev.h>
/* Streaming Mode */
-#define OV9282_REG_MODE_SELECT 0x0100
+#define OV9282_REG_MODE_SELECT CCI_REG8(0x0100)
#define OV9282_MODE_STANDBY 0x00
#define OV9282_MODE_STREAMING 0x01
-#define OV9282_REG_PLL_CTRL_0D 0x030d
+#define OV9282_REG_PLL_CTRL_0D CCI_REG8(0x030d)
#define OV9282_PLL_CTRL_0D_RAW8 0x60
#define OV9282_PLL_CTRL_0D_RAW10 0x50
-#define OV9282_REG_TIMING_HTS 0x380c
+#define OV9282_REG_TIMING_HTS CCI_REG16(0x380c)
#define OV9282_TIMING_HTS_MAX 0x7fff
/* Lines per frame */
-#define OV9282_REG_LPFR 0x380e
+#define OV9282_REG_LPFR CCI_REG16(0x380e)
/* Chip ID */
-#define OV9282_REG_ID 0x300a
+#define OV9282_REG_ID CCI_REG16(0x300a)
#define OV9282_ID 0x9281
/* Output enable registers */
-#define OV9282_REG_OUTPUT_ENABLE4 0x3004
+#define OV9282_REG_OUTPUT_ENABLE4 CCI_REG8(0x3004)
#define OV9282_OUTPUT_ENABLE4_GPIO2 BIT(1)
#define OV9282_OUTPUT_ENABLE4_D9 BIT(0)
-#define OV9282_REG_OUTPUT_ENABLE5 0x3005
+#define OV9282_REG_OUTPUT_ENABLE5 CCI_REG8(0x3005)
#define OV9282_OUTPUT_ENABLE5_D8 BIT(7)
#define OV9282_OUTPUT_ENABLE5_D7 BIT(6)
#define OV9282_OUTPUT_ENABLE5_D6 BIT(5)
@@ -53,7 +55,7 @@
#define OV9282_OUTPUT_ENABLE5_D2 BIT(1)
#define OV9282_OUTPUT_ENABLE5_D1 BIT(0)
-#define OV9282_REG_OUTPUT_ENABLE6 0x3006
+#define OV9282_REG_OUTPUT_ENABLE6 CCI_REG8(0x3006)
#define OV9282_OUTPUT_ENABLE6_D0 BIT(7)
#define OV9282_OUTPUT_ENABLE6_PCLK BIT(6)
#define OV9282_OUTPUT_ENABLE6_HREF BIT(5)
@@ -62,14 +64,14 @@
#define OV9282_OUTPUT_ENABLE6_VSYNC BIT(1)
/* Exposure control */
-#define OV9282_REG_EXPOSURE 0x3500
+#define OV9282_REG_EXPOSURE CCI_REG24(0x3500)
#define OV9282_EXPOSURE_MIN 1
#define OV9282_EXPOSURE_OFFSET 25
#define OV9282_EXPOSURE_STEP 1
#define OV9282_EXPOSURE_DEFAULT 0x0282
/* AEC/AGC manual */
-#define OV9282_REG_AEC_MANUAL 0x3503
+#define OV9282_REG_AEC_MANUAL CCI_REG8(0x3503)
#define OV9282_DIGFRAC_GAIN_DELAY BIT(6)
#define OV9282_GAIN_CHANGE_DELAY BIT(5)
#define OV9282_GAIN_DELAY BIT(4)
@@ -78,28 +80,28 @@
#define OV9282_AEC_MANUAL_DEFAULT 0x00
/* Analog gain control */
-#define OV9282_REG_AGAIN 0x3509
+#define OV9282_REG_AGAIN CCI_REG8(0x3509)
#define OV9282_AGAIN_MIN 0x10
#define OV9282_AGAIN_MAX 0xff
#define OV9282_AGAIN_STEP 1
#define OV9282_AGAIN_DEFAULT 0x10
/* Group hold register */
-#define OV9282_REG_HOLD 0x3308
+#define OV9282_REG_HOLD CCI_REG8(0x3308)
-#define OV9282_REG_ANA_CORE_2 0x3662
+#define OV9282_REG_ANA_CORE_2 CCI_REG8(0x3662)
#define OV9282_ANA_CORE2_RAW8 0x07
#define OV9282_ANA_CORE2_RAW10 0x05
-#define OV9282_REG_TIMING_FORMAT_1 0x3820
-#define OV9282_REG_TIMING_FORMAT_2 0x3821
+#define OV9282_REG_TIMING_FORMAT_1 CCI_REG8(0x3820)
+#define OV9282_REG_TIMING_FORMAT_2 CCI_REG8(0x3821)
#define OV9282_FLIP_BIT BIT(2)
-#define OV9282_REG_MIPI_CTRL00 0x4800
+#define OV9282_REG_MIPI_CTRL00 CCI_REG8(0x4800)
#define OV9282_GATED_CLOCK BIT(5)
/* Flash/Strobe control registers */
-#define OV9282_REG_STROBE_FRAME_SPAN 0x3925
+#define OV9282_REG_STROBE_FRAME_SPAN CCI_REG32(0x3925)
#define OV9282_STROBE_FRAME_SPAN_DEFAULT 0x0000001a
/* Input clock rate */
@@ -139,16 +141,6 @@ static const char * const ov9282_supply_names[] = {
#define OV9282_NUM_SUPPLIES ARRAY_SIZE(ov9282_supply_names)
-/**
- * struct ov9282_reg - ov9282 sensor register
- * @address: Register address
- * @val: Register value
- */
-struct ov9282_reg {
- u16 address;
- u8 val;
-};
-
/**
* struct ov9282_reg_list - ov9282 sensor register list
* @num_of_regs: Number of registers in the list
@@ -156,7 +148,7 @@ struct ov9282_reg {
*/
struct ov9282_reg_list {
u32 num_of_regs;
- const struct ov9282_reg *regs;
+ const struct cci_reg_sequence *regs;
};
/**
@@ -188,6 +180,7 @@ struct ov9282_mode {
* struct ov9282 - ov9282 sensor device structure
* @dev: Pointer to generic device
* @sd: V4L2 sub-device
+ * @regmap: Regmap for sensor register access
* @pad: Media pad. Only one pad supported
* @reset_gpio: Sensor reset gpio
* @inclk: Sensor input clock
@@ -209,6 +202,7 @@ struct ov9282_mode {
struct ov9282 {
struct device *dev;
struct v4l2_subdev sd;
+ struct regmap *regmap;
struct media_pad pad;
struct gpio_desc *reset_gpio;
struct clk *inclk;
@@ -241,73 +235,68 @@ static const s64 link_freq[] = {
* register arrays as some settings are written as part of ov9282_power_on,
* and the reset will clear them.
*/
-static const struct ov9282_reg common_regs[] = {
- {0x0302, 0x32},
- {0x030e, 0x02},
- {0x3001, 0x00},
+static const struct cci_reg_sequence common_regs[] = {
+ {CCI_REG8(0x0302), 0x32},
+ {CCI_REG8(0x030e), 0x02},
+ {CCI_REG8(0x3001), 0x00},
{OV9282_REG_OUTPUT_ENABLE4, 0x00},
{OV9282_REG_OUTPUT_ENABLE5, 0x00},
{OV9282_REG_OUTPUT_ENABLE6, OV9282_OUTPUT_ENABLE6_ILPWM},
- {0x3011, 0x0a},
- {0x3013, 0x18},
- {0x301c, 0xf0},
- {0x3022, 0x01},
- {0x3030, 0x10},
- {0x3039, 0x32},
- {0x303a, 0x00},
+ {CCI_REG8(0x3011), 0x0a},
+ {CCI_REG8(0x3013), 0x18},
+ {CCI_REG8(0x301c), 0xf0},
+ {CCI_REG8(0x3022), 0x01},
+ {CCI_REG8(0x3030), 0x10},
+ {CCI_REG8(0x3039), 0x32},
+ {CCI_REG8(0x303a), 0x00},
{OV9282_REG_AEC_MANUAL, OV9282_GAIN_PREC16_EN},
- {0x3505, 0x8c},
- {0x3507, 0x03},
- {0x3508, 0x00},
- {0x3610, 0x80},
- {0x3611, 0xa0},
- {0x3620, 0x6e},
- {0x3632, 0x56},
- {0x3633, 0x78},
- {0x3666, 0x00},
- {0x366f, 0x5a},
- {0x3680, 0x84},
- {0x3712, 0x80},
- {0x372d, 0x22},
- {0x3731, 0x80},
- {0x3732, 0x30},
- {0x377d, 0x22},
- {0x3788, 0x02},
- {0x3789, 0xa4},
- {0x378a, 0x00},
- {0x378b, 0x4a},
- {0x3799, 0x20},
- {0x3881, 0x42},
- {0x38a8, 0x02},
- {0x38a9, 0x80},
- {0x38b1, 0x00},
- {0x38c4, 0x00},
- {0x38c5, 0xc0},
- {0x38c6, 0x04},
- {0x38c7, 0x80},
- {0x3920, 0xff},
- {0x4010, 0x40},
- {0x4043, 0x40},
- {0x4307, 0x30},
- {0x4317, 0x00},
- {0x4501, 0x00},
- {0x450a, 0x08},
- {0x4601, 0x04},
- {0x470f, 0x00},
- {0x4f07, 0x00},
- {0x5000, 0x9f},
- {0x5001, 0x00},
- {0x5e00, 0x00},
- {0x5d00, 0x07},
- {0x5d01, 0x00},
- {0x0101, 0x01},
- {0x1000, 0x03},
- {0x5a08, 0x84},
-};
-
-static struct ov9282_reg_list common_regs_list = {
- .num_of_regs = ARRAY_SIZE(common_regs),
- .regs = common_regs,
+ {CCI_REG8(0x3505), 0x8c},
+ {CCI_REG8(0x3507), 0x03},
+ {CCI_REG8(0x3508), 0x00},
+ {CCI_REG8(0x3610), 0x80},
+ {CCI_REG8(0x3611), 0xa0},
+ {CCI_REG8(0x3620), 0x6e},
+ {CCI_REG8(0x3632), 0x56},
+ {CCI_REG8(0x3633), 0x78},
+ {CCI_REG8(0x3666), 0x00},
+ {CCI_REG8(0x366f), 0x5a},
+ {CCI_REG8(0x3680), 0x84},
+ {CCI_REG8(0x3712), 0x80},
+ {CCI_REG8(0x372d), 0x22},
+ {CCI_REG8(0x3731), 0x80},
+ {CCI_REG8(0x3732), 0x30},
+ {CCI_REG8(0x377d), 0x22},
+ {CCI_REG8(0x3788), 0x02},
+ {CCI_REG8(0x3789), 0xa4},
+ {CCI_REG8(0x378a), 0x00},
+ {CCI_REG8(0x378b), 0x4a},
+ {CCI_REG8(0x3799), 0x20},
+ {CCI_REG8(0x3881), 0x42},
+ {CCI_REG8(0x38a8), 0x02},
+ {CCI_REG8(0x38a9), 0x80},
+ {CCI_REG8(0x38b1), 0x00},
+ {CCI_REG8(0x38c4), 0x00},
+ {CCI_REG8(0x38c5), 0xc0},
+ {CCI_REG8(0x38c6), 0x04},
+ {CCI_REG8(0x38c7), 0x80},
+ {CCI_REG8(0x3920), 0xff},
+ {CCI_REG8(0x4010), 0x40},
+ {CCI_REG8(0x4043), 0x40},
+ {CCI_REG8(0x4307), 0x30},
+ {CCI_REG8(0x4317), 0x00},
+ {CCI_REG8(0x4501), 0x00},
+ {CCI_REG8(0x450a), 0x08},
+ {CCI_REG8(0x4601), 0x04},
+ {CCI_REG8(0x470f), 0x00},
+ {CCI_REG8(0x4f07), 0x00},
+ {CCI_REG8(0x5000), 0x9f},
+ {CCI_REG8(0x5001), 0x00},
+ {CCI_REG8(0x5e00), 0x00},
+ {CCI_REG8(0x5d00), 0x07},
+ {CCI_REG8(0x5d01), 0x00},
+ {CCI_REG8(0x0101), 0x01},
+ {CCI_REG8(0x1000), 0x03},
+ {CCI_REG8(0x5a08), 0x84},
};
#define MODE_1280_800 0
@@ -317,96 +306,96 @@ static struct ov9282_reg_list common_regs_list = {
#define DEFAULT_MODE MODE_1280_720
/* Sensor mode registers */
-static const struct ov9282_reg mode_1280x800_regs[] = {
- {0x3778, 0x00},
- {0x3800, 0x00},
- {0x3801, 0x00},
- {0x3802, 0x00},
- {0x3803, 0x00},
- {0x3804, 0x05},
- {0x3805, 0x0f},
- {0x3806, 0x03},
- {0x3807, 0x2f},
- {0x3808, 0x05},
- {0x3809, 0x00},
- {0x380a, 0x03},
- {0x380b, 0x20},
- {0x3810, 0x00},
- {0x3811, 0x08},
- {0x3812, 0x00},
- {0x3813, 0x08},
- {0x3814, 0x11},
- {0x3815, 0x11},
+static const struct cci_reg_sequence mode_1280x800_regs[] = {
+ {CCI_REG8(0x3778), 0x00},
+ {CCI_REG8(0x3800), 0x00},
+ {CCI_REG8(0x3801), 0x00},
+ {CCI_REG8(0x3802), 0x00},
+ {CCI_REG8(0x3803), 0x00},
+ {CCI_REG8(0x3804), 0x05},
+ {CCI_REG8(0x3805), 0x0f},
+ {CCI_REG8(0x3806), 0x03},
+ {CCI_REG8(0x3807), 0x2f},
+ {CCI_REG8(0x3808), 0x05},
+ {CCI_REG8(0x3809), 0x00},
+ {CCI_REG8(0x380a), 0x03},
+ {CCI_REG8(0x380b), 0x20},
+ {CCI_REG8(0x3810), 0x00},
+ {CCI_REG8(0x3811), 0x08},
+ {CCI_REG8(0x3812), 0x00},
+ {CCI_REG8(0x3813), 0x08},
+ {CCI_REG8(0x3814), 0x11},
+ {CCI_REG8(0x3815), 0x11},
{OV9282_REG_TIMING_FORMAT_1, 0x40},
{OV9282_REG_TIMING_FORMAT_2, 0x00},
- {0x4003, 0x40},
- {0x4008, 0x04},
- {0x4009, 0x0b},
- {0x400c, 0x00},
- {0x400d, 0x07},
- {0x4507, 0x00},
- {0x4509, 0x00},
+ {CCI_REG8(0x4003), 0x40},
+ {CCI_REG8(0x4008), 0x04},
+ {CCI_REG8(0x4009), 0x0b},
+ {CCI_REG8(0x400c), 0x00},
+ {CCI_REG8(0x400d), 0x07},
+ {CCI_REG8(0x4507), 0x00},
+ {CCI_REG8(0x4509), 0x00},
};
-static const struct ov9282_reg mode_1280x720_regs[] = {
- {0x3778, 0x00},
- {0x3800, 0x00},
- {0x3801, 0x00},
- {0x3802, 0x00},
- {0x3803, 0x00},
- {0x3804, 0x05},
- {0x3805, 0x0f},
- {0x3806, 0x02},
- {0x3807, 0xdf},
- {0x3808, 0x05},
- {0x3809, 0x00},
- {0x380a, 0x02},
- {0x380b, 0xd0},
- {0x3810, 0x00},
- {0x3811, 0x08},
- {0x3812, 0x00},
- {0x3813, 0x08},
- {0x3814, 0x11},
- {0x3815, 0x11},
+static const struct cci_reg_sequence mode_1280x720_regs[] = {
+ {CCI_REG8(0x3778), 0x00},
+ {CCI_REG8(0x3800), 0x00},
+ {CCI_REG8(0x3801), 0x00},
+ {CCI_REG8(0x3802), 0x00},
+ {CCI_REG8(0x3803), 0x00},
+ {CCI_REG8(0x3804), 0x05},
+ {CCI_REG8(0x3805), 0x0f},
+ {CCI_REG8(0x3806), 0x02},
+ {CCI_REG8(0x3807), 0xdf},
+ {CCI_REG8(0x3808), 0x05},
+ {CCI_REG8(0x3809), 0x00},
+ {CCI_REG8(0x380a), 0x02},
+ {CCI_REG8(0x380b), 0xd0},
+ {CCI_REG8(0x3810), 0x00},
+ {CCI_REG8(0x3811), 0x08},
+ {CCI_REG8(0x3812), 0x00},
+ {CCI_REG8(0x3813), 0x08},
+ {CCI_REG8(0x3814), 0x11},
+ {CCI_REG8(0x3815), 0x11},
{OV9282_REG_TIMING_FORMAT_1, 0x3c},
{OV9282_REG_TIMING_FORMAT_2, 0x84},
- {0x4003, 0x40},
- {0x4008, 0x02},
- {0x4009, 0x05},
- {0x400c, 0x00},
- {0x400d, 0x03},
- {0x4507, 0x00},
- {0x4509, 0x80},
+ {CCI_REG8(0x4003), 0x40},
+ {CCI_REG8(0x4008), 0x02},
+ {CCI_REG8(0x4009), 0x05},
+ {CCI_REG8(0x400c), 0x00},
+ {CCI_REG8(0x400d), 0x03},
+ {CCI_REG8(0x4507), 0x00},
+ {CCI_REG8(0x4509), 0x80},
};
-static const struct ov9282_reg mode_640x400_regs[] = {
- {0x3778, 0x10},
- {0x3800, 0x00},
- {0x3801, 0x00},
- {0x3802, 0x00},
- {0x3803, 0x00},
- {0x3804, 0x05},
- {0x3805, 0x0f},
- {0x3806, 0x03},
- {0x3807, 0x2f},
- {0x3808, 0x02},
- {0x3809, 0x80},
- {0x380a, 0x01},
- {0x380b, 0x90},
- {0x3810, 0x00},
- {0x3811, 0x04},
- {0x3812, 0x00},
- {0x3813, 0x04},
- {0x3814, 0x31},
- {0x3815, 0x22},
+static const struct cci_reg_sequence mode_640x400_regs[] = {
+ {CCI_REG8(0x3778), 0x10},
+ {CCI_REG8(0x3800), 0x00},
+ {CCI_REG8(0x3801), 0x00},
+ {CCI_REG8(0x3802), 0x00},
+ {CCI_REG8(0x3803), 0x00},
+ {CCI_REG8(0x3804), 0x05},
+ {CCI_REG8(0x3805), 0x0f},
+ {CCI_REG8(0x3806), 0x03},
+ {CCI_REG8(0x3807), 0x2f},
+ {CCI_REG8(0x3808), 0x02},
+ {CCI_REG8(0x3809), 0x80},
+ {CCI_REG8(0x380a), 0x01},
+ {CCI_REG8(0x380b), 0x90},
+ {CCI_REG8(0x3810), 0x00},
+ {CCI_REG8(0x3811), 0x04},
+ {CCI_REG8(0x3812), 0x00},
+ {CCI_REG8(0x3813), 0x04},
+ {CCI_REG8(0x3814), 0x31},
+ {CCI_REG8(0x3815), 0x22},
{OV9282_REG_TIMING_FORMAT_1, 0x60},
{OV9282_REG_TIMING_FORMAT_2, 0x01},
- {0x4008, 0x02},
- {0x4009, 0x05},
- {0x400c, 0x00},
- {0x400d, 0x03},
- {0x4507, 0x03},
- {0x4509, 0x80},
+ {CCI_REG8(0x4008), 0x02},
+ {CCI_REG8(0x4009), 0x05},
+ {CCI_REG8(0x400c), 0x00},
+ {CCI_REG8(0x400d), 0x03},
+ {CCI_REG8(0x4507), 0x03},
+ {CCI_REG8(0x4509), 0x80},
};
/* Supported sensor mode configurations */
@@ -485,97 +474,6 @@ static inline struct ov9282 *to_ov9282(struct v4l2_subdev *subdev)
return container_of(subdev, struct ov9282, sd);
}
-/**
- * ov9282_read_reg() - Read registers.
- * @ov9282: pointer to ov9282 device
- * @reg: register address
- * @len: length of bytes to read. Max supported bytes is 4
- * @val: pointer to register value to be filled.
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int ov9282_read_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 *val)
-{
- struct i2c_client *client = v4l2_get_subdevdata(&ov9282->sd);
- struct i2c_msg msgs[2] = {0};
- u8 addr_buf[2] = {0};
- u8 data_buf[4] = {0};
- int ret;
-
- if (WARN_ON(len > 4))
- return -EINVAL;
-
- put_unaligned_be16(reg, addr_buf);
-
- /* Write register address */
- msgs[0].addr = client->addr;
- msgs[0].flags = 0;
- msgs[0].len = ARRAY_SIZE(addr_buf);
- msgs[0].buf = addr_buf;
-
- /* Read data from register */
- msgs[1].addr = client->addr;
- msgs[1].flags = I2C_M_RD;
- msgs[1].len = len;
- msgs[1].buf = &data_buf[4 - len];
-
- ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
- if (ret != ARRAY_SIZE(msgs))
- return -EIO;
-
- *val = get_unaligned_be32(data_buf);
-
- return 0;
-}
-
-/**
- * ov9282_write_reg() - Write register
- * @ov9282: pointer to ov9282 device
- * @reg: register address
- * @len: length of bytes. Max supported bytes is 4
- * @val: register value
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int ov9282_write_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 val)
-{
- struct i2c_client *client = v4l2_get_subdevdata(&ov9282->sd);
- u8 buf[6] = {0};
-
- if (WARN_ON(len > 4))
- return -EINVAL;
-
- put_unaligned_be16(reg, buf);
- put_unaligned_be32(val << (8 * (4 - len)), buf + 2);
- if (i2c_master_send(client, buf, len + 2) != len + 2)
- return -EIO;
-
- return 0;
-}
-
-/**
- * ov9282_write_regs() - Write a list of registers
- * @ov9282: pointer to ov9282 device
- * @regs: list of registers to be written
- * @len: length of registers array
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int ov9282_write_regs(struct ov9282 *ov9282,
- const struct ov9282_reg *regs, u32 len)
-{
- unsigned int i;
- int ret;
-
- for (i = 0; i < len; i++) {
- ret = ov9282_write_reg(ov9282, regs[i].address, 1, regs[i].val);
- if (ret)
- return ret;
- }
-
- return 0;
-}
-
/**
* ov9282_update_controls() - Update control ranges based on streaming mode
* @ov9282: pointer to ov9282 device
@@ -639,15 +537,15 @@ static int ov9282_update_exp_gain(struct ov9282 *ov9282, u32 exposure, u32 gain)
dev_dbg(ov9282->dev, "Set exp %u (~%u us), analog gain %u",
exposure, exposure_us, gain);
- ret = ov9282_write_reg(ov9282, OV9282_REG_HOLD, 1, 1);
+ ret = cci_write(ov9282->regmap, OV9282_REG_HOLD, 0x01, NULL);
if (ret)
return ret;
- ret = ov9282_write_reg(ov9282, OV9282_REG_EXPOSURE, 3, exposure << 4);
+ ret = cci_write(ov9282->regmap, OV9282_REG_EXPOSURE, exposure << 4, NULL);
if (ret)
goto error_release_group_hold;
- ret = ov9282_write_reg(ov9282, OV9282_REG_AGAIN, 1, gain);
+ ret = cci_write(ov9282->regmap, OV9282_REG_AGAIN, gain, NULL);
if (ret)
goto error_release_group_hold;
@@ -656,60 +554,9 @@ static int ov9282_update_exp_gain(struct ov9282 *ov9282, u32 exposure, u32 gain)
OV9282_STROBE_FRAME_SPAN_DEFAULT);
error_release_group_hold:
- ov9282_write_reg(ov9282, OV9282_REG_HOLD, 1, 0);
-
- return ret;
-}
-
-static int ov9282_set_ctrl_hflip(struct ov9282 *ov9282, int value)
-{
- u32 current_val;
- int ret = ov9282_read_reg(ov9282, OV9282_REG_TIMING_FORMAT_2, 1,
- ¤t_val);
- if (ret)
- return ret;
+ int ret_hold = cci_write(ov9282->regmap, OV9282_REG_HOLD, 0, NULL);
- if (value)
- current_val |= OV9282_FLIP_BIT;
- else
- current_val &= ~OV9282_FLIP_BIT;
-
- return ov9282_write_reg(ov9282, OV9282_REG_TIMING_FORMAT_2, 1,
- current_val);
-}
-
-static int ov9282_set_ctrl_vflip(struct ov9282 *ov9282, int value)
-{
- u32 current_val;
- int ret = ov9282_read_reg(ov9282, OV9282_REG_TIMING_FORMAT_1, 1,
- ¤t_val);
- if (ret)
- return ret;
-
- if (value)
- current_val |= OV9282_FLIP_BIT;
- else
- current_val &= ~OV9282_FLIP_BIT;
-
- return ov9282_write_reg(ov9282, OV9282_REG_TIMING_FORMAT_1, 1,
- current_val);
-}
-
-static int ov9282_set_ctrl_flash_strobe_oe(struct ov9282 *ov9282, bool enable)
-{
- u32 current_val;
- int ret;
-
- ret = ov9282_read_reg(ov9282, OV9282_REG_OUTPUT_ENABLE6, 1, ¤t_val);
- if (ret)
- return ret;
-
- if (enable)
- current_val |= OV9282_OUTPUT_ENABLE6_STROBE;
- else
- current_val &= ~OV9282_OUTPUT_ENABLE6_STROBE;
-
- return ov9282_write_reg(ov9282, OV9282_REG_OUTPUT_ENABLE6, 1, current_val);
+ return ret ? ret : ret_hold;
}
static u32 ov9282_us_to_flash_duration(struct ov9282 *ov9282, u32 value)
@@ -740,30 +587,6 @@ static u32 ov9282_flash_duration_to_us(struct ov9282 *ov9282, u32 value)
return DIV_ROUND_UP(value * frame_width, OV9282_STROBE_SPAN_FACTOR);
}
-static int ov9282_set_ctrl_flash_duration(struct ov9282 *ov9282, u32 value)
-{
- u32 val = ov9282_us_to_flash_duration(ov9282, value);
- int ret;
-
- ret = ov9282_write_reg(ov9282, OV9282_REG_STROBE_FRAME_SPAN, 1,
- (val >> 24) & 0xff);
- if (ret)
- return ret;
-
- ret = ov9282_write_reg(ov9282, OV9282_REG_STROBE_FRAME_SPAN + 1, 1,
- (val >> 16) & 0xff);
- if (ret)
- return ret;
-
- ret = ov9282_write_reg(ov9282, OV9282_REG_STROBE_FRAME_SPAN + 2, 1,
- (val >> 8) & 0xff);
- if (ret)
- return ret;
-
- return ov9282_write_reg(ov9282, OV9282_REG_STROBE_FRAME_SPAN + 3, 1,
- val & 0xff);
-}
-
/**
* ov9282_set_ctrl() - Set subdevice control
* @ctrl: pointer to v4l2_ctrl structure
@@ -818,23 +641,27 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
break;
case V4L2_CID_VBLANK:
lpfr = ov9282->vblank + ov9282->cur_mode->height;
- ret = ov9282_write_reg(ov9282, OV9282_REG_LPFR, 2, lpfr);
+ ret = cci_write(ov9282->regmap, OV9282_REG_LPFR, lpfr, NULL);
break;
case V4L2_CID_HFLIP:
- ret = ov9282_set_ctrl_hflip(ov9282, ctrl->val);
+ ret = cci_update_bits(ov9282->regmap, OV9282_REG_TIMING_FORMAT_2,
+ OV9282_FLIP_BIT, ctrl->val ? OV9282_FLIP_BIT : 0, NULL);
break;
case V4L2_CID_VFLIP:
- ret = ov9282_set_ctrl_vflip(ov9282, ctrl->val);
+ ret = cci_update_bits(ov9282->regmap, OV9282_REG_TIMING_FORMAT_1,
+ OV9282_FLIP_BIT, ctrl->val ? OV9282_FLIP_BIT : 0, NULL);
break;
case V4L2_CID_HBLANK:
- ret = ov9282_write_reg(ov9282, OV9282_REG_TIMING_HTS, 2,
- (ctrl->val + ov9282->cur_mode->width) >> 1);
+ ret = cci_write(ov9282->regmap, OV9282_REG_TIMING_HTS,
+ (ctrl->val + ov9282->cur_mode->width) >> 1, NULL);
break;
case V4L2_CID_FLASH_STROBE_OE:
- ret = ov9282_set_ctrl_flash_strobe_oe(ov9282, ctrl->val);
+ ret = cci_update_bits(ov9282->regmap, OV9282_REG_OUTPUT_ENABLE6,
+ OV9282_OUTPUT_ENABLE6_STROBE,
+ ctrl->val ? OV9282_OUTPUT_ENABLE6_STROBE : 0, NULL);
break;
case V4L2_CID_FLASH_DURATION:
- ret = ov9282_set_ctrl_flash_duration(ov9282, ctrl->val);
+ ret = cci_write(ov9282->regmap, OV9282_REG_STROBE_FRAME_SPAN, ctrl->val, NULL);
break;
default:
dev_err(ov9282->dev, "Invalid control %d", ctrl->id);
@@ -1114,7 +941,7 @@ static int ov9282_get_selection(struct v4l2_subdev *sd,
*/
static int ov9282_start_streaming(struct ov9282 *ov9282)
{
- const struct ov9282_reg bitdepth_regs[2][2] = {
+ const struct cci_reg_sequence bitdepth_regs[2][2] = {
{
{OV9282_REG_PLL_CTRL_0D, OV9282_PLL_CTRL_0D_RAW10},
{OV9282_REG_ANA_CORE_2, OV9282_ANA_CORE2_RAW10},
@@ -1128,15 +955,16 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
int ret;
/* Write common registers */
- ret = ov9282_write_regs(ov9282, common_regs_list.regs,
- common_regs_list.num_of_regs);
+ ret = cci_multi_reg_write(ov9282->regmap, common_regs,
+ ARRAY_SIZE(common_regs), NULL);
if (ret) {
dev_err(ov9282->dev, "fail to write common registers");
return ret;
}
bitdepth_index = ov9282->code == MEDIA_BUS_FMT_Y10_1X10 ? 0 : 1;
- ret = ov9282_write_regs(ov9282, bitdepth_regs[bitdepth_index], 2);
+ ret = cci_multi_reg_write(ov9282->regmap,
+ bitdepth_regs[bitdepth_index], 2, NULL);
if (ret) {
dev_err(ov9282->dev, "fail to write bitdepth regs");
return ret;
@@ -1144,7 +972,8 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
/* Write sensor mode registers */
reg_list = &ov9282->cur_mode->reg_list;
- ret = ov9282_write_regs(ov9282, reg_list->regs, reg_list->num_of_regs);
+ ret = cci_multi_reg_write(ov9282->regmap, reg_list->regs,
+ reg_list->num_of_regs, NULL);
if (ret) {
dev_err(ov9282->dev, "fail to write initial registers");
return ret;
@@ -1158,8 +987,8 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
}
/* Start streaming */
- ret = ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT,
- 1, OV9282_MODE_STREAMING);
+ ret = cci_write(ov9282->regmap, OV9282_REG_MODE_SELECT,
+ OV9282_MODE_STREAMING, NULL);
if (ret) {
dev_err(ov9282->dev, "fail to start streaming");
return ret;
@@ -1176,8 +1005,8 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
*/
static int ov9282_stop_streaming(struct ov9282 *ov9282)
{
- return ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT,
- 1, OV9282_MODE_STANDBY);
+ return cci_write(ov9282->regmap, OV9282_REG_MODE_SELECT,
+ OV9282_MODE_STANDBY, NULL);
}
/**
@@ -1228,14 +1057,14 @@ static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
static int ov9282_detect(struct ov9282 *ov9282)
{
int ret;
- u32 val;
+ u64 val;
- ret = ov9282_read_reg(ov9282, OV9282_REG_ID, 2, &val);
+ ret = cci_read(ov9282->regmap, OV9282_REG_ID, &val, NULL);
if (ret)
return ret;
if (val != OV9282_ID) {
- dev_err(ov9282->dev, "chip id mismatch: %x!=%x",
+ dev_err(ov9282->dev, "chip id mismatch: %x!=%llx",
OV9282_ID, val);
return -ENXIO;
}
@@ -1397,9 +1226,8 @@ static int ov9282_power_on(struct device *dev)
usleep_range(400, 600);
- ret = ov9282_write_reg(ov9282, OV9282_REG_MIPI_CTRL00, 1,
- ov9282->noncontinuous_clock ?
- OV9282_GATED_CLOCK : 0);
+ ret = cci_write(ov9282->regmap, OV9282_REG_MIPI_CTRL00,
+ ov9282->noncontinuous_clock ? OV9282_GATED_CLOCK : 0, NULL);
if (ret) {
dev_err(ov9282->dev, "fail to write MIPI_CTRL00");
goto error_clk;
@@ -1576,6 +1404,11 @@ static int ov9282_probe(struct i2c_client *client)
return ret;
}
+ ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
+ if (IS_ERR(ov9282->regmap))
+ return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
+ "Failed to init CCI\n");
+
mutex_init(&ov9282->mutex);
ret = ov9282_power_on(ov9282->dev);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 2/3] media: i2c: ov9282: Switch to using the sub-device state lock
2026-03-05 4:33 [PATCH v4 0/3] media: i2c: ov9282: Modernize driver with CCI and streams API Xiaolei Wang
2026-03-05 4:33 ` [PATCH v4 1/3] media: i2c: ov9282: Convert to CCI register access helpers Xiaolei Wang
@ 2026-03-05 4:33 ` Xiaolei Wang
2026-03-05 14:44 ` Dave Stevenson
2026-03-05 4:33 ` [PATCH v4 3/3] media: i2c: ov9282: switch to {enable,disable}_streams Xiaolei Wang
2 siblings, 1 reply; 7+ messages in thread
From: Xiaolei Wang @ 2026-03-05 4:33 UTC (permalink / raw)
To: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab,
prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede,
hverkuil-cisco, jai.luthra, dave.stevenson, Xiaolei.Wang
Cc: linux-media, linux-kernel
Switch to using the sub-device state lock and properly call
v4l2_subdev_init_finalize() / v4l2_subdev_cleanup() on probe() /
remove().
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
---
drivers/media/i2c/ov9282.c | 51 +++++++++++++++-----------------------
1 file changed, 20 insertions(+), 31 deletions(-)
diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 56f854a4d04f..98e0a0732ef7 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -221,7 +221,6 @@ struct ov9282 {
bool noncontinuous_clock;
const struct ov9282_mode *cur_mode;
u32 code;
- struct mutex mutex;
};
static const s64 link_freq[] = {
@@ -795,8 +794,6 @@ static int ov9282_get_pad_format(struct v4l2_subdev *sd,
{
struct ov9282 *ov9282 = to_ov9282(sd);
- mutex_lock(&ov9282->mutex);
-
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
struct v4l2_mbus_framefmt *framefmt;
@@ -807,8 +804,6 @@ static int ov9282_get_pad_format(struct v4l2_subdev *sd,
fmt);
}
- mutex_unlock(&ov9282->mutex);
-
return 0;
}
@@ -829,8 +824,6 @@ static int ov9282_set_pad_format(struct v4l2_subdev *sd,
u32 code;
int ret = 0;
- mutex_lock(&ov9282->mutex);
-
mode = v4l2_find_nearest_size(supported_modes,
ARRAY_SIZE(supported_modes),
width, height,
@@ -856,8 +849,6 @@ static int ov9282_set_pad_format(struct v4l2_subdev *sd,
}
}
- mutex_unlock(&ov9282->mutex);
-
return ret;
}
@@ -904,10 +895,8 @@ static int ov9282_get_selection(struct v4l2_subdev *sd,
case V4L2_SEL_TGT_CROP: {
struct ov9282 *ov9282 = to_ov9282(sd);
- mutex_lock(&ov9282->mutex);
sel->r = *__ov9282_get_pad_crop(ov9282, sd_state, sel->pad,
sel->which);
- mutex_unlock(&ov9282->mutex);
return 0;
}
@@ -1019,9 +1008,10 @@ static int ov9282_stop_streaming(struct ov9282 *ov9282)
static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
{
struct ov9282 *ov9282 = to_ov9282(sd);
+ struct v4l2_subdev_state *state;
int ret;
- mutex_lock(&ov9282->mutex);
+ state = v4l2_subdev_lock_and_get_active_state(sd);
if (enable) {
ret = pm_runtime_resume_and_get(ov9282->dev);
@@ -1036,14 +1026,14 @@ static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
pm_runtime_put(ov9282->dev);
}
- mutex_unlock(&ov9282->mutex);
+ v4l2_subdev_unlock_state(state);
return 0;
error_power_off:
pm_runtime_put(ov9282->dev);
error_unlock:
- mutex_unlock(&ov9282->mutex);
+ v4l2_subdev_unlock_state(state);
return ret;
}
@@ -1285,9 +1275,6 @@ static int ov9282_init_controls(struct ov9282 *ov9282)
if (ret)
return ret;
- /* Serialize controls with sensor device */
- ctrl_hdlr->lock = &ov9282->mutex;
-
/* Initialize exposure and gain */
lpfr = mode->vblank + mode->height;
ov9282->exp_ctrl = v4l2_ctrl_new_std(ctrl_hdlr,
@@ -1409,13 +1396,10 @@ static int ov9282_probe(struct i2c_client *client)
return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
"Failed to init CCI\n");
- mutex_init(&ov9282->mutex);
-
ret = ov9282_power_on(ov9282->dev);
- if (ret) {
- dev_err(ov9282->dev, "failed to power-on the sensor");
- goto error_mutex_destroy;
- }
+ if (ret)
+ return dev_err_probe(ov9282->dev, ret,
+ "failed to power-on the sensor");
/* Check module identity */
ret = ov9282_detect(ov9282);
@@ -1448,27 +1432,34 @@ static int ov9282_probe(struct i2c_client *client)
goto error_handler_free;
}
- ret = v4l2_async_register_subdev_sensor(&ov9282->sd);
+ ov9282->sd.state_lock = ov9282->ctrl_handler.lock;
+ ret = v4l2_subdev_init_finalize(&ov9282->sd);
if (ret < 0) {
- dev_err(ov9282->dev,
- "failed to register async subdev: %d", ret);
+ dev_err_probe(ov9282->dev, ret, "failed to init subdev\n");
goto error_media_entity;
}
pm_runtime_set_active(ov9282->dev);
pm_runtime_enable(ov9282->dev);
+
+ ret = v4l2_async_register_subdev_sensor(&ov9282->sd);
+ if (ret < 0)
+ goto v4l2_subdev_cleanup;
+
pm_runtime_idle(ov9282->dev);
return 0;
+v4l2_subdev_cleanup:
+ v4l2_subdev_cleanup(&ov9282->sd);
+ pm_runtime_disable(ov9282->dev);
+ pm_runtime_set_suspended(ov9282->dev);
error_media_entity:
media_entity_cleanup(&ov9282->sd.entity);
error_handler_free:
v4l2_ctrl_handler_free(ov9282->sd.ctrl_handler);
error_power_off:
ov9282_power_off(ov9282->dev);
-error_mutex_destroy:
- mutex_destroy(&ov9282->mutex);
return ret;
}
@@ -1482,9 +1473,9 @@ static int ov9282_probe(struct i2c_client *client)
static void ov9282_remove(struct i2c_client *client)
{
struct v4l2_subdev *sd = i2c_get_clientdata(client);
- struct ov9282 *ov9282 = to_ov9282(sd);
v4l2_async_unregister_subdev(sd);
+ v4l2_subdev_cleanup(sd);
media_entity_cleanup(&sd->entity);
v4l2_ctrl_handler_free(sd->ctrl_handler);
@@ -1492,8 +1483,6 @@ static void ov9282_remove(struct i2c_client *client)
if (!pm_runtime_status_suspended(&client->dev))
ov9282_power_off(&client->dev);
pm_runtime_set_suspended(&client->dev);
-
- mutex_destroy(&ov9282->mutex);
}
static const struct dev_pm_ops ov9282_pm_ops = {
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 3/3] media: i2c: ov9282: switch to {enable,disable}_streams
2026-03-05 4:33 [PATCH v4 0/3] media: i2c: ov9282: Modernize driver with CCI and streams API Xiaolei Wang
2026-03-05 4:33 ` [PATCH v4 1/3] media: i2c: ov9282: Convert to CCI register access helpers Xiaolei Wang
2026-03-05 4:33 ` [PATCH v4 2/3] media: i2c: ov9282: Switch to using the sub-device state lock Xiaolei Wang
@ 2026-03-05 4:33 ` Xiaolei Wang
2026-03-05 14:44 ` Dave Stevenson
2 siblings, 1 reply; 7+ messages in thread
From: Xiaolei Wang @ 2026-03-05 4:33 UTC (permalink / raw)
To: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab,
prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede,
hverkuil-cisco, jai.luthra, dave.stevenson, Xiaolei.Wang
Cc: linux-media, linux-kernel
Switch from s_stream to enable_streams and disable_streams callbacks.
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
---
drivers/media/i2c/ov9282.c | 79 ++++++++++++--------------------------
1 file changed, 25 insertions(+), 54 deletions(-)
diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 98e0a0732ef7..22bea5cd6d14 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -922,13 +922,9 @@ static int ov9282_get_selection(struct v4l2_subdev *sd,
return -EINVAL;
}
-/**
- * ov9282_start_streaming() - Start sensor stream
- * @ov9282: pointer to ov9282 device
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int ov9282_start_streaming(struct ov9282 *ov9282)
+static int ov9282_enable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state, u32 pad,
+ u64 streams_mask)
{
const struct cci_reg_sequence bitdepth_regs[2][2] = {
{
@@ -939,16 +935,21 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
{OV9282_REG_ANA_CORE_2, OV9282_ANA_CORE2_RAW8},
}
};
+ struct ov9282 *ov9282 = to_ov9282(sd);
const struct ov9282_reg_list *reg_list;
int bitdepth_index;
int ret;
+ ret = pm_runtime_resume_and_get(ov9282->dev);
+ if (ret)
+ return ret;
+
/* Write common registers */
ret = cci_multi_reg_write(ov9282->regmap, common_regs,
ARRAY_SIZE(common_regs), NULL);
if (ret) {
dev_err(ov9282->dev, "fail to write common registers");
- return ret;
+ goto err_pm_put;
}
bitdepth_index = ov9282->code == MEDIA_BUS_FMT_Y10_1X10 ? 0 : 1;
@@ -956,7 +957,7 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
bitdepth_regs[bitdepth_index], 2, NULL);
if (ret) {
dev_err(ov9282->dev, "fail to write bitdepth regs");
- return ret;
+ goto err_pm_put;
}
/* Write sensor mode registers */
@@ -965,14 +966,14 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
reg_list->num_of_regs, NULL);
if (ret) {
dev_err(ov9282->dev, "fail to write initial registers");
- return ret;
+ goto err_pm_put;
}
/* Setup handler will write actual exposure and gain */
ret = __v4l2_ctrl_handler_setup(ov9282->sd.ctrl_handler);
if (ret) {
dev_err(ov9282->dev, "fail to setup handler");
- return ret;
+ goto err_pm_put;
}
/* Start streaming */
@@ -980,60 +981,28 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
OV9282_MODE_STREAMING, NULL);
if (ret) {
dev_err(ov9282->dev, "fail to start streaming");
- return ret;
+ goto err_pm_put;
}
return 0;
-}
-/**
- * ov9282_stop_streaming() - Stop sensor stream
- * @ov9282: pointer to ov9282 device
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int ov9282_stop_streaming(struct ov9282 *ov9282)
-{
- return cci_write(ov9282->regmap, OV9282_REG_MODE_SELECT,
- OV9282_MODE_STANDBY, NULL);
+err_pm_put:
+ pm_runtime_put(ov9282->dev);
+
+ return ret;
}
-/**
- * ov9282_set_stream() - Enable sensor streaming
- * @sd: pointer to ov9282 subdevice
- * @enable: set to enable sensor streaming
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
+static int ov9282_disable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state, u32 pad,
+ u64 streams_mask)
{
struct ov9282 *ov9282 = to_ov9282(sd);
- struct v4l2_subdev_state *state;
int ret;
- state = v4l2_subdev_lock_and_get_active_state(sd);
-
- if (enable) {
- ret = pm_runtime_resume_and_get(ov9282->dev);
- if (ret)
- goto error_unlock;
-
- ret = ov9282_start_streaming(ov9282);
- if (ret)
- goto error_power_off;
- } else {
- ov9282_stop_streaming(ov9282);
- pm_runtime_put(ov9282->dev);
- }
-
- v4l2_subdev_unlock_state(state);
-
- return 0;
+ ret = cci_write(ov9282->regmap, OV9282_REG_MODE_SELECT,
+ OV9282_MODE_STANDBY, NULL);
-error_power_off:
pm_runtime_put(ov9282->dev);
-error_unlock:
- v4l2_subdev_unlock_state(state);
return ret;
}
@@ -1165,7 +1134,7 @@ static const struct v4l2_subdev_core_ops ov9282_core_ops = {
};
static const struct v4l2_subdev_video_ops ov9282_video_ops = {
- .s_stream = ov9282_set_stream,
+ .s_stream = v4l2_subdev_s_stream_helper,
};
static const struct v4l2_subdev_pad_ops ov9282_pad_ops = {
@@ -1174,6 +1143,8 @@ static const struct v4l2_subdev_pad_ops ov9282_pad_ops = {
.get_fmt = ov9282_get_pad_format,
.set_fmt = ov9282_set_pad_format,
.get_selection = ov9282_get_selection,
+ .enable_streams = ov9282_enable_streams,
+ .disable_streams = ov9282_disable_streams,
};
static const struct v4l2_subdev_ops ov9282_subdev_ops = {
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] media: i2c: ov9282: Convert to CCI register access helpers
2026-03-05 4:33 ` [PATCH v4 1/3] media: i2c: ov9282: Convert to CCI register access helpers Xiaolei Wang
@ 2026-03-05 10:57 ` Dave Stevenson
0 siblings, 0 replies; 7+ messages in thread
From: Dave Stevenson @ 2026-03-05 10:57 UTC (permalink / raw)
To: Xiaolei Wang
Cc: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab,
prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede,
hverkuil-cisco, jai.luthra, linux-media, linux-kernel
On Thu, 5 Mar 2026 at 04:35, Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
>
> Use the new common CCI register access helpers to replace the private
> register access helpers in the ov9282 driver. This simplifies the driver
> by reducing the amount of code.
>
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
Thanks for the update
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
> drivers/media/i2c/Kconfig | 1 +
> drivers/media/i2c/ov9282.c | 561 +++++++++++++------------------------
> 2 files changed, 198 insertions(+), 364 deletions(-)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 5eb1e0e0a87a..3027e71fd8fb 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -690,6 +690,7 @@ config VIDEO_OV8865
> config VIDEO_OV9282
> tristate "OmniVision OV9282 sensor support"
> depends on OF_GPIO
> + select V4L2_CCI_I2C
> help
> This is a Video4Linux2 sensor driver for the OmniVision
> OV9282 camera sensor.
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index ded9b2044ff8..56f854a4d04f 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -12,38 +12,40 @@
> #include <linux/math.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
>
> +#include <media/v4l2-cci.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-event.h>
> #include <media/v4l2-fwnode.h>
> #include <media/v4l2-subdev.h>
>
> /* Streaming Mode */
> -#define OV9282_REG_MODE_SELECT 0x0100
> +#define OV9282_REG_MODE_SELECT CCI_REG8(0x0100)
> #define OV9282_MODE_STANDBY 0x00
> #define OV9282_MODE_STREAMING 0x01
>
> -#define OV9282_REG_PLL_CTRL_0D 0x030d
> +#define OV9282_REG_PLL_CTRL_0D CCI_REG8(0x030d)
> #define OV9282_PLL_CTRL_0D_RAW8 0x60
> #define OV9282_PLL_CTRL_0D_RAW10 0x50
>
> -#define OV9282_REG_TIMING_HTS 0x380c
> +#define OV9282_REG_TIMING_HTS CCI_REG16(0x380c)
> #define OV9282_TIMING_HTS_MAX 0x7fff
>
> /* Lines per frame */
> -#define OV9282_REG_LPFR 0x380e
> +#define OV9282_REG_LPFR CCI_REG16(0x380e)
>
> /* Chip ID */
> -#define OV9282_REG_ID 0x300a
> +#define OV9282_REG_ID CCI_REG16(0x300a)
> #define OV9282_ID 0x9281
>
> /* Output enable registers */
> -#define OV9282_REG_OUTPUT_ENABLE4 0x3004
> +#define OV9282_REG_OUTPUT_ENABLE4 CCI_REG8(0x3004)
> #define OV9282_OUTPUT_ENABLE4_GPIO2 BIT(1)
> #define OV9282_OUTPUT_ENABLE4_D9 BIT(0)
>
> -#define OV9282_REG_OUTPUT_ENABLE5 0x3005
> +#define OV9282_REG_OUTPUT_ENABLE5 CCI_REG8(0x3005)
> #define OV9282_OUTPUT_ENABLE5_D8 BIT(7)
> #define OV9282_OUTPUT_ENABLE5_D7 BIT(6)
> #define OV9282_OUTPUT_ENABLE5_D6 BIT(5)
> @@ -53,7 +55,7 @@
> #define OV9282_OUTPUT_ENABLE5_D2 BIT(1)
> #define OV9282_OUTPUT_ENABLE5_D1 BIT(0)
>
> -#define OV9282_REG_OUTPUT_ENABLE6 0x3006
> +#define OV9282_REG_OUTPUT_ENABLE6 CCI_REG8(0x3006)
> #define OV9282_OUTPUT_ENABLE6_D0 BIT(7)
> #define OV9282_OUTPUT_ENABLE6_PCLK BIT(6)
> #define OV9282_OUTPUT_ENABLE6_HREF BIT(5)
> @@ -62,14 +64,14 @@
> #define OV9282_OUTPUT_ENABLE6_VSYNC BIT(1)
>
> /* Exposure control */
> -#define OV9282_REG_EXPOSURE 0x3500
> +#define OV9282_REG_EXPOSURE CCI_REG24(0x3500)
> #define OV9282_EXPOSURE_MIN 1
> #define OV9282_EXPOSURE_OFFSET 25
> #define OV9282_EXPOSURE_STEP 1
> #define OV9282_EXPOSURE_DEFAULT 0x0282
>
> /* AEC/AGC manual */
> -#define OV9282_REG_AEC_MANUAL 0x3503
> +#define OV9282_REG_AEC_MANUAL CCI_REG8(0x3503)
> #define OV9282_DIGFRAC_GAIN_DELAY BIT(6)
> #define OV9282_GAIN_CHANGE_DELAY BIT(5)
> #define OV9282_GAIN_DELAY BIT(4)
> @@ -78,28 +80,28 @@
> #define OV9282_AEC_MANUAL_DEFAULT 0x00
>
> /* Analog gain control */
> -#define OV9282_REG_AGAIN 0x3509
> +#define OV9282_REG_AGAIN CCI_REG8(0x3509)
> #define OV9282_AGAIN_MIN 0x10
> #define OV9282_AGAIN_MAX 0xff
> #define OV9282_AGAIN_STEP 1
> #define OV9282_AGAIN_DEFAULT 0x10
>
> /* Group hold register */
> -#define OV9282_REG_HOLD 0x3308
> +#define OV9282_REG_HOLD CCI_REG8(0x3308)
>
> -#define OV9282_REG_ANA_CORE_2 0x3662
> +#define OV9282_REG_ANA_CORE_2 CCI_REG8(0x3662)
> #define OV9282_ANA_CORE2_RAW8 0x07
> #define OV9282_ANA_CORE2_RAW10 0x05
>
> -#define OV9282_REG_TIMING_FORMAT_1 0x3820
> -#define OV9282_REG_TIMING_FORMAT_2 0x3821
> +#define OV9282_REG_TIMING_FORMAT_1 CCI_REG8(0x3820)
> +#define OV9282_REG_TIMING_FORMAT_2 CCI_REG8(0x3821)
> #define OV9282_FLIP_BIT BIT(2)
>
> -#define OV9282_REG_MIPI_CTRL00 0x4800
> +#define OV9282_REG_MIPI_CTRL00 CCI_REG8(0x4800)
> #define OV9282_GATED_CLOCK BIT(5)
>
> /* Flash/Strobe control registers */
> -#define OV9282_REG_STROBE_FRAME_SPAN 0x3925
> +#define OV9282_REG_STROBE_FRAME_SPAN CCI_REG32(0x3925)
> #define OV9282_STROBE_FRAME_SPAN_DEFAULT 0x0000001a
>
> /* Input clock rate */
> @@ -139,16 +141,6 @@ static const char * const ov9282_supply_names[] = {
>
> #define OV9282_NUM_SUPPLIES ARRAY_SIZE(ov9282_supply_names)
>
> -/**
> - * struct ov9282_reg - ov9282 sensor register
> - * @address: Register address
> - * @val: Register value
> - */
> -struct ov9282_reg {
> - u16 address;
> - u8 val;
> -};
> -
> /**
> * struct ov9282_reg_list - ov9282 sensor register list
> * @num_of_regs: Number of registers in the list
> @@ -156,7 +148,7 @@ struct ov9282_reg {
> */
> struct ov9282_reg_list {
> u32 num_of_regs;
> - const struct ov9282_reg *regs;
> + const struct cci_reg_sequence *regs;
> };
>
> /**
> @@ -188,6 +180,7 @@ struct ov9282_mode {
> * struct ov9282 - ov9282 sensor device structure
> * @dev: Pointer to generic device
> * @sd: V4L2 sub-device
> + * @regmap: Regmap for sensor register access
> * @pad: Media pad. Only one pad supported
> * @reset_gpio: Sensor reset gpio
> * @inclk: Sensor input clock
> @@ -209,6 +202,7 @@ struct ov9282_mode {
> struct ov9282 {
> struct device *dev;
> struct v4l2_subdev sd;
> + struct regmap *regmap;
> struct media_pad pad;
> struct gpio_desc *reset_gpio;
> struct clk *inclk;
> @@ -241,73 +235,68 @@ static const s64 link_freq[] = {
> * register arrays as some settings are written as part of ov9282_power_on,
> * and the reset will clear them.
> */
> -static const struct ov9282_reg common_regs[] = {
> - {0x0302, 0x32},
> - {0x030e, 0x02},
> - {0x3001, 0x00},
> +static const struct cci_reg_sequence common_regs[] = {
> + {CCI_REG8(0x0302), 0x32},
> + {CCI_REG8(0x030e), 0x02},
> + {CCI_REG8(0x3001), 0x00},
> {OV9282_REG_OUTPUT_ENABLE4, 0x00},
> {OV9282_REG_OUTPUT_ENABLE5, 0x00},
> {OV9282_REG_OUTPUT_ENABLE6, OV9282_OUTPUT_ENABLE6_ILPWM},
> - {0x3011, 0x0a},
> - {0x3013, 0x18},
> - {0x301c, 0xf0},
> - {0x3022, 0x01},
> - {0x3030, 0x10},
> - {0x3039, 0x32},
> - {0x303a, 0x00},
> + {CCI_REG8(0x3011), 0x0a},
> + {CCI_REG8(0x3013), 0x18},
> + {CCI_REG8(0x301c), 0xf0},
> + {CCI_REG8(0x3022), 0x01},
> + {CCI_REG8(0x3030), 0x10},
> + {CCI_REG8(0x3039), 0x32},
> + {CCI_REG8(0x303a), 0x00},
> {OV9282_REG_AEC_MANUAL, OV9282_GAIN_PREC16_EN},
> - {0x3505, 0x8c},
> - {0x3507, 0x03},
> - {0x3508, 0x00},
> - {0x3610, 0x80},
> - {0x3611, 0xa0},
> - {0x3620, 0x6e},
> - {0x3632, 0x56},
> - {0x3633, 0x78},
> - {0x3666, 0x00},
> - {0x366f, 0x5a},
> - {0x3680, 0x84},
> - {0x3712, 0x80},
> - {0x372d, 0x22},
> - {0x3731, 0x80},
> - {0x3732, 0x30},
> - {0x377d, 0x22},
> - {0x3788, 0x02},
> - {0x3789, 0xa4},
> - {0x378a, 0x00},
> - {0x378b, 0x4a},
> - {0x3799, 0x20},
> - {0x3881, 0x42},
> - {0x38a8, 0x02},
> - {0x38a9, 0x80},
> - {0x38b1, 0x00},
> - {0x38c4, 0x00},
> - {0x38c5, 0xc0},
> - {0x38c6, 0x04},
> - {0x38c7, 0x80},
> - {0x3920, 0xff},
> - {0x4010, 0x40},
> - {0x4043, 0x40},
> - {0x4307, 0x30},
> - {0x4317, 0x00},
> - {0x4501, 0x00},
> - {0x450a, 0x08},
> - {0x4601, 0x04},
> - {0x470f, 0x00},
> - {0x4f07, 0x00},
> - {0x5000, 0x9f},
> - {0x5001, 0x00},
> - {0x5e00, 0x00},
> - {0x5d00, 0x07},
> - {0x5d01, 0x00},
> - {0x0101, 0x01},
> - {0x1000, 0x03},
> - {0x5a08, 0x84},
> -};
> -
> -static struct ov9282_reg_list common_regs_list = {
> - .num_of_regs = ARRAY_SIZE(common_regs),
> - .regs = common_regs,
> + {CCI_REG8(0x3505), 0x8c},
> + {CCI_REG8(0x3507), 0x03},
> + {CCI_REG8(0x3508), 0x00},
> + {CCI_REG8(0x3610), 0x80},
> + {CCI_REG8(0x3611), 0xa0},
> + {CCI_REG8(0x3620), 0x6e},
> + {CCI_REG8(0x3632), 0x56},
> + {CCI_REG8(0x3633), 0x78},
> + {CCI_REG8(0x3666), 0x00},
> + {CCI_REG8(0x366f), 0x5a},
> + {CCI_REG8(0x3680), 0x84},
> + {CCI_REG8(0x3712), 0x80},
> + {CCI_REG8(0x372d), 0x22},
> + {CCI_REG8(0x3731), 0x80},
> + {CCI_REG8(0x3732), 0x30},
> + {CCI_REG8(0x377d), 0x22},
> + {CCI_REG8(0x3788), 0x02},
> + {CCI_REG8(0x3789), 0xa4},
> + {CCI_REG8(0x378a), 0x00},
> + {CCI_REG8(0x378b), 0x4a},
> + {CCI_REG8(0x3799), 0x20},
> + {CCI_REG8(0x3881), 0x42},
> + {CCI_REG8(0x38a8), 0x02},
> + {CCI_REG8(0x38a9), 0x80},
> + {CCI_REG8(0x38b1), 0x00},
> + {CCI_REG8(0x38c4), 0x00},
> + {CCI_REG8(0x38c5), 0xc0},
> + {CCI_REG8(0x38c6), 0x04},
> + {CCI_REG8(0x38c7), 0x80},
> + {CCI_REG8(0x3920), 0xff},
> + {CCI_REG8(0x4010), 0x40},
> + {CCI_REG8(0x4043), 0x40},
> + {CCI_REG8(0x4307), 0x30},
> + {CCI_REG8(0x4317), 0x00},
> + {CCI_REG8(0x4501), 0x00},
> + {CCI_REG8(0x450a), 0x08},
> + {CCI_REG8(0x4601), 0x04},
> + {CCI_REG8(0x470f), 0x00},
> + {CCI_REG8(0x4f07), 0x00},
> + {CCI_REG8(0x5000), 0x9f},
> + {CCI_REG8(0x5001), 0x00},
> + {CCI_REG8(0x5e00), 0x00},
> + {CCI_REG8(0x5d00), 0x07},
> + {CCI_REG8(0x5d01), 0x00},
> + {CCI_REG8(0x0101), 0x01},
> + {CCI_REG8(0x1000), 0x03},
> + {CCI_REG8(0x5a08), 0x84},
> };
>
> #define MODE_1280_800 0
> @@ -317,96 +306,96 @@ static struct ov9282_reg_list common_regs_list = {
> #define DEFAULT_MODE MODE_1280_720
>
> /* Sensor mode registers */
> -static const struct ov9282_reg mode_1280x800_regs[] = {
> - {0x3778, 0x00},
> - {0x3800, 0x00},
> - {0x3801, 0x00},
> - {0x3802, 0x00},
> - {0x3803, 0x00},
> - {0x3804, 0x05},
> - {0x3805, 0x0f},
> - {0x3806, 0x03},
> - {0x3807, 0x2f},
> - {0x3808, 0x05},
> - {0x3809, 0x00},
> - {0x380a, 0x03},
> - {0x380b, 0x20},
> - {0x3810, 0x00},
> - {0x3811, 0x08},
> - {0x3812, 0x00},
> - {0x3813, 0x08},
> - {0x3814, 0x11},
> - {0x3815, 0x11},
> +static const struct cci_reg_sequence mode_1280x800_regs[] = {
> + {CCI_REG8(0x3778), 0x00},
> + {CCI_REG8(0x3800), 0x00},
> + {CCI_REG8(0x3801), 0x00},
> + {CCI_REG8(0x3802), 0x00},
> + {CCI_REG8(0x3803), 0x00},
> + {CCI_REG8(0x3804), 0x05},
> + {CCI_REG8(0x3805), 0x0f},
> + {CCI_REG8(0x3806), 0x03},
> + {CCI_REG8(0x3807), 0x2f},
> + {CCI_REG8(0x3808), 0x05},
> + {CCI_REG8(0x3809), 0x00},
> + {CCI_REG8(0x380a), 0x03},
> + {CCI_REG8(0x380b), 0x20},
> + {CCI_REG8(0x3810), 0x00},
> + {CCI_REG8(0x3811), 0x08},
> + {CCI_REG8(0x3812), 0x00},
> + {CCI_REG8(0x3813), 0x08},
> + {CCI_REG8(0x3814), 0x11},
> + {CCI_REG8(0x3815), 0x11},
> {OV9282_REG_TIMING_FORMAT_1, 0x40},
> {OV9282_REG_TIMING_FORMAT_2, 0x00},
> - {0x4003, 0x40},
> - {0x4008, 0x04},
> - {0x4009, 0x0b},
> - {0x400c, 0x00},
> - {0x400d, 0x07},
> - {0x4507, 0x00},
> - {0x4509, 0x00},
> + {CCI_REG8(0x4003), 0x40},
> + {CCI_REG8(0x4008), 0x04},
> + {CCI_REG8(0x4009), 0x0b},
> + {CCI_REG8(0x400c), 0x00},
> + {CCI_REG8(0x400d), 0x07},
> + {CCI_REG8(0x4507), 0x00},
> + {CCI_REG8(0x4509), 0x00},
> };
>
> -static const struct ov9282_reg mode_1280x720_regs[] = {
> - {0x3778, 0x00},
> - {0x3800, 0x00},
> - {0x3801, 0x00},
> - {0x3802, 0x00},
> - {0x3803, 0x00},
> - {0x3804, 0x05},
> - {0x3805, 0x0f},
> - {0x3806, 0x02},
> - {0x3807, 0xdf},
> - {0x3808, 0x05},
> - {0x3809, 0x00},
> - {0x380a, 0x02},
> - {0x380b, 0xd0},
> - {0x3810, 0x00},
> - {0x3811, 0x08},
> - {0x3812, 0x00},
> - {0x3813, 0x08},
> - {0x3814, 0x11},
> - {0x3815, 0x11},
> +static const struct cci_reg_sequence mode_1280x720_regs[] = {
> + {CCI_REG8(0x3778), 0x00},
> + {CCI_REG8(0x3800), 0x00},
> + {CCI_REG8(0x3801), 0x00},
> + {CCI_REG8(0x3802), 0x00},
> + {CCI_REG8(0x3803), 0x00},
> + {CCI_REG8(0x3804), 0x05},
> + {CCI_REG8(0x3805), 0x0f},
> + {CCI_REG8(0x3806), 0x02},
> + {CCI_REG8(0x3807), 0xdf},
> + {CCI_REG8(0x3808), 0x05},
> + {CCI_REG8(0x3809), 0x00},
> + {CCI_REG8(0x380a), 0x02},
> + {CCI_REG8(0x380b), 0xd0},
> + {CCI_REG8(0x3810), 0x00},
> + {CCI_REG8(0x3811), 0x08},
> + {CCI_REG8(0x3812), 0x00},
> + {CCI_REG8(0x3813), 0x08},
> + {CCI_REG8(0x3814), 0x11},
> + {CCI_REG8(0x3815), 0x11},
> {OV9282_REG_TIMING_FORMAT_1, 0x3c},
> {OV9282_REG_TIMING_FORMAT_2, 0x84},
> - {0x4003, 0x40},
> - {0x4008, 0x02},
> - {0x4009, 0x05},
> - {0x400c, 0x00},
> - {0x400d, 0x03},
> - {0x4507, 0x00},
> - {0x4509, 0x80},
> + {CCI_REG8(0x4003), 0x40},
> + {CCI_REG8(0x4008), 0x02},
> + {CCI_REG8(0x4009), 0x05},
> + {CCI_REG8(0x400c), 0x00},
> + {CCI_REG8(0x400d), 0x03},
> + {CCI_REG8(0x4507), 0x00},
> + {CCI_REG8(0x4509), 0x80},
> };
>
> -static const struct ov9282_reg mode_640x400_regs[] = {
> - {0x3778, 0x10},
> - {0x3800, 0x00},
> - {0x3801, 0x00},
> - {0x3802, 0x00},
> - {0x3803, 0x00},
> - {0x3804, 0x05},
> - {0x3805, 0x0f},
> - {0x3806, 0x03},
> - {0x3807, 0x2f},
> - {0x3808, 0x02},
> - {0x3809, 0x80},
> - {0x380a, 0x01},
> - {0x380b, 0x90},
> - {0x3810, 0x00},
> - {0x3811, 0x04},
> - {0x3812, 0x00},
> - {0x3813, 0x04},
> - {0x3814, 0x31},
> - {0x3815, 0x22},
> +static const struct cci_reg_sequence mode_640x400_regs[] = {
> + {CCI_REG8(0x3778), 0x10},
> + {CCI_REG8(0x3800), 0x00},
> + {CCI_REG8(0x3801), 0x00},
> + {CCI_REG8(0x3802), 0x00},
> + {CCI_REG8(0x3803), 0x00},
> + {CCI_REG8(0x3804), 0x05},
> + {CCI_REG8(0x3805), 0x0f},
> + {CCI_REG8(0x3806), 0x03},
> + {CCI_REG8(0x3807), 0x2f},
> + {CCI_REG8(0x3808), 0x02},
> + {CCI_REG8(0x3809), 0x80},
> + {CCI_REG8(0x380a), 0x01},
> + {CCI_REG8(0x380b), 0x90},
> + {CCI_REG8(0x3810), 0x00},
> + {CCI_REG8(0x3811), 0x04},
> + {CCI_REG8(0x3812), 0x00},
> + {CCI_REG8(0x3813), 0x04},
> + {CCI_REG8(0x3814), 0x31},
> + {CCI_REG8(0x3815), 0x22},
> {OV9282_REG_TIMING_FORMAT_1, 0x60},
> {OV9282_REG_TIMING_FORMAT_2, 0x01},
> - {0x4008, 0x02},
> - {0x4009, 0x05},
> - {0x400c, 0x00},
> - {0x400d, 0x03},
> - {0x4507, 0x03},
> - {0x4509, 0x80},
> + {CCI_REG8(0x4008), 0x02},
> + {CCI_REG8(0x4009), 0x05},
> + {CCI_REG8(0x400c), 0x00},
> + {CCI_REG8(0x400d), 0x03},
> + {CCI_REG8(0x4507), 0x03},
> + {CCI_REG8(0x4509), 0x80},
> };
>
> /* Supported sensor mode configurations */
> @@ -485,97 +474,6 @@ static inline struct ov9282 *to_ov9282(struct v4l2_subdev *subdev)
> return container_of(subdev, struct ov9282, sd);
> }
>
> -/**
> - * ov9282_read_reg() - Read registers.
> - * @ov9282: pointer to ov9282 device
> - * @reg: register address
> - * @len: length of bytes to read. Max supported bytes is 4
> - * @val: pointer to register value to be filled.
> - *
> - * Return: 0 if successful, error code otherwise.
> - */
> -static int ov9282_read_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 *val)
> -{
> - struct i2c_client *client = v4l2_get_subdevdata(&ov9282->sd);
> - struct i2c_msg msgs[2] = {0};
> - u8 addr_buf[2] = {0};
> - u8 data_buf[4] = {0};
> - int ret;
> -
> - if (WARN_ON(len > 4))
> - return -EINVAL;
> -
> - put_unaligned_be16(reg, addr_buf);
> -
> - /* Write register address */
> - msgs[0].addr = client->addr;
> - msgs[0].flags = 0;
> - msgs[0].len = ARRAY_SIZE(addr_buf);
> - msgs[0].buf = addr_buf;
> -
> - /* Read data from register */
> - msgs[1].addr = client->addr;
> - msgs[1].flags = I2C_M_RD;
> - msgs[1].len = len;
> - msgs[1].buf = &data_buf[4 - len];
> -
> - ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> - if (ret != ARRAY_SIZE(msgs))
> - return -EIO;
> -
> - *val = get_unaligned_be32(data_buf);
> -
> - return 0;
> -}
> -
> -/**
> - * ov9282_write_reg() - Write register
> - * @ov9282: pointer to ov9282 device
> - * @reg: register address
> - * @len: length of bytes. Max supported bytes is 4
> - * @val: register value
> - *
> - * Return: 0 if successful, error code otherwise.
> - */
> -static int ov9282_write_reg(struct ov9282 *ov9282, u16 reg, u32 len, u32 val)
> -{
> - struct i2c_client *client = v4l2_get_subdevdata(&ov9282->sd);
> - u8 buf[6] = {0};
> -
> - if (WARN_ON(len > 4))
> - return -EINVAL;
> -
> - put_unaligned_be16(reg, buf);
> - put_unaligned_be32(val << (8 * (4 - len)), buf + 2);
> - if (i2c_master_send(client, buf, len + 2) != len + 2)
> - return -EIO;
> -
> - return 0;
> -}
> -
> -/**
> - * ov9282_write_regs() - Write a list of registers
> - * @ov9282: pointer to ov9282 device
> - * @regs: list of registers to be written
> - * @len: length of registers array
> - *
> - * Return: 0 if successful, error code otherwise.
> - */
> -static int ov9282_write_regs(struct ov9282 *ov9282,
> - const struct ov9282_reg *regs, u32 len)
> -{
> - unsigned int i;
> - int ret;
> -
> - for (i = 0; i < len; i++) {
> - ret = ov9282_write_reg(ov9282, regs[i].address, 1, regs[i].val);
> - if (ret)
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> /**
> * ov9282_update_controls() - Update control ranges based on streaming mode
> * @ov9282: pointer to ov9282 device
> @@ -639,15 +537,15 @@ static int ov9282_update_exp_gain(struct ov9282 *ov9282, u32 exposure, u32 gain)
> dev_dbg(ov9282->dev, "Set exp %u (~%u us), analog gain %u",
> exposure, exposure_us, gain);
>
> - ret = ov9282_write_reg(ov9282, OV9282_REG_HOLD, 1, 1);
> + ret = cci_write(ov9282->regmap, OV9282_REG_HOLD, 0x01, NULL);
> if (ret)
> return ret;
>
> - ret = ov9282_write_reg(ov9282, OV9282_REG_EXPOSURE, 3, exposure << 4);
> + ret = cci_write(ov9282->regmap, OV9282_REG_EXPOSURE, exposure << 4, NULL);
> if (ret)
> goto error_release_group_hold;
>
> - ret = ov9282_write_reg(ov9282, OV9282_REG_AGAIN, 1, gain);
> + ret = cci_write(ov9282->regmap, OV9282_REG_AGAIN, gain, NULL);
> if (ret)
> goto error_release_group_hold;
>
> @@ -656,60 +554,9 @@ static int ov9282_update_exp_gain(struct ov9282 *ov9282, u32 exposure, u32 gain)
> OV9282_STROBE_FRAME_SPAN_DEFAULT);
>
> error_release_group_hold:
> - ov9282_write_reg(ov9282, OV9282_REG_HOLD, 1, 0);
> -
> - return ret;
> -}
> -
> -static int ov9282_set_ctrl_hflip(struct ov9282 *ov9282, int value)
> -{
> - u32 current_val;
> - int ret = ov9282_read_reg(ov9282, OV9282_REG_TIMING_FORMAT_2, 1,
> - ¤t_val);
> - if (ret)
> - return ret;
> + int ret_hold = cci_write(ov9282->regmap, OV9282_REG_HOLD, 0, NULL);
>
> - if (value)
> - current_val |= OV9282_FLIP_BIT;
> - else
> - current_val &= ~OV9282_FLIP_BIT;
> -
> - return ov9282_write_reg(ov9282, OV9282_REG_TIMING_FORMAT_2, 1,
> - current_val);
> -}
> -
> -static int ov9282_set_ctrl_vflip(struct ov9282 *ov9282, int value)
> -{
> - u32 current_val;
> - int ret = ov9282_read_reg(ov9282, OV9282_REG_TIMING_FORMAT_1, 1,
> - ¤t_val);
> - if (ret)
> - return ret;
> -
> - if (value)
> - current_val |= OV9282_FLIP_BIT;
> - else
> - current_val &= ~OV9282_FLIP_BIT;
> -
> - return ov9282_write_reg(ov9282, OV9282_REG_TIMING_FORMAT_1, 1,
> - current_val);
> -}
> -
> -static int ov9282_set_ctrl_flash_strobe_oe(struct ov9282 *ov9282, bool enable)
> -{
> - u32 current_val;
> - int ret;
> -
> - ret = ov9282_read_reg(ov9282, OV9282_REG_OUTPUT_ENABLE6, 1, ¤t_val);
> - if (ret)
> - return ret;
> -
> - if (enable)
> - current_val |= OV9282_OUTPUT_ENABLE6_STROBE;
> - else
> - current_val &= ~OV9282_OUTPUT_ENABLE6_STROBE;
> -
> - return ov9282_write_reg(ov9282, OV9282_REG_OUTPUT_ENABLE6, 1, current_val);
> + return ret ? ret : ret_hold;
> }
>
> static u32 ov9282_us_to_flash_duration(struct ov9282 *ov9282, u32 value)
> @@ -740,30 +587,6 @@ static u32 ov9282_flash_duration_to_us(struct ov9282 *ov9282, u32 value)
> return DIV_ROUND_UP(value * frame_width, OV9282_STROBE_SPAN_FACTOR);
> }
>
> -static int ov9282_set_ctrl_flash_duration(struct ov9282 *ov9282, u32 value)
> -{
> - u32 val = ov9282_us_to_flash_duration(ov9282, value);
> - int ret;
> -
> - ret = ov9282_write_reg(ov9282, OV9282_REG_STROBE_FRAME_SPAN, 1,
> - (val >> 24) & 0xff);
> - if (ret)
> - return ret;
> -
> - ret = ov9282_write_reg(ov9282, OV9282_REG_STROBE_FRAME_SPAN + 1, 1,
> - (val >> 16) & 0xff);
> - if (ret)
> - return ret;
> -
> - ret = ov9282_write_reg(ov9282, OV9282_REG_STROBE_FRAME_SPAN + 2, 1,
> - (val >> 8) & 0xff);
> - if (ret)
> - return ret;
> -
> - return ov9282_write_reg(ov9282, OV9282_REG_STROBE_FRAME_SPAN + 3, 1,
> - val & 0xff);
> -}
> -
> /**
> * ov9282_set_ctrl() - Set subdevice control
> * @ctrl: pointer to v4l2_ctrl structure
> @@ -818,23 +641,27 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
> break;
> case V4L2_CID_VBLANK:
> lpfr = ov9282->vblank + ov9282->cur_mode->height;
> - ret = ov9282_write_reg(ov9282, OV9282_REG_LPFR, 2, lpfr);
> + ret = cci_write(ov9282->regmap, OV9282_REG_LPFR, lpfr, NULL);
> break;
> case V4L2_CID_HFLIP:
> - ret = ov9282_set_ctrl_hflip(ov9282, ctrl->val);
> + ret = cci_update_bits(ov9282->regmap, OV9282_REG_TIMING_FORMAT_2,
> + OV9282_FLIP_BIT, ctrl->val ? OV9282_FLIP_BIT : 0, NULL);
> break;
> case V4L2_CID_VFLIP:
> - ret = ov9282_set_ctrl_vflip(ov9282, ctrl->val);
> + ret = cci_update_bits(ov9282->regmap, OV9282_REG_TIMING_FORMAT_1,
> + OV9282_FLIP_BIT, ctrl->val ? OV9282_FLIP_BIT : 0, NULL);
> break;
> case V4L2_CID_HBLANK:
> - ret = ov9282_write_reg(ov9282, OV9282_REG_TIMING_HTS, 2,
> - (ctrl->val + ov9282->cur_mode->width) >> 1);
> + ret = cci_write(ov9282->regmap, OV9282_REG_TIMING_HTS,
> + (ctrl->val + ov9282->cur_mode->width) >> 1, NULL);
> break;
> case V4L2_CID_FLASH_STROBE_OE:
> - ret = ov9282_set_ctrl_flash_strobe_oe(ov9282, ctrl->val);
> + ret = cci_update_bits(ov9282->regmap, OV9282_REG_OUTPUT_ENABLE6,
> + OV9282_OUTPUT_ENABLE6_STROBE,
> + ctrl->val ? OV9282_OUTPUT_ENABLE6_STROBE : 0, NULL);
> break;
> case V4L2_CID_FLASH_DURATION:
> - ret = ov9282_set_ctrl_flash_duration(ov9282, ctrl->val);
> + ret = cci_write(ov9282->regmap, OV9282_REG_STROBE_FRAME_SPAN, ctrl->val, NULL);
> break;
> default:
> dev_err(ov9282->dev, "Invalid control %d", ctrl->id);
> @@ -1114,7 +941,7 @@ static int ov9282_get_selection(struct v4l2_subdev *sd,
> */
> static int ov9282_start_streaming(struct ov9282 *ov9282)
> {
> - const struct ov9282_reg bitdepth_regs[2][2] = {
> + const struct cci_reg_sequence bitdepth_regs[2][2] = {
> {
> {OV9282_REG_PLL_CTRL_0D, OV9282_PLL_CTRL_0D_RAW10},
> {OV9282_REG_ANA_CORE_2, OV9282_ANA_CORE2_RAW10},
> @@ -1128,15 +955,16 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
> int ret;
>
> /* Write common registers */
> - ret = ov9282_write_regs(ov9282, common_regs_list.regs,
> - common_regs_list.num_of_regs);
> + ret = cci_multi_reg_write(ov9282->regmap, common_regs,
> + ARRAY_SIZE(common_regs), NULL);
> if (ret) {
> dev_err(ov9282->dev, "fail to write common registers");
> return ret;
> }
>
> bitdepth_index = ov9282->code == MEDIA_BUS_FMT_Y10_1X10 ? 0 : 1;
> - ret = ov9282_write_regs(ov9282, bitdepth_regs[bitdepth_index], 2);
> + ret = cci_multi_reg_write(ov9282->regmap,
> + bitdepth_regs[bitdepth_index], 2, NULL);
> if (ret) {
> dev_err(ov9282->dev, "fail to write bitdepth regs");
> return ret;
> @@ -1144,7 +972,8 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
>
> /* Write sensor mode registers */
> reg_list = &ov9282->cur_mode->reg_list;
> - ret = ov9282_write_regs(ov9282, reg_list->regs, reg_list->num_of_regs);
> + ret = cci_multi_reg_write(ov9282->regmap, reg_list->regs,
> + reg_list->num_of_regs, NULL);
> if (ret) {
> dev_err(ov9282->dev, "fail to write initial registers");
> return ret;
> @@ -1158,8 +987,8 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
> }
>
> /* Start streaming */
> - ret = ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT,
> - 1, OV9282_MODE_STREAMING);
> + ret = cci_write(ov9282->regmap, OV9282_REG_MODE_SELECT,
> + OV9282_MODE_STREAMING, NULL);
> if (ret) {
> dev_err(ov9282->dev, "fail to start streaming");
> return ret;
> @@ -1176,8 +1005,8 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
> */
> static int ov9282_stop_streaming(struct ov9282 *ov9282)
> {
> - return ov9282_write_reg(ov9282, OV9282_REG_MODE_SELECT,
> - 1, OV9282_MODE_STANDBY);
> + return cci_write(ov9282->regmap, OV9282_REG_MODE_SELECT,
> + OV9282_MODE_STANDBY, NULL);
> }
>
> /**
> @@ -1228,14 +1057,14 @@ static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
> static int ov9282_detect(struct ov9282 *ov9282)
> {
> int ret;
> - u32 val;
> + u64 val;
>
> - ret = ov9282_read_reg(ov9282, OV9282_REG_ID, 2, &val);
> + ret = cci_read(ov9282->regmap, OV9282_REG_ID, &val, NULL);
> if (ret)
> return ret;
>
> if (val != OV9282_ID) {
> - dev_err(ov9282->dev, "chip id mismatch: %x!=%x",
> + dev_err(ov9282->dev, "chip id mismatch: %x!=%llx",
> OV9282_ID, val);
> return -ENXIO;
> }
> @@ -1397,9 +1226,8 @@ static int ov9282_power_on(struct device *dev)
>
> usleep_range(400, 600);
>
> - ret = ov9282_write_reg(ov9282, OV9282_REG_MIPI_CTRL00, 1,
> - ov9282->noncontinuous_clock ?
> - OV9282_GATED_CLOCK : 0);
> + ret = cci_write(ov9282->regmap, OV9282_REG_MIPI_CTRL00,
> + ov9282->noncontinuous_clock ? OV9282_GATED_CLOCK : 0, NULL);
> if (ret) {
> dev_err(ov9282->dev, "fail to write MIPI_CTRL00");
> goto error_clk;
> @@ -1576,6 +1404,11 @@ static int ov9282_probe(struct i2c_client *client)
> return ret;
> }
>
> + ov9282->regmap = devm_cci_regmap_init_i2c(client, 16);
> + if (IS_ERR(ov9282->regmap))
> + return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> + "Failed to init CCI\n");
> +
> mutex_init(&ov9282->mutex);
>
> ret = ov9282_power_on(ov9282->dev);
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/3] media: i2c: ov9282: Switch to using the sub-device state lock
2026-03-05 4:33 ` [PATCH v4 2/3] media: i2c: ov9282: Switch to using the sub-device state lock Xiaolei Wang
@ 2026-03-05 14:44 ` Dave Stevenson
0 siblings, 0 replies; 7+ messages in thread
From: Dave Stevenson @ 2026-03-05 14:44 UTC (permalink / raw)
To: Xiaolei Wang
Cc: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab,
prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede,
hverkuil-cisco, jai.luthra, linux-media, linux-kernel
On Thu, 5 Mar 2026 at 04:35, Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
>
> Switch to using the sub-device state lock and properly call
> v4l2_subdev_init_finalize() / v4l2_subdev_cleanup() on probe() /
> remove().
>
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
> drivers/media/i2c/ov9282.c | 51 +++++++++++++++-----------------------
> 1 file changed, 20 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index 56f854a4d04f..98e0a0732ef7 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -221,7 +221,6 @@ struct ov9282 {
> bool noncontinuous_clock;
> const struct ov9282_mode *cur_mode;
> u32 code;
> - struct mutex mutex;
> };
>
> static const s64 link_freq[] = {
> @@ -795,8 +794,6 @@ static int ov9282_get_pad_format(struct v4l2_subdev *sd,
> {
> struct ov9282 *ov9282 = to_ov9282(sd);
>
> - mutex_lock(&ov9282->mutex);
> -
> if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> struct v4l2_mbus_framefmt *framefmt;
>
> @@ -807,8 +804,6 @@ static int ov9282_get_pad_format(struct v4l2_subdev *sd,
> fmt);
> }
>
> - mutex_unlock(&ov9282->mutex);
> -
> return 0;
> }
>
> @@ -829,8 +824,6 @@ static int ov9282_set_pad_format(struct v4l2_subdev *sd,
> u32 code;
> int ret = 0;
>
> - mutex_lock(&ov9282->mutex);
> -
> mode = v4l2_find_nearest_size(supported_modes,
> ARRAY_SIZE(supported_modes),
> width, height,
> @@ -856,8 +849,6 @@ static int ov9282_set_pad_format(struct v4l2_subdev *sd,
> }
> }
>
> - mutex_unlock(&ov9282->mutex);
> -
> return ret;
> }
>
> @@ -904,10 +895,8 @@ static int ov9282_get_selection(struct v4l2_subdev *sd,
> case V4L2_SEL_TGT_CROP: {
> struct ov9282 *ov9282 = to_ov9282(sd);
>
> - mutex_lock(&ov9282->mutex);
> sel->r = *__ov9282_get_pad_crop(ov9282, sd_state, sel->pad,
> sel->which);
> - mutex_unlock(&ov9282->mutex);
>
> return 0;
> }
> @@ -1019,9 +1008,10 @@ static int ov9282_stop_streaming(struct ov9282 *ov9282)
> static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
> {
> struct ov9282 *ov9282 = to_ov9282(sd);
> + struct v4l2_subdev_state *state;
> int ret;
>
> - mutex_lock(&ov9282->mutex);
> + state = v4l2_subdev_lock_and_get_active_state(sd);
>
> if (enable) {
> ret = pm_runtime_resume_and_get(ov9282->dev);
> @@ -1036,14 +1026,14 @@ static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
> pm_runtime_put(ov9282->dev);
> }
>
> - mutex_unlock(&ov9282->mutex);
> + v4l2_subdev_unlock_state(state);
>
> return 0;
>
> error_power_off:
> pm_runtime_put(ov9282->dev);
> error_unlock:
> - mutex_unlock(&ov9282->mutex);
> + v4l2_subdev_unlock_state(state);
>
> return ret;
> }
> @@ -1285,9 +1275,6 @@ static int ov9282_init_controls(struct ov9282 *ov9282)
> if (ret)
> return ret;
>
> - /* Serialize controls with sensor device */
> - ctrl_hdlr->lock = &ov9282->mutex;
> -
> /* Initialize exposure and gain */
> lpfr = mode->vblank + mode->height;
> ov9282->exp_ctrl = v4l2_ctrl_new_std(ctrl_hdlr,
> @@ -1409,13 +1396,10 @@ static int ov9282_probe(struct i2c_client *client)
> return dev_err_probe(ov9282->dev, PTR_ERR(ov9282->regmap),
> "Failed to init CCI\n");
>
> - mutex_init(&ov9282->mutex);
> -
> ret = ov9282_power_on(ov9282->dev);
> - if (ret) {
> - dev_err(ov9282->dev, "failed to power-on the sensor");
> - goto error_mutex_destroy;
> - }
> + if (ret)
> + return dev_err_probe(ov9282->dev, ret,
> + "failed to power-on the sensor");
>
> /* Check module identity */
> ret = ov9282_detect(ov9282);
> @@ -1448,27 +1432,34 @@ static int ov9282_probe(struct i2c_client *client)
> goto error_handler_free;
> }
>
> - ret = v4l2_async_register_subdev_sensor(&ov9282->sd);
> + ov9282->sd.state_lock = ov9282->ctrl_handler.lock;
> + ret = v4l2_subdev_init_finalize(&ov9282->sd);
> if (ret < 0) {
> - dev_err(ov9282->dev,
> - "failed to register async subdev: %d", ret);
> + dev_err_probe(ov9282->dev, ret, "failed to init subdev\n");
> goto error_media_entity;
> }
>
> pm_runtime_set_active(ov9282->dev);
> pm_runtime_enable(ov9282->dev);
> +
> + ret = v4l2_async_register_subdev_sensor(&ov9282->sd);
> + if (ret < 0)
> + goto v4l2_subdev_cleanup;
> +
> pm_runtime_idle(ov9282->dev);
>
> return 0;
>
> +v4l2_subdev_cleanup:
> + v4l2_subdev_cleanup(&ov9282->sd);
> + pm_runtime_disable(ov9282->dev);
> + pm_runtime_set_suspended(ov9282->dev);
> error_media_entity:
> media_entity_cleanup(&ov9282->sd.entity);
> error_handler_free:
> v4l2_ctrl_handler_free(ov9282->sd.ctrl_handler);
> error_power_off:
> ov9282_power_off(ov9282->dev);
> -error_mutex_destroy:
> - mutex_destroy(&ov9282->mutex);
>
> return ret;
> }
> @@ -1482,9 +1473,9 @@ static int ov9282_probe(struct i2c_client *client)
> static void ov9282_remove(struct i2c_client *client)
> {
> struct v4l2_subdev *sd = i2c_get_clientdata(client);
> - struct ov9282 *ov9282 = to_ov9282(sd);
>
> v4l2_async_unregister_subdev(sd);
> + v4l2_subdev_cleanup(sd);
> media_entity_cleanup(&sd->entity);
> v4l2_ctrl_handler_free(sd->ctrl_handler);
>
> @@ -1492,8 +1483,6 @@ static void ov9282_remove(struct i2c_client *client)
> if (!pm_runtime_status_suspended(&client->dev))
> ov9282_power_off(&client->dev);
> pm_runtime_set_suspended(&client->dev);
> -
> - mutex_destroy(&ov9282->mutex);
> }
>
> static const struct dev_pm_ops ov9282_pm_ops = {
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 3/3] media: i2c: ov9282: switch to {enable,disable}_streams
2026-03-05 4:33 ` [PATCH v4 3/3] media: i2c: ov9282: switch to {enable,disable}_streams Xiaolei Wang
@ 2026-03-05 14:44 ` Dave Stevenson
0 siblings, 0 replies; 7+ messages in thread
From: Dave Stevenson @ 2026-03-05 14:44 UTC (permalink / raw)
To: Xiaolei Wang
Cc: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab,
prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede,
hverkuil-cisco, jai.luthra, linux-media, linux-kernel
On Thu, 5 Mar 2026 at 04:35, Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
>
> Switch from s_stream to enable_streams and disable_streams callbacks.
>
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
For the series:
Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
> drivers/media/i2c/ov9282.c | 79 ++++++++++++--------------------------
> 1 file changed, 25 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index 98e0a0732ef7..22bea5cd6d14 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -922,13 +922,9 @@ static int ov9282_get_selection(struct v4l2_subdev *sd,
> return -EINVAL;
> }
>
> -/**
> - * ov9282_start_streaming() - Start sensor stream
> - * @ov9282: pointer to ov9282 device
> - *
> - * Return: 0 if successful, error code otherwise.
> - */
> -static int ov9282_start_streaming(struct ov9282 *ov9282)
> +static int ov9282_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> {
> const struct cci_reg_sequence bitdepth_regs[2][2] = {
> {
> @@ -939,16 +935,21 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
> {OV9282_REG_ANA_CORE_2, OV9282_ANA_CORE2_RAW8},
> }
> };
> + struct ov9282 *ov9282 = to_ov9282(sd);
> const struct ov9282_reg_list *reg_list;
> int bitdepth_index;
> int ret;
>
> + ret = pm_runtime_resume_and_get(ov9282->dev);
> + if (ret)
> + return ret;
> +
> /* Write common registers */
> ret = cci_multi_reg_write(ov9282->regmap, common_regs,
> ARRAY_SIZE(common_regs), NULL);
> if (ret) {
> dev_err(ov9282->dev, "fail to write common registers");
> - return ret;
> + goto err_pm_put;
> }
>
> bitdepth_index = ov9282->code == MEDIA_BUS_FMT_Y10_1X10 ? 0 : 1;
> @@ -956,7 +957,7 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
> bitdepth_regs[bitdepth_index], 2, NULL);
> if (ret) {
> dev_err(ov9282->dev, "fail to write bitdepth regs");
> - return ret;
> + goto err_pm_put;
> }
>
> /* Write sensor mode registers */
> @@ -965,14 +966,14 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
> reg_list->num_of_regs, NULL);
> if (ret) {
> dev_err(ov9282->dev, "fail to write initial registers");
> - return ret;
> + goto err_pm_put;
> }
>
> /* Setup handler will write actual exposure and gain */
> ret = __v4l2_ctrl_handler_setup(ov9282->sd.ctrl_handler);
> if (ret) {
> dev_err(ov9282->dev, "fail to setup handler");
> - return ret;
> + goto err_pm_put;
> }
>
> /* Start streaming */
> @@ -980,60 +981,28 @@ static int ov9282_start_streaming(struct ov9282 *ov9282)
> OV9282_MODE_STREAMING, NULL);
> if (ret) {
> dev_err(ov9282->dev, "fail to start streaming");
> - return ret;
> + goto err_pm_put;
> }
>
> return 0;
> -}
>
> -/**
> - * ov9282_stop_streaming() - Stop sensor stream
> - * @ov9282: pointer to ov9282 device
> - *
> - * Return: 0 if successful, error code otherwise.
> - */
> -static int ov9282_stop_streaming(struct ov9282 *ov9282)
> -{
> - return cci_write(ov9282->regmap, OV9282_REG_MODE_SELECT,
> - OV9282_MODE_STANDBY, NULL);
> +err_pm_put:
> + pm_runtime_put(ov9282->dev);
> +
> + return ret;
> }
>
> -/**
> - * ov9282_set_stream() - Enable sensor streaming
> - * @sd: pointer to ov9282 subdevice
> - * @enable: set to enable sensor streaming
> - *
> - * Return: 0 if successful, error code otherwise.
> - */
> -static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
> +static int ov9282_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> {
> struct ov9282 *ov9282 = to_ov9282(sd);
> - struct v4l2_subdev_state *state;
> int ret;
>
> - state = v4l2_subdev_lock_and_get_active_state(sd);
> -
> - if (enable) {
> - ret = pm_runtime_resume_and_get(ov9282->dev);
> - if (ret)
> - goto error_unlock;
> -
> - ret = ov9282_start_streaming(ov9282);
> - if (ret)
> - goto error_power_off;
> - } else {
> - ov9282_stop_streaming(ov9282);
> - pm_runtime_put(ov9282->dev);
> - }
> -
> - v4l2_subdev_unlock_state(state);
> -
> - return 0;
> + ret = cci_write(ov9282->regmap, OV9282_REG_MODE_SELECT,
> + OV9282_MODE_STANDBY, NULL);
>
> -error_power_off:
> pm_runtime_put(ov9282->dev);
> -error_unlock:
> - v4l2_subdev_unlock_state(state);
>
> return ret;
> }
> @@ -1165,7 +1134,7 @@ static const struct v4l2_subdev_core_ops ov9282_core_ops = {
> };
>
> static const struct v4l2_subdev_video_ops ov9282_video_ops = {
> - .s_stream = ov9282_set_stream,
> + .s_stream = v4l2_subdev_s_stream_helper,
> };
>
> static const struct v4l2_subdev_pad_ops ov9282_pad_ops = {
> @@ -1174,6 +1143,8 @@ static const struct v4l2_subdev_pad_ops ov9282_pad_ops = {
> .get_fmt = ov9282_get_pad_format,
> .set_fmt = ov9282_set_pad_format,
> .get_selection = ov9282_get_selection,
> + .enable_streams = ov9282_enable_streams,
> + .disable_streams = ov9282_disable_streams,
> };
>
> static const struct v4l2_subdev_ops ov9282_subdev_ops = {
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-03-05 14:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-05 4:33 [PATCH v4 0/3] media: i2c: ov9282: Modernize driver with CCI and streams API Xiaolei Wang
2026-03-05 4:33 ` [PATCH v4 1/3] media: i2c: ov9282: Convert to CCI register access helpers Xiaolei Wang
2026-03-05 10:57 ` Dave Stevenson
2026-03-05 4:33 ` [PATCH v4 2/3] media: i2c: ov9282: Switch to using the sub-device state lock Xiaolei Wang
2026-03-05 14:44 ` Dave Stevenson
2026-03-05 4:33 ` [PATCH v4 3/3] media: i2c: ov9282: switch to {enable,disable}_streams Xiaolei Wang
2026-03-05 14:44 ` Dave Stevenson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox