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

[Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>]

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-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
> 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.

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.

Regards

Marcel