27 Feb
2017
27 Feb
'17
4 p.m.
On 27.02.2017 16:09, Aric Stewart wrote:
Signed-off-by: Aric Stewart
<aric(a)codeweavers.com>
---
dlls/winebus.sys/bus.h | 3 +
dlls/winebus.sys/bus_udev.c | 150 ++++++++++++++++++++++++++++++++++++++++++--
dlls/winebus.sys/main.c | 26 ++++++++
3 files changed, 173 insertions(+), 6 deletions(-)
I assume this is meant as a fallback, if we don't have access to hidraw, right?
If you plan to use this as a replacement for dinput, it would be better to
implement it without fixed dependency on udev.
0001-winebus.sys-Handle-linux-input-event-device-via-udev.txt
diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h
index effbd4afd0..fed35e8c1e 100644
--- a/dlls/winebus.sys/bus.h
+++ b/dlls/winebus.sys/bus.h
@@ -43,3 +43,6 @@ DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void
*platform_dev
void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN;
NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN;
void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length)
DECLSPEC_HIDDEN;
+
+/* General Bus Functions */
+DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option)
DECLSPEC_HIDDEN;
diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c
index 415bd3db08..2dcc7fb442 100644
--- a/dlls/winebus.sys/bus_udev.c
+++ b/dlls/winebus.sys/bus_udev.c
@@ -42,6 +42,14 @@
# include <sys/ioctl.h>
#endif
+#ifdef HAVE_LINUX_INPUT_H
+# include <linux/input.h>
+# undef SW_MAX
+# if defined(EVIOCGBIT) && defined(EV_ABS) && defined(BTN_PINKIE)
+# define HAS_PROPER_INPUT_HEADER
+# endif
+#endif
+
#define NONAMELESSUNION
#include "ntstatus.h"
@@ -65,11 +73,15 @@ WINE_DECLARE_DEBUG_CHANNEL(hid_report);
static struct udev *udev_context = NULL;
static DRIVER_OBJECT *udev_driver_obj = NULL;
+static DWORD disable_hidraw = 0;
+static DWORD disable_input = 0;
static const WCHAR hidraw_busidW[] =
{'H','I','D','R','A','W',0};
+static const WCHAR lnxev_busidW[] =
{'L','N','X','E','V',0};
#include "initguid.h"
DEFINE_GUID(GUID_DEVCLASS_HIDRAW,
0x3def44ad,0x242e,0x46e5,0x82,0x6d,0x70,0x72,0x13,0xf3,0xaa,0x81);
+DEFINE_GUID(GUID_DEVCLASS_LINUXEVENT,
0x1b932c0d,0xfea7,0x42cd,0x8e,0xaa,0x0e,0x48,0x79,0xb6,0x9e,0xaa);
struct platform_private
{
@@ -392,6 +404,52 @@ static const platform_vtbl hidraw_vtbl =
hidraw_set_feature_report,
};
+#ifdef HAS_PROPER_INPUT_HEADER
+
+static NTSTATUS lnxev_get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD
length, DWORD *out_length)
+{
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer,
DWORD length)
+{
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_begin_report_processing(DEVICE_OBJECT *device)
+{
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report,
DWORD length, ULONG_PTR *written)
+{
+ *written = 0;
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_get_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report,
DWORD length, ULONG_PTR *read)
+{
+ *read = 0;
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_set_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report,
DWORD length, ULONG_PTR *written)
+{
+ *written = 0;
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static const platform_vtbl lnxev_vtbl = {
+ compare_platform_device,
+ lnxev_get_reportdescriptor,
+ lnxev_get_string,
+ lnxev_begin_report_processing,
+ lnxev_set_output_report,
+ lnxev_get_feature_report,
+ lnxev_set_feature_report,
+};
+#endif
+
static void try_add_device(struct udev_device *dev)
{
DWORD vid = 0, pid = 0, version = 0;
@@ -400,6 +458,7 @@ static void try_add_device(struct udev_device *dev)
const char *subsystem;
const char *devnode;
WCHAR *serial = NULL;
+ const char* gamepad = NULL;
int fd;
if (!(devnode = udev_device_get_devnode(dev)))
@@ -419,6 +478,26 @@ static void try_add_device(struct udev_device *dev)
version = get_sysattr_dword(usbdev, "version", 10);
serial = get_sysattr_string(usbdev, "serial");
}
+#ifdef HAS_PROPER_INPUT_HEADER
+ else
+ {
+ struct input_id device_id = {0};
+ char device_uid[255] = {0};
+
+ if (ioctl(fd, EVIOCGID, &device_id) == -1)
+ WARN("ioctl(EVIOCGID) failed: %d %s\n", errno, strerror(errno));
+ if (ioctl(fd, EVIOCGUNIQ(254), device_uid) && device_uid[0])
+ MultiByteToWideChar(CP_UNIXCP, 0, device_uid, -1, serial, 255);
I don't think this will work as expected. serial is a NULL pointer, and the check
if ioctl() was successful is also done wrong. On error a value < 0 is returned.
+
+ gamepad = udev_device_get_property_value(dev, "ID_INPUT_JOYSTICK");
+ vid = device_id.vendor;
+ pid = device_id.product;
+ version = device_id.version;
+ }
+#else
+ else
+ WARN("Could not get device to query VID, PID, Version and Serial\n");
+#endif
TRACE("Found udev device %s (vid %04x, pid %04x, version %u, serial
%s)\n",
debugstr_a(devnode), vid, pid, version, debugstr_w(serial));
@@ -429,6 +508,12 @@ static void try_add_device(struct udev_device *dev)
device = bus_create_hid_device(udev_driver_obj, hidraw_busidW, vid, pid,
version, 0, serial, FALSE,
&GUID_DEVCLASS_HIDRAW, &hidraw_vtbl,
sizeof(struct platform_private));
}
+#ifdef HAS_PROPER_INPUT_HEADER
+ else if (strcmp(subsystem, "input") == 0)
+ {
+ device = bus_create_hid_device(udev_driver_obj, lnxev_busidW, vid, pid, version,
0, serial, (gamepad != NULL), &GUID_DEVCLASS_LINUXEVENT, &lnxev_vtbl,
sizeof(struct platform_private));
Some linebreaks wouldn't hurt.
+ }
+#endif
if (device)
{
@@ -448,8 +533,17 @@ static void try_add_device(struct udev_device *dev)
static void try_remove_device(struct udev_device *dev)
{
- DEVICE_OBJECT *device = bus_find_hid_device(&hidraw_vtbl, dev);
- struct platform_private *private;
+ DEVICE_OBJECT *device = NULL;
+ const char* subsystem;
+ struct platform_private* private;
+
+ subsystem = udev_device_get_subsystem(dev);
+ if (strcmp(subsystem, "hidraw") == 0)
+ device = bus_find_hid_device(&hidraw_vtbl, dev);
+#ifdef HAS_PROPER_INPUT_HEADER
+ else if (strcmp(subsystem, "input") == 0)
+ device = bus_find_hid_device(&lnxev_vtbl, dev);
+#endif
It might be easier to just check both after each other, like:
device = bus_find_hid_device(...);
if (!device) device = bus_find_hid_device(...);
if (!device) return;
IoInvalidateDeviceRelations(device, RemovalRelations);
@@ -483,8 +577,18 @@ static void build_initial_deviceset(void)
return;
}
- if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0)
- WARN("Failed to add subsystem 'hidraw' to enumeration\n");
+ if (!disable_hidraw)
+ if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0)
+ WARN("Failed to add subsystem 'hidraw' to
enumeration\n");
+#ifdef HAS_PROPER_INPUT_HEADER
+ if (!disable_input)
+ {
+ if (udev_enumerate_add_match_subsystem(enumerate, "event") < 0)
+ WARN("Failed to add subsystem 'event' to enumeration\n");
Why do you add this match when results are not processed anywhere?
+ if
(udev_enumerate_add_match_subsystem(enumerate, "input") < 0)
+ WARN("Failed to add subsystem 'input' to enumeration\n");
+ }
+#endif
if (udev_enumerate_scan_devices(enumerate) < 0)
WARN("Enumeration scan failed\n");
@@ -509,6 +613,7 @@ static void build_initial_deviceset(void)
static struct udev_monitor *create_monitor(struct pollfd *pfd)
{
struct udev_monitor *monitor;
+ int systems = 0;
monitor = udev_monitor_new_from_netlink(udev_context, "udev");
if (!monitor)
@@ -517,8 +622,27 @@ static struct udev_monitor *create_monitor(struct pollfd *pfd)
return NULL;
}
- if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw",
NULL) < 0)
- WARN("Failed to add subsystem 'hidraw' to monitor\n");
+ if (!disable_hidraw)
+ {
+ if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw",
NULL) < 0)
+ WARN("Failed to add 'hidraw' subsystem to monitor\n");
+ else
+ systems++;
+ }
+#ifdef HAS_PROPER_INPUT_HEADER
+ if (!disable_input)
+ {
+ if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "input",
NULL) < 0)
+ WARN("Failed to add 'input' subsystem to monitor\n");
+ else
+ systems++;
+ }
+#endif
+ if (systems == 0)
+ {
+ WARN("No subsystems added to monitor\n");
+ goto error;
+ }
if (udev_monitor_enable_receiving(monitor) < 0)
goto error;
@@ -589,6 +713,10 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver,
UNICODE_STRING *registry
{
HANDLE events[2];
DWORD result;
+ static const WCHAR hidraw_disabledW[] =
{'D','i','s','a','b','l','e','
','h','i','d','r','a','w',0};
+ static const UNICODE_STRING hidraw_disabled = {sizeof(hidraw_disabledW) -
sizeof(WCHAR), sizeof(hidraw_disabledW), (WCHAR*)hidraw_disabledW};
+ static const WCHAR input_disabledW[] =
{'D','i','s','a','b','l','e','
','i','n','p','u','t',0};
+ static const UNICODE_STRING input_disabled = {sizeof(input_disabledW) -
sizeof(WCHAR), sizeof(input_disabledW), (WCHAR*)input_disabledW};
TRACE("(%p, %s)\n", driver, debugstr_w(registry_path->Buffer));
@@ -602,6 +730,16 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver,
UNICODE_STRING *registry
driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch;
driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch;
+ disable_hidraw = check_bus_option(registry_path, &hidraw_disabled);
+ if (disable_hidraw)
+ TRACE("UDEV hidraw devices disabled in registry\n");
+
+#ifdef HAS_PROPER_INPUT_HEADER
+ disable_input = check_bus_option(registry_path, &input_disabled);
+ if (disable_input)
+ TRACE("UDEV input devices disabled in registry\n");
+#endif
Does it hurt when both hidraw and input event devices are found? If yes, it
would be better to filter duplicates somewhere else, instead of using registry
keys.
+
if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL)))
goto error;
if (!(events[1] = CreateThread(NULL, 0, deviceloop_thread, events[0], 0, NULL)))
diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c
index 978540bb17..0388f0263c 100644
--- a/dlls/winebus.sys/main.c
+++ b/dlls/winebus.sys/main.c
@@ -625,6 +625,32 @@ void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD
length)
LeaveCriticalSection(&ext->report_cs);
}
+DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option)
+{
+ OBJECT_ATTRIBUTES attr;
+ HANDLE key;
+ DWORD output = 0;
+
+ InitializeObjectAttributes(&attr, registry_path, OBJ_CASE_INSENSITIVE |
OBJ_KERNEL_HANDLE, NULL, NULL);
+ if (NtOpenKey(&key, KEY_ALL_ACCESS, &attr) == STATUS_SUCCESS)
+ {
+ DWORD size;
+ char buffer[FIELD_OFFSET(KEY_VALUE_PARTIAL_INFORMATION, Data[sizeof(DWORD)])];
+
+ KEY_VALUE_PARTIAL_INFORMATION *info = (KEY_VALUE_PARTIAL_INFORMATION*)buffer;
+
+ if (NtQueryValueKey(key, option, KeyValuePartialInformation, info,
sizeof(buffer), &size) == STATUS_SUCCESS)
+ {
+ if (info->Type == REG_DWORD)
+ output = *(DWORD*)info->Data;
+ }
+
+ NtClose(key);
+ }
+
+ return output;
+}
+
NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path )
{
static const WCHAR udevW[] =
{'\\','D','r','i','v','e','r','\\','U','D','E','V',0};
27 Feb
27 Feb
4:22 p.m.
New subject: [PATCH 1/5] winebus.sys: Handle linux input event device via udev
Thanks for the review!
On 2/27/17 10:00 AM, Sebastian Lackner wrote:
Thanks I will look at that.
:)
either way, seems mostly stylistic
legacy code. I will remove it.
You can get duplicate devices, on being talking to the hidraw device and one to the linux
event device, but both are the same physical device. I have explored a little bit to try
to find if there is a good way to detect if a device is duplicated but it has not proven
to be immediately easy. Also then the question because which device is preferred if we
find a duplicate.
thanks again for the review, I am seeing that I have been testing with either or and not
both (hidraw/event) so you are finding very valid issues in the code I need to resolve.
I was starting to go cross-eyed looking at it all.
-aric
On 27.02.2017 16:09, Aric Stewart wrote:
Likely users using HID for gamepads and joysticks will end up this route by default
because all the permissions are automatically setup for us. Also then user will get to
leverage all the work done in the kernel to handle devices that do not behave well on the
HID side or which do not show up at all using hidraw (Xbox Controllers for example)
The dependency for udev is for common device discovery. I feel like I still want that
dependency unless we write our own device plug and unplug discovery system.
Signed-off-by: Aric Stewart
<aric(a)codeweavers.com>
---
dlls/winebus.sys/bus.h | 3 +
dlls/winebus.sys/bus_udev.c | 150 ++++++++++++++++++++++++++++++++++++++++++--
dlls/winebus.sys/main.c | 26 ++++++++
3 files changed, 173 insertions(+), 6 deletions(-)
I assume this is meant as a fallback, if we don't have access to hidraw, right?
If you plan to use this as a replacement for dinput, it would be better to
implement it without fixed dependency on udev.
0001-winebus.sys-Handle-linux-input-event-device-via-udev.txt
diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h
index effbd4afd0..fed35e8c1e 100644
--- a/dlls/winebus.sys/bus.h
+++ b/dlls/winebus.sys/bus.h
@@ -43,3 +43,6 @@ DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void
*platform_dev
void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN;
NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN;
void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length)
DECLSPEC_HIDDEN;
+
+/* General Bus Functions */
+DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option)
DECLSPEC_HIDDEN;
diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c
index 415bd3db08..2dcc7fb442 100644
--- a/dlls/winebus.sys/bus_udev.c
+++ b/dlls/winebus.sys/bus_udev.c
@@ -42,6 +42,14 @@
# include <sys/ioctl.h>
#endif
+#ifdef HAVE_LINUX_INPUT_H
+# include <linux/input.h>
+# undef SW_MAX
+# if defined(EVIOCGBIT) && defined(EV_ABS) && defined(BTN_PINKIE)
+# define HAS_PROPER_INPUT_HEADER
+# endif
+#endif
+
#define NONAMELESSUNION
#include "ntstatus.h"
@@ -65,11 +73,15 @@ WINE_DECLARE_DEBUG_CHANNEL(hid_report);
static struct udev *udev_context = NULL;
static DRIVER_OBJECT *udev_driver_obj = NULL;
+static DWORD disable_hidraw = 0;
+static DWORD disable_input = 0;
static const WCHAR hidraw_busidW[] =
{'H','I','D','R','A','W',0};
+static const WCHAR lnxev_busidW[] =
{'L','N','X','E','V',0};
#include "initguid.h"
DEFINE_GUID(GUID_DEVCLASS_HIDRAW,
0x3def44ad,0x242e,0x46e5,0x82,0x6d,0x70,0x72,0x13,0xf3,0xaa,0x81);
+DEFINE_GUID(GUID_DEVCLASS_LINUXEVENT,
0x1b932c0d,0xfea7,0x42cd,0x8e,0xaa,0x0e,0x48,0x79,0xb6,0x9e,0xaa);
struct platform_private
{
@@ -392,6 +404,52 @@ static const platform_vtbl hidraw_vtbl =
hidraw_set_feature_report,
};
+#ifdef HAS_PROPER_INPUT_HEADER
+
+static NTSTATUS lnxev_get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD
length, DWORD *out_length)
+{
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer,
DWORD length)
+{
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_begin_report_processing(DEVICE_OBJECT *device)
+{
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report,
DWORD length, ULONG_PTR *written)
+{
+ *written = 0;
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_get_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report,
DWORD length, ULONG_PTR *read)
+{
+ *read = 0;
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static NTSTATUS lnxev_set_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report,
DWORD length, ULONG_PTR *written)
+{
+ *written = 0;
+ return STATUS_NOT_IMPLEMENTED;
+}
+
+static const platform_vtbl lnxev_vtbl = {
+ compare_platform_device,
+ lnxev_get_reportdescriptor,
+ lnxev_get_string,
+ lnxev_begin_report_processing,
+ lnxev_set_output_report,
+ lnxev_get_feature_report,
+ lnxev_set_feature_report,
+};
+#endif
+
static void try_add_device(struct udev_device *dev)
{
DWORD vid = 0, pid = 0, version = 0;
@@ -400,6 +458,7 @@ static void try_add_device(struct udev_device *dev)
const char *subsystem;
const char *devnode;
WCHAR *serial = NULL;
+ const char* gamepad = NULL;
int fd;
if (!(devnode = udev_device_get_devnode(dev)))
@@ -419,6 +478,26 @@ static void try_add_device(struct udev_device *dev)
version = get_sysattr_dword(usbdev, "version", 10);
serial = get_sysattr_string(usbdev, "serial");
}
+#ifdef HAS_PROPER_INPUT_HEADER
+ else
+ {
+ struct input_id device_id = {0};
+ char device_uid[255] = {0};
+
+ if (ioctl(fd, EVIOCGID, &device_id) == -1)
+ WARN("ioctl(EVIOCGID) failed: %d %s\n", errno, strerror(errno));
+ if (ioctl(fd, EVIOCGUNIQ(254), device_uid) && device_uid[0])
+ MultiByteToWideChar(CP_UNIXCP, 0, device_uid, -1, serial, 255);
I don't think this will work as expected. serial is a NULL pointer, and the check
if ioctl() was successful is also done wrong. On error a value < 0 is returned.
+
+ gamepad = udev_device_get_property_value(dev, "ID_INPUT_JOYSTICK");
+ vid = device_id.vendor;
+ pid = device_id.product;
+ version = device_id.version;
+ }
+#else
+ else
+ WARN("Could not get device to query VID, PID, Version and Serial\n");
+#endif
TRACE("Found udev device %s (vid %04x, pid %04x, version %u, serial
%s)\n",
debugstr_a(devnode), vid, pid, version, debugstr_w(serial));
@@ -429,6 +508,12 @@ static void try_add_device(struct udev_device *dev)
device = bus_create_hid_device(udev_driver_obj, hidraw_busidW, vid, pid,
version, 0, serial, FALSE,
&GUID_DEVCLASS_HIDRAW, &hidraw_vtbl,
sizeof(struct platform_private));
}
+#ifdef HAS_PROPER_INPUT_HEADER
+ else if (strcmp(subsystem, "input") == 0)
+ {
+ device = bus_create_hid_device(udev_driver_obj, lnxev_busidW, vid, pid, version,
0, serial, (gamepad != NULL), &GUID_DEVCLASS_LINUXEVENT, &lnxev_vtbl,
sizeof(struct platform_private));
Some linebreaks wouldn't hurt. + }
+#endif
if (device)
{
@@ -448,8 +533,17 @@ static void try_add_device(struct udev_device *dev)
static void try_remove_device(struct udev_device *dev)
{
- DEVICE_OBJECT *device = bus_find_hid_device(&hidraw_vtbl, dev);
- struct platform_private *private;
+ DEVICE_OBJECT *device = NULL;
+ const char* subsystem;
+ struct platform_private* private;
+
+ subsystem = udev_device_get_subsystem(dev);
+ if (strcmp(subsystem, "hidraw") == 0)
+ device = bus_find_hid_device(&hidraw_vtbl, dev);
+#ifdef HAS_PROPER_INPUT_HEADER
+ else if (strcmp(subsystem, "input") == 0)
+ device = bus_find_hid_device(&lnxev_vtbl, dev);
+#endif
It might be easier to just check both after each other, like:
device = bus_find_hid_device(...);
if (!device) device = bus_find_hid_device(...);
if
(!device) return;
IoInvalidateDeviceRelations(device, RemovalRelations);
@@ -483,8 +577,18 @@ static void build_initial_deviceset(void)
return;
}
- if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0)
- WARN("Failed to add subsystem 'hidraw' to enumeration\n");
+ if (!disable_hidraw)
+ if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0)
+ WARN("Failed to add subsystem 'hidraw' to
enumeration\n");
+#ifdef HAS_PROPER_INPUT_HEADER
+ if (!disable_input)
+ {
+ if (udev_enumerate_add_match_subsystem(enumerate, "event") < 0)
+ WARN("Failed to add subsystem 'event' to enumeration\n");
Why do you add this match when results are not processed anywhere? + if
(udev_enumerate_add_match_subsystem(enumerate, "input") < 0)
+ WARN("Failed to add subsystem 'input' to enumeration\n");
+ }
+#endif
if (udev_enumerate_scan_devices(enumerate) < 0)
WARN("Enumeration scan failed\n");
@@ -509,6 +613,7 @@ static void build_initial_deviceset(void)
static struct udev_monitor *create_monitor(struct pollfd *pfd)
{
struct udev_monitor *monitor;
+ int systems = 0;
monitor = udev_monitor_new_from_netlink(udev_context, "udev");
if (!monitor)
@@ -517,8 +622,27 @@ static struct udev_monitor *create_monitor(struct pollfd *pfd)
return NULL;
}
- if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw",
NULL) < 0)
- WARN("Failed to add subsystem 'hidraw' to monitor\n");
+ if (!disable_hidraw)
+ {
+ if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw",
NULL) < 0)
+ WARN("Failed to add 'hidraw' subsystem to monitor\n");
+ else
+ systems++;
+ }
+#ifdef HAS_PROPER_INPUT_HEADER
+ if (!disable_input)
+ {
+ if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "input",
NULL) < 0)
+ WARN("Failed to add 'input' subsystem to monitor\n");
+ else
+ systems++;
+ }
+#endif
+ if (systems == 0)
+ {
+ WARN("No subsystems added to monitor\n");
+ goto error;
+ }
if (udev_monitor_enable_receiving(monitor) < 0)
goto error;
@@ -589,6 +713,10 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver,
UNICODE_STRING *registry
{
HANDLE events[2];
DWORD result;
+ static const WCHAR hidraw_disabledW[] =
{'D','i','s','a','b','l','e','
','h','i','d','r','a','w',0};
+ static const UNICODE_STRING hidraw_disabled = {sizeof(hidraw_disabledW) -
sizeof(WCHAR), sizeof(hidraw_disabledW), (WCHAR*)hidraw_disabledW};
+ static const WCHAR input_disabledW[] =
{'D','i','s','a','b','l','e','
','i','n','p','u','t',0};
+ static const UNICODE_STRING input_disabled = {sizeof(input_disabledW) -
sizeof(WCHAR), sizeof(input_disabledW), (WCHAR*)input_disabledW};
TRACE("(%p, %s)\n", driver, debugstr_w(registry_path->Buffer));
@@ -602,6 +730,16 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver,
UNICODE_STRING *registry
driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch;
driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch;
+ disable_hidraw = check_bus_option(registry_path, &hidraw_disabled);
+ if (disable_hidraw)
+ TRACE("UDEV hidraw devices disabled in registry\n");
+
+#ifdef HAS_PROPER_INPUT_HEADER
+ disable_input = check_bus_option(registry_path, &input_disabled);
+ if (disable_input)
+ TRACE("UDEV input devices disabled in registry\n");
+#endif
Does it hurt when both hidraw and input event devices are found? If yes, it
would be better to filter duplicates somewhere else, instead of using registry
keys.
+
if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL)))
goto error;
if (!(events[1] = CreateThread(NULL, 0, deviceloop_thread, events[0], 0, NULL)))
diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c
index 978540bb17..0388f0263c 100644
--- a/dlls/winebus.sys/main.c
+++ b/dlls/winebus.sys/main.c
@@ -625,6 +625,32 @@ void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD
length)
LeaveCriticalSection(&ext->report_cs);
}
+DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option)
+{
+ OBJECT_ATTRIBUTES attr;
+ HANDLE key;
+ DWORD output = 0;
+
+ InitializeObjectAttributes(&attr, registry_path, OBJ_CASE_INSENSITIVE |
OBJ_KERNEL_HANDLE, NULL, NULL);
+ if (NtOpenKey(&key, KEY_ALL_ACCESS, &attr) == STATUS_SUCCESS)
+ {
+ DWORD size;
+ char buffer[FIELD_OFFSET(KEY_VALUE_PARTIAL_INFORMATION, Data[sizeof(DWORD)])];
+
+ KEY_VALUE_PARTIAL_INFORMATION *info = (KEY_VALUE_PARTIAL_INFORMATION*)buffer;
+
+ if (NtQueryValueKey(key, option, KeyValuePartialInformation, info,
sizeof(buffer), &size) == STATUS_SUCCESS)
+ {
+ if (info->Type == REG_DWORD)
+ output = *(DWORD*)info->Data;
+ }
+
+ NtClose(key);
+ }
+
+ return output;
+}
+
NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path )
{
static const WCHAR udevW[] =
{'\\','D','r','i','v','e','r','\\','U','D','E','V',0};
4:52 p.m.
New subject: [PATCH 1/5] winebus.sys: Handle linux input event device via udev
On 27.02.2017 17:22, Aric Stewart wrote:
If it can cause problems it is probably still worth the effort. There
might also be systems where hidraw devices have access permissions by
default, and we don't want to get duplicated input events then.
On 2/27/17 10:00 AM, Sebastian Lackner wrote:
Are you sure that there are no systems out there where we have linux
events, but no udev? If yes the current solution is fine, but otherwise
we should at least implement a fallback method similar to how its done
in dinput (basically, just trying to open all devices in the range 0 ..
MAX_JOYDEV).
I assume this is meant as a fallback, if we
don't have access to
hidraw, right? If you plan to use this as a replacement for dinput,
it would be better to implement it without fixed dependency on
udev.
Likely users using HID for gamepads and joysticks will end up this
route by default because all the permissions are automatically setup
for us. Also then user will get to leverage all the work done in the
kernel to handle devices that do not behave well on the HID side or
which do not show up at all using hidraw (Xbox Controllers for
example)
The dependency for udev is for common device discovery. I feel like I
still want that dependency unless we write our own device plug and
unplug discovery system. Does it hurt
when both hidraw and input event devices are found? If
yes, it would be better to filter duplicates somewhere else,
instead of using registry keys.
You can get duplicate devices, on being talking to the hidraw device
and one to the linux event device, but both are the same physical
device. I have explored a little bit to try to find if there is a
good way to detect if a device is duplicated but it has not proven to
be immediately easy. Also then the question because which device is
preferred if we find a duplicate.
I think preferring hidraw might be slightly better. Some devices also
offer their own control utilities, and those would probably not work when
using linux input events. Nevertheless, its a matter of taste and we can
certainly discuss about that.
Best regards,
Sebastian
4:58 p.m.
New subject: [PATCH 1/5] winebus.sys: Handle linux input event device via udev
On 2/27/17 10:52 AM, Sebastian Lackner wrote:
If it can cause problems it is probably still worth the effort. There
might also be systems where hidraw devices have access permissions by
default, and we don't want to get duplicated input events then.
On 27.02.2017 17:22, Aric Stewart wrote:
I am not a linux expert but according to quick web research udev has been default since
about Linux kernel version 2.6.13. That is sometime around 2006. It became part of systemd
in 2012. So I am reasonably sure that any modern linux system will have it. I am not sure
how much we worry about pre 2006 systems.
-aric
On 2/27/17 10:00 AM, Sebastian Lackner wrote:
Are you sure that there are no systems out there where we have linux
events, but no udev? If yes the current solution is fine, but otherwise
we should at least implement a fallback method similar to how its done
in dinput (basically, just trying to open all devices in the range 0 ..
MAX_JOYDEV).
I assume this is meant as a fallback, if we
don't have access to
hidraw, right? If you plan to use this as a replacement for dinput,
it would be better to implement it without fixed dependency on
udev.
Likely users using HID for gamepads and joysticks will end up this
route by default because all the permissions are automatically setup
for us. Also then user will get to leverage all the work done in the
kernel to handle devices that do not behave well on the HID side or
which do not show up at all using hidraw (Xbox Controllers for
example)
The dependency for udev is for common device discovery. I feel like I
still want that dependency unless we write our own device plug and
unplug discovery system. Does it hurt when both hidraw and input event devices
are found? If
yes, it would be better to filter duplicates somewhere else,
instead of using registry keys.
You can get duplicate devices, on being talking to the hidraw device
and one to the linux event device, but both are the same physical
device. I have explored a little bit to try to find if there is a
good way to detect if a device is duplicated but it has not proven to
be immediately easy. Also then the question because which device is
preferred if we find a duplicate.
I think preferring hidraw might be slightly better. Some devices also
offer their own control utilities, and those would probably not work when
using linux input events. Nevertheless, its a matter of taste and we can
certainly discuss about that.
Best regards,
Sebastian
2 Mar
2 Mar
6:49 a.m.
New subject: [PATCH 1/5] winebus.sys: Handle linux input event device via udev
On Mon, Feb 27, 2017 at 8:52 AM, Sebastian Lackner
<sebastian(a)fds-team.de> wrote:
On 27.02.2017 17:22, Aric Stewart wrote:
If it can cause problems it is probably still worth the effort. There
might also be systems where hidraw devices have access permissions by
default, and we don't want to get duplicated input events then.
For some devices hidraw is a decent option, but in general my strong
recommendation is to stay the hell away from hidraw. The HID spec
itself has many holes (e.g. no good button mapping) and many vendors
implement device specific methods to do certain things (e.g. for
controller rumble, sensors, ..). The Linux kernel has a generic HID
parser, but there are so many fixups for devices, quirks and custom
handling. It is best to have the Linux kernel's evdev framework do the
work.
However the downside of evdev is that there is no 1:1 mapping between
HID devices and evdev devices. More and more devices are reported as
composite devices through evdev including Wacom devices, Wii
controllers, Sony controllers and more. It means for each HID device
there are multiple evdev nodes with the same product/vendor/version,
but different capabilities. Some separate out touch functionality,
others do it for sensors. Ideally this patch series would 'merge'
devices, maybe based on EVIOCGPHYS or something (or serial number).
On 2/27/17 10:00 AM, Sebastian Lackner wrote:
Are you sure that there are no systems out there where we have linux
events, but no udev? If yes the current solution is fine, but otherwise
we should at least implement a fallback method similar to how its done
in dinput (basically, just trying to open all devices in the range 0 ..
MAX_JOYDEV).
I assume this is meant as a fallback, if we
don't have access to
hidraw, right? If you plan to use this as a replacement for dinput,
it would be better to implement it without fixed dependency on
udev.
Likely users using HID for gamepads and joysticks will end up this
route by default because all the permissions are automatically setup
for us. Also then user will get to leverage all the work done in the
kernel to handle devices that do not behave well on the HID side or
which do not show up at all using hidraw (Xbox Controllers for
example)
The dependency for udev is for common device discovery. I feel like I
still want that dependency unless we write our own device plug and
unplug discovery system. Does it
hurt when both hidraw and input event devices are found? If
yes, it would be better to filter duplicates somewhere else,
instead of using registry keys.
You can get duplicate devices, on being talking to the hidraw device
and one to the linux event device, but both are the same physical
device. I have explored a little bit to try to find if there is a
good way to detect if a device is duplicated but it has not proven to
be immediately easy. Also then the question because which device is
preferred if we find a duplicate.
I think preferring hidraw might be slightly better. Some devices also
offer their own control utilities, and those would probably not work when
using linux input events. Nevertheless, its a matter of taste and we can
certainly discuss about that.
Best regards,
Sebastian
9 Mar
9 Mar
1:50 p.m.
New subject: [PATCH 1/5] winebus.sys: Handle linux input event device via udev
I have a few moments heads up from my current task so I can circle back here at least a
bit.
On 3/2/17 12:49 AM, Roderick Colenbrander wrote:
On Mon, Feb 27, 2017 at 8:52 AM, Sebastian Lackner
<sebastian(a)fds-team.de> wrote:
Well I am at first ecstatic that you did not read over these patches and just say
"NO". I got most of this from dinput code and my very beginning reading about
evdev. I am very much not a Linux or evdev developer.
On 27.02.2017 17:22, Aric Stewart wrote:
If it can cause problems it is probably still worth the effort. There
might also be systems where hidraw devices have access permissions by
default, and we don't want to get duplicated input events then.
On 2/27/17 10:00 AM, Sebastian Lackner wrote:
Are you sure that there are no systems out there where we have linux
events, but no udev? If yes the current solution is fine, but otherwise
we should at least implement a fallback method similar to how its done
in dinput (basically, just trying to open all devices in the range 0 ..
MAX_JOYDEV).
I assume this is meant as a fallback, if we
don't have access to
hidraw, right? If you plan to use this as a replacement for dinput,
it would be better to implement it without fixed dependency on
udev.
Likely users using HID for gamepads and joysticks will end up this
route by default because all the permissions are automatically setup
for us. Also then user will get to leverage all the work done in the
kernel to handle devices that do not behave well on the HID side or
which do not show up at all using hidraw (Xbox Controllers for
example)
The dependency for udev is for common device discovery. I feel like I
still want that dependency unless we write our own device plug and
unplug discovery system. Does it
hurt when both hidraw and input event devices are found? If
yes, it would be better to filter duplicates somewhere else,
instead of using registry keys.
You can get duplicate devices, on being talking to the hidraw device
and one to the linux event device, but both are the same physical
device. I have explored a little bit to try to find if there is a
good way to detect if a device is duplicated but it has not proven to
be immediately easy. Also then the question because which device is
preferred if we find a duplicate.
I think preferring hidraw might be slightly better. Some devices also
offer their own control utilities, and those would probably not work when
using linux input events. Nevertheless, its a matter of taste and we can
certainly discuss about that. For some devices hidraw is a decent option, but in
general my strong
recommendation is to stay the hell away from hidraw. The HID spec
itself has many holes (e.g. no good button mapping) and many vendors
implement device specific methods to do certain things (e.g. for
controller rumble, sensors, ..). The Linux kernel has a generic HID
parser, but there are so many fixups for devices, quirks and custom
handling. It is best to have the Linux kernel's evdev framework do the
work.
My vision is that for gamepad/joystick we will likely stick mostly with evdev. I agree
that seems to have the best support and I love being able to leverage all the kernel work.
Then for other hid devices, scales, golf simulators, whatever, they can be hidraw devices
as likely those applications already know they need to do whatever funky things they need
to do to communicate to those devices.
However the downside of evdev is that there is no 1:1
mapping between
HID devices and evdev devices. More and more devices are reported as
composite devices through evdev including Wacom devices, Wii
controllers, Sony controllers and more. It means for each HID device
there are multiple evdev nodes with the same product/vendor/version,
but different capabilities. Some separate out touch functionality,
others do it for sensors. Ideally this patch series would 'merge'
devices, maybe based on EVIOCGPHYS or something (or serial number).
Yeah, I have been aware of composite devices, but presently have done no work on handling
them. Mostly because my box of test devices have no composite devices in them and because
it gets really tricky really quickly. I have been designing with it in the back of my
mind so hopefully when we get demands for composite devices to work sanely it will be able
to be worked in without extensive re-writing.
Thanks!
-aric
>
> Best regards,
> Sebastian
>
>
2612
days inactive
2622
days old
5 comments
3 participants
participants (3)
-
Aric Stewart -
Roderick Colenbrander -
Sebastian Lackner