| Message ID | 20240907-bq256xx-omit-battery-class-v1-0-45f6d8dbd1e5@mainlining.org |
|---|---|
| Headers | show |
| Series | Add omit-battery-class property for bq256xxx | expand |
On 07/09/2024 13:07, Barnabás Czémán wrote: > Add omit-battery-class property for avoid system create a battery device. This does not help much, basically repeats commit subject. You need to answer to "why?". > > Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org> > --- > Documentation/devicetree/bindings/power/supply/bq256xx.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/power/supply/bq256xx.yaml b/Documentation/devicetree/bindings/power/supply/bq256xx.yaml > index a76afe3ca299..744f5782e8e7 100644 > --- a/Documentation/devicetree/bindings/power/supply/bq256xx.yaml > +++ b/Documentation/devicetree/bindings/power/supply/bq256xx.yaml > @@ -62,6 +62,12 @@ properties: > $ref: /schemas/types.yaml#/definitions/phandle > description: phandle to the battery node being monitored > > + omit-battery-class: > + type: boolean > + description: | > + If this property is set, the operating system does not try to create a > + battery device. You described the desired Linux feature or behavior, not the actual hardware. The bindings are about the latter, so instead you need to rephrase the property and its description to match actual hardware capabilities/features/configuration etc. Best regards, Krzysztof
Hi, On Sat, Sep 07, 2024 at 01:11:57PM GMT, Krzysztof Kozlowski wrote: > On 07/09/2024 13:07, Barnabás Czémán wrote: > > Add omit-battery-class property for avoid system create a battery device. > > This does not help much, basically repeats commit subject. You need to > answer to "why?". Exposing two battery devices for a single battery is a bug, since that means there are two distinct batteries. Also note, that platforms having multiple batteries is a real thing. e.g. some Thinkpads used to have an internal battery and a hot-swappable one. > > Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org> > > --- > > Documentation/devicetree/bindings/power/supply/bq256xx.yaml | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/power/supply/bq256xx.yaml b/Documentation/devicetree/bindings/power/supply/bq256xx.yaml > > index a76afe3ca299..744f5782e8e7 100644 > > --- a/Documentation/devicetree/bindings/power/supply/bq256xx.yaml > > +++ b/Documentation/devicetree/bindings/power/supply/bq256xx.yaml > > @@ -62,6 +62,12 @@ properties: > > $ref: /schemas/types.yaml#/definitions/phandle > > description: phandle to the battery node being monitored > > > > + omit-battery-class: > > + type: boolean > > + description: | > > + If this property is set, the operating system does not try to create a > > + battery device. > > You described the desired Linux feature or behavior, not the actual > hardware. The bindings are about the latter, so instead you need to > rephrase the property and its description to match actual hardware > capabilities/features/configuration etc. Fully agreed. Also I think we already have the necessary information in the DT bindings. If there is a fuel-gauge in addition to the bq256xx charger, there should be a power-supplies link [0] between those two. Without the fuel-gauge obviously no such link exists. So the existance of this link can be used to decide wether a battery device should be registered or not. If the link exists, the charger driver should not create its own battery device. In the future the extension API might be used to let the charger extend the fuel gauge in case it is missing support for some battery properties, which can be provided by the charger. Note, that the link is going the other way around (from the battery to the charger). Also the fuel-gauge device will be registered after the charger device, so you cannot rely on power_supply_for_each_device when the charger probes. I see two options to solve that: 1. Create a new power-supply core function, which goes through all DT nodes, check that the nodename is "battery" or "fuel-gauge" and have a property "power-supplies" with a phandle pointing to the charger node. 2. Register the battery device at charger probe time and when the fuel-gauge driver is registered, call a (to be introduced) callback function in the charger driver, which unregisters the charger's battery. I have a slight preference for the second option, but I'm fine with either way. This does not create new ABI and can be easily changed in the future. [0] Documentation/devicetree/bindings/power/supply/power-supply.yaml Greetings, -- Sebastian
Add new omit-battery-class property for bq256xxx for avoid creating a battery device when a fuel gauge make one. In my case i have a Redmi Note 5A with bq25601 charger and bq27426 fg and two battery device is created one for the charger and one for the fg It seems battery device created by fg would be enough. Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org> --- Barnabás Czémán (2): dt-bindings: power: supply: bq256xx: Add omit-battery-class property power: supply: bq256xx: Add ability to omit battery class .../devicetree/bindings/power/supply/bq256xx.yaml | 6 ++++++ drivers/power/supply/bq256xx_charger.c | 14 ++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) --- base-commit: 9aaeb87ce1e966169a57f53a02ba05b30880ffb8 change-id: 20240907-bq256xx-omit-battery-class-e2cff68b3cee Best regards,