Re: [PATCH V2 4/6] regulator: qcom_smd: Add support to define the bootup voltage
From: Devi Priya
Date: Mon Apr 03 2023 - 10:09:23 EST
On 3/28/2023 1:58 PM, Konrad Dybcio wrote:
On 28.03.2023 08:12, Devi Priya wrote:
On 3/27/2023 2:56 PM, Konrad Dybcio wrote:
On 27.03.2023 10:40, Devi Priya wrote:
On 3/18/2023 7:38 PM, Konrad Dybcio wrote:
On 14.03.2023 18:15, Devi Priya wrote:
On 3/8/2023 3:57 PM, Konrad Dybcio wrote:
On 7.03.2023 07:55, Devi Priya wrote:
On 3/6/2023 6:39 PM, Devi Priya wrote:
On 3/3/2023 6:57 PM, Konrad Dybcio wrote:
On 3.03.2023 14:21, Devi Priya wrote:
On 2/23/2023 4:31 AM, Mark Brown wrote:
On Wed, Feb 22, 2023 at 11:11:42PM +0100, Konrad Dybcio wrote:
Thinking about it again, this seems like something that could be
generalized and introduced into regulator core.. Hardcoding this
will not end well.. Not to mention it'll affect all mp5496-using
boards that are already upstream.
WDYT about regulator-init-microvolts Mark?
The overwhelming majority of devices that have variable voltages
support readback, these Qualcomm firmware devices are pretty much
unique in this regard. We don't want a general property to set a
specific voltage since normally we should be using the
constraints and don't normally need to adjust things immediately
since we can tell what the current voltage is.
This is pretty much just going to be a device specific bodge,
ideally something that does know what the voltage is would be
able to tell us at runtime but if that's not possible then
there's no good options. If the initial voltage might vary based
on board then a device specific DT property might be less
terrible, if it's determined by the regulator the current code
seems fine. Or just leave the current behavour, if the
constraints are accurate then hopefully a temporary dip in
voltage is just inelegant rather than an issue. Indeed the
current behaviour might well save power if you've got a voltage
range configured and nothing actually ever gets round to setting
the voltage (which is depressingly common, people seem keen on
setting voltage ranges even when the voltage is never varied in
practice).
Hi Mark, The initial bootup voltage is actually blown into the OTP register of the PMIC and it remains the same across boards for IPQ9574 SoC.
But what about IPQ6018 which also uses MP5496? That's also gonna
set the voltage on there, it may be too high/low..
For IPQ6018, the bootup voltage is the same as that of IPQ9574 which is
875mV
Okay, but what about any other design that employs or may employ
MP5496 in the future?
Initially the SoC runs at 800MHz with a voltage of 875mV set by the bootloaders. As kernel does not know the initial voltage, during regulator registration the framework considers the current voltage to be zero and tries to bring up the regulator to minimum supported voltage of 600mV. This causes the dip which might be of concern in SS parts where the voltage might be insufficient leading to silent reboots.
That's an SoC-specific thing, the same regulator can be used with
many different ones. We can't just assume it'll always be like this.
I see the problem, but I believe this is not the correct solution
Okay, As we had discussions on reading back the voltage & the generic
DT property, do you suggest any other possible solutions here?
Due to the sudden influx of various IPQ SoCs on the mailing list lately
I have no idea if it concerned this one too, but at least one of them
was said not to use RPM for controlling the clocks. If that's the case,
I see no reason at all to use it for scaling the regulators, the PMIC
could be addressed directly over I2C as a normal device. You'd probably
want to keep VDD_[CM]X scaling through rpmpd, but it's easily done by
simply not registering the CX/MX registers as children of the I2C
regulator IC.
IPQ9574 SoC has RPM and uses it for controlling the regulators.
Currently, the RPM firmware does not have read support implemented
and so, we were not able to read the bootup voltage.
As we randomly saw silent reboots when the kernel boots up,
do you think we could proceed with this change for time being
and revisit the same when any SoC in the future employs MP5496?
I'm still thinking about a cleaner fix because hardcoding voltages
in kernel is just dangerous. Could you check whether attaching a CPU
supply and adding an OPP table where each level has an opp-microvolt
property would resolve your issue?
Konrad
Yes, Understood! We already have the CPU supply and OPP table where
each level has an opp-microvolt property changes in place and it did
not help in our case.
Ouch. That totally sounds like a bug.. Would you be willing to dig
a bit further into why this happens?
As the regulator registration happens before the cpu freq scaling comes
into picture, having an OPP table did not help to set the bootup voltage
I see, but the order should not affect it. If your regulator driver
returns -EPROBE_DEFER, so should the cpufreq one.
Konrad
Sorry konrad, don't exactly get your point here.
The cpufreq driver depends on the regulator driver to be probed as
the regulator serves as the cpu-supply.
But, when the regulator driver comes up, it tries to bring up the
regulators to the minimum supported voltage provided with the
regulator-min-microvolt property in the DT.
The regulator being the CPU only supply, updating the
regulator-min-microvolt with SVS voltage (725000uV) would ideally help
us with the issue. That way, we could update the DT and drop this patch.
This approach seems quite ideal to us. Shall we proceed with it if we
don't foresee any issues?
Thanks,
Devi Priya
Konrad
We have now planned to update the regulator-min-microvolt property
with the SVS voltage (725000uV) in the board DT such that the regulators
are brought up with the nominal voltage which would be sufficient
for all the corner parts operating at 800MHz.
That way, we would update the DT and drop this patch in the next spin.
Thanks,
Devi Priya
Konrad
Konrad
Best Regards,
Devi Priya