[RFC PATCH 0/6] UCSI backend API refactoring

From: Christian A. Ehrhardt
Date: Sun Feb 18 2024 - 17:21:22 EST


The (new) API for UCSI backends has some problems:

The UCSI spec assumes a mailbox style communication:
1. The OPM fills data (UCSI_MESSAGE_OUT and UCSI_CONTROL)
into the mailbox memory region. In the ACPI case these are
simple host memory accesses.
2. The OPM notifies the PPM that a command should be executed.
In the ACPI case this is a call the _DSM ACPI function.
Within that function the UCSI_CONTROL and the UCSI_MESSAGE_OUT
data is transferred to something like the embedded controller
and then the PPM is notified e.g. by an SMI.
3. The PPM executes the command. As a result UCSI_MESSAGE_IN and
UCSI_CCI in the mailbox memory are updated and the OPM
is notified. In the ACPI case the notification is an SCI and
ACPI provides a handler that syncs UCSI_MESSAGE_IN and
UCSI_CCI from the EC into host memory before notifying the
OPM via the ACPI Notify() command.

The major problem is that steps 1 (update mailbox memory) and
step 2 (notify PPM and start command execution) are two different
steps. The current API only knows about a write function that
combines writing to mailbox memory and starting command execution.

This only works as long as the commands do not have arguments
that must be in UCSI_MESSAGE_OUT.

Additionally, Step 3 (at least in the ACPI case) above suggests
that UCSI_CCI and UCSI_MESSAGE_IN should generally be read from
mailbox memory. Except for some special cases the sync operation
via the _DSM-Read function in ACPI is not required and seems to
do more harm then good.

E.g. the zenbook quirks could be avoided if we didn't re-read the
UCSI_CCI and UCSI_MESSAGE_IN data from the embedded controller
on each read operation. The recent fix to the CCG backend seems
to address the same issue.

This change series proposes a new API with the following design
goals:
- Retain the nice features of the previous API redesign. In
particular the synchronous execution of commands in the
backend and the ability to handle quirks there.
- The API must still be able to handle all backends (obviously).
Thus each new API function is currently implemented in all
backends in the same change.
- Separate writing to mailbox data and starting command execution
to allow for commands with UCSI_MESSAGE_OUT data.
- Avoid hardware accesses (as opposed to mailbox memory accesses)
outside of the command execution context as much as possible.
- Avoid use of explicit mailbox offsets (that are subject to
change with later UCSI revisions) in the UCSI core.

The proposed new API features:
- read_data() and write_data() for access to the message data
fields. This is supposed to be a pure mailbox access that
even in the case of a write does not start a command.
- sync_cmd() and async_cmd() to write the command register and
start command execution.
- A cached version of the current contents of CCI in the core
UCSI structure. This value is only updated if a notification
is received from the hardware.
- poll_cci() to force an update of the cached CCI value from
hardware. This is required to poll for PPM reset completion.

CAVEATs:
- The series will certainly need more polishing etc. before
it can be accepted. I will do this provided that the series
is considered a good idea in general.
- The series compiles, passes smatch, sparse and checkpatch but
I only have ucsi_acpi hardware to test this. The other backends
(ccg, glink, stm32g0) will need testers and likely some fixes
as well.

So, what do you think?

Christian A. Ehrhardt (6):
usb: ucsi_glink: Fix endianness issues
ucsi_ccg: Cleanup endianness confusion
usb: typec: ucsi: Make Version a parameter to ucsi_register
usb: typec: ucsi: Cache current CCI value in struct ucsi
usb: typec: ucsi: Introdcue ->read_data and ->write_data
usb: typec: ucsi: Convert a?sync_write to a?sync_cmd

drivers/usb/typec/ucsi/ucsi.c | 53 ++++--------
drivers/usb/typec/ucsi/ucsi.h | 28 +++---
drivers/usb/typec/ucsi/ucsi_acpi.c | 119 ++++++++++++--------------
drivers/usb/typec/ucsi/ucsi_ccg.c | 107 ++++++++++++-----------
drivers/usb/typec/ucsi/ucsi_glink.c | 104 +++++++++++++++++-----
drivers/usb/typec/ucsi/ucsi_stm32g0.c | 80 ++++++++++++++---
6 files changed, 290 insertions(+), 201 deletions(-)

--
2.40.1