| Message ID | 20240313-qcom-ucsi-fixes-v1-0-74d90cb48a00@linaro.org |
|---|---|
| Headers | show |
| Series | usb: typec: ucsi: fix several issues manifesting on Qualcomm platforms | expand |
On Wed, Mar 13, 2024 at 05:54:10AM +0200, Dmitry Baryshkov wrote: > Fix several issues discovered while debugging UCSI implementation on > Qualcomm platforms (ucsi_glink). With these patches I was able to > get a working Type-C port managament implementation. Tested on SC8280XP > (Lenovo X13s laptop) and SM8350-HDK. > Dmitry Baryshkov (7): > usb: typec: ucsi: fix race condition in connection change ACK'ing > usb: typec: ucsi: acknowledge the UCSI_CCI_NOT_SUPPORTED > usb: typec: ucsi: make ACK_CC_CI rules more obvious > usb: typec: ucsi: allow non-partner GET_PDOS for Qualcomm devices > usb: typec: ucsi: limit the UCSI_NO_PARTNER_PDOS even further > usb: typec: ucsi: properly register partner's PD device > soc: qcom: pmic_glink: reenable UCSI on sc8280xp I just gave this series a quick spin on my X13s and it seems there are still some issues that needs to be resolved before merging at least the final patch in this series: [ 7.786167] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 [ 7.786445] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) [ 7.883493] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 [ 7.883614] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) [ 7.905194] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 [ 7.905295] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) [ 7.913340] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 [ 7.913409] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) I see these errors on boot both with and without my charger and ethernet device connected. I'm afraid I won't have to time to help debug this myself at least for another week. Johan
On Fri, 22 Mar 2024 at 14:16, Johan Hovold <johan@kernel.org> wrote: > > On Wed, Mar 13, 2024 at 05:54:10AM +0200, Dmitry Baryshkov wrote: > > Fix several issues discovered while debugging UCSI implementation on > > Qualcomm platforms (ucsi_glink). With these patches I was able to > > get a working Type-C port managament implementation. Tested on SC8280XP > > (Lenovo X13s laptop) and SM8350-HDK. > > > Dmitry Baryshkov (7): > > usb: typec: ucsi: fix race condition in connection change ACK'ing > > usb: typec: ucsi: acknowledge the UCSI_CCI_NOT_SUPPORTED > > usb: typec: ucsi: make ACK_CC_CI rules more obvious > > usb: typec: ucsi: allow non-partner GET_PDOS for Qualcomm devices > > usb: typec: ucsi: limit the UCSI_NO_PARTNER_PDOS even further > > usb: typec: ucsi: properly register partner's PD device > > > soc: qcom: pmic_glink: reenable UCSI on sc8280xp > > I just gave this series a quick spin on my X13s and it seems there are > still some issues that needs to be resolved before merging at least the > final patch in this series: > > [ 7.786167] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 > [ 7.786445] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) > [ 7.883493] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 > [ 7.883614] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) > [ 7.905194] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 > [ 7.905295] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) > [ 7.913340] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 > [ 7.913409] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) > > I see these errors on boot both with and without my charger and ethernet > device connected. Just to doublecheck: do you have latest adsp installed? Do you have your bootloaders updated? If you back up the patch #5 ("limit the UCSI_NO_PARTNER_PDOS even further"), does it still break for you? > > I'm afraid I won't have to time to help debug this myself at least for > another week. > > Johan -- With best wishes Dmitry
On Fri, Mar 22, 2024 at 03:39:36PM +0200, Dmitry Baryshkov wrote: > On Fri, 22 Mar 2024 at 14:16, Johan Hovold <johan@kernel.org> wrote: > > On Wed, Mar 13, 2024 at 05:54:10AM +0200, Dmitry Baryshkov wrote: > > > Dmitry Baryshkov (7): > > > usb: typec: ucsi: fix race condition in connection change ACK'ing > > > usb: typec: ucsi: acknowledge the UCSI_CCI_NOT_SUPPORTED > > > usb: typec: ucsi: make ACK_CC_CI rules more obvious > > > usb: typec: ucsi: allow non-partner GET_PDOS for Qualcomm devices > > > usb: typec: ucsi: limit the UCSI_NO_PARTNER_PDOS even further > > > usb: typec: ucsi: properly register partner's PD device > > > > > soc: qcom: pmic_glink: reenable UCSI on sc8280xp > > > > I just gave this series a quick spin on my X13s and it seems there are > > still some issues that needs to be resolved before merging at least the > > final patch in this series: > > > > [ 7.786167] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 > > [ 7.786445] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) > > I see these errors on boot both with and without my charger and ethernet > > device connected. > > Just to doublecheck: do you have latest adsp installed? Yes, there has only been one adsp fw released to linux-firmware and that's the one I'm using. If this depends on anything newer that should have been mentioned in the commit message and we obviously can't enable UCSI until that firmware has been released. > Do you have your bootloaders updated? Yes. > If you back up the patch #5 ("limit the UCSI_NO_PARTNER_PDOS even > further"), does it still break for you? I don't understand what you mean by "back up the patch #5". Revert (drop) patch 5? The errors are still there with patch 5 reverted. Johan
On Tue, 26 Mar 2024 at 12:22, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Tue, 26 Mar 2024 at 10:41, Johan Hovold <johan@kernel.org> wrote: > > > > On Mon, Mar 25, 2024 at 10:56:21PM +0200, Dmitry Baryshkov wrote: > > > On Fri, 22 Mar 2024 at 14:16, Johan Hovold <johan@kernel.org> wrote: > > > > > > I just gave this series a quick spin on my X13s and it seems there are > > > > still some issues that needs to be resolved before merging at least the > > > > final patch in this series: > > > > > > > > [ 7.786167] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 > > > > [ 7.786445] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) > > > > [ 7.883493] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 > > > > [ 7.883614] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) > > > > [ 7.905194] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 > > > > [ 7.905295] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) > > > > [ 7.913340] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0 > > > > [ 7.913409] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: UCSI_GET_PDOS failed (-5) > > > > > > I have traced what is causing these messages. During UCSI startup the > > > ucsi_register_port() function queries for PDOs associated with the > > > on-board USB-C port. This is allowed by the spec. Qualcomm firmware > > > detects that there is no PD-device connected and instead of returning > > > corresponding set of PDOs returns Eerror Indicator set to 1b but then > > > it returns zero error status in response to GET_ERROR_STATUS, causing > > > "unknown error 0" code. I have checked the PPM, it doesn't even have > > > the code to set the error status properly in this case (not to mention > > > that asking for device's PDOs should not be an error, unless the > > > command is inappropriate for the target. > > > > > > Thus said, I think the driver is behaving correctly. Granted that > > > these messages are harmless, we can ignore them for now. I'll later > > > check if we can update PD information for the device's ports when PD > > > device is attached. I have verified that once the PD device is > > > attached, corresponding GET_PDOS command returns correct set of PD > > > objects. Ccurrently the driver registers usb_power_delivery devices, > > > but with neither source nor sink set of capabilities. > > > > > > An alternative option is to drop patches 4 and 5, keeping > > > 'NO_PARTNER_PDOS' quirk equivalent to 'don't send GET_PDOS at all'. > > > However I'd like to abstain from this option, since it doesn't allow > > > us to check PD capabilities of the attached device. > > > > > > Heikki, Johan, WDYT? > > > > Whatever you do, you need to suppress those errors above before enabling > > anything more on sc8280xp (e.g. even if it means adding a quirk to the > > driver). > > Why? The errors are legitimate. Just to expand my answer. We have the 'driver should be quiet by default' policy. Which usually means that there should be no 'foo registered' or 'foo bound' messages if everything goes smooth. We do not have 'don't warn about manufacturer issues' or 'don't barf if firmware returns an error' kind of requirements. We can be nicer and skip something if we know that it may return an error. But we don't have to. -- With best wishes Dmitry
Fix several issues discovered while debugging UCSI implementation on Qualcomm platforms (ucsi_glink). With these patches I was able to get a working Type-C port managament implementation. Tested on SC8280XP (Lenovo X13s laptop) and SM8350-HDK. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Dmitry Baryshkov (7): usb: typec: ucsi: fix race condition in connection change ACK'ing usb: typec: ucsi: acknowledge the UCSI_CCI_NOT_SUPPORTED usb: typec: ucsi: make ACK_CC_CI rules more obvious usb: typec: ucsi: allow non-partner GET_PDOS for Qualcomm devices usb: typec: ucsi: limit the UCSI_NO_PARTNER_PDOS even further usb: typec: ucsi: properly register partner's PD device soc: qcom: pmic_glink: reenable UCSI on sc8280xp drivers/soc/qcom/pmic_glink.c | 1 - drivers/usb/typec/ucsi/ucsi.c | 51 +++++++++++++++++++++++++++++++++---------- 2 files changed, 39 insertions(+), 13 deletions(-) --- base-commit: ea86f16f605361af3779af0e0b4be18614282091 change-id: 20240312-qcom-ucsi-fixes-6578d236b60b Best regards,