[v1] Bluetooth: btintel: Allow lowering of drive strength of BRI

BRI (Bluetooth Radio Interface) traffic from CNVr to CNVi was found
causing cross talk step errors to WiFi. As a workaround, driver needs to
reduce the drive strength of BRI. During *setup*, driver reads the drive
strength value from efi variable and passes it controller via vendor
specific command with opcode 0xfc0a.

Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel.c | 115 ++++++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

Hi Luiz,

Thanks for your comments.

>
>Hi Kiran,
>
>On Thu, Jun 20, 2024 at 10:52 AM Kiran K <kiran.k@intel.com> wrote:
>>
>> BRI (Bluetooth Radio Interface) traffic from CNVr to CNVi was found
>> causing cross talk step errors to WiFi. As a workaround, driver needs
>> to reduce the drive strength of BRI. During *setup*, driver reads the
>> drive strength value from efi variable and passes it controller via
>> vendor specific command with opcode 0xfc0a.
>>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>>  drivers/bluetooth/btintel.c | 115
>> ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 115 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
>> index 5d735391545a..1d6586b30c8d 100644
>> --- a/drivers/bluetooth/btintel.c
>> +++ b/drivers/bluetooth/btintel.c
>> @@ -12,6 +12,8 @@
>>  #include <linux/acpi.h>
>>  #include <acpi/acpi_bus.h>
>>  #include <asm/unaligned.h>
>> +#include <linux/efi.h>
>> +
>>
>>  #include <net/bluetooth/bluetooth.h>
>>  #include <net/bluetooth/hci_core.h>
>> @@ -49,6 +51,39 @@ static const guid_t btintel_guid_dsm =
>>         GUID_INIT(0xaa10f4e0, 0x81ac, 0x4233,
>>                   0xab, 0xf6, 0x3b, 0x2a, 0xc5, 0x0e, 0x28, 0xd9);
>>
>> +static void *btintel_uefi_get_variable(efi_char16_t *name, efi_guid_t
>> +*guid) { #if defined(CONFIG_EFI)
>
>iirc IS_ENABLED is normally preferred over #if defined, well except if you have
>undefined symbols but that usually indicates the APIs have not been done with
>usage of IS_ENABLED in mind.

Ack. I will fix it  in v2 version.

>
>> +       void *data;
>> +       efi_status_t status;
>> +       size_t data_size = 0;
>> +
>> +       if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
>> +               return ERR_PTR(-EOPNOTSUPP);
>> +
>> +       status = efi.get_variable(name, guid, NULL, &data_size, NULL);
>> +
>> +       if (status != EFI_BUFFER_TOO_SMALL || !data_size)
>> +               return ERR_PTR(-EIO);
>> +
>> +       data = kmalloc(data_size, GFP_KERNEL);
>> +
>> +       if (!data)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       status = efi.get_variable(name, guid, NULL, &data_size, data);
>> +
>> +       if (status != EFI_SUCCESS) {
>> +               kfree(data);
>> +               return ERR_PTR(-ENXIO);
>> +       }
>> +
>> +       return data;
>> +#else
>> +       return ERR_PTR(-EOPNOTSUPP);
>> +#endif
>> +}
>> +
>>  int btintel_check_bdaddr(struct hci_dev *hdev)  {
>>         struct hci_rp_read_bd_addr *bda; @@ -2615,6 +2650,80 @@ static
>> u8 btintel_classify_pkt_type(struct hci_dev *hdev, struct sk_buff *skb)
>>         return hci_skb_pkt_type(skb);
>>  }
>>
>> +static int btintel_set_dsbr(struct hci_dev *hdev, struct
>> +intel_version_tlv *ver) {
>> +       struct btintel_dsbr_cmd {
>> +               u8 enable;
>> +               u8 dsbr;
>> +       } __packed;
>> +
>> +       struct btintel_dsbr {
>> +               u8 header;
>> +               u32 dsbr;
>> +       } __packed;
>> +
>> +       struct btintel_dsbr *dsbr;
>> +       struct btintel_dsbr_cmd cmd;
>> +       struct sk_buff *skb;
>> +       u8 status;
>> +       efi_guid_t guid = EFI_GUID(0xe65d8884, 0xd4af, 0x4b20, 0x8d, 0x03,
>> +                                  0x77, 0x2e, 0xcc, 0x3d, 0xa5,
>> + 0x31);
>> +
>> +       memset(&cmd, 0, sizeof(cmd));
>> +       dsbr = btintel_uefi_get_variable(L"UefiCnvCommonDSBR", &guid);
>> +       if (IS_ERR(dsbr)) {
>> +               /* If efi variable is not present, driver still needs to send
>> +                * 0xfc0a command with default values
>> +                */
>> +               bt_dev_dbg(hdev, "Error reading efi DSBR (%ld)",
>> +                          PTR_ERR(dsbr));
>> +               dsbr = NULL;
>> +       }
>> +
>> +       if (dsbr) {
>> +               /* bit0: 0 - Use firmware default value
>> +                *       1 - Override firmware value
>> +                * bit3:1 - Reserved
>> +                * bit7:4 - DSBR override values
>> +                * bt31:7 - Reserved
>> +                */
>> +               cmd.enable = dsbr->dsbr & BIT(0);
>> +               if (cmd.enable)
>> +                       cmd.dsbr = dsbr->dsbr >> 4 & 0xF;
>> +               kfree(dsbr);
>> +       }
>> +
>> +       bt_dev_info(hdev, "dsbr: enabled: 0x%2.2x value: 0x%2.2x",
>cmd.enable,
>> +                   cmd.dsbr);
>> +
>> +       skb = __hci_cmd_sync(hdev, 0xfc0a, sizeof(cmd), &cmd,
>HCI_CMD_TIMEOUT);
>> +       if (IS_ERR(skb)) {
>> +               bt_dev_err(hdev, "Failed to send Intel DSBR command (%ld)",
>> +                          PTR_ERR(skb));
>> +               return -bt_to_errno(PTR_ERR(skb));
>> +       }
>> +
>> +       status = skb->data[0];
>> +       kfree_skb(skb);
>> +
>> +       if (status) {
>> +               bt_dev_err(hdev, "Set DSBR failed 0x%2.2x", status);
>> +               return -bt_to_errno(status);
>> +       }
>> +       return 0;
>> +}
>> +
>> +static int btintel_apply_dsbr(struct hci_dev *hdev,
>> +                             struct intel_version_tlv *ver) {
>> +       /* For BlazarI + B0 step, DSBR command needs to be sent just after
>> +        * downloading IML firmware
>> +        */
>> +       return ver->img_type == BTINTEL_IMG_IML &&
>> +               ((ver->cnvi_top & 0xfff) == BTINTEL_CNVI_BLAZARI) &&
>> +               INTEL_CNVX_TOP_STEP(ver->cnvi_top) == 0x01; }
>> +
>>  int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
>>                                  struct intel_version_tlv *ver)  { @@
>> -2649,6 +2758,12 @@ int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
>>         if (err)
>>                 return err;
>>
>> +       if (btintel_apply_dsbr(hdev, ver)) {
>> +               err = btintel_set_dsbr(hdev, ver);
>> +               if (err)
>> +                       return err;
>> +       }
>> +
>>         /* If image type returned is BTINTEL_IMG_IML, then controller supports
>>          * intermediae loader image
>>          */
>> --
>> 2.40.1
>>
>>
>
>
>--
>Luiz Augusto von Dentz

Regards,
Kiran