Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <2262737.ElGaqSPkdT@kreacher> <5647715.DvuYhMxLoT@kreacher>
 <2283816.ElGaqSPkdT@kreacher> <e7eb0e0c9aea30c0e3205b2f3d96b74a52283b40.camel@hadess.net>
In-Reply-To: <e7eb0e0c9aea30c0e3205b2f3d96b74a52283b40.camel@hadess.net>
From:   Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date:   Wed, 7 Dec 2022 10:48:34 +0100
Message-ID: <CAO-hwJLoAXt_uhT7bwphw1PW=4oQsOc_G3yACg-LQrerNx3yFA@mail.gmail.com>
Subject: Re: [PATCH v1 2/2] HID: logitech-hidpp: Add Bluetooth Mouse
 M336/M337/M535 to unhandled_hidpp_devices[]
To:     Bastien Nocera <hadess@hadess.net>
Cc:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Jiri Kosina <jikos@kernel.org>,
        =?UTF-8?Q?Filipe_La=C3=ADns?= <lains@riseup.net>,
        linux-input@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Thorsten Leemhuis <regressions@leemhuis.info>,
        Nestor Lopez Casado <nlopezcasad@logitech.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Dec 7, 2022 at 10:29 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Wed, 2022-12-07 at 10:12 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Evidently, Logitech Bluetooth Mouse M336/M337/M535 (0xb016) does not
> > work when HID++ is enabled for it,
>
> This needs the output of the hidpp-list-features tool mentioned earlier
> in the thread so we can avoid words like "evidently" and provide
> concrete proof.
>
> But why is it needed in this case? We purposefully try to avoid blanket
> blocklists. The lack of HID++ can be probed, so the device should work
> just as it used to (if the fallback code works).

If I read the probe function correctly, we should have the HID++
reports present, so a static analysis will not allow us to detect that
information.

However, this reminds me of the Litra Glow[0]. On this device,
hidpp_root_get_protocol_version() also reports an error when it is
obvious it is connected.
And AFAICT, a BLE device is supposed to always be connected when it is
presented to the kernel (because disconnect is handled in the BLE
layer, in bluez).

Apparently Rafael's mouse is Bluetooth classic, so I have a doubt on
what happens when it goes into low power.

>
> We should only list devices that need special handling, and the ones
> that don't work once HID++ was probed unsuccessfully.
>

Given the current state of Rafael's mouse it goes into the second
category. But I suspect we should be smarter in the probe's decision
to consider if a device is connected or not.

Cheers,
Benjamin

[0] https://lore.kernel.org/linux-input/CABfF9mO3SQZvkQGOC09H5s7EEd2UGhpE=GYB46g_zF3aEOVn=Q@mail.gmail.com/