linux-kernel - Re: [PATCH V1 4/4] phy: qcom-qmp-ufs: read max-microamp values from device tree

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <acf89420-743b-4178-ac05-d4ca492bfee3@sirena.org.uk>
Date: Thu, 7 Aug 2025 14:44:58 +0100
From: Mark Brown <broonie@...nel.org>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Nitin Rawat <quic_nitirawa@...cinc.com>, vkoul@...nel.org,
	kishon@...nel.org, mani@...nel.org, conor+dt@...nel.org,
	bvanassche@....org, andersson@...nel.org, neil.armstrong@...aro.org,
	dmitry.baryshkov@....qualcomm.com, konradybcio@...nel.org,
	krzk+dt@...nel.org, linux-arm-msm@...r.kernel.org,
	linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH V1 4/4] phy: qcom-qmp-ufs: read max-microamp values from
 device tree

On Thu, Aug 07, 2025 at 03:06:01PM +0200, Konrad Dybcio wrote:
> On 8/6/25 6:51 PM, Mark Brown wrote:

> > I'm not clear why the driver is trying to do this at all, the driver is
> > AFAICT making no other effort to manage the load at all.  We already
> > impose any constraints that are defined for a regulator while initially
> > parsing them so it's not clear to me what this is supposed to
> > accomplish, and it'll be broken if the supply is ever shared since it'll
> > set the load from this individual consumer to the maximum that's
> > permitted for the regulator as a whole.

> Qualcomm regulators feature a low- and a high-power mode. As one may
> imagine, low- is preferred, and high- needs to be engaged if we go
> over a current threshold.

Sure, but the driver is like I say doing nothing to actively manage the
current reporting.  It's just pulling a random number not specific to
the device (the max-microamp configuration is part of the constraints
which apply to the regualtor as a whole) out of the DT and throwing it
at the framework.

> The specific regulator instances in question are often shared between
> a couple PHYs (UFS, PCIe, USB..) and we need to convey to the
> framework how much each consumer requires (and consumers can of course
> go on/off at runtime). The current value varies between platforms, so
> we want to read from DT.

In that case this will definitely encounter the bug I mentioned above
where it's trying to read the maximum load permitted for the regulator
as a whole and report it as the load from this one specific device.

> The intended use is to set the load requirement and then only en/disable
> the consumer, so that the current load is updated in core (like in the
> kerneldoc of _regulator_handle_consumer_enable())

> My question was about moving the custom parsing of
> $supplyname-max-micromap introduced in this patch into the regulator
> core, as this seems like a rather common problem.

Wait, is this supposed to be some new property that you want to
standardise?  I didn't see a proposal for that, it's not something that
currently exists - the only standard properties that currently exist are
for the regulator as a whole.  

I'm not super convinced this is a particularly common issue, most
devices perform pretty much the same regardless of the board design so
the driver should just know their peak consumption and it's becoming
less and less common for regulators to need software help to adapt to
loads.  The main use case these days seems to be for safety (eg,
constraining how much can be drawn via a USB port) which seems like it'd
either be for the full supply or just known by the driver.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)