[PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Mario Limonciello]

[-- Attachment #1.1: Type: text/plain, Size: 616 bytes --]

Hi:

After the previous cleanup submitted to hid2hci, the code for finding
the sibling device changed significantly.  Although this works perfectly
fine when all the devices are on the bus, it doesn't for the recovery
from S3 situation.  The device is already off the bus by the time
hid2hci is called so the devpath is not found in the udev database.  The
easiest way to solve this is to use the parent's devpath instead, but
this does introduce a dependency on importing the env variable DEVPATH
from the parent.

Regards
-- 
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: s3_again.diff --]
[-- Type: text/x-patch; name="s3_again.diff", Size: 1967 bytes --]

Since the BT device is off the bus, trying to query it from the udev
database fails.  We need to send the parent's DEVPATH (unfortunately
by pulling in from the $env) and then go from there.

=== modified file 'extras/hid2hci/70-hid2hci.rules'
--- extras/hid2hci/70-hid2hci.rules	2009-07-24 16:06:22 +0000
+++ extras/hid2hci/70-hid2hci.rules	2009-07-27 21:03:40 +0000
@@ -20,8 +20,8 @@
 # Unfortunately the only event seen is the BT device disappearing, so the mouse
 # device needs to be chased down on the USB bus.
 ATTR{bDeviceClass}=="e0", ATTR{bDeviceSubClass}=="01", ATTR{bDeviceProtocol}=="01", \
-  ATTR{idVendor}=="413c", ATTR{bmAttributes}=="e0", \
-  ENV{REMOVE_CMD}="hid2hci --method=dell --devpath=%p --find-sibling-intf=03:01:02"
+  ATTR{idVendor}=="413c", ATTR{bmAttributes}=="e0", IMPORT{parent}="DEVPATH", \
+  ENV{REMOVE_CMD}="hid2hci --method=dell --devpath=$env{DEVPATH} --find-sibling-intf=03:01:02"
 
 # CSR devices
 ATTR{idVendor}=="0a12|0458|05ac", ATTR{idProduct}=="1000", RUN+="hid2hci --method=csr --devpath=%p"

=== modified file 'extras/hid2hci/hid2hci.c'
--- extras/hid2hci/hid2hci.c	2009-07-27 20:45:34 +0000
+++ extras/hid2hci/hid2hci.c	2009-07-27 21:03:40 +0000
@@ -210,11 +210,6 @@
 		return usb_open(dev);
 	}
 
-	/* find matching sibling of the current usb_device, they share the same hub */
-	udev_parent = udev_device_get_parent_with_subsystem_devtype(udev_dev, "usb", "usb_device");
-	if (udev_parent == NULL)
-		return NULL;
-
 	enumerate = udev_enumerate_new(udev);
 	if (enumerate == NULL)
 		return NULL;
@@ -222,7 +217,7 @@
 	udev_enumerate_add_match_subsystem(enumerate, "usb");
 
 	/* match all childs of the parent */
-	util_strscpyl(str, sizeof(str), udev_device_get_sysname(udev_parent), "*", NULL);
+	util_strscpyl(str, sizeof(str), udev_device_get_sysname(udev_dev), "*", NULL);
 	udev_enumerate_add_match_sysname(enumerate, str);
 
 	/* match the specified interface */


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Marcel Holtmann]

Hi Mario,

> After the previous cleanup submitted to hid2hci, the code for finding
> the sibling device changed significantly.  Although this works perfectly
> fine when all the devices are on the bus, it doesn't for the recovery
> from S3 situation.  The device is already off the bus by the time
> hid2hci is called so the devpath is not found in the udev database.  The
> easiest way to solve this is to use the parent's devpath instead, but
> this does introduce a dependency on importing the env variable DEVPATH
> from the parent.

at least this should be a private variable.

Regards

Marcel

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Mario Limonciello]

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

Hi Marcel:

Marcel Holtmann wrote:
> Hi Mario,
>   
>
> at least this should be a private variable.
>
>   
Looking through udev/udev-rules.c, I don't see how this is possible to
do with the current code.  import_parent_into_properties imports
directly into the database without letting you specify a different
variable to import into.
-- 
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Marcel Holtmann]

Hi Mario,

> > at least this should be a private variable.
> >
> >   
> Looking through udev/udev-rules.c, I don't see how this is possible to
> do with the current code.  import_parent_into_properties imports
> directly into the database without letting you specify a different
> variable to import into.

not sure if IMPORT supports .DEVPATH alike like ENV does. That is a
question for Kay, but just importing the parent DEVPATH looks really
dodgy to me.

Regards

Marcel

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Kay Sievers]

On Mon, Jul 27, 2009 at 23:19, Mario
Limonciello<mario_limonciello@dell.com> wrote:
> After the previous cleanup submitted to hid2hci, the code for finding
> the sibling device changed significantly.  Although this works perfectly
> fine when all the devices are on the bus, it doesn't for the recovery
> from S3 situation.  The device is already off the bus by the time
> hid2hci is called so the devpath is not found in the udev database.  The
> easiest way to solve this is to use the parent's devpath instead, but
> this does introduce a dependency on importing the env variable DEVPATH
> from the parent.

No, we can't do this. DEVPATH is the unique property of every device,
it can not be imported from a different device.

What does:
  ls -l /sys/bus/usb/devices/
print, and which device is the one the remove event hooks into and
which one is the one that needs to be found?

Kay

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Mario Limonciello]

[-- Attachment #1: Type: text/plain, Size: 3419 bytes --]

Hi Kay:

Kay Sievers wrote:
> On Mon, Jul 27, 2009 at 23:19, Mario
> Limonciello<mario_limonciello@dell.com> wrote:
>   
>
> No, we can't do this. DEVPATH is the unique property of every device,
> it can not be imported from a different device.
>   
OK.
> What does:
>   ls -l /sys/bus/usb/devices/
> print, and which device is the one the remove event hooks into and
> which one is the one that needs to be found?
>   
Here's that output:

    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 1-0:1.0 ->
    ../../../devices/pci0000:00/0000:00:04.1/usb1/1-0:1.0
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 2-0:1.0 ->
    ../../../devices/pci0000:00/0000:00:06.1/usb2/2-0:1.0
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 2-3 ->
    ../../../devices/pci0000:00/0000:00:06.1/usb2/2-3
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 2-3:1.0 ->
    ../../../devices/pci0000:00/0000:00:06.1/usb2/2-3/2-3:1.0
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 2-3:1.1 ->
    ../../../devices/pci0000:00/0000:00:06.1/usb2/2-3/2-3:1.1
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 3-0:1.0 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-0:1.0
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 3-4 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4
    lrwxrwxrwx 1 root root 0 2009-07-28 12:36 3-4.1 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.1
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 3-4:1.0 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4:1.0
    lrwxrwxrwx 1 root root 0 2009-07-28 12:36 3-4.1:1.0 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.1/3-4.1:1.0
    lrwxrwxrwx 1 root root 0 2009-07-28 12:36 3-4.2 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.2
    lrwxrwxrwx 1 root root 0 2009-07-28 12:36 3-4.2:1.0 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.2/3-4.2:1.0
    lrwxrwxrwx 1 root root 0 2009-07-28 12:36 3-4.3 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.3
    lrwxrwxrwx 1 root root 0 2009-07-28 12:36 3-4.3:1.0 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.3/3-4.3:1.0
    lrwxrwxrwx 1 root root 0 2009-07-28 12:36 3-4.3:1.1 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.3/3-4.3:1.1
    lrwxrwxrwx 1 root root 0 2009-07-28 12:36 3-4.3:1.2 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.3/3-4.3:1.2
    lrwxrwxrwx 1 root root 0 2009-07-28 12:36 3-4.3:1.3 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.3/3-4.3:1.3
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 4-0:1.0 ->
    ../../../devices/pci0000:00/0000:00:06.0/usb4/4-0:1.0
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 usb1 ->
    ../../../devices/pci0000:00/0000:00:04.1/usb1
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 usb2 ->
    ../../../devices/pci0000:00/0000:00:06.1/usb2
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 usb3 ->
    ../../../devices/pci0000:00/0000:00:04.0/usb3
    lrwxrwxrwx 1 root root 0 2009-07-28 12:35 usb4 ->
    ../../../devices/pci0000:00/0000:00:06.0/usb4

The device that is gone after S3 (the BT device), is:

    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.3


The parent device that I was calling with in that patch to find the
sibling was:

    ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4


-- 
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Kay Sievers]

On Tue, 2009-07-28 at 13:10 -0500, Mario Limonciello wrote:
> Kay Sievers wrote:
> > On Mon, Jul 27, 2009 at 23:19, Mario
> > Limonciello<mario_limonciello@dell.com> wrote:
> >
> > No, we can't do this. DEVPATH is the unique property of every device,
> > it can not be imported from a different device.

> The device that is gone after S3 (the BT device), is:
> 
>     ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4/3-4.3

> The parent device that I was calling with in that patch to find the
> sibling was:
> 
>     ../../../devices/pci0000:00/0000:00:04.0/usb3/3-4

That should work (in theory) with the current logic. I can do here:

$ UDEV_LOG=7 extras/hid2hci/hid2hci -p /devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.4 --methodfill --find-sibling-intf\x01:01:00
libudev: udev_device_new_from_syspath: device 0x60b0d0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.4'
libudev: udev_device_new_from_syspath: device 0x60b3d0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1'
libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1'
libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1:1.0'
libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3'
libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.0'
libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.0'
libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.0'
libudev: udev_device_new_from_syspath: device 0x6205e0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.1'
libudev: udev_device_new_from_syspath: device 0x6205e0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.1'
libudev: udev_device_new_from_syspath: device 0x6205e0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.4'
libudev: udev_device_new_from_syspath: device 0x6205e0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.4/2-1.4:1.0'
libudev: udev_device_new_from_syspath: device 0x60c5a0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.0'
libudev: udev_device_new_from_syspath: device 0x60c850 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3'
libusb couldn't open USB device /dev/bus/usb/002/006: Permission denied.

Where /dev/bus/usb/002/006 belongs to "2-1.3", which has a matching
interface and is the sibling from "2-1.4".

Can you try the same on the real hardware?

Thanks,
Kay

--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Mario Limonciello]

[-- Attachment #1: Type: text/plain, Size: 2633 bytes --]

Hi Kay:

Kay Sievers wrote:
> On Tue, 2009-07-28 at 13:10 -0500, Mario Limonciello wrote:
> That should work (in theory) with the current logic. I can do here:
>
> $ UDEV_LOG=7 extras/hid2hci/hid2hci -p /devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.4 --method=dell --find-sibling-intf=01:01:00
> libudev: udev_device_new_from_syspath: device 0x60b0d0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.4'
> libudev: udev_device_new_from_syspath: device 0x60b3d0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1'
> libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1'
> libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1:1.0'
> libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3'
> libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.0'
> libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.0'
> libudev: udev_device_new_from_syspath: device 0x61c580 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.0'
> libudev: udev_device_new_from_syspath: device 0x6205e0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.1'
> libudev: udev_device_new_from_syspath: device 0x6205e0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.1'
> libudev: udev_device_new_from_syspath: device 0x6205e0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.4'
> libudev: udev_device_new_from_syspath: device 0x6205e0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.4/2-1.4:1.0'
> libudev: udev_device_new_from_syspath: device 0x60c5a0 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3/2-1.3:1.0'
> libudev: udev_device_new_from_syspath: device 0x60c850 has devpath '/devices/pci0000:00/0000:00:1d.7/usb2/2-1/2-1.3'
> libusb couldn't open USB device /dev/bus/usb/002/006: Permission denied.
>
> Where /dev/bus/usb/002/006 belongs to "2-1.3", which has a matching
> interface and is the sibling from "2-1.4".
>
> Can you try the same on the real hardware?
>   
Unfortunately I did try on real hardware, and it didn't work.  That's
what pre-empted my patch to try to use the parent's DEVPATH instead.

The problem is that the device is already gone (and not in the udev
database) by the time you call the command.

-- 
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Kay Sievers]

On Fri, Jul 31, 2009 at 21:35, Mario Limonciello<mario_limonciello@dell.com> wrote:

> The problem is that the device is already gone (and not in the udev
> database) by the time you call the command.

I guess, we can get rid of all these hacks and the search code, by
tagging the mouse device we need to find, with:
..., ENV{HID2HCI_SWITCH}="1", RUN+="hid2hci --methodfill --devpath=%p"

and for the bluetooth device we just let it trigger the
tagged mouse device when it goes away, with:
..., ENV{REMOVE_CMD}="/sbin/udevadm trigger --action=change --subsystem-match=usb --property-match=HID2HCI_SWITCH=1"

That way the disappearing bluetooth device would just cause
a "change" event at the mouse device, which would trigger
the original rule again.
It would not need any further search logic, and the resume case
would run the same rules as the plug-in case.

Kay

--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Mario Limonciello]

[-- Attachment #1.1: Type: text/plain, Size: 1008 bytes --]

Hi Kay:

Kay Sievers wrote:
> On Fri, Jul 31, 2009 at 21:35, Mario Limonciello<mario_limonciello@dell.com> wrote:
>   
>
> I guess, we can get rid of all these hacks and the search code, by
> tagging the mouse device we need to find, with:
> ..., ENV{HID2HCI_SWITCH}="1", RUN+="hid2hci --method=dell --devpath=%p"
>
> and for the bluetooth device we just let it trigger the
> tagged mouse device when it goes away, with:
> ..., ENV{REMOVE_CMD}="/sbin/udevadm trigger --action=change --subsystem-match=usb --property-match=HID2HCI_SWITCH=1"
>
> That way the disappearing bluetooth device would just cause
> a "change" event at the mouse device, which would trigger
> the original rule again.
> It would not need any further search logic, and the resume case
> would run the same rules as the plug-in case.
>
>   
Thanks for the suggestion!   I've cleaned up per this advice and am
attaching a new patch.

-- 
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: remove_sibling.diff --]
[-- Type: text/x-patch; name="remove_sibling.diff", Size: 6151 bytes --]

=== modified file 'extras/hid2hci/70-hid2hci.rules'
--- extras/hid2hci/70-hid2hci.rules	2009-07-24 16:06:22 +0000
+++ extras/hid2hci/70-hid2hci.rules	2009-08-06 11:45:36 +0000
@@ -1,6 +1,6 @@
 # do not edit this file, it will be overwritten on update
 
-ACTION!="add", GOTO="hid2hci_end"
+ACTION!="add|change", GOTO="hid2hci_end"
 SUBSYSTEM!="usb", GOTO="hid2hci_end"
 
 # Variety of Dell Bluetooth devices - match on a mouse device that is
@@ -8,7 +8,7 @@
 # Known supported devices: 413c:8154, 413c:8158, 413c:8162
 ATTR{bInterfaceClass}=="03", ATTR{bInterfaceSubClass}=="01", ATTR{bInterfaceProtocol}=="02", \
   ATTRS{bDeviceClass}=="00", ATTRS{idVendor}=="413c", ATTRS{bmAttributes}=="e0", \
-  RUN+="hid2hci --method=dell --devpath=%p"
+  RUN+="hid2hci --method=dell --devpath=%p", ENV{HID2HCI_SWITCH}="1"
 
 # Logitech devices (hidraw)
 KERNEL=="hidraw*", ATTRS{idVendor}=="046d", ATTRS{idProduct}=="c70[345abce]|c71[34bc]", \
@@ -21,7 +21,7 @@
 # device needs to be chased down on the USB bus.
 ATTR{bDeviceClass}=="e0", ATTR{bDeviceSubClass}=="01", ATTR{bDeviceProtocol}=="01", \
   ATTR{idVendor}=="413c", ATTR{bmAttributes}=="e0", \
-  ENV{REMOVE_CMD}="hid2hci --method=dell --devpath=%p --find-sibling-intf=03:01:02"
+  ENV{REMOVE_CMD}="/sbin/udevadm trigger --action=change --subsystem-match=usb --property-match=HID2HCI_SWITCH=1"
 
 # CSR devices
 ATTR{idVendor}=="0a12|0458|05ac", ATTR{idProduct}=="1000", RUN+="hid2hci --method=csr --devpath=%p"

=== modified file 'extras/hid2hci/hid2hci.c'
--- extras/hid2hci/hid2hci.c	2009-07-24 16:06:22 +0000
+++ extras/hid2hci/hid2hci.c	2009-08-05 08:07:51 +0000
@@ -191,70 +191,14 @@
 	return NULL;
 }
 
-static struct usb_dev_handle *find_device(struct udev_device *udev_dev, const char *sibling_intf)
+static struct usb_dev_handle *find_device(struct udev_device *udev_dev)
 {
-	struct udev *udev = udev_device_get_udev(udev_dev);
 	struct usb_device *dev;
-	struct udev_device *udev_parent;
-	char str[UTIL_NAME_SIZE];
-	struct udev_enumerate *enumerate;
-	struct udev_list_entry *entry;
-	struct usb_dev_handle *handle = NULL;
-
-	if (sibling_intf == NULL) {
-		dev = usb_device_open_from_udev(udev_dev);
-		if (dev == NULL)
-			return NULL;
-		return usb_open(dev);
-	}
-
-	/* find matching sibling of the current usb_device, they share the same hub */
-	udev_parent = udev_device_get_parent_with_subsystem_devtype(udev_dev, "usb", "usb_device");
-	if (udev_parent == NULL)
-		return NULL;
-
-	enumerate = udev_enumerate_new(udev);
-	if (enumerate == NULL)
-		return NULL;
-
-	udev_enumerate_add_match_subsystem(enumerate, "usb");
-
-	/* match all childs of the parent */
-	util_strscpyl(str, sizeof(str), udev_device_get_sysname(udev_parent), "*", NULL);
-	udev_enumerate_add_match_sysname(enumerate, str);
-
-	/* match the specified interface */
-	util_strscpy(str, sizeof(str), sibling_intf);
-	str[2] = '\0';
-	str[5] = '\0';
-	str[8] = '\0';
-	if (strcmp(str, "-1") != 0)
-		udev_enumerate_add_match_sysattr(enumerate, "bInterfaceClass", str);
-	if (strcmp(&str[3], "-1") != 0)
-		udev_enumerate_add_match_sysattr(enumerate, "bInterfaceSubClass", &str[3]);
-	if (strcmp(&str[6], "-1") != 0)
-		udev_enumerate_add_match_sysattr(enumerate, "bInterfaceProtocol", &str[6]);
-
-	udev_enumerate_scan_devices(enumerate);
-	udev_list_entry_foreach(entry, udev_enumerate_get_list_entry(enumerate)) {
-		struct udev_device *udev_device;
-
-		udev_device = udev_device_new_from_syspath(udev, udev_list_entry_get_name(entry));
-		if (udev_device != NULL) {
-			/* get the usb_device of the usb_interface we matched */
-			udev_parent = udev_device_get_parent_with_subsystem_devtype(udev_device, "usb", "usb_device");
-			if (udev_parent == NULL)
-				continue;
-			/* only look at the first matching device */
-			dev = usb_device_open_from_udev(udev_parent);
-			if (dev != NULL)
-				handle = usb_open(dev);
-			udev_device_unref(udev_device);
-			break;
-		}
-}
-	udev_enumerate_unref(enumerate);
-	return handle;
+
+	dev = usb_device_open_from_udev(udev_dev);
+	if (dev == NULL)
+		return NULL;
+	return usb_open(dev);
 }
 
 static void usage(const char *error)
@@ -268,7 +212,6 @@
 		"  --mode=               mode to switch to [hid|hci] (default hci)\n"
 		"  --devpath=            sys device path\n"
 		"  --method=             method to use to switch [csr|logitech-hid|dell]\n"
-		"  --find-sibling-intf=  find the sibling device with 00:00:00 (class:subclass:prot)\n"
 		"  --help\n\n");
 }
 
@@ -279,7 +222,6 @@
 		{ "mode", required_argument, NULL, 'm' },
 		{ "devpath", required_argument, NULL, 'p' },
 		{ "method", required_argument, NULL, 'M' },
-		{ "find-sibling-intf", required_argument, NULL, 'I' },
 		{ }
 	};
 	enum method {
@@ -294,14 +236,13 @@
 	int (*usb_switch)(struct usb_dev_handle *dev, enum mode mode) = NULL;
 	enum mode mode = HCI;
 	const char *devpath = NULL;
-	const char *sibling_intf = NULL;
 	int err = -1;
 	int rc = 1;
 
 	for (;;) {
 		int option;
 
-		option = getopt_long(argc, argv, "m:p:M:I:qh", options, NULL);
+		option = getopt_long(argc, argv, "m:p:M:qh", options, NULL);
 		if (option == -1)
 			break;
 
@@ -333,9 +274,6 @@
 				exit(1);
 			}
 			break;
-		case 'I':
-			sibling_intf = optarg;
-			break;
 		case 'h':
 			usage(NULL);
 		default:
@@ -377,10 +315,10 @@
 			}
 		}
 
-		handle = find_device(dev, sibling_intf);
+		handle = find_device(dev);
 		if (handle == NULL) {
-			fprintf(stderr, "error: unable to handle '%s' (intf=%s)\n",
-				udev_device_get_syspath(dev), sibling_intf);
+			fprintf(stderr, "error: unable to handle '%s'\n",
+				udev_device_get_syspath(dev));
 			goto exit;
 		}
 		err = usb_switch(handle, mode);
@@ -402,8 +340,8 @@
 	}
 
 	if (err < 0)
-		fprintf(stderr, "error: switching device '%s' (intf=%s) failed.\n",
-			udev_device_get_syspath(udev_dev), sibling_intf);
+		fprintf(stderr, "error: switching device '%s' failed.\n",
+			udev_device_get_syspath(udev_dev));
 exit:
 	udev_device_unref(udev_dev);
 	udev_unref(udev);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Kay Sievers]

On Fri, Aug 7, 2009 at 00:03, Mario
Limonciello<mario_limonciello@dell.com> wrote:

>> I guess, we can get rid of all these hacks and the search code, by
>> tagging the mouse device we need to find, with:
>> ..., ENV{HID2HCI_SWITCH}="1", RUN+="hid2hci --methodÞll --devpath=%p"

> Thanks for the suggestion!   I've cleaned up per this advice and am
> attaching a new patch.

Great, that this works. Applied. Now lets wait for a reply, if we can
get libusb fixed, otherwise we will just replace the dumb scanning
code with our own thing.

Thanks,
Kay

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Marcel Holtmann]

Hi Kay,

> >> I guess, we can get rid of all these hacks and the search code, by
> >> tagging the mouse device we need to find, with:
> >> ..., ENV{HID2HCI_SWITCH}="1", RUN+="hid2hci --methodÞll --devpath=%p"
> 
> > Thanks for the suggestion!   I've cleaned up per this advice and am
> > attaching a new patch.
> 
> Great, that this works. Applied. Now lets wait for a reply, if we can
> get libusb fixed, otherwise we will just replace the dumb scanning
> code with our own thing.

actually you might wanna do that anyway. The whole point to put hid2hci
into udev was that we can have it run really early at boot when /usr is
not mounted yet (I don't buy that /usr argument, but whatever). With
libusb installed under /usr/lib that goal is moot again ;)

Regards

Marcel

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Kay Sievers]

Hey Marcel,

On Fri, Aug 7, 2009 at 02:44, Marcel Holtmann<marcel@holtmann.org> wrote:
>> >> I guess, we can get rid of all these hacks and the search code, by
>> >> tagging the mouse device we need to find, with:
>> >> ..., ENV{HID2HCI_SWITCH}="1", RUN+="hid2hci --methodÞll --devpath=%p"
>>
>> > Thanks for the suggestion!   I've cleaned up per this advice and am
>> > attaching a new patch.
>>
>> Great, that this works. Applied. Now lets wait for a reply, if we can
>> get libusb fixed, otherwise we will just replace the dumb scanning
>> code with our own thing.
>
> actually you might wanna do that anyway. The whole point to put hid2hci
> into udev was that we can have it run really early at boot when /usr is
> not mounted yet (I don't buy that /usr argument, but whatever). With
> libusb installed under /usr/lib that goal is moot again ;)

Yeah, that's a valid point.

We could move it to / though, if it will ever get fixed. Oh well, the
/usr on a different filesystem thing -- many things break in
not-so-interesting ways and this is just one issue on top. :)

Kay

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Marco d'Itri]

On Aug 07, Kay Sievers <kay.sievers@vrfy.org> wrote:

> We could move it to / though, if it will ever get fixed. Oh well, the
> /usr on a different filesystem thing -- many things break in
> not-so-interesting ways and this is just one issue on top. :)
Not so many, it is expected to work on Debian and Ubuntu systems.

-- 
ciao,
Marco

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Kay Sievers]

On Fri, Aug 7, 2009 at 03:28, Marco d'Itri<md@linux.it> wrote:
> On Aug 07, Kay Sievers <kay.sievers@vrfy.org> wrote:
>
>> We could move it to / though, if it will ever get fixed. Oh well, the
>> /usr on a different filesystem thing -- many things break in
>> not-so-interesting ways and this is just one issue on top. :)
> Not so many, it is expected to work on Debian and Ubuntu systems.

Oh, I guess you didn't try. :) Really, it does not work for many
non-trivial things.

The only sane option is a read-only rootfs. Having /usr somewhere else
is just a pretty useless exercise, and many people gave up supporting
that in their software, and it will not get easier in the future.

Honestly the entire split of / and /usr, like we usually do, is just
plain stupid in the first place. It might be ok if we would put the
desktop stuff in /usr, but spreading random commandline tools and
libraries around causes nothing but trouble for no good reason.

Thanks,
Kay

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Marco d'Itri]

On Aug 07, Kay Sievers <kay.sievers@vrfy.org> wrote:

> Oh, I guess you didn't try. :) Really, it does not work for many
> non-trivial things.
Our users do, and regularly report bugs which are then fixed.

-- 
ciao,
Marco

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Karl O. Pinc]

On 08/06/2009 08:47:10 PM, Kay Sievers wrote:
> Honestly the entire split of / and /usr, like we usually do, is just
> plain stupid in the first place. It might be ok if we would put the
> desktop stuff in /usr, but spreading random commandline tools and
> libraries around causes nothing but trouble for no good reason.

Theory says that /usr can be mounted read-only, which could
improve system security for those binaries that live there
or have other advantages regarding media used, storage sharing, etc.
http://www.pathname.com/fhs/pub/fhs-2.3.html#THEUSRHIERARCHY

The other side of the argument is that / is supposed to be
able to be kept small.
http://www.pathname.com/fhs/pub/fhs-2.3.html#THEROOTFILESYSTEM

As storage gets increasingly cheaper relative
to the rest of the hardware it seems most of the
arguments for a separate /usr get weaker, but not all.
I suppose if you have a compelling argument for re-balancing
what goes where you could convince the folks on
freestandards-fhs-discuss@lists.sourceforge.net.
(At least that's where such talk went last I paid attention.)

Regards,

Karl <kop@meme.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

Re: [PATCH 2/2] hid2hci: when recovering from S3 use parent devpath

[Marcel Holtmann]

Hi Karl,

> > Honestly the entire split of / and /usr, like we usually do, is just
> > plain stupid in the first place. It might be ok if we would put the
> > desktop stuff in /usr, but spreading random commandline tools and
> > libraries around causes nothing but trouble for no good reason.
> 
> Theory says that /usr can be mounted read-only, which could
> improve system security for those binaries that live there
> or have other advantages regarding media used, storage sharing, etc.
> http://www.pathname.com/fhs/pub/fhs-2.3.html#THEUSRHIERARCHY

the security argument is non-sense. Only SELinux can really protect you
here. Once you are root, you just re-mount it read-write.

> The other side of the argument is that / is supposed to be
> able to be kept small.
> http://www.pathname.com/fhs/pub/fhs-2.3.html#THEROOTFILESYSTEM

As Kay mentioned, that is fine for the UI components, but mostly all
system components (even that are not needed for boot) should be just be
living in /bin and /sbin. And so even the separation between bin and
sbin is kinda pointless nowadays.

It is Unix legacy and at some point Linux has to cut some of its roots
to get ready for the future. Similar to what MacOS X did.

Regards

Marcel