Hi Kay: Kay Sievers wrote: > On Tue, Jul 14, 2009 at 20:46, Marcel Holtmann<marcel@xxxxxxxxxxxx> wrote: > > > That looks crazy. What are you trying to solve here? > My apologies. I'll split that up into some more readable for loops. I was just trying to walk through the bus to find the appropriate usb_device to match and return. > We also don't want these weird multiline rules with on match per line, > and the use of properties merged by earlier rules. I complained too > many times about this now. > I'll switch over to one line. I had thought it was more readable as multiline. Since the rule is referring to the parent's information, I wasn't sure that this was doable without importing the properties of the parent's earlier rules. > That whole thing get pretty annoying, it does not seem to be ready to > be shipped with udev. > Please properly fix _and_ test the stuff now. > Your earlier version already broke random keyboards and hubs, because > you obviously did not test it properly. If you don't provide a sane > solution now, I'm going to remove it from udev, and you need to create > your own package. > The utility itself is stable. The previous error with the matching against hubs within docking stations was something I didn't anticipate matching up the chain, indeed my mistake. It was corrected by being more specific on things to match on, which is what I am attempting again with this second rule. Attached is an updated patch. Again, i'm having to rely on earlier rules for parent device information. If there is a better way to do this, can you please advise rather than be rash about removing this utility from udev? There shouldn't be any worry for mismatching devices this time. The new rule looks specifically for Dell Wireless Class Controllers, with a Bluetooth protocol not matching further up the chain. I broke up the for loops by relying on find_device instead. My code is quite similar to the example that is on the libusb documentation for matching a property: http://libusb.sourceforge.net/doc/examples-code.html I'm not really sure of any way to make it more readable other than adding superfluous spacing and comments as it's a very basic example. Thanks, -- Mario Limonciello *Dell | Linux Engineering* mario_limonciello@xxxxxxxx
=== modified file 'extras/hid2hci/70-hid2hci.rules'
--- extras/hid2hci/70-hid2hci.rules 2009-06-26 06:17:23 +0000
+++ extras/hid2hci/70-hid2hci.rules 2009-07-14 20:56:36 +0000
@@ -11,6 +11,11 @@
ATTR{bInterfaceClass}=="03", ATTR{bInterfaceSubClass}=="01", ATTR{bInterfaceProtocol}=="02", ATTRS{bDeviceClass}=="00", ATTRS{idVendor}=="413c", ATTRS{bmAttributes}=="e0", \
RUN+="hid2hci --method dell -v $attr{idVendor} -p $attr{idProduct} --mode hci"
+# When a Dell device recovers from S3, the mouse child needs to be repoked
+# 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", IMPORT{parent}="ID_*", ENV{REMOVE_CMD}="hid2hci --method dell -v $env{ID_VENDOR_ID} -p $env{ID_MODEL_ID} --mode hci -s 02"
+
ENV{DEVTYPE}!="usb_device", GOTO="hid2hci_end"
# Logitech devices
=== modified file 'extras/hid2hci/hid2hci.c'
--- extras/hid2hci/hid2hci.c 2009-06-16 17:30:22 +0000
+++ extras/hid2hci/hid2hci.c 2009-07-14 20:56:39 +0000
@@ -271,6 +271,23 @@
return 0;
}
+static int find_resuscitated_device(struct device_info* devinfo, uint8_t bInterfaceProtocol)
+{
+ int i,j,k,l;
+
+ /* Using the base device, attempt to find the child with the
+ * matching bInterfaceProtocol */
+ for (i = 0; i < devinfo->dev->num_children; i++)
+ for (j = 0; j < devinfo->dev->children[i]->descriptor.bNumConfigurations; j++)
+ for (k = 0; k < devinfo->dev->children[i]->config[j].bNumInterfaces; k++)
+ for (l = 0; l < devinfo->dev->children[i]->config[j].interface[k].num_altsetting; l++)
+ if (devinfo->dev->children[i]->config[j].interface[k].altsetting[l].bInterfaceProtocol == bInterfaceProtocol) {
+ devinfo->dev = devinfo->dev->children[i];
+ return 1;
+ }
+ return 0;
+}
+
static void usage(char* error)
{
if (error)
@@ -289,6 +306,7 @@
"\t-v, --vendor= Vendor ID to act upon\n"
"\t-p, --product= Product ID to act upon\n"
"\t-m, --method= Method to use to switch [csr, logitech, dell]\n"
+ "\t-s, --resuscitate= Find the child device with this bInterfaceProtocol to run on \n"
"\n");
if (error)
exit(1);
@@ -301,6 +319,7 @@
{ "vendor", required_argument, 0, 'v' },
{ "product", required_argument, 0, 'p' },
{ "method", required_argument, 0, 'm' },
+ { "resuscitate",required_argument, 0, 's' },
{ 0, 0, 0, 0 }
};
@@ -309,8 +328,9 @@
struct device_info dev = { NULL, HCI, 0, 0 };
int opt, quiet = 0;
int (*method)(struct device_info *dev) = NULL;
+ uint8_t resuscitate = 0;
- while ((opt = getopt_long(argc, argv, "+r:v:p:m:qh", main_options, NULL)) != -1) {
+ while ((opt = getopt_long(argc, argv, "+s:r:v:p:m:qh", main_options, NULL)) != -1) {
switch (opt) {
case 'r':
if (optarg && !strcmp(optarg, "hid"))
@@ -339,6 +359,9 @@
case 'q':
quiet = 1;
break;
+ case 's':
+ sscanf(optarg, "%2hx", (short unsigned int*) &resuscitate);
+ break;
case 'h':
usage(NULL);
default:
@@ -362,6 +385,13 @@
exit(1);
}
+ if (resuscitate && !find_resuscitated_device(&dev, resuscitate)) {
+ if (!quiet)
+ fprintf(stderr, "Device %04x:%04x was unable to resucitate any child devices.\n",
+ dev.vendor,dev.product);
+ exit(1);
+ }
+
if (!quiet)
printf("Attempting to switch device %04x:%04x to %s mode ",
dev.vendor, dev.product, dev.mode ? "HID" : "HCI");
Attachment:
signature.asc
Description: OpenPGP digital signature
