Linux-USB Archive mirror
 help / color / mirror / Atom feed
From: Christian Ehrhardt <lk@c--e.de>
To: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
Cc: heikki.krogerus@linux.intel.com, gregkh@linuxfoundation.org,
	neil.armstrong@linaro.org, quic_prashk@quicinc.com,
	dmitry.baryshkov@linaro.org, fabrice.gasnier@foss.st.com,
	saranya.gopal@intel.com, linux-usb@vger.kernel.org
Subject: Re: [RFC PATCH] usb: typec: ucsi: ack connector change after all tasks finish
Date: Thu, 28 Mar 2024 23:40:09 +0100	[thread overview]
Message-ID: <ZgXxyWsdA7YML3mR@cae.in-ulm.de> (raw)
In-Reply-To: <ynrqweb7hhfkrlvjr6suajq3jpgi4sqexz44qt4chekce7phiw@cyofo73xztdg>


Hi,

On Thu, Mar 28, 2024 at 03:42:40PM +0000, Diogo Ivo wrote:
> On Wed, Mar 27, 2024 at 05:06:38PM +0100, Christian Ehrhardt wrote:
> > On Wed, Mar 27, 2024 at 12:39:04PM +0000, Diogo Ivo wrote:
> > > This fixes a problem with some LG Gram laptops where the PPM sometimes
> > > notifies the OPM with the Connector Change Indicator field set in the
> > > CCI after an ACK_CC_CI command is sent,causing the UCSI stack to check
> > > the connector status indefinitely since the EVENT_PENDING bit is already
> > > cleared. This causes an interrupt storm and an artificial high load on
> > > these platforms.
> > 
> > If the PPM does this for a connector change ACK_CC_CI command it is
> > IMHO violating the spec (unless there is a _new_ event).
> 
> Yes, the problem is exactly that the PPM in these laptops is really not
> conformant with the spec and moving the command change ACK_CC_CI to the
> end circumvented the problems in the PPM. If [1] is the way to go then
> we need some sort of quirk for these devices and I'll have to dig
> deeper.

Just to make this clear: This is not my call to make.

> > When I saw this type of loops the connector change indicator was set
> > in response to an ACK_CC_CI command for a command (sent by a different
> > thread for a different connector) between clearing the EVENT_PENDING
> > bit and acquiring the PPM lock.
> > 
> > Can you test if the changes that already are in usb-linus are
> > sufficient to fix your issues?
> 
> I am seeing these problems when addressing one connector only, so other
> threads for other connectors do not play a role here. I have tested the
> latest usb-linus with and without your early ack patch set [1] on top
> and the issue is still not fixed.

There are legitimate reaons why the connector change indicator
is set in response to a command:
- If the condition was reported previously it is sticky until
  cleared.
- Something else changed on the connector.

For a more complicated device that I have here, there are five
different connector change events after plugging it in.

I'd like to understand why you run into a loop here.
Printing the completed command (if any) and the CCI in
ucsi_acpi_notify() and the details of the connector status in
ucsi_handle_connector_change() could shed some light on this.


> [1]: https://lore.kernel.org/linux-usb/20240327224554.1772525-1-lk@c--e.de/

Best regards   Christian

  reply	other threads:[~2024-03-28 22:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-27 12:39 [RFC PATCH] usb: typec: ucsi: ack connector change after all tasks finish Diogo Ivo
2024-03-27 13:06 ` Dmitry Baryshkov
2024-03-27 14:37   ` Diogo Ivo
2024-03-27 16:06 ` Christian Ehrhardt
2024-03-28 15:42   ` Diogo Ivo
2024-03-28 22:40     ` Christian Ehrhardt [this message]
2024-04-24 11:23       ` Diogo Ivo
2024-04-25 18:05         ` Christian Ehrhardt
2024-03-27 16:42 ` Christian Ehrhardt
2024-03-28 16:04   ` Diogo Ivo

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=ZgXxyWsdA7YML3mR@cae.in-ulm.de \
    --to=lk@c--e.de \
    --cc=diogo.ivo@tecnico.ulisboa.pt \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=fabrice.gasnier@foss.st.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=quic_prashk@quicinc.com \
    --cc=saranya.gopal@intel.com \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).