[PATCH v5 0/3] Add Sony IMX471 camera sensor driver

[PATCH v5 0/3] Add Sony IMX471 camera sensor driver

[Kate Hsuan]

This patchset adds the Sony IMX471 camera sensor driver to the Linux
kernel and resolves the IPU7 camera can't work issueon Lenovo X9
laptops [1].

The patchset contains two patches:
1. Add DMI information of Lenovo X9 to the image upside-down list
2. Add Sony IMX471 image sensor driver

The IMX471 driver can be found in the Intel ipu6-drivers repository [2].
To comply with the sensor driver implementation, the clean-up work
includes:

1. Use CCI register helpers.

2. Enable and disable streams using enable_streams and disable_streams
   functions in struct v4l2_subdev_pad_ops. Invoke
   v4l2_subdev_s_stream_helper() to manage the streaming state.

3. Get rotation information from fwnode properties using
   v4l2_fwnode_device_parse().

4. Finalizes the initialization of the subdev, including allocation of
   the active state using v4l2_subdev_init_finalize().

5. Add the IMX471 driver to the Makefile and Kconfig file.

6. The mutex lock is managed by the V4l2 core.

7. Replace the supported link frequency with v4l2_link_freq_to_bitmap().

8. Drop unused codes.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2454119
[2] https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c

Changes in v5:
1. Add a description for the DMI_BOARD_NAME in the ipu-bridge driver.
2. Check the numbers of MIPI lanes. The driver only support 4 lanes mode.
3. Fix many rumetime PM issues.
4. Drop pixel_rate control variable.
5. Drop unnecessary comments.
6. Name the registers.
7. Fix a leak in imx471_init_controls().

Changes in v4:
1. Add TBE20A0 (found on Lenovo X1 Carbon G14) to the supported sensors list.
2. Revert the sensor upside-down list to v1.
3. Decrease the max analog gain to 800 to mitigate the image flickering problem.
4. Return the error value when __v4l2_ctrl_modify_range() fails.
5. Fix indentation issue in Kconfig.
6. Fix the cci error value issue.
7. Drop unnecessary comments.
8. Drop unused link_freq control variable.

Changes in v3:
1. Naming the register addresses and set up the value with the correct value length.
2. Implement the .get_selection().
3. Drop "identified" field from struct imx471.
4. Drop "streaming" field from struct imx471 and use the __v4l2_ctrl_grab() instead.
5. Moreover, The naming for the register can be found in a seperated patch. If we
   agree with the patch, I will squash it into one patch.

Changes in v2:
1. Change the Bayer format setting according to the vertical and horizontal flip settings.
2. Replace the self-owned mutex with the v4l2 subdev state.
3. Rework the flip control.
4. Manage the regulators using devm_regulator_bulk_get|disable|enbale API
5. Invoke devm_v4l2_sensor_clk_get to get clock-frequency

Kate Hsuan (3):
  media: ipu-bridge: Add DMI information of Lenovo X9 to the image
    upside-down list
  media: ipu-bridge: Add Sony IMX471 for Lenovo X1 Carbon G14
  media: i2c: imx471: Add Sony IMX471 image sensor driver

 MAINTAINERS                          |   6 +
 drivers/media/i2c/Kconfig            |  10 +
 drivers/media/i2c/Makefile           |   1 +
 drivers/media/i2c/imx471.c           | 971 +++++++++++++++++++++++++++
 drivers/media/pci/intel/ipu-bridge.c |  41 ++
 5 files changed, 1029 insertions(+)
 create mode 100644 drivers/media/i2c/imx471.c

--
2.54.0

[PATCH v5 1/3] media: ipu-bridge: Add DMI information of Lenovo X9 to the image upside-down list

[Kate Hsuan]

The Lenovo X9 has an upside-down-mounted Sony IMX471 sensor so the image
was displayed upside-down. Add the DMI information of Lenovo X9 to
resolve the issue.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/media/pci/intel/ipu-bridge.c | 39 ++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 88581a4c081d..2474452b3015 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -134,6 +134,45 @@ static const struct dmi_system_id upside_down_sensor_dmi_ids[] = {
 		},
 		.driver_data = "OVTI02C1",
 	},
+	/*
+	 * The first four characters of DMI_BOARD_NAME identify the Lenovo
+	 * machine type/model. For example, a DMI_BOARD_NAME starting with
+	 * "21Q6" indicates a ThinkPad X9-15.
+	 *
+	 * Reference: https://psref.lenovo.com/
+	 */
+	{
+		/* Lenovo X9-14 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_BOARD_NAME, "21QA"),
+		},
+		.driver_data = "SONY471A",
+	},
+	{
+		/* Lenovo X9-14 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_BOARD_NAME, "21QB"),
+		},
+		.driver_data = "SONY471A",
+	},
+	{
+		/* Lenovo X9-15 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_BOARD_NAME, "21Q6"),
+		},
+		.driver_data = "SONY471A",
+	},
+	{
+		/* Lenovo X9-15 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_BOARD_NAME, "21Q7"),
+		},
+		.driver_data = "SONY471A",
+	},
 	{} /* Terminating entry */
 };
 
-- 
2.54.0

[PATCH v5 2/3] media: ipu-bridge: Add Sony IMX471 for Lenovo X1 Carbon G14

[Kate Hsuan]

The HID for Sony IMX471 is TBE20A0 on Lenovo X1 Carbon G14.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/media/pci/intel/ipu-bridge.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 2474452b3015..8ddfab357922 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -97,6 +97,8 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = {
 	IPU_SENSOR_CONFIG("OVTI8856", 3, 180000000, 360000000, 720000000),
 	/* Sony IMX471 */
 	IPU_SENSOR_CONFIG("SONY471A", 1, 200000000),
+	/* Sony IMX471 (found on Lenovo X1 Carbon G14) */
+	IPU_SENSOR_CONFIG("TBE20A0", 1, 200000000),
 	/* Toshiba T4KA3 */
 	IPU_SENSOR_CONFIG("XMCC0003", 1, 321468000),
 };
-- 
2.54.0

[PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Kate Hsuan]

Add a new driver for Sony imx471 camera sensor. It is based on
Jimmy Su <jimmy.su@intel.com> implementation and the driver can be found
in the following URL.
https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c

This sensor can be found on Lenovo X1 Carbon G14, X9-14 and X9-15 laptops
and it is a part of IPU7 solution. The driver was tested on Lenovo X1
Carbon G14, X9-14 and X9-15 laptops.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 MAINTAINERS                |   6 +
 drivers/media/i2c/Kconfig  |  10 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/imx471.c | 971 +++++++++++++++++++++++++++++++++++++
 4 files changed, 988 insertions(+)
 create mode 100644 drivers/media/i2c/imx471.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6b4560681b51..586958b1816d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25219,6 +25219,12 @@ T:	git git://linuxtv.org/media.git
 F:	Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
 F:	drivers/media/i2c/imx415.c
 
+SONY IMX471 SENSOR DRIVER
+M:	Kate Hsuan <hpa@redhat.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+F:	drivers/media/i2c/imx471.c
+
 SONY MEMORYSTICK SUBSYSTEM
 M:	Maxim Levitsky <maximlevitsky@gmail.com>
 M:	Alex Dubov <oakad@yahoo.com>
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 5d173e0ecf42..b7199f9f5a0c 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -287,6 +287,16 @@ config VIDEO_IMX415
 	  To compile this driver as a module, choose M here: the
 	  module will be called imx415.
 
+config VIDEO_IMX471
+	tristate "Sony IMX471 sensor support"
+	select V4L2_CCI_I2C
+	help
+	  This is a Video4Linux2 sensor driver for the Sony
+	  IMX471 camera.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called imx471.
+
 config VIDEO_MAX9271_LIB
 	tristate
 
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index e45359efe0e4..acbd321fc12e 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_IMX335) += imx335.o
 obj-$(CONFIG_VIDEO_IMX355) += imx355.o
 obj-$(CONFIG_VIDEO_IMX412) += imx412.o
 obj-$(CONFIG_VIDEO_IMX415) += imx415.o
+obj-$(CONFIG_VIDEO_IMX471) += imx471.o
 obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o
 obj-$(CONFIG_VIDEO_KS0127) += ks0127.o
diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
new file mode 100644
index 000000000000..1e1bff69ea3d
--- /dev/null
+++ b/drivers/media/i2c/imx471.c
@@ -0,0 +1,971 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * imx471.c - imx471 sensor driver
+ *
+ * Copyright (C) 2025 Intel Corporation
+ * Copyright (C) 2026 Kate Hsuan <hpa@redhat.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.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 IMX471_REG_MODE_SELECT			CCI_REG8(0x0100)
+#define IMX471_MODE_STANDBY			0x00
+#define IMX471_MODE_STREAMING			0x01
+
+/* Chip ID */
+#define IMX471_REG_CHIP_ID			CCI_REG16(0x0016)
+#define IMX471_CHIP_ID				0x0471
+
+/* V_TIMING internal */
+#define IMX471_REG_FLL				CCI_REG16(0x0340)
+#define IMX471_FLL_MAX				0xffff
+
+/* Exposure control */
+#define IMX471_REG_EXPOSURE			CCI_REG16(0x0202)
+#define IMX471_EXPOSURE_MIN			1
+#define IMX471_EXPOSURE_STEP			1
+#define IMX471_EXPOSURE_DEFAULT			1270
+
+/* Default exposure margin */
+#define IMX471_EXPOSURE_MARGIN			18
+
+/* Analog gain control */
+#define IMX471_REG_ANALOG_GAIN			CCI_REG16(0x0204)
+#define IMX471_ANA_GAIN_MIN			0
+#define IMX471_ANA_GAIN_MAX			800
+#define IMX471_ANA_GAIN_STEP			1
+#define IMX471_ANA_GAIN_DEFAULT			0
+
+/* Digital gain control */
+#define IMX471_REG_DPGA_USE_GLOBAL_GAIN		CCI_REG16(0x3ff9)
+#define IMX471_REG_DIG_GAIN_GLOBAL		CCI_REG16(0x020e)
+#define IMX471_DGTL_GAIN_MIN			256
+#define IMX471_DGTL_GAIN_MAX			4095
+#define IMX471_DGTL_GAIN_STEP			1
+#define IMX471_DGTL_GAIN_DEFAULT		256
+
+/* HFLIP and VFLIP control */
+#define IMX471_REG_ORIENTATION			CCI_REG8(0x0101)
+#define IMX471_HFLIP_BIT			BIT(0)
+#define IMX471_VFLIP_BIT			BIT(1)
+
+/* Test Pattern Control */
+#define IMX471_REG_TEST_PATTERN			CCI_REG8(0x0600)
+#define IMX471_TEST_PATTERN_DISABLED		0
+#define IMX471_TEST_PATTERN_SOLID_COLOR		1
+#define IMX471_TEST_PATTERN_COLOR_BARS		2
+#define IMX471_TEST_PATTERN_GRAY_COLOR_BARS	3
+#define IMX471_TEST_PATTERN_PN9			4
+
+/* default link frequency and external clock */
+#define IMX471_LINK_FREQ_DEFAULT		200000000LL
+#define IMX471_EXT_CLK				19200000
+#define IMX471_LINK_FREQ_INDEX			0
+
+/* PLL */
+#define IMX471_REG_VTPXCK_DIV			CCI_REG8(0x0301)
+#define IMX471_REG_VTSYCK_DIV			CCI_REG8(0x0303)
+#define IMX471_REG_PREPLLCK_VT_DIV		CCI_REG8(0x0305)
+#define IMX471_REG_PLL_VT_MPY			CCI_REG16(0x0306)
+#define IMX471_REG_OPPXCK_DIV			CCI_REG8(0x0309)
+#define IMX471_REG_OPSYCK_DIV			CCI_REG8(0x030b)
+#define IMX471_REG_PLL_MULT_DRIV		CCI_REG8(0x0310)
+#define IMX471_PLL_SINGLE			0
+#define IMX471_PLL_DUAL				1
+
+/* IMX471 native and active pixel array size */
+#define IMX471_NATIVE_WIDTH			4672
+#define IMX471_NATIVE_HEIGHT			3512
+#define IMX471_PIXEL_ARRAY_LEFT			8
+#define IMX471_PIXEL_ARRAY_TOP			8
+#define IMX471_PIXEL_ARRAY_WIDTH		4656
+#define IMX471_PIXEL_ARRAY_HEIGHT		3496
+
+#define IMX471_REG_EXCK_FREQ			CCI_REG16(0x0136)
+#define IMX471_EXCK_FREQ(n)			((n) * 256)	/* n in MHz */
+
+#define IMX471_REG_CSI_DATA_FORMAT		CCI_REG16(0x0112)
+#define IMX471_CSI_DATA_FORMAT_RAW10		0x0a0a
+
+#define IMX471_REG_CSI_LANE_MODE		CCI_REG8(0x0114)
+#define IMX471_CSI_2_LANE_MODE			1
+#define IMX471_CSI_4_LANE_MODE			3
+
+#define IMX471_REG_X_ADD_STA			CCI_REG16(0x0344)
+#define IMX471_REG_Y_ADD_STA			CCI_REG16(0x0346)
+#define IMX471_REG_X_ADD_END			CCI_REG16(0x0348)
+#define IMX471_REG_Y_ADD_END			CCI_REG16(0x034a)
+#define IMX471_REG_X_OUTPUT_SIZE		CCI_REG16(0x034c)
+#define IMX471_REG_Y_OUTPUT_SIZE		CCI_REG16(0x034e)
+#define IMX471_REG_X_EVEN_INC			CCI_REG8(0x0381)
+#define IMX471_REG_X_ODD_INC			CCI_REG8(0x0383)
+#define IMX471_REG_Y_EVEN_INC			CCI_REG8(0x0385)
+#define IMX471_REG_Y_ODD_INC			CCI_REG8(0x0387)
+
+#define IMX471_REG_DIG_CROP_X_OFFSET		CCI_REG16(0x0408)
+#define IMX471_REG_DIG_CROP_Y_OFFSET		CCI_REG16(0x040a)
+#define IMX471_REG_DIG_CROP_WIDTH		CCI_REG16(0x040c)
+#define IMX471_REG_DIG_CROP_HEIGHT		CCI_REG16(0x040e)
+
+/* Binning mode */
+#define IMX471_REG_BINNING_MODE			CCI_REG8(0x0900)
+#define IMX471_BINNING_NONE			0
+#define IMX471_BINNING_ENABLE			1
+#define IMX471_REG_BINNING_TYPE			CCI_REG8(0x0901)
+#define IMX471_REG_BINNING_WEIGHTING		CCI_REG8(0x0902)
+
+#define to_imx471(_sd) container_of_const(_sd, struct imx471, sd)
+
+static const char * const imx471_supply_name[] = {
+	"avdd",
+};
+
+struct imx471_mode {
+	u32 width;
+	u32 height;
+
+	/* V-timing */
+	u32 fll_def;
+	u32 fll_min;
+
+	/* H-timing */
+	u32 llp;
+
+	u32 link_freq_index;
+
+	const struct cci_reg_sequence *default_mode_regs;
+	unsigned int default_mode_regs_length;
+};
+
+struct imx471 {
+	struct v4l2_subdev sd;
+	struct media_pad pad;
+
+	struct v4l2_ctrl_handler ctrl_handler;
+	struct v4l2_ctrl *vblank;
+	struct v4l2_ctrl *hblank;
+	struct v4l2_ctrl *vflip;
+	struct v4l2_ctrl *hflip;
+	struct v4l2_ctrl *exposure;
+
+	struct gpio_desc *reset_gpio;
+	struct regulator_bulk_data supplies[ARRAY_SIZE(imx471_supply_name)];
+	struct clk *img_clk;
+
+	struct device *dev;
+	struct regmap *regmap;
+};
+
+static const struct cci_reg_sequence imx471_global_regs[] = {
+	{ IMX471_REG_EXCK_FREQ, IMX471_EXCK_FREQ(19.2) },
+	{ CCI_REG8(0x3c7e), 0x08 },
+	{ CCI_REG8(0x3c7f), 0x05 },
+	{ CCI_REG8(0x3e35), 0x00 },
+	{ CCI_REG8(0x3e36), 0x00 },
+	{ CCI_REG8(0x3e37), 0x00 },
+	{ CCI_REG8(0x3f7f), 0x01 },
+	{ CCI_REG8(0x4431), 0x04 },
+	{ CCI_REG8(0x531c), 0x01 },
+	{ CCI_REG8(0x531d), 0x02 },
+	{ CCI_REG8(0x531e), 0x04 },
+	{ CCI_REG8(0x5928), 0x00 },
+	{ CCI_REG8(0x5929), 0x2f },
+	{ CCI_REG8(0x592a), 0x00 },
+	{ CCI_REG8(0x592b), 0x85 },
+	{ CCI_REG8(0x592c), 0x00 },
+	{ CCI_REG8(0x592d), 0x32 },
+	{ CCI_REG8(0x592e), 0x00 },
+	{ CCI_REG8(0x592f), 0x88 },
+	{ CCI_REG8(0x5930), 0x00 },
+	{ CCI_REG8(0x5931), 0x3d },
+	{ CCI_REG8(0x5932), 0x00 },
+	{ CCI_REG8(0x5933), 0x93 },
+	{ CCI_REG8(0x5938), 0x00 },
+	{ CCI_REG8(0x5939), 0x24 },
+	{ CCI_REG8(0x593a), 0x00 },
+	{ CCI_REG8(0x593b), 0x7a },
+	{ CCI_REG8(0x593c), 0x00 },
+	{ CCI_REG8(0x593d), 0x24 },
+	{ CCI_REG8(0x593e), 0x00 },
+	{ CCI_REG8(0x593f), 0x7a },
+	{ CCI_REG8(0x5940), 0x00 },
+	{ CCI_REG8(0x5941), 0x2f },
+	{ CCI_REG8(0x5942), 0x00 },
+	{ CCI_REG8(0x5943), 0x85 },
+	{ CCI_REG8(0x5f0e), 0x6e },
+	{ CCI_REG8(0x5f11), 0xc6 },
+	{ CCI_REG8(0x5f17), 0x5e },
+	{ CCI_REG8(0x7990), 0x01 },
+	{ CCI_REG8(0x7993), 0x5d },
+	{ CCI_REG8(0x7994), 0x5d },
+	{ CCI_REG8(0x7995), 0xa1 },
+	{ CCI_REG8(0x799a), 0x01 },
+	{ CCI_REG8(0x799d), 0x00 },
+	{ CCI_REG8(0x8169), 0x01 },
+	{ CCI_REG8(0x8359), 0x01 },
+	{ CCI_REG8(0x9302), 0x1e },
+	{ CCI_REG8(0x9306), 0x1f },
+	{ CCI_REG8(0x930a), 0x26 },
+	{ CCI_REG8(0x930e), 0x23 },
+	{ CCI_REG8(0x9312), 0x23 },
+	{ CCI_REG8(0x9316), 0x2c },
+	{ CCI_REG8(0x9317), 0x19 },
+	{ CCI_REG8(0xb046), 0x01 },
+	{ CCI_REG8(0xb048), 0x01 },
+};
+
+static const struct cci_reg_sequence mode_1928x1088_regs[] = {
+	{ IMX471_REG_CSI_DATA_FORMAT, IMX471_CSI_DATA_FORMAT_RAW10 },
+	{ IMX471_REG_CSI_LANE_MODE, IMX471_CSI_4_LANE_MODE },
+	{ IMX471_REG_X_ADD_STA, 8 },
+	{ IMX471_REG_Y_ADD_STA, 408 },
+	{ IMX471_REG_X_ADD_END, 4647 },
+	{ IMX471_REG_Y_ADD_END, 3051 },
+	{ IMX471_REG_X_EVEN_INC, 1 },
+	{ IMX471_REG_X_ODD_INC, 1 },
+	{ IMX471_REG_Y_EVEN_INC, 1 },
+	{ IMX471_REG_Y_ODD_INC, 1 },
+	{ IMX471_REG_BINNING_MODE, IMX471_BINNING_ENABLE },
+	{ IMX471_REG_BINNING_TYPE, 0x22 },
+	{ IMX471_REG_BINNING_WEIGHTING, 0x08 },
+	{ IMX471_REG_DIG_CROP_X_OFFSET, 208 },
+	{ IMX471_REG_DIG_CROP_Y_OFFSET, 108 },
+	{ IMX471_REG_DIG_CROP_WIDTH, 1928 },
+	{ IMX471_REG_DIG_CROP_HEIGHT, 1088 },
+	{ IMX471_REG_X_OUTPUT_SIZE, 1928 },
+	{ IMX471_REG_Y_OUTPUT_SIZE, 1088 },
+	{ IMX471_REG_VTPXCK_DIV, 0x06 },
+	{ IMX471_REG_VTSYCK_DIV, 0x02 },
+	{ IMX471_REG_PREPLLCK_VT_DIV, 0x02 },
+	{ IMX471_REG_PLL_VT_MPY, 0x0079 },
+	{ IMX471_REG_OPSYCK_DIV, 0x01 },
+	{ CCI_REG8(0x030d), 0x02 },
+	{ CCI_REG8(0x030e), 0x00 },
+	{ CCI_REG8(0x030f), 0x53 },
+	{ IMX471_REG_PLL_MULT_DRIV, IMX471_PLL_DUAL },
+	{ CCI_REG8(0x3f4c), 0x81 },
+	{ CCI_REG8(0x3f4d), 0x81 },
+	{ CCI_REG8(0x3f78), 0x01 },
+	{ CCI_REG8(0x3f79), 0x31 },
+	{ CCI_REG8(0x3ffe), 0x00 },
+	{ CCI_REG8(0x3fff), 0x8a },
+	{ CCI_REG8(0x5f0a), 0xb6 },
+};
+
+static const char * const imx471_test_pattern_menu[] = {
+	"Disabled",
+	"Solid Colour",
+	"Eight Vertical Colour Bars",
+	"Colour Bars With Fade to Grey",
+	"Pseudorandom Sequence (PN9)",
+};
+
+static const s64 link_freq_menu_items[] = {
+	IMX471_LINK_FREQ_DEFAULT,
+};
+
+/*
+ * The Bayer formats for the flipping.
+ * - no flip
+ * - h flip
+ * - v flip
+ * - h and v flips
+ */
+static const u32 imx471_hv_flips_bayer_order[] = {
+	MEDIA_BUS_FMT_SRGGB10_1X10,
+	MEDIA_BUS_FMT_SGRBG10_1X10,
+	MEDIA_BUS_FMT_SGBRG10_1X10,
+	MEDIA_BUS_FMT_SBGGR10_1X10,
+};
+
+static const struct imx471_mode imx471_modes[] = {
+	{
+		.width = 1928,
+		.height = 1088,
+		.fll_def = 1308,
+		.fll_min = 1308,
+		.llp = 2328,
+		.link_freq_index = IMX471_LINK_FREQ_INDEX,
+		.default_mode_regs = mode_1928x1088_regs,
+		.default_mode_regs_length = ARRAY_SIZE(mode_1928x1088_regs),
+	},
+};
+
+static int imx471_get_regulators(struct device *dev, struct imx471 *sensor)
+{
+	for (unsigned int i = 0; i < ARRAY_SIZE(imx471_supply_name); i++)
+		sensor->supplies[i].supply = imx471_supply_name[i];
+
+	return devm_regulator_bulk_get(dev, ARRAY_SIZE(imx471_supply_name),
+				       sensor->supplies);
+}
+
+static int imx471_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct imx471 *sensor = container_of_const(ctrl->handler,
+						   struct imx471,
+						   ctrl_handler);
+	struct v4l2_subdev_state *state =
+			v4l2_subdev_get_locked_active_state(&sensor->sd);
+	const struct v4l2_mbus_framefmt *format =
+			v4l2_subdev_state_get_format(state, 0);
+	int ret;
+
+	if (ctrl->id == V4L2_CID_VBLANK) {
+		s64 exposure_max = format->height + ctrl->val -
+				   IMX471_EXPOSURE_MARGIN;
+		ret = __v4l2_ctrl_modify_range(sensor->exposure,
+					       sensor->exposure->minimum,
+					       exposure_max,
+					       sensor->exposure->step,
+					       exposure_max);
+		if (ret)
+			return ret;
+	}
+
+	if (!pm_runtime_get_if_in_use(sensor->dev))
+		return 0;
+
+	switch (ctrl->id) {
+	case V4L2_CID_ANALOGUE_GAIN:
+		ret = cci_write(sensor->regmap, IMX471_REG_ANALOG_GAIN,
+				ctrl->val, NULL);
+		break;
+	case V4L2_CID_DIGITAL_GAIN:
+		ret = cci_write(sensor->regmap, IMX471_REG_DIG_GAIN_GLOBAL,
+				ctrl->val, NULL);
+		break;
+	case V4L2_CID_EXPOSURE:
+		ret = cci_write(sensor->regmap, IMX471_REG_EXPOSURE,
+				ctrl->val, &ret);
+		break;
+	case V4L2_CID_VBLANK:
+		/* Update FLL that meets expected vertical blanking */
+		ret = cci_write(sensor->regmap, IMX471_REG_FLL,
+				format->height + ctrl->val, &ret);
+		break;
+	case V4L2_CID_TEST_PATTERN:
+		ret = cci_write(sensor->regmap, IMX471_REG_TEST_PATTERN,
+				ctrl->val, NULL);
+		break;
+	case V4L2_CID_HFLIP:
+	case V4L2_CID_VFLIP:
+		ret = cci_write(sensor->regmap, IMX471_REG_ORIENTATION,
+				sensor->hflip->val | sensor->vflip->val << 1,
+				NULL);
+		break;
+	default:
+		ret = -EINVAL;
+		dev_err(sensor->dev, "ctrl(id:0x%x,val:0x%x) is not handled\n",
+			ctrl->id, ctrl->val);
+		break;
+	}
+
+	pm_runtime_put(sensor->dev);
+
+	return ret;
+}
+
+static const struct v4l2_ctrl_ops imx471_ctrl_ops = {
+	.s_ctrl = imx471_set_ctrl,
+};
+
+static u32 imx471_get_format_code(struct imx471 *sensor)
+{
+	unsigned int i;
+
+	i = (sensor->vflip->val ? 2 : 0) | (sensor->hflip->val ? 1 : 0);
+
+	return imx471_hv_flips_bayer_order[i];
+}
+
+static int imx471_enum_mbus_code(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *sd_state,
+				 struct v4l2_subdev_mbus_code_enum *code)
+{
+	struct imx471 *sensor = to_imx471(sd);
+
+	if (code->index >= (ARRAY_SIZE(imx471_hv_flips_bayer_order) / 4))
+		return -EINVAL;
+
+	code->code = imx471_get_format_code(sensor);
+
+	return 0;
+}
+
+static int imx471_enum_frame_size(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *sd_state,
+				  struct v4l2_subdev_frame_size_enum *fse)
+{
+	if (fse->index >= ARRAY_SIZE(imx471_modes))
+		return -EINVAL;
+
+	fse->min_width = imx471_modes[fse->index].width;
+	fse->max_width = fse->min_width;
+	fse->min_height = imx471_modes[fse->index].height;
+	fse->max_height = fse->min_height;
+
+	return 0;
+}
+
+static void imx471_update_pad_format(struct imx471 *sensor,
+				     const struct imx471_mode *mode,
+				     struct v4l2_subdev_format *fmt)
+{
+	fmt->format.code = imx471_get_format_code(sensor);
+	fmt->format.width = mode->width;
+	fmt->format.height = mode->height;
+	fmt->format.field = V4L2_FIELD_NONE;
+}
+
+static int imx471_set_pad_format(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *sd_state,
+				 struct v4l2_subdev_format *fmt)
+{
+	struct imx471 *sensor = to_imx471(sd);
+	const struct imx471_mode *mode;
+	int h_blank, ret;
+
+	mode = v4l2_find_nearest_size(imx471_modes, ARRAY_SIZE(imx471_modes),
+				      width, height, fmt->format.width,
+				      fmt->format.height);
+
+	imx471_update_pad_format(sensor, mode, fmt);
+
+	*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
+
+	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
+		return 0;
+
+	if (media_entity_is_streaming(&sensor->sd.entity))
+		return -EBUSY;
+
+	ret = __v4l2_ctrl_modify_range(sensor->vblank,
+				       mode->fll_min - mode->height,
+				       IMX471_FLL_MAX - mode->height,
+				       1,
+				       mode->fll_def - mode->height);
+	if (ret)
+		return ret;
+
+	h_blank = mode->llp - mode->width;
+	/*
+	 * Currently hblank is not changeable.
+	 * So FPS control is done only by vblank.
+	 */
+	return __v4l2_ctrl_modify_range(sensor->hblank, h_blank,
+					h_blank, 1, h_blank);
+}
+
+static int imx471_get_selection(struct v4l2_subdev *sd,
+				struct v4l2_subdev_state *sd_state,
+				struct v4l2_subdev_selection *sel)
+{
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP:
+		sel->r = *v4l2_subdev_state_get_crop(sd_state, sel->pad);
+		break;
+
+	case V4L2_SEL_TGT_NATIVE_SIZE:
+		sel->r.top = 0;
+		sel->r.left = 0;
+		sel->r.width = IMX471_NATIVE_WIDTH;
+		sel->r.height = IMX471_NATIVE_HEIGHT;
+		return 0;
+
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		sel->r.top = IMX471_PIXEL_ARRAY_TOP;
+		sel->r.left = IMX471_PIXEL_ARRAY_LEFT;
+		sel->r.width = IMX471_PIXEL_ARRAY_WIDTH;
+		sel->r.height = IMX471_PIXEL_ARRAY_HEIGHT;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int imx471_init_state(struct v4l2_subdev *sd,
+			     struct v4l2_subdev_state *sd_state)
+{
+	struct v4l2_subdev_format fmt = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+		.format = {
+			.code = MEDIA_BUS_FMT_SRGGB10_1X10,
+			.width = imx471_modes[0].width,
+			.height = imx471_modes[0].height,
+		},
+	};
+
+	return imx471_set_pad_format(sd, sd_state, &fmt);
+}
+
+static int imx471_identify_module(struct imx471 *sensor)
+{
+	int ret;
+	u64 val;
+
+	ret = cci_read(sensor->regmap, IMX471_REG_CHIP_ID, &val, NULL);
+	if (ret)
+		return dev_err_probe(sensor->dev, ret,
+				     "failed to read chip id\n");
+
+	if (val != IMX471_CHIP_ID)
+		return dev_err_probe(sensor->dev, -EIO,
+				     "chip id mismatch: %x!=%llx\n",
+				     IMX471_CHIP_ID, val);
+
+	return 0;
+}
+
+static int imx471_power_off(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct imx471 *sensor = to_imx471(sd);
+
+	clk_disable_unprepare(sensor->img_clk);
+	gpiod_set_value_cansleep(sensor->reset_gpio, 1);
+
+	regulator_bulk_disable(ARRAY_SIZE(imx471_supply_name),
+			       sensor->supplies);
+
+	return 0;
+}
+
+static int imx471_power_on(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct imx471 *sensor = to_imx471(sd);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(imx471_supply_name),
+				    sensor->supplies);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(sensor->img_clk);
+	if (ret < 0) {
+		regulator_bulk_disable(ARRAY_SIZE(imx471_supply_name),
+				       sensor->supplies);
+		dev_err(dev, "failed to enable imaging clock: %d\n", ret);
+		return ret;
+	}
+
+	gpiod_set_value_cansleep(sensor->reset_gpio, 0);
+
+	usleep_range(10000, 15000);
+
+	return 0;
+}
+
+static int imx471_enable_stream(struct v4l2_subdev *sd,
+				struct v4l2_subdev_state *state,
+				u32 pad, u64 streams_mask)
+{
+	struct imx471 *sensor = to_imx471(sd);
+	const struct imx471_mode *mode;
+	struct v4l2_mbus_framefmt *fmt;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(sensor->dev);
+	if (ret)
+		return ret;
+
+	ret = imx471_identify_module(sensor);
+	if (ret)
+		goto error_powerdown;
+
+	ret = cci_multi_reg_write(sensor->regmap, imx471_global_regs,
+				  ARRAY_SIZE(imx471_global_regs), NULL);
+	if (ret) {
+		dev_err(sensor->dev, "failed to set global settings: %d\n",
+			ret);
+		goto error_powerdown;
+	}
+
+	fmt = v4l2_subdev_state_get_format(state, 0);
+	mode = v4l2_find_nearest_size(imx471_modes, ARRAY_SIZE(imx471_modes),
+				      width, height, fmt->width, fmt->height);
+
+	ret = cci_multi_reg_write(sensor->regmap, mode->default_mode_regs,
+				  mode->default_mode_regs_length, NULL);
+	if (ret) {
+		dev_err(sensor->dev, "failed to set mode: %d\n", ret);
+		goto error_powerdown;
+	}
+
+	ret = cci_write(sensor->regmap, IMX471_REG_DPGA_USE_GLOBAL_GAIN, 1,
+			NULL);
+	if (ret)
+		goto error_powerdown;
+
+	ret = __v4l2_ctrl_handler_setup(&sensor->ctrl_handler);
+	if (ret)
+		goto error_powerdown;
+
+	ret = cci_write(sensor->regmap, IMX471_REG_MODE_SELECT,
+			IMX471_MODE_STREAMING, NULL);
+	if (ret)
+		goto error_powerdown;
+
+	__v4l2_ctrl_grab(sensor->vflip, true);
+	__v4l2_ctrl_grab(sensor->hflip, true);
+
+	return ret;
+
+error_powerdown:
+	pm_runtime_put(sensor->dev);
+
+	return ret;
+}
+
+static int imx471_disable_stream(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state,
+				 u32 pad, u64 streams_mask)
+{
+	struct imx471 *sensor = to_imx471(sd);
+	int ret;
+
+	ret = cci_write(sensor->regmap, IMX471_REG_MODE_SELECT,
+			IMX471_MODE_STANDBY, NULL);
+	pm_runtime_put(sensor->dev);
+
+	if (ret)
+		dev_err(sensor->dev,
+			"failed to disable stream with return value: %d\n",
+			ret);
+
+	__v4l2_ctrl_grab(sensor->vflip, false);
+	__v4l2_ctrl_grab(sensor->hflip, false);
+
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops imx471_video_ops = {
+	.s_stream = v4l2_subdev_s_stream_helper,
+};
+
+static const struct v4l2_subdev_pad_ops imx471_pad_ops = {
+	.enum_mbus_code = imx471_enum_mbus_code,
+	.get_fmt = v4l2_subdev_get_fmt,
+	.set_fmt = imx471_set_pad_format,
+	.get_selection = imx471_get_selection,
+	.enum_frame_size = imx471_enum_frame_size,
+	.enable_streams = imx471_enable_stream,
+	.disable_streams = imx471_disable_stream,
+};
+
+static const struct v4l2_subdev_ops imx471_subdev_ops = {
+	.video = &imx471_video_ops,
+	.pad = &imx471_pad_ops,
+};
+
+static const struct v4l2_subdev_internal_ops imx471_internal_ops = {
+	.init_state = imx471_init_state,
+};
+
+static int imx471_init_controls(struct imx471 *sensor)
+{
+	const struct imx471_mode *mode = &imx471_modes[0];
+	struct v4l2_fwnode_device_properties props;
+	struct v4l2_ctrl_handler *ctrl_hdlr;
+	struct v4l2_ctrl *link_freq;
+	s64 exposure_max, hblank;
+	u64 pixel_rate;
+	int ret;
+
+	ret = v4l2_fwnode_device_parse(sensor->dev, &props);
+	if (ret) {
+		dev_err(sensor->dev, "failed to parse fwnode: %d\n", ret);
+		return ret;
+	}
+
+	ctrl_hdlr = &sensor->ctrl_handler;
+	v4l2_ctrl_handler_init(ctrl_hdlr, 12);
+
+	v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &imx471_ctrl_ops, &props);
+
+	link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
+					   &imx471_ctrl_ops,
+					   V4L2_CID_LINK_FREQ,
+					   ARRAY_SIZE(link_freq_menu_items) - 1,
+					   0,
+					   link_freq_menu_items);
+
+	/* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
+	pixel_rate = div_u64(IMX471_LINK_FREQ_DEFAULT * 2 * 4, 10);
+
+	v4l2_ctrl_new_std(ctrl_hdlr, &imx471_ctrl_ops,
+			  V4L2_CID_PIXEL_RATE, pixel_rate,
+			  pixel_rate, 1, pixel_rate);
+
+	sensor->vblank = v4l2_ctrl_new_std(ctrl_hdlr,
+					   &imx471_ctrl_ops,
+					   V4L2_CID_VBLANK,
+					   mode->fll_min - mode->height,
+					   IMX471_FLL_MAX - mode->height,
+					   1,
+					   mode->fll_def - mode->height);
+
+	hblank = mode->llp - mode->width;
+	sensor->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx471_ctrl_ops,
+					   V4L2_CID_HBLANK, hblank, hblank,
+					   1, hblank);
+
+	/* fll >= exposure time + adjust parameter (default value is 18) */
+	exposure_max = mode->fll_def - IMX471_EXPOSURE_MARGIN;
+	sensor->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx471_ctrl_ops,
+					     V4L2_CID_EXPOSURE,
+					     IMX471_EXPOSURE_MIN, exposure_max,
+					     IMX471_EXPOSURE_STEP,
+					     IMX471_EXPOSURE_DEFAULT);
+
+	v4l2_ctrl_new_std(ctrl_hdlr, &imx471_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
+			  IMX471_ANA_GAIN_MIN, IMX471_ANA_GAIN_MAX,
+			  IMX471_ANA_GAIN_STEP, IMX471_ANA_GAIN_DEFAULT);
+
+	v4l2_ctrl_new_std(ctrl_hdlr, &imx471_ctrl_ops, V4L2_CID_DIGITAL_GAIN,
+			  IMX471_DGTL_GAIN_MIN, IMX471_DGTL_GAIN_MAX,
+			  IMX471_DGTL_GAIN_STEP, IMX471_DGTL_GAIN_DEFAULT);
+
+	v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &imx471_ctrl_ops,
+				     V4L2_CID_TEST_PATTERN,
+				     ARRAY_SIZE(imx471_test_pattern_menu) - 1,
+				     0, 0, imx471_test_pattern_menu);
+
+	sensor->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx471_ctrl_ops,
+					  V4L2_CID_HFLIP, 0, 1, 1, 0);
+
+	sensor->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx471_ctrl_ops,
+					  V4L2_CID_VFLIP, 0, 1, 1, 0);
+
+	if (ctrl_hdlr->error) {
+		dev_err(sensor->dev, "%s control init failed: %d\n",
+			__func__, ctrl_hdlr->error);
+		goto error;
+	}
+
+	link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	sensor->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	sensor->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+	sensor->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+
+	sensor->sd.ctrl_handler = ctrl_hdlr;
+
+	return 0;
+
+error:
+	v4l2_ctrl_handler_free(ctrl_hdlr);
+
+	return ctrl_hdlr->error;
+}
+
+static int imx471_check_hwcfg(struct imx471 *sensor)
+{
+	struct v4l2_fwnode_endpoint bus_cfg = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY,
+	};
+	struct fwnode_handle *ep, *fwnode = dev_fwnode(sensor->dev);
+	unsigned long link_freq_bitmap;
+	struct clk *clk;
+	int ret;
+
+	clk = devm_v4l2_sensor_clk_get(sensor->dev, NULL);
+	if (IS_ERR(clk))
+		return dev_err_probe(sensor->dev, PTR_ERR(clk),
+				     "can't get clock frequency\n");
+
+	if (clk_get_rate(clk) != IMX471_EXT_CLK)
+		return dev_err_probe(sensor->dev, -EINVAL,
+				     "external clock %lu is not supported\n",
+				     clk_get_rate(clk));
+
+	ep = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0, 0);
+	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
+	fwnode_handle_put(ep);
+	if (ret)
+		return dev_err_probe(sensor->dev, ret,
+				     "parsing endpoint failed\n");
+
+	if (bus_cfg.bus.mipi_csi2.num_data_lanes != 4) {
+		ret = dev_err_probe(sensor->dev, -EINVAL,
+				    "number of CSI2 data lanes %u is not supported\n",
+				    bus_cfg.bus.mipi_csi2.num_data_lanes);
+		goto done_endpoint_free;
+	}
+
+	ret = v4l2_link_freq_to_bitmap(sensor->dev, bus_cfg.link_frequencies,
+				       bus_cfg.nr_of_link_frequencies,
+				       link_freq_menu_items,
+				       ARRAY_SIZE(link_freq_menu_items),
+				       &link_freq_bitmap);
+
+done_endpoint_free:
+	v4l2_fwnode_endpoint_free(&bus_cfg);
+
+	return ret;
+}
+
+static int imx471_probe(struct i2c_client *client)
+{
+	struct imx471 *sensor;
+	int ret;
+
+	sensor = devm_kzalloc(&client->dev, sizeof(*sensor), GFP_KERNEL);
+	if (!sensor)
+		return dev_err_probe(&client->dev, -ENOMEM,
+				     "failed to allocate memory\n");
+
+	sensor->dev = &client->dev;
+
+	ret = imx471_check_hwcfg(sensor);
+	if (ret)
+		return dev_err_probe(sensor->dev, ret,
+				     "failed to check hwcfg: %d\n", ret);
+
+	ret = imx471_get_regulators(sensor->dev, sensor);
+	if (ret)
+		return dev_err_probe(sensor->dev, ret,
+				     "failed to get regulators\n");
+
+	sensor->reset_gpio = devm_gpiod_get_optional(sensor->dev, "reset",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(sensor->reset_gpio))
+		return dev_err_probe(sensor->dev, PTR_ERR(sensor->reset_gpio),
+				     "failed to get reset gpio\n");
+
+	sensor->img_clk = devm_v4l2_sensor_clk_get(sensor->dev, NULL);
+	if (IS_ERR(sensor->img_clk))
+		return dev_err_probe(sensor->dev, PTR_ERR(sensor->img_clk),
+				     "failed to get imaging clock\n");
+
+	v4l2_i2c_subdev_init(&sensor->sd, client, &imx471_subdev_ops);
+
+	sensor->regmap = devm_cci_regmap_init_i2c(client, 16);
+	if (IS_ERR(sensor->regmap))
+		return dev_err_probe(sensor->dev, PTR_ERR(sensor->regmap),
+				     "failed to initialize CCI\n");
+
+	ret = imx471_power_on(sensor->dev);
+	if (ret)
+		return dev_err_probe(sensor->dev, ret,
+				     "failed to power on\n");
+
+	ret = imx471_identify_module(sensor);
+	if (ret) {
+		dev_err_probe(sensor->dev, ret, "failed to find sensor: %d\n",
+			      ret);
+		goto error_power_off;
+	}
+
+	ret = imx471_init_controls(sensor);
+	if (ret) {
+		dev_err_probe(sensor->dev, ret, "failed to init controls: %d\n",
+			      ret);
+		goto error_power_off;
+	}
+
+	sensor->sd.internal_ops = &imx471_internal_ops;
+	sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
+	sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+
+	ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad);
+	if (ret) {
+		dev_err_probe(sensor->dev, ret,
+			      "failed to init entity pads: %d\n", ret);
+		goto error_v4l2_ctrl_handler_free;
+	}
+
+	sensor->sd.state_lock = sensor->ctrl_handler.lock;
+	ret = v4l2_subdev_init_finalize(&sensor->sd);
+	if (ret < 0) {
+		dev_err_probe(sensor->dev, ret, "failed to init subdev: %d\n",
+			      ret);
+		goto error_media_entity_pm;
+	}
+
+	pm_runtime_set_active(sensor->dev);
+	pm_runtime_enable(sensor->dev);
+
+	ret = v4l2_async_register_subdev_sensor(&sensor->sd);
+	if (ret < 0)
+		goto error_v4l2_subdev_cleanup;
+
+	pm_runtime_idle(sensor->dev);
+
+	return 0;
+
+error_v4l2_subdev_cleanup:
+	pm_runtime_disable(sensor->dev);
+	pm_runtime_set_suspended(sensor->dev);
+	v4l2_subdev_cleanup(&sensor->sd);
+
+error_media_entity_pm:
+	media_entity_cleanup(&sensor->sd.entity);
+
+error_v4l2_ctrl_handler_free:
+	v4l2_ctrl_handler_free(sensor->sd.ctrl_handler);
+
+error_power_off:
+	imx471_power_off(sensor->dev);
+
+	return ret;
+}
+
+static void imx471_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+
+	v4l2_async_unregister_subdev(sd);
+	v4l2_subdev_cleanup(sd);
+	media_entity_cleanup(&sd->entity);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
+
+	pm_runtime_disable(&client->dev);
+
+	if (!pm_runtime_status_suspended(&client->dev)) {
+		imx471_power_off(&client->dev);
+		pm_runtime_set_suspended(&client->dev);
+	}
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(imx471_pm_ops, imx471_power_off,
+				 imx471_power_on, NULL);
+
+static const struct acpi_device_id imx471_acpi_ids[] __maybe_unused = {
+	{ "SONY471A" },
+	{ "TBE20A0" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(acpi, imx471_acpi_ids);
+
+static struct i2c_driver imx471_i2c_driver = {
+	.driver = {
+		.name = "imx471",
+		.acpi_match_table = ACPI_PTR(imx471_acpi_ids),
+		.pm = pm_sleep_ptr(&imx471_pm_ops),
+	},
+	.probe = imx471_probe,
+	.remove = imx471_remove,
+};
+module_i2c_driver(imx471_i2c_driver);
+
+MODULE_AUTHOR("Jimmy Su <jimmy.su@intel.com>");
+MODULE_AUTHOR("Serin Yeh <serin.yeh@intel.com>");
+MODULE_AUTHOR("Kate Hsuan <hpa@redhat.com>");
+MODULE_DESCRIPTION("Sony imx471 sensor driver");
+MODULE_LICENSE("GPL");
-- 
2.54.0

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Tarang Raval]

Hi Kate,

> Add a new driver for Sony imx471 camera sensor. It is based on
> Jimmy Su <jimmy.su@intel.com> implementation and the driver can be found
> in the following URL.
> https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c
>
> This sensor can be found on Lenovo X1 Carbon G14, X9-14 and X9-15 laptops
> and it is a part of IPU7 solution. The driver was tested on Lenovo X1
> Carbon G14, X9-14 and X9-15 laptops.
>
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---

...

> +static const char * const imx471_supply_name[] = {
> +       "avdd",
> +};

As we discussed in the last revision, this should be "vana" instead of "avdd".
Better to use the regulator name as defined in the datasheet.

Other than that, the driver looks good to me.

Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>

Best Regards,
Tarang

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Sakari Ailus]

Hi Kate,

Thanks for the update.

On Wed, Jun 24, 2026 at 11:35:08AM +0800, Kate Hsuan wrote:
...

> +static int imx471_set_pad_format(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_state *sd_state,
> +				 struct v4l2_subdev_format *fmt)
> +{
> +	struct imx471 *sensor = to_imx471(sd);
> +	const struct imx471_mode *mode;
> +	int h_blank, ret;
> +
> +	mode = v4l2_find_nearest_size(imx471_modes, ARRAY_SIZE(imx471_modes),
> +				      width, height, fmt->format.width,
> +				      fmt->format.height);
> +
> +	imx471_update_pad_format(sensor, mode, fmt);
> +
> +	*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
> +
> +	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> +		return 0;
> +
> +	if (media_entity_is_streaming(&sensor->sd.entity))
> +		return -EBUSY;
> +
> +	ret = __v4l2_ctrl_modify_range(sensor->vblank,
> +				       mode->fll_min - mode->height,
> +				       IMX471_FLL_MAX - mode->height,
> +				       1,
> +				       mode->fll_def - mode->height);
> +	if (ret)
> +		return ret;
> +
> +	h_blank = mode->llp - mode->width;
> +	/*
> +	 * Currently hblank is not changeable.
> +	 * So FPS control is done only by vblank.
> +	 */
> +	return __v4l2_ctrl_modify_range(sensor->hblank, h_blank,
> +					h_blank, 1, h_blank);
> +}

...

> +
> +static int imx471_init_state(struct v4l2_subdev *sd,
> +			     struct v4l2_subdev_state *sd_state)
> +{
> +	struct v4l2_subdev_format fmt = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,

The purpose of the init_state op is to initialise the provided state only,
it's not allowed to change the sensor configuration.

> +		.format = {
> +			.code = MEDIA_BUS_FMT_SRGGB10_1X10,
> +			.width = imx471_modes[0].width,
> +			.height = imx471_modes[0].height,
> +		},
> +	};
> +
> +	return imx471_set_pad_format(sd, sd_state, &fmt);
> +}

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Kate Hsuan]

Hi Tarang,

Thank you for reviewing.

On Wed, Jun 24, 2026 at 4:43 PM Tarang Raval
<tarang.raval@siliconsignals.io> wrote:
>
> Hi Kate,
>
> > Add a new driver for Sony imx471 camera sensor. It is based on
> > Jimmy Su <jimmy.su@intel.com> implementation and the driver can be found
> > in the following URL.
> > https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c
> >
> > This sensor can be found on Lenovo X1 Carbon G14, X9-14 and X9-15 laptops
> > and it is a part of IPU7 solution. The driver was tested on Lenovo X1
> > Carbon G14, X9-14 and X9-15 laptops.
> >
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > ---
>
> ...
>
> > +static const char * const imx471_supply_name[] = {
> > +       "avdd",
> > +};
>
> As we discussed in the last revision, this should be "vana" instead of "avdd".
> Better to use the regulator name as defined in the datasheet.

"avdd" is exposed by INT3472. If it was changed to "vana", the driver complained
[  205.839599] imx471 i2c-SONY471A:00: supply vana not found, using
dummy regulator
[  205.854821] imx471 i2c-SONY471A:00: Error reading reg 0x0016: -121
[  205.854830] imx471 i2c-SONY471A:00: error -EREMOTEIO: failed to read chip id
[  205.854834] imx471 i2c-SONY471A:00: error -EREMOTEIO: failed to
find sensor: -121
[  205.855354] imx471 i2c-SONY471A:00: probe with driver imx471 failed
with error -121

So, avdd is necessary.

>
> Other than that, the driver looks good to me.
>
> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
>
> Best Regards,
> Tarang
>


-- 
BR,
Kate

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Kate Hsuan]

Hi Sakari,

Thank you for reviewing.

On Wed, Jun 24, 2026 at 4:56 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Kate,
>
> Thanks for the update.
>
> On Wed, Jun 24, 2026 at 11:35:08AM +0800, Kate Hsuan wrote:
> ...
>
> > +static int imx471_set_pad_format(struct v4l2_subdev *sd,
> > +                              struct v4l2_subdev_state *sd_state,
> > +                              struct v4l2_subdev_format *fmt)
> > +{
> > +     struct imx471 *sensor = to_imx471(sd);
> > +     const struct imx471_mode *mode;
> > +     int h_blank, ret;
> > +
> > +     mode = v4l2_find_nearest_size(imx471_modes, ARRAY_SIZE(imx471_modes),
> > +                                   width, height, fmt->format.width,
> > +                                   fmt->format.height);
> > +
> > +     imx471_update_pad_format(sensor, mode, fmt);
> > +
> > +     *v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
> > +
> > +     if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > +             return 0;
> > +
> > +     if (media_entity_is_streaming(&sensor->sd.entity))
> > +             return -EBUSY;
> > +
> > +     ret = __v4l2_ctrl_modify_range(sensor->vblank,
> > +                                    mode->fll_min - mode->height,
> > +                                    IMX471_FLL_MAX - mode->height,
> > +                                    1,
> > +                                    mode->fll_def - mode->height);
> > +     if (ret)
> > +             return ret;
> > +
> > +     h_blank = mode->llp - mode->width;
> > +     /*
> > +      * Currently hblank is not changeable.
> > +      * So FPS control is done only by vblank.
> > +      */
> > +     return __v4l2_ctrl_modify_range(sensor->hblank, h_blank,
> > +                                     h_blank, 1, h_blank);
> > +}
>
> ...
>
> > +
> > +static int imx471_init_state(struct v4l2_subdev *sd,
> > +                          struct v4l2_subdev_state *sd_state)
> > +{
> > +     struct v4l2_subdev_format fmt = {
> > +             .which = V4L2_SUBDEV_FORMAT_ACTIVE,
>
> The purpose of the init_state op is to initialise the provided state only,
> it's not allowed to change the sensor configuration.

The input parameter sd_state could initialise it. I'll change this as follows
.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;

>
> > +             .format = {
> > +                     .code = MEDIA_BUS_FMT_SRGGB10_1X10,
> > +                     .width = imx471_modes[0].width,
> > +                     .height = imx471_modes[0].height,
> > +             },
> > +     };
> > +
> > +     return imx471_set_pad_format(sd, sd_state, &fmt);
> > +}
>
> --
> Kind regards,
>
> Sakari Ailus
>


-- 
BR,
Kate

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Tarang Raval]

Hi kate,

> On Wed, Jun 24, 2026 at 4:43 PM Tarang Raval
> <tarang.raval@siliconsignals.io> wrote:
> >
> > Hi Kate,
> >
> > > Add a new driver for Sony imx471 camera sensor. It is based on
> > > Jimmy Su <jimmy.su@intel.com> implementation and the driver can be found
> > > in the following URL.
> > > https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c
> > >
> > > This sensor can be found on Lenovo X1 Carbon G14, X9-14 and X9-15 laptops
> > > and it is a part of IPU7 solution. The driver was tested on Lenovo X1
> > > Carbon G14, X9-14 and X9-15 laptops.
> > >
> > > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > > ---
> >
> > ...
> >
> > > +static const char * const imx471_supply_name[] = {
> > > +       "avdd",
> > > +};
> >
> > As we discussed in the last revision, this should be "vana" instead of "avdd".
> > Better to use the regulator name as defined in the datasheet.
>
> "avdd" is exposed by INT3472. If it was changed to "vana", the driver complained
> [  205.839599] imx471 i2c-SONY471A:00: supply vana not found, using
> dummy regulator
> [  205.854821] imx471 i2c-SONY471A:00: Error reading reg 0x0016: -121
> [  205.854830] imx471 i2c-SONY471A:00: error -EREMOTEIO: failed to read chip id
> [  205.854834] imx471 i2c-SONY471A:00: error -EREMOTEIO: failed to
> find sensor: -121
> [  205.855354] imx471 i2c-SONY471A:00: probe with driver imx471 failed
> with error -121
>
> So, avdd is necessary.

Could you please try adding a mapping for SONY471A in
int3472_gpio_map[] in drivers/platform/x86/intel/int3472/discrete.c?

Not fully sure, I have to check, but hopefully it will work.

Best Regards,
Tarang

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Sakari Ailus]

On Wed, Jun 24, 2026 at 09:58:11AM +0000, Tarang Raval wrote:
> > So, avdd is necessary.
> 
> Could you please try adding a mapping for SONY471A in
> int3472_gpio_map[] in drivers/platform/x86/intel/int3472/discrete.c?
> 
> Not fully sure, I have to check, but hopefully it will work.

That's the intent, yes.

-- 
Sakari Ailus

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Sakari Ailus]

Hi Kate,

On Wed, Jun 24, 2026 at 05:30:59PM +0800, Kate Hsuan wrote:
> Hi Sakari,
> 
> Thank you for reviewing.
> 
> On Wed, Jun 24, 2026 at 4:56 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Kate,
> >
> > Thanks for the update.
> >
> > On Wed, Jun 24, 2026 at 11:35:08AM +0800, Kate Hsuan wrote:
> > ...
> >
> > > +static int imx471_set_pad_format(struct v4l2_subdev *sd,
> > > +                              struct v4l2_subdev_state *sd_state,
> > > +                              struct v4l2_subdev_format *fmt)
> > > +{
> > > +     struct imx471 *sensor = to_imx471(sd);
> > > +     const struct imx471_mode *mode;
> > > +     int h_blank, ret;
> > > +
> > > +     mode = v4l2_find_nearest_size(imx471_modes, ARRAY_SIZE(imx471_modes),
> > > +                                   width, height, fmt->format.width,
> > > +                                   fmt->format.height);
> > > +
> > > +     imx471_update_pad_format(sensor, mode, fmt);
> > > +
> > > +     *v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
> > > +
> > > +     if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > > +             return 0;
> > > +
> > > +     if (media_entity_is_streaming(&sensor->sd.entity))
> > > +             return -EBUSY;
> > > +
> > > +     ret = __v4l2_ctrl_modify_range(sensor->vblank,
> > > +                                    mode->fll_min - mode->height,
> > > +                                    IMX471_FLL_MAX - mode->height,
> > > +                                    1,
> > > +                                    mode->fll_def - mode->height);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     h_blank = mode->llp - mode->width;
> > > +     /*
> > > +      * Currently hblank is not changeable.
> > > +      * So FPS control is done only by vblank.
> > > +      */
> > > +     return __v4l2_ctrl_modify_range(sensor->hblank, h_blank,
> > > +                                     h_blank, 1, h_blank);
> > > +}
> >
> > ...
> >
> > > +
> > > +static int imx471_init_state(struct v4l2_subdev *sd,
> > > +                          struct v4l2_subdev_state *sd_state)
> > > +{
> > > +     struct v4l2_subdev_format fmt = {
> > > +             .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> >
> > The purpose of the init_state op is to initialise the provided state only,
> > it's not allowed to change the sensor configuration.
> 
> The input parameter sd_state could initialise it. I'll change this as follows
> .which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;

sd_state is supposed to be always non-NULL. So you can use FORMAT_TRY
always.

-- 
Regards,

Sakari Ailus

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Vladimir Zapolskiy]

On 6/24/26 06:35, Kate Hsuan wrote:
> Add a new driver for Sony imx471 camera sensor. It is based on
> Jimmy Su <jimmy.su@intel.com> implementation and the driver can be found
> in the following URL.
> https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c
> 
> This sensor can be found on Lenovo X1 Carbon G14, X9-14 and X9-15 laptops
> and it is a part of IPU7 solution. The driver was tested on Lenovo X1
> Carbon G14, X9-14 and X9-15 laptops.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>

Please find a few minor nitpicks below.

> ---
>   MAINTAINERS                |   6 +
>   drivers/media/i2c/Kconfig  |  10 +
>   drivers/media/i2c/Makefile |   1 +
>   drivers/media/i2c/imx471.c | 971 +++++++++++++++++++++++++++++++++++++
>   4 files changed, 988 insertions(+)
>   create mode 100644 drivers/media/i2c/imx471.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6b4560681b51..586958b1816d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -25219,6 +25219,12 @@ T:	git git://linuxtv.org/media.git
>   F:	Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
>   F:	drivers/media/i2c/imx415.c
>   
> +SONY IMX471 SENSOR DRIVER
> +M:	Kate Hsuan <hpa@redhat.com>
> +L:	linux-media@vger.kernel.org
> +S:	Maintained
> +F:	drivers/media/i2c/imx471.c
> +
>   SONY MEMORYSTICK SUBSYSTEM
>   M:	Maxim Levitsky <maximlevitsky@gmail.com>
>   M:	Alex Dubov <oakad@yahoo.com>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 5d173e0ecf42..b7199f9f5a0c 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -287,6 +287,16 @@ config VIDEO_IMX415
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called imx415.
>   
> +config VIDEO_IMX471
> +	tristate "Sony IMX471 sensor support"
> +	select V4L2_CCI_I2C
> +	help
> +	  This is a Video4Linux2 sensor driver for the Sony
> +	  IMX471 camera.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called imx471.
> +
>   config VIDEO_MAX9271_LIB
>   	tristate
>   
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index e45359efe0e4..acbd321fc12e 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_IMX335) += imx335.o
>   obj-$(CONFIG_VIDEO_IMX355) += imx355.o
>   obj-$(CONFIG_VIDEO_IMX412) += imx412.o
>   obj-$(CONFIG_VIDEO_IMX415) += imx415.o
> +obj-$(CONFIG_VIDEO_IMX471) += imx471.o
>   obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
>   obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o
>   obj-$(CONFIG_VIDEO_KS0127) += ks0127.o
> diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
> new file mode 100644
> index 000000000000..1e1bff69ea3d
> --- /dev/null
> +++ b/drivers/media/i2c/imx471.c
> @@ -0,0 +1,971 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * imx471.c - imx471 sensor driver
> + *
> + * Copyright (C) 2025 Intel Corporation
> + * Copyright (C) 2026 Kate Hsuan <hpa@redhat.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/unaligned.h>

No declarations coming from linux/unaligned.h header are used in the driver.

> +#include <media/v4l2-cci.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>

No declarations coming from media/v4l-event.h header are used in the driver,
likely it's intended to include media/v4l2-subdev.h instead.

> +#include <media/v4l2-event.h>

No declarations coming from media/v4l-event.h header are used in the driver.

> +#include <media/v4l2-fwnode.h>
> +
> +#define IMX471_REG_MODE_SELECT			CCI_REG8(0x0100)
> +#define IMX471_MODE_STANDBY			0x00
> +#define IMX471_MODE_STREAMING			0x01
> +
> +/* Chip ID */
> +#define IMX471_REG_CHIP_ID			CCI_REG16(0x0016)
> +#define IMX471_CHIP_ID				0x0471
> +
> +/* V_TIMING internal */
> +#define IMX471_REG_FLL				CCI_REG16(0x0340)
> +#define IMX471_FLL_MAX				0xffff
> +
> +/* Exposure control */
> +#define IMX471_REG_EXPOSURE			CCI_REG16(0x0202)
> +#define IMX471_EXPOSURE_MIN			1
> +#define IMX471_EXPOSURE_STEP			1
> +#define IMX471_EXPOSURE_DEFAULT			1270
> +
> +/* Default exposure margin */
> +#define IMX471_EXPOSURE_MARGIN			18
> +
> +/* Analog gain control */
> +#define IMX471_REG_ANALOG_GAIN			CCI_REG16(0x0204)
> +#define IMX471_ANA_GAIN_MIN			0
> +#define IMX471_ANA_GAIN_MAX			800
> +#define IMX471_ANA_GAIN_STEP			1
> +#define IMX471_ANA_GAIN_DEFAULT			0
> +
> +/* Digital gain control */
> +#define IMX471_REG_DPGA_USE_GLOBAL_GAIN		CCI_REG16(0x3ff9)
> +#define IMX471_REG_DIG_GAIN_GLOBAL		CCI_REG16(0x020e)
> +#define IMX471_DGTL_GAIN_MIN			256
> +#define IMX471_DGTL_GAIN_MAX			4095
> +#define IMX471_DGTL_GAIN_STEP			1
> +#define IMX471_DGTL_GAIN_DEFAULT		256
> +
> +/* HFLIP and VFLIP control */
> +#define IMX471_REG_ORIENTATION			CCI_REG8(0x0101)
> +#define IMX471_HFLIP_BIT			BIT(0)
> +#define IMX471_VFLIP_BIT			BIT(1)

IMX471_HFLIP_BIT and IMX471_VFLIP_BIT macro do not have any users in the driver.

> +
> +/* Test Pattern Control */
> +#define IMX471_REG_TEST_PATTERN			CCI_REG8(0x0600)
> +#define IMX471_TEST_PATTERN_DISABLED		0
> +#define IMX471_TEST_PATTERN_SOLID_COLOR		1
> +#define IMX471_TEST_PATTERN_COLOR_BARS		2
> +#define IMX471_TEST_PATTERN_GRAY_COLOR_BARS	3
> +#define IMX471_TEST_PATTERN_PN9			4

IMX471_TEST_PATTERN_* macro do not have any users in the driver.

> +
> +/* default link frequency and external clock */
> +#define IMX471_LINK_FREQ_DEFAULT		200000000LL
> +#define IMX471_EXT_CLK				19200000
> +#define IMX471_LINK_FREQ_INDEX			0

Please drop IMX471_LINK_FREQ_INDEX macro along with .link_freq_index from
struct imx471_mode.

-- 
Best wishes,
Vladimir

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Kieran Bingham]

Hi Kate,

Quoting Kate Hsuan (2026-06-24 04:35:08)
> Add a new driver for Sony imx471 camera sensor. It is based on
> Jimmy Su <jimmy.su@intel.com> implementation and the driver can be found
> in the following URL.
> https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c
> 
> This sensor can be found on Lenovo X1 Carbon G14, X9-14 and X9-15 laptops
> and it is a part of IPU7 solution. The driver was tested on Lenovo X1
> Carbon G14, X9-14 and X9-15 laptops.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---

... <snip>

> +#define IMX471_REG_EXCK_FREQ                   CCI_REG16(0x0136)
> +#define IMX471_EXCK_FREQ(n)                    ((n) * 256)     /* n in MHz */
> 

<snip>

> +
> +static const struct cci_reg_sequence imx471_global_regs[] = {
> +       { IMX471_REG_EXCK_FREQ, IMX471_EXCK_FREQ(19.2) },


Does this work? Is this a compile time constant multiplying floating
point with the compiler, or something happening in the kernel?

I'll be happy if it does, but it catches my attention as something I
thought we couldn't do.

--
Kieran

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Tarang Raval]

Hi Kieran,

> Quoting Kate Hsuan (2026-06-24 04:35:08)                                      
> > Add a new driver for Sony imx471 camera sensor. It is based on              
> > Jimmy Su <jimmy.su@intel.com> implementation and the driver can be found    
> > in the following URL.                                                       
> > https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c
> >                                                                             
> > This sensor can be found on Lenovo X1 Carbon G14, X9-14 and X9-15 laptops   
> > and it is a part of IPU7 solution. The driver was tested on Lenovo X1       
> > Carbon G14, X9-14 and X9-15 laptops.                                        
> >                                                                             
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>                                  
> > ---                                                                         
>                                                                               
> ... <snip>                                                                    
>                                                                               
> > +#define IMX471_REG_EXCK_FREQ                   CCI_REG16(0x0136)           
> > +#define IMX471_EXCK_FREQ(n)                    ((n) * 256)     /* n in MHz */
> >                                                                             
>                                                                               
> <snip>                                                                        
>                                                                               
> > +                                                                           
> > +static const struct cci_reg_sequence imx471_global_regs[] = {              
> > +       { IMX471_REG_EXCK_FREQ, IMX471_EXCK_FREQ(19.2) },                   
>                                                                               
>                                                                               
> Does this work? Is this a compile time constant multiplying floating          
> point with the compiler, or something happening in the kernel?                
>                                                                               
> I'll be happy if it does, but it catches my attention as something I          
> thought we couldn't do.                                                       
                                                                                
I tested this with GCC. It evaluates the expression at compile time and
folds it into the integer constant 4915. The generated assembly/object
file contains .quad 4915 in the initialized data and no floating-point 
instructions are emitted, so the kernel only ever sees the integer value.                                                                          
                                                                                
Best Regards,                                                                   
Tarang

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Kate Hsuan]

Hi Sakari and Tarang,


On Wed, Jun 24, 2026 at 6:01 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> On Wed, Jun 24, 2026 at 09:58:11AM +0000, Tarang Raval wrote:
> > > So, avdd is necessary.
> >
> > Could you please try adding a mapping for SONY471A in
> > int3472_gpio_map[] in drivers/platform/x86/intel/int3472/discrete.c?
> >
> > Not fully sure, I have to check, but hopefully it will work.
>
> That's the intent, yes.
>
> --
> Sakari Ailus
>

I'll propose a change for it.

-- 
BR,
Kate

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Kate Hsuan]

Hi Vladimir,

Thank you for reviewing.

On Wed, Jun 24, 2026 at 6:12 AM Vladimir Zapolskiy
<vladimir.zapolskiy@linaro.org> wrote:
>
> On 6/24/26 06:35, Kate Hsuan wrote:
> > Add a new driver for Sony imx471 camera sensor. It is based on
> > Jimmy Su <jimmy.su@intel.com> implementation and the driver can be found
> > in the following URL.
> > https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c
> >
> > This sensor can be found on Lenovo X1 Carbon G14, X9-14 and X9-15 laptops
> > and it is a part of IPU7 solution. The driver was tested on Lenovo X1
> > Carbon G14, X9-14 and X9-15 laptops.
> >
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
>
> Please find a few minor nitpicks below.
>
> > ---
> >   MAINTAINERS                |   6 +
> >   drivers/media/i2c/Kconfig  |  10 +
> >   drivers/media/i2c/Makefile |   1 +
> >   drivers/media/i2c/imx471.c | 971 +++++++++++++++++++++++++++++++++++++
> >   4 files changed, 988 insertions(+)
> >   create mode 100644 drivers/media/i2c/imx471.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 6b4560681b51..586958b1816d 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -25219,6 +25219,12 @@ T:   git git://linuxtv.org/media.git
> >   F:  Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
> >   F:  drivers/media/i2c/imx415.c
> >
> > +SONY IMX471 SENSOR DRIVER
> > +M:   Kate Hsuan <hpa@redhat.com>
> > +L:   linux-media@vger.kernel.org
> > +S:   Maintained
> > +F:   drivers/media/i2c/imx471.c
> > +
> >   SONY MEMORYSTICK SUBSYSTEM
> >   M:  Maxim Levitsky <maximlevitsky@gmail.com>
> >   M:  Alex Dubov <oakad@yahoo.com>
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 5d173e0ecf42..b7199f9f5a0c 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -287,6 +287,16 @@ config VIDEO_IMX415
> >         To compile this driver as a module, choose M here: the
> >         module will be called imx415.
> >
> > +config VIDEO_IMX471
> > +     tristate "Sony IMX471 sensor support"
> > +     select V4L2_CCI_I2C
> > +     help
> > +       This is a Video4Linux2 sensor driver for the Sony
> > +       IMX471 camera.
> > +
> > +       To compile this driver as a module, choose M here: the
> > +       module will be called imx471.
> > +
> >   config VIDEO_MAX9271_LIB
> >       tristate
> >
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index e45359efe0e4..acbd321fc12e 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_IMX335) += imx335.o
> >   obj-$(CONFIG_VIDEO_IMX355) += imx355.o
> >   obj-$(CONFIG_VIDEO_IMX412) += imx412.o
> >   obj-$(CONFIG_VIDEO_IMX415) += imx415.o
> > +obj-$(CONFIG_VIDEO_IMX471) += imx471.o
> >   obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
> >   obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o
> >   obj-$(CONFIG_VIDEO_KS0127) += ks0127.o
> > diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
> > new file mode 100644
> > index 000000000000..1e1bff69ea3d
> > --- /dev/null
> > +++ b/drivers/media/i2c/imx471.c
> > @@ -0,0 +1,971 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * imx471.c - imx471 sensor driver
> > + *
> > + * Copyright (C) 2025 Intel Corporation
> > + * Copyright (C) 2026 Kate Hsuan <hpa@redhat.com>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/unaligned.h>
>
> No declarations coming from linux/unaligned.h header are used in the driver.
>
> > +#include <media/v4l2-cci.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
>
> No declarations coming from media/v4l-event.h header are used in the driver,
> likely it's intended to include media/v4l2-subdev.h instead.
>
> > +#include <media/v4l2-event.h>
>
> No declarations coming from media/v4l-event.h header are used in the driver.

I'll drop them.

>
> > +#include <media/v4l2-fwnode.h>
> > +
> > +#define IMX471_REG_MODE_SELECT                       CCI_REG8(0x0100)
> > +#define IMX471_MODE_STANDBY                  0x00
> > +#define IMX471_MODE_STREAMING                        0x01
> > +
> > +/* Chip ID */
> > +#define IMX471_REG_CHIP_ID                   CCI_REG16(0x0016)
> > +#define IMX471_CHIP_ID                               0x0471
> > +
> > +/* V_TIMING internal */
> > +#define IMX471_REG_FLL                               CCI_REG16(0x0340)
> > +#define IMX471_FLL_MAX                               0xffff
> > +
> > +/* Exposure control */
> > +#define IMX471_REG_EXPOSURE                  CCI_REG16(0x0202)
> > +#define IMX471_EXPOSURE_MIN                  1
> > +#define IMX471_EXPOSURE_STEP                 1
> > +#define IMX471_EXPOSURE_DEFAULT                      1270
> > +
> > +/* Default exposure margin */
> > +#define IMX471_EXPOSURE_MARGIN                       18
> > +
> > +/* Analog gain control */
> > +#define IMX471_REG_ANALOG_GAIN                       CCI_REG16(0x0204)
> > +#define IMX471_ANA_GAIN_MIN                  0
> > +#define IMX471_ANA_GAIN_MAX                  800
> > +#define IMX471_ANA_GAIN_STEP                 1
> > +#define IMX471_ANA_GAIN_DEFAULT                      0
> > +
> > +/* Digital gain control */
> > +#define IMX471_REG_DPGA_USE_GLOBAL_GAIN              CCI_REG16(0x3ff9)
> > +#define IMX471_REG_DIG_GAIN_GLOBAL           CCI_REG16(0x020e)
> > +#define IMX471_DGTL_GAIN_MIN                 256
> > +#define IMX471_DGTL_GAIN_MAX                 4095
> > +#define IMX471_DGTL_GAIN_STEP                        1
> > +#define IMX471_DGTL_GAIN_DEFAULT             256
> > +
> > +/* HFLIP and VFLIP control */
> > +#define IMX471_REG_ORIENTATION                       CCI_REG8(0x0101)
> > +#define IMX471_HFLIP_BIT                     BIT(0)
> > +#define IMX471_VFLIP_BIT                     BIT(1)
>
> IMX471_HFLIP_BIT and IMX471_VFLIP_BIT macro do not have any users in the driver.
I'll drop them.

>
> > +
> > +/* Test Pattern Control */
> > +#define IMX471_REG_TEST_PATTERN                      CCI_REG8(0x0600)
> > +#define IMX471_TEST_PATTERN_DISABLED         0
> > +#define IMX471_TEST_PATTERN_SOLID_COLOR              1
> > +#define IMX471_TEST_PATTERN_COLOR_BARS               2
> > +#define IMX471_TEST_PATTERN_GRAY_COLOR_BARS  3
> > +#define IMX471_TEST_PATTERN_PN9                      4
>
> IMX471_TEST_PATTERN_* macro do not have any users in the driver.
I'll drop them.

>
> > +
> > +/* default link frequency and external clock */
> > +#define IMX471_LINK_FREQ_DEFAULT             200000000LL
> > +#define IMX471_EXT_CLK                               19200000
> > +#define IMX471_LINK_FREQ_INDEX                       0
>
> Please drop IMX471_LINK_FREQ_INDEX macro along with .link_freq_index from
> struct imx471_mode.
Okay

>
> --
> Best wishes,
> Vladimir
>


-- 
BR,
Kate

Re: [PATCH v5 3/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Kate Hsuan]

Hi Kieran and Tarang,

Thank you for reviewing.

On Wed, Jun 24, 2026 at 10:09 AM Tarang Raval
<tarang.raval@siliconsignals.io> wrote:
>
> Hi Kieran,
>
> > Quoting Kate Hsuan (2026-06-24 04:35:08)
> > > Add a new driver for Sony imx471 camera sensor. It is based on
> > > Jimmy Su <jimmy.su@intel.com> implementation and the driver can be found
> > > in the following URL.
> > > https://github.com/intel/ipu6-drivers/commits/master/drivers/media/i2c/imx471.c
> > >
> > > This sensor can be found on Lenovo X1 Carbon G14, X9-14 and X9-15 laptops
> > > and it is a part of IPU7 solution. The driver was tested on Lenovo X1
> > > Carbon G14, X9-14 and X9-15 laptops.
> > >
> > > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > > ---
> >
> > ... <snip>
> >
> > > +#define IMX471_REG_EXCK_FREQ                   CCI_REG16(0x0136)
> > > +#define IMX471_EXCK_FREQ(n)                    ((n) * 256)     /* n in MHz */
> > >
> >
> > <snip>
> >
> > > +
> > > +static const struct cci_reg_sequence imx471_global_regs[] = {
> > > +       { IMX471_REG_EXCK_FREQ, IMX471_EXCK_FREQ(19.2) },
> >
> >
> > Does this work? Is this a compile time constant multiplying floating
> > point with the compiler, or something happening in the kernel?
> >
> > I'll be happy if it does, but it catches my attention as something I
> > thought we couldn't do.
>
> I tested this with GCC. It evaluates the expression at compile time and
> folds it into the integer constant 4915. The generated assembly/object
> file contains .quad 4915 in the initialized data and no floating-point
> instructions are emitted, so the kernel only ever sees the integer value.

The compiler didn't complain anything and covert it to a integer but
putting a floating point here is not a good way.
I focued on finding a register value without noticing that it is a
floating point
I'll set a value IMX471_REG_EXCK_FREQ directly and  drop IMX471_EXCK_FREQ.


>
> Best Regards,
> Tarang
>


-- 
BR,
Kate