Bug 219108

I have tested usb keyboards, and they do not have this problem at all.

Some system details are as follows:
      Processor: Intel i5-12500H
  
      Current Kernel: 6.6.36-1
   
      Model Number: K3502ZA
        
      BIOS verison: 306

I cannot think of any other system specification that would be important.
I am willing to assist in any way possible, including compiling kernels with any patches necessary.

Steps to reproduce:
   
      Own a 2022 Asus Vivobook S15 OLED (model number K3502ZA)
    
      Boot into kernel version 6.6.37-1 or higher
       
      Plug the laptop to charge it
       
      The keyboard has bad registration.

There is another bug there might be similar here: https://bugzilla.kernel.org/show_bug.cgi?id=218929

However, it seems to be different, and also might be resolved pretty easily in the future.

I have attached the dmesg logs from Arch's linux-lts-6.6.42-1 package.

None of these errors are new to the 6.6.36 kernel where the issue isn't present, save for the `USBC000:00` errors. But the keyboard issue is still there in linux-lts 6.6.37-1 and that kernel doesn't have those particular errors.

Once again, I am willing to help in any way possible, like testing patches, recording video, etc. Thank you in advance.

Created attachment 306634 [details]
dmesg log

Created attachment 306635 [details]
dmesg error log (for convinience)

I have also filed a bug in the Arch Linux bug tracker.
https://gitlab.archlinux.org/archlinux/packaging/packages/linux-lts/-/issues/9

First test 6.10.2.

If it's still broken, please perform regression testing using:

https://docs.kernel.org/admin-guide/bug-bisect.html

(In reply to Artem S. Tashkinov from comment #5)
> First test 6.10.2.

I checked the latest LTS release (6.6.43), so I don't see why it should matter if I checked the latest mainline kernel.

Never the less, I have checked 6.10.2, and can confirm that it has the same issue.

> If it's still broken, please perform regression testing using:
> 
> https://docs.kernel.org/admin-guide/bug-bisect.html

I will do this as soon as I get enough time to do so. Thanks for the guide ! I have never bisected something before, so I might take some time....

(In reply to icaliberdev from comment #6)
> (In reply to Artem S. Tashkinov from comment #5)
>
> I checked the latest LTS release (6.6.43), so I don't see why it should
> matter if I checked the latest mainline kernel.

In short: because that series is maintained by different people -- and those won't fix the problem. More on this can be found on https://linux-regtracking.leemhuis.info/post/frequent-reasons-why-linux-kernel-bug-reports-are-ignored/
 
> > https://docs.kernel.org/admin-guide/bug-bisect.html

FWIW, a more detailed guide can be found here: https://docs.kernel.org/admin-guide/verify-bugs-and-bisect-regressions.html

Christian Heusel over at the Arch Linux forum bisected the kernel for me !

Here is the defective commit and it's message : de52aca4d9d56c3b2f00b638d457075914b1a227

```
Author: Christian A. Ehrhardt <lk@c--e.de>
Date:   Wed Mar 27 23:45:53 2024 +0100

    usb: typec: ucsi: Never send a lone connector change ack

    Some PPM implementation do not like UCSI_ACK_CONNECTOR_CHANGE
    without UCSI_ACK_COMMAND_COMPLETE. Moreover, doing this is racy
    as it requires sending two UCSI_ACK_CC_CI commands in a row and
    the second one will be started with UCSI_CCI_ACK_COMPLETE already
    set in CCI.

    Bundle the UCSI_ACK_CONNECTOR_CHANGE with the UCSI_ACK_COMMAND_COMPLETE
    for the UCSI_GET_CONNECTOR_STATUS command that is sent while
    handling a connector change event.

    Signed-off-by: Christian A. Ehrhardt <lk@c--e.de>
    Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
    Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
    Link: https://lore.kernel.org/r/20240327224554.1772525-3-lk@c--e.de
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

```
Bisected-by: Christian Heusel <christian@heusel.e>

Seems like it was something related to USB C, which is the charging cable type for my laptop.

I would have also liked to test a mainline commit with the questionable commit reverted but I get a build error with the commit reverted on 6.10 and a conflict on 6.11-rc1 :(

```
  CC [M]  drivers/input/rmi4/rmi_f34v7.o
drivers/usb/typec/ucsi/ucsi.c: In function ‘ucsi_exec_command’:
drivers/usb/typec/ucsi/ucsi.c:163:31: error: implicit declaration of function ‘ucsi_acknowledge’; did you mean ‘ucsi_acknowledge_command’? [-Wimplicit-function-declaration]
  163 |                         ret = ucsi_acknowledge(ucsi, false);
      |                               ^~~~~~~~~~~~~~~~
      |                               ucsi_acknowledge_command
  CC [M]  fs/dlm/rcom.o
  CC [M]  drivers/net/ethernet/intel/i40e/i40e_adminq.o
  CC [M]  drivers/net/wireless/ath/ath11k/mhi.o
  CC [M]  drivers/net/ethernet/intel/i40e/i40e_common.o
  LD [M]  drivers/staging/fieldbus/fieldbus_dev.o
  LD [M]  drivers/staging/vt6655/vt6655_stage.o
make[6]: *** [scripts/Makefile.build:244: drivers/usb/typec/ucsi/ucsi.o] Error 1
make[5]: *** [scripts/Makefile.build:485: drivers/usb/typec/ucsi] Error 2
```

Forwarded by mail: https://lore.kernel.org/all/8a541683-36da-4cea-a8a9-db6ee875c86d@leemhuis.info/

Hm, I don't see how this change could trigger the symptoms described. But apparently, it does.

I take it the ucsi related error messages are not present on a working kernel?

For starters and given that this is also an ASUS, can you try to enable the ASUS zenbook quirk in drivers/usb/typec/ucsi/ucsi_acpi.c?

If that doesn't help we will have to dig deeper.

Hey everyone, sorry about the long silence, I was busy with college.

(In reply to Christian A. Ehrhardt from comment #11)
> I take it the ucsi related error messages are not present on a working
> kernel?

No, currently the 6.6.36-1 kernel also has this error:
```
kernel: ucsi_acpi USBC000:00: ucsi_handle_connector_change: GET_CONNECTOR_STATUS failed (-110)
```

> For starters and given that this is also an ASUS, can you try to enable the
> ASUS zenbook quirk in drivers/usb/typec/ucsi/ucsi_acpi.c?
I have no idea what that is, I couldn't find it on google. Can you direct me to it ? Also some resources on how to apply patches because I don't know how to do that :D.

I have some good news though. I got rid of this error:
(In reply to Christian Heusel from comment #9)
> I would have also liked to test a mainline commit with the questionable
> commit reverted but I get a build error with the commit reverted on 6.10 and
> a conflict on 6.11-rc1 :(

I just replaced the ucsi_acknowledge with ucsi_acknowledge_command after reverting the commit. It compiled (version 6.10.5), but I'm scared to boot into it to test it. If someone can reassure me that my laptop won't explode if I boot into it, I'll test it out.

As mentioned though all 6.11 versions had merge conflicts, which I didn't want to attempt to fix.

The change:

```
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index d42fc7578e2d..719065b2da83 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -160,7 +160,7 @@ static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd)

        if (cci & UCSI_CCI_ERROR) {
                if (cmd == UCSI_GET_ERROR_STATUS) {
-                       ret = ucsi_acknowledge_command(*ucsi);
+                       ret = ucsi_acknowledge_command(ucsi);
                        if (ret)
                                return ret;
```

I'm looking into this...

Debugging showed that there are several issues here:

First the quirk for some ASUS zenbooks is required for the VivoBook, too. Otherwise we will read bogus values in CCI.

Doing this reveals that UCSI on the affected laptop does two things that are in violation of the spec:
- It sends busy notifications that contain additional bogus data (e.g. a
  bogus connector change notification or stale command completion bits).
- In response to a GET_CONNECTOR_STATUS command the CCI value always contains
  the connector number (which is interpreted as a new connector change
  event).

The bogus busy notifications result in a command timeout. In response to
this command timeout the EVENT_PENDING bit is cleared in an attempt to recover from the timeout. Most of the time this is ok but in this case the timeout happens on the ACK command and this causes the EVENT_PENDING bit to go out of sync.

As a result ucsi_handle_connector_change() is entered with the EVENT_PENDING bit clear. This again results in an infinite loop:
- ucsi_handle_connector_change is entered with EVENT_PENDING clear.
- ucsi_handle_connector_change() sends a GET_CONNECTOR_STATUS command.
- The response to this command includes the connector number in the lower bits.
- This is interpreted as a new connector change event. Normally, this event
  would be ignored as EVENT_PENDING is still set. However, in this case
  EVENT_PENDING is set and the connector work is scheduled again.
- When we send the ACK command for the GET_CONNECTOR_STATUS command,
  we immediately clear EVENT_PENDING on the grounds that we just handled
  and cleared the connector change event.
- However, the connector change work is still scheduled and everything starts
  all over again.

Apparently, this infinite loop keeps some resource (the embedded controller?) so busy that it fails to handle keystrokes on the builtin keyboard...

I will send a patch that:
- Enables the zenbook quirks for the affected laptops
- ignores the contents of CCI if the busy indicator is set.
- makes sure that EVENT_PENDING is set on entry to
  ucsi_handle_connector_change().

Fix is merged:
| commit 7fa6b25dfb43dafc0e16510e2fcfd63634fc95c2
| Author: Christian A. Ehrhardt <lk@c--e.de>
| Date:   Thu Sep 12 09:41:32 2024 +0200
|
|     usb: typec: ucsi: Fix busy loop on ASUS VivoBooks