[PATCH v6 0/4] Add Sony IMX471 camera sensor driver

[PATCH v6 0/4] 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 v6:
1. Add the con_id "vana" for the power enable to the int3472_gpio_map table.
2. Drop unnecessary macros.
3. Name the registers.

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 (4):
  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
  platform: int3472: discrete: con_id vana for Sony IMX471 as power
    enable
  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                    | 957 ++++++++++++++++++
 drivers/media/pci/intel/ipu-bridge.c          |  41 +
 drivers/platform/x86/intel/int3472/discrete.c |  18 +
 6 files changed, 1033 insertions(+)
 create mode 100644 drivers/media/i2c/imx471.c

-- 
2.54.0

[PATCH v6 1/4] 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 v6 2/4] 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 v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Kate Hsuan]

Update the con_id for the Sony IMX471 sensor to "vana" to serve as the
power enable. Additionally, the HID values SONY471A and TBE20A0, both
associated with the IMX471 image sensor, have been identified on Lenovo
laptops.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/platform/x86/intel/int3472/discrete.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 115bb37577a1..adff564bf3fd 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -164,6 +164,24 @@ static const struct int3472_gpio_map int3472_gpio_map[] = {
 		.con_id = "dvdd",
 		.enable_time_us = 45 * USEC_PER_MSEC,
 	},
+	{	/* imx471 expects "vana" as con_id for power enable */
+		.hid = "SONY471A",
+		.type_from = INT3472_GPIO_TYPE_POWER_ENABLE,
+		.type_to = INT3472_GPIO_TYPE_POWER_ENABLE,
+		.con_id = "vana",
+		.enable_time_us = GPIO_REGULATOR_ENABLE_TIME,
+	},
+	{
+		/*
+		 * imx471 (on Lenovo ThinkPads X1 G14) expects "vana" as con_id
+		 * for power enable
+		 */
+		.hid = "TBE20A0",
+		.type_from = INT3472_GPIO_TYPE_POWER_ENABLE,
+		.type_to = INT3472_GPIO_TYPE_POWER_ENABLE,
+		.con_id = "vana",
+		.enable_time_us = GPIO_REGULATOR_ENABLE_TIME,
+	},
 };
 
 static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3472, u8 *type,
-- 
2.54.0

[PATCH v6 4/4] 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>
Tested-by: computman <anis@talbi.fr>
Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
---
 MAINTAINERS                |   6 +
 drivers/media/i2c/Kconfig  |  10 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/imx471.c | 957 +++++++++++++++++++++++++++++++++++++
 4 files changed, 974 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..6d358b11e96d
--- /dev/null
+++ b/drivers/media/i2c/imx471.c
@@ -0,0 +1,957 @@
+// 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 <media/v4l2-cci.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-subdev.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)
+
+/* Test Pattern Control */
+#define IMX471_REG_TEST_PATTERN			CCI_REG8(0x0600)
+
+/* default link frequency and external clock */
+#define IMX471_LINK_FREQ_DEFAULT		200000000LL
+#define IMX471_EXT_CLK				19200000
+
+/* 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_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[] = {
+	"vana",
+};
+
+struct imx471_mode {
+	u32 width;
+	u32 height;
+
+	/* V-timing */
+	u32 fll_def;
+	u32 fll_min;
+
+	/* H-timing */
+	u32 llp;
+
+	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, 0x1333 },
+	{ 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,
+		.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_TRY,
+		.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 v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Tarang Raval]

Hi Kate,

> Update the con_id for the Sony IMX471 sensor to "vana" to serve as the
> power enable. Additionally, the HID values SONY471A and TBE20A0, both
> associated with the IMX471 image sensor, have been identified on Lenovo
> laptops.
>
> Signed-off-by: Kate Hsuan <hpa@redhat.com>

Thanks, looks good.

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

Best Regards,
Tarang

> ---
>  drivers/platform/x86/intel/int3472/discrete.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 115bb37577a1..adff564bf3fd 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -164,6 +164,24 @@ static const struct int3472_gpio_map int3472_gpio_map[] = {
>                 .con_id = "dvdd",
>                 .enable_time_us = 45 * USEC_PER_MSEC,
>         },
> +       {       /* imx471 expects "vana" as con_id for power enable */
> +               .hid = "SONY471A",
> +               .type_from = INT3472_GPIO_TYPE_POWER_ENABLE,
> +               .type_to = INT3472_GPIO_TYPE_POWER_ENABLE,
> +               .con_id = "vana",
> +               .enable_time_us = GPIO_REGULATOR_ENABLE_TIME,
> +       },
> +       {
> +               /*
> +                * imx471 (on Lenovo ThinkPads X1 G14) expects "vana" as con_id
> +                * for power enable
> +                */
> +               .hid = "TBE20A0",
> +               .type_from = INT3472_GPIO_TYPE_POWER_ENABLE,
> +               .type_to = INT3472_GPIO_TYPE_POWER_ENABLE,
> +               .con_id = "vana",
> +               .enable_time_us = GPIO_REGULATOR_ENABLE_TIME,
> +       },
>  };
>
>  static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3472, u8 *type,
> --
> 2.54.0

Re: [PATCH v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Hans de Goede]

Hi All,

On 30-Jun-26 09:32, Tarang Raval wrote:
> Hi Kate,
> 
>> Update the con_id for the Sony IMX471 sensor to "vana" to serve as the
>> power enable. Additionally, the HID values SONY471A and TBE20A0, both
>> associated with the IMX471 image sensor, have been identified on Lenovo
>> laptops.
>>
>> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> 
> Thanks, looks good.
> 
> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>

Hmm, the imx471 driver is still pending upstream:

https://lore.kernel.org/linux-media/20260629074026.35490-5-hpa@redhat.com/

As part of this series.

Please just use the standardized "avdd" in that driver instead
of "vana" (which also seems to refer to the analog supply vdd,
which is what avdd stands for).

Then this whole patch is unnecessary and can be dropped from
this series.

Regards,

Hans




>> ---
>>  drivers/platform/x86/intel/int3472/discrete.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
>> index 115bb37577a1..adff564bf3fd 100644
>> --- a/drivers/platform/x86/intel/int3472/discrete.c
>> +++ b/drivers/platform/x86/intel/int3472/discrete.c
>> @@ -164,6 +164,24 @@ static const struct int3472_gpio_map int3472_gpio_map[] = {
>>                 .con_id = "dvdd",
>>                 .enable_time_us = 45 * USEC_PER_MSEC,
>>         },
>> +       {       /* imx471 expects "vana" as con_id for power enable */
>> +               .hid = "SONY471A",
>> +               .type_from = INT3472_GPIO_TYPE_POWER_ENABLE,
>> +               .type_to = INT3472_GPIO_TYPE_POWER_ENABLE,
>> +               .con_id = "vana",
>> +               .enable_time_us = GPIO_REGULATOR_ENABLE_TIME,
>> +       },
>> +       {
>> +               /*
>> +                * imx471 (on Lenovo ThinkPads X1 G14) expects "vana" as con_id
>> +                * for power enable
>> +                */
>> +               .hid = "TBE20A0",
>> +               .type_from = INT3472_GPIO_TYPE_POWER_ENABLE,
>> +               .type_to = INT3472_GPIO_TYPE_POWER_ENABLE,
>> +               .con_id = "vana",
>> +               .enable_time_us = GPIO_REGULATOR_ENABLE_TIME,
>> +       },
>>  };
>>
>>  static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3472, u8 *type,
>> --
>> 2.54.0

Re: [PATCH v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Tarang Raval]

Hi Hans,                                                                       
                                                                               
> On 30-Jun-26 09:32, Tarang Raval wrote:                                      
> > Hi Kate,                                                                   
> >                                                                            
> >> Update the con_id for the Sony IMX471 sensor to "vana" to serve as the    
> >> power enable. Additionally, the HID values SONY471A and TBE20A0, both     
> >> associated with the IMX471 image sensor, have been identified on Lenovo   
> >> laptops.                                                                  
> >>                                                                           
> >> Signed-off-by: Kate Hsuan <hpa@redhat.com>                                
> >                                                                            
> > Thanks, looks good.                                                        
> >                                                                            
> > Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>                 
>                                                                              
> Hmm, the imx471 driver is still pending upstream:                            
>                                                                              
> https://lore.kernel.org/linux-media/20260629074026.35490-5-hpa@redhat.com/   
>                                                                              
> As part of this series.                                                      
>                                                                              
> Please just use the standardized "avdd" in that driver instead               
> of "vana" (which also seems to refer to the analog supply vdd,               
> which is what avdd stands for).                                              
>                                                                              
> Then this whole patch is unnecessary and can be dropped from                 
> this series.                                                                 
                                                                               
The regulator name "vana" comes directly from the Sony IMX471 sensor           
datasheet, which typically refers to the analog supply voltage. Using the      
datasheet name helps keep the driver consistent with the hardware              
documentation and makes it easier to cross-reference.                          
                                                                               
as per my understanding, the more standardized way is to use the regulator     
name as per the sensor datasheet. Therefore, I respectfully disagree with 
your suggestion.                                                           
                                                                               
Best Regards,                                                                  
Tarang                                                                         

Re: [PATCH v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Hans de Goede]

Hi,

On 1-Jul-26 08:19, Tarang Raval wrote:
> Hi Hans,                                                                       
>                                                                                
>> On 30-Jun-26 09:32, Tarang Raval wrote:                                      
>>> Hi Kate,                                                                   
>>>                                                                            
>>>> Update the con_id for the Sony IMX471 sensor to "vana" to serve as the    
>>>> power enable. Additionally, the HID values SONY471A and TBE20A0, both     
>>>> associated with the IMX471 image sensor, have been identified on Lenovo   
>>>> laptops.                                                                  
>>>>                                                                           
>>>> Signed-off-by: Kate Hsuan <hpa@redhat.com>                                
>>>                                                                            
>>> Thanks, looks good.                                                        
>>>                                                                            
>>> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>                 
>>                                                                              
>> Hmm, the imx471 driver is still pending upstream:                            
>>                                                                              
>> https://lore.kernel.org/linux-media/20260629074026.35490-5-hpa@redhat.com/   
>>                                                                              
>> As part of this series.                                                      
>>                                                                              
>> Please just use the standardized "avdd" in that driver instead               
>> of "vana" (which also seems to refer to the analog supply vdd,               
>> which is what avdd stands for).                                              
>>                                                                              
>> Then this whole patch is unnecessary and can be dropped from                 
>> this series.                                                                 
>                                                                                
> The regulator name "vana" comes directly from the Sony IMX471 sensor           
> datasheet, which typically refers to the analog supply voltage. Using the      
> datasheet name helps keep the driver consistent with the hardware              
> documentation and makes it easier to cross-reference.                          
>                                                                                
> as per my understanding, the more standardized way is to use the regulator     
> name as per the sensor datasheet. Therefore, I respectfully disagree with 
> your suggestion.                                                           

As shown by the need for this patch on x86 at least because there
is no devicetree it greatly helps if all Linux sensor drivers use
standardized names for their regulators rather then using the exact name
from the datasheet which often is not very consistent.

And "avdd" is the name we've standardized on for this, so lets use that:

hans@shalem:~/projects/linux$ grep -l '"vana"' drivers/media/i2c/*.c | wc -l
4
hans@shalem:~/projects/linux$ grep -l '"avdd"' drivers/media/i2c/*.c | wc -l
36

The alternative is needing to add more and more quirks as different
sensors are used, which is not great.

Regards,

Hans

Re: [PATCH v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Sakari Ailus]

Hi Hans,

On Wed, Jul 01, 2026 at 01:01:58PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 1-Jul-26 08:19, Tarang Raval wrote:
> > Hi Hans,                                                                       
> >                                                                                
> >> On 30-Jun-26 09:32, Tarang Raval wrote:                                      
> >>> Hi Kate,                                                                   
> >>>                                                                            
> >>>> Update the con_id for the Sony IMX471 sensor to "vana" to serve as the    
> >>>> power enable. Additionally, the HID values SONY471A and TBE20A0, both     
> >>>> associated with the IMX471 image sensor, have been identified on Lenovo   
> >>>> laptops.                                                                  
> >>>>                                                                           
> >>>> Signed-off-by: Kate Hsuan <hpa@redhat.com>                                
> >>>                                                                            
> >>> Thanks, looks good.                                                        
> >>>                                                                            
> >>> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>                 
> >>                                                                              
> >> Hmm, the imx471 driver is still pending upstream:                            
> >>                                                                              
> >> https://lore.kernel.org/linux-media/20260629074026.35490-5-hpa@redhat.com/   
> >>                                                                              
> >> As part of this series.                                                      
> >>                                                                              
> >> Please just use the standardized "avdd" in that driver instead               
> >> of "vana" (which also seems to refer to the analog supply vdd,               
> >> which is what avdd stands for).                                              
> >>                                                                              
> >> Then this whole patch is unnecessary and can be dropped from                 
> >> this series.                                                                 
> >                                                                                
> > The regulator name "vana" comes directly from the Sony IMX471 sensor           
> > datasheet, which typically refers to the analog supply voltage. Using the      
> > datasheet name helps keep the driver consistent with the hardware              
> > documentation and makes it easier to cross-reference.                          
> >                                                                                
> > as per my understanding, the more standardized way is to use the regulator     
> > name as per the sensor datasheet. Therefore, I respectfully disagree with 
> > your suggestion.                                                           
> 
> As shown by the need for this patch on x86 at least because there
> is no devicetree it greatly helps if all Linux sensor drivers use
> standardized names for their regulators rather then using the exact name
> from the datasheet which often is not very consistent.
> 
> And "avdd" is the name we've standardized on for this, so lets use that:
> 
> hans@shalem:~/projects/linux$ grep -l '"vana"' drivers/media/i2c/*.c | wc -l
> 4
> hans@shalem:~/projects/linux$ grep -l '"avdd"' drivers/media/i2c/*.c | wc -l
> 36
> 
> The alternative is needing to add more and more quirks as different
> sensors are used, which is not great.

I do agree that having a constant name for the regulators would be
beneficial for the int3472 driver. Still, if, and presumably, when that
sensor gets DT support, the bindings will use the regulator name from the
datasheet.

Let's just use the datasheet name now and add the few lines needed to the
int3472 driver and avoid the churn in the future. There's a limited number
of sensor drivers that need this after all.

I'd be more concerned of what's going on in tps68470_board_data.c for
instance.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Tarang Raval]

Hi Hans, Sakari

> On Wed, Jul 01, 2026 at 01:01:58PM +0200, Hans de Goede wrote:
> > Hi,
> >
> > On 1-Jul-26 08:19, Tarang Raval wrote:
> > > Hi Hans,
> > >
> > >> On 30-Jun-26 09:32, Tarang Raval wrote:
> > >>> Hi Kate,
> > >>>
> > >>>> Update the con_id for the Sony IMX471 sensor to "vana" to serve as the
> > >>>> power enable. Additionally, the HID values SONY471A and TBE20A0, both
> > >>>> associated with the IMX471 image sensor, have been identified on Lenovo
> > >>>> laptops.
> > >>>>
> > >>>> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > >>>
> > >>> Thanks, looks good.
> > >>>
> > >>> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
> > >>
> > >> Hmm, the imx471 driver is still pending upstream:
> > >>
> > >> https://lore.kernel.org/linux-media/20260629074026.35490-5-hpa@redhat.com/
> > >>
> > >> As part of this series.
> > >>
> > >> Please just use the standardized "avdd" in that driver instead
> > >> of "vana" (which also seems to refer to the analog supply vdd,
> > >> which is what avdd stands for).
> > >>
> > >> Then this whole patch is unnecessary and can be dropped from
> > >> this series.
> > >
> > > The regulator name "vana" comes directly from the Sony IMX471 sensor
> > > datasheet, which typically refers to the analog supply voltage. Using the
> > > datasheet name helps keep the driver consistent with the hardware
> > > documentation and makes it easier to cross-reference.
> > >
> > > as per my understanding, the more standardized way is to use the regulator
> > > name as per the sensor datasheet. Therefore, I respectfully disagree with
> > > your suggestion.
> >
> > As shown by the need for this patch on x86 at least because there
> > is no devicetree it greatly helps if all Linux sensor drivers use
> > standardized names for their regulators rather then using the exact name
> > from the datasheet which often is not very consistent.
> >
> > And "avdd" is the name we've standardized on for this, so lets use that:
> >
> > hans@shalem:~/projects/linux$ grep -l '"vana"' drivers/media/i2c/*.c | wc -l
> > 4
> > hans@shalem:~/projects/linux$ grep -l '"avdd"' drivers/media/i2c/*.c | wc -l
> > 36
> >
> > The alternative is needing to add more and more quirks as different
> > sensors are used, which is not great.
>
> I do agree that having a constant name for the regulators would be
> beneficial for the int3472 driver. Still, if, and presumably, when that
> sensor gets DT support, the bindings will use the regulator name from the
> datasheet.
>
> Let's just use the datasheet name now and add the few lines needed to the
> int3472 driver and avoid the churn in the future. There's a limited number
> of sensor drivers that need this after all.
>
> I'd be more concerned of what's going on in tps68470_board_data.c for
> instance.

I went through the INT3472 driver and would like to propose a generic     
approach that satisfies both sides without per-HID quirks or sensor driver
changes.                                                                  
                                                                          
The problem is:                                                      
 - INT3472 standardizes on "avdd" internally                             
 - Sony IMX sensor drivers use "vana" per datasheet, and all existing    
   Sony DT bindings (imx219, imx290, imx415) already use vana-supply     
 - Changing imx471 to "avdd" now will create inconsistency with those    
   bindings, or require a rename later                                   
                                                                          
Instead of fixing this per-sensor (either by changing the driver or adding
a per-HID entry to int3472_gpio_map), we can add a small vendor alias     
table inside skl_int3472_register_regulator().                            
                                                                          
Currently that function registers two consumer supply entries per sensor: 
lowercase ("avdd") and uppercase ("AVDD"). We can extend it to also       
register vendor datasheet aliases from a static table, e.g.:              
                                                                          
  avdd -> vana   (Sony IMX series: imx219, imx290, imx415, imx471, ...)   
                                                                          
This way, when a sensor driver requests "vana", the regulator framework   
finds it in the supply map without any extra quirks.                      

Best Regards,
Tarang

Re: [PATCH v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Hans de Goede]

Hi,

On 1-Jul-26 13:12, Sakari Ailus wrote:
> Hi Hans,
> 
> On Wed, Jul 01, 2026 at 01:01:58PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 1-Jul-26 08:19, Tarang Raval wrote:
>>> Hi Hans,                                                                       
>>>                                                                                
>>>> On 30-Jun-26 09:32, Tarang Raval wrote:                                      
>>>>> Hi Kate,                                                                   
>>>>>                                                                            
>>>>>> Update the con_id for the Sony IMX471 sensor to "vana" to serve as the    
>>>>>> power enable. Additionally, the HID values SONY471A and TBE20A0, both     
>>>>>> associated with the IMX471 image sensor, have been identified on Lenovo   
>>>>>> laptops.                                                                  
>>>>>>                                                                           
>>>>>> Signed-off-by: Kate Hsuan <hpa@redhat.com>                                
>>>>>                                                                            
>>>>> Thanks, looks good.                                                        
>>>>>                                                                            
>>>>> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>                 
>>>>                                                                              
>>>> Hmm, the imx471 driver is still pending upstream:                            
>>>>                                                                              
>>>> https://lore.kernel.org/linux-media/20260629074026.35490-5-hpa@redhat.com/   
>>>>                                                                              
>>>> As part of this series.                                                      
>>>>                                                                              
>>>> Please just use the standardized "avdd" in that driver instead               
>>>> of "vana" (which also seems to refer to the analog supply vdd,               
>>>> which is what avdd stands for).                                              
>>>>                                                                              
>>>> Then this whole patch is unnecessary and can be dropped from                 
>>>> this series.                                                                 
>>>                                                                                
>>> The regulator name "vana" comes directly from the Sony IMX471 sensor           
>>> datasheet, which typically refers to the analog supply voltage. Using the      
>>> datasheet name helps keep the driver consistent with the hardware              
>>> documentation and makes it easier to cross-reference.                          
>>>                                                                                
>>> as per my understanding, the more standardized way is to use the regulator     
>>> name as per the sensor datasheet. Therefore, I respectfully disagree with 
>>> your suggestion.                                                           
>>
>> As shown by the need for this patch on x86 at least because there
>> is no devicetree it greatly helps if all Linux sensor drivers use
>> standardized names for their regulators rather then using the exact name
>> from the datasheet which often is not very consistent.
>>
>> And "avdd" is the name we've standardized on for this, so lets use that:
>>
>> hans@shalem:~/projects/linux$ grep -l '"vana"' drivers/media/i2c/*.c | wc -l
>> 4
>> hans@shalem:~/projects/linux$ grep -l '"avdd"' drivers/media/i2c/*.c | wc -l
>> 36
>>
>> The alternative is needing to add more and more quirks as different
>> sensors are used, which is not great.
> 
> I do agree that having a constant name for the regulators would be
> beneficial for the int3472 driver. Still, if, and presumably, when that
> sensor gets DT support, the bindings will use the regulator name from the
> datasheet.
> 
> Let's just use the datasheet name now and add the few lines needed to the
> int3472 driver and avoid the churn in the future. There's a limited number
> of sensor drivers that need this after all.

Ok, if your happy with using "vana" + an int3472 quirk, that works
for me.

Regards,

Hans

Re: [PATCH v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Hans de Goede]

Hi,

On 1-Jul-26 14:33, Tarang Raval wrote:
> Hi Hans, Sakari
> 
>> On Wed, Jul 01, 2026 at 01:01:58PM +0200, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 1-Jul-26 08:19, Tarang Raval wrote:
>>>> Hi Hans,
>>>>
>>>>> On 30-Jun-26 09:32, Tarang Raval wrote:
>>>>>> Hi Kate,
>>>>>>
>>>>>>> Update the con_id for the Sony IMX471 sensor to "vana" to serve as the
>>>>>>> power enable. Additionally, the HID values SONY471A and TBE20A0, both
>>>>>>> associated with the IMX471 image sensor, have been identified on Lenovo
>>>>>>> laptops.
>>>>>>>
>>>>>>> Signed-off-by: Kate Hsuan <hpa@redhat.com>
>>>>>>
>>>>>> Thanks, looks good.
>>>>>>
>>>>>> Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
>>>>>
>>>>> Hmm, the imx471 driver is still pending upstream:
>>>>>
>>>>> https://lore.kernel.org/linux-media/20260629074026.35490-5-hpa@redhat.com/
>>>>>
>>>>> As part of this series.
>>>>>
>>>>> Please just use the standardized "avdd" in that driver instead
>>>>> of "vana" (which also seems to refer to the analog supply vdd,
>>>>> which is what avdd stands for).
>>>>>
>>>>> Then this whole patch is unnecessary and can be dropped from
>>>>> this series.
>>>>
>>>> The regulator name "vana" comes directly from the Sony IMX471 sensor
>>>> datasheet, which typically refers to the analog supply voltage. Using the
>>>> datasheet name helps keep the driver consistent with the hardware
>>>> documentation and makes it easier to cross-reference.
>>>>
>>>> as per my understanding, the more standardized way is to use the regulator
>>>> name as per the sensor datasheet. Therefore, I respectfully disagree with
>>>> your suggestion.
>>>
>>> As shown by the need for this patch on x86 at least because there
>>> is no devicetree it greatly helps if all Linux sensor drivers use
>>> standardized names for their regulators rather then using the exact name
>>> from the datasheet which often is not very consistent.
>>>
>>> And "avdd" is the name we've standardized on for this, so lets use that:
>>>
>>> hans@shalem:~/projects/linux$ grep -l '"vana"' drivers/media/i2c/*.c | wc -l
>>> 4
>>> hans@shalem:~/projects/linux$ grep -l '"avdd"' drivers/media/i2c/*.c | wc -l
>>> 36
>>>
>>> The alternative is needing to add more and more quirks as different
>>> sensors are used, which is not great.
>>
>> I do agree that having a constant name for the regulators would be
>> beneficial for the int3472 driver. Still, if, and presumably, when that
>> sensor gets DT support, the bindings will use the regulator name from the
>> datasheet.
>>
>> Let's just use the datasheet name now and add the few lines needed to the
>> int3472 driver and avoid the churn in the future. There's a limited number
>> of sensor drivers that need this after all.
>>
>> I'd be more concerned of what's going on in tps68470_board_data.c for
>> instance.
> 
> I went through the INT3472 driver and would like to propose a generic     
> approach that satisfies both sides without per-HID quirks or sensor driver
> changes.                                                                  
>                                                                           
> The problem is:                                                      
>  - INT3472 standardizes on "avdd" internally                             
>  - Sony IMX sensor drivers use "vana" per datasheet, and all existing    
>    Sony DT bindings (imx219, imx290, imx415) already use vana-supply     
>  - Changing imx471 to "avdd" now will create inconsistency with those    
>    bindings, or require a rename later

Ack, as mentioned in my reply to Sakari from 1 minute ago I'm ok
with sticking with vana for the imx* case,
                                  
> Instead of fixing this per-sensor (either by changing the driver or adding
> a per-HID entry to int3472_gpio_map), we can add a small vendor alias     
> table inside skl_int3472_register_regulator().                            
>                                                                           
> Currently that function registers two consumer supply entries per sensor: 
> lowercase ("avdd") and uppercase ("AVDD"). We can extend it to also       
> register vendor datasheet aliases from a static table, e.g.:              
>                                                                           
>   avdd -> vana   (Sony IMX series: imx219, imx290, imx415, imx471, ...)   
>                                                                           
> This way, when a sensor driver requests "vana", the regulator framework   
> finds it in the supply map without any extra quirks.                      

Interesting proposal. For now I think just going with the quirk from
this patch is fine. But if we get more Sony sensors doing what you
suggests might make sense.

IIRC currently all regulators get an upper-cased alias to avoid needing
quirks just for the case where drivers only differ in case.

Special casing avdd is going to require adding special code to the generic
bits. Or maybe just an alt_name parameter ? Either way I think for just
the one sensor the quirk is fine. But if we get more (which is somewhat
likely) then your suggestion is probably a good idea.

Regards,

Hans

Re: [PATCH v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Sakari Ailus]

Hi Hans, Tarang,

On Thu, Jul 02, 2026 at 08:05:25PM +0200, Hans de Goede wrote:
> > I went through the INT3472 driver and would like to propose a generic     
> > approach that satisfies both sides without per-HID quirks or sensor driver
> > changes.                                                                  
> >                                                                           
> > The problem is:                                                      
> >  - INT3472 standardizes on "avdd" internally                             
> >  - Sony IMX sensor drivers use "vana" per datasheet, and all existing    
> >    Sony DT bindings (imx219, imx290, imx415) already use vana-supply     
> >  - Changing imx471 to "avdd" now will create inconsistency with those    
> >    bindings, or require a rename later
> 
> Ack, as mentioned in my reply to Sakari from 1 minute ago I'm ok
> with sticking with vana for the imx* case,

At least some Sony sensors use "INT" PnP vendor prefix and so telling them
apart from the rest doesn't work at least this way. There could also be
other prefixes as well, they're not all "SONY". Right now there is one with
INT prefix and three with SONY prefix.

If we start having lots of devices with the same quirk, we could also
introduce a pointer to an array of IDs to avoid repeating the same quirk
over and over.

Kate's patch adds two quirks so this could be already considered (and only
one of these IDs is using SONY prefix).

-- 
Regards,

Sakari Ailus