Hi Fred,
>>> Split btbcm_setup_patchram in 3 functions : btbcm_setup_patchram,
>>> btbcm_patchram (already existing) and btbcm_setup_post.
>>> For UART controllers this will allow to reset speed to default one
>>> after firmware loading, which will have reseted controller speed to
>>> default one.
>>>
>>> Signed-off-by: Frederic Danis <frederic.danis@xxxxxxxxxxxxxxx>
>>> ---
>>> drivers/bluetooth/btbcm.c | 29 +++++++++++++++++------------
>>> drivers/bluetooth/btbcm.h | 11 +++++++++--
>>> drivers/bluetooth/btusb.c | 23 ++++++++++++++++++++++-
>>> drivers/bluetooth/hci_bcm.c | 19 ++++++++++++++++++-
>>> 4 files changed, 66 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>>> index b258454..f2efbbd 100644
>>> --- a/drivers/bluetooth/btbcm.c
>>> +++ b/drivers/bluetooth/btbcm.c
>>> @@ -277,9 +277,8 @@ static const struct {
>>> { }
>>> };
>>>
>>> -int btbcm_setup_patchram(struct hci_dev *hdev)
>>> +int btbcm_setup_patchram(struct hci_dev *hdev, char *fw_name, size_t len)
>>
>> what is this len for? The fw_name is not a return value. I think the functions for doing the patchram should just get a firmware and then just do patchram.
>
> I split btbcm_setup_patchram() in 3 functions:
> - btbcm_setup_patchram() which resets the controller and returns the firmware name.
> - btbtcm_patchram() (already existing and public), which takes the firmware name as parameter, requests the firmware and loads it to the controller.
> - btbcm_setup_post() which resets the controller, reads local version and checks if the controller address is a default one or not.
>
> I split it between btbcm_patchram() and btbcm_setup_post() as firmware loading may reset the controller UART speed and it needs to set host UART speed back to init speed.
>
> I also split it between btbcm_setup_patchram() and btbtcm_patchram() because error is not managed the same way for both of them.
> When btbcm_setup_patchram() returns an error, this error is forwarded to caller.
> When btbtcm_patchram() returns an error, if the error is -ENOENT then this error is discarded and 0 is returned to caller. Otherwise, the error is also discarded but there is a need to call btbcm_setup_post() to reset controller.
my proposal to make this a bit smoother and have less changes is to keep the btbcm_setup_patchram as is. Lets keep using that as is for USB and introduce helper functions to be used by the UART side of things.
To get things moving, lets assume that firmware filename comes in from somewhere. Maybe coded in DTS or ACPI or something else. In the long run, we need to have a “manifest” file that lets us map original Broadcom firmware file names to platforms. For USB that will become simple since it is USB VID/PID map. For ACPI we can use the ACPI ID mapping. So my thinking is to use a simple table mapping modalias to filename and that be it. However I would put that a little bit as a problem to solve later and bring the basic UART and speed handling into place.
This means that UART setup routine should be doing this:
- UART call to switch to default speed
-> switch to default speed
- btbcm_initialize()
- UART call to switch operational speed
-> trigger vendor command
-> switch speed
- btbcm_patchram()
- UART call to switch to default speed
-> switch to default speed
- UART call to switch to operational speed
-> trigger vendor command
-> switch speed
- btbcm_finalize()
Maybe default speed and operational speed changes should always be combined. We have to see there.
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
