Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200316123914.Bluez.v1.1.I2c83372de789a015c1ee506690bb795ee0b0b0d9@changeid>
In-Reply-To: <20200316123914.Bluez.v1.1.I2c83372de789a015c1ee506690bb795ee0b0b0d9@changeid>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Mon, 16 Mar 2020 13:57:57 -0700
Message-ID: <CABBYNZL1D44M4z2c+4zbjLgZ=PMHsTwy1VpYZpsJbpFJ9POZug@mail.gmail.com>
Subject: Re: [Bluez PATCH v1] input: disconnect intr channel before ctrl channel
To:     Archie Pusaka <apusaka@google.com>
Cc:     linux-bluetooth <linux-bluetooth@vger.kernel.org>,
        Archie Pusaka <apusaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Archie,

On Sun, Mar 15, 2020 at 9:40 PM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> According to bluetooth HID Profile spec Ver 1.0, section 7.2.2, A
> host or device shall always complete the disconnection of the
> interrupt channel before disconnecting the control channel.
> However, the current implementation disconnects them both
> simultaneously.
>
> This patch postpone the disconnection of control channel to the
> callback of interrupt watch, which shall be called upon receiving
> interrupt channel disconnection response.
> ---
>
>  profiles/input/device.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/profiles/input/device.c b/profiles/input/device.c
> index 8ada1b4ff..8ef3714c9 100644
> --- a/profiles/input/device.c
> +++ b/profiles/input/device.c
> @@ -1010,14 +1010,19 @@ static bool is_connected(struct input_device *idev)
>
>  static int connection_disconnect(struct input_device *idev, uint32_t flags)
>  {
> +       int sock;
> +
>         if (!is_connected(idev))
>                 return -ENOTCONN;
>
> -       /* Standard HID disconnect */
> -       if (idev->intr_io)
> -               g_io_channel_shutdown(idev->intr_io, TRUE, NULL);
> -       if (idev->ctrl_io)
> -               g_io_channel_shutdown(idev->ctrl_io, TRUE, NULL);
> +       /* Standard HID disconnect
> +        * Intr channel must be disconnected before ctrl channel, so only
> +        * disconnect intr here, ctrl is disconnected in intr_watch_cb.
> +        */
> +       if (idev->intr_io) {
> +               sock = g_io_channel_unix_get_fd(idev->intr_io);
> +               shutdown(sock, SHUT_RDWR);
> +       }
>
>         if (idev->uhid)
>                 return 0;
> --
> 2.25.1.481.gfbce0eb801-goog

Just to confirm, have you checked if this works with both situation
where the device is being removed or just being disconnected,
specially the case of HIDP_CTRL_VIRTUAL_CABLE_UNPLUG which perhaps was
not working before as well since we disconnect the ctrl channel before
transmitting it.

-- 
Luiz Augusto von Dentz