Re: [PATCH RFC 1/2] acpi: typec: ucsi: Introduce a ->poll_cci method

[Christian A. Ehrhardt]

Hi,

On Fri, Feb 07, 2025 at 02:38:03AM +0200, Dmitry Baryshkov wrote:
> On Thu, 6 Feb 2025 at 20:43, Fedor Pchelkin <boddah8794@xxxxxxxxx> wrote:
> >
> > From: "Christian A. Ehrhardt" <lk@xxxxxxx>
> >
> > For the ACPI backend of UCSI the UCSI "registers" are just a memory copy
> > of the register values in an opregion. The ACPI implementation in the
> > BIOS ensures that the opregion contents are synced to the embedded
> > controller and it ensures that the registers (in particular CCI) are
> > synced back to the opregion on notifications. While there is an ACPI call
> > that syncs the actual registers to the opregion there is rarely a need to
> > do this and on some ACPI implementations it actually breaks in various
> > interesting ways.
> >
> > The only reason to force a sync from the embedded controller is to poll
> > CCI while notifications are disabled. Only the ucsi core knows if this
> > is the case and guessing based on the current command is suboptimal, i.e.
> > leading to the following spurious assertion splat:
> >
> > WARNING: CPU: 3 PID: 76 at drivers/usb/typec/ucsi/ucsi.c:1388 ucsi_reset_ppm+0x1b4/0x1c0 [typec_ucsi]
> > CPU: 3 UID: 0 PID: 76 Comm: kworker/3:0 Not tainted 6.12.11-200.fc41.x86_64 #1
> > Hardware name: LENOVO 21D0/LNVNB161216, BIOS J6CN45WW 03/17/2023
> > Workqueue: events_long ucsi_init_work [typec_ucsi]
> > RIP: 0010:ucsi_reset_ppm+0x1b4/0x1c0 [typec_ucsi]
> > Call Trace:
> >  <TASK>
> >  ucsi_init_work+0x3c/0xac0 [typec_ucsi]
> >  process_one_work+0x179/0x330
> >  worker_thread+0x252/0x390
> >  kthread+0xd2/0x100
> >  ret_from_fork+0x34/0x50
> >  ret_from_fork_asm+0x1a/0x30
> >  </TASK>
> >
> > Thus introduce a ->poll_cci() method that works like ->read_cci() with an
> > additional forced sync and document that this should be used when polling
> > with notifications disabled. For all other backends that presumably don't
> > have this issue use the same implementation for both methods.
> 
> Should the ucsi_init() also use ->poll_cci instead of ->read_cci?
> Although it's executed with notifications enabled, it looks as if it
> might need the additional resync.

I don't think it should be neccessary. The command completion event
for the ucsi_send_command just above should have synced already and
anything that happens after that ought to generate an event.

Best regards,
Christian