typecd: Update Cable initialization
Now that SOP' plug alt modes generate udev events (thanks to a fix in
the kernel), fix the cable and cable plug registration to be more
deterministic. This is done by:
- Adding a callback for a udev add event for a cable plug (SOP'): This
gets invoked during daemon init (for pre-connected cables) and whenever
a new cable is connected.
- Adding a callback (and handling) for cable plug (SOP') alt mode add
events: These get generated any time after initial cable plug (SOP')
registration, whenever a cable plug (SOP') alt mode is added.
BUG=b:172097194
TEST=All unit tests still pass. Ensure from the typecd.log file that alt
modes for cables are getting parsed as expected on daemon init and also
on cable hotplug.
Change-Id: I04d19524b887a01e534a95b8f4c5c4498ab879af
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2600216
Tested-by: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Mengqi Guo <mqg@chromium.org>
Reviewed-by: Prashant Malani <pmalani@chromium.org>
Commit-Queue: Prashant Malani <pmalani@chromium.org>
diff --git a/typecd/cable.cc b/typecd/cable.cc
index 92073b8..051f371 100644
--- a/typecd/cable.cc
+++ b/typecd/cable.cc
@@ -17,9 +17,10 @@
namespace typecd {
-void Cable::SearchForAltModes(const base::FilePath& plug_syspath) {
- base::FileEnumerator iter(plug_syspath, false,
- base::FileEnumerator::DIRECTORIES);
+void Cable::RegisterCablePlug(const base::FilePath& syspath) {
+ // Search for all alt modes which were already registered prior to daemon
+ // init.
+ base::FileEnumerator iter(syspath, false, base::FileEnumerator::DIRECTORIES);
for (auto path = iter.Next(); !path.empty(); path = iter.Next())
AddAltMode(path);
}
@@ -31,7 +32,7 @@
return false;
if (IsAltModePresent(index)) {
- LOG(ERROR) << "Alt mode already registered for syspath " << mode_syspath;
+ LOG(INFO) << "Alt mode already registered for syspath " << mode_syspath;
return false;
}
diff --git a/typecd/cable.h b/typecd/cable.h
index ed804e3..c623daa 100644
--- a/typecd/cable.h
+++ b/typecd/cable.h
@@ -21,17 +21,17 @@
// cable.
class Cable : public Peripheral {
public:
- explicit Cable(const base::FilePath& syspath) : Peripheral(syspath) {}
+ explicit Cable(const base::FilePath& syspath)
+ : Peripheral(syspath), num_alt_modes_(-1) {}
Cable(const Cable&) = delete;
Cable& operator=(const Cable&) = delete;
- // The Linux Type C connector class framework doesn't generate udev events for
- // alternate modes added to plugs. Therefore, the alternate mode addition code
- // needs to be handled a little differently from partners.
- //
- // Search |plug_syspath| for directories that follow the SOP' alternate mode
- // regex, and run the |AddAltMode| for each such path.
- void SearchForAltModes(const base::FilePath& plug_syspath);
+ // Register the contents of a cable plug (SOP') for this cable. The Linux
+ // kernel Type C connector class creates two devices for the two types of
+ // cable plugs (SOP' & SOP''). Since the Chrome OS Embedded Controller only
+ // parses SOP', we don't create a separate object for the plug and instead
+ // fold its contents into the Cable object itself.
+ void RegisterCablePlug(const base::FilePath& syspath);
// Add an alternate mode for the plug associated with the cable.
// NOTE: We currently only process SOP' plugs.
diff --git a/typecd/port.cc b/typecd/port.cc
index c07f3c7..e9eff1a 100644
--- a/typecd/port.cc
+++ b/typecd/port.cc
@@ -50,6 +50,15 @@
LOG(INFO) << "Cable removed for port " << port_num_;
}
+void Port::AddCablePlug(const base::FilePath& syspath) {
+ if (!cable_) {
+ LOG(WARNING) << "No cable present for port " << port_num_;
+ return;
+ }
+
+ cable_->RegisterCablePlug(syspath);
+}
+
void Port::AddPartner(const base::FilePath& path) {
if (partner_) {
LOG(WARNING) << "Partner already exists for port " << port_num_;
@@ -93,7 +102,10 @@
return;
}
- cable_->SearchForAltModes(path);
+ if (!cable_->AddAltMode(path)) {
+ LOG(ERROR) << "Failed to add SOP' alt mode for port " << port_num_
+ << " at path " << path;
+ }
}
void Port::PartnerChanged() {
diff --git a/typecd/port.h b/typecd/port.h
index 7577bd1..d455662 100644
--- a/typecd/port.h
+++ b/typecd/port.h
@@ -30,6 +30,8 @@
void AddCable(const base::FilePath& path);
void RemoveCable();
+ void AddCablePlug(const base::FilePath& path);
+
// Add/remove an alternate mode for the partner.
void AddRemovePartnerAltMode(const base::FilePath& path, bool added);
diff --git a/typecd/port_manager.cc b/typecd/port_manager.cc
index 6b4f9a8..8b85c99 100644
--- a/typecd/port_manager.cc
+++ b/typecd/port_manager.cc
@@ -81,6 +81,18 @@
}
}
+void PortManager::OnCablePlugAdded(const base::FilePath& path, int port_num) {
+ auto it = ports_.find(port_num);
+ if (it == ports_.end()) {
+ LOG(WARNING) << "Cable plug (SOP') add attempted for non-existent port "
+ << port_num;
+ return;
+ }
+
+ auto port = it->second.get();
+ port->AddCablePlug(path);
+}
+
void PortManager::OnCableAltModeAdded(const base::FilePath& path,
int port_num) {
auto it = ports_.find(port_num);
diff --git a/typecd/port_manager.h b/typecd/port_manager.h
index 4b7a5c7..e881e35 100644
--- a/typecd/port_manager.h
+++ b/typecd/port_manager.h
@@ -46,6 +46,7 @@
void OnCableAddedOrRemoved(const base::FilePath& path,
int port_num,
bool added) override;
+ void OnCablePlugAdded(const base::FilePath& path, int port_num) override;
void OnCableAltModeAdded(const base::FilePath& path, int port_num) override;
void OnPartnerChanged(int port_num) override;
diff --git a/typecd/test_constants.h b/typecd/test_constants.h
index f5bd50e..1561494 100644
--- a/typecd/test_constants.h
+++ b/typecd/test_constants.h
@@ -39,9 +39,9 @@
"/sys/class/typec/port0/port0-partner";
constexpr char kFakePort0CableSysPath[] = "/sys/class/typec/port0/port0-cable";
constexpr char kFakePort0SOPPrimeAltModeSysPath[] =
- "/sys/class/typec/port0/port0-cable/port0-plug0";
+ "/sys/class/typec/port0/port0-cable/port0-plug0/port0-plug0.0";
constexpr char kFakePort0SOPDoublePrimeAltModeSysPath[] =
- "/sys/class/typec/port0/port0-cable/port0-plug1";
+ "/sys/class/typec/port0/port0-cable/port0-plug1/port0-plug1.0";
} // namespace typecd
diff --git a/typecd/udev_monitor.cc b/typecd/udev_monitor.cc
index f5b91d1..a404d23 100644
--- a/typecd/udev_monitor.cc
+++ b/typecd/udev_monitor.cc
@@ -16,7 +16,8 @@
constexpr char kCableRegex[] = R"(port(\d+)-cable)";
constexpr char kPortRegex[] = R"(port(\d+))";
// TODO(pmalani): Add SOP'' support when the kernel also supports it.
-constexpr char kSOPPrimePlugAltModeRegex[] = R"(port(\d+)-plug0)";
+constexpr char kSOPPrimePlugRegex[] = R"(port(\d+)-plug0)";
+constexpr char kSOPPrimePlugAltModeRegex[] = R"(port(\d+)-plug0.(\d+))";
} // namespace
@@ -115,6 +116,8 @@
observer.OnPartnerAltModeAddedOrRemoved(path, port_num, added);
else if (RE2::FullMatch(name.value(), kCableRegex, &port_num))
observer.OnCableAddedOrRemoved(path, port_num, added);
+ else if (RE2::FullMatch(name.value(), kSOPPrimePlugRegex, &port_num))
+ observer.OnCablePlugAdded(path, port_num);
else if (RE2::FullMatch(name.value(), kSOPPrimePlugAltModeRegex,
&port_num) &&
added)
diff --git a/typecd/udev_monitor.h b/typecd/udev_monitor.h
index b57577d..d1fed9c 100644
--- a/typecd/udev_monitor.h
+++ b/typecd/udev_monitor.h
@@ -86,6 +86,12 @@
int port_num,
bool added) = 0;
+ // Callback that is executed when a cable plug (SOP') device is registered.
+ //
+ // The |path| argument refers to the sysfs device path of the cable plug
+ // (SOP'). The |port_num| argument refers to the port's index number.
+ virtual void OnCablePlugAdded(const base::FilePath& path, int port_num) = 0;
+
// Callback that is executed when a cable (SOP') alternate mode is
// registered.
//
diff --git a/typecd/udev_monitor_test.cc b/typecd/udev_monitor_test.cc
index 31344d9..479c9ec 100644
--- a/typecd/udev_monitor_test.cc
+++ b/typecd/udev_monitor_test.cc
@@ -62,6 +62,8 @@
num_cables_--;
};
+ void OnCablePlugAdded(const base::FilePath& path, int port_num) override {}
+
void OnCableAltModeAdded(const base::FilePath& path, int port_num) override {
num_cable_altmodes_++;
};
