[PATCH 00/15] power_supply: Fixes & cleanups

[PATCH 00/15] power_supply: Fixes & cleanups

[Viresh Kumar]

Hi Guys,

I was required to work on a battery driver and found some fixes/cleanups while
going through the core.

First two are fixes and rest are cleanups. Please see if they make any sense at
all.

Pushed here: git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git battery/fixes

--
viresh

Viresh Kumar (15):
  power-supply: Don't over-allocate memory for "supplied-from" array
  power-supply: Return early if "power-supplies" property isn't valid
  Documentation: Charger Manager: Fix spelling mistakes
  power-supply: Forward declare structs together
  power-supply: Drop unnecessary typecasts
  power-supply: Use 'break' instead of 'continue' to end loop
  power-supply: Rearrange code to remove duplicate lines
  power-supply: Propagate error returned by
    power_supply_find_supply_from_node()
  power-supply: Don't return -EINVAL from
    __power_supply_find_supply_from_node()
  power-supply: Drop useless 'if (ret.intval)' statements
  power-supply: Mark 'if' blocks in power_supply_changed_work() with
    'likely'
  power-supply: Use PTR_ERR_OR_ZERO() routine
  power-supply: Check for failures only when we can fail
  power-supply: Avoid unnecessary 'goto' statements
  power_supply: Don't iterate over devices to return -EPROBE_DEFER

 Documentation/power/charger-manager.txt |   2 +-
 drivers/power/power_supply_core.c       | 100 +++++++++++++++-----------------
 drivers/power/power_supply_leds.c       |  19 ++----
 drivers/power/power_supply_sysfs.c      |  21 +++----
 include/linux/power_supply.h            |   3 +-
 5 files changed, 64 insertions(+), 81 deletions(-)

-- 
2.0.3.693.g996b0fd

[PATCH 01/15] power-supply: Don't over-allocate memory for "supplied-from" array

[Viresh Kumar]

In routine power_supply_check_supplies(), 'cnt' is counting the number of
supplies passed in "power-supplies" field of a node. The value of 'cnt' will
always be one more than the number of supplies after the do-while loop ends. And
so we need to allocate memory for 'cnt - 1' char pointers. But we are allocating
memory for 'cnt' instead.

Fix this by not over-allocating memory.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 078afd6..10f0b57 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -234,7 +234,7 @@ static int power_supply_check_supplies(struct power_supply *psy)
 		return -ENOMEM;
 	}
 
-	*psy->supplied_from = devm_kzalloc(psy->dev, sizeof(char *) * cnt,
+	*psy->supplied_from = devm_kzalloc(psy->dev, sizeof(char *) * (cnt - 1),
 					   GFP_KERNEL);
 	if (!*psy->supplied_from) {
 		dev_err(psy->dev, "Couldn't allocate memory for supply list\n");
-- 
2.0.3.693.g996b0fd

[PATCH 02/15] power-supply: Return early if "power-supplies" property isn't valid

[Viresh Kumar]

If power-supply's DT node doesn't have a valid "power-supplies" entry, then
power_supply_check_supplies() should return early instead of trying to allocate
memory for "supplied_from" array.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 10f0b57..414384a 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -226,6 +226,10 @@ static int power_supply_check_supplies(struct power_supply *psy)
 		of_node_put(np);
 	} while (np);
 
+	/* Missing valid "power-supplies" entries */
+	if (cnt == 1)
+		return 0;
+
 	/* All supplies found, allocate char ** array for filling */
 	psy->supplied_from = devm_kzalloc(psy->dev, sizeof(psy->supplied_from),
 					  GFP_KERNEL);
-- 
2.0.3.693.g996b0fd

[PATCH 03/15] Documentation: Charger Manager: Fix spelling mistakes

[Viresh Kumar]

'unnecessary' was wrongly spelled as 'unncessary', also it should have been
'unnecessarily'.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/power/charger-manager.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/power/charger-manager.txt b/Documentation/power/charger-manager.txt
index b4f7f4b..9ff1105 100644
--- a/Documentation/power/charger-manager.txt
+++ b/Documentation/power/charger-manager.txt
@@ -29,7 +29,7 @@ the system may need multiple instances of Charger Manager.
 	While the battery is being charged and the system is in suspend-to-RAM,
 	we may need to monitor the battery health by looking at the ambient or
 	battery temperature. We can accomplish this by waking up the system
-	periodically. However, such a method wakes up devices unncessary for
+	periodically. However, such a method wakes up devices unnecessarily for
 	monitoring the battery health and tasks, and user processes that are
 	supposed to be kept suspended. That, in turn, incurs unnecessary power
 	consumption and slow down charging process. Or even, such peak power
-- 
2.0.3.693.g996b0fd

[PATCH 04/15] power-supply: Forward declare structs together

[Viresh Kumar]

power_supply.h requires to forward declare few structures. One of them is done
at the top of the file and other one just before it is used. Declare them
together for better readability.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/power_supply.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index f3dea41..8c18412 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -18,8 +18,6 @@
 #include <linux/spinlock.h>
 #include <linux/notifier.h>
 
-struct device;
-
 /*
  * All voltages, currents, charges, energies, time and temperatures in uV,
  * µA, µAh, µWh, seconds and tenths of degree Celsius unless otherwise
@@ -172,6 +170,7 @@ union power_supply_propval {
 	const char *strval;
 };
 
+struct device;
 struct device_node;
 
 struct power_supply {
-- 
2.0.3.693.g996b0fd

[PATCH 05/15] power-supply: Drop unnecessary typecasts

[Viresh Kumar]

Typecast from 'void *' to any other pointer type falls under implicit typecasts
category and so doesn't require explicit typecasts. Drop them.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 414384a..71d00ec 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -58,7 +58,7 @@ static bool __power_supply_is_supplied_by(struct power_supply *supplier,
 
 static int __power_supply_changed_work(struct device *dev, void *data)
 {
-	struct power_supply *psy = (struct power_supply *)data;
+	struct power_supply *psy = data;
 	struct power_supply *pst = dev_get_drvdata(dev);
 
 	if (__power_supply_is_supplied_by(psy, pst)) {
@@ -119,7 +119,7 @@ EXPORT_SYMBOL_GPL(power_supply_changed);
 static int __power_supply_populate_supplied_from(struct device *dev,
 						 void *data)
 {
-	struct power_supply *psy = (struct power_supply *)data;
+	struct power_supply *psy = data;
 	struct power_supply *epsy = dev_get_drvdata(dev);
 	struct device_node *np;
 	int i = 0;
@@ -158,7 +158,7 @@ static int power_supply_populate_supplied_from(struct power_supply *psy)
 static int  __power_supply_find_supply_from_node(struct device *dev,
 						 void *data)
 {
-	struct device_node *np = (struct device_node *)data;
+	struct device_node *np = data;
 	struct power_supply *epsy = dev_get_drvdata(dev);
 
 	/* return error breaks out of class_for_each_device loop */
@@ -257,7 +257,7 @@ static inline int power_supply_check_supplies(struct power_supply *psy)
 static int __power_supply_am_i_supplied(struct device *dev, void *data)
 {
 	union power_supply_propval ret = {0,};
-	struct power_supply *psy = (struct power_supply *)data;
+	struct power_supply *psy = data;
 	struct power_supply *epsy = dev_get_drvdata(dev);
 
 	if (__power_supply_is_supplied_by(epsy, psy))
-- 
2.0.3.693.g996b0fd

[PATCH 06/15] power-supply: Use 'break' instead of 'continue' to end loop

[Viresh Kumar]

In few routines, we need to end the do-while loop when no more "power-supplies"
are available. Currently we are doing 'continue' which will make the
'while(np)' conditional statement run again.

Skip this by doing a 'break' instead.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 71d00ec..7657335 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -127,7 +127,7 @@ static int __power_supply_populate_supplied_from(struct device *dev,
 	do {
 		np = of_parse_phandle(psy->of_node, "power-supplies", i++);
 		if (!np)
-			continue;
+			break;
 
 		if (np == epsy->of_node) {
 			dev_info(psy->dev, "%s: Found supply : %s\n",
@@ -215,7 +215,7 @@ static int power_supply_check_supplies(struct power_supply *psy)
 
 		np = of_parse_phandle(psy->of_node, "power-supplies", cnt++);
 		if (!np)
-			continue;
+			break;
 
 		ret = power_supply_find_supply_from_node(np);
 		if (ret) {
-- 
2.0.3.693.g996b0fd

[PATCH 07/15] power-supply: Rearrange code to remove duplicate lines

[Viresh Kumar]

of_node_put() was called twice in power_supply_check_supplies() whereas a single
call will also work. Rearrange code a bit to make that feasible.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 7657335..8a86cd1 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -218,12 +218,12 @@ static int power_supply_check_supplies(struct power_supply *psy)
 			break;
 
 		ret = power_supply_find_supply_from_node(np);
+		of_node_put(np);
+
 		if (ret) {
 			dev_dbg(psy->dev, "Failed to find supply, defer!\n");
-			of_node_put(np);
 			return -EPROBE_DEFER;
 		}
-		of_node_put(np);
 	} while (np);
 
 	/* Missing valid "power-supplies" entries */
-- 
2.0.3.693.g996b0fd

[PATCH 08/15] power-supply: Propagate error returned by power_supply_find_supply_from_node()

[Viresh Kumar]

Callers of power_supply_find_supply_from_node(), i.e.
power_supply_check_supplies(), must propagate the errors returned by it instead
of returning their own.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 8a86cd1..ab1cf8b 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -221,8 +221,8 @@ static int power_supply_check_supplies(struct power_supply *psy)
 		of_node_put(np);
 
 		if (ret) {
-			dev_dbg(psy->dev, "Failed to find supply, defer!\n");
-			return -EPROBE_DEFER;
+			dev_dbg(psy->dev, "Failed to find supply!\n");
+			return ret;
 		}
 	} while (np);
 
-- 
2.0.3.693.g996b0fd

[PATCH 09/15] power-supply: Don't return -EINVAL from __power_supply_find_supply_from_node()

[Viresh Kumar]

We need to stop 'class_for_each_device' loop when a supply matches with the
of-node. In order to achieve this we currently return -EINVAL from
__power_supply_populate_supplied_from() on successful match.

class_for_each_device() is free to return similar errors in other cases as well
and so the choice of return value here isn't particularly great.

This commit isn't removing the Hack but making it more elegant by returning '1'
instead.

Also power_supply_find_supply_from_node() can return errors other than
-EPROBE_DEFER now if class_for_each_device() fails.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index ab1cf8b..55140eb 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -161,9 +161,9 @@ static int  __power_supply_find_supply_from_node(struct device *dev,
 	struct device_node *np = data;
 	struct power_supply *epsy = dev_get_drvdata(dev);
 
-	/* return error breaks out of class_for_each_device loop */
+	/* returning non-zero breaks out of class_for_each_device loop */
 	if (epsy->of_node == np)
-		return -EINVAL;
+		return 1;
 
 	return 0;
 }
@@ -186,15 +186,19 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
 		return -EPROBE_DEFER;
 
 	/*
-	 * We have to treat the return value as inverted, because if
-	 * we return error on not found, then it won't continue looking.
-	 * So we trick it by returning error on success to stop looking
-	 * once the matching device is found.
+	 * class_for_each_device() either returns its own errors or values
+	 * returned by __power_supply_find_supply_from_node().
+	 *
+	 * __power_supply_find_supply_from_node() will return 0 (no match)
+	 * or 1 (match).
+	 *
+	 * We return 0 if class_for_each_device() returned 1, -EPROBE_DEFER if
+	 * it returned 0, or error as returned by it.
 	 */
 	error = class_for_each_device(power_supply_class, NULL, supply_node,
 				       __power_supply_find_supply_from_node);
 
-	return error ? 0 : -EPROBE_DEFER;
+	return error ? (error == 1 ? 0 : error) : -EPROBE_DEFER;
 }
 
 static int power_supply_check_supplies(struct power_supply *psy)
-- 
2.0.3.693.g996b0fd

[PATCH 10/15] power-supply: Drop useless 'if (ret.intval)' statements

[Viresh Kumar]

There is no need to check the value of ret.intval before returning it, as we
will be returning zero explicitly when ret.intval is zero.

So essentially we will end up returning value of ret.intval as it is. Drop the
unnecessary 'if' statements.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 55140eb..bcff7fd 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -265,10 +265,8 @@ static int __power_supply_am_i_supplied(struct device *dev, void *data)
 	struct power_supply *epsy = dev_get_drvdata(dev);
 
 	if (__power_supply_is_supplied_by(epsy, psy))
-		if (!epsy->get_property(epsy, POWER_SUPPLY_PROP_ONLINE, &ret)) {
-			if (ret.intval)
-				return ret.intval;
-		}
+		if (!epsy->get_property(epsy, POWER_SUPPLY_PROP_ONLINE, &ret))
+			return ret.intval;
 
 	return 0;
 }
@@ -293,12 +291,10 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data)
 	unsigned int *count = data;
 
 	(*count)++;
-	if (psy->type != POWER_SUPPLY_TYPE_BATTERY) {
-		if (psy->get_property(psy, POWER_SUPPLY_PROP_ONLINE, &ret))
-			return 0;
-		if (ret.intval)
+	if (psy->type != POWER_SUPPLY_TYPE_BATTERY)
+		if (!psy->get_property(psy, POWER_SUPPLY_PROP_ONLINE, &ret))
 			return ret.intval;
-	}
+
 	return 0;
 }
 
-- 
2.0.3.693.g996b0fd

[PATCH 11/15] power-supply: Mark 'if' blocks in power_supply_changed_work() with 'likely'

[Viresh Kumar]

The 'if' statements in power_supply_changed_work() are mostly there for taking
care of races and normally they will always evaluate to true. Optimize them for
fast execution with 'likely' statements.

Also there is need to have better comments in code to mention about the races
clearly. Get them in place.

Cc: Zoran Markovic <zrn.markovic@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index bcff7fd..26518c8 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -78,7 +78,14 @@ static void power_supply_changed_work(struct work_struct *work)
 	dev_dbg(psy->dev, "%s\n", __func__);
 
 	spin_lock_irqsave(&psy->changed_lock, flags);
-	if (psy->changed) {
+	/*
+	 * Check 'changed' here to avoid issues due to race between
+	 * power_supply_changed() and this routine. In worst case
+	 * power_supply_changed() can be called again just before we take above
+	 * lock. During the first call of this routine we will mark 'changed' as
+	 * false and it will stay false for the next call as well.
+	 */
+	if (likely(psy->changed)) {
 		psy->changed = false;
 		spin_unlock_irqrestore(&psy->changed_lock, flags);
 		class_for_each_device(power_supply_class, NULL, psy,
@@ -89,12 +96,13 @@ static void power_supply_changed_work(struct work_struct *work)
 		kobject_uevent(&psy->dev->kobj, KOBJ_CHANGE);
 		spin_lock_irqsave(&psy->changed_lock, flags);
 	}
+
 	/*
-	 * Dependent power supplies (e.g. battery) may have changed state
-	 * as a result of this event, so poll again and hold the
-	 * wakeup_source until all events are processed.
+	 * Hold the wakeup_source until all events are processed.
+	 * power_supply_changed() might have called again and have set 'changed'
+	 * to true.
 	 */
-	if (!psy->changed)
+	if (likely(!psy->changed))
 		pm_relax(psy->dev);
 	spin_unlock_irqrestore(&psy->changed_lock, flags);
 }
-- 
2.0.3.693.g996b0fd

[PATCH 12/15] power-supply: Use PTR_ERR_OR_ZERO() routine

[Viresh Kumar]

At multiple places we are doing exactly what PTR_ERR_OR_ZERO() does. And so that
routine can be reused instead of increasing lines of code here.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 26518c8..376464e 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -435,9 +435,7 @@ static int psy_register_thermal(struct power_supply *psy)
 		if (psy->properties[i] == POWER_SUPPLY_PROP_TEMP) {
 			psy->tzd = thermal_zone_device_register(psy->name, 0, 0,
 					psy, &psy_tzd_ops, NULL, 0, 0);
-			if (IS_ERR(psy->tzd))
-				return PTR_ERR(psy->tzd);
-			break;
+			return PTR_ERR_OR_ZERO(psy->tzd);
 		}
 	}
 	return 0;
@@ -515,9 +513,7 @@ static int psy_register_cooler(struct power_supply *psy)
 			psy->tcd = thermal_cooling_device_register(
 							(char *)psy->name,
 							psy, &psy_tcd_ops);
-			if (IS_ERR(psy->tcd))
-				return PTR_ERR(psy->tcd);
-			break;
+			return PTR_ERR_OR_ZERO(psy->tcd);
 		}
 	}
 	return 0;
-- 
2.0.3.693.g996b0fd

[PATCH 13/15] power-supply: Check for failures only when we can fail

[Viresh Kumar]

In power_supply_show_property() routine, we call ->get_property() conditionally
and should check for failure in that case only. There is no point comparing
'ret' for errors when 'ret' is surely zero.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_sysfs.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
index 750a202..c23b1b2 100644
--- a/drivers/power/power_supply_sysfs.c
+++ b/drivers/power/power_supply_sysfs.c
@@ -73,19 +73,20 @@ static ssize_t power_supply_show_property(struct device *dev,
 	const ptrdiff_t off = attr - power_supply_attrs;
 	union power_supply_propval value;
 
-	if (off == POWER_SUPPLY_PROP_TYPE)
+	if (off == POWER_SUPPLY_PROP_TYPE) {
 		value.intval = psy->type;
-	else
+	} else {
 		ret = psy->get_property(psy, off, &value);
 
-	if (ret < 0) {
-		if (ret == -ENODATA)
-			dev_dbg(dev, "driver has no data for `%s' property\n",
-				attr->attr.name);
-		else if (ret != -ENODEV)
-			dev_err(dev, "driver failed to report `%s' property: %zd\n",
-				attr->attr.name, ret);
-		return ret;
+		if (ret < 0) {
+			if (ret == -ENODATA)
+				dev_dbg(dev, "driver has no data for `%s' property\n",
+					attr->attr.name);
+			else if (ret != -ENODEV)
+				dev_err(dev, "driver failed to report `%s' property: %zd\n",
+					attr->attr.name, ret);
+			return ret;
+		}
 	}
 
 	if (off == POWER_SUPPLY_PROP_STATUS)
-- 
2.0.3.693.g996b0fd

[PATCH 14/15] power-supply: Avoid unnecessary 'goto' statements

[Viresh Kumar]

Using 'goto' statements for freeing resources on failures is a good choice as it
makes code very clean, and reduces the chances of human errors.

Though in most cases compiler may take care of this. But adding unnecessary
'goto' statements wouldn't make anything better. Code becomes less readable
actually.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
I am not sure if Maintainers would like this patch specially, but I just wanted
to give a try at making code better. Sorry if you didn't like it :)
---
 drivers/power/power_supply_core.c |  3 +--
 drivers/power/power_supply_leds.c | 19 ++++---------------
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 376464e..81177e2 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -599,7 +599,7 @@ static int __power_supply_register(struct device *parent,
 
 	power_supply_changed(psy);
 
-	goto success;
+	return 0;
 
 create_triggers_failed:
 	psy_unregister_cooler(psy);
@@ -612,7 +612,6 @@ static int __power_supply_register(struct device *parent,
 check_supplies_failed:
 dev_set_name_failed:
 	put_device(dev);
-success:
 	return rc;
 }
 
diff --git a/drivers/power/power_supply_leds.c b/drivers/power/power_supply_leds.c
index 995f966..effa093 100644
--- a/drivers/power/power_supply_leds.c
+++ b/drivers/power/power_supply_leds.c
@@ -57,8 +57,6 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
 
 static int power_supply_create_bat_triggers(struct power_supply *psy)
 {
-	int rc = 0;
-
 	psy->charging_full_trig_name = kasprintf(GFP_KERNEL,
 					"%s-charging-or-full", psy->name);
 	if (!psy->charging_full_trig_name)
@@ -87,7 +85,7 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
 	led_trigger_register_simple(psy->charging_blink_full_solid_trig_name,
 				    &psy->charging_blink_full_solid_trig);
 
-	goto success;
+	return 0;
 
 charging_blink_full_solid_failed:
 	kfree(psy->full_trig_name);
@@ -96,9 +94,7 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
 charging_failed:
 	kfree(psy->charging_full_trig_name);
 charging_full_failed:
-	rc = -ENOMEM;
-success:
-	return rc;
+	return -ENOMEM;
 }
 
 static void power_supply_remove_bat_triggers(struct power_supply *psy)
@@ -132,20 +128,13 @@ static void power_supply_update_gen_leds(struct power_supply *psy)
 
 static int power_supply_create_gen_triggers(struct power_supply *psy)
 {
-	int rc = 0;
-
 	psy->online_trig_name = kasprintf(GFP_KERNEL, "%s-online", psy->name);
 	if (!psy->online_trig_name)
-		goto online_failed;
+		return -ENOMEM;
 
 	led_trigger_register_simple(psy->online_trig_name, &psy->online_trig);
 
-	goto success;
-
-online_failed:
-	rc = -ENOMEM;
-success:
-	return rc;
+	return 0;
 }
 
 static void power_supply_remove_gen_triggers(struct power_supply *psy)
-- 
2.0.3.693.g996b0fd

[PATCH 15/15] power_supply: Don't iterate over devices to return -EPROBE_DEFER

[Viresh Kumar]

This piece of code was added so that we return -EPROBE_DEFER when no devices are
registered. But even if class_for_each_device() returns 0, we are going to
return -EPROBE_DEFER only.

And so this code isn't required at all. Remove it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/power/power_supply_core.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 81177e2..6cb7fe5 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -179,19 +179,6 @@ static int  __power_supply_find_supply_from_node(struct device *dev,
 static int power_supply_find_supply_from_node(struct device_node *supply_node)
 {
 	int error;
-	struct device *dev;
-	struct class_dev_iter iter;
-
-	/*
-	 * Use iterator to see if any other device is registered.
-	 * This is required since class_for_each_device returns 0
-	 * if there are no devices registered.
-	 */
-	class_dev_iter_init(&iter, power_supply_class, NULL, NULL);
-	dev = class_dev_iter_next(&iter);
-
-	if (!dev)
-		return -EPROBE_DEFER;
 
 	/*
 	 * class_for_each_device() either returns its own errors or values
-- 
2.0.3.693.g996b0fd

Re: [PATCH 00/15] power_supply: Fixes & cleanups

[Viresh Kumar]

On 4 September 2014 17:30, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Hi Guys,
>
> I was required to work on a battery driver and found some fixes/cleanups while
> going through the core.
>
> First two are fixes and rest are cleanups. Please see if they make any sense at
> all.
>
> Pushed here: git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git battery/fixes

Ping!!

Re: [PATCH 00/15] power_supply: Fixes & cleanups

[Sebastian Reichel]

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

Hi,

On Wed, Oct 01, 2014 at 08:49:30AM +0530, Viresh Kumar wrote:
> On 4 September 2014 17:30, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > I was required to work on a battery driver and found some
> > fixes/cleanups while going through the core.
> >
> > First two are fixes and rest are cleanups. Please see if they
> > make any sense at all.
> >
> > Pushed here: git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git battery/fixes
> 
> Ping!!

I merged the patchset two weeks ago to the battery-2.6 tree [0], so
they should be in linux-next already.

[0] git://git.infradead.org/battery-2.6.git master

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 00/15] power_supply: Fixes & cleanups

[Viresh Kumar]

On 1 October 2014 20:38, Sebastian Reichel <sre@kernel.org> wrote:
> On Wed, Oct 01, 2014 at 08:49:30AM +0530, Viresh Kumar wrote:
>> On 4 September 2014 17:30, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> > I was required to work on a battery driver and found some
>> > fixes/cleanups while going through the core.
>> >
>> > First two are fixes and rest are cleanups. Please see if they
>> > make any sense at all.
>> >
>> > Pushed here: git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git battery/fixes
>>
>> Ping!!
>
> I merged the patchset two weeks ago to the battery-2.6 tree [0], so
> they should be in linux-next already.

Oh I thought there would be some counter comments atleast
and so didn't bother checking linux-next for it :)

Thanks.