usb: typec: ucsi_acpi: Add LG Gram quirk

Commit Message

Diogo Ivo June 12, 2024, 1:13 p.m. UTC

Some LG Gram laptops report a bogus connector change event after a
GET_PDOS command for the partner's source PDOs, which disappears from
the CCI after acknowledging the command. However, the subsequent
GET_CONNECTOR_STATUS in ucsi_handle_connector_change() still reports
this bogus change in bits 5 and 6, leading to the UCSI core re-checking
the partner's source PDOs and thus to an infinite loop.

Fix this by adding a quirk that signals when a potentially buggy GET_PDOS
command is used, checks the status change report and clears it if it is a
bogus event before sending it to the UCSI core.

Signed-off-by: Diogo Ivo <[e-mail]>
---
The models affected by this bug have been reported to be of several forms:
1xZ90Q, 1xZD90Q, 1xZB90Q, x = {5, 6, 7}, and as such this patch matches
only on the final 90Q as well as the product family since the "90Q" string
may collide with other LG models by being too short. If there are other
better ways of achieving this match I would be happy to hear about them.
---
 drivers/usb/typec/ucsi/ucsi_acpi.c | 61 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)


---
base-commit: 5821bf2dffbe18fe1f097dbb027415fa15a38e9a
change-id: 20240612-gram_quirk-ac150257c415

Best regards,

Comments

Heikki Krogerus June 13, 2024, 10:53 a.m. UTC | #1

On Wed, Jun 12, 2024 at 02:13:10PM +0100, Diogo Ivo wrote:
> Some LG Gram laptops report a bogus connector change event after a
> GET_PDOS command for the partner's source PDOs, which disappears from
> the CCI after acknowledging the command. However, the subsequent
> GET_CONNECTOR_STATUS in ucsi_handle_connector_change() still reports
> this bogus change in bits 5 and 6, leading to the UCSI core re-checking
> the partner's source PDOs and thus to an infinite loop.
> 
> Fix this by adding a quirk that signals when a potentially buggy GET_PDOS
> command is used, checks the status change report and clears it if it is a
> bogus event before sending it to the UCSI core.
> 
> Signed-off-by: Diogo Ivo <[e-mail]>

Reviewed-by: Heikki Krogerus <[e-mail]>

> ---
> The models affected by this bug have been reported to be of several forms:
> 1xZ90Q, 1xZD90Q, 1xZB90Q, x = {5, 6, 7}, and as such this patch matches
> only on the final 90Q as well as the product family since the "90Q" string
> may collide with other LG models by being too short. If there are other
> better ways of achieving this match I would be happy to hear about them.
> ---
>  drivers/usb/typec/ucsi/ucsi_acpi.c | 61 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
> index 8d112c3edae5..adf32ca0f761 100644
> --- a/drivers/usb/typec/ucsi/ucsi_acpi.c
> +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
> @@ -25,6 +25,7 @@ struct ucsi_acpi {
>  	unsigned long flags;
>  #define UCSI_ACPI_COMMAND_PENDING	1
>  #define UCSI_ACPI_ACK_PENDING		2
> +#define UCSI_ACPI_CHECK_BOGUS_EVENT	3
>  	guid_t guid;
>  	u64 cmd;
>  };
> @@ -128,6 +129,58 @@ static const struct ucsi_operations ucsi_zenbook_ops = {
>  	.async_write = ucsi_acpi_async_write
>  };
>  
> +static int ucsi_gram_read(struct ucsi *ucsi, unsigned int offset,
> +			  void *val, size_t val_len)
> +{
> +	u16 bogus_change = UCSI_CONSTAT_POWER_LEVEL_CHANGE |
> +			   UCSI_CONSTAT_PDOS_CHANGE;
> +	struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
> +	struct ucsi_connector_status *status;
> +	int ret;
> +
> +	ret = ucsi_acpi_read(ucsi, offset, val, val_len);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (UCSI_COMMAND(ua->cmd) == UCSI_GET_CONNECTOR_STATUS &&
> +	    test_bit(UCSI_ACPI_CHECK_BOGUS_EVENT, &ua->flags) &&
> +	    offset == UCSI_MESSAGE_IN) {
> +		status = (struct ucsi_connector_status *)val;
> +
> +		/* Clear the bogus change */
> +		if (status->change == bogus_change)
> +			status->change = 0;
> +
> +		clear_bit(UCSI_ACPI_CHECK_BOGUS_EVENT, &ua->flags);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ucsi_gram_sync_write(struct ucsi *ucsi, unsigned int offset,
> +				const void *val, size_t val_len)
> +{
> +	struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
> +	int ret;
> +
> +	ret = ucsi_acpi_sync_write(ucsi, offset, val, val_len);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (UCSI_COMMAND(ua->cmd) == UCSI_GET_PDOS &&
> +	    ua->cmd & UCSI_GET_PDOS_PARTNER_PDO(1) &&
> +	    ua->cmd & UCSI_GET_PDOS_SRC_PDOS)
> +		set_bit(UCSI_ACPI_CHECK_BOGUS_EVENT, &ua->flags);
> +
> +	return ret;
> +}
> +
> +static const struct ucsi_operations ucsi_gram_ops = {
> +	.read = ucsi_gram_read,
> +	.sync_write = ucsi_gram_sync_write,
> +	.async_write = ucsi_acpi_async_write
> +};
> +
>  static const struct dmi_system_id ucsi_acpi_quirks[] = {
>  	{
>  		.matches = {
> @@ -136,6 +189,14 @@ static const struct dmi_system_id ucsi_acpi_quirks[] = {
>  		},
>  		.driver_data = (void *)&ucsi_zenbook_ops,
>  	},
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "LG Electronics"),
> +			DMI_MATCH(DMI_PRODUCT_FAMILY, "LG gram PC"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "90Q"),
> +		},
> +		.driver_data = (void *)&ucsi_gram_ops,
> +	},
>  	{ }
>  };
>  
> 
> ---
> base-commit: 5821bf2dffbe18fe1f097dbb027415fa15a38e9a
> change-id: 20240612-gram_quirk-ac150257c415
> 
> Best regards,
> -- 
> Diogo Ivo <[e-mail]>

Patch

diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index 8d112c3edae5..adf32ca0f761 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -25,6 +25,7 @@  struct ucsi_acpi {
 	unsigned long flags;
 #define UCSI_ACPI_COMMAND_PENDING	1
 #define UCSI_ACPI_ACK_PENDING		2
+#define UCSI_ACPI_CHECK_BOGUS_EVENT	3
 	guid_t guid;
 	u64 cmd;
 };
@@ -128,6 +129,58 @@  static const struct ucsi_operations ucsi_zenbook_ops = {
 	.async_write = ucsi_acpi_async_write
 };
 
+static int ucsi_gram_read(struct ucsi *ucsi, unsigned int offset,
+			  void *val, size_t val_len)
+{
+	u16 bogus_change = UCSI_CONSTAT_POWER_LEVEL_CHANGE |
+			   UCSI_CONSTAT_PDOS_CHANGE;
+	struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
+	struct ucsi_connector_status *status;
+	int ret;
+
+	ret = ucsi_acpi_read(ucsi, offset, val, val_len);
+	if (ret < 0)
+		return ret;
+
+	if (UCSI_COMMAND(ua->cmd) == UCSI_GET_CONNECTOR_STATUS &&
+	    test_bit(UCSI_ACPI_CHECK_BOGUS_EVENT, &ua->flags) &&
+	    offset == UCSI_MESSAGE_IN) {
+		status = (struct ucsi_connector_status *)val;
+
+		/* Clear the bogus change */
+		if (status->change == bogus_change)
+			status->change = 0;
+
+		clear_bit(UCSI_ACPI_CHECK_BOGUS_EVENT, &ua->flags);
+	}
+
+	return ret;
+}
+
+static int ucsi_gram_sync_write(struct ucsi *ucsi, unsigned int offset,
+				const void *val, size_t val_len)
+{
+	struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
+	int ret;
+
+	ret = ucsi_acpi_sync_write(ucsi, offset, val, val_len);
+	if (ret < 0)
+		return ret;
+
+	if (UCSI_COMMAND(ua->cmd) == UCSI_GET_PDOS &&
+	    ua->cmd & UCSI_GET_PDOS_PARTNER_PDO(1) &&
+	    ua->cmd & UCSI_GET_PDOS_SRC_PDOS)
+		set_bit(UCSI_ACPI_CHECK_BOGUS_EVENT, &ua->flags);
+
+	return ret;
+}
+
+static const struct ucsi_operations ucsi_gram_ops = {
+	.read = ucsi_gram_read,
+	.sync_write = ucsi_gram_sync_write,
+	.async_write = ucsi_acpi_async_write
+};
+
 static const struct dmi_system_id ucsi_acpi_quirks[] = {
 	{
 		.matches = {
@@ -136,6 +189,14 @@  static const struct dmi_system_id ucsi_acpi_quirks[] = {
 		},
 		.driver_data = (void *)&ucsi_zenbook_ops,
 	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LG Electronics"),
+			DMI_MATCH(DMI_PRODUCT_FAMILY, "LG gram PC"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "90Q"),
+		},
+		.driver_data = (void *)&ucsi_gram_ops,
+	},
 	{ }
 };