[PATCH] power_supply: charger-manager: Clean up for better readability

[PATCH] power_supply: charger-manager: Clean up for better readability

[Axel Lin]

This patch includes below changes:

* Some code change for better readability.
* Current code in probe already ensures desc->charger_regulators is not NULL.
  No need to check if it is NULL or not before calling regulator_bulk_free().
* Use dev_get_drvdata() in cm_suspend_prepare() and cm_suspend_complete()

Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
 drivers/power/charger-manager.c |   61 ++++++++++++++++----------------------
 1 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 88fd971..e610e6b 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -134,12 +134,11 @@ static int get_batt_uV(struct charger_manager *cm, int *uV)
 	union power_supply_propval val;
 	int ret;
 
-	if (cm->fuel_gauge)
-		ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
-				POWER_SUPPLY_PROP_VOLTAGE_NOW, &val);
-	else
+	if (!cm->fuel_gauge)
 		return -ENODEV;
 
+	ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
+				POWER_SUPPLY_PROP_VOLTAGE_NOW, &val);
 	if (ret)
 		return ret;
 
@@ -245,9 +244,7 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
 	struct charger_desc *desc = cm->desc;
 
 	/* Ignore if it's redundent command */
-	if (enable && cm->charger_enabled)
-		return 0;
-	if (!enable && !cm->charger_enabled)
+	if (enable == cm->charger_enabled)
 		return 0;
 
 	if (enable) {
@@ -309,9 +306,7 @@ static void uevent_notify(struct charger_manager *cm, const char *event)
 
 		if (!strncmp(env_str_save, event, UEVENT_BUF_SIZE))
 			return; /* Duplicated. */
-		else
-			strncpy(env_str_save, event, UEVENT_BUF_SIZE);
-
+		strncpy(env_str_save, event, UEVENT_BUF_SIZE);
 		return;
 	}
 
@@ -387,8 +382,10 @@ static bool cm_monitor(void)
 
 	mutex_lock(&cm_list_mtx);
 
-	list_for_each_entry(cm, &cm_list, entry)
-		stop = stop || _cm_monitor(cm);
+	list_for_each_entry(cm, &cm_list, entry) {
+		if (_cm_monitor(cm))
+			stop = true;
+	}
 
 	mutex_unlock(&cm_list_mtx);
 
@@ -697,8 +694,10 @@ bool cm_suspend_again(void)
 	mutex_lock(&cm_list_mtx);
 	list_for_each_entry(cm, &cm_list, entry) {
 		if (cm->status_save_ext_pwr_inserted != is_ext_pwr_online(cm) ||
-		    cm->status_save_batt != is_batt_present(cm))
+		    cm->status_save_batt != is_batt_present(cm)) {
 			ret = false;
+			break;
+		}
 	}
 	mutex_unlock(&cm_list_mtx);
 
@@ -855,11 +854,10 @@ static int charger_manager_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, cm);
 
-	memcpy(&cm->charger_psy, &psy_default,
-				sizeof(psy_default));
+	memcpy(&cm->charger_psy, &psy_default, sizeof(psy_default));
+
 	if (!desc->psy_name) {
-		strncpy(cm->psy_name_buf, psy_default.name,
-				PSY_NAME_MAX);
+		strncpy(cm->psy_name_buf, psy_default.name, PSY_NAME_MAX);
 	} else {
 		strncpy(cm->psy_name_buf, desc->psy_name, PSY_NAME_MAX);
 	}
@@ -894,15 +892,15 @@ static int charger_manager_probe(struct platform_device *pdev)
 				POWER_SUPPLY_PROP_CURRENT_NOW;
 		cm->charger_psy.num_properties++;
 	}
-	if (!desc->measure_battery_temp) {
-		cm->charger_psy.properties[cm->charger_psy.num_properties] =
-				POWER_SUPPLY_PROP_TEMP_AMBIENT;
-		cm->charger_psy.num_properties++;
-	}
+
 	if (desc->measure_battery_temp) {
 		cm->charger_psy.properties[cm->charger_psy.num_properties] =
 				POWER_SUPPLY_PROP_TEMP;
 		cm->charger_psy.num_properties++;
+	} else {
+		cm->charger_psy.properties[cm->charger_psy.num_properties] =
+				POWER_SUPPLY_PROP_TEMP_AMBIENT;
+		cm->charger_psy.num_properties++;
 	}
 
 	ret = power_supply_register(NULL, &cm->charger_psy);
@@ -933,9 +931,8 @@ static int charger_manager_probe(struct platform_device *pdev)
 	return 0;
 
 err_chg_enable:
-	if (desc->charger_regulators)
-		regulator_bulk_free(desc->num_charger_regulators,
-					desc->charger_regulators);
+	regulator_bulk_free(desc->num_charger_regulators,
+			    desc->charger_regulators);
 err_bulk_get:
 	power_supply_unregister(&cm->charger_psy);
 err_register:
@@ -961,10 +958,8 @@ static int __devexit charger_manager_remove(struct platform_device *pdev)
 	list_del(&cm->entry);
 	mutex_unlock(&cm_list_mtx);
 
-	if (desc->charger_regulators)
-		regulator_bulk_free(desc->num_charger_regulators,
-					desc->charger_regulators);
-
+	regulator_bulk_free(desc->num_charger_regulators,
+			    desc->charger_regulators);
 	power_supply_unregister(&cm->charger_psy);
 	kfree(cm->charger_psy.properties);
 	kfree(cm->charger_stat);
@@ -982,9 +977,7 @@ MODULE_DEVICE_TABLE(platform, charger_manager_id);
 
 static int cm_suspend_prepare(struct device *dev)
 {
-	struct platform_device *pdev = container_of(dev, struct platform_device,
-						    dev);
-	struct charger_manager *cm = platform_get_drvdata(pdev);
+	struct charger_manager *cm = dev_get_drvdata(dev);
 
 	if (!cm_suspended) {
 		if (rtc_dev) {
@@ -1020,9 +1013,7 @@ static int cm_suspend_prepare(struct device *dev)
 
 static void cm_suspend_complete(struct device *dev)
 {
-	struct platform_device *pdev = container_of(dev, struct platform_device,
-						    dev);
-	struct charger_manager *cm = platform_get_drvdata(pdev);
+	struct charger_manager *cm = dev_get_drvdata(dev);
 
 	if (cm_suspended) {
 		if (rtc_dev) {
-- 
1.7.5.4

Re: [PATCH] power_supply: charger-manager: Clean up for better readability

[Anton Vorontsov]

On Thu, Jan 12, 2012 at 12:56:35PM +0800, Axel Lin wrote:
> This patch includes below changes:
> 
> * Some code change for better readability.
> * Current code in probe already ensures desc->charger_regulators is not NULL.
>   No need to check if it is NULL or not before calling regulator_bulk_free().
> * Use dev_get_drvdata() in cm_suspend_prepare() and cm_suspend_complete()
> 
> Signed-off-by: Axel Lin <axel.lin@gmail.com>
> ---

Applied, thanks a lot!


FWIW, I also applied the following patch:

commit f304f0a31442f4c8b98208acedc3da2f6c60a771
Author: Anton Vorontsov <anton.vorontsov@linaro.org>
Date:   Thu Mar 15 21:01:28 2012 +0400

    charger-manager: Simplify charger_get_property(), get rid of a warning
    
    This patch fixes the following warning and makes the code a little
    bit more simpler:
    
      CC      drivers/power/charger-manager.o
    charger-manager.c: In function 'charger_get_property':
    charger-manager.c:429:15: warning: 'i' may be used uninitialized in this function [-Wuninitialized]
    
    Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index e610e6b..9eca9f1 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -399,7 +399,8 @@ static int charger_get_property(struct power_supply *psy,
 	struct charger_manager *cm = container_of(psy,
 			struct charger_manager, charger_psy);
 	struct charger_desc *desc = cm->desc;
-	int i, ret = 0, uV;
+	int ret = 0;
+	int uV;
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:
@@ -425,8 +426,7 @@ static int charger_get_property(struct power_supply *psy,
 			val->intval = 0;
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
-		ret = get_batt_uV(cm, &i);
-		val->intval = i;
+		ret = get_batt_uV(cm, &val->intval);
 		break;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
 		ret = cm->fuel_gauge->get_property(cm->fuel_gauge,