From patchwork Sun Jan 21 20:41:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Christian A. Ehrhardt" X-Patchwork-Id: 13524675 Received: from cae.in-ulm.de (cae.in-ulm.de [217.10.14.231]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 96DAD381AC; Sun, 21 Jan 2024 20:41:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.10.14.231 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705869703; cv=none; b=Mx53Vo0AGVJ5TRaYFW0DgLTZrDo2m6NaS+qgj8xrAoRzrdP8XQ5KLXlVbrDh8gsUDg1Jz/5beF/L983tSQQraJPwTaWY7QdODQ00fy93aeh1bFC0hggRJxQMxri3hETxO126qIP+og99sNYefIj/k3bxIwIWGhC2blL+8kKhEbE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705869703; c=relaxed/simple; bh=a74cBVmfYeJUZpWdG+gkCPScISxo1VpIOiqyEL6S6W8=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=UXj9FEOY6s9/E3m4XCTIWvP8LrKgzYS8/WwBwF8cVmluYdcSKXfN49mkZ1mAI75Q31GLTCmoLHyOrwbH3j79BtXqGcurKiwpDlDB7fO2wU2qkcwL2lIxnFkkTj+8ZucVihhBz63N2/2prRaHklgTUDsNh7qXf13MsqVjk/THhcs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=c--e.de; spf=pass smtp.mailfrom=c--e.de; arc=none smtp.client-ip=217.10.14.231 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=c--e.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=c--e.de Received: by cae.in-ulm.de (Postfix, from userid 1000) id 9B3C7140255; Sun, 21 Jan 2024 21:41:39 +0100 (CET) From: "Christian A. Ehrhardt" To: Heikki Krogerus , linux-usb@vger.kernel.org Cc: "Christian A. Ehrhardt" , Dell.Client.Kernel@dell.com, Greg Kroah-Hartman , Neil Armstrong , Hans de Goede , Jack Pham , Fabrice Gasnier , =?utf-8?q?Samuel_=C4=8Cavoj?= , linux-kernel@vger.kernel.org Subject: [PATCH v3 1/3] usb: ucsi: Add missing ppm_lock Date: Sun, 21 Jan 2024 21:41:21 +0100 Message-Id: <20240121204123.275441-2-lk@c--e.de> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20240121204123.275441-1-lk@c--e.de> References: <20240121204123.275441-1-lk@c--e.de> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Calling ->sync_write must be done while holding the PPM lock as the mailbox logic does not support concurrent commands. At least since the addition of partner task this means that ucsi_acknowledge_connector_change should be called with the PPM lock held as it calls ->sync_write. Thus protect the only call to ucsi_acknowledge_connector_change with the PPM. All other calls to ->sync_write already happen under the PPM lock. Fixes: b9aa02ca39a4 ("usb: typec: ucsi: Add polling mechanism for partner tasks like alt mode checking") Cc: stable@vger.kernel.org Signed-off-by: Christian A. Ehrhardt Reviewed-by: Heikki Krogerus --- drivers/usb/typec/ucsi/ucsi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index 61b64558f96c..8f9dff993b3d 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -935,7 +935,9 @@ static void ucsi_handle_connector_change(struct work_struct *work) clear_bit(EVENT_PENDING, &con->ucsi->flags); + mutex_lock(&ucsi->ppm_lock); ret = ucsi_acknowledge_connector_change(ucsi); + mutex_unlock(&ucsi->ppm_lock); if (ret) dev_err(ucsi->dev, "%s: ACK failed (%d)", __func__, ret); From patchwork Sun Jan 21 20:41:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Christian A. Ehrhardt" X-Patchwork-Id: 13524676 Received: from cae.in-ulm.de (cae.in-ulm.de [217.10.14.231]) by smtp.subspace.kernel.org (Postfix) with ESMTP id A483A38FA1; Sun, 21 Jan 2024 20:41:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.10.14.231 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705869707; cv=none; b=e662Hui0FwNxfKjHRcrpfvy8Mn2iv3WrNw6i4Dfue/XsWu6gZqJUxPVh0QN8sQ+3LyBq6VSk9Y27Mrv8o4eiUVRSE+94TwPKVj+iK6jlygZbLuHHgS8v/yeP6aWMYJnLUI1GNhLx0k4+1VWgnLcVeGh4EYpgeFJod9lHQJFPZ3M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705869707; c=relaxed/simple; bh=YPimAc9/op1FgIOJZ7Xs+meT1zX8exfGnSTp7c1KsRE=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=ji00rhKEhw0dlbS9xniSL2cy6u0tgJs8KGzsp4A7OwI0yd39bWxjDwxvxIHQwKqNTYlPWGdZ1ErfenFV6MRCjg/mG6+uFyKHrWl8GWg/JmZHblWztgrbGPZSav58ixrARUGuQfmrjv0y0uMESI7H3JpkYmoOFWGZd977OtxNpVI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=c--e.de; spf=pass smtp.mailfrom=c--e.de; arc=none smtp.client-ip=217.10.14.231 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=c--e.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=c--e.de Received: by cae.in-ulm.de (Postfix, from userid 1000) id E29B11402D8; Sun, 21 Jan 2024 21:41:44 +0100 (CET) From: "Christian A. Ehrhardt" To: Heikki Krogerus , linux-usb@vger.kernel.org Cc: "Christian A. Ehrhardt" , Dell.Client.Kernel@dell.com, Greg Kroah-Hartman , Neil Armstrong , Hans de Goede , Jack Pham , Fabrice Gasnier , =?utf-8?q?Samuel_=C4=8Cavoj?= , linux-kernel@vger.kernel.org Subject: [PATCH v3 2/3] usb: ucsi_acpi: Fix command completion handling Date: Sun, 21 Jan 2024 21:41:22 +0100 Message-Id: <20240121204123.275441-3-lk@c--e.de> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20240121204123.275441-1-lk@c--e.de> References: <20240121204123.275441-1-lk@c--e.de> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 In case of a spurious or otherwise delayed notification it is possible that CCI still reports the previous completion. The UCSI spec is aware of this and provides two completion bits in CCI, one for normal commands and one for acks. As acks and commands alternate the notification handler can determine if the completion bit is from the current command. The initial UCSI code correctly handled this but the distinction between the two completion bits was lost with the introduction of the new API. To fix this revive the ACK_PENDING bit for ucsi_acpi and only complete commands if the completion bit matches. Fixes: f56de278e8ec ("usb: typec: ucsi: acpi: Move to the new API") Cc: stable@vger.kernel.org Signed-off-by: Christian A. Ehrhardt Acked-by: Heikki Krogerus --- drivers/usb/typec/ucsi/ucsi_acpi.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c index 6bbf490ac401..fa222080887d 100644 --- a/drivers/usb/typec/ucsi/ucsi_acpi.c +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c @@ -73,9 +73,13 @@ static int ucsi_acpi_sync_write(struct ucsi *ucsi, unsigned int offset, const void *val, size_t val_len) { struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi); + bool ack = UCSI_COMMAND(*(u64 *)val) == UCSI_ACK_CC_CI; int ret; - set_bit(COMMAND_PENDING, &ua->flags); + if (ack) + set_bit(ACK_PENDING, &ua->flags); + else + set_bit(COMMAND_PENDING, &ua->flags); ret = ucsi_acpi_async_write(ucsi, offset, val, val_len); if (ret) @@ -85,7 +89,10 @@ static int ucsi_acpi_sync_write(struct ucsi *ucsi, unsigned int offset, ret = -ETIMEDOUT; out_clear_bit: - clear_bit(COMMAND_PENDING, &ua->flags); + if (ack) + clear_bit(ACK_PENDING, &ua->flags); + else + clear_bit(COMMAND_PENDING, &ua->flags); return ret; } @@ -142,8 +149,10 @@ static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data) if (UCSI_CCI_CONNECTOR(cci)) ucsi_connector_change(ua->ucsi, UCSI_CCI_CONNECTOR(cci)); - if (test_bit(COMMAND_PENDING, &ua->flags) && - cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE)) + 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)) complete(&ua->complete); } From patchwork Sun Jan 21 20:41:23 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Christian A. Ehrhardt" X-Patchwork-Id: 13524677 Received: from cae.in-ulm.de (cae.in-ulm.de [217.10.14.231]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 218B438FA3; Sun, 21 Jan 2024 20:41:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.10.14.231 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705869709; cv=none; b=Ie+OwxdCkjOWLIScxvvCltFl/2ffm5DJpMJSPlhKruFl3npVfMOLVhYhoYc9CnFJ59Spn+YBWYL+v25YFOeLxnSF2hYLwBjF/aYsWeczkmWeol6qV7xpKEUDX7J+KYnF9H/FUc3i6p9zsNAQk+kHLtHuQmJQMKIrt3YMDdt3v2I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705869709; c=relaxed/simple; bh=525/nQ/RCbmWlPgDj+Mt2E13AsRmF5QqZ/hM3AZ3pkE=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=k6ilXTQ2clGQhErg+yKDoodBjh5v47Ov7OH9+0ZcRM6UQAs42aigclOOO2mYRpP85gahg4bkMGR439HH3Wwmk2//QTLgyRIWCz98yq3uIE0Q2HNS6lGDsVtFW7Ci1ufbrz4PZaHOsZgmqbGD8DoiuUMuR3oh7ctisVRgtJjdaQI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=c--e.de; spf=pass smtp.mailfrom=c--e.de; arc=none smtp.client-ip=217.10.14.231 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=c--e.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=c--e.de Received: by cae.in-ulm.de (Postfix, from userid 1000) id D281E1402E4; Sun, 21 Jan 2024 21:41:45 +0100 (CET) From: "Christian A. Ehrhardt" To: Heikki Krogerus , linux-usb@vger.kernel.org Cc: "Christian A. Ehrhardt" , Dell.Client.Kernel@dell.com, Greg Kroah-Hartman , Neil Armstrong , Hans de Goede , Jack Pham , Fabrice Gasnier , =?utf-8?q?Samuel_=C4=8Cavoj?= , linux-kernel@vger.kernel.org Subject: [PATCH v3 3/3] usb: ucsi_acpi: Quirk to ack a connector change ack cmd Date: Sun, 21 Jan 2024 21:41:23 +0100 Message-Id: <20240121204123.275441-4-lk@c--e.de> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20240121204123.275441-1-lk@c--e.de> References: <20240121204123.275441-1-lk@c--e.de> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The PPM on some Dell laptops seems to expect that the ACK_CC_CI command to clear the connector change notification is in turn followed by another ACK_CC_CI to acknowledge the ACK_CC_CI command itself. This is in violation of the UCSI spec that states: "The only notification that is not acknowledged by the OPM is the command completion notification for the ACK_CC_CI or the PPM_RESET command." Add a quirk to send this ack anyway. Apply the quirk to all Dell systems. On the first command that acks a connector change send a dummy command to determine if it runs into a timeout. Only activate the quirk if it does. This ensure that we do not break Dell systems that do not need the quirk. Signed-off-by: Christian A. Ehrhardt Reviewed-by: Heikki Krogerus --- drivers/usb/typec/ucsi/ucsi_acpi.c | 71 ++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c index fa222080887d..928eacbeb21a 100644 --- a/drivers/usb/typec/ucsi/ucsi_acpi.c +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c @@ -25,6 +25,8 @@ struct ucsi_acpi { unsigned long flags; guid_t guid; u64 cmd; + bool dell_quirk_probed; + bool dell_quirk_active; }; static int ucsi_acpi_dsm(struct ucsi_acpi *ua, int func) @@ -126,12 +128,73 @@ static const struct ucsi_operations ucsi_zenbook_ops = { .async_write = ucsi_acpi_async_write }; -static const struct dmi_system_id zenbook_dmi_id[] = { +/* + * 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. + */ +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; + int ret; + + if (UCSI_COMMAND(cmd) == UCSI_ACK_CC_CI && + cmd & UCSI_ACK_CONNECTOR_CHANGE) + ack = UCSI_ACK_CC_CI | 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; + + 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) + 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)); +} + +static const struct ucsi_operations ucsi_dell_ops = { + .read = ucsi_acpi_read, + .sync_write = ucsi_dell_sync_write, + .async_write = ucsi_acpi_async_write +}; + +static const struct dmi_system_id ucsi_acpi_quirks[] = { { .matches = { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), DMI_MATCH(DMI_PRODUCT_NAME, "ZenBook UX325UA_UM325UA"), }, + .driver_data = (void *)&ucsi_zenbook_ops, + }, + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + }, + .driver_data = (void *)&ucsi_dell_ops, }, { } }; @@ -160,6 +223,7 @@ static int ucsi_acpi_probe(struct platform_device *pdev) { struct acpi_device *adev = ACPI_COMPANION(&pdev->dev); const struct ucsi_operations *ops = &ucsi_acpi_ops; + const struct dmi_system_id *id; struct ucsi_acpi *ua; struct resource *res; acpi_status status; @@ -189,8 +253,9 @@ static int ucsi_acpi_probe(struct platform_device *pdev) init_completion(&ua->complete); ua->dev = &pdev->dev; - if (dmi_check_system(zenbook_dmi_id)) - ops = &ucsi_zenbook_ops; + id = dmi_first_match(ucsi_acpi_quirks); + if (id) + ops = id->driver_data; ua->ucsi = ucsi_create(&pdev->dev, ops); if (IS_ERR(ua->ucsi))