[PATCH] usb.agent should processes usermap before other maps.

[PATCH] usb.agent should processes usermap before other maps.

[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.

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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[David Brownell]

Maksim (Max) Krasnyanskiy wrote:
> 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.


> 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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[Maksim (Max) Krasnyanskiy]

>>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.
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.

>>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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[David Brownell]

>> 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).


> 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.


> 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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[Johannes Erdfelt]

On Thu, Jul 18, 2002, Maksim (Max) Krasnyanskiy <maxk@qualcomm.com> wrote:
> 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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[Maksim (Max) Krasnyanskiy]

> > 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.orig      Thu Jul 18 01:03:23 2002
+++ usb.agent   Thu Jul 18 01:11:30 2002
@@ -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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[David Brownell]

Johannes Erdfelt wrote:
> On Thu, Jul 18, 2002, Maksim (Max) Krasnyanskiy <maxk@qualcomm.com> wrote:
> 
>>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.

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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[Maksim (Max) Krasnyanskiy]

>>>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.
>
>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.orig      Thu Jul 18 01:03:23 2002
+++ usb.agent   Thu Jul 18 01:11:30 2002
@@ -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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[David Brownell]

>> 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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[Maksim (Max) Krasnyanskiy]

>>>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 ?

>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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[David Brownell]

>> 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.


> 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


> 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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[Maksim (Max) Krasnyanskiy]

>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.

>>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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

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

[David Brownell]

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
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel