Re: [PATCH 52/57] power: abx500_chargalg: Use hrtimer
On Fri, Sep 28, 2012 at 12:33:51PM -0600, Mathieu Poirier wrote:
[...]
> >> @@ -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.
I was partly referencing to my answer here
http://lkml.org/lkml/2012/9/27/678
There I was suggesting how to make abx500_chargalg_check_safety_timer()
simpler, but since in this patch you switching code to hrtimers, the logic
have to change too.
So, to put it even simpler, in this patch you don't even need to check for
time values, in abx500_chargalg_start_safety_timer(), you can just write
if (hrtimer_active(&di->safety_timer))
return;
dev_dbg(di->dev, "starting safety timer\n");
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);
And you don't have to check for hrtimer_active() anywhere else...
[...]
> >> @@ -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");
> >>}
..so with the above suggestion, this code will turn into just
if (di->batt_data.percent < 100) {
abx500_chargalg_stop_safety_timer(di);
return;
}
abx500_chargalg_start_safety_timer(di);
Thanks,
Anton.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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.poir...@linaro.org wrote:
>> From: Hakan Berg
>>
>> 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
>> Signed-off-by: Mathieu Poirier
>> Reviewed-by: Mian Yousaf KAUKAB
>> ---
>> 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
>> * Karl Komierowski
>> * Arun R Murthy
>> + * Imre Sunyi
>> */
>>
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -32,6 +35,12 @@
>> /* End-of-charge criteria counter */
>> #define EOC_COND_CNT10
>>
>> +/* 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;
>>
Re: [PATCH 52/57] power: abx500_chargalg: Use hrtimer
On Tue, Sep 25, 2012 at 10:12:49AM -0600, mathieu.poir...@linaro.org wrote:
> From: Hakan Berg
>
> 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
> Signed-off-by: Mathieu Poirier
> Reviewed-by: Mian Yousaf KAUKAB
> ---
> 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
> * Karl Komierowski
> * Arun R Murthy
> + * Imre Sunyi
> */
>
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -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
[PATCH 52/57] power: abx500_chargalg: Use hrtimer
From: Hakan Berg
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
Signed-off-by: Mathieu Poirier
Reviewed-by: Mian Yousaf KAUKAB
---
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
* Karl Komierowski
* Arun R Murthy
+ * Imre Sunyi
*/
#include
#include
#include
+#include
#include
#include
#include
@@ -32,6 +35,12 @@
/* End-of-charge criteria counter */
#define EOC_COND_CNT 10
+/* One hour expressed in seconds */
+#define ONE_HOUR_IN_SECONDS3600
+
+/* Five minutes expressed in seconds */
+#define FIVE_MINUTES_IN_SECONDS300
+
#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);
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;
-
