Hi Luiz, Luckily you asked, because I found out that actually the patch didn't wait for the disconnection response for any case. It does delay the disconnection of the ctrl channel slightly though, but that doesn't guarantee a proper order of disconnection. Therefore, as of now, this patch is not fixing anything. Digging more into this matter, I found out by removing this condition (sk->sk_shutdown == SHUTDOWN_MASK) in [1], it makes intr_watch_cb() called after truly receiving the interrupt disconnection response. However, I haven't checked whether removal of such condition will break other things. Do you have any suggestions? With this patch and removal of that condition, I confirm that it works with situations where the device is being removed and/or just being disconnected. It also works with virtual cable unplug when UHID is used. * Virtual cable unplug has another problem which doesn't adhere to the specs, but it is unrelated to disconnection. [1] https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/af_bluetooth.c#n470 Thanks, Archie On Tue, 17 Mar 2020 at 04:58, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > Hi Archie, > > On Sun, Mar 15, 2020 at 9:40 PM Archie Pusaka <apusaka@xxxxxxxxxx> wrote: > > > > From: Archie Pusaka <apusaka@xxxxxxxxxxxx> > > > > 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
