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 08.05.23 13:35, Sebastian Reichel wrote:
Hi,

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.

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

+
+	power_supply_put(charger);
+	return val.intval;
+}
+
  static int rt5033_battery_get_capacity(struct i2c_client *client)
  {
  	struct rt5033_battery *battery = i2c_get_clientdata(client);
@@ -84,6 +104,9 @@ static int rt5033_battery_get_property(struct power_supply *psy,
  	case POWER_SUPPLY_PROP_CAPACITY:
  		val->intval = rt5033_battery_get_capacity(battery->client);
  		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = rt5033_battery_get_status(battery->client);
+		break;
  	default:
  		return -EINVAL;
  	}
@@ -96,6 +119,7 @@ static enum power_supply_property rt5033_battery_props[] = {
  	POWER_SUPPLY_PROP_VOLTAGE_OCV,
  	POWER_SUPPLY_PROP_PRESENT,
  	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_STATUS,
  };
static const struct regmap_config rt5033_battery_regmap_config = {
--
2.39.2


Otherwise LGTM.

-- Sebastian

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