From: Wen-chien Jesse Sung <[email protected]>
There is an user space firmware loading tool which can be found at
http://marc.info/?l=linux-bluetooth&m=132039175324993&w=2
This tool requires hci device to do the firmware loading, but this
may cause some race condition between patchram tool and bluetoothd
or something that also works on hci interface.
Also it needs some hooks to make firmware loads after bootup, s3,
s4, rfkill, and device hotplug events. Implement this loader in kernel
module would make things more easier.
These patches are sent to the mailing list for comment on Aug-15:
http://marc.info/?t=134495475100008&r=1&w=2
No further comment since then. I've rebased these patches against
current bluetooth git tree and would like to propose them for merging.
Wen-chien Jesse Sung (2):
Implement broadcom patchram firmware loader
Cache firmware images for later use
drivers/bluetooth/btusb.c | 148 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 143 insertions(+), 5 deletions(-)
--
1.7.9.5
Hi Jesse,
> From: Wen-chien Jesse Sung <[email protected]>
>
>
> Signed-off-by: Wen-chien Jesse Sung <[email protected]>
please learn on how to write commit messages. I want a full blown commit
messages with /sys/kernel/debug/usb/devices output (before and after)
and something that explains what is actually done.
> ---
> drivers/bluetooth/btusb.c | 103 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 99 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 654e248..7189fed 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -23,6 +23,8 @@
>
> #include <linux/module.h>
> #include <linux/usb.h>
> +#include <linux/delay.h>
> +#include <linux/firmware.h>
>
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
> @@ -47,6 +49,7 @@ static struct usb_driver btusb_driver;
> #define BTUSB_BROKEN_ISOC 0x20
> #define BTUSB_WRONG_SCO_MTU 0x40
> #define BTUSB_ATH3012 0x80
> +#define BTUSB_BCM_PATCHRAM 0x100
>
> static struct usb_device_id btusb_table[] = {
> /* Generic Bluetooth USB device */
> @@ -96,14 +99,15 @@ static struct usb_device_id btusb_table[] = {
> { USB_DEVICE(0x0c10, 0x0000) },
>
> /* Broadcom BCM20702A0 */
> + { USB_DEVICE(0x0489, 0xe031), .driver_info = BTUSB_BCM_PATCHRAM },
> { USB_DEVICE(0x0489, 0xe042) },
> - { USB_DEVICE(0x413c, 0x8197) },
> + { USB_DEVICE(0x413c, 0x8197), .driver_info = BTUSB_BCM_PATCHRAM },
These drivers did work without firmware before. So why is this change
required? Or is that Broadcom wide?
> /* Foxconn - Hon Hai */
> { USB_DEVICE(0x0489, 0xe033) },
>
> /*Broadcom devices with vendor specific id */
> - { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01) },
> + { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01), .driver_info = BTUSB_BCM_PATCHRAM },
>
> { } /* Terminating entry */
> };
> @@ -197,6 +201,37 @@ static struct usb_device_id blacklist_table[] = {
> { } /* Terminating entry */
> };
>
> +#define PATCHRAM_TIMEOUT 1000
> +#define FW_0489_E031 "fw-0489_e031.hcd"
> +#define FW_0A5C_21D3 "fw-0a5c_21d3.hcd"
> +#define FW_0A5C_21D7 "fw-0a5c_21d7.hcd"
> +#define FW_0A5C_21E6 "fw-0a5c_21e6.hcd"
> +#define FW_0A5C_21F3 "fw-0a5c_21f3.hcd"
> +#define FW_0A5C_21F4 "fw-0a5c_21f4.hcd"
> +#define FW_413C_8197 "fw-413c_8197.hcd"
> +
> +MODULE_FIRMWARE(FW_0489_E031);
> +MODULE_FIRMWARE(FW_0A5C_21D3);
> +MODULE_FIRMWARE(FW_0A5C_21D7);
> +MODULE_FIRMWARE(FW_0A5C_21E6);
> +MODULE_FIRMWARE(FW_0A5C_21F3);
> +MODULE_FIRMWARE(FW_0A5C_21F4);
> +MODULE_FIRMWARE(FW_413C_8197);
> +
> +static struct usb_device_id patchram_table[] = {
> + /* Dell DW1704 */
> + { USB_DEVICE(0x0a5c, 0x21d3), .driver_info = (kernel_ulong_t) FW_0A5C_21D3 },
> + { USB_DEVICE(0x0a5c, 0x21d7), .driver_info = (kernel_ulong_t) FW_0A5C_21D7 },
> + /* Dell DW380 */
> + { USB_DEVICE(0x413c, 0x8197), .driver_info = (kernel_ulong_t) FW_413C_8197 },
> + /* FoxConn Hon Hai */
> + { USB_DEVICE(0x0489, 0xe031), .driver_info = (kernel_ulong_t) FW_0489_E031 },
> + /* Lenovo */
> + { USB_DEVICE(0x0a5c, 0x21e6), .driver_info = (kernel_ulong_t) FW_0A5C_21E6 },
> + { USB_DEVICE(0x0a5c, 0x21f3), .driver_info = (kernel_ulong_t) FW_0A5C_21F3 },
> + { USB_DEVICE(0x0a5c, 0x21f4), .driver_info = (kernel_ulong_t) FW_0A5C_21F4 },
> +};
> +
And this looks like totally wasted details to me. Either build the
firmware name from USB VID:PID or only include the ones that we are
actually supporting.
> #define BTUSB_MAX_ISOC_FRAMES 10
>
> #define BTUSB_INTR_RUNNING 0
> @@ -914,6 +949,55 @@ static void btusb_waker(struct work_struct *work)
> usb_autopm_put_interface(data->intf);
> }
>
> +static inline void load_patchram_fw(struct usb_device *udev, const struct usb_device_id *id)
> +{
> + size_t pos = 0;
> + int err = 0;
> + const struct firmware *fw;
> +
> + unsigned char reset_cmd[] = { 0x03, 0x0c, 0x00 };
> + unsigned char download_cmd[] = { 0x2e, 0xfc, 0x00 };
> +
> + if (request_firmware(&fw, (const char *) id->driver_info, &udev->dev) < 0) {
> + BT_INFO("can't load firmware, may not work correctly");
> + return;
> + }
> +
> + if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
> + reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0) {
> + err = -1;
> + goto out;
> + }
> + msleep(300);
> +
> + if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
> + download_cmd, sizeof(download_cmd), PATCHRAM_TIMEOUT) < 0) {
> + err = -1;
> + goto out;
> + }
> + msleep(300);
> +
> + while (pos < fw->size) {
> + size_t len;
> + len = fw->data[pos + 2] + 3;
> + if ((pos + len > fw->size) ||
> + (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0,
> + USB_TYPE_CLASS, 0, 0, (void *)fw->data + pos, len,
> + PATCHRAM_TIMEOUT) < 0)) {
> + err = -1;
> + goto out;
> + }
> + pos += len;
> + }
> +
> + err = (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
> + reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0);
> +out:
> + if (err)
> + BT_INFO("fail to load firmware, may not work correctly");
> + release_firmware(fw);
> +}
> +
> static int btusb_probe(struct usb_interface *intf,
> const struct usb_device_id *id)
> {
> @@ -1078,15 +1162,26 @@ static int btusb_probe(struct usb_interface *intf,
> }
> }
>
> + usb_set_intfdata(intf, data);
> +
> + if (id->driver_info & BTUSB_BCM_PATCHRAM) {
> + const struct usb_device_id *match;
> + match = usb_match_id(intf, patchram_table);
> + if (match) {
> + btusb_open(hdev);
> + load_patchram_fw(interface_to_usbdev(intf), match);
> + btusb_close(hdev);
> + }
> + }
> +
So we are now blocking every other USB devices on that bus here? I
actually do not like this idea very much.
Also the call of btusb_open() before hdev is actually registered is
kinda fishy to me. I am not even sure that works how you think it would.
And why can't Broadcom just change the PID once the patchram has been
loaded to something else. That way we can nicely iterate through this.
This also does not really belong in a standard driver. Quirks fine, but
a complete ROM patches procedure that is vendor specific.
As I said above, I want to see the /sys/kernel/debug/usb/devices from
before and after first. Until then NAK.
Regards
Marcel
From: Wen-chien Jesse Sung <[email protected]>
Since request_firmware() may fail when resume from suspend, store
used firmware image in ram to make sure that we can have what we need
on resume.
Signed-off-by: Wen-chien Jesse Sung <[email protected]>
---
drivers/bluetooth/btusb.c | 77 +++++++++++++++++++++++++++++++++++----------
1 file changed, 60 insertions(+), 17 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7189fed..8bf9e89 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -218,18 +218,34 @@ MODULE_FIRMWARE(FW_0A5C_21F3);
MODULE_FIRMWARE(FW_0A5C_21F4);
MODULE_FIRMWARE(FW_413C_8197);
+struct firmware_cache {
+ const char *filename;
+ u8* data;
+ size_t size;
+};
+
+static struct firmware_cache firmware[] = {
+ { .filename = FW_0A5C_21D3, },
+ { .filename = FW_0A5C_21D7, },
+ { .filename = FW_413C_8197, },
+ { .filename = FW_0489_E031, },
+ { .filename = FW_0A5C_21E6, },
+ { .filename = FW_0A5C_21F3, },
+ { .filename = FW_0A5C_21F4, },
+};
+
static struct usb_device_id patchram_table[] = {
/* Dell DW1704 */
- { USB_DEVICE(0x0a5c, 0x21d3), .driver_info = (kernel_ulong_t) FW_0A5C_21D3 },
- { USB_DEVICE(0x0a5c, 0x21d7), .driver_info = (kernel_ulong_t) FW_0A5C_21D7 },
+ { USB_DEVICE(0x0a5c, 0x21d3), .driver_info = (kernel_ulong_t) &firmware[0] },
+ { USB_DEVICE(0x0a5c, 0x21d7), .driver_info = (kernel_ulong_t) &firmware[1] },
/* Dell DW380 */
- { USB_DEVICE(0x413c, 0x8197), .driver_info = (kernel_ulong_t) FW_413C_8197 },
+ { USB_DEVICE(0x413c, 0x8197), .driver_info = (kernel_ulong_t) &firmware[2] },
/* FoxConn Hon Hai */
- { USB_DEVICE(0x0489, 0xe031), .driver_info = (kernel_ulong_t) FW_0489_E031 },
+ { USB_DEVICE(0x0489, 0xe031), .driver_info = (kernel_ulong_t) &firmware[3] },
/* Lenovo */
- { USB_DEVICE(0x0a5c, 0x21e6), .driver_info = (kernel_ulong_t) FW_0A5C_21E6 },
- { USB_DEVICE(0x0a5c, 0x21f3), .driver_info = (kernel_ulong_t) FW_0A5C_21F3 },
- { USB_DEVICE(0x0a5c, 0x21f4), .driver_info = (kernel_ulong_t) FW_0A5C_21F4 },
+ { USB_DEVICE(0x0a5c, 0x21e6), .driver_info = (kernel_ulong_t) &firmware[4] },
+ { USB_DEVICE(0x0a5c, 0x21f3), .driver_info = (kernel_ulong_t) &firmware[5] },
+ { USB_DEVICE(0x0a5c, 0x21f4), .driver_info = (kernel_ulong_t) &firmware[6] },
};
#define BTUSB_MAX_ISOC_FRAMES 10
@@ -953,14 +969,27 @@ static inline void load_patchram_fw(struct usb_device *udev, const struct usb_de
{
size_t pos = 0;
int err = 0;
- const struct firmware *fw;
+ struct firmware_cache *fwcache;
unsigned char reset_cmd[] = { 0x03, 0x0c, 0x00 };
unsigned char download_cmd[] = { 0x2e, 0xfc, 0x00 };
- if (request_firmware(&fw, (const char *) id->driver_info, &udev->dev) < 0) {
- BT_INFO("can't load firmware, may not work correctly");
- return;
+ fwcache = (struct firmware_cache *)id->driver_info;
+ if (!fwcache->data) {
+ const struct firmware *fw;
+ if (request_firmware(&fw, fwcache->filename, &udev->dev) < 0) {
+ BT_INFO("can't load firmware, may not work correctly");
+ return;
+ }
+ fwcache->data = kmalloc(fw->size, GFP_KERNEL);
+ if (!fwcache->data) {
+ BT_INFO("OOM");
+ release_firmware(fw);
+ return;
+ }
+ fwcache->size = fw->size;
+ memcpy(fwcache->data, fw->data, fwcache->size);
+ release_firmware(fw);
}
if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
@@ -977,12 +1006,12 @@ static inline void load_patchram_fw(struct usb_device *udev, const struct usb_de
}
msleep(300);
- while (pos < fw->size) {
+ while (pos < fwcache->size) {
size_t len;
- len = fw->data[pos + 2] + 3;
- if ((pos + len > fw->size) ||
+ len = fwcache->data[pos + 2] + 3;
+ if ((pos + len > fwcache->size) ||
(usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0,
- USB_TYPE_CLASS, 0, 0, (void *)fw->data + pos, len,
+ USB_TYPE_CLASS, 0, 0, (void *)(fwcache->data + pos), len,
PATCHRAM_TIMEOUT) < 0)) {
err = -1;
goto out;
@@ -995,7 +1024,6 @@ static inline void load_patchram_fw(struct usb_device *udev, const struct usb_de
out:
if (err)
BT_INFO("fail to load firmware, may not work correctly");
- release_firmware(fw);
}
static int btusb_probe(struct usb_interface *intf,
@@ -1326,7 +1354,22 @@ static struct usb_driver btusb_driver = {
.disable_hub_initiated_lpm = 1,
};
-module_usb_driver(btusb_driver);
+static int __init btusb_init(void)
+{
+ return usb_register(&btusb_driver);
+}
+
+static void __exit btusb_exit(void)
+{
+ int i;
+ for (i = 0; i < sizeof(firmware) / sizeof(firmware[0]); i++)
+ if (firmware[i].data)
+ kfree(firmware[i].data);
+ usb_deregister(&btusb_driver);
+}
+
+module_init(btusb_init);
+module_exit(btusb_exit);
module_param(ignore_dga, bool, 0644);
MODULE_PARM_DESC(ignore_dga, "Ignore devices with id 08fd:0001");
--
1.7.9.5
From: Wen-chien Jesse Sung <[email protected]>
Signed-off-by: Wen-chien Jesse Sung <[email protected]>
---
drivers/bluetooth/btusb.c | 103 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 99 insertions(+), 4 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 654e248..7189fed 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -23,6 +23,8 @@
#include <linux/module.h>
#include <linux/usb.h>
+#include <linux/delay.h>
+#include <linux/firmware.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -47,6 +49,7 @@ static struct usb_driver btusb_driver;
#define BTUSB_BROKEN_ISOC 0x20
#define BTUSB_WRONG_SCO_MTU 0x40
#define BTUSB_ATH3012 0x80
+#define BTUSB_BCM_PATCHRAM 0x100
static struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
@@ -96,14 +99,15 @@ static struct usb_device_id btusb_table[] = {
{ USB_DEVICE(0x0c10, 0x0000) },
/* Broadcom BCM20702A0 */
+ { USB_DEVICE(0x0489, 0xe031), .driver_info = BTUSB_BCM_PATCHRAM },
{ USB_DEVICE(0x0489, 0xe042) },
- { USB_DEVICE(0x413c, 0x8197) },
+ { USB_DEVICE(0x413c, 0x8197), .driver_info = BTUSB_BCM_PATCHRAM },
/* Foxconn - Hon Hai */
{ USB_DEVICE(0x0489, 0xe033) },
/*Broadcom devices with vendor specific id */
- { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01) },
+ { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01), .driver_info = BTUSB_BCM_PATCHRAM },
{ } /* Terminating entry */
};
@@ -197,6 +201,37 @@ static struct usb_device_id blacklist_table[] = {
{ } /* Terminating entry */
};
+#define PATCHRAM_TIMEOUT 1000
+#define FW_0489_E031 "fw-0489_e031.hcd"
+#define FW_0A5C_21D3 "fw-0a5c_21d3.hcd"
+#define FW_0A5C_21D7 "fw-0a5c_21d7.hcd"
+#define FW_0A5C_21E6 "fw-0a5c_21e6.hcd"
+#define FW_0A5C_21F3 "fw-0a5c_21f3.hcd"
+#define FW_0A5C_21F4 "fw-0a5c_21f4.hcd"
+#define FW_413C_8197 "fw-413c_8197.hcd"
+
+MODULE_FIRMWARE(FW_0489_E031);
+MODULE_FIRMWARE(FW_0A5C_21D3);
+MODULE_FIRMWARE(FW_0A5C_21D7);
+MODULE_FIRMWARE(FW_0A5C_21E6);
+MODULE_FIRMWARE(FW_0A5C_21F3);
+MODULE_FIRMWARE(FW_0A5C_21F4);
+MODULE_FIRMWARE(FW_413C_8197);
+
+static struct usb_device_id patchram_table[] = {
+ /* Dell DW1704 */
+ { USB_DEVICE(0x0a5c, 0x21d3), .driver_info = (kernel_ulong_t) FW_0A5C_21D3 },
+ { USB_DEVICE(0x0a5c, 0x21d7), .driver_info = (kernel_ulong_t) FW_0A5C_21D7 },
+ /* Dell DW380 */
+ { USB_DEVICE(0x413c, 0x8197), .driver_info = (kernel_ulong_t) FW_413C_8197 },
+ /* FoxConn Hon Hai */
+ { USB_DEVICE(0x0489, 0xe031), .driver_info = (kernel_ulong_t) FW_0489_E031 },
+ /* Lenovo */
+ { USB_DEVICE(0x0a5c, 0x21e6), .driver_info = (kernel_ulong_t) FW_0A5C_21E6 },
+ { USB_DEVICE(0x0a5c, 0x21f3), .driver_info = (kernel_ulong_t) FW_0A5C_21F3 },
+ { USB_DEVICE(0x0a5c, 0x21f4), .driver_info = (kernel_ulong_t) FW_0A5C_21F4 },
+};
+
#define BTUSB_MAX_ISOC_FRAMES 10
#define BTUSB_INTR_RUNNING 0
@@ -914,6 +949,55 @@ static void btusb_waker(struct work_struct *work)
usb_autopm_put_interface(data->intf);
}
+static inline void load_patchram_fw(struct usb_device *udev, const struct usb_device_id *id)
+{
+ size_t pos = 0;
+ int err = 0;
+ const struct firmware *fw;
+
+ unsigned char reset_cmd[] = { 0x03, 0x0c, 0x00 };
+ unsigned char download_cmd[] = { 0x2e, 0xfc, 0x00 };
+
+ if (request_firmware(&fw, (const char *) id->driver_info, &udev->dev) < 0) {
+ BT_INFO("can't load firmware, may not work correctly");
+ return;
+ }
+
+ if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
+ reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0) {
+ err = -1;
+ goto out;
+ }
+ msleep(300);
+
+ if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
+ download_cmd, sizeof(download_cmd), PATCHRAM_TIMEOUT) < 0) {
+ err = -1;
+ goto out;
+ }
+ msleep(300);
+
+ while (pos < fw->size) {
+ size_t len;
+ len = fw->data[pos + 2] + 3;
+ if ((pos + len > fw->size) ||
+ (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0,
+ USB_TYPE_CLASS, 0, 0, (void *)fw->data + pos, len,
+ PATCHRAM_TIMEOUT) < 0)) {
+ err = -1;
+ goto out;
+ }
+ pos += len;
+ }
+
+ err = (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
+ reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0);
+out:
+ if (err)
+ BT_INFO("fail to load firmware, may not work correctly");
+ release_firmware(fw);
+}
+
static int btusb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
@@ -1078,15 +1162,26 @@ static int btusb_probe(struct usb_interface *intf,
}
}
+ usb_set_intfdata(intf, data);
+
+ if (id->driver_info & BTUSB_BCM_PATCHRAM) {
+ const struct usb_device_id *match;
+ match = usb_match_id(intf, patchram_table);
+ if (match) {
+ btusb_open(hdev);
+ load_patchram_fw(interface_to_usbdev(intf), match);
+ btusb_close(hdev);
+ }
+ }
+
err = hci_register_dev(hdev);
if (err < 0) {
hci_free_dev(hdev);
+ usb_set_intfdata(intf, NULL);
kfree(data);
return err;
}
- usb_set_intfdata(intf, data);
-
return 0;
}
--
1.7.9.5
Hi Jesse,
> >> static int btusb_probe(struct usb_interface *intf,
> >> const struct usb_device_id *id)
> >> {
> >> @@ -1078,15 +1162,26 @@ static int btusb_probe(struct usb_interface *intf,
> >> }
> >> }
> >>
> >> + usb_set_intfdata(intf, data);
> >> +
> >> + if (id->driver_info & BTUSB_BCM_PATCHRAM) {
> >> + const struct usb_device_id *match;
> >> + match = usb_match_id(intf, patchram_table);
> >> + if (match) {
> >> + btusb_open(hdev);
> >> + load_patchram_fw(interface_to_usbdev(intf), match);
> >> + btusb_close(hdev);
> >> + }
> >> + }
> >> +
> >
> > So we are now blocking every other USB devices on that bus here? I
> > actually do not like this idea very much.
>
> Do you mean the usleep() in load_patchram_fw()? These only affects people
> who have this kind of device, so the impact should be limited.
and what about the global USB lock that is hold when probing devices.
> > Also the call of btusb_open() before hdev is actually registered is
> > kinda fishy to me. I am not even sure that works how you think it would.
>
> Humm.. If there are concerns about calling btusb_open() and btusb_close(),
> I'll try to find another way to hook usb callbacks.
They are callbacks exposed from hci_dev and not for direct calling.
I have been telling this before multiple times. If you require a
firmware patching stage, then integrate that into hci_dev directly and
use the core for driving this properly.
> > And why can't Broadcom just change the PID once the patchram has been
> > loaded to something else. That way we can nicely iterate through this.
>
> Unfortunately that's how they do it now, and these modules are already in
> users' machines.
Some companies never ever learn. Most funny is that we had the bcm203x
driver that was doing this correctly. And 8 years later they have
forgotten all about it.
Regards
Marcel
Hi Marcel,
2012/9/1 Marcel Holtmann <[email protected]>:
> Hi Jesse,
>
>> From: Wen-chien Jesse Sung <[email protected]>
>>
>>
>> Signed-off-by: Wen-chien Jesse Sung <[email protected]>
>
> please learn on how to write commit messages. I want a full blown commit
> messages with /sys/kernel/debug/usb/devices output (before and after)
> and something that explains what is actually done.
Sorry, I'll try to do that better next time.
>> ---
>> drivers/bluetooth/btusb.c | 103 +++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 99 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index 654e248..7189fed 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -23,6 +23,8 @@
>>
>> #include <linux/module.h>
>> #include <linux/usb.h>
>> +#include <linux/delay.h>
>> +#include <linux/firmware.h>
>>
>> #include <net/bluetooth/bluetooth.h>
>> #include <net/bluetooth/hci_core.h>
>> @@ -47,6 +49,7 @@ static struct usb_driver btusb_driver;
>> #define BTUSB_BROKEN_ISOC 0x20
>> #define BTUSB_WRONG_SCO_MTU 0x40
>> #define BTUSB_ATH3012 0x80
>> +#define BTUSB_BCM_PATCHRAM 0x100
>>
>> static struct usb_device_id btusb_table[] = {
>> /* Generic Bluetooth USB device */
>> @@ -96,14 +99,15 @@ static struct usb_device_id btusb_table[] = {
>> { USB_DEVICE(0x0c10, 0x0000) },
>>
>> /* Broadcom BCM20702A0 */
>> + { USB_DEVICE(0x0489, 0xe031), .driver_info = BTUSB_BCM_PATCHRAM },
>> { USB_DEVICE(0x0489, 0xe042) },
>> - { USB_DEVICE(0x413c, 0x8197) },
>> + { USB_DEVICE(0x413c, 0x8197), .driver_info = BTUSB_BCM_PATCHRAM },
>
> These drivers did work without firmware before. So why is this change
> required? Or is that Broadcom wide?
Currently patchram applies to BCM43142 and BCM20702 modules. Dunno if
there will be
more or not.
Some of these modules have PROM or something like that holding the
needed images.
For them, adding IDs to this table is enough for them to work. But as
what we have found,
these images may have bugs and need an updated image to be loaded to
fix the bugs.
Since failed to load firmware may not trigger an error while doing
probe, these kind
of modules will still work even if there's no firmware provided.
For other modules that do not have images with them, patchram is
needed to bring them
into a working state.
>> /* Foxconn - Hon Hai */
>> { USB_DEVICE(0x0489, 0xe033) },
>>
>> /*Broadcom devices with vendor specific id */
>> - { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01) },
>> + { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01), .driver_info = BTUSB_BCM_PATCHRAM },
>>
>> { } /* Terminating entry */
>> };
>> @@ -197,6 +201,37 @@ static struct usb_device_id blacklist_table[] = {
>> { } /* Terminating entry */
>> };
>>
>> +#define PATCHRAM_TIMEOUT 1000
>> +#define FW_0489_E031 "fw-0489_e031.hcd"
>> +#define FW_0A5C_21D3 "fw-0a5c_21d3.hcd"
>> +#define FW_0A5C_21D7 "fw-0a5c_21d7.hcd"
>> +#define FW_0A5C_21E6 "fw-0a5c_21e6.hcd"
>> +#define FW_0A5C_21F3 "fw-0a5c_21f3.hcd"
>> +#define FW_0A5C_21F4 "fw-0a5c_21f4.hcd"
>> +#define FW_413C_8197 "fw-413c_8197.hcd"
>> +
>> +MODULE_FIRMWARE(FW_0489_E031);
>> +MODULE_FIRMWARE(FW_0A5C_21D3);
>> +MODULE_FIRMWARE(FW_0A5C_21D7);
>> +MODULE_FIRMWARE(FW_0A5C_21E6);
>> +MODULE_FIRMWARE(FW_0A5C_21F3);
>> +MODULE_FIRMWARE(FW_0A5C_21F4);
>> +MODULE_FIRMWARE(FW_413C_8197);
>> +
>> +static struct usb_device_id patchram_table[] = {
>> + /* Dell DW1704 */
>> + { USB_DEVICE(0x0a5c, 0x21d3), .driver_info = (kernel_ulong_t) FW_0A5C_21D3 },
>> + { USB_DEVICE(0x0a5c, 0x21d7), .driver_info = (kernel_ulong_t) FW_0A5C_21D7 },
>> + /* Dell DW380 */
>> + { USB_DEVICE(0x413c, 0x8197), .driver_info = (kernel_ulong_t) FW_413C_8197 },
>> + /* FoxConn Hon Hai */
>> + { USB_DEVICE(0x0489, 0xe031), .driver_info = (kernel_ulong_t) FW_0489_E031 },
>> + /* Lenovo */
>> + { USB_DEVICE(0x0a5c, 0x21e6), .driver_info = (kernel_ulong_t) FW_0A5C_21E6 },
>> + { USB_DEVICE(0x0a5c, 0x21f3), .driver_info = (kernel_ulong_t) FW_0A5C_21F3 },
>> + { USB_DEVICE(0x0a5c, 0x21f4), .driver_info = (kernel_ulong_t) FW_0A5C_21F4 },
>> +};
>> +
>
> And this looks like totally wasted details to me. Either build the
> firmware name from USB VID:PID or only include the ones that we are
> actually supporting.
I'll get the firmware name from VID and PID instead.
>> #define BTUSB_MAX_ISOC_FRAMES 10
>>
>> #define BTUSB_INTR_RUNNING 0
>> @@ -914,6 +949,55 @@ static void btusb_waker(struct work_struct *work)
>> usb_autopm_put_interface(data->intf);
>> }
>>
>> +static inline void load_patchram_fw(struct usb_device *udev, const struct usb_device_id *id)
>> +{
>> + size_t pos = 0;
>> + int err = 0;
>> + const struct firmware *fw;
>> +
>> + unsigned char reset_cmd[] = { 0x03, 0x0c, 0x00 };
>> + unsigned char download_cmd[] = { 0x2e, 0xfc, 0x00 };
>> +
>> + if (request_firmware(&fw, (const char *) id->driver_info, &udev->dev) < 0) {
>> + BT_INFO("can't load firmware, may not work correctly");
>> + return;
>> + }
>> +
>> + if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
>> + reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0) {
>> + err = -1;
>> + goto out;
>> + }
>> + msleep(300);
>> +
>> + if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
>> + download_cmd, sizeof(download_cmd), PATCHRAM_TIMEOUT) < 0) {
>> + err = -1;
>> + goto out;
>> + }
>> + msleep(300);
>> +
>> + while (pos < fw->size) {
>> + size_t len;
>> + len = fw->data[pos + 2] + 3;
>> + if ((pos + len > fw->size) ||
>> + (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0,
>> + USB_TYPE_CLASS, 0, 0, (void *)fw->data + pos, len,
>> + PATCHRAM_TIMEOUT) < 0)) {
>> + err = -1;
>> + goto out;
>> + }
>> + pos += len;
>> + }
>> +
>> + err = (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
>> + reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0);
>> +out:
>> + if (err)
>> + BT_INFO("fail to load firmware, may not work correctly");
>> + release_firmware(fw);
>> +}
>> +
>> static int btusb_probe(struct usb_interface *intf,
>> const struct usb_device_id *id)
>> {
>> @@ -1078,15 +1162,26 @@ static int btusb_probe(struct usb_interface *intf,
>> }
>> }
>>
>> + usb_set_intfdata(intf, data);
>> +
>> + if (id->driver_info & BTUSB_BCM_PATCHRAM) {
>> + const struct usb_device_id *match;
>> + match = usb_match_id(intf, patchram_table);
>> + if (match) {
>> + btusb_open(hdev);
>> + load_patchram_fw(interface_to_usbdev(intf), match);
>> + btusb_close(hdev);
>> + }
>> + }
>> +
>
> So we are now blocking every other USB devices on that bus here? I
> actually do not like this idea very much.
Do you mean the usleep() in load_patchram_fw()? These only affects people
who have this kind of device, so the impact should be limited.
> Also the call of btusb_open() before hdev is actually registered is
> kinda fishy to me. I am not even sure that works how you think it would.
Humm.. If there are concerns about calling btusb_open() and btusb_close(),
I'll try to find another way to hook usb callbacks.
> And why can't Broadcom just change the PID once the patchram has been
> loaded to something else. That way we can nicely iterate through this.
Unfortunately that's how they do it now, and these modules are already in
users' machines.
> This also does not really belong in a standard driver. Quirks fine, but
> a complete ROM patches procedure that is vendor specific.
>
> As I said above, I want to see the /sys/kernel/debug/usb/devices from
> before and after first. Until then NAK.
before:
T: Bus=01 Lev=02 Prnt=02 Port=04 Cnt=02 Dev#= 4 Spd=12 MxCh= 0
D: Ver= 2.00 Cls=ff(vend.) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=0a5c ProdID=21d3 Rev= 1.12
S: Manufacturer=Broadcom Corp
S: Product=BCM43142A0
S: SerialNumber=3859F9D6199A
C:* #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=01 Prot=01 Driver=(none)
E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=(none)
E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
I: If#= 1 Alt= 1 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=(none)
E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms
I: If#= 1 Alt= 2 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=(none)
E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms
I: If#= 1 Alt= 3 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=(none)
E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms
I: If#= 1 Alt= 4 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=(none)
E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms
I: If#= 1 Alt= 5 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=(none)
E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms
I:* If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
E: Ad=84(I) Atr=02(Bulk) MxPS= 32 Ivl=0ms
E: Ad=04(O) Atr=02(Bulk) MxPS= 32 Ivl=0ms
I:* If#= 3 Alt= 0 #EPs= 0 Cls=fe(app. ) Sub=01 Prot=01 Driver=(none)
after:
T: Bus=01 Lev=02 Prnt=02 Port=04 Cnt=02 Dev#= 4 Spd=12 MxCh= 0
D: Ver= 2.00 Cls=ff(vend.) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=0a5c ProdID=21d3 Rev= 1.12
S: Manufacturer=Broadcom Corp
S: Product=BCM43142A0
S: SerialNumber=3859F9D6199A
C:* #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=01 Prot=01 Driver=btusb
E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=btusb
E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
I: If#= 1 Alt= 1 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=btusb
E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms
I: If#= 1 Alt= 2 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=btusb
E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms
I: If#= 1 Alt= 3 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=btusb
E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms
I: If#= 1 Alt= 4 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=btusb
E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms
I: If#= 1 Alt= 5 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=btusb
E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms
E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms
I:* If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
E: Ad=84(I) Atr=02(Bulk) MxPS= 32 Ivl=0ms
E: Ad=04(O) Atr=02(Bulk) MxPS= 32 Ivl=0ms
I:* If#= 3 Alt= 0 #EPs= 0 Cls=fe(app. ) Sub=01 Prot=01 Driver=(none)
Thanks,
Jesse