| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAd53p5OM-deg1o=YuzLJ0-kjuj0ZJviMArT2BvghaceJRgVjw@mail.gmail.com>
Date: Thu, 24 Aug 2017 17:33:40 +0800
From: Kai-Heng Feng <kai.heng.feng@...onical.com>
To: Marcel Holtmann <marcel@...tmann.org>
Cc: "Gustavo F. Padovan" <gustavo@...ovan.org>,
Johan Hedberg <johan.hedberg@...il.com>,
linux-bluetooth@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] Bluetooth: btusb: ath3k: Cache firmware for already
patched Bluetooth chip
On Thu, Aug 24, 2017 at 5:15 PM, Marcel Holtmann <marcel@...tmann.org> wrote:
> Hi Kai-Heng,
>
>> When a system reboot, the USB power never gets cut off, so the firmware
>> is already updated on the Bluetooth chip.
>>
>> Several btusb setup functions check firmware updated status before
>> download firmware, the loading part will be skipped if it's updated.
>> Because the firmware is never asked by request_firmware(),
>> firmware_class does not know it needs to be cached before system enters
>> sleep.
>>
>> Now, system suspend/resume may cause the driver failed to request the
>> firmware because it's not in the firmware cache:
>>
>> [ 87.539434] firmware request while host is not available
>>
>> This can be solved by calling request_firmware() even if the chip is
>> already updated - now the firmware_class knows what to cache.
>>
>> In this case, we don't really need to wait for the firmware content, so
>> we use the async version of request_firmware().
>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@...onical.com>
>> ---
>> drivers/bluetooth/ath3k.c | 49 +++++++++++++++++++++++++++++++++++++++++
>> drivers/bluetooth/btusb.c | 56 +++++++++++++++++++++++++++++++++++++++++++++--
>> 2 files changed, 103 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
>> index 280849dba51e..27a7415f9fd7 100644
>> --- a/drivers/bluetooth/ath3k.c
>> +++ b/drivers/bluetooth/ath3k.c
>> @@ -208,6 +208,54 @@ static const struct usb_device_id ath3k_blist_tbl[] = {
>> #define TIMEGAP_USEC_MIN 50
>> #define TIMEGAP_USEC_MAX 100
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +static void ath3k_request_firmware_done(const struct firmware *firmware,
>> + void *context)
>> +{
>> + const char *name = (const char *)context;
>> +
>> + if (!firmware) {
>> + BT_WARN("firmware %s will not be cached", name);
>> + goto done;
>> + }
>> +
>> + BT_DBG("firmware %s will be cached", name);
>> +
>> + release_firmware(firmware);
>> +done:
>> + kfree_const(name);
>> +}
>> +
>> +static int ath3k_request_firmware_async(struct usb_device *udev,
>> + const char *fwname)
>> +{
>> + const char *name;
>> + int err;
>> +
>> + name = kstrdup_const(fwname, GFP_KERNEL);
>> + if (!name)
>> + return -ENOMEM;
>> +
>> + err = request_firmware_nowait(THIS_MODULE, true, name, &udev->dev,
>> + GFP_KERNEL, (void *)name,
>> + ath3k_request_firmware_done);
>> + if (err) {
>> + BT_WARN("%s %s: failed to async request firmware for file: %s (%d)",
>> + udev->manufacturer, udev->product, name, err);
>> + kfree_const(name);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +#else
>> +static int ath3k_request_firmware_async(struct usb_device *udev,
>> + const char *fwname)
>> +{
>> + return 0;
>> +}
>> +#endif
>> +
>> static int ath3k_load_firmware(struct usb_device *udev,
>> const struct firmware *firmware)
>> {
>> @@ -420,6 +468,7 @@ static int ath3k_load_patch(struct usb_device *udev)
>>
>> if (fw_state & ATH3K_PATCH_UPDATE) {
>> BT_DBG("Patch was already downloaded");
>> + ath3k_request_firmware_async(udev, filename);
>> return 0;
>> }
>>
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index 732fe6c3e789..7de2156debd8 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -1459,6 +1459,54 @@ static void btusb_waker(struct work_struct *work)
>> usb_autopm_put_interface(data->intf);
>> }
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +static void btusb_request_firmware_done(const struct firmware *firmware,
>> + void *context)
>> +{
>> + const char *name = (const char *)context;
>> +
>> + if (!firmware) {
>> + BT_WARN("firmware %s will not be cached", name);
>> + goto done;
>> + }
>> +
>> + BT_DBG("firmware %s will be cached", name);
>> +
>> + release_firmware(firmware);
>> +done:
>> + kfree_const(name);
>> +}
>> +
>> +static int btusb_request_firmware_async(struct hci_dev *hdev,
>> + const char *fwname)
>> +{
>> + const char *name;
>> + int err;
>> +
>> + name = kstrdup_const(fwname, GFP_KERNEL);
>> + if (!name)
>> + return -ENOMEM;
>> +
>> + err = request_firmware_nowait(THIS_MODULE, true, name, &hdev->dev,
>> + GFP_KERNEL, (void *)name,
>> + btusb_request_firmware_done);
>> + if (err) {
>> + BT_WARN("%s: failed to async request firmware for file: %s (%d)",
>> + hdev->name, name, err);
>> + kfree_const(name);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +#else
>> +static int btusb_request_firmware_async(struct hci_dev *hdev,
>> + const char *fwname)
>> +{
>> + return 0;
>> +}
>> +#endif
>> +
>> static int btusb_setup_bcm92035(struct hci_dev *hdev)
>> {
>> struct sk_buff *skb;
>> @@ -1724,6 +1772,8 @@ static int btusb_setup_intel(struct hci_dev *hdev)
>> if (ver.fw_patch_num) {
>> BT_INFO("%s: Intel device is already patched. patch num: %02x",
>> hdev->name, ver.fw_patch_num);
>> + btusb_request_firmware_async(hdev, fwname);
>> + btusb_request_firmware_async(hdev, default_fwname);
>> goto complete;
>> }
>
> I do not like intermixing of different vendors in a single patch.
Thanks, I'll split them into Intel/QCA/ath3k patches.
Do you have any concern about the approach?
>
> Regards
>
> Marcel
>
Powered by blists - more mailing lists