The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: "Erik Håkansson" <erikhakan@gmail.com>
To: Bastien Nocera <hadess@hadess.net>, jikos@kernel.org, bentiss@kernel.org
Cc: lains@riseup.net, k8ie@mcld.eu, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/1] HID: logitech: add Bolt receiver support for Logitech HID++ devices
Date: Sun, 2 Aug 2026 01:18:21 +0200	[thread overview]
Message-ID: <bce18a69-a7ce-4aa3-b631-59613c8ce114@gmail.com> (raw)
In-Reply-To: <87314b89c73bd06c9824da73811a53b3f88a5059.camel@hadess.net>

Hi!

Thanks for the review!
I've looked into the issues a bit.

On 7/30/26 10:36, Bastien Nocera wrote:
> Hey Eric,
>
> On Mon, 2026-07-13 at 22:12 +0200, Erik Håkansson wrote:
>> Add Logitech Bolt receiver support to the Logitech HID receiver and
>> HID++ drivers.
>>
>> Handle Bolt receiver notifications in hid-logitech-dj and add a
>> Bolt-specific initialization path in hid-logitech-hidpp, separate from
>> the existing Unifying receiver path.
>>
>> This allows Bolt-connected HID++ devices to expose battery information
>> through the kernel power_supply path, so userspace tools can report
>> their battery status with the correct device model.
>>
>> Tested with:
>> - Logitech MX Keys for Business via Bolt receiver
> I tested this with a Bolt receiver connected to the same M650 mouse I
> used in another test earlier, and my Slim Solar+ keyboard.
>
> Every time the mouse is turned off, I get:
> kernel: logitech-hidpp-device 0003:046D:B02A.0009: hidpp_root_get_protocol_version: received protocol error 0x04
> which probably shouldn't happen.

I managed to reproduce this and have a fix for it locally. It turns out
Bolt sends 0x04 (HIDPP_ERROR_CONNECT_FAIL) whereas Lightspeed (the only
other device I have to test with) send out 0x09
(HIDPP_ERROR_RESOURCE_ERROR) on device disconnect. So the fix is simply
to support 0x04 as well.

>
> I also see the battery not going away, but that looks to be a separate
> problem.
>
> The only thing I haven't tested, and which might need fixing before we
> can merge this is figuring out how quirks can be applied.
>
> The battery reporting works because it probes the available interfaces,
> and reports that.
>
> But what about things like:
>          { /* Signature M650 over Bluetooth */
>            HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
>                                 HIDPP_PRODUCT_SIGNATURE_M650),
>            .driver_data = HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS },
> which were added in:
> https://patchwork.kernel.org/project/linux-input/list/?series=1121633
>
> The recent "HID: logitech-hidpp: Add support for HID++ Multi-Platform
> feature (0x4531)" doesn't work either:
> https://patchwork.kernel.org/project/linux-input/list/?series=1119501
>
> Cheers

Regarding both the quirks and discrepancy with the bluetooth route, as
well as the Multi-platform feature, I'm still looking into that and
will hopefully have a patch ready in a few days.

Regards,
Erik

>
>> Reported-by: Kateřina Medvědová <k8ie@mcld.eu>
>> Link: https://lore.kernel.org/linux-input/345f7347-30a8-4568-a7a6-c70f54a52a8d@mcld.eu/
>> Signed-off-by: Erik Håkansson <erikhakan@gmail.com>
>> ---
>> Changes in v2:
>> - Handle only the Bolt receiver/control interface in hid-logitech-dj and
>>    leave the other Bolt receiver interfaces to generic HID handling.
>> - Handle Bolt HID++ unpair notifications so child HID devices are removed
>>    after unpairing.
>>
>>   drivers/hid/hid-logitech-dj.c    | 48 +++++++++++++++++++++++++++++---
>>   drivers/hid/hid-logitech-hidpp.c | 48 ++++++++++++++++++++++++++++++--
>>   2 files changed, 89 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
>> index 9c574ab8b60b..571d5caa5bb5 100644
>> --- a/drivers/hid/hid-logitech-dj.c
>> +++ b/drivers/hid/hid-logitech-dj.c
>> @@ -121,6 +121,7 @@ enum recvr_type {
>>   	recvr_type_27mhz,
>>   	recvr_type_bluetooth,
>>   	recvr_type_dinovo,
>> +	recvr_type_bolt,
>>   };
>>   
>>   struct dj_report {
>> @@ -1156,6 +1157,10 @@ static void logi_hidpp_recv_queue_notif(struct hid_device *hdev,
>>   		logi_hidpp_dev_conn_notif_equad(hdev, hidpp_report, &workitem);
>>   		workitem.reports_supported |= STD_KEYBOARD;
>>   		break;
>> +	case 0x10:
>> +		device_type = "Bolt";
>> +		logi_hidpp_dev_conn_notif_equad(hdev, hidpp_report, &workitem);
>> +		break;
>>   	}
>>   
>>   	/* custom receiver device (eg. powerplay) */
>> @@ -1745,6 +1750,24 @@ static int logi_dj_hidpp_event(struct hid_device *hdev,
>>   
>>   	dj_dev = djrcv_dev->paired_dj_devices[device_index];
>>   
>> +	/*
>> +	 * Bolt receivers send explicit unpair notifications as HID++ events;
>> +	 * queue device removal when we receive one.
>> +	 */
>> +	if (djrcv_dev->type == recvr_type_bolt &&
>> +	    hidpp_report->report_id == REPORT_ID_HIDPP_SHORT &&
>> +	    hidpp_report->sub_id == REPORT_TYPE_NOTIF_DEVICE_UNPAIRED) {
>> +		struct dj_workitem workitem = {
>> +			.device_index = device_index,
>> +			.type = WORKITEM_TYPE_UNPAIRED,
>> +		};
>> +
>> +		kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem));
>> +		schedule_work(&djrcv_dev->work);
>> +		spin_unlock_irqrestore(&djrcv_dev->lock, flags);
>> +		return false;
>> +	}
>> +
>>   	/*
>>   	 * With 27 MHz receivers, we do not get an explicit unpair event,
>>   	 * remove the old device if the user has paired a *different* device.
>> @@ -1884,6 +1907,9 @@ static int logi_dj_probe(struct hid_device *hdev,
>>   	 * treat these as logitech-dj interfaces then this causes input events
>>   	 * reported through this extra interface to not be reported correctly.
>>   	 * To avoid this, we treat these as generic-hid devices.
>> +	 *
>> +	 * Bolt receivers only use LOGITECH_DJ_INTERFACE_NUMBER for receiver
>> +	 * reporting. Treat all other Bolt interfaces as generic-hid devices.
>>   	 */
>>   	switch (id->driver_data) {
>>   	case recvr_type_dj:		no_dj_interfaces = 3; break;
>> @@ -1897,10 +1923,20 @@ static int logi_dj_probe(struct hid_device *hdev,
>>   	}
>>   	if (hid_is_usb(hdev)) {
>>   		intf = to_usb_interface(hdev->dev.parent);
>> -		if (intf && intf->altsetting->desc.bInterfaceNumber >=
>> -							no_dj_interfaces) {
>> -			hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
>> -			return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>> +		if (intf) {
>> +			bool generic_hid_interface;
>> +
>> +			if (id->driver_data == recvr_type_bolt)
>> +				generic_hid_interface =
>> +					intf->altsetting->desc.bInterfaceNumber !=
>> +					LOGITECH_DJ_INTERFACE_NUMBER;
>> +			else
>> +				generic_hid_interface =
>> +					intf->altsetting->desc.bInterfaceNumber >= no_dj_interfaces;
>> +			if (generic_hid_interface) {
>> +				hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
>> +				return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>> +			}
>>   		}
>>   	}
>>   
>> @@ -2103,6 +2139,10 @@ static const struct hid_device_id logi_dj_receivers[] = {
>>   	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>>   		USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_3),
>>   	 .driver_data = recvr_type_gaming_hidpp_ls_1_3},
>> +	{ /* Logitech Bolt receiver (0xc548) */
>> +	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>> +			 USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER),
>> +	 .driver_data = recvr_type_bolt},
>>   	{ /* Logitech lightspeed receiver (0xc54d) */
>>   	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>>   		USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_4),
>> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
>> index 90b0184df777..0b5d0a322ae8 100644
>> --- a/drivers/hid/hid-logitech-hidpp.c
>> +++ b/drivers/hid/hid-logitech-hidpp.c
>> @@ -4161,8 +4161,50 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
>>   	return ret;
>>   }
>>   
>> +static bool hidpp_is_bolt_child(struct hid_device *hdev)
>> +{
>> +	struct device *parent = hdev->dev.parent;
>> +	struct hid_device *receiver_hdev;
>> +
>> +	if (!parent)
>> +		return false;
>> +
>> +	receiver_hdev = to_hid_device(parent);
>> +	return receiver_hdev->vendor == USB_VENDOR_ID_LOGITECH &&
>> +	       receiver_hdev->product == USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER;
>> +}
>> +
>> +static int hidpp_bolt_init(struct hidpp_device *hidpp)
>> +{
>> +	struct hid_device *hdev = hidpp->hid_dev;
>> +	char *name;
>> +	int ret;
>> +
>> +	ret = hidpp_serial_init(hidpp);
>> +	if (ret)
>> +		return ret;
>> +
>> +	name = hidpp_get_device_name(hidpp);
>> +	if (!name)
>> +		return -EIO;
>> +
>> +	snprintf(hdev->name, sizeof(hdev->name), "%s", name);
>> +	dbg_hid("HID++ Bolt: Got name: %s\n", name);
>> +
>> +	kfree(name);
>> +	return 0;
>> +}
>> +
>> +static int hidpp_receiver_init(struct hidpp_device *hidpp)
>> +{
>> +	if (hidpp_is_bolt_child(hidpp->hid_dev))
>> +		return hidpp_bolt_init(hidpp);
>> +
>> +	return hidpp_unifying_init(hidpp);
>> +}
>> +
>>   /* Get name + serial for USB and Bluetooth HID++ devices */
>> -static void hidpp_non_unifying_init(struct hidpp_device *hidpp)
>> +static void hidpp_non_receiver_init(struct hidpp_device *hidpp)
>>   {
>>   	struct hid_device *hdev = hidpp->hid_dev;
>>   	char *name;
>> @@ -4510,9 +4552,9 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>   
>>   	/* Get name + serial, store in hdev->name + hdev->uniq */
>>   	if (id->group == HID_GROUP_LOGITECH_DJ_DEVICE)
>> -		hidpp_unifying_init(hidpp);
>> +		hidpp_receiver_init(hidpp);
>>   	else
>> -		hidpp_non_unifying_init(hidpp);
>> +		hidpp_non_receiver_init(hidpp);
>>   
>>   	if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
>>   		connect_mask &= ~HID_CONNECT_HIDINPUT;

      reply	other threads:[~2026-08-01 23:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-12  0:30 [PATCH 0/1] HID: logitech: add Bolt receiver support for Logitech HID++ devices Erik Håkansson
2026-07-12  0:30 ` [PATCH 1/1] " Erik Håkansson
2026-07-12 18:03   ` Kateřina Medvědová
2026-07-13 20:10     ` Erik Håkansson
2026-07-13 20:12     ` [PATCH v2 0/1] " Erik Håkansson
2026-07-13 20:12       ` [PATCH v2 1/1] " Erik Håkansson
2026-07-13 21:09         ` Kateřina Medvědová
2026-07-14  0:54           ` Erik Håkansson
2026-07-14 10:32             ` Kateřina Medvědová
2026-07-14 15:33               ` Erik Håkansson
2026-07-30  8:38               ` Bastien Nocera
2026-08-01 23:25                 ` Erik Håkansson
2026-07-30  8:36         ` Bastien Nocera
2026-08-01 23:18           ` Erik Håkansson [this message]

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=bce18a69-a7ce-4aa3-b631-59613c8ce114@gmail.com \
    --to=erikhakan@gmail.com \
    --cc=bentiss@kernel.org \
    --cc=hadess@hadess.net \
    --cc=jikos@kernel.org \
    --cc=k8ie@mcld.eu \
    --cc=lains@riseup.net \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox