diff mbox series

[v1] Bluetooth: btintel: Fix MSFT extension support for Intel controllers

Message ID 20211221034246.2416-1-kiran.k@intel.com (mailing list archive)
State Rejected
Headers show
Series [v1] Bluetooth: btintel: Fix MSFT extension support for Intel controllers | expand

Checks

Context Check Description
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS
tedd_an/buildkernel success Build Kernel PASS
tedd_an/incremental_build success Pass
tedd_an/testrunnersetup success Test Runner Setup PASS
tedd_an/testrunnerl2cap-tester success Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnerbnep-tester success Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnermgmt-tester success Total: 493, Passed: 493 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnerrfcomm-tester success Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnersco-tester success Total: 12, Passed: 12 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnersmp-tester success Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunneruserchan-tester success Total: 4, Passed: 4 (100.0%), Failed: 0, Not Run: 0

Commit Message

K, Kiran Dec. 21, 2021, 3:42 a.m. UTC 
From: Chethan T N <chethan.tumkur.narayan@intel.com>

For ThP2/JfP2/HrP2 and CcP2 Intel controllers, MSFT extension
opcode was not set in operational mode.

Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

An, Tedd Dec. 21, 2021, 6:43 p.m. UTC | #1 
Hi Kiran,

On Tue, 2021-12-21 at 09:12 +0530, Kiran K wrote:
> From: Chethan T N <chethan.tumkur.narayan@intel.com>
> 
> For ThP2/JfP2/HrP2 and CcP2 Intel controllers, MSFT extension
> opcode was not set in operational mode.
> 
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>  drivers/bluetooth/btintel.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index e1f96df847b8..0dd389746439 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2549,10 +2549,6 @@ static int btintel_setup_combined(struct hci_dev *hdev)
>                 if (INTEL_HW_VARIANT(ver_tlv.cnvi_bt) == 0x18)
>                         set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks);
>  
> -               /* Setup MSFT Extension support */
> -               btintel_set_msft_opcode(hdev,
> -                                       INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
> -
>                 err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
>                 break;
>         default:
> @@ -2560,6 +2556,8 @@ static int btintel_setup_combined(struct hci_dev *hdev)
>                            INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
>                 return -EINVAL;
>         }
> +       /* Setup MSFT Extension support */
> +       btintel_set_msft_opcode(hdev, INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
> 
It should be fixed in this patch. Please confirm.
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=a33d805b322583a3505e95f3e57eada81cac34bd

>  
>  exit_error:
>         kfree_skb(skb);

Regards,
Tedd
Luiz Augusto von Dentz Dec. 21, 2021, 6:56 p.m. UTC | #2 
Hi Kiran,

On Tue, Dec 21, 2021 at 1:49 AM Kiran K <kiran.k@intel.com> wrote:
>
> From: Chethan T N <chethan.tumkur.narayan@intel.com>
>
> For ThP2/JfP2/HrP2 and CcP2 Intel controllers, MSFT extension
> opcode was not set in operational mode.

You should probably add a Fixes for the original patch that introduces
this problem.

> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>  drivers/bluetooth/btintel.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index e1f96df847b8..0dd389746439 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2549,10 +2549,6 @@ static int btintel_setup_combined(struct hci_dev *hdev)
>                 if (INTEL_HW_VARIANT(ver_tlv.cnvi_bt) == 0x18)
>                         set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks);
>
> -               /* Setup MSFT Extension support */
> -               btintel_set_msft_opcode(hdev,
> -                                       INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
> -
>                 err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
>                 break;
>         default:
> @@ -2560,6 +2556,8 @@ static int btintel_setup_combined(struct hci_dev *hdev)
>                            INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
>                 return -EINVAL;
>         }

Please add a comment stating why it is correct to do it here instead.

> +       /* Setup MSFT Extension support */
> +       btintel_set_msft_opcode(hdev, INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
>
>  exit_error:
>         kfree_skb(skb);
> --
> 2.17.1
>
K, Kiran Jan. 3, 2022, 7:23 a.m. UTC | #3 
Hi Tedd,

> Subject: Re: [PATCH v1] Bluetooth: btintel: Fix MSFT extension support for
> Intel controllers
> 
> Hi Kiran,
> 
> On Tue, 2021-12-21 at 09:12 +0530, Kiran K wrote:
> > From: Chethan T N <chethan.tumkur.narayan@intel.com>
> >
> > For ThP2/JfP2/HrP2 and CcP2 Intel controllers, MSFT extension opcode
> > was not set in operational mode.
> >
> > Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > ---
> >  drivers/bluetooth/btintel.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index e1f96df847b8..0dd389746439 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -2549,10 +2549,6 @@ static int btintel_setup_combined(struct
> > hci_dev *hdev)
> >                 if (INTEL_HW_VARIANT(ver_tlv.cnvi_bt) == 0x18)
> >                         set_bit(HCI_QUIRK_VALID_LE_STATES,
> > &hdev->quirks);
> >
> > -               /* Setup MSFT Extension support */
> > -               btintel_set_msft_opcode(hdev,
> > -
> > INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
> > -
> >                 err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
> >                 break;
> >         default:
> > @@ -2560,6 +2556,8 @@ static int btintel_setup_combined(struct hci_dev
> > *hdev)
> >                            INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
> >                 return -EINVAL;
> >         }
> > +       /* Setup MSFT Extension support */
> > +       btintel_set_msft_opcode(hdev,
> > +INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
> >
> It should be fixed in this patch. Please confirm.
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-
> next.git/commit/?id=a33d805b322583a3505e95f3e57eada81cac34bd

Yes. This patch also fixes the issue. Thanks.

Regards,
Kiran
diff mbox series

Patch

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e1f96df847b8..0dd389746439 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -2549,10 +2549,6 @@  static int btintel_setup_combined(struct hci_dev *hdev)
 		if (INTEL_HW_VARIANT(ver_tlv.cnvi_bt) == 0x18)
 			set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks);
 
-		/* Setup MSFT Extension support */
-		btintel_set_msft_opcode(hdev,
-					INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
-
 		err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
 		break;
 	default:
@@ -2560,6 +2556,8 @@  static int btintel_setup_combined(struct hci_dev *hdev)
 			   INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
 		return -EINVAL;
 	}
+	/* Setup MSFT Extension support */
+	btintel_set_msft_opcode(hdev, INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
 
 exit_error:
 	kfree_skb(skb);