Re: [PATCH] HID: hid-input: occasionally report stylus battery even if not changed

[Kenneth Albanowski <kenalba@google.com>]

I've tested this locally backported to a 4.19 variant; it does work,
and provides the expected additional CHANGE events while safely
limiting the rate.

This seems like a reasonable minimum improvement, just ensuring
information already being pushed by HID reports can be utilized.

I'm unsure about mapping Digitizers.InRange to power_supply 'present',
in the abstract. It seems there could be a device where
Digitizers.BatteryStrength is sent, despite Digitizers.InRange=0, and
the HID Usage Tables don't quite seem to rule this out (depending on
how battery status collection interacts with 'the region where
digitizing is possible', section 16.3.1.).

As-is, this is useful and sufficient to get more timely reports up to userspace.

- Kenneth Albanowski


On Wed, Sep 30, 2020 at 3:47 PM <dmitry.torokhov@gmail.com> wrote:
>
> There are styluses that only report their battery status when they are
> touching the touchscreen; additionally we currently suppress battery
> reports if capacity has not changed. To help userspace recognize how long
> ago the device reported battery status, let's send the change event through
> if either capacity has changed, or at least 30 seconds have passed since
> last report we've let through.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> This is a bit of RFC. Another option would be to mark the power supply
> as either offline or not present when stylus leaves the surface instead
> of saying it is online... Sebastian, any ideas/suggestions?
>
>  drivers/hid/hid-input.c | 5 ++++-
>  include/linux/hid.h     | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 5da631d2ec9b..92b4c9bb6619 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -534,9 +534,12 @@ static void hidinput_update_battery(struct hid_device *dev, int value)
>         capacity = hidinput_scale_battery_capacity(dev, value);
>
>         if (dev->battery_status != HID_BATTERY_REPORTED ||
> -           capacity != dev->battery_capacity) {
> +           capacity != dev->battery_capacity ||
> +           ktime_after(ktime_get_coarse(), dev->battery_ratelimit_time)) {
>                 dev->battery_capacity = capacity;
>                 dev->battery_status = HID_BATTERY_REPORTED;
> +               dev->battery_ratelimit_time =
> +                       ktime_add_ms(ktime_get_coarse(), 30 * 1000);
>                 power_supply_changed(dev->battery);
>         }
>  }
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 875f71132b14..c76a18f88262 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -583,6 +583,7 @@ struct hid_device {                                                 /* device report descriptor */
>         __s32 battery_report_id;
>         enum hid_battery_status battery_status;
>         bool battery_avoid_query;
> +       ktime_t battery_ratelimit_time;
>  #endif
>
>         unsigned long status;                                           /* see STAT flags above */
> --
> 2.28.0.709.gb0816b6eb0-goog
>
>
> --
> Dmitry