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 <diogo.ivo@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> 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 <diogo.ivo@xxxxxxxxxxxxxxxxxx>
--
heikki
