All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harry Cutts <hcutts@chromium.org>
To: benjamin.tissoires@redhat.com
Cc: Nestor Lopez Casado <nlopezcasad@logitech.com>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	jikos@kernel.org
Subject: Re: [PATCH 3/3] Enable high-resolution scrolling on Logitech mice
Date: Thu, 30 Aug 2018 13:37:58 -0700	[thread overview]
Message-ID: <CA+jURcsDJbL_3aF7wE0MQ_6ZSVV3Aba-tUDX-f=1+y+LBhqmng@mail.gmail.com> (raw)
In-Reply-To: <CAO-hwJ+tb6XdEM76r0jxBfCx2r_PqQp9T5=A2ALociyHOKUeMg@mail.gmail.com>

Hi Benjamin,

On Thu, 30 Aug 2018 at 00:18, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>> On Thu, Aug 30, 2018 at 1:06 AM Harry Cutts <hcutts@chromium.org> wrote:
> > > The conversion input_report_rel(... REL_WHEEL,...) to
> > > hid_scroll_counter_handle_scroll() should be dealt in a separate
> > > patch.
> >
> > OK, I'll do that in v2, but might I ask why? I don't see how this
> > particular hunk is special.
>
> I tend to consider that existing code rewrite need to be in their
> special commit, especially if the change isn't supposed to change the
> behaviour. This is my personal taste in case something goes wrong and
> (in this case) a m560 suddenly starts complaining about an issue with
> this mouse.
> I wouldn't mind too much if you rather keep the
> hid_scroll_counter_handle_scroll() introduction to this commit though.

Yes, I see the reasoning for that, but this hunk is pretty tied to the
main change in that scrolling on the M560 would be 8x too fast without
it. I'll keep it in the same one, if you don't mind.

> > [snip]
> > Yes, it seems to work fine without it (at least for the MX Master 2S).
> > Unfortunately, while testing this I encountered a bug with high-res
> > scrolling over Bluetooth. (It seems that if you turn off the MX Master
> > 2S while it's connected over Bluetooth, we don't detect that and
> > remove the input device, meaning that when it reconnects the driver
> > thinks it's in high-res mode but the mouse is in low-res.) I'm
> > investigating, but in the meantime I'll remove the Bluetooth support
> > from v2 and add it back in later.
>
> As far as I can see, the MX Master 2S is connected over BLE. Bluez
> keeps the uhid node opened (and thus the existing bluetooth HID
> device) to be able to reconnect faster.
> I would suppose you should get notified in the connect event of a
> reconnection, but it doesn't seem to be the case.
>
> Nestor, is there any event emitted by the mouse when it gets
> reconnected over BLE or is that a bluez issue?

Ah, interesting. The MX Master 2S is indeed using BLE, and testing
with another Logitech BLE mouse (the M585) also leaves the input
device around. I think this is something Logitech-specific, though, as
the Microsoft Surface Precision mouse (also BLE) does have its input
device removed when it turns off. I notice that btmon does show
"device disconnected" and "device connected" events when I turn the
M585 on and off; maybe we need to plumb those through to the driver.
We've decided to delay Bluetooth support for high-res scrolling until
the Chrome OS Bluetooth stack is more stable.

Harry Cutts
Chrome OS Touch/Input team

  reply	other threads:[~2018-08-30 20:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-23 18:30 [PATCH 0/3] Add support for high-resolution scrolling on Logitech mice Harry Cutts
2018-08-23 18:30 ` [PATCH 1/3] Add the `REL_WHEEL_HI_RES` event code Harry Cutts
2018-08-23 18:42   ` Dmitry Torokhov
2018-08-23 18:57   ` Matthew Wilcox
2018-08-23 19:01     ` Dmitry Torokhov
2018-08-28  9:02       ` Jiri Kosina
2018-08-28 17:49         ` Harry Cutts
2018-08-23 18:30 ` [PATCH 2/3] Create a utility class for counting scroll events Harry Cutts
2018-08-23 18:30 ` [PATCH 3/3] Enable high-resolution scrolling on Logitech mice Harry Cutts
2018-08-28  8:47   ` Benjamin Tissoires
     [not found]     ` <CA+jURcvguwXAVnpQ9+84QQOaJG74oeV8U4zzM1X0UabaY5DJBQ@mail.gmail.com>
2018-08-30  7:18       ` Benjamin Tissoires
2018-08-30 20:37         ` Harry Cutts [this message]
     [not found]           ` <CAE7qMrrBdZSg1P+JTr9SA5a1ui-zdAmsg6pE-B1N5La_1WwvHw@mail.gmail.com>
2018-08-31  9:14             ` Nestor Lopez Casado

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+jURcsDJbL_3aF7wE0MQ_6ZSVV3Aba-tUDX-f=1+y+LBhqmng@mail.gmail.com' \
    --to=hcutts@chromium.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nlopezcasad@logitech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.