[RFC v1 5/8] Bluetooth: btrtl: add support for the RTL8723BS and RTL8723DS chips

[martin.blumenstingl@googlemail.com (Martin Blumenstingl)]

Hi Marcel,

On Sun, Nov 19, 2017 at 9:25 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Martin,
>
>> The Realtek RTL8723BS and RTL8723DS chipsets are SDIO wifi chips. They
>> also contain a Bluetooth module which is connected via UART to the host.
>>
>> Realtek's userspace initialization tool (rtk_hciattach) differentiates
>> these two via the HCI version and revision returned by the
>> HCI_OP_READ_LOCAL_VERSION command.
>> Additionally we apply these checks only the for UART devices. Everything
>> else is assumed to be a "RTL8723B" which was originally supported by the
>> driver (communicating via USB).
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>> drivers/bluetooth/btrtl.c | 32 ++++++++++++++++++++++++++++++--
>> 1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
>> index 45b872f5ad22..d896f9421250 100644
>> --- a/drivers/bluetooth/btrtl.c
>> +++ b/drivers/bluetooth/btrtl.c
>> @@ -418,9 +418,33 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev)
>>               has_rom_version = false;
>>               break;
>>       case RTL_ROM_LMP_8723B:
>> -             fw_name = "rtl_bt/rtl8723b_fw.bin";
>> -             cfg_name = "rtl_bt/rtl8723b_config.bin";
>> +             /* all variants support reading the ROM version */
>>               has_rom_version = true;
>> +
>> +             /*
>> +              * RTL8723 devices exist in different variants:
>> +              * - RTL8723BS (SDIO chip with UART Bluetooth)
>> +              * - RTL8723DS (SDIO chip with UART Bluetooth)
>> +              * - for backwards-compatibility everything else is assumed to
>> +              *   be an RTL8723B communicating over USB
>> +              *
>> +              * Only UART devices really need the config because that
>> +              * contains the UART speed / flow control settings.
>> +              */
>> +             if (hdev->bus == HCI_UART && resp->hci_ver == 6 &&
>> +                 le16_to_cpu(resp->hci_rev) == 0xb) {
>> +                     fw_name = "rtl_bt/rtl8723bs_fw.bin";
>> +                     cfg_name = "rtl_bt/rtl8723bs_config.bin";
>> +                     cfg_needed = true;
>> +             } else if (hdev->bus == HCI_UART && resp->hci_ver == 8 &&
>> +                        le16_to_cpu(resp->hci_rev) == 0xd) {
>> +                     fw_name = "rtl_bt/rtl8723ds_fw.bin";
>> +                     cfg_name = "rtl_bt/rtl872ds_config.bin";
>> +                     cfg_needed = true;
>> +             } else {
>> +                     fw_name = "rtl_bt/rtl8723b_fw.bin";
>> +                     cfg_name = "rtl_bt/rtl8723b_config.bin";
>> +             }
>
> so the main question is why is this a config file in the first place? So far all other drivers have expressed UART settings via DT (or even via ACPI). If we are just getting the baudrates, then better have this in DT. Since otherwise we end up with board specific filesystems again where different config files need to be installed. That is really not useful in the end.
this is an excellent question (and also the reason why it's an "RFC" patch)!
I'll try to figure out whether:
a) we can skip the config file completely
b) we can generate the config file in-memory
c) we have to stick with this config file

> What is actually in these config files? Can we have some parsing and friendly display tool in bluez.git like we have for other manufactures.
to my knowledge this config file contains:
- the baudrate
- whether flow control is enabled or disabled
- (in some cases) the local-bd-address

do you mean a tool that is similar to bluez.git tools/nokfw.c?


Regards
Martin


[0] https://github.com/NextThingCo/rtl8723ds_bt/blob/f56ef37217665070e574253d23a595ee1ca5ca23/rtk_hciattach/hciattach_rtk.c#L1781