| Message ID | 20230413181743.7849-1-jason.gerecke@wacom.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 7fc68653fc2e1a3457bb886e10164119ac48734f |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [1/2] HID: wacom: Lazy-init batteries | expand |
[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
On Thu, 13 Apr 2023, Jason Gerecke wrote: > 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> I have added the Link: tags suggested by Mario and applied, thanks.
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;