Commit 6aaceb7d authored by Christian A. Ehrhardt's avatar Christian A. Ehrhardt Committed by Greg Kroah-Hartman
Browse files

usb: typec: ucsi_acpi: Refactor and fix DELL quirk 



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: f3be347e ("usb: ucsi_acpi: Quirk to ack a connector change ack cmd")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarChristian A. Ehrhardt <lk@c--e.de>
Reviewed-by: default avatarHeikki 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: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 6b5c85dd

drivers/usb/typec/ucsi/ucsi_acpi.c

+33 −42
Original line number Diff line number Diff line
@@ -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;



	if (!ua->dell_quirk_probed) {

		ua->dell_quirk_probed = true;

		/*

		 * 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);



		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);

}