Hi Jesse,
> >> >> load_firmware will be called at the end of hci_dev_open() if it
> >> >> is defined.
> >> >>
> >> >> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@xxxxxxxxxxxxx>
> >> >> ---
> >> >> include/net/bluetooth/hci_core.h | 1 +
> >> >> net/bluetooth/hci_core.c | 2 ++
> >> >> 2 files changed, 3 insertions(+)
> >> >>
> >> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> >> index 593cd1d..40972a3 100644
> >> >> --- a/include/net/bluetooth/hci_core.h
> >> >> +++ b/include/net/bluetooth/hci_core.h
> >> >> @@ -281,6 +281,7 @@ struct hci_dev {
> >> >> int (*send)(struct sk_buff *skb);
> >> >> void (*notify)(struct hci_dev *hdev, unsigned int evt);
> >> >> int (*ioctl)(struct hci_dev *hdev, unsigned int cmd, unsigned long arg);
> >> >> + void (*load_firmware)(struct hci_dev *hdev);
> >> >> };
> >> >>
> >> >> struct hci_conn {
> >> >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >> >> index d4de5db..49be87a 100644
> >> >> --- a/net/bluetooth/hci_core.c
> >> >> +++ b/net/bluetooth/hci_core.c
> >> >> @@ -725,6 +725,8 @@ int hci_dev_open(__u16 dev)
> >> >> done:
> >> >> hci_req_unlock(hdev);
> >> >> hci_dev_put(hdev);
> >> >> + if (!ret && hdev->load_firmware)
> >> >> + hdev->load_firmware(hdev);
> >> >> return ret;
> >> >> }
> >> >>
> >> >
> >> > has anybody thought this through actually? Do we need to reload the
> >> > firmware after every HCI_Reset? Since hci_dev_open() is used at least
> >> > twice during normal operation. And for every RFKILL or power down/up
> >> > cycle of the chip.
> >> >
> >> > And there is an internal process of hci_dev_open() trigger on
> >> > registration and others triggered by hciconfig hci0 up. I am pretty much
> >> > against having to wait for all this firmware loading crap during every
> >> > bring up of the device. Especially since it always does a trip via
> >> > request_firmware().
> >>
> >> In the second patch, firmware loading would be done only once per
> >> power cycle of the chip. Since I think it should be the device driver, not hci,
> >> who knows when and how to load firmware, the lock is placed in btusb.c.
> >
> > and how does the driver knows these details? That makes no sense. How
> > does the driver know it got rebooted?
> >
> > The hci_dev_open() will start the transport. And as I explained before,
> > that can happen twice during boot time.
>
> Please take a look at the second part of this patchset, which is in patch 2/2.
> Loading firmware is needed when the chip is rebooted, and a reboot would trigger
> a probe in btusb. So btusb can know a firmware loading is needed whenever
> a new patchram device is probed. And the load_firmware callback in
> btusb would do
> test_and_set_bit to ensure that the loading process would only be done once.
that is horrible hackish. So NAK from my side. Look at what I told the
Intel guys to do on supporting their USB dongle. You have the same
problem and I want it solved in a similar way.
The probe() callback is actually not a good idea either btw. It is for
binding a driver. Binding and unbinding the driver has nothing to do
with reboot of the device.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
