From e848475c33fc42e8bd7052227e65ca35d6ff046f Mon Sep 17 00:00:00 2001
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Date: Tue, 15 Oct 2024 09:59:52 +0300
Subject: [PATCH 01/16] media: ccs: Try a little longer to access the sensor
 before giving up

Some sensors take longer to respond after reset than the spec-required
time. Try up to 1 s for the sensor to become accessible.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/ccs/ccs-core.c | 26 +++++++++++++-------------
 drivers/media/i2c/ccs/ccs.h      |  2 ++
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 06e0ba53f2a8..a47c56d9779f 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1354,8 +1354,10 @@ static int ccs_change_cci_addr(struct ccs_sensor *sensor)
 
 	client->addr = sensor->hwcfg.i2c_addr_dfl;
 
-	rval = ccs_write(sensor, CCI_ADDRESS_CTRL,
-			 sensor->hwcfg.i2c_addr_alt << 1);
+	rval = read_poll_timeout(ccs_write, rval, !rval, CCS_RESET_DELAY_US,
+				 CCS_RESET_TIMEOUT_US, false, sensor,
+				 CCI_ADDRESS_CTRL,
+				 sensor->hwcfg.i2c_addr_alt << 1);
 	if (rval)
 		return rval;
 
@@ -1575,27 +1577,25 @@ static int ccs_power_on(struct device *dev)
 		if (ccsdev->flags & CCS_DEVICE_FLAG_IS_SMIA)
 			sleep = SMIAPP_RESET_DELAY(sensor->hwcfg.ext_clk);
 		else
-			sleep = 5000;
+			sleep = CCS_RESET_DELAY_US;
 
 		usleep_range(sleep, sleep);
 	}
 
 	/*
-	 * Failures to respond to the address change command have been noticed.
-	 * Those failures seem to be caused by the sensor requiring a longer
-	 * boot time than advertised. An additional 10ms delay seems to work
-	 * around the issue, but the SMIA++ I2C write retry hack makes the delay
-	 * unnecessary. The failures need to be investigated to find a proper
-	 * fix, and a delay will likely need to be added here if the I2C write
-	 * retry hack is reverted before the root cause of the boot time issue
-	 * is found.
+	 * Some devices take longer than the spec-defined time to respond
+	 * after reset. Try until some time has passed before flagging it
+	 * an error.
 	 */
-
 	if (!sensor->reset && !sensor->xshutdown) {
 		u8 retry = 100;
 		u32 reset;
 
-		rval = ccs_write(sensor, SOFTWARE_RESET, CCS_SOFTWARE_RESET_ON);
+		rval = read_poll_timeout(ccs_write, rval, !rval,
+					 CCS_RESET_DELAY_US,
+					 CCS_RESET_TIMEOUT_US,
+					 false, sensor, SOFTWARE_RESET,
+					 CCS_SOFTWARE_RESET_ON);
 		if (rval < 0) {
 			dev_err(dev, "software reset failed\n");
 			goto out_cci_addr_fail;
diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
index 096573845a10..0726c4687f0f 100644
--- a/drivers/media/i2c/ccs/ccs.h
+++ b/drivers/media/i2c/ccs/ccs.h
@@ -43,6 +43,8 @@
 #define SMIAPP_RESET_DELAY(clk)				\
 	(1000 +	(SMIAPP_RESET_DELAY_CLOCKS * 1000	\
 		 + (clk) / 1000 - 1) / ((clk) / 1000))
+#define CCS_RESET_DELAY_US		5000
+#define CCS_RESET_TIMEOUT_US		1000000
 
 #define CCS_COLOUR_COMPONENTS		4
 
-- 
2.51.0


From bb468fc5a4d902c9202f32ed4ee1f0980c17fe52 Mon Sep 17 00:00:00 2001
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Date: Tue, 15 Oct 2024 10:07:02 +0300
Subject: [PATCH 02/16] media: ccs: Use read_poll_timeout() in reset polling

Use read_poll_timeout() in polling the device after a reset, either hard
or soft. While at it, improve the related error message.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/ccs/ccs-core.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index a47c56d9779f..01744ebd3e06 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1588,7 +1588,6 @@ static int ccs_power_on(struct device *dev)
 	 * an error.
 	 */
 	if (!sensor->reset && !sensor->xshutdown) {
-		u8 retry = 100;
 		u32 reset;
 
 		rval = read_poll_timeout(ccs_write, rval, !rval,
@@ -1601,18 +1600,15 @@ static int ccs_power_on(struct device *dev)
 			goto out_cci_addr_fail;
 		}
 
-		do {
-			rval = ccs_read(sensor, SOFTWARE_RESET, &reset);
-			reset = !rval && reset == CCS_SOFTWARE_RESET_OFF;
-			if (reset)
-				break;
-
-			usleep_range(1000, 2000);
-		} while (--retry);
-
-		if (!reset) {
-			dev_err(dev, "software reset failed\n");
-			rval = -EIO;
+		rval = read_poll_timeout(ccs_read, rval,
+					 !rval &&
+						reset == CCS_SOFTWARE_RESET_OFF,
+					 CCS_RESET_DELAY_US,
+					 CCS_RESET_TIMEOUT_US, false, sensor,
+					 SOFTWARE_RESET, &reset);
+		if (rval < 0) {
+			dev_err_probe(dev, rval,
+				      "failed to respond after reset\n");
 			goto out_cci_addr_fail;
 		}
 	}
-- 
2.51.0


From 932518f6f8713d9a23b2ec3e0ebf5ab9e17af728 Mon Sep 17 00:00:00 2001
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Date: Tue, 15 Oct 2024 10:28:42 +0300
Subject: [PATCH 03/16] =?utf8?q?media:=20ccs:=20Remove=20I=C2=B2C=20write?=
 =?utf8?q?=20retry=20hack?=
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

The I²C retry hack has been there in order to address transient I²C
register write access issues on a few very old sensors and possibly it has
addressed also first I²C access problems (device not responding until a
certain amount of time has passed) but that is now separately handled. The
retry hack has a good potential for introducing hard to debug problems in
updating sensor settings while streaming. Remove it and instead pass those
rare errors to the user space -- which is also what virtually all other
drivers do.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/ccs/ccs-reg-access.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-reg-access.c b/drivers/media/i2c/ccs/ccs-reg-access.c
index a696a0ec8ff5..fd36889ccc1d 100644
--- a/drivers/media/i2c/ccs/ccs-reg-access.c
+++ b/drivers/media/i2c/ccs/ccs-reg-access.c
@@ -210,7 +210,6 @@ int ccs_read_addr_noconv(struct ccs_sensor *sensor, u32 reg, u32 *val)
  */
 int ccs_write_addr(struct ccs_sensor *sensor, u32 reg, u32 val)
 {
-	unsigned int retries = 10;
 	int rval;
 
 	rval = ccs_call_quirk(sensor, reg_access, true, &reg, &val);
@@ -219,13 +218,7 @@ int ccs_write_addr(struct ccs_sensor *sensor, u32 reg, u32 val)
 	if (rval < 0)
 		return rval;
 
-	rval = 0;
-	do {
-		if (cci_write(sensor->regmap, reg, val, &rval))
-			fsleep(1000);
-	} while (rval && --retries);
-
-	return rval;
+	return cci_write(sensor->regmap, reg, val, NULL);
 }
 
 #define MAX_WRITE_LEN	32U
-- 
2.51.0


From 980d2c914cbe0af7c9239dacb89ae083a2094d83 Mon Sep 17 00:00:00 2001
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Date: Tue, 15 Oct 2024 13:38:01 +0300
Subject: [PATCH 04/16] media: ccs: Don't complain about lack of quirks

Generally any deviance from the standard is handled via CCS static data
nowadays and so not having quirks is expected. Don't warn about it.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/ccs/ccs-core.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 01744ebd3e06..27b94a399b17 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -2853,10 +2853,6 @@ static int ccs_identify_module(struct ccs_sensor *sensor)
 		break;
 	}
 
-	if (i >= ARRAY_SIZE(ccs_module_idents))
-		dev_warn(&client->dev,
-			 "no quirks for this module; let's hope it's fully compliant\n");
-
 	dev_dbg(&client->dev, "the sensor is called %s\n", minfo->name);
 
 	return 0;
-- 
2.51.0


From 5050bc60cc16ffa35a962a53271210d427a11535 Mon Sep 17 00:00:00 2001
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Date: Fri, 11 Oct 2024 13:56:44 +0300
Subject: [PATCH 05/16] media: ccs: Don't complain about missing
 "clock-frequency" property

The clock frequency is often available via the clock itself and not read
by the driver from the "clock-frequency" property. Don't complain if the
property doesn't exist.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/ccs/ccs-core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 27b94a399b17..487bcabb4a19 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -3123,8 +3123,6 @@ static int ccs_get_hwconfig(struct ccs_sensor *sensor, struct device *dev)
 
 	rval = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
 					&hwcfg->ext_clk);
-	if (rval)
-		dev_info(dev, "can't get clock-frequency\n");
 
 	dev_dbg(dev, "clk %u, mode %u\n", hwcfg->ext_clk,
 		hwcfg->csi_signalling_mode);
-- 
2.51.0


From 1284c9693953aed2a9974a5a384ce2a42a1b9ae8 Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Date: Thu, 10 Apr 2025 11:52:13 +0200
Subject: [PATCH 06/16] media: intel/ipu6: Minor dma_mask clenaup

Remove unused dma_mask field and ipu-dma.h includes.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/pci/intel/ipu6/ipu6-bus.h | 1 -
 drivers/media/pci/intel/ipu6/ipu6-dma.h | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-bus.h b/drivers/media/pci/intel/ipu6/ipu6-bus.h
index b790f9cc37e3..a08c5468d536 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-bus.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-bus.h
@@ -26,7 +26,6 @@ struct ipu6_bus_device {
 	struct ipu6_mmu *mmu;
 	struct ipu6_device *isp;
 	const struct ipu6_buttress_ctrl *ctrl;
-	u64 dma_mask;
 	const struct firmware *fw;
 	struct sg_table fw_sgt;
 	u64 *pkg_dir;
diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.h b/drivers/media/pci/intel/ipu6/ipu6-dma.h
index 2882850d9366..ae9b9a5df57f 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-dma.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-dma.h
@@ -4,9 +4,6 @@
 #ifndef IPU6_DMA_H
 #define IPU6_DMA_H
 
-#include <linux/dma-map-ops.h>
-#include <linux/dma-mapping.h>
-#include <linux/iova.h>
 #include <linux/iova.h>
 #include <linux/scatterlist.h>
 #include <linux/types.h>
-- 
2.51.0


From 0209916ebe2475079ce6d8dc4114afbc0ccad1c2 Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Date: Thu, 10 Apr 2025 11:47:06 +0200
Subject: [PATCH 07/16] media: intel/ipu6: Fix dma mask for non-secure mode

We use dma_get_mask() of auxdev device for calculate iova pfn limit.
This is always 32 bit mask as we do not initialize the mask (and we can
not do so, since dev->dev_mask is NULL anyways for auxdev).

Since we need 31 bit mask for non-secure mode use mmu_info->aperture_end
which is properly initialized to correct mask for both modes.

Fixes: daabc5c64703 ("media: ipu6: not override the dma_ops of device in driver")
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/pci/intel/ipu6/ipu6-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.c b/drivers/media/pci/intel/ipu6/ipu6-dma.c
index 1ca60ca79dba..7296373d36b0 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-dma.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
@@ -172,7 +172,7 @@ void *ipu6_dma_alloc(struct ipu6_bus_device *sys, size_t size,
 	count = PHYS_PFN(size);
 
 	iova = alloc_iova(&mmu->dmap->iovad, count,
-			  PHYS_PFN(dma_get_mask(dev)), 0);
+			  PHYS_PFN(mmu->dmap->mmu_info->aperture_end), 0);
 	if (!iova)
 		goto out_kfree;
 
@@ -398,7 +398,7 @@ int ipu6_dma_map_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
 		nents, npages);
 
 	iova = alloc_iova(&mmu->dmap->iovad, npages,
-			  PHYS_PFN(dma_get_mask(dev)), 0);
+			  PHYS_PFN(mmu->dmap->mmu_info->aperture_end), 0);
 	if (!iova)
 		return 0;
 
-- 
2.51.0


From 78bc2ff83c76db622ae52c2bbf3113a5c6df580a Mon Sep 17 00:00:00 2001
From: Tarang Raval <tarang.raval@siliconsignals.io>
Date: Sat, 29 Mar 2025 11:13:23 +0530
Subject: [PATCH 08/16] media: i2c: imx334: Simplify with dev_err_probe()

Error handling in probe() can be a bit simpler with dev_err_probe().

also, Added missing newline characters (\n) in error messages.

Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/imx334.c | 61 +++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 8cd1eecd0143..ad0b03a3f573 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -658,7 +658,7 @@ static int imx334_update_exp_gain(struct imx334 *imx334, u32 exposure, u32 gain)
 	lpfr = imx334->vblank + imx334->cur_mode->height;
 	shutter = lpfr - exposure;
 
-	dev_dbg(imx334->dev, "Set long exp %u analog gain %u sh0 %u lpfr %u",
+	dev_dbg(imx334->dev, "Set long exp %u analog gain %u sh0 %u lpfr %u\n",
 		exposure, gain, shutter, lpfr);
 
 	ret = imx334_write_reg(imx334, IMX334_REG_HOLD, 1, 1);
@@ -705,7 +705,7 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_VBLANK:
 		imx334->vblank = imx334->vblank_ctrl->val;
 
-		dev_dbg(imx334->dev, "Received vblank %u, new lpfr %u",
+		dev_dbg(imx334->dev, "Received vblank %u, new lpfr %u\n",
 			imx334->vblank,
 			imx334->vblank + imx334->cur_mode->height);
 
@@ -725,7 +725,7 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl)
 		exposure = ctrl->val;
 		analog_gain = imx334->again_ctrl->val;
 
-		dev_dbg(imx334->dev, "Received exp %u analog gain %u",
+		dev_dbg(imx334->dev, "Received exp %u analog gain %u\n",
 			exposure, analog_gain);
 
 		ret = imx334_update_exp_gain(imx334, exposure, analog_gain);
@@ -759,7 +759,7 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl)
 		ret = 0;
 		break;
 	default:
-		dev_err(imx334->dev, "Invalid control %d", ctrl->id);
+		dev_err(imx334->dev, "Invalid control %d\n", ctrl->id);
 		ret = -EINVAL;
 	}
 
@@ -986,7 +986,7 @@ static int imx334_start_streaming(struct imx334 *imx334)
 	ret = imx334_write_regs(imx334, common_mode_regs,
 				ARRAY_SIZE(common_mode_regs));
 	if (ret) {
-		dev_err(imx334->dev, "fail to write common registers");
+		dev_err(imx334->dev, "fail to write common registers\n");
 		return ret;
 	}
 
@@ -995,7 +995,7 @@ static int imx334_start_streaming(struct imx334 *imx334)
 	ret = imx334_write_regs(imx334, reg_list->regs,
 				reg_list->num_of_regs);
 	if (ret) {
-		dev_err(imx334->dev, "fail to write initial registers");
+		dev_err(imx334->dev, "fail to write initial registers\n");
 		return ret;
 	}
 
@@ -1009,7 +1009,7 @@ static int imx334_start_streaming(struct imx334 *imx334)
 	/* Setup handler will write actual exposure and gain */
 	ret =  __v4l2_ctrl_handler_setup(imx334->sd.ctrl_handler);
 	if (ret) {
-		dev_err(imx334->dev, "fail to setup handler");
+		dev_err(imx334->dev, "fail to setup handler\n");
 		return ret;
 	}
 
@@ -1017,7 +1017,7 @@ static int imx334_start_streaming(struct imx334 *imx334)
 	ret = imx334_write_reg(imx334, IMX334_REG_MODE_SELECT,
 			       1, IMX334_MODE_STREAMING);
 	if (ret) {
-		dev_err(imx334->dev, "fail to start streaming");
+		dev_err(imx334->dev, "fail to start streaming\n");
 		return ret;
 	}
 
@@ -1091,7 +1091,7 @@ static int imx334_detect(struct imx334 *imx334)
 		return ret;
 
 	if (val != IMX334_ID) {
-		dev_err(imx334->dev, "chip id mismatch: %x!=%x",
+		dev_err(imx334->dev, "chip id mismatch: %x!=%x\n",
 			IMX334_ID, val);
 		return -ENXIO;
 	}
@@ -1121,24 +1121,20 @@ static int imx334_parse_hw_config(struct imx334 *imx334)
 	/* Request optional reset pin */
 	imx334->reset_gpio = devm_gpiod_get_optional(imx334->dev, "reset",
 						     GPIOD_OUT_LOW);
-	if (IS_ERR(imx334->reset_gpio)) {
-		dev_err(imx334->dev, "failed to get reset gpio %ld",
-			PTR_ERR(imx334->reset_gpio));
-		return PTR_ERR(imx334->reset_gpio);
-	}
+	if (IS_ERR(imx334->reset_gpio))
+		return dev_err_probe(imx334->dev, PTR_ERR(imx334->reset_gpio),
+				     "failed to get reset gpio\n");
 
 	/* Get sensor input clock */
 	imx334->inclk = devm_clk_get(imx334->dev, NULL);
-	if (IS_ERR(imx334->inclk)) {
-		dev_err(imx334->dev, "could not get inclk");
-		return PTR_ERR(imx334->inclk);
-	}
+	if (IS_ERR(imx334->inclk))
+		return dev_err_probe(imx334->dev, PTR_ERR(imx334->inclk),
+					 "could not get inclk\n");
 
 	rate = clk_get_rate(imx334->inclk);
-	if (rate != IMX334_INCLK_RATE) {
-		dev_err(imx334->dev, "inclk frequency mismatch");
-		return -EINVAL;
-	}
+	if (rate != IMX334_INCLK_RATE)
+		return dev_err_probe(imx334->dev, -EINVAL,
+					 "inclk frequency mismatch\n");
 
 	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
 	if (!ep)
@@ -1151,7 +1147,7 @@ static int imx334_parse_hw_config(struct imx334 *imx334)
 
 	if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX334_NUM_DATA_LANES) {
 		dev_err(imx334->dev,
-			"number of CSI2 data lanes %d is not supported",
+			"number of CSI2 data lanes %d is not supported\n",
 			bus_cfg.bus.mipi_csi2.num_data_lanes);
 		ret = -EINVAL;
 		goto done_endpoint_free;
@@ -1205,7 +1201,7 @@ static int imx334_power_on(struct device *dev)
 
 	ret = clk_prepare_enable(imx334->inclk);
 	if (ret) {
-		dev_err(imx334->dev, "fail to enable inclk");
+		dev_err(imx334->dev, "fail to enable inclk\n");
 		goto error_reset;
 	}
 
@@ -1349,23 +1345,22 @@ static int imx334_probe(struct i2c_client *client)
 	imx334->sd.internal_ops = &imx334_internal_ops;
 
 	ret = imx334_parse_hw_config(imx334);
-	if (ret) {
-		dev_err(imx334->dev, "HW configuration is not supported");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(imx334->dev, ret,
+					"HW configuration is not supported\n");
 
 	mutex_init(&imx334->mutex);
 
 	ret = imx334_power_on(imx334->dev);
 	if (ret) {
-		dev_err(imx334->dev, "failed to power-on the sensor");
+		dev_err_probe(imx334->dev, ret, "failed to power-on the sensor\n");
 		goto error_mutex_destroy;
 	}
 
 	/* Check module identity */
 	ret = imx334_detect(imx334);
 	if (ret) {
-		dev_err(imx334->dev, "failed to find sensor: %d", ret);
+		dev_err(imx334->dev, "failed to find sensor: %d\n", ret);
 		goto error_power_off;
 	}
 
@@ -1376,7 +1371,7 @@ static int imx334_probe(struct i2c_client *client)
 
 	ret = imx334_init_controls(imx334);
 	if (ret) {
-		dev_err(imx334->dev, "failed to init controls: %d", ret);
+		dev_err(imx334->dev, "failed to init controls: %d\n", ret);
 		goto error_power_off;
 	}
 
@@ -1388,14 +1383,14 @@ static int imx334_probe(struct i2c_client *client)
 	imx334->pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_pads_init(&imx334->sd.entity, 1, &imx334->pad);
 	if (ret) {
-		dev_err(imx334->dev, "failed to init entity pads: %d", ret);
+		dev_err(imx334->dev, "failed to init entity pads: %d\n", ret);
 		goto error_handler_free;
 	}
 
 	ret = v4l2_async_register_subdev_sensor(&imx334->sd);
 	if (ret < 0) {
 		dev_err(imx334->dev,
-			"failed to register async subdev: %d", ret);
+			"failed to register async subdev: %d\n", ret);
 		goto error_media_entity;
 	}
 
-- 
2.51.0


From 7b19b0fc8ac8466b9140df17e0c7fcf135f1f063 Mon Sep 17 00:00:00 2001
From: Tarang Raval <tarang.raval@siliconsignals.io>
Date: Sat, 29 Mar 2025 11:13:24 +0530
Subject: [PATCH 09/16] media: i2c: imx334: Convert to CCI register access
 helpers

Use the new common CCI register access helpers to replace the private
register access helpers in the imx334 driver. This simplifies the driver
by reducing the amount of code.

Acked-by: Shravan Chippa <Shravan.Chippa@microchip.com>
Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/Kconfig  |   1 +
 drivers/media/i2c/imx334.c | 746 ++++++++++++++++---------------------
 2 files changed, 319 insertions(+), 428 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 7b8af1c87a0e..205360c3ae52 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -217,6 +217,7 @@ config VIDEO_IMX319
 config VIDEO_IMX334
 	tristate "Sony IMX334 sensor support"
 	depends on OF_GPIO
+	select V4L2_CCI_I2C
 	help
 	  This is a Video4Linux2 sensor driver for the Sony
 	  IMX334 camera.
diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index ad0b03a3f573..0785bf213d91 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -12,77 +12,123 @@
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 
+#include <media/v4l2-cci.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
 /* Streaming Mode */
-#define IMX334_REG_MODE_SELECT	0x3000
-#define IMX334_MODE_STANDBY	0x01
-#define IMX334_MODE_STREAMING	0x00
+#define IMX334_REG_MODE_SELECT		CCI_REG8(0x3000)
+#define IMX334_MODE_STANDBY		0x01
+#define IMX334_MODE_STREAMING		0x00
 
 /* Lines per frame */
-#define IMX334_REG_LPFR		0x3030
+#define IMX334_REG_VMAX			CCI_REG24_LE(0x3030)
+
+#define IMX334_REG_HMAX			CCI_REG16_LE(0x3034)
+
+#define IMX334_REG_OPB_SIZE_V		CCI_REG8(0x304c)
+#define IMX334_REG_ADBIT		CCI_REG8(0x3050)
+#define IMX334_REG_MDBIT		CCI_REG8(0x319d)
+#define IMX334_REG_ADBIT1		CCI_REG16_LE(0x341c)
+#define IMX334_REG_Y_OUT_SIZE		CCI_REG16_LE(0x3308)
+#define IMX334_REG_XVS_XHS_OUTSEL	CCI_REG8(0x31a0)
+#define IMX334_REG_XVS_XHS_DRV		CCI_REG8(0x31a1)
 
 /* Chip ID */
-#define IMX334_REG_ID		0x3044
-#define IMX334_ID		0x1e
+#define IMX334_REG_ID			CCI_REG8(0x3044)
+#define IMX334_ID			0x1e
 
 /* Exposure control */
-#define IMX334_REG_SHUTTER	0x3058
-#define IMX334_EXPOSURE_MIN	1
-#define IMX334_EXPOSURE_OFFSET	5
-#define IMX334_EXPOSURE_STEP	1
-#define IMX334_EXPOSURE_DEFAULT	0x0648
+#define IMX334_REG_SHUTTER		CCI_REG24_LE(0x3058)
+#define IMX334_EXPOSURE_MIN		1
+#define IMX334_EXPOSURE_OFFSET		5
+#define IMX334_EXPOSURE_STEP		1
+#define IMX334_EXPOSURE_DEFAULT		0x0648
+
+#define IMX334_REG_LANEMODE		CCI_REG8(0x3a01)
+
+/* Window cropping Settings */
+#define IMX334_REG_AREA3_ST_ADR_1	CCI_REG16_LE(0x3074)
+#define IMX334_REG_AREA3_ST_ADR_2	CCI_REG16_LE(0x308e)
+#define IMX334_REG_UNREAD_PARAM5	CCI_REG16_LE(0x30b6)
+#define IMX334_REG_AREA3_WIDTH_1	CCI_REG16_LE(0x3076)
+#define IMX334_REG_AREA3_WIDTH_2	CCI_REG16_LE(0x3090)
+#define IMX334_REG_BLACK_OFSET_ADR	CCI_REG16_LE(0x30c6)
+#define IMX334_REG_UNRD_LINE_MAX	CCI_REG16_LE(0x30ce)
+#define IMX334_REG_UNREAD_ED_ADR	CCI_REG16_LE(0x30d8)
+#define IMX334_REG_UNREAD_PARAM6	CCI_REG16_LE(0x3116)
+
+#define IMX334_REG_VREVERSE		CCI_REG8(0x304f)
+#define IMX334_REG_HREVERSE		CCI_REG8(0x304e)
+
+/* Binning Settings */
+#define IMX334_REG_HADD_VADD		CCI_REG8(0x3199)
+#define IMX334_REG_VALID_EXPAND		CCI_REG8(0x31dd)
+#define IMX334_REG_TCYCLE		CCI_REG8(0x3300)
 
 /* Analog gain control */
-#define IMX334_REG_AGAIN	0x30e8
-#define IMX334_AGAIN_MIN	0
-#define IMX334_AGAIN_MAX	240
-#define IMX334_AGAIN_STEP	1
-#define IMX334_AGAIN_DEFAULT	0
+#define IMX334_REG_AGAIN		CCI_REG16_LE(0x30e8)
+#define IMX334_AGAIN_MIN		0
+#define IMX334_AGAIN_MAX		240
+#define IMX334_AGAIN_STEP		1
+#define IMX334_AGAIN_DEFAULT		0
 
 /* Group hold register */
-#define IMX334_REG_HOLD		0x3001
+#define IMX334_REG_HOLD			CCI_REG8(0x3001)
+
+#define IMX334_REG_MASTER_MODE		CCI_REG8(0x3002)
+#define IMX334_REG_WINMODE		CCI_REG8(0x3018)
+#define IMX334_REG_HTRIMMING_START	CCI_REG16_LE(0x302c)
+#define IMX334_REG_HNUM			CCI_REG16_LE(0x302e)
 
 /* Input clock rate */
-#define IMX334_INCLK_RATE	24000000
+#define IMX334_INCLK_RATE		24000000
+
+/* INCK Setting Register */
+#define IMX334_REG_BCWAIT_TIME		CCI_REG8(0x300c)
+#define IMX334_REG_CPWAIT_TIME		CCI_REG8(0x300d)
+#define IMX334_REG_INCKSEL1		CCI_REG16_LE(0x314c)
+#define IMX334_REG_INCKSEL2		CCI_REG8(0x315a)
+#define IMX334_REG_INCKSEL3		CCI_REG8(0x3168)
+#define IMX334_REG_INCKSEL4		CCI_REG8(0x316a)
+#define IMX334_REG_SYS_MODE		CCI_REG8(0x319e)
+
+#define IMX334_REG_TCLKPOST		CCI_REG16_LE(0x3a18)
+#define IMX334_REG_TCLKPREPARE		CCI_REG16_LE(0x3a1a)
+#define IMX334_REG_TCLKTRAIL		CCI_REG16_LE(0x3a1c)
+#define IMX334_REG_TCLKZERO		CCI_REG16_LE(0x3a1e)
+#define IMX334_REG_THSPREPARE		CCI_REG16_LE(0x3a20)
+#define IMX334_REG_THSZERO		CCI_REG16_LE(0x3a22)
+#define IMX334_REG_THSTRAIL		CCI_REG16_LE(0x3a24)
+#define IMX334_REG_THSEXIT		CCI_REG16_LE(0x3a26)
+#define IMX334_REG_TPLX			CCI_REG16_LE(0x3a28)
 
 /* CSI2 HW configuration */
-#define IMX334_LINK_FREQ_891M	891000000
-#define IMX334_LINK_FREQ_445M	445500000
-#define IMX334_NUM_DATA_LANES	4
+#define IMX334_LINK_FREQ_891M		891000000
+#define IMX334_LINK_FREQ_445M		445500000
+#define IMX334_NUM_DATA_LANES		4
 
-#define IMX334_REG_MIN		0x00
-#define IMX334_REG_MAX		0xfffff
+#define IMX334_REG_MIN			0x00
+#define IMX334_REG_MAX			0xfffff
 
 /* Test Pattern Control */
-#define IMX334_REG_TP		0x329e
-#define IMX334_TP_COLOR_HBARS	0xA
-#define IMX334_TP_COLOR_VBARS	0xB
+#define IMX334_REG_TP			CCI_REG8(0x329e)
+#define IMX334_TP_COLOR_HBARS		0xa
+#define IMX334_TP_COLOR_VBARS		0xb
 
-#define IMX334_TPG_EN_DOUT	0x329c
-#define IMX334_TP_ENABLE	0x1
-#define IMX334_TP_DISABLE	0x0
+#define IMX334_TPG_EN_DOUT		CCI_REG8(0x329c)
+#define IMX334_TP_ENABLE		0x1
+#define IMX334_TP_DISABLE		0x0
 
-#define IMX334_TPG_COLORW	0x32a0
-#define IMX334_TPG_COLORW_120P	0x13
+#define IMX334_TPG_COLORW		CCI_REG8(0x32a0)
+#define IMX334_TPG_COLORW_120P		0x13
 
-#define IMX334_TP_CLK_EN	0x3148
-#define IMX334_TP_CLK_EN_VAL	0x10
-#define IMX334_TP_CLK_DIS_VAL	0x0
+#define IMX334_TP_CLK_EN		CCI_REG8(0x3148)
+#define IMX334_TP_CLK_EN_VAL		0x10
+#define IMX334_TP_CLK_DIS_VAL		0x0
 
-#define IMX334_DIG_CLP_MODE	0x3280
-
-/**
- * struct imx334_reg - imx334 sensor register
- * @address: Register address
- * @val: Register value
- */
-struct imx334_reg {
-	u16 address;
-	u8 val;
-};
+#define IMX334_DIG_CLP_MODE		CCI_REG8(0x3280)
 
 /**
  * struct imx334_reg_list - imx334 sensor register list
@@ -91,7 +137,7 @@ struct imx334_reg {
  */
 struct imx334_reg_list {
 	u32 num_of_regs;
-	const struct imx334_reg *regs;
+	const struct cci_reg_sequence *regs;
 };
 
 /**
@@ -121,6 +167,7 @@ struct imx334_mode {
 /**
  * struct imx334 - imx334 sensor device structure
  * @dev: Pointer to generic device
+ * @cci: CCI register map
  * @client: Pointer to i2c client
  * @sd: V4L2 sub-device
  * @pad: Media pad. Only one pad supported
@@ -141,6 +188,7 @@ struct imx334_mode {
  */
 struct imx334 {
 	struct device *dev;
+	struct regmap *cci;
 	struct i2c_client *client;
 	struct v4l2_subdev sd;
 	struct media_pad pad;
@@ -168,250 +216,191 @@ static const s64 link_freq[] = {
 };
 
 /* Sensor common mode registers values */
-static const struct imx334_reg common_mode_regs[] = {
-	{0x3000, 0x01},
-	{0x3018, 0x04},
-	{0x3030, 0xca},
-	{0x3031, 0x08},
-	{0x3032, 0x00},
-	{0x3034, 0x4c},
-	{0x3035, 0x04},
-	{0x30c6, 0x00},
-	{0x30c7, 0x00},
-	{0x30ce, 0x00},
-	{0x30cf, 0x00},
-	{0x304c, 0x00},
-	{0x304e, 0x00},
-	{0x304f, 0x00},
-	{0x3050, 0x00},
-	{0x30b6, 0x00},
-	{0x30b7, 0x00},
-	{0x3116, 0x08},
-	{0x3117, 0x00},
-	{0x31a0, 0x20},
-	{0x31a1, 0x0f},
-	{0x300c, 0x3b},
-	{0x300d, 0x2a},
-	{0x314c, 0x29},
-	{0x314d, 0x01},
-	{0x315a, 0x06},
-	{0x3168, 0xa0},
-	{0x316a, 0x7e},
-	{0x319e, 0x02},
-	{0x3199, 0x00},
-	{0x319d, 0x00},
-	{0x31dd, 0x03},
-	{0x3300, 0x00},
-	{0x341c, 0xff},
-	{0x341d, 0x01},
-	{0x3a01, 0x03},
-	{0x3a18, 0x7f},
-	{0x3a19, 0x00},
-	{0x3a1a, 0x37},
-	{0x3a1b, 0x00},
-	{0x3a1c, 0x37},
-	{0x3a1d, 0x00},
-	{0x3a1e, 0xf7},
-	{0x3a1f, 0x00},
-	{0x3a20, 0x3f},
-	{0x3a21, 0x00},
-	{0x3a20, 0x6f},
-	{0x3a21, 0x00},
-	{0x3a20, 0x3f},
-	{0x3a21, 0x00},
-	{0x3a20, 0x5f},
-	{0x3a21, 0x00},
-	{0x3a20, 0x2f},
-	{0x3a21, 0x00},
-	{0x3078, 0x02},
-	{0x3079, 0x00},
-	{0x307a, 0x00},
-	{0x307b, 0x00},
-	{0x3080, 0x02},
-	{0x3081, 0x00},
-	{0x3082, 0x00},
-	{0x3083, 0x00},
-	{0x3088, 0x02},
-	{0x3094, 0x00},
-	{0x3095, 0x00},
-	{0x3096, 0x00},
-	{0x309b, 0x02},
-	{0x309c, 0x00},
-	{0x309d, 0x00},
-	{0x309e, 0x00},
-	{0x30a4, 0x00},
-	{0x30a5, 0x00},
-	{0x3288, 0x21},
-	{0x328a, 0x02},
-	{0x3414, 0x05},
-	{0x3416, 0x18},
-	{0x35Ac, 0x0e},
-	{0x3648, 0x01},
-	{0x364a, 0x04},
-	{0x364c, 0x04},
-	{0x3678, 0x01},
-	{0x367c, 0x31},
-	{0x367e, 0x31},
-	{0x3708, 0x02},
-	{0x3714, 0x01},
-	{0x3715, 0x02},
-	{0x3716, 0x02},
-	{0x3717, 0x02},
-	{0x371c, 0x3d},
-	{0x371d, 0x3f},
-	{0x372c, 0x00},
-	{0x372d, 0x00},
-	{0x372e, 0x46},
-	{0x372f, 0x00},
-	{0x3730, 0x89},
-	{0x3731, 0x00},
-	{0x3732, 0x08},
-	{0x3733, 0x01},
-	{0x3734, 0xfe},
-	{0x3735, 0x05},
-	{0x375d, 0x00},
-	{0x375e, 0x00},
-	{0x375f, 0x61},
-	{0x3760, 0x06},
-	{0x3768, 0x1b},
-	{0x3769, 0x1b},
-	{0x376a, 0x1a},
-	{0x376b, 0x19},
-	{0x376c, 0x18},
-	{0x376d, 0x14},
-	{0x376e, 0x0f},
-	{0x3776, 0x00},
-	{0x3777, 0x00},
-	{0x3778, 0x46},
-	{0x3779, 0x00},
-	{0x377a, 0x08},
-	{0x377b, 0x01},
-	{0x377c, 0x45},
-	{0x377d, 0x01},
-	{0x377e, 0x23},
-	{0x377f, 0x02},
-	{0x3780, 0xd9},
-	{0x3781, 0x03},
-	{0x3782, 0xf5},
-	{0x3783, 0x06},
-	{0x3784, 0xa5},
-	{0x3788, 0x0f},
-	{0x378a, 0xd9},
-	{0x378b, 0x03},
-	{0x378c, 0xeb},
-	{0x378d, 0x05},
-	{0x378e, 0x87},
-	{0x378f, 0x06},
-	{0x3790, 0xf5},
-	{0x3792, 0x43},
-	{0x3794, 0x7a},
-	{0x3796, 0xa1},
-	{0x37b0, 0x37},
-	{0x3e04, 0x0e},
-	{0x30e8, 0x50},
-	{0x30e9, 0x00},
-	{0x3e04, 0x0e},
-	{0x3002, 0x00},
+static const struct cci_reg_sequence common_mode_regs[] = {
+	{ IMX334_REG_MODE_SELECT, IMX334_MODE_STANDBY },
+	{ IMX334_REG_WINMODE,		0x04 },
+	{ IMX334_REG_VMAX,		0x0008ca },
+	{ IMX334_REG_HMAX,		0x044c },
+	{ IMX334_REG_BLACK_OFSET_ADR,	0x0000 },
+	{ IMX334_REG_UNRD_LINE_MAX,	0x0000 },
+	{ IMX334_REG_OPB_SIZE_V,	0x00 },
+	{ IMX334_REG_HREVERSE,		0x00 },
+	{ IMX334_REG_VREVERSE,		0x00 },
+	{ IMX334_REG_ADBIT,		0x00 },
+	{ IMX334_REG_UNREAD_PARAM5,	0x0000 },
+	{ IMX334_REG_UNREAD_PARAM6,	0x0008 },
+	{ IMX334_REG_XVS_XHS_OUTSEL,	0x20 },
+	{ IMX334_REG_XVS_XHS_DRV,	0x0f },
+	{ IMX334_REG_BCWAIT_TIME,	0x3b },
+	{ IMX334_REG_CPWAIT_TIME,	0x2a },
+	{ IMX334_REG_INCKSEL1,		0x0129 },
+	{ IMX334_REG_INCKSEL2,		0x06 },
+	{ IMX334_REG_INCKSEL3,		0xa0 },
+	{ IMX334_REG_INCKSEL4,		0x7e },
+	{ IMX334_REG_SYS_MODE,		0x02 },
+	{ IMX334_REG_HADD_VADD,		0x00 },
+	{ IMX334_REG_MDBIT,		0x00 },
+	{ IMX334_REG_VALID_EXPAND,	0x03 },
+	{ IMX334_REG_TCYCLE,		0x00 },
+	{ IMX334_REG_ADBIT1,		0x01ff },
+	{ IMX334_REG_LANEMODE,		0x03 },
+	{ IMX334_REG_TCLKPOST,		0x007f },
+	{ IMX334_REG_TCLKPREPARE,	0x0037 },
+	{ IMX334_REG_TCLKTRAIL,		0x0037 },
+	{ IMX334_REG_TCLKZERO,		0xf7 },
+	{ IMX334_REG_THSPREPARE,	0x003f },
+	{ IMX334_REG_THSPREPARE,	0x006f },
+	{ IMX334_REG_THSPREPARE,	0x003f },
+	{ IMX334_REG_THSPREPARE,	0x005f },
+	{ IMX334_REG_THSPREPARE,	0x002f },
+	{ CCI_REG8(0x3078), 0x02 },
+	{ CCI_REG8(0x3079), 0x00 },
+	{ CCI_REG8(0x307a), 0x00 },
+	{ CCI_REG8(0x307b), 0x00 },
+	{ CCI_REG8(0x3080), 0x02 },
+	{ CCI_REG8(0x3081), 0x00 },
+	{ CCI_REG8(0x3082), 0x00 },
+	{ CCI_REG8(0x3083), 0x00 },
+	{ CCI_REG8(0x3088), 0x02 },
+	{ CCI_REG8(0x3094), 0x00 },
+	{ CCI_REG8(0x3095), 0x00 },
+	{ CCI_REG8(0x3096), 0x00 },
+	{ CCI_REG8(0x309b), 0x02 },
+	{ CCI_REG8(0x309c), 0x00 },
+	{ CCI_REG8(0x309d), 0x00 },
+	{ CCI_REG8(0x309e), 0x00 },
+	{ CCI_REG8(0x30a4), 0x00 },
+	{ CCI_REG8(0x30a5), 0x00 },
+	{ CCI_REG8(0x3288), 0x21 },
+	{ CCI_REG8(0x328a), 0x02 },
+	{ CCI_REG8(0x3414), 0x05 },
+	{ CCI_REG8(0x3416), 0x18 },
+	{ CCI_REG8(0x35Ac), 0x0e },
+	{ CCI_REG8(0x3648), 0x01 },
+	{ CCI_REG8(0x364a), 0x04 },
+	{ CCI_REG8(0x364c), 0x04 },
+	{ CCI_REG8(0x3678), 0x01 },
+	{ CCI_REG8(0x367c), 0x31 },
+	{ CCI_REG8(0x367e), 0x31 },
+	{ CCI_REG8(0x3708), 0x02 },
+	{ CCI_REG8(0x3714), 0x01 },
+	{ CCI_REG8(0x3715), 0x02 },
+	{ CCI_REG8(0x3716), 0x02 },
+	{ CCI_REG8(0x3717), 0x02 },
+	{ CCI_REG8(0x371c), 0x3d },
+	{ CCI_REG8(0x371d), 0x3f },
+	{ CCI_REG8(0x372c), 0x00 },
+	{ CCI_REG8(0x372d), 0x00 },
+	{ CCI_REG8(0x372e), 0x46 },
+	{ CCI_REG8(0x372f), 0x00 },
+	{ CCI_REG8(0x3730), 0x89 },
+	{ CCI_REG8(0x3731), 0x00 },
+	{ CCI_REG8(0x3732), 0x08 },
+	{ CCI_REG8(0x3733), 0x01 },
+	{ CCI_REG8(0x3734), 0xfe },
+	{ CCI_REG8(0x3735), 0x05 },
+	{ CCI_REG8(0x375d), 0x00 },
+	{ CCI_REG8(0x375e), 0x00 },
+	{ CCI_REG8(0x375f), 0x61 },
+	{ CCI_REG8(0x3760), 0x06 },
+	{ CCI_REG8(0x3768), 0x1b },
+	{ CCI_REG8(0x3769), 0x1b },
+	{ CCI_REG8(0x376a), 0x1a },
+	{ CCI_REG8(0x376b), 0x19 },
+	{ CCI_REG8(0x376c), 0x18 },
+	{ CCI_REG8(0x376d), 0x14 },
+	{ CCI_REG8(0x376e), 0x0f },
+	{ CCI_REG8(0x3776), 0x00 },
+	{ CCI_REG8(0x3777), 0x00 },
+	{ CCI_REG8(0x3778), 0x46 },
+	{ CCI_REG8(0x3779), 0x00 },
+	{ CCI_REG8(0x377a), 0x08 },
+	{ CCI_REG8(0x377b), 0x01 },
+	{ CCI_REG8(0x377c), 0x45 },
+	{ CCI_REG8(0x377d), 0x01 },
+	{ CCI_REG8(0x377e), 0x23 },
+	{ CCI_REG8(0x377f), 0x02 },
+	{ CCI_REG8(0x3780), 0xd9 },
+	{ CCI_REG8(0x3781), 0x03 },
+	{ CCI_REG8(0x3782), 0xf5 },
+	{ CCI_REG8(0x3783), 0x06 },
+	{ CCI_REG8(0x3784), 0xa5 },
+	{ CCI_REG8(0x3788), 0x0f },
+	{ CCI_REG8(0x378a), 0xd9 },
+	{ CCI_REG8(0x378b), 0x03 },
+	{ CCI_REG8(0x378c), 0xeb },
+	{ CCI_REG8(0x378d), 0x05 },
+	{ CCI_REG8(0x378e), 0x87 },
+	{ CCI_REG8(0x378f), 0x06 },
+	{ CCI_REG8(0x3790), 0xf5 },
+	{ CCI_REG8(0x3792), 0x43 },
+	{ CCI_REG8(0x3794), 0x7a },
+	{ CCI_REG8(0x3796), 0xa1 },
+	{ CCI_REG8(0x37b0), 0x37 },
+	{ CCI_REG8(0x3e04), 0x0e },
+	{ IMX334_REG_AGAIN, 0x0050 },
+	{ CCI_REG8(0x3e04), 0x0e },
+	{ IMX334_REG_MASTER_MODE, 0x00 },
 };
 
 /* Sensor mode registers for 640x480@30fps */
-static const struct imx334_reg mode_640x480_regs[] = {
-	{0x302c, 0x70},
-	{0x302d, 0x06},
-	{0x302e, 0x80},
-	{0x302f, 0x02},
-	{0x3074, 0x48},
-	{0x3075, 0x07},
-	{0x308e, 0x49},
-	{0x308f, 0x07},
-	{0x3076, 0xe0},
-	{0x3077, 0x01},
-	{0x3090, 0xe0},
-	{0x3091, 0x01},
-	{0x3308, 0xe0},
-	{0x3309, 0x01},
-	{0x30d8, 0x30},
-	{0x30d9, 0x0b},
+static const struct cci_reg_sequence mode_640x480_regs[] = {
+	{ IMX334_REG_HTRIMMING_START,	0x0670 },
+	{ IMX334_REG_HNUM,		0x0280 },
+	{ IMX334_REG_AREA3_ST_ADR_1,	0x0748 },
+	{ IMX334_REG_AREA3_ST_ADR_2,	0x0749 },
+	{ IMX334_REG_AREA3_WIDTH_1,	0x01e0 },
+	{ IMX334_REG_AREA3_WIDTH_2,	0x01e0 },
+	{ IMX334_REG_Y_OUT_SIZE,	0x01e0 },
+	{ IMX334_REG_UNREAD_ED_ADR,	0x0b30 },
 };
 
 /* Sensor mode registers for 1280x720@30fps */
-static const struct imx334_reg mode_1280x720_regs[] = {
-	{0x302c, 0x30},
-	{0x302d, 0x05},
-	{0x302e, 0x00},
-	{0x302f, 0x05},
-	{0x3074, 0x84},
-	{0x3075, 0x03},
-	{0x308e, 0x85},
-	{0x308f, 0x03},
-	{0x3076, 0xd0},
-	{0x3077, 0x02},
-	{0x3090, 0xd0},
-	{0x3091, 0x02},
-	{0x3308, 0xd0},
-	{0x3309, 0x02},
-	{0x30d8, 0x30},
-	{0x30d9, 0x0b},
+static const struct cci_reg_sequence mode_1280x720_regs[] = {
+	{ IMX334_REG_HTRIMMING_START,	0x0530 },
+	{ IMX334_REG_HNUM,		0x0500 },
+	{ IMX334_REG_AREA3_ST_ADR_1,	0x0384 },
+	{ IMX334_REG_AREA3_ST_ADR_2,	0x0385 },
+	{ IMX334_REG_AREA3_WIDTH_1,	0x02d0 },
+	{ IMX334_REG_AREA3_WIDTH_2,	0x02d0 },
+	{ IMX334_REG_Y_OUT_SIZE,	0x02d0 },
+	{ IMX334_REG_UNREAD_ED_ADR,	0x0b30 },
 };
 
 /* Sensor mode registers for 1920x1080@30fps */
-static const struct imx334_reg mode_1920x1080_regs[] = {
-	{0x302c, 0xf0},
-	{0x302d, 0x03},
-	{0x302e, 0x80},
-	{0x302f, 0x07},
-	{0x3074, 0xcc},
-	{0x3075, 0x02},
-	{0x308e, 0xcd},
-	{0x308f, 0x02},
-	{0x3076, 0x38},
-	{0x3077, 0x04},
-	{0x3090, 0x38},
-	{0x3091, 0x04},
-	{0x3308, 0x38},
-	{0x3309, 0x04},
-	{0x30d8, 0x18},
-	{0x30d9, 0x0a},
+static const struct cci_reg_sequence mode_1920x1080_regs[] = {
+	{ IMX334_REG_HTRIMMING_START,	0x03f0 },
+	{ IMX334_REG_HNUM,		0x0780 },
+	{ IMX334_REG_AREA3_ST_ADR_1,	0x02cc },
+	{ IMX334_REG_AREA3_ST_ADR_2,	0x02cd },
+	{ IMX334_REG_AREA3_WIDTH_1,	0x0438 },
+	{ IMX334_REG_AREA3_WIDTH_2,	0x0438 },
+	{ IMX334_REG_Y_OUT_SIZE,	0x0438 },
+	{ IMX334_REG_UNREAD_ED_ADR,	0x0a18 },
 };
 
 /* Sensor mode registers for 3840x2160@30fps */
-static const struct imx334_reg mode_3840x2160_regs[] = {
-	{0x3034, 0x26},
-	{0x3035, 0x02},
-	{0x315a, 0x02},
-	{0x302c, 0x3c},
-	{0x302d, 0x00},
-	{0x302e, 0x00},
-	{0x302f, 0x0f},
-	{0x3074, 0xb0},
-	{0x3075, 0x00},
-	{0x308e, 0xb1},
-	{0x308f, 0x00},
-	{0x30d8, 0x20},
-	{0x30d9, 0x12},
-	{0x3076, 0x70},
-	{0x3077, 0x08},
-	{0x3090, 0x70},
-	{0x3091, 0x08},
-	{0x3308, 0x70},
-	{0x3309, 0x08},
-	{0x319e, 0x00},
-	{0x3a00, 0x01},
-	{0x3a18, 0xbf},
-	{0x3a1a, 0x67},
-	{0x3a1c, 0x6f},
-	{0x3a1e, 0xd7},
-	{0x3a1f, 0x01},
-	{0x3a20, 0x6f},
-	{0x3a21, 0x00},
-	{0x3a22, 0xcf},
-	{0x3a23, 0x00},
-	{0x3a24, 0x6f},
-	{0x3a25, 0x00},
-	{0x3a26, 0xb7},
-	{0x3a27, 0x00},
-	{0x3a28, 0x5f},
-	{0x3a29, 0x00},
+static const struct cci_reg_sequence mode_3840x2160_regs[] = {
+	{ IMX334_REG_HMAX,		0x0226 },
+	{ IMX334_REG_INCKSEL2,		0x02 },
+	{ IMX334_REG_HTRIMMING_START,	0x003c },
+	{ IMX334_REG_HNUM,		0x0f00 },
+	{ IMX334_REG_AREA3_ST_ADR_1,	0x00b0 },
+	{ IMX334_REG_AREA3_ST_ADR_2,	0x00b1 },
+	{ IMX334_REG_UNREAD_ED_ADR,	0x1220 },
+	{ IMX334_REG_AREA3_WIDTH_1,	0x0870 },
+	{ IMX334_REG_AREA3_WIDTH_2,	0x0870 },
+	{ IMX334_REG_Y_OUT_SIZE,	0x0870 },
+	{ IMX334_REG_SYS_MODE,		0x0100 },
+	{ IMX334_REG_TCLKPOST,		0x00bf },
+	{ IMX334_REG_TCLKPREPARE,	0x0067 },
+	{ IMX334_REG_TCLKTRAIL,		0x006f },
+	{ IMX334_REG_TCLKZERO,		0x1d7 },
+	{ IMX334_REG_THSPREPARE,	0x006f },
+	{ IMX334_REG_THSZERO,		0x00cf },
+	{ IMX334_REG_THSTRAIL,		0x006f },
+	{ IMX334_REG_THSEXIT,		0x00b7 },
+	{ IMX334_REG_TPLX,		0x005f },
 };
 
 static const char * const imx334_test_pattern_menu[] = {
@@ -426,18 +415,16 @@ static const int imx334_test_pattern_val[] = {
 	IMX334_TP_COLOR_VBARS,
 };
 
-static const struct imx334_reg raw10_framefmt_regs[] = {
-	{0x3050, 0x00},
-	{0x319d, 0x00},
-	{0x341c, 0xff},
-	{0x341d, 0x01},
+static const struct cci_reg_sequence raw10_framefmt_regs[] = {
+	{ IMX334_REG_ADBIT,  0x00 },
+	{ IMX334_REG_MDBIT,  0x00 },
+	{ IMX334_REG_ADBIT1, 0x01ff },
 };
 
-static const struct imx334_reg raw12_framefmt_regs[] = {
-	{0x3050, 0x01},
-	{0x319d, 0x01},
-	{0x341c, 0x47},
-	{0x341d, 0x00},
+static const struct cci_reg_sequence raw12_framefmt_regs[] = {
+	{ IMX334_REG_ADBIT,  0x01 },
+	{ IMX334_REG_MDBIT,  0x01 },
+	{ IMX334_REG_ADBIT1, 0x0047 },
 };
 
 static const u32 imx334_mbus_codes[] = {
@@ -513,101 +500,6 @@ static inline struct imx334 *to_imx334(struct v4l2_subdev *subdev)
 	return container_of(subdev, struct imx334, sd);
 }
 
-/**
- * imx334_read_reg() - Read registers.
- * @imx334: pointer to imx334 device
- * @reg: register address
- * @len: length of bytes to read. Max supported bytes is 4
- * @val: pointer to register value to be filled.
- *
- * Big endian register addresses with little endian values.
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int imx334_read_reg(struct imx334 *imx334, u16 reg, u32 len, u32 *val)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(&imx334->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;
-
-	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
-	if (ret != ARRAY_SIZE(msgs))
-		return -EIO;
-
-	*val = get_unaligned_le32(data_buf);
-
-	return 0;
-}
-
-/**
- * imx334_write_reg() - Write register
- * @imx334: pointer to imx334 device
- * @reg: register address
- * @len: length of bytes. Max supported bytes is 4
- * @val: register value
- *
- * Big endian register addresses with little endian values.
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int imx334_write_reg(struct imx334 *imx334, u16 reg, u32 len, u32 val)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(&imx334->sd);
-	u8 buf[6] = {0};
-
-	if (WARN_ON(len > 4))
-		return -EINVAL;
-
-	put_unaligned_be16(reg, buf);
-	put_unaligned_le32(val, buf + 2);
-	if (i2c_master_send(client, buf, len + 2) != len + 2)
-		return -EIO;
-
-	return 0;
-}
-
-/**
- * imx334_write_regs() - Write a list of registers
- * @imx334: pointer to imx334 device
- * @regs: list of registers to be written
- * @len: length of registers array
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int imx334_write_regs(struct imx334 *imx334,
-			     const struct imx334_reg *regs, u32 len)
-{
-	unsigned int i;
-	int ret;
-
-	for (i = 0; i < len; i++) {
-		ret = imx334_write_reg(imx334, regs[i].address, 1, regs[i].val);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
 /**
  * imx334_update_controls() - Update control ranges based on streaming mode
  * @imx334: pointer to imx334 device
@@ -653,7 +545,7 @@ static int imx334_update_controls(struct imx334 *imx334,
 static int imx334_update_exp_gain(struct imx334 *imx334, u32 exposure, u32 gain)
 {
 	u32 lpfr, shutter;
-	int ret;
+	int ret, ret_hold;
 
 	lpfr = imx334->vblank + imx334->cur_mode->height;
 	shutter = lpfr - exposure;
@@ -661,22 +553,14 @@ static int imx334_update_exp_gain(struct imx334 *imx334, u32 exposure, u32 gain)
 	dev_dbg(imx334->dev, "Set long exp %u analog gain %u sh0 %u lpfr %u\n",
 		exposure, gain, shutter, lpfr);
 
-	ret = imx334_write_reg(imx334, IMX334_REG_HOLD, 1, 1);
-	if (ret)
-		return ret;
-
-	ret = imx334_write_reg(imx334, IMX334_REG_LPFR, 3, lpfr);
-	if (ret)
-		goto error_release_group_hold;
-
-	ret = imx334_write_reg(imx334, IMX334_REG_SHUTTER, 3, shutter);
-	if (ret)
-		goto error_release_group_hold;
-
-	ret = imx334_write_reg(imx334, IMX334_REG_AGAIN, 1, gain);
+	cci_write(imx334->cci, IMX334_REG_HOLD, 1, &ret);
+	cci_write(imx334->cci, IMX334_REG_VMAX, lpfr, &ret);
+	cci_write(imx334->cci, IMX334_REG_SHUTTER, shutter, &ret);
+	cci_write(imx334->cci, IMX334_REG_AGAIN, gain, &ret);
 
-error_release_group_hold:
-	imx334_write_reg(imx334, IMX334_REG_HOLD, 1, 0);
+	ret_hold = cci_write(imx334->cci, IMX334_REG_HOLD, 0, NULL);
+	if (ret_hold)
+		return ret_hold;
 
 	return ret;
 }
@@ -740,21 +624,21 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	case V4L2_CID_TEST_PATTERN:
 		if (ctrl->val) {
-			imx334_write_reg(imx334, IMX334_TP_CLK_EN, 1,
-					 IMX334_TP_CLK_EN_VAL);
-			imx334_write_reg(imx334, IMX334_DIG_CLP_MODE, 1, 0x0);
-			imx334_write_reg(imx334, IMX334_TPG_COLORW, 1,
-					 IMX334_TPG_COLORW_120P);
-			imx334_write_reg(imx334, IMX334_REG_TP, 1,
-					 imx334_test_pattern_val[ctrl->val]);
-			imx334_write_reg(imx334, IMX334_TPG_EN_DOUT, 1,
-					 IMX334_TP_ENABLE);
+			cci_write(imx334->cci, IMX334_TP_CLK_EN,
+				  IMX334_TP_CLK_EN_VAL, NULL);
+			cci_write(imx334->cci, IMX334_DIG_CLP_MODE, 0x0, NULL);
+			cci_write(imx334->cci, IMX334_TPG_COLORW,
+				  IMX334_TPG_COLORW_120P, NULL);
+			cci_write(imx334->cci, IMX334_REG_TP,
+				  imx334_test_pattern_val[ctrl->val], NULL);
+			cci_write(imx334->cci, IMX334_TPG_EN_DOUT,
+				  IMX334_TP_ENABLE, NULL);
 		} else {
-			imx334_write_reg(imx334, IMX334_DIG_CLP_MODE, 1, 0x1);
-			imx334_write_reg(imx334, IMX334_TP_CLK_EN, 1,
-					 IMX334_TP_CLK_DIS_VAL);
-			imx334_write_reg(imx334, IMX334_TPG_EN_DOUT, 1,
-					 IMX334_TP_DISABLE);
+			cci_write(imx334->cci, IMX334_DIG_CLP_MODE, 0x1, NULL);
+			cci_write(imx334->cci, IMX334_TP_CLK_EN,
+				  IMX334_TP_CLK_DIS_VAL, NULL);
+			cci_write(imx334->cci, IMX334_TPG_EN_DOUT,
+				  IMX334_TP_DISABLE, NULL);
 		}
 		ret = 0;
 		break;
@@ -961,12 +845,13 @@ static int imx334_set_framefmt(struct imx334 *imx334)
 {
 	switch (imx334->cur_code) {
 	case MEDIA_BUS_FMT_SRGGB10_1X10:
-		return imx334_write_regs(imx334, raw10_framefmt_regs,
-					 ARRAY_SIZE(raw10_framefmt_regs));
+		return cci_multi_reg_write(imx334->cci, raw10_framefmt_regs,
+					ARRAY_SIZE(raw10_framefmt_regs), NULL);
+
 
 	case MEDIA_BUS_FMT_SRGGB12_1X12:
-		return imx334_write_regs(imx334, raw12_framefmt_regs,
-					 ARRAY_SIZE(raw12_framefmt_regs));
+		return cci_multi_reg_write(imx334->cci, raw12_framefmt_regs,
+					ARRAY_SIZE(raw12_framefmt_regs), NULL);
 	}
 
 	return -EINVAL;
@@ -983,8 +868,8 @@ static int imx334_start_streaming(struct imx334 *imx334)
 	const struct imx334_reg_list *reg_list;
 	int ret;
 
-	ret = imx334_write_regs(imx334, common_mode_regs,
-				ARRAY_SIZE(common_mode_regs));
+	ret = cci_multi_reg_write(imx334->cci, common_mode_regs,
+				  ARRAY_SIZE(common_mode_regs), NULL);
 	if (ret) {
 		dev_err(imx334->dev, "fail to write common registers\n");
 		return ret;
@@ -992,8 +877,8 @@ static int imx334_start_streaming(struct imx334 *imx334)
 
 	/* Write sensor mode registers */
 	reg_list = &imx334->cur_mode->reg_list;
-	ret = imx334_write_regs(imx334, reg_list->regs,
-				reg_list->num_of_regs);
+	ret = cci_multi_reg_write(imx334->cci, reg_list->regs,
+				  reg_list->num_of_regs, NULL);
 	if (ret) {
 		dev_err(imx334->dev, "fail to write initial registers\n");
 		return ret;
@@ -1014,8 +899,8 @@ static int imx334_start_streaming(struct imx334 *imx334)
 	}
 
 	/* Start streaming */
-	ret = imx334_write_reg(imx334, IMX334_REG_MODE_SELECT,
-			       1, IMX334_MODE_STREAMING);
+	ret = cci_write(imx334->cci, IMX334_REG_MODE_SELECT,
+			IMX334_MODE_STREAMING, NULL);
 	if (ret) {
 		dev_err(imx334->dev, "fail to start streaming\n");
 		return ret;
@@ -1032,8 +917,8 @@ static int imx334_start_streaming(struct imx334 *imx334)
  */
 static int imx334_stop_streaming(struct imx334 *imx334)
 {
-	return imx334_write_reg(imx334, IMX334_REG_MODE_SELECT,
-				1, IMX334_MODE_STANDBY);
+	return cci_write(imx334->cci, IMX334_REG_MODE_SELECT,
+			IMX334_MODE_STANDBY, NULL);
 }
 
 /**
@@ -1084,14 +969,14 @@ error_unlock:
 static int imx334_detect(struct imx334 *imx334)
 {
 	int ret;
-	u32 val;
+	u64 val;
 
-	ret = imx334_read_reg(imx334, IMX334_REG_ID, 2, &val);
+	ret = cci_read(imx334->cci, IMX334_REG_ID, &val, NULL);
 	if (ret)
 		return ret;
 
 	if (val != IMX334_ID) {
-		dev_err(imx334->dev, "chip id mismatch: %x!=%x\n",
+		dev_err(imx334->dev, "chip id mismatch: %x!=%llx\n",
 			IMX334_ID, val);
 		return -ENXIO;
 	}
@@ -1339,6 +1224,11 @@ static int imx334_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	imx334->dev = &client->dev;
+	imx334->cci = devm_cci_regmap_init_i2c(client, 16);
+	if (IS_ERR(imx334->cci)) {
+		dev_err(imx334->dev, "Unable to initialize I2C\n");
+		return -ENODEV;
+	}
 
 	/* Initialize subdev */
 	v4l2_i2c_subdev_init(&imx334->sd, client, &imx334_subdev_ops);
-- 
2.51.0


From 731c8efd5b74f8aa4c57ffd8e8561d93e425b007 Mon Sep 17 00:00:00 2001
From: Tarang Raval <tarang.raval@siliconsignals.io>
Date: Sat, 29 Mar 2025 11:13:25 +0530
Subject: [PATCH 10/16] media: i2c: imx334: Remove redundant register entries

IMX334_REG_{ADBIT, MDBIT, ADBIT1}: Already written in
imx334_set_framefmt function.

IMX334_REG_THSPREPARE: Unnecessary repeated writes removed.

CCI_REG8(0x3e04): Unnecessary repeated writes removed.

Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/imx334.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 0785bf213d91..9d4d15df8dcf 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -226,7 +226,6 @@ static const struct cci_reg_sequence common_mode_regs[] = {
 	{ IMX334_REG_OPB_SIZE_V,	0x00 },
 	{ IMX334_REG_HREVERSE,		0x00 },
 	{ IMX334_REG_VREVERSE,		0x00 },
-	{ IMX334_REG_ADBIT,		0x00 },
 	{ IMX334_REG_UNREAD_PARAM5,	0x0000 },
 	{ IMX334_REG_UNREAD_PARAM6,	0x0008 },
 	{ IMX334_REG_XVS_XHS_OUTSEL,	0x20 },
@@ -239,19 +238,13 @@ static const struct cci_reg_sequence common_mode_regs[] = {
 	{ IMX334_REG_INCKSEL4,		0x7e },
 	{ IMX334_REG_SYS_MODE,		0x02 },
 	{ IMX334_REG_HADD_VADD,		0x00 },
-	{ IMX334_REG_MDBIT,		0x00 },
 	{ IMX334_REG_VALID_EXPAND,	0x03 },
 	{ IMX334_REG_TCYCLE,		0x00 },
-	{ IMX334_REG_ADBIT1,		0x01ff },
 	{ IMX334_REG_LANEMODE,		0x03 },
 	{ IMX334_REG_TCLKPOST,		0x007f },
 	{ IMX334_REG_TCLKPREPARE,	0x0037 },
 	{ IMX334_REG_TCLKTRAIL,		0x0037 },
 	{ IMX334_REG_TCLKZERO,		0xf7 },
-	{ IMX334_REG_THSPREPARE,	0x003f },
-	{ IMX334_REG_THSPREPARE,	0x006f },
-	{ IMX334_REG_THSPREPARE,	0x003f },
-	{ IMX334_REG_THSPREPARE,	0x005f },
 	{ IMX334_REG_THSPREPARE,	0x002f },
 	{ CCI_REG8(0x3078), 0x02 },
 	{ CCI_REG8(0x3079), 0x00 },
@@ -339,7 +332,6 @@ static const struct cci_reg_sequence common_mode_regs[] = {
 	{ CCI_REG8(0x37b0), 0x37 },
 	{ CCI_REG8(0x3e04), 0x0e },
 	{ IMX334_REG_AGAIN, 0x0050 },
-	{ CCI_REG8(0x3e04), 0x0e },
 	{ IMX334_REG_MASTER_MODE, 0x00 },
 };
 
-- 
2.51.0


From 9e089a649a229fb08942ada75b0f1c7f5814e065 Mon Sep 17 00:00:00 2001
From: Tarang Raval <tarang.raval@siliconsignals.io>
Date: Sat, 29 Mar 2025 11:13:26 +0530
Subject: [PATCH 11/16] media: i2c: imx334: Configure lane mode dynamically

Configure the lane mode dynamically from the streaming
function instead of using a hardcoded value.

Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
[Sakari Ailus: Fix checkpatch.pl issue, lower Dynamically.]
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/imx334.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 9d4d15df8dcf..561ed2c87005 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -47,6 +47,8 @@
 #define IMX334_EXPOSURE_DEFAULT		0x0648
 
 #define IMX334_REG_LANEMODE		CCI_REG8(0x3a01)
+#define IMX334_CSI_4_LANE_MODE		3
+#define IMX334_CSI_8_LANE_MODE		7
 
 /* Window cropping Settings */
 #define IMX334_REG_AREA3_ST_ADR_1	CCI_REG16_LE(0x3074)
@@ -240,7 +242,6 @@ static const struct cci_reg_sequence common_mode_regs[] = {
 	{ IMX334_REG_HADD_VADD,		0x00 },
 	{ IMX334_REG_VALID_EXPAND,	0x03 },
 	{ IMX334_REG_TCYCLE,		0x00 },
-	{ IMX334_REG_LANEMODE,		0x03 },
 	{ IMX334_REG_TCLKPOST,		0x007f },
 	{ IMX334_REG_TCLKPREPARE,	0x0037 },
 	{ IMX334_REG_TCLKTRAIL,		0x0037 },
@@ -876,6 +877,13 @@ static int imx334_start_streaming(struct imx334 *imx334)
 		return ret;
 	}
 
+	ret = cci_write(imx334->cci, IMX334_REG_LANEMODE,
+			IMX334_CSI_4_LANE_MODE, NULL);
+	if (ret) {
+		dev_err(imx334->dev, "failed to configure lanes\n");
+		return ret;
+	}
+
 	ret = imx334_set_framefmt(imx334);
 	if (ret) {
 		dev_err(imx334->dev, "%s failed to set frame format: %d\n",
-- 
2.51.0


From a6dde677b93795a04f43f90427723bb4c2a50e5e Mon Sep 17 00:00:00 2001
From: Tarang Raval <tarang.raval@siliconsignals.io>
Date: Sat, 29 Mar 2025 11:13:27 +0530
Subject: [PATCH 12/16] media: i2c: imx334: Fix power management and control
 handling

Some controls may need the sensor to be powered on to update their
values. Currently, only the exposure control does this. To ensure
proper handling, the power-up sequence is moved outside the switch-case.

Additionally, VBLANK control is now processed earlier so its changes
can correctly affect other controls.

Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/imx334.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 561ed2c87005..e8422d2fadfd 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -578,8 +578,7 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl)
 	u32 exposure;
 	int ret;
 
-	switch (ctrl->id) {
-	case V4L2_CID_VBLANK:
+	if (ctrl->id == V4L2_CID_VBLANK) {
 		imx334->vblank = imx334->vblank_ctrl->val;
 
 		dev_dbg(imx334->dev, "Received vblank %u, new lpfr %u\n",
@@ -592,13 +591,24 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl)
 					       imx334->cur_mode->height -
 					       IMX334_EXPOSURE_OFFSET,
 					       1, IMX334_EXPOSURE_DEFAULT);
+		if (ret)
+			return ret;
+	}
+
+	/* Set controls only if sensor is in power on state */
+	if (!pm_runtime_get_if_in_use(imx334->dev))
+		return 0;
+
+	switch (ctrl->id) {
+	case V4L2_CID_VBLANK:
+		exposure = imx334->exp_ctrl->val;
+		analog_gain = imx334->again_ctrl->val;
+
+		ret = imx334_update_exp_gain(imx334, exposure, analog_gain);
+
 		break;
 	case V4L2_CID_EXPOSURE:
 
-		/* Set controls only if sensor is in power on state */
-		if (!pm_runtime_get_if_in_use(imx334->dev))
-			return 0;
-
 		exposure = ctrl->val;
 		analog_gain = imx334->again_ctrl->val;
 
@@ -607,8 +617,6 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl)
 
 		ret = imx334_update_exp_gain(imx334, exposure, analog_gain);
 
-		pm_runtime_put(imx334->dev);
-
 		break;
 	case V4L2_CID_PIXEL_RATE:
 	case V4L2_CID_LINK_FREQ:
@@ -640,6 +648,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl)
 		ret = -EINVAL;
 	}
 
+	pm_runtime_put(imx334->dev);
+
 	return ret;
 }
 
-- 
2.51.0


From b493cd3c03641f9bbaa9787e43ca92163cb50051 Mon Sep 17 00:00:00 2001
From: Tarang Raval <tarang.raval@siliconsignals.io>
Date: Sat, 29 Mar 2025 11:13:28 +0530
Subject: [PATCH 13/16] media: i2c: imx334: Fix runtime PM handling in remove
 function

pm_runtime_suspended() only checks the current runtime PM status and does
not modify it, making it ineffective in this context. This could result in
improper power management if the device remains active when removed.

This patch fixes the issue by introducing a check with
pm_runtime_status_suspended() to determine if the device is already
suspended. If it is not, it calls imx334_power_off() to power down the
device and then uses pm_runtime_set_suspended() to correctly update the
runtime PM status to suspended.

Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/imx334.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index e8422d2fadfd..2ede0787be14 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -1328,7 +1328,10 @@ static void imx334_remove(struct i2c_client *client)
 	v4l2_ctrl_handler_free(sd->ctrl_handler);
 
 	pm_runtime_disable(&client->dev);
-	pm_runtime_suspended(&client->dev);
+	if (!pm_runtime_status_suspended(&client->dev)) {
+		imx334_power_off(&client->dev);
+		pm_runtime_set_suspended(&client->dev);
+	}
 
 	mutex_destroy(&imx334->mutex);
 }
-- 
2.51.0


From 01dfdf6a80c57151af0589af0db7adbbdd1361c7 Mon Sep 17 00:00:00 2001
From: Tarang Raval <tarang.raval@siliconsignals.io>
Date: Sat, 29 Mar 2025 11:13:29 +0530
Subject: [PATCH 14/16] media: i2c: imx334: Enable runtime PM before sub-device
 registration

Runtime PM is fully initialized before calling
v4l2_async_register_subdev_sensor(). Moving the runtime PM initialization
earlier prevents potential access to an uninitialized or powered-down
device.

Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/imx334.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 2ede0787be14..412ab469b130 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -1287,6 +1287,9 @@ static int imx334_probe(struct i2c_client *client)
 		goto error_handler_free;
 	}
 
+	pm_runtime_set_active(imx334->dev);
+	pm_runtime_enable(imx334->dev);
+
 	ret = v4l2_async_register_subdev_sensor(&imx334->sd);
 	if (ret < 0) {
 		dev_err(imx334->dev,
@@ -1294,13 +1297,13 @@ static int imx334_probe(struct i2c_client *client)
 		goto error_media_entity;
 	}
 
-	pm_runtime_set_active(imx334->dev);
-	pm_runtime_enable(imx334->dev);
 	pm_runtime_idle(imx334->dev);
 
 	return 0;
 
 error_media_entity:
+	pm_runtime_disable(imx334->dev);
+	pm_runtime_set_suspended(imx334->dev);
 	media_entity_cleanup(&imx334->sd.entity);
 error_handler_free:
 	v4l2_ctrl_handler_free(imx334->sd.ctrl_handler);
-- 
2.51.0


From 9d382f6a9978916317b3fb4ef07b5fec684adde0 Mon Sep 17 00:00:00 2001
From: Tarang Raval <tarang.raval@siliconsignals.io>
Date: Sat, 29 Mar 2025 11:13:30 +0530
Subject: [PATCH 15/16] media: i2c: imx334: Use subdev state lock for
 synchronization

Replace the custom mutex in the imx334 driver with the V4L2 subdev state
lock for control synchronization. Initialize the subdev with
v4l2_subdev_init_finalize in imx334_probe, adding proper cleanup in error
paths and imx334_remove. This aligns the driver with V4L2 standards.

Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/imx334.c | 52 +++++++++++++-------------------------
 1 file changed, 17 insertions(+), 35 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 412ab469b130..fa11619bb368 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -184,7 +184,6 @@ struct imx334_mode {
  * @again_ctrl: Pointer to analog gain control
  * @vblank: Vertical blanking in lines
  * @cur_mode: Pointer to current selected sensor mode
- * @mutex: Mutex for serializing sensor controls
  * @link_freq_bitmap: Menu bitmap for link_freq_ctrl
  * @cur_code: current selected format code
  */
@@ -207,7 +206,6 @@ struct imx334 {
 	};
 	u32 vblank;
 	const struct imx334_mode *cur_mode;
-	struct mutex mutex;
 	unsigned long link_freq_bitmap;
 	u32 cur_code;
 };
@@ -755,8 +753,6 @@ static int imx334_get_pad_format(struct v4l2_subdev *sd,
 {
 	struct imx334 *imx334 = to_imx334(sd);
 
-	mutex_lock(&imx334->mutex);
-
 	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
 		struct v4l2_mbus_framefmt *framefmt;
 
@@ -767,8 +763,6 @@ static int imx334_get_pad_format(struct v4l2_subdev *sd,
 		imx334_fill_pad_format(imx334, imx334->cur_mode, fmt);
 	}
 
-	mutex_unlock(&imx334->mutex);
-
 	return 0;
 }
 
@@ -788,8 +782,6 @@ static int imx334_set_pad_format(struct v4l2_subdev *sd,
 	const struct imx334_mode *mode;
 	int ret = 0;
 
-	mutex_lock(&imx334->mutex);
-
 	mode = v4l2_find_nearest_size(supported_modes,
 				      ARRAY_SIZE(supported_modes),
 				      width, height,
@@ -810,8 +802,6 @@ static int imx334_set_pad_format(struct v4l2_subdev *sd,
 			imx334->cur_mode = mode;
 	}
 
-	mutex_unlock(&imx334->mutex);
-
 	return ret;
 }
 
@@ -830,8 +820,6 @@ static int imx334_init_state(struct v4l2_subdev *sd,
 
 	fmt.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
 
-	mutex_lock(&imx334->mutex);
-
 	imx334_fill_pad_format(imx334, imx334->cur_mode, &fmt);
 
 	__v4l2_ctrl_modify_range(imx334->link_freq_ctrl, 0,
@@ -839,8 +827,6 @@ static int imx334_init_state(struct v4l2_subdev *sd,
 				 ~(imx334->link_freq_bitmap),
 				 __ffs(imx334->link_freq_bitmap));
 
-	mutex_unlock(&imx334->mutex);
-
 	return imx334_set_pad_format(sd, sd_state, &fmt);
 }
 
@@ -943,12 +929,10 @@ static int imx334_set_stream(struct v4l2_subdev *sd, int enable)
 	struct imx334 *imx334 = to_imx334(sd);
 	int ret;
 
-	mutex_lock(&imx334->mutex);
-
 	if (enable) {
 		ret = pm_runtime_resume_and_get(imx334->dev);
 		if (ret < 0)
-			goto error_unlock;
+			return ret;
 
 		ret = imx334_start_streaming(imx334);
 		if (ret)
@@ -958,15 +942,10 @@ static int imx334_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(imx334->dev);
 	}
 
-	mutex_unlock(&imx334->mutex);
-
 	return 0;
 
 error_power_off:
 	pm_runtime_put(imx334->dev);
-error_unlock:
-	mutex_unlock(&imx334->mutex);
-
 	return ret;
 }
 
@@ -1145,9 +1124,6 @@ static int imx334_init_controls(struct imx334 *imx334)
 	if (ret)
 		return ret;
 
-	/* Serialize controls with sensor device */
-	ctrl_hdlr->lock = &imx334->mutex;
-
 	/* Initialize exposure and gain */
 	lpfr = mode->vblank + mode->height;
 	imx334->exp_ctrl = v4l2_ctrl_new_std(ctrl_hdlr,
@@ -1249,12 +1225,10 @@ static int imx334_probe(struct i2c_client *client)
 		return dev_err_probe(imx334->dev, ret,
 					"HW configuration is not supported\n");
 
-	mutex_init(&imx334->mutex);
-
 	ret = imx334_power_on(imx334->dev);
 	if (ret) {
 		dev_err_probe(imx334->dev, ret, "failed to power-on the sensor\n");
-		goto error_mutex_destroy;
+		return ret;
 	}
 
 	/* Check module identity */
@@ -1287,6 +1261,13 @@ static int imx334_probe(struct i2c_client *client)
 		goto error_handler_free;
 	}
 
+	imx334->sd.state_lock = imx334->ctrl_handler.lock;
+	ret = v4l2_subdev_init_finalize(&imx334->sd);
+	if (ret < 0) {
+		dev_err(imx334->dev, "subdev init error: %d\n", ret);
+		goto error_media_entity;
+	}
+
 	pm_runtime_set_active(imx334->dev);
 	pm_runtime_enable(imx334->dev);
 
@@ -1294,23 +1275,26 @@ static int imx334_probe(struct i2c_client *client)
 	if (ret < 0) {
 		dev_err(imx334->dev,
 			"failed to register async subdev: %d\n", ret);
-		goto error_media_entity;
+		goto error_subdev_cleanup;
 	}
 
 	pm_runtime_idle(imx334->dev);
 
 	return 0;
 
-error_media_entity:
+error_subdev_cleanup:
+	v4l2_subdev_cleanup(&imx334->sd);
 	pm_runtime_disable(imx334->dev);
 	pm_runtime_set_suspended(imx334->dev);
+
+error_media_entity:
 	media_entity_cleanup(&imx334->sd.entity);
+
 error_handler_free:
 	v4l2_ctrl_handler_free(imx334->sd.ctrl_handler);
+
 error_power_off:
 	imx334_power_off(imx334->dev);
-error_mutex_destroy:
-	mutex_destroy(&imx334->mutex);
 
 	return ret;
 }
@@ -1324,9 +1308,9 @@ error_mutex_destroy:
 static void imx334_remove(struct i2c_client *client)
 {
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct imx334 *imx334 = to_imx334(sd);
 
 	v4l2_async_unregister_subdev(sd);
+	v4l2_subdev_cleanup(sd);
 	media_entity_cleanup(&sd->entity);
 	v4l2_ctrl_handler_free(sd->ctrl_handler);
 
@@ -1335,8 +1319,6 @@ static void imx334_remove(struct i2c_client *client)
 		imx334_power_off(&client->dev);
 		pm_runtime_set_suspended(&client->dev);
 	}
-
-	mutex_destroy(&imx334->mutex);
 }
 
 static const struct dev_pm_ops imx334_pm_ops = {
-- 
2.51.0


From 6f1b74c1a686c93b404bd57d73577a6b5b19c5c3 Mon Sep 17 00:00:00 2001
From: Tarang Raval <tarang.raval@siliconsignals.io>
Date: Sat, 29 Mar 2025 11:13:31 +0530
Subject: [PATCH 16/16] media: i2c: imx334: switch to {enable,disable}_streams

Switch from s_stream to enable_streams and disable_streams callbacks.

Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/media/i2c/imx334.c | 78 +++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 40 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index fa11619bb368..fc875072f859 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -847,21 +847,31 @@ static int imx334_set_framefmt(struct imx334 *imx334)
 }
 
 /**
- * imx334_start_streaming() - Start sensor stream
- * @imx334: pointer to imx334 device
+ * imx334_enable_streams() - Enable specified streams for the sensor
+ * @sd: pointer to the V4L2 subdevice
+ * @state: pointer to the subdevice state
+ * @pad: pad number for which streams are enabled
+ * @streams_mask: bitmask specifying the streams to enable
  *
  * Return: 0 if successful, error code otherwise.
  */
-static int imx334_start_streaming(struct imx334 *imx334)
+static int imx334_enable_streams(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state, u32 pad,
+				 u64 streams_mask)
 {
+	struct imx334 *imx334 = to_imx334(sd);
 	const struct imx334_reg_list *reg_list;
 	int ret;
 
+	ret = pm_runtime_resume_and_get(imx334->dev);
+	if (ret < 0)
+		return ret;
+
 	ret = cci_multi_reg_write(imx334->cci, common_mode_regs,
 				  ARRAY_SIZE(common_mode_regs), NULL);
 	if (ret) {
 		dev_err(imx334->dev, "fail to write common registers\n");
-		return ret;
+		goto err_rpm_put;
 	}
 
 	/* Write sensor mode registers */
@@ -870,28 +880,28 @@ static int imx334_start_streaming(struct imx334 *imx334)
 				  reg_list->num_of_regs, NULL);
 	if (ret) {
 		dev_err(imx334->dev, "fail to write initial registers\n");
-		return ret;
+		goto err_rpm_put;
 	}
 
 	ret = cci_write(imx334->cci, IMX334_REG_LANEMODE,
 			IMX334_CSI_4_LANE_MODE, NULL);
 	if (ret) {
 		dev_err(imx334->dev, "failed to configure lanes\n");
-		return ret;
+		goto err_rpm_put;
 	}
 
 	ret = imx334_set_framefmt(imx334);
 	if (ret) {
 		dev_err(imx334->dev, "%s failed to set frame format: %d\n",
 			__func__, ret);
-		return ret;
+		goto err_rpm_put;
 	}
 
 	/* Setup handler will write actual exposure and gain */
 	ret =  __v4l2_ctrl_handler_setup(imx334->sd.ctrl_handler);
 	if (ret) {
 		dev_err(imx334->dev, "fail to setup handler\n");
-		return ret;
+		goto err_rpm_put;
 	}
 
 	/* Start streaming */
@@ -899,53 +909,39 @@ static int imx334_start_streaming(struct imx334 *imx334)
 			IMX334_MODE_STREAMING, NULL);
 	if (ret) {
 		dev_err(imx334->dev, "fail to start streaming\n");
-		return ret;
+		goto err_rpm_put;
 	}
 
 	return 0;
-}
 
-/**
- * imx334_stop_streaming() - Stop sensor stream
- * @imx334: pointer to imx334 device
- *
- * Return: 0 if successful, error code otherwise.
- */
-static int imx334_stop_streaming(struct imx334 *imx334)
-{
-	return cci_write(imx334->cci, IMX334_REG_MODE_SELECT,
-			IMX334_MODE_STANDBY, NULL);
+err_rpm_put:
+	pm_runtime_put(imx334->dev);
+	return ret;
 }
 
 /**
- * imx334_set_stream() - Enable sensor streaming
- * @sd: pointer to imx334 subdevice
- * @enable: set to enable sensor streaming
+ * imx334_disable_streams() - Enable specified streams for the sensor
+ * @sd: pointer to the V4L2 subdevice
+ * @state: pointer to the subdevice state
+ * @pad: pad number for which streams are disabled
+ * @streams_mask: bitmask specifying the streams to disable
  *
  * Return: 0 if successful, error code otherwise.
  */
-static int imx334_set_stream(struct v4l2_subdev *sd, int enable)
+static int imx334_disable_streams(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state, u32 pad,
+				  u64 streams_mask)
 {
 	struct imx334 *imx334 = to_imx334(sd);
 	int ret;
 
-	if (enable) {
-		ret = pm_runtime_resume_and_get(imx334->dev);
-		if (ret < 0)
-			return ret;
-
-		ret = imx334_start_streaming(imx334);
-		if (ret)
-			goto error_power_off;
-	} else {
-		imx334_stop_streaming(imx334);
-		pm_runtime_put(imx334->dev);
-	}
-
-	return 0;
+	ret = cci_write(imx334->cci, IMX334_REG_MODE_SELECT,
+			IMX334_MODE_STANDBY, NULL);
+	if (ret)
+		dev_err(imx334->dev, "%s failed to stop stream\n", __func__);
 
-error_power_off:
 	pm_runtime_put(imx334->dev);
+
 	return ret;
 }
 
@@ -1040,7 +1036,7 @@ done_endpoint_free:
 
 /* V4l2 subdevice ops */
 static const struct v4l2_subdev_video_ops imx334_video_ops = {
-	.s_stream = imx334_set_stream,
+	.s_stream = v4l2_subdev_s_stream_helper,
 };
 
 static const struct v4l2_subdev_pad_ops imx334_pad_ops = {
@@ -1048,6 +1044,8 @@ static const struct v4l2_subdev_pad_ops imx334_pad_ops = {
 	.enum_frame_size = imx334_enum_frame_size,
 	.get_fmt = imx334_get_pad_format,
 	.set_fmt = imx334_set_pad_format,
+	.enable_streams = imx334_enable_streams,
+	.disable_streams = imx334_disable_streams,
 };
 
 static const struct v4l2_subdev_ops imx334_subdev_ops = {
-- 
2.51.0