On Thu, 2014-01-30 at 21:24 +0530, Jahnavi wrote: > From: Jahnavi Meher <jahnavi.meher@xxxxxxxxxxxxxxxxxx> > > This file adds the rsi_usb module, enabling the USB interface for > the 91x chipsets. > > Signed-off-by: Jahnavi Meher <jahnavi.meher@xxxxxxxxxxxxxxxxxx> > --- In general the code is very clean and mostly follows kernel style. It's also fairly easy to read. That's great. However, I've got some architecture comments below... > rsi_usb.c | 642 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 642 insertions(+) > > diff -uprN a/drivers/net/wireless/rsi/common/rsi_usb.c b/drivers/net/wireless/rsi/common/rsi_usb.c > --- a/drivers/net/wireless/rsi/common/rsi_usb.c 1970-01-01 05:30:00.000000000 +0530 > +++ b/drivers/net/wireless/rsi/common/rsi_usb.c 2014-01-30 16:15:14.126308228 +0530 > @@ -0,0 +1,642 @@ > +/** > + * @file rsi_usb.c > + * @author > + * @version 1.0 > + * > + * @section LICENSE > + * Copyright (c) 2013 Redpine Signals Inc. > + * > + * Permission to use, copy, modify, and/or distribute this software for any > + * purpose with or without fee is hereby granted, provided that the above > + * copyright notice and this permission notice appear in all copies. > + * > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > + * > + * @section DESCRIPTION > + * > + * The file contains Generic HAL layer for USB. > + */ > + > +#include "../include/rsi_main.h" > +#include "../include/rsi_hw_intf.h" > +#include "../include/rsi_device_ops.h" > + > +static unsigned short fw; Why does 'fw' need to be static? It's only used in one function, and it's read from the device every time. > +static unsigned char reset_card = 1; See my notes below about reset_card; why is this a static global for the file? First, it prevents using multiple USB devices with the driver. Second, what does it actually prevent and is that actually the best way to do that? > +/** > + * This function writes to the USB Card. > + * > + * @param adapter Pointer to the adapter structure. > + * @param buf Pointer to the buffer from where the data has to be taken. > + * @param len Length to be written. > + * @param endpoint Type of endpoint. > + * @return status: 0 on success, -1 on failure. > + */ > +static int rsi_usb_card_write(struct rsi_hw *adapter, > + void *buf, > + unsigned short len, > + unsigned char endpoint) Use types like u16, u8, and u32 where the types are not endian-dependent. Use types like __le32 or __be32 or __le16 or __be16 where you interface with firmware, which is endian-dependent. That way you can run 'sparse' and find out where your driver may have mismatching endian problems between host and the device. > +{ > + int status; > + int transfer; > + > + status = usb_bulk_msg(adapter->usbdev, > + usb_sndbulkpipe(adapter->usbdev, > + adapter->bulkout_endpoint_addr[endpoint - 1]), > + buf, > + len, > + &transfer, > + HZ * 5); > + > + if (status < 0) { > + rsi_dbg(ERR_ZONE, > + "Card write failed with error code :%10d\n", status); > + adapter->write_fail = 1; > + } > + return status; > +} > + > +/** > + * This function writes multiple bytes of information to the USB card. > + * > + * @param adapter Pointer to the adapter structure. > + * @param addr Address of the register. > + * @param data Pointer to the data that has to be written. > + * @param count Number of multiple bytes to be written. > + * @return 0 on success, -1 on failure. > + */ > +static int rsi_write_multiple(struct rsi_hw *adapter, > + unsigned int addr, > + unsigned char *data, > + unsigned int count) Use u32, u8, etc. > +{ > + if ((adapter == NULL) || (adapter->write_fail) || (addr == 0)) { > + return 0; Since this is returning, no need to put the rest of the code below into an indented block. > + } else { > + unsigned char *seg = adapter->tx_buffer; > + > + if (addr == 1) { Why is addr = 1 special? I see later on that it's determined from the queue that the packet is supposed to be in. But rsi_write_multiple() actually calls rsi_usb_card_write() and passes 'addr' as the USB endpoint? So correct me if I'm wrong, but what really *is* happening here is that the driver is doing WMM internally and keeps a management queue and a data queue. Then the USB bus layer inspects the host->device packet and send that packet to a specific USB endpoint depending on whether it's a mgmt frame or a data frame? This should really be more explicit. Or maybe just add a new parameter to host_intf_write_pkt like 'bool mgmt' or something? Or, if Johannes has a better suggestion for cleaning up the queue stuff, I'd defer to that. > + memset(seg, 0, RSI_USB_TX_HEAD_ROOM); > + memcpy(seg + RSI_USB_TX_HEAD_ROOM, data, count); > + } else { > + seg = ((unsigned char *)data - RSI_USB_TX_HEAD_ROOM); > + } > + return rsi_usb_card_write(adapter, > + seg, > + count + RSI_USB_TX_HEAD_ROOM, > + addr); > + } > +} > + > +/** > + * This function initializes the bulk endpoints to the device. > + * > + * @param interface Pointer to the USB interface structure. > + * @param adapter Pointer to the adapter structure. > + * @return ret_val: 0 on success, -ENOMEM on failure. > + */ > +static int rsi_find_bulk_in_and_out_endpoints(struct usb_interface *interface, > + struct rsi_hw *adapter) > +{ > + struct usb_host_interface *iface_desc; > + struct usb_endpoint_descriptor *endpoint; > + int buffer_size; > + int ii, bep_found = 0; > + > + if ((interface == NULL) || (adapter == NULL)) > + return -1; > + > + iface_desc = &(interface->altsetting[0]); > + > + for (ii = 0; ii < iface_desc->desc.bNumEndpoints; ++ii) { > + endpoint = &(iface_desc->endpoint[ii].desc); > + > + if ((!(adapter->bulkin_endpoint_addr)) && > + (endpoint->bEndpointAddress & USB_DIR_IN) && > + ((endpoint->bmAttributes & > + USB_ENDPOINT_XFERTYPE_MASK) == > + USB_ENDPOINT_XFER_BULK)) { > + buffer_size = endpoint->wMaxPacketSize; > + adapter->bulkin_size = buffer_size; > + adapter->bulkin_endpoint_addr = > + endpoint->bEndpointAddress; > + } > + > + if (!adapter->bulkout_endpoint_addr[bep_found] && > + !(endpoint->bEndpointAddress & USB_DIR_IN) && > + ((endpoint->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == > + USB_ENDPOINT_XFER_BULK)) { > + adapter->bulkout_endpoint_addr[bep_found] = > + endpoint->bEndpointAddress; > + buffer_size = endpoint->wMaxPacketSize; > + adapter->bulkout_size[bep_found] = buffer_size; > + bep_found++; > + } > + > + if (bep_found >= MAX_BULK_EP) > + break; > + } > + > + if (!(adapter->bulkin_endpoint_addr) && > + (adapter->bulkout_endpoint_addr[0])) > + return -1; > + > + return 0; > +} > + > +/* This function reads the data from given register address. > + * > + * @param usbdev Pointer to the usb_device structure. > + * @param reg Address of the register to be read. > + * @param value Value to be read. > + * @param len length of data to be read. > + * @return status: 0 on success, -1 on failure. > + */ > +static int rsi_usb_reg_read(struct usb_device *usbdev, > + unsigned int reg, u32 perhaps? > + unsigned short *value, > + unsigned short len) u16 instead of unsigned short. > +{ > + unsigned char temp_buf[4]; Use 'u8' instead of unsigned char. > + int status = 0; > + > + len = 2;/* FIXME */ Every caller of rsi_usb_reg_read() already passed len = 2, so why is this function overriding the length the callers want? > + > + status = usb_control_msg(usbdev, > + usb_rcvctrlpipe(usbdev, 0), > + USB_VENDOR_REGISTER_READ, > + USB_TYPE_VENDOR, > + ((reg & 0xffff0000) >> 16), (reg & 0xffff), > + (void *)temp_buf, > + len, > + HZ * 5); > + > + *value = (temp_buf[0] | (temp_buf[1] << 8)); > + if (status < 0) { > + rsi_dbg(ERR_ZONE, > + "%s: Reg read failed with error code :%d\n", > + __func__, status); > + } > + return status; > +} > + > +/** > + * This function writes the given data into the given register address. > + * > + * @param usbdev Pointer to the usb_device structure. > + * @param reg Address of the register. > + * @param value Value to write. > + * @param len Length of data to be written. > + * @return status: 0 on success, -1 on failure. > + */ > +static int rsi_usb_reg_write(struct usb_device *usbdev, > + unsigned int reg, > + unsigned short value, > + unsigned short len) > +{ > + unsigned char usb_reg_buf[4]; u8 > + int status = 0; > + > + usb_reg_buf[0] = (value & 0x00ff); > + usb_reg_buf[1] = (value & 0xff00) >> 8; > + usb_reg_buf[2] = 0x0; > + usb_reg_buf[3] = 0x0; > + > + status = usb_control_msg(usbdev, > + usb_sndctrlpipe(usbdev, 0), > + USB_VENDOR_REGISTER_WRITE, > + USB_TYPE_VENDOR, > + ((reg & 0xffff0000) >> 16), > + (reg & 0xffff), > + (void *)usb_reg_buf, > + len, > + HZ * 5); > + if (status < 0) { > + rsi_dbg(ERR_ZONE, > + "%s: Reg write failed with error code :%d\n", > + __func__, status); > + } > + return status; > +} > + > +/** > + * This function is called when a packet is received from USB > + * the stack. This is a callback to recieve done. > + * > + * @param urb Received URB. > + * @return None. > + */ > +static void rsi_rx_done_handler(struct urb *urb) > +{ > + struct rsi_hw *adapter = urb->context; > + struct rsi_common *common = adapter->priv; > + > + if (urb->status) > + return; > + > + adapter->total_usb_rx_urbs_done++; I can't find where this is used, but for whatever reason I don't see patches 5, 6, or 8 in my mailbox. If it's not used, it should be removed. > + rsi_set_event(&common->rx_event); > + return; > +} > + > +/** > + * This function submits the given URB to the USB stack. > + * > + * @param adapter Pointer to the adapter structure. > + * @return 0 on success, -1 on failure. > + */ > +int rsi_rx_urb_submit(struct rsi_hw *adapter) > +{ > + struct urb *urb = adapter->rx_usb_urb[0]; > + > + adapter->total_usb_rx_urbs_submitted++; > + usb_fill_bulk_urb(urb, > + adapter->usbdev, > + usb_rcvbulkpipe(adapter->usbdev, > + adapter->bulkin_endpoint_addr), > + urb->transfer_buffer, > + 3000, > + rsi_rx_done_handler, > + adapter); > + if (usb_submit_urb(urb, GFP_KERNEL)) { > + rsi_dbg(ERR_ZONE, "%s: Failed in urb submission\n", __func__); > + return -1; > + } else { > + return 0; > + } This would typically be written as: if (usb_submit_urb(..)) { rsi_dbg(); return -1; } return 0; > +} > + > +/** > + * This function writes multiple bytes of information to multiple registers. > + * > + * @param adapter Pointer to the adapter structure. > + * @param addr Address of the register. > + * @param data Pointer to the data that has to be written. > + * @param count Number of multiple bytes to be written on to the registers. > + * @return status: 0 on success, -1 on failure. > + */ > +int rsi_write_ta_register_multiple(struct rsi_hw *adapter, > + unsigned int addr, > + unsigned char *data, > + unsigned int count) u32, u8, etc, you get the point. > +{ > + unsigned char *buf; > + unsigned char transfer; > + int status = 0; > + > + buf = kzalloc(4096, GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > + > + while (count) { > + transfer = min_t(int, count, 4096); > + memcpy(buf, data, transfer); > + status = usb_control_msg(adapter->usbdev, > + usb_sndctrlpipe(adapter->usbdev, 0), > + USB_VENDOR_REGISTER_WRITE, > + USB_TYPE_VENDOR, > + ((addr & 0xffff0000) >> 16), > + (addr & 0xffff), > + (void *)buf, > + transfer, > + HZ * 5); > + if (status < 0) { > + rsi_dbg(ERR_ZONE, > + "Reg write failed with error code :%d\n", > + status); > + } else { > + count -= transfer; > + data += transfer; > + addr += transfer; > + } > + } > + > + kfree(buf); > + return 0; > +} > + > +/** > + * This function writes the packet to the USB card. > + * > + * @param adapter Pointer to the adapter structure. > + * @param pkt Pointer to the data to be written on to the card. > + * @param len Length of the data to be written on to the card. > + * @return 0 on success, -1 on failure. > + */ > +int rsi_write_pkt(struct rsi_hw *adapter, > + unsigned char *pkt, > + unsigned int len) > +{ > + unsigned int block_size = adapter->tx_blk_size; > + unsigned int num_blocks, address; > + unsigned int queueno = ((pkt[1] >> 4) & 0xf); > + > + if ((!len) && (queueno == RSI_WIFI_DATA_Q)) { > + rsi_dbg(ERR_ZONE, "%s: Wrong length\n", __func__); > + return -1; > + } See comments about queues and endpoints above. But really, if it's an error to send a zero-length data packet, then perhaps it should be BUG_ON() since the driver should never, ever do that? > + num_blocks = (len / block_size); > + > + if (len % block_size) > + num_blocks++; num_blocks doesn't get used anywhere, should be removed. > + if (((*(unsigned short *)pkt) < 14) && (queueno == RSI_WIFI_DATA_Q)) { > + rsi_dbg(ERR_ZONE, "%s: Too small pkt\n", __func__); > + return -1; > + } > + > + address = ((queueno == RSI_WIFI_MGMT_Q) ? 1 : 2); > + > + return rsi_write_multiple(adapter, > + address, > + (unsigned char *)pkt, > + len); > +} > + > +/** > + * This function initializes the usb interface. > + * > + * @param adapter Pointer to the adapter structure. > + * @param pfunction Pointer to USB interface structure. > + * @return 0 on success, -1 on failure. > + */ > +static int rsi_init_usb_interface(struct rsi_hw *adapter, > + struct usb_interface *pfunction) > +{ > + adapter->usbdev = interface_to_usbdev(pfunction); > + > + if (rsi_find_bulk_in_and_out_endpoints(pfunction, adapter)) > + return -1; > + > + adapter->device = &pfunction->dev; > + usb_set_intfdata(pfunction, adapter); > + > + adapter->tx_buffer = kmalloc(2048, GFP_ATOMIC); > + adapter->rx_usb_urb[0] = usb_alloc_urb(0, GFP_KERNEL); > + adapter->rx_usb_urb[0]->transfer_buffer = adapter->priv->rx_data_pkt; > + adapter->tx_blk_size = 252; > + > + rsi_dbg(INIT_ZONE, "%s: Enabled the interface\n", __func__); > + return 0; > +} > + > +/** > + * This function writes into various registers to do USB reset. > + * > + * @param usbdev Pointer to the usb_device structure. > + * @param addr Address to write to. > + * @param data Data to be written onto the address. > + * @param len_in_bits Length in bits. > + * @return 0 on success, -1 on failure. > + */ > +static int rsi_ulp_read_write(struct usb_device *usbdev, > + unsigned short addr, > + unsigned short *data, > + unsigned short len_in_bits) > +{ > + unsigned short buffer = 0; > + > + if (rsi_usb_reg_write(usbdev, > + GSPI_DATA_REG2, > + *(unsigned short *)&data[2], 2) < 0) > + return -1; > + > + if (rsi_usb_reg_write(usbdev, > + GSPI_DATA_REG1, > + ((addr << 6) | (data[1] & 0x3f)), > + 2) < 0) > + return -1; > + > + if (rsi_usb_reg_write(usbdev, > + GSPI_DATA_REG0, > + *(unsigned short *)&data[0], > + 2) < 0) > + return -1; > + > + if (rsi_usb_reg_read(usbdev, GSPI_CTRL_REG0, &buffer, 2) < 0) > + return -1; > + > + buffer &= ~GSPI_2_ULP; > + > + if (rsi_usb_reg_write(usbdev, GSPI_CTRL_REG0, buffer, 2) < 0) > + return -1; > + > + if (rsi_usb_reg_write(usbdev, > + GSPI_CTRL_REG1, > + ((len_in_bits - 1) | GSPI_TRIG), > + 2) < 0) > + return -1; > + > + if (rsi_usb_reg_read(usbdev, GSPI_CTRL_REG0, &buffer, 2) < 0) > + return -1; > + > + buffer |= GSPI_2_ULP; > + > + if (rsi_usb_reg_write(usbdev, GSPI_CTRL_REG0, buffer, 2) < 0) > + return -1; > + return 0; > +} > + > +/** > + * This function resets the USB card. > + * > + * @param pfunction Pointer to the USB interface structure. > + * @return 0 on success, -1 on failure. > + */ > +static int rsi_reset_card(struct usb_interface *pfunction) > +{ > + struct usb_device *usbdev = interface_to_usbdev(pfunction); > + unsigned short temp[4]; > + > + memset(temp, 0, sizeof(temp)); > + *(unsigned int *)temp = 10; > + if (rsi_ulp_read_write(usbdev, WATCH_DOG_TIMER_1, &temp[0], 32) < 0) > + return -1; > + > + *(unsigned int *)temp = 0; > + if (rsi_ulp_read_write(usbdev, WATCH_DOG_TIMER_2, temp, 32) < 0) > + return -1; > + > + *(unsigned int *)temp = 1; > + if (rsi_ulp_read_write(usbdev, WATCH_DOG_DELAY_TIMER_1, temp, 32) < 0) > + return -1; > + *(unsigned int *)temp = 0; > + if (rsi_ulp_read_write(usbdev, WATCH_DOG_DELAY_TIMER_2, temp, 32) < 0) > + return -1; > + > + *(unsigned int *)temp = ((0xaa000) | RESTART_WDT | BYPASS_ULP_ON_WDT); > + if (rsi_ulp_read_write(usbdev, WATCH_DOG_TIMER_ENABLE, temp, 32) < 0) > + return -1; > + > + return 0; > +} > + > +/** > + * This function is called by kernel when the driver provided > + * Vendor and device IDs are matched. All the initialization > + * work is done here. > + * > + * @param pfunction Pointer to the USB interface structure. > + * @param id Pointer to the usb_device_id structure. > + * @return 0 on success, -1 on failure. > + */ > +static int rsi_probe(struct usb_interface *pfunction, > + const struct usb_device_id *id) > +{ > + struct rsi_hw *adapter; > + > + rsi_dbg(INIT_ZONE, "%s: Init function called\n", __func__); > + > + if (reset_card) { > + rsi_dbg(INIT_ZONE, "%s: Resetting card here\n", __func__); > + reset_card = 0; > + if (rsi_reset_card(pfunction) < 0) { > + rsi_dbg(ERR_ZONE, "%s: Card reset failed\n", __func__); > + return 1; > + } else { > + return 0; > + } > + } Ok, reset_card... The first time a card is probed by the module, it resets the device. Then, if on any further probe or if the card fails to initialize, reset_card is set to 1, and then the *next* time probe() is called the card is reset? That's quite hacktastic. If the driver always wants to ensure clean firmware state when it loads, it should reset the device any time firmware is loaded and alive to clear that firmware and get some new firmware. If there are any errors, just reset the card. Since any reset of the card (presumably!) causes the device to drop off the bus, probe() will simply get re-run when the firmware reboots. No need to dance around with reset_card across probe()s. > + adapter = rsi_init_os_intf_ops(); > + if (!adapter) { > + rsi_dbg(ERR_ZONE, "%s: Failed to init os intf ops\n", > + __func__); > + return 1; > + } > + > + if (rsi_init_usb_interface(adapter, pfunction)) { > + rsi_dbg(ERR_ZONE, "%s: Failed to init usb interface\n", > + __func__); > + goto fail; > + } > + > + rsi_dbg(ERR_ZONE, "%s: Initialized os intf ops\n", __func__); > + > + if (rsi_usb_reg_read(adapter->usbdev, 0x41050012, &fw, 2) < 0) There should really be a #define for this magic number. > + goto fail; > + else > + fw &= 1; There should be some comments about 'fw' here, what does the returned value from rsi_usb_reg_read() mean? Does it mean the firmware is alive? If so, then it should be named something more descriptive. > + > + if (rsi_device_init(adapter->priv, fw)) { And it was a bit unclear to track down this usage, where passing anything > 0 to rsi_device_init() really just causes rsi_load_ta_instructions() to fill in a common firmware version struct from the on-disk firmware file (but not running firmware?), and at least for USB, not to attempt to send more firmware to the device. > + rsi_dbg(ERR_ZONE, "%s: Failed in device init\n", > + __func__); > + goto fail; > + } > + > + if (!fw) { > + if (rsi_usb_reg_write(adapter->usbdev, 0x25000, 0xab, 1) < 0) More magic numbers, these should really have defines. > + goto fail; > + rsi_dbg(INIT_ZONE, "%s: Performed device init\n", __func__); > + } else { > + reset_card = 1; > + } > + > + if (rsi_rx_urb_submit(adapter)) > + goto fail; > + > + return 0; > +fail: > + reset_card = 1; > + rsi_deinit_os_intf_ops(adapter); > + rsi_dbg(ERR_ZONE, "%s: Failed in probe...Exiting\n", __func__); > + return 1; > +} > + > +/** > + * This function performs the reverse of the probe function, > + * it deintialize the driver structure. > + * > + * @param pfunction Pointer to the USB interface structure. > + * @return None. > + */ > +static void rsi_disconnect(struct usb_interface *pfunction) > +{ > + struct rsi_hw *adapter = usb_get_intfdata(pfunction); > + > + if (!adapter) > + return; > + > + rsi_device_deinit(adapter); > + rsi_deinit_os_intf_ops(adapter); > + > + rsi_dbg(INFO_ZONE, "%s: Deinitialization completed\n", __func__); > + return; > +} > + > +#ifdef CONFIG_PM > +static int rsi_suspend(struct usb_interface *intf, pm_message_t message) > +{ > + /* Not yet implemented */ > + return -ENOSYS; > +} > + > +static int rsi_resume(struct usb_interface *intf) > +{ > + /* Not yet implemented */ > + return -ENOSYS; > +} > +#endif > + > +static const struct usb_device_id rsi_dev_table[] = { > + { USB_DEVICE(0x0303, 0x0100) }, > + { USB_DEVICE(0x041B, 0x0301) }, > + { USB_DEVICE(0x041B, 0x0201) }, > + { USB_DEVICE(0x041B, 0x9330) }, > + { /* Blank */}, > +}; > + > +static struct usb_driver rsi_driver = { > + .name = "RSI-USB WLAN", > + .probe = rsi_probe, > + .disconnect = rsi_disconnect, > + .id_table = rsi_dev_table, > +#ifdef CONFIG_PM > + .suspend = rsi_suspend, > + .resume = rsi_resume, > +#endif > +}; > + > +/** > + * This function registers the client driver. > + * > + * @param Void. > + * @return 0 on success. > + */ > +static int rsi_module_init(void) > +{ > + usb_register(&rsi_driver); > + rsi_dbg(INIT_ZONE, "%s: Registering driver\n", __func__); > + return 0; > +} > + > +/** > + * This function unregisters the client driver. > + * > + * @param Void. > + * @return None. > + */ > +static void rsi_module_exit(void) > +{ > + usb_deregister(&rsi_driver); > + rsi_dbg(INFO_ZONE, "%s: Unregistering driver\n", __func__); > + return; > +} > + > +module_init(rsi_module_init); > +module_exit(rsi_module_exit); > + > +MODULE_AUTHOR("Redpine Signals Inc"); > +MODULE_DESCRIPTION("Common layer for RSI drivers"); > +MODULE_SUPPORTED_DEVICE("RSI-91x"); > +MODULE_VERSION("0.1"); > +MODULE_LICENSE("GPL"); As mentioned in other patches, if the firmware that's being loaded by rsi_load_ta_instructions() is really bus-type dependent (eg, USB or SDIO) then each bus type module should have MODULE_FIRMWARE() tags too. Dan -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html