Return-Path: From: Ilya Faenson To: Frederic Danis , Arend Van Spriel CC: Marcel Holtmann , "linux-bluetooth@vger.kernel.org" Subject: RE: [PATCH v6 4/5] Bluetooth: hci_uart: Update Broadcom UART setup Date: Fri, 22 May 2015 13:55:18 +0000 Message-ID: References: <1432136770-13328-1-git-send-email-frederic.danis@linux.intel.com> <1432136770-13328-5-git-send-email-frederic.danis@linux.intel.com> <555DE2BE.8000109@broadcom.com> <555F25F8.4080408@linux.intel.com> <555F2EB5.1080103@broadcom.com>,<555F33E8.8010703@linux.intel.com> In-Reply-To: <555F33E8.8010703@linux.intel.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 List-ID: Hi Fred, ________________________________________ From: Frederic Danis [frederic.danis@linux.intel.com] Sent: Friday, May 22, 2015 9:49 AM To: Arend Van Spriel Cc: Marcel Holtmann; linux-bluetooth@vger.kernel.org; Ilya Faenson Subject: Re: [PATCH v6 4/5] Bluetooth: hci_uart: Update Broadcom UART setup 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 >>>> --- >>>> 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 =3D btbcm_set_bdaddr; >>>> >>>> - return btbcm_setup_patchram(hu->hdev); >>>> + err =3D btbcm_initialize(hu->hdev, fw_name, sizeof(fw_name)); >>>> + if (err) >>>> + return err; >>>> + >>>> + err =3D 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 =3D=3D -ENOENT) >>>> + return 0; >>>> + >>>> + if (hu->proto->init_speed) >>>> + hci_uart_set_baudrate(hu, hu->proto->init_speed); >>>> + >>>> + err =3D btbcm_finalize(hu->hdev); >>>> + >>>> + return err; >>>> } >>>> >>>> static const struct h4_recv_pkt bcm_recv_pkts[] =3D { >> >> 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 =3D 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 =3D 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 =3D 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 =3D 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 =3D 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. IF: There is generally no "current firmware" if the firmware download fails= or not attempted. The device would run out of the ROM then which is not te= rribly useful if Bluetooth functionality is needed. Thanks, -Ilya Regards Fred=