power: fix USB power supply type on newer kernels
When initializing a PowerStatus::Port from sysfs, use both the type and
usb_type files to determine the power supply type.
Some drivers in newer kernels (4.19+) report a static type of USB, and
separately report all supported connection types in usb_type, with the
active value in brackets.
Powerd was already partially updated to handle this new file in
crrev.com/c/1684359, but only for dual-role ports. Rework this fix so
that port.type takes both the type and usb_type files into account. To
maintain compatibility across kernel versions, "USB_" is prefixed to the
usb_type when appropriate.
Added a new unit test for usb_type that partially borrows from the
DualRolePowerSources test. Removed one of the values from
kInvalidUsbTypeValues that isn't actually invalid.
BUG=b:204295196
TEST=cros_workon_make --test --install power_manager
TEST=Deploy to a Lenovo X1 Gen8 (reven board) and connect a high-power USB-C adapter.
TEST=In the settings, check that the power source shows as an AC adapter.
Change-Id: I1d507de6aa3cc8c1f9ab62e5373d30a02311faa9
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3370303
Tested-by: Nicholas Bishop <nicholasbishop@google.com>
Reviewed-by: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Mengqi Guo <mqg@chromium.org>
Reviewed-by: Benson Leung <bleung@google.com>
Commit-Queue: Nicholas Bishop <nicholasbishop@google.com>
diff --git a/power_manager/docs/power_supplies.md b/power_manager/docs/power_supplies.md
index 77c3785..350ada7 100644
--- a/power_manager/docs/power_supplies.md
+++ b/power_manager/docs/power_supplies.md
@@ -35,11 +35,17 @@
port on a pre-USB-C device.
* `USB`, `USB_ACA`, `USB_CDP`, `USB_DCP` - A low-power USB BC1.2 power
supply. USB-C ports with nothing connected to them may report a `USB`
- type in conjunction with an `online` value of `0`.
+ type in conjunction with an `online` value of `0`. Note that some
+ drivers use a static type of `USB` and report the active connection in
+ `usb_type`, described below.
* `USB_C`, `USB_PD` - A USB Type-C power supply.
* `USB_PD_DRP` - A dual-role USB Type-C device (i.e. one capable of either
delivering or receiving power).
* `BrickID` - An Apple legacy USB charger. May be either USB-A or USB-C.
+* `usb_type` is used by some drivers in newer kernels (4.19+) that have a
+ `type` of USB to report all supported connection types, with the active
+ connection value in brackets. For example, a `usb_type` of `C [PD] PD_PPS`
+ is the same as a `type` of `USB_PD`.
* `online` typically describes whether anything is connected to the port; a
value of `1` indicates a connection. If `type` is `USB_PD_DRP`, an `online`
value of `0` indicates that a dual-role device is connected but that it is
diff --git a/power_manager/powerd/system/power_supply.cc b/power_manager/powerd/system/power_supply.cc
index ad7e78c..0a593a3 100644
--- a/power_manager/powerd/system/power_supply.cc
+++ b/power_manager/powerd/system/power_supply.cc
@@ -121,19 +121,35 @@
base::CompareCase::SENSITIVE);
}
-// Returns true if |type|, a power supply type read from a "type" file in
-// sysfs, indicates USB_PD_DRP, meaning a USB Power Delivery Dual Role Port.
-bool IsDualRoleType(const std::string& type, const base::FilePath& path) {
- // 4.19+ kernels have the type as just "USB", and an extra usb_type file
- // in the form:
- // Unknown SDP DCP CDP C PD [PD_DRP] BrickID
- if (type == PowerSupply::kUsbType) {
- std::string usb_type;
- if (ReadBracketSelectedString(path, "usb_type", &usb_type))
- return IsPdDrpType(usb_type);
- }
+// Returns the type of connection for the power supply. If the type
+// cannot be read, kUnknownType is returned.
+std::string ReadPowerSupplyType(const base::FilePath& path) {
+ std::string type;
+ if (!ReadAndTrimString(path, "type", &type))
+ return PowerSupply::kUnknownType;
- return IsPdDrpType(type);
+ if (type != PowerSupply::kUsbType)
+ return type;
+
+ // Some drivers in newer kernels (4.19+) report a static type of USB,
+ // and separately report all supported connection types in a usb_type
+ // file, with the active value in brackets. For example:
+ // "Unknown SDP DCP CDP C PD [PD_DRP] BrickID".
+ std::string usb_type;
+ if (!ReadBracketSelectedString(path, "usb_type", &usb_type))
+ return PowerSupply::kUsbType;
+
+ // The exact type is unknown, but we still know it's USB.
+ if (usb_type == PowerSupply::kUnknownType || usb_type.empty())
+ return PowerSupply::kUsbType;
+
+ // For compatibility with the old dynamic type, prepend "USB_"
+ // to the type. The only exception is for the BrickId type,
+ // which is unprefixed.
+ if (usb_type != PowerSupply::kBrickIdType)
+ usb_type = "USB_" + usb_type;
+
+ return usb_type;
}
// Returns true if |path|, a sysfs directory, corresponds to an external
@@ -947,11 +963,11 @@
status->supports_dual_role_devices = true;
// An "Unknown" type indicates a sink-only device that can't supply power.
- ReadAndTrimString(path, "type", &port->type);
+ port->type = ReadPowerSupplyType(path);
if (port->type == kUnknownType)
return;
- const bool dual_role_connected = IsDualRoleType(port->type, path);
+ const bool dual_role_connected = IsPdDrpType(port->type);
// If "online" is 0, nothing is connected unless it is USB_PD_DRP, in which
// case a value of 0 indicates we're connected to a dual-role device but not
diff --git a/power_manager/powerd/system/power_supply_test.cc b/power_manager/powerd/system/power_supply_test.cc
index 692f6c3..0769fe8 100644
--- a/power_manager/powerd/system/power_supply_test.cc
+++ b/power_manager/powerd/system/power_supply_test.cc
@@ -39,6 +39,7 @@
const char* const kMainsType = PowerSupply::kMainsType;
const char* const kBatteryType = PowerSupply::kBatteryType;
const char* const kUsbType = PowerSupply::kUsbType;
+const char* const kUsbPdType = PowerSupply::kUsbPdType;
const char* const kUsbPdDrpType = PowerSupply::kUsbPdDrpType;
const char* const kUnknownType = PowerSupply::kUnknownType;
@@ -65,6 +66,15 @@
// Starting value used by |power_supply_| as "now".
const base::TimeTicks kStartTime = base::TimeTicks::FromInternalValue(1000);
+// Invalid values for usb_type.
+const char* kInvalidUsbTypeValues[] = {
+ "Unknown SDP DCP CDP C PD PD_DRP BrickID",
+ "Unknown SDP DCP CDP C PD [] BrickID",
+ "Unknown SDP DCP CDP C PD ]PD_DRP[ BrickID",
+ "[",
+ "]",
+ "[]"};
+
class TestObserver : public PowerSupplyObserver {
public:
explicit TestObserver(PowerSupply* power_supply)
@@ -463,6 +473,39 @@
EXPECT_FALSE(power_status.supports_dual_role_devices);
}
+// Test that the supply type is correctly read from usb_type when present.
+TEST_F(PowerSupplyTest, LinePowerWithUsbType) {
+ WriteDefaultValues(PowerSource::AC);
+ UpdatePowerSourceAndBatteryStatus(PowerSource::AC, kUsbType, kCharging);
+ Init();
+
+ // With the type set to USB and no usb_type set, the supply is treated
+ // as a low-power USB connection.
+ PowerStatus power_status;
+ ASSERT_TRUE(UpdateStatus(&power_status));
+ EXPECT_EQ(kUsbType, power_status.ports[0].type);
+ EXPECT_EQ(PowerSupplyProperties_ExternalPower_USB,
+ power_status.external_power);
+
+ // With usb_type set to PD, the supply is treated as AC.
+ WriteValue(ac_dir_, "usb_type", "C [PD] PD_PPS");
+ ASSERT_TRUE(UpdateStatus(&power_status));
+ EXPECT_EQ(kUsbPdType, power_status.ports[0].type);
+ EXPECT_EQ(PowerSupplyProperties_ExternalPower_AC,
+ power_status.external_power);
+
+ // Invalid usb_type values should report as low-power USB.
+ for (size_t i = 0; i < std::size(kInvalidUsbTypeValues); ++i) {
+ const char* kType = kInvalidUsbTypeValues[i];
+ SCOPED_TRACE(kType);
+ WriteValue(ac_dir_, "usb_type", kType);
+ ASSERT_TRUE(UpdateStatus(&power_status));
+ ASSERT_EQ(kUsbType, power_status.ports[0].type);
+ EXPECT_EQ(PowerSupplyProperties_ExternalPower_USB,
+ power_status.external_power);
+ }
+}
+
// Tests that when multiple line power sources are reported (e.g. because both
// the PD and ACPI drivers are present), powerd favors the non-Mains source.
TEST_F(PowerSupplyTest, MultipleLinePowerSources) {
@@ -730,14 +773,6 @@
ASSERT_EQ(Role::DUAL_ROLE, status.ports[1].role);
// USB should not report as dual role if usb_type is craaaazy.
- const char* const kInvalidUsbTypeValues[] = {
- "Unknown SDP DCP CDP C PD PD_DRP BrickID",
- "Unknown SDP DCP CDP C PD [] BrickID",
- "Unknown SDP DCP CDP C [PD] PD_DRP BrickID",
- "Unknown SDP DCP CDP C PD ]PD_DRP[ BrickID",
- "[",
- "]",
- "[]"};
for (size_t i = 0; i < std::size(kInvalidUsbTypeValues); ++i) {
const char* kType = kInvalidUsbTypeValues[i];
SCOPED_TRACE(kType);
