[RESEND] USB PD broken on Lenovo P15gen2

[RESEND] USB PD broken on Lenovo P15gen2

[Nikolay Borisov]

[Resending as I had initially attached  a full acpi dump and it got 
bounced from the usb mailing list]

Hello,

I'm not able to use usb PD on a Lenovo Thinkpad P15gen2 laptop. It's 
equipped with 2 thunderbolt ports and a usb 3.2 gen2 usb port, all of 
which are supposed to support PD 2.0:

cat /sys/class/typec/port*/usb_power_delivery_revision
2.0
2.0
2.0


Whenever I connect a pd-capable device (i.e a mobile phone or a tablet) 
with a known good cable i.e i'm able to achieve fast charging (~30w) on 
the device when using the same cable but connected to an external 
monitor and not the laptop, I'm not able to get fast charge to work. In 
dmesg I get the following:

[ 1262.933106] usb 3-12: new high-speed USB device number 8 using xhci_hcd
[ 1263.083343] usb 3-12: New USB device found, idVendor=22d9, 
idProduct=2046, bcdDevice= 2.23
[ 1263.083350] usb 3-12: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3
[ 1263.083353] usb 3-12: Product: IV2201
[ 1263.083355] usb 3-12: Manufacturer: OnePlus
[ 1263.083356] usb 3-12: SerialNumber: xxxxxxx
[ 1263.116184] ucsi_acpi USBC000:00: UCSI_GET_PDOS failed (-95)
[ 1268.676590] ucsi_acpi USBC000:00: UCSI_GET_PDOS failed (-95)
[ 1273.816117] ucsi_acpi USBC000:00: UCSI_GET_PDOS failed (-95)
[ 1273.904187] ucsi_acpi USBC000:00: UCSI_GET_PDOS failed (-95)

When looking at the state of the partner device:
$ cat /sys/class/typec/port2-partner/supports_usb_power_delivery
yes

$ cat /sys/class/typec/port2-partner/usb_power_delivery_revision
0.0


After some debugging it seems this is because the lenovo is not 
supporting CCI which seems to be some sort of a communication mechanism. 
This was asserted with the following bpftrace script:

bpftrace -e 'kretprobe:ucsi_acpi_read { printf("ucsi_acpi_read returned: 
%d\n", retval); } kretprobe:ucsi_exec_command { 
printf("ucsi_exec_command returned: %d\n", retval); }'

Attaching 2 probes...
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_exec_command returned: 9
ucsi_acpi_read returned: 0
ucsi_exec_command returned: 9
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_exec_command returned: 9
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_exec_command returned: -95
ucsi_exec_command returned: -110
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_exec_command returned: -95
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_exec_command returned: 0
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_exec_command returned: -95
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_exec_command returned: -95
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0
ucsi_exec_command returned: 0
ucsi_acpi_read returned: 0
ucsi_acpi_read returned: 0

This would correspond to the following snippet in ucsi_exec_command:

if (cci & UCSI_CCI_NOT_SUPPORTED)
              return -EOPNOTSUPP;


I can provide a full acpidump if need be. And I'm also using the latest 
available firmware from Lenovo, installed via lvfs. This issue happens 
on kernel 6.2 based ubuntu kernel as well as on upstream 6.4.3 but it 
seems it's not dependent on the kernel version as it's somehow related 
to lenovo's firmware.

I'm wondering whether a similar quirk like the one for the zenbook is 
also required for lenovo ?

Re: [RESEND] USB PD broken on Lenovo P15gen2

[Nikolay Borisov]

On 28.08.23 г. 17:52 ч., Nikolay Borisov wrote:
> 
> [Resending as I had initially attached  a full acpi dump and it got 
> bounced from the usb mailing list]
> 
> Hello,
> 
> I'm not able to use usb PD on a Lenovo Thinkpad P15gen2 laptop. It's 
> equipped with 2 thunderbolt ports and a usb 3.2 gen2 usb port, all of 
> which are supposed to support PD 2.0:

<snip>
So I've been debugging this and what the PPM reports is the following:

modprobe-529501  [004] ..... 33507.058332: ucsi_register: Supported UCSI spec: 100
      kworker/4:0-524223  [004] ..... 33507.486591: ucsi_init_work: Connectors supported: 3
      kworker/4:0-524223  [004] ..... 33507.486592: ucsi_init_work: GET_CAP: USB_PD: 0 TYPEC_CURRENT: 1 POWER_VBUS: 0, POWER_OTHER: 0, POWER_AC_SUPPLY: 1, BATTERY_CHARGING: 0 bcVersion: 0x102 typec_version: 0x100 pd_version: 0x200 PDO_DETAILS: 0
      kworker/4:0-524223  [004] ..... 33507.682726: ucsi_init_work: [Register port 1]: OPMODE: E4 flag:1
      kworker/4:0-524223  [004] ..... 33508.850438: ucsi_init_work: [Register port 2]: OPMODE: E4 flag:1
      kworker/4:0-524223  [004] ..... 33509.986672: ucsi_init_work: [Register port 3]: OPMODE: E4 flag:1


So all three ports support DRP/USB2/USB3/ALT_MODE and they can be a provider.


I find it strange that USB_PD is reported as 0 yet pd_version is reported as 2. I contacted Lenovo's support and they confirmed that this particular model indeed supports PD 3.0 on all USBC ports.

I see a couple of problems with the current upstream code:

1. It assumes that USB_PD is valid because the PD version from pd_version is being propagated to several places (like in ucsi_register_port() cap->pd_revision = ucsi->cap.pd_version;)
2. When typec_register_port() is called from ucsi_register_port() cap->pd is 0 hence the port->pd = cap->pd; assignment in typec_register_port is a noop. In fact I don't see where cap->pd is being initialized since we initialize con->pd when we call usb_power_delivery_register in ucsi_register_port().


Is it mandatory that GET_PDOS is supported if PD is supported, the UCSI spec doesn't say anything other than GET_PDOS is optional and signaled by bit in the GET_CAP call ?

Re: [RESEND] USB PD broken on Lenovo P15gen2

[Heikki Krogerus]

Hi Nikolay,

Thanks for the report.

On Wed, Aug 30, 2023 at 04:07:55PM +0300, Nikolay Borisov wrote:
> 
> 
> On 28.08.23 г. 17:52 ч., Nikolay Borisov wrote:
> > 
> > [Resending as I had initially attached  a full acpi dump and it got
> > bounced from the usb mailing list]
> > 
> > Hello,
> > 
> > I'm not able to use usb PD on a Lenovo Thinkpad P15gen2 laptop. It's
> > equipped with 2 thunderbolt ports and a usb 3.2 gen2 usb port, all of
> > which are supposed to support PD 2.0:
> 
> <snip>
> So I've been debugging this and what the PPM reports is the following:
> 
> modprobe-529501  [004] ..... 33507.058332: ucsi_register: Supported UCSI spec: 100
>      kworker/4:0-524223  [004] ..... 33507.486591: ucsi_init_work: Connectors supported: 3
>      kworker/4:0-524223  [004] ..... 33507.486592: ucsi_init_work: GET_CAP: USB_PD: 0 TYPEC_CURRENT: 1 POWER_VBUS: 0, POWER_OTHER: 0, POWER_AC_SUPPLY: 1, BATTERY_CHARGING: 0 bcVersion: 0x102 typec_version: 0x100 pd_version: 0x200 PDO_DETAILS: 0
>      kworker/4:0-524223  [004] ..... 33507.682726: ucsi_init_work: [Register port 1]: OPMODE: E4 flag:1
>      kworker/4:0-524223  [004] ..... 33508.850438: ucsi_init_work: [Register port 2]: OPMODE: E4 flag:1
>      kworker/4:0-524223  [004] ..... 33509.986672: ucsi_init_work: [Register port 3]: OPMODE: E4 flag:1
> 
> 
> So all three ports support DRP/USB2/USB3/ALT_MODE and they can be a provider.
> 
> 
> I find it strange that USB_PD is reported as 0 yet pd_version is reported as 2. I contacted Lenovo's support and they confirmed that this particular model indeed supports PD 3.0 on all USBC ports.
> 
> I see a couple of problems with the current upstream code:
> 
> 1. It assumes that USB_PD is valid because the PD version from pd_version is being propagated to several places (like in ucsi_register_port() cap->pd_revision = ucsi->cap.pd_version;)

This part should be fixed.

> 2. When typec_register_port() is called from ucsi_register_port() cap->pd is 0 hence the port->pd = cap->pd; assignment in typec_register_port is a noop. In fact I don't see where cap->pd is being initialized since we initialize con->pd when we call usb_power_delivery_register in ucsi_register_port().

That "pd" member in struct typec_capability is optional. It can be
used if the driver has a set of USB PD capabilities meant for USB
Type-C port ready before the port is registered, but in UCSI driver
the PD stuff are registered after the port.

So I'm not sure there is anything wrong here.

> Is it mandatory that GET_PDOS is supported if PD is supported, the UCSI spec doesn't say anything other than GET_PDOS is optional and signaled by bit in the GET_CAP call ?

It looks like nobody ever checked is the command supported or not
before using it. That's a bug.

thanks,

-- 
heikki

Re: [RESEND] USB PD broken on Lenovo P15gen2

[Nikolay Borisov]

On 31.08.23 г. 16:41 ч., Heikki Krogerus wrote:
> Hi Nikolay,
> 
> Thanks for the report.
> 
> On Wed, Aug 30, 2023 at 04:07:55PM +0300, Nikolay Borisov wrote:
>>
>>
>> On 28.08.23 г. 17:52 ч., Nikolay Borisov wrote:
>>>
>>> [Resending as I had initially attached  a full acpi dump and it got
>>> bounced from the usb mailing list]
>>>
>>> Hello,
>>>
>>> I'm not able to use usb PD on a Lenovo Thinkpad P15gen2 laptop. It's
>>> equipped with 2 thunderbolt ports and a usb 3.2 gen2 usb port, all of
>>> which are supposed to support PD 2.0:
>>
>> <snip>
>> So I've been debugging this and what the PPM reports is the following:
>>
>> modprobe-529501  [004] ..... 33507.058332: ucsi_register: Supported UCSI spec: 100
>>       kworker/4:0-524223  [004] ..... 33507.486591: ucsi_init_work: Connectors supported: 3
>>       kworker/4:0-524223  [004] ..... 33507.486592: ucsi_init_work: GET_CAP: USB_PD: 0 TYPEC_CURRENT: 1 POWER_VBUS: 0, POWER_OTHER: 0, POWER_AC_SUPPLY: 1, BATTERY_CHARGING: 0 bcVersion: 0x102 typec_version: 0x100 pd_version: 0x200 PDO_DETAILS: 0
>>       kworker/4:0-524223  [004] ..... 33507.682726: ucsi_init_work: [Register port 1]: OPMODE: E4 flag:1
>>       kworker/4:0-524223  [004] ..... 33508.850438: ucsi_init_work: [Register port 2]: OPMODE: E4 flag:1
>>       kworker/4:0-524223  [004] ..... 33509.986672: ucsi_init_work: [Register port 3]: OPMODE: E4 flag:1
>>
>>
>> So all three ports support DRP/USB2/USB3/ALT_MODE and they can be a provider.
>>
>>
>> I find it strange that USB_PD is reported as 0 yet pd_version is reported as 2. I contacted Lenovo's support and they confirmed that this particular model indeed supports PD 3.0 on all USBC ports.
>>
>> I see a couple of problems with the current upstream code:
>>
>> 1. It assumes that USB_PD is valid because the PD version from pd_version is being propagated to several places (like in ucsi_register_port() cap->pd_revision = ucsi->cap.pd_version;)
> 
> This part should be fixed.
> 
>> 2. When typec_register_port() is called from ucsi_register_port() cap->pd is 0 hence the port->pd = cap->pd; assignment in typec_register_port is a noop. In fact I don't see where cap->pd is being initialized since we initialize con->pd when we call usb_power_delivery_register in ucsi_register_port().
> 
> That "pd" member in struct typec_capability is optional. It can be
> used if the driver has a set of USB PD capabilities meant for USB
> Type-C port ready before the port is registered, but in UCSI driver
> the PD stuff are registered after the port.
> 
> So I'm not sure there is anything wrong here.
> 
>> Is it mandatory that GET_PDOS is supported if PD is supported, the UCSI spec doesn't say anything other than GET_PDOS is optional and signaled by bit in the GET_CAP call ?
> 
> It looks like nobody ever checked is the command supported or not
> before using it. That's a bug.


If we assume support of this command is checked based on the respective 
bit in the optional capabilities member of GET_CAPS message. Is this 
command actually mandatory to properly support PD. I'm currently in 
contact with Lenovo as it seems there might be problems in their 
firmware as well i.e what they are reporting.
> 
> thanks,
>