Re: [PATCH v6 4/5] Bluetooth: hci_uart: Update Broadcom UART setup

[Frederic Danis <frederic.danis@linux.intel.com>]

On 22/05/2015 15:27, Arend van Spriel wrote:
> On 05/22/15 14:50, Frederic Danis wrote:
>> Hello Arend,
>>
>> On 21/05/2015 15:50, Arend van Spriel wrote:
>>> On 05/20/15 17:46, Frederic Danis wrote:
>>>> Use btbcm helpers to perform controller setup.
>>>> Perform host UART reset to init speed between btbcm_patchram() and
>>>> btbcm_finalize(). This may be need because firmware loading may have
>>>> reseted controller UART to init speed.
>>>>
>>>> Signed-off-by: Frederic Danis<frederic.danis@linux.intel.com>
>>>> ---
>>>> drivers/bluetooth/hci_bcm.c | 19 ++++++++++++++++++-
>>>> 1 file changed, 18 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>>>> index 1ec0b4a..cede445 100644
>>>> --- a/drivers/bluetooth/hci_bcm.c
>>>> +++ b/drivers/bluetooth/hci_bcm.c
>>>> @@ -79,11 +79,28 @@ static int bcm_flush(struct hci_uart *hu)
>>>>
>>>> static int bcm_setup(struct hci_uart *hu)
>>>> {
>>>> + char fw_name[64];
>>>> + int err;
>>>> +
>>>> BT_DBG("hu %p", hu);
>>>>
>>>> hu->hdev->set_bdaddr = btbcm_set_bdaddr;
>>>>
>>>> - return btbcm_setup_patchram(hu->hdev);
>>>> + err = btbcm_initialize(hu->hdev, fw_name, sizeof(fw_name));
>>>> + if (err)
>>>> + return err;
>>>> +
>>>> + err = btbcm_patchram(hu->hdev, fw_name);
>>>> + /* If there is no firmware (-ENOENT), discard the error and
>>>> continue */
>>>
>>> I guess -ENOENT means no firmware is required and not a
>>> request_firmware() failure. Not sure what is meant here.
>>>
>>> Regards,
>>> Arend
>>>
>>>> + if (err == -ENOENT)
>>>> + return 0;
>>>> +
>>>> + if (hu->proto->init_speed)
>>>> + hci_uart_set_baudrate(hu, hu->proto->init_speed);
>>>> +
>>>> + err = btbcm_finalize(hu->hdev);
>>>> +
>>>> + return err;
>>>> }
>>>>
>>>> static const struct h4_recv_pkt bcm_recv_pkts[] = {
>>
>> You're right, btbcm_patchram() return test does not work as I expected.
>>
>> I can change this by performing uart speed change only if it returns no
>> error.
>>
>> err = btbcm_patchram(hu->hdev, fw_name);
>> if (!err) {
>> /* Firmware loading may have reseted controller UART to init speed */
>> if (hu->proto->init_speed)
>> hci_uart_set_baudrate(hu, hu->proto->init_speed);
>> }
>>
>> err = btbcm_finalize(hu->hdev);
>>
>>
>> Or I can move request_firmware() out of btbcm_patchram() and test it
>> before calling btbcm_patchram(). This will imply to change
>> btbcm_patchram() to accept a firmware instead of firmware name.
>>
>> err = request_firmware(&fw, fw_name, &hu->hdev->dev);
>> if (err < 0) {
>> BT_INFO("%s: BCM: patch %s not found", hu->hdev->name, fw_name);
>> return 0;
>> }
>>
>> err = btbcm_patchram(hu->hdev, fw);
>> if (!err) {
>> /* Firmware loading may have reseted controller UART to init speed */
>> if (hu->proto->init_speed)
>> hci_uart_set_baudrate(hu, hu->proto->init_speed);
>> }
>>
>> err = btbcm_finalize(hu->hdev);
>>
>> Arend, Marcel, any advice regarding this ?
>
> Well, functionally it does not make a difference as the request_firmware
> is the first call done in btbcm_patchram(). So if it solves your problem
> I would go for option 2. Other question: is there a reason why the error
> code from request_firmware is not returned?

If an error is returned it will stop Bluetooth setup.
So, when request_firmware() returns an error, the Bluetooth controller 
will be used with current firmware and 0 is returned as bcm_setup is 
completed.

Regards

Fred