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

[Nitin Rawat <quic_nitirawa@quicinc.com>]

On 8/11/2025 9:20 PM, Bjorn Andersson wrote:
> On Thu, Aug 07, 2025 at 08:09:56PM +0100, Mark Brown wrote:
>> On Thu, Aug 07, 2025 at 07:43:15PM +0200, Konrad Dybcio wrote:
>>> On 8/7/25 7:26 PM, Mark Brown wrote:
>>
>>>> Note that that's specifying OPPs which is different...
>>
>>> The microamp properties are in the top-level, not under OPP if
>>> that's what you meant
>>
>> I mean the OPPs use case is an existing well known one for dumping stuff
>> into DT.
>>
>>>> That doesn't mean that it's a good idea to put that information in the
>>>> DT, nor if it is sensible to put in DT does it mean that it's a good
>>>> idea to define a generic property that applies to all regulator
>>>> consumers which is what I now think Konrad is proposing.
>>
>>> Yeah, that's what I had in mind
>>
>>> I was never able to get a reliable source for those numbers myselfe
>>> either.. At least some of them are prooooobably? chosen based on the
>>> used regulator type, to ensure it's always in HPM..
>>
>> That's what set_mode() is for.  Like I say it's becoming less and less
>> relevant though.
>>
> 
> set_mode() just applies the mode to the regulator_dev, so in cases where
> you have multiple consumers of a regulator_dev things would break.
> 
> Further, there are numerous cases where we have multiple consumers each
> needing a "low" mode, but their combined load requires a "high" mode.
> 
> set_load() and its aggregation of the inputs deals with both of these
> issues.
> 
> 
> Whether mode setting is becoming less relevant in our hardware, that I
> don't have the definitive answer to.
> 
>>> That said, our drivers cover a wide variety of hardware, built on a
>>> wide variety of process nodes, with different configurations, etc.,
>>> so it's either polluting the DT, or polluting the driver with
>>> per-compatible hardcoded data (and additional compatibles because
>>> fallbacks wouldn't work most of the time)
> 
> If this is our reason for putting it in DeviceTree, then we should write
> that in the commit message :)
> 
>>
>> That's really not a persuasive argument for adding a genric property
>> that applies to all regulator consumers...
>>
> 
> I agree, even if we determine that this belongs in DT, because it needs
> to be tweaked on a per-board basis, it's still only applicable to a
> fraction of our device nodes.

Hi Bjorn & Mark,

I had a follow-up discussion with the PHY designer to confirm whether 
this value could vary at the board level. Based on their response, it's 
a fixed value for the SoC and remains consistent across different 
boards. Therefore, I'm comfortable removing it from the device tree and 
using hardcoded, per-compatible data in the driver.

The only concern is that this approach may lead to driver bloat over 
time, as more SoCs are added and each requires its own hardcoded 
configuration.

Regards,
Nitin






> 
> Regards,
> Bjorn
> 
>> My instinct with this stuff is generally to avoid putting it in the DT,
>> we see far too many instances where someone's typed some numbers in
>> wrongly or discovers the ability to drive the hardware harder and needs
>> to tune the numbers - once something is ABI you're stuck just trusting
>> the numbers.  That said I'm not going to stop you putting something
>> specific to this driver in there, I just don't think this is a good idea
>> as a generic property.
>