| Message ID | 20240620150635.2119398-1-kiran.k@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] Bluetooth: btintel: Allow lowering of drive strength of BRI | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/BuildKernel32 | success | BuildKernel32 PASS |
| tedd_an/TestRunnerSetup | success | TestRunnerSetup PASS |
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. > + 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 > >
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
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) + 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 */
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(+)