mbox series

[RFC,0/6] UCSI backend API refactoring

Message ID 20240218222039.822040-1-lk@c--e.de
Headers
Series UCSI backend API refactoring |

Message

Christian A. Ehrhardt Feb. 18, 2024, 10:20 p.m. UTC 
  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(-)