| Message ID | 20220408080633.20463-1-kiran.k@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] Bluetooth: btintel: Add support to configure TX power | expand |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=630327 ---Test result--- Test Summary: CheckPatch PASS 1.91 seconds GitLint PASS 1.00 seconds SubjectPrefix PASS 0.82 seconds BuildKernel PASS 31.71 seconds BuildKernel32 PASS 28.41 seconds Incremental Build with patchesPASS 38.53 seconds TestRunner: Setup PASS 473.64 seconds TestRunner: l2cap-tester PASS 16.07 seconds TestRunner: bnep-tester PASS 6.37 seconds TestRunner: mgmt-tester PASS 106.73 seconds TestRunner: rfcomm-tester PASS 8.30 seconds TestRunner: sco-tester PASS 8.07 seconds TestRunner: smp-tester PASS 7.94 seconds TestRunner: userchan-tester PASS 6.61 seconds --- Regards, Linux Bluetooth
[Cc: +linux-acpi to check for best practices in ACPI table handling] Dear Kiran, Am 08.04.22 um 10:06 schrieb Kiran K: Thank you for the patch. Maybe as commit message summary: Support to configure TX power using BRDS ACPI table > BRDS - Bluetooth Regulatory Domain Specific absorption rate > ----------------------------------------------------------- Why this header? Integrate the long table name into the message below. > > Bluetooth has regulatory limitations which prohibit or allow usage > of certain bands or channels as well as limiting Tx power. The Tx power > values can be configured in ACPI table. This patch reads ACPI entry of > Bluetooth SAR and configures the controller accordingly. It’d be great if you elaborated a little on the implementation? For example, what is the legacy SAR needed for? How did you test this? What device and firmware providing the BRDS table? Maybe even paste the ASL table here as an example. > Signed-off-by: Kiran K <kiran.k@intel.com> > Signed-off-by: Seema S <seema.sreemantha@intel.com> > --- > drivers/bluetooth/btintel.c | 229 ++++++++++++++++++++++++++++++++++++ > drivers/bluetooth/btintel.h | 18 +++ > 2 files changed, 247 insertions(+) > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > index 818681c89db8..d3dc703eba78 100644 > --- a/drivers/bluetooth/btintel.c > +++ b/drivers/bluetooth/btintel.c > @@ -9,6 +9,7 @@ > #include <linux/module.h> > #include <linux/firmware.h> > #include <linux/regmap.h> > +#include <linux/acpi.h> > #include <asm/unaligned.h> > > #include <net/bluetooth/bluetooth.h> > @@ -32,6 +33,9 @@ struct cmd_write_boot_params { > u8 fw_build_yy; > } __packed; > > +#define BTINTEL_SAR_NAME "BRDS" > +#define BTINTEL_SAR_PREFIX "\\_SB_.PC00.XHCI.RHUB" > + > int btintel_check_bdaddr(struct hci_dev *hdev) > { > struct hci_rp_read_bd_addr *bda; > @@ -2250,6 +2254,228 @@ static int btintel_configure_offload(struct hci_dev *hdev) > return err; > } > > +static acpi_status btintel_sar_callback(acpi_handle handle, u32 lvl, void *data, > + void **ret) > +{ > + acpi_status status; > + int len; size_t > + struct btintel_sar *sar; > + union acpi_object *p, *elements; > + struct acpi_buffer string = {ACPI_ALLOCATE_BUFFER, NULL}; > + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL}; > + > + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &string); > + if (ACPI_FAILURE(status)) { > + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); > + return status; > + } > + > + if (strncmp(BTINTEL_SAR_PREFIX, string.pointer, > + strlen(BTINTEL_SAR_PREFIX))) { > + kfree(string.pointer); > + return AE_OK; > + } > + > + len = strlen(string.pointer); > + if (strncmp((char *)string.pointer + len - 4, BTINTEL_SAR_NAME, 4)) { > + kfree(string.pointer); > + return AE_OK; > + } > + kfree(string.pointer); > + > + status = acpi_evaluate_object(handle, NULL, NULL, &buffer); > + if (ACPI_FAILURE(status)) { > + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); > + return status; > + } > + > + p = buffer.pointer; > + sar = data; > + > + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 2) { > + kfree(buffer.pointer); > + BT_DBG("Invalid object type or package count"); Please add the variable values to the debug line. > + return AE_ERROR; > + } > + > + elements = p->package.elements; > + > + /* SAR table is located at element[1] */ > + p = &elements[1]; > + > + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 8) { > + kfree(buffer.pointer); > + return AE_ERROR; > + } > + > + sar->domain = (u8)p->package.elements[0].integer.value; > + sar->type = (u8)p->package.elements[1].integer.value; > + sar->br = (u32)p->package.elements[2].integer.value; > + sar->edr2 = (u32)p->package.elements[3].integer.value; > + sar->edr3 = (u32)p->package.elements[4].integer.value; > + sar->le = (u32)p->package.elements[5].integer.value; > + sar->le_2mhz = (u32)p->package.elements[6].integer.value; > + sar->le_lr = (u32)p->package.elements[7].integer.value; > + kfree(buffer.pointer); > + return AE_CTRL_TERMINATE; > +} > + > +static void btintel_send_sar_ddc(struct hci_dev *hdev, void *data, u8 len) Use native type for `len`? `unsigned int`? > +{ > + struct sk_buff *skb; > + > + skb = __hci_cmd_sync(hdev, 0xfc8b, len, data, HCI_CMD_TIMEOUT); > + if (IS_ERR(skb)) { > + bt_dev_warn(hdev, "Failed to send Intel Write SAR DDC (%ld)", PTR_ERR(skb)); > + return; > + } > + kfree_skb(skb); > +} > + > +static int btintel_set_legacy_sar(struct hci_dev *hdev, struct btintel_sar *sar) > +{ > + struct btintel_cp_ddc_write *cmd; > + u8 buffer[64]; Don’t try to align with tabs? > + > + if (!sar) > + return -EINVAL; > + > + cmd = (void *)buffer; > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x0131); Add names for the command id’s? > + cmd->data[0] = sar->br >> 3; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x0132); > + cmd->data[0] = sar->br >> 3; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 5; > + cmd->id = cpu_to_le16(0x0137); > + cmd->data[0] = sar->br >> 3; > + cmd->data[1] = sar->edr2 >> 3; > + cmd->data[2] = sar->edr3 >> 3; > + btintel_send_sar_ddc(hdev, cmd, 6); > + > + cmd->len = 5; > + cmd->id = cpu_to_le16(0x0138); > + cmd->data[0] = sar->br >> 3; > + cmd->data[1] = sar->edr2 >> 3; > + cmd->data[2] = sar->edr3 >> 3; > + btintel_send_sar_ddc(hdev, cmd, 6); > + > + cmd->len = 5; > + cmd->id = cpu_to_le16(0x013b); > + cmd->data[0] = sar->br >> 3; > + cmd->data[1] = sar->edr2 >> 3; > + cmd->data[2] = sar->edr3 >> 3; > + btintel_send_sar_ddc(hdev, cmd, 6); > + > + cmd->len = 5; > + cmd->id = cpu_to_le16(0x013c); > + cmd->data[0] = sar->br >> 3; > + cmd->data[1] = sar->edr2 >> 3; > + cmd->data[2] = sar->edr3 >> 3; > + btintel_send_sar_ddc(hdev, cmd, 6); > + > + return 0; > +} > + > +static int btintel_set_mutual_sar(struct hci_dev *hdev, struct btintel_sar *sar) > +{ > + u8 buffer[64]; > + struct btintel_cp_ddc_write *cmd; > + u8 enable[1] = {1}; > + struct sk_buff *skb; > + > + if (!sar) > + return -EINVAL; > + > + cmd = (void *)buffer; > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x019e); > + if (!(sar->le_2mhz & BIT(7))) > + cmd->data[0] = 0x01; > + else > + cmd->data[0] = 0x00; Use ternary operator? > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x019f); > + cmd->data[0] = sar->le_lr; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x01a0); > + cmd->data[0] = sar->br; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x01a1); > + cmd->data[0] = sar->edr2; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x01a2); > + cmd->data[0] = sar->edr3; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x01a3); > + cmd->data[0] = sar->le; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + skb = __hci_cmd_sync(hdev, 0xfe25, 1, enable, HCI_CMD_TIMEOUT); > + if (IS_ERR(skb)) { > + bt_dev_warn(hdev, "Failed to send Intel SAR Enable (%ld)", PTR_ERR(skb)); > + return PTR_ERR(skb); > + } > + kfree_skb(skb); > + > + return 0; > +} > + > +static int btintel_set_specific_absorption_rate(struct hci_dev *hdev, > + struct intel_version_tlv *ver) > +{ > + acpi_status status; > + struct btintel_sar sar; > + > + switch (ver->cnvr_top & 0xfff) { > + case 0x810: /* MsP */ > + break; > + default: > + return 0; > + } > + > + memset(&sar, 0, sizeof(sar)); > + > + status = acpi_walk_namespace(ACPI_TYPE_METHOD, ACPI_ROOT_OBJECT, > + ACPI_UINT32_MAX, NULL, > + btintel_sar_callback, &sar, NULL); > + > + if (ACPI_FAILURE(status)) > + return -1; > + > + if (sar.domain != 0x12) > + return -1; > + > + /* No need to configure controller if Bluetooth SAR is disabled in BIOS > + */ Put it on the line above? > + if (!sar.type) > + return 0; > + > + if (sar.type == 1) { > + bt_dev_info(hdev, "Applying both legacy and mutual Bluetooth SAR"); > + btintel_set_legacy_sar(hdev, &sar); > + btintel_set_mutual_sar(hdev, &sar); > + } > + return 0; > +} > + > static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, > struct intel_version_tlv *ver) > { > @@ -2294,6 +2520,9 @@ static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, > /* Read supported use cases and set callbacks to fetch datapath id */ > btintel_configure_offload(hdev); > > + /* Set Specific Absorption Rate */ > + btintel_set_specific_absorption_rate(hdev, ver); > + > hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT); > > /* Read the Intel version information after loading the FW */ > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h > index e0060e58573c..7aa58fb7b02a 100644 > --- a/drivers/bluetooth/btintel.h > +++ b/drivers/bluetooth/btintel.h > @@ -137,6 +137,24 @@ struct intel_offload_use_cases { > __u8 preset[8]; > } __packed; > > +/* structure to store the data read from ACPI table */ > +struct btintel_sar { > + u8 domain; > + u8 type; > + u32 br; > + u32 edr2; > + u32 edr3; > + u32 le; > + u32 le_2mhz; > + u32 le_lr; Can’t native types be used? > +}; > + > +struct btintel_cp_ddc_write { > + u8 len; > + __le16 id; > + u8 data[0]; > +} __packed; > + > #define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & 0x0000ff00) >> 8)) > #define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & 0x003f0000) >> 16)) > #define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff)
Hi Kiran, On Fri, Apr 8, 2022 at 1:02 AM Kiran K <kiran.k@intel.com> wrote: > > BRDS - Bluetooth Regulatory Domain Specific absorption rate > ----------------------------------------------------------- > > Bluetooth has regulatory limitations which prohibit or allow usage > of certain bands or channels as well as limiting Tx power. The Tx power > values can be configured in ACPI table. This patch reads ACPI entry of > Bluetooth SAR and configures the controller accordingly. It should be great to have a trace example of the commands in use, so if we can have btmon decoding these to show what is being sent that way we can also debug if something goes wrong. > Signed-off-by: Kiran K <kiran.k@intel.com> > Signed-off-by: Seema S <seema.sreemantha@intel.com> > --- > drivers/bluetooth/btintel.c | 229 ++++++++++++++++++++++++++++++++++++ > drivers/bluetooth/btintel.h | 18 +++ > 2 files changed, 247 insertions(+) > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > index 818681c89db8..d3dc703eba78 100644 > --- a/drivers/bluetooth/btintel.c > +++ b/drivers/bluetooth/btintel.c > @@ -9,6 +9,7 @@ > #include <linux/module.h> > #include <linux/firmware.h> > #include <linux/regmap.h> > +#include <linux/acpi.h> > #include <asm/unaligned.h> > > #include <net/bluetooth/bluetooth.h> > @@ -32,6 +33,9 @@ struct cmd_write_boot_params { > u8 fw_build_yy; > } __packed; > > +#define BTINTEL_SAR_NAME "BRDS" > +#define BTINTEL_SAR_PREFIX "\\_SB_.PC00.XHCI.RHUB" > + > int btintel_check_bdaddr(struct hci_dev *hdev) > { > struct hci_rp_read_bd_addr *bda; > @@ -2250,6 +2254,228 @@ static int btintel_configure_offload(struct hci_dev *hdev) > return err; > } > > +static acpi_status btintel_sar_callback(acpi_handle handle, u32 lvl, void *data, > + void **ret) > +{ > + acpi_status status; > + int len; > + struct btintel_sar *sar; > + union acpi_object *p, *elements; > + struct acpi_buffer string = {ACPI_ALLOCATE_BUFFER, NULL}; > + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL}; > + > + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &string); > + if (ACPI_FAILURE(status)) { > + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); > + return status; > + } > + > + if (strncmp(BTINTEL_SAR_PREFIX, string.pointer, > + strlen(BTINTEL_SAR_PREFIX))) { > + kfree(string.pointer); > + return AE_OK; > + } > + > + len = strlen(string.pointer); > + if (strncmp((char *)string.pointer + len - 4, BTINTEL_SAR_NAME, 4)) { > + kfree(string.pointer); > + return AE_OK; > + } > + kfree(string.pointer); > + > + status = acpi_evaluate_object(handle, NULL, NULL, &buffer); > + if (ACPI_FAILURE(status)) { > + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); > + return status; > + } > + > + p = buffer.pointer; > + sar = data; > + > + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 2) { > + kfree(buffer.pointer); > + BT_DBG("Invalid object type or package count"); > + return AE_ERROR; > + } > + > + elements = p->package.elements; > + > + /* SAR table is located at element[1] */ > + p = &elements[1]; > + > + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 8) { > + kfree(buffer.pointer); > + return AE_ERROR; > + } > + > + sar->domain = (u8)p->package.elements[0].integer.value; > + sar->type = (u8)p->package.elements[1].integer.value; > + sar->br = (u32)p->package.elements[2].integer.value; > + sar->edr2 = (u32)p->package.elements[3].integer.value; > + sar->edr3 = (u32)p->package.elements[4].integer.value; > + sar->le = (u32)p->package.elements[5].integer.value; > + sar->le_2mhz = (u32)p->package.elements[6].integer.value; > + sar->le_lr = (u32)p->package.elements[7].integer.value; > + kfree(buffer.pointer); > + return AE_CTRL_TERMINATE; > +} > + > +static void btintel_send_sar_ddc(struct hci_dev *hdev, void *data, u8 len) > +{ > + struct sk_buff *skb; > + > + skb = __hci_cmd_sync(hdev, 0xfc8b, len, data, HCI_CMD_TIMEOUT); > + if (IS_ERR(skb)) { > + bt_dev_warn(hdev, "Failed to send Intel Write SAR DDC (%ld)", PTR_ERR(skb)); > + return; > + } > + kfree_skb(skb); > +} > + > +static int btintel_set_legacy_sar(struct hci_dev *hdev, struct btintel_sar *sar) > +{ > + struct btintel_cp_ddc_write *cmd; > + u8 buffer[64]; > + > + if (!sar) > + return -EINVAL; Add a memset to 0 as this may trigger uninitialized errors in the likes of static analyzers. > + cmd = (void *)buffer; > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x0131); > + cmd->data[0] = sar->br >> 3; > + btintel_send_sar_ddc(hdev, cmd, 4); We should probably handle the errors when sending these commands and don't continue, also perhaps it would be better to have each command in its own function with a proper name indicating what it is doing. > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x0132); > + cmd->data[0] = sar->br >> 3; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 5; > + cmd->id = cpu_to_le16(0x0137); > + cmd->data[0] = sar->br >> 3; > + cmd->data[1] = sar->edr2 >> 3; > + cmd->data[2] = sar->edr3 >> 3; > + btintel_send_sar_ddc(hdev, cmd, 6); > + > + cmd->len = 5; > + cmd->id = cpu_to_le16(0x0138); > + cmd->data[0] = sar->br >> 3; > + cmd->data[1] = sar->edr2 >> 3; > + cmd->data[2] = sar->edr3 >> 3; > + btintel_send_sar_ddc(hdev, cmd, 6); > + > + cmd->len = 5; > + cmd->id = cpu_to_le16(0x013b); > + cmd->data[0] = sar->br >> 3; > + cmd->data[1] = sar->edr2 >> 3; > + cmd->data[2] = sar->edr3 >> 3; > + btintel_send_sar_ddc(hdev, cmd, 6); > + > + cmd->len = 5; > + cmd->id = cpu_to_le16(0x013c); > + cmd->data[0] = sar->br >> 3; > + cmd->data[1] = sar->edr2 >> 3; > + cmd->data[2] = sar->edr3 >> 3; > + btintel_send_sar_ddc(hdev, cmd, 6); > + > + return 0; > +} > + > +static int btintel_set_mutual_sar(struct hci_dev *hdev, struct btintel_sar *sar) > +{ > + u8 buffer[64]; > + struct btintel_cp_ddc_write *cmd; > + u8 enable[1] = {1}; > + struct sk_buff *skb; > + > + if (!sar) > + return -EINVAL; > + > + cmd = (void *)buffer; > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x019e); > + if (!(sar->le_2mhz & BIT(7))) > + cmd->data[0] = 0x01; > + else > + cmd->data[0] = 0x00; > + btintel_send_sar_ddc(hdev, cmd, 4); Ditto as above. > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x019f); > + cmd->data[0] = sar->le_lr; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x01a0); > + cmd->data[0] = sar->br; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x01a1); > + cmd->data[0] = sar->edr2; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x01a2); > + cmd->data[0] = sar->edr3; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + cmd->len = 3; > + cmd->id = cpu_to_le16(0x01a3); > + cmd->data[0] = sar->le; > + btintel_send_sar_ddc(hdev, cmd, 4); > + > + skb = __hci_cmd_sync(hdev, 0xfe25, 1, enable, HCI_CMD_TIMEOUT); > + if (IS_ERR(skb)) { > + bt_dev_warn(hdev, "Failed to send Intel SAR Enable (%ld)", PTR_ERR(skb)); > + return PTR_ERR(skb); > + } > + kfree_skb(skb); > + > + return 0; > +} > + > +static int btintel_set_specific_absorption_rate(struct hci_dev *hdev, > + struct intel_version_tlv *ver) > +{ > + acpi_status status; > + struct btintel_sar sar; > + > + switch (ver->cnvr_top & 0xfff) { > + case 0x810: /* MsP */ > + break; > + default: > + return 0; > + } > + > + memset(&sar, 0, sizeof(sar)); > + > + status = acpi_walk_namespace(ACPI_TYPE_METHOD, ACPI_ROOT_OBJECT, > + ACPI_UINT32_MAX, NULL, > + btintel_sar_callback, &sar, NULL); > + > + if (ACPI_FAILURE(status)) > + return -1; > + > + if (sar.domain != 0x12) > + return -1; > + > + /* No need to configure controller if Bluetooth SAR is disabled in BIOS > + */ > + if (!sar.type) > + return 0; > + > + if (sar.type == 1) { > + bt_dev_info(hdev, "Applying both legacy and mutual Bluetooth SAR"); > + btintel_set_legacy_sar(hdev, &sar); > + btintel_set_mutual_sar(hdev, &sar); > + } > + return 0; > +} > + > static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, > struct intel_version_tlv *ver) > { > @@ -2294,6 +2520,9 @@ static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, > /* Read supported use cases and set callbacks to fetch datapath id */ > btintel_configure_offload(hdev); > > + /* Set Specific Absorption Rate */ > + btintel_set_specific_absorption_rate(hdev, ver); > + > hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT); > > /* Read the Intel version information after loading the FW */ > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h > index e0060e58573c..7aa58fb7b02a 100644 > --- a/drivers/bluetooth/btintel.h > +++ b/drivers/bluetooth/btintel.h > @@ -137,6 +137,24 @@ struct intel_offload_use_cases { > __u8 preset[8]; > } __packed; > > +/* structure to store the data read from ACPI table */ > +struct btintel_sar { > + u8 domain; > + u8 type; > + u32 br; > + u32 edr2; > + u32 edr3; > + u32 le; > + u32 le_2mhz; > + u32 le_lr; > +}; The above should also be using __packed, also for data packets the multibyte field shall tell what is the expected endianness so given the id below is using __le16 I assume the fields above shall be __le32. > +struct btintel_cp_ddc_write { > + u8 len; > + __le16 id; > + u8 data[0]; > +} __packed; > + > #define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & 0x0000ff00) >> 8)) > #define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & 0x003f0000) >> 16)) > #define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff) > -- > 2.17.1 >
Hi Paul, Sorry for the delay. Thanks for your valuable comments. > -----Original Message----- > From: Paul Menzel <pmenzel@molgen.mpg.de> > Sent: Saturday, April 9, 2022 12:53 PM > To: K, Kiran <kiran.k@intel.com> > Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar > <ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan > <chethan.tumkur.narayan@intel.com>; Sreemantha, Seema > <seema.sreemantha@intel.com>; linux-acpi@vger.kernel.org > Subject: Re: [PATCH v1] Bluetooth: btintel: Add support to configure TX > power > > [Cc: +linux-acpi to check for best practices in ACPI table handling] > > > Dear Kiran, > > > Am 08.04.22 um 10:06 schrieb Kiran K: > > Thank you for the patch. Maybe as commit message summary: > > Support to configure TX power using BRDS ACPI table > > > BRDS - Bluetooth Regulatory Domain Specific absorption rate > > ----------------------------------------------------------- > > Why this header? Integrate the long table name into the message below. > Ack. > > > > Bluetooth has regulatory limitations which prohibit or allow usage of > > certain bands or channels as well as limiting Tx power. The Tx power > > values can be configured in ACPI table. This patch reads ACPI entry of > > Bluetooth SAR and configures the controller accordingly. > > It’d be great if you elaborated a little on the implementation? For example, > what is the legacy SAR needed for? > > How did you test this? What device and firmware providing the BRDS table? > Maybe even paste the ASL table here as an example. > Ack. I will update the commit message with details in v2 version of patch. > > Signed-off-by: Kiran K <kiran.k@intel.com> > > Signed-off-by: Seema S <seema.sreemantha@intel.com> > > --- > > drivers/bluetooth/btintel.c | 229 > ++++++++++++++++++++++++++++++++++++ > > drivers/bluetooth/btintel.h | 18 +++ > > 2 files changed, 247 insertions(+) > > > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > > index 818681c89db8..d3dc703eba78 100644 > > --- a/drivers/bluetooth/btintel.c > > +++ b/drivers/bluetooth/btintel.c > > @@ -9,6 +9,7 @@ > > #include <linux/module.h> > > #include <linux/firmware.h> > > #include <linux/regmap.h> > > +#include <linux/acpi.h> > > #include <asm/unaligned.h> > > > > #include <net/bluetooth/bluetooth.h> @@ -32,6 +33,9 @@ struct > > cmd_write_boot_params { > > u8 fw_build_yy; > > } __packed; > > > > +#define BTINTEL_SAR_NAME "BRDS" > > +#define BTINTEL_SAR_PREFIX "\\_SB_.PC00.XHCI.RHUB" > > + > > int btintel_check_bdaddr(struct hci_dev *hdev) > > { > > struct hci_rp_read_bd_addr *bda; > > @@ -2250,6 +2254,228 @@ static int btintel_configure_offload(struct > hci_dev *hdev) > > return err; > > } > > > > +static acpi_status btintel_sar_callback(acpi_handle handle, u32 lvl, void > *data, > > + void **ret) > > +{ > > + acpi_status status; > > + int len; > > size_t Ack. > > > + struct btintel_sar *sar; > > + union acpi_object *p, *elements; > > + struct acpi_buffer string = {ACPI_ALLOCATE_BUFFER, NULL}; > > + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL}; > > + > > + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &string); > > + if (ACPI_FAILURE(status)) { > > + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); > > + return status; > > + } > > + > > + if (strncmp(BTINTEL_SAR_PREFIX, string.pointer, > > + strlen(BTINTEL_SAR_PREFIX))) { > > + kfree(string.pointer); > > + return AE_OK; > > + } > > + > > + len = strlen(string.pointer); > > + if (strncmp((char *)string.pointer + len - 4, BTINTEL_SAR_NAME, 4)) { > > + kfree(string.pointer); > > + return AE_OK; > > + } > > + kfree(string.pointer); > > + > > + status = acpi_evaluate_object(handle, NULL, NULL, &buffer); > > + if (ACPI_FAILURE(status)) { > > + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); > > + return status; > > + } > > + > > + p = buffer.pointer; > > + sar = data; > > + > > + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 2) { > > + kfree(buffer.pointer); > > + BT_DBG("Invalid object type or package count"); > > Please add the variable values to the debug line. > Ack. > > + return AE_ERROR; > > + } > > + > > + elements = p->package.elements; > > + > > + /* SAR table is located at element[1] */ > > + p = &elements[1]; > > + > > + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 8) { > > + kfree(buffer.pointer); > > + return AE_ERROR; > > + } > > + > > + sar->domain = (u8)p->package.elements[0].integer.value; > > + sar->type = (u8)p->package.elements[1].integer.value; > > + sar->br = (u32)p->package.elements[2].integer.value; > > + sar->edr2 = (u32)p->package.elements[3].integer.value; > > + sar->edr3 = (u32)p->package.elements[4].integer.value; > > + sar->le = (u32)p->package.elements[5].integer.value; > > + sar->le_2mhz = (u32)p->package.elements[6].integer.value; > > + sar->le_lr = (u32)p->package.elements[7].integer.value; > > + kfree(buffer.pointer); > > + return AE_CTRL_TERMINATE; > > +} > > + > > +static void btintel_send_sar_ddc(struct hci_dev *hdev, void *data, u8 > > +len) > > Use native type for `len`? `unsigned int`? > Ack. > > +{ > > + struct sk_buff *skb; > > + > > + skb = __hci_cmd_sync(hdev, 0xfc8b, len, data, HCI_CMD_TIMEOUT); > > + if (IS_ERR(skb)) { > > + bt_dev_warn(hdev, "Failed to send Intel Write SAR DDC > (%ld)", PTR_ERR(skb)); > > + return; > > + } > > + kfree_skb(skb); > > +} > > + > > +static int btintel_set_legacy_sar(struct hci_dev *hdev, struct > > +btintel_sar *sar) { > > + struct btintel_cp_ddc_write *cmd; > > + u8 buffer[64]; > > Don’t try to align with tabs? > Ack. > > + > > + if (!sar) > > + return -EINVAL; > > + > > + cmd = (void *)buffer; > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x0131); > > Add names for the command id’s? > Ack. > > + cmd->data[0] = sar->br >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x0132); > > + cmd->data[0] = sar->br >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 5; > > + cmd->id = cpu_to_le16(0x0137); > > + cmd->data[0] = sar->br >> 3; > > + cmd->data[1] = sar->edr2 >> 3; > > + cmd->data[2] = sar->edr3 >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 6); > > + > > + cmd->len = 5; > > + cmd->id = cpu_to_le16(0x0138); > > + cmd->data[0] = sar->br >> 3; > > + cmd->data[1] = sar->edr2 >> 3; > > + cmd->data[2] = sar->edr3 >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 6); > > + > > + cmd->len = 5; > > + cmd->id = cpu_to_le16(0x013b); > > + cmd->data[0] = sar->br >> 3; > > + cmd->data[1] = sar->edr2 >> 3; > > + cmd->data[2] = sar->edr3 >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 6); > > + > > + cmd->len = 5; > > + cmd->id = cpu_to_le16(0x013c); > > + cmd->data[0] = sar->br >> 3; > > + cmd->data[1] = sar->edr2 >> 3; > > + cmd->data[2] = sar->edr3 >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 6); > > + > > + return 0; > > +} > > + > > +static int btintel_set_mutual_sar(struct hci_dev *hdev, struct > > +btintel_sar *sar) { > > + u8 buffer[64]; > > + struct btintel_cp_ddc_write *cmd; > > + u8 enable[1] = {1}; > > + struct sk_buff *skb; > > + > > + if (!sar) > > + return -EINVAL; > > + > > + cmd = (void *)buffer; > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x019e); > > + if (!(sar->le_2mhz & BIT(7))) > > + cmd->data[0] = 0x01; > > + else > > + cmd->data[0] = 0x00; > > Use ternary operator? Ack. > > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x019f); > > + cmd->data[0] = sar->le_lr; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x01a0); > > + cmd->data[0] = sar->br; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x01a1); > > + cmd->data[0] = sar->edr2; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x01a2); > > + cmd->data[0] = sar->edr3; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x01a3); > > + cmd->data[0] = sar->le; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + skb = __hci_cmd_sync(hdev, 0xfe25, 1, enable, HCI_CMD_TIMEOUT); > > + if (IS_ERR(skb)) { > > + bt_dev_warn(hdev, "Failed to send Intel SAR Enable (%ld)", > PTR_ERR(skb)); > > + return PTR_ERR(skb); > > + } > > + kfree_skb(skb); > > + > > + return 0; > > +} > > + > > +static int btintel_set_specific_absorption_rate(struct hci_dev *hdev, > > + struct intel_version_tlv *ver) > > +{ > > + acpi_status status; > > + struct btintel_sar sar; > > + > > + switch (ver->cnvr_top & 0xfff) { > > + case 0x810: /* MsP */ > > + break; > > + default: > > + return 0; > > + } > > + > > + memset(&sar, 0, sizeof(sar)); > > + > > + status = acpi_walk_namespace(ACPI_TYPE_METHOD, > ACPI_ROOT_OBJECT, > > + ACPI_UINT32_MAX, NULL, > > + btintel_sar_callback, &sar, NULL); > > + > > + if (ACPI_FAILURE(status)) > > + return -1; > > + > > + if (sar.domain != 0x12) > > + return -1; > > + > > + /* No need to configure controller if Bluetooth SAR is disabled in > BIOS > > + */ > > Put it on the line above? > Ack. > > + if (!sar.type) > > + return 0; > > + > > + if (sar.type == 1) { > > + bt_dev_info(hdev, "Applying both legacy and mutual > Bluetooth SAR"); > > + btintel_set_legacy_sar(hdev, &sar); > > + btintel_set_mutual_sar(hdev, &sar); > > + } > > + return 0; > > +} > > + > > static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, > > struct intel_version_tlv *ver) > > { > > @@ -2294,6 +2520,9 @@ static int btintel_bootloader_setup_tlv(struct > hci_dev *hdev, > > /* Read supported use cases and set callbacks to fetch datapath id */ > > btintel_configure_offload(hdev); > > > > + /* Set Specific Absorption Rate */ > > + btintel_set_specific_absorption_rate(hdev, ver); > > + > > hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT); > > > > /* Read the Intel version information after loading the FW */ diff > > --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h > > index e0060e58573c..7aa58fb7b02a 100644 > > --- a/drivers/bluetooth/btintel.h > > +++ b/drivers/bluetooth/btintel.h > > @@ -137,6 +137,24 @@ struct intel_offload_use_cases { > > __u8 preset[8]; > > } __packed; > > > > +/* structure to store the data read from ACPI table */ struct > > +btintel_sar { > > + u8 domain; > > + u8 type; > > + u32 br; > > + u32 edr2; > > + u32 edr3; > > + u32 le; > > + u32 le_2mhz; > > + u32 le_lr; > > Can’t native types be used? No. some fields size is 4 bytes. > > > +}; > > + > > +struct btintel_cp_ddc_write { > > + u8 len; > > + __le16 id; > > + u8 data[0]; > > +} __packed; > > + > > #define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & > 0x0000ff00) >> 8)) > > #define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & > 0x003f0000) >> 16)) > > #define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff) Thanks, Kiran
Hi Luiz, > -----Original Message----- > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > Sent: Tuesday, April 12, 2022 12:15 AM > To: K, Kiran <kiran.k@intel.com> > Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar > <ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan > <chethan.tumkur.narayan@intel.com>; Sreemantha, Seema > <seema.sreemantha@intel.com> > Subject: Re: [PATCH v1] Bluetooth: btintel: Add support to configure TX > power > > Hi Kiran, > > On Fri, Apr 8, 2022 at 1:02 AM Kiran K <kiran.k@intel.com> wrote: > > > > BRDS - Bluetooth Regulatory Domain Specific absorption rate > > ----------------------------------------------------------- > > > > Bluetooth has regulatory limitations which prohibit or allow usage of > > certain bands or channels as well as limiting Tx power. The Tx power > > values can be configured in ACPI table. This patch reads ACPI entry of > > Bluetooth SAR and configures the controller accordingly. > > It should be great to have a trace example of the commands in use, so if we > can have btmon decoding these to show what is being sent that way we can > also debug if something goes wrong. > Ack. I will add decoding of ddc commands to btmon and submit a separate patch. > > Signed-off-by: Kiran K <kiran.k@intel.com> > > Signed-off-by: Seema S <seema.sreemantha@intel.com> > > --- > > drivers/bluetooth/btintel.c | 229 > > ++++++++++++++++++++++++++++++++++++ > > drivers/bluetooth/btintel.h | 18 +++ > > 2 files changed, 247 insertions(+) > > > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > > index 818681c89db8..d3dc703eba78 100644 > > --- a/drivers/bluetooth/btintel.c > > +++ b/drivers/bluetooth/btintel.c > > @@ -9,6 +9,7 @@ > > #include <linux/module.h> > > #include <linux/firmware.h> > > #include <linux/regmap.h> > > +#include <linux/acpi.h> > > #include <asm/unaligned.h> > > > > #include <net/bluetooth/bluetooth.h> > > @@ -32,6 +33,9 @@ struct cmd_write_boot_params { > > u8 fw_build_yy; > > } __packed; > > > > +#define BTINTEL_SAR_NAME "BRDS" > > +#define BTINTEL_SAR_PREFIX "\\_SB_.PC00.XHCI.RHUB" > > + > > int btintel_check_bdaddr(struct hci_dev *hdev) { > > struct hci_rp_read_bd_addr *bda; @@ -2250,6 +2254,228 @@ > > static int btintel_configure_offload(struct hci_dev *hdev) > > return err; > > } > > > > +static acpi_status btintel_sar_callback(acpi_handle handle, u32 lvl, void > *data, > > + void **ret) { > > + acpi_status status; > > + int len; > > + struct btintel_sar *sar; > > + union acpi_object *p, *elements; > > + struct acpi_buffer string = {ACPI_ALLOCATE_BUFFER, NULL}; > > + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL}; > > + > > + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &string); > > + if (ACPI_FAILURE(status)) { > > + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); > > + return status; > > + } > > + > > + if (strncmp(BTINTEL_SAR_PREFIX, string.pointer, > > + strlen(BTINTEL_SAR_PREFIX))) { > > + kfree(string.pointer); > > + return AE_OK; > > + } > > + > > + len = strlen(string.pointer); > > + if (strncmp((char *)string.pointer + len - 4, BTINTEL_SAR_NAME, 4)) { > > + kfree(string.pointer); > > + return AE_OK; > > + } > > + kfree(string.pointer); > > + > > + status = acpi_evaluate_object(handle, NULL, NULL, &buffer); > > + if (ACPI_FAILURE(status)) { > > + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); > > + return status; > > + } > > + > > + p = buffer.pointer; > > + sar = data; > > + > > + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 2) { > > + kfree(buffer.pointer); > > + BT_DBG("Invalid object type or package count"); > > + return AE_ERROR; > > + } > > + > > + elements = p->package.elements; > > + > > + /* SAR table is located at element[1] */ > > + p = &elements[1]; > > + > > + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 8) { > > + kfree(buffer.pointer); > > + return AE_ERROR; > > + } > > + > > + sar->domain = (u8)p->package.elements[0].integer.value; > > + sar->type = (u8)p->package.elements[1].integer.value; > > + sar->br = (u32)p->package.elements[2].integer.value; > > + sar->edr2 = (u32)p->package.elements[3].integer.value; > > + sar->edr3 = (u32)p->package.elements[4].integer.value; > > + sar->le = (u32)p->package.elements[5].integer.value; > > + sar->le_2mhz = (u32)p->package.elements[6].integer.value; > > + sar->le_lr = (u32)p->package.elements[7].integer.value; > > + kfree(buffer.pointer); > > + return AE_CTRL_TERMINATE; > > +} > > + > > +static void btintel_send_sar_ddc(struct hci_dev *hdev, void *data, u8 > > +len) { > > + struct sk_buff *skb; > > + > > + skb = __hci_cmd_sync(hdev, 0xfc8b, len, data, HCI_CMD_TIMEOUT); > > + if (IS_ERR(skb)) { > > + bt_dev_warn(hdev, "Failed to send Intel Write SAR DDC (%ld)", > PTR_ERR(skb)); > > + return; > > + } > > + kfree_skb(skb); > > +} > > + > > +static int btintel_set_legacy_sar(struct hci_dev *hdev, struct > > +btintel_sar *sar) { > > + struct btintel_cp_ddc_write *cmd; > > + u8 buffer[64]; > > + > > + if (!sar) > > + return -EINVAL; > > Add a memset to 0 as this may trigger uninitialized errors in the likes of static > analyzers. > Ack. > > + cmd = (void *)buffer; > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x0131); > > + cmd->data[0] = sar->br >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > We should probably handle the errors when sending these commands and > don't continue, also perhaps it would be better to have each command in its > own function with a proper name indicating what it is doing. > The command structure is same for all the commands. To address one of Paul's comment, need to define marcos for command id. Let me know if this is OK. > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x0132); > > + cmd->data[0] = sar->br >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 5; > > + cmd->id = cpu_to_le16(0x0137); > > + cmd->data[0] = sar->br >> 3; > > + cmd->data[1] = sar->edr2 >> 3; > > + cmd->data[2] = sar->edr3 >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 6); > > + > > + cmd->len = 5; > > + cmd->id = cpu_to_le16(0x0138); > > + cmd->data[0] = sar->br >> 3; > > + cmd->data[1] = sar->edr2 >> 3; > > + cmd->data[2] = sar->edr3 >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 6); > > + > > + cmd->len = 5; > > + cmd->id = cpu_to_le16(0x013b); > > + cmd->data[0] = sar->br >> 3; > > + cmd->data[1] = sar->edr2 >> 3; > > + cmd->data[2] = sar->edr3 >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 6); > > + > > + cmd->len = 5; > > + cmd->id = cpu_to_le16(0x013c); > > + cmd->data[0] = sar->br >> 3; > > + cmd->data[1] = sar->edr2 >> 3; > > + cmd->data[2] = sar->edr3 >> 3; > > + btintel_send_sar_ddc(hdev, cmd, 6); > > + > > + return 0; > > +} > > + > > +static int btintel_set_mutual_sar(struct hci_dev *hdev, struct > > +btintel_sar *sar) { > > + u8 buffer[64]; > > + struct btintel_cp_ddc_write *cmd; > > + u8 enable[1] = {1}; > > + struct sk_buff *skb; > > + > > + if (!sar) > > + return -EINVAL; > > + > > + cmd = (void *)buffer; > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x019e); > > + if (!(sar->le_2mhz & BIT(7))) > > + cmd->data[0] = 0x01; > > + else > > + cmd->data[0] = 0x00; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > Ditto as above. > > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x019f); > > + cmd->data[0] = sar->le_lr; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x01a0); > > + cmd->data[0] = sar->br; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x01a1); > > + cmd->data[0] = sar->edr2; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x01a2); > > + cmd->data[0] = sar->edr3; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + cmd->len = 3; > > + cmd->id = cpu_to_le16(0x01a3); > > + cmd->data[0] = sar->le; > > + btintel_send_sar_ddc(hdev, cmd, 4); > > + > > + skb = __hci_cmd_sync(hdev, 0xfe25, 1, enable, HCI_CMD_TIMEOUT); > > + if (IS_ERR(skb)) { > > + bt_dev_warn(hdev, "Failed to send Intel SAR Enable (%ld)", > PTR_ERR(skb)); > > + return PTR_ERR(skb); > > + } > > + kfree_skb(skb); > > + > > + return 0; > > +} > > + > > +static int btintel_set_specific_absorption_rate(struct hci_dev *hdev, > > + struct > > +intel_version_tlv *ver) { > > + acpi_status status; > > + struct btintel_sar sar; > > + > > + switch (ver->cnvr_top & 0xfff) { > > + case 0x810: /* MsP */ > > + break; > > + default: > > + return 0; > > + } > > + > > + memset(&sar, 0, sizeof(sar)); > > + > > + status = acpi_walk_namespace(ACPI_TYPE_METHOD, > ACPI_ROOT_OBJECT, > > + ACPI_UINT32_MAX, NULL, > > + btintel_sar_callback, &sar, > > + NULL); > > + > > + if (ACPI_FAILURE(status)) > > + return -1; > > + > > + if (sar.domain != 0x12) > > + return -1; > > + > > + /* No need to configure controller if Bluetooth SAR is disabled in BIOS > > + */ > > + if (!sar.type) > > + return 0; > > + > > + if (sar.type == 1) { > > + bt_dev_info(hdev, "Applying both legacy and mutual Bluetooth > SAR"); > > + btintel_set_legacy_sar(hdev, &sar); > > + btintel_set_mutual_sar(hdev, &sar); > > + } > > + return 0; > > +} > > + > > static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, > > struct intel_version_tlv *ver) > > { @@ -2294,6 +2520,9 @@ static int btintel_bootloader_setup_tlv(struct > > hci_dev *hdev, > > /* Read supported use cases and set callbacks to fetch datapath id */ > > btintel_configure_offload(hdev); > > > > + /* Set Specific Absorption Rate */ > > + btintel_set_specific_absorption_rate(hdev, ver); > > + > > hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT); > > > > /* Read the Intel version information after loading the FW */ > > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h > > index e0060e58573c..7aa58fb7b02a 100644 > > --- a/drivers/bluetooth/btintel.h > > +++ b/drivers/bluetooth/btintel.h > > @@ -137,6 +137,24 @@ struct intel_offload_use_cases { > > __u8 preset[8]; > > } __packed; > > > > +/* structure to store the data read from ACPI table */ struct > > +btintel_sar { > > + u8 domain; > > + u8 type; > > + u32 br; > > + u32 edr2; > > + u32 edr3; > > + u32 le; > > + u32 le_2mhz; > > + u32 le_lr; > > +}; > > The above should also be using __packed, also for data packets the multibyte > field shall tell what is the expected endianness so given the id below is using > __le16 I assume the fields above shall be __le32. > > > +struct btintel_cp_ddc_write { > > + u8 len; > > + __le16 id; > > + u8 data[0]; > > +} __packed; > > + > > #define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & 0x0000ff00) > >> 8)) > > #define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & 0x003f0000) > >> 16)) > > #define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff) > > -- > > 2.17.1 > > > > > -- > Luiz Augusto von Dentz Thanks, Kiran
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c index 818681c89db8..d3dc703eba78 100644 --- a/drivers/bluetooth/btintel.c +++ b/drivers/bluetooth/btintel.c @@ -9,6 +9,7 @@ #include <linux/module.h> #include <linux/firmware.h> #include <linux/regmap.h> +#include <linux/acpi.h> #include <asm/unaligned.h> #include <net/bluetooth/bluetooth.h> @@ -32,6 +33,9 @@ struct cmd_write_boot_params { u8 fw_build_yy; } __packed; +#define BTINTEL_SAR_NAME "BRDS" +#define BTINTEL_SAR_PREFIX "\\_SB_.PC00.XHCI.RHUB" + int btintel_check_bdaddr(struct hci_dev *hdev) { struct hci_rp_read_bd_addr *bda; @@ -2250,6 +2254,228 @@ static int btintel_configure_offload(struct hci_dev *hdev) return err; } +static acpi_status btintel_sar_callback(acpi_handle handle, u32 lvl, void *data, + void **ret) +{ + acpi_status status; + int len; + struct btintel_sar *sar; + union acpi_object *p, *elements; + struct acpi_buffer string = {ACPI_ALLOCATE_BUFFER, NULL}; + struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL}; + + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &string); + if (ACPI_FAILURE(status)) { + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); + return status; + } + + if (strncmp(BTINTEL_SAR_PREFIX, string.pointer, + strlen(BTINTEL_SAR_PREFIX))) { + kfree(string.pointer); + return AE_OK; + } + + len = strlen(string.pointer); + if (strncmp((char *)string.pointer + len - 4, BTINTEL_SAR_NAME, 4)) { + kfree(string.pointer); + return AE_OK; + } + kfree(string.pointer); + + status = acpi_evaluate_object(handle, NULL, NULL, &buffer); + if (ACPI_FAILURE(status)) { + BT_DBG("ACPI Failure: %s", acpi_format_exception(status)); + return status; + } + + p = buffer.pointer; + sar = data; + + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 2) { + kfree(buffer.pointer); + BT_DBG("Invalid object type or package count"); + return AE_ERROR; + } + + elements = p->package.elements; + + /* SAR table is located at element[1] */ + p = &elements[1]; + + if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 8) { + kfree(buffer.pointer); + return AE_ERROR; + } + + sar->domain = (u8)p->package.elements[0].integer.value; + sar->type = (u8)p->package.elements[1].integer.value; + sar->br = (u32)p->package.elements[2].integer.value; + sar->edr2 = (u32)p->package.elements[3].integer.value; + sar->edr3 = (u32)p->package.elements[4].integer.value; + sar->le = (u32)p->package.elements[5].integer.value; + sar->le_2mhz = (u32)p->package.elements[6].integer.value; + sar->le_lr = (u32)p->package.elements[7].integer.value; + kfree(buffer.pointer); + return AE_CTRL_TERMINATE; +} + +static void btintel_send_sar_ddc(struct hci_dev *hdev, void *data, u8 len) +{ + struct sk_buff *skb; + + skb = __hci_cmd_sync(hdev, 0xfc8b, len, data, HCI_CMD_TIMEOUT); + if (IS_ERR(skb)) { + bt_dev_warn(hdev, "Failed to send Intel Write SAR DDC (%ld)", PTR_ERR(skb)); + return; + } + kfree_skb(skb); +} + +static int btintel_set_legacy_sar(struct hci_dev *hdev, struct btintel_sar *sar) +{ + struct btintel_cp_ddc_write *cmd; + u8 buffer[64]; + + if (!sar) + return -EINVAL; + + cmd = (void *)buffer; + + cmd->len = 3; + cmd->id = cpu_to_le16(0x0131); + cmd->data[0] = sar->br >> 3; + btintel_send_sar_ddc(hdev, cmd, 4); + + cmd->len = 3; + cmd->id = cpu_to_le16(0x0132); + cmd->data[0] = sar->br >> 3; + btintel_send_sar_ddc(hdev, cmd, 4); + + cmd->len = 5; + cmd->id = cpu_to_le16(0x0137); + cmd->data[0] = sar->br >> 3; + cmd->data[1] = sar->edr2 >> 3; + cmd->data[2] = sar->edr3 >> 3; + btintel_send_sar_ddc(hdev, cmd, 6); + + cmd->len = 5; + cmd->id = cpu_to_le16(0x0138); + cmd->data[0] = sar->br >> 3; + cmd->data[1] = sar->edr2 >> 3; + cmd->data[2] = sar->edr3 >> 3; + btintel_send_sar_ddc(hdev, cmd, 6); + + cmd->len = 5; + cmd->id = cpu_to_le16(0x013b); + cmd->data[0] = sar->br >> 3; + cmd->data[1] = sar->edr2 >> 3; + cmd->data[2] = sar->edr3 >> 3; + btintel_send_sar_ddc(hdev, cmd, 6); + + cmd->len = 5; + cmd->id = cpu_to_le16(0x013c); + cmd->data[0] = sar->br >> 3; + cmd->data[1] = sar->edr2 >> 3; + cmd->data[2] = sar->edr3 >> 3; + btintel_send_sar_ddc(hdev, cmd, 6); + + return 0; +} + +static int btintel_set_mutual_sar(struct hci_dev *hdev, struct btintel_sar *sar) +{ + u8 buffer[64]; + struct btintel_cp_ddc_write *cmd; + u8 enable[1] = {1}; + struct sk_buff *skb; + + if (!sar) + return -EINVAL; + + cmd = (void *)buffer; + + cmd->len = 3; + cmd->id = cpu_to_le16(0x019e); + if (!(sar->le_2mhz & BIT(7))) + cmd->data[0] = 0x01; + else + cmd->data[0] = 0x00; + btintel_send_sar_ddc(hdev, cmd, 4); + + cmd->len = 3; + cmd->id = cpu_to_le16(0x019f); + cmd->data[0] = sar->le_lr; + btintel_send_sar_ddc(hdev, cmd, 4); + + cmd->len = 3; + cmd->id = cpu_to_le16(0x01a0); + cmd->data[0] = sar->br; + btintel_send_sar_ddc(hdev, cmd, 4); + + cmd->len = 3; + cmd->id = cpu_to_le16(0x01a1); + cmd->data[0] = sar->edr2; + btintel_send_sar_ddc(hdev, cmd, 4); + + cmd->len = 3; + cmd->id = cpu_to_le16(0x01a2); + cmd->data[0] = sar->edr3; + btintel_send_sar_ddc(hdev, cmd, 4); + + cmd->len = 3; + cmd->id = cpu_to_le16(0x01a3); + cmd->data[0] = sar->le; + btintel_send_sar_ddc(hdev, cmd, 4); + + skb = __hci_cmd_sync(hdev, 0xfe25, 1, enable, HCI_CMD_TIMEOUT); + if (IS_ERR(skb)) { + bt_dev_warn(hdev, "Failed to send Intel SAR Enable (%ld)", PTR_ERR(skb)); + return PTR_ERR(skb); + } + kfree_skb(skb); + + return 0; +} + +static int btintel_set_specific_absorption_rate(struct hci_dev *hdev, + struct intel_version_tlv *ver) +{ + acpi_status status; + struct btintel_sar sar; + + switch (ver->cnvr_top & 0xfff) { + case 0x810: /* MsP */ + break; + default: + return 0; + } + + memset(&sar, 0, sizeof(sar)); + + status = acpi_walk_namespace(ACPI_TYPE_METHOD, ACPI_ROOT_OBJECT, + ACPI_UINT32_MAX, NULL, + btintel_sar_callback, &sar, NULL); + + if (ACPI_FAILURE(status)) + return -1; + + if (sar.domain != 0x12) + return -1; + + /* No need to configure controller if Bluetooth SAR is disabled in BIOS + */ + if (!sar.type) + return 0; + + if (sar.type == 1) { + bt_dev_info(hdev, "Applying both legacy and mutual Bluetooth SAR"); + btintel_set_legacy_sar(hdev, &sar); + btintel_set_mutual_sar(hdev, &sar); + } + return 0; +} + static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver) { @@ -2294,6 +2520,9 @@ static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, /* Read supported use cases and set callbacks to fetch datapath id */ btintel_configure_offload(hdev); + /* Set Specific Absorption Rate */ + btintel_set_specific_absorption_rate(hdev, ver); + hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT); /* Read the Intel version information after loading the FW */ diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h index e0060e58573c..7aa58fb7b02a 100644 --- a/drivers/bluetooth/btintel.h +++ b/drivers/bluetooth/btintel.h @@ -137,6 +137,24 @@ struct intel_offload_use_cases { __u8 preset[8]; } __packed; +/* structure to store the data read from ACPI table */ +struct btintel_sar { + u8 domain; + u8 type; + u32 br; + u32 edr2; + u32 edr3; + u32 le; + u32 le_2mhz; + u32 le_lr; +}; + +struct btintel_cp_ddc_write { + u8 len; + __le16 id; + u8 data[0]; +} __packed; + #define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & 0x0000ff00) >> 8)) #define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & 0x003f0000) >> 16)) #define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff)