Re: [PATCH 1/8] dt-bindings: qcom: rpmh-regulator: Add compatible for SA8155p-adp board pmics

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Mon 14 Jun 03:05 CDT 2021, Bhupesh Sharma wrote:

> Hello Bjorn,
> 
> Thanks for the review comments.
> 
> On Fri, 11 Jun 2021 at 08:18, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote:
> >
> > > Add compatible strings for pmm8155au_1 and pmm8155au_2 pmics
> > > found on SA8155p-adp board.
> > >
> > > Cc: Linus Walleij <linus.walleij@linaro.org>
> > > Cc: Liam Girdwood <lgirdwood@gmail.com>
> > > Cc: Mark Brown <broonie@kernel.org>
> > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > Cc: Vinod Koul <vkoul@kernel.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Andy Gross <agross@kernel.org>
> > > Cc: devicetree@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org
> > > Cc: linux-gpio@vger.kernel.org
> > > Cc: bhupesh.linux@gmail.com
> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> > > ---
> > >  .../devicetree/bindings/regulator/qcom,rpmh-regulator.yaml      | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> > > index e561a5b941e4..ea5cd71aa0c7 100644
> > > --- a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> > > +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> > > @@ -55,6 +55,8 @@ properties:
> > >        - qcom,pm8009-1-rpmh-regulators
> > >        - qcom,pm8150-rpmh-regulators
> > >        - qcom,pm8150l-rpmh-regulators
> > > +      - qcom,pmm8155au-1-rpmh-regulators
> > > +      - qcom,pmm8155au-2-rpmh-regulators
> >
> > Looking at the component documentation and the schematics I think the
> > component is "PMM8155AU" and we have two of them.
> >
> > Unless I'm mistaken we should have the compatible describe the single
> > component and we should have DT describe the fact that we have 2 of
> > them.
> 
> If we refer to the PM8155AU device specifications, there are two
> regulators mentioned there PMM8155AU_1 and PMM8155AU_2. Although most
> parameters of the regulators seem similar the smps regulator summary
> for both appear different (Transient Load, mA ratings etc).
> 
> Although most of these differences don't probably matter to the Linux
> world, others like the gpios on the pmic are different.
> 
> So, IMO, it makes sense to mention the different pmic types on the board.
> 
> Please let me know your views on the same.
> 

Afaict, they are both physically the same component, but there is some
configuration differences between them. I don't see any differences that
will show up in Linux, but afaict we would capture those in the DT
anyways.

Let me know if you see anything I'm missing, but I think we should have
a single compatible.

Regards,
Bjorn

> Thanks,
> Bhupesh
> 
> >
> > >        - qcom,pm8350-rpmh-regulators
> > >        - qcom,pm8350c-rpmh-regulators
> > >        - qcom,pm8998-rpmh-regulators
> > > --
> > > 2.31.1
> > >