Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1681646904.git.jahau@rocketmail.com> <9275af790e6e21b5cf661a2444effe4caf2be02e.1681646904.git.jahau@rocketmail.com>
 <CACRpkdZEtG=OjTECDO=SvFk89MqL10sKKMOABPEs-xxYv1hmqw@mail.gmail.com>
In-Reply-To: <CACRpkdZEtG=OjTECDO=SvFk89MqL10sKKMOABPEs-xxYv1hmqw@mail.gmail.com>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Thu, 20 Apr 2023 10:03:54 +0200
Message-ID: <CACRpkdaRkJ-JVNqAOQLuOgDztDfUP7DBQU9QP7AMbnK=eN2HWQ@mail.gmail.com>
Subject: Re: [PATCH v2 9/9] dt-bindings: Add documentation for rt5033 mfd,
 regulator and charger
To:     Jakob Hauser <jahau@rocketmail.com>
Cc:     Sebastian Reichel <sre@kernel.org>, Lee Jones <lee@kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Beomho Seo <beomho.seo@samsung.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Stephan Gerhold <stephan@gerhold.net>,
        Raymond Hackley <raymondhackley@protonmail.com>,
        Pavel Machek <pavel@ucw.cz>, Axel Lin <axel.lin@ingics.com>,
        ChiYuan Huang <cy_huang@richtek.com>, linux-pm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        phone-devel@vger.kernel.org, ~postmarketos/upstreaming@lists.sr.ht
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Apr 20, 2023 at 9:59=E2=80=AFAM Linus Walleij <linus.walleij@linaro=
.org> wrote:
>
> Hi Jakob,
>
> thanks for your patch!
>
> The following caught my eye:
>
> On Sun, Apr 16, 2023 at 2:50=E2=80=AFPM Jakob Hauser <jahau@rocketmail.co=
m> wrote:
>
> > Add device tree binding documentation for rt5033 multifunction device, =
voltage
> > regulator and battery charger.
> >
> > Cc: Beomho Seo <beomho.seo@samsung.com>
> > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> > ---
> > The patch is based on linux-next (tag "next-20230413").
> (...)
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-cha=
rger.yaml
> (...)
> > +  richtek,pre-microamp:
> > +    description:
> > +      Current of pre-charge mode. The pre-charge current levels are 35=
0 mA to
> > +      650 mA programmed by I2C per 100 mA.
> > +    maxItems: 1
> > +
> > +  richtek,fast-microamp:
> > +    description:
> > +      Current of fast-charge mode. The fast-charge current levels are =
700 mA
> > +      to 2000 mA programmed by I2C per 100 mA.
> > +    maxItems: 1
> > +
> > +  richtek,eoc-microamp:
> > +    description:
> > +      This property is end of charge current. Its level ranges from 15=
0 mA to
> > +      600 mA. Between 150 mA and 300 mA in 50 mA steps, between 300 mA=
 and 600 mA
> > +      in 100 mA steps.
> > +    maxItems: 1
> > +
> > +  richtek,pre-threshold-microvolt:
> > +    description:
> > +      Voltage of pre-charge mode. If the battery voltage is below the =
pre-charge
> > +      threshold voltage, the charger is in pre-charge mode with pre-ch=
arge current.
> > +      Its levels are 2.3 V to 3.8 V programmed by I2C per 0.1 V.
> > +    maxItems: 1
> > +
> > +  richtek,const-microvolt:
> > +    description:
> > +      Battery regulation voltage of constant voltage mode. This voltag=
e levels from
> > +      3.65 V to 4.4 V by I2C per 0.025 V.
> > +    maxItems: 1
>
> These are very generic currents and voltages, and their usage is well kno=
wn
> and generic. So they should not be prefixed "richtek,".
>
> Use the properties already defined in
> Documentation/devicetree/bindings/power/supply/battery.yaml
> for these:
>
> precharge-current-microamp
> constant-charge-current-max-microamp
> charge-term-current-microamp
> precharge-upper-limit-microvolt
> constant-charge-voltage-max-microvolt
>
> Please double-check, I think those are the ones you need.
>
> Perhaps it is possible to just $ref these properties directly and add
> the additional restrictions on top.

On second thought, these are really weird properties to have on the
*charger* isn't it?

It is really *battery* restrictions.

A charger can charge many different batteries with different CC/CV
settings.

I think your charger should contain a phandle to a battery and the battery
node should contain these limits.

  monitored-battery:
    $ref: /schemas/types.yaml#/definitions/phandle
    description: phandle to battery node

Then you can just use the standard battery bindings for these properties
on the battery.

See for example:
Documentation/devicetree/bindings/power/supply/stericsson,ab8500-charger.ya=
ml

There will be driver changes needed too, but this will be way cleaner.

Yours,
Linus Walleij