* [PATCH 0/6] media: i2c: imx334: Miscellaneous cleanups and improvements
@ 2025-03-10 7:17 Tarang Raval
2025-03-10 7:17 ` [PATCH 1/6] media: i2c: imx334: Simplify with dev_err_probe() Tarang Raval
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Tarang Raval @ 2025-03-10 7:17 UTC (permalink / raw)
To: sakari.ailus, kieran.bingham
Cc: Shravan.Chippa, Tarang Raval, Mauro Carvalho Chehab, Hans Verkuil,
Laurent Pinchart, AngeloGioacchino Del Regno, Julien Massot,
Zhi Mao, Kory Maincent, Benjamin Mugnier, linux-media,
linux-kernel
This patch series is a collection of miscellaneous cleanups and
improvements to the imx334 driver.
The changes simplify error handling, optimize register access,
enhance power management, and add support for different lane
configurations.
This series is on top of Shravan's patches [1]
(media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions)
Link [1]: https://lore.kernel.org/linux-media/20250305051442.3716817-1-shravan.chippa@microchip.com/T/#t
Tarang Raval (6):
media: i2c: imx334: Simplify with dev_err_probe()
media: i2c: imx334: Convert to CCI register access helpers
media: i2c: imx334: Remove redundant register entries
media: i2c: imx334: Support 4 or 8 lane operation modes
media: i2c: imx334: Fix power management and control handling
media: i2c: imx334: Enable runtime PM before sub-device registration
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/imx334.c | 800 ++++++++++++++++---------------------
2 files changed, 353 insertions(+), 448 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/6] media: i2c: imx334: Simplify with dev_err_probe() 2025-03-10 7:17 [PATCH 0/6] media: i2c: imx334: Miscellaneous cleanups and improvements Tarang Raval @ 2025-03-10 7:17 ` Tarang Raval 2025-03-10 7:17 ` [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers Tarang Raval ` (4 subsequent siblings) 5 siblings, 0 replies; 21+ messages in thread From: Tarang Raval @ 2025-03-10 7:17 UTC (permalink / raw) To: sakari.ailus, kieran.bingham Cc: Shravan.Chippa, Tarang Raval, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, Julien Massot, AngeloGioacchino Del Regno, Zhi Mao, Luis Garcia, Kory Maincent, Benjamin Mugnier, linux-media, linux-kernel 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> --- 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.34.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers 2025-03-10 7:17 [PATCH 0/6] media: i2c: imx334: Miscellaneous cleanups and improvements Tarang Raval 2025-03-10 7:17 ` [PATCH 1/6] media: i2c: imx334: Simplify with dev_err_probe() Tarang Raval @ 2025-03-10 7:17 ` Tarang Raval 2025-03-11 6:14 ` Shravan.Chippa 2025-03-10 7:17 ` [PATCH 3/6] media: i2c: imx334: Remove redundant register entries Tarang Raval ` (3 subsequent siblings) 5 siblings, 1 reply; 21+ messages in thread From: Tarang Raval @ 2025-03-10 7:17 UTC (permalink / raw) To: sakari.ailus, kieran.bingham Cc: Shravan.Chippa, Tarang Raval, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, Umang Jain, Zhi Mao, Julien Massot, Mikhail Rudenko, Benjamin Mugnier, linux-media, linux-kernel 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. Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io> --- drivers/media/i2c/Kconfig | 1 + drivers/media/i2c/imx334.c | 698 ++++++++++++++++--------------------- 2 files changed, 295 insertions(+), 404 deletions(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 85ecb2aeefdb..c8138c1079fd 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..1fed03494813 100644 --- a/drivers/media/i2c/imx334.c +++ b/drivers/media/i2c/imx334.c @@ -12,42 +12,98 @@ #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_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_REG_ID CCI_REG8(0x3044) #define IMX334_ID 0x1e /* Exposure control */ -#define IMX334_REG_SHUTTER 0x3058 +#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_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 +/* 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 @@ -57,32 +113,22 @@ #define IMX334_REG_MAX 0xfffff /* Test Pattern Control */ -#define IMX334_REG_TP 0x329e +#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_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 CCI_REG8(0x32a0) #define IMX334_TPG_COLORW_120P 0x13 -#define IMX334_TP_CLK_EN 0x3148 +#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 @@ static int imx334_set_stream(struct v4l2_subdev *sd, int enable) 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.34.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers 2025-03-10 7:17 ` [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers Tarang Raval @ 2025-03-11 6:14 ` Shravan.Chippa 2025-03-11 6:38 ` Laurent Pinchart 0 siblings, 1 reply; 21+ messages in thread From: Shravan.Chippa @ 2025-03-11 6:14 UTC (permalink / raw) To: tarang.raval, sakari.ailus, kieran.bingham Cc: mchehab, hverkuil, laurent.pinchart, umang.jain, zhi.mao, julien.massot, mike.rudenko, benjamin.mugnier, linux-media, linux-kernel Hi Tarang, Thanks for the patch series with CCI register access helpers on top of my patches I have tested (1080p,720p, 480p resolution only) and working on my board with small PLL changes to make it compatible with pfsoc board (mpfs-video-kit). Acked-by: Shravan.Chippa@microchip.com Thanks, Shravan > -----Original Message----- > From: Tarang Raval <tarang.raval@siliconsignals.io> > Sent: Monday, March 10, 2025 12:48 PM > To: sakari.ailus@linux.intel.com; kieran.bingham@ideasonboard.com > Cc: shravan Chippa - I35088 <Shravan.Chippa@microchip.com>; Tarang Raval > <tarang.raval@siliconsignals.io>; Mauro Carvalho Chehab > <mchehab@kernel.org>; Hans Verkuil <hverkuil@xs4all.nl>; Laurent Pinchart > <laurent.pinchart@ideasonboard.com>; Umang Jain > <umang.jain@ideasonboard.com>; Zhi Mao <zhi.mao@mediatek.com>; Julien > Massot <julien.massot@collabora.com>; Mikhail Rudenko > <mike.rudenko@gmail.com>; Benjamin Mugnier > <benjamin.mugnier@foss.st.com>; linux-media@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access > helpers > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > 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. > > Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io> > --- > drivers/media/i2c/Kconfig | 1 + > drivers/media/i2c/imx334.c | 698 ++++++++++++++++--------------------- > 2 files changed, 295 insertions(+), 404 deletions(-) > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index > 85ecb2aeefdb..c8138c1079fd 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..1fed03494813 100644 > --- a/drivers/media/i2c/imx334.c > +++ b/drivers/media/i2c/imx334.c > @@ -12,42 +12,98 @@ > #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_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_REG_ID CCI_REG8(0x3044) > #define IMX334_ID 0x1e > > /* Exposure control */ > -#define IMX334_REG_SHUTTER 0x3058 > +#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_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 > > +/* 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 @@ -57,32 +113,22 @@ > #define IMX334_REG_MAX 0xfffff > > /* Test Pattern Control */ > -#define IMX334_REG_TP 0x329e > +#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_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 CCI_REG8(0x32a0) > #define IMX334_TPG_COLORW_120P 0x13 > > -#define IMX334_TP_CLK_EN 0x3148 > +#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 @@ static int imx334_set_stream(struct v4l2_subdev > *sd, int enable) 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.34.1 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers 2025-03-11 6:14 ` Shravan.Chippa @ 2025-03-11 6:38 ` Laurent Pinchart 2025-03-11 6:51 ` Shravan.Chippa 0 siblings, 1 reply; 21+ messages in thread From: Laurent Pinchart @ 2025-03-11 6:38 UTC (permalink / raw) To: Shravan.Chippa Cc: tarang.raval, sakari.ailus, kieran.bingham, mchehab, hverkuil, umang.jain, zhi.mao, julien.massot, mike.rudenko, benjamin.mugnier, linux-media, linux-kernel Hi Shravan, On Tue, Mar 11, 2025 at 06:14:28AM +0000, Shravan.Chippa@microchip.com wrote: > Hi Tarang, > > Thanks for the patch series with CCI register access helpers on top of my patches > I have tested (1080p,720p, 480p resolution only) and working on my > board with small PLL changes to make it compatible with pfsoc board > (mpfs-video-kit). Could you please provide more information about what those PLL changes are ? > > Acked-by: Shravan.Chippa@microchip.com > > Thanks, > Shravan > > > -----Original Message----- > > From: Tarang Raval <tarang.raval@siliconsignals.io> > > Sent: Monday, March 10, 2025 12:48 PM > > To: sakari.ailus@linux.intel.com; kieran.bingham@ideasonboard.com > > Cc: shravan Chippa - I35088 <Shravan.Chippa@microchip.com>; Tarang Raval > > <tarang.raval@siliconsignals.io>; Mauro Carvalho Chehab > > <mchehab@kernel.org>; Hans Verkuil <hverkuil@xs4all.nl>; Laurent Pinchart > > <laurent.pinchart@ideasonboard.com>; Umang Jain > > <umang.jain@ideasonboard.com>; Zhi Mao <zhi.mao@mediatek.com>; Julien > > Massot <julien.massot@collabora.com>; Mikhail Rudenko > > <mike.rudenko@gmail.com>; Benjamin Mugnier > > <benjamin.mugnier@foss.st.com>; linux-media@vger.kernel.org; linux- > > kernel@vger.kernel.org > > Subject: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access > > helpers > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > content is safe > > > > 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. > > > > Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io> > > --- > > drivers/media/i2c/Kconfig | 1 + > > drivers/media/i2c/imx334.c | 698 ++++++++++++++++--------------------- > > 2 files changed, 295 insertions(+), 404 deletions(-) > > > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index > > 85ecb2aeefdb..c8138c1079fd 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..1fed03494813 100644 > > --- a/drivers/media/i2c/imx334.c > > +++ b/drivers/media/i2c/imx334.c > > @@ -12,42 +12,98 @@ > > #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_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_REG_ID CCI_REG8(0x3044) > > #define IMX334_ID 0x1e > > > > /* Exposure control */ > > -#define IMX334_REG_SHUTTER 0x3058 > > +#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_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 > > > > +/* 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 @@ -57,32 +113,22 @@ > > #define IMX334_REG_MAX 0xfffff > > > > /* Test Pattern Control */ > > -#define IMX334_REG_TP 0x329e > > +#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_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 CCI_REG8(0x32a0) > > #define IMX334_TPG_COLORW_120P 0x13 > > > > -#define IMX334_TP_CLK_EN 0x3148 > > +#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 @@ static int imx334_set_stream(struct v4l2_subdev > > *sd, int enable) 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); -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers 2025-03-11 6:38 ` Laurent Pinchart @ 2025-03-11 6:51 ` Shravan.Chippa 2025-03-11 9:46 ` Sakari Ailus 0 siblings, 1 reply; 21+ messages in thread From: Shravan.Chippa @ 2025-03-11 6:51 UTC (permalink / raw) To: laurent.pinchart Cc: tarang.raval, sakari.ailus, kieran.bingham, mchehab, hverkuil, umang.jain, zhi.mao, julien.massot, mike.rudenko, benjamin.mugnier, linux-media, linux-kernel Hi Laurent, > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Tuesday, March 11, 2025 12:09 PM > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > Cc: tarang.raval@siliconsignals.io; sakari.ailus@linux.intel.com; > kieran.bingham@ideasonboard.com; mchehab@kernel.org; > hverkuil@xs4all.nl; umang.jain@ideasonboard.com; zhi.mao@mediatek.com; > julien.massot@collabora.com; mike.rudenko@gmail.com; > benjamin.mugnier@foss.st.com; linux-media@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access > helpers > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Hi Shravan, > > On Tue, Mar 11, 2025 at 06:14:28AM +0000, Shravan.Chippa@microchip.com > wrote: > > Hi Tarang, > > > > Thanks for the patch series with CCI register access helpers on top of > > my patches I have tested (1080p,720p, 480p resolution only) and > > working on my board with small PLL changes to make it compatible with > > pfsoc board (mpfs-video-kit). > > Could you please provide more information about what those PLL changes are > ? Here is the change for mpfs-video-kit board. diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c index 375367314416..30470dbd1f3c 100644 --- a/drivers/media/i2c/imx334.c +++ b/drivers/media/i2c/imx334.c @@ -236,9 +236,9 @@ static const struct cci_reg_sequence common_mode_regs[] = { { IMX334_REG_XVS_XHS_OUTSEL, 0x20}, { IMX334_REG_XVS_XHS_DRV, 0x0f}, { IMX334_REG_BCWAIT_TIME, 0x3b}, - { IMX334_REG_CPWAIT_TIME, 0x2a}, + { IMX334_REG_CPWAIT_TIME, 0x29}, { IMX334_REG_INCKSEL1, 0x0129}, - { IMX334_REG_INCKSEL2, 0x06}, + { IMX334_REG_INCKSEL2, 0x0a}, { IMX334_REG_INCKSEL3, 0xa0}, { IMX334_REG_INCKSEL4, 0x7e}, { IMX334_REG_SYS_MODE, 0x02}, Thanks, Shravan > > > > > Acked-by: Shravan.Chippa@microchip.com > > > > Thanks, > > Shravan > > > > > -----Original Message----- > > > From: Tarang Raval <tarang.raval@siliconsignals.io> > > > Sent: Monday, March 10, 2025 12:48 PM > > > To: sakari.ailus@linux.intel.com; kieran.bingham@ideasonboard.com > > > Cc: shravan Chippa - I35088 <Shravan.Chippa@microchip.com>; Tarang > > > Raval <tarang.raval@siliconsignals.io>; Mauro Carvalho Chehab > > > <mchehab@kernel.org>; Hans Verkuil <hverkuil@xs4all.nl>; Laurent > > > Pinchart <laurent.pinchart@ideasonboard.com>; Umang Jain > > > <umang.jain@ideasonboard.com>; Zhi Mao <zhi.mao@mediatek.com>; > > > Julien Massot <julien.massot@collabora.com>; Mikhail Rudenko > > > <mike.rudenko@gmail.com>; Benjamin Mugnier > > > <benjamin.mugnier@foss.st.com>; linux-media@vger.kernel.org; linux- > > > kernel@vger.kernel.org > > > Subject: [PATCH 2/6] media: i2c: imx334: Convert to CCI register > > > access helpers > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > > know the content is safe > > > > > > 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. > > > > > > Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io> > > > --- > > > drivers/media/i2c/Kconfig | 1 + > > > drivers/media/i2c/imx334.c | 698 > > > ++++++++++++++++--------------------- > > > 2 files changed, 295 insertions(+), 404 deletions(-) > > > > > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > > > index 85ecb2aeefdb..c8138c1079fd 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..1fed03494813 100644 > > > --- a/drivers/media/i2c/imx334.c > > > +++ b/drivers/media/i2c/imx334.c > > > @@ -12,42 +12,98 @@ > > > #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_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_REG_ID CCI_REG8(0x3044) > > > #define IMX334_ID 0x1e > > > > > > /* Exposure control */ > > > -#define IMX334_REG_SHUTTER 0x3058 > > > +#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_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 > > > > > > +/* 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 @@ -57,32 +113,22 @@ > > > #define IMX334_REG_MAX 0xfffff > > > > > > /* Test Pattern Control */ > > > -#define IMX334_REG_TP 0x329e > > > +#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_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 CCI_REG8(0x32a0) > > > #define IMX334_TPG_COLORW_120P 0x13 > > > > > > -#define IMX334_TP_CLK_EN 0x3148 > > > +#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 @@ static int imx334_set_stream(struct > > > v4l2_subdev *sd, int enable) 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); > > -- > Regards, > > Laurent Pinchart ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers 2025-03-11 6:51 ` Shravan.Chippa @ 2025-03-11 9:46 ` Sakari Ailus 2025-03-11 9:59 ` Shravan.Chippa 2025-03-11 10:05 ` Tarang Raval 0 siblings, 2 replies; 21+ messages in thread From: Sakari Ailus @ 2025-03-11 9:46 UTC (permalink / raw) To: Shravan.Chippa Cc: laurent.pinchart, tarang.raval, kieran.bingham, mchehab, hverkuil, umang.jain, zhi.mao, julien.massot, mike.rudenko, benjamin.mugnier, linux-media, linux-kernel Hi Shravan, Tarang, On Tue, Mar 11, 2025 at 06:51:48AM +0000, Shravan.Chippa@microchip.com wrote: > Hi Laurent, > > > -----Original Message----- > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Sent: Tuesday, March 11, 2025 12:09 PM > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > Cc: tarang.raval@siliconsignals.io; sakari.ailus@linux.intel.com; > > kieran.bingham@ideasonboard.com; mchehab@kernel.org; > > hverkuil@xs4all.nl; umang.jain@ideasonboard.com; zhi.mao@mediatek.com; > > julien.massot@collabora.com; mike.rudenko@gmail.com; > > benjamin.mugnier@foss.st.com; linux-media@vger.kernel.org; linux- > > kernel@vger.kernel.org > > Subject: Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access > > helpers > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > content is safe > > > > Hi Shravan, > > > > On Tue, Mar 11, 2025 at 06:14:28AM +0000, Shravan.Chippa@microchip.com > > wrote: > > > Hi Tarang, > > > > > > Thanks for the patch series with CCI register access helpers on top of > > > my patches I have tested (1080p,720p, 480p resolution only) and > > > working on my board with small PLL changes to make it compatible with > > > pfsoc board (mpfs-video-kit). > > > > Could you please provide more information about what those PLL changes are > > ? > > Here is the change for mpfs-video-kit board. > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > index 375367314416..30470dbd1f3c 100644 > --- a/drivers/media/i2c/imx334.c > +++ b/drivers/media/i2c/imx334.c > @@ -236,9 +236,9 @@ static const struct cci_reg_sequence common_mode_regs[] = { > { IMX334_REG_XVS_XHS_OUTSEL, 0x20}, > { IMX334_REG_XVS_XHS_DRV, 0x0f}, > { IMX334_REG_BCWAIT_TIME, 0x3b}, > - { IMX334_REG_CPWAIT_TIME, 0x2a}, > + { IMX334_REG_CPWAIT_TIME, 0x29}, A patch converting the driver to use V4L2 CCI / human-readable register names should not change the values written. This change is exactly the same than your 2nd patch does. It'd be good to understand why it is different and what is effect of that difference. > { IMX334_REG_INCKSEL1, 0x0129}, > - { IMX334_REG_INCKSEL2, 0x06}, > + { IMX334_REG_INCKSEL2, 0x0a}, This is a bigger change indeed. > { IMX334_REG_INCKSEL3, 0xa0}, > { IMX334_REG_INCKSEL4, 0x7e}, > { IMX334_REG_SYS_MODE, 0x02}, -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers 2025-03-11 9:46 ` Sakari Ailus @ 2025-03-11 9:59 ` Shravan.Chippa 2025-03-11 10:05 ` Tarang Raval 1 sibling, 0 replies; 21+ messages in thread From: Shravan.Chippa @ 2025-03-11 9:59 UTC (permalink / raw) To: sakari.ailus Cc: laurent.pinchart, tarang.raval, kieran.bingham, mchehab, hverkuil, umang.jain, zhi.mao, julien.massot, mike.rudenko, benjamin.mugnier, linux-media, linux-kernel Hi Sakari , > -----Original Message----- > From: Sakari Ailus <sakari.ailus@linux.intel.com> > Sent: Tuesday, March 11, 2025 3:17 PM > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > Cc: laurent.pinchart@ideasonboard.com; tarang.raval@siliconsignals.io; > kieran.bingham@ideasonboard.com; mchehab@kernel.org; > hverkuil@xs4all.nl; umang.jain@ideasonboard.com; zhi.mao@mediatek.com; > julien.massot@collabora.com; mike.rudenko@gmail.com; > benjamin.mugnier@foss.st.com; linux-media@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access > helpers > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Hi Shravan, Tarang, > > On Tue, Mar 11, 2025 at 06:51:48AM +0000, Shravan.Chippa@microchip.com > wrote: > > Hi Laurent, > > > > > -----Original Message----- > > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Sent: Tuesday, March 11, 2025 12:09 PM > > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > > Cc: tarang.raval@siliconsignals.io; sakari.ailus@linux.intel.com; > > > kieran.bingham@ideasonboard.com; mchehab@kernel.org; > > > hverkuil@xs4all.nl; umang.jain@ideasonboard.com; > > > zhi.mao@mediatek.com; julien.massot@collabora.com; > > > mike.rudenko@gmail.com; benjamin.mugnier@foss.st.com; > > > linux-media@vger.kernel.org; linux- kernel@vger.kernel.org > > > Subject: Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register > > > access helpers > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > > know the content is safe > > > > > > Hi Shravan, > > > > > > On Tue, Mar 11, 2025 at 06:14:28AM +0000, > > > Shravan.Chippa@microchip.com > > > wrote: > > > > Hi Tarang, > > > > > > > > Thanks for the patch series with CCI register access helpers on > > > > top of my patches I have tested (1080p,720p, 480p resolution only) > > > > and working on my board with small PLL changes to make it > > > > compatible with pfsoc board (mpfs-video-kit). > > > > > > Could you please provide more information about what those PLL > > > changes are ? > > > > Here is the change for mpfs-video-kit board. > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > index 375367314416..30470dbd1f3c 100644 > > --- a/drivers/media/i2c/imx334.c > > +++ b/drivers/media/i2c/imx334.c > > @@ -236,9 +236,9 @@ static const struct cci_reg_sequence > common_mode_regs[] = { > > { IMX334_REG_XVS_XHS_OUTSEL, 0x20}, > > { IMX334_REG_XVS_XHS_DRV, 0x0f}, > > { IMX334_REG_BCWAIT_TIME, 0x3b}, > > - { IMX334_REG_CPWAIT_TIME, 0x2a}, > > + { IMX334_REG_CPWAIT_TIME, 0x29}, > > A patch converting the driver to use V4L2 CCI / human-readable register > names should not change the values written. > > This change is exactly the same than your 2nd patch does. It'd be good to > understand why it is different and what is effect of that difference. > > > { IMX334_REG_INCKSEL1, 0x0129}, > > - { IMX334_REG_INCKSEL2, 0x06}, > > + { IMX334_REG_INCKSEL2, 0x0a}, > > This is a bigger change indeed. > > > { IMX334_REG_INCKSEL3, 0xa0}, > > { IMX334_REG_INCKSEL4, 0x7e}, > > { IMX334_REG_SYS_MODE, 0x02}, Whatever I have send is as per the data sheet. This should work on other board As per my understanding these changes are pfsoc mpfs-video-kit specific. The video pipeline is not able to process the data very fast, this change will make incoming stream little slow, so that video pipeline will not hang. Thanks, Shravan > > -- > Regards, > > Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers 2025-03-11 9:46 ` Sakari Ailus 2025-03-11 9:59 ` Shravan.Chippa @ 2025-03-11 10:05 ` Tarang Raval 2025-03-11 10:13 ` Sakari Ailus 1 sibling, 1 reply; 21+ messages in thread From: Tarang Raval @ 2025-03-11 10:05 UTC (permalink / raw) To: Sakari Ailus, Shravan.Chippa@microchip.com Cc: laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, mchehab@kernel.org, hverkuil@xs4all.nl, umang.jain@ideasonboard.com, zhi.mao@mediatek.com, julien.massot@collabora.com, mike.rudenko@gmail.com, benjamin.mugnier@foss.st.com, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Hi Sakari, > On Tue, Mar 11, 2025 at 06:51:48AM +0000, Shravan.Chippa@microchip.com wrote: > > Hi Laurent, > > > > > -----Original Message----- > > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Sent: Tuesday, March 11, 2025 12:09 PM > > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > > Cc: tarang.raval@siliconsignals.io; sakari.ailus@linux.intel.com; > > > kieran.bingham@ideasonboard.com; mchehab@kernel.org; > > > hverkuil@xs4all.nl; umang.jain@ideasonboard.com; zhi.mao@mediatek.com; > > > julien.massot@collabora.com; mike.rudenko@gmail.com; > > > benjamin.mugnier@foss.st.com; linux-media@vger.kernel.org; linux- > > > kernel@vger.kernel.org > > > Subject: Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access > > > helpers > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > > content is safe > > > > > > Hi Shravan, > > > > > > On Tue, Mar 11, 2025 at 06:14:28AM +0000, Shravan.Chippa@microchip.com > > > wrote: > > > > Hi Tarang, > > > > > > > > Thanks for the patch series with CCI register access helpers on top of > > > > my patches I have tested (1080p,720p, 480p resolution only) and > > > > working on my board with small PLL changes to make it compatible with > > > > pfsoc board (mpfs-video-kit). > > > > > > Could you please provide more information about what those PLL changes are > > > ? > > > > Here is the change for mpfs-video-kit board. > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > index 375367314416..30470dbd1f3c 100644 > > --- a/drivers/media/i2c/imx334.c > > +++ b/drivers/media/i2c/imx334.c > > @@ -236,9 +236,9 @@ static const struct cci_reg_sequence common_mode_regs[] = { > > { IMX334_REG_XVS_XHS_OUTSEL, 0x20}, > > { IMX334_REG_XVS_XHS_DRV, 0x0f}, > > { IMX334_REG_BCWAIT_TIME, 0x3b}, > > - { IMX334_REG_CPWAIT_TIME, 0x2a}, > > + { IMX334_REG_CPWAIT_TIME, 0x29}, > > A patch converting the driver to use V4L2 CCI / human-readable register > names should not change the values written. This change is not from my patch. You can recheck Shravan’s second patch, there is no change in the CPWAIT_TIME value from my patch or his. The change Shravan mentioned was made locally to ensure compatibility with the mpfs-video-kit board, as Laurent requested. That’s why he provided those details. Best Regards, Tarang > This change is exactly the same than your 2nd patch does. It'd be good to > understand why it is different and what is effect of that difference. > > > { IMX334_REG_INCKSEL1, 0x0129}, > > - { IMX334_REG_INCKSEL2, 0x06}, > > + { IMX334_REG_INCKSEL2, 0x0a}, > > This is a bigger change indeed. > > > { IMX334_REG_INCKSEL3, 0xa0}, > > { IMX334_REG_INCKSEL4, 0x7e}, > > { IMX334_REG_SYS_MODE, 0x02}, > > -- > Regards, > > > Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers 2025-03-11 10:05 ` Tarang Raval @ 2025-03-11 10:13 ` Sakari Ailus 0 siblings, 0 replies; 21+ messages in thread From: Sakari Ailus @ 2025-03-11 10:13 UTC (permalink / raw) To: Tarang Raval Cc: Shravan.Chippa@microchip.com, laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, mchehab@kernel.org, hverkuil@xs4all.nl, umang.jain@ideasonboard.com, zhi.mao@mediatek.com, julien.massot@collabora.com, mike.rudenko@gmail.com, benjamin.mugnier@foss.st.com, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Hi Tarang, On Tue, Mar 11, 2025 at 10:05:12AM +0000, Tarang Raval wrote: > Hi Sakari, > > > On Tue, Mar 11, 2025 at 06:51:48AM +0000, Shravan.Chippa@microchip.com wrote: > > > Hi Laurent, > > > > > > > -----Original Message----- > > > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Sent: Tuesday, March 11, 2025 12:09 PM > > > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com> > > > > Cc: tarang.raval@siliconsignals.io; sakari.ailus@linux.intel.com; > > > > kieran.bingham@ideasonboard.com; mchehab@kernel.org; > > > > hverkuil@xs4all.nl; umang.jain@ideasonboard.com; zhi.mao@mediatek.com; > > > > julien.massot@collabora.com; mike.rudenko@gmail.com; > > > > benjamin.mugnier@foss.st.com; linux-media@vger.kernel.org; linux- > > > > kernel@vger.kernel.org > > > > Subject: Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access > > > > helpers > > > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > > > content is safe > > > > > > > > Hi Shravan, > > > > > > > > On Tue, Mar 11, 2025 at 06:14:28AM +0000, Shravan.Chippa@microchip.com > > > > wrote: > > > > > Hi Tarang, > > > > > > > > > > Thanks for the patch series with CCI register access helpers on top of > > > > > my patches I have tested (1080p,720p, 480p resolution only) and > > > > > working on my board with small PLL changes to make it compatible with > > > > > pfsoc board (mpfs-video-kit). > > > > > > > > Could you please provide more information about what those PLL changes are > > > > ? > > > > > > Here is the change for mpfs-video-kit board. > > > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > > index 375367314416..30470dbd1f3c 100644 > > > --- a/drivers/media/i2c/imx334.c > > > +++ b/drivers/media/i2c/imx334.c > > > @@ -236,9 +236,9 @@ static const struct cci_reg_sequence common_mode_regs[] = { > > > { IMX334_REG_XVS_XHS_OUTSEL, 0x20}, > > > { IMX334_REG_XVS_XHS_DRV, 0x0f}, > > > { IMX334_REG_BCWAIT_TIME, 0x3b}, > > > - { IMX334_REG_CPWAIT_TIME, 0x2a}, > > > + { IMX334_REG_CPWAIT_TIME, 0x29}, > > > > A patch converting the driver to use V4L2 CCI / human-readable register > > names should not change the values written. > > This change is not from my patch. > > You can recheck Shravan’s second patch, there is no change in the > CPWAIT_TIME value from my patch or his. > > The change Shravan mentioned was made locally to ensure compatibility > with the mpfs-video-kit board, as Laurent requested. > That’s why he provided those details. Right. I don't know what this or the other change do exactly, but presumably the other one is a clock tree divider that reduces the link frequency (or pixel rate in pixel array). That should be handled separately. -- Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/6] media: i2c: imx334: Remove redundant register entries 2025-03-10 7:17 [PATCH 0/6] media: i2c: imx334: Miscellaneous cleanups and improvements Tarang Raval 2025-03-10 7:17 ` [PATCH 1/6] media: i2c: imx334: Simplify with dev_err_probe() Tarang Raval 2025-03-10 7:17 ` [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers Tarang Raval @ 2025-03-10 7:17 ` Tarang Raval 2025-03-10 7:17 ` [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes Tarang Raval ` (2 subsequent siblings) 5 siblings, 0 replies; 21+ messages in thread From: Tarang Raval @ 2025-03-10 7:17 UTC (permalink / raw) To: sakari.ailus, kieran.bingham Cc: Shravan.Chippa, Tarang Raval, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, Julien Massot, Umang Jain, Zhi Mao, Luis Garcia, Mikhail Rudenko, Benjamin Mugnier, linux-media, linux-kernel 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> --- 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 1fed03494813..24ccfd1d0986 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.34.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes 2025-03-10 7:17 [PATCH 0/6] media: i2c: imx334: Miscellaneous cleanups and improvements Tarang Raval ` (2 preceding siblings ...) 2025-03-10 7:17 ` [PATCH 3/6] media: i2c: imx334: Remove redundant register entries Tarang Raval @ 2025-03-10 7:17 ` Tarang Raval 2025-03-27 10:09 ` Sakari Ailus 2025-03-10 7:17 ` [PATCH 5/6] media: i2c: imx334: Fix power management and control handling Tarang Raval 2025-03-10 7:17 ` [PATCH 6/6] media: i2c: imx334: Enable runtime PM before sub-device registration Tarang Raval 5 siblings, 1 reply; 21+ messages in thread From: Tarang Raval @ 2025-03-10 7:17 UTC (permalink / raw) To: sakari.ailus, kieran.bingham Cc: Shravan.Chippa, Tarang Raval, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, Umang Jain, Zhi Mao, Julien Massot, Luis Garcia, Benjamin Mugnier, linux-media, linux-kernel imx334 can support both 4 and 8 lane configurations. Extend the driver to configure the lane mode accordingly. Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io> --- drivers/media/i2c/imx334.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c index 24ccfd1d0986..23bfc64969cc 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) @@ -107,7 +109,6 @@ /* CSI2 HW configuration */ #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 @@ -181,6 +182,7 @@ struct imx334_mode { * @exp_ctrl: Pointer to exposure control * @again_ctrl: Pointer to analog gain control * @vblank: Vertical blanking in lines + * @lane_mode: Mode for number of connected data lanes * @cur_mode: Pointer to current selected sensor mode * @mutex: Mutex for serializing sensor controls * @link_freq_bitmap: Menu bitmap for link_freq_ctrl @@ -204,6 +206,7 @@ struct imx334 { struct v4l2_ctrl *again_ctrl; }; u32 vblank; + u32 lane_mode; const struct imx334_mode *cur_mode; struct mutex mutex; unsigned long link_freq_bitmap; @@ -240,7 +243,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 +878,13 @@ static int imx334_start_streaming(struct imx334 *imx334) return ret; } + ret = cci_write(imx334->cci, IMX334_REG_LANEMODE, + imx334->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", @@ -1022,7 +1031,14 @@ static int imx334_parse_hw_config(struct imx334 *imx334) if (ret) return ret; - if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX334_NUM_DATA_LANES) { + switch (bus_cfg.bus.mipi_csi2.num_data_lanes) { + case 4: + imx334->lane_mode = IMX334_CSI_4_LANE_MODE; + break; + case 8: + imx334->lane_mode = IMX334_CSI_8_LANE_MODE; + break; + default: dev_err(imx334->dev, "number of CSI2 data lanes %d is not supported\n", bus_cfg.bus.mipi_csi2.num_data_lanes); -- 2.34.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes 2025-03-10 7:17 ` [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes Tarang Raval @ 2025-03-27 10:09 ` Sakari Ailus 2025-03-27 10:54 ` Tarang Raval 0 siblings, 1 reply; 21+ messages in thread From: Sakari Ailus @ 2025-03-27 10:09 UTC (permalink / raw) To: Tarang Raval Cc: kieran.bingham, Shravan.Chippa, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, Umang Jain, Zhi Mao, Julien Massot, Luis Garcia, Benjamin Mugnier, linux-media, linux-kernel Hi Tarang, On Mon, Mar 10, 2025 at 12:47:46PM +0530, Tarang Raval wrote: > imx334 can support both 4 and 8 lane configurations. > Extend the driver to configure the lane mode accordingly. > > Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io> > --- > drivers/media/i2c/imx334.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > index 24ccfd1d0986..23bfc64969cc 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) > @@ -107,7 +109,6 @@ > /* CSI2 HW configuration */ > #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 > @@ -181,6 +182,7 @@ struct imx334_mode { > * @exp_ctrl: Pointer to exposure control > * @again_ctrl: Pointer to analog gain control > * @vblank: Vertical blanking in lines > + * @lane_mode: Mode for number of connected data lanes > * @cur_mode: Pointer to current selected sensor mode > * @mutex: Mutex for serializing sensor controls > * @link_freq_bitmap: Menu bitmap for link_freq_ctrl > @@ -204,6 +206,7 @@ struct imx334 { > struct v4l2_ctrl *again_ctrl; > }; > u32 vblank; > + u32 lane_mode; > const struct imx334_mode *cur_mode; > struct mutex mutex; > unsigned long link_freq_bitmap; > @@ -240,7 +243,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}, Not a fault of this patch but also the closing brace should have a space before it. Could you address it in the earlier patches? > { IMX334_REG_TCLKPOST, 0x007f}, > { IMX334_REG_TCLKPREPARE, 0x0037}, > { IMX334_REG_TCLKTRAIL, 0x0037}, > @@ -876,6 +878,13 @@ static int imx334_start_streaming(struct imx334 *imx334) > return ret; > } > > + ret = cci_write(imx334->cci, IMX334_REG_LANEMODE, > + imx334->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", > @@ -1022,7 +1031,14 @@ static int imx334_parse_hw_config(struct imx334 *imx334) > if (ret) > return ret; > > - if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX334_NUM_DATA_LANES) { > + switch (bus_cfg.bus.mipi_csi2.num_data_lanes) { > + case 4: > + imx334->lane_mode = IMX334_CSI_4_LANE_MODE; > + break; > + case 8: > + imx334->lane_mode = IMX334_CSI_8_LANE_MODE; Doesn't this affect the PLL configuration? Presumably higher frame rates could be achieved at least. > + break; > + default: > dev_err(imx334->dev, > "number of CSI2 data lanes %d is not supported\n", > bus_cfg.bus.mipi_csi2.num_data_lanes); -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes 2025-03-27 10:09 ` Sakari Ailus @ 2025-03-27 10:54 ` Tarang Raval 2025-03-27 14:32 ` Dave Stevenson 0 siblings, 1 reply; 21+ messages in thread From: Tarang Raval @ 2025-03-27 10:54 UTC (permalink / raw) To: Sakari Ailus Cc: kieran.bingham@ideasonboard.com, Shravan.Chippa@microchip.com, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, Umang Jain, Zhi Mao, Julien Massot, Luis Garcia, Benjamin Mugnier, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Hi Sakari, Thanks for the review. > On Mon, Mar 10, 2025 at 12:47:46PM +0530, Tarang Raval wrote: > > imx334 can support both 4 and 8 lane configurations. > > Extend the driver to configure the lane mode accordingly. > > > > Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io> > > --- > > drivers/media/i2c/imx334.c | 22 +++++++++++++++++++--- > > 1 file changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > index 24ccfd1d0986..23bfc64969cc 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) > > @@ -107,7 +109,6 @@ > > /* CSI2 HW configuration */ > > #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 > > @@ -181,6 +182,7 @@ struct imx334_mode { > > * @exp_ctrl: Pointer to exposure control > > * @again_ctrl: Pointer to analog gain control > > * @vblank: Vertical blanking in lines > > + * @lane_mode: Mode for number of connected data lanes > > * @cur_mode: Pointer to current selected sensor mode > > * @mutex: Mutex for serializing sensor controls > > * @link_freq_bitmap: Menu bitmap for link_freq_ctrl > > @@ -204,6 +206,7 @@ struct imx334 { > > struct v4l2_ctrl *again_ctrl; > > }; > > u32 vblank; > > + u32 lane_mode; > > const struct imx334_mode *cur_mode; > > struct mutex mutex; > > unsigned long link_freq_bitmap; > > @@ -240,7 +243,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}, > > Not a fault of this patch but also the closing brace should have a space > before it. Could you address it in the earlier patches? Okay, I will correct it. > > { IMX334_REG_TCLKPOST, 0x007f}, > > { IMX334_REG_TCLKPREPARE, 0x0037}, > > { IMX334_REG_TCLKTRAIL, 0x0037}, > > @@ -876,6 +878,13 @@ static int imx334_start_streaming(struct imx334 *imx334) > > return ret; > > } > > > > + ret = cci_write(imx334->cci, IMX334_REG_LANEMODE, > > + imx334->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", > > @@ -1022,7 +1031,14 @@ static int imx334_parse_hw_config(struct imx334 *imx334) > > if (ret) > > return ret; > > > > - if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX334_NUM_DATA_LANES) { > > + switch (bus_cfg.bus.mipi_csi2.num_data_lanes) { > > + case 4: > > + imx334->lane_mode = IMX334_CSI_4_LANE_MODE; > > + break; > > + case 8: > > + imx334->lane_mode = IMX334_CSI_8_LANE_MODE; > > Doesn't this affect the PLL configuration? Presumably higher frame rates > could be achieved at least. Sorry, my commit message is misleading. The intention of this patch is to configure the lane mode dynamically from the streaming function instead of using a hardcoded value. You are correct that supporting an 8-lane mode requires changes to the PLL configuration. This patch does not address that aspect yet. Best Regards, Tarang > > + break; > > + default: > > dev_err(imx334->dev, > > "number of CSI2 data lanes %d is not supported\n", > > bus_cfg.bus.mipi_csi2.num_data_lanes); > > -- > Regards, > > Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes 2025-03-27 10:54 ` Tarang Raval @ 2025-03-27 14:32 ` Dave Stevenson 2025-03-27 15:38 ` Tarang Raval 0 siblings, 1 reply; 21+ messages in thread From: Dave Stevenson @ 2025-03-27 14:32 UTC (permalink / raw) To: Tarang Raval Cc: Sakari Ailus, kieran.bingham@ideasonboard.com, Shravan.Chippa@microchip.com, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, Umang Jain, Zhi Mao, Julien Massot, Luis Garcia, Benjamin Mugnier, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Hi Tarang & Sakari On Thu, 27 Mar 2025 at 10:55, Tarang Raval <tarang.raval@siliconsignals.io> wrote: > > Hi Sakari, > > Thanks for the review. > > > On Mon, Mar 10, 2025 at 12:47:46PM +0530, Tarang Raval wrote: > > > imx334 can support both 4 and 8 lane configurations. > > > Extend the driver to configure the lane mode accordingly. > > > > > > Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io> > > > --- > > > drivers/media/i2c/imx334.c | 22 +++++++++++++++++++--- > > > 1 file changed, 19 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > > index 24ccfd1d0986..23bfc64969cc 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) > > > @@ -107,7 +109,6 @@ > > > /* CSI2 HW configuration */ > > > #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 > > > @@ -181,6 +182,7 @@ struct imx334_mode { > > > * @exp_ctrl: Pointer to exposure control > > > * @again_ctrl: Pointer to analog gain control > > > * @vblank: Vertical blanking in lines > > > + * @lane_mode: Mode for number of connected data lanes > > > * @cur_mode: Pointer to current selected sensor mode > > > * @mutex: Mutex for serializing sensor controls > > > * @link_freq_bitmap: Menu bitmap for link_freq_ctrl > > > @@ -204,6 +206,7 @@ struct imx334 { > > > struct v4l2_ctrl *again_ctrl; > > > }; > > > u32 vblank; > > > + u32 lane_mode; > > > const struct imx334_mode *cur_mode; > > > struct mutex mutex; > > > unsigned long link_freq_bitmap; > > > @@ -240,7 +243,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}, > > > > Not a fault of this patch but also the closing brace should have a space > > before it. Could you address it in the earlier patches? > > Okay, I will correct it. > > > > { IMX334_REG_TCLKPOST, 0x007f}, > > > { IMX334_REG_TCLKPREPARE, 0x0037}, > > > { IMX334_REG_TCLKTRAIL, 0x0037}, > > > @@ -876,6 +878,13 @@ static int imx334_start_streaming(struct imx334 *imx334) > > > return ret; > > > } > > > > > > + ret = cci_write(imx334->cci, IMX334_REG_LANEMODE, > > > + imx334->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", > > > @@ -1022,7 +1031,14 @@ static int imx334_parse_hw_config(struct imx334 *imx334) > > > if (ret) > > > return ret; > > > > > > - if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX334_NUM_DATA_LANES) { > > > + switch (bus_cfg.bus.mipi_csi2.num_data_lanes) { > > > + case 4: > > > + imx334->lane_mode = IMX334_CSI_4_LANE_MODE; > > > + break; > > > + case 8: > > > + imx334->lane_mode = IMX334_CSI_8_LANE_MODE; > > > > Doesn't this affect the PLL configuration? Presumably higher frame rates > > could be achieved at least. > > Sorry, my commit message is misleading. The intention of this patch is to > configure the lane mode dynamically from the streaming function instead > of using a hardcoded value. > > You are correct that supporting an 8-lane mode requires changes to the PLL > configuration. This patch does not address that aspect yet. Is it actually required, or just a nicety? The datasheet [1] says: "Maximum frame rate in All-pixel scan mode 3840(H)×2160(V) AD12bit: 60 frame / s" The current driver configuration for the 3840x2160 mode is a pixel clock 594MHz with total timings of (3840+560) x (2160+90), which gives a framerate of 60fps. So you already have the maximum capabilities of the sensor exposed. Adding the 8 lane mode gives you the option to run at half the link frequency of the 4 lane, but Sony Starvis sensors have a FIFO between pixel array and MIPI block. All the other Starvis sensors I've encountered are quite happy at any of the link frequencies as long as the horizontal blanking makes the line period sufficient to send each line. The datasheet does say "The bit rate maximum value are 1782 Mbps / Lane in 4 Lane mode and 1188 Mbps / Lane in 8 Lane mode. " (page 78 "CSI-2 output"), but then also "The maximum bit rate of each Lane are 1782 Mbps / Lane." (page 81 "MIPI transmitter"). Surely all lanes can either do 1782Mbps, or they can't. They won't have downrated just lanes 5-8. Presumably it works at 1782Mbps/lane in 8 lane mode or you wouldn't have submitted the patch, We've been here before with the imx290 and imx415 drivers and what can be supported with each combination of lanes and link frequency. Cheers Dave [1] https://en.sunnywale.com/uploadfile/2022/1205/IMX334LQR-C%20full%20datasheet_Awin.pdf > Best Regards, > Tarang > > > + break; > > > + default: > > > dev_err(imx334->dev, > > > "number of CSI2 data lanes %d is not supported\n", > > > bus_cfg.bus.mipi_csi2.num_data_lanes); > > > > -- > > Regards, > > > > Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes 2025-03-27 14:32 ` Dave Stevenson @ 2025-03-27 15:38 ` Tarang Raval 0 siblings, 0 replies; 21+ messages in thread From: Tarang Raval @ 2025-03-27 15:38 UTC (permalink / raw) To: Dave Stevenson Cc: Sakari Ailus, kieran.bingham@ideasonboard.com, Shravan.Chippa@microchip.com, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, Umang Jain, Zhi Mao, Julien Massot, Luis Garcia, Benjamin Mugnier, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Hi Dave, Thank you for your detailed feedback. > On Thu, 27 Mar 2025 at 10:55, Tarang Raval > <tarang.raval@siliconsignals.io> wrote: > > > > Hi Sakari, > > > > Thanks for the review. > > > > > On Mon, Mar 10, 2025 at 12:47:46PM +0530, Tarang Raval wrote: > > > > imx334 can support both 4 and 8 lane configurations. > > > > Extend the driver to configure the lane mode accordingly. > > > > > > > > Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io> > > > > --- > > > > drivers/media/i2c/imx334.c | 22 +++++++++++++++++++--- > > > > 1 file changed, 19 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > > > index 24ccfd1d0986..23bfc64969cc 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) > > > > @@ -107,7 +109,6 @@ > > > > /* CSI2 HW configuration */ > > > > #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 > > > > @@ -181,6 +182,7 @@ struct imx334_mode { > > > > * @exp_ctrl: Pointer to exposure control > > > > * @again_ctrl: Pointer to analog gain control > > > > * @vblank: Vertical blanking in lines > > > > + * @lane_mode: Mode for number of connected data lanes > > > > * @cur_mode: Pointer to current selected sensor mode > > > > * @mutex: Mutex for serializing sensor controls > > > > * @link_freq_bitmap: Menu bitmap for link_freq_ctrl > > > > @@ -204,6 +206,7 @@ struct imx334 { > > > > struct v4l2_ctrl *again_ctrl; > > > > }; > > > > u32 vblank; > > > > + u32 lane_mode; > > > > const struct imx334_mode *cur_mode; > > > > struct mutex mutex; > > > > unsigned long link_freq_bitmap; > > > > @@ -240,7 +243,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}, > > > > > > Not a fault of this patch but also the closing brace should have a space > > > before it. Could you address it in the earlier patches? > > > > Okay, I will correct it. > > > > > > { IMX334_REG_TCLKPOST, 0x007f}, > > > > { IMX334_REG_TCLKPREPARE, 0x0037}, > > > > { IMX334_REG_TCLKTRAIL, 0x0037}, > > > > @@ -876,6 +878,13 @@ static int imx334_start_streaming(struct imx334 *imx334) > > > > return ret; > > > > } > > > > > > > > + ret = cci_write(imx334->cci, IMX334_REG_LANEMODE, > > > > + imx334->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", > > > > @@ -1022,7 +1031,14 @@ static int imx334_parse_hw_config(struct imx334 *imx334) > > > > if (ret) > > > > return ret; > > > > > > > > - if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX334_NUM_DATA_LANES) { > > > > + switch (bus_cfg.bus.mipi_csi2.num_data_lanes) { > > > > + case 4: > > > > + imx334->lane_mode = IMX334_CSI_4_LANE_MODE; > > > > + break; > > > > + case 8: > > > > + imx334->lane_mode = IMX334_CSI_8_LANE_MODE; > > > > > > Doesn't this affect the PLL configuration? Presumably higher frame rates > > > could be achieved at least. > > > > Sorry, my commit message is misleading. The intention of this patch is to > > configure the lane mode dynamically from the streaming function instead > > of using a hardcoded value. > > > > You are correct that supporting an 8-lane mode requires changes to the PLL > > configuration. This patch does not address that aspect yet. > > Is it actually required, or just a nicety? > The datasheet [1] says: > "Maximum frame rate in All-pixel scan mode 3840(H)×2160(V) AD12bit: 60 > frame / s" > The current driver configuration for the 3840x2160 mode is a pixel > clock 594MHz with total timings of (3840+560) x (2160+90), which gives > a framerate of 60fps. So you already have the maximum capabilities of > the sensor exposed. > > Adding the 8 lane mode gives you the option to run at half the link > frequency of the 4 lane, but Sony Starvis sensors have a FIFO between > pixel array and MIPI block. All the other Starvis sensors I've > encountered are quite happy at any of the link frequencies as long as > the horizontal blanking makes the line period sufficient to send each > line. > > The datasheet does say "The bit rate maximum value are 1782 Mbps / > Lane in 4 Lane mode and 1188 Mbps / Lane in 8 Lane mode. " (page 78 > "CSI-2 output"), but then also "The maximum bit rate of each Lane are > 1782 Mbps / Lane." (page 81 "MIPI transmitter"). Surely all lanes can > either do 1782Mbps, or they can't. They won't have downrated just > lanes 5-8. > Presumably it works at 1782Mbps/lane in 8 lane mode or you wouldn't > have submitted the patch, My patch aimed to make lane mode dynamic (4 or 8 lanes) based on hardware. not to add 8-lane support my commit message was off, and I haven’t tested 8 lanes. Thanks for pointing out the PLL and datasheet inconsistencies. I’ll fix the message and leave 8-lane support for a well-tested implementation in the future. Best Regards, Tarang > We've been here before with the imx290 and imx415 drivers and what can > be supported with each combination of lanes and link frequency. > > Cheers > Dave > > [1] https://en.sunnywale.com/uploadfile/2022/1205/IMX334LQR-C%20full%20datasheet_Awin.pdf > > > Best Regards, > > Tarang > > > > + break; > > > > + default: > > > > dev_err(imx334->dev, > > > > "number of CSI2 data lanes %d is not supported\n", > > > > bus_cfg.bus.mipi_csi2.num_data_lanes); > > > > > > -- > > > Regards, > > > > > > Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/6] media: i2c: imx334: Fix power management and control handling 2025-03-10 7:17 [PATCH 0/6] media: i2c: imx334: Miscellaneous cleanups and improvements Tarang Raval ` (3 preceding siblings ...) 2025-03-10 7:17 ` [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes Tarang Raval @ 2025-03-10 7:17 ` Tarang Raval 2025-03-10 7:17 ` [PATCH 6/6] media: i2c: imx334: Enable runtime PM before sub-device registration Tarang Raval 5 siblings, 0 replies; 21+ messages in thread From: Tarang Raval @ 2025-03-10 7:17 UTC (permalink / raw) To: sakari.ailus, kieran.bingham Cc: Shravan.Chippa, Tarang Raval, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, Umang Jain, Zhi Mao, Julien Massot, Mikhail Rudenko, Benjamin Mugnier, linux-media, linux-kernel 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> --- 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 23bfc64969cc..ffa39bb317f7 100644 --- a/drivers/media/i2c/imx334.c +++ b/drivers/media/i2c/imx334.c @@ -579,8 +579,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", @@ -593,13 +592,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; @@ -608,8 +618,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: @@ -641,6 +649,8 @@ static int imx334_set_ctrl(struct v4l2_ctrl *ctrl) ret = -EINVAL; } + pm_runtime_put(imx334->dev); + return ret; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 6/6] media: i2c: imx334: Enable runtime PM before sub-device registration 2025-03-10 7:17 [PATCH 0/6] media: i2c: imx334: Miscellaneous cleanups and improvements Tarang Raval ` (4 preceding siblings ...) 2025-03-10 7:17 ` [PATCH 5/6] media: i2c: imx334: Fix power management and control handling Tarang Raval @ 2025-03-10 7:17 ` Tarang Raval 2025-03-27 10:06 ` Sakari Ailus 5 siblings, 1 reply; 21+ messages in thread From: Tarang Raval @ 2025-03-10 7:17 UTC (permalink / raw) To: sakari.ailus, kieran.bingham Cc: Shravan.Chippa, Tarang Raval, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, AngeloGioacchino Del Regno, Julien Massot, Zhi Mao, Mikhail Rudenko, Benjamin Mugnier, Luis Garcia, linux-media, linux-kernel 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> --- drivers/media/i2c/imx334.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c index ffa39bb317f7..8964d60324e2 100644 --- a/drivers/media/i2c/imx334.c +++ b/drivers/media/i2c/imx334.c @@ -1295,6 +1295,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, @@ -1302,8 +1305,6 @@ 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; -- 2.34.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 6/6] media: i2c: imx334: Enable runtime PM before sub-device registration 2025-03-10 7:17 ` [PATCH 6/6] media: i2c: imx334: Enable runtime PM before sub-device registration Tarang Raval @ 2025-03-27 10:06 ` Sakari Ailus 2025-03-27 11:02 ` Tarang Raval 0 siblings, 1 reply; 21+ messages in thread From: Sakari Ailus @ 2025-03-27 10:06 UTC (permalink / raw) To: Tarang Raval Cc: kieran.bingham, Shravan.Chippa, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, AngeloGioacchino Del Regno, Julien Massot, Zhi Mao, Mikhail Rudenko, Benjamin Mugnier, Luis Garcia, linux-media, linux-kernel Hi Tarang, On Mon, Mar 10, 2025 at 12:47:48PM +0530, Tarang Raval wrote: > 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> > --- > drivers/media/i2c/imx334.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > index ffa39bb317f7..8964d60324e2 100644 > --- a/drivers/media/i2c/imx334.c > +++ b/drivers/media/i2c/imx334.c > @@ -1295,6 +1295,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, > @@ -1302,8 +1305,6 @@ static int imx334_probe(struct i2c_client *client) > goto error_media_entity; > } > > - pm_runtime_set_active(imx334->dev); > - pm_runtime_enable(imx334->dev); Please also change error handling accordingly. Btw. the remove function calls pm_runtime_suspended() that does nothing (use pm_runtime_set_suspended() instead). Could you add a patch to fix it? > pm_runtime_idle(imx334->dev); > > return 0; -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 6/6] media: i2c: imx334: Enable runtime PM before sub-device registration 2025-03-27 10:06 ` Sakari Ailus @ 2025-03-27 11:02 ` Tarang Raval 2025-03-27 11:14 ` Sakari Ailus 0 siblings, 1 reply; 21+ messages in thread From: Tarang Raval @ 2025-03-27 11:02 UTC (permalink / raw) To: Sakari Ailus Cc: kieran.bingham@ideasonboard.com, Shravan.Chippa@microchip.com, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, AngeloGioacchino Del Regno, Julien Massot, Zhi Mao, Mikhail Rudenko, Benjamin Mugnier, Luis Garcia, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Hi Sakari, Thanks for the review. > On Mon, Mar 10, 2025 at 12:47:48PM +0530, Tarang Raval wrote: > > 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> > > --- > > drivers/media/i2c/imx334.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > index ffa39bb317f7..8964d60324e2 100644 > > --- a/drivers/media/i2c/imx334.c > > +++ b/drivers/media/i2c/imx334.c > > @@ -1295,6 +1295,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, > > @@ -1302,8 +1305,6 @@ static int imx334_probe(struct i2c_client *client) > > goto error_media_entity; > > } > > > > - pm_runtime_set_active(imx334->dev); > > - pm_runtime_enable(imx334->dev); > > Please also change error handling accordingly. Oh, I missed that. > Btw. the remove function calls pm_runtime_suspended() that does nothing > (use pm_runtime_set_suspended() instead). Could you add a patch to fix it? Should I include these changes in this patch, or should I create a separate patch for them. Best Regards, Tarang > > pm_runtime_idle(imx334->dev); > > > > return 0; > > -- > Regards, > > Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 6/6] media: i2c: imx334: Enable runtime PM before sub-device registration 2025-03-27 11:02 ` Tarang Raval @ 2025-03-27 11:14 ` Sakari Ailus 0 siblings, 0 replies; 21+ messages in thread From: Sakari Ailus @ 2025-03-27 11:14 UTC (permalink / raw) To: Tarang Raval Cc: kieran.bingham@ideasonboard.com, Shravan.Chippa@microchip.com, Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart, AngeloGioacchino Del Regno, Julien Massot, Zhi Mao, Mikhail Rudenko, Benjamin Mugnier, Luis Garcia, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org On Thu, Mar 27, 2025 at 11:02:33AM +0000, Tarang Raval wrote: > Hi Sakari, > > Thanks for the review. > > > On Mon, Mar 10, 2025 at 12:47:48PM +0530, Tarang Raval wrote: > > > 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> > > > --- > > > drivers/media/i2c/imx334.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > > index ffa39bb317f7..8964d60324e2 100644 > > > --- a/drivers/media/i2c/imx334.c > > > +++ b/drivers/media/i2c/imx334.c > > > @@ -1295,6 +1295,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, > > > @@ -1302,8 +1305,6 @@ static int imx334_probe(struct i2c_client *client) > > > goto error_media_entity; > > > } > > > > > > - pm_runtime_set_active(imx334->dev); > > > - pm_runtime_enable(imx334->dev); > > > > Please also change error handling accordingly. > > Oh, I missed that. > > > Btw. the remove function calls pm_runtime_suspended() that does nothing > > (use pm_runtime_set_suspended() instead). Could you add a patch to fix it? > > Should I include these changes in this patch, or should I create a separate > patch for them. A separate patch would be better as it could be cc'd to stable as well. Probably this issue has been there all along... -- Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-03-27 15:38 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-10 7:17 [PATCH 0/6] media: i2c: imx334: Miscellaneous cleanups and improvements Tarang Raval 2025-03-10 7:17 ` [PATCH 1/6] media: i2c: imx334: Simplify with dev_err_probe() Tarang Raval 2025-03-10 7:17 ` [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers Tarang Raval 2025-03-11 6:14 ` Shravan.Chippa 2025-03-11 6:38 ` Laurent Pinchart 2025-03-11 6:51 ` Shravan.Chippa 2025-03-11 9:46 ` Sakari Ailus 2025-03-11 9:59 ` Shravan.Chippa 2025-03-11 10:05 ` Tarang Raval 2025-03-11 10:13 ` Sakari Ailus 2025-03-10 7:17 ` [PATCH 3/6] media: i2c: imx334: Remove redundant register entries Tarang Raval 2025-03-10 7:17 ` [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes Tarang Raval 2025-03-27 10:09 ` Sakari Ailus 2025-03-27 10:54 ` Tarang Raval 2025-03-27 14:32 ` Dave Stevenson 2025-03-27 15:38 ` Tarang Raval 2025-03-10 7:17 ` [PATCH 5/6] media: i2c: imx334: Fix power management and control handling Tarang Raval 2025-03-10 7:17 ` [PATCH 6/6] media: i2c: imx334: Enable runtime PM before sub-device registration Tarang Raval 2025-03-27 10:06 ` Sakari Ailus 2025-03-27 11:02 ` Tarang Raval 2025-03-27 11:14 ` Sakari Ailus
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox