Re: [PATCH v3 4/6] regulator: qcom_spmi: Add support for PMP8074 regulators

[Robert Marko]

On Wed, 18 May 2022 at 15:42, Konrad Dybcio
<konrad.dybcio@xxxxxxxxxxxxxx> wrote:
>
>
> On 17/05/2022 22:53, Robert Marko wrote:
> > PMP8074 is a companion PMIC for the Qualcomm IPQ8074 WiSoC-s.
> >
> > It features 5 HF-SMPS and 13 LDO regulators.
> >
> > This commit adds support for S3 and S4 HF-SMPS buck regulators of
> > the HFS430 type and LDO11 of the HT_P150 type.
> > S3 is the CPU cluster voltage supply, S4 supplies the UBI32 NPU cores
> > and LDO11 is the SDIO/eMMC I/O voltage regulator required for high speeds.
> >
> > Signed-off-by: Robert Marko <robimarko@xxxxxxxxx>
> > ---
> >   drivers/regulator/qcom_spmi-regulator.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> > index 38bbc70241ae..696b088aae40 100644
> > --- a/drivers/regulator/qcom_spmi-regulator.c
> > +++ b/drivers/regulator/qcom_spmi-regulator.c
> > @@ -2137,6 +2137,13 @@ static const struct spmi_regulator_data pms405_regulators[] = {
> >       { }
> >   };
> >
> > +static const struct spmi_regulator_data pmp8074_regulators[] = {
>
> Please sort the struct alphabletically.

Hi,
Will fixup in v3.

>
>
> > +     { "s3", 0x1a00, "vdd_s3"},
> > +     { "s4", 0x1d00, "vdd_s4"},
> > +     { "l11", 0x4a00, "vdd_l10_l11_l12_l13"},
>
> Are the other regulators somehow not controllable through SPMI? Please
> leave a comment if that's the case.

No, they are all controllable via SPMI as far as I know.
Though the output table completely leaves out L7, L9, and L10.
L5 and L6 are specified as Ebuck-4 subtypes (0x3d), while L10 is not
listed at all (0x34).
These are not currently supported and I don't have enough information
to support them.
L1, L2, L3, L4, L7, L8, L9, L11, L12, and L13 appear to have their
subtype already supported.
L1, L2, L3, L8, L9 subtype (0x32)
L4 subtype (0x30)
L7, L11, L12, L13 subtype (0x35)

S3, S4, and L11 are the most important ones.
>
>
> > +     { }
> > +};
> > +
> >   static const struct of_device_id qcom_spmi_regulator_match[] = {
> >       { .compatible = "qcom,pm8004-regulators", .data = &pm8004_regulators },
> >       { .compatible = "qcom,pm8005-regulators", .data = &pm8005_regulators },
> > @@ -2150,6 +2157,7 @@ static const struct of_device_id qcom_spmi_regulator_match[] = {
> >       { .compatible = "qcom,pm660-regulators", .data = &pm660_regulators },
> >       { .compatible = "qcom,pm660l-regulators", .data = &pm660l_regulators },
> >       { .compatible = "qcom,pms405-regulators", .data = &pms405_regulators },
> > +     { .compatible = "qcom,pmp8074-regulators", .data = &pmp8074_regulators },
>
> Please sort the compatible too.

Will fixup in v3.

Regards,
Robert
>
>
> Konrad
>
> >       { }
> >   };
> >   MODULE_DEVICE_TABLE(of, qcom_spmi_regulator_match);
> >