2022-11-01 20:05:56

by Ivaylo Dimitrov

Subject: [PATCH 0/3] power: supply: cpcap-battery improvements

This patch series implements few improvememts to cpcap-battery driver

2022-11-01 20:28:31

by Ivaylo Dimitrov

Subject: [PATCH 1/3] power: cpcap-battery: Do not issue low signal too frequently

It seems that low battery irq may be generated tens of times per second,
leading to userspace being flooded with unnecessary events.

Fix that by preventing such events being generated more than once every 30
seconds.

Signed-off-by: Ivaylo Dimitrov <[email protected]>
---
drivers/power/supply/cpcap-battery.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 4676560..2659df7 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -137,6 +137,7 @@ struct cpcap_battery_ddata {
struct power_supply *psy;
struct cpcap_battery_config config;
struct cpcap_battery_state_data state[CPCAP_BATTERY_STATE_NR];
+ struct timer_list low_timer;
u32 cc_lsb; /* μAms per LSB */
atomic_t active;
int charge_full;
@@ -914,9 +915,14 @@ static irqreturn_t cpcap_battery_irq_thread(int irq, void *data)
dev_info(ddata->dev, "Coulomb counter calibration done\n");
break;
case CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW:
- if (latest->current_ua >= 0)
+ if (latest->current_ua >= 0 &&
+ !timer_pending(&ddata->low_timer)) {
dev_warn(ddata->dev, "Battery low at %imV!\n",
latest->voltage / 1000);
+ mod_timer(&ddata->low_timer,
+ jiffies + msecs_to_jiffies(30000));
+ disable_irq_nosync(d->irq);
+ }
break;
case CPCAP_BATTERY_IRQ_ACTION_POWEROFF:
if (latest->current_ua >= 0 && latest->voltage <= 3200000) {
@@ -1087,6 +1093,19 @@ static int cpcap_battery_calibrate(struct cpcap_battery_ddata *ddata)
return error;
}

+static void cpcap_battery_lowbph_enable(struct timer_list *t)
+{
+ struct cpcap_battery_ddata *ddata = from_timer(ddata, t, low_timer);
+ struct cpcap_interrupt_desc *d;
+
+ list_for_each_entry(d, &ddata->irq_list, node) {
+ if (d->action == CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW)
+ break;
+ }
+
+ enable_irq(d->irq);
+}
+
#ifdef CONFIG_OF
static const struct of_device_id cpcap_battery_id_table[] = {
{
@@ -1118,6 +1137,8 @@ static int cpcap_battery_probe(struct platform_device *pdev)
if (!ddata)
return -ENOMEM;

+ timer_setup(&ddata->low_timer, cpcap_battery_lowbph_enable, 0);
+
cpcap_battery_detect_battery_type(ddata);

INIT_LIST_HEAD(&ddata->irq_list);
@@ -1185,6 +1206,8 @@ static int cpcap_battery_remove(struct platform_device *pdev)
if (error)
dev_err(&pdev->dev, "could not disable: %i\n", error);

+ del_timer_sync(&ddata->low_timer);
+
return 0;
}

--
1.9.1

2022-11-04 06:34:36

by Ivaylo Dimitrov

Subject: Re: [PATCH 1/3] power: cpcap-battery: Do not issue low signal too frequently

This patch has issues (calling enable_irq() from timer function), will
send v2 that uses delayed_work.

On 1.11.22 г. 21:53 ч., Ivaylo Dimitrov wrote:
> It seems that low battery irq may be generated tens of times per second,
> leading to userspace being flooded with unnecessary events.
>
> Fix that by preventing such events being generated more than once every 30
> seconds.
>
> Signed-off-by: Ivaylo Dimitrov <[email protected]>
> ---
> drivers/power/supply/cpcap-battery.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
> index 4676560..2659df7 100644
> --- a/drivers/power/supply/cpcap-battery.c
> +++ b/drivers/power/supply/cpcap-battery.c
> @@ -137,6 +137,7 @@ struct cpcap_battery_ddata {
> struct power_supply *psy;
> struct cpcap_battery_config config;
> struct cpcap_battery_state_data state[CPCAP_BATTERY_STATE_NR];
> + struct timer_list low_timer;
> u32 cc_lsb; /* μAms per LSB */
> atomic_t active;
> int charge_full;
> @@ -914,9 +915,14 @@ static irqreturn_t cpcap_battery_irq_thread(int irq, void *data)
> dev_info(ddata->dev, "Coulomb counter calibration done\n");
> break;
> case CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW:
> - if (latest->current_ua >= 0)
> + if (latest->current_ua >= 0 &&
> + !timer_pending(&ddata->low_timer)) {
> dev_warn(ddata->dev, "Battery low at %imV!\n",
> latest->voltage / 1000);
> + mod_timer(&ddata->low_timer,
> + jiffies + msecs_to_jiffies(30000));
> + disable_irq_nosync(d->irq);
> + }
> break;
> case CPCAP_BATTERY_IRQ_ACTION_POWEROFF:
> if (latest->current_ua >= 0 && latest->voltage <= 3200000) {
> @@ -1087,6 +1093,19 @@ static int cpcap_battery_calibrate(struct cpcap_battery_ddata *ddata)
> return error;
> }
>
> +static void cpcap_battery_lowbph_enable(struct timer_list *t)
> +{
> + struct cpcap_battery_ddata *ddata = from_timer(ddata, t, low_timer);
> + struct cpcap_interrupt_desc *d;
> +
> + list_for_each_entry(d, &ddata->irq_list, node) {
> + if (d->action == CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW)
> + break;
> + }
> +
> + enable_irq(d->irq);
> +}
> +
> #ifdef CONFIG_OF
> static const struct of_device_id cpcap_battery_id_table[] = {
> {
> @@ -1118,6 +1137,8 @@ static int cpcap_battery_probe(struct platform_device *pdev)
> if (!ddata)
> return -ENOMEM;
>
> + timer_setup(&ddata->low_timer, cpcap_battery_lowbph_enable, 0);
> +
> cpcap_battery_detect_battery_type(ddata);
>
> INIT_LIST_HEAD(&ddata->irq_list);
> @@ -1185,6 +1206,8 @@ static int cpcap_battery_remove(struct platform_device *pdev)
> if (error)
> dev_err(&pdev->dev, "could not disable: %i\n", error);
>
> + del_timer_sync(&ddata->low_timer);
> +
> return 0;
> }
>
>