| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3zvstdxhgobn64ttcggmk44ejsroxmwe7y25rraebh3p4evsy6@3g64a5tuvt7w>
Date: Sat, 23 Nov 2024 18:32:07 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Song Xue <quic_songxue@...cinc.com>
Cc: Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>, Manu Gautam <mgautam@...eaurora.org>,
Vivek Gautam <vivek.gautam@...eaurora.org>, kernel@...cinc.com, linux-arm-msm@...r.kernel.org,
linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] phy: qcom-qusb2: Add regulator_set_load to Qualcomm
usb phy
On Sat, Nov 23, 2024 at 01:07:06PM +0800, Song Xue wrote:
>
>
> On 11/22/2024 6:24 AM, Dmitry Baryshkov 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@...cinc.com>
> > > ---
> > > 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@quicinc.com
> > > ---
> > > 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);
> >
> > Please use regulator_set_mode() instead. Or just fix the mode in the
> > device tree, if the device can not operate if the regulator is in
> > non-HPM mode.
> >
> Thanks for comment.
>
> From my point, regulator_set_mode() will change the regulator's operating
> mode including current and voltage, which will also influence the other
> shared consumers. Meanwhile it is unacceptable to fix mode in the device
> tree because it is determined by regulator's device tree.
>
> According to the required fix, regulator_set_load() simply aggregates the
> current load for the regulator and does not affect other shared consumers.
> Setting the current load is relevant to the issue.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>
> Regards,
> Song
> > > + 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@...cinc.com>
> > >
> >
>
>
> --
> linux-phy mailing list
> linux-phy@...ts.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy
--
With best wishes
Dmitry
Powered by blists - more mailing lists