From 040915a7627042784124cbfe11822cb2f63c8759 Mon Sep 17 00:00:00 2001 From: Anduin1109 Date: Thu, 4 Sep 2025 18:46:02 +0800 Subject: [PATCH 1/5] fix setPortRole bugs Signed-off-by: Anduin1109 --- usb/hdi_service/include/usbd_port.h | 9 +++ usb/hdi_service/src/usbd_port.cpp | 104 +++++++++++++++++++++++++--- 2 files changed, 102 insertions(+), 11 deletions(-) diff --git a/usb/hdi_service/include/usbd_port.h b/usb/hdi_service/include/usbd_port.h index 2e1bc18d70..7c64bea8f0 100644 --- a/usb/hdi_service/include/usbd_port.h +++ b/usb/hdi_service/include/usbd_port.h @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include "usb_host_data.h" #include "usbd.h" @@ -28,6 +30,7 @@ #include "v2_0/iusbd_subscriber.h" #include "v2_0/iusb_device_interface.h" #define DEFAULT_PORT_ID 1 +#define MAX_SWITCH_SEM 10 #define DATA_ROLE_NONE_STR "none" #define DATA_ROLE_UFP_STR "host" @@ -100,6 +103,8 @@ public: int32_t QueryPort(int32_t &portId, int32_t &powerRole, int32_t &dataRole, int32_t &mode); int32_t UpdatePort(int32_t mode, const sptr &subscriber); int32_t UpdateUsbPort(int32_t mode, const sptr &subscriber); + void waitPowerSwitch(uint32_t timeout); + void finishPowerSwitch(); void setPortPath(const std::string &path); int32_t GetSupportedModes(int32_t &supported_modes); @@ -118,6 +123,8 @@ private: int32_t ReadPortFile(int32_t &role, const std::string &subPath); int32_t SetPortInit(int32_t portId, int32_t powerRole, int32_t dataRole); int32_t WritePdPortFile(int32_t powerRole, int32_t dataRole); + int32_t SetPowerRole(int32_t powerRole, bool needSwitchDataRole); + int32_t SetDataRole(int32_t dataRole); void QueryPdPort(int32_t &powerRole, int32_t &dataRole, int32_t &mode); void UpdatePdPort(int32_t mode, const sptr &subscriber); void UpdatePdPorts(int32_t mode, const sptr &subscriber); @@ -131,6 +138,8 @@ private: std::string DEFAULT_USB_MODE_PATH = "/data/service/el1/public/usb/mode"; bool isPdV2_0 = false; sptr usbDeviceInterface_ = nullptr; + std::counting_semaphore powerSwitchSemaphore_{0}; + bool isPowerSwitching_ = false; }; } // namespace V1_2 } // namespace Usb diff --git a/usb/hdi_service/src/usbd_port.cpp b/usb/hdi_service/src/usbd_port.cpp index 0302a787d1..9ab43b11f0 100644 --- a/usb/hdi_service/src/usbd_port.cpp +++ b/usb/hdi_service/src/usbd_port.cpp @@ -29,6 +29,9 @@ namespace Usb { namespace V1_2 { constexpr uint32_t PERSIST_CONFIG_NAME_MAX_LEN = 32; +constexpr uint32_t POWER_SWITCH_TIMEOUT_SECOND = 2; +constexpr uint32_t RETRY_INTERVAL_MS = 500; +constexpr uint32_t RETRY_TIMES = 3; UsbdPort &UsbdPort::GetInstance() { @@ -234,29 +237,24 @@ int32_t UsbdPort::SetPortInit(int32_t portId, int32_t powerRole, int32_t dataRol return HDF_SUCCESS; } + bool needSwitchDataRole = (currentPortInfo_.dataRole != dataRole); if (currentPortInfo_.powerRole != powerRole) { HDF_LOGI("%{public}s: powerRole switch from %{public}d to %{public}d", __func__, currentPortInfo_.powerRole, powerRole); - ret = WritePortFile(powerRole, POWER_ROLE_PATH); + ret = SetPowerRole(powerRole, needSwitchDataRole); if (ret != HDF_SUCCESS) { - HDF_LOGE("%{public}s: write powerRole failed, ret: %{public}d", __func__, ret); + HDF_LOGE("%{public}s: switch powerRole failed, ret: %{public}d", __func__, ret); return ret; } } - if (currentPortInfo_.dataRole != dataRole) { + if (needSwitchDataRole) { HDF_LOGI("%{public}s: dataRole switch from %{public}d to %{public}d", __func__, currentPortInfo_.dataRole, dataRole); - ret = WritePortFile(dataRole, DATA_ROLE_PATH); + ret = SetDataRole(dataRole); if (ret != HDF_SUCCESS) { - HDF_LOGE("%{public}s: write dataRole failed, ret: %{public}d", __func__, ret); + HDF_LOGE("%{public}s: switch dataRole failed, ret: %{public}d", __func__, ret); return ret; } - if (currentPortInfo_.dataRole == DATA_ROLE_DEVICE && dataRole == DATA_ROLE_HOST) { - ret = SwitchFunction(DATA_ROLE_HOST); - } - if (currentPortInfo_.dataRole == DATA_ROLE_HOST && dataRole == DATA_ROLE_DEVICE) { - ret = SwitchFunction(DATA_ROLE_DEVICE); - } } currentPortInfo_.portId = portId; currentPortInfo_.powerRole = powerRole; @@ -264,6 +262,90 @@ int32_t UsbdPort::SetPortInit(int32_t portId, int32_t powerRole, int32_t dataRol return HDF_SUCCESS; } +int32_t UsbdPort::SetPowerRole(int32_t powerRole, bool needSwitchDataRole) +{ + HDF_LOGI("%{public}s: enter", __func__); + int32_t ret = WritePortFile(powerRole, POWER_ROLE_PATH); + if (ret != HDF_SUCCESS) { + HDF_LOGE("%{public}s: write powerRole failed, ret: %{public}d", __func__, ret); + return ret; + } + if (!needSwitchDataRole || path_.find("/otg_default") != std::string::npos) { + // otg_default does not need to wait semaphore here + return HDF_SUCCESS; + } + + // if going to switch data role, first we need to finish power role switching + waitPowerSwitch(POWER_SWITCH_TIMEOUT_SECOND); + int32_t devRole = -1; + ret = ReadPortFile(devRole, POWER_ROLE_PATH); + if (ret != HDF_SUCCESS || devRole != powerRole) { + HDF_LOGE("%{public}s: device power role %{public}d does not match target role %{public}d", + __func__, devRole, powerRole); + return HDF_FAILURE; + } + return HDF_SUCCESS; +} + +int32_t UsbdPort::SetDataRole(int32_t dataRole) +{ + HDF_LOGI("%{public}s: enter", __func__); + uint32_t retryTimes = RETRY_TIMES; + int32_t ret = -1; + int32_t devRole = -1; + + do { + ret = WritePortFile(dataRole, DATA_ROLE_PATH); + if (ret != HDF_SUCCESS) { + HDF_LOGE("%{public}s: write dataRole failed, ret: %{public}d", __func__, ret); + return ret; + } + ret = ReadPortFile(devRole, DATA_ROLE_PATH); + if (ret == HDF_SUCCESS && devRole == dataRole) { + break; + } + HDF_LOGE("%{public}s: device data role %{public}d does not match target role %{public}d", + __func__, devRole, dataRole); + std::this_thread::sleep_for(std::chrono::milliseconds(RETRY_INTERVAL_MS)); + } while (--retryTimes > 0); + + if (ret != HDF_SUCCESS || devRole != powerRole) { + HDF_LOGE("%{public}s: failed, no retryTimes left", __func__); + return HDF_FAILURE; + } + + if (currentPortInfo_.dataRole == DATA_ROLE_DEVICE && dataRole == DATA_ROLE_HOST) { + ret = SwitchFunction(DATA_ROLE_HOST); + } + if (currentPortInfo_.dataRole == DATA_ROLE_HOST && dataRole == DATA_ROLE_DEVICE) { + ret = SwitchFunction(DATA_ROLE_DEVICE); + } + if (ret != HDF_SUCCESS) { + HDF_LOGW("%{public}s: switch function failed, ret = %{public}d", __func__, ret); + } + return HDF_SUCCESS; +} + +void UsbdPort::waitPowerSwitch(uint32_t timeout) +{ + isPowerSwitching_ = true; + if (powerSwitchSemaphore_.try_acquire_for(std::chrono::seconds(timeout))) { + isPowerSwitching_ = false; + HDF_LOGI("%{public}s: powerRole switch finished", __func__); + return; + } + isPowerSwitching_ = false; + HDF_LOGE("%{public}s: failed to get powerRole switch callback", __func__); + return; +} + +void UsbdPort::finishPowerSwitch() +{ + if (isPowerSwitching_) { + powerSwitchSemaphore_.release(); + } +} + bool UsbdPort::IsHdcOpen() { char persistConfig[PERSIST_CONFIG_NAME_MAX_LEN] = {0}; -- Gitee From a723b6e54c9af10f2e19ed931e95148ea912a766 Mon Sep 17 00:00:00 2001 From: Anduin1109 Date: Thu, 4 Sep 2025 18:48:44 +0800 Subject: [PATCH 2/5] fix setPortRole bugs Signed-off-by: Anduin1109 --- usb/hdi_service/src/usb_port_impl.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/usb/hdi_service/src/usb_port_impl.cpp b/usb/hdi_service/src/usb_port_impl.cpp index 5ccf7b13ae..59f0fa877f 100644 --- a/usb/hdi_service/src/usb_port_impl.cpp +++ b/usb/hdi_service/src/usb_port_impl.cpp @@ -168,14 +168,18 @@ int32_t UsbPortImpl::UsbdPnpLoaderEventReceived(void *priv, uint32_t id, HdfSBuf if (g_productFlag) { return V1_2::UsbdPorts::GetInstance().UpdatePort(PORT_MODE_HOST, subscriber); } else { - return V1_2::UsbdPort::GetInstance().UpdateUsbPort(PORT_MODE_HOST, subscriber); + auto ret = V1_2::UsbdPort::GetInstance().UpdateUsbPort(PORT_MODE_HOST, subscriber); + V1_2::UsbdPort::GetInstance().finishPowerSwitch(); + return ret; } } else if (id == USB_PNP_DRIVER_PORT_DEVICE) { HITRACE_METER_NAME(HITRACE_TAG_HDF, "USB_PNP_DRIVER_PORT_DEVICE"); if (g_productFlag) { return V1_2::UsbdPorts::GetInstance().UpdatePort(PORT_MODE_DEVICE, subscriber); } else { - return V1_2::UsbdPort::GetInstance().UpdateUsbPort(PORT_MODE_DEVICE, subscriber); + auto ret = V1_2::UsbdPort::GetInstance().UpdateUsbPort(PORT_MODE_DEVICE, subscriber); + V1_2::UsbdPort::GetInstance().finishPowerSwitch(); + return ret; } } else { HDF_LOGW("%{public}s: port not support this id %{public}u", __func__, id); -- Gitee From 3fd42f11d6efc725ad21650efebc978349f25d45 Mon Sep 17 00:00:00 2001 From: Anduin1109 Date: Thu, 4 Sep 2025 20:44:49 +0800 Subject: [PATCH 3/5] fix setPortRole bugs Signed-off-by: Anduin1109 --- usb/hdi_service/include/usbd_port.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usb/hdi_service/include/usbd_port.h b/usb/hdi_service/include/usbd_port.h index 7c64bea8f0..49b7692141 100644 --- a/usb/hdi_service/include/usbd_port.h +++ b/usb/hdi_service/include/usbd_port.h @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include #include "usb_host_data.h" -- Gitee From af95370e7b40a115ad1a3821e3265c48076af21d Mon Sep 17 00:00:00 2001 From: Anduin1109 Date: Thu, 4 Sep 2025 20:52:40 +0800 Subject: [PATCH 4/5] fix setPortRole bugs Signed-off-by: Anduin1109 --- usb/hdi_service/src/usbd_port.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usb/hdi_service/src/usbd_port.cpp b/usb/hdi_service/src/usbd_port.cpp index 9ab43b11f0..aa3996f633 100644 --- a/usb/hdi_service/src/usbd_port.cpp +++ b/usb/hdi_service/src/usbd_port.cpp @@ -309,7 +309,7 @@ int32_t UsbdPort::SetDataRole(int32_t dataRole) std::this_thread::sleep_for(std::chrono::milliseconds(RETRY_INTERVAL_MS)); } while (--retryTimes > 0); - if (ret != HDF_SUCCESS || devRole != powerRole) { + if (ret != HDF_SUCCESS || devRole != dataRole) { HDF_LOGE("%{public}s: failed, no retryTimes left", __func__); return HDF_FAILURE; } -- Gitee From d66dc05c2f0399d2db7558d2bda20f1ac340878a Mon Sep 17 00:00:00 2001 From: Anduin1109 Date: Thu, 4 Sep 2025 21:01:19 +0800 Subject: [PATCH 5/5] fix setPortRole bugs Signed-off-by: Anduin1109 --- usb/hdi_service/include/usbd_port.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/usb/hdi_service/include/usbd_port.h b/usb/hdi_service/include/usbd_port.h index 49b7692141..8a1c261ec7 100644 --- a/usb/hdi_service/include/usbd_port.h +++ b/usb/hdi_service/include/usbd_port.h @@ -30,7 +30,6 @@ #include "v2_0/iusbd_subscriber.h" #include "v2_0/iusb_device_interface.h" #define DEFAULT_PORT_ID 1 -#define MAX_SWITCH_SEM 10 #define DATA_ROLE_NONE_STR "none" #define DATA_ROLE_UFP_STR "host" @@ -138,7 +137,7 @@ private: std::string DEFAULT_USB_MODE_PATH = "/data/service/el1/public/usb/mode"; bool isPdV2_0 = false; sptr usbDeviceInterface_ = nullptr; - std::counting_semaphore powerSwitchSemaphore_{0}; + std::binary_semaphore powerSwitchSemaphore_{0}; bool isPowerSwitching_ = false; }; } // namespace V1_2 -- Gitee