MIME-Version: 1.0
References: <20211221034246.2416-1-kiran.k@intel.com>
In-Reply-To: <20211221034246.2416-1-kiran.k@intel.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 21 Dec 2021 10:56:43 -0800
Message-ID: <CABBYNZ+ON-jVfM_g4T-sXaPY-7n0mv4JTvvpH88GNwdPoVs=Pw@mail.gmail.com>
Subject: Re: [PATCH v1] Bluetooth: btintel: Fix MSFT extension support for
 Intel controllers
To:     Kiran K <kiran.k@intel.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "Srivatsa, Ravishankar" <ravishankar.srivatsa@intel.com>,
        Chethan T N <chethan.tumkur.narayan@intel.com>,
        "An, Tedd" <tedd.an@intel.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>


-- 
Luiz Augusto von Dentz