platform/x86: int3472: add HP Spectre handshake delay

[PATCH] platform/x86: int3472: add HP Spectre handshake delay

Posted by James Alexander 1 month, 1 week ago

The OVTI08F4 sensor on an HP Spectre x360 14-eu0xxx failed to read its
chip ID with error -121 when the existing 45 ms handshake delay was used.
Increasing the delay to 150 ms allowed the sensor to probe.

Keep the existing delay for other OVTI08F4 systems. Add a reusable
handshake-delay field to the discrete-device quirks and set it only for the
affected HP Spectre family through DMI matching.

Compile-tested against Linux base fd923b32d761. Hardware-tested with the
full patch set across 25 consecutive reboots on the affected laptop running
Ubuntu 7.0.0-29-generic; the sensor probed successfully on every boot.

Signed-off-by: James Alexander <opensource@inspiredexperts.com>
---
diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 115bb3757..80efeb452 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -193,6 +193,9 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 			      GPIO_ACTIVE_LOW : GPIO_ACTIVE_HIGH;
 		*con_id = int3472_gpio_map[i].con_id;
 		*enable_time_us = int3472_gpio_map[i].enable_time_us;
+		if (int3472_gpio_map[i].type_to == INT3472_GPIO_TYPE_HANDSHAKE &&
+		    int3472->quirks.handshake_enable_time_us)
+			*enable_time_us = int3472->quirks.handshake_enable_time_us;
 		return;
 	}
 
diff --git a/drivers/platform/x86/intel/int3472/discrete_quirks.c b/drivers/platform/x86/intel/int3472/discrete_quirks.c
index 552869ef9..d31f7c764 100644
--- a/drivers/platform/x86/intel/int3472/discrete_quirks.c
+++ b/drivers/platform/x86/intel/int3472/discrete_quirks.c
@@ -8,6 +8,10 @@ static const struct int3472_discrete_quirks lenovo_miix_510_quirks = {
 	.avdd_second_sensor = "i2c-OVTI2680:00",
 };
 
+static const struct int3472_discrete_quirks hp_spectre_14_eu0xxx_quirks = {
+	.handshake_enable_time_us = 150 * USEC_PER_MSEC,
+};
+
 const struct dmi_system_id skl_int3472_discrete_quirks[] = {
 	{
 		/* Lenovo Miix 510-12IKB */
@@ -17,5 +21,14 @@ const struct dmi_system_id skl_int3472_discrete_quirks[] = {
 		},
 		.driver_data = (void *)&lenovo_miix_510_quirks,
 	},
+	{
+		/* HP Spectre x360 2-in-1 Laptop 14-eu0xxx */
+		.matches = {
+			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_EXACT_MATCH(DMI_PRODUCT_NAME,
+					"HP Spectre x360 2-in-1 Laptop 14-eu0xxx"),
+		},
+		.driver_data = (void *)&hp_spectre_14_eu0xxx_quirks,
+	},
 	{ }
 };
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index a73841dfa..797694aa8 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -96,6 +96,8 @@ struct int3472_cldb {
 struct int3472_discrete_quirks {
 	/* For models where AVDD GPIO is shared between sensors */
 	const char *avdd_second_sensor;
+	/* Override the default delay for handshake GPIO regulators */
+	unsigned int handshake_enable_time_us;
 };
 
 struct int3472_gpio_regulator {
-- 
2.46.0

Re: [PATCH] platform/x86: int3472: add HP Spectre handshake delay

Posted by Hans de Goede 1 month, 1 week ago

Hi,

On 16-Aug-26 22:42, James Alexander wrote:
> The OVTI08F4 sensor on an HP Spectre x360 14-eu0xxx failed to read its
> chip ID with error -121 when the existing 45 ms handshake delay was used.
> Increasing the delay to 150 ms allowed the sensor to probe.
> 
> Keep the existing delay for other OVTI08F4 systems. Add a reusable
> handshake-delay field to the discrete-device quirks and set it only for the
> affected HP Spectre family through DMI matching.
> 
> Compile-tested against Linux base fd923b32d761. Hardware-tested with the
> full patch set across 25 consecutive reboots on the affected laptop running
> Ubuntu 7.0.0-29-generic; the sensor probed successfully on every boot.
> 
> Signed-off-by: James Alexander <opensource@inspiredexperts.com>

Thank you for your patch.

There have been more reports about the 45 ms not being enough of a delay,
but IIRC I never got around to bumping the delay because even with
a bigger delay on the laptop in question the probe would still fail
approx. every 1 out of 10 boots and we got stuck on debugging that.

And then I dropped the bal on this, sorry.

See:
https://bugzilla.redhat.com/show_bug.cgi?id=2333331
https://bugzilla.redhat.com/show_bug.cgi?id=2333647

Note as mentioned there in some cases 200ms is necessary.

So please instead of adding this quirk, just bump the existing
OVTI08F4 delay from 45 ms to 200 ms as various other HP laptops
also need a bigger delay and 200 ms seems to do the trick everywhere.

Note it would be also good to add Link: tags to the 2 bugzillas
above.

Regards,

Hans





> ---
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 115bb3757..80efeb452 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -193,6 +193,9 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>  			      GPIO_ACTIVE_LOW : GPIO_ACTIVE_HIGH;
>  		*con_id = int3472_gpio_map[i].con_id;
>  		*enable_time_us = int3472_gpio_map[i].enable_time_us;
> +		if (int3472_gpio_map[i].type_to == INT3472_GPIO_TYPE_HANDSHAKE &&
> +		    int3472->quirks.handshake_enable_time_us)
> +			*enable_time_us = int3472->quirks.handshake_enable_time_us;
>  		return;
>  	}
>  
> diff --git a/drivers/platform/x86/intel/int3472/discrete_quirks.c b/drivers/platform/x86/intel/int3472/discrete_quirks.c
> index 552869ef9..d31f7c764 100644
> --- a/drivers/platform/x86/intel/int3472/discrete_quirks.c
> +++ b/drivers/platform/x86/intel/int3472/discrete_quirks.c
> @@ -8,6 +8,10 @@ static const struct int3472_discrete_quirks lenovo_miix_510_quirks = {
>  	.avdd_second_sensor = "i2c-OVTI2680:00",
>  };
>  
> +static const struct int3472_discrete_quirks hp_spectre_14_eu0xxx_quirks = {
> +	.handshake_enable_time_us = 150 * USEC_PER_MSEC,
> +};
> +
>  const struct dmi_system_id skl_int3472_discrete_quirks[] = {
>  	{
>  		/* Lenovo Miix 510-12IKB */
> @@ -17,5 +21,14 @@ const struct dmi_system_id skl_int3472_discrete_quirks[] = {
>  		},
>  		.driver_data = (void *)&lenovo_miix_510_quirks,
>  	},
> +	{
> +		/* HP Spectre x360 2-in-1 Laptop 14-eu0xxx */
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "HP"),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_NAME,
> +					"HP Spectre x360 2-in-1 Laptop 14-eu0xxx"),
> +		},
> +		.driver_data = (void *)&hp_spectre_14_eu0xxx_quirks,
> +	},
>  	{ }
>  };
> diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
> index a73841dfa..797694aa8 100644
> --- a/include/linux/platform_data/x86/int3472.h
> +++ b/include/linux/platform_data/x86/int3472.h
> @@ -96,6 +96,8 @@ struct int3472_cldb {
>  struct int3472_discrete_quirks {
>  	/* For models where AVDD GPIO is shared between sensors */
>  	const char *avdd_second_sensor;
> +	/* Override the default delay for handshake GPIO regulators */
> +	unsigned int handshake_enable_time_us;
>  };
>  
>  struct int3472_gpio_regulator {

Re: [PATCH] platform/x86: int3472: add HP Spectre handshake delay

Posted by Hans de Goede 1 month, 1 week ago

Hi Alexander,

On 18-Aug-26 12:43, Hans de Goede wrote:
> Hi,
> 
> On 16-Aug-26 22:42, James Alexander wrote:
>> The OVTI08F4 sensor on an HP Spectre x360 14-eu0xxx failed to read its
>> chip ID with error -121 when the existing 45 ms handshake delay was used.
>> Increasing the delay to 150 ms allowed the sensor to probe.
>>
>> Keep the existing delay for other OVTI08F4 systems. Add a reusable
>> handshake-delay field to the discrete-device quirks and set it only for the
>> affected HP Spectre family through DMI matching.
>>
>> Compile-tested against Linux base fd923b32d761. Hardware-tested with the
>> full patch set across 25 consecutive reboots on the affected laptop running
>> Ubuntu 7.0.0-29-generic; the sensor probed successfully on every boot.
>>
>> Signed-off-by: James Alexander <opensource@inspiredexperts.com>
> 
> Thank you for your patch.
> 
> There have been more reports about the 45 ms not being enough of a delay,
> but IIRC I never got around to bumping the delay because even with
> a bigger delay on the laptop in question the probe would still fail
> approx. every 1 out of 10 boots and we got stuck on debugging that.
> 
> And then I dropped the bal on this, sorry.
> 
> See:
> https://bugzilla.redhat.com/show_bug.cgi?id=2333331

Funny after re-reading that bug I see you are one of the reporters
of it, again sorry for dropping the ball on this.

> https://bugzilla.redhat.com/show_bug.cgi?id=2333647

And this is more or less the same thing, but here the sensor is
a hi556. So it looks like the fix is to simply boost the handshake
pin default delay to 200 ms instead of having a model / device
specific quirk, dropping the existing OVTI08F4 quirk.

I'll go and prepare a patch for this and submit it upstream myself.

Regards,

Hans

Re: [PATCH] platform/x86: int3472: add HP Spectre handshake delay

Posted by Ilpo Järvinen 1 month, 1 week ago

On Sun, 16 Aug 2026, James Alexander wrote:

> The OVTI08F4 sensor on an HP Spectre x360 14-eu0xxx failed to read its
> chip ID with error -121 when the existing 45 ms handshake delay was used.
> Increasing the delay to 150 ms allowed the sensor to probe.
> 
> Keep the existing delay for other OVTI08F4 systems. Add a reusable
> handshake-delay field to the discrete-device quirks and set it only for the
> affected HP Spectre family through DMI matching.
> 
> Compile-tested against Linux base fd923b32d761. Hardware-tested with the
> full patch set across 25 consecutive reboots on the affected laptop running
> Ubuntu 7.0.0-29-generic; the sensor probed successfully on every boot.
> 
> Signed-off-by: James Alexander <opensource@inspiredexperts.com>
> ---
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 115bb3757..80efeb452 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -193,6 +193,9 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>  			      GPIO_ACTIVE_LOW : GPIO_ACTIVE_HIGH;
>  		*con_id = int3472_gpio_map[i].con_id;
>  		*enable_time_us = int3472_gpio_map[i].enable_time_us;
> +		if (int3472_gpio_map[i].type_to == INT3472_GPIO_TYPE_HANDSHAKE &&
> +		    int3472->quirks.handshake_enable_time_us)
> +			*enable_time_us = int3472->quirks.handshake_enable_time_us;
>  		return;
>  	}
>  
> diff --git a/drivers/platform/x86/intel/int3472/discrete_quirks.c b/drivers/platform/x86/intel/int3472/discrete_quirks.c
> index 552869ef9..d31f7c764 100644
> --- a/drivers/platform/x86/intel/int3472/discrete_quirks.c
> +++ b/drivers/platform/x86/intel/int3472/discrete_quirks.c
> @@ -8,6 +8,10 @@ static const struct int3472_discrete_quirks lenovo_miix_510_quirks = {
>  	.avdd_second_sensor = "i2c-OVTI2680:00",
>  };
>  
> +static const struct int3472_discrete_quirks hp_spectre_14_eu0xxx_quirks = {
> +	.handshake_enable_time_us = 150 * USEC_PER_MSEC,

Hi,

Please add include for USEC_PER_MSEC.

--
 i.

> +};
> +
>  const struct dmi_system_id skl_int3472_discrete_quirks[] = {
>  	{
>  		/* Lenovo Miix 510-12IKB */
> @@ -17,5 +21,14 @@ const struct dmi_system_id skl_int3472_discrete_quirks[] = {
>  		},
>  		.driver_data = (void *)&lenovo_miix_510_quirks,
>  	},
> +	{
> +		/* HP Spectre x360 2-in-1 Laptop 14-eu0xxx */
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "HP"),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_NAME,
> +					"HP Spectre x360 2-in-1 Laptop 14-eu0xxx"),
> +		},
> +		.driver_data = (void *)&hp_spectre_14_eu0xxx_quirks,
> +	},
>  	{ }
>  };
> diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
> index a73841dfa..797694aa8 100644
> --- a/include/linux/platform_data/x86/int3472.h
> +++ b/include/linux/platform_data/x86/int3472.h
> @@ -96,6 +96,8 @@ struct int3472_cldb {
>  struct int3472_discrete_quirks {
>  	/* For models where AVDD GPIO is shared between sensors */
>  	const char *avdd_second_sensor;
> +	/* Override the default delay for handshake GPIO regulators */
> +	unsigned int handshake_enable_time_us;
>  };
>  
>  struct int3472_gpio_regulator {
>