From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Jiri Kosina <jikos@kernel.org>
Cc: Kenneth Albanowski <kenalba@google.com>,
Sebastian Reichel <sre@kernel.org>,
Benjamin Tissoires <benjamin.tissoires@redhat.com>,
Johan Korsnes <jkorsnes@cisco.com>,
linux-input@vger.kernel.org
Subject: Re: [PATCH] HID: hid-input: occasionally report stylus battery even if not changed
Date: Tue, 27 Oct 2020 10:38:20 -0700 [thread overview]
Message-ID: <20201027173820.GK444962@dtor-ws> (raw)
In-Reply-To: <CALvoSf7kZEhrZZEZmzpAkEMbd+TQbNTCOmjvjX6RfxeWiQZz2g@mail.gmail.com>
M
On Wed, Oct 07, 2020 at 03:05:13PM -0700, Kenneth Albanowski wrote:
> 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.
Jiri, if there are no better ideas, maybe this one can be applied?
Thanks!
>
> - 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
--
Dmitry
next prev parent reply other threads:[~2020-10-27 17:38 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-30 22:47 [PATCH] HID: hid-input: occasionally report stylus battery even if not changed dmitry.torokhov
2020-10-07 22:05 ` Kenneth Albanowski
2020-10-27 17:38 ` Dmitry Torokhov [this message]
2020-10-29 15:15 ` Jiri Kosina
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201027173820.GK444962@dtor-ws \
--to=dmitry.torokhov@gmail.com \
--cc=benjamin.tissoires@redhat.com \
--cc=jikos@kernel.org \
--cc=jkorsnes@cisco.com \
--cc=kenalba@google.com \
--cc=linux-input@vger.kernel.org \
--cc=sre@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).