usb: typec: ucsi_acpi: Refactor and fix DELL quirk
commit 6aaceb7d9cd00f3e065dc4b054ecfe52c5253b03 upstream. Some DELL systems don't like UCSI_ACK_CC_CI commands with the UCSI_ACK_CONNECTOR_CHANGE but not the UCSI_ACK_COMMAND_COMPLETE bit set. The current quirk still leaves room for races because it requires two consecutive ACK commands to be sent. Refactor and significantly simplify the quirk to fix this: Send a dummy command and bundle the connector change ack with the command completion ack in a single UCSI_ACK_CC_CI command. This removes the need to probe for the quirk. While there define flag bits for struct ucsi_acpi->flags in ucsi_acpi.c and don't re-use definitions from ucsi.h for struct ucsi->flags. Fixes: f3be347ea42d ("usb: ucsi_acpi: Quirk to ack a connector change ack cmd") Cc: stable@vger.kernel.org Signed-off-by: Christian A. Ehrhardt <lk@c--e.de> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550-QRD Link: https://lore.kernel.org/r/20240320073927.1641788-5-lk@c--e.de Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
5857494b50
commit
1f510af8db
|
@ -23,10 +23,11 @@ struct ucsi_acpi {
|
|||
void *base;
|
||||
struct completion complete;
|
||||
unsigned long flags;
|
||||
#define UCSI_ACPI_SUPPRESS_EVENT 0
|
||||
#define UCSI_ACPI_COMMAND_PENDING 1
|
||||
#define UCSI_ACPI_ACK_PENDING 2
|
||||
guid_t guid;
|
||||
u64 cmd;
|
||||
bool dell_quirk_probed;
|
||||
bool dell_quirk_active;
|
||||
};
|
||||
|
||||
static int ucsi_acpi_dsm(struct ucsi_acpi *ua, int func)
|
||||
|
@ -79,9 +80,9 @@ static int ucsi_acpi_sync_write(struct ucsi *ucsi, unsigned int offset,
|
|||
int ret;
|
||||
|
||||
if (ack)
|
||||
set_bit(ACK_PENDING, &ua->flags);
|
||||
set_bit(UCSI_ACPI_ACK_PENDING, &ua->flags);
|
||||
else
|
||||
set_bit(COMMAND_PENDING, &ua->flags);
|
||||
set_bit(UCSI_ACPI_COMMAND_PENDING, &ua->flags);
|
||||
|
||||
ret = ucsi_acpi_async_write(ucsi, offset, val, val_len);
|
||||
if (ret)
|
||||
|
@ -92,9 +93,9 @@ static int ucsi_acpi_sync_write(struct ucsi *ucsi, unsigned int offset,
|
|||
|
||||
out_clear_bit:
|
||||
if (ack)
|
||||
clear_bit(ACK_PENDING, &ua->flags);
|
||||
clear_bit(UCSI_ACPI_ACK_PENDING, &ua->flags);
|
||||
else
|
||||
clear_bit(COMMAND_PENDING, &ua->flags);
|
||||
clear_bit(UCSI_ACPI_COMMAND_PENDING, &ua->flags);
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
@ -129,51 +130,40 @@ static const struct ucsi_operations ucsi_zenbook_ops = {
|
|||
};
|
||||
|
||||
/*
|
||||
* Some Dell laptops expect that an ACK command with the
|
||||
* UCSI_ACK_CONNECTOR_CHANGE bit set is followed by a (separate)
|
||||
* ACK command that only has the UCSI_ACK_COMMAND_COMPLETE bit set.
|
||||
* If this is not done events are not delivered to OSPM and
|
||||
* subsequent commands will timeout.
|
||||
* Some Dell laptops don't like ACK commands with the
|
||||
* UCSI_ACK_CONNECTOR_CHANGE but not the UCSI_ACK_COMMAND_COMPLETE
|
||||
* bit set. To work around this send a dummy command and bundle the
|
||||
* UCSI_ACK_CONNECTOR_CHANGE with the UCSI_ACK_COMMAND_COMPLETE
|
||||
* for the dummy command.
|
||||
*/
|
||||
static int
|
||||
ucsi_dell_sync_write(struct ucsi *ucsi, unsigned int offset,
|
||||
const void *val, size_t val_len)
|
||||
{
|
||||
struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
|
||||
u64 cmd = *(u64 *)val, ack = 0;
|
||||
u64 cmd = *(u64 *)val;
|
||||
u64 dummycmd = UCSI_GET_CAPABILITY;
|
||||
int ret;
|
||||
|
||||
if (UCSI_COMMAND(cmd) == UCSI_ACK_CC_CI &&
|
||||
cmd & UCSI_ACK_CONNECTOR_CHANGE)
|
||||
ack = UCSI_ACK_CC_CI | UCSI_ACK_COMMAND_COMPLETE;
|
||||
if (cmd == (UCSI_ACK_CC_CI | UCSI_ACK_CONNECTOR_CHANGE)) {
|
||||
cmd |= UCSI_ACK_COMMAND_COMPLETE;
|
||||
|
||||
ret = ucsi_acpi_sync_write(ucsi, offset, val, val_len);
|
||||
if (ret != 0)
|
||||
return ret;
|
||||
if (ack == 0)
|
||||
return ret;
|
||||
/*
|
||||
* The UCSI core thinks it is sending a connector change ack
|
||||
* and will accept new connector change events. We don't want
|
||||
* this to happen for the dummy command as its response will
|
||||
* still report the very event that the core is trying to clear.
|
||||
*/
|
||||
set_bit(UCSI_ACPI_SUPPRESS_EVENT, &ua->flags);
|
||||
ret = ucsi_acpi_sync_write(ucsi, UCSI_CONTROL, &dummycmd,
|
||||
sizeof(dummycmd));
|
||||
clear_bit(UCSI_ACPI_SUPPRESS_EVENT, &ua->flags);
|
||||
|
||||
if (!ua->dell_quirk_probed) {
|
||||
ua->dell_quirk_probed = true;
|
||||
|
||||
cmd = UCSI_GET_CAPABILITY;
|
||||
ret = ucsi_acpi_sync_write(ucsi, UCSI_CONTROL, &cmd,
|
||||
sizeof(cmd));
|
||||
if (ret == 0)
|
||||
return ucsi_acpi_sync_write(ucsi, UCSI_CONTROL,
|
||||
&ack, sizeof(ack));
|
||||
if (ret != -ETIMEDOUT)
|
||||
if (ret < 0)
|
||||
return ret;
|
||||
|
||||
ua->dell_quirk_active = true;
|
||||
dev_err(ua->dev, "Firmware bug: Additional ACK required after ACKing a connector change.\n");
|
||||
dev_err(ua->dev, "Firmware bug: Enabling workaround\n");
|
||||
}
|
||||
|
||||
if (!ua->dell_quirk_active)
|
||||
return ret;
|
||||
|
||||
return ucsi_acpi_sync_write(ucsi, UCSI_CONTROL, &ack, sizeof(ack));
|
||||
return ucsi_acpi_sync_write(ucsi, UCSI_CONTROL, &cmd, sizeof(cmd));
|
||||
}
|
||||
|
||||
static const struct ucsi_operations ucsi_dell_ops = {
|
||||
|
@ -209,13 +199,14 @@ static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
|
|||
if (ret)
|
||||
return;
|
||||
|
||||
if (UCSI_CCI_CONNECTOR(cci))
|
||||
if (UCSI_CCI_CONNECTOR(cci) &&
|
||||
!test_bit(UCSI_ACPI_SUPPRESS_EVENT, &ua->flags))
|
||||
ucsi_connector_change(ua->ucsi, UCSI_CCI_CONNECTOR(cci));
|
||||
|
||||
if (cci & UCSI_CCI_ACK_COMPLETE && test_bit(ACK_PENDING, &ua->flags))
|
||||
complete(&ua->complete);
|
||||
if (cci & UCSI_CCI_COMMAND_COMPLETE &&
|
||||
test_bit(COMMAND_PENDING, &ua->flags))
|
||||
test_bit(UCSI_ACPI_COMMAND_PENDING, &ua->flags))
|
||||
complete(&ua->complete);
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue