[PATCH 0/1] MR6268: winebus.sys: Add devtype parameter to get_device_subsystem_info().
Narrow the search for a parent subsystem to a specific devtype. For the usb subsystem in particular, only the "usb_device" devtype includes all the necessary attributes, so no need to iterate through the usb interfaces. Additionally fixes an issue with the device revision (bcdDevice) not being extracted from the usb subsystem, causing the bcdHID to be extracted in error. Also fixes an issue with the product name including the manufacturer in the same string. Both attributes are correctly extracted from the usb subsystem. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54838 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56987 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6268
From: Ivo Ivanov <logos128(a)gmail.com> Narrow the search for a parent subsystem to a specific devtype. For the usb subsystem in particular, only the "usb_device" devtype includes all the necessary attributes, so no need to iterate through the usb interfaces. Additionally fixes an issue with the device revision (bcdDevice) not being extracted from the usb subsystem, causing the bcdHID to be extracted in error. Also fixes an issue with the product name including the manufacturer in the same string. Both attributes are correctly extracted from the usb subsystem. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54838 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56987 --- dlls/winebus.sys/bus_udev.c | 49 ++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index 6ec0fbd356f..71988f815dd 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -1138,14 +1138,15 @@ static const struct hid_device_vtbl lnxev_device_vtbl = }; #endif /* HAS_PROPER_INPUT_HEADER */ -static void get_device_subsystem_info(struct udev_device *dev, char const *subsystem, struct device_desc *desc, - int *bus) +static void get_device_subsystem_info(struct udev_device *dev, const char *subsystem, const char *devtype, + struct device_desc *desc, int *bus_type) { struct udev_device *parent = NULL; const char *ptr, *next, *tmp; char buffer[MAX_PATH]; + UINT version; - if (!(parent = udev_device_get_parent_with_subsystem_devtype(dev, subsystem, NULL))) return; + if (!(parent = udev_device_get_parent_with_subsystem_devtype(dev, subsystem, devtype))) return; if ((next = udev_device_get_sysattr_value(parent, "uevent"))) { @@ -1174,28 +1175,36 @@ static void get_device_subsystem_info(struct udev_device *dev, char const *subsy } if (!strncmp(ptr, "HID_ID=", 7)) { - if (*bus || desc->vid || desc->pid) continue; - sscanf(ptr, "HID_ID=%x:%x:%x\n", bus, &desc->vid, &desc->pid); + if (*bus_type || desc->vid || desc->pid) continue; + sscanf(ptr, "HID_ID=%x:%x:%x\n", bus_type, &desc->vid, &desc->pid); } - if (!strncmp(ptr, "PRODUCT=", 8) && *bus != BUS_BLUETOOTH) + if (!strncmp(ptr, "PRODUCT=", 8)) { if (desc->version) continue; if (!strcmp(subsystem, "usb")) - sscanf(ptr, "PRODUCT=%x/%x/%x\n", &desc->vid, &desc->pid, &desc->version); - else - sscanf(ptr, "PRODUCT=%x/%x/%x/%x\n", bus, &desc->vid, &desc->pid, &desc->version); + { + if (*bus_type != BUS_BLUETOOTH) + sscanf(ptr, "PRODUCT=%x/%x/%x\n", &desc->vid, &desc->pid, &desc->version); + } + else if (sscanf(ptr, "PRODUCT=%x/%x/%x/%x\n", bus_type, &desc->vid, &desc->pid, &version) == 4 && + *bus_type == BUS_BLUETOOTH) + desc->version = version; } } } - if (!desc->manufacturer[0] && (tmp = udev_device_get_sysattr_value(dev, "manufacturer"))) - ntdll_umbstowcs(tmp, strlen(tmp) + 1, desc->manufacturer, ARRAY_SIZE(desc->manufacturer)); + if (!strcmp(subsystem, "usb") && *bus_type != BUS_BLUETOOTH) + { + if ((tmp = udev_device_get_sysattr_value(parent, "manufacturer"))) + ntdll_umbstowcs(tmp, strlen(tmp) + 1, desc->manufacturer, ARRAY_SIZE(desc->manufacturer)); + + if ((tmp = udev_device_get_sysattr_value(parent, "product"))) + ntdll_umbstowcs(tmp, strlen(tmp) + 1, desc->product, ARRAY_SIZE(desc->product)); - if (!desc->product[0] && (tmp = udev_device_get_sysattr_value(dev, "product"))) - ntdll_umbstowcs(tmp, strlen(tmp) + 1, desc->product, ARRAY_SIZE(desc->product)); + if ((tmp = udev_device_get_sysattr_value(parent, "serial"))) + ntdll_umbstowcs(tmp, strlen(tmp) + 1, desc->serialnumber, ARRAY_SIZE(desc->serialnumber)); - if (!desc->serialnumber[0] && (tmp = udev_device_get_sysattr_value(dev, "serial"))) - ntdll_umbstowcs(tmp, strlen(tmp) + 1, desc->serialnumber, ARRAY_SIZE(desc->serialnumber)); + } } static void udev_add_device(struct udev_device *dev, int fd) @@ -1207,7 +1216,7 @@ static void udev_add_device(struct udev_device *dev, int fd) struct base_device *impl; const char *subsystem; const char *devnode; - int bus = 0; + int bus_type = 0; if (!(devnode = udev_device_get_devnode(dev))) { @@ -1223,10 +1232,10 @@ static void udev_add_device(struct udev_device *dev, int fd) TRACE("udev %s syspath %s\n", debugstr_a(devnode), udev_device_get_syspath(dev)); - get_device_subsystem_info(dev, "hid", &desc, &bus); - get_device_subsystem_info(dev, "input", &desc, &bus); - get_device_subsystem_info(dev, "usb", &desc, &bus); - if (bus == BUS_BLUETOOTH) desc.is_bluetooth = TRUE; + get_device_subsystem_info(dev, "hid", NULL, &desc, &bus_type); + get_device_subsystem_info(dev, "input", NULL, &desc, &bus_type); + get_device_subsystem_info(dev, "usb", "usb_device", &desc, &bus_type); + if (bus_type == BUS_BLUETOOTH) desc.is_bluetooth = TRUE; subsystem = udev_device_get_subsystem(dev); if (!strcmp(subsystem, "hidraw")) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6268
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147717 Your paranoid android. === debian11b (64 bit WoW report) === ddraw: ddraw4.c:3969: Test failed: Expected message 0x5, but didn't receive it.
Thanks, looks good. I've opened (https://gitlab.winehq.org/wine/wine/-/merge_requests/6276) with the changes split and a couple of adjustments to some make future changes easier. I think we could parse additional information from the "input" subsystem, as well as (possibly) use the PHYS strings to implement device container IDs (https://gitlab.winehq.org/wine/wine/-/merge_requests/535) which are needed for some applications to match between controllers and audio devices with the Dualshock controllers. As far as I can see pulseaudio exposes the same string in its device names. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6268#note_78640
On Wed Aug 14 13:45:25 2024 +0000, Rémi Bernon wrote:
Thanks, looks good. I've opened (https://gitlab.winehq.org/wine/wine/-/merge_requests/6276) with the changes split and a couple of adjustments to some make future changes easier. I think we could parse additional information from the "input" subsystem, as well as (possibly) use the PHYS strings to implement device container IDs (https://gitlab.winehq.org/wine/wine/-/merge_requests/535) which are needed for some applications to match between controllers and audio devices with the Dualshock controllers. As far as I can see pulseaudio exposes the same string in its device names. I did this as part of a larger patchset implementing IOCTL_HID_GET_INDEXED_STRING in hidclass.sys -> winebus.sys. Have been using it for a year or so without any problems. Still plan to push an MR for the remaining part when time permits. Just saw the bug reports about the product/manufacturer string issues and decided to push this patch separately.
Many apps/games need these strings to match exactly (esp. product name), to properly recognize the device and apply predefined configuration, etc. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6268#note_78664
On Wed Aug 14 13:45:25 2024 +0000, Ivo Ivanov wrote:
I did this as part of a larger patchset implementing IOCTL_HID_GET_INDEXED_STRING in hidclass.sys -> winebus.sys. Have been using it for a year or so without any problems. Still plan to push an MR for the remaining part when time permits. Just saw the bug reports about the product/manufacturer string issues and decided to push this patch separately. Many apps/games need these strings to match exactly (esp. product name), to properly recognize the device and apply predefined configuration, etc. Fwiw in several cases the names simply do not match between the Linux driver and the Windows drivers. We forcefully override the device names for such cases in hidclass.sys.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6268#note_78665
On Wed Aug 14 13:52:34 2024 +0000, Rémi Bernon wrote:
Fwiw in several cases the names simply do not match between the Linux driver and the Windows drivers. We forcefully override the device names for such cases in hidclass.sys. Thanks. Didn't know that.
I use the 'hidraw' backend for all my devices, incl. force feedback wheel, shifter, pedals, joysticks, etc., and hopefully this way the strings match better between Linux and Windows. So far didn't spot any issues. The devices get properly recognized/configured by apps and games. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6268#note_78685
This merge request was closed by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6268
participants (4)
-
Ivo Ivanov -
Ivo Ivanov (＠logos128)
-
Marvin -
Rémi Bernon