Re: [PATCH 3/7] regulator: qcom-rpmh: Correct the pmic5_hfsmps515 buck

[Dmitry Baryshkov]

Hello,

On Mon, 1 Mar 2021 at 13:37, <skakit@xxxxxxxxxxxxxx> wrote:
>
> On 2021-02-26 15:57, Dmitry Baryshkov wrote:
> > On Fri, 26 Feb 2021 at 09:59, <skakit@xxxxxxxxxxxxxx> wrote:
> >>
> >> Hi,
> >>
> >> On 2021-02-25 16:39, Dmitry Baryshkov wrote:
> >> > On 24/02/2021 11:33, satya priya wrote:
> >> >> Correct the REGULATOR_LINEAR_RANGE and n_voltges for
> >> >> pmic5_hfsmps515 buck.
> >> >>
> >> >> Signed-off-by: satya priya <skakit@xxxxxxxxxxxxxx>
> >> >> ---
> >> >>   drivers/regulator/qcom-rpmh-regulator.c | 4 ++--
> >> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/drivers/regulator/qcom-rpmh-regulator.c
> >> >> b/drivers/regulator/qcom-rpmh-regulator.c
> >> >> index 79a554f..36542c3 100644
> >> >> --- a/drivers/regulator/qcom-rpmh-regulator.c
> >> >> +++ b/drivers/regulator/qcom-rpmh-regulator.c
> >> >> @@ -726,8 +726,8 @@ static const struct rpmh_vreg_hw_data
> >> >> pmic5_ftsmps510 = {
> >> >>   static const struct rpmh_vreg_hw_data pmic5_hfsmps515 = {
> >> >>      .regulator_type = VRM,
> >> >>      .ops = &rpmh_regulator_vrm_ops,
> >> >> -    .voltage_range = REGULATOR_LINEAR_RANGE(2800000, 0, 4, 16000),
> >> >> -    .n_voltages = 5,
> >> >> +    .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 235, 16000),
> >> >> +    .n_voltages = 236,
> >> >
> >> > I've checked the docs for pm8009, the chip which also uses hfsmps515
> >> > regulators. The pdf clearly states that the 'Output voltage operating
> >> > range' is from 2.8 V to 2.85 V.
> >> >
> >> > So we'd probably need to define different versions of HFS515 regulator
> >> > data (like I had to create for pm8009-1).
> >> >
> >> >
> >>
> >> The min-max voltages for S1C (PM8350c) regulator are 2190000-2210000uV
> >> for sc7280(kodiak), so we had to modify this buck to support this
> >> regulator.
> >>
> >> AFAIK, this struct defines the HW constraints of a regulator, but the
> >> platform specific min-max values can be controlled from DT files. So,
> >> can't we modify it like above instead of adding a new definition? the
> >> new min_uV value (32000) is anyway not exceeding the old value
> >> (2800000)
> >> right? please correct me if wrong.
> >
> > As far as I understand for other regulators we put 'output voltage
> > limitations' from the docs into the regulator definition and further
> > constrain it by the platform device tree. Please correct me if I'm
> > wrong.
>
> I see that for most of the regulators, these specifications are specific
> to regulator buck (like HFS515) but not chipset specific, we set the
> chipset specific(like pm8009/pm8350c) requirements from DT files.
>
> For example:
> pmic5_nldo regulator spec mentions LLIMIT= 0.32V and ULIMIT =1.304V with
> step 8mV
>
> .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 123, 8000),
> max output voltage supported by this regulator is 123*8000 + 320000 =
> 1304000mV which is same as mentioned in the regulator spec.
>
> > For pm8009 the data from the datasheet matches the regulators defined
> > in the source file. Unfortunately I don't have kodiak specs at hand.
>
>  From the HFS515 spec I got below info
> "HFS510 and lower max output voltage is limited to 2.04V max, and
> Yoda(pm8009) requirement was 2.4V for IOT PA and 2.85V for camera
> application.  Hence, HFS515 added a new register and corresponding HW
> changes to support the higher voltage.  Table 5‑24 shows the new
> FB_RANGE bit.  When configured to 0 the buck works as earlier where Vout
> max = 2.04V in 8mV steps, but when configured to 1 the buck range
> doubles and can now support a Vout max = 4.08V in 16mV steps."
>
> As per above, the max output voltage supported by HFS515 buck is 4.08V
> (which is kodiak pm8350c pmic's requirement).
> So, we have modified the buck data to support pm8350c(palani) along with
> pm8009(yoda).

I'd still prefer to have two different regulator types (as we did for
pm8009 P=0 and P=1 variants). However it's probably up to the
maintainers to decide.


-- 
With best wishes
Dmitry