Re: [PATCH 0/7] usb: typec: ucsi: fix several issues manifesting on Qualcomm platforms

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 22 Mar 2024 at 14:16, Johan Hovold <johan@xxxxxxxxxx> 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 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?

For reference, here is a trace of relevant messages exchanged over the
GLINK interface during UCSI bootstrap:

[   11.030838] write: 00000000: 10 00 01 00 07 00 00 00

GET_PDOS(connection 1, Source, 3 PDOs)

[   11.044171] write ack: 0
[   11.044263] notify: 00000000: 00 00 00 c0 00 00 00 00

Command Complete, Error

[   11.044458] read: 00000000: 00 01 00 00 00 00 00 c0 00 00 00 00 00 00 00 00
[   11.044460] read: 00000010: e7 3f 00 00 00 00 00 00 02 00 20 01 00 03 30 01
[   11.044462] read: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   11.059790] read: 00000000: 00 01 00 00 00 00 00 c0 00 00 00 00 00 00 00 00
[   11.059797] read: 00000010: e7 3f 00 00 00 00 00 00 02 00 20 01 00 03 30 01
[   11.059801] read: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   11.059814] write: 00000000: 04 00 02 00 00 00 00 00

Ack_CC command

[   11.075509] write ack: 0
[   11.075544] notify: 00000000: 00 00 00 20 00 00 00 00

Ack for Ack_CC

[   11.091828] read: 00000000: 00 01 00 00 00 00 00 20 00 00 00 00 00 00 00 00
[   11.091864] read: 00000010: e7 3f 00 00 00 00 00 00 02 00 20 01 00 03 30 01
[   11.091879] read: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   11.092339] write: 00000000: 13 00 00 00 00 00 00 00

GET_ERROR_STATUS

[   11.106398] write ack: 0
[   11.106435] notify: 00000000: 00 10 00 80 00 00 00 00

command complete, 0x10 bytes of response

[   11.122729] read: 00000000: 00 01 00 00 00 10 00 80 00 00 00 00 00 00 00 00
[   11.122758] read: 00000010: 00 00 00 00 00 00 00 00 02 00 20 01 00 03 30 01
[   11.122770] read: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Zero response data


[   11.137523] read: 00000000: 00 01 00 00 00 10 00 80 00 00 00 00 00 00 00 00
[   11.137548] read: 00000010: 00 00 00 00 00 00 00 00 02 00 20 01 00 03 30 01
[   11.137559] read: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   11.153028] read: 00000000: 00 01 00 00 00 10 00 80 00 00 00 00 00 00 00 00
[   11.153064] read: 00000010: 00 00 00 00 00 00 00 00 02 00 20 01 00 03 30 01
[   11.153080] read: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   11.153492] write: 00000000: 04 00 02 00 00 00 00 00

Ack_CC for the GET_ERROR_STATUS command

[   11.169060] write ack: 0
[   11.169108] notify: 00000000: 00 00 00 20 00 00 00 00

Ack for ACK_CC

[   11.184114] read: 00000000: 00 01 00 00 00 00 00 20 00 00 00 00 00 00 00 00
[   11.184140] read: 00000010: 00 00 00 00 00 00 00 00 02 00 20 01 00 03 30 01
[   11.184152] read: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   11.184548] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: unknown error 0
[   11.184695] ------------[ cut here ]------------
[   11.184703] WARNING: CPU: 2 PID: 28 at
drivers/usb/typec/ucsi/ucsi.c:140 ucsi_exec_command+0x284/0x328
[typec_ucsi]
[   11.185488] Call trace:
[   11.185494]  ucsi_exec_command+0x284/0x328 [typec_ucsi]
[   11.185519]  ucsi_send_command+0x54/0x118 [typec_ucsi]
[   11.185543]  ucsi_read_pdos+0x5c/0xdc [typec_ucsi]
[   11.185567]  ucsi_get_pdos+0x30/0xa4 [typec_ucsi]
[   11.185590]  ucsi_init_work+0x3bc/0x82c [typec_ucsi]
[   11.185614]  process_one_work+0x148/0x2a0
[   11.185638]  worker_thread+0x2fc/0x40c
[   11.185655]  kthread+0x110/0x114
[   11.185668]  ret_from_fork+0x10/0x20

Then comes the same log for the Connector=1, Sink PDOs, Connector=2
Source PDOs and finally Connector=2 Sink PDOs.


-- 
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux