linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Christian A. Ehrhardt" <lk@c--e.de>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	linux-usb@vger.kernel.org
Cc: "Christian A. Ehrhardt" <lk@c--e.de>,
	Dell.Client.Kernel@dell.com,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Hans de Goede" <hdegoede@redhat.com>,
	"Jack Pham" <quic_jackp@quicinc.com>,
	"Fabrice Gasnier" <fabrice.gasnier@foss.st.com>,
	"Samuel Čavoj" <samuel@cavoj.net>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 0/4] Make UCSI on Dell Latitude work
Date: Sun,  7 Jan 2024 01:16:57 +0100	[thread overview]
Message-ID: <20240107001701.130535-1-lk@c--e.de> (raw)

The UCSI interface on a Dell Latitude 5431 stops working after
the first async event with:

    GET_CONNECTOR_STATUS failed (-110)

The core problem is that when sending the ACK_CC_CI command to
clear the connector status changed condition the PPM expects us
to send anothr ack for the command completion condition. However,
the UCSI spec states that no ack for the command completion is
required when the command is ACK_CC_CI (or PPM_RESET).

There are various reports that suggest that several Dell laptops
are affected by this problem. E.g. the kernel bugzilla has this
report which is most likely an instance of this bug:

    https://bugzilla.kernel.org/show_bug.cgi?id=216426

This led me to the somewhat bold conclusion that the quirk should
probably be applied to all Dell systems. However, I'm open to
suggestions on how to proceed with this.

While debugging another issue was found that is present for all systems
but only triggered with the quirk active: The ACK_CC_CI command
to ack the connector status change is sent without any lock which
allows for a race where this command is sent while another command
is currently in progress.

The series consists of 4 changes:
- Add the missing lock around ucsi_acknowledge_connector_change.
  This change is a generic bugfix.
- Add infrastructure for quirks and a quirk override module parameter
  to the typec_ucsi module. There's at least one other change in the
  works that wants something similar here:
    https://lore.kernel.org/all/20231023215327.695720-2-dmitry.baryshkov@linaro.org/
  Additionally, doing the dell quirk is a bit easier in the gereric
  UCSI code. The module parameter will allow users to fix things if
  the DMI matching goes wrong. Additionally, we can easily ask users
  to test different quirks without the need to recompile.
- Actually add the required quirk to ucsi.c.
- Finally, refactor DMI matching in ucsi_acpi.c and enable the new
  quirk for all DELL systems. The latter is kind of bold and I'm open
  to suggestings on how to proceed here.
  I'm CC'ing Dell.Client.Kernel@dell.com because this seems to be a
  list where Dell people are that might be able to provide more
  insight on this.

Christian A. Ehrhardt (4):
  usb: ucsi: Add quirk infrastructure
  usb: ucsi: Add missing ppm_lock
  usb: ucsi: Quirk to ack a connector change ack cmd
  usb: ucsi: Apply UCSI_ACK_CONNECTOR_CHANGE_ACK_CMD to DELL systems

 .../admin-guide/kernel-parameters.txt         |  5 +++
 drivers/usb/typec/ucsi/ucsi.c                 | 21 ++++++++-
 drivers/usb/typec/ucsi/ucsi.h                 |  7 ++-
 drivers/usb/typec/ucsi/ucsi_acpi.c            | 45 ++++++++++++++++---
 drivers/usb/typec/ucsi/ucsi_ccg.c             |  2 +-
 drivers/usb/typec/ucsi/ucsi_glink.c           |  2 +-
 drivers/usb/typec/ucsi/ucsi_stm32g0.c         |  2 +-
 7 files changed, 73 insertions(+), 11 deletions(-)

-- 
2.40.1


             reply	other threads:[~2024-01-07  0:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-07  0:16 Christian A. Ehrhardt [this message]
2024-01-07  0:16 ` [PATCH 1/4] usb: ucsi: Add missing ppm_lock Christian A. Ehrhardt
2024-01-07  0:16 ` [PATCH 2/4] usb: ucsi: Add quirk infrastructure Christian A. Ehrhardt
2024-01-15  7:53   ` Heikki Krogerus
2024-01-07  0:17 ` [PATCH 3/4] usb: ucsi: Quirk to ack a connector change ack cmd Christian A. Ehrhardt
2024-01-07  0:17 ` [PATCH 4/4] usb: ucsi: Apply UCSI_ACK_CONNECTOR_CHANGE_ACK_CMD to Dell systems Christian A. Ehrhardt
2024-01-15  9:05   ` Heikki Krogerus
2024-01-15 11:57     ` Christian A. Ehrhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240107001701.130535-1-lk@c--e.de \
    --to=lk@c--e.de \
    --cc=Dell.Client.Kernel@dell.com \
    --cc=fabrice.gasnier@foss.st.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=quic_jackp@quicinc.com \
    --cc=samuel@cavoj.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).