Re: [PATCH v2] phy: qcom-qusb2: Add regulator_set_load to Qualcomm usb phy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/29/2024 12:43 AM, Bjorn Andersson wrote:
On Thu, Nov 21, 2024 at 04:09:27PM +0800, Song Xue wrote:
Set the current load before enable regulator supplies at QUSB phy.

Encountered one issue where the board powered down instantly once the UVC
camera was attached to USB port while adding host mode on usb port and
testing a UVC camera with the driver on QCS615 platform. The extensible
boot loader mentioned that OCP(Over Current Protection) occurred at LDO12
from regulators-0 upon powered on board again. That indicates that the
current load set for QUSB phy, which use the regulator supply, is lower
than expected.

As per QUSB spec, set the maximum current load at 30mA to avoid overcurrent
load when attach a device to the USB port.

Fixes: 937e17f36a32 ("phy: qcom-qusb2: Power-on PHY before initialization")
Signed-off-by: Song Xue <quic_songxue@xxxxxxxxxxx>

The patch looks good. But if we describe the regulator(s) with
regulator-allow-set-load; and not all the consumers vote for load, the
sum of the load when USB phy is disabled goes to 0 and we will enter
LPM.

Hi, Bjorn

Thanks for comment.

We dived into the code and found the other all Qualcomm platform's device tree using the phy-qcom-qusb2's compatible don't use the "regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>" and "regulator-allow-set-load" together at the same time. We think the patch is safe now.

Therefore, can we merge the patch?

Thanks,
Song
For this reason we're not doing any load requests today. Can you confirm
that this works fine with a dtb where only HPM is permitted (as well as
LPM and HPM)? If so I'd be in favor of us merging this change, but
keeping the dts HPM-only until someone confirms that all consumers of
these regulators specify load-votes.

Regards,
Bjorn

---
Changes in v2:
- Removed "---" above the Fixes.
- Link to v1: https://lore.kernel.org/r/20241121-add_set_load_to_qusb_phy-v1-1-0f44f3a3290e@xxxxxxxxxxx
---
  drivers/phy/qualcomm/phy-qcom-qusb2.c | 13 ++++++++++++-
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index c52655a383cef008552ed4533b9f31d1cbf34a13..80f0d17c42717e843937255a9a780bbae5998535 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -722,16 +722,27 @@ static int __maybe_unused qusb2_phy_runtime_resume(struct device *dev)
  	return ret;
  }
+#define QUSB2PHY_HPM_LOAD 30000 /*uA*/
+
  static int qusb2_phy_init(struct phy *phy)
  {
  	struct qusb2_phy *qphy = phy_get_drvdata(phy);
  	const struct qusb2_phy_cfg *cfg = qphy->cfg;
  	unsigned int val = 0;
  	unsigned int clk_scheme;
-	int ret;
+	int ret, i;
dev_vdbg(&phy->dev, "%s(): Initializing QUSB2 phy\n", __func__); + /* set the current load */
+	for (i = 0; i < ARRAY_SIZE(qphy->vregs); i++) {
+		ret = regulator_set_load(qphy->vregs[i].consumer, QUSB2PHY_HPM_LOAD);
+		if (ret) {
+			dev_err(&phy->dev, "failed to set load at %s\n", qphy->vregs[i].supply);
+			return ret;
+		}
+	}
+
  	/* turn on regulator supplies */
  	ret = regulator_bulk_enable(ARRAY_SIZE(qphy->vregs), qphy->vregs);
  	if (ret)

---
base-commit: decc701f41d07481893fdea942c0ac6b226e84cd
change-id: 20241121-add_set_load_to_qusb_phy-d1327c797ffe

Best regards,
--
Song Xue <quic_songxue@xxxxxxxxxxx>







[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux