Re: [PATCH 1/2] Implement broadcom patchram firmware loader

[Marcel Holtmann <marcel@holtmann.org>]

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