Hi,
On Sat, Nov 05, 2022 at 01:25:42PM +0200, 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 <ivo.g.dimitrov.75@xxxxxxxxx>
---
Concept looks ok to me, but the code is slightly racy, since the
thread is flushed before the IRQ is disabled in the remove routine.
drivers/power/supply/cpcap-battery.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 4676560..8869067 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 delayed_work low_irq_work;
u32 cc_lsb; /* μAms per LSB */
atomic_t active;
int charge_full;
@@ -914,9 +915,13 @@ 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 &&
+ !delayed_work_pending((&ddata->low_irq_work))) {
dev_warn(ddata->dev, "Battery low at %imV!\n",
latest->voltage / 1000);
+ schedule_delayed_work(&ddata->low_irq_work, 30 * HZ);
+ disable_irq_nosync(d->irq);
+ }
break;
case CPCAP_BATTERY_IRQ_ACTION_POWEROFF:
if (latest->current_ua >= 0 && latest->voltage <= 3200000) {
@@ -1087,6 +1092,21 @@ static int cpcap_battery_calibrate(struct cpcap_battery_ddata *ddata)
return error;
}
+static void cpcap_battery_lowbph_enable(struct work_struct *work)
+{
+ struct delayed_work *d_work = to_delayed_work(work);
+ struct cpcap_battery_ddata *ddata = container_of(d_work,
+ struct cpcap_battery_ddata, low_irq_work);
+ 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 +1138,8 @@ static int cpcap_battery_probe(struct platform_device *pdev)
if (!ddata)
return -ENOMEM;
+ INIT_DELAYED_WORK(&ddata->low_irq_work, cpcap_battery_lowbph_enable);
use devm_delayed_work_autocancel() and put it directly before
cpcap_battery_init_interrupts().
cpcap_battery_detect_battery_type(ddata);
INIT_LIST_HEAD(&ddata->irq_list);
@@ -1185,6 +1207,9 @@ static int cpcap_battery_remove(struct platform_device *pdev)
if (error)
dev_err(&pdev->dev, "could not disable: %i\n", error);
+ /* make sure to call enable_irq() if needed */
+ flush_delayed_work(&ddata->low_irq_work);
and this can be dropped afterwards.
+
return 0;
}
Thanks,
-- Sebastian