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
