On Wed, 2021-06-09 at 15:56 +0300, Heikki Krogerus wrote: > On Wed, Jun 09, 2021 at 03:18:04PM +0300, Heikki Krogerus wrote: > > > > I'm trying to get a confirmation on my suspecion that we do always > > actually get an event from the EC firmware, but we just end up > > filtering it out in this case because we are too slow in the driver. > > I > > have an idea what could be done about that, but I need to test if > > that > > really is the case. > > > > I'll prepare a new version out of this entire series. > > Actually, it's easier if you could just test this attached patch on > top of this series. It makes sure the every single event is > considered. I'm sorry about the hassle. No worries! I probably should have included some more information earlier (i.e. enabling the debug print for spurious events). With the patch, I am seeing the following on plug kworker/u16:1-6847 [002] .... 1375.485010: ucsi_connector_change: port1 status: change=4a04, opmode=5, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 kworker/u16:2-6848 [006] .... 1375.561811: ucsi_connector_change: port1 status: change=4000, opmode=5, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 kworker/u16:2-6848 [007] .... 1375.634275: ucsi_connector_change: port1 status: change=4000, opmode=5, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 kworker/u16:2-6848 [003] .... 1375.743161: ucsi_connector_change: port1 status: change=4000, opmode=3, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 and unplug kworker/u16:1-6847 [005] .... 1394.062501: ucsi_connector_change: port1 status: change=4804, opmode=0, connected=0, sourcing=0, partner_flags=0, partner_type=0, request_data_obj=00000000, BC status=0 kworker/u16:1-6847 [005] .... 1394.161612: ucsi_connector_change: port1 status: change=4000, opmode=0, connected=0, sourcing=0, partner_flags=0, partner_type=0, request_data_obj=00000000, BC status=0 kworker/u16:1-6847 [005] .... 1394.251503: ucsi_connector_change: port1 status: change=4000, opmode=0, connected=0, sourcing=0, partner_flags=0, partner_type=0, request_data_obj=00000000, BC status=0 where all but the first event are spurious events. I believe that in the above spurious event with the change to opmode=3, the PPM should be reporting change=0004 (i.e. UCSI_CONSTAT_POWER_OPMODE_CHANGE). Occasionally I also see the following on plug. Note the non-spurious event with change=0040 (UCSI_CONSTAT_POWER_LEVEL_CHANGE) right before the event where opmode changes. kworker/u16:11-2201 [001] .... 3240.124431: ucsi_connector_change: port1 status: change=4a04, opmode=5, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 kworker/u16:3-7469 [003] .... 3240.222799: ucsi_connector_change: port1 status: change=4000, opmode=5, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 kworker/u16:3-7469 [003] .... 3240.325946: ucsi_connector_change: port1 status: change=0040, opmode=5, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 kworker/u16:3-7469 [003] .... 3240.423503: ucsi_connector_change: port1 status: change=4000, opmode=3, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 kworker/u16:3-7469 [003] .... 3240.861986: ucsi_connector_change: port1 status: change=4000, opmode=3, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 kworker/u16:3-7469 [007] .... 3240.999048: ucsi_connector_change: port1 status: change=4000, opmode=3, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 My thought when I first ran into the issue was that the PPM simply resets the change bitfield on ACK, effectively discarding any changes that happened after the last GET_CONNECTOR_STATUS call. I believed to have confirmed this by inserting an msleep in between. However, I have to admit that I have never really looked for alternative explanations. Benjamin
Attachment:
signature.asc
Description: This is a digitally signed message part
