On Thu, Apr 20, 2023 at 9:59 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> Hi Jakob,
>
> thanks for your patch!
>
> The following caught my eye:
>
> On Sun, Apr 16, 2023 at 2:50 PM Jakob Hauser <jahau@xxxxxxxxxxxxxx> wrote:
>
> > Add device tree binding documentation for rt5033 multifunction device, voltage
> > regulator and battery charger.
> >
> > Cc: Beomho Seo <beomho.seo@xxxxxxxxxxx>
> > Cc: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>
> > Signed-off-by: Jakob Hauser <jahau@xxxxxxxxxxxxxx>
> > ---
> > The patch is based on linux-next (tag "next-20230413").
> (...)
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
> (...)
> > + richtek,pre-microamp:
> > + description:
> > + Current of pre-charge mode. The pre-charge current levels are 350 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 150 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-charge 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 voltage 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 known
> 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.yaml
There will be driver changes needed too, but this will be way cleaner.
Yours,
Linus Walleij
