On Wed, 22. Jan 22:11, Christian A. Ehrhardt wrote:
>
> Hi Fedor,
>
> On Sun, Jan 19, 2025 at 04:23:21PM +0300, Fedor Pchelkin wrote:
> > Christian A. Ehrhardt wrote:
> > > The (compile tested) diff below should fix it and I can turn this
> > > into a proper patch but I lost access to test hardware with UCSI,
> > > thus this would need a "Tested-by:" from someone else before it can
> > > be included. Maybe Saranya can do this?
> > >
> > > Best regards Christian
> > >
> > >
> > > commit b44ba223cd840e6dbab6c7f69da6203c7a8ba570
> > > Author: Christian A. Ehrhardt <lk@xxxxxxx>
> > > Date: Mon Dec 16 21:52:46 2024 +0100
> > >
> > > acpi: typec: ucsi: Introduce a ->poll_cci method
> >
> > WARNING: CPU: 0 PID: 8 at drivers/usb/typec/ucsi/ucsi.c:1377 ucsi_reset_ppm+0x1af/0x1c0 [typec_ucsi]
> > is triggered on my laptop on roughly every system boot. When it's not,
> > there is a
> > ucsi_acpi USBC000:00: error -ETIMEDOUT: PPM init failed
> > message observed in the log.
> >
> > I've tried the above patch "acpi: typec: ucsi: Introduce a ->poll_cci
> > method" but the issue is still triggered [1].
> >
> > Is there any useful info/logs I can provide you for further
> > investigation of the warning in question?
> >
> > As the warning is quite reliably triggered on my system, I may help with
> > the testing of other patches.
>
> Hard to say what might be going on. Some obvious questions to
> narrow it down, though:
> - Is this something new and UCSI worked before or has UCSI been broken
> with older kernels as well (maybe with different or no error
> messages).
Thanks for the feedback and sorry for the delay!
Yep, I've eventually checked this: as it stands, there's always been a
"-ETIMEDOUT: PPM init failed" observed on starting ucsi_init_work during
the boot. Back to v5.12 at least - the oldest kernel I've managed to boot
on this laptop.
On the other hand, the WARNING appears only after the commit fa48d7e81624
("usb: typec: ucsi: Do not call ACPI _DSM method for UCSI read operations").
> - If you get the warning but not the "PPM init failed" message,
> does UCSI actually work? Try to plug something into the USB-C
> ports and watch out for additional error messages (possibly after
> a timeout). Do new files/devices show up in sysfs?
Well, it's interesting. When there is a WARNING and no "PPM init failed"
message, it works because ucsi_init() goes on. When there is a "PPM init
failed", UCSI doesn't actually initialize successfully and it doesn't work.
And I probably didn't pay attention to the "PPM init failed" messages
earlier because I'm not an active UCSI user, utilize Type-C port only for
the power supply (this always works and I guess the not working UCSI
doesn't affect this directly). On the opposite, the big WARNING during the
boot now became more visible :)
It appears like PPM is not ready yet for communication during the boot.
Increasing a timeout just to 2x eliminates the errors in my case:
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index fcf499cc9458..b1a4470214b6 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1362,7 +1362,7 @@ static int ucsi_reset_ppm(struct ucsi *ucsi)
if (ret < 0)
goto out;
- tmo = jiffies + msecs_to_jiffies(UCSI_TIMEOUT_MS);
+ tmo = jiffies + msecs_to_jiffies(UCSI_TIMEOUT_MS * 2);
do {
ret = ucsi->ops->read_cci(ucsi, &cci);
if (ret < 0)
@@ -1382,7 +1382,7 @@ static int ucsi_reset_ppm(struct ucsi *ucsi)
if (ret < 0)
goto out;
- tmo = jiffies + msecs_to_jiffies(UCSI_TIMEOUT_MS);
+ tmo = jiffies + msecs_to_jiffies(UCSI_TIMEOUT_MS * 2);
do {
if (time_is_before_jiffies(tmo)) {
Turning the laptop several times on and off, I'd say the average time
taken for the initial reset takes around 8000ms:
[ 2.568534] ucsi_acpi USBC000:00: enter ucsi_reset_ppm()
[ 10.875710] ucsi_acpi USBC000:00: exit ucsi_reset_ppm(), ret 0
I see that UCSI_TIMEOUT_MS is already chosen to be a rather significant
value, much bigger than what the specs say. Maybe ucsi_init_work races
with something? Could this ever happen here? Or just a firmware/hardware
issue...
> - Printing the value of CCI at various stages of the init process
> might help us to understand what's going on.
During ucsi_reset_ppm() in case of a timeout the reported value of CCI is
always zero and doesn't change on read/poll attempts. In case of the
WARNING it's always read as UCSI_CCI_RESET_COMPLETE thus it WARNs but
ucsi_reset_ppm() returns zero and the further initialization goes on
without any errors.
Is the usage of WARN macros justifiable here if it may potentially be
caused only by the firmware/hardware errors (well, at a quick glance) and
not an issue which can fixed at the kernel level? E.g. the timeout
situation here is not reported by WARN, but by simple printks..
