Re: [PATCH 1/8] ath9k: Determine Firmware on probe

[Luis R. Rodriguez]

On Wed, Jun 2, 2010 at 3:23 AM, Sujith <Sujith.Manoharan@xxxxxxxxxxx> wrote:
> Do not assign the FW name to driver_info but determine
> it dynamically on device probe. This facilitates adding new
> firmware.
>
> Signed-off-by: Sujith <Sujith.Manoharan@xxxxxxxxxxx>
> ---
> Âdrivers/net/wireless/ath/ath9k/hif_usb.c | Â 39 ++++++++++++++++++++----------
> Âdrivers/net/wireless/ath/ath9k/hif_usb.h | Â Â1 +
> Â2 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
> index 77b3591..7da55eb 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.c
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
> @@ -16,12 +16,9 @@
>
> Â#include "htc.h"
>
> -#define ATH9K_FW_USB_DEV(devid, fw) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> - Â Â Â { USB_DEVICE(0x0cf3, devid), .driver_info = (unsigned long) fw }
> -
> Âstatic struct usb_device_id ath9k_hif_usb_ids[] = {
> - Â Â Â ATH9K_FW_USB_DEV(0x9271, "ar9271.fw"),
> - Â Â Â ATH9K_FW_USB_DEV(0x1006, "ar9271.fw"),
> + Â Â Â { USB_DEVICE(0x0cf3, 0x9271) },
> + Â Â Â { USB_DEVICE(0x0cf3, 0x1006) },
> Â Â Â Â{ },
> Â};
>
> @@ -790,21 +787,21 @@ static int ath9k_hif_usb_download_fw(struct hif_device_usb *hif_dev)
> Â Â Â Â Â Â Â Âreturn -EIO;
>
> Â Â Â Âdev_info(&hif_dev->udev->dev, "ath9k_htc: Transferred FW: %s, size: %ld\n",
> - Â Â Â Â Â Â Â Â"ar9271.fw", (unsigned long) hif_dev->firmware->size);
> + Â Â Â Â Â Â Â Âhif_dev->fw_name, (unsigned long) hif_dev->firmware->size);
>
> Â Â Â Âreturn 0;
> Â}
>
> -static int ath9k_hif_usb_dev_init(struct hif_device_usb *hif_dev,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â const char *fw_name)
> +static int ath9k_hif_usb_dev_init(struct hif_device_usb *hif_dev)
> Â{
> Â Â Â Âint ret;
>
> Â Â Â Â/* Request firmware */
> - Â Â Â ret = request_firmware(&hif_dev->firmware, fw_name, &hif_dev->udev->dev);
> + Â Â Â ret = request_firmware(&hif_dev->firmware, hif_dev->fw_name,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â&hif_dev->udev->dev);
> Â Â Â Âif (ret) {
> Â Â Â Â Â Â Â Âdev_err(&hif_dev->udev->dev,
> - Â Â Â Â Â Â Â Â Â Â Â "ath9k_htc: Firmware - %s not found\n", fw_name);
> + Â Â Â Â Â Â Â Â Â Â Â "ath9k_htc: Firmware - %s not found\n", hif_dev->fw_name);
> Â Â Â Â Â Â Â Âgoto err_fw_req;
> Â Â Â Â}
>
> @@ -820,7 +817,8 @@ static int ath9k_hif_usb_dev_init(struct hif_device_usb *hif_dev,
> Â Â Â Âret = ath9k_hif_usb_download_fw(hif_dev);
> Â Â Â Âif (ret) {
> Â Â Â Â Â Â Â Âdev_err(&hif_dev->udev->dev,
> - Â Â Â Â Â Â Â Â Â Â Â "ath9k_htc: Firmware - %s download failed\n", fw_name);
> + Â Â Â Â Â Â Â Â Â Â Â "ath9k_htc: Firmware - %s download failed\n",
> + Â Â Â Â Â Â Â Â Â Â Â hif_dev->fw_name);
> Â Â Â Â Â Â Â Âgoto err_fw_download;
> Â Â Â Â}
>
> @@ -847,7 +845,6 @@ static int ath9k_hif_usb_probe(struct usb_interface *interface,
> Â{
> Â Â Â Âstruct usb_device *udev = interface_to_usbdev(interface);
> Â Â Â Âstruct hif_device_usb *hif_dev;
> - Â Â Â const char *fw_name = (const char *) id->driver_info;
> Â Â Â Âint ret = 0;
>
> Â Â Â Âhif_dev = kzalloc(sizeof(struct hif_device_usb), GFP_KERNEL);
> @@ -872,7 +869,23 @@ static int ath9k_hif_usb_probe(struct usb_interface *interface,
> Â Â Â Â Â Â Â Âgoto err_htc_hw_alloc;
> Â Â Â Â}
>
> - Â Â Â ret = ath9k_hif_usb_dev_init(hif_dev, fw_name);
> + Â Â Â /* Find out which firmware to load */
> +
> + Â Â Â switch(hif_dev->device_id) {
> + Â Â Â case 0x9271:
> + Â Â Â case 0x1006:
> + Â Â Â Â Â Â Â hif_dev->fw_name = "ar9271.fw";
> + Â Â Â Â Â Â Â break;
> + Â Â Â default:
> + Â Â Â Â Â Â Â break;
> + Â Â Â }
> +
> + Â Â Â if (!hif_dev->fw_name) {
> + Â Â Â Â Â Â Â dev_err(&udev->dev, "Can't determine firmware !\n");
> + Â Â Â Â Â Â Â goto err_htc_hw_alloc;
> + Â Â Â }
> +
> + Â Â Â ret = ath9k_hif_usb_dev_init(hif_dev);
> Â Â Â Âif (ret) {
> Â Â Â Â Â Â Â Âret = -EINVAL;
> Â Â Â Â Â Â Â Âgoto err_hif_init_usb;
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
> index 0aca49b..b2647e8 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.h
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
> @@ -90,6 +90,7 @@ struct hif_device_usb {
> Â Â Â Âstruct usb_anchor regout_submitted;
> Â Â Â Âstruct usb_anchor rx_submitted;
> Â Â Â Âstruct sk_buff *remain_skb;
> + Â Â Â const char *fw_name;
> Â Â Â Âint rx_remain_len;
> Â Â Â Âint rx_pkt_len;
> Â Â Â Âint rx_transfer_len;

I had done some something similar for ar9170 a while back [1] when
adding AVM FRITZ support to ar9170 and then got complaints from Marcel
on this approach since it requires synching the PCI device IDs in two
places. I frankly don't care how we do this but we should try to at
least stick to one way of doing this.

[1] http://osdir.com/ml/linux-wireless/2009-05/msg01219.html

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/