[PATCH 0/5] platform/x86: int3472: Fix AVDD and add DOVDD + DVDD mappings

[PATCH 0/5] platform/x86: int3472: Fix AVDD and add DOVDD + DVDD mappings

[Hans de Goede]

Hi All,

Here is a patch-set to sort out the problems with the Linux INT3472 code
mapping the 0x10 GPIO type which is meant for AVDD to DOVDD.

This should allow the OV5678 front camera of the Lenovo Yoga Duet 7 13ITL6
to properly get powered-on by Linux, without breaking any existing setups.

See "[PATCH 5/5] platform/x86: int3472: Properly map the AVDD GPIO type to
a "avdd" regulator" for more details on this. The other patches are pretty
trivial prep patches.

Note this series is ordered so that if [PATCH 5/5] somehow does cause
regressions it can be easily reverted without needing to revert any of
the other changes (which should all be safe).

Regards,

Hans


Hans de Goede (5):
  media: ov05c10: Support full range of power rails
  platform/x86: int3472: Rename DOVDD GPIO type to AVDD
  platform/x86: int3472: Add support for DOVDD and DVDD GPIO types
  platform/x86: int3472: Rename avdd_second_sensor to
    power_enable_second_sensor
  platform/x86: int3472: Properly map the AVDD GPIO type to a "avdd"
    regulator

 drivers/media/i2c/ov05c10.c                   | 32 +++++++++++++------
 drivers/platform/x86/intel/int3472/discrete.c | 16 ++++++++--
 .../x86/intel/int3472/discrete_quirks.c       |  2 +-
 include/linux/platform_data/x86/int3472.h     |  8 +++--
 4 files changed, 43 insertions(+), 15 deletions(-)

-- 
2.55.0

[PATCH 1/5] media: ov05c10: Support full range of power rails

[Hans de Goede]

Add support for the full standard set of power rails for Omnivision
sensors: "dovdd", "avdd" and "dvdd", using the regulator_bulk_* APIs to
manage to regulators.

Previously the driver only supported avdd, but depending on the design
some of the other power rails may need to be controlled by the sensor
driver too.

This is a prep patch for some INT3472 GPIO -> regulator mapping changes,
which require sensor drivers to support all 3 rails to avoid regressions.

Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/media/i2c/ov05c10.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/ov05c10.c b/drivers/media/i2c/ov05c10.c
index 8fe4dc0d9405..f94752dc626f 100644
--- a/drivers/media/i2c/ov05c10.c
+++ b/drivers/media/i2c/ov05c10.c
@@ -7,6 +7,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 #include <linux/regmap.h>
 #include <media/v4l2-cci.h>
 #include <media/v4l2-ctrls.h>
@@ -279,6 +280,12 @@ static const struct ov05c10_mode supported_modes[] = {
 	},
 };
 
+static const char * const ov05c10_supply_names[] = {
+	"dovdd",	/* Digital I/O power */
+	"avdd",		/* Analog power */
+	"dvdd",		/* Digital core power */
+};
+
 struct ov05c10 {
 	struct v4l2_subdev sd;
 	struct media_pad pad;
@@ -291,8 +298,8 @@ struct ov05c10 {
 	struct regmap *regmap;
 	unsigned long link_freq_bitmap;
 
+	struct regulator_bulk_data supplies[ARRAY_SIZE(ov05c10_supply_names)];
 	struct clk *img_clk;
-	struct regulator *avdd;
 	struct gpio_desc *reset;
 	bool identified;
 };
@@ -753,16 +760,20 @@ static int ov05c10_get_pm_resources(struct device *dev)
 {
 	struct v4l2_subdev *sd = dev_get_drvdata(dev);
 	struct ov05c10 *ov05c10 = to_ov05c10(sd);
+	int ret;
 
 	ov05c10->img_clk = devm_v4l2_sensor_clk_get(dev, NULL);
 	if (IS_ERR(ov05c10->img_clk))
 		return dev_err_probe(dev, PTR_ERR(ov05c10->img_clk),
 				     "failed to get imaging clock\n");
 
-	ov05c10->avdd = devm_regulator_get(dev, "avdd");
-	if (IS_ERR(ov05c10->avdd))
-		return dev_err_probe(dev, PTR_ERR(ov05c10->avdd),
-				     "failed to get avdd regulator\n");
+	for (unsigned int i = 0; i < ARRAY_SIZE(ov05c10_supply_names); i++)
+		ov05c10->supplies[i].supply = ov05c10_supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov05c10_supply_names),
+				      ov05c10->supplies);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
 
 	ov05c10->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(ov05c10->reset))
@@ -778,7 +789,8 @@ static int ov05c10_power_off(struct device *dev)
 	struct ov05c10 *ov05c10 = to_ov05c10(sd);
 
 	gpiod_set_value_cansleep(ov05c10->reset, 1);
-	regulator_disable(ov05c10->avdd);
+	regulator_bulk_disable(ARRAY_SIZE(ov05c10_supply_names),
+			       ov05c10->supplies);
 	clk_disable_unprepare(ov05c10->img_clk);
 
 	return 0;
@@ -796,9 +808,10 @@ static int ov05c10_power_on(struct device *dev)
 		return ret;
 	}
 
-	ret = regulator_enable(ov05c10->avdd);
+	ret = regulator_bulk_enable(ARRAY_SIZE(ov05c10_supply_names),
+				    ov05c10->supplies);
 	if (ret < 0) {
-		dev_err(dev, "failed to enable avdd: %d", ret);
+		dev_err(dev, "failed to enable regulators: %d", ret);
 		goto err_clk_disable_unprepare;
 	}
 
@@ -821,7 +834,8 @@ static int ov05c10_power_on(struct device *dev)
 
 err_power_off:
 	gpiod_set_value_cansleep(ov05c10->reset, 1);
-	regulator_disable(ov05c10->avdd);
+	regulator_bulk_disable(ARRAY_SIZE(ov05c10_supply_names),
+			       ov05c10->supplies);
 
 err_clk_disable_unprepare:
 	clk_disable_unprepare(ov05c10->img_clk);
-- 
2.55.0

Re: [PATCH 1/5] media: ov05c10: Support full range of power rails

[Andy Shevchenko]

On Fri, Oct 16, 2026 at 5:17 PM Hans de Goede
<johannes.goede@oss.qualcomm.com> wrote:
>
> Add support for the full standard set of power rails for Omnivision
> sensors: "dovdd", "avdd" and "dvdd", using the regulator_bulk_* APIs to
> manage to regulators.
>
> Previously the driver only supported avdd, but depending on the design
> some of the other power rails may need to be controlled by the sensor
> driver too.
>
> This is a prep patch for some INT3472 GPIO -> regulator mapping changes,
> which require sensor drivers to support all 3 rails to avoid regressions.

Seems quite straightforward.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
One nit-pick below.

...

>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>

Perhaps keep it sorted?

>  #include <linux/regmap.h>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] media: ov05c10: Support full range of power rails

[Dan Scally]

Morning Hans, thanks for the series (and the legwork in untangling everything last week)

On 16/10/2026 15:17, Hans de Goede wrote:
> Add support for the full standard set of power rails for Omnivision
> sensors: "dovdd", "avdd" and "dvdd", using the regulator_bulk_* APIs to
> manage to regulators.
> 
> Previously the driver only supported avdd, but depending on the design
> some of the other power rails may need to be controlled by the sensor
> driver too.
> 
> This is a prep patch for some INT3472 GPIO -> regulator mapping changes,
> which require sensor drivers to support all 3 rails to avoid regressions.
> 
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---
>   drivers/media/i2c/ov05c10.c | 32 +++++++++++++++++++++++---------
>   1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov05c10.c b/drivers/media/i2c/ov05c10.c
> index 8fe4dc0d9405..f94752dc626f 100644
> --- a/drivers/media/i2c/ov05c10.c
> +++ b/drivers/media/i2c/ov05c10.c
> @@ -7,6 +7,7 @@
>   #include <linux/i2c.h>
>   #include <linux/module.h>
>   #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
>   #include <linux/regmap.h>
>   #include <media/v4l2-cci.h>
>   #include <media/v4l2-ctrls.h>
> @@ -279,6 +280,12 @@ static const struct ov05c10_mode supported_modes[] = {
>   	},
>   };
>   
> +static const char * const ov05c10_supply_names[] = {
> +	"dovdd",	/* Digital I/O power */
> +	"avdd",		/* Analog power */
> +	"dvdd",		/* Digital core power */
> +};

I don't have the datasheet to verify the supplies but it certainly matches the norm and the 
remainder of the patch looks good, so:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

> +
>   struct ov05c10 {
>   	struct v4l2_subdev sd;
>   	struct media_pad pad;
> @@ -291,8 +298,8 @@ struct ov05c10 {
>   	struct regmap *regmap;
>   	unsigned long link_freq_bitmap;
>   
> +	struct regulator_bulk_data supplies[ARRAY_SIZE(ov05c10_supply_names)];
>   	struct clk *img_clk;
> -	struct regulator *avdd;
>   	struct gpio_desc *reset;
>   	bool identified;
>   };
> @@ -753,16 +760,20 @@ static int ov05c10_get_pm_resources(struct device *dev)
>   {
>   	struct v4l2_subdev *sd = dev_get_drvdata(dev);
>   	struct ov05c10 *ov05c10 = to_ov05c10(sd);
> +	int ret;
>   
>   	ov05c10->img_clk = devm_v4l2_sensor_clk_get(dev, NULL);
>   	if (IS_ERR(ov05c10->img_clk))
>   		return dev_err_probe(dev, PTR_ERR(ov05c10->img_clk),
>   				     "failed to get imaging clock\n");
>   
> -	ov05c10->avdd = devm_regulator_get(dev, "avdd");
> -	if (IS_ERR(ov05c10->avdd))
> -		return dev_err_probe(dev, PTR_ERR(ov05c10->avdd),
> -				     "failed to get avdd regulator\n");
> +	for (unsigned int i = 0; i < ARRAY_SIZE(ov05c10_supply_names); i++)
> +		ov05c10->supplies[i].supply = ov05c10_supply_names[i];
> +
> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov05c10_supply_names),
> +				      ov05c10->supplies);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
>   
>   	ov05c10->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>   	if (IS_ERR(ov05c10->reset))
> @@ -778,7 +789,8 @@ static int ov05c10_power_off(struct device *dev)
>   	struct ov05c10 *ov05c10 = to_ov05c10(sd);
>   
>   	gpiod_set_value_cansleep(ov05c10->reset, 1);
> -	regulator_disable(ov05c10->avdd);
> +	regulator_bulk_disable(ARRAY_SIZE(ov05c10_supply_names),
> +			       ov05c10->supplies);
>   	clk_disable_unprepare(ov05c10->img_clk);
>   
>   	return 0;
> @@ -796,9 +808,10 @@ static int ov05c10_power_on(struct device *dev)
>   		return ret;
>   	}
>   
> -	ret = regulator_enable(ov05c10->avdd);
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ov05c10_supply_names),
> +				    ov05c10->supplies);
>   	if (ret < 0) {
> -		dev_err(dev, "failed to enable avdd: %d", ret);
> +		dev_err(dev, "failed to enable regulators: %d", ret);
>   		goto err_clk_disable_unprepare;
>   	}
>   
> @@ -821,7 +834,8 @@ static int ov05c10_power_on(struct device *dev)
>   
>   err_power_off:
>   	gpiod_set_value_cansleep(ov05c10->reset, 1);
> -	regulator_disable(ov05c10->avdd);
> +	regulator_bulk_disable(ARRAY_SIZE(ov05c10_supply_names),
> +			       ov05c10->supplies);
>   
>   err_clk_disable_unprepare:
>   	clk_disable_unprepare(ov05c10->img_clk);

[PATCH 2/5] platform/x86: int3472: Rename DOVDD GPIO type to AVDD

[Hans de Goede]

The window drivers call GPIO type 0x10 AVDD not DOVDD, rename
the INT3472_GPIO_TYPE_DOVDD define to INT3472_GPIO_TYPE_AVDD to match.

Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/platform/x86/intel/int3472/discrete.c | 7 ++++---
 include/linux/platform_data/x86/int3472.h     | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index b19ea6ee3374..f3d6c6ccad0d 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -277,7 +277,8 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 		*con_id = "dvdd";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
-	case INT3472_GPIO_TYPE_DOVDD:
+	case INT3472_GPIO_TYPE_AVDD:
+		/* Should be avdd kept as dovdd for now to avoid regressions */
 		*con_id = "dovdd";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
@@ -402,7 +403,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 	case INT3472_GPIO_TYPE_STROBE:
 	case INT3472_GPIO_TYPE_POWER_ENABLE:
 	case INT3472_GPIO_TYPE_POWER1:
-	case INT3472_GPIO_TYPE_DOVDD:
+	case INT3472_GPIO_TYPE_AVDD:
 	case INT3472_GPIO_TYPE_HANDSHAKE:
 		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
 		if (IS_ERR(gpio))
@@ -429,7 +430,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 			second_sensor = int3472->quirks.avdd_second_sensor;
 			fallthrough;
 		case INT3472_GPIO_TYPE_POWER1:
-		case INT3472_GPIO_TYPE_DOVDD:
+		case INT3472_GPIO_TYPE_AVDD:
 		case INT3472_GPIO_TYPE_HANDSHAKE:
 			ret = skl_int3472_register_regulator(int3472, gpio, enable_time_us,
 							     con_id, second_sensor);
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index b1040e36deb8..e1d5c4915ef7 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -30,7 +30,7 @@
 #define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
 #define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
 #define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d
-#define INT3472_GPIO_TYPE_DOVDD					0x10
+#define INT3472_GPIO_TYPE_AVDD					0x10
 #define INT3472_GPIO_TYPE_HANDSHAKE				0x12
 #define INT3472_GPIO_TYPE_HOTPLUG_DETECT			0x13
 
-- 
2.55.0

Re: [PATCH 2/5] platform/x86: int3472: Rename DOVDD GPIO type to AVDD

[Andy Shevchenko]

On Fri, Oct 16, 2026 at 04:17:13PM +0200, Hans de Goede wrote:
> The window drivers call GPIO type 0x10 AVDD not DOVDD, rename
> the INT3472_GPIO_TYPE_DOVDD define to INT3472_GPIO_TYPE_AVDD to match.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/5] platform/x86: int3472: Rename DOVDD GPIO type to AVDD

[Dan Scally]

Hi Hans

On 16/10/2026 15:17, Hans de Goede wrote:
> The window drivers call GPIO type 0x10 AVDD not DOVDD, rename
> the INT3472_GPIO_TYPE_DOVDD define to INT3472_GPIO_TYPE_AVDD to match.
> 
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/platform/x86/intel/int3472/discrete.c | 7 ++++---
>   include/linux/platform_data/x86/int3472.h     | 2 +-
>   2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index b19ea6ee3374..f3d6c6ccad0d 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -277,7 +277,8 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>   		*con_id = "dvdd";
>   		*gpio_flags = GPIO_ACTIVE_HIGH;
>   		break;
> -	case INT3472_GPIO_TYPE_DOVDD:
> +	case INT3472_GPIO_TYPE_AVDD:
> +		/* Should be avdd kept as dovdd for now to avoid regressions */
>   		*con_id = "dovdd";
>   		*gpio_flags = GPIO_ACTIVE_HIGH;
>   		break;
> @@ -402,7 +403,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   	case INT3472_GPIO_TYPE_STROBE:
>   	case INT3472_GPIO_TYPE_POWER_ENABLE:
>   	case INT3472_GPIO_TYPE_POWER1:
> -	case INT3472_GPIO_TYPE_DOVDD:
> +	case INT3472_GPIO_TYPE_AVDD:
>   	case INT3472_GPIO_TYPE_HANDSHAKE:
>   		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
>   		if (IS_ERR(gpio))
> @@ -429,7 +430,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   			second_sensor = int3472->quirks.avdd_second_sensor;
>   			fallthrough;
>   		case INT3472_GPIO_TYPE_POWER1:
> -		case INT3472_GPIO_TYPE_DOVDD:
> +		case INT3472_GPIO_TYPE_AVDD:
>   		case INT3472_GPIO_TYPE_HANDSHAKE:
>   			ret = skl_int3472_register_regulator(int3472, gpio, enable_time_us,
>   							     con_id, second_sensor);
> diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
> index b1040e36deb8..e1d5c4915ef7 100644
> --- a/include/linux/platform_data/x86/int3472.h
> +++ b/include/linux/platform_data/x86/int3472.h
> @@ -30,7 +30,7 @@
>   #define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
>   #define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
>   #define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d
> -#define INT3472_GPIO_TYPE_DOVDD					0x10
> +#define INT3472_GPIO_TYPE_AVDD					0x10
>   #define INT3472_GPIO_TYPE_HANDSHAKE				0x12
>   #define INT3472_GPIO_TYPE_HOTPLUG_DETECT			0x13
>   

[PATCH 3/5] platform/x86: int3472: Add support for DOVDD and DVDD GPIO types

[Hans de Goede]

Add defines for INT3472_GPIO_TYPE_DOVDD (type 0x0f) and
INT3472_GPIO_TYPE_DVDD (type 0x11) and map them to regulators with
matching names.

Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/platform/x86/intel/int3472/discrete.c | 12 ++++++++++++
 include/linux/platform_data/x86/int3472.h     |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index f3d6c6ccad0d..046f93ed1c9f 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -277,11 +277,19 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 		*con_id = "dvdd";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
+	case INT3472_GPIO_TYPE_DOVDD:
+		*con_id = "dovdd";
+		*gpio_flags = GPIO_ACTIVE_HIGH;
+		break;
 	case INT3472_GPIO_TYPE_AVDD:
 		/* Should be avdd kept as dovdd for now to avoid regressions */
 		*con_id = "dovdd";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
+	case INT3472_GPIO_TYPE_DVDD:
+		*con_id = "dvdd";
+		*gpio_flags = GPIO_ACTIVE_HIGH;
+		break;
 	case INT3472_GPIO_TYPE_HANDSHAKE:
 		*con_id = "dvdd";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
@@ -403,7 +411,9 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 	case INT3472_GPIO_TYPE_STROBE:
 	case INT3472_GPIO_TYPE_POWER_ENABLE:
 	case INT3472_GPIO_TYPE_POWER1:
+	case INT3472_GPIO_TYPE_DOVDD:
 	case INT3472_GPIO_TYPE_AVDD:
+	case INT3472_GPIO_TYPE_DVDD:
 	case INT3472_GPIO_TYPE_HANDSHAKE:
 		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
 		if (IS_ERR(gpio))
@@ -430,7 +440,9 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 			second_sensor = int3472->quirks.avdd_second_sensor;
 			fallthrough;
 		case INT3472_GPIO_TYPE_POWER1:
+		case INT3472_GPIO_TYPE_DOVDD:
 		case INT3472_GPIO_TYPE_AVDD:
+		case INT3472_GPIO_TYPE_DVDD:
 		case INT3472_GPIO_TYPE_HANDSHAKE:
 			ret = skl_int3472_register_regulator(int3472, gpio, enable_time_us,
 							     con_id, second_sensor);
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index e1d5c4915ef7..f0ec2e7c68fa 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -30,7 +30,9 @@
 #define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
 #define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
 #define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d
+#define INT3472_GPIO_TYPE_DOVDD					0x0f
 #define INT3472_GPIO_TYPE_AVDD					0x10
+#define INT3472_GPIO_TYPE_DVDD					0x11
 #define INT3472_GPIO_TYPE_HANDSHAKE				0x12
 #define INT3472_GPIO_TYPE_HOTPLUG_DETECT			0x13
 
-- 
2.55.0

Re: [PATCH 3/5] platform/x86: int3472: Add support for DOVDD and DVDD GPIO types

[Andy Shevchenko]

On Fri, Oct 16, 2026 at 04:17:14PM +0200, Hans de Goede wrote:
> Add defines for INT3472_GPIO_TYPE_DOVDD (type 0x0f) and
> INT3472_GPIO_TYPE_DVDD (type 0x11) and map them to regulators with
> matching names.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/5] platform/x86: int3472: Add support for DOVDD and DVDD GPIO types

[Dan Scally]

Hi Hans

On 16/10/2026 15:17, Hans de Goede wrote:
> Add defines for INT3472_GPIO_TYPE_DOVDD (type 0x0f) and
> INT3472_GPIO_TYPE_DVDD (type 0x11) and map them to regulators with
> matching names.
> 
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---
>   drivers/platform/x86/intel/int3472/discrete.c | 12 ++++++++++++
>   include/linux/platform_data/x86/int3472.h     |  2 ++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index f3d6c6ccad0d..046f93ed1c9f 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -277,11 +277,19 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>   		*con_id = "dvdd";
>   		*gpio_flags = GPIO_ACTIVE_HIGH;
>   		break;
> +	case INT3472_GPIO_TYPE_DOVDD:
> +		*con_id = "dovdd";
> +		*gpio_flags = GPIO_ACTIVE_HIGH;
> +		break;
>   	case INT3472_GPIO_TYPE_AVDD:
>   		/* Should be avdd kept as dovdd for now to avoid regressions */
>   		*con_id = "dovdd";

Won't this cause any device that has a gpio type of both 0x0f and 0x10 to fail on this commit 
because of the duplicate mapping? On the other hand, if it needs both rails then it wasn't working 
anyway, so:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   		*gpio_flags = GPIO_ACTIVE_HIGH;
>   		break;
> +	case INT3472_GPIO_TYPE_DVDD:
> +		*con_id = "dvdd";
> +		*gpio_flags = GPIO_ACTIVE_HIGH;
> +		break;
>   	case INT3472_GPIO_TYPE_HANDSHAKE:
>   		*con_id = "dvdd";
>   		*gpio_flags = GPIO_ACTIVE_HIGH;
> @@ -403,7 +411,9 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   	case INT3472_GPIO_TYPE_STROBE:
>   	case INT3472_GPIO_TYPE_POWER_ENABLE:
>   	case INT3472_GPIO_TYPE_POWER1:
> +	case INT3472_GPIO_TYPE_DOVDD:
>   	case INT3472_GPIO_TYPE_AVDD:
> +	case INT3472_GPIO_TYPE_DVDD:
>   	case INT3472_GPIO_TYPE_HANDSHAKE:
>   		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
>   		if (IS_ERR(gpio))
> @@ -430,7 +440,9 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   			second_sensor = int3472->quirks.avdd_second_sensor;
>   			fallthrough;
>   		case INT3472_GPIO_TYPE_POWER1:
> +		case INT3472_GPIO_TYPE_DOVDD:
>   		case INT3472_GPIO_TYPE_AVDD:
> +		case INT3472_GPIO_TYPE_DVDD:
>   		case INT3472_GPIO_TYPE_HANDSHAKE:
>   			ret = skl_int3472_register_regulator(int3472, gpio, enable_time_us,
>   							     con_id, second_sensor);
> diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
> index e1d5c4915ef7..f0ec2e7c68fa 100644
> --- a/include/linux/platform_data/x86/int3472.h
> +++ b/include/linux/platform_data/x86/int3472.h
> @@ -30,7 +30,9 @@
>   #define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
>   #define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
>   #define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d
> +#define INT3472_GPIO_TYPE_DOVDD					0x0f
>   #define INT3472_GPIO_TYPE_AVDD					0x10
> +#define INT3472_GPIO_TYPE_DVDD					0x11
>   #define INT3472_GPIO_TYPE_HANDSHAKE				0x12
>   #define INT3472_GPIO_TYPE_HOTPLUG_DETECT			0x13
>   

[PATCH 4/5] platform/x86: int3472: Rename avdd_second_sensor to power_enable_second_sensor

[Hans de Goede]

The avdd_second_sensor quirk is for when a GPIO with the power-enable type
is shared between sensors. Sofar power-enable happened to be mapped to
a "avdd" regulator. But that is going to change in future patches, rename
the quirk to better reflect what it actually is for.

Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/platform/x86/intel/int3472/discrete.c        | 2 +-
 drivers/platform/x86/intel/int3472/discrete_quirks.c | 2 +-
 include/linux/platform_data/x86/int3472.h            | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 046f93ed1c9f..18649ee448a4 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -437,7 +437,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 
 			break;
 		case INT3472_GPIO_TYPE_POWER_ENABLE:
-			second_sensor = int3472->quirks.avdd_second_sensor;
+			second_sensor = int3472->quirks.power_enable_second_sensor;
 			fallthrough;
 		case INT3472_GPIO_TYPE_POWER1:
 		case INT3472_GPIO_TYPE_DOVDD:
diff --git a/drivers/platform/x86/intel/int3472/discrete_quirks.c b/drivers/platform/x86/intel/int3472/discrete_quirks.c
index 552869ef91ab..4e57d3f73111 100644
--- a/drivers/platform/x86/intel/int3472/discrete_quirks.c
+++ b/drivers/platform/x86/intel/int3472/discrete_quirks.c
@@ -5,7 +5,7 @@
 #include <linux/platform_data/x86/int3472.h>
 
 static const struct int3472_discrete_quirks lenovo_miix_510_quirks = {
-	.avdd_second_sensor = "i2c-OVTI2680:00",
+	.power_enable_second_sensor = "i2c-OVTI2680:00",
 };
 
 const struct dmi_system_id skl_int3472_discrete_quirks[] = {
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index f0ec2e7c68fa..86b7510d5a45 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -98,8 +98,8 @@ struct int3472_cldb {
 };
 
 struct int3472_discrete_quirks {
-	/* For models where AVDD GPIO is shared between sensors */
-	const char *avdd_second_sensor;
+	/* For models where power-enable GPIO is shared between sensors */
+	const char *power_enable_second_sensor;
 };
 
 struct int3472_gpio_regulator {
-- 
2.55.0

Re: [PATCH 4/5] platform/x86: int3472: Rename avdd_second_sensor to power_enable_second_sensor

[Andy Shevchenko]

On Fri, Oct 16, 2026 at 04:17:15PM +0200, Hans de Goede wrote:
> The avdd_second_sensor quirk is for when a GPIO with the power-enable type
> is shared between sensors. Sofar power-enable happened to be mapped to
> a "avdd" regulator. But that is going to change in future patches, rename
> the quirk to better reflect what it actually is for.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] platform/x86: int3472: Rename avdd_second_sensor to power_enable_second_sensor

[Dan Scally]

Hi Hans

On 16/10/2026 15:17, Hans de Goede wrote:
> The avdd_second_sensor quirk is for when a GPIO with the power-enable type
> is shared between sensors. Sofar power-enable happened to be mapped to
> a "avdd" regulator. But that is going to change in future patches, rename
> the quirk to better reflect what it actually is for.
> 
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/platform/x86/intel/int3472/discrete.c        | 2 +-
>   drivers/platform/x86/intel/int3472/discrete_quirks.c | 2 +-
>   include/linux/platform_data/x86/int3472.h            | 4 ++--
>   3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 046f93ed1c9f..18649ee448a4 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -437,7 +437,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   
>   			break;
>   		case INT3472_GPIO_TYPE_POWER_ENABLE:
> -			second_sensor = int3472->quirks.avdd_second_sensor;
> +			second_sensor = int3472->quirks.power_enable_second_sensor;
>   			fallthrough;
>   		case INT3472_GPIO_TYPE_POWER1:
>   		case INT3472_GPIO_TYPE_DOVDD:
> diff --git a/drivers/platform/x86/intel/int3472/discrete_quirks.c b/drivers/platform/x86/intel/int3472/discrete_quirks.c
> index 552869ef91ab..4e57d3f73111 100644
> --- a/drivers/platform/x86/intel/int3472/discrete_quirks.c
> +++ b/drivers/platform/x86/intel/int3472/discrete_quirks.c
> @@ -5,7 +5,7 @@
>   #include <linux/platform_data/x86/int3472.h>
>   
>   static const struct int3472_discrete_quirks lenovo_miix_510_quirks = {
> -	.avdd_second_sensor = "i2c-OVTI2680:00",
> +	.power_enable_second_sensor = "i2c-OVTI2680:00",
>   };
>   
>   const struct dmi_system_id skl_int3472_discrete_quirks[] = {
> diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
> index f0ec2e7c68fa..86b7510d5a45 100644
> --- a/include/linux/platform_data/x86/int3472.h
> +++ b/include/linux/platform_data/x86/int3472.h
> @@ -98,8 +98,8 @@ struct int3472_cldb {
>   };
>   
>   struct int3472_discrete_quirks {
> -	/* For models where AVDD GPIO is shared between sensors */
> -	const char *avdd_second_sensor;
> +	/* For models where power-enable GPIO is shared between sensors */
> +	const char *power_enable_second_sensor;
>   };
>   
>   struct int3472_gpio_regulator {

[PATCH 5/5] platform/x86: int3472: Properly map the AVDD GPIO type to a "avdd" regulator

[Hans de Goede]

Before commit 34e31390a3b9 ("platform/x86: int3472: support the POWER1 GPIO
type") the int3472/discrete.c code mapped 3 GPIO types to regulators:

0x0b INT3472_GPIO_TYPE_POWER_ENABLE -> avdd
0x10 INT3472_GPIO_TYPE_AVDD         -> dovdd
0x12 INT3472_GPIO_TYPE_HANDSHAKE    -> dvdd

(with the INT3472_GPIO_TYPE_AVDD define wrongly being named
INT3472_GPIO_TYPE_DOVDD in the past)

The OV5678 front camera of the Lenovo Yoga Duet 7 13ITL6 uses the following
GPIO types:

0x0f INT3472_GPIO_TYPE_DOVDD
0x10 INT3472_GPIO_TYPE_AVDD
0x11 INT3472_GPIO_TYPE_DVDD

A previous patch in this series maps the DOVDD and DVDD types to "dovdd"
resp. "dvdd" regulators. This creates a regulator naming conflict on this
laptop for the old AVDD to "dovdd" mapping.

This patch fixes this by changing the unfortunate mapping of the AVDD type
to a "dovdd" regulator by swapping the POWER_ENABLE and AVDD mappings,
mapping POWER_ENABLE to "dovdd" and AVDD to "avdd".

This is safe to do because all sensors which may be used with the INT3472
code are listed in drivers/media/pci/intel/ipu-bridge.c and a manual check
of all drivers for sensors listed there in current media-committers/next
has found that all drivers either support all 3 of "dovdd", "avdd" and
"dvdd" or none of them and enable all 3 of them at the same time. So this
swap ends up being a no-op.

With the exception being the new in -next ov05c10 driver which only
supports "avdd", this is fixed by an earlier patch in this series.

Note this change does not impact the recent adding of mapping POWER1 to
a "dvdd" regulator since it does not touch any "dvdd" mappings.

Reported-by: Alexander Patrakov <patrakov@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/CAN_LGv0qkxARw4bxO8K-1hUkWNg2ts6q50dooCB5UM5en=bo0w@mail.gmail.com/
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/platform/x86/intel/int3472/discrete.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 18649ee448a4..ff94dc2ad8a1 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -270,7 +270,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
 	case INT3472_GPIO_TYPE_POWER_ENABLE:
-		*con_id = "avdd";
+		*con_id = "dovdd";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
 	case INT3472_GPIO_TYPE_POWER1:
@@ -282,8 +282,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
 	case INT3472_GPIO_TYPE_AVDD:
-		/* Should be avdd kept as dovdd for now to avoid regressions */
-		*con_id = "dovdd";
+		*con_id = "avdd";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
 	case INT3472_GPIO_TYPE_DVDD:
-- 
2.55.0

Re: [PATCH 5/5] platform/x86: int3472: Properly map the AVDD GPIO type to a "avdd" regulator

[Andy Shevchenko]

On Fri, Oct 16, 2026 at 04:17:16PM +0200, Hans de Goede wrote:
> Before commit 34e31390a3b9 ("platform/x86: int3472: support the POWER1 GPIO
> type") the int3472/discrete.c code mapped 3 GPIO types to regulators:
> 
> 0x0b INT3472_GPIO_TYPE_POWER_ENABLE -> avdd
> 0x10 INT3472_GPIO_TYPE_AVDD         -> dovdd
> 0x12 INT3472_GPIO_TYPE_HANDSHAKE    -> dvdd
> 
> (with the INT3472_GPIO_TYPE_AVDD define wrongly being named
> INT3472_GPIO_TYPE_DOVDD in the past)
> 
> The OV5678 front camera of the Lenovo Yoga Duet 7 13ITL6 uses the following
> GPIO types:
> 
> 0x0f INT3472_GPIO_TYPE_DOVDD
> 0x10 INT3472_GPIO_TYPE_AVDD
> 0x11 INT3472_GPIO_TYPE_DVDD
> 
> A previous patch in this series maps the DOVDD and DVDD types to "dovdd"
> resp. "dvdd" regulators. This creates a regulator naming conflict on this
> laptop for the old AVDD to "dovdd" mapping.
> 
> This patch fixes this by changing the unfortunate mapping of the AVDD type
> to a "dovdd" regulator by swapping the POWER_ENABLE and AVDD mappings,
> mapping POWER_ENABLE to "dovdd" and AVDD to "avdd".
> 
> This is safe to do because all sensors which may be used with the INT3472
> code are listed in drivers/media/pci/intel/ipu-bridge.c and a manual check
> of all drivers for sensors listed there in current media-committers/next
> has found that all drivers either support all 3 of "dovdd", "avdd" and
> "dvdd" or none of them and enable all 3 of them at the same time. So this
> swap ends up being a no-op.
> 
> With the exception being the new in -next ov05c10 driver which only
> supports "avdd", this is fixed by an earlier patch in this series.
> 
> Note this change does not impact the recent adding of mapping POWER1 to
> a "dvdd" regulator since it does not touch any "dvdd" mappings.

Well found and fixed, thanks!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 5/5] platform/x86: int3472: Properly map the AVDD GPIO type to a "avdd" regulator

[Dan Scally]

Hi Hans

On 16/10/2026 15:17, Hans de Goede wrote:
> Before commit 34e31390a3b9 ("platform/x86: int3472: support the POWER1 GPIO
> type") the int3472/discrete.c code mapped 3 GPIO types to regulators:
> 
> 0x0b INT3472_GPIO_TYPE_POWER_ENABLE -> avdd
> 0x10 INT3472_GPIO_TYPE_AVDD         -> dovdd
> 0x12 INT3472_GPIO_TYPE_HANDSHAKE    -> dvdd
> 
> (with the INT3472_GPIO_TYPE_AVDD define wrongly being named
> INT3472_GPIO_TYPE_DOVDD in the past)
> 
> The OV5678 front camera of the Lenovo Yoga Duet 7 13ITL6 uses the following
> GPIO types:
> 
> 0x0f INT3472_GPIO_TYPE_DOVDD
> 0x10 INT3472_GPIO_TYPE_AVDD
> 0x11 INT3472_GPIO_TYPE_DVDD
> 
> A previous patch in this series maps the DOVDD and DVDD types to "dovdd"
> resp. "dvdd" regulators. This creates a regulator naming conflict on this
> laptop for the old AVDD to "dovdd" mapping.
> 
> This patch fixes this by changing the unfortunate mapping of the AVDD type
> to a "dovdd" regulator by swapping the POWER_ENABLE and AVDD mappings,
> mapping POWER_ENABLE to "dovdd" and AVDD to "avdd".
> 
> This is safe to do because all sensors which may be used with the INT3472
> code are listed in drivers/media/pci/intel/ipu-bridge.c and a manual check
> of all drivers for sensors listed there in current media-committers/next
> has found that all drivers either support all 3 of "dovdd", "avdd" and
> "dvdd" or none of them and enable all 3 of them at the same time. So this
> swap ends up being a no-op.
> 
> With the exception being the new in -next ov05c10 driver which only
> supports "avdd", this is fixed by an earlier patch in this series.
> 
> Note this change does not impact the recent adding of mapping POWER1 to
> a "dvdd" regulator since it does not touch any "dvdd" mappings.
> 
> Reported-by: Alexander Patrakov <patrakov@gmail.com>
> Closes: https://lore.kernel.org/platform-driver-x86/CAN_LGv0qkxARw4bxO8K-1hUkWNg2ts6q50dooCB5UM5en=bo0w@mail.gmail.com/
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---

Seconding Andy's comment!

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/platform/x86/intel/int3472/discrete.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 18649ee448a4..ff94dc2ad8a1 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -270,7 +270,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>   		*gpio_flags = GPIO_ACTIVE_HIGH;
>   		break;
>   	case INT3472_GPIO_TYPE_POWER_ENABLE:
> -		*con_id = "avdd";
> +		*con_id = "dovdd";
>   		*gpio_flags = GPIO_ACTIVE_HIGH;
>   		break;
>   	case INT3472_GPIO_TYPE_POWER1:
> @@ -282,8 +282,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>   		*gpio_flags = GPIO_ACTIVE_HIGH;
>   		break;
>   	case INT3472_GPIO_TYPE_AVDD:
> -		/* Should be avdd kept as dovdd for now to avoid regressions */
> -		*con_id = "dovdd";
> +		*con_id = "avdd";
>   		*gpio_flags = GPIO_ACTIVE_HIGH;
>   		break;
>   	case INT3472_GPIO_TYPE_DVDD:

Re: [PATCH 0/5] platform/x86: int3472: Fix AVDD and add DOVDD + DVDD mappings

[Alexander Patrakov]

On Fri, Oct 16, 2026 at 10:17 PM Hans de Goede
<johannes.goede@oss.qualcomm.com> wrote:
>
> Hi All,
>
> Here is a patch-set to sort out the problems with the Linux INT3472 code
> mapping the 0x10 GPIO type which is meant for AVDD to DOVDD.
>
> This should allow the OV5678 front camera of the Lenovo Yoga Duet 7 13ITL6
> to properly get powered-on by Linux, without breaking any existing setups.
>
> See "[PATCH 5/5] platform/x86: int3472: Properly map the AVDD GPIO type to
> a "avdd" regulator" for more details on this. The other patches are pretty
> trivial prep patches.
>
> Note this series is ordered so that if [PATCH 5/5] somehow does cause
> regressions it can be easily reverted without needing to revert any of
> the other changes (which should all be safe).

For the whole series:

Tested-by: Alexander Patrakov <patrakov@gmail.com>

I finally found time to test your patch series (on top of
next-20261016) together with three more patches that Claude made for
me, which I will post in response to this message. The camera "works"
to the degree limited by the incorrect format declaration.

AI was also able to generate a standalone tool that demosaics this
complex layout to a proper color image and feeds v4l2loopback, and
that's how I participate in online meetings.

-- 
Alexander Patrakov

[PATCH 1/3] media: ipu-bridge: Add sensor configuration for OV5675 (OVTI5678)

[Alexander Patrakov]

From: Sahan Nissanka <adee.sahan@gmail.com>

The front camera of the Dell Latitude 7320 Detachable is an OV5675
described in ACPI as OVTI5678. Without an entry here ipu-bridge creates
no software node for it, so no fwnode endpoint exists and the sensor
cannot be connected to the IPU6 even once a driver binds it:

  intel-ipu6 0000:00:05.0: Found supported sensor OVTI8856:00
  intel-ipu6 0000:00:05.0: Connected 1 cameras

450 MHz is OV5675_LINK_FREQ_450MHZ, the only link frequency ov5675.c
supports, and matches the two data lanes the machine's SSDB reports.

With this entry both cameras are found and the front sensor appears in
the media graph:

  intel-ipu6 0000:00:05.0: Found supported sensor OVTI5678:00
  intel-ipu6 0000:00:05.0: Found supported sensor OVTI8856:00
  intel-ipu6 0000:00:05.0: Connected 2 cameras

[Rebased onto next-20261016 and adjusted the comment]

Signed-off-by: Sahan Nissanka <adee.sahan@gmail.com>
Signed-off-by: Alexander Patrakov <patrakov@gmail.com>
Link: https://lore.kernel.org/linux-media/20260809042540.15849-4-adee.sahan@gmail.com/
---
 drivers/media/pci/intel/ipu-bridge.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 3739c4a2a8c4..0b6272c4d826 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -111,8 +111,9 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = {
 	IPU_SENSOR_CONFIG("OVTIDB10", 1, 560000000),
 	/* Omnivision OV2680 */
 	IPU_SENSOR_CONFIG("OVTI2680", 1, 331200000),
-	/* Omnivision OV5675 */
+	/* Omnivision OV5675 and its RGB-IR variant OV5678 */
 	IPU_SENSOR_CONFIG("OVTI5675", 1, 450000000),
+	IPU_SENSOR_CONFIG("OVTI5678", 1, 450000000),
 	/* Omnivision OV5693 */
 	IPU_SENSOR_CONFIG_MATCH_FL("OVTI5693", ipu6_ov5693_ncont_clk,
 				   IPU_BR_FL_CSI2_CLK_NONCONTINUOUS,
-- 
2.56.0

[PATCH 2/3] media: i2c: ov5675: Defer the probe until the endpoint exists

[Alexander Patrakov]

On ACPI systems the sensor's endpoint is a software node which ipu-bridge
creates when the IPU probes. Nothing orders that before the probe of the
sensor. If the sensor is first, there is no endpoint yet, parsing it
fails, and the probe fails for good: the camera is missing until the
next boot.

Return -EPROBE_DEFER when there is no endpoint, as ov2740, ov13b10 and,
since commit 437e1f6a9600 ("media: mt9m114: Return -EPROBE_DEFER if no
endpoint is found"), mt9m114 do.

This failure mode was seen once on a Lenovo Yoga Duet 7 13ITL6.
Reproduction efforts did not succeed.

Signed-off-by: Alexander Patrakov <patrakov@gmail.com>
---
 drivers/media/i2c/ov5675.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c
index c5eb07576129..9cf600c9c94a 100644
--- a/drivers/media/i2c/ov5675.c
+++ b/drivers/media/i2c/ov5675.c
@@ -1182,8 +1182,16 @@ static int ov5675_get_hwcfg(struct ov5675 *ov5675)
 	if (!fwnode)
 		return -ENXIO;
 
+	/*
+	 * On ACPI systems the endpoint is a software node which ipu-bridge
+	 * creates when the IPU probes, which may be after this probe.
+	 */
 	ep = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0,
 					     FWNODE_GRAPH_ENDPOINT_NEXT);
+	if (!ep)
+		return dev_err_probe(dev, -EPROBE_DEFER,
+				     "waiting for fwnode graph endpoint\n");
+
 	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
 	fwnode_handle_put(ep);
 	if (ret)
-- 
2.56.0

Re: [PATCH 2/3] media: i2c: ov5675: Defer the probe until the endpoint exists

[Andy Shevchenko]

On Tue, Oct 20, 2026 at 4:50 AM Alexander Patrakov <patrakov@gmail.com> wrote:
>
> On ACPI systems the sensor's endpoint is a software node which ipu-bridge
> creates when the IPU probes. Nothing orders that before the probe of the
> sensor. If the sensor is first, there is no endpoint yet, parsing it
> fails, and the probe fails for good: the camera is missing until the
> next boot.
>
> Return -EPROBE_DEFER when there is no endpoint, as ov2740, ov13b10 and,
> since commit 437e1f6a9600 ("media: mt9m114: Return -EPROBE_DEFER if no
> endpoint is found"), mt9m114 do.

It is also possible (I think) to call reprobe in the ipu-bridge
driver, but I dunno which approach is less hackish.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] media: i2c: ov5675: Defer the probe until the endpoint exists

[Sakari Ailus]

Hi Alexander,

On Tue, Oct 20, 2026 at 09:50:00AM +0800, Alexander Patrakov wrote:
> On ACPI systems the sensor's endpoint is a software node which ipu-bridge
> creates when the IPU probes. Nothing orders that before the probe of the
> sensor. If the sensor is first, there is no endpoint yet, parsing it
> fails, and the probe fails for good: the camera is missing until the
> next boot.
> 
> Return -EPROBE_DEFER when there is no endpoint, as ov2740, ov13b10 and,
> since commit 437e1f6a9600 ("media: mt9m114: Return -EPROBE_DEFER if no
> endpoint is found"), mt9m114 do.
> 
> This failure mode was seen once on a Lenovo Yoga Duet 7 13ITL6.
> Reproduction efforts did not succeed.
> 
> Signed-off-by: Alexander Patrakov <patrakov@gmail.com>

This patch is unnecessary as v4l2_fwnode_endpoint_alloc_parse() already
validates the endpoint. Also the comment is redundant.

-- 
Regards,

Sakari Ailus

[PATCH 3/3] media: i2c: ov5675: Add the OVTI5678 RGB-IR variant

[Alexander Patrakov]

The front camera of the Lenovo Yoga Duet 7 13ITL6 and of the Dell
Latitude 7320 Detachable is described in ACPI as OVTI5678. The part
reports the chip id of the OV5675 and takes its register settings, but
its color filter array is a complex 4x4 RGB-IR one.

The silicon is likely the same as the one of the OVTI5675, so it still
has some features (binned mode, hardware defective pixel concealment)
that assume the classic 2x2 GRBG Bayer array and thus produce meaningless
output with the RGB-IR array. Disable them for OVTI5678.

NOTE: This patch cannot be upstreamed as there is no way to properly
describe the color filter array layout of the OVTI5678. The color format
is a placeholder at this point.

Signed-off-by: Alexander Patrakov <patrakov@gmail.com>
---
 drivers/media/i2c/ov5675.c | 149 ++++++++++++++++++++++++++++++++-----
 1 file changed, 131 insertions(+), 18 deletions(-)

diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c
index 9cf600c9c94a..7563a462abea 100644
--- a/drivers/media/i2c/ov5675.c
+++ b/drivers/media/i2c/ov5675.c
@@ -6,8 +6,10 @@
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/unaligned.h>
 
@@ -28,6 +30,10 @@
 #define OV5675_REG_CHIP_ID		0x300a
 #define OV5675_CHIP_ID			0x5675
 
+/* On-chip processing; bits 1 and 2 enable the defect pixel correction */
+#define OV5675_REG_ISP_CTRL		0x5000
+#define OV5675_ISP_CTRL_NO_DPC		0x71
+
 #define OV5675_REG_MODE_SELECT		0x0100
 #define OV5675_MODE_STANDBY		0x00
 #define OV5675_MODE_STREAMING		0x01
@@ -492,8 +498,58 @@ static const struct ov5675_mode supported_modes[] = {
 	}
 };
 
+/* The modes of the RGB-IR part, see ov5675_get_modes() */
+static const struct ov5675_mode ov5678_supported_modes[] = {
+	{
+		.width = 2592,
+		.height = 1944,
+		.hts = 1500,
+		.vts_def = OV5675_VTS_30FPS,
+		.vts_min = OV5675_VTS_30FPS_MIN,
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
+			.regs = mode_2592x1944_regs,
+		},
+		.link_freq_index = OV5675_LINK_FREQ_900MBPS,
+	}
+};
+
+/**
+ * struct ov5675_variant - What differs between the parts this driver handles
+ * @model: Name of the sub-device, by which user space tells the parts apart
+ * @code: Media bus code of the output
+ */
+struct ov5675_variant {
+	const char *model;
+	u32 code;
+};
+
+static const struct ov5675_variant ov5675_variant_bayer = {
+	.model = "ov5675",
+	.code = MEDIA_BUS_FMT_SGRBG10_1X10,
+};
+
+/*
+ * The part ACPI calls OVTI5678 reports the chip id of the OV5675 and takes
+ * its register settings, but has a 4x4 RGB-IR color filter array:
+ *
+ *	G I G I
+ *	R G B G
+ *	G I G I
+ *	B G R G
+ *
+ * No media bus code describes that. MEDIA_BUS_FMT_Y10_1X10 is used because it
+ * is the 10-bit code that claims no Bayer order, so that nothing demosaics the
+ * data as if it were Bayer. The samples are not luma.
+ */
+static const struct ov5675_variant ov5675_variant_rgbir = {
+	.model = "ov5678",
+	.code = MEDIA_BUS_FMT_Y10_1X10,
+};
+
 struct ov5675 {
 	struct device *dev;
+	const struct ov5675_variant *variant;
 
 	struct v4l2_subdev sd;
 	struct media_pad pad;
@@ -857,12 +913,30 @@ static int ov5675_init_controls(struct ov5675 *ov5675)
 	return ret;
 }
 
-static void ov5675_update_pad_format(const struct ov5675_mode *mode,
+/*
+ * The 1296x972 mode is 2x2 binned. The hardware averages pixels two positions
+ * apart, which have the same color in a Bayer array, but not in the RGB-IR
+ * array. Therefore, the RGB-IR color filter array is incompatible with binning.
+ */
+static const struct ov5675_mode *ov5675_get_modes(const struct ov5675 *ov5675,
+						  unsigned int *num_modes)
+{
+	if (ov5675->variant->code != MEDIA_BUS_FMT_SGRBG10_1X10) {
+		*num_modes = ARRAY_SIZE(ov5678_supported_modes);
+		return ov5678_supported_modes;
+	}
+
+	*num_modes = ARRAY_SIZE(supported_modes);
+	return supported_modes;
+}
+
+static void ov5675_update_pad_format(struct ov5675 *ov5675,
+				     const struct ov5675_mode *mode,
 				     struct v4l2_mbus_framefmt *fmt)
 {
 	fmt->width = mode->width;
 	fmt->height = mode->height;
-	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+	fmt->code = ov5675->variant->code;
 	fmt->field = V4L2_FIELD_NONE;
 }
 
@@ -914,6 +988,25 @@ static int ov5675_start_streaming(struct ov5675 *ov5675)
 		return ret;
 	}
 
+	/*
+	 * The mode settings enable the on-chip defect pixel correction. It
+	 * compares a pixel with the ones two positions away, which have the
+	 * same color in a Bayer array. In the RGB-IR array the pixel two
+	 * positions from a red one is blue, and so comparing or mixing them
+	 * makes no sense. So defect pixel correction is on for the Bayer part
+	 * only. Intel's Windows driver for the OVTI5678 turns it off in the
+	 * same way, by writing 0x71 to this register.
+	 */
+	if (ov5675->variant->code != MEDIA_BUS_FMT_SGRBG10_1X10) {
+		ret = ov5675_write_reg(ov5675, OV5675_REG_ISP_CTRL,
+				       OV5675_REG_VALUE_08BIT,
+				       OV5675_ISP_CTRL_NO_DPC);
+		if (ret) {
+			dev_err(ov5675->dev, "failed to disable defect correction");
+			return ret;
+		}
+	}
+
 	ret = __v4l2_ctrl_handler_setup(ov5675->sd.ctrl_handler);
 	if (ret)
 		return ret;
@@ -1020,16 +1113,16 @@ static int ov5675_set_format(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_format *fmt)
 {
 	struct ov5675 *ov5675 = to_ov5675(sd);
-	const struct ov5675_mode *mode;
+	const struct ov5675_mode *modes, *mode;
+	unsigned int num_modes;
 	s32 vblank_def, h_blank;
 
-	mode = v4l2_find_nearest_size(supported_modes,
-				      ARRAY_SIZE(supported_modes), width,
-				      height, fmt->format.width,
-				      fmt->format.height);
+	modes = ov5675_get_modes(ov5675, &num_modes);
+	mode = v4l2_find_nearest_size(modes, num_modes, width, height,
+				      fmt->format.width, fmt->format.height);
 
 	mutex_lock(&ov5675->mutex);
-	ov5675_update_pad_format(mode, &fmt->format);
+	ov5675_update_pad_format(ov5675, mode, &fmt->format);
 	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
 		*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
 	} else {
@@ -1067,7 +1160,8 @@ static int ov5675_get_format(struct v4l2_subdev *sd,
 		fmt->format = *v4l2_subdev_state_get_format(sd_state,
 							    fmt->pad);
 	else
-		ov5675_update_pad_format(ov5675->cur_mode, &fmt->format);
+		ov5675_update_pad_format(ov5675, ov5675->cur_mode,
+					 &fmt->format);
 
 	mutex_unlock(&ov5675->mutex);
 
@@ -1104,10 +1198,12 @@ static int ov5675_enum_mbus_code(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_state *sd_state,
 				 struct v4l2_subdev_mbus_code_enum *code)
 {
+	struct ov5675 *ov5675 = to_ov5675(sd);
+
 	if (code->index > 0)
 		return -EINVAL;
 
-	code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+	code->code = ov5675->variant->code;
 
 	return 0;
 }
@@ -1116,15 +1212,20 @@ static int ov5675_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(supported_modes))
+	struct ov5675 *ov5675 = to_ov5675(sd);
+	const struct ov5675_mode *modes;
+	unsigned int num_modes;
+
+	modes = ov5675_get_modes(ov5675, &num_modes);
+	if (fse->index >= num_modes)
 		return -EINVAL;
 
-	if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10)
+	if (fse->code != ov5675->variant->code)
 		return -EINVAL;
 
-	fse->min_width = supported_modes[fse->index].width;
+	fse->min_width = modes[fse->index].width;
 	fse->max_width = fse->min_width;
-	fse->min_height = supported_modes[fse->index].height;
+	fse->min_height = modes[fse->index].height;
 	fse->max_height = fse->min_height;
 
 	return 0;
@@ -1133,9 +1234,13 @@ static int ov5675_enum_frame_size(struct v4l2_subdev *sd,
 static int ov5675_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 {
 	struct ov5675 *ov5675 = to_ov5675(sd);
+	const struct ov5675_mode *modes;
+	unsigned int num_modes;
+
+	modes = ov5675_get_modes(ov5675, &num_modes);
 
 	mutex_lock(&ov5675->mutex);
-	ov5675_update_pad_format(&supported_modes[0],
+	ov5675_update_pad_format(ov5675, &modes[0],
 				 v4l2_subdev_state_get_format(fh->state, 0));
 	mutex_unlock(&ov5675->mutex);
 
@@ -1281,6 +1386,7 @@ static void ov5675_remove(struct i2c_client *client)
 static int ov5675_probe(struct i2c_client *client)
 {
 	struct ov5675 *ov5675;
+	unsigned int num_modes;
 	bool full_power;
 	int ret;
 
@@ -1290,11 +1396,17 @@ static int ov5675_probe(struct i2c_client *client)
 
 	ov5675->dev = &client->dev;
 
+	ov5675->variant = device_get_match_data(ov5675->dev);
+	if (!ov5675->variant)
+		ov5675->variant = &ov5675_variant_bayer;
+
 	ret = ov5675_get_hwcfg(ov5675);
 	if (ret)
 		return ret;
 
 	v4l2_i2c_subdev_init(&ov5675->sd, client, &ov5675_subdev_ops);
+	v4l2_i2c_subdev_set_name(&ov5675->sd, client, ov5675->variant->model,
+				 NULL);
 
 	ret = ov5675_power_on(ov5675->dev);
 	if (ret) {
@@ -1312,7 +1424,7 @@ static int ov5675_probe(struct i2c_client *client)
 	}
 
 	mutex_init(&ov5675->mutex);
-	ov5675->cur_mode = &supported_modes[0];
+	ov5675->cur_mode = ov5675_get_modes(ov5675, &num_modes);
 	ret = ov5675_init_controls(ov5675);
 	if (ret) {
 		dev_err(ov5675->dev, "failed to init controls: %d", ret);
@@ -1363,7 +1475,8 @@ static const struct dev_pm_ops ov5675_pm_ops = {
 
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id ov5675_acpi_ids[] = {
-	{"OVTI5675"},
+	{"OVTI5675", (kernel_ulong_t)&ov5675_variant_bayer},
+	{"OVTI5678", (kernel_ulong_t)&ov5675_variant_rgbir},
 	{}
 };
 
@@ -1371,7 +1484,7 @@ MODULE_DEVICE_TABLE(acpi, ov5675_acpi_ids);
 #endif
 
 static const struct of_device_id ov5675_of_match[] = {
-	{ .compatible = "ovti,ov5675", },
+	{ .compatible = "ovti,ov5675", .data = &ov5675_variant_bayer },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, ov5675_of_match);
-- 
2.56.0

Re: [PATCH 3/3] media: i2c: ov5675: Add the OVTI5678 RGB-IR variant

[Andy Shevchenko]

On Tue, Oct 20, 2026 at 4:50 AM Alexander Patrakov <patrakov@gmail.com> wrote:
>
> The front camera of the Lenovo Yoga Duet 7 13ITL6 and of the Dell
> Latitude 7320 Detachable is described in ACPI as OVTI5678. The part
> reports the chip id of the OV5675 and takes its register settings, but
> its color filter array is a complex 4x4 RGB-IR one.
>
> The silicon is likely the same as the one of the OVTI5675, so it still
> has some features (binned mode, hardware defective pixel concealment)
> that assume the classic 2x2 GRBG Bayer array and thus produce meaningless
> output with the RGB-IR array. Disable them for OVTI5678.

Should be marked as RFC...

> NOTE: This patch cannot be upstreamed as there is no way to properly
> describe the color filter array layout of the OVTI5678. The color format
> is a placeholder at this point.

...and this paragraph should go to the comment block, ...

>
> Signed-off-by: Alexander Patrakov <patrakov@gmail.com>
> ---

...somewhere here.

>  drivers/media/i2c/ov5675.c | 149 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 131 insertions(+), 18 deletions(-)

...

> +#include <linux/mod_devicetable.h>

Not in a new code. Now it's provided by i2c.h. We probably need to add
an ACPI ID header to it.

...

>  {
>         struct ov5675 *ov5675 = to_ov5675(sd);
> +       const struct ov5675_mode *modes;
> +       unsigned int num_modes;
> +
> +       modes = ov5675_get_modes(ov5675, &num_modes);
>
>         mutex_lock(&ov5675->mutex);
> -       ov5675_update_pad_format(&supported_modes[0],
> +       ov5675_update_pad_format(ov5675, &modes[0],

&modes[0] --> modes
in this case

>                                  v4l2_subdev_state_get_format(fh->state, 0));

...

> +       ov5675->variant = device_get_match_data(ov5675->dev);
> +       if (!ov5675->variant)
> +               ov5675->variant = &ov5675_variant_bayer;

With driver_data it makes no sense to support driver_override.
Better to return -ENODATA instead of pinning some defaults.

...

>  static const struct of_device_id ov5675_of_match[] = {
> -       { .compatible = "ovti,ov5675", },
> +       { .compatible = "ovti,ov5675", .data = &ov5675_variant_bayer },
>         { /* sentinel */ },

At some point the trailing comma should be dropped.

>  };

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] media: i2c: ov5675: Add the OVTI5678 RGB-IR variant

[Alexander Patrakov]

On Tue, Oct 20, 2026 at 2:43 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Oct 20, 2026 at 4:50 AM Alexander Patrakov <patrakov@gmail.com> wrote:
> >
> > The front camera of the Lenovo Yoga Duet 7 13ITL6 and of the Dell
> > Latitude 7320 Detachable is described in ACPI as OVTI5678. The part
> > reports the chip id of the OV5675 and takes its register settings, but
> > its color filter array is a complex 4x4 RGB-IR one.
> >
> > The silicon is likely the same as the one of the OVTI5675, so it still
> > has some features (binned mode, hardware defective pixel concealment)
> > that assume the classic 2x2 GRBG Bayer array and thus produce meaningless
> > output with the RGB-IR array. Disable them for OVTI5678.
>
> Should be marked as RFC...
>
> > NOTE: This patch cannot be upstreamed as there is no way to properly
> > describe the color filter array layout of the OVTI5678. The color format
> > is a placeholder at this point.
>
> ...and this paragraph should go to the comment block, ...
>
> >
> > Signed-off-by: Alexander Patrakov <patrakov@gmail.com>
> > ---
>
> ...somewhere here.

Hello Andy,

Thanks for the review. In fact, all three extra patches are RFC or
even just "FYI", to show what exactly was tested.

To satisfy the formalities, I will resubmit them as RFC with your
remarks addressed when I have time.

>
> >  drivers/media/i2c/ov5675.c | 149 ++++++++++++++++++++++++++++++++-----
> >  1 file changed, 131 insertions(+), 18 deletions(-)
>
> ...
>
> > +#include <linux/mod_devicetable.h>
>
> Not in a new code. Now it's provided by i2c.h. We probably need to add
> an ACPI ID header to it.
>
> ...
>
> >  {
> >         struct ov5675 *ov5675 = to_ov5675(sd);
> > +       const struct ov5675_mode *modes;
> > +       unsigned int num_modes;
> > +
> > +       modes = ov5675_get_modes(ov5675, &num_modes);
> >
> >         mutex_lock(&ov5675->mutex);
> > -       ov5675_update_pad_format(&supported_modes[0],
> > +       ov5675_update_pad_format(ov5675, &modes[0],
>
> &modes[0] --> modes
> in this case
>
> >                                  v4l2_subdev_state_get_format(fh->state, 0));
>
> ...
>
> > +       ov5675->variant = device_get_match_data(ov5675->dev);
> > +       if (!ov5675->variant)
> > +               ov5675->variant = &ov5675_variant_bayer;
>
> With driver_data it makes no sense to support driver_override.
> Better to return -ENODATA instead of pinning some defaults.
>
> ...
>
> >  static const struct of_device_id ov5675_of_match[] = {
> > -       { .compatible = "ovti,ov5675", },
> > +       { .compatible = "ovti,ov5675", .data = &ov5675_variant_bayer },
> >         { /* sentinel */ },
>
> At some point the trailing comma should be dropped.
>
> >  };
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
Alexander Patrakov

Re: [PATCH 3/3] media: i2c: ov5675: Add the OVTI5678 RGB-IR variant

[Sakari Ailus]

Hi Alexander,

On Tue, Oct 20, 2026 at 09:50:01AM +0800, Alexander Patrakov wrote:
> The front camera of the Lenovo Yoga Duet 7 13ITL6 and of the Dell
> Latitude 7320 Detachable is described in ACPI as OVTI5678. The part
> reports the chip id of the OV5675 and takes its register settings, but
> its color filter array is a complex 4x4 RGB-IR one.
> 
> The silicon is likely the same as the one of the OVTI5675, so it still
> has some features (binned mode, hardware defective pixel concealment)
> that assume the classic 2x2 GRBG Bayer array and thus produce meaningless
> output with the RGB-IR array. Disable them for OVTI5678.
> 
> NOTE: This patch cannot be upstreamed as there is no way to properly
> describe the color filter array layout of the OVTI5678. The color format
> is a placeholder at this point.
> 
> Signed-off-by: Alexander Patrakov <patrakov@gmail.com>
> ---
>  drivers/media/i2c/ov5675.c | 149 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 131 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c
> index 9cf600c9c94a..7563a462abea 100644
> --- a/drivers/media/i2c/ov5675.c
> +++ b/drivers/media/i2c/ov5675.c
> @@ -6,8 +6,10 @@
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>

#include <linux/device-id/acpi.h>

>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/property.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/unaligned.h>
>  
> @@ -28,6 +30,10 @@
>  #define OV5675_REG_CHIP_ID		0x300a
>  #define OV5675_CHIP_ID			0x5675
>  
> +/* On-chip processing; bits 1 and 2 enable the defect pixel correction */
> +#define OV5675_REG_ISP_CTRL		0x5000
> +#define OV5675_ISP_CTRL_NO_DPC		0x71
> +
>  #define OV5675_REG_MODE_SELECT		0x0100
>  #define OV5675_MODE_STANDBY		0x00
>  #define OV5675_MODE_STREAMING		0x01
> @@ -492,8 +498,58 @@ static const struct ov5675_mode supported_modes[] = {
>  	}
>  };
>  
> +/* The modes of the RGB-IR part, see ov5675_get_modes() */
> +static const struct ov5675_mode ov5678_supported_modes[] = {
> +	{
> +		.width = 2592,
> +		.height = 1944,
> +		.hts = 1500,
> +		.vts_def = OV5675_VTS_30FPS,
> +		.vts_min = OV5675_VTS_30FPS_MIN,
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
> +			.regs = mode_2592x1944_regs,
> +		},
> +		.link_freq_index = OV5675_LINK_FREQ_900MBPS,
> +	}
> +};
> +
> +/**
> + * struct ov5675_variant - What differs between the parts this driver handles
> + * @model: Name of the sub-device, by which user space tells the parts apart
> + * @code: Media bus code of the output
> + */
> +struct ov5675_variant {
> +	const char *model;
> +	u32 code;
> +};
> +
> +static const struct ov5675_variant ov5675_variant_bayer = {
> +	.model = "ov5675",
> +	.code = MEDIA_BUS_FMT_SGRBG10_1X10,
> +};
> +
> +/*
> + * The part ACPI calls OVTI5678 reports the chip id of the OV5675 and takes
> + * its register settings, but has a 4x4 RGB-IR color filter array:
> + *
> + *	G I G I
> + *	R G B G
> + *	G I G I
> + *	B G R G
> + *
> + * No media bus code describes that. MEDIA_BUS_FMT_Y10_1X10 is used because it
> + * is the 10-bit code that claims no Bayer order, so that nothing demosaics the
> + * data as if it were Bayer. The samples are not luma.
> + */
> +static const struct ov5675_variant ov5675_variant_rgbir = {
> +	.model = "ov5678",
> +	.code = MEDIA_BUS_FMT_Y10_1X10,

This needs to wait until we have proper non-Bayer pattern support in.
Hopefully this will be in for 7.5 or 7.6, but let's see.

> +};
> +
>  struct ov5675 {
>  	struct device *dev;
> +	const struct ov5675_variant *variant;
>  
>  	struct v4l2_subdev sd;
>  	struct media_pad pad;
> @@ -857,12 +913,30 @@ static int ov5675_init_controls(struct ov5675 *ov5675)
>  	return ret;
>  }
>  
> -static void ov5675_update_pad_format(const struct ov5675_mode *mode,
> +/*
> + * The 1296x972 mode is 2x2 binned. The hardware averages pixels two positions
> + * apart, which have the same color in a Bayer array, but not in the RGB-IR
> + * array. Therefore, the RGB-IR color filter array is incompatible with binning.
> + */
> +static const struct ov5675_mode *ov5675_get_modes(const struct ov5675 *ov5675,
> +						  unsigned int *num_modes)
> +{
> +	if (ov5675->variant->code != MEDIA_BUS_FMT_SGRBG10_1X10) {
> +		*num_modes = ARRAY_SIZE(ov5678_supported_modes);
> +		return ov5678_supported_modes;
> +	}
> +
> +	*num_modes = ARRAY_SIZE(supported_modes);
> +	return supported_modes;
> +}
> +
> +static void ov5675_update_pad_format(struct ov5675 *ov5675,
> +				     const struct ov5675_mode *mode,
>  				     struct v4l2_mbus_framefmt *fmt)
>  {
>  	fmt->width = mode->width;
>  	fmt->height = mode->height;
> -	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> +	fmt->code = ov5675->variant->code;
>  	fmt->field = V4L2_FIELD_NONE;
>  }
>  
> @@ -914,6 +988,25 @@ static int ov5675_start_streaming(struct ov5675 *ov5675)
>  		return ret;
>  	}
>  
> +	/*
> +	 * The mode settings enable the on-chip defect pixel correction. It
> +	 * compares a pixel with the ones two positions away, which have the
> +	 * same color in a Bayer array. In the RGB-IR array the pixel two
> +	 * positions from a red one is blue, and so comparing or mixing them
> +	 * makes no sense. So defect pixel correction is on for the Bayer part
> +	 * only. Intel's Windows driver for the OVTI5678 turns it off in the
> +	 * same way, by writing 0x71 to this register.
> +	 */
> +	if (ov5675->variant->code != MEDIA_BUS_FMT_SGRBG10_1X10) {
> +		ret = ov5675_write_reg(ov5675, OV5675_REG_ISP_CTRL,
> +				       OV5675_REG_VALUE_08BIT,
> +				       OV5675_ISP_CTRL_NO_DPC);
> +		if (ret) {
> +			dev_err(ov5675->dev, "failed to disable defect correction");
> +			return ret;
> +		}
> +	}
> +
>  	ret = __v4l2_ctrl_handler_setup(ov5675->sd.ctrl_handler);
>  	if (ret)
>  		return ret;
> @@ -1020,16 +1113,16 @@ static int ov5675_set_format(struct v4l2_subdev *sd,
>  			     struct v4l2_subdev_format *fmt)
>  {
>  	struct ov5675 *ov5675 = to_ov5675(sd);
> -	const struct ov5675_mode *mode;
> +	const struct ov5675_mode *modes, *mode;
> +	unsigned int num_modes;
>  	s32 vblank_def, h_blank;
>  
> -	mode = v4l2_find_nearest_size(supported_modes,
> -				      ARRAY_SIZE(supported_modes), width,
> -				      height, fmt->format.width,
> -				      fmt->format.height);
> +	modes = ov5675_get_modes(ov5675, &num_modes);
> +	mode = v4l2_find_nearest_size(modes, num_modes, width, height,
> +				      fmt->format.width, fmt->format.height);
>  
>  	mutex_lock(&ov5675->mutex);
> -	ov5675_update_pad_format(mode, &fmt->format);
> +	ov5675_update_pad_format(ov5675, mode, &fmt->format);
>  	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
>  		*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
>  	} else {
> @@ -1067,7 +1160,8 @@ static int ov5675_get_format(struct v4l2_subdev *sd,
>  		fmt->format = *v4l2_subdev_state_get_format(sd_state,
>  							    fmt->pad);
>  	else
> -		ov5675_update_pad_format(ov5675->cur_mode, &fmt->format);
> +		ov5675_update_pad_format(ov5675, ov5675->cur_mode,
> +					 &fmt->format);
>  
>  	mutex_unlock(&ov5675->mutex);
>  
> @@ -1104,10 +1198,12 @@ static int ov5675_enum_mbus_code(struct v4l2_subdev *sd,
>  				 struct v4l2_subdev_state *sd_state,
>  				 struct v4l2_subdev_mbus_code_enum *code)
>  {
> +	struct ov5675 *ov5675 = to_ov5675(sd);
> +
>  	if (code->index > 0)
>  		return -EINVAL;
>  
> -	code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> +	code->code = ov5675->variant->code;
>  
>  	return 0;
>  }
> @@ -1116,15 +1212,20 @@ static int ov5675_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(supported_modes))
> +	struct ov5675 *ov5675 = to_ov5675(sd);
> +	const struct ov5675_mode *modes;
> +	unsigned int num_modes;
> +
> +	modes = ov5675_get_modes(ov5675, &num_modes);
> +	if (fse->index >= num_modes)
>  		return -EINVAL;
>  
> -	if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10)
> +	if (fse->code != ov5675->variant->code)
>  		return -EINVAL;
>  
> -	fse->min_width = supported_modes[fse->index].width;
> +	fse->min_width = modes[fse->index].width;
>  	fse->max_width = fse->min_width;
> -	fse->min_height = supported_modes[fse->index].height;
> +	fse->min_height = modes[fse->index].height;
>  	fse->max_height = fse->min_height;
>  
>  	return 0;
> @@ -1133,9 +1234,13 @@ static int ov5675_enum_frame_size(struct v4l2_subdev *sd,
>  static int ov5675_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>  {
>  	struct ov5675 *ov5675 = to_ov5675(sd);
> +	const struct ov5675_mode *modes;
> +	unsigned int num_modes;
> +
> +	modes = ov5675_get_modes(ov5675, &num_modes);
>  
>  	mutex_lock(&ov5675->mutex);
> -	ov5675_update_pad_format(&supported_modes[0],
> +	ov5675_update_pad_format(ov5675, &modes[0],
>  				 v4l2_subdev_state_get_format(fh->state, 0));
>  	mutex_unlock(&ov5675->mutex);
>  
> @@ -1281,6 +1386,7 @@ static void ov5675_remove(struct i2c_client *client)
>  static int ov5675_probe(struct i2c_client *client)
>  {
>  	struct ov5675 *ov5675;
> +	unsigned int num_modes;
>  	bool full_power;
>  	int ret;
>  
> @@ -1290,11 +1396,17 @@ static int ov5675_probe(struct i2c_client *client)
>  
>  	ov5675->dev = &client->dev;
>  
> +	ov5675->variant = device_get_match_data(ov5675->dev);
> +	if (!ov5675->variant)
> +		ov5675->variant = &ov5675_variant_bayer;
> +
>  	ret = ov5675_get_hwcfg(ov5675);
>  	if (ret)
>  		return ret;
>  
>  	v4l2_i2c_subdev_init(&ov5675->sd, client, &ov5675_subdev_ops);
> +	v4l2_i2c_subdev_set_name(&ov5675->sd, client, ov5675->variant->model,
> +				 NULL);
>  
>  	ret = ov5675_power_on(ov5675->dev);
>  	if (ret) {
> @@ -1312,7 +1424,7 @@ static int ov5675_probe(struct i2c_client *client)
>  	}
>  
>  	mutex_init(&ov5675->mutex);
> -	ov5675->cur_mode = &supported_modes[0];
> +	ov5675->cur_mode = ov5675_get_modes(ov5675, &num_modes);
>  	ret = ov5675_init_controls(ov5675);
>  	if (ret) {
>  		dev_err(ov5675->dev, "failed to init controls: %d", ret);
> @@ -1363,7 +1475,8 @@ static const struct dev_pm_ops ov5675_pm_ops = {
>  
>  #ifdef CONFIG_ACPI
>  static const struct acpi_device_id ov5675_acpi_ids[] = {
> -	{"OVTI5675"},
> +	{"OVTI5675", (kernel_ulong_t)&ov5675_variant_bayer},
> +	{"OVTI5678", (kernel_ulong_t)&ov5675_variant_rgbir},
>  	{}
>  };
>  
> @@ -1371,7 +1484,7 @@ MODULE_DEVICE_TABLE(acpi, ov5675_acpi_ids);
>  #endif
>  
>  static const struct of_device_id ov5675_of_match[] = {
> -	{ .compatible = "ovti,ov5675", },
> +	{ .compatible = "ovti,ov5675", .data = &ov5675_variant_bayer },

It'd be nice to add DT bindings, too.

>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, ov5675_of_match);

-- 
Regards,

Sakari Ailus

Re: [PATCH 3/3] media: i2c: ov5675: Add the OVTI5678 RGB-IR variant

[Andy Shevchenko]

+ Uwe.

On Tue, Oct 20, 2026 at 11:42:07AM +0300, Sakari Ailus wrote:
> On Tue, Oct 20, 2026 at 09:50:01AM +0800, Alexander Patrakov wrote:

...

> >  #include <linux/gpio/consumer.h>
> >  #include <linux/i2c.h>
> > +#include <linux/mod_devicetable.h>
> 
> #include <linux/device-id/acpi.h>

No, this has to be part of i2c.h on par with device-id/of.h

Uwe, can we get this fixed, please?

The basic idea that whenever we have -id/of.h included, we must have -id/acpi.h
included as well. Otherwise we will get tons of this explicit include.

Or let's do it other way around, drop -id/of.h from i2c.h/spi.h/et cetera (but
I predict the answer to this approach).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] media: i2c: ov5675: Add the OVTI5678 RGB-IR variant

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

Hello Andy,

On Tue, Oct 20, 2026 at 11:57:34AM +0300, Andy Shevchenko wrote:
> On Tue, Oct 20, 2026 at 11:42:07AM +0300, Sakari Ailus wrote:
> > On Tue, Oct 20, 2026 at 09:50:01AM +0800, Alexander Patrakov wrote:
> 
> ...
> 
> > >  #include <linux/gpio/consumer.h>
> > >  #include <linux/i2c.h>
> > > +#include <linux/mod_devicetable.h>
> > 
> > #include <linux/device-id/acpi.h>
> 
> No, this has to be part of i2c.h on par with device-id/of.h

Well, <linux/i2c.h> includes <linux/acpi.h> which in turn includes
<linux/device-id/acpi.h>, so <linux/i2c.h> already provides
acpi_device_id.
 
> Uwe, can we get this fixed, please?

With the above said, there is nothing to fix, is there?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/3] media: i2c: ov5675: Add the OVTI5678 RGB-IR variant

[Andy Shevchenko]

On Tue, Oct 20, 2026 at 3:13 PM Uwe Kleine-König <ukleinek@kernel.org> wrote:
> On Tue, Oct 20, 2026 at 11:57:34AM +0300, Andy Shevchenko wrote:
> > On Tue, Oct 20, 2026 at 11:42:07AM +0300, Sakari Ailus wrote:
> > > On Tue, Oct 20, 2026 at 09:50:01AM +0800, Alexander Patrakov wrote:

...

> > > >  #include <linux/gpio/consumer.h>
> > > >  #include <linux/i2c.h>
> > > > +#include <linux/mod_devicetable.h>
> > >
> > > #include <linux/device-id/acpi.h>
> >
> > No, this has to be part of i2c.h on par with device-id/of.h
>
> Well, <linux/i2c.h> includes <linux/acpi.h> which in turn includes
> <linux/device-id/acpi.h>, so <linux/i2c.h> already provides
> acpi_device_id.
>
> > Uwe, can we get this fixed, please?
>
> With the above said, there is nothing to fix, is there?

I believe it's just a coincidence. We need to include both explicitly
(I mean -id/of.h and -id/acpi.h). Do spi.h and platform_device.h
include acpi.h as well?

TL;DR: we have an inconsistency behind this, so it's definitely
something to address.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] media: i2c: ov5675: Add the OVTI5678 RGB-IR variant

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]

Hello Andy,

On Wed, Oct 21, 2026 at 08:57:58AM +0300, Andy Shevchenko wrote:
> On Tue, Oct 20, 2026 at 3:13 PM Uwe Kleine-König <ukleinek@kernel.org> wrote:
> > On Tue, Oct 20, 2026 at 11:57:34AM +0300, Andy Shevchenko wrote:
> > > On Tue, Oct 20, 2026 at 11:42:07AM +0300, Sakari Ailus wrote:
> > > > On Tue, Oct 20, 2026 at 09:50:01AM +0800, Alexander Patrakov wrote:
> 
> ...
> 
> > > > >  #include <linux/gpio/consumer.h>
> > > > >  #include <linux/i2c.h>
> > > > > +#include <linux/mod_devicetable.h>
> > > >
> > > > #include <linux/device-id/acpi.h>
> > >
> > > No, this has to be part of i2c.h on par with device-id/of.h
> >
> > Well, <linux/i2c.h> includes <linux/acpi.h> which in turn includes
> > <linux/device-id/acpi.h>, so <linux/i2c.h> already provides
> > acpi_device_id.
> >
> > > Uwe, can we get this fixed, please?
> >
> > With the above said, there is nothing to fix, is there?
> 
> I believe it's just a coincidence. We need to include both explicitly
> (I mean -id/of.h and -id/acpi.h). Do spi.h and platform_device.h
> include acpi.h as well?
> 
> TL;DR: we have an inconsistency behind this, so it's definitely
> something to address.

IMHO the transitive include is good enough, so I won't address your
concern. But if you consider it important, I also won't (try to) stop
you.

Best regards
Uwe


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] media: ipu-bridge: Add sensor configuration for OV5675 (OVTI5678)

[Andy Shevchenko]

On Tue, Oct 20, 2026 at 4:50 AM Alexander Patrakov <patrakov@gmail.com> wrote:
>
> The front camera of the Dell Latitude 7320 Detachable is an OV5675
> described in ACPI as OVTI5678. Without an entry here ipu-bridge creates
> no software node for it, so no fwnode endpoint exists and the sensor
> cannot be connected to the IPU6 even once a driver binds it:
>
>   intel-ipu6 0000:00:05.0: Found supported sensor OVTI8856:00
>   intel-ipu6 0000:00:05.0: Connected 1 cameras
>
> 450 MHz is OV5675_LINK_FREQ_450MHZ, the only link frequency ov5675.c
> supports, and matches the two data lanes the machine's SSDB reports.
>
> With this entry both cameras are found and the front sensor appears in
> the media graph:
>
>   intel-ipu6 0000:00:05.0: Found supported sensor OVTI5678:00
>   intel-ipu6 0000:00:05.0: Found supported sensor OVTI8856:00
>   intel-ipu6 0000:00:05.0: Connected 2 cameras

> [Rebased onto next-20261016 and adjusted the comment]

This should usually go just before your SoB line.

> Signed-off-by: Sahan Nissanka <adee.sahan@gmail.com>
> Signed-off-by: Alexander Patrakov <patrakov@gmail.com>

> Link: https://lore.kernel.org/linux-media/20260809042540.15849-4-adee.sahan@gmail.com/

What is this Link for? There is no reference to it in the text of the
commit message.


> -       /* Omnivision OV5675 */
> +       /* Omnivision OV5675 and its RGB-IR variant OV5678 */
>         IPU_SENSOR_CONFIG("OVTI5675", 1, 450000000),
> +       IPU_SENSOR_CONFIG("OVTI5678", 1, 450000000),

Please, split the comment, id est leave original one and add a new one like
       /* Omnivision OV5678 (variant of OV5678 with RGB-IR) */

Note, there was discussion at LPC 2026 about RGB-IR support. My
understanding is that the kernel lacks it right now, but media people
(Hans, Laurent) may correct me if I'm wrong.

-- 
With Best Regards,
Andy Shevchenko