2016-02-10 20:00:46

by Petri Gynther

Subject: [PATCH] Bluetooth: btbcm: Fix handling of firmware not found

If the call to request_firmware() fails in btbcm_setup_patchram(),
the BCM chip will be operating with its default firmware.

In this case, btbcm_setup_patchram() should not return immediately
but instead should skip to btbcm_check_bdaddr() and quirk setup.

Signed-off-by: Petri Gynther <[email protected]>
---
drivers/bluetooth/btbcm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 0b69794..7d82b1a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -467,7 +467,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
err = request_firmware(&fw, fw_name, &hdev->dev);
if (err < 0) {
BT_INFO("%s: BCM: Patch %s not found", hdev->name, fw_name);
- return 0;
+ goto check_bdaddr;
}

btbcm_patchram(hdev, fw);
@@ -501,6 +501,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
BT_INFO("%s: %s", hdev->name, (char *)(skb->data + 1));
kfree_skb(skb);

+check_bdaddr:
btbcm_check_bdaddr(hdev);

set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
--
2.7.0.rc3.207.g0ac5344

2016-02-20 01:40:24

by Petri Gynther

Subject: Re: [PATCH] Bluetooth: btbcm: Fix handling of firmware not found

On Fri, Feb 19, 2016 at 5:16 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Petri,
>
>>> If the call to request_firmware() fails in btbcm_setup_patchram(),
>>> the BCM chip will be operating with its default firmware.
>>>
>>> In this case, btbcm_setup_patchram() should not return immediately
>>> but instead should skip to btbcm_check_bdaddr() and quirk setup.
>>>
>>> Signed-off-by: Petri Gynther <[email protected]>
>>> ---
>>> drivers/bluetooth/btbcm.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>>> index 0b69794..7d82b1a 100644
>>> --- a/drivers/bluetooth/btbcm.c
>>> +++ b/drivers/bluetooth/btbcm.c
>>> @@ -467,7 +467,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
>>> err = request_firmware(&fw, fw_name, &hdev->dev);
>>> if (err < 0) {
>>> BT_INFO("%s: BCM: Patch %s not found", hdev->name, fw_name);
>>> - return 0;
>>> + goto check_bdaddr;
>>> }
>>>
>>> btbcm_patchram(hdev, fw);
>>> @@ -501,6 +501,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
>>> BT_INFO("%s: %s", hdev->name, (char *)(skb->data + 1));
>>> kfree_skb(skb);
>>>
>>> +check_bdaddr:
>>
>> lets just call this label "done". I think using check_bdaddr might be a bit of misleading labeling here. Since we actually want it to run all success test including the duplicate filter setting.
>
> actually I just did an inline edit of the patch to change the label name and applied the patch. There is really no point in you wasting time to respin it.
>
> Regards
>
> Marcel
>

The patch applied to bluetooth-next looks good. Thanks.

2016-02-20 01:16:49

by Marcel Holtmann

Subject: Re: [PATCH] Bluetooth: btbcm: Fix handling of firmware not found

Hi Petri,

>> If the call to request_firmware() fails in btbcm_setup_patchram(),
>> the BCM chip will be operating with its default firmware.
>>
>> In this case, btbcm_setup_patchram() should not return immediately
>> but instead should skip to btbcm_check_bdaddr() and quirk setup.
>>
>> Signed-off-by: Petri Gynther <[email protected]>
>> ---
>> drivers/bluetooth/btbcm.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>> index 0b69794..7d82b1a 100644
>> --- a/drivers/bluetooth/btbcm.c
>> +++ b/drivers/bluetooth/btbcm.c
>> @@ -467,7 +467,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
>> err = request_firmware(&fw, fw_name, &hdev->dev);
>> if (err < 0) {
>> BT_INFO("%s: BCM: Patch %s not found", hdev->name, fw_name);
>> - return 0;
>> + goto check_bdaddr;
>> }
>>
>> btbcm_patchram(hdev, fw);
>> @@ -501,6 +501,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
>> BT_INFO("%s: %s", hdev->name, (char *)(skb->data + 1));
>> kfree_skb(skb);
>>
>> +check_bdaddr:
>
> lets just call this label "done". I think using check_bdaddr might be a bit of misleading labeling here. Since we actually want it to run all success test including the duplicate filter setting.

actually I just did an inline edit of the patch to change the label name and applied the patch. There is really no point in you wasting time to respin it.

Regards

Marcel

2016-02-20 01:16:10

by Marcel Holtmann

Subject: Re: [PATCH] Bluetooth: btbcm: Fix handling of firmware not found

Hi Petri,

> If the call to request_firmware() fails in btbcm_setup_patchram(),
> the BCM chip will be operating with its default firmware.
>
> In this case, btbcm_setup_patchram() should not return immediately
> but instead should skip to btbcm_check_bdaddr() and quirk setup.
>
> Signed-off-by: Petri Gynther <[email protected]>
> ---
> drivers/bluetooth/btbcm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

2016-02-20 00:55:22

by Marcel Holtmann

Subject: Re: [PATCH] Bluetooth: btbcm: Fix handling of firmware not found

Hi Petri,

> If the call to request_firmware() fails in btbcm_setup_patchram(),
> the BCM chip will be operating with its default firmware.
>
> In this case, btbcm_setup_patchram() should not return immediately
> but instead should skip to btbcm_check_bdaddr() and quirk setup.
>
> Signed-off-by: Petri Gynther <[email protected]>
> ---
> drivers/bluetooth/btbcm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index 0b69794..7d82b1a 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -467,7 +467,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
> err = request_firmware(&fw, fw_name, &hdev->dev);
> if (err < 0) {
> BT_INFO("%s: BCM: Patch %s not found", hdev->name, fw_name);
> - return 0;
> + goto check_bdaddr;
> }
>
> btbcm_patchram(hdev, fw);
> @@ -501,6 +501,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
> BT_INFO("%s: %s", hdev->name, (char *)(skb->data + 1));
> kfree_skb(skb);
>
> +check_bdaddr:

lets just call this label "done". I think using check_bdaddr might be a bit of misleading labeling here. Since we actually want it to run all success test including the duplicate filter setting.

> btbcm_check_bdaddr(hdev);
>
> set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);

Regards

Marcel

2016-02-19 05:31:14

by Petri Gynther

Subject: Re: [PATCH] Bluetooth: btbcm: Fix handling of firmware not found

On Wed, Feb 10, 2016 at 12:00 PM, Petri Gynther <[email protected]> wrote:
>
> If the call to request_firmware() fails in btbcm_setup_patchram(),
> the BCM chip will be operating with its default firmware.
>
> In this case, btbcm_setup_patchram() should not return immediately
> but instead should skip to btbcm_check_bdaddr() and quirk setup.
>
> Signed-off-by: Petri Gynther <[email protected]>
> ---
> drivers/bluetooth/btbcm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index 0b69794..7d82b1a 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -467,7 +467,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
> err = request_firmware(&fw, fw_name, &hdev->dev);
> if (err < 0) {
> BT_INFO("%s: BCM: Patch %s not found", hdev->name, fw_name);
> - return 0;
> + goto check_bdaddr;
> }
>
> btbcm_patchram(hdev, fw);
> @@ -501,6 +501,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
> BT_INFO("%s: %s", hdev->name, (char *)(skb->data + 1));
> kfree_skb(skb);
>
> +check_bdaddr:
> btbcm_check_bdaddr(hdev);
>
> set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> --
> 2.7.0.rc3.207.g0ac5344
>

Any comments?

BCM BT controllers can operate with their default firmware if newer
firmware is not available in the system, so btbcm_setup_patchram()
should take the same actions whether the firmware got updated or not.