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

[Dmitry Baryshkov]

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.

>
> Fixes: fa48d7e81624 ("usb: typec: ucsi: Do not call ACPI _DSM method for UCSI read operations")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Christian A. Ehrhardt <lk@xxxxxxx>
> Tested-by: Fedor Pchelkin <boddah8794@xxxxxxxxx>
> Signed-off-by: Fedor Pchelkin <boddah8794@xxxxxxxxx>
> ---



-- 
With best wishes
Dmitry