| Message ID | 20230810145653.1780-1-lokendra.singh@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] Bluetooth: btintel: Send new command for PPAG | expand |
Hi Lokendra, On Thu, Aug 10, 2023 at 8:18 AM Lokendra Singh <lokendra.singh@intel.com> wrote: > > Added support for the new command opcode FE0B > (HCI Intel PPAG Enable). > > btmon log: > < HCI Command: Intel PPAG Enable (0x3f|0x020b) plen 4 > Enable: 0x00000002 > > HCI Event: Command Complete (0x0e) plen 4 > Intel PPAG Enable (0x3f|0x020b) ncmd 1 > Status: Success (0x00) > > Signed-off-by: Seema Sreemantha <seema.sreemantha@intel.com> > Signed-off-by: Lokendra Singh <lokendra.singh@intel.com> > --- > drivers/bluetooth/btintel.c | 28 +++++++++++++++++++--------- > drivers/bluetooth/btintel.h | 8 +++----- > 2 files changed, 22 insertions(+), 14 deletions(-) > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > index 633e8d9bf58f..d2c93b88c527 100644 > --- a/drivers/bluetooth/btintel.c > +++ b/drivers/bluetooth/btintel.c > @@ -2401,7 +2401,7 @@ static void btintel_set_ppag(struct hci_dev *hdev, struct intel_version_tlv *ver > { > struct btintel_ppag ppag; > struct sk_buff *skb; > - struct btintel_loc_aware_reg ppag_cmd; > + struct hci_ppag_enable_cmd ppag_cmd; > acpi_handle handle; > > /* PPAG is not supported if CRF is HrP2, Jfp2, JfP1 */ > @@ -2409,6 +2409,8 @@ static void btintel_set_ppag(struct hci_dev *hdev, struct intel_version_tlv *ver > case 0x504: /* Hrp2 */ > case 0x202: /* Jfp2 */ > case 0x201: /* Jfp1 */ > + bt_dev_warn(hdev, "PPAG not supported for Intel CNVr (0x%3x)", > + ver->cnvr_top & 0xFFF); If this doesn't change the functionality I'd recommend using bt_dev_dbg so we don't warn users for no reason. > return; > } > > @@ -2434,24 +2436,32 @@ static void btintel_set_ppag(struct hci_dev *hdev, struct intel_version_tlv *ver > } > > if (ppag.domain != 0x12) { > - bt_dev_warn(hdev, "PPAG-BT: domain is not bluetooth"); > + bt_dev_warn(hdev, "PPAG-BT: Bluetooth domain is disabled in ACPI firmware"); Ditto. > return; > } > > - /* PPAG mode, BIT0 = 0 Disabled, BIT0 = 1 Enabled */ > - if (!(ppag.mode & BIT(0))) { > - bt_dev_dbg(hdev, "PPAG-BT: disabled"); > + /* PPAG mode > + * BIT 0 : 0 Disabled in EU > + * 1 Enabled in EU > + * BIT 1 : 0 Disabled in China > + * 1 Enabled in China > + */ > + if ((ppag.mode & 0x01) != BIT(0) && (ppag.mode & 0x02) != BIT(1)) { > + bt_dev_warn(hdev, "PPAG-BT: EU, China mode are disabled in CB/BIOS"); Ditto. > return; > } > > - ppag_cmd.mcc = cpu_to_le32(0); > - ppag_cmd.sel = cpu_to_le32(0); /* 0 - Enable , 1 - Disable, 2 - Testing mode */ > - ppag_cmd.delta = cpu_to_le32(0); > - skb = __hci_cmd_sync(hdev, 0xfe19, sizeof(ppag_cmd), &ppag_cmd, HCI_CMD_TIMEOUT); > + /* HCI_Intel_PpagEnable_CMD - opcode: 0xFE0B > + * ppag_enable_flags - ppag mode > + */ > + ppag_cmd.ppag_enable_flags = ppag.mode; > + > + skb = __hci_cmd_sync(hdev, 0xfe0b, sizeof(ppag_cmd), &ppag_cmd, HCI_CMD_TIMEOUT); > if (IS_ERR(skb)) { > bt_dev_warn(hdev, "Failed to send PPAG Enable (%ld)", PTR_ERR(skb)); > return; > } > + bt_dev_info(hdev, "PPAG-BT: Enabled (Mode %d)", ppag.mode); > kfree_skb(skb); > } > > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h > index 2ed646609dee..01e87f68fab0 100644 > --- a/drivers/bluetooth/btintel.h > +++ b/drivers/bluetooth/btintel.h > @@ -137,11 +137,9 @@ struct intel_offload_use_cases { > __u8 preset[8]; > } __packed; > > -struct btintel_loc_aware_reg { > - __le32 mcc; > - __le32 sel; > - __le32 delta; > -} __packed; > +struct hci_ppag_enable_cmd { > + u32 ppag_enable_flags; > +}; Lets also add defines to command opcodes so we don't have to add comments on what the opcode means around the codebase. > #define INTEL_TLV_TYPE_ID 0x01 > > -- > 2.25.1 >
Hi Luiz, > -----Original Message----- > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > Sent: Thursday, 10 August, 2023 11:20 PM > To: Singh, Lokendra <lokendra.singh@intel.com> > Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar > <ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan > <chethan.tumkur.narayan@intel.com>; Von Dentz, Luiz > <luiz.von.dentz@intel.com>; An, Tedd <tedd.an@intel.com>; K, Kiran > <kiran.k@intel.com>; Seema Sreemantha <seema.sreemantha@intel.com> > Subject: Re: [PATCH v2] Bluetooth: btintel: Send new command for PPAG > > Hi Lokendra, > > On Thu, Aug 10, 2023 at 8:18 AM Lokendra Singh > <lokendra.singh@intel.com> wrote: > > > > Added support for the new command opcode FE0B (HCI Intel PPAG Enable). > > > > btmon log: > > < HCI Command: Intel PPAG Enable (0x3f|0x020b) plen 4 > > Enable: 0x00000002 > > > HCI Event: Command Complete (0x0e) plen 4 > > Intel PPAG Enable (0x3f|0x020b) ncmd 1 > > Status: Success (0x00) > > > > Signed-off-by: Seema Sreemantha <seema.sreemantha@intel.com> > > Signed-off-by: Lokendra Singh <lokendra.singh@intel.com> > > --- > > drivers/bluetooth/btintel.c | 28 +++++++++++++++++++--------- > > drivers/bluetooth/btintel.h | 8 +++----- > > 2 files changed, 22 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > > index 633e8d9bf58f..d2c93b88c527 100644 > > --- a/drivers/bluetooth/btintel.c > > +++ b/drivers/bluetooth/btintel.c > > @@ -2401,7 +2401,7 @@ static void btintel_set_ppag(struct hci_dev > > *hdev, struct intel_version_tlv *ver { > > struct btintel_ppag ppag; > > struct sk_buff *skb; > > - struct btintel_loc_aware_reg ppag_cmd; > > + struct hci_ppag_enable_cmd ppag_cmd; > > acpi_handle handle; > > > > /* PPAG is not supported if CRF is HrP2, Jfp2, JfP1 */ @@ > > -2409,6 +2409,8 @@ static void btintel_set_ppag(struct hci_dev *hdev, > struct intel_version_tlv *ver > > case 0x504: /* Hrp2 */ > > case 0x202: /* Jfp2 */ > > case 0x201: /* Jfp1 */ > > + bt_dev_warn(hdev, "PPAG not supported for Intel CNVr > (0x%3x)", > > + ver->cnvr_top & 0xFFF); > > If this doesn't change the functionality I'd recommend using bt_dev_dbg > so we don't warn users for no reason. Ack. I will update it in next patch. > > > return; > > } > > > > @@ -2434,24 +2436,32 @@ static void btintel_set_ppag(struct hci_dev > *hdev, struct intel_version_tlv *ver > > } > > > > if (ppag.domain != 0x12) { > > - bt_dev_warn(hdev, "PPAG-BT: domain is not bluetooth"); > > + bt_dev_warn(hdev, "PPAG-BT: Bluetooth domain is > > + disabled in ACPI firmware"); > > Ditto. Ack. I will update it in next patch. > > > return; > > } > > > > - /* PPAG mode, BIT0 = 0 Disabled, BIT0 = 1 Enabled */ > > - if (!(ppag.mode & BIT(0))) { > > - bt_dev_dbg(hdev, "PPAG-BT: disabled"); > > + /* PPAG mode > > + * BIT 0 : 0 Disabled in EU > > + * 1 Enabled in EU > > + * BIT 1 : 0 Disabled in China > > + * 1 Enabled in China > > + */ > > + if ((ppag.mode & 0x01) != BIT(0) && (ppag.mode & 0x02) != > BIT(1)) { > > + bt_dev_warn(hdev, "PPAG-BT: EU, China mode are > > + disabled in CB/BIOS"); > > Ditto. Ack. I will update it in next patch. > > > return; > > } > > > > - ppag_cmd.mcc = cpu_to_le32(0); > > - ppag_cmd.sel = cpu_to_le32(0); /* 0 - Enable , 1 - Disable, 2 > - Testing mode */ > > - ppag_cmd.delta = cpu_to_le32(0); > > - skb = __hci_cmd_sync(hdev, 0xfe19, sizeof(ppag_cmd), > &ppag_cmd, HCI_CMD_TIMEOUT); > > + /* HCI_Intel_PpagEnable_CMD - opcode: 0xFE0B > > + * ppag_enable_flags - ppag mode > > + */ > > + ppag_cmd.ppag_enable_flags = ppag.mode; > > + > > + skb = __hci_cmd_sync(hdev, 0xfe0b, sizeof(ppag_cmd), > > + &ppag_cmd, HCI_CMD_TIMEOUT); > > if (IS_ERR(skb)) { > > bt_dev_warn(hdev, "Failed to send PPAG Enable (%ld)", > PTR_ERR(skb)); > > return; > > } > > + bt_dev_info(hdev, "PPAG-BT: Enabled (Mode %d)", ppag.mode); > > kfree_skb(skb); > > } > > > > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h > > index 2ed646609dee..01e87f68fab0 100644 > > --- a/drivers/bluetooth/btintel.h > > +++ b/drivers/bluetooth/btintel.h > > @@ -137,11 +137,9 @@ struct intel_offload_use_cases { > > __u8 preset[8]; > > } __packed; > > > > -struct btintel_loc_aware_reg { > > - __le32 mcc; > > - __le32 sel; > > - __le32 delta; > > -} __packed; > > +struct hci_ppag_enable_cmd { > > + u32 ppag_enable_flags; > > +}; > > Lets also add defines to command opcodes so we don't have to add > comments on what the opcode means around the codebase. Ack. I will update it in next patch. > > > #define INTEL_TLV_TYPE_ID 0x01 > > > > -- > > 2.25.1 > > > > > -- > Luiz Augusto von Dentz
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c index 633e8d9bf58f..d2c93b88c527 100644 --- a/drivers/bluetooth/btintel.c +++ b/drivers/bluetooth/btintel.c @@ -2401,7 +2401,7 @@ static void btintel_set_ppag(struct hci_dev *hdev, struct intel_version_tlv *ver { struct btintel_ppag ppag; struct sk_buff *skb; - struct btintel_loc_aware_reg ppag_cmd; + struct hci_ppag_enable_cmd ppag_cmd; acpi_handle handle; /* PPAG is not supported if CRF is HrP2, Jfp2, JfP1 */ @@ -2409,6 +2409,8 @@ static void btintel_set_ppag(struct hci_dev *hdev, struct intel_version_tlv *ver case 0x504: /* Hrp2 */ case 0x202: /* Jfp2 */ case 0x201: /* Jfp1 */ + bt_dev_warn(hdev, "PPAG not supported for Intel CNVr (0x%3x)", + ver->cnvr_top & 0xFFF); return; } @@ -2434,24 +2436,32 @@ static void btintel_set_ppag(struct hci_dev *hdev, struct intel_version_tlv *ver } if (ppag.domain != 0x12) { - bt_dev_warn(hdev, "PPAG-BT: domain is not bluetooth"); + bt_dev_warn(hdev, "PPAG-BT: Bluetooth domain is disabled in ACPI firmware"); return; } - /* PPAG mode, BIT0 = 0 Disabled, BIT0 = 1 Enabled */ - if (!(ppag.mode & BIT(0))) { - bt_dev_dbg(hdev, "PPAG-BT: disabled"); + /* PPAG mode + * BIT 0 : 0 Disabled in EU + * 1 Enabled in EU + * BIT 1 : 0 Disabled in China + * 1 Enabled in China + */ + if ((ppag.mode & 0x01) != BIT(0) && (ppag.mode & 0x02) != BIT(1)) { + bt_dev_warn(hdev, "PPAG-BT: EU, China mode are disabled in CB/BIOS"); return; } - ppag_cmd.mcc = cpu_to_le32(0); - ppag_cmd.sel = cpu_to_le32(0); /* 0 - Enable , 1 - Disable, 2 - Testing mode */ - ppag_cmd.delta = cpu_to_le32(0); - skb = __hci_cmd_sync(hdev, 0xfe19, sizeof(ppag_cmd), &ppag_cmd, HCI_CMD_TIMEOUT); + /* HCI_Intel_PpagEnable_CMD - opcode: 0xFE0B + * ppag_enable_flags - ppag mode + */ + ppag_cmd.ppag_enable_flags = ppag.mode; + + skb = __hci_cmd_sync(hdev, 0xfe0b, sizeof(ppag_cmd), &ppag_cmd, HCI_CMD_TIMEOUT); if (IS_ERR(skb)) { bt_dev_warn(hdev, "Failed to send PPAG Enable (%ld)", PTR_ERR(skb)); return; } + bt_dev_info(hdev, "PPAG-BT: Enabled (Mode %d)", ppag.mode); kfree_skb(skb); } diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h index 2ed646609dee..01e87f68fab0 100644 --- a/drivers/bluetooth/btintel.h +++ b/drivers/bluetooth/btintel.h @@ -137,11 +137,9 @@ struct intel_offload_use_cases { __u8 preset[8]; } __packed; -struct btintel_loc_aware_reg { - __le32 mcc; - __le32 sel; - __le32 delta; -} __packed; +struct hci_ppag_enable_cmd { + u32 ppag_enable_flags; +}; #define INTEL_TLV_TYPE_ID 0x01
Added support for the new command opcode FE0B (HCI Intel PPAG Enable). btmon log: < HCI Command: Intel PPAG Enable (0x3f|0x020b) plen 4 Enable: 0x00000002 > HCI Event: Command Complete (0x0e) plen 4 Intel PPAG Enable (0x3f|0x020b) ncmd 1 Status: Success (0x00) Signed-off-by: Seema Sreemantha <seema.sreemantha@intel.com> Signed-off-by: Lokendra Singh <lokendra.singh@intel.com> --- drivers/bluetooth/btintel.c | 28 +++++++++++++++++++--------- drivers/bluetooth/btintel.h | 8 +++----- 2 files changed, 22 insertions(+), 14 deletions(-)