Message ID | 20250206184327.16308-3-boddah8794@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | usb: typec: ucsi: avoid problems during initial PPM reset | expand |
On Thu, Feb 06, 2025 at 09:43:15PM +0300, Fedor Pchelkin wrote: > It is observed that on some systems an initial PPM reset during the boot > phase can trigger a timeout: > > [ 6.482546] ucsi_acpi USBC000:00: failed to reset PPM! > [ 6.482551] ucsi_acpi USBC000:00: error -ETIMEDOUT: PPM init failed > > Still, increasing the timeout value, albeit being the most straightforward > solution, eliminates the problem: the initial PPM reset may take up to > ~8000-10000ms on some Lenovo laptops. When it is reset after the above > period of time (or even if ucsi_reset_ppm() is not called overall), UCSI > works as expected. > > Moreover, if the ucsi_acpi module is loaded/unloaded manually after the > system has booted, reading the CCI values and resetting the PPM works > perfectly, without any timeout. Thus it's only a boot-time issue. > > The reason for this behavior is not clear but it may be the consequence > of some tricks that the firmware performs or be an actual firmware bug. > As a workaround, increase the timeout to avoid failing the UCSI > initialization prematurely. > > Fixes: b1b59e16075f ("usb: typec: ucsi: Increase command completion timeout value") > Cc: stable@vger.kernel.org > Signed-off-by: Fedor Pchelkin <boddah8794@gmail.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/ucsi/ucsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index 0fe1476f4c29..7a56d3f840d7 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -25,7 +25,7 @@ > * difficult to estimate the time it takes for the system to process the command > * before it is actually passed to the PPM. > */ > -#define UCSI_TIMEOUT_MS 5000 > +#define UCSI_TIMEOUT_MS 10000 > > /* > * UCSI_SWAP_TIMEOUT_MS - Timeout for role swap requests > -- > 2.48.1
On Thu, 13. Feb 15:58, Heikki Krogerus wrote: > On Thu, Feb 06, 2025 at 09:43:15PM +0300, Fedor Pchelkin wrote: > > It is observed that on some systems an initial PPM reset during the boot > > phase can trigger a timeout: > > > > [ 6.482546] ucsi_acpi USBC000:00: failed to reset PPM! > > [ 6.482551] ucsi_acpi USBC000:00: error -ETIMEDOUT: PPM init failed > > > > Still, increasing the timeout value, albeit being the most straightforward > > solution, eliminates the problem: the initial PPM reset may take up to > > ~8000-10000ms on some Lenovo laptops. When it is reset after the above > > period of time (or even if ucsi_reset_ppm() is not called overall), UCSI > > works as expected. > > > > Moreover, if the ucsi_acpi module is loaded/unloaded manually after the > > system has booted, reading the CCI values and resetting the PPM works > > perfectly, without any timeout. Thus it's only a boot-time issue. > > > > The reason for this behavior is not clear but it may be the consequence > > of some tricks that the firmware performs or be an actual firmware bug. > > As a workaround, increase the timeout to avoid failing the UCSI > > initialization prematurely. > > > > Fixes: b1b59e16075f ("usb: typec: ucsi: Increase command completion timeout value") > > Cc: stable@vger.kernel.org > > Signed-off-by: Fedor Pchelkin <boddah8794@gmail.com> > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Thanks for review! Should I respin the series or it can be taken as is despite being initially tagged an RFC material?
On Mon, Feb 17, 2025 at 01:18:25PM +0300, Fedor Pchelkin wrote: > On Thu, 13. Feb 15:58, Heikki Krogerus wrote: > > On Thu, Feb 06, 2025 at 09:43:15PM +0300, Fedor Pchelkin wrote: > > > It is observed that on some systems an initial PPM reset during the boot > > > phase can trigger a timeout: > > > > > > [ 6.482546] ucsi_acpi USBC000:00: failed to reset PPM! > > > [ 6.482551] ucsi_acpi USBC000:00: error -ETIMEDOUT: PPM init failed > > > > > > Still, increasing the timeout value, albeit being the most straightforward > > > solution, eliminates the problem: the initial PPM reset may take up to > > > ~8000-10000ms on some Lenovo laptops. When it is reset after the above > > > period of time (or even if ucsi_reset_ppm() is not called overall), UCSI > > > works as expected. > > > > > > Moreover, if the ucsi_acpi module is loaded/unloaded manually after the > > > system has booted, reading the CCI values and resetting the PPM works > > > perfectly, without any timeout. Thus it's only a boot-time issue. > > > > > > The reason for this behavior is not clear but it may be the consequence > > > of some tricks that the firmware performs or be an actual firmware bug. > > > As a workaround, increase the timeout to avoid failing the UCSI > > > initialization prematurely. > > > > > > Fixes: b1b59e16075f ("usb: typec: ucsi: Increase command completion timeout value") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Fedor Pchelkin <boddah8794@gmail.com> > > > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Thanks for review! > > Should I respin the series or it can be taken as is despite being > initially tagged an RFC material? For obvious reasons, I can't take RFC patches as obviously you didn't think they were worthy of being taken, hence you marking them that way :) thanks, greg k-h
Hi Fedor, On Thu, Feb 6, 2025, at 1:43 PM, Fedor Pchelkin wrote: > It is observed that on some systems an initial PPM reset during the boot > phase can trigger a timeout: > > [ 6.482546] ucsi_acpi USBC000:00: failed to reset PPM! > [ 6.482551] ucsi_acpi USBC000:00: error -ETIMEDOUT: PPM init failed > > Still, increasing the timeout value, albeit being the most straightforward > solution, eliminates the problem: the initial PPM reset may take up to > ~8000-10000ms on some Lenovo laptops. When it is reset after the above > period of time (or even if ucsi_reset_ppm() is not called overall), UCSI > works as expected. > > Moreover, if the ucsi_acpi module is loaded/unloaded manually after the > system has booted, reading the CCI values and resetting the PPM works > perfectly, without any timeout. Thus it's only a boot-time issue. > > The reason for this behavior is not clear but it may be the consequence > of some tricks that the firmware performs or be an actual firmware bug. > As a workaround, increase the timeout to avoid failing the UCSI > initialization prematurely. > Could you let me know which Lenovo platform(s) you see the issue on? I don't have any concerns with the patch below, but if the platform is in the Linux program I can reach out to the FW team and try to determine if there's an expected time needed (and how close we are to it). Thanks Mark > Fixes: b1b59e16075f ("usb: typec: ucsi: Increase command completion > timeout value") > Cc: stable@vger.kernel.org > Signed-off-by: Fedor Pchelkin <boddah8794@gmail.com> > --- > drivers/usb/typec/ucsi/ucsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c > b/drivers/usb/typec/ucsi/ucsi.c > index 0fe1476f4c29..7a56d3f840d7 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -25,7 +25,7 @@ > * difficult to estimate the time it takes for the system to process > the command > * before it is actually passed to the PPM. > */ > -#define UCSI_TIMEOUT_MS 5000 > +#define UCSI_TIMEOUT_MS 10000 > > /* > * UCSI_SWAP_TIMEOUT_MS - Timeout for role swap requests > -- > 2.48.1
On Tue, 18. Feb 11:57, Mark Pearson wrote: > Hi Fedor, > > On Thu, Feb 6, 2025, at 1:43 PM, Fedor Pchelkin wrote: > > It is observed that on some systems an initial PPM reset during the boot > > phase can trigger a timeout: > > > > [ 6.482546] ucsi_acpi USBC000:00: failed to reset PPM! > > [ 6.482551] ucsi_acpi USBC000:00: error -ETIMEDOUT: PPM init failed > > > > Still, increasing the timeout value, albeit being the most straightforward > > solution, eliminates the problem: the initial PPM reset may take up to > > ~8000-10000ms on some Lenovo laptops. When it is reset after the above > > period of time (or even if ucsi_reset_ppm() is not called overall), UCSI > > works as expected. > > > > Moreover, if the ucsi_acpi module is loaded/unloaded manually after the > > system has booted, reading the CCI values and resetting the PPM works > > perfectly, without any timeout. Thus it's only a boot-time issue. > > > > The reason for this behavior is not clear but it may be the consequence > > of some tricks that the firmware performs or be an actual firmware bug. > > As a workaround, increase the timeout to avoid failing the UCSI > > initialization prematurely. > > > > Could you let me know which Lenovo platform(s) you see the issue on? > > I don't have any concerns with the patch below, but if the platform is in > the Linux program I can reach out to the FW team and try to determine if > there's an expected time needed (and how close we are to it). Yep, here. (also at the bottom of the [PATCH RFC 0/2] mail) Please let me know if there's more info needed. Machine: Type: Laptop System: LENOVO product: 21D0 v: ThinkBook 14 G4+ ARA serial: <superuser required> Mobo: LENOVO model: LNVNB161216 v: SDK0T76479 WIN serial: <superuser required> UEFI: LENOVO v: J6CN50WW date: 09/27/2024 Relevant part of the ACPI dump. Scope (\_SB) { Device (UBTC) { Name (_HID, EisaId ("USBC000")) // _HID: Hardware ID Name (_CID, EisaId ("PNP0CA0")) // _CID: Compatible ID Name (_UID, Zero) // _UID: Unique ID Name (_DDN, "USB Type C") // _DDN: DOS Device Name Name (_ADR, Zero) // _ADR: Address Name (_DEP, Package (0x01) // _DEP: Dependencies { \_SB.PCI0.LPC0.EC0 }) Method (_STA, 0, NotSerialized) // _STA: Status { Return (0x0F) } Method (_PS0, 0, Serialized) // _PS0: Power State 0 { Sleep (0x07D0) } Method (_PS3, 0, Serialized) // _PS3: Power State 3 { Sleep (0x07D0) } Method (TPLD, 2, Serialized) { Name (PCKG, Package (0x01) { Buffer (0x10){} }) CreateField (DerefOf (PCKG [Zero]), Zero, 0x07, REV) REV = One CreateField (DerefOf (PCKG [Zero]), 0x40, One, VISI) VISI = Arg0 CreateField (DerefOf (PCKG [Zero]), 0x57, 0x08, GPOS) GPOS = Arg1 CreateField (DerefOf (PCKG [Zero]), 0x4A, 0x04, SHAP) SHAP = One CreateField (DerefOf (PCKG [Zero]), 0x20, 0x10, WID) WID = 0x08 CreateField (DerefOf (PCKG [Zero]), 0x30, 0x10, HGT) HGT = 0x03 Return (PCKG) /* \_SB_.UBTC.TPLD.PCKG */ } Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x7AF66000, // Address Base 0x00001000, // Address Length ) }) Return (RBUF) /* \_SB_.UBTC._CRS.RBUF */ } Device (HSP1) { Name (_ADR, One) // _ADR: Address Name (_UPC, Package (0x04) // _UPC: USB Port Capabilities { 0xFF, 0x09, Zero, Zero }) Method (_PLD, 0, NotSerialized) // _PLD: Physical Location of Device { Return (TPLD (One, 0x04)) } } Device (HSP2) { Name (_ADR, One) // _ADR: Address Name (_UPC, Package (0x04) // _UPC: USB Port Capabilities { 0xFF, 0x09, Zero, Zero }) Method (_PLD, 0, NotSerialized) // _PLD: Physical Location of Device { Return (TPLD (One, 0x04)) } } OperationRegion (USBC, SystemMemory, 0x7AF66000, 0x30) Field (USBC, ByteAcc, Lock, Preserve) { VER1, 8, VER2, 8, RSV1, 8, RSV2, 8, CCI0, 8, CCI1, 8, CCI2, 8, CCI3, 8, CTL0, 8, CTL1, 8, CTL2, 8, CTL3, 8, CTL4, 8, CTL5, 8, CTL6, 8, CTL7, 8, MGI0, 8, MGI1, 8, MGI2, 8, MGI3, 8, MGI4, 8, MGI5, 8, MGI6, 8, MGI7, 8, MGI8, 8, MGI9, 8, MGIA, 8, MGIB, 8, MGIC, 8, MGID, 8, MGIE, 8, MGIF, 8, MGO0, 8, MGO1, 8, MGO2, 8, MGO3, 8, MGO4, 8, MGO5, 8, MGO6, 8, MGO7, 8, MGO8, 8, MGO9, 8, MGOA, 8, MGOB, 8, MGOC, 8, MGOD, 8, MGOE, 8, MGOF, 8 } OperationRegion (DBG0, SystemIO, 0x80, One) Field (DBG0, ByteAcc, NoLock, Preserve) { IO80, 8 } Method (NTFY, 0, Serialized) { ECRD () Sleep (One) Notify (\_SB.UBTC, 0x80) // Status Change } Method (ECWR, 0, Serialized) { IO80 = 0xA0 \_SB.PCI0.LPC0.EC0.ECWT (MGO0, RefOf (\_SB.PCI0.LPC0.EC0.MGO0)) \_SB.PCI0.LPC0.EC0.ECWT (MGO1, RefOf (\_SB.PCI0.LPC0.EC0.MGO1)) \_SB.PCI0.LPC0.EC0.ECWT (MGO2, RefOf (\_SB.PCI0.LPC0.EC0.MGO2)) \_SB.PCI0.LPC0.EC0.ECWT (MGO3, RefOf (\_SB.PCI0.LPC0.EC0.MGO3)) \_SB.PCI0.LPC0.EC0.ECWT (MGO4, RefOf (\_SB.PCI0.LPC0.EC0.MGO4)) \_SB.PCI0.LPC0.EC0.ECWT (MGO5, RefOf (\_SB.PCI0.LPC0.EC0.MGO5)) \_SB.PCI0.LPC0.EC0.ECWT (MGO6, RefOf (\_SB.PCI0.LPC0.EC0.MGO6)) \_SB.PCI0.LPC0.EC0.ECWT (MGO7, RefOf (\_SB.PCI0.LPC0.EC0.MGO7)) \_SB.PCI0.LPC0.EC0.ECWT (MGO8, RefOf (\_SB.PCI0.LPC0.EC0.MGO8)) \_SB.PCI0.LPC0.EC0.ECWT (MGO9, RefOf (\_SB.PCI0.LPC0.EC0.MGO9)) \_SB.PCI0.LPC0.EC0.ECWT (MGOA, RefOf (\_SB.PCI0.LPC0.EC0.MGOA)) \_SB.PCI0.LPC0.EC0.ECWT (MGOB, RefOf (\_SB.PCI0.LPC0.EC0.MGOB)) \_SB.PCI0.LPC0.EC0.ECWT (MGOC, RefOf (\_SB.PCI0.LPC0.EC0.MGOC)) \_SB.PCI0.LPC0.EC0.ECWT (MGOD, RefOf (\_SB.PCI0.LPC0.EC0.MGOD)) \_SB.PCI0.LPC0.EC0.ECWT (MGOE, RefOf (\_SB.PCI0.LPC0.EC0.MGOE)) \_SB.PCI0.LPC0.EC0.ECWT (MGOF, RefOf (\_SB.PCI0.LPC0.EC0.MGOF)) \_SB.PCI0.LPC0.EC0.ECWT (CTL0, RefOf (\_SB.PCI0.LPC0.EC0.CTL0)) \_SB.PCI0.LPC0.EC0.ECWT (CTL1, RefOf (\_SB.PCI0.LPC0.EC0.CTL1)) \_SB.PCI0.LPC0.EC0.ECWT (CTL2, RefOf (\_SB.PCI0.LPC0.EC0.CTL2)) \_SB.PCI0.LPC0.EC0.ECWT (CTL3, RefOf (\_SB.PCI0.LPC0.EC0.CTL3)) \_SB.PCI0.LPC0.EC0.ECWT (CTL4, RefOf (\_SB.PCI0.LPC0.EC0.CTL4)) \_SB.PCI0.LPC0.EC0.ECWT (CTL5, RefOf (\_SB.PCI0.LPC0.EC0.CTL5)) \_SB.PCI0.LPC0.EC0.ECWT (CTL6, RefOf (\_SB.PCI0.LPC0.EC0.CTL6)) \_SB.PCI0.LPC0.EC0.ECWT (CTL7, RefOf (\_SB.PCI0.LPC0.EC0.CTL7)) \_SB.PCI0.LPC0.EC0.ECWT (0xE0, RefOf (\_SB.PCI0.LPC0.EC0.USDC)) IO80 = 0xA1 } Method (ECRD, 0, Serialized) { IO80 = 0xA3 MGI0 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI0)) MGI1 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI1)) MGI2 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI2)) MGI3 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI3)) MGI4 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI4)) MGI5 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI5)) MGI6 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI6)) MGI7 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI7)) MGI8 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI8)) MGI9 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGI9)) MGIA = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGIA)) MGIB = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGIB)) MGIC = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGIC)) MGID = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGID)) MGIE = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGIE)) MGIF = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.MGIF)) VER1 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.VER1)) VER2 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.VER2)) RSV1 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.RSV1)) RSV2 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.RSV2)) CCI0 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.CCI0)) CCI1 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.CCI1)) CCI2 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.CCI2)) CCI3 = \_SB.PCI0.LPC0.EC0.ECRD (RefOf (\_SB.PCI0.LPC0.EC0.CCI3)) \_SB.PCI0.LPC0.EC0.ECWT (0xE1, RefOf (\_SB.PCI0.LPC0.EC0.USGC)) IO80 = 0xA4 } Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method { If ((Arg0 == ToUUID ("6f8398c2-7ca4-11e4-ad36-631042b5008f") /* Unknown UUID */)) { If ((ToInteger (Arg2) == Zero)) { Return (Buffer (One) { 0x0F // . }) } ElseIf ((ToInteger (Arg2) == One)) { ECWR () } ElseIf ((ToInteger (Arg2) == 0x02)) { ECRD () } Else { Return (Zero) } } Return (Zero) } } }
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index 0fe1476f4c29..7a56d3f840d7 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -25,7 +25,7 @@ * difficult to estimate the time it takes for the system to process the command * before it is actually passed to the PPM. */ -#define UCSI_TIMEOUT_MS 5000 +#define UCSI_TIMEOUT_MS 10000 /* * UCSI_SWAP_TIMEOUT_MS - Timeout for role swap requests
It is observed that on some systems an initial PPM reset during the boot phase can trigger a timeout: [ 6.482546] ucsi_acpi USBC000:00: failed to reset PPM! [ 6.482551] ucsi_acpi USBC000:00: error -ETIMEDOUT: PPM init failed Still, increasing the timeout value, albeit being the most straightforward solution, eliminates the problem: the initial PPM reset may take up to ~8000-10000ms on some Lenovo laptops. When it is reset after the above period of time (or even if ucsi_reset_ppm() is not called overall), UCSI works as expected. Moreover, if the ucsi_acpi module is loaded/unloaded manually after the system has booted, reading the CCI values and resetting the PPM works perfectly, without any timeout. Thus it's only a boot-time issue. The reason for this behavior is not clear but it may be the consequence of some tricks that the firmware performs or be an actual firmware bug. As a workaround, increase the timeout to avoid failing the UCSI initialization prematurely. Fixes: b1b59e16075f ("usb: typec: ucsi: Increase command completion timeout value") Cc: stable@vger.kernel.org Signed-off-by: Fedor Pchelkin <boddah8794@gmail.com> --- drivers/usb/typec/ucsi/ucsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)