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: <599b8a4b-324a-4543-ba27-0451f05c3dfd@quicinc.com>
Date: Thu, 7 Aug 2025 21:12:53 +0530
From: Nitin Rawat <quic_nitirawa@...cinc.com>
To: Mark Brown <broonie@...nel.org>,
        Konrad Dybcio
	<konrad.dybcio@....qualcomm.com>
CC: <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 8/7/2025 7:14 PM, Mark Brown wrote:
> 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.


Hi Mark,

The UFS QMP PHY driver is not the first client to use regulator_set_load 
or alternatively set load requirements and invoke enable/disable or 
alternatively

Similar to other PHY drivers (such as USB, Display, and Combo PHYs),
as well as various subsystem drivers like UFS, SDHCI, Bluetooth, and 
others, the QMP UFS PHY driver is communicating/setting its load 
requirements.

These drivers also define corresponding binding properties, as seen in 
the UFS instances documented here:

UFS Common DT Binding ((link - 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/ufs/ufs-common.yaml?h=next-20250807) 


Relevant properties include:

vcc-max-microamp: Specifies the maximum load that can be drawn from the 
VCC supply
vccq-max-microamp: Specifies the maximum load that can be drawn from the 
VCCQ supply
vccq2-max-microamp: Specifies the maximum load that can be drawn from 
the VCCQ2 supply

There was a previous effort to introduce similar properties 
(vdda-phy-max-microamp and vdda-pll-max-microamp) in the device tree 
bindings.
Link - (link- 
https://patchwork.kernel.org/project/linux-arm-msm/patch/20220418205509.1102109-3-bhupesh.sharma@linaro.org/#24820481)

However, at that time, the driver-side implementation for aggregating 
load requests was not in place, which led to the patch not being merged:
Patch Reference

Currently, the regulator framework does support automatic aggregation of 
load requests from multiple client drivers. Therefore, it is reasonable 
and necessary for each client to individually communicate its expected 
runtime load to the regulator framework to put the regulators in current
operation mode.

Regards,
Nitin

> 
> 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.