Re: [PATCH v4] Bluetooth: Keep MSFT ext info throughout a hci_dev's life cycle

[Miao-chen Chou]

Hi Marcel,

A friendly ping on this patch. :)

Regards,
Miao

On Wed, Jan 27, 2021 at 9:17 AM Miao-chen Chou <mcchou@xxxxxxxxxxxx> wrote:
>
> This moves msft_do_close() from hci_dev_do_close() to
> hci_unregister_dev() to avoid clearing MSFT extension info. This also
> re-reads MSFT info upon every msft_do_open() even if MSFT extension has
> been initialized.
>
> The following test steps were performed.
> (1) boot the test device and verify the MSFT support debug log in syslog
> (2) restart bluetoothd and verify msft_do_close() doesn't get invoked
>     and msft_do_open re-reads the MSFT support.
>
> Signed-off-by: Miao-chen Chou <mcchou@xxxxxxxxxxxx>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx>
> Reviewed-by: Archie Pusaka <apusaka@xxxxxxxxxxxx>
> Reviewed-by: Alain Michaud <alainm@xxxxxxxxxxxx>
> ---
>
> Changes in v4:
> - Re-read the MSFT data instead of skipping if it's initiated already
>
> Changes in v3:
> - Remove the accepted commits from the series
>
>  net/bluetooth/hci_core.c |  4 ++--
>  net/bluetooth/msft.c     | 21 ++++++++++++++++++---
>  2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index eeafed2efc0da..8056f0d4ae172 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1764,8 +1764,6 @@ int hci_dev_do_close(struct hci_dev *hdev)
>
>         hci_sock_dev_event(hdev, HCI_DEV_DOWN);
>
> -       msft_do_close(hdev);
> -
>         if (hdev->flush)
>                 hdev->flush(hdev);
>
> @@ -3844,6 +3842,8 @@ void hci_unregister_dev(struct hci_dev *hdev)
>         unregister_pm_notifier(&hdev->suspend_notifier);
>         cancel_work_sync(&hdev->suspend_prepare);
>
> +       msft_do_close(hdev);
> +
>         hci_dev_do_close(hdev);
>
>         if (!test_bit(HCI_INIT, &hdev->flags) &&
> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> index 8579bfeb28364..4465d018280eb 100644
> --- a/net/bluetooth/msft.c
> +++ b/net/bluetooth/msft.c
> @@ -73,16 +73,31 @@ static bool read_supported_features(struct hci_dev *hdev,
>
>  void msft_do_open(struct hci_dev *hdev)
>  {
> -       struct msft_data *msft;
> +       struct msft_data *msft = NULL;
>
>         if (hdev->msft_opcode == HCI_OP_NOP)
>                 return;
>
>         bt_dev_dbg(hdev, "Initialize MSFT extension");
>
> -       msft = kzalloc(sizeof(*msft), GFP_KERNEL);
> -       if (!msft)
> +       /* If MSFT data exists, reset its members */
> +       if (hdev->msft_data) {
> +               msft = hdev->msft_data;
> +               hdev->msft_data = NULL;
> +
> +               msft->features = 0;
> +               kfree(msft->evt_prefix);
> +               msft->evt_prefix = NULL;
> +               msft->evt_prefix_len = 0;
> +
> +       } else {
> +               msft = kzalloc(sizeof(*msft), GFP_KERNEL);
> +       }
> +
> +       if (!msft) {
> +               bt_dev_err(hdev, "Failed to init MSFT extension");
>                 return;
> +       }
>
>         if (!read_supported_features(hdev, msft)) {
>                 kfree(msft);
> --
> 2.30.0.280.ga3ce27912f-goog
>