[PATCH v2 01/14] power: Make power_supply_am_i_supplied return -ENODEV if there are no suppliers

[PATCH v2 01/14] power: Make power_supply_am_i_supplied return -ENODEV if there are no suppliers

[Hans de Goede]

It is sensible to assume that the hardware actually always has a
way of charging the battery so when power_supply_am_i_supplied does not
find any suppliers, that does not mean that there are none, but simply
that no power_supply-drivers are registered / bound for any suppliers for
the supply calling power_supply_am_i_supplied.

At which point a fuel-gauge driver calling power_supply_am_i_supplied()
cannot determine whether the battery is being charged or not.

Allow a caller of power_supply_am_i_supplied to differentiate between
there not being any suppliers, vs no suppliers being online by returning
-ENODEV if there are no suppliers matching supplied_to / supplied_from,
which allows fuel-gauge drivers to return POWER_SUPPLY_STATUS_UNKNOWN
rather then POWER_SUPPLY_STATUS_DISCHARGING if there are no suppliers.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Improve commit message
---
 drivers/power/supply/power_supply_core.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 1e0960b..13a39da 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -280,13 +280,19 @@ static inline int power_supply_check_supplies(struct power_supply *psy)
 }
 #endif
 
-static int __power_supply_am_i_supplied(struct device *dev, void *data)
+struct am_i_supplied_data {
+	struct power_supply *psy;
+	unsigned int count;
+};
+
+static int __power_supply_am_i_supplied(struct device *dev, void *_data)
 {
 	union power_supply_propval ret = {0,};
-	struct power_supply *psy = data;
 	struct power_supply *epsy = dev_get_drvdata(dev);
+	struct am_i_supplied_data *data = _data;
 
-	if (__power_supply_is_supplied_by(epsy, psy))
+	data->count++;
+	if (__power_supply_is_supplied_by(epsy, data->psy))
 		if (!epsy->desc->get_property(epsy, POWER_SUPPLY_PROP_ONLINE,
 					&ret))
 			return ret.intval;
@@ -296,12 +302,16 @@ static int __power_supply_am_i_supplied(struct device *dev, void *data)
 
 int power_supply_am_i_supplied(struct power_supply *psy)
 {
+	struct am_i_supplied_data data = { psy, 0 };
 	int error;
 
-	error = class_for_each_device(power_supply_class, NULL, psy,
+	error = class_for_each_device(power_supply_class, NULL, &data,
 				      __power_supply_am_i_supplied);
 
-	dev_dbg(&psy->dev, "%s %d\n", __func__, error);
+	dev_dbg(&psy->dev, "%s count %u err %d\n", __func__, data.count, error);
+
+	if (data.count == 0)
+		return -ENODEV;
 
 	return error;
 }
-- 
2.9.3

[PATCH v2 02/14] power: max17042_battery: Use sign_extend32 instead of DIY code

[Hans de Goede]

Use sign_extend32 to sign-extend variables where necessary instead of
DIY code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/power/supply/max17042_battery.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index da7a75f..790dfa9 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -106,13 +106,7 @@ static int max17042_get_temperature(struct max17042_chip *chip, int *temp)
 	if (ret < 0)
 		return ret;
 
-	*temp = data;
-	/* The value is signed. */
-	if (*temp & 0x8000) {
-		*temp = (0x7fff & ~*temp) + 1;
-		*temp *= -1;
-	}
-
+	*temp = sign_extend32(data, 15);
 	/* The value is converted into deci-centigrade scale */
 	/* Units of LSB = 1 / 256 degree Celsius */
 	*temp = *temp * 10 / 256;
@@ -302,13 +296,7 @@ static int max17042_get_property(struct power_supply *psy,
 			if (ret < 0)
 				return ret;
 
-			val->intval = data;
-			if (val->intval & 0x8000) {
-				/* Negative */
-				val->intval = ~val->intval & 0x7fff;
-				val->intval++;
-				val->intval *= -1;
-			}
+			val->intval = sign_extend32(data, 15);
 			val->intval *= 1562500 / chip->pdata->r_sns;
 		} else {
 			return -EINVAL;
@@ -320,13 +308,7 @@ static int max17042_get_property(struct power_supply *psy,
 			if (ret < 0)
 				return ret;
 
-			val->intval = data;
-			if (val->intval & 0x8000) {
-				/* Negative */
-				val->intval = ~val->intval & 0x7fff;
-				val->intval++;
-				val->intval *= -1;
-			}
+			val->intval = sign_extend32(data, 15);
 			val->intval *= 1562500 / chip->pdata->r_sns;
 		} else {
 			return -EINVAL;
-- 
2.9.3

[PATCH v2 03/14] power: max17047_battery: The temp alert values are 8-bit 2's complement

[Hans de Goede]

The temp alert values are 8-bit 2's complement, so sign-extend them
before reporting them back to the caller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/power/supply/max17042_battery.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index 790dfa9..a51b296 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -270,14 +270,14 @@ static int max17042_get_property(struct power_supply *psy,
 		if (ret < 0)
 			return ret;
 		/* LSB is Alert Minimum. In deci-centigrade */
-		val->intval = (data & 0xff) * 10;
+		val->intval = sign_extend32(data & 0xff, 7) * 10;
 		break;
 	case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
 		ret = regmap_read(map, MAX17042_TALRT_Th, &data);
 		if (ret < 0)
 			return ret;
 		/* MSB is Alert Maximum. In deci-centigrade */
-		val->intval = (data >> 8) * 10;
+		val->intval = sign_extend32(data >> 8, 7) * 10;
 		break;
 	case POWER_SUPPLY_PROP_TEMP_MIN:
 		val->intval = chip->pdata->temp_min;
-- 
2.9.3

[PATCH v2 04/14] power: max17042_battery: Add default platform_data fallback data

[Hans de Goede]

Some x86 machines use a max17047 fuel-gauge and x86 might be missing
platform_data if not provided by SFI.

This commit adds default platform_data as fallback option so that the
driver can work on boards where no platform_data is provided.

Since not all boards have a thermistor hooked up, set temp_min to 0 and
change the health checks from temp <= temp_min to temp < temp_min to
not trigger on such boards (where temp reads 0).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Reword commit message
-Add MAX17042_DEFAULT_FOO defines to include/linux/power/max17042.h and
 use these to set the default pdata values
---
 drivers/power/supply/max17042_battery.c | 60 +++++++++++++++++++++++++++++----
 include/linux/power/max17042_battery.h  |  6 +++-
 2 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index a51b296..f0ff6e8 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -150,12 +150,12 @@ static int max17042_get_battery_health(struct max17042_chip *chip, int *health)
 	if (ret < 0)
 		goto health_error;
 
-	if (temp <= chip->pdata->temp_min) {
+	if (temp < chip->pdata->temp_min) {
 		*health = POWER_SUPPLY_HEALTH_COLD;
 		goto out;
 	}
 
-	if (temp >= chip->pdata->temp_max) {
+	if (temp > chip->pdata->temp_max) {
 		*health = POWER_SUPPLY_HEALTH_OVERHEAT;
 		goto out;
 	}
@@ -772,8 +772,9 @@ static void max17042_init_worker(struct work_struct *work)
 
 #ifdef CONFIG_OF
 static struct max17042_platform_data *
-max17042_get_pdata(struct device *dev)
+max17042_get_pdata(struct max17042_chip *chip)
 {
+	struct device *dev = &chip->client->dev;
 	struct device_node *np = dev->of_node;
 	u32 prop;
 	struct max17042_platform_data *pdata;
@@ -806,10 +807,55 @@ max17042_get_pdata(struct device *dev)
 	return pdata;
 }
 #else
+static struct max17042_reg_data max17047_default_pdata_init_regs[] = {
+	/*
+	 * Some firmwares do not set FullSOCThr, Enable End-of-Charge Detection
+	 * when the voltage FG reports 95%, as recommended in the datasheet.
+	 */
+	{ MAX17047_FullSOCThr, MAX17042_BATTERY_FULL << 8 },
+};
+
 static struct max17042_platform_data *
-max17042_get_pdata(struct device *dev)
+max17042_get_pdata(struct max17042_chip *chip)
 {
-	return dev->platform_data;
+	struct device *dev = &chip->client->dev;
+	struct max17042_platform_data *pdata;
+	int ret, misc_cfg;
+
+	if (dev->platform_data)
+		return dev->platform_data;
+
+	/*
+	 * The MAX17047 gets used on x86 where we might not have pdata, assume
+	 * the firmware will already have initialized the fuel-gauge and provide
+	 * default values for the non init bits to make things work.
+	 */
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return pdata;
+
+	if (chip->chip_type != MAXIM_DEVICE_TYPE_MAX17042) {
+		pdata->init_data = max17047_default_pdata_init_regs;
+		pdata->num_init_data =
+			ARRAY_SIZE(max17047_default_pdata_init_regs);
+	}
+
+	ret = regmap_read(chip->regmap, MAX17042_MiscCFG, &misc_cfg);
+	if (ret < 0)
+		return NULL;
+
+	/* If bits 0-1 are set to 3 then only Voltage readings are used */
+	if ((misc_cfg & 0x3) == 0x3)
+		pdata->enable_current_sense = false;
+	else
+		pdata->enable_current_sense = true;
+
+	pdata->vmin = MAX17042_DEFAULT_VMIN;
+	pdata->vmax = MAX17042_DEFAULT_VMAX;
+	pdata->temp_min = MAX17042_DEFAULT_TEMP_MIN;
+	pdata->temp_max = MAX17042_DEFAULT_TEMP_MAX;
+
+	return pdata;
 }
 #endif
 
@@ -858,20 +904,20 @@ static int max17042_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	chip->client = client;
+	chip->chip_type = id->driver_data;
 	chip->regmap = devm_regmap_init_i2c(client, &max17042_regmap_config);
 	if (IS_ERR(chip->regmap)) {
 		dev_err(&client->dev, "Failed to initialize regmap\n");
 		return -EINVAL;
 	}
 
-	chip->pdata = max17042_get_pdata(&client->dev);
+	chip->pdata = max17042_get_pdata(chip);
 	if (!chip->pdata) {
 		dev_err(&client->dev, "no platform data provided\n");
 		return -EINVAL;
 	}
 
 	i2c_set_clientdata(client, chip);
-	chip->chip_type = id->driver_data;
 	psy_cfg.drv_data = chip;
 
 	/* When current is not measured,
diff --git a/include/linux/power/max17042_battery.h b/include/linux/power/max17042_battery.h
index 522757a..3489fb0 100644
--- a/include/linux/power/max17042_battery.h
+++ b/include/linux/power/max17042_battery.h
@@ -24,8 +24,12 @@
 #define __MAX17042_BATTERY_H_
 
 #define MAX17042_STATUS_BattAbsent	(1 << 3)
-#define MAX17042_BATTERY_FULL	(100)
+#define MAX17042_BATTERY_FULL		(95)   /* Recommend. FullSOCThr value */
 #define MAX17042_DEFAULT_SNS_RESISTOR	(10000)
+#define MAX17042_DEFAULT_VMIN		(3000)
+#define MAX17042_DEFAULT_VMAX		(4500) /* LiHV cell max */
+#define MAX17042_DEFAULT_TEMP_MIN	(0)    /* For sys without temp sensor */
+#define MAX17042_DEFAULT_TEMP_MAX	(700)  /* 70 degrees Celcius */
 
 #define MAX17042_CHARACTERIZATION_DATA_SIZE 48
 
-- 
2.9.3

[PATCH v2 05/14] power: max17042_battery: Change name in power_supply_desc to "main-battery"

[Hans de Goede]

The max17042 will typically be coupled with some charger IC, almost all
charger drivers contain:

static char *xxx_charger_supplied_to[] = {
        "main-battery",
};

probe()
{
	struct power_supply_config cfg;

	cfg.supplied_to = xxx_charger_supplied_to;
	cfg.num_supplicants = ARRAY_SIZE(xxx_charger_supplied_to);
	xxx->charger = power_supply_register(dev, &x_charger_desc, &_cfg);
}

Change the name in max17042's power_supply_desc to "main-battery" to
match, so that power_supply_am_i_supplied() can be used to implement
POWER_SUPPLY_PROP_STATUS.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/power/supply/max17042_battery.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index f0ff6e8..3249eb0 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -866,7 +866,7 @@ static const struct regmap_config max17042_regmap_config = {
 };
 
 static const struct power_supply_desc max17042_psy_desc = {
-	.name		= "max170xx_battery",
+	.name		= "main-battery",
 	.type		= POWER_SUPPLY_TYPE_BATTERY,
 	.get_property	= max17042_get_property,
 	.set_property	= max17042_set_property,
@@ -876,7 +876,7 @@ static const struct power_supply_desc max17042_psy_desc = {
 };
 
 static const struct power_supply_desc max17042_no_current_sense_psy_desc = {
-	.name		= "max170xx_battery",
+	.name		= "main-battery",
 	.type		= POWER_SUPPLY_TYPE_BATTERY,
 	.get_property	= max17042_get_property,
 	.set_property	= max17042_set_property,
-- 
2.9.3

[PATCH v2 06/14] power: max17042_battery: Add support for the STATUS property

[Hans de Goede]

Userspace prefers the driver having a status property over having to guess
itself. Specifically this will properly make the GNOME3 UI (and likely
others) properly show discharging / charging / full status, instead
of always showing discharging as status.

Note that in the case there is no charger driver supplying the max17042,
then a status of unknown will get returned. At least upower treats
this the same as not having a status attribute, so in this case nothing
changes from a userspace pov.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Move MAX17042_FULL_THRESHOLD define to max17042.h
---
 drivers/power/supply/max17042_battery.c | 46 +++++++++++++++++++++++++++++++++
 include/linux/power/max17042_battery.h  |  3 +++
 2 files changed, 49 insertions(+)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index 3249eb0..13909eb 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -76,6 +76,7 @@ struct max17042_chip {
 };
 
 static enum power_supply_property max17042_battery_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_CYCLE_COUNT,
 	POWER_SUPPLY_PROP_VOLTAGE_MAX,
@@ -113,6 +114,46 @@ static int max17042_get_temperature(struct max17042_chip *chip, int *temp)
 	return 0;
 }
 
+static int max17042_get_status(struct max17042_chip *chip, int *status)
+{
+	int ret, charge_full, charge_now;
+
+	ret = power_supply_am_i_supplied(chip->battery);
+	if (ret < 0) {
+		*status = POWER_SUPPLY_STATUS_UNKNOWN;
+		return 0;
+	}
+	if (ret == 0) {
+		*status = POWER_SUPPLY_STATUS_DISCHARGING;
+		return 0;
+	}
+
+	/*
+	 * The MAX170xx has builtin end-of-charge detection and will update
+	 * FullCAP to match RepCap when it detects end of charging.
+	 *
+	 * When this cycle the battery gets charged to a higher (calculated)
+	 * capacity then the previous cycle then FullCAP will get updated
+	 * contineously once end-of-charge detection kicks in, so allow the
+	 * 2 to differ a bit.
+	 */
+
+	ret = regmap_read(chip->regmap, MAX17042_FullCAP, &charge_full);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(chip->regmap, MAX17042_RepCap, &charge_now);
+	if (ret < 0)
+		return ret;
+
+	if ((charge_full - charge_now) <= MAX17042_FULL_THRESHOLD)
+		*status = POWER_SUPPLY_STATUS_FULL;
+	else
+		*status = POWER_SUPPLY_STATUS_CHARGING;
+
+	return 0;
+}
+
 static int max17042_get_battery_health(struct max17042_chip *chip, int *health)
 {
 	int temp, vavg, vbatt, ret;
@@ -182,6 +223,11 @@ static int max17042_get_property(struct power_supply *psy,
 		return -EAGAIN;
 
 	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		ret = max17042_get_status(chip, &val->intval);
+		if (ret < 0)
+			return ret;
+		break;
 	case POWER_SUPPLY_PROP_PRESENT:
 		ret = regmap_read(map, MAX17042_STATUS, &data);
 		if (ret < 0)
diff --git a/include/linux/power/max17042_battery.h b/include/linux/power/max17042_battery.h
index 3489fb0..a7ed29b 100644
--- a/include/linux/power/max17042_battery.h
+++ b/include/linux/power/max17042_battery.h
@@ -31,6 +31,9 @@
 #define MAX17042_DEFAULT_TEMP_MIN	(0)    /* For sys without temp sensor */
 #define MAX17042_DEFAULT_TEMP_MAX	(700)  /* 70 degrees Celcius */
 
+/* Consider RepCap which is less then 10 units below FullCAP full */
+#define MAX17042_FULL_THRESHOLD		10
+
 #define MAX17042_CHARACTERIZATION_DATA_SIZE 48
 
 enum max17042_register {
-- 
2.9.3

[PATCH v2 07/14] power: max17042_battery: Add external_power_changed callback

[Hans de Goede]

If our supplier changes status, chances are we've changed status too,
let any listeners know about this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/power/supply/max17042_battery.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index 13909eb..6fc41ec 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -429,6 +429,11 @@ static int max17042_property_is_writeable(struct power_supply *psy,
 	return ret;
 }
 
+static void max17042_external_power_changed(struct power_supply *psy)
+{
+	power_supply_changed(psy);
+}
+
 static int max17042_write_verify_reg(struct regmap *map, u8 reg, u32 value)
 {
 	int retries = 8;
@@ -917,6 +922,7 @@ static const struct power_supply_desc max17042_psy_desc = {
 	.get_property	= max17042_get_property,
 	.set_property	= max17042_set_property,
 	.property_is_writeable	= max17042_property_is_writeable,
+	.external_power_changed	= max17042_external_power_changed,
 	.properties	= max17042_battery_props,
 	.num_properties	= ARRAY_SIZE(max17042_battery_props),
 };
-- 
2.9.3

[PATCH v2 08/14] power: max17042_battery: Add support for the TECHNOLOGY attribute

[Hans de Goede]

The max17042 is intended for Li-Ion or Li-Po batteries, add a TECHNOLOGY
attribute to reflect this. Note this is hardcoded to Li-Ion as there is
no way to tell the difference, and Lithium-Ion Polymer batteries are
a sub-family of Lithium-Ion so Li-Ion technically is correct for both.

Using Li-Ion for both is already done by many drivers and is much
better then not providing any technology info at all.

Suggested-by: Wolfgang Wiedmeyer <wolfgit@wiedmeyer.de>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes for v2:
-Improved commit msg
-Added Suggested-by: Wolfgang Wiedmeyer
---
 drivers/power/supply/max17042_battery.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index 6fc41ec..c51847a 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -78,6 +78,7 @@ struct max17042_chip {
 static enum power_supply_property max17042_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
 	POWER_SUPPLY_PROP_CYCLE_COUNT,
 	POWER_SUPPLY_PROP_VOLTAGE_MAX,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
@@ -238,6 +239,9 @@ static int max17042_get_property(struct power_supply *psy,
 		else
 			val->intval = 1;
 		break;
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+		break;
 	case POWER_SUPPLY_PROP_CYCLE_COUNT:
 		ret = regmap_read(map, MAX17042_Cycles, &data);
 		if (ret < 0)
-- 
2.9.3

[PATCH v2 09/14] power: max17042_battery: Add support for the VOLT_MIN property

[Hans de Goede]

The info is there, so lets export it, like we already do for VOLT_MAX.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/power/supply/max17042_battery.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index c51847a..0ce6c65 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -81,6 +81,7 @@ static enum power_supply_property max17042_battery_props[] = {
 	POWER_SUPPLY_PROP_TECHNOLOGY,
 	POWER_SUPPLY_PROP_CYCLE_COUNT,
 	POWER_SUPPLY_PROP_VOLTAGE_MAX,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_AVG,
@@ -257,6 +258,13 @@ static int max17042_get_property(struct power_supply *psy,
 		val->intval = data >> 8;
 		val->intval *= 20000; /* Units of LSB = 20mV */
 		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		ret = regmap_read(map, MAX17042_MinMaxVolt, &data);
+		if (ret < 0)
+			return ret;
+
+		val->intval = (data & 0xff) * 20000; /* Units of 20mV */
+		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
 		if (chip->chip_type == MAXIM_DEVICE_TYPE_MAX17042)
 			ret = regmap_read(map, MAX17042_V_empty, &data);
-- 
2.9.3

[PATCH v2 10/14] power: max17042_battery: mAh readings depend on r_sns value

[Hans de Goede]

The PROP_CHARGE_FULL code was hardcoded for the default sense
resistor of 0.010 Ohm, make it use r_sns which contains the
actual sense resistor value in micro-Ohms instead.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-This is a new patch in v2 of this patch-set
---
 drivers/power/supply/max17042_battery.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index 0ce6c65..c380c5f 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -220,6 +220,7 @@ static int max17042_get_property(struct power_supply *psy,
 	struct regmap *map = chip->regmap;
 	int ret;
 	u32 data;
+	u64 data64;
 
 	if (!chip->init_complete)
 		return -EAGAIN;
@@ -309,7 +310,9 @@ static int max17042_get_property(struct power_supply *psy,
 		if (ret < 0)
 			return ret;
 
-		val->intval = data * 1000 / 2;
+		data64 = data * 5000000ll;
+		do_div(data64, chip->pdata->r_sns);
+		val->intval = data64;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_COUNTER:
 		ret = regmap_read(map, MAX17042_QH, &data);
-- 
2.9.3

[PATCH v2 11/14] power: max17042_battery: Add support for the CHARGE_FULL_DESIGN property

[Hans de Goede]

The info is there, lets export it as a property.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Take r_sns value into account when converting the register value to uAh
---
 drivers/power/supply/max17042_battery.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index c380c5f..39b0f86 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -87,6 +87,7 @@ static enum power_supply_property max17042_battery_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_AVG,
 	POWER_SUPPLY_PROP_VOLTAGE_OCV,
 	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
 	POWER_SUPPLY_PROP_CHARGE_FULL,
 	POWER_SUPPLY_PROP_CHARGE_COUNTER,
 	POWER_SUPPLY_PROP_TEMP,
@@ -305,6 +306,15 @@ static int max17042_get_property(struct power_supply *psy,
 
 		val->intval = data >> 8;
 		break;
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+		ret = regmap_read(map, MAX17042_DesignCap, &data);
+		if (ret < 0)
+			return ret;
+
+		data64 = data * 5000000ll;
+		do_div(data64, chip->pdata->r_sns);
+		val->intval = data64;
+		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL:
 		ret = regmap_read(map, MAX17042_FullCAP, &data);
 		if (ret < 0)
-- 
2.9.3

[PATCH v2 12/14] power: max17042_battery: Add support for the CHARGE_NOW property

[Hans de Goede]

Atleast upower prefers the more precise charge_now sysfs value over
capacity and the max17042 has the info, so lets export it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Take r_sns value into account when converting the register value to uAh
---
 drivers/power/supply/max17042_battery.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index 39b0f86..e80ff90 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -89,6 +89,7 @@ static enum power_supply_property max17042_battery_props[] = {
 	POWER_SUPPLY_PROP_CAPACITY,
 	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
 	POWER_SUPPLY_PROP_CHARGE_FULL,
+	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_CHARGE_COUNTER,
 	POWER_SUPPLY_PROP_TEMP,
 	POWER_SUPPLY_PROP_TEMP_ALERT_MIN,
@@ -324,6 +325,15 @@ static int max17042_get_property(struct power_supply *psy,
 		do_div(data64, chip->pdata->r_sns);
 		val->intval = data64;
 		break;
+	case POWER_SUPPLY_PROP_CHARGE_NOW:
+		ret = regmap_read(map, MAX17042_RepCap, &data);
+		if (ret < 0)
+			return ret;
+
+		data64 = data * 5000000ll;
+		do_div(data64, chip->pdata->r_sns);
+		val->intval = data64;
+		break;
 	case POWER_SUPPLY_PROP_CHARGE_COUNTER:
 		ret = regmap_read(map, MAX17042_QH, &data);
 		if (ret < 0)
-- 
2.9.3

[PATCH v2 13/14] power: max17042_battery: Add support for the SCOPE property

[Hans de Goede]

Add support for the SCOPE property, always return SCOPE_SYSTEM,
as the max170xx is used for the main battery on all known systems
with a max170xx.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/power/supply/max17042_battery.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index e80ff90..575a822 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -97,6 +97,7 @@ static enum power_supply_property max17042_battery_props[] = {
 	POWER_SUPPLY_PROP_TEMP_MIN,
 	POWER_SUPPLY_PROP_TEMP_MAX,
 	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_SCOPE,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
 	POWER_SUPPLY_PROP_CURRENT_AVG,
 };
@@ -371,6 +372,9 @@ static int max17042_get_property(struct power_supply *psy,
 		if (ret < 0)
 			return ret;
 		break;
+	case POWER_SUPPLY_PROP_SCOPE:
+		val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
+		break;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
 		if (chip->pdata->enable_current_sense) {
 			ret = regmap_read(map, MAX17042_Current, &data);
-- 
2.9.3

[PATCH v2 14/14] power: max77693_charger: Add supplied_to info to power_supply_config

[Hans de Goede]

On the exynos4412-trats2 the max77693 is used together with a
max17047 fuel-gauge. Add supplied_to info containing "main-battery",
so that the get_status code in the max17042_battery driver can use
power_supply_am_i_supplied and can properly let userspace know
if the battery is being charged or not.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/power/supply/max77693_charger.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/power/supply/max77693_charger.c b/drivers/power/supply/max77693_charger.c
index 6c78884..1eeed26 100644
--- a/drivers/power/supply/max77693_charger.c
+++ b/drivers/power/supply/max77693_charger.c
@@ -254,6 +254,10 @@ static int max77693_charger_get_property(struct power_supply *psy,
 	return ret;
 }
 
+static char *max77693_charger_supplied_to[] = {
+	"main-battery",
+};
+
 static const struct power_supply_desc max77693_charger_desc = {
 	.name		= MAX77693_CHARGER_NAME,
 	.type		= POWER_SUPPLY_TYPE_BATTERY,
@@ -697,6 +701,8 @@ static int max77693_charger_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	psy_cfg.supplied_to = max77693_charger_supplied_to;
+	psy_cfg.num_supplicants = ARRAY_SIZE(max77693_charger_supplied_to),
 	psy_cfg.drv_data = chg;
 
 	ret = device_create_file(&pdev->dev, &dev_attr_fast_charge_timer);
-- 
2.9.3

Re: [PATCH v2 01/14] power: Make power_supply_am_i_supplied return -ENODEV if there are no suppliers

[Krzysztof Kozlowski]

On Fri, Apr 14, 2017 at 08:32:46PM +0200, Hans de Goede wrote:
> It is sensible to assume that the hardware actually always has a
> way of charging the battery so when power_supply_am_i_supplied does not
> find any suppliers, that does not mean that there are none, but simply
> that no power_supply-drivers are registered / bound for any suppliers for
> the supply calling power_supply_am_i_supplied.
> 
> At which point a fuel-gauge driver calling power_supply_am_i_supplied()
> cannot determine whether the battery is being charged or not.
> 
> Allow a caller of power_supply_am_i_supplied to differentiate between
> there not being any suppliers, vs no suppliers being online by returning
> -ENODEV if there are no suppliers matching supplied_to / supplied_from,
> which allows fuel-gauge drivers to return POWER_SUPPLY_STATUS_UNKNOWN
> rather then POWER_SUPPLY_STATUS_DISCHARGING if there are no suppliers.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Improve commit message
> ---
>  drivers/power/supply/power_supply_core.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 1e0960b..13a39da 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -280,13 +280,19 @@ static inline int power_supply_check_supplies(struct power_supply *psy)
>  }
>  #endif
>  
> -static int __power_supply_am_i_supplied(struct device *dev, void *data)
> +struct am_i_supplied_data {
> +	struct power_supply *psy;
> +	unsigned int count;
> +};

I think you missed my previous comment about adding a prefix - psy?

Best regards,
Krzysztof

Re: [PATCH v2 04/14] power: max17042_battery: Add default platform_data fallback data

[Krzysztof Kozlowski]

On Fri, Apr 14, 2017 at 08:32:49PM +0200, Hans de Goede wrote:
> Some x86 machines use a max17047 fuel-gauge and x86 might be missing
> platform_data if not provided by SFI.
> 
> This commit adds default platform_data as fallback option so that the
> driver can work on boards where no platform_data is provided.
> 
> Since not all boards have a thermistor hooked up, set temp_min to 0 and
> change the health checks from temp <= temp_min to temp < temp_min to
> not trigger on such boards (where temp reads 0).
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Reword commit message
> -Add MAX17042_DEFAULT_FOO defines to include/linux/power/max17042.h and
>  use these to set the default pdata values
> ---
>  drivers/power/supply/max17042_battery.c | 60 +++++++++++++++++++++++++++++----
>  include/linux/power/max17042_battery.h  |  6 +++-
>  2 files changed, 58 insertions(+), 8 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2 06/14] power: max17042_battery: Add support for the STATUS property

[Krzysztof Kozlowski]

On Fri, Apr 14, 2017 at 08:32:51PM +0200, Hans de Goede wrote:
> Userspace prefers the driver having a status property over having to guess
> itself. Specifically this will properly make the GNOME3 UI (and likely
> others) properly show discharging / charging / full status, instead
> of always showing discharging as status.
> 
> Note that in the case there is no charger driver supplying the max17042,
> then a status of unknown will get returned. At least upower treats
> this the same as not having a status attribute, so in this case nothing
> changes from a userspace pov.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Move MAX17042_FULL_THRESHOLD define to max17042.h
> ---
>  drivers/power/supply/max17042_battery.c | 46 +++++++++++++++++++++++++++++++++
>  include/linux/power/max17042_battery.h  |  3 +++
>  2 files changed, 49 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2 08/14] power: max17042_battery: Add support for the TECHNOLOGY attribute

[Krzysztof Kozlowski]

On Fri, Apr 14, 2017 at 08:32:53PM +0200, Hans de Goede wrote:
> The max17042 is intended for Li-Ion or Li-Po batteries, add a TECHNOLOGY
> attribute to reflect this. Note this is hardcoded to Li-Ion as there is
> no way to tell the difference, and Lithium-Ion Polymer batteries are
> a sub-family of Lithium-Ion so Li-Ion technically is correct for both.
> 
> Using Li-Ion for both is already done by many drivers and is much
> better then not providing any technology info at all.
> 
> Suggested-by: Wolfgang Wiedmeyer <wolfgit@wiedmeyer.de>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes for v2:
> -Improved commit msg
> -Added Suggested-by: Wolfgang Wiedmeyer
> ---
>  drivers/power/supply/max17042_battery.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2 10/14] power: max17042_battery: mAh readings depend on r_sns value

[Krzysztof Kozlowski]

On Fri, Apr 14, 2017 at 08:32:55PM +0200, Hans de Goede wrote:
> The PROP_CHARGE_FULL code was hardcoded for the default sense
> resistor of 0.010 Ohm, make it use r_sns which contains the
> actual sense resistor value in micro-Ohms instead.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -This is a new patch in v2 of this patch-set
> ---
>  drivers/power/supply/max17042_battery.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2 11/14] power: max17042_battery: Add support for the CHARGE_FULL_DESIGN property

[Krzysztof Kozlowski]

On Fri, Apr 14, 2017 at 08:32:56PM +0200, Hans de Goede wrote:
> The info is there, lets export it as a property.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Take r_sns value into account when converting the register value to uAh
> ---
>  drivers/power/supply/max17042_battery.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2 12/14] power: max17042_battery: Add support for the CHARGE_NOW property

[Krzysztof Kozlowski]

On Fri, Apr 14, 2017 at 08:32:57PM +0200, Hans de Goede wrote:
> Atleast upower prefers the more precise charge_now sysfs value over

s/Atleast/At least/ ?

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

> capacity and the max17042 has the info, so lets export it.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Take r_sns value into account when converting the register value to uAh
> ---
>  drivers/power/supply/max17042_battery.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Re: [PATCH v2 02/14] power: max17042_battery: Use sign_extend32 instead of DIY code

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:47PM +0200, Hans de Goede wrote:
> Use sign_extend32 to sign-extend variables where necessary instead of
> DIY code.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 03/14] power: max17047_battery: The temp alert values are 8-bit 2's complement

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:48PM +0200, Hans de Goede wrote:
> The temp alert values are 8-bit 2's complement, so sign-extend them
> before reporting them back to the caller.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 04/14] power: max17042_battery: Add default platform_data fallback data

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:49PM +0200, Hans de Goede wrote:
> Some x86 machines use a max17047 fuel-gauge and x86 might be missing
> platform_data if not provided by SFI.
> 
> This commit adds default platform_data as fallback option so that the
> driver can work on boards where no platform_data is provided.
> 
> Since not all boards have a thermistor hooked up, set temp_min to 0 and
> change the health checks from temp <= temp_min to temp < temp_min to
> not trigger on such boards (where temp reads 0).
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 06/14] power: max17042_battery: Add support for the STATUS property

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:51PM +0200, Hans de Goede wrote:
> Userspace prefers the driver having a status property over having to guess
> itself. Specifically this will properly make the GNOME3 UI (and likely
> others) properly show discharging / charging / full status, instead
> of always showing discharging as status.
> 
> Note that in the case there is no charger driver supplying the max17042,
> then a status of unknown will get returned. At least upower treats
> this the same as not having a status attribute, so in this case nothing
> changes from a userspace pov.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 07/14] power: max17042_battery: Add external_power_changed callback

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:52PM +0200, Hans de Goede wrote:
> If our supplier changes status, chances are we've changed status too,
> let any listeners know about this.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 08/14] power: max17042_battery: Add support for the TECHNOLOGY attribute

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:53PM +0200, Hans de Goede wrote:
> The max17042 is intended for Li-Ion or Li-Po batteries, add a TECHNOLOGY
> attribute to reflect this. Note this is hardcoded to Li-Ion as there is
> no way to tell the difference, and Lithium-Ion Polymer batteries are
> a sub-family of Lithium-Ion so Li-Ion technically is correct for both.
> 
> Using Li-Ion for both is already done by many drivers and is much
> better then not providing any technology info at all.
> 
> Suggested-by: Wolfgang Wiedmeyer <wolfgit@wiedmeyer.de>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 09/14] power: max17042_battery: Add support for the VOLT_MIN property

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:54PM +0200, Hans de Goede wrote:
> The info is there, so lets export it, like we already do for VOLT_MAX.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 10/14] power: max17042_battery: mAh readings depend on r_sns value

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:55PM +0200, Hans de Goede wrote:
> The PROP_CHARGE_FULL code was hardcoded for the default sense
> resistor of 0.010 Ohm, make it use r_sns which contains the
> actual sense resistor value in micro-Ohms instead.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 11/14] power: max17042_battery: Add support for the CHARGE_FULL_DESIGN property

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:56PM +0200, Hans de Goede wrote:
> The info is there, lets export it as a property.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 12/14] power: max17042_battery: Add support for the CHARGE_NOW property

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:57PM +0200, Hans de Goede wrote:
> Atleast upower prefers the more precise charge_now sysfs value over
> capacity and the max17042 has the info, so lets export it.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 13/14] power: max17042_battery: Add support for the SCOPE property

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:58PM +0200, Hans de Goede wrote:
> Add support for the SCOPE property, always return SCOPE_SYSTEM,
> as the max170xx is used for the main battery on all known systems
> with a max170xx.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Thanks, queued.

-- Sebastian

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

Re: [PATCH v2 14/14] power: max77693_charger: Add supplied_to info to power_supply_config

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:59PM +0200, Hans de Goede wrote:
> On the exynos4412-trats2 the max77693 is used together with a
> max17047 fuel-gauge.

exynos4412-trats2 is DT based, so it should use the method
described in Documentation/devicetree/bindings/power/supply/power_supply.txt
to describe the supply chain.

-- Sebastian

> Add supplied_to info containing "main-battery",
> so that the get_status code in the max17042_battery driver can use
> power_supply_am_i_supplied and can properly let userspace know
> if the battery is being charged or not.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  drivers/power/supply/max77693_charger.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/power/supply/max77693_charger.c b/drivers/power/supply/max77693_charger.c
> index 6c78884..1eeed26 100644
> --- a/drivers/power/supply/max77693_charger.c
> +++ b/drivers/power/supply/max77693_charger.c
> @@ -254,6 +254,10 @@ static int max77693_charger_get_property(struct power_supply *psy,
>  	return ret;
>  }
>  
> +static char *max77693_charger_supplied_to[] = {
> +	"main-battery",
> +};
> +
>  static const struct power_supply_desc max77693_charger_desc = {
>  	.name		= MAX77693_CHARGER_NAME,
>  	.type		= POWER_SUPPLY_TYPE_BATTERY,
> @@ -697,6 +701,8 @@ static int max77693_charger_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	psy_cfg.supplied_to = max77693_charger_supplied_to;
> +	psy_cfg.num_supplicants = ARRAY_SIZE(max77693_charger_supplied_to),
>  	psy_cfg.drv_data = chg;
>  
>  	ret = device_create_file(&pdev->dev, &dev_attr_fast_charge_timer);

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

Re: [PATCH v2 05/14] power: max17042_battery: Change name in power_supply_desc to "main-battery"

[Sebastian Reichel]

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

Hi,

On Fri, Apr 14, 2017 at 08:32:50PM +0200, Hans de Goede wrote:
> The max17042 will typically be coupled with some charger IC, almost all
> charger drivers contain:
> 
> static char *xxx_charger_supplied_to[] = {
>         "main-battery",
> };
> 
> probe()
> {
> 	struct power_supply_config cfg;
> 
> 	cfg.supplied_to = xxx_charger_supplied_to;
> 	cfg.num_supplicants = ARRAY_SIZE(xxx_charger_supplied_to);
> 	xxx->charger = power_supply_register(dev, &x_charger_desc, &_cfg);
> }
> 
> Change the name in max17042's power_supply_desc to "main-battery" to
> match, so that power_supply_am_i_supplied() can be used to implement
> POWER_SUPPLY_PROP_STATUS.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

I did not queue this patch and patch 14. First of all: Currently 4/30
chargers have this, which I wouldn't call "almost all". I think this
is not the right solution, especially as it will break once a system
has more than one "main" battery (like some of the newer thinkpads).

For DT based systems we have generic support to specify the
dependency as phandle. This obviously won't work for you.
I suggest to use a device property for supplying the correct name
to the charger instead.

-- Sebastian

> ---
>  drivers/power/supply/max17042_battery.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
> index f0ff6e8..3249eb0 100644
> --- a/drivers/power/supply/max17042_battery.c
> +++ b/drivers/power/supply/max17042_battery.c
> @@ -866,7 +866,7 @@ static const struct regmap_config max17042_regmap_config = {
>  };
>  
>  static const struct power_supply_desc max17042_psy_desc = {
> -	.name		= "max170xx_battery",
> +	.name		= "main-battery",
>  	.type		= POWER_SUPPLY_TYPE_BATTERY,
>  	.get_property	= max17042_get_property,
>  	.set_property	= max17042_set_property,
> @@ -876,7 +876,7 @@ static const struct power_supply_desc max17042_psy_desc = {
>  };
>  
>  static const struct power_supply_desc max17042_no_current_sense_psy_desc = {
> -	.name		= "max170xx_battery",
> +	.name		= "main-battery",
>  	.type		= POWER_SUPPLY_TYPE_BATTERY,
>  	.get_property	= max17042_get_property,
>  	.set_property	= max17042_set_property,
> -- 
> 2.9.3
> 

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

Re: [PATCH v2 05/14] power: max17042_battery: Change name in power_supply_desc to "main-battery"

[Hans de Goede]

Hi Sebastian,

Thank you for merging all my recent patches.

On 01-05-17 14:22, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Apr 14, 2017 at 08:32:50PM +0200, Hans de Goede wrote:
>> The max17042 will typically be coupled with some charger IC, almost all
>> charger drivers contain:
>>
>> static char *xxx_charger_supplied_to[] = {
>>          "main-battery",
>> };
>>
>> probe()
>> {
>> 	struct power_supply_config cfg;
>>
>> 	cfg.supplied_to = xxx_charger_supplied_to;
>> 	cfg.num_supplicants = ARRAY_SIZE(xxx_charger_supplied_to);
>> 	xxx->charger = power_supply_register(dev, &x_charger_desc, &_cfg);
>> }
>>
>> Change the name in max17042's power_supply_desc to "main-battery" to
>> match, so that power_supply_am_i_supplied() can be used to implement
>> POWER_SUPPLY_PROP_STATUS.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> I did not queue this patch and patch 14. First of all: Currently 4/30
> chargers have this, which I wouldn't call "almost all". I think this
> is not the right solution, especially as it will break once a system
> has more than one "main" battery (like some of the newer thinkpads).
> 
> For DT based systems we have generic support to specify the
> dependency as phandle. This obviously won't work for you.
> I suggest to use a device property for supplying the correct name
> to the charger instead.

Ok, fair enough.

So to be clear you are suggesting that we modify the charger driver
(or the power-supply-core) to check for a "supplied-to"
string-array device property which then overrides the power_supply_config.supplied_to
array's default value in the driver?

Or do you want to modify the fuel-gauge driver (or the power-supply-core)
to check for a "supplied-from" string-array device property which then
is used to fill in the power_supply.supplied_from array ?

Looking at how currently the power-supply-core fills in power_supply.supplied_from
from devicetree when #ifdef CONFIG_OF is true, I think the best (and
most consistent) solution would be to change the :

#else
static inline int power_supply_check_supplies(struct power_supply *psy)
{
         return 0;
}
#endif

Code block to check for a "supplied-from" string-array device property
and populate the power_supply.supplied_from string array with its
contents if found.

If you can let me know how exactly you want to solve this I can whip
up (and test) patch for this.

Regards,

Hans



> 
> -- Sebastian
> 
>> ---
>>   drivers/power/supply/max17042_battery.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
>> index f0ff6e8..3249eb0 100644
>> --- a/drivers/power/supply/max17042_battery.c
>> +++ b/drivers/power/supply/max17042_battery.c
>> @@ -866,7 +866,7 @@ static const struct regmap_config max17042_regmap_config = {
>>   };
>>   
>>   static const struct power_supply_desc max17042_psy_desc = {
>> -	.name		= "max170xx_battery",
>> +	.name		= "main-battery",
>>   	.type		= POWER_SUPPLY_TYPE_BATTERY,
>>   	.get_property	= max17042_get_property,
>>   	.set_property	= max17042_set_property,
>> @@ -876,7 +876,7 @@ static const struct power_supply_desc max17042_psy_desc = {
>>   };
>>   
>>   static const struct power_supply_desc max17042_no_current_sense_psy_desc = {
>> -	.name		= "max170xx_battery",
>> +	.name		= "main-battery",
>>   	.type		= POWER_SUPPLY_TYPE_BATTERY,
>>   	.get_property	= max17042_get_property,
>>   	.set_property	= max17042_set_property,
>> -- 
>> 2.9.3
>>

Re: [PATCH v2 05/14] power: max17042_battery: Change name in power_supply_desc to "main-battery"

[Sebastian Reichel]

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

Hi Hans,

On Mon, May 01, 2017 at 05:11:13PM +0200, Hans de Goede wrote:
> Thank you for merging all my recent patches.

You are welcome!

> On 01-05-17 14:22, Sebastian Reichel wrote:
> > > Change the name in max17042's power_supply_desc to "main-battery" to
> > > match, so that power_supply_am_i_supplied() can be used to implement
> > > POWER_SUPPLY_PROP_STATUS.
> > 
> > I did not queue this patch and patch 14. First of all: Currently 4/30
> > chargers have this, which I wouldn't call "almost all". I think this
> > is not the right solution, especially as it will break once a system
> > has more than one "main" battery (like some of the newer thinkpads).
> > 
> > For DT based systems we have generic support to specify the
> > dependency as phandle. This obviously won't work for you.
> > I suggest to use a device property for supplying the correct name
> > to the charger instead.
> 
> Ok, fair enough.
> 
> So to be clear you are suggesting that we modify the charger driver
> (or the power-supply-core) to check for a "supplied-to"
> string-array device property which then overrides the power_supply_config.supplied_to
> array's default value in the driver?
> 
> Or do you want to modify the fuel-gauge driver (or the power-supply-core)
> to check for a "supplied-from" string-array device property which then
> is used to fill in the power_supply.supplied_from array ?
> 
> Looking at how currently the power-supply-core fills in power_supply.supplied_from
> from devicetree when #ifdef CONFIG_OF is true, I think the best (and
> most consistent) solution would be to change the :
> 
> #else
> static inline int power_supply_check_supplies(struct power_supply *psy)
> {
>         return 0;
> }
> #endif
> 
> Code block to check for a "supplied-from" string-array device property
> and populate the power_supply.supplied_from string array with its
> contents if found.
> 
> If you can let me know how exactly you want to solve this I can whip
> up (and test) patch for this.

Adding this to non-DT power_supply_check_supplies() and naming the
property "supplied-from" sounds good.

-- Sebastian

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