Re: cardbus and kldunload issue

2011-02-28 Thread John Baldwin
On Saturday, February 26, 2011 10:25:41 am Bernhard Schmidt wrote:
 Hi,
 
 while working on a wireless driver for a Cardbus card I stumbled over
 an issue which bugs me quite a bit.
 
 The device:
 
 % none3@pci0:22:0:0:  class=0x028000 card=0x107f1043 chip=0x02011814 
rev=0x01 hdr=0x00
 
 Loading the module attaches nicely to the device:
 
 # kldload if_ral
 % ral0: Ralink Technology RT2560 mem 0xf480-0xf4801fff irq 16 at 
device 0.0 on cardbus0
 % ral0: MAC/BBP RT2560 (rev 0x04), RF RT2525
 % ral0: [ITHREAD]
 # pciconf -l
 % ral0@pci0:22:0:0:   class=0x028000 card=0x107f1043 chip=0x02011814 
rev=0x01 hdr=0x00
 
 Though, kldunload doesn't detach the device, it doesn't even call the
 module's detach function.
 
 # kldunload if_ral
 # pciconf -l
 % ral0@pci0:22:0:0:   class=0x028000 card=0x107f1043 chip=0x02011814 
rev=0x01 hdr=0x00
 # kldstat
 % Id Refs AddressSize Name
 %  1   27 0x8010 e640a0   kernel
 # ifconfig ral0
 % ral0: flags=8802BROADCAST,SIMPLEX,MULTICAST metric 0 mtu 2290
 % ether 00:0e:a6:a6:1b:70
 % media: IEEE 802.11 Wireless Ethernet autoselect (autoselect)
 % status: no carrier
 
 And of course trying to use the device at that point will result in
 instant panics. Playing around a bit I've noticed that changing the bus
 name in:
 
 % DRIVER_MODULE(ral, pci, ral_pci_driver, ral_devclass, 0, 0);
 
 from pci to cardbus makes a big difference. On module unload the detach
 function is then called as expected. So, question is, are we doing some
 too strict checks on module unload while matching the bus? Or is this
 expected behavior and the drivers are supposed to indiciated that they
 support both pci and cardbus? I don't see the later anywhere in tree.

This sounds like a bug in how the inheritance stuff in new-bus drivers works.
The DRIVER_MODULE() line for cardbus is implicit because the 'cardbus' driver
inherits from the generic 'pci' bus driver.  That is how kldload works 
correctly.  It seems we need to do some extra plumbing for the kldunload case.  

The bug is that 189574 only patched the devclass add driver path, not the 
delete driver path.  Try this:

Index: subr_bus.c
===
--- subr_bus.c  (revision 219096)
+++ subr_bus.c  (working copy)
@@ -987,11 +987,13 @@ devclass_find(const char *classname)
  * is called by devclass_add_driver to accomplish the recursive
  * notification of all the children classes of dc, as well as dc.
  * Each layer will have BUS_DRIVER_ADDED() called for all instances of
- * the devclass.  We do a full search here of the devclass list at
- * each iteration level to save storing children-lists in the devclass
- * structure.  If we ever move beyond a few dozen devices doing this,
- * we may need to reevaluate...
+ * the devclass.
  *
+ * We do a full search here of the devclass list at each iteration
+ * level to save storing children-lists in the devclass structure.  If
+ * we ever move beyond a few dozen devices doing this, we may need to
+ * reevaluate...
+ *
  * @param dc   the devclass to edit
  * @param driver   the driver that was just added
  */
@@ -1085,6 +1087,78 @@ devclass_add_driver(devclass_t dc, driver_t *drive
 }
 
 /**
+ * @brief Register that a device driver has been deleted from a devclass
+ *
+ * Register that a device driver has been removed from a devclass.
+ * This is called by devclass_delete_driver to accomplish the
+ * recursive notification of all the children classes of busclass, as
+ * well as busclass.  Each layer will attempt to detach the driver
+ * from any devices that are children of the bus's devclass.  The function
+ * will return an error if a device fails to detach.
+ * 
+ * We do a full search here of the devclass list at each iteration
+ * level to save storing children-lists in the devclass structure.  If
+ * we ever move beyond a few dozen devices doing this, we may need to
+ * reevaluate...
+ *
+ * @param busclass the devclass of the parent bus
+ * @param dc   the devclass of the driver being deleted
+ * @param driver   the driver being deleted
+ */
+static int
+devclass_driver_deleted(devclass_t busclass, devclass_t dc, driver_t *driver)
+{
+   devclass_t parent;
+   device_t dev;
+   int error, i;
+
+   /*
+* Disassociate from any devices.  We iterate through all the
+* devices in the devclass of the driver and detach any which are
+* using the driver and which have a parent in the devclass which
+* we are deleting from.
+*
+* Note that since a driver can be in multiple devclasses, we
+* should not detach devices which are not children of devices in
+* the affected devclass.
+*/
+   for (i = 0; i  dc-maxunit; i++) {
+   if (dc-devices[i]) {
+   dev = dc-devices[i];
+   if (dev-driver == driver  dev-parent 
+  

Re: cardbus and kldunload issue

2011-02-28 Thread Bernhard Schmidt
On Saturday 26 February 2011 19:36:14 Paul B. Mahol wrote:
 On Sat, Feb 26, 2011 at 4:25 PM, Bernhard Schmidt bschm...@freebsd.org 
 wrote:
  Hi,
 
  while working on a wireless driver for a Cardbus card I stumbled over
  an issue which bugs me quite a bit.
 
  The device:
 
  % none3@pci0:22:0:0:  class=0x028000 card=0x107f1043 chip=0x02011814 
  rev=0x01 hdr=0x00
 
  Loading the module attaches nicely to the device:
 
  # kldload if_ral
  % ral0: Ralink Technology RT2560 mem 0xf480-0xf4801fff irq 16 at 
  device 0.0 on cardbus0
  % ral0: MAC/BBP RT2560 (rev 0x04), RF RT2525
  % ral0: [ITHREAD]
  # pciconf -l
  % ral0@pci0:22:0:0:   class=0x028000 card=0x107f1043 chip=0x02011814 
  rev=0x01 hdr=0x00
 
  Though, kldunload doesn't detach the device, it doesn't even call the
  module's detach function.
 
  # kldunload if_ral
  # pciconf -l
  % ral0@pci0:22:0:0:   class=0x028000 card=0x107f1043 chip=0x02011814 
  rev=0x01 hdr=0x00
  # kldstat
  % Id Refs AddressSize Name
  %  1   27 0x8010 e640a0   kernel
  # ifconfig ral0
  % ral0: flags=8802BROADCAST,SIMPLEX,MULTICAST metric 0 mtu 2290
  % ether 00:0e:a6:a6:1b:70
  % media: IEEE 802.11 Wireless Ethernet autoselect (autoselect)
  % status: no carrier
 
  And of course trying to use the device at that point will result in
  instant panics. Playing around a bit I've noticed that changing the bus
  name in:
 
  % DRIVER_MODULE(ral, pci, ral_pci_driver, ral_devclass, 0, 0);
 
  from pci to cardbus makes a big difference. On module unload the detach
  function is then called as expected. So, question is, are we doing some
  too strict checks on module unload while matching the bus? Or is this
  expected behavior and the drivers are supposed to indiciated that they
  support both pci and cardbus? I don't see the later anywhere in tree.
 
 There is MODULE_DEPEND(), if_ndis depends on pccard, pci and usb
 modules and use both MODULE_DEPEND() and DRIVER_MODULE() with them.
 
 If I'm not mistaken pccard depends on cardbus.

I tried playing around with various MODULE_DEPEND() values, without any
differences.

-- 
Bernhard
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: cardbus and kldunload issue

2011-02-28 Thread Bernhard Schmidt
On Monday 28 February 2011 14:37:31 John Baldwin wrote:
 On Saturday, February 26, 2011 10:25:41 am Bernhard Schmidt wrote:
  Hi,
  
  while working on a wireless driver for a Cardbus card I stumbled over
  an issue which bugs me quite a bit.
  
  The device:
  
  % none3@pci0:22:0:0:  class=0x028000 card=0x107f1043 chip=0x02011814 
 rev=0x01 hdr=0x00
  
  Loading the module attaches nicely to the device:
  
  # kldload if_ral
  % ral0: Ralink Technology RT2560 mem 0xf480-0xf4801fff irq 16 at 
 device 0.0 on cardbus0
  % ral0: MAC/BBP RT2560 (rev 0x04), RF RT2525
  % ral0: [ITHREAD]
  # pciconf -l
  % ral0@pci0:22:0:0:   class=0x028000 card=0x107f1043 chip=0x02011814 
 rev=0x01 hdr=0x00
  
  Though, kldunload doesn't detach the device, it doesn't even call the
  module's detach function.
  
  # kldunload if_ral
  # pciconf -l
  % ral0@pci0:22:0:0:   class=0x028000 card=0x107f1043 chip=0x02011814 
 rev=0x01 hdr=0x00
  # kldstat
  % Id Refs AddressSize Name
  %  1   27 0x8010 e640a0   kernel
  # ifconfig ral0
  % ral0: flags=8802BROADCAST,SIMPLEX,MULTICAST metric 0 mtu 2290
  % ether 00:0e:a6:a6:1b:70
  % media: IEEE 802.11 Wireless Ethernet autoselect (autoselect)
  % status: no carrier
  
  And of course trying to use the device at that point will result in
  instant panics. Playing around a bit I've noticed that changing the bus
  name in:
  
  % DRIVER_MODULE(ral, pci, ral_pci_driver, ral_devclass, 0, 0);
  
  from pci to cardbus makes a big difference. On module unload the detach
  function is then called as expected. So, question is, are we doing some
  too strict checks on module unload while matching the bus? Or is this
  expected behavior and the drivers are supposed to indiciated that they
  support both pci and cardbus? I don't see the later anywhere in tree.
 
 This sounds like a bug in how the inheritance stuff in new-bus drivers works.
 The DRIVER_MODULE() line for cardbus is implicit because the 'cardbus' driver
 inherits from the generic 'pci' bus driver.  That is how kldload works 
 correctly.  It seems we need to do some extra plumbing for the kldunload 
 case.  
 
 The bug is that 189574 only patched the devclass add driver path, not the 
 delete driver path.  Try this:

That fixes it, thanks! kldunload now successfully calls the driver's
detach method.

 Index: subr_bus.c
 ===
 --- subr_bus.c(revision 219096)
 +++ subr_bus.c(working copy)
 @@ -987,11 +987,13 @@ devclass_find(const char *classname)
   * is called by devclass_add_driver to accomplish the recursive
   * notification of all the children classes of dc, as well as dc.
   * Each layer will have BUS_DRIVER_ADDED() called for all instances of
 - * the devclass.  We do a full search here of the devclass list at
 - * each iteration level to save storing children-lists in the devclass
 - * structure.  If we ever move beyond a few dozen devices doing this,
 - * we may need to reevaluate...
 + * the devclass.
   *
 + * We do a full search here of the devclass list at each iteration
 + * level to save storing children-lists in the devclass structure.  If
 + * we ever move beyond a few dozen devices doing this, we may need to
 + * reevaluate...
 + *
   * @param dc the devclass to edit
   * @param driver the driver that was just added
   */
 @@ -1085,6 +1087,78 @@ devclass_add_driver(devclass_t dc, driver_t *drive
  }
  
  /**
 + * @brief Register that a device driver has been deleted from a devclass
 + *
 + * Register that a device driver has been removed from a devclass.
 + * This is called by devclass_delete_driver to accomplish the
 + * recursive notification of all the children classes of busclass, as
 + * well as busclass.  Each layer will attempt to detach the driver
 + * from any devices that are children of the bus's devclass.  The function
 + * will return an error if a device fails to detach.
 + * 
 + * We do a full search here of the devclass list at each iteration
 + * level to save storing children-lists in the devclass structure.  If
 + * we ever move beyond a few dozen devices doing this, we may need to
 + * reevaluate...
 + *
 + * @param busclass   the devclass of the parent bus
 + * @param dc the devclass of the driver being deleted
 + * @param driver the driver being deleted
 + */
 +static int
 +devclass_driver_deleted(devclass_t busclass, devclass_t dc, driver_t *driver)
 +{
 + devclass_t parent;
 + device_t dev;
 + int error, i;
 +
 + /*
 +  * Disassociate from any devices.  We iterate through all the
 +  * devices in the devclass of the driver and detach any which are
 +  * using the driver and which have a parent in the devclass which
 +  * we are deleting from.
 +  *
 +  * Note that since a driver can be in multiple devclasses, we
 +  * should not detach devices which are not children of devices in
 +  

cardbus and kldunload issue

2011-02-26 Thread Bernhard Schmidt
Hi,

while working on a wireless driver for a Cardbus card I stumbled over
an issue which bugs me quite a bit.

The device:

% none3@pci0:22:0:0:  class=0x028000 card=0x107f1043 chip=0x02011814 
rev=0x01 hdr=0x00

Loading the module attaches nicely to the device:

# kldload if_ral
% ral0: Ralink Technology RT2560 mem 0xf480-0xf4801fff irq 16 at device 
0.0 on cardbus0
% ral0: MAC/BBP RT2560 (rev 0x04), RF RT2525
% ral0: [ITHREAD]
# pciconf -l
% ral0@pci0:22:0:0:   class=0x028000 card=0x107f1043 chip=0x02011814 
rev=0x01 hdr=0x00

Though, kldunload doesn't detach the device, it doesn't even call the
module's detach function.

# kldunload if_ral
# pciconf -l
% ral0@pci0:22:0:0:   class=0x028000 card=0x107f1043 chip=0x02011814 
rev=0x01 hdr=0x00
# kldstat
% Id Refs AddressSize Name
%  1   27 0x8010 e640a0   kernel
# ifconfig ral0
% ral0: flags=8802BROADCAST,SIMPLEX,MULTICAST metric 0 mtu 2290
% ether 00:0e:a6:a6:1b:70
% media: IEEE 802.11 Wireless Ethernet autoselect (autoselect)
% status: no carrier

And of course trying to use the device at that point will result in
instant panics. Playing around a bit I've noticed that changing the bus
name in:

% DRIVER_MODULE(ral, pci, ral_pci_driver, ral_devclass, 0, 0);

from pci to cardbus makes a big difference. On module unload the detach
function is then called as expected. So, question is, are we doing some
too strict checks on module unload while matching the bus? Or is this
expected behavior and the drivers are supposed to indiciated that they
support both pci and cardbus? I don't see the later anywhere in tree.

-- 
Bernhard
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: cardbus and kldunload issue

2011-02-26 Thread Paul B. Mahol
On Sat, Feb 26, 2011 at 4:25 PM, Bernhard Schmidt bschm...@freebsd.org wrote:
 Hi,

 while working on a wireless driver for a Cardbus card I stumbled over
 an issue which bugs me quite a bit.

 The device:

 % none3@pci0:22:0:0:      class=0x028000 card=0x107f1043 chip=0x02011814 
 rev=0x01 hdr=0x00

 Loading the module attaches nicely to the device:

 # kldload if_ral
 % ral0: Ralink Technology RT2560 mem 0xf480-0xf4801fff irq 16 at device 
 0.0 on cardbus0
 % ral0: MAC/BBP RT2560 (rev 0x04), RF RT2525
 % ral0: [ITHREAD]
 # pciconf -l
 % ral0@pci0:22:0:0:       class=0x028000 card=0x107f1043 chip=0x02011814 
 rev=0x01 hdr=0x00

 Though, kldunload doesn't detach the device, it doesn't even call the
 module's detach function.

 # kldunload if_ral
 # pciconf -l
 % ral0@pci0:22:0:0:       class=0x028000 card=0x107f1043 chip=0x02011814 
 rev=0x01 hdr=0x00
 # kldstat
 % Id Refs Address            Size     Name
 %  1   27 0x8010 e640a0   kernel
 # ifconfig ral0
 % ral0: flags=8802BROADCAST,SIMPLEX,MULTICAST metric 0 mtu 2290
 %         ether 00:0e:a6:a6:1b:70
 %         media: IEEE 802.11 Wireless Ethernet autoselect (autoselect)
 %         status: no carrier

 And of course trying to use the device at that point will result in
 instant panics. Playing around a bit I've noticed that changing the bus
 name in:

 % DRIVER_MODULE(ral, pci, ral_pci_driver, ral_devclass, 0, 0);

 from pci to cardbus makes a big difference. On module unload the detach
 function is then called as expected. So, question is, are we doing some
 too strict checks on module unload while matching the bus? Or is this
 expected behavior and the drivers are supposed to indiciated that they
 support both pci and cardbus? I don't see the later anywhere in tree.

There is MODULE_DEPEND(), if_ndis depends on pccard, pci and usb
modules and use both MODULE_DEPEND() and DRIVER_MODULE() with them.

If I'm not mistaken pccard depends on cardbus.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org