[PATCH 0/2] Add omit-battery-class property for bq256xxx

[PATCH 0/2] Add omit-battery-class property for bq256xxx

[Barnabás Czémán]

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,
-- 
Barnabás Czémán <barnabas.czeman@mainlining.org>

[PATCH 1/2] dt-bindings: power: supply: bq256xx: Add omit-battery-class property

[Barnabás Czémán]

Add omit-battery-class property for avoid system create a battery device.

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.
+
   interrupts:
     maxItems: 1
     description: |

-- 
2.46.0

Re: [PATCH 1/2] dt-bindings: power: supply: bq256xx: Add omit-battery-class property

[Krzysztof Kozlowski]

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

Re: [PATCH 1/2] dt-bindings: power: supply: bq256xx: Add omit-battery-class property

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3334 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] dt-bindings: power: supply: bq256xx: Add omit-battery-class property

[Krzysztof Kozlowski]

On Sat, Sep 07, 2024 at 01:07:45PM +0200, Barnabás Czémán wrote:
> Add omit-battery-class property for avoid system create a battery device.
> 
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run  and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

Your SoB does not match.

Best regards,
Krzysztof

[PATCH 2/2] power: supply: bq256xx: Add ability to omit battery class

[Barnabás Czémán]

Add omit-battery-class property to avoid make a battery device
for charger when fg makes one.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 drivers/power/supply/bq256xx_charger.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/power/supply/bq256xx_charger.c b/drivers/power/supply/bq256xx_charger.c
index 5514d1896bb8..62721d564b39 100644
--- a/drivers/power/supply/bq256xx_charger.c
+++ b/drivers/power/supply/bq256xx_charger.c
@@ -1547,12 +1547,14 @@ static int bq256xx_power_supply_init(struct bq256xx_device *bq,
 		return PTR_ERR(bq->charger);
 	}
 
-	bq->battery = devm_power_supply_register(bq->dev,
-						      &bq256xx_battery_desc,
-						      psy_cfg);
-	if (IS_ERR(bq->battery)) {
-		dev_err(dev, "power supply register battery failed\n");
-		return PTR_ERR(bq->battery);
+	if (!device_property_read_bool(dev, "omit-battery-class")) {
+		bq->battery = devm_power_supply_register(bq->dev,
+							 &bq256xx_battery_desc,
+							 psy_cfg);
+		if (IS_ERR(bq->battery)) {
+			dev_err(dev, "power supply register battery failed\n");
+			return PTR_ERR(bq->battery);
+		}
 	}
 	return 0;
 }

-- 
2.46.0