| Message ID | 20230822133205.2063210-1-heikki.krogerus@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | usb: Link USB devices with their USB Type-C partner counterparts | expand |
Hi Heikki, On Tue, Aug 22, 2023 at 6:32 AM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > Hi Benson, > > RFC for now. I can't test these properly. If you guys could take over > this, I would much appreciated. I hope this is at least close to your > proposal. Much appreciated for the quick turnaround. Yes, I can test this on our systems that have subsystem linking enabled. (speaking of which, would be interested in getting a sample of one of our Chromebook systems that fully implement the typec subsystem linking)? > > With this (or something like it) you should be able to get > notification about USB connections and disconnections to your port > driver by implementing the new "attach" and "deattach" callbacks in > struct typec_partner_desc. The typec partner devices will also have > symlinks to the enumerated USB devices and vise versa. > Got it, i'll modify the cros_ec_typec driver to implement these callbacks, and look for the new symlinks. > I took a little shortcut and did not implement a proper device list. > Instead there is now only a member for the USB2 device and a member > for the USB3 device in struct typec_port, so with this only USB is > supported. But the API does not deal with struct usb_device, so > extending this to support other devices (TBT, Displayport, etc.) by > adding the actual device list should be fairly easy. Excellent! Thank you. Benson > > thanks, > > Heikki Krogerus (2): > usb: typec: Link enumerated USB devices with Type-C partner > usb: Inform the USB Type-C class about enumerated devices > > drivers/usb/core/hub.c | 4 ++ > drivers/usb/core/hub.h | 3 + > drivers/usb/core/port.c | 19 +++++- > drivers/usb/typec/class.c | 108 +++++++++++++++++++++++++++++--- > drivers/usb/typec/class.h | 16 +++++ > drivers/usb/typec/port-mapper.c | 9 ++- > include/linux/usb/typec.h | 37 +++++++++++ > 7 files changed, 184 insertions(+), 12 deletions(-) > > -- > 2.40.1 > -- Benson Leung Staff Software Engineer Chrome OS Kernel Google Inc. bleung@google.com Chromium OS Project bleung@chromium.org
On 2023-08-22 09:32, Heikki Krogerus wrote: > Hi Benson, > > RFC for now. I can't test these properly. If you guys could take over > this, I would much appreciated. I hope this is at least close to your > proposal. > > With this (or something like it) you should be able to get > notification about USB connections and disconnections to your port > driver by implementing the new "attach" and "deattach" callbacks in > struct typec_partner_desc. The typec partner devices will also have > symlinks to the enumerated USB devices and vise versa. > > I took a little shortcut and did not implement a proper device list. > Instead there is now only a member for the USB2 device and a member > for the USB3 device in struct typec_port, so with this only USB is > supported. But the API does not deal with struct usb_device, so > extending this to support other devices (TBT, Displayport, etc.) by > adding the actual device list should be fairly easy. On a related matter, I wonder why there aren't symlinks between typec ports (under /sys/class/typec ) and/or the corresponding pd objects (under /sys/class/usb_power_delivery ) to the related power_supply objects under /sys/class/power_supply . For example under the latter directory I see: $ ls | more AC BAT0 hidpp_battery_1 ucsi-source-psy-USBC000:001 ucsi-source-psy-USBC000:002 Those last two power supplies are obviously connected to typec port0 and port1 (but offset by 1). Those power_supply objects hold inaccurate data which I hope will improve in time. Significantly power_supply objects don't seem to report the direction of the power. Here is a little utility I have been working on to report the USB Type-C port/pd disposition on my machine: $ lsucpd port0 [pd0] > {5V, 0.9A} port1 [pd1] <<=== partner: [pd8] My laptop (Thinkpad X13 G3) has two type-C ports and port1 is a sink with a PD contract. I would like that second line to have 20V, 3.25A appended to it but there are several issues: - no typec or pd symlink to ucsi-source-psy-USBC000:002 - that power supply_object says it is online (correct) with a voltage_now: 5000000 uV (incorrect) and current_now: 3000000 uA (incorrect). See below. ucsi-source-psy-USBC000:002 $ ls_name_value current_max : 3250000 current_now : 3000000 online : 1 scope : Unknown type : USB uevent : <removed> usb_type : C [PD] PD_PPS voltage_max : 20000000 voltage_min : 5000000 voltage_now : 5000000 Doug Gilbert
Hi Heikki, On Tue, Aug 22, 2023 at 6:32 AM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > Hi Benson, > > RFC for now. I can't test these properly. If you guys could take over > this, I would much appreciated. I hope this is at least close to your > proposal. > > With this (or something like it) you should be able to get > notification about USB connections and disconnections to your port > driver by implementing the new "attach" and "deattach" callbacks in > struct typec_partner_desc. The typec partner devices will also have > symlinks to the enumerated USB devices and vise versa. > > I took a little shortcut and did not implement a proper device list. > Instead there is now only a member for the USB2 device and a member > for the USB3 device in struct typec_port, so with this only USB is > supported. But the API does not deal with struct usb_device, so > extending this to support other devices (TBT, Displayport, etc.) by > adding the actual device list should be fairly easy. > > thanks, > > Heikki Krogerus (2): > usb: typec: Link enumerated USB devices with Type-C partner > usb: Inform the USB Type-C class about enumerated devices > > drivers/usb/core/hub.c | 4 ++ > drivers/usb/core/hub.h | 3 + > drivers/usb/core/port.c | 19 +++++- > drivers/usb/typec/class.c | 108 +++++++++++++++++++++++++++++--- > drivers/usb/typec/class.h | 16 +++++ > drivers/usb/typec/port-mapper.c | 9 ++- > include/linux/usb/typec.h | 37 +++++++++++ > 7 files changed, 184 insertions(+), 12 deletions(-) > > -- > 2.40.1 > Tested-by: Benson Leung <bleung@chromium.org> I picked these two changes back to my Brya/Redrix Chromebook which has the PLD changes to link subsystems. First I plugged in a USB-C to USB-A receptacle adapter with a USB3 thumbdrive into port0, and went to the port0-partner path. redrix-rev3 /sys/class/typec/port0-partner # ls -lh total 0 lrwxrwxrwx. 1 root root 0 Aug 22 17:16 2-1 -> ../../../../../../../0000:00:0d.0/usb2/2-1 -r--r--r--. 1 root root 4.0K Aug 22 17:14 accessory_mode lrwxrwxrwx. 1 root root 0 Aug 22 17:14 device -> ../../port0 drwxr-xr-x. 2 root root 0 Aug 22 17:14 identity drwxr-xr-x. 2 root root 0 Aug 22 17:14 power lrwxrwxrwx. 1 root root 0 Aug 22 17:14 subsystem -> ../../../../../../../../../class/typec -r--r--r--. 1 root root 4.0K Aug 22 17:14 supports_usb_power_delivery -r--r--r--. 1 root root 4.0K Aug 22 17:14 type -rw-r--r--. 1 root root 4.0K Aug 22 17:14 uevent -r--r--r--. 1 root root 4.0K Aug 22 17:14 usb_power_delivery_revision 2-1 symlink appears, which is the SuperSpeed usb device associated with the thumbdrive. Unplugging the USB3 thumbdrive without unplugging the C-to-A adapter, and then plugging in a USB2.0 security key: redrix-rev3 /sys/class/typec/port0-partner # ls -lh total 0 lrwxrwxrwx. 1 root root 0 Aug 22 17:19 3-1 -> ../../../../../../../0000:00:14.0/usb3/3-1 -r--r--r--. 1 root root 4.0K Aug 22 17:14 accessory_mode lrwxrwxrwx. 1 root root 0 Aug 22 17:14 device -> ../../port0 drwxr-xr-x. 2 root root 0 Aug 22 17:14 identity drwxr-xr-x. 2 root root 0 Aug 22 17:14 power lrwxrwxrwx. 1 root root 0 Aug 22 17:14 subsystem -> ../../../../../../../../../class/typec -r--r--r--. 1 root root 4.0K Aug 22 17:14 supports_usb_power_delivery -r--r--r--. 1 root root 4.0K Aug 22 17:14 type -rw-r--r--. 1 root root 4.0K Aug 22 17:14 uevent -r--r--r--. 1 root root 4.0K Aug 22 17:14 usb_power_delivery_revision 2-1 node disappears. 3-1 appears Unplugging the adapter, plugging in a USB4 hub: redrix-rev3 /sys/class/typec/port0-partner # ls -lh total 0 lrwxrwxrwx. 1 root root 0 Aug 22 17:21 2-1 -> ../../../../../../../0000:00:0d.0/usb2/2-1 lrwxrwxrwx. 1 root root 0 Aug 22 17:21 3-1 -> ../../../../../../../0000:00:14.0/usb3/3-1 -r--r--r--. 1 root root 4.0K Aug 22 17:21 accessory_mode lrwxrwxrwx. 1 root root 0 Aug 22 17:21 device -> ../../port0 drwxr-xr-x. 2 root root 0 Aug 22 17:21 identity -r--r--r--. 1 root root 4.0K Aug 22 17:21 number_of_alternate_modes drwxr-xr-x. 5 root root 0 Aug 22 17:21 pd0 drwxr-xr-x. 4 root root 0 Aug 22 17:21 port0-partner.0 drwxr-xr-x. 2 root root 0 Aug 22 17:21 power lrwxrwxrwx. 1 root root 0 Aug 22 17:21 subsystem -> ../../../../../../../../../class/typec -r--r--r--. 1 root root 4.0K Aug 22 17:21 supports_usb_power_delivery -r--r--r--. 1 root root 4.0K Aug 22 17:21 type -rw-r--r--. 1 root root 4.0K Aug 22 17:21 uevent lrwxrwxrwx. 1 root root 0 Aug 22 17:21 usb_power_delivery -> pd0 -r--r--r--. 1 root root 4.0K Aug 22 17:21 usb_power_delivery_revision Both 2-1 and 3-1 are linked. Thanks so much for this, Heikki! I can look a little closer at the attach and deattach callbacks in our typec port driver in a little while. Benson
Hi Douglas, On Tue, Aug 22, 2023 at 10:52:12AM -0400, Douglas Gilbert wrote: > On 2023-08-22 09:32, Heikki Krogerus wrote: > On a related matter, I wonder why there aren't symlinks between typec ports > (under /sys/class/typec ) and/or the corresponding pd objects (under > /sys/class/usb_power_delivery ) to the related power_supply objects under > /sys/class/power_supply . For example under the latter directory I see: > $ ls | more > AC > BAT0 > hidpp_battery_1 > ucsi-source-psy-USBC000:001 > ucsi-source-psy-USBC000:002 > > Those last two power supplies are obviously connected to typec port0 and port1 > (but offset by 1). Those power_supply objects hold inaccurate data which I hope > will improve in time. Significantly power_supply objects don't seem to report > the direction of the power. Here is a little utility I have been working on > to report the USB Type-C port/pd disposition on my machine: > $ lsucpd > port0 [pd0] > {5V, 0.9A} > port1 [pd1] <<=== partner: [pd8] > > My laptop (Thinkpad X13 G3) has two type-C ports and port1 is a sink with a > PD contract. I would like that second line to have 20V, 3.25A appended to it > but there are several issues: > - no typec or pd symlink to ucsi-source-psy-USBC000:002 > - that power supply_object says it is online (correct) with a voltage_now: > 5000000 uV (incorrect) and current_now: 3000000 uA (incorrect). See below. > > ucsi-source-psy-USBC000:002 $ ls_name_value > current_max : 3250000 > current_now : 3000000 > online : 1 > scope : Unknown > type : USB > uevent : <removed> > usb_type : C [PD] PD_PPS > voltage_max : 20000000 > voltage_min : 5000000 > voltage_now : 5000000 I'm glad you brought that up. The major problem with the Type-C power supplies is that the Type-C connector class does not actually take care of them. They are all registered by the device drivers, and all of them seem to expose different kind of information. In your case the power supplies are registered by the UCSI driver, and the above may indicate a bug in that driver. To improve the situation, I originally proposed that instead of adding a separate device class for USB Power Delivery objects, we would utilise the already existing power supply class. That proposal was not seen acceptable by many (including Benson if I recall), and I now tend to agree with that because of several reasons, starting from the fact that USB PD objects supply other informations on top of power delivery details (so completely unrelated to PM). Even before that I had proposed that the Type-C connector class could supply API for the drivers to take care of the registration of the power supplies. I proposed that not only the Type-C ports should register the power supplies but also the partners should represent their own power supplies. That would make things much more clear for the user space IMO. The port and partner would always create a "chain" of supplies where the other is the supply the other the sink of power. That is already supported by the power supply class. For some reason this proposal was also not seen as a good idea at the time, but it may be that I just failed to explain it properly. Nevertheless, I still think that that is exactly how the Type-C power supplies should be always presented - separate supplies for both ports and partners - and that obviously the Type-C connector class should take care of those supplies so that they always supply the same information. Unfortunately I do not have any time at the moment to work on this right now. Br,
On Tue, Aug 22, 2023 at 05:24:44PM -0700, Benson Leung wrote: > Hi Heikki, > > On Tue, Aug 22, 2023 at 6:32 AM Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > > > > Hi Benson, > > > > RFC for now. I can't test these properly. If you guys could take over > > this, I would much appreciated. I hope this is at least close to your > > proposal. > > > > With this (or something like it) you should be able to get > > notification about USB connections and disconnections to your port > > driver by implementing the new "attach" and "deattach" callbacks in > > struct typec_partner_desc. The typec partner devices will also have > > symlinks to the enumerated USB devices and vise versa. > > > > I took a little shortcut and did not implement a proper device list. > > Instead there is now only a member for the USB2 device and a member > > for the USB3 device in struct typec_port, so with this only USB is > > supported. But the API does not deal with struct usb_device, so > > extending this to support other devices (TBT, Displayport, etc.) by > > adding the actual device list should be fairly easy. > > > > thanks, > > > > Heikki Krogerus (2): > > usb: typec: Link enumerated USB devices with Type-C partner > > usb: Inform the USB Type-C class about enumerated devices > > > > drivers/usb/core/hub.c | 4 ++ > > drivers/usb/core/hub.h | 3 + > > drivers/usb/core/port.c | 19 +++++- > > drivers/usb/typec/class.c | 108 +++++++++++++++++++++++++++++--- > > drivers/usb/typec/class.h | 16 +++++ > > drivers/usb/typec/port-mapper.c | 9 ++- > > include/linux/usb/typec.h | 37 +++++++++++ > > 7 files changed, 184 insertions(+), 12 deletions(-) > > > > -- > > 2.40.1 > > > > Tested-by: Benson Leung <bleung@chromium.org> > > > I picked these two changes back to my Brya/Redrix Chromebook which has > the PLD changes to link subsystems. > > First I plugged in a USB-C to USB-A receptacle adapter with a USB3 > thumbdrive into port0, and went to the port0-partner path. > > redrix-rev3 /sys/class/typec/port0-partner # ls -lh > total 0 > lrwxrwxrwx. 1 root root 0 Aug 22 17:16 2-1 -> > ../../../../../../../0000:00:0d.0/usb2/2-1 > -r--r--r--. 1 root root 4.0K Aug 22 17:14 accessory_mode > lrwxrwxrwx. 1 root root 0 Aug 22 17:14 device -> ../../port0 > drwxr-xr-x. 2 root root 0 Aug 22 17:14 identity > drwxr-xr-x. 2 root root 0 Aug 22 17:14 power > lrwxrwxrwx. 1 root root 0 Aug 22 17:14 subsystem -> > ../../../../../../../../../class/typec > -r--r--r--. 1 root root 4.0K Aug 22 17:14 supports_usb_power_delivery > -r--r--r--. 1 root root 4.0K Aug 22 17:14 type > -rw-r--r--. 1 root root 4.0K Aug 22 17:14 uevent > -r--r--r--. 1 root root 4.0K Aug 22 17:14 usb_power_delivery_revision > > 2-1 symlink appears, which is the SuperSpeed usb device associated > with the thumbdrive. > Unplugging the USB3 thumbdrive without unplugging the C-to-A adapter, > and then plugging in a USB2.0 security key: > > redrix-rev3 /sys/class/typec/port0-partner # ls -lh > total 0 > lrwxrwxrwx. 1 root root 0 Aug 22 17:19 3-1 -> > ../../../../../../../0000:00:14.0/usb3/3-1 > -r--r--r--. 1 root root 4.0K Aug 22 17:14 accessory_mode > lrwxrwxrwx. 1 root root 0 Aug 22 17:14 device -> ../../port0 > drwxr-xr-x. 2 root root 0 Aug 22 17:14 identity > drwxr-xr-x. 2 root root 0 Aug 22 17:14 power > lrwxrwxrwx. 1 root root 0 Aug 22 17:14 subsystem -> > ../../../../../../../../../class/typec > -r--r--r--. 1 root root 4.0K Aug 22 17:14 supports_usb_power_delivery > -r--r--r--. 1 root root 4.0K Aug 22 17:14 type > -rw-r--r--. 1 root root 4.0K Aug 22 17:14 uevent > -r--r--r--. 1 root root 4.0K Aug 22 17:14 usb_power_delivery_revision > > 2-1 node disappears. 3-1 appears > > Unplugging the adapter, plugging in a USB4 hub: > redrix-rev3 /sys/class/typec/port0-partner # ls -lh > total 0 > lrwxrwxrwx. 1 root root 0 Aug 22 17:21 2-1 -> > ../../../../../../../0000:00:0d.0/usb2/2-1 > lrwxrwxrwx. 1 root root 0 Aug 22 17:21 3-1 -> > ../../../../../../../0000:00:14.0/usb3/3-1 > -r--r--r--. 1 root root 4.0K Aug 22 17:21 accessory_mode > lrwxrwxrwx. 1 root root 0 Aug 22 17:21 device -> ../../port0 > drwxr-xr-x. 2 root root 0 Aug 22 17:21 identity > -r--r--r--. 1 root root 4.0K Aug 22 17:21 number_of_alternate_modes > drwxr-xr-x. 5 root root 0 Aug 22 17:21 pd0 > drwxr-xr-x. 4 root root 0 Aug 22 17:21 port0-partner.0 > drwxr-xr-x. 2 root root 0 Aug 22 17:21 power > lrwxrwxrwx. 1 root root 0 Aug 22 17:21 subsystem -> > ../../../../../../../../../class/typec > -r--r--r--. 1 root root 4.0K Aug 22 17:21 supports_usb_power_delivery > -r--r--r--. 1 root root 4.0K Aug 22 17:21 type > -rw-r--r--. 1 root root 4.0K Aug 22 17:21 uevent > lrwxrwxrwx. 1 root root 0 Aug 22 17:21 usb_power_delivery -> pd0 > -r--r--r--. 1 root root 4.0K Aug 22 17:21 usb_power_delivery_revision > > Both 2-1 and 3-1 are linked. > > Thanks so much for this, Heikki! I can look a little closer at the > attach and deattach callbacks in our typec port driver in a little > while. Cool! Thank you!
On 2023-08-23 04:56, Heikki Krogerus wrote: > Hi Douglas, > > On Tue, Aug 22, 2023 at 10:52:12AM -0400, Douglas Gilbert wrote: >> On 2023-08-22 09:32, Heikki Krogerus wrote: >> On a related matter, I wonder why there aren't symlinks between typec ports >> (under /sys/class/typec ) and/or the corresponding pd objects (under >> /sys/class/usb_power_delivery ) to the related power_supply objects under >> /sys/class/power_supply . For example under the latter directory I see: >> $ ls | more >> AC >> BAT0 >> hidpp_battery_1 >> ucsi-source-psy-USBC000:001 >> ucsi-source-psy-USBC000:002 >> >> Those last two power supplies are obviously connected to typec port0 and port1 >> (but offset by 1). Those power_supply objects hold inaccurate data which I hope >> will improve in time. Significantly power_supply objects don't seem to report >> the direction of the power. Here is a little utility I have been working on >> to report the USB Type-C port/pd disposition on my machine: >> $ lsucpd >> port0 [pd0] > {5V, 0.9A} >> port1 [pd1] <<=== partner: [pd8] >> >> My laptop (Thinkpad X13 G3) has two type-C ports and port1 is a sink with a >> PD contract. I would like that second line to have 20V, 3.25A appended to it >> but there are several issues: >> - no typec or pd symlink to ucsi-source-psy-USBC000:002 >> - that power supply_object says it is online (correct) with a voltage_now: >> 5000000 uV (incorrect) and current_now: 3000000 uA (incorrect). See below. >> >> ucsi-source-psy-USBC000:002 $ ls_name_value >> current_max : 3250000 >> current_now : 3000000 >> online : 1 >> scope : Unknown >> type : USB >> uevent : <removed> >> usb_type : C [PD] PD_PPS >> voltage_max : 20000000 >> voltage_min : 5000000 >> voltage_now : 5000000 > > I'm glad you brought that up. The major problem with the Type-C power > supplies is that the Type-C connector class does not actually take > care of them. They are all registered by the device drivers, and all > of them seem to expose different kind of information. In your case the > power supplies are registered by the UCSI driver, and the above may > indicate a bug in that driver. Hi, Thanks for the background. My X13 Gen 3 (i5-1240P) uses the typec_ucsi and ucsi_acpi modules. Some time back in a post you explained how to use debugfs with ucsi. Following that procedure, just after a 20 Volt PD contract is negotiated on port 0 I see: # cat /sys/kernel/debug/tracing/trace .... kworker/0:1-18718 [000] ..... 137813.407189: ucsi_connector_change: port0 status: change=0000, opmode=5, connected=1, sourcing=0, partner_flags=1, partner_type=1, request_data_obj=1304b12c, BC status=1 That RDO is incorrect, the top nibble (1) is the index of the default Vsafe5v PDO. The correct PDO index would be 4 in this case. The source is an Apple 140W USB-C power adapter so I doubt that it is breaking any PD 3.0/3.1 protocol rules. According the a PD analyzer (km002c) only one Request is sent by the sink: 82 10 d6 59 87 43 which it decodes as "Pos: 4 Fixed: 20V, 4.7A" which is Accepted and 200 ms later a PS RDY is sent by the source and Vbus transitions from from 5.17 Volts to 20.4 Volts. So I can see no Request for PDO index 1 being sent. With acpi_listen the following traffic occurs just after the power adapter is plugged into port 0: battery PNP0C0A:00 00000080 00000001 battery PNP0C0A:00 00000080 00000001 ibm/hotkey LEN0268:00 00000080 00006032 ac_adapter ACPI0003:00 00000080 00000001 ac_adapter ACPI0003:00 00000080 00000001 ibm/hotkey LEN0268:00 00000080 00006030 thermal_zone LNXTHERM:00 00000081 00000000 ibm/hotkey LEN0268:00 00000080 00006030 thermal_zone LNXTHERM:00 00000081 00000000 Hope this helps if you find time to look at this. Doug Gilbert > To improve the situation, I originally proposed that instead of > adding a separate device class for USB Power Delivery objects, we > would utilise the already existing power supply class. That proposal > was not seen acceptable by many (including Benson if I recall), and I > now tend to agree with that because of several reasons, starting from > the fact that USB PD objects supply other informations on top of power > delivery details (so completely unrelated to PM). > > Even before that I had proposed that the Type-C connector class could > supply API for the drivers to take care of the registration of the > power supplies. I proposed that not only the Type-C ports should > register the power supplies but also the partners should represent > their own power supplies. That would make things much more clear for > the user space IMO. The port and partner would always create a "chain" > of supplies where the other is the supply the other the sink of power. > That is already supported by the power supply class. For some reason > this proposal was also not seen as a good idea at the time, but it may > be that I just failed to explain it properly. > > Nevertheless, I still think that that is exactly how the Type-C power > supplies should be always presented - separate supplies for both ports > and partners - and that obviously the Type-C connector class should > take care of those supplies so that they always supply the same > information. Unfortunately I do not have any time at the moment to > work on this right now. > > Br, >
On Thu, Aug 24, 2023 at 12:51:04PM -0400, Douglas Gilbert wrote: > On 2023-08-23 04:56, Heikki Krogerus wrote: > > Hi Douglas, > > > > On Tue, Aug 22, 2023 at 10:52:12AM -0400, Douglas Gilbert wrote: > > > On 2023-08-22 09:32, Heikki Krogerus wrote: > > > On a related matter, I wonder why there aren't symlinks between typec ports > > > (under /sys/class/typec ) and/or the corresponding pd objects (under > > > /sys/class/usb_power_delivery ) to the related power_supply objects under > > > /sys/class/power_supply . For example under the latter directory I see: > > > $ ls | more > > > AC > > > BAT0 > > > hidpp_battery_1 > > > ucsi-source-psy-USBC000:001 > > > ucsi-source-psy-USBC000:002 > > > > > > Those last two power supplies are obviously connected to typec port0 and port1 > > > (but offset by 1). Those power_supply objects hold inaccurate data which I hope > > > will improve in time. Significantly power_supply objects don't seem to report > > > the direction of the power. Here is a little utility I have been working on > > > to report the USB Type-C port/pd disposition on my machine: > > > $ lsucpd > > > port0 [pd0] > {5V, 0.9A} > > > port1 [pd1] <<=== partner: [pd8] > > > > > > My laptop (Thinkpad X13 G3) has two type-C ports and port1 is a sink with a > > > PD contract. I would like that second line to have 20V, 3.25A appended to it > > > but there are several issues: > > > - no typec or pd symlink to ucsi-source-psy-USBC000:002 > > > - that power supply_object says it is online (correct) with a voltage_now: > > > 5000000 uV (incorrect) and current_now: 3000000 uA (incorrect). See below. > > > > > > ucsi-source-psy-USBC000:002 $ ls_name_value > > > current_max : 3250000 > > > current_now : 3000000 > > > online : 1 > > > scope : Unknown > > > type : USB > > > uevent : <removed> > > > usb_type : C [PD] PD_PPS > > > voltage_max : 20000000 > > > voltage_min : 5000000 > > > voltage_now : 5000000 > > > > I'm glad you brought that up. The major problem with the Type-C power > > supplies is that the Type-C connector class does not actually take > > care of them. They are all registered by the device drivers, and all > > of them seem to expose different kind of information. In your case the > > power supplies are registered by the UCSI driver, and the above may > > indicate a bug in that driver. > > Hi, > Thanks for the background. > > My X13 Gen 3 (i5-1240P) uses the typec_ucsi and ucsi_acpi modules. Some time > back in a post you explained how to use debugfs with ucsi. Following that > procedure, just after a 20 Volt PD contract is negotiated on port 0 I see: > > # cat /sys/kernel/debug/tracing/trace > .... > kworker/0:1-18718 [000] ..... 137813.407189: ucsi_connector_change: > port0 status: change=0000, opmode=5, connected=1, sourcing=0, > partner_flags=1, partner_type=1, > request_data_obj=1304b12c, BC status=1 > > That RDO is incorrect, the top nibble (1) is the index of the default Vsafe5v > PDO. The correct PDO index would be 4 in this case. The source is an Apple 140W > USB-C power adapter so I doubt that it is breaking any PD 3.0/3.1 protocol > rules. The driver reads the RDO from the UCSI interface, so if it's wrong, there is possibly a problem in the Embedded Controller firmware :-(. > According the a PD analyzer (km002c) only one Request is sent by the sink: > 82 10 d6 59 87 43 which it decodes as "Pos: 4 Fixed: 20V, 4.7A" which is > Accepted and 200 ms later a PS RDY is sent by the source and Vbus > transitions from from 5.17 Volts to 20.4 Volts. So I can see no Request for > PDO index 1 being sent. > > With acpi_listen the following traffic occurs just after the power adapter > is plugged into port 0: > battery PNP0C0A:00 00000080 00000001 > battery PNP0C0A:00 00000080 00000001 > ibm/hotkey LEN0268:00 00000080 00006032 > ac_adapter ACPI0003:00 00000080 00000001 > ac_adapter ACPI0003:00 00000080 00000001 > ibm/hotkey LEN0268:00 00000080 00006030 > thermal_zone LNXTHERM:00 00000081 00000000 > ibm/hotkey LEN0268:00 00000080 00006030 > thermal_zone LNXTHERM:00 00000081 00000000 > > Hope this helps if you find time to look at this. Thank you. I'll try to reproduce the issue this week, but I don't have that exact model of Thinkpad available I'm afraid (UCSI tends to behave a little bit differently on every single platform).
On 2023-08-28 05:21, Heikki Krogerus wrote: > On Thu, Aug 24, 2023 at 12:51:04PM -0400, Douglas Gilbert wrote: >> On 2023-08-23 04:56, Heikki Krogerus wrote: >>> Hi Douglas, >>> >>> On Tue, Aug 22, 2023 at 10:52:12AM -0400, Douglas Gilbert wrote: >>>> On 2023-08-22 09:32, Heikki Krogerus wrote: >>>> On a related matter, I wonder why there aren't symlinks between typec ports >>>> (under /sys/class/typec ) and/or the corresponding pd objects (under >>>> /sys/class/usb_power_delivery ) to the related power_supply objects under >>>> /sys/class/power_supply . For example under the latter directory I see: >>>> $ ls | more >>>> AC >>>> BAT0 >>>> hidpp_battery_1 >>>> ucsi-source-psy-USBC000:001 >>>> ucsi-source-psy-USBC000:002 >>>> >>>> Those last two power supplies are obviously connected to typec port0 and port1 >>>> (but offset by 1). Those power_supply objects hold inaccurate data which I hope >>>> will improve in time. Significantly power_supply objects don't seem to report >>>> the direction of the power. Here is a little utility I have been working on >>>> to report the USB Type-C port/pd disposition on my machine: >>>> $ lsucpd >>>> port0 [pd0] > {5V, 0.9A} >>>> port1 [pd1] <<=== partner: [pd8] >>>> >>>> My laptop (Thinkpad X13 G3) has two type-C ports and port1 is a sink with a >>>> PD contract. I would like that second line to have 20V, 3.25A appended to it >>>> but there are several issues: >>>> - no typec or pd symlink to ucsi-source-psy-USBC000:002 >>>> - that power supply_object says it is online (correct) with a voltage_now: >>>> 5000000 uV (incorrect) and current_now: 3000000 uA (incorrect). See below. >>>> >>>> ucsi-source-psy-USBC000:002 $ ls_name_value >>>> current_max : 3250000 >>>> current_now : 3000000 >>>> online : 1 >>>> scope : Unknown >>>> type : USB >>>> uevent : <removed> >>>> usb_type : C [PD] PD_PPS >>>> voltage_max : 20000000 >>>> voltage_min : 5000000 >>>> voltage_now : 5000000 >>> >>> I'm glad you brought that up. The major problem with the Type-C power >>> supplies is that the Type-C connector class does not actually take >>> care of them. They are all registered by the device drivers, and all >>> of them seem to expose different kind of information. In your case the >>> power supplies are registered by the UCSI driver, and the above may >>> indicate a bug in that driver. >> >> Hi, >> Thanks for the background. >> >> My X13 Gen 3 (i5-1240P) uses the typec_ucsi and ucsi_acpi modules. Some time >> back in a post you explained how to use debugfs with ucsi. Following that >> procedure, just after a 20 Volt PD contract is negotiated on port 0 I see: >> >> # cat /sys/kernel/debug/tracing/trace >> .... >> kworker/0:1-18718 [000] ..... 137813.407189: ucsi_connector_change: >> port0 status: change=0000, opmode=5, connected=1, sourcing=0, >> partner_flags=1, partner_type=1, >> request_data_obj=1304b12c, BC status=1 >> >> That RDO is incorrect, the top nibble (1) is the index of the default Vsafe5v >> PDO. The correct PDO index would be 4 in this case. The source is an Apple 140W >> USB-C power adapter so I doubt that it is breaking any PD 3.0/3.1 protocol >> rules. > > The driver reads the RDO from the UCSI interface, so if it's wrong, > there is possibly a problem in the Embedded Controller firmware :-(. > >> According the a PD analyzer (km002c) only one Request is sent by the sink: >> 82 10 d6 59 87 43 which it decodes as "Pos: 4 Fixed: 20V, 4.7A" which is >> Accepted and 200 ms later a PS RDY is sent by the source and Vbus >> transitions from from 5.17 Volts to 20.4 Volts. So I can see no Request for >> PDO index 1 being sent. >> >> With acpi_listen the following traffic occurs just after the power adapter >> is plugged into port 0: >> battery PNP0C0A:00 00000080 00000001 >> battery PNP0C0A:00 00000080 00000001 >> ibm/hotkey LEN0268:00 00000080 00006032 >> ac_adapter ACPI0003:00 00000080 00000001 >> ac_adapter ACPI0003:00 00000080 00000001 >> ibm/hotkey LEN0268:00 00000080 00006030 >> thermal_zone LNXTHERM:00 00000081 00000000 >> ibm/hotkey LEN0268:00 00000080 00006030 >> thermal_zone LNXTHERM:00 00000081 00000000 >> >> Hope this helps if you find time to look at this. > > Thank you. I'll try to reproduce the issue this week, but I don't have > that exact model of Thinkpad available I'm afraid (UCSI tends to > behave a little bit differently on every single platform). Could it be a CPU generation thing? My CPU is 12th generation (2022) and there is another report of a Lenovo P15gen2 (11th generation 2021 I assume) not reporting PDOs at all. I have an older Dell XPS 9380 which has an 8th generation CPU (3 USB-C port and no Type A ports) that has no UCSI support. So do PDOs and the active RDO get properly reported on 13th generation CPUs? Doug Gilbert
On Tue, Aug 29, 2023 at 02:42:29PM -0400, Douglas Gilbert wrote: > On 2023-08-28 05:21, Heikki Krogerus wrote: > > On Thu, Aug 24, 2023 at 12:51:04PM -0400, Douglas Gilbert wrote: > > > On 2023-08-23 04:56, Heikki Krogerus wrote: > > > > Hi Douglas, > > > > > > > > On Tue, Aug 22, 2023 at 10:52:12AM -0400, Douglas Gilbert wrote: > > > > > On 2023-08-22 09:32, Heikki Krogerus wrote: > > > > > On a related matter, I wonder why there aren't symlinks between typec ports > > > > > (under /sys/class/typec ) and/or the corresponding pd objects (under > > > > > /sys/class/usb_power_delivery ) to the related power_supply objects under > > > > > /sys/class/power_supply . For example under the latter directory I see: > > > > > $ ls | more > > > > > AC > > > > > BAT0 > > > > > hidpp_battery_1 > > > > > ucsi-source-psy-USBC000:001 > > > > > ucsi-source-psy-USBC000:002 > > > > > > > > > > Those last two power supplies are obviously connected to typec port0 and port1 > > > > > (but offset by 1). Those power_supply objects hold inaccurate data which I hope > > > > > will improve in time. Significantly power_supply objects don't seem to report > > > > > the direction of the power. Here is a little utility I have been working on > > > > > to report the USB Type-C port/pd disposition on my machine: > > > > > $ lsucpd > > > > > port0 [pd0] > {5V, 0.9A} > > > > > port1 [pd1] <<=== partner: [pd8] > > > > > > > > > > My laptop (Thinkpad X13 G3) has two type-C ports and port1 is a sink with a > > > > > PD contract. I would like that second line to have 20V, 3.25A appended to it > > > > > but there are several issues: > > > > > - no typec or pd symlink to ucsi-source-psy-USBC000:002 > > > > > - that power supply_object says it is online (correct) with a voltage_now: > > > > > 5000000 uV (incorrect) and current_now: 3000000 uA (incorrect). See below. > > > > > > > > > > ucsi-source-psy-USBC000:002 $ ls_name_value > > > > > current_max : 3250000 > > > > > current_now : 3000000 > > > > > online : 1 > > > > > scope : Unknown > > > > > type : USB > > > > > uevent : <removed> > > > > > usb_type : C [PD] PD_PPS > > > > > voltage_max : 20000000 > > > > > voltage_min : 5000000 > > > > > voltage_now : 5000000 > > > > > > > > I'm glad you brought that up. The major problem with the Type-C power > > > > supplies is that the Type-C connector class does not actually take > > > > care of them. They are all registered by the device drivers, and all > > > > of them seem to expose different kind of information. In your case the > > > > power supplies are registered by the UCSI driver, and the above may > > > > indicate a bug in that driver. > > > > > > Hi, > > > Thanks for the background. > > > > > > My X13 Gen 3 (i5-1240P) uses the typec_ucsi and ucsi_acpi modules. Some time > > > back in a post you explained how to use debugfs with ucsi. Following that > > > procedure, just after a 20 Volt PD contract is negotiated on port 0 I see: > > > > > > # cat /sys/kernel/debug/tracing/trace > > > .... > > > kworker/0:1-18718 [000] ..... 137813.407189: ucsi_connector_change: > > > port0 status: change=0000, opmode=5, connected=1, sourcing=0, > > > partner_flags=1, partner_type=1, > > > request_data_obj=1304b12c, BC status=1 > > > > > > That RDO is incorrect, the top nibble (1) is the index of the default Vsafe5v > > > PDO. The correct PDO index would be 4 in this case. The source is an Apple 140W > > > USB-C power adapter so I doubt that it is breaking any PD 3.0/3.1 protocol > > > rules. > > > > The driver reads the RDO from the UCSI interface, so if it's wrong, > > there is possibly a problem in the Embedded Controller firmware :-(. > > > > > According the a PD analyzer (km002c) only one Request is sent by the sink: > > > 82 10 d6 59 87 43 which it decodes as "Pos: 4 Fixed: 20V, 4.7A" which is > > > Accepted and 200 ms later a PS RDY is sent by the source and Vbus > > > transitions from from 5.17 Volts to 20.4 Volts. So I can see no Request for > > > PDO index 1 being sent. > > > > > > With acpi_listen the following traffic occurs just after the power adapter > > > is plugged into port 0: > > > battery PNP0C0A:00 00000080 00000001 > > > battery PNP0C0A:00 00000080 00000001 > > > ibm/hotkey LEN0268:00 00000080 00006032 > > > ac_adapter ACPI0003:00 00000080 00000001 > > > ac_adapter ACPI0003:00 00000080 00000001 > > > ibm/hotkey LEN0268:00 00000080 00006030 > > > thermal_zone LNXTHERM:00 00000081 00000000 > > > ibm/hotkey LEN0268:00 00000080 00006030 > > > thermal_zone LNXTHERM:00 00000081 00000000 > > > > > > Hope this helps if you find time to look at this. > > > > Thank you. I'll try to reproduce the issue this week, but I don't have > > that exact model of Thinkpad available I'm afraid (UCSI tends to > > behave a little bit differently on every single platform). > > Could it be a CPU generation thing? My CPU is 12th generation (2022) and > there is another report of a Lenovo P15gen2 (11th generation 2021 I assume) > not reporting PDOs at all. I have an older Dell XPS 9380 which has an 8th > generation CPU (3 USB-C port and no Type A ports) that has no UCSI support. > So do PDOs and the active RDO get properly reported on 13th generation > CPUs? Probable not. It really depends on the embedded controller or PD controller firmware, so ultimately the platform and product. It could be that the reference embedded controller firmware from Intel is only patched ones for every CPU generation (I don't know), but for example Dell does not use Intel's reference embedded controller firmware, or they at least patch it heavily for every product, so the CPU gen should not play a huge role there. Now some (most?) USB PD controllers also support UCSI natively, so the EC firmware may be just a proxy between the OS and the PD controller. The PD controller can be anything regardless of the CPU generation, and theare are several vendors, and on top of that, the PD controller firmwares may be customised for every single product line. thanks,