Hi Linus,
On 21.04.23 11:20, Linus Walleij wrote:
On Thu, Apr 20, 2023 at 11:16 PM Jakob Hauser <jahau@xxxxxxxxxxxxxx> wrote:
...
I agree to you that actually the physical battery is determining how
these values should be set. In the end, as far as I can see, it is a
representation thing in the devicetree. At least in our case here.
The DT bindings should reflect the hardware, and not what some
or any driver "would like to see" (to make its life easier...)
As these things are programmed into registers, clearly the
hardware is adoptable for different batteries, and the purpose
of these registers is to support different batteries. Ergo: they
belong in a battery node.
Not sure how to proceed here. I would stick to the current
implementation. If someone strongly prefers the "battery" representation
style, I'm open to switch to this.
Again this is not an implementation but a hardware description.
It should use a phandle to a montored-battery and follow that to
read the battery properties.
OK, this is expressing a strong preference. I’ll switch to the "battery"
node in v3.
However, I'm not sure how the dt-bindings would look like in that case.
Just like you sketched above, just reuse simple-battery if the battery
is hardcoded into the platform, such as soldered in or has a form
factor such that no different battery can be fitted.
Those battery properties would not be part of the RT5033 node, thus they
basically would not be part of the RT5033 documentation. Again I think
it makes sense to handle those properties within the charger node as
"charger settings" properties.
Why?
This is like saying that the number of pixels on your monitor should
be part of the graphics card DT node as "configuration". And we
clearly do not do that.
The charger driver needs five parameters. And the person writing the dts
file should know about what range and granularity is possible for those
values. This information could be put into the description of the
"monitored-battery", as shown below, but that's less strict and clear as
it is currently in patch 9.
title: Richtek RT5033 PIMC Battery Charger
maintainers:
- Jakob Hauser <jahau@xxxxxxxxxxxxxx>
description:
The battery charger of the multifunction device RT5033 has to be
instantiated under sub-node named "charger" using the following
format.
properties:
compatible:
const: richtek,rt5033-charger
monitored-battery:
description: |
Phandle to the monitored battery according to battery.yaml.
The battery node needs to contain five parameters.
precharge-current-microamp:
Current of pre-charge mode. The pre-charge current levels are
350 mA to 650 mA programmed by I2C per 100 mA.
constant-charge-current-max-microamp:
Current of fast-charge mode. The fast-charge current levels
are 700 mA to 2000 mA programmed by I2C per 100 mA.
charge-term-current-microamp:
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.
precharge-upper-limit-microvolt:
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.
constant-charge-voltage-max-microvolt:
Battery regulation voltage of constant voltage mode. This
voltage levels from 3.65 V to 4.4 V by I2C per 0.025 V.
extcon:
description:
Phandle to the extcon device.
maxItems: 1
required:
- monitored-battery
additionalProperties: false
examples:
- |
charger {
compatible = "richtek,rt5033-charger";
monitored-battery = <&battery>;
extcon = <&muic>;
};
Kind regards,
Jakob
