| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 28 Sep 2012 12:33:51 -0600
From: Mathieu Poirier <mathieu.poirier@...aro.org>
To: Anton Vorontsov <anton.vorontsov@...aro.org>
CC: linux-kernel@...r.kernel.org, dwmw2@...radead.org
Subject: Re: [PATCH 52/57] power: abx500_chargalg: Use hrtimer
On 12-09-27 08:47 PM, Anton Vorontsov wrote:
> On Tue, Sep 25, 2012 at 10:12:49AM -0600, mathieu.poirier@...aro.org wrote:
>> From: Hakan Berg <hakan.berg@...ricsson.com>
>>
>> Timers used for charging safety and maintenance must work even when
>> CPU is power collapsed. By using hrtimers with realtime clock, system
>> is able to trigger an alarm that wakes the CPU up and make it possible
>> to handle the event.
>>
>> Allow a little slack of 5 minutes to the hrtimers to allow CPU to be
>> waked up in a more optimal power saving way. A 5 minute delay to
>> time out timers on hours does not impact on safety.
>>
>> Signed-off-by: Hakan Berg <hakan.berg@...ricsson.com>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@...aro.org>
>> Reviewed-by: Mian Yousaf KAUKAB <mian.yousaf.kaukab@...ricsson.com>
>> ---
>> drivers/power/abx500_chargalg.c | 94 ++++++++++++++++++++++-----------------
>> 1 files changed, 53 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/power/abx500_chargalg.c b/drivers/power/abx500_chargalg.c
>> index 636d970..c8849af 100644
>> --- a/drivers/power/abx500_chargalg.c
>> +++ b/drivers/power/abx500_chargalg.c
>> @@ -1,5 +1,6 @@
>> /*
>> * Copyright (C) ST-Ericsson SA 2012
>> + * Copyright (c) 2012 Sony Mobile Communications AB
>> *
>> * Charging algorithm driver for abx500 variants
>> *
>> @@ -8,11 +9,13 @@
>> * Johan Palsson <johan.palsson@...ricsson.com>
>> * Karl Komierowski <karl.komierowski@...ricsson.com>
>> * Arun R Murthy <arun.murthy@...ricsson.com>
>> + * Imre Sunyi <imre.sunyi@...ymobile.com>
>> */
>>
>> #include <linux/init.h>
>> #include <linux/module.h>
>> #include <linux/device.h>
>> +#include <linux/hrtimer.h>
>> #include <linux/interrupt.h>
>> #include <linux/delay.h>
>> #include <linux/slab.h>
>> @@ -32,6 +35,12 @@
>> /* End-of-charge criteria counter */
>> #define EOC_COND_CNT 10
>>
>> +/* One hour expressed in seconds */
>> +#define ONE_HOUR_IN_SECONDS 3600
>> +
>> +/* Five minutes expressed in seconds */
>> +#define FIVE_MINUTES_IN_SECONDS 300
>> +
>> #define to_abx500_chargalg_device_info(x) container_of((x), \
>> struct abx500_chargalg, chargalg_psy);
>>
>> @@ -245,8 +254,8 @@ struct abx500_chargalg {
>> struct delayed_work chargalg_periodic_work;
>> struct delayed_work chargalg_wd_work;
>> struct work_struct chargalg_work;
>> - struct timer_list safety_timer;
>> - struct timer_list maintenance_timer;
>> + struct hrtimer safety_timer;
>> + struct hrtimer maintenance_timer;
>> struct kobject chargalg_kobject;
>> };
>>
>> @@ -261,38 +270,47 @@ BLOCKING_NOTIFIER_HEAD(charger_notifier_list);
>>
>> /**
>> * abx500_chargalg_safety_timer_expired() - Expiration of the safety timer
>> - * @data: pointer to the abx500_chargalg structure
>> + * @timer: pointer to the hrtimer structure
>> *
>> * This function gets called when the safety timer for the charger
>> * expires
>> */
>> -static void abx500_chargalg_safety_timer_expired(unsigned long data)
>> +static enum hrtimer_restart
>> +abx500_chargalg_safety_timer_expired(struct hrtimer *timer)
>> {
>> - struct abx500_chargalg *di = (struct abx500_chargalg *) data;
>> + struct abx500_chargalg *di = container_of(timer, struct abx500_chargalg,
>> + safety_timer);
>
> Empty line here.
>
>> dev_err(di->dev, "Safety timer expired\n");
>> di->events.safety_timer_expired = true;
>>
>> /* Trigger execution of the algorithm instantly */
>> queue_work(di->chargalg_wq, &di->chargalg_work);
>> +
>> + return HRTIMER_NORESTART;
>> }
>>
>> /**
>> * abx500_chargalg_maintenance_timer_expired() - Expiration of
>> * the maintenance timer
>> - * @i: pointer to the abx500_chargalg structure
>> + * @timer: pointer to the timer structure
>> *
>> * This function gets called when the maintenence timer
>> * expires
>> */
>> -static void abx500_chargalg_maintenance_timer_expired(unsigned long data)
>> +static enum hrtimer_restart
>> +abx500_chargalg_maintenance_timer_expired(struct hrtimer *timer)
>> +
>> {
>>
>> - struct abx500_chargalg *di = (struct abx500_chargalg *) data;
>> + struct abx500_chargalg *di = container_of(timer, struct abx500_chargalg,
>> + maintenance_timer);
>> dev_dbg(di->dev, "Maintenance timer expired\n");
>> di->events.maintenance_timer_expired = true;
>>
>> /* Trigger execution of the algorithm instantly */
>> queue_work(di->chargalg_wq, &di->chargalg_work);
>> +
>> + return HRTIMER_NORESTART;
>> }
>>
>> /**
>> @@ -392,19 +410,16 @@ static int abx500_chargalg_check_charger_connection(struct abx500_chargalg *di)
>> */
>> static void abx500_chargalg_start_safety_timer(struct abx500_chargalg *di)
>> {
>> - unsigned long timer_expiration = 0;
>> + /* Charger-dependent expiration time in hours*/
>> + int timer_expiration = 0;
>>
>> switch (di->chg_info.charger_type) {
>> case AC_CHG:
>> - timer_expiration =
>> - round_jiffies(jiffies +
>> - (di->bat->main_safety_tmr_h * 3600 * HZ));
>> + timer_expiration = di->bat->main_safety_tmr_h;
>> break;
>>
>> case USB_CHG:
>> - timer_expiration =
>> - round_jiffies(jiffies +
>> - (di->bat->usb_safety_tmr_h * 3600 * HZ));
>> + timer_expiration = di->bat->usb_safety_tmr_h;
>> break;
>>
>> default:
>> @@ -413,11 +428,10 @@ static void abx500_chargalg_start_safety_timer(struct abx500_chargalg *di)
>> }
>>
>> di->events.safety_timer_expired = false;
>> - di->safety_timer.expires = timer_expiration;
>> - if (!timer_pending(&di->safety_timer))
>> - add_timer(&di->safety_timer);
>> - else
>> - mod_timer(&di->safety_timer, timer_expiration);
>> + hrtimer_set_expires_range(&di->safety_timer,
>> + ktime_set(timer_expiration * ONE_HOUR_IN_SECONDS, 0),
>> + ktime_set(FIVE_MINUTES_IN_SECONDS, 0));
>> + hrtimer_start_expires(&di->safety_timer, HRTIMER_MODE_REL);
>
> OK, now we're chaning from timers to hrtimers, and their behaviour
> might be different. I guess in this case you should check whether
> old value 'before' new value, and if so, do nothing.
>
Would you mind being more descriptive - I'm affraid that I don't
understand your suggestion.
>> }
>>
>> /**
>> @@ -428,8 +442,8 @@ static void abx500_chargalg_start_safety_timer(struct abx500_chargalg *di)
>> */
>> static void abx500_chargalg_stop_safety_timer(struct abx500_chargalg *di)
>> {
>> - di->events.safety_timer_expired = false;
>> - del_timer(&di->safety_timer);
>> + if (hrtimer_try_to_cancel(&di->safety_timer) >= 0)
>> + di->events.safety_timer_expired = false;
>> }
>>
>> /**
>> @@ -444,17 +458,11 @@ static void abx500_chargalg_stop_safety_timer(struct abx500_chargalg *di)
>> static void abx500_chargalg_start_maintenance_timer(struct abx500_chargalg *di,
>> int duration)
>> {
>> - unsigned long timer_expiration;
>> -
>> - /* Convert from hours to jiffies */
>> - timer_expiration = round_jiffies(jiffies + (duration * 3600 * HZ));
>> -
>> + hrtimer_set_expires_range(&di->maintenance_timer,
>> + ktime_set(duration * ONE_HOUR_IN_SECONDS, 0),
>> + ktime_set(FIVE_MINUTES_IN_SECONDS, 0));
>> di->events.maintenance_timer_expired = false;
>> - di->maintenance_timer.expires = timer_expiration;
>> - if (!timer_pending(&di->maintenance_timer))
>> - add_timer(&di->maintenance_timer);
>> - else
>> - mod_timer(&di->maintenance_timer, timer_expiration);
>> + hrtimer_start_expires(&di->maintenance_timer, HRTIMER_MODE_REL);
>> }
>>
>> /**
>> @@ -466,8 +474,8 @@ static void abx500_chargalg_start_maintenance_timer(struct abx500_chargalg *di,
>> */
>> static void abx500_chargalg_stop_maintenance_timer(struct abx500_chargalg *di)
>> {
>> - di->events.maintenance_timer_expired = false;
>> - del_timer(&di->maintenance_timer);
>> + if (hrtimer_try_to_cancel(&di->maintenance_timer) >= 0)
>> + di->events.maintenance_timer_expired = false;
>> }
>>
>> /**
>> @@ -910,11 +918,11 @@ static void abx500_chargalg_check_safety_timer(struct abx500_chargalg *di)
>> * and charging will be stopped to protect the battery.
>> */
>> if (di->batt_data.percent == 100 &&
>> - !timer_pending(&di->safety_timer)) {
>> + !hrtimer_active(&di->safety_timer)) {
>> abx500_chargalg_start_safety_timer(di);
>> dev_dbg(di->dev, "start safety timer\n");
>> } else if (di->batt_data.percent != 100 &&
>> - timer_pending(&di->safety_timer)) {
>> + hrtimer_active(&di->safety_timer)) {
>> abx500_chargalg_stop_safety_timer(di);
>> dev_dbg(di->dev, "stop safety timer\n");
>> }
>> @@ -1932,10 +1940,16 @@ static int __devexit abx500_chargalg_remove(struct platform_device *pdev)
>> /* sysfs interface to enable/disbale charging from user space */
>> abx500_chargalg_sysfs_exit(di);
>>
>> + hrtimer_cancel(&di->safety_timer);
>> + hrtimer_cancel(&di->maintenance_timer);
>> +
>> + cancel_delayed_work_sync(&di->chargalg_periodic_work);
>> + cancel_delayed_work_sync(&di->chargalg_wd_work);
>> + cancel_work_sync(&di->chargalg_work);
>> +
>
> These cancel_ calls seem to be separate bugfixes?
>
>> /* Delete the work queue */
>> destroy_workqueue(di->chargalg_wq);
>>
>> - flush_scheduled_work();
>> power_supply_unregister(&di->chargalg_psy);
>> platform_set_drvdata(pdev, NULL);
>> kfree(di);
>> @@ -1987,15 +2001,13 @@ static int __devinit abx500_chargalg_probe(struct platform_device *pdev)
>> abx500_chargalg_external_power_changed;
>>
>> /* Initilialize safety timer */
>> - init_timer(&di->safety_timer);
>> + hrtimer_init(&di->safety_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
>> di->safety_timer.function = abx500_chargalg_safety_timer_expired;
>> - di->safety_timer.data = (unsigned long) di;
>>
>> /* Initilialize maintenance timer */
>> - init_timer(&di->maintenance_timer);
>> + hrtimer_init(&di->maintenance_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
>> di->maintenance_timer.function =
>> abx500_chargalg_maintenance_timer_expired;
>> - di->maintenance_timer.data = (unsigned long) di;
>>
>> /* Create a work queue for the chargalg */
>> di->chargalg_wq =
>> --
>> 1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists