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

[Marcel Holtmann]

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 at 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

these two are better done as part of the DT.

> - (in some cases) the local-bd-address

There is essentially DT support for that as well, but we also have Set Public Address mgmt command to handle this. Then again, first of all the hdev->set_bdaddr callback needs to be supported for that.

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

Yes.

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

Please never ever make me look at code that defined struct sk_buff in userspace ;)

Regards

Marcel