[PATCH v1] Bluetooth: btintel: Add UEFI standalone static SAR support

[PATCH v1] Bluetooth: btintel: Add UEFI standalone static SAR support

[Sai Teja Aluvala]

Scorpius Peak2 and later platforms, excluding BlazarI and BlazarIW,
provide standalone static SAR limits through the UefiCnvBtBSSS
variable. Applying these limits requires SAR to be enabled in
UefiCnvBtBRDS as well as in BSSS.

Read the revision 2 UEFI BRDS and BSSS variables using their shared
layout. Program the BRDS 2.4 GHz limits, then enable standalone SAR
and write the BSSS per-chain sub-band limits. Send SAR initialization
complete after the DDC writes.

Reuse the existing SAR DDC helpers and configure UEFI SAR before
running the existing ACPI BRDS path. Keep the ACPI platform and
revision checks unchanged, independently of the UEFI result.

Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
---
 drivers/bluetooth/btintel.c | 151 ++++++++++++++++++++++++++++++++----
 1 file changed, 134 insertions(+), 17 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index a0aa110f4cdd..fe0b6b379245 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -29,6 +29,21 @@
 #define ECDSA_HEADER_LEN	320
 
 #define BTINTEL_EFI_DSBR	L"UefiCnvCommonDSBR"
+#define BTINTEL_EFI_BRDS	L"UefiCnvBtBRDS"
+#define BTINTEL_EFI_BSSS	L"UefiCnvBtBSSS"
+
+static efi_guid_t btintel_efi_sar_guid =
+	EFI_GUID(0x42780dd5, 0x9a7d, 0x404c,
+		 0x80, 0xe4, 0x7f, 0x70, 0x94, 0x36, 0x03, 0x94);
+
+/* BRDS and BSSS revision 2 use the same 19-byte UEFI layout. */
+struct btintel_uefi_sar {
+	u8 revision;
+	__le32 mode;
+	__le32 inc_power_mode;
+	u8 chain_a[5];
+	u8 chain_b[5];
+} __packed;
 
 enum {
 	DSM_SET_WDISABLE2_DELAY = 1,
@@ -3093,6 +3108,8 @@ static union acpi_object *btintel_acpi_get_bt_pkg(union acpi_object *buffer)
 	return ERR_PTR(-ENOENT);
 }
 
+#endif /* CONFIG_ACPI */
+
 static int btintel_send_sar_ddc(struct hci_dev *hdev, struct btintel_cp_ddc_write *data, u8 len)
 {
 	struct sk_buff *skb;
@@ -3107,6 +3124,18 @@ static int btintel_send_sar_ddc(struct hci_dev *hdev, struct btintel_cp_ddc_writ
 	return 0;
 }
 
+static int btintel_send_sar_rev2_band(struct hci_dev *hdev,
+				      struct btintel_cp_ddc_write *cmd,
+				      u16 id, u8 chain_a, u8 chain_b)
+{
+	cmd->len = 4;
+	cmd->id = cpu_to_le16(id);
+	cmd->data[0] = chain_a;
+	cmd->data[1] = chain_b;
+	return btintel_send_sar_ddc(hdev, cmd, 5);
+}
+
+#ifdef CONFIG_ACPI
 static int btintel_send_edr(struct hci_dev *hdev, struct btintel_cp_ddc_write *cmd,
 			    int id, struct btintel_sar_inc_pwr *sar)
 {
@@ -3260,23 +3289,6 @@ static int btintel_set_mutual_sar(struct hci_dev *hdev, struct btintel_sar_inc_p
 	return 0;
 }
 
-/* btintel_send_sar_rev2_band - send DDC command for one Rev2 sub-band
- *
- * Each DDC 0x0311-0x0316 carries 2 bytes: [ChainA_value, ChainB_value].
- * cmd->len = 4  (2 id + 2 data)
- * HCI total  = 5 bytes (1 len + 4)
- */
-static int btintel_send_sar_rev2_band(struct hci_dev *hdev,
-				      struct btintel_cp_ddc_write *cmd,
-				      u16 id, u8 chain_a, u8 chain_b)
-{
-	cmd->len = 4;
-	cmd->id = cpu_to_le16(id);
-	cmd->data[0] = chain_a;
-	cmd->data[1] = chain_b;
-	return btintel_send_sar_ddc(hdev, cmd, 5);
-}
-
 static int btintel_set_sar_rev2(struct hci_dev *hdev,
 				struct btintel_sar_rev2 *sar)
 {
@@ -3544,9 +3556,114 @@ static int btintel_acpi_set_sar(struct hci_dev *hdev, struct intel_version_tlv *
 }
 #endif /* CONFIG_ACPI */
 
+static int btintel_uefi_read_sar(struct hci_dev *hdev, efi_char16_t *name,
+				 struct btintel_uefi_sar *sar)
+{
+	unsigned long size = sizeof(*sar);
+	efi_status_t status;
+
+	if (!IS_ENABLED(CONFIG_EFI) ||
+	    !efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
+		return -EOPNOTSUPP;
+
+	status = efi.get_variable(name, &btintel_efi_sar_guid, NULL, &size, sar);
+	if (status == EFI_NOT_FOUND)
+		return -ENOENT;
+	if (status == EFI_BUFFER_TOO_SMALL)
+		return -EOPNOTSUPP;
+	if (status != EFI_SUCCESS) {
+		bt_dev_warn(hdev, "Failed to read UEFI SAR variable: 0x%lx",
+			    status);
+		return -EIO;
+	}
+
+	if (size != sizeof(*sar))
+		return -EINVAL;
+	if (sar->revision != BTINTEL_SAR_REV2)
+		return -EOPNOTSUPP;
+	if (!(le32_to_cpu(sar->mode) & BIT(0)))
+		return -EOPNOTSUPP;
+
+	return 0;
+}
+
+static int btintel_set_uefi_sar(struct hci_dev *hdev)
+{
+	/* 5G8 and 5G9 share a limit but have separate DDC identifiers. */
+	static const struct {
+		u16 id;
+		u8 band;
+	} bands[] = {
+		{ 0x0311, 0 },
+		{ 0x0312, 1 },
+		{ 0x0313, 2 },
+		{ 0x0314, 2 },
+		{ 0x0315, 3 },
+		{ 0x0316, 4 },
+	};
+	u8 buffer[sizeof(struct btintel_cp_ddc_write) + 2];
+	struct btintel_cp_ddc_write *cmd = (void *)buffer;
+	struct btintel_uefi_sar sar;
+	struct sk_buff *skb;
+	u8 enable = 1;
+	int ret, i;
+
+	ret = btintel_uefi_read_sar(hdev, BTINTEL_EFI_BRDS, &sar);
+	if (ret)
+		return ret;
+
+	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0311,
+					 sar.chain_a[0], sar.chain_b[0]);
+	if (ret)
+		return ret;
+
+	/* Standalone limits require both BRDS and BSSS to be enabled. */
+	ret = btintel_uefi_read_sar(hdev, BTINTEL_EFI_BSSS, &sar);
+	if (!ret) {
+		cmd->len = 3;
+		/* BSSS requires 0x019e = 1 regardless of UEFI inc_power_mode. */
+		cmd->id = cpu_to_le16(0x019e);
+		cmd->data[0] = enable;
+		ret = btintel_send_sar_ddc(hdev, cmd, sizeof(*cmd) + 1);
+		if (ret)
+			return ret;
+
+		/* Firmware interprets these DDC IDs as BSSS limits after enablement. */
+		for (i = 0; i < ARRAY_SIZE(bands); i++) {
+			ret = btintel_send_sar_rev2_band(hdev, cmd, bands[i].id,
+							 sar.chain_a[bands[i].band],
+							 sar.chain_b[bands[i].band]);
+			if (ret)
+				return ret;
+		}
+	} else if (ret != -ENOENT && ret != -EOPNOTSUPP) {
+		return ret;
+	}
+
+	skb = __hci_cmd_sync(hdev, 0xfe25, sizeof(enable), &enable,
+			     HCI_CMD_TIMEOUT);
+	if (IS_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)
 {
+	u8 hw_variant = INTEL_HW_VARIANT(ver->cnvi_bt);
+	int ret;
+
+	if (hw_variant >= BTINTEL_HWID_SCP2 &&
+	    hw_variant != BTINTEL_HWID_BZRI &&
+	    hw_variant != BTINTEL_HWID_BZRIW) {
+		ret = btintel_set_uefi_sar(hdev);
+		if (ret && ret != -ENOENT && ret != -EOPNOTSUPP)
+			bt_dev_warn(hdev, "Failed to configure UEFI SAR (%d)", ret);
+	}
+
+	/* UEFI SAR configuration does not replace ACPI BRDS handling. */
 #ifdef CONFIG_ACPI
 	return btintel_acpi_set_sar(hdev, ver);
 #endif
-- 
2.53.0

RE: [v1] Bluetooth: btintel: Add UEFI standalone static SAR support

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

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/series/1204807/

---Test result---

Test Summary:
CheckPatch                    PASS      1.49 seconds
VerifyFixes                   PASS      0.06 seconds
VerifySignedoff               PASS      0.27 seconds
GitLint                       PASS      2.34 seconds
SubjectPrefix                 PASS      2.35 seconds
BuildKernel                   PASS      31.80 seconds
CheckAllWarning               PASS      34.52 seconds
CheckSparse                   PASS      37.13 seconds
BuildKernel32                 PASS      30.19 seconds
CheckKernelLLVM               PASS      34.46 seconds
TestRunnerSetup               PASS      741.17 seconds
IncrementalBuild              PASS      32.02 seconds



https://github.com/bluez/bluetooth-next/pull/1085

---
Regards,
Linux Bluetooth

[PATCH v2] Bluetooth: btintel: Add UEFI standalone static SAR support

[Sai Teja Aluvala]

Scorpius Peak2 and later platforms, excluding BlazarI and BlazarIW,
provide standalone static SAR limits through the UefiCnvBtBSSS
variable. Applying these limits requires SAR to be enabled in
UefiCnvBtBRDS as well as in BSSS.

Read the revision 2 UEFI BRDS and BSSS variables using their shared
layout. Program the BRDS 2.4 GHz limits, then enable standalone SAR
and write the BSSS per-chain sub-band limits. Send SAR initialization
complete after the DDC writes.

Reuse the existing SAR DDC helpers and configure UEFI SAR before
running the existing ACPI BRDS path. Keep the ACPI platform and
revision checks unchanged, independently of the UEFI result.

Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
---
Changes in v2:
Address all six findings from the netdev Sashiko cross-review:
- Attempt SAR initialization complete after any UEFI DDC attempt,
  including error paths, while preserving the original error.
- Clarify the other five findings with firmware-confirmed comments on
  BRDS/BSSS DDC semantics, initial mode independence, ACPI coexistence
  and CNVr coverage.

 drivers/bluetooth/btintel.c | 173 ++++++++++++++++++++++++++++++++----
 1 file changed, 156 insertions(+), 17 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index a0aa110f4cdd..3612d8da81e5 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -29,6 +29,21 @@
 #define ECDSA_HEADER_LEN	320
 
 #define BTINTEL_EFI_DSBR	L"UefiCnvCommonDSBR"
+#define BTINTEL_EFI_BRDS	L"UefiCnvBtBRDS"
+#define BTINTEL_EFI_BSSS	L"UefiCnvBtBSSS"
+
+static efi_guid_t btintel_efi_sar_guid =
+	EFI_GUID(0x42780dd5, 0x9a7d, 0x404c,
+		 0x80, 0xe4, 0x7f, 0x70, 0x94, 0x36, 0x03, 0x94);
+
+/* BRDS and BSSS revision 2 use the same 19-byte UEFI layout. */
+struct btintel_uefi_sar {
+	u8 revision;
+	__le32 mode;
+	__le32 inc_power_mode;
+	u8 chain_a[5];
+	u8 chain_b[5];
+} __packed;
 
 enum {
 	DSM_SET_WDISABLE2_DELAY = 1,
@@ -3093,6 +3108,8 @@ static union acpi_object *btintel_acpi_get_bt_pkg(union acpi_object *buffer)
 	return ERR_PTR(-ENOENT);
 }
 
+#endif /* CONFIG_ACPI */
+
 static int btintel_send_sar_ddc(struct hci_dev *hdev, struct btintel_cp_ddc_write *data, u8 len)
 {
 	struct sk_buff *skb;
@@ -3107,6 +3124,24 @@ static int btintel_send_sar_ddc(struct hci_dev *hdev, struct btintel_cp_ddc_writ
 	return 0;
 }
 
+/* btintel_send_sar_rev2_band - send DDC command for one Rev2 sub-band
+ *
+ * Each DDC 0x0311-0x0316 carries 2 bytes: [ChainA_value, ChainB_value].
+ * cmd->len = 4  (2 id + 2 data)
+ * HCI total  = 5 bytes (1 len + 4)
+ */
+static int btintel_send_sar_rev2_band(struct hci_dev *hdev,
+				      struct btintel_cp_ddc_write *cmd,
+				      u16 id, u8 chain_a, u8 chain_b)
+{
+	cmd->len = 4;
+	cmd->id = cpu_to_le16(id);
+	cmd->data[0] = chain_a;
+	cmd->data[1] = chain_b;
+	return btintel_send_sar_ddc(hdev, cmd, 5);
+}
+
+#ifdef CONFIG_ACPI
 static int btintel_send_edr(struct hci_dev *hdev, struct btintel_cp_ddc_write *cmd,
 			    int id, struct btintel_sar_inc_pwr *sar)
 {
@@ -3260,23 +3295,6 @@ static int btintel_set_mutual_sar(struct hci_dev *hdev, struct btintel_sar_inc_p
 	return 0;
 }
 
-/* btintel_send_sar_rev2_band - send DDC command for one Rev2 sub-band
- *
- * Each DDC 0x0311-0x0316 carries 2 bytes: [ChainA_value, ChainB_value].
- * cmd->len = 4  (2 id + 2 data)
- * HCI total  = 5 bytes (1 len + 4)
- */
-static int btintel_send_sar_rev2_band(struct hci_dev *hdev,
-				      struct btintel_cp_ddc_write *cmd,
-				      u16 id, u8 chain_a, u8 chain_b)
-{
-	cmd->len = 4;
-	cmd->id = cpu_to_le16(id);
-	cmd->data[0] = chain_a;
-	cmd->data[1] = chain_b;
-	return btintel_send_sar_ddc(hdev, cmd, 5);
-}
-
 static int btintel_set_sar_rev2(struct hci_dev *hdev,
 				struct btintel_sar_rev2 *sar)
 {
@@ -3544,9 +3562,130 @@ static int btintel_acpi_set_sar(struct hci_dev *hdev, struct intel_version_tlv *
 }
 #endif /* CONFIG_ACPI */
 
+static int btintel_uefi_read_sar(struct hci_dev *hdev, efi_char16_t *name,
+				 struct btintel_uefi_sar *sar)
+{
+	unsigned long size = sizeof(*sar);
+	efi_status_t status;
+
+	if (!IS_ENABLED(CONFIG_EFI) ||
+	    !efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
+		return -EOPNOTSUPP;
+
+	status = efi.get_variable(name, &btintel_efi_sar_guid, NULL, &size, sar);
+	if (status == EFI_NOT_FOUND)
+		return -ENOENT;
+	if (status == EFI_BUFFER_TOO_SMALL)
+		return -EOPNOTSUPP;
+	if (status != EFI_SUCCESS) {
+		bt_dev_warn(hdev, "Failed to read UEFI SAR variable: 0x%lx",
+			    status);
+		return -EIO;
+	}
+
+	if (size != sizeof(*sar))
+		return -EINVAL;
+	if (sar->revision != BTINTEL_SAR_REV2)
+		return -EOPNOTSUPP;
+	if (!(le32_to_cpu(sar->mode) & BIT(0)))
+		return -EOPNOTSUPP;
+
+	return 0;
+}
+
+static int btintel_set_uefi_sar(struct hci_dev *hdev)
+{
+	/* 5G8 and 5G9 share a limit but have separate DDC identifiers. */
+	static const struct {
+		u16 id;
+		u8 band;
+	} bands[] = {
+		{ 0x0311, 0 },
+		{ 0x0312, 1 },
+		{ 0x0313, 2 },
+		{ 0x0314, 2 },
+		{ 0x0315, 3 },
+		{ 0x0316, 4 },
+	};
+	u8 buffer[sizeof(struct btintel_cp_ddc_write) + 2];
+	struct btintel_cp_ddc_write *cmd = (void *)buffer;
+	struct btintel_uefi_sar sar;
+	struct sk_buff *skb;
+	u8 enable = 1;
+	int ret, i;
+
+	ret = btintel_uefi_read_sar(hdev, BTINTEL_EFI_BRDS, &sar);
+	if (ret)
+		return ret;
+
+	/*
+	 * UEFI BRDS uses only the 2.4 GHz limits; BSSS supplies sub-band limits.
+	 * Firmware accepts this BRDS write regardless of the existing 0x019e
+	 * value, including a value previously set by the DDC configuration.
+	 */
+	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0311,
+					 sar.chain_a[0], sar.chain_b[0]);
+	if (ret)
+		goto complete;
+
+	/* Standalone limits require both BRDS and BSSS to be enabled. */
+	ret = btintel_uefi_read_sar(hdev, BTINTEL_EFI_BSSS, &sar);
+	if (!ret) {
+		cmd->len = 3;
+		/* UEFI BSSS requires 0x019e = 1; inc_power_mode is not used here. */
+		cmd->id = cpu_to_le16(0x019e);
+		cmd->data[0] = enable;
+		ret = btintel_send_sar_ddc(hdev, cmd, sizeof(*cmd) + 1);
+		if (ret)
+			goto complete;
+
+		/* In this UEFI sequence, firmware interprets these writes as BSSS limits. */
+		for (i = 0; i < ARRAY_SIZE(bands); i++) {
+			ret = btintel_send_sar_rev2_band(hdev, cmd, bands[i].id,
+							 sar.chain_a[bands[i].band],
+							 sar.chain_b[bands[i].band]);
+			if (ret)
+				goto complete;
+		}
+	} else if (ret != -ENOENT && ret != -EOPNOTSUPP) {
+		goto complete;
+	} else {
+		ret = 0;
+	}
+
+complete:
+	skb = __hci_cmd_sync(hdev, 0xfe25, sizeof(enable), &enable,
+			     HCI_CMD_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_warn(hdev, "Failed to complete UEFI SAR setup (%ld)",
+			    PTR_ERR(skb));
+		return ret ? ret : PTR_ERR(skb);
+	}
+
+	kfree_skb(skb);
+	return ret;
+}
+
 static int btintel_set_specific_absorption_rate(struct hci_dev *hdev,
 						struct intel_version_tlv *ver)
 {
+	u8 hw_variant = INTEL_HW_VARIANT(ver->cnvi_bt);
+	int ret;
+
+	/* Supported SCP2+ platforms use this UEFI DDC map regardless of CNVr. */
+	if (hw_variant >= BTINTEL_HWID_SCP2 &&
+	    hw_variant != BTINTEL_HWID_BZRI &&
+	    hw_variant != BTINTEL_HWID_BZRIW) {
+		ret = btintel_set_uefi_sar(hdev);
+		if (ret && ret != -ENOENT && ret != -EOPNOTSUPP)
+			bt_dev_warn(hdev, "Failed to configure UEFI SAR (%d)", ret);
+	}
+
+	/*
+	 * Firmware preserves BSSS configuration across subsequent ACPI BRDS
+	 * programming, including writes to the shared DDC IDs and 0x019e.
+	 * Keep ACPI BRDS handling independent of the UEFI result.
+	 */
 #ifdef CONFIG_ACPI
 	return btintel_acpi_set_sar(hdev, ver);
 #endif
-- 
2.53.0

RE: [v2] Bluetooth: btintel: Add UEFI standalone static SAR support

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

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/series/1205289/

---Test result---

Test Summary:
CheckPatch                    PASS      0.77 seconds
VerifyFixes                   PASS      0.13 seconds
VerifySignedoff               PASS      6.36 seconds
GitLint                       PASS      0.94 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      24.01 seconds
CheckAllWarning               PASS      27.90 seconds
CheckSparse                   PASS      29.71 seconds
BuildKernel32                 PASS      24.93 seconds
CheckKernelLLVM               PASS      25.29 seconds
TestRunnerSetup               PASS      566.79 seconds
IncrementalBuild              PASS      23.17 seconds



https://github.com/bluez/bluetooth-next/pull/1089

---
Regards,
Linux Bluetooth

Re: [PATCH v2] Bluetooth: btintel: Add UEFI standalone static SAR support

[Paul Menzel]

Dear Sai,


Thank you for patch.

Am 08.11.26 um 10:36 schrieb Sai Teja Aluvala:
> Scorpius Peak2 and later platforms, excluding BlazarI and BlazarIW,
> provide standalone static SAR limits through the UefiCnvBtBSSS
> variable. Applying these limits requires SAR to be enabled in
> UefiCnvBtBRDS as well as in BSSS.

For non-UEFI elaborating what BSSS is would be helpful.

> Read the revision 2 UEFI BRDS and BSSS variables using their shared
> layout. Program the BRDS 2.4 GHz limits, then enable standalone SAR
> and write the BSSS per-chain sub-band limits. Send SAR initialization
> complete after the DDC writes.
> 
> Reuse the existing SAR DDC helpers and configure UEFI SAR before
> running the existing ACPI BRDS path. Keep the ACPI platform and

Why before and not after?

> revision checks unchanged, independently of the UEFI result.

Please add how you tested this, including how to manually read the 
variables and possible new log message.

> Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
> ---
> Changes in v2:
> Address all six findings from the netdev Sashiko cross-review:
> - Attempt SAR initialization complete after any UEFI DDC attempt,
>    including error paths, while preserving the original error.
> - Clarify the other five findings with firmware-confirmed comments on
>    BRDS/BSSS DDC semantics, initial mode independence, ACPI coexistence
>    and CNVr coverage.
> 
>   drivers/bluetooth/btintel.c | 173 ++++++++++++++++++++++++++++++++----
>   1 file changed, 156 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index a0aa110f4cdd..3612d8da81e5 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -29,6 +29,21 @@
>   #define ECDSA_HEADER_LEN	320
>   
>   #define BTINTEL_EFI_DSBR	L"UefiCnvCommonDSBR"
> +#define BTINTEL_EFI_BRDS	L"UefiCnvBtBRDS"
> +#define BTINTEL_EFI_BSSS	L"UefiCnvBtBSSS"
> +
> +static efi_guid_t btintel_efi_sar_guid =
> +	EFI_GUID(0x42780dd5, 0x9a7d, 0x404c,
> +		 0x80, 0xe4, 0x7f, 0x70, 0x94, 0x36, 0x03, 0x94);
> +
> +/* BRDS and BSSS revision 2 use the same 19-byte UEFI layout. */
> +struct btintel_uefi_sar {
> +	u8 revision;
> +	__le32 mode;
> +	__le32 inc_power_mode;
> +	u8 chain_a[5];
> +	u8 chain_b[5];
> +} __packed;
>   
>   enum {
>   	DSM_SET_WDISABLE2_DELAY = 1,
> @@ -3093,6 +3108,8 @@ static union acpi_object *btintel_acpi_get_bt_pkg(union acpi_object *buffer)
>   	return ERR_PTR(-ENOENT);
>   }
>   
> +#endif /* CONFIG_ACPI */
> +
>   static int btintel_send_sar_ddc(struct hci_dev *hdev, struct btintel_cp_ddc_write *data, u8 len)
>   {
>   	struct sk_buff *skb;
> @@ -3107,6 +3124,24 @@ static int btintel_send_sar_ddc(struct hci_dev *hdev, struct btintel_cp_ddc_writ
>   	return 0;
>   }
>   
> +/* btintel_send_sar_rev2_band - send DDC command for one Rev2 sub-band

Follow comment style from Linux coding style?

/*
  * …

> + *
> + * Each DDC 0x0311-0x0316 carries 2 bytes: [ChainA_value, ChainB_value].
> + * cmd->len = 4  (2 id + 2 data)
> + * HCI total  = 5 bytes (1 len + 4)
> + */
> +static int btintel_send_sar_rev2_band(struct hci_dev *hdev,
> +				      struct btintel_cp_ddc_write *cmd,
> +				      u16 id, u8 chain_a, u8 chain_b)
> +{
> +	cmd->len = 4;
> +	cmd->id = cpu_to_le16(id);
> +	cmd->data[0] = chain_a;
> +	cmd->data[1] = chain_b;
> +	return btintel_send_sar_ddc(hdev, cmd, 5);
> +}
> +
> +#ifdef CONFIG_ACPI
>   static int btintel_send_edr(struct hci_dev *hdev, struct btintel_cp_ddc_write *cmd,
>   			    int id, struct btintel_sar_inc_pwr *sar)
>   {
> @@ -3260,23 +3295,6 @@ static int btintel_set_mutual_sar(struct hci_dev *hdev, struct btintel_sar_inc_p
>   	return 0;
>   }
>   
> -/* btintel_send_sar_rev2_band - send DDC command for one Rev2 sub-band
> - *
> - * Each DDC 0x0311-0x0316 carries 2 bytes: [ChainA_value, ChainB_value].
> - * cmd->len = 4  (2 id + 2 data)
> - * HCI total  = 5 bytes (1 len + 4)
> - */
> -static int btintel_send_sar_rev2_band(struct hci_dev *hdev,
> -				      struct btintel_cp_ddc_write *cmd,
> -				      u16 id, u8 chain_a, u8 chain_b)
> -{
> -	cmd->len = 4;
> -	cmd->id = cpu_to_le16(id);
> -	cmd->data[0] = chain_a;
> -	cmd->data[1] = chain_b;
> -	return btintel_send_sar_ddc(hdev, cmd, 5);
> -}
> -
>   static int btintel_set_sar_rev2(struct hci_dev *hdev,
>   				struct btintel_sar_rev2 *sar)
>   {
> @@ -3544,9 +3562,130 @@ static int btintel_acpi_set_sar(struct hci_dev *hdev, struct intel_version_tlv *
>   }
>   #endif /* CONFIG_ACPI */
>   
> +static int btintel_uefi_read_sar(struct hci_dev *hdev, efi_char16_t *name,
> +				 struct btintel_uefi_sar *sar)
> +{
> +	unsigned long size = sizeof(*sar);
> +	efi_status_t status;
> +
> +	if (!IS_ENABLED(CONFIG_EFI) ||
> +	    !efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
> +		return -EOPNOTSUPP;
> +
> +	status = efi.get_variable(name, &btintel_efi_sar_guid, NULL, &size, sar);
> +	if (status == EFI_NOT_FOUND)
> +		return -ENOENT;
> +	if (status == EFI_BUFFER_TOO_SMALL)
> +		return -EOPNOTSUPP;
> +	if (status != EFI_SUCCESS) {
> +		bt_dev_warn(hdev, "Failed to read UEFI SAR variable: 0x%lx",

Mention what the user should do in this case?

> +			    status);
> +		return -EIO;
> +	}
> +
> +	if (size != sizeof(*sar))
> +		return -EINVAL;
> +	if (sar->revision != BTINTEL_SAR_REV2)
> +		return -EOPNOTSUPP;
> +	if (!(le32_to_cpu(sar->mode) & BIT(0)))
> +		return -EOPNOTSUPP;

Log at least debug messages for al the above error cases?

> +
> +	return 0;
> +}
> +
> +static int btintel_set_uefi_sar(struct hci_dev *hdev)
> +{
> +	/* 5G8 and 5G9 share a limit but have separate DDC identifiers. */
> +	static const struct {
> +		u16 id;
> +		u8 band;
> +	} bands[] = {
> +		{ 0x0311, 0 },
> +		{ 0x0312, 1 },
> +		{ 0x0313, 2 },
> +		{ 0x0314, 2 },
> +		{ 0x0315, 3 },
> +		{ 0x0316, 4 },
> +	};
> +	u8 buffer[sizeof(struct btintel_cp_ddc_write) + 2];
> +	struct btintel_cp_ddc_write *cmd = (void *)buffer;
> +	struct btintel_uefi_sar sar;
> +	struct sk_buff *skb;
> +	u8 enable = 1;
> +	int ret, i;
> +
> +	ret = btintel_uefi_read_sar(hdev, BTINTEL_EFI_BRDS, &sar);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * UEFI BRDS uses only the 2.4 GHz limits; BSSS supplies sub-band limits.
> +	 * Firmware accepts this BRDS write regardless of the existing 0x019e
> +	 * value, including a value previously set by the DDC configuration.
> +	 */
> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0311,
> +					 sar.chain_a[0], sar.chain_b[0]);
> +	if (ret)
> +		goto complete;
> +
> +	/* Standalone limits require both BRDS and BSSS to be enabled. */
> +	ret = btintel_uefi_read_sar(hdev, BTINTEL_EFI_BSSS, &sar);
> +	if (!ret) {
> +		cmd->len = 3;
> +		/* UEFI BSSS requires 0x019e = 1; inc_power_mode is not used here. */
> +		cmd->id = cpu_to_le16(0x019e);
> +		cmd->data[0] = enable;
> +		ret = btintel_send_sar_ddc(hdev, cmd, sizeof(*cmd) + 1);
> +		if (ret)
> +			goto complete;
> +
> +		/* In this UEFI sequence, firmware interprets these writes as BSSS limits. */

What is an “UEFI sequence”?

> +		for (i = 0; i < ARRAY_SIZE(bands); i++) {
> +			ret = btintel_send_sar_rev2_band(hdev, cmd, bands[i].id,
> +							 sar.chain_a[bands[i].band],
> +							 sar.chain_b[bands[i].band]);
> +			if (ret)
> +				goto complete;
> +		}
> +	} else if (ret != -ENOENT && ret != -EOPNOTSUPP) {
> +		goto complete;
> +	} else {
> +		ret = 0;
> +	}
> +
> +complete:
> +	skb = __hci_cmd_sync(hdev, 0xfe25, sizeof(enable), &enable,
> +			     HCI_CMD_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		bt_dev_warn(hdev, "Failed to complete UEFI SAR setup (%ld)",
> +			    PTR_ERR(skb));
> +		return ret ? ret : PTR_ERR(skb);

Should `ret` be added to the warning, should it exist?

> +	}
> +
> +	kfree_skb(skb);
> +	return ret;
> +}
> +
>   static int btintel_set_specific_absorption_rate(struct hci_dev *hdev,
>   						struct intel_version_tlv *ver)
>   {
> +	u8 hw_variant = INTEL_HW_VARIANT(ver->cnvi_bt);
> +	int ret;
> +
> +	/* Supported SCP2+ platforms use this UEFI DDC map regardless of CNVr. */
> +	if (hw_variant >= BTINTEL_HWID_SCP2 &&
> +	    hw_variant != BTINTEL_HWID_BZRI &&
> +	    hw_variant != BTINTEL_HWID_BZRIW) {
> +		ret = btintel_set_uefi_sar(hdev);
> +		if (ret && ret != -ENOENT && ret != -EOPNOTSUPP)
> +			bt_dev_warn(hdev, "Failed to configure UEFI SAR (%d)", ret);

Two warnings are logged now?

> +	}
> +
> +	/*
> +	 * Firmware preserves BSSS configuration across subsequent ACPI BRDS
> +	 * programming, including writes to the shared DDC IDs and 0x019e.
> +	 * Keep ACPI BRDS handling independent of the UEFI result.
> +	 */
>   #ifdef CONFIG_ACPI
>   	return btintel_acpi_set_sar(hdev, ver);
>   #endif

Is it possible that SAR programming is done twice now?


Kind regards,

Paul

RE: [PATCH v2] Bluetooth: btintel: Add UEFI standalone static SAR support

[Aluvala Sai Teja]

Hi Paul,

Thanks for the review. Please see my replies inline.


-----Original Message-----
From: Paul Menzel <pmenzel@molgen.mpg.de> 
Sent: 09 November 2026 02:03 AM
To: Aluvala Sai Teja <aluvala.sai.teja@intel.com>
Cc: K, Kiran <kiran.k@intel.com>; linux-bluetooth@vger.kernel.org; Tumkur Narayan, Chethan <chethan.tumkur.narayan@intel.com>
Subject: Re: [PATCH v2] Bluetooth: btintel: Add UEFI standalone static SAR support

Dear Sai,


Thank you for patch.

Am 08.11.26 um 10:36 schrieb Sai Teja Aluvala:
> Scorpius Peak2 and later platforms, excluding BlazarI and BlazarIW, 
> provide standalone static SAR limits through the UefiCnvBtBSSS 
> variable. Applying these limits requires SAR to be enabled in 
> UefiCnvBtBRDS as well as in BSSS.

For non-UEFI elaborating what BSSS is would be helpful.
[ Sai ]: BSSS provides Bluetooth standalone static SAR sub-band limits.
It is available through the UEFI BSSS variable; there is no corresponding
ACPI BSSS configuration. I will clarify this in the commit description
of next patch

> Read the revision 2 UEFI BRDS and BSSS variables using their shared 
> layout. Program the BRDS 2.4 GHz limits, then enable standalone SAR 
> and write the BSSS per-chain sub-band limits. Send SAR initialization 
> complete after the DDC writes.
> 
> Reuse the existing SAR DDC helpers and configure UEFI SAR before 
> running the existing ACPI BRDS path. Keep the ACPI platform and

Why before and not after?
[Sai]: Platforms that provide UEFI BSSS use the UEFI BRDS/BSSS path and
do not have an ACPI BRDS configuration, so there is no later ACPI BRDS
write on those platforms.

On coreboot systems, neither ACPI BSSS nor UEFI BSSS is available, but
UEFI BRDS may be available. In that case, the UEFI path writes the BRDS
2.4 GHz limit to DDC 0x0311 and skips BSSS programming; the existing
ACPI BRDS path then runs and may write 0x0311 again using its own data.
The later ACPI write determines the final BRDS value. The UEFI path is
kept before ACPI so the existing ACPI BRDS handling and precedence
remain unchanged on these systems.


> revision checks unchanged, independently of the UEFI result.

Please add how you tested this, including how to manually read the variables and possible new log message.
[Sai]: I will add the testing details, including how to read the UEFI
variables and the relevant log messages, in the next patchset.


> Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
> ---
> Changes in v2:
> Address all six findings from the netdev Sashiko cross-review:
> - Attempt SAR initialization complete after any UEFI DDC attempt,
>    including error paths, while preserving the original error.
> - Clarify the other five findings with firmware-confirmed comments on
>    BRDS/BSSS DDC semantics, initial mode independence, ACPI coexistence
>    and CNVr coverage.
> 
>   drivers/bluetooth/btintel.c | 173 ++++++++++++++++++++++++++++++++----
>   1 file changed, 156 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c 
> index a0aa110f4cdd..3612d8da81e5 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -29,6 +29,21 @@
>   #define ECDSA_HEADER_LEN	320
>   
>   #define BTINTEL_EFI_DSBR	L"UefiCnvCommonDSBR"
> +#define BTINTEL_EFI_BRDS	L"UefiCnvBtBRDS"
> +#define BTINTEL_EFI_BSSS	L"UefiCnvBtBSSS"
> +
> +static efi_guid_t btintel_efi_sar_guid =
> +	EFI_GUID(0x42780dd5, 0x9a7d, 0x404c,
> +		 0x80, 0xe4, 0x7f, 0x70, 0x94, 0x36, 0x03, 0x94);
> +
> +/* BRDS and BSSS revision 2 use the same 19-byte UEFI layout. */ 
> +struct btintel_uefi_sar {
> +	u8 revision;
> +	__le32 mode;
> +	__le32 inc_power_mode;
> +	u8 chain_a[5];
> +	u8 chain_b[5];
> +} __packed;
>   
>   enum {
>   	DSM_SET_WDISABLE2_DELAY = 1,
> @@ -3093,6 +3108,8 @@ static union acpi_object *btintel_acpi_get_bt_pkg(union acpi_object *buffer)
>   	return ERR_PTR(-ENOENT);
>   }
>   
> +#endif /* CONFIG_ACPI */
> +
>   static int btintel_send_sar_ddc(struct hci_dev *hdev, struct btintel_cp_ddc_write *data, u8 len)
>   {
>   	struct sk_buff *skb;
> @@ -3107,6 +3124,24 @@ static int btintel_send_sar_ddc(struct hci_dev *hdev, struct btintel_cp_ddc_writ
>   	return 0;
>   }
>   
> +/* btintel_send_sar_rev2_band - send DDC command for one Rev2 
> +sub-band

Follow comment style from Linux coding style?
[Sai]: Ack. I will update the comment style in the next patch.

/*
  * …

> + *
> + * Each DDC 0x0311-0x0316 carries 2 bytes: [ChainA_value, ChainB_value].
> + * cmd->len = 4  (2 id + 2 data)
> + * HCI total  = 5 bytes (1 len + 4)
> + */
> +static int btintel_send_sar_rev2_band(struct hci_dev *hdev,
> +				      struct btintel_cp_ddc_write *cmd,
> +				      u16 id, u8 chain_a, u8 chain_b) {
> +	cmd->len = 4;
> +	cmd->id = cpu_to_le16(id);
> +	cmd->data[0] = chain_a;
> +	cmd->data[1] = chain_b;
> +	return btintel_send_sar_ddc(hdev, cmd, 5); }
> +
> +#ifdef CONFIG_ACPI
>   static int btintel_send_edr(struct hci_dev *hdev, struct btintel_cp_ddc_write *cmd,
>   			    int id, struct btintel_sar_inc_pwr *sar)
>   {
> @@ -3260,23 +3295,6 @@ static int btintel_set_mutual_sar(struct hci_dev *hdev, struct btintel_sar_inc_p
>   	return 0;
>   }
>   
> -/* btintel_send_sar_rev2_band - send DDC command for one Rev2 
> sub-band
> - *
> - * Each DDC 0x0311-0x0316 carries 2 bytes: [ChainA_value, ChainB_value].
> - * cmd->len = 4  (2 id + 2 data)
> - * HCI total  = 5 bytes (1 len + 4)
> - */
> -static int btintel_send_sar_rev2_band(struct hci_dev *hdev,
> -				      struct btintel_cp_ddc_write *cmd,
> -				      u16 id, u8 chain_a, u8 chain_b)
> -{
> -	cmd->len = 4;
> -	cmd->id = cpu_to_le16(id);
> -	cmd->data[0] = chain_a;
> -	cmd->data[1] = chain_b;
> -	return btintel_send_sar_ddc(hdev, cmd, 5);
> -}
> -
>   static int btintel_set_sar_rev2(struct hci_dev *hdev,
>   				struct btintel_sar_rev2 *sar)
>   {
> @@ -3544,9 +3562,130 @@ static int btintel_acpi_set_sar(struct hci_dev *hdev, struct intel_version_tlv *
>   }
>   #endif /* CONFIG_ACPI */
>   
> +static int btintel_uefi_read_sar(struct hci_dev *hdev, efi_char16_t *name,
> +				 struct btintel_uefi_sar *sar)
> +{
> +	unsigned long size = sizeof(*sar);
> +	efi_status_t status;
> +
> +	if (!IS_ENABLED(CONFIG_EFI) ||
> +	    !efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
> +		return -EOPNOTSUPP;
> +
> +	status = efi.get_variable(name, &btintel_efi_sar_guid, NULL, &size, sar);
> +	if (status == EFI_NOT_FOUND)
> +		return -ENOENT;
> +	if (status == EFI_BUFFER_TOO_SMALL)
> +		return -EOPNOTSUPP;
> +	if (status != EFI_SUCCESS) {
> +		bt_dev_warn(hdev, "Failed to read UEFI SAR variable: 0x%lx",

Mention what the user should do in this case?
[Sai]: If the UEFI BRDS variable is unavailable or unsupported, the UEFI
path skips its BRDS write, and the existing ACPI handling can continue.

If BRDS is available but BSSS is missing or unsupported, the UEFI BRDS
write has already been sent, so the BSSS enable and band writes are
skipped and 0xfe25 is still attempted. Unexpected variable-read errors
are reported in the kernel log.

> +			    status);
> +		return -EIO;
> +	}
> +
> +	if (size != sizeof(*sar))
> +		return -EINVAL;
> +	if (sar->revision != BTINTEL_SAR_REV2)
> +		return -EOPNOTSUPP;
> +	if (!(le32_to_cpu(sar->mode) & BIT(0)))
> +		return -EOPNOTSUPP;

Log at least debug messages for al the above error cases?
[Sai]: I will add debug messages for expected non-fatal cases, including
missing variables, unsupported revisions, disabled SAR, and invalid
variable sizes in next patch. Unexpected EFI read errors will remain
warnings.

> +
> +	return 0;
> +}
> +
> +static int btintel_set_uefi_sar(struct hci_dev *hdev) {
> +	/* 5G8 and 5G9 share a limit but have separate DDC identifiers. */
> +	static const struct {
> +		u16 id;
> +		u8 band;
> +	} bands[] = {
> +		{ 0x0311, 0 },
> +		{ 0x0312, 1 },
> +		{ 0x0313, 2 },
> +		{ 0x0314, 2 },
> +		{ 0x0315, 3 },
> +		{ 0x0316, 4 },
> +	};
> +	u8 buffer[sizeof(struct btintel_cp_ddc_write) + 2];
> +	struct btintel_cp_ddc_write *cmd = (void *)buffer;
> +	struct btintel_uefi_sar sar;
> +	struct sk_buff *skb;
> +	u8 enable = 1;
> +	int ret, i;
> +
> +	ret = btintel_uefi_read_sar(hdev, BTINTEL_EFI_BRDS, &sar);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * UEFI BRDS uses only the 2.4 GHz limits; BSSS supplies sub-band limits.
> +	 * Firmware accepts this BRDS write regardless of the existing 0x019e
> +	 * value, including a value previously set by the DDC configuration.
> +	 */
> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0311,
> +					 sar.chain_a[0], sar.chain_b[0]);
> +	if (ret)
> +		goto complete;
> +
> +	/* Standalone limits require both BRDS and BSSS to be enabled. */
> +	ret = btintel_uefi_read_sar(hdev, BTINTEL_EFI_BSSS, &sar);
> +	if (!ret) {
> +		cmd->len = 3;
> +		/* UEFI BSSS requires 0x019e = 1; inc_power_mode is not used here. */
> +		cmd->id = cpu_to_le16(0x019e);
> +		cmd->data[0] = enable;
> +		ret = btintel_send_sar_ddc(hdev, cmd, sizeof(*cmd) + 1);
> +		if (ret)
> +			goto complete;
> +
> +		/* In this UEFI sequence, firmware interprets these writes as BSSS 
> +limits. */

What is an “UEFI sequence”?
[Sai]: I meant the DDC commands after BSSS is enabled: write 0x019e = 1, then
write limits to 0x0311-0x0316. I will clarify the code comment:

  /*
   * After writing 0x019e = 1, firmware interprets DDC 0x0311-0x0316
   * as BSSS limits.
   */


> +		for (i = 0; i < ARRAY_SIZE(bands); i++) {
> +			ret = btintel_send_sar_rev2_band(hdev, cmd, bands[i].id,
> +							 sar.chain_a[bands[i].band],
> +							 sar.chain_b[bands[i].band]);
> +			if (ret)
> +				goto complete;
> +		}
> +	} else if (ret != -ENOENT && ret != -EOPNOTSUPP) {
> +		goto complete;
> +	} else {
> +		ret = 0;
> +	}
> +
> +complete:
> +	skb = __hci_cmd_sync(hdev, 0xfe25, sizeof(enable), &enable,
> +			     HCI_CMD_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		bt_dev_warn(hdev, "Failed to complete UEFI SAR setup (%ld)",
> +			    PTR_ERR(skb));
> +		return ret ? ret : PTR_ERR(skb);

Should `ret` be added to the warning, should it exist?
[Sai]: Yes. If both SAR programming and the 0xfe25 completion command fail,
the warning should report both errors. The function will continue to
return the original programming/read error when present; otherwise it
will return the completion error.

> +	}
> +
> +	kfree_skb(skb);
> +	return ret;
> +}
> +
>   static int btintel_set_specific_absorption_rate(struct hci_dev *hdev,
>   						struct intel_version_tlv *ver)
>   {
> +	u8 hw_variant = INTEL_HW_VARIANT(ver->cnvi_bt);
> +	int ret;
> +
> +	/* Supported SCP2+ platforms use this UEFI DDC map regardless of CNVr. */
> +	if (hw_variant >= BTINTEL_HWID_SCP2 &&
> +	    hw_variant != BTINTEL_HWID_BZRI &&
> +	    hw_variant != BTINTEL_HWID_BZRIW) {
> +		ret = btintel_set_uefi_sar(hdev);
> +		if (ret && ret != -ENOENT && ret != -EOPNOTSUPP)
> +			bt_dev_warn(hdev, "Failed to configure UEFI SAR (%d)", ret);

Two warnings are logged now?
[Sai]: Yes. I will remove the redundant caller warning while ensuring
failures that otherwise have no diagnostic are still reported. The existing DDC helper
warning will remain.

> +	}
> +
> +	/*
> +	 * Firmware preserves BSSS configuration across subsequent ACPI BRDS
> +	 * programming, including writes to the shared DDC IDs and 0x019e.
> +	 * Keep ACPI BRDS handling independent of the UEFI result.
> +	 */
>   #ifdef CONFIG_ACPI
>   	return btintel_acpi_set_sar(hdev, ver);
>   #endif

Is it possible that SAR programming is done twice now?
[Sai]: Yes. On coreboot systems where UEFI BRDS is available but UEFI BSSS is
not, the UEFI path writes the BRDS 2.4 GHz limit to DDC 0x0311, skips
BSSS programming, and then the existing ACPI BRDS path runs. ACPI may
write DDC 0x0311 again using its own data, so BRDS programming can
occur through both paths; the later ACPI write determines the final
0x0311 value. No BSSS limits are configured in this case.

Kind regards,

Paul