On Wed, 2021-05-19 at 14:33 +0300, Heikki Krogerus wrote:

On Tue, May 18, 2021 at 08:04:14PM +0200, Benjamin Berg wrote:

quoted

On Tue, 2021-05-18 at 16:29 +0300, Heikki Krogerus wrote:

quoted

On Mon, May 17, 2021 at 02:57:28PM +0200, Benjamin Berg wrote:

quoted

[SNIP]
Unfortunately, I don't feel it'll work. The problem that I was
seeing
looked like a race condition in the PPM itself, where the
window is
the
time between the UCSI_GET_CONNECTOR_STATUS command and the
subsequent
ACK.
For such a firmware level bug in the PPM, we need a way to
detect
the
race condition when it happens (or get a fix for the firmware).

OK. Let me know does the patch bring the issue back for you.

So, I just tried the patch, and I can occasionally reproduce the
issue
where "online" for the ucsi power adapter is stuck at "1" after
unplugging with the patch applied.

Thanks for testing it.

I'm still not sure that the PPM is the culprit here. I have a feeling
that the problem you are seeing is caused by the workaround (bad
workaround) that we have for the issue where the EC firmware does not
return with the BUSY bit set in the CCI when it should in many cases.
The UCSI ACPI driver has one minute timeout value for command
completion because of that, which is way too long. So if the EC
firmware decides to take its time before acknowledging command, the
driver is stuck, and we start loosing the events...

Hmm, interesting, yes. If the PPM is sending a second change event
before or after we ACK'ed the first one, and we loose this event, then
that would absolutely explain the issue I am seeing.

In my case, I was able to considerably increase the probability of the
bug by inserting a sleep just before acknowledging the connector
change. Not sure whether this is meaningful, but maybe that tells us
something about who is at fault here.

Benjamin

Well, I guess
technically the PPM would be the culprit in the end in any case, but
I'm just not sure that there is any race like you suspected.

But this is off topic. I'll send you an RFC proposal what I think we
could do about that.


thanks,