From: Benjamin Berg <[email protected]>
Hi all,
so, I kept running in an issue where the UCSI port information was saying
that power was being delivered (online: 1), while no cable was attached.
The core of the problem is that there are scenarios where UCSI change
notifications are lost. This happens because querying the changes that
happened is done using the GET_CONNECTOR_STATUS command while clearing the
bitfield happens from the separate ACK command. Any change in between will
be lost.
Note that the problem may be almost invisible in the UI as e.g. GNOME will
still show the battery as discharging. But some policies like automatic
suspend may be applied incorrectly.
Cc: Hans de Goede <[email protected]>
Cc: Heikki Krogerus <[email protected]>
Benjamin Berg (2):
usb: typec: ucsi: acpi: Always decode connector change information
usb: typec: ucsi: Work around PPM losing change information
drivers/usb/typec/ucsi/ucsi.c | 125 ++++++++++++++++++++++++-----
drivers/usb/typec/ucsi/ucsi.h | 2 +
drivers/usb/typec/ucsi/ucsi_acpi.c | 5 +-
3 files changed, 110 insertions(+), 22 deletions(-)
--
2.26.2
On Fri, Oct 09, 2020 at 04:40:45PM +0200, Benjamin Berg wrote:
> From: Benjamin Berg <[email protected]>
>
> Hi all,
>
> so, I kept running in an issue where the UCSI port information was saying
> that power was being delivered (online: 1), while no cable was attached.
>
> The core of the problem is that there are scenarios where UCSI change
> notifications are lost. This happens because querying the changes that
> happened is done using the GET_CONNECTOR_STATUS command while clearing the
> bitfield happens from the separate ACK command. Any change in between will
> be lost.
>
> Note that the problem may be almost invisible in the UI as e.g. GNOME will
> still show the battery as discharging. But some policies like automatic
> suspend may be applied incorrectly.
>
> Cc: Hans de Goede <[email protected]>
> Cc: Heikki Krogerus <[email protected]>
Both patches:
Acked-by: Heikki Krogerus <[email protected]>
> Benjamin Berg (2):
> usb: typec: ucsi: acpi: Always decode connector change information
> usb: typec: ucsi: Work around PPM losing change information
>
> drivers/usb/typec/ucsi/ucsi.c | 125 ++++++++++++++++++++++++-----
> drivers/usb/typec/ucsi/ucsi.h | 2 +
> drivers/usb/typec/ucsi/ucsi_acpi.c | 5 +-
> 3 files changed, 110 insertions(+), 22 deletions(-)
thanks,
--
heikki
On Fri, Oct 09, 2020 at 04:40:45PM +0200, Benjamin Berg wrote:
> From: Benjamin Berg <[email protected]>
>
> Hi all,
>
> so, I kept running in an issue where the UCSI port information was saying
> that power was being delivered (online: 1), while no cable was attached.
>
> The core of the problem is that there are scenarios where UCSI change
> notifications are lost. This happens because querying the changes that
> happened is done using the GET_CONNECTOR_STATUS command while clearing the
> bitfield happens from the separate ACK command. Any change in between will
> be lost.
>
> Note that the problem may be almost invisible in the UI as e.g. GNOME will
> still show the battery as discharging. But some policies like automatic
> suspend may be applied incorrectly.
>
> Cc: Hans de Goede <[email protected]>
> Cc: Heikki Krogerus <[email protected]>
>
> Benjamin Berg (2):
> usb: typec: ucsi: acpi: Always decode connector change information
> usb: typec: ucsi: Work around PPM losing change information
Do these need to be backported to stable kernel releases? If so, how
far back?
thjanks,
greg k-h
On Wed, Oct 28, 2020 at 10:10:43AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Oct 09, 2020 at 04:40:45PM +0200, Benjamin Berg wrote:
> > From: Benjamin Berg <[email protected]>
> >
> > Hi all,
> >
> > so, I kept running in an issue where the UCSI port information was saying
> > that power was being delivered (online: 1), while no cable was attached.
> >
> > The core of the problem is that there are scenarios where UCSI change
> > notifications are lost. This happens because querying the changes that
> > happened is done using the GET_CONNECTOR_STATUS command while clearing the
> > bitfield happens from the separate ACK command. Any change in between will
> > be lost.
> >
> > Note that the problem may be almost invisible in the UI as e.g. GNOME will
> > still show the battery as discharging. But some policies like automatic
> > suspend may be applied incorrectly.
> >
> > Cc: Hans de Goede <[email protected]>
> > Cc: Heikki Krogerus <[email protected]>
> >
> > Benjamin Berg (2):
> > usb: typec: ucsi: acpi: Always decode connector change information
> > usb: typec: ucsi: Work around PPM losing change information
>
> Do these need to be backported to stable kernel releases? If so, how
> far back?
Due to the lack of response, I guess they don't need to go to any stable
kernel, so will queue them up for 5.11-rc1.
thanks,
greg k-h
Hi,
On Fri, 2020-11-06 at 11:47 +0100, Greg Kroah-Hartman wrote:
> Due to the lack of response, I guess they don't need to go to any
> stable kernel, so will queue them up for 5.11-rc1.
Sorry, forgot to reply.
Not including them in stable seems reasonable as I have not seen it
cause major trouble in the wild so far (i.e. only one user report that
seems related).
Benjamin
Hi Greg,
On Fri, Nov 06, 2020 at 11:47:25AM +0100, Greg Kroah-Hartman wrote:
> On Wed, Oct 28, 2020 at 10:10:43AM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Oct 09, 2020 at 04:40:45PM +0200, Benjamin Berg wrote:
> > > From: Benjamin Berg <[email protected]>
> > >
> > > Hi all,
> > >
> > > so, I kept running in an issue where the UCSI port information was saying
> > > that power was being delivered (online: 1), while no cable was attached.
> > >
> > > The core of the problem is that there are scenarios where UCSI change
> > > notifications are lost. This happens because querying the changes that
> > > happened is done using the GET_CONNECTOR_STATUS command while clearing the
> > > bitfield happens from the separate ACK command. Any change in between will
> > > be lost.
> > >
> > > Note that the problem may be almost invisible in the UI as e.g. GNOME will
> > > still show the battery as discharging. But some policies like automatic
> > > suspend may be applied incorrectly.
> > >
> > > Cc: Hans de Goede <[email protected]>
> > > Cc: Heikki Krogerus <[email protected]>
> > >
> > > Benjamin Berg (2):
> > > usb: typec: ucsi: acpi: Always decode connector change information
> > > usb: typec: ucsi: Work around PPM losing change information
> >
> > Do these need to be backported to stable kernel releases? If so, how
> > far back?
>
> Due to the lack of response, I guess they don't need to go to any stable
> kernel, so will queue them up for 5.11-rc1.
At least one user in Debian (https://bugs.debian.org/992004) would be
happy to have those backported as well to the 5.10.y series (which we
will pick up).
So if Benjamin ack's this, this would be great to have in 5.10.y.
Regards,
Salvatore
Hi,
On Fri, 2021-08-20 at 15:01 +0200, Salvatore Bonaccorso wrote:
> Hi Greg,
>
> On Fri, Nov 06, 2020 at 11:47:25AM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Oct 28, 2020 at 10:10:43AM +0100, Greg Kroah-Hartman wrote:
> > > On Fri, Oct 09, 2020 at 04:40:45PM +0200, Benjamin Berg wrote:
> > > > From: Benjamin Berg <[email protected]>
> > > >
> > > > Hi all,
> > > >
> > > > so, I kept running in an issue where the UCSI port information was saying
> > > > that power was being delivered (online: 1), while no cable was attached.
> > > >
> > > > The core of the problem is that there are scenarios where UCSI change
> > > > notifications are lost. This happens because querying the changes that
> > > > happened is done using the GET_CONNECTOR_STATUS command while clearing the
> > > > bitfield happens from the separate ACK command. Any change in between will
> > > > be lost.
> > > >
> > > > Note that the problem may be almost invisible in the UI as e.g. GNOME will
> > > > still show the battery as discharging. But some policies like automatic
> > > > suspend may be applied incorrectly.
> > > >
> > > > Cc: Hans de Goede <[email protected]>
> > > > Cc: Heikki Krogerus <[email protected]>
> > > >
> > > > Benjamin Berg (2):
> > > > usb: typec: ucsi: acpi: Always decode connector change information
> > > > usb: typec: ucsi: Work around PPM losing change information
> > >
> > > Do these need to be backported to stable kernel releases? If so, how
> > > far back?
> >
> > Due to the lack of response, I guess they don't need to go to any stable
> > kernel, so will queue them up for 5.11-rc1.
>
> At least one user in Debian (https://bugs.debian.org/992004) would be
> happy to have those backported as well to the 5.10.y series (which we
> will pick up).
>
> So if Benjamin ack's this, this would be great to have in 5.10.y.
Sure, it is reasonable to pull it into 5.10. At the time it just seemed
to me that it was enough of a corner case to not bother.
Note that there was a somewhat related fix later on (for Qualcomm UCSI
firmware), which probably makes sense to pull in too then.
Including Bjorn into the CC list for that.
commit 8c9b3caab3ac26db1da00b8117901640c55a69dd
Author: Bjorn Andersson <[email protected]>
Date: Sat May 15 21:09:53 2021 -0700
usb: typec: ucsi: Clear pending after acking connector change
Benjamin
On 8/20/21 9:29 AM, Benjamin Berg wrote:
> On Fri, 2021-08-20 at 15:01 +0200, Salvatore Bonaccorso wrote:
>> At least one user in Debian (https://bugs.debian.org/992004) would be
>> happy to have those backported as well to the 5.10.y series (which we
>> will pick up).
>>
>> So if Benjamin ack's this, this would be great to have in 5.10.y.
> Sure, it is reasonable to pull it into 5.10. At the time it just seemed
> to me that it was enough of a corner case to not bother.
>
> Note that there was a somewhat related fix later on (for Qualcomm UCSI
> firmware), which probably makes sense to pull in too then.
>
> Including Bjorn into the CC list for that.
>
> commit 8c9b3caab3ac26db1da00b8117901640c55a69dd
> Author: Bjorn Andersson <[email protected]>
> Date: Sat May 15 21:09:53 2021 -0700
>
> usb: typec: ucsi: Clear pending after acking connector change
>
> Benjamin
I feel that I should mention that I haven't actually tested this change,
so it's just conjecture on my part that it would fix my issue (though it
does seem to track pretty closely). I am happy to do that testing if it
would save others time.
Ian Turner
Hi Ian,
On Fri, Aug 20, 2021 at 07:08:57PM -0400, Ian Turner wrote:
> On 8/20/21 9:29 AM, Benjamin Berg wrote:
> > On Fri, 2021-08-20 at 15:01 +0200, Salvatore Bonaccorso wrote:
> > > At least one user in Debian (https://bugs.debian.org/992004) would be
> > > happy to have those backported as well to the 5.10.y series (which we
> > > will pick up).
> > >
> > > So if Benjamin ack's this, this would be great to have in 5.10.y.
> > Sure, it is reasonable to pull it into 5.10. At the time it just seemed
> > to me that it was enough of a corner case to not bother.
> >
> > Note that there was a somewhat related fix later on (for Qualcomm UCSI
> > firmware), which probably makes sense to pull in too then.
> >
> > Including Bjorn into the CC list for that.
> >
> > commit 8c9b3caab3ac26db1da00b8117901640c55a69dd
> > Author: Bjorn Andersson <[email protected]>
> > Date: Sat May 15 21:09:53 2021 -0700
> >
> > usb: typec: ucsi: Clear pending after acking connector change
> > Benjamin
>
> I feel that I should mention that I haven't actually tested this change, so
> it's just conjecture on my part that it would fix my issue (though it does
> seem to track pretty closely). I am happy to do that testing if it would
> save others time.
Ah apologies, I was under the impression that you confirmed that this
was already the right fix. It is pretty close to what you describe, so
if you can additionally confirm the fix, that would be great.
Regards,
Salvatore
On Fri, Aug 20, 2021 at 03:01:53PM +0200, Salvatore Bonaccorso wrote:
> Hi Greg,
>
> On Fri, Nov 06, 2020 at 11:47:25AM +0100, Greg Kroah-Hartman wrote:
Note, you are responding to an email from a very long time ago...
> > Due to the lack of response, I guess they don't need to go to any stable
> > kernel, so will queue them up for 5.11-rc1.
>
> At least one user in Debian (https://bugs.debian.org/992004) would be
> happy to have those backported as well to the 5.10.y series (which we
> will pick up).
>
> So if Benjamin ack's this, this would be great to have in 5.10.y.
What are the git commit ids? Just ask for them to be applied to stable
like normal...
thanks,
greg k-h
Hi Greg,
On Sat, Aug 21, 2021 at 02:09:45PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Aug 20, 2021 at 03:01:53PM +0200, Salvatore Bonaccorso wrote:
> > Hi Greg,
> >
> > On Fri, Nov 06, 2020 at 11:47:25AM +0100, Greg Kroah-Hartman wrote:
>
> Note, you are responding to an email from a very long time ago...
Yeah, was sort of purpose :) (to try to retain the original context of
the question of if the commits should be backported to stable series,
which back then had no need raised)
>
> > > Due to the lack of response, I guess they don't need to go to any stable
> > > kernel, so will queue them up for 5.11-rc1.
> >
> > At least one user in Debian (https://bugs.debian.org/992004) would be
> > happy to have those backported as well to the 5.10.y series (which we
> > will pick up).
> >
> > So if Benjamin ack's this, this would be great to have in 5.10.y.
>
> What are the git commit ids? Just ask for them to be applied to stable
> like normal...
Right, aplogies. The two commits were
47ea2929d58c35598e681212311d35b240c373ce and
217504a055325fe76ec1142aa15f14d3db77f94f.
47ea2929d58c ("usb: typec: ucsi: acpi: Always decode connector change information")
217504a05532 ("usb: typec: ucsi: Work around PPM losing change information")
and in the followup Benjamin Berg mentioned to pick as well
8c9b3caab3ac26db1da00b8117901640c55a69dd
8c9b3caab3ac ("usb: typec: ucsi: Clear pending after acking connector change"
a related fix later on.
Regards,
Salvatore
On 8/21/21 2:31 AM, Salvatore Bonaccorso wrote:
> Ah apologies, I was under the impression that you confirmed that this
> was already the right fix. It is pretty close to what you describe, so
> if you can additionally confirm the fix, that would be great.
>
> Regards,
> Salvatore
Well, it would seem that I owe everyone here an apology.
Not only did I not observe this issue with a patched kernel, but I'm
also now unable to reproduce it booting into a stock kernel. I can only
speculate about the reasons for this.
I'll come back here if I learn more. Sorry about the noise.
Ian Turner
On Sat, Aug 21, 2021 at 03:01:26PM +0200, Salvatore Bonaccorso wrote:
> Hi Greg,
>
> On Sat, Aug 21, 2021 at 02:09:45PM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Aug 20, 2021 at 03:01:53PM +0200, Salvatore Bonaccorso wrote:
> > > Hi Greg,
> > >
> > > On Fri, Nov 06, 2020 at 11:47:25AM +0100, Greg Kroah-Hartman wrote:
> >
> > Note, you are responding to an email from a very long time ago...
>
> Yeah, was sort of purpose :) (to try to retain the original context of
> the question of if the commits should be backported to stable series,
> which back then had no need raised)
> >
> > > > Due to the lack of response, I guess they don't need to go to any stable
> > > > kernel, so will queue them up for 5.11-rc1.
> > >
> > > At least one user in Debian (https://bugs.debian.org/992004) would be
> > > happy to have those backported as well to the 5.10.y series (which we
> > > will pick up).
> > >
> > > So if Benjamin ack's this, this would be great to have in 5.10.y.
> >
> > What are the git commit ids? Just ask for them to be applied to stable
> > like normal...
>
> Right, aplogies. The two commits were
> 47ea2929d58c35598e681212311d35b240c373ce and
> 217504a055325fe76ec1142aa15f14d3db77f94f.
>
> 47ea2929d58c ("usb: typec: ucsi: acpi: Always decode connector change information")
> 217504a05532 ("usb: typec: ucsi: Work around PPM losing change information")
>
> and in the followup Benjamin Berg mentioned to pick as well
>
> 8c9b3caab3ac26db1da00b8117901640c55a69dd
>
> 8c9b3caab3ac ("usb: typec: ucsi: Clear pending after acking connector change"
>
> a related fix later on.
All now queued up, thanks.
greg k-h