[PATCH V3] media: i2c: imx334: add new link frequency configuration

From: Shravan Chippa <[email protected]>

Add 222 MHz link frequency support and refactor clock registers into
per-frequency register lists. Introduce imx334_clk_params to map each
link frequency to its register set and supported resolution range.
Make the link frequency control writable so userspace can select the
desired operating point.

Signed-off-by: Shravan Chippa <[email protected]>

---
Changes from V2 -> V3

- Refactor: Introduce struct imx334_clk_params
    - Maps each link frequency to its clock register set and supported
      resolution range (width_max, height_max, width_min, height_min).
    - Includes a default_mode pointer for fallback when resolution is
      out of range for the selected link frequency.

- Enhancement: Make link_freq control writable
    - Removed V4L2_CTRL_FLAG_READ_ONLY from link_freq_ctrl so userspace
      can select the desired link frequency operating point.

- Enhancement: Add resolution bounds checking in set_pad_format
    - imx334_set_pad_format() now validates the selected mode against
      the clk_params width/height constraints and falls back to the
      default mode if out of range.

- Cleanup: Remove __v4l2_ctrl_s_ctrl for link_freq in update_controls
    - Link frequency is no longer tied to mode, so the explicit control
      update in imx334_update_controls() was removed.


Changes from V1 -> V2

- Fix: Default mode selection bug
    - Problem: Used __ffs(link_freq_bitmap) as index into the compacted
      active_modes array. This index corresponds to a bit position in the
      bitmap, not an array index in the filtered modes list.
    - Fix: Use index 0, which is always the first valid mode in the
      filtered array.

- Fix: Rename fields for clarity
    - Renamed struct fields:
      * new_supported_modes -> active_modes
      * new_modes_size -> num_active_modes
    - Updated kernel-doc header for struct imx334 to document new fields.

- Fix: INCKSEL2 register write - add error handling and switch-case
    - Location: imx334_enable_streams()
    - Problem: Original code only handled 222 MHz case with no error
      checking (passed NULL to cci_write).
    - Fix: Replaced with switch-case covering all three link frequencies:
      * 891 MHz -> INCKSEL2 = 0x02
      * 445 MHz -> INCKSEL2 = 0x06
      * 222 MHz -> INCKSEL2 = 0x0a
    - Added proper error handling using &ret accumulator pattern with
      dev_err and goto err_rpm_put on failure.

- Fix: Use BIT() macro
    - Location: imx334_update_supported_mode_array()
    - Replaced (1 << i) with BIT(i) for kernel coding style compliance.

- Fix: Use devm_kmalloc_array() for overflow-safe allocation
    - Location: imx334_update_supported_mode_array()
    - Replaced: devm_kmalloc(dev, n * sizeof(struct imx334_mode),
 GFP_KERNEL)
    - With: devm_kmalloc_array(dev, n, sizeof(*temp_ptr), GFP_KERNEL)
    - Provides overflow-safe multiplication and uses sizeof(*ptr) idiom.

- Cleanup: Condensed copy loop
    - Location: imx334_update_supported_mode_array(), second loop
    - Simplified the struct copy with post-increment:
      temp_ptr[size++] = supported_modes[j];

- Fix: Updated function kernel-doc comment
    - Location: imx334_update_supported_mode_array()
    - Reworded: "Search for the supported modes add them in the new list"
    - To: "Build filtered modes array based on DTS link frequencies"

- IMX334_LINK_FREQ_222M changed from 222500000 to 222750000

---

 drivers/media/i2c/imx334.c | 132 +++++++++++++++++++++++++++++++------
 1 file changed, 112 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 553a16b84f4d..fb036873859c 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -109,6 +109,7 @@
 /* CSI2 HW configuration */
 #define IMX334_LINK_FREQ_891M		891000000
 #define IMX334_LINK_FREQ_445M		445500000
+#define IMX334_LINK_FREQ_222M		222750000
 #define IMX334_NUM_DATA_LANES		4
 
 #define IMX334_REG_MIN			0x00
@@ -154,7 +155,6 @@ struct imx334_reg_list {
  * @vblank_min: Minimal vertical blanking in lines
  * @vblank_max: Maximum vertical blanking in lines
  * @pclk: Sensor pixel clock
- * @link_freq_idx: Link frequency index
  * @reg_list: Register list for sensor mode
  */
 struct imx334_mode {
@@ -165,7 +165,28 @@ struct imx334_mode {
 	u32 vblank_min;
 	u32 vblank_max;
 	u64 pclk;
-	u32 link_freq_idx;
+	struct imx334_reg_list reg_list;
+};
+
+/**
+ * struct imx334_clk_params - imx334 sensor clock parameters
+ * @data_rate_per_lane: Data rate per lane in bits per second
+ * @link_freq: Link frequency in Hz
+ * @width_max: Maximum image width in pixels
+ * @height_max: Maximum image height in pixels
+ * @width_min: Minimum image width in pixels
+ * @height_min: Minimum image height in pixels
+ * @default_mode: Pointer to the default sensor mode
+ * @reg_list: Register list for clock configuration
+ */
+struct imx334_clk_params {
+	u32 data_rate_per_lane;
+	u32 link_freq;
+	u32 width_max;
+	u32 height_max;
+	u32 width_min;
+	u32 height_min;
+	const struct imx334_mode *default_mode;
 	struct imx334_reg_list reg_list;
 };
 
@@ -216,6 +237,7 @@ struct imx334 {
 static const s64 link_freq[] = {
 	IMX334_LINK_FREQ_891M,
 	IMX334_LINK_FREQ_445M,
+	IMX334_LINK_FREQ_222M,
 };
 
 /* Sensor common mode registers values */
@@ -233,13 +255,6 @@ static const struct cci_reg_sequence common_mode_regs[] = {
 	{ 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_VALID_EXPAND,	0x03 },
 	{ IMX334_REG_TCYCLE,		0x00 },
@@ -397,6 +412,39 @@ static const struct cci_reg_sequence mode_3840x2160_regs[] = {
 	{ IMX334_REG_TPLX,		0x005f },
 };
 
+/* Data rate 1782Mbps per lane and 891Mhz link frequency */
+static const struct cci_reg_sequence link_freq_891m_regs[] = {
+	{ IMX334_REG_BCWAIT_TIME,	0x3b },
+	{ IMX334_REG_CPWAIT_TIME,	0x2a },
+	{ IMX334_REG_INCKSEL1,		0x0129 },
+	{ IMX334_REG_INCKSEL2,		0x02 },
+	{ IMX334_REG_INCKSEL3,		0xa0 },
+	{ IMX334_REG_INCKSEL4,		0x7e },
+	{ IMX334_REG_SYS_MODE,		0x00 },
+};
+
+/* Data rate 891Mbps per lane and 445Mhz link frequency */
+static const struct cci_reg_sequence link_freq_445m_regs[] = {
+	{ 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 },
+};
+
+/* Data rate 445Mbps per lane and 222Mhz link frequency */
+static const struct cci_reg_sequence link_freq_222m_regs[] = {
+	{ IMX334_REG_BCWAIT_TIME,	0x3b },
+	{ IMX334_REG_CPWAIT_TIME,	0x2a },
+	{ IMX334_REG_INCKSEL1,		0x0129 },
+	{ IMX334_REG_INCKSEL2,		0x0a },
+	{ IMX334_REG_INCKSEL3,		0xa0 },
+	{ IMX334_REG_INCKSEL4,		0x7e },
+	{ IMX334_REG_SYS_MODE,		0x02 },
+};
+
 static const char * const imx334_test_pattern_menu[] = {
 	"Disabled",
 	"Vertical Color Bars",
@@ -442,7 +490,6 @@ static const struct imx334_mode supported_modes[] = {
 		.vblank_min = 90,
 		.vblank_max = 132840,
 		.pclk = 594000000,
-		.link_freq_idx = 0,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_3840x2160_regs),
 			.regs = mode_3840x2160_regs,
@@ -455,7 +502,6 @@ static const struct imx334_mode supported_modes[] = {
 		.vblank_min = 45,
 		.vblank_max = 132840,
 		.pclk = 297000000,
-		.link_freq_idx = 1,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_1920x1080_regs),
 			.regs = mode_1920x1080_regs,
@@ -468,7 +514,6 @@ static const struct imx334_mode supported_modes[] = {
 		.vblank_min = 45,
 		.vblank_max = 132840,
 		.pclk = 297000000,
-		.link_freq_idx = 1,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
 			.regs = mode_1280x720_regs,
@@ -481,7 +526,6 @@ static const struct imx334_mode supported_modes[] = {
 		.vblank_min = 45,
 		.vblank_max = 132840,
 		.pclk = 297000000,
-		.link_freq_idx = 1,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_640x480_regs),
 			.regs = mode_640x480_regs,
@@ -489,6 +533,46 @@ static const struct imx334_mode supported_modes[] = {
 	},
 };
 
+static const struct imx334_clk_params imx334_clk_params[] = {
+	{
+		.data_rate_per_lane = 1782000000,
+		.link_freq = IMX334_LINK_FREQ_891M,
+		.width_max = 3840,
+		.height_max = 2160,
+		.width_min = 3840,
+		.height_min = 2160,
+		.default_mode = &supported_modes[0], /* 3840x2160 */
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(link_freq_891m_regs),
+			.regs = link_freq_891m_regs,
+		},
+	}, {
+		.data_rate_per_lane = 891000000,
+		.link_freq = IMX334_LINK_FREQ_445M,
+		.width_max = 1920,
+		.height_max = 1080,
+		.width_min = 640,
+		.height_min = 480,
+		.default_mode = &supported_modes[1], /* 1920x1080 */
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(link_freq_445m_regs),
+			.regs = link_freq_445m_regs,
+		},
+	}, {
+		.data_rate_per_lane = 445500000,
+		.link_freq = IMX334_LINK_FREQ_222M,
+		.width_max = 1920,
+		.height_max = 1080,
+		.width_min = 640,
+		.height_min = 480,
+		.default_mode = &supported_modes[1], /* 1920x1080 */
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(link_freq_222m_regs),
+			.regs = link_freq_222m_regs,
+		},
+	}
+};
+
 /**
  * to_imx334() - imv334 V4L2 sub-device to imx334 device.
  * @subdev: pointer to imx334 V4L2 sub-device
@@ -512,10 +596,6 @@ static int imx334_update_controls(struct imx334 *imx334,
 {
 	int ret;
 
-	ret = __v4l2_ctrl_s_ctrl(imx334->link_freq_ctrl, mode->link_freq_idx);
-	if (ret)
-		return ret;
-
 	ret = __v4l2_ctrl_modify_range(imx334->pclk_ctrl, mode->pclk,
 				       mode->pclk, 1, mode->pclk);
 	if (ret)
@@ -746,6 +826,7 @@ static int imx334_set_pad_format(struct v4l2_subdev *sd,
 {
 	struct imx334 *imx334 = to_imx334(sd);
 	const struct imx334_mode *mode;
+	const struct imx334_clk_params *clk_params;
 	int ret = 0;
 
 	mode = v4l2_find_nearest_size(supported_modes,
@@ -753,6 +834,11 @@ static int imx334_set_pad_format(struct v4l2_subdev *sd,
 				      width, height,
 				      fmt->format.width, fmt->format.height);
 
+	clk_params = &imx334_clk_params[imx334->link_freq_ctrl->val];
+	if (mode->width > clk_params->width_max || mode->height > clk_params->height_max ||
+	    mode->width < clk_params->width_min || mode->height < clk_params->height_min)
+		mode = clk_params->default_mode;
+
 	imx334_fill_pad_format(imx334, mode, fmt);
 	fmt->format.code = imx334_get_format_code(imx334, fmt->format.code);
 
@@ -824,6 +910,15 @@ static int imx334_enable_streams(struct v4l2_subdev *sd,
 		goto err_rpm_put;
 	}
 
+	/* Write sensor link freq registers */
+	reg_list = &imx334_clk_params[imx334->link_freq_ctrl->val].reg_list;
+	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");
+		goto err_rpm_put;
+	}
+
 	/* Write sensor mode registers */
 	reg_list = &imx334->cur_mode->reg_list;
 	ret = cci_multi_reg_write(imx334->cci, reg_list->regs,
@@ -1096,9 +1191,6 @@ static int imx334_init_controls(struct imx334 *imx334)
 							__ffs(imx334->link_freq_bitmap),
 							link_freq);
 
-	if (imx334->link_freq_ctrl)
-		imx334->link_freq_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
-
 	imx334->hblank_ctrl = v4l2_ctrl_new_std(ctrl_hdlr,
 						&imx334_ctrl_ops,
 						V4L2_CID_HBLANK,
-- 
2.34.1