Re: [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sebastian,

On 09.05.23 00:06, Sebastian Reichel wrote:
Hi,

On Mon, May 08, 2023 at 11:18:28PM +0200, Jakob Hauser wrote:
On 08.05.23 13:35, Sebastian Reichel wrote:
On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote:
The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger
can, let's get this value.

Tested-by: Raymond Hackley <raymondhackley@xxxxxxxxxxxxxx>
Signed-off-by: Jakob Hauser <jahau@xxxxxxxxxxxxxx>
---
   drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++
   1 file changed, 24 insertions(+)

diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c
index 5c04cf305219..a6520716d813 100644
--- a/drivers/power/supply/rt5033_battery.c
+++ b/drivers/power/supply/rt5033_battery.c
@@ -12,6 +12,26 @@
   #include <linux/mfd/rt5033-private.h>
   #include <linux/mfd/rt5033.h>
+static int rt5033_battery_get_status(struct i2c_client *client)
+{
+	struct power_supply *charger;
+	union power_supply_propval val;
+	int ret;
+
+	charger = power_supply_get_by_name("rt5033-charger");
+	if (!charger)
+		return POWER_SUPPLY_STATUS_UNKNOWN;
+
+	ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val);
+	if (ret) {
+		power_supply_put(charger);
+		return POWER_SUPPLY_STATUS_UNKNOWN;
+	}

struct rt5033_battery *battery = i2c_get_clientdata(client);
ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val);
if (ret)
      val.intval = POWER_SUPPLY_STATUS_UNKNOWN;

I don't think this works. There is no direct relationship between
rt5033-charger and rt5033-battery. They operate independently from each
other.

That should be fine as long as the supply dependency is properly declared.

I had a short try and the status property of rt5033-battery was "unknown".

Just for the record, the full function I tried was:

static int rt5033_battery_get_status(struct i2c_client *client)
{
         struct rt5033_battery *battery = i2c_get_clientdata(client);
         union power_supply_propval val;
         int ret;

         ret = power_supply_get_property_from_supplier(battery->psy,
                                              POWER_SUPPLY_PROP_STATUS,
                                              &val);
         if (ret)
                 val.intval = POWER_SUPPLY_STATUS_UNKNOWN;

         return val.intval;
}

Later on I added a read-out of the "ret" value. It is "-19". I guess that's
the "return -ENODEV;" from function
power_supply_get_property_from_supplier(). [2]

[2] https://github.com/torvalds/linux/blob/v6.4-rc1/drivers/power/supply/power_supply_core.c#L397-L421

I suppose your DT is missing the connection between the charger and
the battery:

rt5033_charger: charger {
     compatible = "rt5033-charger";
     ...
}

fuel-gauge {
     compatible = "rt5033-battery";
     ...
     power-supplies = <&rt5033_charger>; // you are probably missing this
};

See also Documentation/devicetree/bindings/power/supply/power-supply.yaml

...

Thanks for the hints.

This leads to updating the dt-bindings because adding the "power-supplies" property is important to be aware of.

Btw. first it didn't work. It took me quite some time to debug. I needed to add "psy_cfg.of_node = client->dev.of_node;" to the rt5033-battery probe function.

Now it works. However, there is a new problem. The battery driver gets probed first. The charger driver a bit later. In the meantime the battery driver spams dmesg with several "Failed to register power supply" because the charger driver isn't available yet. Once the charger driver is there, it works fine and dmesg becomes silent.

With the current state of the patchset:
dmesg | grep rt5033
[   13.628030] rt5033 6-0034: Device found (rev. 6)
[   13.633552] rt5033-led: Failed to locate of_node [id: -1]
[   13.790478] rt5033-charger rt5033-charger: DMA mask not set

With the changes discussed here:
dmesg | grep rt5033
[   15.741915] rt5033-battery 4-0035: Failed to register power supply
[   15.752894] rt5033-battery 4-0035: Failed to register power supply
[   15.795458] rt5033-battery 4-0035: Failed to register power supply
[   15.910760] rt5033-battery 4-0035: Failed to register power supply
[   15.913187] rt5033 6-0034: Device found (rev. 6)
[   15.914341] rt5033-led: Failed to locate of_node [id: -1]
[   15.920052] rt5033-battery 4-0035: Failed to register power supply
[   15.927262] rt5033-battery 4-0035: Failed to register power supply
[   16.017131] rt5033-battery 4-0035: Failed to register power supply
[   16.017401] rt5033-charger rt5033-charger: DMA mask not set

The message is comming from the rt5033-battery probe function, it's the power_supply_register() that fails.

Any ideas what could be done about this?

Kind regards,
Jakob



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux