Hi Vijay,
On Fri, Oct 25, 2024 at 8:17 AM Vijay Satija <vijay.satija@xxxxxxxxx> wrote:
>
> Bluetooth has regulatory limitations which prohibit or allow usage
> of certain bands or channels or limiting Tx power. The Tx power
> values can be configured in ACPI table. This patch reads from ACPI entry
> and configures the TX power to be used accordingly.
>
> Signed-off-by: Vijay Satija <vijay.satija@xxxxxxxxx>
> Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
> ---
> drivers/bluetooth/btintel.c | 313 ++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h | 35 ++++
> 2 files changed, 348 insertions(+)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index d496cf2c3411..aa4ebe230cc5 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -29,6 +29,11 @@
>
> #define BTINTEL_EFI_DSBR L"UefiCnvCommonDSBR"
>
> +#define BTINTEL_BT_DOMAIN 0x12
> +#define BTINTEL_SAR_LEGACY 0
> +#define BTINTEL_SAR_INC_PWR 1
> +#define BTINTEL_SAR_INC_PWR_SUPPORTED 0
> +
> enum {
> DSM_SET_WDISABLE2_DELAY = 1,
> DSM_SET_RESET_METHOD = 3,
> @@ -2799,6 +2804,311 @@ static int btintel_set_dsbr(struct hci_dev *hdev, struct intel_version_tlv *ver)
> return 0;
> }
>
> +#ifdef CONFIG_ACPI
> +static acpi_status btintel_evaluate_acpi_method(struct hci_dev *hdev,
> + acpi_string method,
> + union acpi_object **ptr,
> + u8 pkg_size)
> +{
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *p;
> + acpi_status status;
> + acpi_handle handle;
> +
> + handle = ACPI_HANDLE(GET_HCIDEV_DEV(hdev));
> + if (!handle) {
> + bt_dev_dbg(hdev, "ACPI-BT: No ACPI support for Bluetooth device");
> + return AE_NOT_EXIST;
> + }
> +
> + status = acpi_evaluate_object(handle, method, NULL, &buffer);
> +
> + if (ACPI_FAILURE(status)) {
> + bt_dev_dbg(hdev, "ACPI-BT: ACPI Failure: %s method: %s",
> + acpi_format_exception(status), method);
> + return status;
> + }
> +
> + p = buffer.pointer;
> +
> + if (p->type != ACPI_TYPE_PACKAGE || p->package.count < pkg_size) {
> + bt_dev_warn(hdev, "ACPI-BT: Invalid object type: %d or package count: %d",
> + p->type, p->package.count);
> + kfree(buffer.pointer);
> + return AE_ERROR;
> + }
> +
> + *ptr = buffer.pointer;
> + return 0;
> +}
> +
> +static union acpi_object *btintel_acpi_get_bt_pkg(union acpi_object *buffer)
> +{
> + union acpi_object *domain, *bt_pkg;
> + int i;
> +
> + for (i = 1; i < buffer->package.count; i++) {
> + bt_pkg = &buffer->package.elements[i];
> + domain = &bt_pkg->package.elements[0];
> + if (domain->type == ACPI_TYPE_INTEGER &&
> + domain->integer.value == BTINTEL_BT_DOMAIN)
> + return bt_pkg;
> + }
> + return ERR_PTR(-ENOENT);
> +}
> +
> +static int btintel_send_sar_ddc(struct hci_dev *hdev, struct btintel_cp_ddc_write *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 sar ddc id:0x%4.4x (%ld)",
> + le16_to_cpu(data->id), PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> + kfree_skb(skb);
> + return 0;
> +}
> +
> +static int btintel_set_legacy_sar(struct hci_dev *hdev,
> + struct btintel_sar_inc_pwr *sar)
> +{
> + struct btintel_cp_ddc_write *cmd;
> + u8 buffer[64];
> + int ret;
> +
> + cmd = (void *)buffer;
> + cmd->len = 3;
> + cmd->id = cpu_to_le16(0x0131);
> + cmd->data[0] = sar->br >> 3;
> + ret = btintel_send_sar_ddc(hdev, cmd, 4);
> + if (ret)
> + return ret;
It might be a good idea to handle each command on its own function and
then name it according to what the command would be doing.
> +
> + cmd->len = 3;
> + cmd->id = cpu_to_le16(0x0132);
> + cmd->data[0] = sar->br >> 3;
> + ret = btintel_send_sar_ddc(hdev, cmd, 4);
> + if (ret)
> + return ret;
> +
> + cmd->len = 3;
> + cmd->id = cpu_to_le16(0x0133);
> + cmd->data[0] = min3(sar->le, sar->le_lr, sar->le_2mhz) >> 3;
> + ret = btintel_send_sar_ddc(hdev, cmd, 4);
> + if (ret)
> + return ret;
> +
> + 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;
> + ret = btintel_send_sar_ddc(hdev, cmd, 6);
> + if (ret)
> + return ret;
> +
> + 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;
> + ret = btintel_send_sar_ddc(hdev, cmd, 6);
> + if (ret)
> + return ret;
> +
> + 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;
> + ret = btintel_send_sar_ddc(hdev, cmd, 6);
> + if (ret)
> + return ret;
> +
> + 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;
> + ret = btintel_send_sar_ddc(hdev, cmd, 6);
> +
> + return ret;
> +}
> +
> +static int btintel_set_mutual_sar(struct hci_dev *hdev,
> + struct btintel_sar_inc_pwr *sar)
> +{
> + struct btintel_cp_ddc_write *cmd;
> + struct sk_buff *skb;
> + u8 buffer[64];
> + bool enable;
> + int ret;
> +
> + cmd = (void *)buffer;
> +
> + cmd->len = 3;
> + cmd->id = cpu_to_le16(0x019e);
> +
> + if (sar->revision == BTINTEL_SAR_INC_PWR &&
> + sar->inc_power_mode == BTINTEL_SAR_INC_PWR_SUPPORTED)
> + cmd->data[0] = 0x01;
> + else
> + cmd->data[0] = 0x00;
> +
> + ret = btintel_send_sar_ddc(hdev, cmd, 4);
> + if (ret)
> + return ret;
> +
> + if (sar->revision == BTINTEL_SAR_INC_PWR &&
> + sar->inc_power_mode == BTINTEL_SAR_INC_PWR_SUPPORTED) {
> + cmd->len = 3;
> + cmd->id = cpu_to_le16(0x019f);
> + cmd->data[0] = sar->sar_2400_chain_a;
> +
> + ret = btintel_send_sar_ddc(hdev, cmd, 4);
> + if (ret)
> + return ret;
> + }
> +
> + cmd->len = 3;
> + cmd->id = cpu_to_le16(0x01a0);
> + cmd->data[0] = sar->br;
> + ret = btintel_send_sar_ddc(hdev, cmd, 4);
> + if (ret)
> + return ret;
Ditto, hopefully if we have dedicated functions for each of these
commands we can reuse some of them.
> + cmd->len = 3;
> + cmd->id = cpu_to_le16(0x01a1);
> + cmd->data[0] = sar->edr2;
> + ret = btintel_send_sar_ddc(hdev, cmd, 4);
> + if (ret)
> + return ret;
> +
> + cmd->len = 3;
> + cmd->id = cpu_to_le16(0x01a2);
> + cmd->data[0] = sar->edr3;
> + ret = btintel_send_sar_ddc(hdev, cmd, 4);
> + if (ret)
> + return ret;
> +
> + cmd->len = 3;
> + cmd->id = cpu_to_le16(0x01a3);
> + cmd->data[0] = min3(sar->le, sar->le_lr, sar->le_2mhz);
> + ret = btintel_send_sar_ddc(hdev, cmd, 4);
> + if (ret)
> + return ret;
> +
> + enable = true;
> + 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_sar_send_to_device(struct hci_dev *hdev, struct btintel_sar_inc_pwr *sar,
> + struct intel_version_tlv *ver)
> +{
> + u16 cnvi, cnvr;
> + int ret;
> +
> + cnvi = ver->cnvi_top & 0xfff;
> + cnvr = ver->cnvr_top & 0xfff;
> +
> + if (cnvi < BTINTEL_CNVI_BLAZARI && cnvr < BTINTEL_CNVR_FMP2) {
> + bt_dev_info(hdev, "Applying legacy Bluetooth SAR");
> + ret = btintel_set_legacy_sar(hdev, sar);
> + } else if (cnvi == BTINTEL_CNVI_GAP || cnvr == BTINTEL_CNVR_FMP2) {
> + bt_dev_info(hdev, "Applying Mutual Bluetooth SAR");
> + ret = btintel_set_mutual_sar(hdev, sar);
> + } else {
> + ret = -EOPNOTSUPP;
> + }
> +
> + return ret;
> +}
> +
> +static int btintel_acpi_set_sar(struct hci_dev *hdev, struct intel_version_tlv *ver)
> +{
> + union acpi_object *bt_pkg, *buffer = NULL;
> + struct btintel_sar_inc_pwr sar;
> + acpi_status status;
> + u8 revision;
> + int ret;
> +
> + status = btintel_evaluate_acpi_method(hdev, "BRDS", &buffer, 2);
> + if (ACPI_FAILURE(status))
> + return -ENOENT;
> +
> + bt_pkg = btintel_acpi_get_bt_pkg(buffer);
> +
> + if (IS_ERR(bt_pkg)) {
> + ret = PTR_ERR(bt_pkg);
> + goto error;
> + }
> +
> + revision = buffer->package.elements[0].integer.value;
> +
> + if (revision > BTINTEL_SAR_INC_PWR) {
> + bt_dev_dbg(hdev, "BT_SAR: revision: 0x%2.2x not supported", revision);
> + ret = -EOPNOTSUPP;
> + goto error;
> + }
> +
> + if (revision == BTINTEL_SAR_LEGACY && bt_pkg->package.count != 7) {
> + sar.revision = revision;
> + sar.bt_sar_bios = (u32)bt_pkg->package.elements[1].integer.value;
> + sar.br = (u8)bt_pkg->package.elements[2].integer.value;
> + sar.edr2 = (u8)bt_pkg->package.elements[3].integer.value;
> + sar.edr3 = (u8)bt_pkg->package.elements[4].integer.value;
> + sar.le = (u8)bt_pkg->package.elements[5].integer.value;
> + sar.le_2mhz = (u8)bt_pkg->package.elements[6].integer.value;
> + sar.le_lr = (u8)bt_pkg->package.elements[7].integer.value;
Hmm, are you sure the check is correct? It is != 7 but then it goes
ahead and access element[7]? Are these values aligned and are the acpi
values guaranteed to be in the same endianness as the host? Even if
they are, I'd say using the likes of le32_to_cpu is probably a good
idea anyway.
> +
> + } else if (revision == BTINTEL_SAR_INC_PWR && bt_pkg->package.count != 9) {
> + sar.revision = revision;
> + sar.bt_sar_bios = (u32)bt_pkg->package.elements[1].integer.value;
> + sar.inc_power_mode = (u32)bt_pkg->package.elements[2].integer.value;
> + sar.sar_2400_chain_a = (u8)bt_pkg->package.elements[3].integer.value;
> + sar.br = (u8)bt_pkg->package.elements[4].integer.value;
> + sar.edr2 = (u8)bt_pkg->package.elements[5].integer.value;
> + sar.edr3 = (u8)bt_pkg->package.elements[6].integer.value;
> + sar.le = (u8)bt_pkg->package.elements[7].integer.value;
> + sar.le_2mhz = (u8)bt_pkg->package.elements[8].integer.value;
> + sar.le_lr = (u8)bt_pkg->package.elements[9].integer.value;
Ditto, != 9 then it attempts to access element[9]
> + } else {
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + /* Apply only if it is enabled in BIOS */
> + if (sar.bt_sar_bios != 1) {
> + bt_dev_dbg(hdev, "Bluetooth SAR is not enabled");
> + ret = -EOPNOTSUPP;
> + goto error;
> + }
> +
> + ret = btintel_sar_send_to_device(hdev, &sar, ver);
> +error:
> + kfree(buffer);
> + return ret;
> +}
> +#endif /* CONFIG_ACPI */
> +
> +static int btintel_set_specific_absorption_rate(struct hci_dev *hdev,
> + struct intel_version_tlv *ver)
> +{
> +#ifdef CONFIG_ACPI
> + return btintel_acpi_set_sar(hdev, ver);
> +#endif
> + return 0;
> +}
> +
> int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
> struct intel_version_tlv *ver)
> {
> @@ -2876,6 +3186,9 @@ int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
>
> hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT);
>
> + /* Send sar values to controller */
> + btintel_set_specific_absorption_rate(hdev, ver);
> +
> /* Set PPAG feature */
> btintel_set_ppag(hdev, ver);
>
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index fa43eb137821..0b355cb49836 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -56,7 +56,10 @@ struct intel_tlv {
> #define BTINTEL_CNVI_BLAZARIW 0x901
> #define BTINTEL_CNVI_GAP 0x910
> #define BTINTEL_CNVI_BLAZARU 0x930
> +#define BTINTEL_CNVI_GAP 0x910
>
> +/* CNVR */
> +#define BTINTEL_CNVR_FMP2 0x910
> #define BTINTEL_IMG_BOOTLOADER 0x01 /* Bootloader image */
> #define BTINTEL_IMG_IML 0x02 /* Intermediate image */
> #define BTINTEL_IMG_OP 0x03 /* Operational image */
> @@ -164,6 +167,38 @@ struct hci_ppag_enable_cmd {
> #define INTEL_TLV_DEBUG_EXCEPTION 0x02
> #define INTEL_TLV_TEST_EXCEPTION 0xDE
>
> +struct btintel_cp_ddc_write {
> + u8 len;
> + __le16 id;
> + u8 data[];
> +} __packed;
> +
> +/* Bluetooth SAR feature (BRDS), Revision 0 */
> +struct btintel_sar {
> + u8 revision;
> + u32 bt_sar_bios; /* Mode of SAR control to be used, 1:enabled in bios */
> + u8 br;
> + u8 edr2;
> + u8 edr3;
> + u8 le;
> + u8 le_2mhz;
> + u8 le_lr;
> +} __packed;
> +
> +/* Bluetooth SAR feature (BRDS), Revision 1 */
> +struct btintel_sar_inc_pwr {
> + u8 revision;
> + u32 bt_sar_bios;
> + u32 inc_power_mode; /* Increased power mode */
For a packed multi-byte field I think it is always a good idea to have
it as a specific byte order rather than expecting it to be the same as
the host order.
> + u8 sar_2400_chain_a; /* Sar power restriction LB */
> + u8 br;
> + u8 edr2;
> + u8 edr3;
> + u8 le;
> + u8 le_2mhz;
> + u8 le_lr;
> +} __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.40.1
>
>
--
Luiz Augusto von Dentz
