* Re: [PATCH 03/18] backlight: corgi_lcd: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-3-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:01PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 04/18] backlight: hx8357: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-4-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:02PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 05/18] backlight: ili922x: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-5-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:03PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 06/18] backlight: ili9320: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-6-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:04PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 07/18] backlight: jornada720_lcd: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:52 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-7-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:05PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 08/18] backlight: l4f00242t03: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:53 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-8-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:06PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 09/18] backlight: lms283gf05: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-9-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:07PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 10/18] backlight: lms501kf03: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-10-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:08PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 11/18] backlight: ltv350qv: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-11-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:09PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 12/18] backlight: otm3225a: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-12-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:10PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 13/18] backlight: platform_lcd: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-13-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:11PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 14/18] backlight: tdo24m: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-14-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:36:12PM +0200, Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 00/18] backlight: Constify lcd_ops
From: Daniel Thompson @ 2024-04-15 12:58 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-0-9b37fcbf546a@kernel.org>
On Sun, Apr 14, 2024 at 06:35:58PM +0200, Krzysztof Kozlowski wrote:
> Hi,
>
> Dependencies
> ============
> All further patches depend on the first patch. Therefore everything
> could go via backlight tree (please ack) or via cross-tree pulls. Or
> whatever maintainer choose, just coordinate this with backlight.
Thanks for the tidy up.
I've added my Reviewed-by: to all the backlight patches (for Lee) and
I'm happy with the other patches too... but I didn't want my R-b on the
HID and fbdev patches to be confused for an ack.
Daniel.
> ---
> Krzysztof Kozlowski (18):
> backlight: Constify lcd_ops
> backlight: ams369fg06: Constify lcd_ops
> backlight: corgi_lcd: Constify lcd_ops
> backlight: hx8357: Constify lcd_ops
> backlight: ili922x: Constify lcd_ops
> backlight: ili9320: Constify lcd_ops
> backlight: jornada720_lcd: Constify lcd_ops
> backlight: l4f00242t03: Constify lcd_ops
> backlight: lms283gf05: Constify lcd_ops
> backlight: lms501kf03: Constify lcd_ops
> backlight: ltv350qv: Constify lcd_ops
> backlight: otm3225a: Constify lcd_ops
> backlight: platform_lcd: Constify lcd_ops
> backlight: tdo24m: Constify lcd_ops
> HID: picoLCD: Constify lcd_ops
> fbdev: clps711x: Constify lcd_ops
> fbdev: imx: Constify lcd_ops
> fbdev: omap: lcd_ams_delta: Constify lcd_ops
>
> drivers/hid/hid-picolcd_lcd.c | 2 +-
> drivers/video/backlight/ams369fg06.c | 2 +-
> drivers/video/backlight/corgi_lcd.c | 2 +-
> drivers/video/backlight/hx8357.c | 2 +-
> drivers/video/backlight/ili922x.c | 2 +-
> drivers/video/backlight/ili9320.c | 2 +-
> drivers/video/backlight/jornada720_lcd.c | 2 +-
> drivers/video/backlight/l4f00242t03.c | 2 +-
> drivers/video/backlight/lcd.c | 4 ++--
> drivers/video/backlight/lms283gf05.c | 2 +-
> drivers/video/backlight/lms501kf03.c | 2 +-
> drivers/video/backlight/ltv350qv.c | 2 +-
> drivers/video/backlight/otm3225a.c | 2 +-
> drivers/video/backlight/platform_lcd.c | 2 +-
> drivers/video/backlight/tdo24m.c | 2 +-
> drivers/video/fbdev/clps711x-fb.c | 2 +-
> drivers/video/fbdev/imxfb.c | 2 +-
> drivers/video/fbdev/omap/lcd_ams_delta.c | 2 +-
> include/linux/lcd.h | 6 +++---
> 19 files changed, 22 insertions(+), 22 deletions(-)
> ---
> base-commit: 9ed46da14b9b9b2ad4edb3b0c545b6dbe5c00d39
> change-id: 20240414-video-backlight-lcd-ops-276d8439ffb8
>
> Best regards,
> --
> Krzysztof Kozlowski <krzk@kernel.org>
>
^ permalink raw reply
* Re: [PATCH v2] HID: i2c-hid: wait for i2c touchpad deep-sleep to power-up transition
From: Johan Hovold @ 2024-04-15 13:22 UTC (permalink / raw)
To: Kenny Levinsen
Cc: lma, benjamin.tissoires, dianders, dtor, hdegoede, jikos,
johan+linaro, kai.heng.feng, linux-input, linux-kernel, mripard,
rad
In-Reply-To: <81e1b870-37f9-4ef2-9a3d-87015f32546b@kl.wtf>
On Mon, Apr 15, 2024 at 12:28:28PM +0200, Kenny Levinsen wrote:
> > The problem is that the probe function calling i2c_smbus_read_byte()
> > is not aware that
> > uC on the other end is in a deep sleep state so the first read will
> > fail and so the whole probe.
> Well, the probe was just added to "avoid scary messages", so we could
> just do away with it and fix the "scary messages" instead.
We also use it for devices that may not be populated (e.g. an optional
touchscreen) and in that case we should not print anything.
Johan
^ permalink raw reply
* Re: [PATCH v2] HID: i2c-hid: wait for i2c touchpad deep-sleep to power-up transition
From: Johan Hovold @ 2024-04-15 13:37 UTC (permalink / raw)
To: Radoslaw Biernacki
Cc: Łukasz Majczak, Jiri Kosina, Dmitry Torokhov,
Benjamin Tissoires, Douglas Anderson, Hans de Goede,
Maxime Ripard, Kai-Heng Feng, Johan Hovold, linux-input,
linux-kernel, Radoslaw Biernacki
In-Reply-To: <CAM4=RnLQRPmYDt_c+dpsia4WFZVA1vukXa35a0Uupu7jMZxtVQ@mail.gmail.com>
On Mon, Apr 15, 2024 at 02:26:42PM +0200, Radoslaw Biernacki wrote:
> On Mon, Apr 15, 2024 at 11:08 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Thu, Apr 11, 2024 at 04:23:27PM +0200, Łukasz Majczak wrote:
> > > > Sure, but what about other transactions that are initiated by the host
> > > > (e.g. SET_POWER)?
> > > >
> > > Somehow it is problematic only on reboot and works just fine on
> > > suspend/resume and
> > > set_power.
> > > I will dig more and try to find out what the difference is.
> >
> > Sounds like it may be related to the i2c_hid_set_power() on shutdown()
> > then as Kai-Heng pointed out.
> > Yes, if this turns out to be needed then there should be a comment
> > explaining why it is there (and currently also as the delays you used
> > seem specific for your particular platform).
> >
> > But hopefully you can find a generic solution to this.
>
> Yes, we might need a more generic solution though it is not yet clear
> for us which would be the cleanest one.
> As I wrote in the reply to Kenny, the design back in the day was made
> to use events rather than
> level driven IO line, to drive the power state of the device.
> Consequence is we need to request
> a low power state before the kernel goes down as there is no guarantee
> the kernel will wake up soon
> (prevent battery power leak). This event/level logic problem (event
> design for level type problem).
Ok, and do I understand you correctly that it is indeed the SET_POWER
command on shutdown() that makes the device enter the sleep state and
which then needs an extra transaction on next boot to be woken up?
And that the firmware will not enter that low power state on its own
based on lack of activity as the current commit message suggests?
If so, then it seems a variant of this patch would be ok as long as the
commit message and a comment in the code explains the problem correctly.
Johan
^ permalink raw reply
* Re: [PATCH] Input: amimouse - Mark driver struct with __refdata to prevent section mismatch
From: Uwe Kleine-König @ 2024-04-15 14:34 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-kbuild, kernel, linux-input
In-Reply-To: <2e3783106bf6bd9a7bdeb12b706378fb16316471.1711748999.git.u.kleine-koenig@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]
Hello,
On Fri, Mar 29, 2024 at 10:54:38PM +0100, Uwe Kleine-König wrote:
> As described in the added code comment, a reference to .exit.text is ok
> for drivers registered via module_platform_driver_probe(). Make this
> explicit to prevent the following section mismatch warning
>
> WARNING: modpost: drivers/input/mouse/amimouse: section mismatch in reference: amimouse_driver+0x8 (section: .data) -> amimouse_remove (section: .exit.text)
>
> that triggers on an allmodconfig W=1 build.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
I'd like to enable these warnings even for W=0 builds, so it would be
great to get it into the main line soon.
If you apply it, please notice that I fat-fingered the parameters to git
send-email and it was sent in a thread. So (assuming you're using b4)
you'd need:
b4 am -P _ -v1 2e3783106bf6bd9a7bdeb12b706378fb16316471.1711748999.git.u.kleine-koenig@pengutronix.de
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] Logitech Anywhere 3SB support
From: Benjamin Tissoires @ 2024-04-15 15:54 UTC (permalink / raw)
To: kde
Cc: lains, hadess, jikos, benjamin.tissoires, linux-input,
linux-kernel, Hans de Goede, Allan Sandfeld Jensen
In-Reply-To: <20240413095453.14816-1-kde@carewolf.com>
[Ccing Hans as well for input]
On Apr 13 2024, kde@carewolf.com wrote:
> From: Allan Sandfeld Jensen <allan.jensen@qt.io>
>
FWIW, this patch neesd a commit description and signed-offs
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-logitech-dj.c | 10 +++++++++-
> drivers/hid/hid-logitech-hidpp.c | 2 ++
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 2235d78784b1..4b79c4578d32 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -849,6 +849,7 @@
> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1 0xc539
> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1 0xc53f
> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY 0xc53a
> +#define USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER 0xc548
> #define USB_DEVICE_ID_SPACETRAVELLER 0xc623
> #define USB_DEVICE_ID_SPACENAVIGATOR 0xc626
> #define USB_DEVICE_ID_DINOVO_DESKTOP 0xc704
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index c358778e070b..92b41ae5a47c 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -120,6 +120,7 @@ enum recvr_type {
> recvr_type_27mhz,
> recvr_type_bluetooth,
> recvr_type_dinovo,
> + recvr_type_bolt,
I am *really* hesitant in integrating the bolt receiver into
logitech-dj.ko:
- the bolt receiver is not capable of making the distinction between the
source of the events (so only one mouse/keyboard can be used at the
time)
- we still have a couple of outstanding and impossible to debug issues
with some high resolution mice connected over the unifying receiver,
and adding one more receiver makes me nervous
- I have a strong feeling by reading the code that the keyboard part
will fail (there is a comment "For the keyboard, we can reuse the same
report by using the second byte which is constant in the USB HID
report descriptor." though I can't seem to find this constant report
on the bolt receiver)
- what are the benefits of adding it?
- will it break fwupd?
> };
>
> struct dj_report {
> @@ -1068,6 +1069,7 @@ static void logi_hidpp_recv_queue_notif(struct hid_device *hdev,
> workitem.reports_supported |= STD_KEYBOARD;
> break;
> case 0x0f:
> + case 0x10:
> case 0x11:
> device_type = "eQUAD Lightspeed 1.2";
> logi_hidpp_dev_conn_notif_equad(hdev, hidpp_report, &workitem);
> @@ -1430,7 +1432,8 @@ static int logi_dj_ll_parse(struct hid_device *hid)
> dbg_hid("%s: sending a mouse descriptor, reports_supported: %llx\n",
> __func__, djdev->reports_supported);
> if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp ||
> - djdev->dj_receiver_dev->type == recvr_type_mouse_only)
> + djdev->dj_receiver_dev->type == recvr_type_mouse_only ||
> + djdev->dj_receiver_dev->type == recvr_type_bolt)
> rdcat(rdesc, &rsize, mse_high_res_descriptor,
> sizeof(mse_high_res_descriptor));
> else if (djdev->dj_receiver_dev->type == recvr_type_27mhz)
> @@ -1773,6 +1776,7 @@ static int logi_dj_probe(struct hid_device *hdev,
> case recvr_type_dj: no_dj_interfaces = 3; break;
> case recvr_type_hidpp: no_dj_interfaces = 2; break;
> case recvr_type_gaming_hidpp: no_dj_interfaces = 3; break;
> + case recvr_type_bolt: no_dj_interfaces = 4; break;
4? The device I have here only has 3 (unless I misremember how this is
supposed to be working).
> case recvr_type_mouse_only: no_dj_interfaces = 2; break;
> case recvr_type_27mhz: no_dj_interfaces = 2; break;
> case recvr_type_bluetooth: no_dj_interfaces = 2; break;
> @@ -1950,6 +1954,10 @@ static const struct hid_device_id logi_dj_receivers[] = {
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER_2),
> .driver_data = recvr_type_dj},
> + { /* Logitech bolt receiver (0xc548) */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> + USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER),
> + .driver_data = recvr_type_bolt},
>
> { /* Logitech Nano mouse only receiver (0xc52f) */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 3c00e6ac8e76..509142982daa 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4380,6 +4380,8 @@ static const struct hid_device_id hidpp_devices[] = {
> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb023) },
> { /* MX Master 3S mouse over Bluetooth */
> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb034) },
> + { /* MX Anywhere 3SB mouse over Bluetooth */
> + HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb038) },
That I can accept, however know that there is a regression in bluez
0.73[0] (but it should be fixed in 0.74)
Cheers,
Benjamin
[0] https://github.com/bluez/bluez/issues/778#issuecomment-2048870358
> {}
> };
>
> --
> 2.39.2
>
^ permalink raw reply
* Re: [PATCH] Logitech Anywhere 3SB support
From: Allan Sandfeld Jensen @ 2024-04-15 16:36 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: lains, hadess, jikos, benjamin.tissoires, linux-input,
linux-kernel, Hans de Goede
In-Reply-To: <ntsifcsfo5i6xisxbgfjdpe4uenygqxrt3v5sceflgipznw6cb@gnhvkjmglrtg>
On Monday 15 April 2024 17:54:57 CEST Benjamin Tissoires wrote:
> [You don't often get email from bentiss@kernel.org. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
>
> [Ccing Hans as well for input]
>
> On Apr 13 2024, kde@carewolf.com wrote:
> > From: Allan Sandfeld Jensen <allan.jensen@qt.io>
>
> FWIW, this patch neesd a commit description and signed-offs
>
> > ---
> >
> > drivers/hid/hid-ids.h | 1 +
> > drivers/hid/hid-logitech-dj.c | 10 +++++++++-
> > drivers/hid/hid-logitech-hidpp.c | 2 ++
> > 3 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index 2235d78784b1..4b79c4578d32 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -849,6 +849,7 @@
> >
> > #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1 0xc539
> > #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1 0xc53f
> > #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY 0xc53a
> >
> > +#define USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER 0xc548
> >
> > #define USB_DEVICE_ID_SPACETRAVELLER 0xc623
> > #define USB_DEVICE_ID_SPACENAVIGATOR 0xc626
> > #define USB_DEVICE_ID_DINOVO_DESKTOP 0xc704
> >
> > diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> > index c358778e070b..92b41ae5a47c 100644
> > --- a/drivers/hid/hid-logitech-dj.c
> > +++ b/drivers/hid/hid-logitech-dj.c
> > @@ -120,6 +120,7 @@ enum recvr_type {
> >
> > recvr_type_27mhz,
> > recvr_type_bluetooth,
> > recvr_type_dinovo,
> >
> > + recvr_type_bolt,
>
> I am *really* hesitant in integrating the bolt receiver into
> logitech-dj.ko:
> - the bolt receiver is not capable of making the distinction between the
> source of the events (so only one mouse/keyboard can be used at the
> time)
> - we still have a couple of outstanding and impossible to debug issues
> with some high resolution mice connected over the unifying receiver,
> and adding one more receiver makes me nervous
> - I have a strong feeling by reading the code that the keyboard part
> will fail (there is a comment "For the keyboard, we can reuse the same
> report by using the second byte which is constant in the USB HID
> report descriptor." though I can't seem to find this constant report
> on the bolt receiver)
> - what are the benefits of adding it?
> - will it break fwupd?
>
I added it to get hires scroll wheel events working out of the box.
The main differences for the bolt receiver as I briefly skimmed it, are
different peering commands, which I didn't want to touch that.
For my purpose I discovered the bolt receiver operated much like the gaming-
hidpp receiver, except that I have four interfaces.
> > };
> >
> > struct dj_report {
> >
> > @@ -1068,6 +1069,7 @@ static void logi_hidpp_recv_queue_notif(struct
> > hid_device *hdev,>
> > workitem.reports_supported |= STD_KEYBOARD;
> > break;
> >
> > case 0x0f:
> > + case 0x10:
> > case 0x11:
> > device_type = "eQUAD Lightspeed 1.2";
> > logi_hidpp_dev_conn_notif_equad(hdev, hidpp_report,
> > &workitem);
> >
> > @@ -1430,7 +1432,8 @@ static int logi_dj_ll_parse(struct hid_device *hid)
> >
> > dbg_hid("%s: sending a mouse descriptor, reports_supported:
> > %llx\n",
> >
> > __func__, djdev->reports_supported);
> >
> > if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp
> > ||
> >
> > - djdev->dj_receiver_dev->type == recvr_type_mouse_only)
> > + djdev->dj_receiver_dev->type == recvr_type_mouse_only ||
> > + djdev->dj_receiver_dev->type == recvr_type_bolt)
> >
> > rdcat(rdesc, &rsize, mse_high_res_descriptor,
> >
> > sizeof(mse_high_res_descriptor));
> >
> > else if (djdev->dj_receiver_dev->type == recvr_type_27mhz)
> >
> > @@ -1773,6 +1776,7 @@ static int logi_dj_probe(struct hid_device *hdev,
> >
> > case recvr_type_dj: no_dj_interfaces = 3; break;
> > case recvr_type_hidpp: no_dj_interfaces = 2; break;
> > case recvr_type_gaming_hidpp: no_dj_interfaces = 3; break;
> >
> > + case recvr_type_bolt: no_dj_interfaces = 4; break;
>
> 4? The device I have here only has 3 (unless I misremember how this is
> supposed to be working).
>
I am getting four. The fourth one is the one with 0x10 case I added above.
[ 5.706399] logitech-djreceiver 0003:046D:C548.0003: device of type eQUAD
Lightspeed 1.2 (0x10) connected on slot 2
You can leave out the added 0x10 case, and just treat the bolt receiver as a
gaming_hidpp receiver I assume. I got an error there about unknown device,
when I originaly tried just using the gaming_hidpp type, but it is possible it
could still work (I hit another bug when I originally tried that) . I can go
back and check if you would prefer this minimalist solution?
I dont have any additional bolt capable devices, so I can't test how it would
work if I peered more devices.
Best regards
Allan
^ permalink raw reply
* [PATCH 0/3] HID: i2c-hid: Probe and wake device with HID descriptor fetch
From: Kenny Levinsen @ 2024-04-15 17:04 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires,
Douglas Anderson, Hans de Goede, Maxime Ripard, Kai-Heng Feng,
Johan Hovold, linux-input, linux-kernel, Radoslaw Biernacki,
Lukasz Majczak
This is in response to
https://lore.kernel.org/all/Zh0qKeI-YPDE-NVT@hovoldconsulting.com/
Instead of extending the existing smbus probe to include a retry loop,
this patch takes the same approach as i2c_hid_set_power() by retrying on
EREMOTEIO.
This maintains the "silent" error in case no device is present without
having to send any dummy commands. Tested with a disconnected touchpad
on a Dell XPS 13.
I left out the particular sleep. If one is needed, it should also be
added to i2c_hid_set_power() which is where we'd wake the device after
resuming from suspend.
Lukasz and Radoslaw, can you please test if this still does the trick?
^ permalink raw reply
* [PATCH 1/3] HID: i2c-hid: Rely on HID descriptor fetch to probe
From: Kenny Levinsen @ 2024-04-15 17:04 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires,
Douglas Anderson, Hans de Goede, Maxime Ripard, Kai-Heng Feng,
Johan Hovold, linux-input, linux-kernel, Radoslaw Biernacki,
Lukasz Majczak
Cc: Kenny Levinsen
In-Reply-To: <20240415170517.18780-1-kl@kl.wtf>
To avoid error messages when a device is not present, b3a81b6c4fc6 added
an initial bus probe using a dummy i2c_smbus_read_byte() call.
Without this probe, i2c_hid_fetch_hid_descriptor() will fail with
EREMOTEIO. Propagate the error up so the caller can handle EREMOTEIO
gracefully, and remove the probe as it is no longer necessary.
Signed-off-by: Kenny Levinsen <kl@kl.wtf>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 2df1ab3c31cc..515a80dbf6c7 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -894,12 +894,8 @@ static int i2c_hid_fetch_hid_descriptor(struct i2c_hid *ihid)
ihid->wHIDDescRegister,
&ihid->hdesc,
sizeof(ihid->hdesc));
- if (error) {
- dev_err(&ihid->client->dev,
- "failed to fetch HID descriptor: %d\n",
- error);
- return -ENODEV;
- }
+ if (error)
+ return error;
}
/* Validate the length of HID descriptor, the 4 first bytes:
@@ -1014,17 +1010,13 @@ static int __i2c_hid_core_probe(struct i2c_hid *ihid)
struct hid_device *hid = ihid->hid;
int ret;
- /* Make sure there is something at this address */
- ret = i2c_smbus_read_byte(client);
- if (ret < 0) {
+ ret = i2c_hid_fetch_hid_descriptor(ihid);
+ if (ret == -EREMOTEIO) {
i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
return -ENXIO;
- }
-
- ret = i2c_hid_fetch_hid_descriptor(ihid);
- if (ret < 0) {
+ } else if (ret < 0) {
dev_err(&client->dev,
- "Failed to fetch the HID Descriptor\n");
+ "failed to fetch HID descriptor: %d\n", ret);
return ret;
}
--
2.44.0
^ permalink raw reply related
* [PATCH 2/3] HID: i2c-hid: Retry HID descriptor read to wake up STM devices
From: Kenny Levinsen @ 2024-04-15 17:04 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires,
Douglas Anderson, Hans de Goede, Maxime Ripard, Kai-Heng Feng,
Johan Hovold, linux-input, linux-kernel, Radoslaw Biernacki,
Lukasz Majczak
Cc: Kenny Levinsen
In-Reply-To: <20240415170517.18780-1-kl@kl.wtf>
Some STM microcontrollers need some time after rising clock edge in
order to come out of their deep sleep state. This in turn means that the
first command send to them timeout and fail with EREMOTEIO.
Retry once on EREMOTEIO to see if the device came alive, otherwise treat
the error as if no device was present like before.
Link: https://lore.kernel.org/all/20240405102436.3479210-1-lma@chromium.org/#t
Signed-off-by: Kenny Levinsen <kl@kl.wtf>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 515a80dbf6c7..ac661199d2c8 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1010,7 +1010,15 @@ static int __i2c_hid_core_probe(struct i2c_hid *ihid)
struct hid_device *hid = ihid->hid;
int ret;
+ /*
+ * Some STM-based devices need some time after a rising clock edge to
+ * wake from deep sleep, which in turn means that our first command
+ * will fail EREMOTEIO. Retry the command in this case.
+ */
ret = i2c_hid_fetch_hid_descriptor(ihid);
+ if (ret == -EREMOTEIO)
+ ret = i2c_hid_fetch_hid_descriptor(ihid);
+
if (ret == -EREMOTEIO) {
i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
return -ENXIO;
--
2.44.0
^ permalink raw reply related
* [PATCH 3/3] HID: i2c-hid: Align i2c_hid_set_power() retry with HID descriptor read
From: Kenny Levinsen @ 2024-04-15 17:04 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires,
Douglas Anderson, Hans de Goede, Maxime Ripard, Kai-Heng Feng,
Johan Hovold, linux-input, linux-kernel, Radoslaw Biernacki,
Lukasz Majczak
Cc: Kenny Levinsen
In-Reply-To: <20240415170517.18780-1-kl@kl.wtf>
Instead of retrying on any error, just retry on EREMOTEIO and simplify
the retry handling a bit.
Signed-off-by: Kenny Levinsen <kl@kl.wtf>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index ac661199d2c8..998e7aa140d7 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -395,21 +395,14 @@ static int i2c_hid_set_power(struct i2c_hid *ihid, int power_state)
* The call will get a return value (EREMOTEIO) but device will be
* triggered and activated. After that, it goes like a normal device.
*/
- if (power_state == I2C_HID_PWR_ON) {
+ ret = i2c_hid_set_power_command(ihid, power_state);
+ if (ret == -EREMOTEIO && power_state == I2C_HID_PWR_ON)
ret = i2c_hid_set_power_command(ihid, I2C_HID_PWR_ON);
- /* Device was already activated */
- if (!ret)
- goto set_pwr_exit;
- }
-
- ret = i2c_hid_set_power_command(ihid, power_state);
if (ret)
dev_err(&ihid->client->dev,
"failed to change power setting.\n");
-set_pwr_exit:
-
/*
* The HID over I2C specification states that if a DEVICE needs time
* after the PWR_ON request, it should utilise CLOCK stretching.
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v2] HID: i2c-hid: wait for i2c touchpad deep-sleep to power-up transition
From: Kenny Levinsen @ 2024-04-15 17:14 UTC (permalink / raw)
To: Johan Hovold
Cc: lma, benjamin.tissoires, dianders, dtor, hdegoede, jikos,
johan+linaro, kai.heng.feng, linux-input, linux-kernel, mripard,
rad
In-Reply-To: <Zh0qKeI-YPDE-NVT@hovoldconsulting.com>
On 4/15/24 3:22 PM, Johan Hovold wrote:
> We also use it for devices that may not be populated (e.g. an optional
> touchscreen) and in that case we should not print anything.
I sent a patch series[0] that shows how the same can be achieved
(gracefully handling missing device + retry to wake device) without any
smbus probes, following the existing retry style in i2c_hid_set_power().
Radoslaw and Lukasz, it somehow felt rude to tag and blame you for code
you hadn't seen yet. If my patch ends up favored we should make sure to
have the appropriate attribution tags added...
[0]: https://lore.kernel.org/all/20240415170517.18780-1-kl@kl.wtf/
^ permalink raw reply
* Re: [PATCH] Logitech Anywhere 3SB support
From: Hans de Goede @ 2024-04-15 18:31 UTC (permalink / raw)
To: Benjamin Tissoires, kde
Cc: lains, hadess, jikos, benjamin.tissoires, linux-input,
linux-kernel, Allan Sandfeld Jensen
In-Reply-To: <ntsifcsfo5i6xisxbgfjdpe4uenygqxrt3v5sceflgipznw6cb@gnhvkjmglrtg>
Hi,
On 4/15/24 5:54 PM, Benjamin Tissoires wrote:
> [Ccing Hans as well for input]
>
> On Apr 13 2024, kde@carewolf.com wrote:
>> From: Allan Sandfeld Jensen <allan.jensen@qt.io>
>>
>
> FWIW, this patch neesd a commit description and signed-offs
>
>> ---
>> drivers/hid/hid-ids.h | 1 +
>> drivers/hid/hid-logitech-dj.c | 10 +++++++++-
>> drivers/hid/hid-logitech-hidpp.c | 2 ++
>> 3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> index 2235d78784b1..4b79c4578d32 100644
>> --- a/drivers/hid/hid-ids.h
>> +++ b/drivers/hid/hid-ids.h
>> @@ -849,6 +849,7 @@
>> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1 0xc539
>> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1 0xc53f
>> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY 0xc53a
>> +#define USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER 0xc548
>> #define USB_DEVICE_ID_SPACETRAVELLER 0xc623
>> #define USB_DEVICE_ID_SPACENAVIGATOR 0xc626
>> #define USB_DEVICE_ID_DINOVO_DESKTOP 0xc704
>> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
>> index c358778e070b..92b41ae5a47c 100644
>> --- a/drivers/hid/hid-logitech-dj.c
>> +++ b/drivers/hid/hid-logitech-dj.c
>> @@ -120,6 +120,7 @@ enum recvr_type {
>> recvr_type_27mhz,
>> recvr_type_bluetooth,
>> recvr_type_dinovo,
>> + recvr_type_bolt,
>
> I am *really* hesitant in integrating the bolt receiver into
> logitech-dj.ko:
> - the bolt receiver is not capable of making the distinction between the
> source of the events (so only one mouse/keyboard can be used at the
> time)
> - we still have a couple of outstanding and impossible to debug issues
> with some high resolution mice connected over the unifying receiver,
> and adding one more receiver makes me nervous
> - I have a strong feeling by reading the code that the keyboard part
> will fail (there is a comment "For the keyboard, we can reuse the same
> report by using the second byte which is constant in the USB HID
> report descriptor." though I can't seem to find this constant report
> on the bolt receiver)
> - what are the benefits of adding it?
> - will it break fwupd?
FWIW I'm also not in favor of stretching drivers/hid/hid-logitech-dj.c
even further to also support the new bolt stuff.
AFAIK the new bolt stuff is significantly different.
Allan, I see in your other reply that you are mainly after
highres scrolling and since the bolt receiver does not do
per paired device addressing I wonder if you cannot just
get that by treating the bolt receiver as a wired HIDPP
device and just directly listing it as such in
hid-logitech-hidpp.c ?
The whole purpose of hid-logitech-dj.c is to create 1 virtual
hidpp devices per paired device and with bolt that is not
possible, so I think that we should circumvent hid-logitech-dj.c
for bolt and if we want to use any hidpp features do so
by directly listing the receivers in hid-logitech-hidpp.c .
Regards,
Hans
>
>> };
>>
>> struct dj_report {
>> @@ -1068,6 +1069,7 @@ static void logi_hidpp_recv_queue_notif(struct hid_device *hdev,
>> workitem.reports_supported |= STD_KEYBOARD;
>> break;
>> case 0x0f:
>> + case 0x10:
>> case 0x11:
>> device_type = "eQUAD Lightspeed 1.2";
>> logi_hidpp_dev_conn_notif_equad(hdev, hidpp_report, &workitem);
>> @@ -1430,7 +1432,8 @@ static int logi_dj_ll_parse(struct hid_device *hid)
>> dbg_hid("%s: sending a mouse descriptor, reports_supported: %llx\n",
>> __func__, djdev->reports_supported);
>> if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp ||
>> - djdev->dj_receiver_dev->type == recvr_type_mouse_only)
>> + djdev->dj_receiver_dev->type == recvr_type_mouse_only ||
>> + djdev->dj_receiver_dev->type == recvr_type_bolt)
>> rdcat(rdesc, &rsize, mse_high_res_descriptor,
>> sizeof(mse_high_res_descriptor));
>> else if (djdev->dj_receiver_dev->type == recvr_type_27mhz)
>> @@ -1773,6 +1776,7 @@ static int logi_dj_probe(struct hid_device *hdev,
>> case recvr_type_dj: no_dj_interfaces = 3; break;
>> case recvr_type_hidpp: no_dj_interfaces = 2; break;
>> case recvr_type_gaming_hidpp: no_dj_interfaces = 3; break;
>> + case recvr_type_bolt: no_dj_interfaces = 4; break;
>
> 4? The device I have here only has 3 (unless I misremember how this is
> supposed to be working).
>
>> case recvr_type_mouse_only: no_dj_interfaces = 2; break;
>> case recvr_type_27mhz: no_dj_interfaces = 2; break;
>> case recvr_type_bluetooth: no_dj_interfaces = 2; break;
>> @@ -1950,6 +1954,10 @@ static const struct hid_device_id logi_dj_receivers[] = {
>> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>> USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER_2),
>> .driver_data = recvr_type_dj},
>> + { /* Logitech bolt receiver (0xc548) */
>> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>> + USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER),
>> + .driver_data = recvr_type_bolt},
>>
>> { /* Logitech Nano mouse only receiver (0xc52f) */
>> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
>> index 3c00e6ac8e76..509142982daa 100644
>> --- a/drivers/hid/hid-logitech-hidpp.c
>> +++ b/drivers/hid/hid-logitech-hidpp.c
>> @@ -4380,6 +4380,8 @@ static const struct hid_device_id hidpp_devices[] = {
>> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb023) },
>> { /* MX Master 3S mouse over Bluetooth */
>> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb034) },
>> + { /* MX Anywhere 3SB mouse over Bluetooth */
>> + HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb038) },
>
> That I can accept, however know that there is a regression in bluez
> 0.73[0] (but it should be fixed in 0.74)
>
> Cheers,
> Benjamin
>
>
> [0] https://github.com/bluez/bluez/issues/778#issuecomment-2048870358
>
>> {}
>> };
>>
>> --
>> 2.39.2
>>
>
^ permalink raw reply
* Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes
From: Dmitry Torokhov @ 2024-04-15 19:32 UTC (permalink / raw)
To: Peter Hutterer
Cc: Mark Pearson, hdegoede, ilpo.jarvinen, hmh, ibm-acpi-devel,
platform-driver-x86, linux-input, linux-kernel, njoshi1, vsankar
In-Reply-To: <5fe3b171-afc0-47de-802c-28470ce40674@redhat.com>
On Wed, Apr 10, 2024 at 02:32:56PM +1000, Peter Hutterer wrote:
> On 10/04/2024 11:20, Dmitry Torokhov wrote:
> > On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
> > > On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> > > > On 09/04/2024 09:31, Dmitry Torokhov wrote:
> > > > > Hi Mark,
> > > > >
> > > > > On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> > > > > > Add support for new input events on Lenovo laptops that need exporting to
> > > > > > user space.
> > > > > >
> > > > > > Lenovo trackpoints are adding the ability to generate a doubletap event.
> > > > > > Add a new keycode to allow this to be used by userspace.
> > > > >
> > > > > What is the intended meaning of this keycode? How does it differ from
> > > > > the driver sending BTN_LEFT press/release twice?
> > > > > >
> > > > > > Lenovo support is using FN+N with Windows to collect needed details for
> > > > > > support cases. Add a keycode so that we'll be able to provide similar
> > > > > > support on Linux.
> > > > >
> > > > > Is there a userspace consumer for this?
> > > >
> > > > Funnily enough XKB has had a keysym for this for decades but it's not
> > > > hooked up anywhere due to the way it's pointer keys accessibility
> > > > feature was implemented. Theory is that most of userspace just needs
> > > > to patch the various pieces together for the new evdev code + keysym,
> > > > it's not really any different to handling a volume key (except this
> > > > one needs to be assignable).
> > >
> > > What is the keysym? If we can make them relatable to each other that
> > > would be good. Or maybe we could find a matching usage from HID usage
> > > tables...
>
> There's a set of XK_Pointer_ keysyms defined in X11/keysym.h,
> including XK_Pointer_DblClick1 and XK_Pointer_DblClickDefault.
> Unfortunately they're not hooked up to anything atm, see this draft
> MR:
> https://gitlab.freedesktop.org/xkeyboard-config/xkeyboard-config/-/merge_requests/659
> Because they're not hooked up anywhere they'll also need to be hooked
> into the client space, same as the various XF86FooBar symbols we've
> added over the years.
>
> If we were to add KEY_DOUBLECLICK we can patch xkeyboard-config to
> bind that to the existing XK_Pointer_DblClickDefault symbol (it would
> get XF86DoubleClick assigned by the current automated scripts), so in
> theory that key would work like any other key with that symbol
> assigned.
>
> > I was looking through the existing codes and I see:
> >
> > #define KEY_INFO 0x166 /* AL OEM Features/Tips/Tutorial */
> >
> > We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> > thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> > specific debug info collection application (which I honestly doubt will
> > materialize).
>
> fwiw, I suggested KEY_DOUBLECLICK because that is the action the user
> takes. Whether that starts a particular application is mostly a
> question of configuration, defaulting to something that emulates a
> double-click seems prudent though. And if someone wants to remap that
> to the compose key that'd be trivial too then.
I think whether to create and use KEY_DOUBLECLICK is very much depends
if we associate this with the pointer somehow, or if we keep it as a
completely separate action.
If we continue with KEY_DOUBLECLICK then we need to try and define what
exactly it means to the applications. Actually same goes if we want
another new keycode.
As far as easy remapping, I think one can map this to KEY_RESERVED and
then remap to whatever they want, you do not need to have a new keycode
for that.
Thanks.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox