From: Benjamin Berg <bberg@redhat.com>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 0/7] usb: typec: ucsi: Polling the alt modes and PDOs
Date: Wed, 09 Jun 2021 19:39:41 +0200 [thread overview]
Message-ID: <0bf4d8fc6d64ac553a319b8c5af49a3d7705842d.camel@redhat.com> (raw)
In-Reply-To: <YMC6fgoWiAe1C3uZ@kuha.fi.intel.com>
[-- Attachment #1: Type: text/plain, Size: 4370 bytes --]
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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2021-06-09 17:39 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-07 13:14 [RFC PATCH 0/7] usb: typec: ucsi: Polling the alt modes and PDOs Heikki Krogerus
2021-06-07 13:14 ` [RFC PATCH 1/7] usb: typec: ucsi: Always cancel the command if PPM reports BUSY condition Heikki Krogerus
2021-06-07 13:14 ` [RFC PATCH 2/7] usb: typec: ucsi: Don't stop alt mode registration on busy condition Heikki Krogerus
2021-06-08 9:31 ` Sergei Shtylyov
2021-06-08 13:18 ` Heikki Krogerus
2021-06-07 13:14 ` [RFC PATCH 3/7] usb: typec: ucsi: Add poll worker for alternate modes Heikki Krogerus
2021-06-07 13:14 ` [RFC PATCH 4/7] usb: typec: ucsi: acpi: Reduce the command completion timeout Heikki Krogerus
2021-06-07 13:14 ` [RFC PATCH 5/7] usb: typec: ucsi: Process every connector change as unique connector state Heikki Krogerus
2021-06-07 13:14 ` [RFC PATCH 6/7] usb: typec: ucsi: Filter out spurious events Heikki Krogerus
2021-06-07 13:14 ` [RFC PATCH 7/7] usb: typec: ucsi: Read the PDOs in separate work Heikki Krogerus
2021-06-07 20:09 ` [RFC PATCH 0/7] usb: typec: ucsi: Polling the alt modes and PDOs Benjamin Berg
2021-06-08 6:42 ` Heikki Krogerus
2021-06-08 6:54 ` Heikki Krogerus
2021-06-08 19:32 ` Benjamin Berg
2021-06-09 11:25 ` Heikki Krogerus
2021-06-09 12:18 ` Heikki Krogerus
2021-06-09 12:56 ` Heikki Krogerus
2021-06-09 17:39 ` Benjamin Berg [this message]
2021-06-10 12:07 ` Heikki Krogerus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0bf4d8fc6d64ac553a319b8c5af49a3d7705842d.camel@redhat.com \
--to=bberg@redhat.com \
--cc=heikki.krogerus@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).