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 <[email protected]>
---
drivers/usb/typec/ucsi/ucsi_acpi.c | 69 ++++++++++++++++++++++++++++--
1 file changed, 66 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index 33dac67154d2..a0b08b10c4a9 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,71 @@ 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;
+ }
+
+ 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 +221,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 +251,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))
--
2.40.1
Hi Christian,
kernel test robot noticed the following build warnings:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus westeri-thunderbolt/next linus/master v6.7 next-20240117]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Christian-A-Ehrhardt/usb-ucsi-Add-missing-ppm_lock/20240117-064726
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20240116224041.220740-4-lk%40c--e.de
patch subject: [PATCH 3/3] usb: ucsi_acpi: Quirk to ack a connector change ack cmd
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240118/[email protected]/config)
compiler: ClangBuiltLinux clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240118/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/usb/typec/ucsi/ucsi_acpi.c:132: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Some Dell laptops expect that an ACK command with the
vim +132 drivers/usb/typec/ucsi/ucsi_acpi.c
130
131 /**
> 132 * Some Dell laptops expect that an ACK command with the
133 * UCSI_ACK_CONNECTOR_CHANGE bit set is followed by a (separate)
134 * ACK command that only has the UCSI_ACK_COMMAND_COMPLETE bit set.
135 * If this is not done events are not delivered to OSPM and
136 * subsequent commands will timeout.
137 */
138 static int
139 ucsi_dell_sync_write(struct ucsi *ucsi, unsigned int offset,
140 const void *val, size_t val_len)
141 {
142 struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
143 u64 cmd = *(u64*)val, ack = 0;
144 int ret;
145
146 if (UCSI_COMMAND(cmd) == UCSI_ACK_CC_CI &&
147 cmd & UCSI_ACK_CONNECTOR_CHANGE)
148 ack = UCSI_ACK_CC_CI | UCSI_ACK_COMMAND_COMPLETE;
149
150 ret = ucsi_acpi_sync_write(ucsi, offset, val, val_len);
151 if (ret != 0)
152 return ret;
153 if (ack == 0)
154 return ret;
155
156 if (!ua->dell_quirk_probed) {
157 ua->dell_quirk_probed = true;
158
159 cmd = UCSI_GET_CAPABILITY;
160 ret = ucsi_acpi_sync_write(ucsi, UCSI_CONTROL, &cmd,
161 sizeof(cmd));
162 if (ret == 0)
163 return ucsi_acpi_sync_write(ucsi, UCSI_CONTROL,
164 &ack, sizeof(ack));
165 if (ret != -ETIMEDOUT)
166 return ret;
167
168 ua->dell_quirk_active = true;
169 }
170
171 if (!ua->dell_quirk_active)
172 return ret;
173
174 return ucsi_acpi_sync_write(ucsi, UCSI_CONTROL, &ack, sizeof(ack));
175 }
176
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki