From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
To: Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: Tianshu Qiu <tian.shu.qiu@intel.com>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
David Heidelberg <david@ixit.cz>,
20260414-imx355-24mhz-v1-1-9ae77bc6e7ec@ixit.cz
Subject: Re: [PATCH 12/13] media: imx355: Convert to new CCI register access helpers
Date: Thu, 7 May 2026 16:49:41 +0200 [thread overview]
Message-ID: <afylhGLYx713aRUQ@zed> (raw)
In-Reply-To: <20260506-media-imx355-v1-12-660685030455@raspberrypi.com>
Hi Dave
On Wed, May 06, 2026 at 07:23:50PM +0100, Dave Stevenson wrote:
> Use the new comon CCI register access helpers to replace the private
> register access helpers in the imx355 driver.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
> drivers/media/i2c/Kconfig | 1 +
> drivers/media/i2c/imx355.c | 502 ++++++++++++++++++---------------------------
> 2 files changed, 196 insertions(+), 307 deletions(-)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 8f2ba4121586..38f23306722c 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -259,6 +259,7 @@ config VIDEO_IMX335
>
> config VIDEO_IMX355
> tristate "Sony IMX355 sensor support"
> + select V4L2_CCI_I2C
> help
> This is a Video4Linux2 sensor driver for the Sony
> IMX355 camera.
> diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> index c6fcd649c32a..d0e0e81d1e7c 100644
> --- a/drivers/media/i2c/imx355.c
> +++ b/drivers/media/i2c/imx355.c
> @@ -9,78 +9,80 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/unaligned.h>
>
> +#include <media/v4l2-cci.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-event.h>
> #include <media/v4l2-fwnode.h>
>
> -#define IMX355_REG_MODE_SELECT 0x0100
> +#define IMX355_REG_MODE_SELECT CCI_REG8(0x0100)
> #define IMX355_MODE_STANDBY 0x00
> #define IMX355_MODE_STREAMING 0x01
>
> /* Chip ID */
> -#define IMX355_REG_CHIP_ID 0x0016
> +#define IMX355_REG_CHIP_ID CCI_REG16(0x0016)
> #define IMX355_CHIP_ID 0x0355
>
> /* PLL registers that depend on the external clock frequency */
> -#define IMX355_REG_EXTCLK_FREQ 0x0136
> -#define IMX355_REG_PLL_VT_MUL 0x0306
> -#define IMX355_REG_PLL_OP_MUL 0x030e
> +#define IMX355_REG_EXTCLK_FREQ CCI_REG16(0x0136)
> +#define IMX355_REG_PLL_VT_MUL CCI_REG16(0x0306)
> +#define IMX355_REG_PLL_OP_MUL CCI_REG16(0x030e)
>
> /* V_TIMING internal */
> -#define IMX355_REG_FLL 0x0340
> +#define IMX355_REG_FLL CCI_REG16(0x0340)
> #define IMX355_FLL_MAX 0xffff
> /* Number of lines above frame height that are required. */
> #define IMX355_FLL_OFFSET 20
>
> -#define IMX355_REG_LLP 0x0342
> +#define IMX355_REG_LLP CCI_REG16(0x0342)
> #define IMX355_LLP_MAX 0xffff
>
> -#define IMX355_REG_X_ADD_START 0x0344
> -#define IMX355_REG_Y_ADD_START 0x0346
> -#define IMX355_REG_X_ADD_END 0x0348
> -#define IMX355_REG_Y_ADD_END 0x034a
> -#define IMX355_REG_X_OUT_SIZE 0x034c
> -#define IMX355_REG_Y_OUT_SIZE 0x034e
> +#define IMX355_REG_X_ADD_START CCI_REG16(0x0344)
> +#define IMX355_REG_Y_ADD_START CCI_REG16(0x0346)
> +#define IMX355_REG_X_ADD_END CCI_REG16(0x0348)
> +#define IMX355_REG_Y_ADD_END CCI_REG16(0x034a)
> +#define IMX355_REG_X_OUT_SIZE CCI_REG16(0x034c)
> +#define IMX355_REG_Y_OUT_SIZE CCI_REG16(0x034e)
>
> /* Exposure control */
> -#define IMX355_REG_EXPOSURE 0x0202
> +#define IMX355_REG_EXPOSURE CCI_REG16(0x0202)
> #define IMX355_EXPOSURE_MIN 1
> #define IMX355_EXPOSURE_STEP 1
> #define IMX355_EXPOSURE_DEFAULT 0x0282
>
> /* Analog gain control */
> -#define IMX355_REG_ANALOG_GAIN 0x0204
> +#define IMX355_REG_ANALOG_GAIN CCI_REG16(0x0204)
> #define IMX355_ANA_GAIN_MIN 0
> #define IMX355_ANA_GAIN_MAX 960
> #define IMX355_ANA_GAIN_STEP 1
> #define IMX355_ANA_GAIN_DEFAULT 0
>
> /* Digital gain control */
> -#define IMX355_REG_DPGA_USE_GLOBAL_GAIN 0x3070
> -#define IMX355_REG_DIG_GAIN_GLOBAL 0x020e
> +#define IMX355_REG_DPGA_USE_GLOBAL_GAIN CCI_REG8(0x3070)
> +#define IMX355_REG_DIG_GAIN_GLOBAL CCI_REG16(0x020e)
> #define IMX355_DGTL_GAIN_MIN 256
> #define IMX355_DGTL_GAIN_MAX 4095
> #define IMX355_DGTL_GAIN_STEP 1
> #define IMX355_DGTL_GAIN_DEFAULT 256
>
> /* Test Pattern Control */
> -#define IMX355_REG_TEST_PATTERN 0x0600
> +#define IMX355_REG_TEST_PATTERN CCI_REG8(0x0600)
> #define IMX355_TEST_PATTERN_DISABLED 0
> #define IMX355_TEST_PATTERN_SOLID_COLOR 1
> #define IMX355_TEST_PATTERN_COLOR_BARS 2
> #define IMX355_TEST_PATTERN_GRAY_COLOR_BARS 3
> #define IMX355_TEST_PATTERN_PN9 4
>
> -#define IMX355_REG_BINNING_MODE 0x0900
> -#define IMX355_REG_BINNING_TYPE 0x0901
> -#define IMX355_REG_BINNING_WEIGHTING 0x0902
> +#define IMX355_REG_BINNING_MODE CCI_REG8(0x0900)
> +#define IMX355_REG_BINNING_TYPE CCI_REG8(0x0901)
> +#define IMX355_REG_BINNING_WEIGHTING CCI_REG8(0x0902)
>
> /* Flip Control */
> -#define IMX355_REG_ORIENTATION 0x0101
> +#define IMX355_REG_ORIENTATION CCI_REG8(0x0101)
>
> /* default link frequency and external clock */
> #define IMX355_LINK_FREQ_DEFAULT 360000000LL
> @@ -93,14 +95,9 @@
> #define IMX355_PIXEL_ARRAY_WIDTH 3280
> #define IMX355_PIXEL_ARRAY_HEIGHT 2464
>
> -struct imx355_reg {
> - u16 address;
> - u8 val;
> -};
> -
> struct imx355_reg_list {
> u32 num_of_regs;
> - const struct imx355_reg *regs;
> + const struct cci_reg_sequence *regs;
> };
>
> /* Mode : resolution and related config&values */
> @@ -160,6 +157,7 @@ struct imx355_hwcfg {
> struct imx355 {
> struct device *dev;
> struct clk *clk;
> + struct regmap *regmap;
>
> struct v4l2_subdev sd;
> struct media_pad pad;
> @@ -196,152 +194,147 @@ static const struct regulator_bulk_data imx355_supplies[] = {
> { .supply = "dovdd" },
> };
>
> -static const struct imx355_reg imx355_global_regs[] = {
> - { 0x304e, 0x03 },
> - { 0x4348, 0x16 },
> - { 0x4350, 0x19 },
> - { 0x4408, 0x0a },
> - { 0x440c, 0x0b },
> - { 0x4411, 0x5f },
> - { 0x4412, 0x2c },
> - { 0x4623, 0x00 },
> - { 0x462c, 0x0f },
> - { 0x462d, 0x00 },
> - { 0x462e, 0x00 },
> - { 0x4684, 0x54 },
> - { 0x480a, 0x07 },
> - { 0x4908, 0x07 },
> - { 0x4909, 0x07 },
> - { 0x490d, 0x0a },
> - { 0x491e, 0x0f },
> - { 0x4921, 0x06 },
> - { 0x4923, 0x28 },
> - { 0x4924, 0x28 },
> - { 0x4925, 0x29 },
> - { 0x4926, 0x29 },
> - { 0x4927, 0x1f },
> - { 0x4928, 0x20 },
> - { 0x4929, 0x20 },
> - { 0x492a, 0x20 },
> - { 0x492c, 0x05 },
> - { 0x492d, 0x06 },
> - { 0x492e, 0x06 },
> - { 0x492f, 0x06 },
> - { 0x4930, 0x03 },
> - { 0x4931, 0x04 },
> - { 0x4932, 0x04 },
> - { 0x4933, 0x05 },
> - { 0x595e, 0x01 },
> - { 0x5963, 0x01 },
> - { 0x3030, 0x01 },
> - { 0x3031, 0x01 },
> - { 0x3045, 0x01 },
> - { 0x4010, 0x00 },
> - { 0x4011, 0x00 },
> - { 0x4012, 0x00 },
> - { 0x4013, 0x01 },
> - { 0x68a8, 0xfe },
> - { 0x68a9, 0xff },
> - { 0x6888, 0x00 },
> - { 0x6889, 0x00 },
> - { 0x68b0, 0x00 },
> - { 0x3058, 0x00 },
> - { 0x305a, 0x00 },
> - { 0x0112, 0x0a },
> - { 0x0113, 0x0a },
> - { 0x0114, 0x03 },
> - { 0x0301, 0x05 },
> - { 0x0303, 0x01 },
> - { 0x0305, 0x02 },
> - { 0x030d, 0x02 },
> - { 0x0310, 0x00 },
> - { 0x0220, 0x00 },
> - { 0x0222, 0x01 },
> - { 0x0820, 0x0b },
> - { 0x0821, 0x40 },
> - { 0x3088, 0x04 },
> - { 0x6813, 0x02 },
> - { 0x6835, 0x07 },
> - { 0x6836, 0x01 },
> - { 0x6837, 0x04 },
> - { 0x684d, 0x07 },
> - { 0x684e, 0x01 },
> - { 0x684f, 0x04 },
> +static const struct cci_reg_sequence imx355_global_regs[] = {
> + { CCI_REG8(0x304e), 0x03 },
> + { CCI_REG8(0x4348), 0x16 },
> + { CCI_REG8(0x4350), 0x19 },
> + { CCI_REG8(0x4408), 0x0a },
> + { CCI_REG8(0x440c), 0x0b },
> + { CCI_REG8(0x4411), 0x5f },
> + { CCI_REG8(0x4412), 0x2c },
> + { CCI_REG8(0x4623), 0x00 },
> + { CCI_REG8(0x462c), 0x0f },
> + { CCI_REG8(0x462d), 0x00 },
> + { CCI_REG8(0x462e), 0x00 },
> + { CCI_REG8(0x4684), 0x54 },
> + { CCI_REG8(0x480a), 0x07 },
> + { CCI_REG8(0x4908), 0x07 },
> + { CCI_REG8(0x4909), 0x07 },
> + { CCI_REG8(0x490d), 0x0a },
> + { CCI_REG8(0x491e), 0x0f },
> + { CCI_REG8(0x4921), 0x06 },
> + { CCI_REG8(0x4923), 0x28 },
> + { CCI_REG8(0x4924), 0x28 },
> + { CCI_REG8(0x4925), 0x29 },
> + { CCI_REG8(0x4926), 0x29 },
> + { CCI_REG8(0x4927), 0x1f },
> + { CCI_REG8(0x4928), 0x20 },
> + { CCI_REG8(0x4929), 0x20 },
> + { CCI_REG8(0x492a), 0x20 },
> + { CCI_REG8(0x492c), 0x05 },
> + { CCI_REG8(0x492d), 0x06 },
> + { CCI_REG8(0x492e), 0x06 },
> + { CCI_REG8(0x492f), 0x06 },
> + { CCI_REG8(0x4930), 0x03 },
> + { CCI_REG8(0x4931), 0x04 },
> + { CCI_REG8(0x4932), 0x04 },
> + { CCI_REG8(0x4933), 0x05 },
> + { CCI_REG8(0x595e), 0x01 },
> + { CCI_REG8(0x5963), 0x01 },
> + { CCI_REG8(0x3030), 0x01 },
> + { CCI_REG8(0x3031), 0x01 },
> + { CCI_REG8(0x3045), 0x01 },
> + { CCI_REG8(0x4010), 0x00 },
> + { CCI_REG8(0x4011), 0x00 },
> + { CCI_REG8(0x4012), 0x00 },
> + { CCI_REG8(0x4013), 0x01 },
> + { CCI_REG8(0x68a8), 0xfe },
> + { CCI_REG8(0x68a9), 0xff },
> + { CCI_REG8(0x6888), 0x00 },
> + { CCI_REG8(0x6889), 0x00 },
> + { CCI_REG8(0x68b0), 0x00 },
> + { CCI_REG8(0x3058), 0x00 },
> + { CCI_REG8(0x305a), 0x00 },
> + { CCI_REG8(0x0112), 0x0a },
> + { CCI_REG8(0x0113), 0x0a },
> + { CCI_REG8(0x0114), 0x03 },
> + { CCI_REG8(0x0301), 0x05 },
> + { CCI_REG8(0x0303), 0x01 },
> + { CCI_REG8(0x0305), 0x02 },
> + { CCI_REG8(0x030d), 0x02 },
> + { CCI_REG8(0x0310), 0x00 },
> + { CCI_REG8(0x0220), 0x00 },
> + { CCI_REG8(0x0222), 0x01 },
> + { CCI_REG8(0x0820), 0x0b },
> + { CCI_REG8(0x0821), 0x40 },
> + { CCI_REG8(0x3088), 0x04 },
> + { CCI_REG8(0x6813), 0x02 },
> + { CCI_REG8(0x6835), 0x07 },
> + { CCI_REG8(0x6836), 0x01 },
> + { CCI_REG8(0x6837), 0x04 },
> + { CCI_REG8(0x684d), 0x07 },
> + { CCI_REG8(0x684e), 0x01 },
> + { CCI_REG8(0x684f), 0x04 },
> };
I admit I haven't gone through the registers to check their effective
sizes
>
> -static const struct imx355_reg_list imx355_global_setting = {
> - .num_of_regs = ARRAY_SIZE(imx355_global_regs),
> - .regs = imx355_global_regs,
> +static const struct cci_reg_sequence mode_3268x2448_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_3268x2448_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_3264x2448_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_3264x2448_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_3280x2464_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_3280x2464_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1940x1096_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1940x1096_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1936x1096_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1936x1096_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1924x1080_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1924x1080_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1920x1080_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1920x1080_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1640x1232_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1640x1232_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1640x922_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1640x922_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1300x736_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1300x736_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1296x736_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1296x736_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1284x720_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1284x720_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> +static const struct cci_reg_sequence mode_1280x720_regs[] = {
> + { CCI_REG8(0x0700), 0x00 },
> + { CCI_REG8(0x0701), 0x10 },
> };
>
> -static const struct imx355_reg mode_1280x720_regs[] = {
> - { 0x0700, 0x00 },
> - { 0x0701, 0x10 },
> -};
> -
> -static const struct imx355_reg mode_820x616_regs[] = {
> - { 0x0700, 0x02 },
> - { 0x0701, 0x78 },
> +static const struct cci_reg_sequence mode_820x616_regs[] = {
> + { CCI_REG8(0x0700), 0x02 },
> + { CCI_REG8(0x0701), 0x78 },
> };
>
> static const char * const imx355_test_pattern_menu[] = {
> @@ -598,78 +591,6 @@ static u32 imx355_get_format_code(struct imx355 *imx355)
> return code;
> }
>
> -/* Read registers up to 4 at a time */
> -static int imx355_read_reg(struct imx355 *imx355, u16 reg, u32 len, u32 *val)
> -{
> - struct i2c_client *client = v4l2_get_subdevdata(&imx355->sd);
> - struct i2c_msg msgs[2];
> - u8 addr_buf[2];
> - u8 data_buf[4] = { 0 };
> - int ret;
> -
> - if (len > 4)
> - return -EINVAL;
> -
> - put_unaligned_be16(reg, addr_buf);
> - /* Write register address */
> - msgs[0].addr = client->addr;
> - msgs[0].flags = 0;
> - msgs[0].len = ARRAY_SIZE(addr_buf);
> - msgs[0].buf = addr_buf;
> -
> - /* Read data from register */
> - msgs[1].addr = client->addr;
> - msgs[1].flags = I2C_M_RD;
> - msgs[1].len = len;
> - msgs[1].buf = &data_buf[4 - len];
> -
> - ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> - if (ret != ARRAY_SIZE(msgs))
> - return -EIO;
> -
> - *val = get_unaligned_be32(data_buf);
> -
> - return 0;
> -}
> -
> -/* Write registers up to 4 at a time */
> -static int imx355_write_reg(struct imx355 *imx355, u16 reg, u32 len, u32 val)
> -{
> - struct i2c_client *client = v4l2_get_subdevdata(&imx355->sd);
> - u8 buf[6];
> -
> - if (len > 4)
> - return -EINVAL;
> -
> - put_unaligned_be16(reg, buf);
> - put_unaligned_be32(val << (8 * (4 - len)), buf + 2);
> - if (i2c_master_send(client, buf, len + 2) != len + 2)
> - return -EIO;
> -
> - return 0;
> -}
> -
> -/* Write a list of registers */
> -static int imx355_write_regs(struct imx355 *imx355,
> - const struct imx355_reg *regs, u32 len)
> -{
> - int ret;
> - u32 i;
> -
> - for (i = 0; i < len; i++) {
> - ret = imx355_write_reg(imx355, regs[i].address, 1, regs[i].val);
> - if (ret) {
> - dev_err_ratelimited(imx355->dev,
> - "write reg 0x%4.4x return err %d",
> - regs[i].address, ret);
> -
> - return ret;
> - }
> - }
> -
> - return 0;
> -}
> -
> /* Open sub-device */
> static int imx355_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> {
> @@ -724,31 +645,31 @@ static int imx355_set_ctrl(struct v4l2_ctrl *ctrl)
> switch (ctrl->id) {
> case V4L2_CID_ANALOGUE_GAIN:
> /* Analog gain = 1024/(1024 - ctrl->val) times */
> - ret = imx355_write_reg(imx355, IMX355_REG_ANALOG_GAIN, 2,
> - ctrl->val);
> + ret = cci_write(imx355->regmap, IMX355_REG_ANALOG_GAIN,
> + ctrl->val, NULL);
> break;
> case V4L2_CID_DIGITAL_GAIN:
> - ret = imx355_write_reg(imx355, IMX355_REG_DIG_GAIN_GLOBAL, 2,
> - ctrl->val);
> + ret = cci_write(imx355->regmap, IMX355_REG_DIG_GAIN_GLOBAL,
> + ctrl->val, NULL);
> break;
> case V4L2_CID_EXPOSURE:
> - ret = imx355_write_reg(imx355, IMX355_REG_EXPOSURE, 2,
> - ctrl->val);
> + ret = cci_write(imx355->regmap, IMX355_REG_EXPOSURE,
> + ctrl->val, NULL);
> break;
> case V4L2_CID_VBLANK:
> /* Update FLL that meets expected vertical blanking */
> - ret = imx355_write_reg(imx355, IMX355_REG_FLL, 2,
> - imx355->cur_mode->height + ctrl->val);
> + ret = cci_write(imx355->regmap, IMX355_REG_FLL,
> + imx355->cur_mode->height + ctrl->val, NULL);
> break;
> case V4L2_CID_TEST_PATTERN:
> - ret = imx355_write_reg(imx355, IMX355_REG_TEST_PATTERN,
> - 2, ctrl->val);
> + ret = cci_write(imx355->regmap, IMX355_REG_TEST_PATTERN,
> + ctrl->val, NULL);
> break;
> case V4L2_CID_HFLIP:
> case V4L2_CID_VFLIP:
> - ret = imx355_write_reg(imx355, IMX355_REG_ORIENTATION, 1,
> - imx355->hflip->val |
> - imx355->vflip->val << 1);
> + ret = cci_write(imx355->regmap, IMX355_REG_ORIENTATION,
> + imx355->hflip->val | imx355->vflip->val << 1,
> + NULL);
> break;
> default:
> ret = -EINVAL;
> @@ -948,103 +869,64 @@ static int imx355_start_streaming(struct imx355 *imx355)
> {
> const struct imx355_reg_list *reg_list;
> const struct imx355_mode *mode;
> - int ret;
> + int ret = 0;
>
> /* Global Setting */
> - reg_list = &imx355_global_setting;
> - ret = imx355_write_regs(imx355, reg_list->regs, reg_list->num_of_regs);
> - if (ret) {
> - dev_err(imx355->dev, "failed to set global settings");
> - return ret;
> - }
> + cci_multi_reg_write(imx355->regmap, imx355_global_regs,
> + ARRAY_SIZE(imx355_global_regs), &ret);
>
> /* Apply default values of current mode */
> mode = imx355->cur_mode;
> reg_list = &mode->reg_list;
> - ret = imx355_write_regs(imx355, reg_list->regs, reg_list->num_of_regs);
> - if (ret) {
> - dev_err(imx355->dev, "failed to set mode");
> - return ret;
> - }
> + cci_multi_reg_write(imx355->regmap, reg_list->regs,
> + reg_list->num_of_regs, &ret);
I wonder if it makes sense to continue if this fails
>
> /* Set readout crop and size registers */
> - ret = imx355_write_reg(imx355, IMX355_REG_X_ADD_START, 2,
> - mode->x_add_start);
> - if (ret)
> - return ret;
> - ret = imx355_write_reg(imx355, IMX355_REG_Y_ADD_START, 2,
> - mode->y_add_start);
> - if (ret)
> - return ret;
> - ret = imx355_write_reg(imx355, IMX355_REG_X_ADD_END, 2,
> - mode->x_add_end);
> - if (ret)
> - return ret;
> - ret = imx355_write_reg(imx355, IMX355_REG_Y_ADD_END, 2,
> - mode->y_add_end);
> - if (ret)
> - return ret;
> - ret = imx355_write_reg(imx355, IMX355_REG_X_OUT_SIZE, 2,
> - mode->width);
> - if (ret)
> - return ret;
> - ret = imx355_write_reg(imx355, IMX355_REG_Y_OUT_SIZE, 2,
> - mode->height);
> - if (ret)
> - return ret;
> -
> - ret = imx355_write_reg(imx355, IMX355_REG_BINNING_MODE, 1,
> - mode->binning_mode == 0x11 ? 0x00 : 0x01);
> - if (ret)
> - return ret;
> - ret = imx355_write_reg(imx355, IMX355_REG_BINNING_TYPE, 1,
> - mode->binning_mode);
> - if (ret)
> - return ret;
> - ret = imx355_write_reg(imx355, IMX355_REG_BINNING_WEIGHTING, 1, 0x00);
> - if (ret)
> - return ret;
> + cci_write(imx355->regmap, IMX355_REG_X_ADD_START, mode->x_add_start,
> + &ret);
> + cci_write(imx355->regmap, IMX355_REG_Y_ADD_START, mode->y_add_start,
> + &ret);
> + cci_write(imx355->regmap, IMX355_REG_X_ADD_END, mode->x_add_end, &ret);
> + cci_write(imx355->regmap, IMX355_REG_Y_ADD_END, mode->y_add_end, &ret);
> + cci_write(imx355->regmap, IMX355_REG_X_OUT_SIZE, mode->width, &ret);
> + cci_write(imx355->regmap, IMX355_REG_Y_OUT_SIZE, mode->height, &ret);
> + cci_write(imx355->regmap, IMX355_REG_BINNING_MODE,
> + mode->binning_mode == 0x11 ? 0x00 : 0x01, &ret);
> + cci_write(imx355->regmap, IMX355_REG_BINNING_TYPE, mode->binning_mode,
> + &ret);
> + cci_write(imx355->regmap, IMX355_REG_BINNING_WEIGHTING, 0x00, &ret);
>
> /* Set PLL registers for the external clock frequency */
> - ret = imx355_write_reg(imx355, IMX355_REG_EXTCLK_FREQ, 2,
> - imx355->clk_params->extclk_freq);
> - if (ret)
> - return ret;
> - ret = imx355_write_reg(imx355, IMX355_REG_PLL_VT_MUL, 2,
> - imx355->clk_params->pll_vt_mpy);
> - if (ret)
> - return ret;
> - ret = imx355_write_reg(imx355, IMX355_REG_PLL_OP_MUL, 2,
> - imx355->clk_params->pll_op_mpy);
> - if (ret)
> - return ret;
> + cci_write(imx355->regmap, IMX355_REG_EXTCLK_FREQ,
> + imx355->clk_params->extclk_freq, &ret);
> + cci_write(imx355->regmap, IMX355_REG_PLL_VT_MUL,
> + imx355->clk_params->pll_vt_mpy, &ret);
> + cci_write(imx355->regmap, IMX355_REG_PLL_OP_MUL,
> + imx355->clk_params->pll_op_mpy, &ret);
>
> /* set digital gain control to all color mode */
> - ret = imx355_write_reg(imx355, IMX355_REG_DPGA_USE_GLOBAL_GAIN, 1, 1);
> - if (ret)
> - return ret;
> + cci_write(imx355->regmap, IMX355_REG_DPGA_USE_GLOBAL_GAIN, 1, &ret);
>
> /* set line length */
> - ret = imx355_write_reg(imx355, IMX355_REG_LLP,
> - imx355->hblank->val + imx355->cur_mode->width,
> - 2);
> - if (ret)
> - return ret;
> + cci_write(imx355->regmap, IMX355_REG_LLP,
> + imx355->hblank->val + imx355->cur_mode->width, &ret);
>
> /* Apply customized values from user */
> - ret = __v4l2_ctrl_handler_setup(imx355->sd.ctrl_handler);
> + __v4l2_ctrl_handler_setup(imx355->sd.ctrl_handler);
This now looks a bit weird
/* set line length */
cci_write(imx355->regmap, IMX355_REG_LLP,
imx355->hblank->val + imx355->cur_mode->width, &ret);
/* Apply customized values from user */
__v4l2_ctrl_handler_setup(imx355->sd.ctrl_handler);
if (ret)
return ret;
> if (ret)
> return ret;
>
> - return imx355_write_reg(imx355, IMX355_REG_MODE_SELECT,
> - 1, IMX355_MODE_STREAMING);
> + cci_write(imx355->regmap, IMX355_REG_MODE_SELECT, IMX355_MODE_STREAMING,
> + &ret);
> +
> + return ret;
> }
>
> /* Stop streaming */
> static int imx355_stop_streaming(struct imx355 *imx355)
> {
> - return imx355_write_reg(imx355, IMX355_REG_MODE_SELECT,
> - 1, IMX355_MODE_STANDBY);
> + return cci_write(imx355->regmap, IMX355_REG_MODE_SELECT,
> + IMX355_MODE_STANDBY, NULL);
> }
>
> static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
> @@ -1091,14 +973,14 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
> static int imx355_identify_module(struct imx355 *imx355)
> {
> int ret;
> - u32 val;
> + u64 val;
>
> - ret = imx355_read_reg(imx355, IMX355_REG_CHIP_ID, 2, &val);
> + ret = cci_read(imx355->regmap, IMX355_REG_CHIP_ID, &val, NULL);
> if (ret)
> return ret;
>
> if (val != IMX355_CHIP_ID) {
> - dev_err(imx355->dev, "chip id mismatch: %x!=%x",
> + dev_err(imx355->dev, "chip id mismatch: %x!=%llx",
> IMX355_CHIP_ID, val);
> return -EIO;
> }
> @@ -1345,6 +1227,12 @@ static int imx355_probe(struct i2c_client *client)
>
> mutex_init(&imx355->mutex);
>
> + imx355->regmap = devm_cci_regmap_init_i2c(client, 16);
> + if (IS_ERR(imx355->regmap)) {
> + dev_err(imx355->dev, "Unable to initialize I2C\n");
> + return -ENODEV;
> + }
> +
> imx355->clk = devm_v4l2_sensor_clk_get(imx355->dev, NULL);
> if (IS_ERR(imx355->clk))
> return dev_err_probe(imx355->dev, PTR_ERR(imx355->clk),
>
> --
> 2.34.1
>
>
next prev parent reply other threads:[~2026-05-07 14:49 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-06 18:23 [PATCH 00/13] media/imx355: General code cleanups, and adding support for 2 lane operation Dave Stevenson
2026-05-06 18:23 ` [PATCH 01/13] media: imx355: Remove duplicated registers from the mode tables Dave Stevenson
2026-05-07 13:41 ` Jacopo Mondi
2026-05-06 18:23 ` [PATCH 02/13] media: imx355: Remove setting FRM_LENGTH_LINES in the mode regs Dave Stevenson
2026-05-07 13:50 ` Jacopo Mondi
2026-05-06 18:23 ` [PATCH 03/13] media: imx355: Programmatically set the crop parameters for each mode Dave Stevenson
2026-05-07 14:00 ` Jacopo Mondi
2026-05-07 16:01 ` Dave Stevenson
2026-05-06 18:23 ` [PATCH 04/13] media: imx355: Remove the duplication between width/height and x/y_out_size Dave Stevenson
2026-05-06 18:23 ` [PATCH 05/13] media: imx355: Set register LINE_LENGTH_PCK programmatically Dave Stevenson
2026-05-07 14:09 ` Jacopo Mondi
2026-05-07 15:18 ` Dave Stevenson
2026-05-06 18:23 ` [PATCH 06/13] media: imx355: Set binning mode registers programmatically Dave Stevenson
2026-05-07 14:12 ` Jacopo Mondi
2026-05-06 18:23 ` [PATCH 07/13] media: imx355: Remove link_freq_index from each mode as ununsed Dave Stevenson
2026-05-07 14:12 ` Jacopo Mondi
2026-05-06 18:23 ` [PATCH 08/13] media: imx355: pixel_rate never changes, so don't recompute Dave Stevenson
2026-05-07 14:13 ` Jacopo Mondi
2026-05-06 18:23 ` [PATCH 09/13] media: imx355: Remove redundant fll_min, and implement fixed offset Dave Stevenson
2026-05-07 14:29 ` Jacopo Mondi
2026-05-07 15:21 ` Dave Stevenson
2026-05-06 18:23 ` [PATCH 10/13] media: imx355: Add support for get_selection Dave Stevenson
2026-05-07 14:42 ` Jacopo Mondi
2026-05-07 15:02 ` Dave Stevenson
2026-05-06 18:23 ` [PATCH 11/13] media: imx355: Use pm_runtime autosuspend_delay Dave Stevenson
2026-05-07 14:43 ` Jacopo Mondi
2026-05-06 18:23 ` [PATCH 12/13] media: imx355: Convert to new CCI register access helpers Dave Stevenson
2026-05-07 14:49 ` Jacopo Mondi [this message]
2026-05-06 18:23 ` [PATCH 13/13] media: imx355: Support 2 lane readout Dave Stevenson
2026-05-13 18:33 ` Richard Acayan
2026-05-14 10:37 ` Dave Stevenson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=afylhGLYx713aRUQ@zed \
--to=jacopo.mondi@ideasonboard.com \
--cc=20260414-imx355-24mhz-v1-1-9ae77bc6e7ec@ixit.cz \
--cc=dave.stevenson@raspberrypi.com \
--cc=david@ixit.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=tian.shu.qiu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.