Re: [PATCH 2/2] platform/x86: int3472: Handle GPIO type 0x10 (DOVDD) for OV5675

[johannes . goede]

Hi,

Thank you for your patch,

On 10-Feb-26 09:18, Leif Skunberg wrote:
> The Lenovo ThinkPad X1 Fold 16 Gen 1 has an OV5675 sensor (ACPI HID
> OVTI5675) behind an INT3472 discrete PMIC controller. The INT3472
> _DSM returns GPIO type 0x10 for one of the pins, which controls the
> DOVDD (digital I/O power) regulator enable.
> 
> Type 0x10 is not currently handled by the driver, causing the GPIO to
> be ignored with a warning. Add INT3472_GPIO_TYPE_DOVDD (0x10) and
> handle it by:
> 
> 1. Adding an entry to int3472_gpio_map[] that maps the DOVDD type to
>    INT3472_GPIO_TYPE_POWER_ENABLE for the OVTI5675 HID. The con_id
>    is set to "vio" because "dovdd" exceeds GPIO_SUPPLY_NAME_LENGTH.

Note that drivers/media/i2c/ov5675.c: ov5675_supply_names[]: actually
uses "dovdd" as do many other drivers. Please use "dovdd" and make
the GPIO_SUPPLY_NAME_LENGTH define bigger to cover that.

Also please do not add a int3472_gpio_map[] entry, entries there
are for exceptions. Since you are adding support for a new GPIO type
you only have to handle it in what you call the fallback case
(which is the normal non-quirked default case) and then this will
become the default behavior for GPIO type 10 .

> 2. Adding a fallback case in int3472_get_con_id_and_polarity() so
>    devices without a gpio_map match still get a valid con_id.
> 
> 3. Adding INT3472_GPIO_TYPE_DOVDD to both switch statements in
>    skl_int3472_handle_gpio_resources() so the GPIO is acquired via
>    temp lookup and registered as a regulator.
> 
> Signed-off-by: Leif Skunberg <diamondback@xxxxxxxxxx>

Regards,

Hans


> ---
>  drivers/platform/x86/intel/int3472/discrete.c | 14 ++++++++++++++
>  include/linux/platform_data/x86/int3472.h     |  1 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 1505fc3ef..0a293e73c 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -164,6 +164,13 @@ static const struct int3472_gpio_map int3472_gpio_map[] = {
>  		.con_id = "dvdd",
>  		.enable_time_us = 45 * USEC_PER_MSEC,
>  	},
> +	{	/* ov5675: DOVDD is a power regulator enable */
> +		.hid = "OVTI5675",
> +		.type_from = INT3472_GPIO_TYPE_DOVDD,
> +		.type_to = INT3472_GPIO_TYPE_POWER_ENABLE,
> +		.con_id = "vio",
> +		.enable_time_us = GPIO_REGULATOR_ENABLE_TIME,
> +	},
>  };
>  
>  static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3472, u8 *type,
> @@ -223,6 +230,10 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>  		*con_id = "avdd";
>  		*gpio_flags = GPIO_ACTIVE_HIGH;
>  		break;
> +	case INT3472_GPIO_TYPE_DOVDD:
> +		*con_id = "vio";
> +		*gpio_flags = GPIO_ACTIVE_HIGH;
> +		break;
>  	case INT3472_GPIO_TYPE_HANDSHAKE:
>  		*con_id = "dvdd";
>  		*gpio_flags = GPIO_ACTIVE_HIGH;
> @@ -251,6 +262,7 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>   * 0x0b Power enable
>   * 0x0c Clock enable
>   * 0x0d Privacy LED
> + * 0x10 DOVDD (digital I/O voltage)
>   * 0x13 Hotplug detect
>   *
>   * There are some known platform specific quirks where that does not quite
> @@ -332,6 +344,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>  	case INT3472_GPIO_TYPE_CLK_ENABLE:
>  	case INT3472_GPIO_TYPE_PRIVACY_LED:
>  	case INT3472_GPIO_TYPE_POWER_ENABLE:
> +	case INT3472_GPIO_TYPE_DOVDD:
>  	case INT3472_GPIO_TYPE_HANDSHAKE:
>  		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
>  		if (IS_ERR(gpio)) {
> @@ -356,6 +369,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>  		case INT3472_GPIO_TYPE_POWER_ENABLE:
>  			second_sensor = int3472->quirks.avdd_second_sensor;
>  			fallthrough;
> +		case INT3472_GPIO_TYPE_DOVDD:
>  		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 b1b837583..d72c621f7 100644
> --- a/include/linux/platform_data/x86/int3472.h
> +++ b/include/linux/platform_data/x86/int3472.h
> @@ -26,6 +26,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_HANDSHAKE				0x12
>  #define INT3472_GPIO_TYPE_HOTPLUG_DETECT			0x13
>