Skip to contentSkip to conversation

usb.agent should processes usermap before other maps.

PATCH In review Maksim (Max) Krasnyanskiy 3 participants 14 messages Jul 18, 2002 – Aug 9, 2002

lore
Share & tools
lore

Maksim (Max) Krasnyanskiy

author
#0 lore raw

PATCH 1/1 usb.agent should processes usermap before other maps.

Hi Folks,

Currently usb.agent processes usermap after the kernelmap. Unfortunately it doesn't work for
Broadcom USB (BCM) firmware loader. The problem is that Bluetooth HCI USB driver (hci_usb)
has to claim any device with proper Bluetooth class, subclass and protocol regardless
of the vendorid/productid. But at the same time fw loader has to be able to claim devices
with specific vendorid/productid even if they have proper Bluetooth class, subclass, proto.

Here is an example of BCM config descriptors
before fw load
bDeviceClass 224
bDeviceSubClass 1
bDeviceProtocol 1
bMaxPacketSize0 64
idVendor 0x0a5c Broadcom Corp.
idProduct 0x2033

after
bDeviceClass 224
bDeviceSubClass 1
bDeviceProtocol 1
bMaxPacketSize0 64
idVendor 0x0a5c Broadcom Corp.
idProduct 0x2000

I added a5c/2033 to usb.handmap but since kernel map is processed first usb.agent doesn't call the loader.
So, the solution is to process usermap first. It kinda makes sense anyway, usermap should have priority over kernelmap.

Patch against hotplug 2002.04.01 is here

         http://bluez.sourceforge.net/patches/patch.hotplug-2002.04.01.gz

Comments ?

Max

http://bluez.sf.net
http://vtun.sf.net

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#1 lore raw
Quoted 12 lines · Maksim (Max) Krasnyanskiy

Hi Folks,

Currently usb.agent processes usermap after the kernelmap. Unfortunately it doesn't work for
Broadcom USB (BCM) firmware loader. The problem is that Bluetooth HCI USB driver (hci_usb)
has to claim any device with proper Bluetooth class, subclass and protocol regardless
of the vendorid/productid. But at the same time fw loader has to be able to claim devices
with specific vendorid/productid even if they have proper Bluetooth class, subclass, proto.

If hci_usb shouldn't be claiming specific devices, it's simple
to teach it not to do so. I notice it's not using driver_info
for anything, so it'd be easy to store flags there telling it not to bind to the pre-firmware IDs (idProduct = 0x2033); then the with-firmware path would work normally (idProduct = 0x2000).

That'd total about half a dozen lines: define the flag, add a MODULE_DEVICE_TABLE entry to blacklist that one device, add code in the probe() routine to test that flag and return if it's set.

Quoted 2 lines

So, the solution is to process usermap first. It kinda makes sense anyway, usermap should have priority over kernelmap.

No can do ... remember that when drivers are statically linked, the kernel is going to get first whack regardless of what you try to make userland do. Changing that isn't an option today, and wouldn't IMO be a good idea in any case.

That means you must make the kernel driver bind correctly, or in the case of this no-firmware device, not bind.

- Dave

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#2 lore raw

Re: [PATCH] usb.agent should processes usermap before other

In reply to Maksim (Max) Krasnyanskiy
Quoted 18 lines
Quoted 11 lines

Currently usb.agent processes usermap after the kernelmap. Unfortunately it doesn't work for
Broadcom USB (BCM) firmware loader. The problem is that Bluetooth HCI USB driver (hci_usb)
has to claim any device with proper Bluetooth class, subclass and protocol regardless
of the vendorid/productid. But at the same time fw loader has to be able to claim devices
with specific vendorid/productid even if they have proper Bluetooth class, subclass, proto.

If hci_usb shouldn't be claiming specific devices, it's simple
to teach it not to do so. I notice it's not using driver_info
for anything, so it'd be easy to store flags there telling it not to bind to the pre-firmware IDs (idProduct = 0x2033); then the with-firmware path would work normally (idProduct = 0x2000).

Quoted 3 lines

That'd total about half a dozen lines: define the flag, add a MODULE_DEVICE_TABLE entry to blacklist that one device, add code in the probe() routine to test that flag and return if it's set.

Yeah, I was gonna do that anyway because processing usermap first doesn't help when driver is already loaded.
I thought it might be nice if USB core could do that ie something like USB_IGNORE_DEVICE(xxx, yyy) in the driver id_table. But Johannes pointed out that it probably won't be useful for other drivers.

Quoted 10 lines
Quoted 2 lines

So, the solution is to process usermap first. It kinda makes sense anyway, usermap should have priority over kernelmap.

No can do ... remember that when drivers are statically linked, the kernel is going to get first whack regardless of what you try to make userland do. Changing that isn't an option today, and wouldn't IMO be a good idea in any case.

That means you must make the kernel driver bind correctly, or in the case of this no-firmware device, not bind.

I was gonna say that it would not work but then I noticed that it's actually fixed in 2002.04.01.
usb.agent that came with RH 7.2 was doing

         if [ "$FOUND" = "false" -a -r $MAP_USERMAP ];

Which means that usermap won't be processed if kernel driver exists for this device, even if driver ignores these particular ids. I didn't notice that you guys got rid of FOUND = false check in latest usb.agent. Now that this check isn't there it should work correctly if driver ignores pre-fw ids.

Max

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#3 lore raw
In reply to Maksim (Max) Krasnyanskiy
Quoted 10 lines
Quoted 2 lines

If hci_usb shouldn't be claiming specific devices, it's simple to teach it not to do so.

Yeah, I was gonna do that anyway because processing usermap first doesn't help
when driver is already loaded.
I thought it might be nice if USB core could do that ie something like USB_IGNORE_DEVICE(xxx, yyy) in the driver id_table. But Johannes pointed out
that it probably won't be useful for other drivers.

And #defining such a thing (a good example of when not to use inline functions :) in your driver means you get to change how it works later. Like if someday you want a single "declare device" macro you can call in two different modes (build your own table with lots of quirk/status handling, or a usb_device_id table), you can change how you use the driver_info (flags --> pointer).

Quoted 6 lines

I was gonna say that it would not work but then I noticed that it's actually fixed in 2002.04.01.
usb.agent that came with RH 7.2 was doing

        if [ "$FOUND" = "false" -a -r $MAP_USERMAP ];

Which means that usermap won't be processed if kernel driver exists for this device, even if driver ignores these particular ids.

Right, that was a bug. I don't recall the context where it made trouble; maybe it was similar to yours.

Quoted 3 lines

I didn't notice that you guys got rid of FOUND = false check in latest usb.agent. Now that this check isn't there it should work correctly if driver ignores pre-fw ids.

Bug reports help move Linux forward ... and patches too. :)

- Dave

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#4 lore raw

Re: [PATCH] usb.agent should processes usermap before other

In reply to Maksim (Max) Krasnyanskiy

Ok. Works now. Driver has ignore_ids[] table and usb_match_id(ignore_ids) in the probe.

btw How is usb.usermap supposed to be updated. It'd be nice to have per-susbsytem
usermap. Then I could simply install bluetooth specific usermap without patching
usb.usermap.
Comments ?

Max

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#5 lore raw
In reply to Maksim (Max) Krasnyanskiy
Quoted 8 lines · Maksim (Max) Krasnyanskiy

Ok. Works now. Driver has ignore_ids[] table and usb_match_id(ignore_ids) in the probe.

btw How is usb.usermap supposed to be updated. It'd be nice to have per-susbsytem
usermap. Then I could simply install bluetooth specific usermap without patching
usb.usermap.

Yes, I've been thinking the same thing. gphoto2 has it's own usermap which would be much more convenient if it was it's own file.

JE

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#6 lore raw

Re: [PATCH] usb.agent should processes usermap before other

In reply to Maksim (Max) Krasnyanskiy
Quoted 8 lines
Quoted 5 lines

btw How is usb.usermap supposed to be updated. It'd be nice to have per-susbsytem usermap.
Then I could simply install bluetooth specific usermap without patching usb.usermap.

Yes, I've been thinking the same thing. gphoto2 has it's own usermap which would be much more convenient if it was it's own file.

Ok. How about this.

usb.agent Thu Jul 18 01:11:30 2002 renamed from usb.agent.orig Thu Jul 18 01:03:23 2002 +11 -8
@@ -82,8 +82,7 @@
  # config scripts (which mustn't overlap with kernel modules).  Those
  # could change $DEVICE permissions, etc.
  #
-MAP_USERMAP=$HOTPLUG_DIR/usb.usermap
-
+MAP_USERMAP_DIR=$HOTPLUG_DIR/usb
 
  # accumulates list of modules we may care about
  DRIVERS=""

@@ -335,13 +334,17 @@
      fi
 
      # some devices have user-mode drivers (no kernel module, but config)
-    # or specialized user-mode setup helpers
-    if [ -r $MAP_USERMAP ]; then
+    # or specialized user-mode setup helpers
+    MAPS="$MAP_USERMAP_DIR/usermap.*"
+    if [ "$MAPS" != "" ]; then
         MODPROBE=:
-       load_drivers usb $MAP_USERMAP "$LABEL"
-       if [ "$DRIVERS" != "" ]; then
-           FOUND=true
-       fi
+       for map in $MAPS; do
+           load_drivers usb $map "$LABEL"
+           if [ "$DRIVERS" != "" ]; then
+               FOUND=true
+               break
+           fi
+       done
      fi
 
      if [ "$FOUND" = "false" ]; then

(this is just RFC, spaces are probably clobbered.
I'll send real patch if people are ok with this.)

/etc/hotplug/usb/usermap.bluetooth
bluefw 0x0003 0x0a5c 0x2033 0x0000 0x0000 0x00 0x00 0x00 0x00 0x00 0x00 0x00000000

Works fine for me.

Comments ?

Max

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#7 lore raw
In reply to Maksim (Max) Krasnyanskiy
Quoted 12 lines · Johannes Erdfelt
Quoted 8 lines · Maksim (Max) Krasnyanskiy

Ok. Works now. Driver has ignore_ids[] table and usb_match_id(ignore_ids) in the probe.

btw How is usb.usermap supposed to be updated. It'd be nice to have per-susbsytem
usermap. Then I could simply install bluetooth specific usermap without patching
usb.usermap.

Quoted 2 lines

Yes, I've been thinking the same thing. gphoto2 has it's own usermap which would be much more convenient if it was it's own file.

There's some stuff that's Debian-specific just now, builting the /etc/hotplug/usb.usermap from components installed somewhere in the /usr tree... maybe someone should make that not be so specific to Debian.

- dave

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#8 lore raw

Re: [PATCH] usb.agent should processes usermap before other

In reply to Maksim (Max) Krasnyanskiy
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines

btw How is usb.usermap supposed to be updated. It'd be nice to have per-susbsytem
usermap. Then I could simply install bluetooth specific usermap without patching
usb.usermap.

Quoted 7 lines
Quoted 2 lines

Yes, I've been thinking the same thing. gphoto2 has it's own usermap which would be much more convenient if it was it's own file.

There's some stuff that's Debian-specific just now, builting the /etc/hotplug/usb.usermap from components installed somewhere in the /usr tree... maybe someone should make that not be so specific to Debian.

I belive I did :). Did you miss my patch ?

Here it's again.
----------

usb.agent Thu Jul 18 01:11:30 2002 renamed from usb.agent.orig Thu Jul 18 01:03:23 2002 +11 -8
@@ -82,8 +82,7 @@
  # config scripts (which mustn't overlap with kernel modules).  Those
  # could change $DEVICE permissions, etc.
  #
-MAP_USERMAP=$HOTPLUG_DIR/usb.usermap
-
+MAP_USERMAP_DIR=$HOTPLUG_DIR/usb
 
  # accumulates list of modules we may care about
  DRIVERS=""

@@ -335,13 +334,17 @@
      fi
 
      # some devices have user-mode drivers (no kernel module, but config)
-    # or specialized user-mode setup helpers
-    if [ -r $MAP_USERMAP ]; then
+    # or specialized user-mode setup helpers
+    MAPS="$MAP_USERMAP_DIR/usermap.*"
+    if [ "$MAPS" != "" ]; then
         MODPROBE=:
-       load_drivers usb $MAP_USERMAP "$LABEL"
-       if [ "$DRIVERS" != "" ]; then
-           FOUND=true
-       fi
+       for map in $MAPS; do
+           load_drivers usb $map "$LABEL"
+           if [ "$DRIVERS" != "" ]; then
+               FOUND=true
+               break
+           fi
+       done
      fi
 
      if [ "$FOUND" = "false" ]; then

(this is just RFC, spaces are probably clobbered.
I'll send real patch if people are ok with this.)

/etc/hotplug/usb/usermap.bluetooth
bluefw 0x0003 0x0a5c 0x2033 0x0000 0x0000 0x00 0x00 0x00 0x00 0x00 0x00 0x00000000

Works fine for me.

Comments ?

Max

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#9 lore raw
In reply to Maksim (Max) Krasnyanskiy
Quoted 6 lines
Quoted 4 lines

There's some stuff that's Debian-specific just now, builting the /etc/hotplug/usb.usermap from components installed somewhere in the /usr tree... maybe someone should make that not be so specific to Debian.

I belive I did :). Did you miss my patch ?

No, it's just different from what the Debian code does.

I'd rather not have distros diverge needlessly. The reason we have so many incompatible Linux sysadmin toolsets is ... well, I'm sure you can figure it out! :) The goal IMO is to have one set of tools, and I'm not convinced there's a reason to diverge here.

- Dave

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#10 lore raw

Re: [PATCH] usb.agent should processes usermap before other

In reply to Maksim (Max) Krasnyanskiy
Quoted 8 lines
Quoted 6 lines
Quoted 5 lines

There's some stuff that's Debian-specific just now, builting the /etc/hotplug/usb.usermap from components installed somewhere in the /usr tree... maybe someone should make that not be so specific to Debian.

I belive I did :). Did you miss my patch ?

No, it's just different from what the Debian code does.

So ?

Quoted 5 lines

I'd rather not have distros diverge needlessly. The reason we have so many incompatible Linux sysadmin toolsets is ... well, I'm sure you can figure it out! :) The goal IMO is to have one set of tools, and I'm not convinced there's a reason to diverge here.

Yes, but I'm not sure why does it matter what Debian does. They probably had to do something because current hotplug doesn't support multiple usermaps. I'm sure they'd be happy to switch to new hotplug. If they don't they could still generate /etc/hotplug/usb/usermap.debian and it will work. We could also preserve 100% compatibility with older Debian by doing something like this
MAPS="$HOTPLUG_DIR/usb.usermap $MAP_USERMAP_DIR/usermap.*"
ie process old usb.usermap in addition to new maps.
So, old Debian packages that use usb.usermap generator will still work. And new onces will switch to more convenient way of doing it.

btw Generating global usb.usermap doesn't make a lot of sense to me. My patch is much simpler.

Max

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#11 lore raw
In reply to Maksim (Max) Krasnyanskiy
Quoted 7 lines
Quoted 5 lines

I'd rather not have distros diverge needlessly. The reason we have so many incompatible Linux sysadmin toolsets is ... well, I'm sure you can figure it out! :) The goal IMO is to have one set of tools, and I'm not convinced there's a reason to diverge here.

Yes, but I'm not sure why does it matter what Debian does.

Because if you solve the same problem in a different way, you're creating another problem. Maybe we need to do so, but you ought to at least try to avoid creating such problems in the first place -- so they don't need re-fixing again.

Quoted 4 lines

They probably had to do something because current hotplug doesn't support multiple usermaps. I'm sure they'd be happy to switch to new hotplug. If they don't they could still generate /etc/hotplug/usb/usermap.debian and it will work.

I'd be pleased to see you work this issue with the Debian maintainer of hotplug. Maybe he would be; maybe he's seen some other issues that fit in here too.

- Dave

Quoted 7 lines

We could also preserve 100% compatibility with older Debian by doing something like this
MAPS="$HOTPLUG_DIR/usb.usermap $MAP_USERMAP_DIR/usermap.*"
ie process old usb.usermap in addition to new maps.
So, old Debian packages that use usb.usermap generator will still work. And new onces will switch to more convenient way of doing it.

Quoted 6 lines

btw Generating global usb.usermap doesn't make a lot of sense to me. My patch is much simpler.

Max

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#12 lore raw

Re: [PATCH] usb.agent should processes usermap before other

In reply to Maksim (Max) Krasnyanskiy
Quoted 5 lines

Because if you solve the same problem in a different way, you're creating another problem. Maybe we need to do so, but you ought to at least try to avoid creating such problems in the first place -- so they don't need re-fixing again.

Well, they hack original hotplug scripts anyway. And like I said there is no problem.
I looked at their scrips. If we keep support for central usb.usermap we're fine.

Quoted 8 lines
Quoted 4 lines

They probably had to do something because current hotplug doesn't support multiple usermaps. I'm sure they'd be happy to switch to new hotplug. If they don't they could still generate /etc/hotplug/usb/usermap.debian and it will work.

I'd be pleased to see you work this issue with the Debian maintainer of hotplug. Maybe he would be; maybe he's seen some other issues that fit in here too.

Ok. See my next email.

Hopefully you want ask me to talk to RH, Mandrake, SuSe and other folks in order to accept very simple and logical patch that preserves compatibility ;-).

Max

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#13 lore raw
In reply to Maksim (Max) Krasnyanskiy

OK, a version of this patch is now in CVS at sourceforge, along with some minor bugfixes. Changelog:

- load_drivers(): variables are local, and doesn't try usbmodules unless the $DEVICE file exists (it'd fail)
- update hotplug.8 manpage to mention Max'patch
- patch from Max Krasnyanskiy, now usb hotplugging also searches /etc/hotplug/usb/*.usermap

Basically, /etc/hotplug/usb.usermap is searched first, then any /etc/hotplug/usb/*.usermap files. Seems like Debian could easily adopt this, phasing out the need for a separate tool and some new /usr/... directory.

Does anyone have sample "input" hotplug scripts yet? Or SCSI hotplug scripts? (*) I'd like to see the input stuff replace the /etc/hotplug/usb.handmap hacks in some upcoming release.

- Dave

(*) http://lwn.net/Articles/6967/ may be worth discussing
here too ...

-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel