Revert D66529269

Summary: This diff reverts D66529269 This causes PM failing to create SPI devices. The PM hw_test didn't catch the issue because it runs without reload_kmods flag so the SPI devices creation is skipped because they were created by previous PM runs. Reviewed By: somasun Differential Revision: D67064328 fbshipit-source-id: e0772acc79a51d1e8755ca3e81c9c0d7c6a4a432
facebook · Dec 11, 2024 · 9d037a3 · 9d037a3
1 parent 7573ca0
commit 9d037a3
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 32 deletions.
diff --git a/fboss/platform/platform_manager/BUCK b/fboss/platform/platform_manager/BUCK
@@ -87,7 +87,6 @@ cpp_library(
         ":i2c_explorer",
         ":platform_manager_config-cpp2-types",
         ":utils",
-        "//fboss/platform/helpers:platform_fs_utils",
         "//folly:file_util",
         "//folly:string",
         "//folly/logging:logging",

diff --git a/fboss/platform/platform_manager/PciExplorer.cpp b/fboss/platform/platform_manager/PciExplorer.cpp
@@ -10,7 +10,6 @@
 #include <folly/String.h>
 #include <folly/logging/xlog.h>
 
-#include "fboss/platform/helpers/PlatformFsUtils.h"
 #include "fboss/platform/platform_manager/I2cExplorer.h"
 #include "fboss/platform/platform_manager/Utils.h"
 
@@ -51,15 +50,12 @@ fbiob_aux_data getAuxData(
 
 namespace facebook::fboss::platform::platform_manager {
 
-PciDevice::PciDevice(
-    const PciDeviceConfig& pciDeviceConfig,
-    const std::shared_ptr<PlatformFsUtils> platformFsUtils)
+PciDevice::PciDevice(const PciDeviceConfig& pciDeviceConfig)
     : name_(*pciDeviceConfig.pmUnitScopedName()),
       vendorId_(*pciDeviceConfig.vendorId()),
       deviceId_(*pciDeviceConfig.deviceId()),
       subSystemVendorId_(*pciDeviceConfig.subSystemVendorId()),
-      subSystemDeviceId_(*pciDeviceConfig.subSystemDeviceId()),
-      platformFsUtils_(std::move(platformFsUtils)) {
+      subSystemDeviceId_(*pciDeviceConfig.subSystemDeviceId()) {
   checkSysfsReadiness();
 
   // Note: bindDriver() needs to be called after checkSysfsReadiness() but
@@ -130,13 +126,14 @@ void PciDevice::bindDriver(const std::string& desiredDriver) {
     return;
   }
 
-  fs::path desiredDriverPath = fs::path("/sys/bus/pci/drivers") / desiredDriver;
+  auto desiredDriverPath =
+      fmt::format("/sys/bus/pci/drivers/{}", desiredDriver);
   if (!fs::exists(desiredDriverPath)) {
     throw std::runtime_error(fmt::format(
         "Failed to bind driver {} to device {}: {} does not exist",
         desiredDriver,
         name_,
-        desiredDriverPath.string()));
+        desiredDriverPath));
   }
 
   // Add PCI device ID to the driver's "new_id" file. Check below doc for
@@ -153,7 +150,8 @@ void PciDevice::bindDriver(const std::string& desiredDriver) {
       name_,
       pciDevId,
       desiredDriver);
-  platformFsUtils_->writeStringToFile(pciDevId, desiredDriverPath / "new_id");
+  auto cmd = fmt::format("echo {} > {}/new_id", pciDevId, desiredDriverPath);
+  PlatformUtils().execCommand(cmd);
 }
 
 void PciDevice::checkCharDevReadiness() {
@@ -193,9 +191,6 @@ std::string PciDevice::charDevPath() const {
   return charDevPath_;
 }
 
-PciExplorer::PciExplorer(const std::shared_ptr<PlatformFsUtils> platformFsUtils)
-    : platformFsUtils_(std::move(platformFsUtils)) {}
-
 std::vector<uint16_t> PciExplorer::createI2cAdapter(
     const PciDevice& pciDevice,
     const I2cAdapterConfig& i2cAdapterConfig,
@@ -495,12 +490,11 @@ std::map<std::string, std::string> PciExplorer::getSpiDeviceCharDevPaths(
         // For more details on the two commands:
         // https://github.com/torvalds/linux/blob/master/Documentation/spi/spidev.rst#device-creation-driver-binding
         // Overriding driver of the SpiDevice so spidev doesn't fail to probe.
-        platformFsUtils_->writeStringToFile(
-            "spidev",
-            fs::path("/sys/bus/spi/devices") / spiDevId / "driver_override");
+        PlatformUtils().execCommand(fmt::format(
+            "echo spidev > /sys/bus/spi/devices/{}/driver_override", spiDevId));
         // Bind SpiDevice to spidev driver in order to create its char device.
-        platformFsUtils_->writeStringToFile(
-            spiDevId, "/sys/bus/spi/drivers/spidev/bind");
+        PlatformUtils().execCommand(fmt::format(
+            "echo {} > /sys/bus/spi/drivers/spidev/bind", spiDevId));
         XLOG(INFO) << fmt::format(
             "Completed binding SpiDevice {} to {} for SpiController {}",
             spiDevId,

diff --git a/fboss/platform/platform_manager/PciExplorer.h b/fboss/platform/platform_manager/PciExplorer.h
@@ -3,7 +3,6 @@
 #pragma once
 
 #include <re2/re2.h>
-#include "fboss/platform/helpers/PlatformFsUtils.h"
 
 #include "fboss/platform/platform_manager/gen-cpp2/platform_manager_config_types.h"
 #include "fboss/platform/platform_manager/uapi/fbiob-ioctl.h"
@@ -12,10 +11,7 @@ namespace facebook::fboss::platform::platform_manager {
 
 struct PciDevice {
  public:
-  explicit PciDevice(
-      const PciDeviceConfig& pciDevConfig,
-      const std::shared_ptr<PlatformFsUtils> platformFsUtils =
-          std::make_shared<PlatformFsUtils>());
+  explicit PciDevice(const PciDeviceConfig& pciDevConfig);
   std::string sysfsPath() const;
   std::string charDevPath() const;
 
@@ -27,7 +23,6 @@ struct PciDevice {
   std::string subSystemDeviceId_{};
   std::string charDevPath_{};
   std::string sysfsPath_{};
-  const std::shared_ptr<PlatformFsUtils> platformFsUtils_;
 
   void checkSysfsReadiness();
   void bindDriver(const std::string& desiredDriver);
@@ -36,9 +31,6 @@ struct PciDevice {
 
 class PciExplorer {
  public:
-  explicit PciExplorer(
-      const std::shared_ptr<PlatformFsUtils> platformFsUtils =
-          std::make_shared<PlatformFsUtils>());
   // Create the I2C Adapter based on the given i2cAdapterConfig residing
   // at the given PciDevice path. It returns the the kernel assigned i2c bus
   // number(s) for the created adapter(s). Throw std::runtime_error on failure.
@@ -127,8 +119,6 @@ class PciExplorer {
       uint32_t instanceId);
 
  private:
-  const std::shared_ptr<PlatformFsUtils> platformFsUtils_;
-
   std::vector<uint16_t> getI2cAdapterBusNums(
       const PciDevice& pciDevice,
       const I2cAdapterConfig& i2cAdapterConfig,

diff --git a/fboss/platform/platform_manager/PlatformExplorer.cpp b/fboss/platform/platform_manager/PlatformExplorer.cpp
@@ -139,7 +139,6 @@ PlatformExplorer::PlatformExplorer(
     const PlatformConfig& config,
     const std::shared_ptr<PlatformFsUtils> platformFsUtils)
     : platformConfig_(config),
-      pciExplorer_(platformFsUtils),
       dataStore_(platformConfig_),
       devicePathResolver_(dataStore_),
       presenceChecker_(devicePathResolver_),
@@ -462,7 +461,7 @@ void PlatformExplorer::explorePciDevices(
     const std::vector<PciDeviceConfig>& pciDeviceConfigs) {
   for (const auto& pciDeviceConfig : pciDeviceConfigs) {
     try {
-      auto pciDevice = PciDevice(pciDeviceConfig, platformFsUtils_);
+      auto pciDevice = PciDevice(pciDeviceConfig);
       auto charDevPath = pciDevice.charDevPath();
       auto instId =
           getFpgaInstanceId(slotPath, *pciDeviceConfig.pmUnitScopedName());

diff --git a/fboss/platform/platform_manager/PlatformExplorer.h b/fboss/platform/platform_manager/PlatformExplorer.h
@@ -112,7 +112,7 @@ class PlatformExplorer {
 
   PlatformConfig platformConfig_{};
   I2cExplorer i2cExplorer_{};
-  PciExplorer pciExplorer_;
+  PciExplorer pciExplorer_{};
   CachedFbossEepromParser eepromParser_{};
   DataStore dataStore_;
   DevicePathResolver devicePathResolver_;