[PATCH v1] bluetooth: btintel: Add Bluetooth SAR revision 2 support

[PATCH v1] bluetooth: btintel: Add Bluetooth SAR revision 2 support

[Kiran K]

BRDS revision 2 introduces per-chain (Chain A and Chain B) TX power
limits across five sub-bands (2.4G, 5.2G, 5.8/5.9G, 6G-low, 6G-high),
replacing the single-chain per-modulation model of revisions 0 and 1.

- Add btintel_set_sar_rev2() which sends the full Rev2 DDC sequence:
    0x019e  inc-power-mode enable flag        (1 byte)
    0x0311  2.4 GHz sub-band limits           (2 bytes)
    0x0312  5.2 GHz sub-band limits           (2 bytes)
    0x0313  5.8/5.9 GHz sub-band limits       (2 bytes)
    0x0314  5.8/5.9 GHz sub-band limits again (2 bytes, duplicate FW reg)
    0x0315  6 GHz low sub-band limits         (2 bytes)
    0x0316  6 GHz high sub-band limits        (2 bytes)
  followed by the SAR-init-complete command (0xfe25).

Signed-off-by: Ravindra <ravindra@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel.c | 154 +++++++++++++++++++++++++++++++++++-
 drivers/bluetooth/btintel.h |  18 +++++
 2 files changed, 171 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 5e9cac090bd8..97d897623c94 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -51,6 +51,7 @@ enum {
 #define BTINTEL_BT_DOMAIN		0x12
 #define BTINTEL_SAR_LEGACY		0
 #define BTINTEL_SAR_INC_PWR		1
+#define BTINTEL_SAR_REV2		2
 #define BTINTEL_SAR_INC_PWR_SUPPORTED	0
 
 #define CMD_WRITE_BOOT_PARAMS	0xfc0e
@@ -3104,6 +3105,111 @@ 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)
+{
+	struct btintel_cp_ddc_write *cmd;
+	struct sk_buff *skb;
+	u8 buffer[64];
+	bool enable;
+	int ret;
+
+	cmd = (void *)buffer;
+
+	/* DDC 0x019e: enable/disable increased power mode SAR (1 byte) */
+	cmd->len = 3;
+	cmd->id = cpu_to_le16(0x019e);
+	cmd->data[0] = (sar->inc_power_mode == BTINTEL_SAR_INC_PWR_SUPPORTED) ?
+			0x01 : 0x00;
+	ret = btintel_send_sar_ddc(hdev, cmd, 4);
+	if (ret)
+		return ret;
+
+	/* DDC 0x0311-0x0316: per sub-band ChainA + ChainB limits */
+	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0311,
+					 sar->chain_a.subband_2g4,
+					 sar->chain_b.subband_2g4);
+	if (ret)
+		return ret;
+
+	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0312,
+					 sar->chain_a.subband_5g2,
+					 sar->chain_b.subband_5g2);
+	if (ret)
+		return ret;
+
+	/* 0x0313 and 0x0314 both carry the 5G8/5G9 value */
+	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0313,
+					 sar->chain_a.subband_5g8_5g9,
+					 sar->chain_b.subband_5g8_5g9);
+	if (ret)
+		return ret;
+
+	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0314,
+					 sar->chain_a.subband_5g8_5g9,
+					 sar->chain_b.subband_5g8_5g9);
+	if (ret)
+		return ret;
+
+	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0315,
+					 sar->chain_a.subband_6g1,
+					 sar->chain_b.subband_6g1);
+	if (ret)
+		return ret;
+
+	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0316,
+					 sar->chain_a.subband_6g3,
+					 sar->chain_b.subband_6g3);
+	if (ret)
+		return ret;
+
+	/* Notify firmware that SAR initialisation is complete */
+	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 Rev2 Enable (%ld)",
+			    PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+
+	kfree_skb(skb);
+	return 0;
+}
+
+static int btintel_sar_rev2_send_to_device(struct hci_dev *hdev,
+					   struct btintel_sar_rev2 *sar,
+					   struct intel_version_tlv *ver)
+{
+	u16 cnvi = ver->cnvi_top & 0xfff;
+	u16 cnvr = ver->cnvr_top & 0xfff;
+
+	if (cnvi < BTINTEL_CNVI_BLAZARI || cnvr != BTINTEL_CNVR_WHP2) {
+		bt_dev_dbg(hdev, "BT SAR Rev2 not supported on this platform (cnvi=0x%x cnvr=0x%x)",
+			   cnvi, cnvr);
+		return -EOPNOTSUPP;
+	}
+
+	bt_dev_info(hdev, "Applying Bluetooth SAR Rev2");
+	return btintel_set_sar_rev2(hdev, sar);
+}
+
 static int btintel_sar_send_to_device(struct hci_dev *hdev, struct btintel_sar_inc_pwr *sar,
 				      struct intel_version_tlv *ver)
 {
@@ -3130,6 +3236,7 @@ static int btintel_acpi_set_sar(struct hci_dev *hdev, struct intel_version_tlv *
 {
 	union acpi_object *bt_pkg, *buffer = NULL;
 	struct btintel_sar_inc_pwr sar;
+	struct btintel_sar_rev2 sar_rev2;
 	acpi_status status;
 	u8 revision;
 	int ret;
@@ -3150,14 +3257,59 @@ static int btintel_acpi_set_sar(struct hci_dev *hdev, struct intel_version_tlv *
 		goto error;
 	}
 
+	if (buffer->package.elements[0].type != ACPI_TYPE_INTEGER) {
+		bt_dev_warn(hdev, "BT_SAR: unexpected ACPI type for revision field");
+		ret = -EINVAL;
+		goto error;
+	}
+
 	revision = buffer->package.elements[0].integer.value;
 
-	if (revision > BTINTEL_SAR_INC_PWR) {
+	if (revision > BTINTEL_SAR_REV2) {
 		bt_dev_dbg(hdev, "BT_SAR: revision: 0x%2.2x not supported", revision);
 		ret = -EOPNOTSUPP;
 		goto error;
 	}
 
+	if (revision == BTINTEL_SAR_REV2 && bt_pkg->package.count == 13) {
+		memset(&sar_rev2, 0, sizeof(sar_rev2));
+		sar_rev2.revision       = revision;
+		sar_rev2.bt_sar_bios    = bt_pkg->package.elements[1].integer.value;
+		sar_rev2.inc_power_mode = bt_pkg->package.elements[2].integer.value;
+
+		sar_rev2.chain_a.subband_2g4     = bt_pkg->package.elements[3].integer.value;
+		sar_rev2.chain_a.subband_5g2     = bt_pkg->package.elements[4].integer.value;
+		sar_rev2.chain_a.subband_5g8_5g9 = bt_pkg->package.elements[5].integer.value;
+		sar_rev2.chain_a.subband_6g1     = bt_pkg->package.elements[6].integer.value;
+		sar_rev2.chain_a.subband_6g3     = bt_pkg->package.elements[7].integer.value;
+
+		sar_rev2.chain_b.subband_2g4     = bt_pkg->package.elements[8].integer.value;
+		sar_rev2.chain_b.subband_5g2     = bt_pkg->package.elements[9].integer.value;
+		sar_rev2.chain_b.subband_5g8_5g9 = bt_pkg->package.elements[10].integer.value;
+		sar_rev2.chain_b.subband_6g1     = bt_pkg->package.elements[11].integer.value;
+		sar_rev2.chain_b.subband_6g3     = bt_pkg->package.elements[12].integer.value;
+
+		bt_dev_dbg(hdev, "BT SAR Rev2: revision=%u bt_sar_bios=%u inc_power_mode=%u",
+			    sar_rev2.revision, sar_rev2.bt_sar_bios, sar_rev2.inc_power_mode);
+		bt_dev_dbg(hdev, "BT SAR Rev2 Chain A: 2g4=%u 5g2=%u 5g8_5g9=%u 6g1=%u 6g3=%u",
+			    sar_rev2.chain_a.subband_2g4, sar_rev2.chain_a.subband_5g2,
+			    sar_rev2.chain_a.subband_5g8_5g9, sar_rev2.chain_a.subband_6g1,
+			    sar_rev2.chain_a.subband_6g3);
+		bt_dev_dbg(hdev, "BT SAR Rev2 Chain B: 2g4=%u 5g2=%u 5g8_5g9=%u 6g1=%u 6g3=%u",
+			    sar_rev2.chain_b.subband_2g4, sar_rev2.chain_b.subband_5g2,
+			    sar_rev2.chain_b.subband_5g8_5g9, sar_rev2.chain_b.subband_6g1,
+			    sar_rev2.chain_b.subband_6g3);
+
+		if (sar_rev2.bt_sar_bios != 1) {
+			bt_dev_info(hdev, "Bluetooth SAR Rev2 is not enabled");
+			ret = -EOPNOTSUPP;
+			goto error;
+		}
+
+		ret = btintel_sar_rev2_send_to_device(hdev, &sar_rev2, ver);
+		goto error;
+	}
+
 	memset(&sar, 0, sizeof(sar));
 
 	if (revision == BTINTEL_SAR_LEGACY && bt_pkg->package.count == 8) {
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 70d812ad36a2..3b5a228ca3c0 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -64,6 +64,7 @@ struct intel_tlv {
 
 /* CNVR */
 #define BTINTEL_CNVR_FMP2		0x910
+#define BTINTEL_CNVR_WHP2		0xA10	/* Whale Peak2 - Panther Lake */
 
 #define BTINTEL_IMG_BOOTLOADER		0x01	/* Bootloader image */
 #define BTINTEL_IMG_IML			0x02	/* Intermediate image */
@@ -202,6 +203,23 @@ struct btintel_sar_inc_pwr {
 	u8	le_lr;
 };
 
+/* Bluetooth SAR feature (BRDS), Revision 2 - per-chain sub-band power limits */
+struct btintel_sar_band_limits {
+	u8	subband_2g4;
+	u8	subband_5g2;
+	u8	subband_5g8_5g9;
+	u8	subband_6g1;
+	u8	subband_6g3;
+};
+
+struct btintel_sar_rev2 {
+	u8	revision;
+	u32	bt_sar_bios;    /* 1: BIOS-managed SAR enabled */
+	u32	inc_power_mode; /* 0: supported, 1: disabled */
+	struct btintel_sar_band_limits chain_a;
+	struct btintel_sar_band_limits chain_b;
+};
+
 #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.53.0

Re: [PATCH v1] bluetooth: btintel: Add Bluetooth SAR revision 2 support

[Paul Menzel]

Dear Kiran,


Thank you for the patch.

Am 30.04.26 um 18:51 schrieb Kiran K:
> BRDS revision 2 introduces per-chain (Chain A and Chain B) TX power
> limits across five sub-bands (2.4G, 5.2G, 5.8/5.9G, 6G-low, 6G-high),
> replacing the single-chain per-modulation model of revisions 0 and 1.
> 
> - Add btintel_set_sar_rev2() which sends the full Rev2 DDC sequence:
>      0x019e  inc-power-mode enable flag        (1 byte)
>      0x0311  2.4 GHz sub-band limits           (2 bytes)
>      0x0312  5.2 GHz sub-band limits           (2 bytes)
>      0x0313  5.8/5.9 GHz sub-band limits       (2 bytes)
>      0x0314  5.8/5.9 GHz sub-band limits again (2 bytes, duplicate FW reg)
>      0x0315  6 GHz low sub-band limits         (2 bytes)
>      0x0316  6 GHz high sub-band limits        (2 bytes)
>    followed by the SAR-init-complete command (0xfe25).

Please detail how to test it, and please also paste the new debug messages.

> Signed-off-by: Ravindra <ravindra@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>   drivers/bluetooth/btintel.c | 154 +++++++++++++++++++++++++++++++++++-
>   drivers/bluetooth/btintel.h |  18 +++++
>   2 files changed, 171 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 5e9cac090bd8..97d897623c94 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -51,6 +51,7 @@ enum {
>   #define BTINTEL_BT_DOMAIN		0x12
>   #define BTINTEL_SAR_LEGACY		0
>   #define BTINTEL_SAR_INC_PWR		1
> +#define BTINTEL_SAR_REV2		2
>   #define BTINTEL_SAR_INC_PWR_SUPPORTED	0
>   
>   #define CMD_WRITE_BOOT_PARAMS	0xfc0e
> @@ -3104,6 +3105,111 @@ 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)
> +{
> +	struct btintel_cp_ddc_write *cmd;
> +	struct sk_buff *skb;
> +	u8 buffer[64];
> +	bool enable;
> +	int ret;
> +
> +	cmd = (void *)buffer;
> +
> +	/* DDC 0x019e: enable/disable increased power mode SAR (1 byte) */
> +	cmd->len = 3;
> +	cmd->id = cpu_to_le16(0x019e);
> +	cmd->data[0] = (sar->inc_power_mode == BTINTEL_SAR_INC_PWR_SUPPORTED) ?
> +			0x01 : 0x00;
> +	ret = btintel_send_sar_ddc(hdev, cmd, 4);
> +	if (ret)
> +		return ret;
> +
> +	/* DDC 0x0311-0x0316: per sub-band ChainA + ChainB limits */
> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0311,
> +					 sar->chain_a.subband_2g4,
> +					 sar->chain_b.subband_2g4);
> +	if (ret)
> +		return ret;
> +
> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0312,
> +					 sar->chain_a.subband_5g2,
> +					 sar->chain_b.subband_5g2);
> +	if (ret)
> +		return ret;
> +
> +	/* 0x0313 and 0x0314 both carry the 5G8/5G9 value */
> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0313,
> +					 sar->chain_a.subband_5g8_5g9,
> +					 sar->chain_b.subband_5g8_5g9);
> +	if (ret)
> +		return ret;
> +
> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0314,
> +					 sar->chain_a.subband_5g8_5g9,
> +					 sar->chain_b.subband_5g8_5g9);
> +	if (ret)
> +		return ret;
> +
> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0315,
> +					 sar->chain_a.subband_6g1,
> +					 sar->chain_b.subband_6g1);
> +	if (ret)
> +		return ret;
> +
> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0316,
> +					 sar->chain_a.subband_6g3,
> +					 sar->chain_b.subband_6g3);
> +	if (ret)
> +		return ret;
> +
> +	/* Notify firmware that SAR initialisation is complete */
> +	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 Rev2 Enable (%ld)",
> +			    PTR_ERR(skb));
> +		return PTR_ERR(skb);
> +	}
> +
> +	kfree_skb(skb);
> +	return 0;
> +}
> +
> +static int btintel_sar_rev2_send_to_device(struct hci_dev *hdev,
> +					   struct btintel_sar_rev2 *sar,
> +					   struct intel_version_tlv *ver)
> +{
> +	u16 cnvi = ver->cnvi_top & 0xfff;
> +	u16 cnvr = ver->cnvr_top & 0xfff;
> +
> +	if (cnvi < BTINTEL_CNVI_BLAZARI || cnvr != BTINTEL_CNVR_WHP2) {
> +		bt_dev_dbg(hdev, "BT SAR Rev2 not supported on this platform (cnvi=0x%x cnvr=0x%x)",
> +			   cnvi, cnvr);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	bt_dev_info(hdev, "Applying Bluetooth SAR Rev2");
> +	return btintel_set_sar_rev2(hdev, sar);
> +}
> +
>   static int btintel_sar_send_to_device(struct hci_dev *hdev, struct btintel_sar_inc_pwr *sar,
>   				      struct intel_version_tlv *ver)
>   {
> @@ -3130,6 +3236,7 @@ static int btintel_acpi_set_sar(struct hci_dev *hdev, struct intel_version_tlv *
>   {
>   	union acpi_object *bt_pkg, *buffer = NULL;
>   	struct btintel_sar_inc_pwr sar;
> +	struct btintel_sar_rev2 sar_rev2;
>   	acpi_status status;
>   	u8 revision;
>   	int ret;
> @@ -3150,14 +3257,59 @@ static int btintel_acpi_set_sar(struct hci_dev *hdev, struct intel_version_tlv *
>   		goto error;
>   	}
>   
> +	if (buffer->package.elements[0].type != ACPI_TYPE_INTEGER) {
> +		bt_dev_warn(hdev, "BT_SAR: unexpected ACPI type for revision field");
> +		ret = -EINVAL;
> +		goto error;
> +	}
> +
>   	revision = buffer->package.elements[0].integer.value;
>   
> -	if (revision > BTINTEL_SAR_INC_PWR) {
> +	if (revision > BTINTEL_SAR_REV2) {
>   		bt_dev_dbg(hdev, "BT_SAR: revision: 0x%2.2x not supported", revision);
>   		ret = -EOPNOTSUPP;
>   		goto error;
>   	}
>   
> +	if (revision == BTINTEL_SAR_REV2 && bt_pkg->package.count == 13) {
> +		memset(&sar_rev2, 0, sizeof(sar_rev2));
> +		sar_rev2.revision       = revision;
> +		sar_rev2.bt_sar_bios    = bt_pkg->package.elements[1].integer.value;
> +		sar_rev2.inc_power_mode = bt_pkg->package.elements[2].integer.value;
> +
> +		sar_rev2.chain_a.subband_2g4     = bt_pkg->package.elements[3].integer.value;
> +		sar_rev2.chain_a.subband_5g2     = bt_pkg->package.elements[4].integer.value;
> +		sar_rev2.chain_a.subband_5g8_5g9 = bt_pkg->package.elements[5].integer.value;
> +		sar_rev2.chain_a.subband_6g1     = bt_pkg->package.elements[6].integer.value;
> +		sar_rev2.chain_a.subband_6g3     = bt_pkg->package.elements[7].integer.value;
> +
> +		sar_rev2.chain_b.subband_2g4     = bt_pkg->package.elements[8].integer.value;
> +		sar_rev2.chain_b.subband_5g2     = bt_pkg->package.elements[9].integer.value;
> +		sar_rev2.chain_b.subband_5g8_5g9 = bt_pkg->package.elements[10].integer.value;
> +		sar_rev2.chain_b.subband_6g1     = bt_pkg->package.elements[11].integer.value;
> +		sar_rev2.chain_b.subband_6g3     = bt_pkg->package.elements[12].integer.value;

gemini/gemini-3.1-pro-preview has a comment [1]:

> Is it safe to assume elements 1 through 12 are all of type ACPI_TYPE_INTEGER
> here?
> I see an explicit type check was added just above for the revision field
> at buffer->package.elements[0], but elements 1 through 12 in bt_pkg are
> accessed without similar validation.
> If the ACPI table provides a string or buffer for any of these elements, could
> the kernel incorrectly read overlapping union fields (such as string lengths or
> pointers) as integers and send corrupted sub-band limits to the firmware?


> +
> +		bt_dev_dbg(hdev, "BT SAR Rev2: revision=%u bt_sar_bios=%u inc_power_mode=%u",
> +			    sar_rev2.revision, sar_rev2.bt_sar_bios, sar_rev2.inc_power_mode);
> +		bt_dev_dbg(hdev, "BT SAR Rev2 Chain A: 2g4=%u 5g2=%u 5g8_5g9=%u 6g1=%u 6g3=%u",
> +			    sar_rev2.chain_a.subband_2g4, sar_rev2.chain_a.subband_5g2,
> +			    sar_rev2.chain_a.subband_5g8_5g9, sar_rev2.chain_a.subband_6g1,
> +			    sar_rev2.chain_a.subband_6g3);
> +		bt_dev_dbg(hdev, "BT SAR Rev2 Chain B: 2g4=%u 5g2=%u 5g8_5g9=%u 6g1=%u 6g3=%u",
> +			    sar_rev2.chain_b.subband_2g4, sar_rev2.chain_b.subband_5g2,
> +			    sar_rev2.chain_b.subband_5g8_5g9, sar_rev2.chain_b.subband_6g1,
> +			    sar_rev2.chain_b.subband_6g3);
> +
> +		if (sar_rev2.bt_sar_bios != 1) {
> +			bt_dev_info(hdev, "Bluetooth SAR Rev2 is not enabled");
> +			ret = -EOPNOTSUPP;
> +			goto error;
> +		}
> +
> +		ret = btintel_sar_rev2_send_to_device(hdev, &sar_rev2, ver);
> +		goto error;
> +	}
> +
>   	memset(&sar, 0, sizeof(sar));
>   
>   	if (revision == BTINTEL_SAR_LEGACY && bt_pkg->package.count == 8) {
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index 70d812ad36a2..3b5a228ca3c0 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -64,6 +64,7 @@ struct intel_tlv {
>   
>   /* CNVR */
>   #define BTINTEL_CNVR_FMP2		0x910
> +#define BTINTEL_CNVR_WHP2		0xA10	/* Whale Peak2 - Panther Lake */
>   
>   #define BTINTEL_IMG_BOOTLOADER		0x01	/* Bootloader image */
>   #define BTINTEL_IMG_IML			0x02	/* Intermediate image */
> @@ -202,6 +203,23 @@ struct btintel_sar_inc_pwr {
>   	u8	le_lr;
>   };
>   
> +/* Bluetooth SAR feature (BRDS), Revision 2 - per-chain sub-band power limits */
> +struct btintel_sar_band_limits {
> +	u8	subband_2g4;
> +	u8	subband_5g2;
> +	u8	subband_5g8_5g9;
> +	u8	subband_6g1;
> +	u8	subband_6g3;
> +};
> +
> +struct btintel_sar_rev2 {
> +	u8	revision;
> +	u32	bt_sar_bios;    /* 1: BIOS-managed SAR enabled */
> +	u32	inc_power_mode; /* 0: supported, 1: disabled */
> +	struct btintel_sar_band_limits chain_a;
> +	struct btintel_sar_band_limits chain_b;
> +};
> +
>   #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)


[1]: 
https://sashiko.dev/#/patchset/20260430165123.1070214-1-kiran.k%40intel.com

RE: [v1] bluetooth: btintel: Add Bluetooth SAR revision 2 support

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1057 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/project/bluetooth/list/?series=1088244

---Test result---

Test Summary:
CheckPatch                    PASS      1.08 seconds
GitLint                       PASS      0.33 seconds
SubjectPrefix                 FAIL      0.13 seconds
BuildKernel                   PASS      26.51 seconds
CheckAllWarning               PASS      28.79 seconds
CheckSparse                   PASS      27.30 seconds
BuildKernel32                 PASS      25.28 seconds
TestRunnerSetup               PASS      572.34 seconds
IncrementalBuild              PASS      26.13 seconds

Details
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject


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

---
Regards,
Linux Bluetooth

RE: [PATCH v1] bluetooth: btintel: Add Bluetooth SAR revision 2 support

[K, Kiran]

Hi Paul,

Thanks for the comments.

>Subject: Re: [PATCH v1] bluetooth: btintel: Add Bluetooth SAR revision 2
>support
>
>Dear Kiran,
>
>
>Thank you for the patch.
>
>Am 30.04.26 um 18:51 schrieb Kiran K:
>> BRDS revision 2 introduces per-chain (Chain A and Chain B) TX power
>> limits across five sub-bands (2.4G, 5.2G, 5.8/5.9G, 6G-low, 6G-high),
>> replacing the single-chain per-modulation model of revisions 0 and 1.
>>
>> - Add btintel_set_sar_rev2() which sends the full Rev2 DDC sequence:
>>      0x019e  inc-power-mode enable flag        (1 byte)
>>      0x0311  2.4 GHz sub-band limits           (2 bytes)
>>      0x0312  5.2 GHz sub-band limits           (2 bytes)
>>      0x0313  5.8/5.9 GHz sub-band limits       (2 bytes)
>>      0x0314  5.8/5.9 GHz sub-band limits again (2 bytes, duplicate FW reg)
>>      0x0315  6 GHz low sub-band limits         (2 bytes)
>>      0x0316  6 GHz high sub-band limits        (2 bytes)
>>    followed by the SAR-init-complete command (0xfe25).
>
>Please detail how to test it, and please also paste the new debug messages.
Note that ODMs configure the SAR revision and the corresponding per-chain limits directly in coreboot/BIOS. Hardware verification of the actual transmit power levels (per chain and per sub-band) requires a CMW500.

I will include debug logs in v2 version of patch.

>
>> Signed-off-by: Ravindra <ravindra@intel.com>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>>   drivers/bluetooth/btintel.c | 154
>+++++++++++++++++++++++++++++++++++-
>>   drivers/bluetooth/btintel.h |  18 +++++
>>   2 files changed, 171 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
>> index 5e9cac090bd8..97d897623c94 100644
>> --- a/drivers/bluetooth/btintel.c
>> +++ b/drivers/bluetooth/btintel.c
>> @@ -51,6 +51,7 @@ enum {
>>   #define BTINTEL_BT_DOMAIN		0x12
>>   #define BTINTEL_SAR_LEGACY		0
>>   #define BTINTEL_SAR_INC_PWR		1
>> +#define BTINTEL_SAR_REV2		2
>>   #define BTINTEL_SAR_INC_PWR_SUPPORTED	0
>>
>>   #define CMD_WRITE_BOOT_PARAMS	0xfc0e
>> @@ -3104,6 +3105,111 @@ 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)
>> +{
>> +	struct btintel_cp_ddc_write *cmd;
>> +	struct sk_buff *skb;
>> +	u8 buffer[64];
>> +	bool enable;
>> +	int ret;
>> +
>> +	cmd = (void *)buffer;
>> +
>> +	/* DDC 0x019e: enable/disable increased power mode SAR (1 byte) */
>> +	cmd->len = 3;
>> +	cmd->id = cpu_to_le16(0x019e);
>> +	cmd->data[0] = (sar->inc_power_mode ==
>BTINTEL_SAR_INC_PWR_SUPPORTED) ?
>> +			0x01 : 0x00;
>> +	ret = btintel_send_sar_ddc(hdev, cmd, 4);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* DDC 0x0311-0x0316: per sub-band ChainA + ChainB limits */
>> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0311,
>> +					 sar->chain_a.subband_2g4,
>> +					 sar->chain_b.subband_2g4);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0312,
>> +					 sar->chain_a.subband_5g2,
>> +					 sar->chain_b.subband_5g2);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* 0x0313 and 0x0314 both carry the 5G8/5G9 value */
>> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0313,
>> +					 sar->chain_a.subband_5g8_5g9,
>> +					 sar->chain_b.subband_5g8_5g9);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0314,
>> +					 sar->chain_a.subband_5g8_5g9,
>> +					 sar->chain_b.subband_5g8_5g9);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0315,
>> +					 sar->chain_a.subband_6g1,
>> +					 sar->chain_b.subband_6g1);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = btintel_send_sar_rev2_band(hdev, cmd, 0x0316,
>> +					 sar->chain_a.subband_6g3,
>> +					 sar->chain_b.subband_6g3);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Notify firmware that SAR initialisation is complete */
>> +	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 Rev2 Enable
>(%ld)",
>> +			    PTR_ERR(skb));
>> +		return PTR_ERR(skb);
>> +	}
>> +
>> +	kfree_skb(skb);
>> +	return 0;
>> +}
>> +
>> +static int btintel_sar_rev2_send_to_device(struct hci_dev *hdev,
>> +					   struct btintel_sar_rev2 *sar,
>> +					   struct intel_version_tlv *ver) {
>> +	u16 cnvi = ver->cnvi_top & 0xfff;
>> +	u16 cnvr = ver->cnvr_top & 0xfff;
>> +
>> +	if (cnvi < BTINTEL_CNVI_BLAZARI || cnvr != BTINTEL_CNVR_WHP2) {
>> +		bt_dev_dbg(hdev, "BT SAR Rev2 not supported on this
>platform (cnvi=0x%x cnvr=0x%x)",
>> +			   cnvi, cnvr);
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	bt_dev_info(hdev, "Applying Bluetooth SAR Rev2");
>> +	return btintel_set_sar_rev2(hdev, sar); }
>> +
>>   static int btintel_sar_send_to_device(struct hci_dev *hdev, struct
>btintel_sar_inc_pwr *sar,
>>   				      struct intel_version_tlv *ver)
>>   {
>> @@ -3130,6 +3236,7 @@ static int btintel_acpi_set_sar(struct hci_dev
>*hdev, struct intel_version_tlv *
>>   {
>>   	union acpi_object *bt_pkg, *buffer = NULL;
>>   	struct btintel_sar_inc_pwr sar;
>> +	struct btintel_sar_rev2 sar_rev2;
>>   	acpi_status status;
>>   	u8 revision;
>>   	int ret;
>> @@ -3150,14 +3257,59 @@ static int btintel_acpi_set_sar(struct hci_dev
>*hdev, struct intel_version_tlv *
>>   		goto error;
>>   	}
>>
>> +	if (buffer->package.elements[0].type != ACPI_TYPE_INTEGER) {
>> +		bt_dev_warn(hdev, "BT_SAR: unexpected ACPI type for
>revision field");
>> +		ret = -EINVAL;
>> +		goto error;
>> +	}
>> +
>>   	revision = buffer->package.elements[0].integer.value;
>>
>> -	if (revision > BTINTEL_SAR_INC_PWR) {
>> +	if (revision > BTINTEL_SAR_REV2) {
>>   		bt_dev_dbg(hdev, "BT_SAR: revision: 0x%2.2x not supported",
>revision);
>>   		ret = -EOPNOTSUPP;
>>   		goto error;
>>   	}
>>
>> +	if (revision == BTINTEL_SAR_REV2 && bt_pkg->package.count == 13) {
>> +		memset(&sar_rev2, 0, sizeof(sar_rev2));
>> +		sar_rev2.revision       = revision;
>> +		sar_rev2.bt_sar_bios    = bt_pkg-
>>package.elements[1].integer.value;
>> +		sar_rev2.inc_power_mode =
>> +bt_pkg->package.elements[2].integer.value;
>> +
>> +		sar_rev2.chain_a.subband_2g4     = bt_pkg-
>>package.elements[3].integer.value;
>> +		sar_rev2.chain_a.subband_5g2     = bt_pkg-
>>package.elements[4].integer.value;
>> +		sar_rev2.chain_a.subband_5g8_5g9 = bt_pkg-
>>package.elements[5].integer.value;
>> +		sar_rev2.chain_a.subband_6g1     = bt_pkg-
>>package.elements[6].integer.value;
>> +		sar_rev2.chain_a.subband_6g3     = bt_pkg-
>>package.elements[7].integer.value;
>> +
>> +		sar_rev2.chain_b.subband_2g4     = bt_pkg-
>>package.elements[8].integer.value;
>> +		sar_rev2.chain_b.subband_5g2     = bt_pkg-
>>package.elements[9].integer.value;
>> +		sar_rev2.chain_b.subband_5g8_5g9 = bt_pkg-
>>package.elements[10].integer.value;
>> +		sar_rev2.chain_b.subband_6g1     = bt_pkg-
>>package.elements[11].integer.value;
>> +		sar_rev2.chain_b.subband_6g3     = bt_pkg-
>>package.elements[12].integer.value;
>
>gemini/gemini-3.1-pro-preview has a comment [1]:
>
>> Is it safe to assume elements 1 through 12 are all of type
>> ACPI_TYPE_INTEGER here?
>> I see an explicit type check was added just above for the revision
>> field at buffer->package.elements[0], but elements 1 through 12 in
>> bt_pkg are accessed without similar validation.
>> If the ACPI table provides a string or buffer for any of these
>> elements, could the kernel incorrectly read overlapping union fields
>> (such as string lengths or
>> pointers) as integers and send corrupted sub-band limits to the firmware?
>
>
>> +
>> +		bt_dev_dbg(hdev, "BT SAR Rev2: revision=%u bt_sar_bios=%u
>inc_power_mode=%u",
>> +			    sar_rev2.revision, sar_rev2.bt_sar_bios,
>sar_rev2.inc_power_mode);
>> +		bt_dev_dbg(hdev, "BT SAR Rev2 Chain A: 2g4=%u 5g2=%u
>5g8_5g9=%u 6g1=%u 6g3=%u",
>> +			    sar_rev2.chain_a.subband_2g4,
>sar_rev2.chain_a.subband_5g2,
>> +			    sar_rev2.chain_a.subband_5g8_5g9,
>sar_rev2.chain_a.subband_6g1,
>> +			    sar_rev2.chain_a.subband_6g3);
>> +		bt_dev_dbg(hdev, "BT SAR Rev2 Chain B: 2g4=%u 5g2=%u
>5g8_5g9=%u 6g1=%u 6g3=%u",
>> +			    sar_rev2.chain_b.subband_2g4,
>sar_rev2.chain_b.subband_5g2,
>> +			    sar_rev2.chain_b.subband_5g8_5g9,
>sar_rev2.chain_b.subband_6g1,
>> +			    sar_rev2.chain_b.subband_6g3);
>> +
>> +		if (sar_rev2.bt_sar_bios != 1) {
>> +			bt_dev_info(hdev, "Bluetooth SAR Rev2 is not
>enabled");
>> +			ret = -EOPNOTSUPP;
>> +			goto error;
>> +		}
>> +
>> +		ret = btintel_sar_rev2_send_to_device(hdev, &sar_rev2, ver);
>> +		goto error;
>> +	}
>> +
>>   	memset(&sar, 0, sizeof(sar));
>>
>>   	if (revision == BTINTEL_SAR_LEGACY && bt_pkg->package.count == 8)
>{
>> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
>> index 70d812ad36a2..3b5a228ca3c0 100644
>> --- a/drivers/bluetooth/btintel.h
>> +++ b/drivers/bluetooth/btintel.h
>> @@ -64,6 +64,7 @@ struct intel_tlv {
>>
>>   /* CNVR */
>>   #define BTINTEL_CNVR_FMP2		0x910
>> +#define BTINTEL_CNVR_WHP2		0xA10	/* Whale Peak2 -
>Panther Lake */
>>
>>   #define BTINTEL_IMG_BOOTLOADER		0x01	/* Bootloader
>image */
>>   #define BTINTEL_IMG_IML			0x02	/* Intermediate image
>*/
>> @@ -202,6 +203,23 @@ struct btintel_sar_inc_pwr {
>>   	u8	le_lr;
>>   };
>>
>> +/* Bluetooth SAR feature (BRDS), Revision 2 - per-chain sub-band
>> +power limits */ struct btintel_sar_band_limits {
>> +	u8	subband_2g4;
>> +	u8	subband_5g2;
>> +	u8	subband_5g8_5g9;
>> +	u8	subband_6g1;
>> +	u8	subband_6g3;
>> +};
>> +
>> +struct btintel_sar_rev2 {
>> +	u8	revision;
>> +	u32	bt_sar_bios;    /* 1: BIOS-managed SAR enabled */
>> +	u32	inc_power_mode; /* 0: supported, 1: disabled */
>> +	struct btintel_sar_band_limits chain_a;
>> +	struct btintel_sar_band_limits chain_b; };
>> +
>>   #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)
>
>
>[1]:
>https://sashiko.dev/#/patchset/20260430165123.1070214-1-
>kiran.k%40intel.com

Thanks,
Kiran