[PATCH 0/5] hooking udev to SW_TABLET_MODE changes

[PATCH 0/5] hooking udev to SW_TABLET_MODE changes

[Carlos Garnacho]

The following patches make it possible to have udev/userspace notified when
SW_TABLET_MODE changes on the switch device, without having to resort to
poll()/select() for such sparse events. I've got udev patches locally to
complement this.

I've been just able to test on thinkpad_acpi (X220T), hp-wmi (Touchsmart
TX2) and xo15-ebook. The per-module change is straightforward enough, but
an ack would still be great on classmate and fujitsu modules.

Carlos Garnacho (5):
  thinkpad_acpi: Send uevent when SW_TABLET_MODE changes
  hp-wmi: Send uevent when SW_TABLET_MODE changes
  xo15-ebook: Send uevent when SW_TABLET_MODE changes
  classmate-laptop: Send uevent when SW_TABLET_MODE changes
  fujitsu-tablet: Send uevent when SW_TABLET_MODE changes

 drivers/platform/x86/classmate-laptop.c | 2 ++
 drivers/platform/x86/fujitsu-tablet.c   | 1 +
 drivers/platform/x86/hp-wmi.c           | 3 +++
 drivers/platform/x86/thinkpad_acpi.c    | 2 ++
 drivers/platform/x86/xo15-ebook.c       | 1 +
 5 files changed, 9 insertions(+)

-- 
1.8.5.1

[PATCH 1/5] thinkpad_acpi: Send uevent when SW_TABLET_MODE changes

[Carlos Garnacho]

This enables udev and userspace to react to those events without
having to explicitly poll()/select() on the device.

Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
---
 drivers/platform/x86/thinkpad_acpi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 58b0274..5003611 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -2251,6 +2251,8 @@ static void tpacpi_input_send_tabletsw(void)
 		input_sync(tpacpi_inputdev);
 
 		mutex_unlock(&tpacpi_inputdev_send_mutex);
+
+		kobject_uevent(&tpacpi_inputdev->dev.kobj, KOBJ_CHANGE);
 	}
 }
 
-- 
1.8.5.1

[PATCH 2/5] hp-wmi: Send uevent when SW_TABLET_MODE changes

[Carlos Garnacho]

This enables udev and userspace to react to those events without
having to explicitly poll()/select() on the device.

Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
---
 drivers/platform/x86/hp-wmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 8ba8956..d5428ea 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -554,6 +554,7 @@ static void hp_wmi_notify(u32 value, void *context)
 		input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE,
 				    hp_wmi_tablet_state());
 		input_sync(hp_wmi_input_dev);
+		kobject_uevent(&hp_wmi_input_dev->dev.kobj, KOBJ_CHANGE);
 		break;
 	case HPWMI_PARK_HDD:
 		break;
@@ -647,6 +648,7 @@ static int __init hp_wmi_input_setup(void)
 	input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE,
 			    hp_wmi_tablet_state());
 	input_sync(hp_wmi_input_dev);
+	kobject_uevent(&hp_wmi_input_dev->dev.kobj, KOBJ_CHANGE);
 
 	status = wmi_install_notify_handler(HPWMI_EVENT_GUID, hp_wmi_notify, NULL);
 	if (ACPI_FAILURE(status)) {
@@ -955,6 +957,7 @@ static int hp_wmi_resume_handler(struct device *device)
 		input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE,
 				    hp_wmi_tablet_state());
 		input_sync(hp_wmi_input_dev);
+		kobject_uevent(&hp_wmi_input_dev->dev.kobj, KOBJ_CHANGE);
 	}
 
 	if (rfkill2_count)
-- 
1.8.5.1

[PATCH 3/5] xo15-ebook: Send uevent when SW_TABLET_MODE changes

[Carlos Garnacho]

This enables udev and userspace to react to those events without
having to explicitly poll()/select() on the device.

Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
---
 drivers/platform/x86/xo15-ebook.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
index 4b1377b..2deaaa7 100644
--- a/drivers/platform/x86/xo15-ebook.c
+++ b/drivers/platform/x86/xo15-ebook.c
@@ -60,6 +60,7 @@ static int ebook_send_state(struct acpi_device *device)
 	/* input layer checks if event is redundant */
 	input_report_switch(button->input, SW_TABLET_MODE, !state);
 	input_sync(button->input);
+	kobject_uevent(&button->input->dev.kobj, KOBJ_CHANGE);
 	return 0;
 }
 
-- 
1.8.5.1

[PATCH 4/5] classmate-laptop: Send uevent when SW_TABLET_MODE changes

[Carlos Garnacho]

This enables udev and userspace to react to those events without
having to explicitly poll()/select() on the device.

Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
---
 drivers/platform/x86/classmate-laptop.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
index 6dfa8d3..6329a0d 100644
--- a/drivers/platform/x86/classmate-laptop.c
+++ b/drivers/platform/x86/classmate-laptop.c
@@ -728,6 +728,7 @@ static void cmpc_tablet_handler(struct acpi_device *dev, u32 event)
 		if (ACPI_SUCCESS(cmpc_get_tablet(dev->handle, &val))) {
 			input_report_switch(inputdev, SW_TABLET_MODE, !val);
 			input_sync(inputdev);
+			kobject_uevent(&inputdev->dev.kobj, KOBJ_CHANGE);
 		}
 	}
 }
@@ -744,6 +745,7 @@ static void cmpc_tablet_idev_init(struct input_dev *inputdev)
 	if (ACPI_SUCCESS(cmpc_get_tablet(acpi->handle, &val))) {
 		input_report_switch(inputdev, SW_TABLET_MODE, !val);
 		input_sync(inputdev);
+		kobject_uevent(&inputdev->dev.kobj, KOBJ_CHANGE);
 	}
 }
 
-- 
1.8.5.1

[PATCH 5/5] fujitsu-tablet: Send uevent when SW_TABLET_MODE changes

[Carlos Garnacho]

This enables udev and userspace to react to those events without
having to explicitly poll()/select() on the device.

Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
---
 drivers/platform/x86/fujitsu-tablet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/fujitsu-tablet.c b/drivers/platform/x86/fujitsu-tablet.c
index 570926c..935fb4e 100644
--- a/drivers/platform/x86/fujitsu-tablet.c
+++ b/drivers/platform/x86/fujitsu-tablet.c
@@ -178,6 +178,7 @@ static void fujitsu_send_state(void)
 	input_report_switch(fujitsu.idev, SW_DOCK, dock);
 	input_report_switch(fujitsu.idev, SW_TABLET_MODE, tablet_mode);
 	input_sync(fujitsu.idev);
+	kobject_uevent(&fujitsu.idev->dev.kobj, KOBJ_CHANGE);
 }
 
 static void fujitsu_reset(void)
-- 
1.8.5.1

Re: [PATCH 0/5] hooking udev to SW_TABLET_MODE changes

[Henrique de Moraes Holschuh]

On Tue, 17 Dec 2013, Carlos Garnacho wrote:
> The following patches make it possible to have udev/userspace notified when
> SW_TABLET_MODE changes on the switch device, without having to resort to
> poll()/select() for such sparse events. I've got udev patches locally to
> complement this.

I don't like this.  We moved away from uevents for this kind of stuff (in
the general sense) because they, to put it lightly, are NOT appropriate for
input-event-like events.

Moving any sort of input event back to uevents is something I do NOT want to
support, ever.  Any userspace that wants it for backwards compatibility can,
and DOES synthesize uevents from input events.

So, can you make a very strong case for this uevent?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 0/5] hooking udev to SW_TABLET_MODE changes

[Carlos Garnacho]

Hey Henrique,

On lun, 2013-12-16 at 22:03 -0200, Henrique de Moraes Holschuh wrote:
> On Tue, 17 Dec 2013, Carlos Garnacho wrote:
> > The following patches make it possible to have udev/userspace notified when
> > SW_TABLET_MODE changes on the switch device, without having to resort to
> > poll()/select() for such sparse events. I've got udev patches locally to
> > complement this.
> 
> I don't like this.  We moved away from uevents for this kind of stuff (in
> the general sense) because they, to put it lightly, are NOT appropriate for
> input-event-like events.

I would like to know... not appropriate how? I can understand this rule
applies for too often events, as some input devices can get quite
chatty, but the frequency of these uevents is tremendously low. FWIW,
there is precedence of uevents on input devices in the asus-laptop
module [1], where the accelerometer reports uevents on "coarse
orientation changes".

> 
> Moving any sort of input event back to uevents is something I do NOT want to
> support, ever.  Any userspace that wants it for backwards compatibility can,
> and DOES synthesize uevents from input events.

I'm not proposing "moving back", but "complementing", the device still
has to be poked. The problem I see with awaiting for input events in
userspace in this case is that there is no good place to have this done.
My first attempt time ago was doing this in UPower, because it is
already in the business of reading input events, but it was deemed too
unrelated [2]. The next option is having a dedicated daemon, listening
for an event that's very probably not doing to happen at all during
uptime... sounds pretty wasteful to me.

> 
> So, can you make a very strong case for this uevent?
> 

Well, the above. I could add that the accelerometer uevent I pointed out
is put in use in udev and the GNOME stack, I think it makes sense to
give tablet-mode changes a similar treatment, so establishing policies
and whatnot is saner. And I think most of the userspace that's
interested in these events would want that too, with maybe exotic
exceptions.

  Carlos


[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/platform/x86/asus-laptop.c#n1564
[2] https://bugs.freedesktop.org/show_bug.cgi?id=43935

Re: [PATCH 0/5] hooking udev to SW_TABLET_MODE changes

[Henrique de Moraes Holschuh]

On Tue, 17 Dec 2013, Carlos Garnacho wrote:
> On lun, 2013-12-16 at 22:03 -0200, Henrique de Moraes Holschuh wrote:
> > On Tue, 17 Dec 2013, Carlos Garnacho wrote:
> > > The following patches make it possible to have udev/userspace notified when
> > > SW_TABLET_MODE changes on the switch device, without having to resort to
> > > poll()/select() for such sparse events. I've got udev patches locally to
> > > complement this.
> > 
> > I don't like this.  We moved away from uevents for this kind of stuff (in
> > the general sense) because they, to put it lightly, are NOT appropriate for
> > input-event-like events.
> 
> I would like to know... not appropriate how? I can understand this rule

uevents are: heavy-weight, slow, the wrong layer for this stuff (we ended up
mixing important device infrastructure topology/configuration change events
(what it was designed to be used for) with misc user-interface events, thus
making everything more brittle since userspace has to handle it all in the
same place).  They are not power-management-friendly, not scalable as
implemented, not filterable kernel-side, and not versioned.  uevents also
have closer ties to internal kernel models than necessary for
non-device-infrastructure related events (sysfs has it much worse, but
still).

All of those issues are much better addressed by the input events layer,
which was designed from the ground up to better deal with moderate-frequency
user-interface/input events.

> applies for too often events, as some input devices can get quite
> chatty, but the frequency of these uevents is tremendously low. FWIW,

All you need to do is to spin the tablet/laptop/mobile device.  We once
thought backlight change events would be low-frequency too, and it is
causing real problems...

I don't think it is wise to be one sucessfull game away ("spintron, the new
game where you take a photo of something/someone and spin your tablet to do
funny things to it") from that much pain.  So, you need to add
rate-limiters, and since it *is* input events we're talking about, these are
low-latency, state-preserving rate-limiters (which must emit the first and
last events in a chain without much extra latency or it will feel sluggish
to the user, must break long chains, and preserve the effects of supressed
events).

> there is precedence of uevents on input devices in the asus-laptop
> module [1], where the accelerometer reports uevents on "coarse
> orientation changes".

That doesn't make it a good idea.

> > Moving any sort of input event back to uevents is something I do NOT want to
> > support, ever.  Any userspace that wants it for backwards compatibility can,
> > and DOES synthesize uevents from input events.
> 
> I'm not proposing "moving back", but "complementing", the device still
> has to be poked. The problem I see with awaiting for input events in
> userspace in this case is that there is no good place to have this done.

You'd just have to deploy a proper generic input event handler that could
map any sort of native X, native Linux input events, or native Wayland input
events to actions.  Indeed, if there is no such beast already, you'd need to
add it.

> My first attempt time ago was doing this in UPower, because it is
> already in the business of reading input events, but it was deemed too
> unrelated [2]. The next option is having a dedicated daemon, listening
> for an event that's very probably not doing to happen at all during
> uptime... sounds pretty wasteful to me.

Well, a dedicated daemon IS the unix way, and it is only wasteful if you
want it to be.  You could make it the xinetd of input events (probably with
builtin actions that generate DBUS events and other up-to-date desktopish
stuff), and it would be of general use (and rather useful at that, IMHO).

Or you could make it a very specialized small utility where the entire bloat
comes from the fact that it is linked to glibc, and it will have a very
small RSS... there are several like that already, I believe.

> > So, can you make a very strong case for this uevent?
> 
> Well, the above. I could add that the accelerometer uevent I pointed out
> is put in use in udev and the GNOME stack, I think it makes sense to
> give tablet-mode changes a similar treatment, so establishing policies
> and whatnot is saner. And I think most of the userspace that's
> interested in these events would want that too, with maybe exotic
> exceptions.

The big deal here is that, if we do that, we will basically have to live
with it for a decade or more... and we already know uevents are not a very
suitable event delivery system for low-frequency user-interface/input
events, and it is utterly unsuitable for high-frequency events regardless of
type (you need to compress/supress them into low-frequency events).

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh