RE: [PATCH 1/2] HID: wacom: Lazy-init batteries

["Limonciello, Mario" <Mario.Limonciello@amd.com>]

[Public]

> From: Jason Gerecke <killertofu@gmail.com>
> 
> Rather than creating batteries as part of the initial device probe, let's
> make the process lazy. This gives us the opportunity to prevent batteries
> from being created in situations where they are unnecessary.
> 
> There are two cases in particular where batteries are being unnecessarily
> created at initialization. These are AES sensors (for which we don't know
> any battery status information until a battery-powered pen actually comes
> into prox) peripheral tablets which share HID descriptors between the
> wired-only and wireless-capable SKUs of a family of devices.
> 
> This patch will delay battery initialization of the former until a pen
> actually comes into prox. It will delay battery initialization of the
> latter until either a pen comes into prox or a "heartbeat" packet is
> processed.
> 
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>

Some other tags to add:

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217062
Link: https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/2354

> ---
>  drivers/hid/wacom_sys.c | 10 ----------
>  drivers/hid/wacom_wac.c | 13 ++++++-------
>  2 files changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index fb538a6c4add8..8214896adadad 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2372,13 +2372,6 @@ static int wacom_parse_and_register(struct
> wacom *wacom, bool wireless)
>  	if (error)
>  		goto fail;
> 
> -	if (!(features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
> &&
> -	     (features->quirks & WACOM_QUIRK_BATTERY)) {
> -		error = wacom_initialize_battery(wacom);
> -		if (error)
> -			goto fail;
> -	}
> -
>  	error = wacom_register_inputs(wacom);
>  	if (error)
>  		goto fail;
> @@ -2509,9 +2502,6 @@ static void wacom_wireless_work(struct
> work_struct *work)
> 
>  		strscpy(wacom_wac->name, wacom_wac1->name,
>  			sizeof(wacom_wac->name));
> -		error = wacom_initialize_battery(wacom);
> -		if (error)
> -			goto fail;
>  	}
> 
>  	return;
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 4cfa51416dbcb..391fde5bf6024 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -113,6 +113,11 @@ static void wacom_notify_battery(struct
> wacom_wac *wacom_wac,
>  	bool bat_connected, bool ps_connected)
>  {
>  	struct wacom *wacom = container_of(wacom_wac, struct wacom,
> wacom_wac);
> +	bool bat_initialized = wacom->battery.battery;
> +	bool has_quirk = wacom_wac->features.quirks &
> WACOM_QUIRK_BATTERY;
> +
> +	if (bat_initialized != has_quirk)
> +		wacom_schedule_work(wacom_wac,
> WACOM_WORKER_BATTERY);
> 
>  	__wacom_notify_battery(&wacom->battery, bat_status,
> bat_capacity,
>  			       bat_charging, bat_connected, ps_connected);
> @@ -3391,19 +3396,13 @@ static int wacom_status_irq(struct wacom_wac
> *wacom_wac, size_t len)
>  		int battery = (data[8] & 0x3f) * 100 / 31;
>  		bool charging = !!(data[8] & 0x80);
> 
> +		features->quirks |= WACOM_QUIRK_BATTERY;
>  		wacom_notify_battery(wacom_wac,
> WACOM_POWER_SUPPLY_STATUS_AUTO,
>  				     battery, charging, battery || charging, 1);
> -
> -		if (!wacom->battery.battery &&
> -		    !(features->quirks & WACOM_QUIRK_BATTERY)) {
> -			features->quirks |= WACOM_QUIRK_BATTERY;
> -			wacom_schedule_work(wacom_wac,
> WACOM_WORKER_BATTERY);
> -		}
>  	}
>  	else if ((features->quirks & WACOM_QUIRK_BATTERY) &&
>  		 wacom->battery.battery) {
>  		features->quirks &= ~WACOM_QUIRK_BATTERY;
> -		wacom_schedule_work(wacom_wac,
> WACOM_WORKER_BATTERY);
>  		wacom_notify_battery(wacom_wac,
> POWER_SUPPLY_STATUS_UNKNOWN, 0, 0, 0, 0);
>  	}
>  	return 0;
> --
> 2.40.0