Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH 1/3] power: cpcap-battery: Do not issue low signal too
 frequently
To:     sre@kernel.org
Cc:     linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        tony@atomide.com, philipp@uvos.xyz
References: <1667332425-12536-1-git-send-email-ivo.g.dimitrov.75@gmail.com>
 <1667332425-12536-2-git-send-email-ivo.g.dimitrov.75@gmail.com>
From:   Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Message-ID: <ce999c0b-342d-6a29-6a16-b69cf8a1db9d@gmail.com>
Date:   Fri, 4 Nov 2022 08:09:46 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Firefox/60.0 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <1667332425-12536-2-git-send-email-ivo.g.dimitrov.75@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-GB
Content-Transfer-Encoding: 8bit
Precedence: bulk

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 <ivo.g.dimitrov.75@gmail.com>
> ---
>   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;
>   }
>   
>