2014-10-30 12:43:29

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH V2 0/10] Improve charger manager driver for optimized operation.

After charger manager's initially introduced, it has kept its codes without any
big change. However, the charger manager working operation isn't optimized and
it also has unused codes and non-generic interface. This series tries to make
charger manager more generic and maintainable with some fixes.

[Changes in V2]
- Rebase patch series on latest of battery-2.6.git.
- Seperate 'power: charger-manager: Rearrange data and monitor ~' patch into
several patches depends on its purpose.
- Add 2 patches related with polling mode.

Jonghwa Lee (10):
power: charger-manager: Use thermal subsystem interface only to get
temperature.
power: charger-manager: Use power_supply_changed() not private
uevent.
power: charger-manager: Remove deprecated function,
cm_notify_event().
power: charger-manager: Fix to use CHARGE_NOW/FULL property
correctly.
power: charger-manager: Concentrate scattered power_supply_changed()
calls.
power: charger-manager: Get external power souce information only
from EXTCON.
power: charger-manager: Make chraging decision focusing on battery
status.
power: charger-manager: Not to start charging directly in cable
nofitication.
power: charger-manager: Support different polling mode for sleep
state.
power: charger-manager: Support to change polling rate in runtime.

.../bindings/power_supply/charger-manager.txt | 1 -
drivers/power/Kconfig | 1 +
drivers/power/charger-manager.c | 758 ++++++--------------
include/linux/power/charger-manager.h | 34 +-
4 files changed, 219 insertions(+), 575 deletions(-)

--
1.7.9.5


2014-10-30 12:43:34

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 06/10] power: charger-manager: Get external power souce information only from EXTCON.

When charger-manager checks whether external power source is available,
it gets information from charger IC driver. However, it's not correct source,
charger IC doesn't have responsibilty to give cable connection status.
The charger-manager already gets cable information from EXTCON susbsystem,
so it can re-use it.

Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/charger-manager.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index bb44588..172dfe5 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -115,34 +115,28 @@ static bool is_batt_present(struct charger_manager *cm)
* is_ext_pwr_online - See if an external power source is attached to charge
* @cm: the Charger Manager representing the battery.
*
- * Returns true if at least one of the chargers of the battery has an external
- * power source attached to charge the battery regardless of whether it is
- * actually charging or not.
+ * Returns true if there is external power source.
+ * Cable connection information is only obtained by EXTCON class notification.
*/
static bool is_ext_pwr_online(struct charger_manager *cm)
{
- union power_supply_propval val;
- struct power_supply *psy;
- bool online = false;
- int i, ret;

- /* If at least one of them has one, it's yes. */
- for (i = 0; cm->desc->psy_charger_stat[i]; i++) {
- psy = power_supply_get_by_name(cm->desc->psy_charger_stat[i]);
- if (!psy) {
- dev_err(cm->dev, "Cannot find power supply \"%s\"\n",
- cm->desc->psy_charger_stat[i]);
- continue;
- }
+ struct charger_desc *desc = cm->desc;
+ struct charger_regulator *regulators = desc->charger_regulators;
+ struct charger_cable *cables;
+ int i, j, num_cables;

- ret = psy->get_property(psy, POWER_SUPPLY_PROP_ONLINE, &val);
- if (ret == 0 && val.intval) {
- online = true;
- break;
+ /* If at least one of them has one, it's yes. */
+ for (i = 0; i < desc->num_charger_regulators; i++) {
+ cables = regulators[i].cables;
+ num_cables = regulators[i].num_cables;
+ for (j = 0; j < num_cables; j++) {
+ if (cables[j].attached)
+ return true;
}
}

- return online;
+ return false;
}

/**
--
1.7.9.5

2014-10-30 12:43:38

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 10/10] power: charger-manager: Support to change polling rate in runtime.

Add 'polling_ms' sysfs node to change charger-manager's monitoring rate
in runtime. It can set only bigger than 2 jiffies (for 200 HZ system it
is 10 msecs.) as it's allowed for minimum poling rate in previous.
It resets poller and re-configure polling rate based on new input if next
polling time is far enough. Otherwise, it just waits expiration of timer
and new polling rate will affects the next scheduling.

Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/charger-manager.c | 62 +++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 0a0834f..7a007f4 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -1054,6 +1054,63 @@ static ssize_t charger_externally_control_store(struct device *dev,
return count;
}

+static ssize_t show_polling_ms(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct charger_manager *cm;
+ ssize_t len;
+
+ list_for_each_entry(cm, &cm_list, entry)
+ if (cm->charger_psy.dev == dev)
+ break;
+
+ if (cm->charger_psy.dev != dev)
+ return -EINVAL;
+
+ len = sprintf(buf, "%d\n", cm->desc->polling_interval_ms);
+
+ return len;
+}
+
+static ssize_t store_polling_ms(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ struct charger_manager *cm;
+ int polling_ms;
+ int ret;
+
+ ret = sscanf(buf, "%d", &polling_ms);
+ if (ret < 0 )
+ return -EINVAL;
+
+ if (polling_ms < CM_JIFFIES_SMALL * MSEC_PER_SEC / HZ)
+ return -EINVAL;
+
+ list_for_each_entry(cm, &cm_list, entry)
+ if (cm->charger_psy.dev == dev)
+ break;
+
+ if (cm->charger_psy.dev != dev)
+ return -ENODEV;
+
+ cm->desc->polling_interval_ms = polling_ms;
+
+ pr_info("Polling interval's changed to %u ms.\n",
+ cm->desc->polling_interval_ms);
+
+ if (next_polling - jiffies >
+ msecs_to_jiffies(cm->desc->polling_interval_ms)) {
+ pr_info("Reset poller now... \n");
+ cancel_delayed_work(&cm_monitor_work);
+ schedule_work(&setup_polling);
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR(polling_ms, 0644, show_polling_ms, store_polling_ms);
+
/**
* charger_manager_register_sysfs - Register sysfs entry for each charger
* @cm: the Charger Manager representing the battery.
@@ -1077,6 +1134,11 @@ static int charger_manager_register_sysfs(struct charger_manager *cm)
int ret = 0;
int i;

+ /* Create polling_ms sysfs node */
+ ret = device_create_file(cm->charger_psy.dev, &dev_attr_polling_ms);
+ if (ret)
+ pr_err("Failed to create poling_ms sysfs node (%d)\n", ret);
+
/* Create sysfs entry to control charger(regulator) */
for (i = 0; i < desc->num_charger_regulators; i++) {
charger = &desc->charger_regulators[i];
--
1.7.9.5

2014-10-30 12:44:22

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 09/10] power: charger-manager: Support different polling mode for sleep state.

Add additional polling mode for sleep state to define different mode with
normal state. With this change, charger-manager can work differently in
normal state or sleep state. e.g, polling aways for normal and polling
only when charing for sleep. If there is no defined polling mode for
sleep state it just follows the normal state's.
In addition to, polling rate is still same in sleep.

Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/charger-manager.c | 20 +++++++++++++++++---
include/linux/power/charger-manager.h | 9 ++++++---
2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 1ccb9be..0a0834f 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -291,7 +291,15 @@ static bool is_full_charged(struct charger_manager *cm)
*/
static bool is_polling_required(struct charger_manager *cm)
{
- switch (cm->desc->polling_mode) {
+ enum polling_modes polling_mode;
+
+ if (cm_suspended
+ && cm->desc->poll_mode_sleep >= 0)
+ polling_mode = cm->desc->poll_mode_sleep;
+ else
+ polling_mode = cm->desc->poll_mode_normal;
+
+ switch (polling_mode) {
case CM_POLL_DISABLE:
return false;
case CM_POLL_ALWAYS:
@@ -302,7 +310,7 @@ static bool is_polling_required(struct charger_manager *cm)
return is_charging(cm);
default:
dev_warn(cm->dev, "Incorrect polling_mode (%d)\n",
- cm->desc->polling_mode);
+ polling_mode);
}

return false;
@@ -1158,7 +1166,13 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
of_property_read_string(np, "cm-name", &desc->psy_name);

of_property_read_u32(np, "cm-poll-mode", &poll_mode);
- desc->polling_mode = poll_mode;
+ desc->poll_mode_normal = poll_mode;
+
+ /* Polling mode in sleep state */
+ if (!of_property_read_u32(np, "cm-poll-mode-sleep", &poll_mode))
+ desc->poll_mode_sleep = poll_mode;
+ else
+ desc->poll_mode_sleep = -EINVAL;

of_property_read_u32(np, "cm-poll-interval",
&desc->polling_interval_ms);
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index 37fb181..a30a0b6 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -131,8 +131,10 @@ struct charger_regulator {
/**
* struct charger_desc
* @psy_name: the name of power-supply-class for charger manager
- * @polling_mode:
- * Determine which polling mode will be used
+ * @poll_mode_normal:
+ * Determine which polling mode will be used in normal state.
+ * @poll_mode_sleep:
+ * Determine which polling mode will be used in sleep state.
* @fullbatt_vchkdrop_uV:
* Check voltage drop after the battery is fully charged.
* If it has dropped more than fullbatt_vchkdrop_uV
@@ -170,7 +172,8 @@ struct charger_regulator {
struct charger_desc {
const char *psy_name;

- enum polling_modes polling_mode;
+ enum polling_modes poll_mode_normal;
+ enum polling_modes poll_mode_sleep;
unsigned int polling_interval_ms;

unsigned int fullbatt_vchkdrop_uV;
--
1.7.9.5

2014-10-30 12:44:38

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 08/10] power: charger-manager: Not to start charging directly in cable nofitication.

This patch prevents direct charging control in cable notification.
It sets only input current limit according to cable type and yields charging
control to be done by cm_monitor() where charging management proceeds.
It may loose few ms to enable charging compared to before, even though it's
more important that charging is enabled always in safe context.

Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/charger-manager.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 065b92a..1ccb9be 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -849,7 +849,8 @@ static void charger_extcon_work(struct work_struct *work)
cable->min_uA, cable->max_uA);
}

- try_charger_enable(cable->cm, cable->attached);
+ cancel_delayed_work(&cm_monitor_work);
+ queue_delayed_work(cm_wq, &cm_monitor_work, 0);
}

/**
@@ -873,15 +874,6 @@ static int charger_extcon_notifier(struct notifier_block *self,
cable->attached = event;

/*
- * Setup monitoring to check battery state
- * when charger cable is attached.
- */
- if (cable->attached && is_polling_required(cable->cm)) {
- cancel_work_sync(&setup_polling);
- schedule_work(&setup_polling);
- }
-
- /*
* Setup work for controlling charger(regulator)
* according to charger cable.
*/
--
1.7.9.5

2014-10-30 12:45:17

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 07/10] power: charger-manager: Make chraging decision focusing on battery status.

In cm_monitor() where charging management starts, it checks various charging
condition sequentially to decide next charging operation. However, as it
follows sequential process, cascaded IF statements, it does some duplicated
jobs which is already done in previous stage. It results delay in decision
making. And moreover, starting point of charing is spreaded all around, so
it makes maintain codes and debugging difficult.

Both of problems mentioned above becomes clean if it manages battery charging
focusing on battery status not following sequential condition checking.
Now, cm_monitor() moves battery state diagram and does optimal operation for
current state. As a result, it reduces whole monitoring time almost in half.

Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/charger-manager.c | 177 +++++++++++----------------------
include/linux/power/charger-manager.h | 3 +
2 files changed, 62 insertions(+), 118 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 172dfe5..065b92a 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -249,6 +249,19 @@ static bool is_full_charged(struct charger_manager *cm)
if (!fuel_gauge)
return false;

+ /* Full, if it's over the fullbatt voltage */
+ if (desc->fullbatt_uV > 0) {
+ ret = get_batt_uV(cm, &uV);
+ if (!ret) {
+ /* Battery is already full, checks voltage drop. */
+ if (cm->battery_status == POWER_SUPPLY_STATUS_FULL
+ && desc->fullbatt_vchkdrop_uV)
+ uV += desc->fullbatt_vchkdrop_uV;
+ if (uV >= desc->fullbatt_uV)
+ return true;
+ }
+ }
+
if (desc->fullbatt_full_capacity > 0) {
val.intval = 0;

@@ -259,13 +272,6 @@ static bool is_full_charged(struct charger_manager *cm)
return true;
}

- /* Full, if it's over the fullbatt voltage */
- if (desc->fullbatt_uV > 0) {
- ret = get_batt_uV(cm, &uV);
- if (!ret && uV >= desc->fullbatt_uV)
- return true;
- }
-
/* Full, if the capacity is more than fullbatt_soc */
if (desc->fullbatt_soc > 0) {
val.intval = 0;
@@ -376,67 +382,13 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
}
}

- if (!err) {
+ if (!err)
cm->charger_enabled = enable;
- power_supply_changed(&cm->charger_psy);
- }

return err;
}

/**
- * try_charger_restart - Restart charging.
- * @cm: the Charger Manager representing the battery.
- *
- * Restart charging by turning off and on the charger.
- */
-static int try_charger_restart(struct charger_manager *cm)
-{
- int err;
-
- if (cm->emergency_stop)
- return -EAGAIN;
-
- err = try_charger_enable(cm, false);
- if (err)
- return err;
-
- return try_charger_enable(cm, true);
-}
-
-/**
- * fullbatt_vchk - Check voltage drop some times after "FULL" event.
- *
- * If a user has designated "fullbatt_vchkdrop_uV" values with
- * charger_desc, Charger Manager checks voltage drop after the battery
- * "FULL" event. It checks whether the voltage has dropped more than
- * fullbatt_vchkdrop_uV by calling this function after fullbatt_vchkrop_ms.
- */
-static void fullbatt_vchk(struct charger_manager *cm)
-{
- struct charger_desc *desc = cm->desc;
- int batt_uV, err, diff;
-
- if (!desc->fullbatt_vchkdrop_uV)
- return;
-
- err = get_batt_uV(cm, &batt_uV);
- if (err) {
- dev_err(cm->dev, "%s: get_batt_uV error(%d)\n", __func__, err);
- return;
- }
-
- diff = desc->fullbatt_uV - batt_uV;
- if (diff < 0)
- return;
-
- dev_info(cm->dev, "VBATT dropped %duV after full-batt\n", diff);
-
- if (diff > desc->fullbatt_vchkdrop_uV)
- try_charger_restart(cm);
-}
-
-/**
* check_charging_duration - Monitor charging/discharging duration
* @cm: the Charger Manager representing the battery.
*
@@ -463,17 +415,14 @@ static int check_charging_duration(struct charger_manager *cm)
if (duration > desc->charging_max_duration_ms) {
dev_info(cm->dev, "Charging duration exceed %ums\n",
desc->charging_max_duration_ms);
- try_charger_enable(cm, false);
ret = true;
}
- } else if (is_ext_pwr_online(cm) && !cm->charger_enabled) {
+ } else if (cm->battery_status == POWER_SUPPLY_STATUS_NOT_CHARGING) {
duration = curr - cm->charging_end_time;

- if (duration > desc->charging_max_duration_ms &&
- is_ext_pwr_online(cm)) {
+ if (duration > desc->charging_max_duration_ms) {
dev_info(cm->dev, "Discharging duration exceed %ums\n",
desc->discharging_max_duration_ms);
- try_charger_enable(cm, true);
ret = true;
}
}
@@ -527,10 +476,45 @@ static int cm_check_thermal_status(struct charger_manager *cm)
else if (temp < lower_limit)
ret = CM_EVENT_BATT_COLD;

+ cm->emergency_stop = ret;
+
return ret;
}

/**
+ * cm_get_target_status - Check current status and get next target status.
+ * @cm: the Charger Manager representing the battery.
+ */
+static int cm_get_target_status(struct charger_manager *cm)
+{
+ if (!is_ext_pwr_online(cm))
+ return POWER_SUPPLY_STATUS_DISCHARGING;
+
+ if (cm_check_thermal_status(cm)) {
+ /* Check if discharging duration exeeds limit. */
+ if (check_charging_duration(cm))
+ goto charging_ok;
+ return POWER_SUPPLY_STATUS_NOT_CHARGING;
+ }
+
+ switch (cm->battery_status) {
+ case POWER_SUPPLY_STATUS_CHARGING:
+ /* Check if charging duration exeeds limit. */
+ if (check_charging_duration(cm))
+ return POWER_SUPPLY_STATUS_FULL;
+ case POWER_SUPPLY_STATUS_FULL:
+ if (is_full_charged(cm))
+ return POWER_SUPPLY_STATUS_FULL;
+ default:
+ break;
+ }
+
+charging_ok:
+ /* Charging is allowed. */
+ return POWER_SUPPLY_STATUS_CHARGING;
+}
+
+/**
* _cm_monitor - Monitor the temperature and return true for exceptions.
* @cm: the Charger Manager representing the battery.
*
@@ -539,56 +523,18 @@ static int cm_check_thermal_status(struct charger_manager *cm)
*/
static bool _cm_monitor(struct charger_manager *cm)
{
- int temp_alrt;
+ int target;

- temp_alrt = cm_check_thermal_status(cm);
+ target = cm_get_target_status(cm);

- /* It has been stopped already */
- if (temp_alrt && cm->emergency_stop)
- return false;
-
- /*
- * Check temperature whether overheat or cold.
- * If temperature is out of range normal state, stop charging.
- */
- if (temp_alrt) {
- cm->emergency_stop = temp_alrt;
- try_charger_enable(cm, false);
-
- /*
- * Check whole charging duration and discharing duration
- * after full-batt.
- */
- } else if (!cm->emergency_stop && check_charging_duration(cm)) {
- dev_dbg(cm->dev,
- "Charging/Discharging duration is out of range\n");
- /*
- * Check dropped voltage of battery. If battery voltage is more
- * dropped than fullbatt_vchkdrop_uV after fully charged state,
- * charger-manager have to recharge battery.
- */
- } else if (!cm->emergency_stop && is_ext_pwr_online(cm) &&
- !cm->charger_enabled) {
- fullbatt_vchk(cm);
+ try_charger_enable(cm, (target == POWER_SUPPLY_STATUS_CHARGING));

- /*
- * Check whether fully charged state to protect overcharge
- * if charger-manager is charging for battery.
- */
- } else if (!cm->emergency_stop && is_full_charged(cm) &&
- cm->charger_enabled) {
- dev_info(cm->dev, "EVENT_HANDLE: Battery Fully Charged\n");
- try_charger_enable(cm, false);
-
- fullbatt_vchk(cm);
- } else {
- cm->emergency_stop = 0;
- if (is_ext_pwr_online(cm)) {
- try_charger_enable(cm, true);
- }
+ if (cm->battery_status != target) {
+ cm->battery_status = target;
+ power_supply_changed(&cm->charger_psy);
}

- return true;
+ return (cm->battery_status == POWER_SUPPLY_STATUS_NOT_CHARGING);
}

/**
@@ -694,12 +640,7 @@ static int charger_get_property(struct power_supply *psy,

switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
- if (is_charging(cm))
- val->intval = POWER_SUPPLY_STATUS_CHARGING;
- else if (is_ext_pwr_online(cm))
- val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
- else
- val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ val->intval = cm->battery_status;
break;
case POWER_SUPPLY_PROP_HEALTH:
if (cm->emergency_stop > 0)
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index 22a8097..37fb181 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -220,6 +220,7 @@ struct charger_desc {
* saved status of battery before entering suspend-to-RAM
* @charging_start_time: saved start time of enabling charging
* @charging_end_time: saved end time of disabling charging
+ * @battery_status: Current battery status
*/
struct charger_manager {
struct list_head entry;
@@ -237,6 +238,8 @@ struct charger_manager {

u64 charging_start_time;
u64 charging_end_time;
+
+ int battery_status;
};

#endif /* _CHARGER_MANAGER_H */
--
1.7.9.5

2014-10-30 12:46:01

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 03/10] power: charger-manager: Remove deprecated function, cm_notify_event().

cm_notify_event() is introduced to get event associated with battery status
externally, but no one had been used. Moreover it makes charger manager
driver more complicated. This patch tries to drop the function and all data
related to simplify the driver.

Signed-off-by: Jonghwa Lee <[email protected]>
---
.../bindings/power_supply/charger-manager.txt | 1 -
drivers/power/charger-manager.c | 196 +-------------------
include/linux/power/charger-manager.h | 19 +-
3 files changed, 8 insertions(+), 208 deletions(-)

diff --git a/Documentation/devicetree/bindings/power_supply/charger-manager.txt b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
index 2b33750..767aaa5 100644
--- a/Documentation/devicetree/bindings/power_supply/charger-manager.txt
+++ b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
@@ -39,7 +39,6 @@ Example :
cm-poll-mode = <1>;
cm-poll-interval = <30000>;

- cm-fullbatt-vchkdrop-ms = <30000>;
cm-fullbatt-vchkdrop-volt = <150000>;
cm-fullbatt-soc = <100>;

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 33a1a4d..64fdaaf 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -410,25 +410,18 @@ static int try_charger_restart(struct charger_manager *cm)

/**
* fullbatt_vchk - Check voltage drop some times after "FULL" event.
- * @work: the work_struct appointing the function
*
- * If a user has designated "fullbatt_vchkdrop_ms/uV" values with
+ * If a user has designated "fullbatt_vchkdrop_uV" values with
* charger_desc, Charger Manager checks voltage drop after the battery
* "FULL" event. It checks whether the voltage has dropped more than
* fullbatt_vchkdrop_uV by calling this function after fullbatt_vchkrop_ms.
*/
-static void fullbatt_vchk(struct work_struct *work)
+static void fullbatt_vchk(struct charger_manager *cm)
{
- struct delayed_work *dwork = to_delayed_work(work);
- struct charger_manager *cm = container_of(dwork,
- struct charger_manager, fullbatt_vchk_work);
struct charger_desc *desc = cm->desc;
int batt_uV, err, diff;

- /* remove the appointment for fullbatt_vchk */
- cm->fullbatt_vchk_jiffies_at = 0;
-
- if (!desc->fullbatt_vchkdrop_uV || !desc->fullbatt_vchkdrop_ms)
+ if (!desc->fullbatt_vchkdrop_uV)
return;

err = get_batt_uV(cm, &batt_uV);
@@ -585,7 +578,7 @@ static bool _cm_monitor(struct charger_manager *cm)
*/
} else if (!cm->emergency_stop && is_ext_pwr_online(cm) &&
!cm->charger_enabled) {
- fullbatt_vchk(&cm->fullbatt_vchk_work.work);
+ fullbatt_vchk(cm);

/*
* Check whether fully charged state to protect overcharge
@@ -598,7 +591,7 @@ static bool _cm_monitor(struct charger_manager *cm)

try_charger_enable(cm, false);

- fullbatt_vchk(&cm->fullbatt_vchk_work.work);
+ fullbatt_vchk(cm);
} else {
cm->emergency_stop = 0;
if (is_ext_pwr_online(cm)) {
@@ -700,66 +693,6 @@ static void cm_monitor_poller(struct work_struct *work)
schedule_work(&setup_polling);
}

-/**
- * fullbatt_handler - Event handler for CM_EVENT_BATT_FULL
- * @cm: the Charger Manager representing the battery.
- */
-static void fullbatt_handler(struct charger_manager *cm)
-{
- struct charger_desc *desc = cm->desc;
-
- if (!desc->fullbatt_vchkdrop_uV || !desc->fullbatt_vchkdrop_ms)
- goto out;
-
- if (cm_suspended)
- device_set_wakeup_capable(cm->dev, true);
-
- mod_delayed_work(cm_wq, &cm->fullbatt_vchk_work,
- msecs_to_jiffies(desc->fullbatt_vchkdrop_ms));
- cm->fullbatt_vchk_jiffies_at = jiffies + msecs_to_jiffies(
- desc->fullbatt_vchkdrop_ms);
-
- if (cm->fullbatt_vchk_jiffies_at == 0)
- cm->fullbatt_vchk_jiffies_at = 1;
-
-out:
- dev_info(cm->dev, "EVENT_HANDLE: Battery Fully Charged\n");
- power_supply_changed(&cm->charger_psy);
-}
-
-/**
- * battout_handler - Event handler for CM_EVENT_BATT_OUT
- * @cm: the Charger Manager representing the battery.
- */
-static void battout_handler(struct charger_manager *cm)
-{
- if (cm_suspended)
- device_set_wakeup_capable(cm->dev, true);
-
- if (!is_batt_present(cm)) {
- dev_emerg(cm->dev, "Battery Pulled Out!\n");
- power_supply_changed(&cm->charger_psy);
- } else {
- power_supply_changed(&cm->charger_psy);
- }
-}
-
-/**
- * misc_event_handler - Handler for other evnets
- * @cm: the Charger Manager representing the battery.
- * @type: the Charger Manager representing the battery.
- */
-static void misc_event_handler(struct charger_manager *cm,
- enum cm_event_types type)
-{
- if (cm_suspended)
- device_set_wakeup_capable(cm->dev, true);
-
- if (is_polling_required(cm) && cm->desc->polling_interval_ms)
- schedule_work(&setup_polling);
- power_supply_changed(&cm->charger_psy);
-}
-
static int charger_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -948,21 +881,6 @@ static bool cm_setup_timer(void)

mutex_lock(&cm_list_mtx);
list_for_each_entry(cm, &cm_list, entry) {
- unsigned int fbchk_ms = 0;
-
- /* fullbatt_vchk is required. setup timer for that */
- if (cm->fullbatt_vchk_jiffies_at) {
- fbchk_ms = jiffies_to_msecs(cm->fullbatt_vchk_jiffies_at
- - jiffies);
- if (time_is_before_eq_jiffies(
- cm->fullbatt_vchk_jiffies_at) ||
- msecs_to_jiffies(fbchk_ms) < CM_JIFFIES_SMALL) {
- fullbatt_vchk(&cm->fullbatt_vchk_work.work);
- fbchk_ms = 0;
- }
- }
- CM_MIN_VALID(wakeup_ms, fbchk_ms);
-
/* Skip if polling is not required for this CM */
if (!is_polling_required(cm) && !cm->emergency_stop)
continue;
@@ -1346,8 +1264,6 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
of_property_read_u32(np, "cm-poll-interval",
&desc->polling_interval_ms);

- of_property_read_u32(np, "cm-fullbatt-vchkdrop-ms",
- &desc->fullbatt_vchkdrop_ms);
of_property_read_u32(np, "cm-fullbatt-vchkdrop-volt",
&desc->fullbatt_vchkdrop_uV);
of_property_read_u32(np, "cm-fullbatt-voltage", &desc->fullbatt_uV);
@@ -1493,9 +1409,8 @@ static int charger_manager_probe(struct platform_device *pdev)
if (desc->fullbatt_uV == 0) {
dev_info(&pdev->dev, "Ignoring full-battery voltage threshold as it is not supplied\n");
}
- if (!desc->fullbatt_vchkdrop_ms || !desc->fullbatt_vchkdrop_uV) {
+ if (!desc->fullbatt_vchkdrop_uV) {
dev_info(&pdev->dev, "Disabling full-battery voltage drop checking mechanism as it is not supplied\n");
- desc->fullbatt_vchkdrop_ms = 0;
desc->fullbatt_vchkdrop_uV = 0;
}
if (desc->fullbatt_soc == 0) {
@@ -1609,8 +1524,6 @@ static int charger_manager_probe(struct platform_device *pdev)
cm->desc->measure_battery_temp = false;
}

- INIT_DELAYED_WORK(&cm->fullbatt_vchk_work, fullbatt_vchk);
-
ret = power_supply_register(NULL, &cm->charger_psy);
if (ret) {
dev_err(&pdev->dev, "Cannot register charger-manager with name \"%s\"\n",
@@ -1757,8 +1670,6 @@ static bool cm_need_to_awake(void)

static int cm_suspend_prepare(struct device *dev)
{
- struct charger_manager *cm = dev_get_drvdata(dev);
-
if (cm_need_to_awake())
return -EBUSY;

@@ -1770,7 +1681,6 @@ static int cm_suspend_prepare(struct device *dev)
if (cm_timer_set) {
cancel_work_sync(&setup_polling);
cancel_delayed_work_sync(&cm_monitor_work);
- cancel_delayed_work(&cm->fullbatt_vchk_work);
}

return 0;
@@ -1795,31 +1705,6 @@ static void cm_suspend_complete(struct device *dev)

_cm_monitor(cm);

- /* Re-enqueue delayed work (fullbatt_vchk_work) */
- if (cm->fullbatt_vchk_jiffies_at) {
- unsigned long delay = 0;
- unsigned long now = jiffies + CM_JIFFIES_SMALL;
-
- if (time_after_eq(now, cm->fullbatt_vchk_jiffies_at)) {
- delay = (unsigned long)((long)now
- - (long)(cm->fullbatt_vchk_jiffies_at));
- delay = jiffies_to_msecs(delay);
- } else {
- delay = 0;
- }
-
- /*
- * Account for cm_suspend_duration_ms with assuming that
- * timer stops in suspend.
- */
- if (delay > cm_suspend_duration_ms)
- delay -= cm_suspend_duration_ms;
- else
- delay = 0;
-
- queue_delayed_work(cm_wq, &cm->fullbatt_vchk_work,
- msecs_to_jiffies(delay));
- }
device_set_wakeup_capable(cm->dev, false);
}

@@ -1859,75 +1744,6 @@ static void __exit charger_manager_cleanup(void)
}
module_exit(charger_manager_cleanup);

-/**
- * find_power_supply - find the associated power_supply of charger
- * @cm: the Charger Manager representing the battery
- * @psy: pointer to instance of charger's power_supply
- */
-static bool find_power_supply(struct charger_manager *cm,
- struct power_supply *psy)
-{
- int i;
- bool found = false;
-
- for (i = 0; cm->desc->psy_charger_stat[i]; i++) {
- if (!strcmp(psy->name, cm->desc->psy_charger_stat[i])) {
- found = true;
- break;
- }
- }
-
- return found;
-}
-
-/**
- * cm_notify_event - charger driver notify Charger Manager of charger event
- * @psy: pointer to instance of charger's power_supply
- * @type: type of charger event
- * @msg: optional message passed to uevent_notify fuction
- */
-void cm_notify_event(struct power_supply *psy, enum cm_event_types type,
- char *msg)
-{
- struct charger_manager *cm;
- bool found_power_supply = false;
-
- if (psy == NULL)
- return;
-
- mutex_lock(&cm_list_mtx);
- list_for_each_entry(cm, &cm_list, entry) {
- found_power_supply = find_power_supply(cm, psy);
- if (found_power_supply)
- break;
- }
- mutex_unlock(&cm_list_mtx);
-
- if (!found_power_supply)
- return;
-
- switch (type) {
- case CM_EVENT_BATT_FULL:
- fullbatt_handler(cm);
- break;
- case CM_EVENT_BATT_OUT:
- battout_handler(cm);
- break;
- case CM_EVENT_BATT_IN:
- case CM_EVENT_EXT_PWR_IN_OUT ... CM_EVENT_CHG_START_STOP:
- misc_event_handler(cm, type);
- break;
- case CM_EVENT_UNKNOWN:
- case CM_EVENT_OTHERS:
- power_supply_changed(&cm->charger_psy);
- break;
- default:
- dev_err(cm->dev, "%s: type not specified\n", __func__);
- break;
- }
-}
-EXPORT_SYMBOL_GPL(cm_notify_event);
-
MODULE_AUTHOR("MyungJoo Ham <[email protected]>");
MODULE_DESCRIPTION("Charger Manager");
MODULE_LICENSE("GPL");
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index 29d9b19..22a8097 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -133,11 +133,10 @@ struct charger_regulator {
* @psy_name: the name of power-supply-class for charger manager
* @polling_mode:
* Determine which polling mode will be used
- * @fullbatt_vchkdrop_ms:
* @fullbatt_vchkdrop_uV:
* Check voltage drop after the battery is fully charged.
- * If it has dropped more than fullbatt_vchkdrop_uV after
- * fullbatt_vchkdrop_ms, CM will restart charging.
+ * If it has dropped more than fullbatt_vchkdrop_uV
+ * CM will restart charging.
* @fullbatt_uV: voltage in microvolt
* If VBATT >= fullbatt_uV, it is assumed to be full.
* @fullbatt_soc: state of Charge in %
@@ -174,7 +173,6 @@ struct charger_desc {
enum polling_modes polling_mode;
unsigned int polling_interval_ms;

- unsigned int fullbatt_vchkdrop_ms;
unsigned int fullbatt_vchkdrop_uV;
unsigned int fullbatt_uV;
unsigned int fullbatt_soc;
@@ -212,9 +210,6 @@ struct charger_desc {
* @charger_stat: array of power_supply for chargers
* @tzd_batt : thermal zone device for battery
* @charger_enabled: the state of charger
- * @fullbatt_vchk_jiffies_at:
- * jiffies at the time full battery check will occur.
- * @fullbatt_vchk_work: work queue for full battery check
* @emergency_stop:
* When setting true, stop charging
* @psy_name_buf: the name of power-supply-class for charger manager
@@ -235,9 +230,6 @@ struct charger_manager {

bool charger_enabled;

- unsigned long fullbatt_vchk_jiffies_at;
- struct delayed_work fullbatt_vchk_work;
-
int emergency_stop;

char psy_name_buf[PSY_NAME_MAX + 1];
@@ -247,11 +239,4 @@ struct charger_manager {
u64 charging_end_time;
};

-#ifdef CONFIG_CHARGER_MANAGER
-extern void cm_notify_event(struct power_supply *psy,
- enum cm_event_types type, char *msg);
-#else
-static inline void cm_notify_event(struct power_supply *psy,
- enum cm_event_types type, char *msg) { }
-#endif
#endif /* _CHARGER_MANAGER_H */
--
1.7.9.5

2014-10-30 12:46:31

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 01/10] power: charger-manager: Use thermal subsystem interface only to get temperature.

It drops the way of using power_supply interface to reference battery's
temperature. Then it tries to use thermal subsystem's only. This makes driver
more simple and also can remove ifdeferies.

Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/Kconfig | 1 +
drivers/power/charger-manager.c | 113 ++++++++-------------------------
include/linux/power/charger-manager.h | 3 +-
3 files changed, 28 insertions(+), 89 deletions(-)

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 8ff2511..115d153 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -317,6 +317,7 @@ config CHARGER_MANAGER
bool "Battery charger manager for multiple chargers"
depends on REGULATOR
select EXTCON
+ select THERMAL
help
Say Y to enable charger-manager support, which allows multiple
chargers attached to a battery and multiple batteries attached to a
diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 22246b9..b4b101c 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -28,13 +28,6 @@
#include <linux/of.h>
#include <linux/thermal.h>

-/*
- * Default termperature threshold for charging.
- * Every temperature units are in tenth of centigrade.
- */
-#define CM_DEFAULT_RECHARGE_TEMP_DIFF 50
-#define CM_DEFAULT_CHARGE_TEMP_MAX 500
-
static const char * const default_event_names[] = {
[CM_EVENT_UNKNOWN] = "Unknown",
[CM_EVENT_BATT_FULL] = "Battery Full",
@@ -572,40 +565,18 @@ static int check_charging_duration(struct charger_manager *cm)
return ret;
}

-static int cm_get_battery_temperature_by_psy(struct charger_manager *cm,
- int *temp)
-{
- struct power_supply *fuel_gauge;
-
- fuel_gauge = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
- if (!fuel_gauge)
- return -ENODEV;
-
- return fuel_gauge->get_property(fuel_gauge,
- POWER_SUPPLY_PROP_TEMP,
- (union power_supply_propval *)temp);
-}
-
static int cm_get_battery_temperature(struct charger_manager *cm,
int *temp)
{
int ret;

- if (!cm->desc->measure_battery_temp)
+ if (!cm->tzd_batt)
return -ENODEV;

-#ifdef CONFIG_THERMAL
- if (cm->tzd_batt) {
- ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long *)temp);
- if (!ret)
- /* Calibrate temperature unit */
- *temp /= 100;
- } else
-#endif
- {
- /* if-else continued from CONFIG_THERMAL */
- ret = cm_get_battery_temperature_by_psy(cm, temp);
- }
+ ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long *)temp);
+ if (!ret)
+ /* Calibrate temperature unit */
+ *temp /= 100;

return ret;
}
@@ -623,7 +594,7 @@ static int cm_check_thermal_status(struct charger_manager *cm)
* occur hazadous result. We have to handle it
* depending on battery type.
*/
- dev_err(cm->dev, "Failed to get battery temperature\n");
+ dev_dbg(cm->dev, "Failed to get battery temperature\n");
return 0;
}

@@ -1007,12 +978,11 @@ static enum power_supply_property default_charger_props[] = {
POWER_SUPPLY_PROP_CAPACITY,
POWER_SUPPLY_PROP_ONLINE,
POWER_SUPPLY_PROP_CHARGE_FULL,
+ POWER_SUPPLY_PROP_TEMP,
/*
* Optional properties are:
* POWER_SUPPLY_PROP_CHARGE_NOW,
* POWER_SUPPLY_PROP_CURRENT_NOW,
- * POWER_SUPPLY_PROP_TEMP, and
- * POWER_SUPPLY_PROP_TEMP_AMBIENT,
*/
};

@@ -1417,49 +1387,6 @@ err:
return ret;
}

-static int cm_init_thermal_data(struct charger_manager *cm,
- struct power_supply *fuel_gauge)
-{
- struct charger_desc *desc = cm->desc;
- union power_supply_propval val;
- int ret;
-
- /* Verify whether fuel gauge provides battery temperature */
- ret = fuel_gauge->get_property(fuel_gauge,
- POWER_SUPPLY_PROP_TEMP, &val);
-
- if (!ret) {
- cm->charger_psy.properties[cm->charger_psy.num_properties] =
- POWER_SUPPLY_PROP_TEMP;
- cm->charger_psy.num_properties++;
- cm->desc->measure_battery_temp = true;
- }
-#ifdef CONFIG_THERMAL
- if (ret && desc->thermal_zone) {
- cm->tzd_batt =
- thermal_zone_get_zone_by_name(desc->thermal_zone);
- if (IS_ERR(cm->tzd_batt))
- return PTR_ERR(cm->tzd_batt);
-
- /* Use external thermometer */
- cm->charger_psy.properties[cm->charger_psy.num_properties] =
- POWER_SUPPLY_PROP_TEMP_AMBIENT;
- cm->charger_psy.num_properties++;
- cm->desc->measure_battery_temp = true;
- ret = 0;
- }
-#endif
- if (cm->desc->measure_battery_temp) {
- /* NOTICE : Default allowable minimum charge temperature is 0 */
- if (!desc->temp_max)
- desc->temp_max = CM_DEFAULT_CHARGE_TEMP_MAX;
- if (!desc->temp_diff)
- desc->temp_diff = CM_DEFAULT_RECHARGE_TEMP_DIFF;
- }
-
- return ret;
-}
-
static struct of_device_id charger_manager_match[] = {
{
.compatible = "charger-manager",
@@ -1474,6 +1401,7 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
u32 poll_mode = CM_POLL_DISABLE;
u32 battery_stat = CM_NO_BATTERY;
int num_chgs = 0;
+ bool monitor_temp = false;

desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
if (!desc)
@@ -1517,14 +1445,16 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)

of_property_read_string(np, "cm-fuel-gauge", &desc->psy_fuel_gauge);

- of_property_read_string(np, "cm-thermal-zone", &desc->thermal_zone);
-
- of_property_read_u32(np, "cm-battery-cold", &desc->temp_min);
- if (of_get_property(np, "cm-battery-cold-in-minus", NULL))
- desc->temp_min *= -1;
- of_property_read_u32(np, "cm-battery-hot", &desc->temp_max);
+ if (!of_property_read_u32(np, "cm-battery-cold", &desc->temp_min))
+ monitor_temp = true;
+ if (!of_property_read_u32(np, "cm-battery-hot", &desc->temp_max))
+ monitor_temp = true;
of_property_read_u32(np, "cm-battery-temp-diff", &desc->temp_diff);

+ if (monitor_temp && of_property_read_string(np,
+ "cm-thermal-zone", &desc->thermal_zone))
+ desc->thermal_zone = desc->psy_fuel_gauge;
+
of_property_read_u32(np, "cm-charging-max",
&desc->charging_max_duration_ms);
of_property_read_u32(np, "cm-discharging-max",
@@ -1733,7 +1663,16 @@ static int charger_manager_probe(struct platform_device *pdev)
cm->charger_psy.num_properties++;
}

- ret = cm_init_thermal_data(cm, fuel_gauge);
+ if (desc->thermal_zone) {
+ cm->tzd_batt =
+ thermal_zone_get_zone_by_name(desc->thermal_zone);
+ if (!cm->tzd_batt) {
+ pr_err("Failed to get thermal zone (%s).\n",
+ desc->thermal_zone);
+ return -ENODEV;
+ }
+ }
+
if (ret) {
dev_err(&pdev->dev, "Failed to initialize thermal data\n");
cm->desc->measure_battery_temp = false;
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index 416ebeb..29d9b19 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -231,9 +231,8 @@ struct charger_manager {
struct device *dev;
struct charger_desc *desc;

-#ifdef CONFIG_THERMAL
struct thermal_zone_device *tzd_batt;
-#endif
+
bool charger_enabled;

unsigned long fullbatt_vchk_jiffies_at;
--
1.7.9.5

2014-10-30 12:46:32

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 02/10] power: charger-manager: Use power_supply_changed() not private uevent.

Whenever battery status is changed, charger manager tries to trigger uevent
through private interface. This patch modifies it to use power_supply_changed()
since it belongs to power supply subsystem.

Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/charger-manager.c | 91 +++++----------------------------------
1 file changed, 11 insertions(+), 80 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index b4b101c..33a1a4d 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -28,18 +28,6 @@
#include <linux/of.h>
#include <linux/thermal.h>

-static const char * const default_event_names[] = {
- [CM_EVENT_UNKNOWN] = "Unknown",
- [CM_EVENT_BATT_FULL] = "Battery Full",
- [CM_EVENT_BATT_IN] = "Battery Inserted",
- [CM_EVENT_BATT_OUT] = "Battery Pulled Out",
- [CM_EVENT_BATT_OVERHEAT] = "Battery Overheat",
- [CM_EVENT_BATT_COLD] = "Battery Cold",
- [CM_EVENT_EXT_PWR_IN_OUT] = "External Power Attach/Detach",
- [CM_EVENT_CHG_START_STOP] = "Charging Start/Stop",
- [CM_EVENT_OTHERS] = "Other battery events"
-};
-
/*
* Regard CM_JIFFIES_SMALL jiffies is small enough to ignore for
* delayed works so that we can run delayed works with CM_JIFFIES_SMALL
@@ -56,8 +44,6 @@ static const char * const default_event_names[] = {
*/
#define CM_RTC_SMALL (2)

-#define UEVENT_BUF_SIZE 32
-
static LIST_HEAD(cm_list);
static DEFINE_MUTEX(cm_list_mtx);

@@ -423,61 +409,6 @@ static int try_charger_restart(struct charger_manager *cm)
}

/**
- * uevent_notify - Let users know something has changed.
- * @cm: the Charger Manager representing the battery.
- * @event: the event string.
- *
- * If @event is null, it implies that uevent_notify is called
- * by resume function. When called in the resume function, cm_suspended
- * should be already reset to false in order to let uevent_notify
- * notify the recent event during the suspend to users. While
- * suspended, uevent_notify does not notify users, but tracks
- * events so that uevent_notify can notify users later after resumed.
- */
-static void uevent_notify(struct charger_manager *cm, const char *event)
-{
- static char env_str[UEVENT_BUF_SIZE + 1] = "";
- static char env_str_save[UEVENT_BUF_SIZE + 1] = "";
-
- if (cm_suspended) {
- /* Nothing in suspended-event buffer */
- if (env_str_save[0] == 0) {
- if (!strncmp(env_str, event, UEVENT_BUF_SIZE))
- return; /* status not changed */
- strncpy(env_str_save, event, UEVENT_BUF_SIZE);
- return;
- }
-
- if (!strncmp(env_str_save, event, UEVENT_BUF_SIZE))
- return; /* Duplicated. */
- strncpy(env_str_save, event, UEVENT_BUF_SIZE);
- return;
- }
-
- if (event == NULL) {
- /* No messages pending */
- if (!env_str_save[0])
- return;
-
- strncpy(env_str, env_str_save, UEVENT_BUF_SIZE);
- kobject_uevent(&cm->dev->kobj, KOBJ_CHANGE);
- env_str_save[0] = 0;
-
- return;
- }
-
- /* status not changed */
- if (!strncmp(env_str, event, UEVENT_BUF_SIZE))
- return;
-
- /* save the status and notify the update */
- strncpy(env_str, event, UEVENT_BUF_SIZE);
- kobject_uevent(&cm->dev->kobj, KOBJ_CHANGE);
-
- dev_info(cm->dev, "%s\n", event);
-}
-
-/**
* fullbatt_vchk - Check voltage drop some times after "FULL" event.
* @work: the work_struct appointing the function
*
@@ -514,7 +445,7 @@ static void fullbatt_vchk(struct work_struct *work)

if (diff > desc->fullbatt_vchkdrop_uV) {
try_charger_restart(cm);
- uevent_notify(cm, "Recharging");
+ power_supply_changed(&cm->charger_psy);
}
}

@@ -545,7 +476,7 @@ static int check_charging_duration(struct charger_manager *cm)
if (duration > desc->charging_max_duration_ms) {
dev_info(cm->dev, "Charging duration exceed %ums\n",
desc->charging_max_duration_ms);
- uevent_notify(cm, "Discharging");
+ power_supply_changed(&cm->charger_psy);
try_charger_enable(cm, false);
ret = true;
}
@@ -556,7 +487,7 @@ static int check_charging_duration(struct charger_manager *cm)
is_ext_pwr_online(cm)) {
dev_info(cm->dev, "Discharging duration exceed %ums\n",
desc->discharging_max_duration_ms);
- uevent_notify(cm, "Recharging");
+ power_supply_changed(&cm->charger_psy);
try_charger_enable(cm, true);
ret = true;
}
@@ -638,7 +569,7 @@ static bool _cm_monitor(struct charger_manager *cm)
if (temp_alrt) {
cm->emergency_stop = temp_alrt;
if (!try_charger_enable(cm, false))
- uevent_notify(cm, default_event_names[temp_alrt]);
+ power_supply_changed(&cm->charger_psy);

/*
* Check whole charging duration and discharing duration
@@ -663,7 +594,7 @@ static bool _cm_monitor(struct charger_manager *cm)
} else if (!cm->emergency_stop && is_full_charged(cm) &&
cm->charger_enabled) {
dev_info(cm->dev, "EVENT_HANDLE: Battery Fully Charged\n");
- uevent_notify(cm, default_event_names[CM_EVENT_BATT_FULL]);
+ power_supply_changed(&cm->charger_psy);

try_charger_enable(cm, false);

@@ -672,7 +603,7 @@ static bool _cm_monitor(struct charger_manager *cm)
cm->emergency_stop = 0;
if (is_ext_pwr_online(cm)) {
if (!try_charger_enable(cm, true))
- uevent_notify(cm, "CHARGING");
+ power_supply_changed(&cm->charger_psy);
}
}

@@ -793,7 +724,7 @@ static void fullbatt_handler(struct charger_manager *cm)

out:
dev_info(cm->dev, "EVENT_HANDLE: Battery Fully Charged\n");
- uevent_notify(cm, default_event_names[CM_EVENT_BATT_FULL]);
+ power_supply_changed(&cm->charger_psy);
}

/**
@@ -807,9 +738,9 @@ static void battout_handler(struct charger_manager *cm)

if (!is_batt_present(cm)) {
dev_emerg(cm->dev, "Battery Pulled Out!\n");
- uevent_notify(cm, default_event_names[CM_EVENT_BATT_OUT]);
+ power_supply_changed(&cm->charger_psy);
} else {
- uevent_notify(cm, "Battery Reinserted?");
+ power_supply_changed(&cm->charger_psy);
}
}

@@ -826,7 +757,7 @@ static void misc_event_handler(struct charger_manager *cm,

if (is_polling_required(cm) && cm->desc->polling_interval_ms)
schedule_work(&setup_polling);
- uevent_notify(cm, default_event_names[type]);
+ power_supply_changed(&cm->charger_psy);
}

static int charger_get_property(struct power_supply *psy,
@@ -1988,7 +1919,7 @@ void cm_notify_event(struct power_supply *psy, enum cm_event_types type,
break;
case CM_EVENT_UNKNOWN:
case CM_EVENT_OTHERS:
- uevent_notify(cm, msg ? msg : default_event_names[type]);
+ power_supply_changed(&cm->charger_psy);
break;
default:
dev_err(cm->dev, "%s: type not specified\n", __func__);
--
1.7.9.5

2014-10-30 12:46:29

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 05/10] power: charger-manager: Concentrate scattered power_supply_changed() calls.

Current charger-manager calls power_suuply_changed() whenever charging
status is changed. This patch removes seperated power_supply_changed()
use and let it be called at end of try_charger_enable() function which
is called to set charging/discharging.

Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/charger-manager.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 687f109..bb44588 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -382,8 +382,10 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
}
}

- if (!err)
+ if (!err) {
cm->charger_enabled = enable;
+ power_supply_changed(&cm->charger_psy);
+ }

return err;
}
@@ -436,10 +438,8 @@ static void fullbatt_vchk(struct charger_manager *cm)

dev_info(cm->dev, "VBATT dropped %duV after full-batt\n", diff);

- if (diff > desc->fullbatt_vchkdrop_uV) {
+ if (diff > desc->fullbatt_vchkdrop_uV)
try_charger_restart(cm);
- power_supply_changed(&cm->charger_psy);
- }
}

/**
@@ -469,7 +469,6 @@ static int check_charging_duration(struct charger_manager *cm)
if (duration > desc->charging_max_duration_ms) {
dev_info(cm->dev, "Charging duration exceed %ums\n",
desc->charging_max_duration_ms);
- power_supply_changed(&cm->charger_psy);
try_charger_enable(cm, false);
ret = true;
}
@@ -480,7 +479,6 @@ static int check_charging_duration(struct charger_manager *cm)
is_ext_pwr_online(cm)) {
dev_info(cm->dev, "Discharging duration exceed %ums\n",
desc->discharging_max_duration_ms);
- power_supply_changed(&cm->charger_psy);
try_charger_enable(cm, true);
ret = true;
}
@@ -561,8 +559,7 @@ static bool _cm_monitor(struct charger_manager *cm)
*/
if (temp_alrt) {
cm->emergency_stop = temp_alrt;
- if (!try_charger_enable(cm, false))
- power_supply_changed(&cm->charger_psy);
+ try_charger_enable(cm, false);

/*
* Check whole charging duration and discharing duration
@@ -587,16 +584,13 @@ static bool _cm_monitor(struct charger_manager *cm)
} else if (!cm->emergency_stop && is_full_charged(cm) &&
cm->charger_enabled) {
dev_info(cm->dev, "EVENT_HANDLE: Battery Fully Charged\n");
- power_supply_changed(&cm->charger_psy);
-
try_charger_enable(cm, false);

fullbatt_vchk(cm);
} else {
cm->emergency_stop = 0;
if (is_ext_pwr_online(cm)) {
- if (!try_charger_enable(cm, true))
- power_supply_changed(&cm->charger_psy);
+ try_charger_enable(cm, true);
}
}

--
1.7.9.5

2014-10-30 12:46:26

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 04/10] power: charger-manager: Fix to use CHARGE_NOW/FULL property correctly.

The POWER_SUPPLY_CHARGE_NOW/FULL property reflects battery's charges
in uAh unit, but charger-manager has been used it wrongly. This patch
makes it to use those propeties correctly and change to be optional.

Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/charger-manager.c | 85 +++++++++++++--------------------------
1 file changed, 28 insertions(+), 57 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 64fdaaf..687f109 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -796,35 +796,13 @@ static int charger_get_property(struct power_supply *psy,
val->intval = 0;
break;
case POWER_SUPPLY_PROP_CHARGE_FULL:
- if (is_full_charged(cm))
- val->intval = 1;
- else
- val->intval = 0;
- ret = 0;
- break;
case POWER_SUPPLY_PROP_CHARGE_NOW:
- if (is_charging(cm)) {
- fuel_gauge = power_supply_get_by_name(
- cm->desc->psy_fuel_gauge);
- if (!fuel_gauge) {
- ret = -ENODEV;
- break;
- }
-
- ret = fuel_gauge->get_property(fuel_gauge,
- POWER_SUPPLY_PROP_CHARGE_NOW,
- val);
- if (ret) {
- val->intval = 1;
- ret = 0;
- } else {
- /* If CHARGE_NOW is supplied, use it */
- val->intval = (val->intval > 0) ?
- val->intval : 1;
- }
- } else {
- val->intval = 0;
+ fuel_gauge = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
+ if (!fuel_gauge) {
+ ret = -ENODEV;
+ break;
}
+ ret = fuel_gauge->get_property(fuel_gauge, psp, val);
break;
default:
return -EINVAL;
@@ -832,8 +810,7 @@ static int charger_get_property(struct power_supply *psy,
return ret;
}

-#define NUM_CHARGER_PSY_OPTIONAL (4)
-static enum power_supply_property default_charger_props[] = {
+static enum power_supply_property cm_default_props[] = {
/* Guaranteed to provide */
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_HEALTH,
@@ -841,20 +818,21 @@ static enum power_supply_property default_charger_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_NOW,
POWER_SUPPLY_PROP_CAPACITY,
POWER_SUPPLY_PROP_ONLINE,
- POWER_SUPPLY_PROP_CHARGE_FULL,
POWER_SUPPLY_PROP_TEMP,
- /*
- * Optional properties are:
- * POWER_SUPPLY_PROP_CHARGE_NOW,
- * POWER_SUPPLY_PROP_CURRENT_NOW,
- */
};

+static enum power_supply_property cm_optional_props[] = {
+ POWER_SUPPLY_PROP_CHARGE_FULL,
+ POWER_SUPPLY_PROP_CHARGE_NOW,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
+};
+
+#define CM_NUM_OF_PROPS \
+ (ARRAY_SIZE(cm_default_props) + ARRAY_SIZE(cm_optional_props))
+
static struct power_supply psy_default = {
.name = "battery",
.type = POWER_SUPPLY_TYPE_BATTERY,
- .properties = default_charger_props,
- .num_properties = ARRAY_SIZE(default_charger_props),
.get_property = charger_get_property,
.no_thermal = true,
};
@@ -1484,29 +1462,22 @@ static int charger_manager_probe(struct platform_device *pdev)
/* Allocate for psy properties because they may vary */
cm->charger_psy.properties = devm_kzalloc(&pdev->dev,
sizeof(enum power_supply_property)
- * (ARRAY_SIZE(default_charger_props) +
- NUM_CHARGER_PSY_OPTIONAL), GFP_KERNEL);
+ * CM_NUM_OF_PROPS, GFP_KERNEL);
if (!cm->charger_psy.properties)
return -ENOMEM;

- memcpy(cm->charger_psy.properties, default_charger_props,
- sizeof(enum power_supply_property) *
- ARRAY_SIZE(default_charger_props));
- cm->charger_psy.num_properties = psy_default.num_properties;
-
- /* Find which optional psy-properties are available */
- if (!fuel_gauge->get_property(fuel_gauge,
- POWER_SUPPLY_PROP_CHARGE_NOW, &val)) {
- cm->charger_psy.properties[cm->charger_psy.num_properties] =
- POWER_SUPPLY_PROP_CHARGE_NOW;
- cm->charger_psy.num_properties++;
- }
- if (!fuel_gauge->get_property(fuel_gauge,
- POWER_SUPPLY_PROP_CURRENT_NOW,
- &val)) {
- cm->charger_psy.properties[cm->charger_psy.num_properties] =
- POWER_SUPPLY_PROP_CURRENT_NOW;
- cm->charger_psy.num_properties++;
+ memcpy(cm->charger_psy.properties, cm_default_props,
+ sizeof(enum power_supply_property) *
+ ARRAY_SIZE(cm_default_props));
+ cm->charger_psy.num_properties = ARRAY_SIZE(cm_default_props);
+
+ /* Add available optional properties */
+ for (i = 0; i < ARRAY_SIZE(cm_optional_props); i++) {
+ if (fuel_gauge->get_property(fuel_gauge,
+ cm_optional_props[i], &val))
+ continue;
+ cm->charger_psy.properties[cm->charger_psy.num_properties++] =
+ cm_optional_props[i];
}

if (desc->thermal_zone) {
--
1.7.9.5

2014-10-30 13:11:42

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 01/10] power: charger-manager: Use thermal subsystem interface only to get temperature.

On 30.10.2014 13:43, Jonghwa Lee wrote:
> It drops the way of using power_supply interface to reference battery's
> temperature. Then it tries to use thermal subsystem's only. This makes driver
> more simple and also can remove ifdeferies.
>
> Signed-off-by: Jonghwa Lee <[email protected]>
> ---
> drivers/power/Kconfig | 1 +
> drivers/power/charger-manager.c | 113 ++++++++-------------------------
> include/linux/power/charger-manager.h | 3 +-
> 3 files changed, 28 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 8ff2511..115d153 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -317,6 +317,7 @@ config CHARGER_MANAGER
> bool "Battery charger manager for multiple chargers"
> depends on REGULATOR
> select EXTCON
> + select THERMAL

I think both of "select" here could be dangerous. Select should rather
be used for non-visible errors. Just use "depends on".

> help
> Say Y to enable charger-manager support, which allows multiple
> chargers attached to a battery and multiple batteries attached to a
> diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
> index 22246b9..b4b101c 100644
> --- a/drivers/power/charger-manager.c
> +++ b/drivers/power/charger-manager.c
> @@ -28,13 +28,6 @@
> #include <linux/of.h>
> #include <linux/thermal.h>
>
> -/*
> - * Default termperature threshold for charging.
> - * Every temperature units are in tenth of centigrade.
> - */
> -#define CM_DEFAULT_RECHARGE_TEMP_DIFF 50
> -#define CM_DEFAULT_CHARGE_TEMP_MAX 500
> -
> static const char * const default_event_names[] = {
> [CM_EVENT_UNKNOWN] = "Unknown",
> [CM_EVENT_BATT_FULL] = "Battery Full",
> @@ -572,40 +565,18 @@ static int check_charging_duration(struct charger_manager *cm)
> return ret;
> }
>
> -static int cm_get_battery_temperature_by_psy(struct charger_manager *cm,
> - int *temp)
> -{
> - struct power_supply *fuel_gauge;
> -
> - fuel_gauge = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
> - if (!fuel_gauge)
> - return -ENODEV;
> -
> - return fuel_gauge->get_property(fuel_gauge,
> - POWER_SUPPLY_PROP_TEMP,
> - (union power_supply_propval *)temp);
> -}
> -
> static int cm_get_battery_temperature(struct charger_manager *cm,
> int *temp)
> {
> int ret;
>
> - if (!cm->desc->measure_battery_temp)
> + if (!cm->tzd_batt)
> return -ENODEV;
>
> -#ifdef CONFIG_THERMAL
> - if (cm->tzd_batt) {
> - ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long *)temp);
> - if (!ret)
> - /* Calibrate temperature unit */
> - *temp /= 100;
> - } else
> -#endif
> - {
> - /* if-else continued from CONFIG_THERMAL */
> - ret = cm_get_battery_temperature_by_psy(cm, temp);
> - }
> + ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long *)temp);
> + if (!ret)
> + /* Calibrate temperature unit */
> + *temp /= 100;
>
> return ret;
> }
> @@ -623,7 +594,7 @@ static int cm_check_thermal_status(struct charger_manager *cm)
> * occur hazadous result. We have to handle it
> * depending on battery type.
> */
> - dev_err(cm->dev, "Failed to get battery temperature\n");
> + dev_dbg(cm->dev, "Failed to get battery temperature\n");

A valuable change but not strictly related to the commit. Additionally
that is a user-visible change. Could you split it to separate patch?

Best regards,
Krzysztof

2014-10-31 02:25:56

by Jonghwa Lee

[permalink] [raw]
Subject: Re: [PATCH 01/10] power: charger-manager: Use thermal subsystem interface only to get temperature.

Hi,
On 2014년 10월 30일 22:11, Krzysztof Kozłowski wrote:

> On 30.10.2014 13:43, Jonghwa Lee wrote:
>> It drops the way of using power_supply interface to reference battery's
>> temperature. Then it tries to use thermal subsystem's only. This makes driver
>> more simple and also can remove ifdeferies.
>>
>> Signed-off-by: Jonghwa Lee <[email protected]>
>> ---
>> drivers/power/Kconfig | 1 +
>> drivers/power/charger-manager.c | 113 ++++++++-------------------------
>> include/linux/power/charger-manager.h | 3 +-
>> 3 files changed, 28 insertions(+), 89 deletions(-)
>>
>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>> index 8ff2511..115d153 100644
>> --- a/drivers/power/Kconfig
>> +++ b/drivers/power/Kconfig
>> @@ -317,6 +317,7 @@ config CHARGER_MANAGER
>> bool "Battery charger manager for multiple chargers"
>> depends on REGULATOR
>> select EXTCON
>> + select THERMAL
>
> I think both of "select" here could be dangerous. Select should rather
> be used for non-visible errors. Just use "depends on".
>
>> }
>> @@ -623,7 +594,7 @@ static int cm_check_thermal_status(struct charger_manager *cm)
>> * occur hazadous result. We have to handle it
>> * depending on battery type.
>> */
>> - dev_err(cm->dev, "Failed to get battery temperature\n");
>> + dev_dbg(cm->dev, "Failed to get battery temperature\n");
>
> A valuable change but not strictly related to the commit. Additionally
> that is a user-visible change. Could you split it to separate patch?
>
> Best regards,
> Krzysztof
>
>


All your comments are acceptable. I'll fix them all.
Thanks for your comments.

Thanks,
Jonghwa