* [PATCH 0/2] broadcom patchram firmware loader @ 2012-08-31 5:21 Jesse Sung 2012-08-31 5:21 ` [PATCH 1/2] Implement " Jesse Sung 2012-08-31 5:21 ` [PATCH 2/2] Cache firmware images for later use Jesse Sung 0 siblings, 2 replies; 6+ messages in thread From: Jesse Sung @ 2012-08-31 5:21 UTC (permalink / raw) To: linux-bluetooth From: Wen-chien Jesse Sung <jesse.sung@canonical.com> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] Implement broadcom patchram firmware loader 2012-08-31 5:21 [PATCH 0/2] broadcom patchram firmware loader Jesse Sung @ 2012-08-31 5:21 ` Jesse Sung 2012-08-31 16:11 ` Marcel Holtmann 2012-08-31 5:21 ` [PATCH 2/2] Cache firmware images for later use Jesse Sung 1 sibling, 1 reply; 6+ messages in thread From: Jesse Sung @ 2012-08-31 5:21 UTC (permalink / raw) To: linux-bluetooth From: Wen-chien Jesse Sung <jesse.sung@canonical.com> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Implement broadcom patchram firmware loader 2012-08-31 5:21 ` [PATCH 1/2] Implement " Jesse Sung @ 2012-08-31 16:11 ` Marcel Holtmann 2012-09-07 8:07 ` Jesse Sung 0 siblings, 1 reply; 6+ messages in thread From: Marcel Holtmann @ 2012-08-31 16:11 UTC (permalink / raw) To: Jesse Sung; +Cc: linux-bluetooth Hi Jesse, > From: Wen-chien Jesse Sung <jesse.sung@canonical.com> > > > Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Implement broadcom patchram firmware loader 2012-08-31 16:11 ` Marcel Holtmann @ 2012-09-07 8:07 ` Jesse Sung 2012-09-07 20:32 ` Marcel Holtmann 0 siblings, 1 reply; 6+ messages in thread From: Jesse Sung @ 2012-09-07 8:07 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth Hi Marcel, 2012/9/1 Marcel Holtmann <marcel@holtmann.org>: > Hi Jesse, > >> From: Wen-chien Jesse Sung <jesse.sung@canonical.com> >> >> >> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Implement broadcom patchram firmware loader 2012-09-07 8:07 ` Jesse Sung @ 2012-09-07 20:32 ` Marcel Holtmann 0 siblings, 0 replies; 6+ messages in thread From: Marcel Holtmann @ 2012-09-07 20:32 UTC (permalink / raw) To: Jesse Sung; +Cc: linux-bluetooth 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] Cache firmware images for later use 2012-08-31 5:21 [PATCH 0/2] broadcom patchram firmware loader Jesse Sung 2012-08-31 5:21 ` [PATCH 1/2] Implement " Jesse Sung @ 2012-08-31 5:21 ` Jesse Sung 1 sibling, 0 replies; 6+ messages in thread From: Jesse Sung @ 2012-08-31 5:21 UTC (permalink / raw) To: linux-bluetooth From: Wen-chien Jesse Sung <jesse.sung@canonical.com> 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 <jesse.sung@canonical.com> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-07 20:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-31 5:21 [PATCH 0/2] broadcom patchram firmware loader Jesse Sung 2012-08-31 5:21 ` [PATCH 1/2] Implement " Jesse Sung 2012-08-31 16:11 ` Marcel Holtmann 2012-09-07 8:07 ` Jesse Sung 2012-09-07 20:32 ` Marcel Holtmann 2012-08-31 5:21 ` [PATCH 2/2] Cache firmware images for later use Jesse Sung
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox