From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Aaron Armstrong Skomra <skomra@gmail.com>
Cc: Jiri Kosina <jikos@kernel.org>, Ping Cheng <pinglinux@gmail.com>,
Jason Gerecke <killertofu@gmail.com>,
Peter Hutterer <peter.hutterer@who-t.net>,
linux-kernel@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH 22/27] HID: wacom: EKR: attach the power_supply on first connection
Date: Wed, 13 Jul 2016 17:24:44 +0200 [thread overview]
Message-ID: <20160713152444.GI4663@mail.corp.redhat.com> (raw)
In-Reply-To: <20160712092901.GH4663@mail.corp.redhat.com>
On Jul 12 2016 or thereabouts, Benjamin Tissoires wrote:
> On Jul 08 2016 or thereabouts, Aaron Armstrong Skomra wrote:
> > On Tue, Jul 5, 2016 at 7:39 AM, Benjamin Tissoires <
> > benjamin.tissoires@redhat.com> wrote:
> >
> > > Or Gnome complains about an empty battery.
> > >
> > > Hi Benjamin,
> >
> > I tested this series on the 24HD, 21UX2, Intuos P&T (CTH-680), and with 2
> > EKRs.
>
> I somehow missed this email. Sorry for the late answer.
>
> >
> > When I attach (by USB wire) a wireless capable tablet with the rechargeable
> > battery and wireless module connected inside the tablet, I get the same
> > momentary 0% battery notification from gnome (see attached upower output).
>
> Yes, I saw that, but I think it's not a regression. It's the way it's
> already working now (from what I can see), and so it would be nice to
> be fixed, but I didn't in this series.
My bad. I found the issue and it was added by my patches :( It's fixed
now and will be in v2 (coming shortly).
Cheers,
Benjamin
>
> >
> > Aside from that
> >
> > Tested-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
>
> Thanks!
>
> But given Peter's comments regarding grouping, I think I'll need you to
> redo the tests on the 24HD and 21UX2 with the next series as the initial
> kernel implementation of LEDs needs to be reverted, or libwacom will
> fail :(
>
> Cheers,
> Benjamin
>
> >
> > Best,
> > Aaron
> >
> >
> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > ---
> > > drivers/hid/wacom_sys.c | 36 ++++++++++++++++++++++++++++++------
> > > 1 file changed, 30 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > > index 04f5c75..1d79215 100644
> > > --- a/drivers/hid/wacom_sys.c
> > > +++ b/drivers/hid/wacom_sys.c
> > > @@ -1917,6 +1917,10 @@ static void wacom_remote_destroy_one(struct wacom
> > > *wacom, unsigned int index)
> > > remote->remotes[index].registered = false;
> > > spin_unlock_irqrestore(&remote->remote_lock, flags);
> > >
> > > + if (remote->remotes[index].battery.battery)
> > > + devres_release_group(&wacom->hdev->dev,
> > > +
> > > &remote->remotes[index].battery.bat_desc);
> > > +
> > > if (remote->remotes[index].group.name)
> > > devres_release_group(&wacom->hdev->dev,
> > > &remote->remotes[index]);
> > > @@ -1926,6 +1930,7 @@ static void wacom_remote_destroy_one(struct wacom
> > > *wacom, unsigned int index)
> > > remote->remotes[i].serial = 0;
> > > remote->remotes[i].group.name = NULL;
> > > remote->remotes[i].registered = false;
> > > + remote->remotes[i].battery.battery = NULL;
> > > wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
> > > }
> > > }
> > > @@ -1982,11 +1987,6 @@ static int wacom_remote_create_one(struct wacom
> > > *wacom, u32 serial,
> > > if (error)
> > > goto fail;
> > >
> > > - error = __wacom_initialize_battery(wacom,
> > > -
> > > &remote->remotes[index].battery);
> > > - if (error)
> > > - goto fail;
> > > -
> > > remote->remotes[index].registered = true;
> > >
> > > devres_close_group(dev, &remote->remotes[index]);
> > > @@ -1998,6 +1998,28 @@ fail:
> > > return error;
> > > }
> > >
> > > +static int wacom_remote_attach_battery(struct wacom *wacom, int index)
> > > +{
> > > + struct wacom_remote *remote = wacom->remote;
> > > + int error;
> > > +
> > > + if (!remote->remotes[index].registered)
> > > + return 0;
> > > +
> > > + if (remote->remotes[index].battery.battery)
> > > + return 0;
> > > +
> > > + if (wacom->led.groups[index].select == WACOM_STATUS_UNKNOWN)
> > > + return 0;
> > > +
> > > + error = __wacom_initialize_battery(wacom,
> > > +
> > > &wacom->remote->remotes[index].battery);
> > > + if (error)
> > > + return error;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static void wacom_remote_work(struct work_struct *work)
> > > {
> > > struct wacom *wacom = container_of(work, struct wacom,
> > > remote_work);
> > > @@ -2028,8 +2050,10 @@ static void wacom_remote_work(struct work_struct
> > > *work)
> > > serial = data.remote[i].serial;
> > > if (data.remote[i].connected) {
> > >
> > > - if (remote->remotes[i].serial == serial)
> > > + if (remote->remotes[i].serial == serial) {
> > > + wacom_remote_attach_battery(wacom, i);
> > > continue;
> > > + }
> > >
> > > if (remote->remotes[i].serial)
> > > wacom_remote_destroy_one(wacom, i);
> > > --
> > > 2.5.5
> > >
> > >
>
> > [wacom@localhost ~]$ upower -d
> > Device: /org/freedesktop/UPower/devices/mouse_wacom_battery_7
> > native-path: wacom_battery_7
> > power supply: no
> > updated: Thu 07 Jul 2016 01:55:12 PM PDT (1 seconds ago)
> > has history: yes
> > has statistics: yes
> > mouse
> > present: yes
> > rechargeable: yes
> > state: discharging
> > warning-level: critical
> > percentage: 0%
> > icon-name: 'battery-caution-symbolic'
> > History (charge):
> > 1467924912 0.000 unknown
> > 1467924886 96.000 charging
> > 1467924880 0.000 unknown
> > History (rate):
> > 1467924912 0.000 unknown
> > 1467924880 0.000 unknown
> >
> > Device: /org/freedesktop/UPower/devices/DisplayDevice
> > power supply: no
> > updated: Thu 07 Jul 2016 12:09:09 PM PDT (6364 seconds ago)
> > has history: no
> > has statistics: no
> > unknown
> > warning-level: none
> > icon-name: ''
> >
> > Daemon:
> > daemon-version: 0.99.4
> > on-battery: no
> > lid-is-closed: no
> > lid-is-present: no
> > critical-action: PowerOff
> > [wacom@localhost ~]$ upower -d
> > Device: /org/freedesktop/UPower/devices/mouse_wacom_battery_7
> > native-path: wacom_battery_7
> > power supply: no
> > updated: Thu 07 Jul 2016 01:55:18 PM PDT (3 seconds ago)
> > has history: yes
> > has statistics: yes
> > mouse
> > present: yes
> > rechargeable: yes
> > state: charging
> > warning-level: none
> > percentage: 96%
> > icon-name: 'battery-full-charging-symbolic'
> > History (charge):
> > 1467924918 96.000 charging
> > 1467924912 0.000 unknown
> > 1467924886 96.000 charging
> > 1467924880 0.000 unknown
> > History (rate):
> > 1467924912 0.000 unknown
> > 1467924880 0.000 unknown
> >
> > Device: /org/freedesktop/UPower/devices/DisplayDevice
> > power supply: no
> > updated: Thu 07 Jul 2016 12:09:09 PM PDT (6372 seconds ago)
> > has history: no
> > has statistics: no
> > unknown
> > warning-level: none
> > icon-name: ''
> >
> > Daemon:
> > daemon-version: 0.99.4
> > on-battery: no
> > lid-is-closed: no
> > lid-is-present: no
> > critical-action: PowerOff
> >
>
next prev parent reply other threads:[~2016-07-13 15:25 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-05 14:38 [PATCH 00/27] HID: wacom: cleanup/EKR/LED Benjamin Tissoires
2016-07-05 14:38 ` [PATCH 01/27] HID: wacom: actually report the battery level for wireless connected Benjamin Tissoires
2016-07-05 14:38 ` [PATCH 02/27] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2 Benjamin Tissoires
2016-07-05 14:38 ` [PATCH 03/27] HID: wacom: remove cleanup of wacom->remote_dir from wacom_clean_inputs() Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 04/27] HID: wacom: untie leds from inputs Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 05/27] HID: wacom: use one work queue per task Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 06/27] HID: wacom: switch battery to devres Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 07/27] HID: wacom: switch inputs " Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 08/27] HID: wacom: put the managed resources in a group Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 09/27] HID: wacom: convert LEDs to devres Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 10/27] HID: wacom: use devm_kasprintf for allocating the name of the remote Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 11/27] HID: wacom: use devres to allocate driver data Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 12/27] HID: wacom: devres manage the shared data too Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 13/27] HID: wacom: leds: dynamically allocate LED groups Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 14/27] HID: wacom: EKR: add a worker to add/remove resources on addition/removal Benjamin Tissoires
2016-07-05 15:21 ` kbuild test robot
2016-07-05 14:39 ` [PATCH 15/27] HID: wacom: EKR: have the wacom resources dynamically allocated Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 16/27] HID: wacom: rework fail path in probe() and parse_and_register() Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 17/27] HID: wacom: EKR: have proper allocator and destructor Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 18/27] HID: wacom: EKR: use devres groups to manage resources Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 19/27] HID: wacom: EKR: have one array of struct remotes instead of many arrays Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 20/27] HID: wacom: EKR: allocate one input node per remote Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 21/27] HID: wacom: EKR: have one power_supply " Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 22/27] HID: wacom: EKR: attach the power_supply on first connection Benjamin Tissoires
[not found] ` <CAEoswT0inHAzo4pP8sBaXpMR_iqUG_U=kdGagAu_m8DybMMDzA@mail.gmail.com>
2016-07-12 9:29 ` Benjamin Tissoires
2016-07-13 15:24 ` Benjamin Tissoires [this message]
2016-07-05 14:39 ` [PATCH 23/27] HID: wacom: leds: use the ledclass instead of custom made sysfs files Benjamin Tissoires
2016-07-12 1:42 ` Peter Hutterer
2016-07-05 14:39 ` [PATCH 24/27] HID: wacom: leds: actually release the LEDs on disconnect Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 25/27] HID: wacom: leds: fix ordering of LED banks Benjamin Tissoires
2016-07-12 1:52 ` Peter Hutterer
2016-07-05 14:39 ` [PATCH 26/27] HID: wacom: leds: handle the switch of the LEDs directly in the kernel Benjamin Tissoires
2016-07-05 15:58 ` kbuild test robot
2016-07-05 21:32 ` Benjamin Tissoires
2016-07-05 14:39 ` [PATCH 27/27] HID: wacom: leds: handle Cintiq 24HD leds buttons Benjamin Tissoires
2016-07-05 16:19 ` kbuild test robot
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=20160713152444.GI4663@mail.corp.redhat.com \
--to=benjamin.tissoires@redhat.com \
--cc=jikos@kernel.org \
--cc=killertofu@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peter.hutterer@who-t.net \
--cc=pinglinux@gmail.com \
--cc=skomra@gmail.com \
/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).