* Re: [PATCH v3] HID: lg-g15 - Add support for Logitech G13.
From: Hans de Goede @ 2025-09-02 9:07 UTC (permalink / raw)
To: Leo L. Schwab
Cc: Kate Hsuan, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <aLSntMknSv3lMarZ@ewhac.org>
Hi Leo,
On 31-Aug-25 9:51 PM, Leo L. Schwab wrote:
> On Sun, Aug 31, 2025 at 03:01:12PM +0200, Hans de Goede wrote:
>>> +static const u16 g13_keys_for_bits_js[] = {
>>> + /* Joystick buttons */
>>> + /* These keybits are at bit indices 33, 34, and 35. */
>>> + BTN_BASE, /* Left side */
>>> + BTN_BASE2, /* Bottom side */
>>> + BTN_THUMB, /* Stick depress */
>>> +};
>>
>> You are using this 33 offset hardcoded below, maybe
>> at a #define for this, e.g. :
>>
>> #define G13_JS_KEYBITS_OFFSET 33
>>
> Noted.
>
>> g13_keys_for_bits_js[] is contiguous so no need
>> for this if (g13_keys_for_bits_js[i]) test.
>>
> Noted.
>
>>> + if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)) {
>>
>> I do not believe that this IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)
>> is necessary, led_classdev_notify_brightness_hw_changed() has a static
>> inline replacement when CONFIG_LEDS_BRIGHTNESS_HW_CHANGED is not set,
>> so you can just call it unconditionally.
>>
>> This is already called unconditionally in other places of the code.
>>
> I was actually bit by this in the first two revs by the build bot.
> If CONFIG_LEDS_BRIGHTNESS_HW_CHANGED is not enabled, the field
> `cdev.brightness_hw_changed`, referenced a bit further down, does not exist,
> and causes a build failure.
>
> My first attempt at #ifdef-ing around it led to another build bot
> warning about `hw_brightness_changed` being defined but not used. Then I
> leanred about `IS_ENABLED()`, which is evidently now preferred over `#ifdef
> CONFIG_`, and nicely isolated the whole block, so I went with that.
Ah I see. Yes if you do need to do a CONFIG check then using IS_ENABLED()
is good.
But I'm afraid that the underlying problem here is the use of
cdev.brightness_hw_changed this is really only meant for led-class.c
internal use.
The idea of cdev.brightness_hw_changed is that it stores the last
value set by the hw.
But in the mean time that value may have been overwritten by software.
I think that you will fail to call led_classdev_notify_brightness_hw_changed()
(you can add a debug print to check) if the following happens:
1. Brightness set to 255 (RGB 255,255,255) through sysfs
2. State toggled to off by backlight control button, brightness is now 0
3. Brightness set to 255 (RGB 255,255,255) through sysfs
4. State toggled to off by backlight control button, brightness is now 0
In this case the second hw-button toggle will not call
led_classdev_notify_brightness_hw_changed(), since cdev.brightness_hw_changed
still has the 0 value from last time.
I also see that you use TEST_BIT(rep->keybits, 23) ? LED_FULL : LED_OFF
for the brightness value send to led_classdev_notify_brightness_hw_changed()
but I would expect the hw to restore the previous brightness on a toggle
from off -> on through the button? So then that should be send.
And you also never update cdev.brightness and use the cached
struct lg_g15_led.brightness in lg_g13_kbd_led_get(). This means that
after a hw toggle of the backlight reading the brightness from sysfs
will show the wrong (old) value.
I think that instead what you need to do is create a
lg_g13_leds_changed_work() mirroring lg_g15_leds_changed_work()
but then only for the leds[0] instead of using the for loops.
Combined with caching the keybit 23 value (1) and then when
keybit 23 changes value queue the work.
This will also allow you to drop the:
if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED))
but that is just a side effect. The most important thing here
is to:
1. Correctly update the cached brightness after hitting the toggle button
2. Compare the new brightness against the previous cached brightness to
determine if led_classdev_notify_brightness_hw_changed() should be called,
rather then using the possible stale cdev.brightness_hw_changed
3. Pass the actual new brightness to
led_classdev_notify_brightness_hw_changed() and not LED_FULL.
Note I see that lg_g13_get_leds_state() does 2 USB transfers,
when running from the work we only need to get the backlight
state and not the macro keys state. So either give it a parameter
whether it should update the macro-keys or not; or split it into
2 functions, calling both functions from
lg_g15_get_initial_led_brightness() and only calling the one
for the backlight from lg_g13_leds_changed_work().
Regards,
Hans
1) You can just init the keybit23 cache at 1 since I think the bl always
starts on, eating the cost of possibly running the work one time too many
on the first key press if the bl was turned off before the driver probes.
^ permalink raw reply
* Re: [PATCH] dt-bindings: touchscreen: drop any reference to touchscreen.txt
From: Dario Binacchi @ 2025-09-02 6:30 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, linux-amarula, Broadcom internal kernel review list,
Conor Dooley, Dmitry Torokhov, Florian Fainelli,
Krzysztof Kozlowski, devicetree, linux-arm-kernel, linux-input,
linux-rpi-kernel
In-Reply-To: <20250725230342.GA1993803-robh@kernel.org>
Hi all,
On Sat, Jul 26, 2025 at 1:03 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Jul 23, 2025 at 09:14:20AM +0200, Dario Binacchi wrote:
> > With commit 1d6204e2f51f ("dt-bindings: touchscreen: Add touchscreen
> > schema") touchscreen.txt is no longer needed. Remove the file and
> > replace every reference to it with the corresponding YAML schema.
>
> The point of what touchscreen.txt says is to not do this. I'd rather see
> time spent on conversions. But you've already done it, so:
>
> Acked-by: Rob Herring (Arm) <robh@kernel.org>
this patch is Acked-by, but who can take it for merging?
Please let me know the right path forward.
Thanks and regards,
Dario
>
> >
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> >
> > ---
> >
> > .../devicetree/bindings/input/touchscreen/bu21013.txt | 2 +-
> > .../devicetree/bindings/input/touchscreen/eeti.txt | 2 +-
> > .../input/touchscreen/raspberrypi,firmware-ts.txt | 10 +++++-----
> > .../bindings/input/touchscreen/touchscreen.txt | 1 -
> > .../devicetree/bindings/input/touchscreen/zet6223.txt | 10 +++++-----
> > 5 files changed, 12 insertions(+), 13 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
--
Dario Binacchi
Senior Embedded Linux Developer
dario.binacchi@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: input: touchscreen: fsl,imx6ul-tsc: add fsl,glitch-threshold
From: Dario Binacchi @ 2025-09-02 6:24 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, linux-amarula, Conor Dooley, Dmitry Torokhov,
Fabio Estevam, Haibo Chen, Krzysztof Kozlowski,
Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, devicetree, imx,
linux-arm-kernel, linux-input
In-Reply-To: <CABGWkvpWPXz8bFPC3OgqY+C6cgu6hHGh6muCQkoCOEVK048fSA@mail.gmail.com>
Hello Rob,
just a gentle ping — I’ve replied to your comments on this patch, but
I haven’t seen any further feedback.
Thanks and regards,
Dario
On Wed, Jul 23, 2025 at 8:25 AM Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> On Wed, Jul 23, 2025 at 7:03 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Tue, Jul 22, 2025 at 12:36:16PM +0200, Dario Binacchi wrote:
> > > Add support for glitch threshold configuration. A detected signal is valid
> > > only if it lasts longer than the set threshold; otherwise, it is regarded
> > > as a glitch.
> > >
> > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > ---
> > >
> > > .../input/touchscreen/fsl,imx6ul-tsc.yaml | 18 ++++++++++++++++++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> > > index 678756ad0f92..2fee2940213f 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> > > @@ -62,6 +62,23 @@ properties:
> > > description: Number of data samples which are averaged for each read.
> > > enum: [ 1, 4, 8, 16, 32 ]
> > >
> > > + fsl,glitch-threshold:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + default: 0
> > > + enum: [ 0, 1, 2, 3 ]
> > > + description: |
> > > + Indicates the glitch threshold. The threshold is defined by number
> > > + of clock cycles. A detect signal is only valid if it is exist longer
> > > + than threshold; otherwise, it is regarded as a glitch.
> > > + 0: Normal function: 8191 clock cycles
> > > + Low power mode: 9 clock cycles
> > > + 1: Normal function: 4095 clock cycles
> > > + Low power mode: 7 clock cycles
> > > + 2: Normal function: 2047 clock cycles
> > > + Low power mode: 5 clock cycles
> > > + 3: Normal function: 1023 clock cycles
> > > + Low power mode: 3 clock cycles
> >
> > Don't we have common properties for this expressed in time? Debounce
> > time IIRC.
>
> I tried checking in
> Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml,
> but I didn't find anything about it.
>
> It exists in some specific touchscreen bindings:
> - azoteq,iqs7211.yaml
> - brcm,iproc-touchscreen.txt
> - fsl-mx25-tcq.txt,
> - ti,ads7843.yaml.
>
> Only fsl-mx25-tcq.txt expresses it in terms of time (ns).
>
> Thanks and regards,
> Dario
>
> >
> > > +
> > > required:
> > > - compatible
> > > - reg
> > > @@ -94,4 +111,5 @@ examples:
> > > measure-delay-time = <0xfff>;
> > > pre-charge-time = <0xffff>;
> > > touchscreen-average-samples = <32>;
> > > + fsl,glitch-threshold = <2>;
> > > };
> > > --
> > > 2.43.0
> > >
>
>
>
> --
>
> Dario Binacchi
>
> Senior Embedded Linux Developer
>
> dario.binacchi@amarulasolutions.com
>
> __________________________________
>
>
> Amarula Solutions SRL
>
> Via Le Canevare 30, 31100 Treviso, Veneto, IT
>
> T. +39 042 243 5310
> info@amarulasolutions.com
>
> www.amarulasolutions.com
--
Dario Binacchi
Senior Embedded Linux Developer
dario.binacchi@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply
* [PATCH v5] HID: lg-g15 - Add support for Logitech G13.
From: Leo L. Schwab @ 2025-09-02 0:36 UTC (permalink / raw)
To: Hans de Goede
Cc: Kate Hsuan, Leo L. Schwab, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel
The Logitech G13 is a gaming keypad with general-purpose macro keys,
four LED-backlit macro preset keys, five "menu" keys, backlight toggle
key, an analog thumbstick, RGB LED backlight, and a monochrome LCD
display.
Support input event generation for all keys and the thumbstick, and
expose all LEDs.
Signed-off-by: Leo L. Schwab <ewhac@ewhac.org>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Tested-by: Kate Hsuan <hpa@redhat.com>
---
Changes in v5:
- None; resend v4 due to bounced email submission.
Changes in v4:
- Minor changes recommended by Hans de Goede <hansg@kernel.org>.
Changes in v3:
- Re-revise commit message.
- Conditionally compile the section depending on
CONFIG_LEDS_BRIGHTNESS_HW_CHANGED correctly this time.
- Use led-class-multicolor facilities for the RGB backlight.
- Changes recommended by Kate Hsuan <hpa@redhat.com>:
- Use guard(mutex) construct.
- Fix numerous style nits.
Changes in v2:
- Add `#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED` bracket around new
code segment dependent on that feature (fixes test robot build
error).
- Use `guard(mutex)` construct in new code (existing code left
unmodified).
- Commit message revised.
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-lg-g15.c | 426 +++++++++++++++++++++++++++++++++++++--
2 files changed, 411 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 33cc5820f2be..7ed1e402b80a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -870,6 +870,7 @@
#define USB_DEVICE_ID_LOGITECH_DUAL_ACTION 0xc216
#define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2 0xc218
#define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2 0xc219
+#define USB_DEVICE_ID_LOGITECH_G13 0xc21c
#define USB_DEVICE_ID_LOGITECH_G15_LCD 0xc222
#define USB_DEVICE_ID_LOGITECH_G11 0xc225
#define USB_DEVICE_ID_LOGITECH_G15_V2_LCD 0xc227
diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c
index f8605656257b..62cb795c2393 100644
--- a/drivers/hid/hid-lg-g15.c
+++ b/drivers/hid/hid-lg-g15.c
@@ -26,7 +26,11 @@
#define LG_G510_FEATURE_BACKLIGHT_RGB 0x05
#define LG_G510_FEATURE_POWER_ON_RGB 0x06
+#define LG_G13_FEATURE_M_KEYS_LEDS 0x05
+#define LG_G13_FEATURE_BACKLIGHT_RGB 0x07
+
enum lg_g15_model {
+ LG_G13,
LG_G15,
LG_G15_V2,
LG_G510,
@@ -45,6 +49,12 @@ enum lg_g15_led_type {
LG_G15_LED_MAX
};
+struct g13_input_report {
+ u8 report_id; /* Report ID is always set to 1. */
+ u8 joy_x, joy_y;
+ u8 keybits[5];
+};
+
struct lg_g15_led {
union {
struct led_classdev cdev;
@@ -63,12 +73,172 @@ struct lg_g15_data {
struct mutex mutex;
struct work_struct work;
struct input_dev *input;
+ struct input_dev *input_js; /* Separate joystick device for G13. */
struct hid_device *hdev;
enum lg_g15_model model;
struct lg_g15_led leds[LG_G15_LED_MAX];
bool game_mode_enabled;
};
+/********* G13 LED functions ***********/
+/*
+ * G13 retains no state across power cycles, and always powers up with the backlight on,
+ * color #5AFF6E, all macro key LEDs off.
+ */
+static int lg_g13_get_leds_state(struct lg_g15_data *g15)
+{
+ u8 * const tbuf = g15->transfer_buf;
+ int ret, high;
+
+ /* RGB backlight. */
+ ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB,
+ tbuf, 5,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ if (ret != 5) {
+ hid_err(g15->hdev, "Error getting backlight brightness: %d\n", ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+
+ /* Normalize RGB intensities against the highest component. */
+ high = max3(tbuf[1], tbuf[2], tbuf[3]);
+ if (high) {
+ g15->leds[LG_G15_KBD_BRIGHTNESS].red =
+ DIV_ROUND_CLOSEST(tbuf[1] * 255, high);
+ g15->leds[LG_G15_KBD_BRIGHTNESS].green =
+ DIV_ROUND_CLOSEST(tbuf[2] * 255, high);
+ g15->leds[LG_G15_KBD_BRIGHTNESS].blue =
+ DIV_ROUND_CLOSEST(tbuf[3] * 255, high);
+ g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = high;
+ } else {
+ g15->leds[LG_G15_KBD_BRIGHTNESS].red = 255;
+ g15->leds[LG_G15_KBD_BRIGHTNESS].green = 255;
+ g15->leds[LG_G15_KBD_BRIGHTNESS].blue = 255;
+ g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = 0;
+ }
+
+ /* Macro LEDs. */
+ ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS,
+ tbuf, 5,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ if (ret != 5) {
+ hid_err(g15->hdev, "Error getting macro LED brightness: %d\n", ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+
+ for (int i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i)
+ g15->leds[i].brightness = tbuf[1] & (1 << (i - LG_G15_MACRO_PRESET1));
+
+ return 0;
+}
+
+static int lg_g13_kbd_led_write(struct lg_g15_data *g15,
+ struct lg_g15_led *g15_led,
+ enum led_brightness brightness)
+{
+ struct mc_subled const * const subleds = g15_led->mcdev.subled_info;
+ u8 * const tbuf = g15->transfer_buf;
+ int ret;
+
+ guard(mutex)(&g15->mutex);
+
+ led_mc_calc_color_components(&g15_led->mcdev, brightness);
+
+ tbuf[0] = 5;
+ tbuf[1] = subleds[0].brightness;
+ tbuf[2] = subleds[1].brightness;
+ tbuf[3] = subleds[2].brightness;
+ tbuf[4] = 0;
+
+ ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB,
+ tbuf, 5,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != 5) {
+ hid_err(g15->hdev, "Error setting backlight brightness: %d\n", ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+
+ g15_led->brightness = brightness;
+ return 0;
+}
+
+static int lg_g13_kbd_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
+{
+ struct led_classdev_mc *mc = lcdev_to_mccdev(led_cdev);
+ struct lg_g15_led *g15_led =
+ container_of(mc, struct lg_g15_led, mcdev);
+ struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
+
+ /* Ignore LED off on unregister / keyboard unplug */
+ if (led_cdev->flags & LED_UNREGISTERING)
+ return 0;
+
+ return lg_g13_kbd_led_write(g15, g15_led, brightness);
+}
+
+static enum led_brightness lg_g13_kbd_led_get(struct led_classdev *led_cdev)
+{
+ struct led_classdev_mc const * const mc = lcdev_to_mccdev(led_cdev);
+ struct lg_g15_led const *g15_led =
+ container_of(mc, struct lg_g15_led, mcdev);
+
+ return g15_led->brightness;
+}
+
+static int lg_g13_mkey_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
+{
+ struct lg_g15_led *g15_led =
+ container_of(led_cdev, struct lg_g15_led, cdev);
+ struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
+ int i, ret;
+ u8 * const tbuf = g15->transfer_buf;
+ u8 val, mask = 0;
+
+ /* Ignore LED off on unregister / keyboard unplug */
+ if (led_cdev->flags & LED_UNREGISTERING)
+ return 0;
+
+ guard(mutex)(&g15->mutex);
+
+ for (i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i) {
+ if (i == g15_led->led)
+ val = brightness;
+ else
+ val = g15->leds[i].brightness;
+
+ if (val)
+ mask |= 1 << (i - LG_G15_MACRO_PRESET1);
+ }
+
+ tbuf[0] = 5;
+ tbuf[1] = mask;
+ tbuf[2] = 0;
+ tbuf[3] = 0;
+ tbuf[4] = 0;
+
+ ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS,
+ tbuf, 5,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != 5) {
+ hid_err(g15->hdev, "Error setting LED brightness: %d\n", ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+
+ g15_led->brightness = brightness;
+ return 0;
+}
+
+static enum led_brightness lg_g13_mkey_led_get(struct led_classdev *led_cdev)
+{
+ /*
+ * G13 doesn't change macro key LEDs behind our back, so they're
+ * whatever we last set them to.
+ */
+ struct lg_g15_led *g15_led =
+ container_of(led_cdev, struct lg_g15_led, cdev);
+
+ return g15_led->brightness;
+}
+
/******** G15 and G15 v2 LED functions ********/
static int lg_g15_update_led_brightness(struct lg_g15_data *g15)
@@ -390,6 +560,8 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15)
int ret;
switch (g15->model) {
+ case LG_G13:
+ return lg_g13_get_leds_state(g15);
case LG_G15:
case LG_G15_V2:
return lg_g15_update_led_brightness(g15);
@@ -417,6 +589,116 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15)
/******** Input functions ********/
+/**
+ * g13_input_report.keybits[] is not 32-bit aligned, so we can't use the bitops macros.
+ *
+ * @ary: Pointer to array of u8s
+ * @b: Bit index into ary, LSB first. Not range checked.
+ */
+#define TEST_BIT(ary, b) ((1 << ((b) & 7)) & (ary)[(b) >> 3])
+
+/* Table mapping keybits[] bit positions to event codes. */
+/* Note: Indices are discontinuous to aid readability. */
+static const u16 g13_keys_for_bits[] = {
+ /* Main keypad - keys G1 - G22 */
+ [0] = KEY_MACRO1,
+ [1] = KEY_MACRO2,
+ [2] = KEY_MACRO3,
+ [3] = KEY_MACRO4,
+ [4] = KEY_MACRO5,
+ [5] = KEY_MACRO6,
+ [6] = KEY_MACRO7,
+ [7] = KEY_MACRO8,
+ [8] = KEY_MACRO9,
+ [9] = KEY_MACRO10,
+ [10] = KEY_MACRO11,
+ [11] = KEY_MACRO12,
+ [12] = KEY_MACRO13,
+ [13] = KEY_MACRO14,
+ [14] = KEY_MACRO15,
+ [15] = KEY_MACRO16,
+ [16] = KEY_MACRO17,
+ [17] = KEY_MACRO18,
+ [18] = KEY_MACRO19,
+ [19] = KEY_MACRO20,
+ [20] = KEY_MACRO21,
+ [21] = KEY_MACRO22,
+
+ /* LCD menu buttons. */
+ [24] = KEY_KBD_LCD_MENU5, /* "Next page" button */
+ [25] = KEY_KBD_LCD_MENU1, /* Left-most */
+ [26] = KEY_KBD_LCD_MENU2,
+ [27] = KEY_KBD_LCD_MENU3,
+ [28] = KEY_KBD_LCD_MENU4, /* Right-most */
+
+ /* Macro preset and record buttons; have red LEDs under them. */
+ [29] = KEY_MACRO_PRESET1,
+ [30] = KEY_MACRO_PRESET2,
+ [31] = KEY_MACRO_PRESET3,
+ [32] = KEY_MACRO_RECORD_START,
+
+ /* 33-35 handled by joystick device. */
+
+ /* Backlight toggle. */
+ [37] = KEY_LIGHTS_TOGGLE,
+};
+
+#define G13_JS_KEYBITS_OFFSET 33
+
+static const u16 g13_keys_for_bits_js[] = {
+ /* Joystick buttons */
+ /* These keybits are at bit indices 33, 34, and 35. */
+ BTN_BASE, /* Left side */
+ BTN_BASE2, /* Bottom side */
+ BTN_THUMB, /* Stick depress */
+};
+
+static int lg_g13_event(struct lg_g15_data *g15, u8 const *data)
+{
+ struct g13_input_report const * const rep = (struct g13_input_report *) data;
+ int i, val;
+
+ /*
+ * Main macropad and menu keys.
+ * Emit key events defined for each bit position.
+ */
+ for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) {
+ if (g13_keys_for_bits[i]) {
+ val = TEST_BIT(rep->keybits, i);
+ input_report_key(g15->input, g13_keys_for_bits[i], val);
+ }
+ }
+ input_sync(g15->input);
+
+ /*
+ * Joystick.
+ * Emit button and deflection events.
+ */
+ for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) {
+ val = TEST_BIT(rep->keybits, i + G13_JS_KEYBITS_OFFSET);
+ input_report_key(g15->input_js, g13_keys_for_bits_js[i], val);
+ }
+ input_report_abs(g15->input_js, ABS_X, rep->joy_x);
+ input_report_abs(g15->input_js, ABS_Y, rep->joy_y);
+ input_sync(g15->input_js);
+
+ if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)) {
+ /*
+ * Bit 23 of keybits[] reports the current backlight on/off
+ * state. If it has changed from the last cached value, apply
+ * an update.
+ */
+ bool hw_brightness_changed = (!!TEST_BIT(rep->keybits, 23))
+ ^ (g15->leds[0].cdev.brightness_hw_changed > 0);
+ if (hw_brightness_changed)
+ led_classdev_notify_brightness_hw_changed(
+ &g15->leds[0].cdev,
+ TEST_BIT(rep->keybits, 23) ? LED_FULL : LED_OFF);
+ }
+
+ return 0;
+}
+
/* On the G15 Mark I Logitech has been quite creative with which bit is what */
static void lg_g15_handle_lcd_menu_keys(struct lg_g15_data *g15, u8 *data)
{
@@ -572,6 +854,10 @@ static int lg_g15_raw_event(struct hid_device *hdev, struct hid_report *report,
return 0;
switch (g15->model) {
+ case LG_G13:
+ if (data[0] == 0x01 && size == sizeof(struct g13_input_report))
+ return lg_g13_event(g15, data);
+ break;
case LG_G15:
if (data[0] == 0x02 && size == 9)
return lg_g15_event(g15, data);
@@ -616,13 +902,22 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index)
{
int i;
struct mc_subled *subled_info;
-
- g15->leds[index].mcdev.led_cdev.brightness_set_blocking =
- lg_g510_kbd_led_set;
- g15->leds[index].mcdev.led_cdev.brightness_get =
- lg_g510_kbd_led_get;
- g15->leds[index].mcdev.led_cdev.max_brightness = 255;
- g15->leds[index].mcdev.num_colors = 3;
+ struct lg_g15_led * const gled = &g15->leds[index];
+
+ if (g15->model == LG_G13) {
+ gled->mcdev.led_cdev.brightness_set_blocking =
+ lg_g13_kbd_led_set;
+ gled->mcdev.led_cdev.brightness_get =
+ lg_g13_kbd_led_get;
+ gled->mcdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED;
+ } else {
+ gled->mcdev.led_cdev.brightness_set_blocking =
+ lg_g510_kbd_led_set;
+ gled->mcdev.led_cdev.brightness_get =
+ lg_g510_kbd_led_get;
+ }
+ gled->mcdev.led_cdev.max_brightness = 255;
+ gled->mcdev.num_colors = 3;
subled_info = devm_kcalloc(&g15->hdev->dev, 3, sizeof(*subled_info), GFP_KERNEL);
if (!subled_info)
@@ -632,20 +927,20 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index)
switch (i + 1) {
case LED_COLOR_ID_RED:
subled_info[i].color_index = LED_COLOR_ID_RED;
- subled_info[i].intensity = g15->leds[index].red;
+ subled_info[i].intensity = gled->red;
break;
case LED_COLOR_ID_GREEN:
subled_info[i].color_index = LED_COLOR_ID_GREEN;
- subled_info[i].intensity = g15->leds[index].green;
+ subled_info[i].intensity = gled->green;
break;
case LED_COLOR_ID_BLUE:
subled_info[i].color_index = LED_COLOR_ID_BLUE;
- subled_info[i].intensity = g15->leds[index].blue;
+ subled_info[i].intensity = gled->blue;
break;
}
subled_info[i].channel = i;
}
- g15->leds[index].mcdev.subled_info = subled_info;
+ gled->mcdev.subled_info = subled_info;
}
static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
@@ -656,6 +951,23 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
g15->leds[i].cdev.name = name;
switch (g15->model) {
+ case LG_G13:
+ if (i < LG_G15_BRIGHTNESS_MAX) {
+ /* RGB backlight. */
+ lg_g15_setup_led_rgb(g15, i);
+ ret = devm_led_classdev_multicolor_register_ext(&g15->hdev->dev,
+ &g15->leds[i].mcdev,
+ NULL);
+ } else {
+ /* Macro keys */
+ g15->leds[i].cdev.brightness_set_blocking = lg_g13_mkey_led_set;
+ g15->leds[i].cdev.brightness_get = lg_g13_mkey_led_get;
+ g15->leds[i].cdev.max_brightness = 1;
+
+ ret = devm_led_classdev_register(&g15->hdev->dev,
+ &g15->leds[i].cdev);
+ }
+ break;
case LG_G15:
case LG_G15_V2:
g15->leds[i].cdev.brightness_get = lg_g15_led_get;
@@ -702,11 +1014,9 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
}
/* Common input device init code shared between keyboards and Z-10 speaker handling */
-static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input,
- const char *name)
+static void lg_g15_init_input_dev_core(struct hid_device *hdev, struct input_dev *input,
+ char const *name)
{
- int i;
-
input->name = name;
input->phys = hdev->phys;
input->uniq = hdev->uniq;
@@ -717,12 +1027,42 @@ static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *inp
input->dev.parent = &hdev->dev;
input->open = lg_g15_input_open;
input->close = lg_g15_input_close;
+}
+
+static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input,
+ const char *name)
+{
+ int i;
+
+ lg_g15_init_input_dev_core(hdev, input, name);
/* Keys below the LCD, intended for controlling a menu on the LCD */
for (i = 0; i < 5; i++)
input_set_capability(input, EV_KEY, KEY_KBD_LCD_MENU1 + i);
}
+static void lg_g13_init_input_dev(struct hid_device *hdev,
+ struct input_dev *input, const char *name,
+ struct input_dev *input_js, const char *name_js)
+{
+ /* Macropad. */
+ lg_g15_init_input_dev_core(hdev, input, name);
+ for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) {
+ if (g13_keys_for_bits[i])
+ input_set_capability(input, EV_KEY, g13_keys_for_bits[i]);
+ }
+
+ /* OBTW, we're a joystick, too... */
+ lg_g15_init_input_dev_core(hdev, input_js, name_js);
+ for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i)
+ input_set_capability(input_js, EV_KEY, g13_keys_for_bits_js[i]);
+
+ input_set_capability(input_js, EV_ABS, ABS_X);
+ input_set_abs_params(input_js, ABS_X, 0, 255, 0, 0);
+ input_set_capability(input_js, EV_ABS, ABS_Y);
+ input_set_abs_params(input_js, ABS_Y, 0, 255, 0, 0);
+}
+
static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
static const char * const led_names[] = {
@@ -739,7 +1079,7 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
unsigned int connect_mask = 0;
bool has_ff000000 = false;
struct lg_g15_data *g15;
- struct input_dev *input;
+ struct input_dev *input, *input_js;
struct hid_report *rep;
int ret, i, gkeys = 0;
@@ -778,6 +1118,25 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
hid_set_drvdata(hdev, (void *)g15);
switch (g15->model) {
+ case LG_G13:
+ /*
+ * The G13 has an analog thumbstick with nearby buttons. Some
+ * libraries and applications are known to ignore devices that
+ * don't "look like" a joystick, and a device with two ABS axes
+ * and 25+ macro keys would confuse them.
+ *
+ * Create an additional input device dedicated to appear as a
+ * simplified joystick (two ABS axes, three BTN buttons).
+ */
+ input_js = devm_input_allocate_device(&hdev->dev);
+ if (!input_js)
+ return -ENOMEM;
+ g15->input_js = input_js;
+ input_set_drvdata(input_js, hdev);
+
+ connect_mask = HID_CONNECT_HIDRAW;
+ gkeys = 25;
+ break;
case LG_G15:
INIT_WORK(&g15->work, lg_g15_leds_changed_work);
/*
@@ -859,6 +1218,34 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto error_hw_stop;
return 0; /* All done */
+ } else if (g15->model == LG_G13) {
+ static char const * const g13_led_names[] = {
+ /* Backlight is shared between LCD and keys. */
+ "g13:rgb:kbd_backlight",
+ NULL, /* Keep in sync with led_type enum */
+ "g13:red:macro_preset_1",
+ "g13:red:macro_preset_2",
+ "g13:red:macro_preset_3",
+ "g13:red:macro_record",
+ };
+ lg_g13_init_input_dev(hdev,
+ input, "Logitech G13 Gaming Keypad",
+ input_js, "Logitech G13 Thumbstick");
+ ret = input_register_device(input);
+ if (ret)
+ goto error_hw_stop;
+ ret = input_register_device(input_js);
+ if (ret)
+ goto error_hw_stop;
+
+ for (i = 0; i < ARRAY_SIZE(g13_led_names); ++i) {
+ if (g13_led_names[i]) {
+ ret = lg_g15_register_led(g15, i, g13_led_names[i]);
+ if (ret)
+ goto error_hw_stop;
+ }
+ }
+ return 0;
}
/* Setup and register input device */
@@ -903,6 +1290,13 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
static const struct hid_device_id lg_g15_devices[] = {
+ /*
+ * The G13 is a macropad-only device with an LCD, LED backlighing,
+ * and joystick.
+ */
+ { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+ USB_DEVICE_ID_LOGITECH_G13),
+ .driver_data = LG_G13 },
/* The G11 is a G15 without the LCD, treat it as a G15 */
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_G11),
--
2.51.0
^ permalink raw reply related
* [PATCH v3 3/3] platform/x86: thinkpad_acpi: Use trackpoint doubletap interface via sysfs
From: Vishnu Sankar @ 2025-09-01 13:53 UTC (permalink / raw)
To: dmitry.torokhov, hmh, hansg, ilpo.jarvinen, derekjohn.clark
Cc: mpearson-lenovo, linux-input, linux-kernel, ibm-acpi-devel,
platform-driver-x86, vsankar, Vishnu Sankar
In-Reply-To: <20250901135308.52340-1-vishnuocv@gmail.com>
TrackPoint devices supporting doubletap expose a sysfs attribute under
/sys/devices/.../trackpoint/doubletap_enabled. This patch enables
thinkpad_acpi to detect if the system has a TrackPoint device with
doubletap capability, and allows toggling the feature via sysfs.
This avoids direct linking between subsystems and relies on sysfs
as the interface for coordination between input and platform drivers.
Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Changes in v2:
- Updated commit message to clarify dependency on trackpoint driver
- Now handling sysfs read/write of trackpoint driver using file read/write
- Removed sysfs attribute creation of trackpoint double tap here.
- Reversed the logic and return false right away
- Dropped unnecessary debug messages
- Using dev_dbg() instead of pr_xxxx()
Changes in v3:
- No changes
---
drivers/platform/x86/lenovo/thinkpad_acpi.c | 155 +++++++++++++++++++-
1 file changed, 147 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index cc19fe520ea9..f5070442a7ba 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -72,6 +72,13 @@
#include <linux/units.h>
#include <linux/workqueue.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/err.h>
+#include <linux/fcntl.h>
+#include <linux/namei.h>
+#include <linux/kernel_read_file.h>
+
#include <acpi/battery.h>
#include <acpi/video.h>
@@ -373,7 +380,8 @@ static struct {
u32 hotkey_poll_active:1;
u32 has_adaptive_kbd:1;
u32 kbd_lang:1;
- u32 trackpoint_doubletap:1;
+ u32 trackpoint_doubletap_state:1;
+ u32 trackpoint_doubletap_capable:1;
struct quirk_entry *quirks;
} tp_features;
@@ -2879,6 +2887,107 @@ static DEVICE_ATTR_RW(hotkey_poll_freq);
#endif /* CONFIG_THINKPAD_ACPI_HOTKEY_POLL */
+/*
+ * Trackpoint doubletap handlers
+ * These set of functions will communicate with the sysfs attributes of TrackPoint driver
+ * Attribute : /sys/bus/serio/devices/seriox/doubletap_enabled
+ */
+
+/* Global buffer to reuse path */
+static char trackpoint_doubletap_path[128];
+
+/* Function to find the correct serio path with TrackPoint attribute "doubletap_enabled" */
+static int thinkpad_find_trackpoint_path(void)
+{
+ struct path serio_path;
+ char path_buf[128];
+ int i;
+
+ for (i = 0; i < 10; i++) {
+ snprintf(path_buf, sizeof(path_buf),
+ "/sys/bus/serio/devices/serio%d/doubletap_enabled", i);
+
+ if (!kern_path(path_buf, LOOKUP_FOLLOW, &serio_path)) {
+ path_put(&serio_path);
+ snprintf(trackpoint_doubletap_path, sizeof(trackpoint_doubletap_path),
+ "%s", path_buf);
+ pr_info("ThinkPad ACPI: TrackPoint doubletap found at %s\n",
+ trackpoint_doubletap_path);
+ return 0;
+ }
+ }
+ return -ENODEV;
+}
+
+/* Writing to the sysfs attribute of Trackpoint "doubletap_enabled" */
+static int write_doubletap_sysfs_value(const void *buf, size_t count, loff_t *pos)
+{
+ struct file *filp;
+ ssize_t written;
+
+ if (!buf)
+ return -EINVAL;
+
+ filp = filp_open(trackpoint_doubletap_path, O_WRONLY | O_CREAT, 0644);
+ if (IS_ERR(filp))
+ return PTR_ERR(filp);
+
+ /* Required to avoid EINVAL from vfs checks in some cases */
+ if (!(filp->f_mode & FMODE_CAN_WRITE)) {
+ filp_close(filp, NULL);
+ return -EINVAL;
+ }
+
+ /* Write using kernel_write */
+ written = kernel_write(filp, buf, count, pos);
+ filp_close(filp, NULL);
+
+ return written < 0 ? written : 0;
+}
+
+/* Function to read the TrackPoint doubletap status */
+static int trackpoint_read_doubletap_status(bool *enabled)
+{
+ struct file *filp;
+ loff_t pos = 0;
+ char buf[8];
+ ssize_t ret;
+
+ if (!enabled)
+ return -EINVAL;
+
+ if (!trackpoint_doubletap_path[0])
+ return -ENODEV;
+
+ filp = filp_open(trackpoint_doubletap_path, O_RDONLY, 0);
+ if (IS_ERR(filp))
+ return PTR_ERR(filp);
+
+ ret = kernel_read(filp, buf, sizeof(buf) - 1, &pos);
+ filp_close(filp, NULL);
+
+ if (ret < 0)
+ return ret;
+
+ buf[ret] = '\0'; // Safe: ret < sizeof(buf)
+
+ *enabled = (buf[0] == '1');
+
+ return 0;
+}
+
+/* Function to check the TrackPoint doubletap status */
+static int thinkpad_set_doubletap_status(bool enable)
+{
+ const char *val = enable ? "1" : "0";
+ loff_t pos = 0;
+
+ if (!trackpoint_doubletap_path[0])
+ return -ENODEV;
+
+ return write_doubletap_sysfs_value(val, strlen(val), &pos);
+}
+
/* sysfs hotkey radio_sw (pollable) ------------------------------------ */
static ssize_t hotkey_radio_sw_show(struct device *dev,
struct device_attribute *attr,
@@ -3326,6 +3435,8 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
bool radiosw_state = false;
bool tabletsw_state = false;
int hkeyv, res, status, camera_shutter_state;
+ bool dt_state;
+ int rc;
vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
"initializing hotkey subdriver\n");
@@ -3557,9 +3668,22 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
hotkey_poll_setup_safe(true);
- /* Enable doubletap by default */
- tp_features.trackpoint_doubletap = 1;
+ /* Checking doubletap status by default */
+ rc = thinkpad_find_trackpoint_path();
+ if (rc) {
+ dev_dbg(&tpacpi_pdev->dev, "Could not find TrackPoint doubletap sysfs path\n");
+ tp_features.trackpoint_doubletap_capable = false;
+ return 0;
+ }
+ tp_features.trackpoint_doubletap_capable = true;
+ rc = trackpoint_read_doubletap_status(&dt_state);
+ if (rc) {
+ /* Disable if access to register fails */
+ dt_state = false;
+ dev_dbg(&tpacpi_pdev->dev, "Doubletap failed to check status\n");
+ }
+ tp_features.trackpoint_doubletap_state = dt_state;
return 0;
}
@@ -3863,9 +3987,7 @@ static bool hotkey_notify_8xxx(const u32 hkey, bool *send_acpi_ev)
{
switch (hkey) {
case TP_HKEY_EV_TRACK_DOUBLETAP:
- if (tp_features.trackpoint_doubletap)
- tpacpi_input_send_key(hkey, send_acpi_ev);
-
+ *send_acpi_ev = true;
return true;
default:
return false;
@@ -11194,6 +11316,7 @@ static struct platform_driver tpacpi_hwmon_pdriver = {
static bool tpacpi_driver_event(const unsigned int hkey_event)
{
int camera_shutter_state;
+ int rc;
switch (hkey_event) {
case TP_HKEY_EV_BRGHT_UP:
@@ -11285,8 +11408,24 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
mutex_unlock(&tpacpi_inputdev_send_mutex);
return true;
case TP_HKEY_EV_DOUBLETAP_TOGGLE:
- tp_features.trackpoint_doubletap = !tp_features.trackpoint_doubletap;
- return true;
+ if (tp_features.trackpoint_doubletap_capable) {
+ rc = thinkpad_set_doubletap_status(!tp_features.trackpoint_doubletap_state);
+
+ if (rc) {
+ dev_dbg(&tpacpi_pdev->dev, "Trackpoint doubletap toggle failed\n");
+ } else {
+ tp_features.trackpoint_doubletap_state =
+ !tp_features.trackpoint_doubletap_state;
+ dev_dbg(&tpacpi_pdev->dev, "Trackpoint doubletap is %s\n",
+ tp_features.trackpoint_doubletap_state ? "enabled" : "disabled");
+ return true;
+ }
+ }
+ /*
+ * Suppress the event if Doubletap is not supported
+ * or if the trackpoint_set_doubletap_status() is failing
+ */
+ return false;
case TP_HKEY_EV_PROFILE_TOGGLE:
case TP_HKEY_EV_PROFILE_TOGGLE2:
platform_profile_cycle();
--
2.48.1
^ permalink raw reply related
* [PATCH v3 2/3] Documentation/ABI: Add sysfs documentation for TrackPoint doubletap
From: Vishnu Sankar @ 2025-09-01 13:53 UTC (permalink / raw)
To: dmitry.torokhov, hmh, hansg, ilpo.jarvinen, derekjohn.clark
Cc: mpearson-lenovo, linux-input, linux-kernel, ibm-acpi-devel,
platform-driver-x86, vsankar, Vishnu Sankar
In-Reply-To: <20250901135308.52340-1-vishnuocv@gmail.com>
Adding sysfs documentation for trackpoint doubletap.
Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
---
Changes in v3:
- Add sysfs documentation.
---
.../testing/sysfs-driver-trackpoint-doubletap | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-trackpoint-doubletap
diff --git a/Documentation/ABI/testing/sysfs-driver-trackpoint-doubletap b/Documentation/ABI/testing/sysfs-driver-trackpoint-doubletap
new file mode 100644
index 000000000000..d7cd63005523
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-trackpoint-doubletap
@@ -0,0 +1,17 @@
+What: /sys/bus/serio/devices/serioX/doubletap_enabled
+Date: Aug 2025
+KernelVersion: 6.17
+Contact: linux-input@vger.kernel.org, platform-driver-x86@vger.kernel.org
+Description:
+ A read/write attribute controlling TrackPoint doubletap.
+
+ Reading returns the current state:
+ 0 - disabled
+ 1 - enabled
+
+ Writing:
+ 0 - disable doubletap
+ 1 - enable doubletap
+
+ The attribute is only present if the TrackPoint firmware
+ supports doubletap functionality.
--
2.48.1
^ permalink raw reply related
* [PATCH v3 1/3] input: mouse: trackpoint: Add doubletap enable/disable support
From: Vishnu Sankar @ 2025-09-01 13:53 UTC (permalink / raw)
To: dmitry.torokhov, hmh, hansg, ilpo.jarvinen, derekjohn.clark
Cc: mpearson-lenovo, linux-input, linux-kernel, ibm-acpi-devel,
platform-driver-x86, vsankar, Vishnu Sankar
Add support for enabling and disabling doubletap on TrackPoint devices
that support this functionality. The feature is detected using firmware
ID and exposed via sysfs as `doubletap_enabled`.
The feature is only available on newer ThinkPads (2023 and later).The driver
exposes this capability via a new sysfs attribute:
"/sys/bus/serio/devices/seriox/doubletap_enabled".
The attribute is only created if the device is detected to be capable of
doubletap via firmware and variant ID checks. This functionality will be
used by platform drivers such as thinkpad_acpi to expose and control doubletap
via user interfaces.
Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Changes in v2:
- Improve commit messages
- Sysfs attributes moved to trackpoint.c
- Removed unnecessary comments
- Removed unnecessary debug messages
- Using strstarts() instead of strcmp()
- is_trackpoint_dt_capable() modified
- Removed _BIT suffix and used BIT() define.
- Reverse the trackpoint_doubletap_status() logic to return error first.
- Removed export functions as a result of the design change
- Changed trackpoint_dev->psmouse to parent_psmouse
- The path of trackpoint.h is not changed.
Changes in v3:
- No changes.
---
drivers/input/mouse/trackpoint.c | 149 +++++++++++++++++++++++++++++++
drivers/input/mouse/trackpoint.h | 15 ++++
2 files changed, 164 insertions(+)
diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index 5f6643b69a2c..c6f17b0dec3a 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -16,6 +16,8 @@
#include "psmouse.h"
#include "trackpoint.h"
+static struct trackpoint_data *trackpoint_dev;
+
static const char * const trackpoint_variants[] = {
[TP_VARIANT_IBM] = "IBM",
[TP_VARIANT_ALPS] = "ALPS",
@@ -63,6 +65,21 @@ static int trackpoint_write(struct ps2dev *ps2dev, u8 loc, u8 val)
return ps2_command(ps2dev, param, MAKE_PS2_CMD(3, 0, TP_COMMAND));
}
+/* Read function for TrackPoint extended registers */
+static int trackpoint_extended_read(struct ps2dev *ps2dev, u8 loc, u8 *val)
+{
+ u8 ext_param[2] = {TP_READ_MEM, loc};
+ int error;
+
+ error = ps2_command(ps2dev,
+ ext_param, MAKE_PS2_CMD(2, 1, TP_COMMAND));
+
+ if (!error)
+ *val = ext_param[0];
+
+ return error;
+}
+
static int trackpoint_toggle_bit(struct ps2dev *ps2dev, u8 loc, u8 mask)
{
u8 param[3] = { TP_TOGGLE, loc, mask };
@@ -393,6 +410,131 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
return 0;
}
+/* List of known incapable device PNP IDs */
+static const char * const dt_incompatible_devices[] = {
+ "LEN0304",
+ "LEN0306",
+ "LEN0317",
+ "LEN031A",
+ "LEN031B",
+ "LEN031C",
+ "LEN031D",
+};
+
+/*
+ * checks if it’s a doubletap capable device
+ * The PNP ID format eg: is "PNP: LEN030d PNP0f13".
+ */
+static bool is_trackpoint_dt_capable(const char *pnp_id)
+{
+ const char *id_start;
+ char id[8];
+
+ if (!strstarts(pnp_id, "PNP: LEN03"))
+ return false;
+
+ /* Points to "LEN03xxxx" */
+ id_start = pnp_id + 5;
+ if (sscanf(id_start, "%7s", id) != 1)
+ return false;
+
+ /* Check if it's blacklisted */
+ for (size_t i = 0; i < ARRAY_SIZE(dt_incompatible_devices); ++i) {
+ if (strcmp(id, dt_incompatible_devices[i]) == 0)
+ return false;
+ }
+ return true;
+}
+
+/* Trackpoint doubletap status function */
+static int trackpoint_doubletap_status(bool *status)
+{
+ struct trackpoint_data *tp = trackpoint_dev;
+ struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
+ u8 reg_val;
+ int rc;
+
+ /* Reading the Doubletap register using extended read */
+ rc = trackpoint_extended_read(ps2dev, TP_DOUBLETAP, ®_val);
+ if (rc)
+ return rc;
+
+ *status = reg_val & TP_DOUBLETAP_STATUS ? true : false;
+
+ return 0;
+}
+
+/* Trackpoint doubletap enable/disable function */
+static int trackpoint_set_doubletap(bool enable)
+{
+ struct trackpoint_data *tp = trackpoint_dev;
+ struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
+ static u8 doubletap_state;
+ u8 new_val;
+
+ if (!tp)
+ return -ENODEV;
+
+ new_val = enable ? TP_DOUBLETAP_ENABLE : TP_DOUBLETAP_DISABLE;
+
+ if (doubletap_state == new_val)
+ return 0;
+
+ doubletap_state = new_val;
+
+ return trackpoint_write(ps2dev, TP_DOUBLETAP, new_val);
+}
+
+/*
+ * Trackpoint Doubletap Interface
+ * Control/Monitoring of Trackpoint Doubletap from:
+ * /sys/bus/serio/devices/seriox/doubletap_enabled
+ */
+static ssize_t doubletap_enabled_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct serio *serio = to_serio_port(dev);
+ struct psmouse *psmouse = psmouse_from_serio(serio);
+ struct trackpoint_data *tp = psmouse->private;
+ bool status;
+ int rc;
+
+ if (!tp || !tp->doubletap_capable)
+ return -ENODEV;
+
+ rc = trackpoint_doubletap_status(&status);
+ if (rc)
+ return rc;
+
+ return sysfs_emit(buf, "%d\n", status ? 1 : 0);
+}
+
+static ssize_t doubletap_enabled_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct serio *serio = to_serio_port(dev);
+ struct psmouse *psmouse = psmouse_from_serio(serio);
+ struct trackpoint_data *tp = psmouse->private;
+ bool enable;
+ int err;
+
+ if (!tp || !tp->doubletap_capable)
+ return -ENODEV;
+
+ err = kstrtobool(buf, &enable);
+ if (err)
+ return err;
+
+ err = trackpoint_set_doubletap(enable);
+ if (err)
+ return err;
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(doubletap_enabled);
+
int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
{
struct ps2dev *ps2dev = &psmouse->ps2dev;
@@ -425,6 +567,9 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
psmouse->reconnect = trackpoint_reconnect;
psmouse->disconnect = trackpoint_disconnect;
+ trackpoint_dev = psmouse->private;
+ trackpoint_dev->parent_psmouse = psmouse;
+
if (variant_id != TP_VARIANT_IBM) {
/* Newer variants do not support extended button query. */
button_info = 0x33;
@@ -470,6 +615,10 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
psmouse->vendor, firmware_id,
(button_info & 0xf0) >> 4, button_info & 0x0f);
+ tp->doubletap_capable = is_trackpoint_dt_capable(ps2dev->serio->firmware_id);
+ if (tp->doubletap_capable)
+ device_create_file(&psmouse->ps2dev.serio->dev, &dev_attr_doubletap_enabled);
+
return 0;
}
diff --git a/drivers/input/mouse/trackpoint.h b/drivers/input/mouse/trackpoint.h
index eb5412904fe0..256e8cb35581 100644
--- a/drivers/input/mouse/trackpoint.h
+++ b/drivers/input/mouse/trackpoint.h
@@ -8,6 +8,8 @@
#ifndef _TRACKPOINT_H
#define _TRACKPOINT_H
+#include <linux/bitops.h>
+
/*
* These constants are from the TrackPoint System
* Engineering documentation Version 4 from IBM Watson
@@ -69,6 +71,8 @@
/* (how hard it is to drag */
/* with Z-axis pressed) */
+#define TP_DOUBLETAP 0x58 /* TrackPoint doubletap register */
+
#define TP_MINDRAG 0x59 /* Minimum amount of force needed */
/* to trigger dragging */
@@ -139,6 +143,14 @@
#define TP_DEF_TWOHAND 0x00
#define TP_DEF_SOURCE_TAG 0x00
+/* Doubletap register values */
+#define TP_DOUBLETAP_ENABLE 0xFF /* Enable value */
+#define TP_DOUBLETAP_DISABLE 0xFE /* Disable value */
+
+#define TP_DOUBLETAP_STATUS_BIT 0 /* 0th bit defines enable/disable */
+
+#define TP_DOUBLETAP_STATUS BIT(TP_DOUBLETAP_STATUS_BIT)
+
#define MAKE_PS2_CMD(params, results, cmd) ((params<<12) | (results<<8) | (cmd))
struct trackpoint_data {
@@ -150,11 +162,14 @@ struct trackpoint_data {
u8 thresh, upthresh;
u8 ztime, jenks;
u8 drift_time;
+ bool doubletap_capable;
/* toggles */
bool press_to_select;
bool skipback;
bool ext_dev;
+
+ struct psmouse *parent_psmouse;
};
int trackpoint_detect(struct psmouse *psmouse, bool set_properties);
--
2.48.1
^ permalink raw reply related
* [PATCH] HID: lenovo: Use KEY_PERFORMANCE instead of ACPI's platform_profile
From: Janne Grunau @ 2025-09-01 10:20 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Vishnu Sankar
Cc: linux-input, regressions, linux-kernel, stable, Janne Grunau
Commit 84c9d2a968c82 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd
Fn keys") added a dependency on ACPI's platform_profile. This should not
be done for generic USB devices as this prevents using the devices on
non ACPI devices like Apple silicon Macs and other non-ACPI arm64
systems. An attempt to allow using platform_profile on non-ACPI systems
was rejected in [1] and instead platform_profile was made to fail during
init in commit dd133162c9cf ("ACPI: platform_profile: Avoid initializing
on non-ACPI platforms").
So remove the broken dependency and instead let's user space handle this
keycode by sending the new KEY_PERFORMANCE. Stable backport depends on
commit 89c5214639294 ("Input: add keycode for performance mode key").
[1]: https://lore.kernel.org/linux-acpi/CAJZ5v0icRdTSToaKbdf=MdRin4NyB2MstUVaQo8VR6-n7DkVMQ@mail.gmail.com/
Cc: regressions@lists.linux.dev
Cc: stable@vger.kernel.org
Fixes: 84c9d2a968c82 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd Fn keys")
Signed-off-by: Janne Grunau <j@jannau.net>
---
#regzbot introduced: 84c9d2a968c82
---
drivers/hid/Kconfig | 2 --
drivers/hid/hid-lenovo.c | 4 +---
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a57901203aeb284acd23be727d2fad0c137c101c..8ae63f8257cd582448e9683ca7fc654c8e465c0f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -597,8 +597,6 @@ config HID_LED
config HID_LENOVO
tristate "Lenovo / Thinkpad devices"
- depends on ACPI
- select ACPI_PLATFORM_PROFILE
select NEW_LEDS
select LEDS_CLASS
help
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index b3121fa7a72d73f2b9ac12f36bc3d87c2649c69b..654879814f97aaf876ac16c00bf9efca22d116f3 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -32,8 +32,6 @@
#include <linux/leds.h>
#include <linux/workqueue.h>
-#include <linux/platform_profile.h>
-
#include "hid-ids.h"
/* Userspace expects F20 for mic-mute KEY_MICMUTE does not work */
@@ -734,7 +732,7 @@ static int lenovo_raw_event_TP_X12_tab(struct hid_device *hdev, u32 raw_data)
report_key_event(input, KEY_RFKILL);
return 1;
}
- platform_profile_cycle();
+ report_key_event(input, KEY_PERFORMANCE);
return 1;
case TP_X12_RAW_HOTKEY_FN_F10:
/* TAB1 has PICKUP Phone and TAB2 use Snipping tool*/
---
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
change-id: 20250901-hid-lenovo-drop-platform_profile-d59013f79a59
Best regards,
--
Janne Grunau <j@jannau.net>
^ permalink raw reply related
* Re: [PATCH v4] HID: lg-g15 - Add support for Logitech G13.
From: Kate Hsuan @ 2025-09-01 7:56 UTC (permalink / raw)
To: Leo L. Schwab
Cc: Hans de Goede, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <20250901063108.217295-2-ewhac@ewhac.org>
Hi Leo,
On Mon, Sep 1, 2025 at 2:34 PM Leo L. Schwab <ewhac@ewhac.org> wrote:
>
> The Logitech G13 is a gaming keypad with general-purpose macro keys,
> four LED-backlit macro preset keys, five "menu" keys, backlight toggle
> key, an analog thumbstick, RGB LED backlight, and a monochrome LCD
> display.
>
> Support input event generation for all keys and the thumbstick, and
> expose all LEDs.
>
> Signed-off-by: Leo L. Schwab <ewhac@ewhac.org>
> Reviewed-by: Hans de Goede <hansg@kernel.org>
> ---
> Changes in v4:
> - Minor changes recommended by Hans de Goede <hansg@kernel.org>.
> Changes in v3:
> - Re-revise commit message.
> - Conditionally compile the section depending on
> CONFIG_LEDS_BRIGHTNESS_HW_CHANGED correctly this time.
> - Use led-class-multicolor facilities for the RGB backlight.
> - Changes recommended by Kate Hsuan <hpa@redhat.com>:
> - Use guard(mutex) construct.
> - Fix numerous style nits.
> Changes in v2:
> - Add `#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED` bracket around new
> code segment dependent on that feature (fixes test robot build
> error).
> - Use `guard(mutex)` construct in new code (existing code left
> unmodified).
> - Commit message revised.
>
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-lg-g15.c | 426 +++++++++++++++++++++++++++++++++++++--
> 2 files changed, 411 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 33cc5820f2be..7ed1e402b80a 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -870,6 +870,7 @@
> #define USB_DEVICE_ID_LOGITECH_DUAL_ACTION 0xc216
> #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2 0xc218
> #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2 0xc219
> +#define USB_DEVICE_ID_LOGITECH_G13 0xc21c
> #define USB_DEVICE_ID_LOGITECH_G15_LCD 0xc222
> #define USB_DEVICE_ID_LOGITECH_G11 0xc225
> #define USB_DEVICE_ID_LOGITECH_G15_V2_LCD 0xc227
> diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c
> index f8605656257b..62cb795c2393 100644
> --- a/drivers/hid/hid-lg-g15.c
> +++ b/drivers/hid/hid-lg-g15.c
> @@ -26,7 +26,11 @@
> #define LG_G510_FEATURE_BACKLIGHT_RGB 0x05
> #define LG_G510_FEATURE_POWER_ON_RGB 0x06
>
> +#define LG_G13_FEATURE_M_KEYS_LEDS 0x05
> +#define LG_G13_FEATURE_BACKLIGHT_RGB 0x07
> +
> enum lg_g15_model {
> + LG_G13,
> LG_G15,
> LG_G15_V2,
> LG_G510,
> @@ -45,6 +49,12 @@ enum lg_g15_led_type {
> LG_G15_LED_MAX
> };
>
> +struct g13_input_report {
> + u8 report_id; /* Report ID is always set to 1. */
> + u8 joy_x, joy_y;
> + u8 keybits[5];
> +};
> +
> struct lg_g15_led {
> union {
> struct led_classdev cdev;
> @@ -63,12 +73,172 @@ struct lg_g15_data {
> struct mutex mutex;
> struct work_struct work;
> struct input_dev *input;
> + struct input_dev *input_js; /* Separate joystick device for G13. */
> struct hid_device *hdev;
> enum lg_g15_model model;
> struct lg_g15_led leds[LG_G15_LED_MAX];
> bool game_mode_enabled;
> };
>
> +/********* G13 LED functions ***********/
> +/*
> + * G13 retains no state across power cycles, and always powers up with the backlight on,
> + * color #5AFF6E, all macro key LEDs off.
> + */
> +static int lg_g13_get_leds_state(struct lg_g15_data *g15)
> +{
> + u8 * const tbuf = g15->transfer_buf;
> + int ret, high;
> +
> + /* RGB backlight. */
> + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB,
> + tbuf, 5,
> + HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> + if (ret != 5) {
> + hid_err(g15->hdev, "Error getting backlight brightness: %d\n", ret);
> + return (ret < 0) ? ret : -EIO;
> + }
> +
> + /* Normalize RGB intensities against the highest component. */
> + high = max3(tbuf[1], tbuf[2], tbuf[3]);
> + if (high) {
> + g15->leds[LG_G15_KBD_BRIGHTNESS].red =
> + DIV_ROUND_CLOSEST(tbuf[1] * 255, high);
> + g15->leds[LG_G15_KBD_BRIGHTNESS].green =
> + DIV_ROUND_CLOSEST(tbuf[2] * 255, high);
> + g15->leds[LG_G15_KBD_BRIGHTNESS].blue =
> + DIV_ROUND_CLOSEST(tbuf[3] * 255, high);
> + g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = high;
> + } else {
> + g15->leds[LG_G15_KBD_BRIGHTNESS].red = 255;
> + g15->leds[LG_G15_KBD_BRIGHTNESS].green = 255;
> + g15->leds[LG_G15_KBD_BRIGHTNESS].blue = 255;
> + g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = 0;
> + }
> +
> + /* Macro LEDs. */
> + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS,
> + tbuf, 5,
> + HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> + if (ret != 5) {
> + hid_err(g15->hdev, "Error getting macro LED brightness: %d\n", ret);
> + return (ret < 0) ? ret : -EIO;
> + }
> +
> + for (int i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i)
> + g15->leds[i].brightness = tbuf[1] & (1 << (i - LG_G15_MACRO_PRESET1));
> +
> + return 0;
> +}
> +
> +static int lg_g13_kbd_led_write(struct lg_g15_data *g15,
> + struct lg_g15_led *g15_led,
> + enum led_brightness brightness)
> +{
> + struct mc_subled const * const subleds = g15_led->mcdev.subled_info;
> + u8 * const tbuf = g15->transfer_buf;
> + int ret;
> +
> + guard(mutex)(&g15->mutex);
> +
> + led_mc_calc_color_components(&g15_led->mcdev, brightness);
> +
> + tbuf[0] = 5;
> + tbuf[1] = subleds[0].brightness;
> + tbuf[2] = subleds[1].brightness;
> + tbuf[3] = subleds[2].brightness;
> + tbuf[4] = 0;
> +
> + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB,
> + tbuf, 5,
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + if (ret != 5) {
> + hid_err(g15->hdev, "Error setting backlight brightness: %d\n", ret);
> + return (ret < 0) ? ret : -EIO;
> + }
> +
> + g15_led->brightness = brightness;
> + return 0;
> +}
> +
> +static int lg_g13_kbd_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
> +{
> + struct led_classdev_mc *mc = lcdev_to_mccdev(led_cdev);
> + struct lg_g15_led *g15_led =
> + container_of(mc, struct lg_g15_led, mcdev);
> + struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
> +
> + /* Ignore LED off on unregister / keyboard unplug */
> + if (led_cdev->flags & LED_UNREGISTERING)
> + return 0;
> +
> + return lg_g13_kbd_led_write(g15, g15_led, brightness);
> +}
> +
> +static enum led_brightness lg_g13_kbd_led_get(struct led_classdev *led_cdev)
> +{
> + struct led_classdev_mc const * const mc = lcdev_to_mccdev(led_cdev);
> + struct lg_g15_led const *g15_led =
> + container_of(mc, struct lg_g15_led, mcdev);
> +
> + return g15_led->brightness;
> +}
> +
> +static int lg_g13_mkey_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
> +{
> + struct lg_g15_led *g15_led =
> + container_of(led_cdev, struct lg_g15_led, cdev);
> + struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
> + int i, ret;
> + u8 * const tbuf = g15->transfer_buf;
> + u8 val, mask = 0;
> +
> + /* Ignore LED off on unregister / keyboard unplug */
> + if (led_cdev->flags & LED_UNREGISTERING)
> + return 0;
> +
> + guard(mutex)(&g15->mutex);
> +
> + for (i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i) {
> + if (i == g15_led->led)
> + val = brightness;
> + else
> + val = g15->leds[i].brightness;
> +
> + if (val)
> + mask |= 1 << (i - LG_G15_MACRO_PRESET1);
> + }
> +
> + tbuf[0] = 5;
> + tbuf[1] = mask;
> + tbuf[2] = 0;
> + tbuf[3] = 0;
> + tbuf[4] = 0;
> +
> + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS,
> + tbuf, 5,
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + if (ret != 5) {
> + hid_err(g15->hdev, "Error setting LED brightness: %d\n", ret);
> + return (ret < 0) ? ret : -EIO;
> + }
> +
> + g15_led->brightness = brightness;
> + return 0;
> +}
> +
> +static enum led_brightness lg_g13_mkey_led_get(struct led_classdev *led_cdev)
> +{
> + /*
> + * G13 doesn't change macro key LEDs behind our back, so they're
> + * whatever we last set them to.
> + */
> + struct lg_g15_led *g15_led =
> + container_of(led_cdev, struct lg_g15_led, cdev);
> +
> + return g15_led->brightness;
> +}
> +
> /******** G15 and G15 v2 LED functions ********/
>
> static int lg_g15_update_led_brightness(struct lg_g15_data *g15)
> @@ -390,6 +560,8 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15)
> int ret;
>
> switch (g15->model) {
> + case LG_G13:
> + return lg_g13_get_leds_state(g15);
> case LG_G15:
> case LG_G15_V2:
> return lg_g15_update_led_brightness(g15);
> @@ -417,6 +589,116 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15)
>
> /******** Input functions ********/
>
> +/**
> + * g13_input_report.keybits[] is not 32-bit aligned, so we can't use the bitops macros.
> + *
> + * @ary: Pointer to array of u8s
> + * @b: Bit index into ary, LSB first. Not range checked.
> + */
> +#define TEST_BIT(ary, b) ((1 << ((b) & 7)) & (ary)[(b) >> 3])
> +
> +/* Table mapping keybits[] bit positions to event codes. */
> +/* Note: Indices are discontinuous to aid readability. */
> +static const u16 g13_keys_for_bits[] = {
> + /* Main keypad - keys G1 - G22 */
> + [0] = KEY_MACRO1,
> + [1] = KEY_MACRO2,
> + [2] = KEY_MACRO3,
> + [3] = KEY_MACRO4,
> + [4] = KEY_MACRO5,
> + [5] = KEY_MACRO6,
> + [6] = KEY_MACRO7,
> + [7] = KEY_MACRO8,
> + [8] = KEY_MACRO9,
> + [9] = KEY_MACRO10,
> + [10] = KEY_MACRO11,
> + [11] = KEY_MACRO12,
> + [12] = KEY_MACRO13,
> + [13] = KEY_MACRO14,
> + [14] = KEY_MACRO15,
> + [15] = KEY_MACRO16,
> + [16] = KEY_MACRO17,
> + [17] = KEY_MACRO18,
> + [18] = KEY_MACRO19,
> + [19] = KEY_MACRO20,
> + [20] = KEY_MACRO21,
> + [21] = KEY_MACRO22,
> +
> + /* LCD menu buttons. */
> + [24] = KEY_KBD_LCD_MENU5, /* "Next page" button */
> + [25] = KEY_KBD_LCD_MENU1, /* Left-most */
> + [26] = KEY_KBD_LCD_MENU2,
> + [27] = KEY_KBD_LCD_MENU3,
> + [28] = KEY_KBD_LCD_MENU4, /* Right-most */
> +
> + /* Macro preset and record buttons; have red LEDs under them. */
> + [29] = KEY_MACRO_PRESET1,
> + [30] = KEY_MACRO_PRESET2,
> + [31] = KEY_MACRO_PRESET3,
> + [32] = KEY_MACRO_RECORD_START,
> +
> + /* 33-35 handled by joystick device. */
> +
> + /* Backlight toggle. */
> + [37] = KEY_LIGHTS_TOGGLE,
> +};
> +
> +#define G13_JS_KEYBITS_OFFSET 33
> +
> +static const u16 g13_keys_for_bits_js[] = {
> + /* Joystick buttons */
> + /* These keybits are at bit indices 33, 34, and 35. */
> + BTN_BASE, /* Left side */
> + BTN_BASE2, /* Bottom side */
> + BTN_THUMB, /* Stick depress */
> +};
> +
> +static int lg_g13_event(struct lg_g15_data *g15, u8 const *data)
> +{
> + struct g13_input_report const * const rep = (struct g13_input_report *) data;
> + int i, val;
> +
> + /*
> + * Main macropad and menu keys.
> + * Emit key events defined for each bit position.
> + */
> + for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) {
> + if (g13_keys_for_bits[i]) {
> + val = TEST_BIT(rep->keybits, i);
> + input_report_key(g15->input, g13_keys_for_bits[i], val);
> + }
> + }
> + input_sync(g15->input);
> +
> + /*
> + * Joystick.
> + * Emit button and deflection events.
> + */
> + for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) {
> + val = TEST_BIT(rep->keybits, i + G13_JS_KEYBITS_OFFSET);
> + input_report_key(g15->input_js, g13_keys_for_bits_js[i], val);
> + }
> + input_report_abs(g15->input_js, ABS_X, rep->joy_x);
> + input_report_abs(g15->input_js, ABS_Y, rep->joy_y);
> + input_sync(g15->input_js);
> +
> + if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)) {
> + /*
> + * Bit 23 of keybits[] reports the current backlight on/off
> + * state. If it has changed from the last cached value, apply
> + * an update.
> + */
> + bool hw_brightness_changed = (!!TEST_BIT(rep->keybits, 23))
> + ^ (g15->leds[0].cdev.brightness_hw_changed > 0);
> + if (hw_brightness_changed)
> + led_classdev_notify_brightness_hw_changed(
> + &g15->leds[0].cdev,
> + TEST_BIT(rep->keybits, 23) ? LED_FULL : LED_OFF);
> + }
> +
> + return 0;
> +}
> +
> /* On the G15 Mark I Logitech has been quite creative with which bit is what */
> static void lg_g15_handle_lcd_menu_keys(struct lg_g15_data *g15, u8 *data)
> {
> @@ -572,6 +854,10 @@ static int lg_g15_raw_event(struct hid_device *hdev, struct hid_report *report,
> return 0;
>
> switch (g15->model) {
> + case LG_G13:
> + if (data[0] == 0x01 && size == sizeof(struct g13_input_report))
> + return lg_g13_event(g15, data);
> + break;
> case LG_G15:
> if (data[0] == 0x02 && size == 9)
> return lg_g15_event(g15, data);
> @@ -616,13 +902,22 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index)
> {
> int i;
> struct mc_subled *subled_info;
> -
> - g15->leds[index].mcdev.led_cdev.brightness_set_blocking =
> - lg_g510_kbd_led_set;
> - g15->leds[index].mcdev.led_cdev.brightness_get =
> - lg_g510_kbd_led_get;
> - g15->leds[index].mcdev.led_cdev.max_brightness = 255;
> - g15->leds[index].mcdev.num_colors = 3;
> + struct lg_g15_led * const gled = &g15->leds[index];
> +
> + if (g15->model == LG_G13) {
> + gled->mcdev.led_cdev.brightness_set_blocking =
> + lg_g13_kbd_led_set;
> + gled->mcdev.led_cdev.brightness_get =
> + lg_g13_kbd_led_get;
> + gled->mcdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED;
> + } else {
> + gled->mcdev.led_cdev.brightness_set_blocking =
> + lg_g510_kbd_led_set;
> + gled->mcdev.led_cdev.brightness_get =
> + lg_g510_kbd_led_get;
> + }
> + gled->mcdev.led_cdev.max_brightness = 255;
> + gled->mcdev.num_colors = 3;
>
> subled_info = devm_kcalloc(&g15->hdev->dev, 3, sizeof(*subled_info), GFP_KERNEL);
> if (!subled_info)
> @@ -632,20 +927,20 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index)
> switch (i + 1) {
> case LED_COLOR_ID_RED:
> subled_info[i].color_index = LED_COLOR_ID_RED;
> - subled_info[i].intensity = g15->leds[index].red;
> + subled_info[i].intensity = gled->red;
> break;
> case LED_COLOR_ID_GREEN:
> subled_info[i].color_index = LED_COLOR_ID_GREEN;
> - subled_info[i].intensity = g15->leds[index].green;
> + subled_info[i].intensity = gled->green;
> break;
> case LED_COLOR_ID_BLUE:
> subled_info[i].color_index = LED_COLOR_ID_BLUE;
> - subled_info[i].intensity = g15->leds[index].blue;
> + subled_info[i].intensity = gled->blue;
> break;
> }
> subled_info[i].channel = i;
> }
> - g15->leds[index].mcdev.subled_info = subled_info;
> + gled->mcdev.subled_info = subled_info;
> }
>
> static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
> @@ -656,6 +951,23 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
> g15->leds[i].cdev.name = name;
>
> switch (g15->model) {
> + case LG_G13:
> + if (i < LG_G15_BRIGHTNESS_MAX) {
> + /* RGB backlight. */
> + lg_g15_setup_led_rgb(g15, i);
> + ret = devm_led_classdev_multicolor_register_ext(&g15->hdev->dev,
> + &g15->leds[i].mcdev,
> + NULL);
> + } else {
> + /* Macro keys */
> + g15->leds[i].cdev.brightness_set_blocking = lg_g13_mkey_led_set;
> + g15->leds[i].cdev.brightness_get = lg_g13_mkey_led_get;
> + g15->leds[i].cdev.max_brightness = 1;
> +
> + ret = devm_led_classdev_register(&g15->hdev->dev,
> + &g15->leds[i].cdev);
> + }
> + break;
> case LG_G15:
> case LG_G15_V2:
> g15->leds[i].cdev.brightness_get = lg_g15_led_get;
> @@ -702,11 +1014,9 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
> }
>
> /* Common input device init code shared between keyboards and Z-10 speaker handling */
> -static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input,
> - const char *name)
> +static void lg_g15_init_input_dev_core(struct hid_device *hdev, struct input_dev *input,
> + char const *name)
> {
> - int i;
> -
> input->name = name;
> input->phys = hdev->phys;
> input->uniq = hdev->uniq;
> @@ -717,12 +1027,42 @@ static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *inp
> input->dev.parent = &hdev->dev;
> input->open = lg_g15_input_open;
> input->close = lg_g15_input_close;
> +}
> +
> +static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input,
> + const char *name)
> +{
> + int i;
> +
> + lg_g15_init_input_dev_core(hdev, input, name);
>
> /* Keys below the LCD, intended for controlling a menu on the LCD */
> for (i = 0; i < 5; i++)
> input_set_capability(input, EV_KEY, KEY_KBD_LCD_MENU1 + i);
> }
>
> +static void lg_g13_init_input_dev(struct hid_device *hdev,
> + struct input_dev *input, const char *name,
> + struct input_dev *input_js, const char *name_js)
> +{
> + /* Macropad. */
> + lg_g15_init_input_dev_core(hdev, input, name);
> + for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) {
> + if (g13_keys_for_bits[i])
> + input_set_capability(input, EV_KEY, g13_keys_for_bits[i]);
> + }
> +
> + /* OBTW, we're a joystick, too... */
> + lg_g15_init_input_dev_core(hdev, input_js, name_js);
> + for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i)
> + input_set_capability(input_js, EV_KEY, g13_keys_for_bits_js[i]);
> +
> + input_set_capability(input_js, EV_ABS, ABS_X);
> + input_set_abs_params(input_js, ABS_X, 0, 255, 0, 0);
> + input_set_capability(input_js, EV_ABS, ABS_Y);
> + input_set_abs_params(input_js, ABS_Y, 0, 255, 0, 0);
> +}
> +
> static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> {
> static const char * const led_names[] = {
> @@ -739,7 +1079,7 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> unsigned int connect_mask = 0;
> bool has_ff000000 = false;
> struct lg_g15_data *g15;
> - struct input_dev *input;
> + struct input_dev *input, *input_js;
> struct hid_report *rep;
> int ret, i, gkeys = 0;
>
> @@ -778,6 +1118,25 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hid_set_drvdata(hdev, (void *)g15);
>
> switch (g15->model) {
> + case LG_G13:
> + /*
> + * The G13 has an analog thumbstick with nearby buttons. Some
> + * libraries and applications are known to ignore devices that
> + * don't "look like" a joystick, and a device with two ABS axes
> + * and 25+ macro keys would confuse them.
> + *
> + * Create an additional input device dedicated to appear as a
> + * simplified joystick (two ABS axes, three BTN buttons).
> + */
> + input_js = devm_input_allocate_device(&hdev->dev);
> + if (!input_js)
> + return -ENOMEM;
> + g15->input_js = input_js;
> + input_set_drvdata(input_js, hdev);
> +
> + connect_mask = HID_CONNECT_HIDRAW;
> + gkeys = 25;
> + break;
> case LG_G15:
> INIT_WORK(&g15->work, lg_g15_leds_changed_work);
> /*
> @@ -859,6 +1218,34 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> goto error_hw_stop;
>
> return 0; /* All done */
> + } else if (g15->model == LG_G13) {
> + static char const * const g13_led_names[] = {
> + /* Backlight is shared between LCD and keys. */
> + "g13:rgb:kbd_backlight",
> + NULL, /* Keep in sync with led_type enum */
> + "g13:red:macro_preset_1",
> + "g13:red:macro_preset_2",
> + "g13:red:macro_preset_3",
> + "g13:red:macro_record",
> + };
> + lg_g13_init_input_dev(hdev,
> + input, "Logitech G13 Gaming Keypad",
> + input_js, "Logitech G13 Thumbstick");
> + ret = input_register_device(input);
> + if (ret)
> + goto error_hw_stop;
> + ret = input_register_device(input_js);
> + if (ret)
> + goto error_hw_stop;
> +
> + for (i = 0; i < ARRAY_SIZE(g13_led_names); ++i) {
> + if (g13_led_names[i]) {
> + ret = lg_g15_register_led(g15, i, g13_led_names[i]);
> + if (ret)
> + goto error_hw_stop;
> + }
> + }
> + return 0;
> }
>
> /* Setup and register input device */
> @@ -903,6 +1290,13 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> }
>
> static const struct hid_device_id lg_g15_devices[] = {
> + /*
> + * The G13 is a macropad-only device with an LCD, LED backlighing,
> + * and joystick.
> + */
> + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> + USB_DEVICE_ID_LOGITECH_G13),
> + .driver_data = LG_G13 },
> /* The G11 is a G15 without the LCD, treat it as a G15 */
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_G11),
> --
> 2.51.0
>
Thank you for your work.
I tested it on my G510 and G13 and the results are shown as follows.
1. G510 keyboard backlight works properly and the changes didn't
impact the G510 keyboard backlight control.
2. G13 keyboard backlight works and the color and brightness can be controlled.
3. The keypad event works.
4. G13 Joystick works.
Tested-by: Kate Hsuan <hpa@redhat.com>
--
BR,
Kate
^ permalink raw reply
* Re: [PATCH v3] HID: lg-g15 - Add support for Logitech G13.
From: Leo L. Schwab @ 2025-08-31 19:51 UTC (permalink / raw)
To: Hans de Goede
Cc: Kate Hsuan, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <7d356834-5795-4979-9f51-0ffcec52ae1d@kernel.org>
On Sun, Aug 31, 2025 at 03:01:12PM +0200, Hans de Goede wrote:
> > +static const u16 g13_keys_for_bits_js[] = {
> > + /* Joystick buttons */
> > + /* These keybits are at bit indices 33, 34, and 35. */
> > + BTN_BASE, /* Left side */
> > + BTN_BASE2, /* Bottom side */
> > + BTN_THUMB, /* Stick depress */
> > +};
>
> You are using this 33 offset hardcoded below, maybe
> at a #define for this, e.g. :
>
> #define G13_JS_KEYBITS_OFFSET 33
>
Noted.
> g13_keys_for_bits_js[] is contiguous so no need
> for this if (g13_keys_for_bits_js[i]) test.
>
Noted.
> > + if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)) {
>
> I do not believe that this IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)
> is necessary, led_classdev_notify_brightness_hw_changed() has a static
> inline replacement when CONFIG_LEDS_BRIGHTNESS_HW_CHANGED is not set,
> so you can just call it unconditionally.
>
> This is already called unconditionally in other places of the code.
>
I was actually bit by this in the first two revs by the build bot.
If CONFIG_LEDS_BRIGHTNESS_HW_CHANGED is not enabled, the field
`cdev.brightness_hw_changed`, referenced a bit further down, does not exist,
and causes a build failure.
My first attempt at #ifdef-ing around it led to another build bot
warning about `hw_brightness_changed` being defined but not used. Then I
leanred about `IS_ENABLED()`, which is evidently now preferred over `#ifdef
CONFIG_`, and nicely isolated the whole block, so I went with that.
> g13_keys_for_bits_js[] is contiguous so no need
> for this if (g13_keys_for_bits_js[i]) test.
>
> This will also allow you to drop the {} from the for loop.
>
Noted.
I've been mucking around with Rust a bit lately (and badly indented
JavaScript rather more), and am now in the habit putting braces around
single-line `if` clauses.
> Besides from my few small remarks this looks good to me,
> feel free to add:
>
> Reviewed-by: Hans de Goede <hansg@kernel.org>
>
> to the next version.
>
Thank you very kindly, sir!
Schwab
^ permalink raw reply
* Re: [PATCH v3 RESEND] HID: logitech-dj: Add support for a new lightspeed receiver iteration
From: Stuart Hayhurst @ 2025-08-31 19:24 UTC (permalink / raw)
To: mavchatz; +Cc: benjamin.tissoires, hadess, jikos, lains, linux-input
In-Reply-To: <20240627224242.193873-1-mavchatz@protonmail.com>
Is there any chance this can be updated for 6.16 / 6.17?
I'd like to test this with my G915 TKL, I came up with almost exactly this solution myself, then found this while troubleshooting it...
Thanks,
Stuart
^ permalink raw reply
* Re: [PATCH v3] HID: lg-g15 - Add support for Logitech G13.
From: Hans de Goede @ 2025-08-31 13:01 UTC (permalink / raw)
To: Leo L. Schwab
Cc: Kate Hsuan, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <20250814212641.197573-2-ewhac@ewhac.org>
Hi Leo,
On 14-Aug-25 11:26 PM, Leo L. Schwab wrote:
> The Logitech G13 is a gaming keypad with general-purpose macro keys,
> four LED-backlit macro preset keys, five "menu" keys, backlight toggle
> key, an analog thumbstick, RGB LED backlight, and a monochrome LCD
> display.
>
> Support input event generation for all keys and the thumbstick, and
> expose all LEDs.
>
> Signed-off-by: Leo L. Schwab <ewhac@ewhac.org>
Thank you for your patch overal this looks good to me,
some remarks inline below.
...
> diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c
> index f8605656257b..6e1d79abfb23 100644
> --- a/drivers/hid/hid-lg-g15.c
> +++ b/drivers/hid/hid-lg-g15.c
...
> +/**
> + * g13_input_report.keybits[] is not 32-bit aligned, so we can't use the bitops macros.
> + *
> + * @ary: Pointer to array of u8s
> + * @b: Bit index into ary, LSB first. Not range checked.
> + */
> +#define TEST_BIT(ary, b) ((1 << ((b) & 7)) & (ary)[(b) >> 3])
> +
> +/* Table mapping keybits[] bit positions to event codes. */
> +/* Note: Indices are discontinuous to aid readability. */
> +static const u16 g13_keys_for_bits[] = {
> + /* Main keypad - keys G1 - G22 */
> + [0] = KEY_MACRO1,
> + [1] = KEY_MACRO2,
> + [2] = KEY_MACRO3,
> + [3] = KEY_MACRO4,
> + [4] = KEY_MACRO5,
> + [5] = KEY_MACRO6,
> + [6] = KEY_MACRO7,
> + [7] = KEY_MACRO8,
> + [8] = KEY_MACRO9,
> + [9] = KEY_MACRO10,
> + [10] = KEY_MACRO11,
> + [11] = KEY_MACRO12,
> + [12] = KEY_MACRO13,
> + [13] = KEY_MACRO14,
> + [14] = KEY_MACRO15,
> + [15] = KEY_MACRO16,
> + [16] = KEY_MACRO17,
> + [17] = KEY_MACRO18,
> + [18] = KEY_MACRO19,
> + [19] = KEY_MACRO20,
> + [20] = KEY_MACRO21,
> + [21] = KEY_MACRO22,
> +
> + /* LCD menu buttons. */
> + [24] = KEY_KBD_LCD_MENU5, /* "Next page" button */
> + [25] = KEY_KBD_LCD_MENU1, /* Left-most */
> + [26] = KEY_KBD_LCD_MENU2,
> + [27] = KEY_KBD_LCD_MENU3,
> + [28] = KEY_KBD_LCD_MENU4, /* Right-most */
> +
> + /* Macro preset and record buttons; have red LEDs under them. */
> + [29] = KEY_MACRO_PRESET1,
> + [30] = KEY_MACRO_PRESET2,
> + [31] = KEY_MACRO_PRESET3,
> + [32] = KEY_MACRO_RECORD_START,
> +
> + /* 33-35 handled by joystick device. */
> +
> + /* Backlight toggle. */
> + [37] = KEY_LIGHTS_TOGGLE,
> +};
> +
> +static const u16 g13_keys_for_bits_js[] = {
> + /* Joystick buttons */
> + /* These keybits are at bit indices 33, 34, and 35. */
> + BTN_BASE, /* Left side */
> + BTN_BASE2, /* Bottom side */
> + BTN_THUMB, /* Stick depress */
> +};
You are using this 33 offset hardcoded below, maybe
at a #define for this, e.g. :
#define G13_JS_KEYBITS_OFFSET 33
> +
> +static int lg_g13_event(struct lg_g15_data *g15, u8 const *data)
> +{
> + struct g13_input_report const * const rep = (struct g13_input_report *) data;
> + int i, val;
> +
> + /*
> + * Main macropad and menu keys.
> + * Emit key events defined for each bit position.
> + */
> + for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) {
> + if (g13_keys_for_bits[i]) {
> + val = TEST_BIT(rep->keybits, i);
> + input_report_key(g15->input, g13_keys_for_bits[i], val);
> + }
> + }
> + input_sync(g15->input);
> +
> + /*
> + * Joystick.
> + * Emit button and deflection events.
> + */
> + for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) {
> + if (g13_keys_for_bits_js[i]) {
g13_keys_for_bits_js[] is contiguous so no need
for this if (g13_keys_for_bits_js[i]) test.
> + val = TEST_BIT(rep->keybits, i + 33);
> + input_report_key(g15->input_js, g13_keys_for_bits_js[i], val);
> + }
> + }
> + input_report_abs(g15->input_js, ABS_X, rep->joy_x);
> + input_report_abs(g15->input_js, ABS_Y, rep->joy_y);
> + input_sync(g15->input_js);
> +
> + if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)) {
I do not believe that this IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)
is necessary, led_classdev_notify_brightness_hw_changed() has a static
inline replacement when CONFIG_LEDS_BRIGHTNESS_HW_CHANGED is not set,
so you can just call it unconditionally.
This is already called unconditionally in other places of the code.
> + /*
> + * Bit 23 of keybits[] reports the current backlight on/off
> + * state. If it has changed from the last cached value, apply
> + * an update.
> + */
> + bool hw_brightness_changed = (!!TEST_BIT(rep->keybits, 23))
> + ^ (g15->leds[0].cdev.brightness_hw_changed > 0);
> + if (hw_brightness_changed)
> + led_classdev_notify_brightness_hw_changed(
> + &g15->leds[0].cdev,
> + TEST_BIT(rep->keybits, 23) ? LED_FULL : LED_OFF);
> + }
> +
> + return 0;
> +}
> +
...
> +static void lg_g13_init_input_dev(struct hid_device *hdev,
> + struct input_dev *input, const char *name,
> + struct input_dev *input_js, const char *name_js)
> +{
> + /* Macropad. */
> + lg_g15_init_input_dev_core(hdev, input, name);
> + for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) {
> + if (g13_keys_for_bits[i])
> + input_set_capability(input, EV_KEY, g13_keys_for_bits[i]);
> + }
> +
> + /* OBTW, we're a joystick, too... */
> + lg_g15_init_input_dev_core(hdev, input_js, name_js);
> + for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) {
> + if (g13_keys_for_bits_js[i])
> + input_set_capability(input_js, EV_KEY, g13_keys_for_bits_js[i]);
g13_keys_for_bits_js[] is contiguous so no need
for this if (g13_keys_for_bits_js[i]) test.
This will also allow you to drop the {} from the for loop.
> + }
> +
> + input_set_capability(input_js, EV_ABS, ABS_X);
> + input_set_abs_params(input_js, ABS_X, 0, 255, 0, 0);
> + input_set_capability(input_js, EV_ABS, ABS_Y);
> + input_set_abs_params(input_js, ABS_Y, 0, 255, 0, 0);
> +}
> +
> static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> {
> static const char * const led_names[] = {
...
Besides from my few small remarks this looks good to me,
feel free to add:
Reviewed-by: Hans de Goede <hansg@kernel.org>
to the next version.
Regards,
Hans
^ permalink raw reply
* [PATCH] HID: LOFREE: Fix back/forward buttons for
From: stepeos @ 2025-08-31 9:53 UTC (permalink / raw)
To: jikos; +Cc: linux-input, linux-kernel, Steve Oswald
From: Steve Oswald <stevepeter.oswald@gmail.com>
---
drivers/hid/hid-ids.h | 3 ++
drivers/hid/hid-lofree.c | 96 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+)
create mode 100644 drivers/hid/hid-lofree.c
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 33cc5820f2be..cb2704550043 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -848,6 +848,9 @@
#define USB_DEVICE_ID_PXN_V12_LITE_2 0x1211
#define USB_DEVICE_LITE_STAR_GT987_FF 0x2141
+#define USB_VENDOR_ID_LOFREE 0x248A
+#define USB_VENDOR_ID_LOFREE_TOUCH 0x8266
+
#define USB_VENDOR_ID_LOGITECH 0x046d
#define USB_DEVICE_ID_LOGITECH_Z_10_SPK 0x0a07
#define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
diff --git a/drivers/hid/hid-lofree.c b/drivers/hid/hid-lofree.c
new file mode 100644
index 000000000000..89383527e519
--- /dev/null
+++ b/drivers/hid/hid-lofree.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Custom HID driver for Lofree products
+ * Adds support for Back (BTN_SIDE) and Forward (BTN_EXTRA) buttons
+ *
+ * Copyright (c) 2025 Steve Oswald
+ * based on hid-cherry
+ */
+
+#include <linux/module.h>
+#include <linux/hid.h>
+#include <linux/input.h>
+#include "hid-ids.h"
+
+/*
+ * Remaps back/forward buttons to input events for Lofree Touch.
+ */
+static int lofree_touch_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data, int size)
+{
+ struct hid_input *hidinput;
+ struct input_dev *input;
+
+ if (report->id != 3)
+ return 0;
+
+ list_for_each_entry(hidinput, &hdev->inputs, list) {
+ input = hidinput->input;
+ input_report_key(input, BTN_SIDE, !!(data[1] & 0x08));
+ input_report_key(input, BTN_EXTRA, !!(data[1] & 0x10));
+ input_sync(input);
+ }
+
+ return 0;
+}
+
+/*
+ * Apply capabilities.
+ */
+static int lofree_touch_input_configured(struct hid_device *hdev,
+ struct hid_input *hi)
+{
+ struct input_dev *input = hi->input;
+
+ input_set_capability(input, EV_KEY, BTN_SIDE);
+ input_set_capability(input, EV_KEY, BTN_EXTRA);
+
+ return 0;
+}
+
+static int lofree_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "HID parse failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret) {
+ hid_err(hdev, "HID hw start failed\n");
+ return ret;
+ }
+
+ hid_info(hdev, "Touch@LOFREE custom HID driver loaded\n");
+ return 0;
+}
+
+static void lofree_remove(struct hid_device *hdev)
+{
+ hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id lofree_touch_devices[] = {
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOFREE,
+ USB_VENDOR_ID_LOFREE_TOUCH) },
+ {}
+};
+MODULE_DEVICE_TABLE(hid, lofree_touch_devices);
+
+static struct hid_driver lofree_touch_driver = {
+ .name = "hid_lofree_touch",
+ .id_table = lofree_touch_devices,
+ .probe = lofree_probe,
+ .remove = lofree_remove,
+ .raw_event = lofree_touch_raw_event,
+ .input_configured = lofree_touch_input_configured,
+};
+
+module_hid_driver(lofree_touch_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION(
+ "HID driver for some special Lofree devices like Lofree Touch mouse.");
--
2.48.1
^ permalink raw reply related
* [PATCH] HID: LOFREE: Fix back/forward buttons for
From: stepeos @ 2025-08-31 9:52 UTC (permalink / raw)
To: stevepeter.oswald; +Cc: linux-input, linux-kernel
From: Steve Oswald <stevepeter.oswald@gmail.com>
---
drivers/hid/hid-ids.h | 3 ++
drivers/hid/hid-lofree.c | 96 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+)
create mode 100644 drivers/hid/hid-lofree.c
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 33cc5820f2be..cb2704550043 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -848,6 +848,9 @@
#define USB_DEVICE_ID_PXN_V12_LITE_2 0x1211
#define USB_DEVICE_LITE_STAR_GT987_FF 0x2141
+#define USB_VENDOR_ID_LOFREE 0x248A
+#define USB_VENDOR_ID_LOFREE_TOUCH 0x8266
+
#define USB_VENDOR_ID_LOGITECH 0x046d
#define USB_DEVICE_ID_LOGITECH_Z_10_SPK 0x0a07
#define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
diff --git a/drivers/hid/hid-lofree.c b/drivers/hid/hid-lofree.c
new file mode 100644
index 000000000000..89383527e519
--- /dev/null
+++ b/drivers/hid/hid-lofree.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Custom HID driver for Lofree products
+ * Adds support for Back (BTN_SIDE) and Forward (BTN_EXTRA) buttons
+ *
+ * Copyright (c) 2025 Steve Oswald
+ * based on hid-cherry
+ */
+
+#include <linux/module.h>
+#include <linux/hid.h>
+#include <linux/input.h>
+#include "hid-ids.h"
+
+/*
+ * Remaps back/forward buttons to input events for Lofree Touch.
+ */
+static int lofree_touch_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data, int size)
+{
+ struct hid_input *hidinput;
+ struct input_dev *input;
+
+ if (report->id != 3)
+ return 0;
+
+ list_for_each_entry(hidinput, &hdev->inputs, list) {
+ input = hidinput->input;
+ input_report_key(input, BTN_SIDE, !!(data[1] & 0x08));
+ input_report_key(input, BTN_EXTRA, !!(data[1] & 0x10));
+ input_sync(input);
+ }
+
+ return 0;
+}
+
+/*
+ * Apply capabilities.
+ */
+static int lofree_touch_input_configured(struct hid_device *hdev,
+ struct hid_input *hi)
+{
+ struct input_dev *input = hi->input;
+
+ input_set_capability(input, EV_KEY, BTN_SIDE);
+ input_set_capability(input, EV_KEY, BTN_EXTRA);
+
+ return 0;
+}
+
+static int lofree_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "HID parse failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret) {
+ hid_err(hdev, "HID hw start failed\n");
+ return ret;
+ }
+
+ hid_info(hdev, "Touch@LOFREE custom HID driver loaded\n");
+ return 0;
+}
+
+static void lofree_remove(struct hid_device *hdev)
+{
+ hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id lofree_touch_devices[] = {
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOFREE,
+ USB_VENDOR_ID_LOFREE_TOUCH) },
+ {}
+};
+MODULE_DEVICE_TABLE(hid, lofree_touch_devices);
+
+static struct hid_driver lofree_touch_driver = {
+ .name = "hid_lofree_touch",
+ .id_table = lofree_touch_devices,
+ .probe = lofree_probe,
+ .remove = lofree_remove,
+ .raw_event = lofree_touch_raw_event,
+ .input_configured = lofree_touch_input_configured,
+};
+
+module_hid_driver(lofree_touch_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION(
+ "HID driver for some special Lofree devices like Lofree Touch mouse.");
--
2.48.1
^ permalink raw reply related
* Re: [PATCH v3] platform/x86: asus-wmi: map more keys on ExpertBook B9
From: Anton Khirnov @ 2025-08-31 9:21 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Hans de Goede, Corentin Chary, Luke D. Jones, Dmitry Torokhov,
linux-input, platform-driver-x86, LKML
In-Reply-To: <f4c39474-739f-855f-50ec-1aa0d24e8e4f@linux.intel.com>
Hi Ilpo,
Quoting Ilpo Järvinen (2025-08-28 18:18:51)
> Hi Anton,
>
> These days I'm the one handling pdx86 patches. I took this into the
> review-ilpo-fixes branch and reinstated Hans' ack.
Thank you, much appreciated.
> My experience is that long delays are nothing unusual when interacting
> with Dimitry so don't get discouraged by that when it comes to the
> rest of v2 content. Once Dimitry has okay'ed the input side change, please
> resubmit the rest.
Sure, I will.
Cheers,
--
Anton Khirnov
^ permalink raw reply
* [PATCH v2] Input: Improve WinWing Orion2 throttle support
From: Ivan Gorinov @ 2025-08-31 0:26 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel
Add support for Orion2 throttle configurations with more than 32 buttons
on the grip handle (this means the device reports more than 80 buttons).
Map additional button codes to KEY_MACRO1 .. KEY_MACRO30.
Make the module simpler, removing report descriptor fixup.
Changes since v1:
- Correct trivial coding style violations
Signed-off-by: Ivan Gorinov <linux-kernel@altimeter.info>
---
drivers/hid/Kconfig | 2 +
drivers/hid/hid-winwing.c | 166 +++++++++++++++++++++++---------------
2 files changed, 103 insertions(+), 65 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a57901203aeb..3317981e65dc 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1309,6 +1309,8 @@ config HID_WINWING
help
Support for WinWing Orion2 throttle base with the following grips:
+ * TGRIP-15E
+ * TGRIP-15EX
* TGRIP-16EX
* TGRIP-18
diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index d4afbbd27807..705e59f263a5 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -37,6 +37,7 @@ struct winwing_drv_data {
struct hid_device *hdev;
__u8 *report_buf;
struct mutex lock;
+ int map_more_buttons;
unsigned int num_leds;
struct winwing_led leds[];
};
@@ -81,12 +82,10 @@ static int winwing_init_led(struct hid_device *hdev,
int ret;
int i;
- size_t data_size = struct_size(data, leds, 3);
-
- data = devm_kzalloc(&hdev->dev, data_size, GFP_KERNEL);
+ data = hid_get_drvdata(hdev);
if (!data)
- return -ENOMEM;
+ return -EINVAL;
data->report_buf = devm_kmalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
@@ -106,6 +105,7 @@ static int winwing_init_led(struct hid_device *hdev,
"%s::%s",
dev_name(&input->dev),
info->led_name);
+
if (!led->cdev.name)
return -ENOMEM;
@@ -114,14 +114,95 @@ static int winwing_init_led(struct hid_device *hdev,
return ret;
}
- hid_set_drvdata(hdev, data);
-
return ret;
}
+static int winwing_map_button(int button, int map_more_buttons)
+{
+ if (button < 1)
+ return KEY_RESERVED;
+
+ if (button > 112)
+ return KEY_RESERVED;
+
+ if (button <= 16) {
+ /*
+ * Grip buttons [1 .. 16] are mapped to
+ * key codes BTN_TRIGGER .. BTN_DEAD
+ */
+ return (button - 1) + BTN_JOYSTICK;
+ }
+
+ if (button >= 65) {
+ /*
+ * Base buttons [65 .. 112] are mapped to
+ * key codes BTN_TRIGGER_HAPPY17 .. KEY_MAX
+ */
+ return (button - 65) + BTN_TRIGGER_HAPPY17;
+ }
+
+ if (!map_more_buttons) {
+ /* Not mapping numbers [33 .. 64] */
+ if (button >= 33)
+ return KEY_RESERVED;
+ /*
+ * Grip buttons [17 .. 32] are mapped to
+ * BTN_TRIGGER_HAPPY1 .. BTN_TRIGGER_HAPPY16
+ */
+ return (button - 17) + BTN_TRIGGER_HAPPY1;
+ }
+
+ if (button >= 49) {
+ /*
+ * Grip buttons [49 .. 64] are mapped to
+ * BTN_TRIGGER_HAPPY1 .. BTN_TRIGGER_HAPPY16
+ */
+ return (button - 49) + BTN_TRIGGER_HAPPY1;
+ }
+
+ /*
+ * Grip buttons [17 .. 44] are mapped to
+ * key codes KEY_MACRO1 .. KEY_MACRO28;
+ * also mapping numbers [45 .. 48] which
+ * are not assigned to any real buttons.
+ */
+ return (button - 17) + KEY_MACRO1;
+}
+
+static int winwing_input_mapping(struct hid_device *hdev,
+ struct hid_input *hi, struct hid_field *field, struct hid_usage *usage,
+ unsigned long **bit, int *max)
+{
+ struct winwing_drv_data *data;
+ int code = KEY_RESERVED;
+ int button = 0;
+
+ data = hid_get_drvdata(hdev);
+
+ if (!data)
+ return -EINVAL;
+
+ if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
+ return 0;
+
+ if (field->application != HID_GD_JOYSTICK)
+ return 0;
+
+ /* Button numbers start with 1 */
+ button = usage->hid & HID_USAGE;
+
+ code = winwing_map_button(button, data->map_more_buttons);
+
+ hid_map_usage(hi, usage, bit, max, EV_KEY, code);
+
+ return 1;
+}
+
static int winwing_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
+ struct winwing_drv_data *data;
+ size_t data_size = struct_size(data, leds, 3);
int ret;
ret = hid_parse(hdev);
@@ -130,6 +211,15 @@ static int winwing_probe(struct hid_device *hdev,
return ret;
}
+ data = devm_kzalloc(&hdev->dev, data_size, GFP_KERNEL);
+
+ if (!data)
+ return -ENOMEM;
+
+ data->map_more_buttons = id->driver_data;
+
+ hid_set_drvdata(hdev, data);
+
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hw start failed\n");
@@ -152,64 +242,11 @@ static int winwing_input_configured(struct hid_device *hdev,
return ret;
}
-static const __u8 original_rdesc_buttons[] = {
- 0x05, 0x09, 0x19, 0x01, 0x29, 0x6F,
- 0x15, 0x00, 0x25, 0x01, 0x35, 0x00,
- 0x45, 0x01, 0x75, 0x01, 0x95, 0x6F,
- 0x81, 0x02, 0x75, 0x01, 0x95, 0x01,
- 0x81, 0x01
-};
-
-/*
- * HID report descriptor shows 111 buttons, which exceeds maximum
- * number of buttons (80) supported by Linux kernel HID subsystem.
- *
- * This module skips numbers 32-63, unused on some throttle grips.
- */
-
-static const __u8 *winwing_report_fixup(struct hid_device *hdev, __u8 *rdesc,
- unsigned int *rsize)
-{
- int sig_length = sizeof(original_rdesc_buttons);
- int unused_button_numbers = 32;
-
- if (*rsize < 34)
- return rdesc;
-
- if (memcmp(rdesc + 8, original_rdesc_buttons, sig_length) == 0) {
-
- /* Usage Maximum */
- rdesc[13] -= unused_button_numbers;
-
- /* Report Count for buttons */
- rdesc[25] -= unused_button_numbers;
-
- /* Report Count for padding [HID1_11, 6.2.2.9] */
- rdesc[31] += unused_button_numbers;
-
- hid_info(hdev, "winwing descriptor fixed\n");
- }
-
- return rdesc;
-}
-
-static int winwing_raw_event(struct hid_device *hdev,
- struct hid_report *report, u8 *raw_data, int size)
-{
- if (size >= 15) {
- /* Skip buttons 32 .. 63 */
- memmove(raw_data + 5, raw_data + 9, 6);
-
- /* Clear the padding */
- memset(raw_data + 11, 0, 4);
- }
-
- return 0;
-}
-
static const struct hid_device_id winwing_devices[] = {
- { HID_USB_DEVICE(0x4098, 0xbe62) }, /* TGRIP-18 */
- { HID_USB_DEVICE(0x4098, 0xbe68) }, /* TGRIP-16EX */
+ { HID_USB_DEVICE(0x4098, 0xbd65), .driver_data = 1 }, /* TGRIP-15E */
+ { HID_USB_DEVICE(0x4098, 0xbd64), .driver_data = 1 }, /* TGRIP-15EX */
+ { HID_USB_DEVICE(0x4098, 0xbe68), .driver_data = 0 }, /* TGRIP-16EX */
+ { HID_USB_DEVICE(0x4098, 0xbe62), .driver_data = 0 }, /* TGRIP-18 */
{}
};
@@ -218,10 +255,9 @@ MODULE_DEVICE_TABLE(hid, winwing_devices);
static struct hid_driver winwing_driver = {
.name = "winwing",
.id_table = winwing_devices,
+ .input_mapping = winwing_input_mapping,
.probe = winwing_probe,
.input_configured = winwing_input_configured,
- .report_fixup = winwing_report_fixup,
- .raw_event = winwing_raw_event,
};
module_hid_driver(winwing_driver);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] dt-bindings: input: tsc2007: use comma in filename
From: Krzysztof Kozlowski @ 2025-08-30 9:03 UTC (permalink / raw)
To: akemnade, dmitry.torokhov, robh, krzk+dt, conor+dt, Frank.Li,
andreas, linux-input, devicetree, linux-kernel
In-Reply-To: <20250830085326.36120-1-akemnade@kernel.org>
On 30/08/2025 10:53, akemnade@kernel.org wrote:
> From: Andreas Kemnade <andreas@kemnade.info>
>
> Use comma between vendor-prefix and chip name as it is common.
...and preferred naming style.
Fixes: 45601c66b5dd ("dt-bindings: input: touchscreen: convert tsc2007.txt to yaml format")
(no need for backports though)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH] dt-bindings: input: tsc2007: use comma in filename
From: akemnade @ 2025-08-30 8:53 UTC (permalink / raw)
To: dmitry.torokhov, robh, krzk+dt, conor+dt, Frank.Li, andreas,
linux-input, devicetree, linux-kernel
From: Andreas Kemnade <andreas@kemnade.info>
Use comma between vendor-prefix and chip name as it is common.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
.../input/touchscreen/{ti.tsc2007.yaml => ti,tsc2007.yaml} | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
rename Documentation/devicetree/bindings/input/touchscreen/{ti.tsc2007.yaml => ti,tsc2007.yaml} (96%)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti.tsc2007.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2007.yaml
similarity index 96%
rename from Documentation/devicetree/bindings/input/touchscreen/ti.tsc2007.yaml
rename to Documentation/devicetree/bindings/input/touchscreen/ti,tsc2007.yaml
index 8bb4bc7df4fa..a73d110272fa 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/ti.tsc2007.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2007.yaml
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
%YAML 1.2
---
-$id: http://devicetree.org/schemas/input/touchscreen/ti.tsc2007.yaml#
+$id: http://devicetree.org/schemas/input/touchscreen/ti,tsc2007.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
title: Texas Instruments tsc2007 touchscreen controller
--
2.39.5
^ permalink raw reply related
* [PATCH] Input: Improve WinWing Orion2 throttle support
From: Ivan Gorinov @ 2025-08-30 5:21 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel
Add support for Orion2 throttle configurations with more than 32 button codes
on the grip handle (this means the device reports more than 80 button codes).
Map additional button codes to KEY_MACRO1 .. KEY_MACRO28.
Make the module simpler, removing report descriptor fixup.
Signed-off-by: Ivan Gorinov <linux-kernel@altimeter.info>
---
drivers/hid/Kconfig | 2 +
drivers/hid/hid-winwing.c | 167 +++++++++++++++++++++++---------------
2 files changed, 104 insertions(+), 65 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a57901203aeb..3317981e65dc 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1309,6 +1309,8 @@ config HID_WINWING
help
Support for WinWing Orion2 throttle base with the following grips:
+ * TGRIP-15E
+ * TGRIP-15EX
* TGRIP-16EX
* TGRIP-18
diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index d4afbbd27807..54e0f43163a8 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -38,6 +38,7 @@ struct winwing_drv_data {
__u8 *report_buf;
struct mutex lock;
unsigned int num_leds;
+ int too_many_buttons;
struct winwing_led leds[];
};
@@ -81,12 +82,10 @@ static int winwing_init_led(struct hid_device *hdev,
int ret;
int i;
- size_t data_size = struct_size(data, leds, 3);
-
- data = devm_kzalloc(&hdev->dev, data_size, GFP_KERNEL);
+ data = hid_get_drvdata(hdev);
if (!data)
- return -ENOMEM;
+ return -EINVAL;
data->report_buf = devm_kmalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
@@ -106,6 +105,7 @@ static int winwing_init_led(struct hid_device *hdev,
"%s::%s",
dev_name(&input->dev),
info->led_name);
+
if (!led->cdev.name)
return -ENOMEM;
@@ -114,14 +114,96 @@ static int winwing_init_led(struct hid_device *hdev,
return ret;
}
- hid_set_drvdata(hdev, data);
-
return ret;
}
+static int winwing_map_button(int button, int too_many_buttons)
+{
+ if (button < 1)
+ return KEY_RESERVED;
+
+ if (button > 112)
+ return KEY_RESERVED;
+
+ if (button <= 16) {
+ /*
+ * Grip buttons [1 .. 16] are mapped to
+ * key codes BTN_TRIGGER .. BTN_DEAD
+ */
+ return (button - 1) + BTN_JOYSTICK;
+ }
+
+ if (button >= 65) {
+ /*
+ * Base buttons [65 .. 112] are mapped to
+ * key codes BTN_TRIGGER_HAPPY17 .. KEY_MAX
+ */
+ return (button - 65) + BTN_TRIGGER_HAPPY17;
+ }
+
+ if (too_many_buttons) {
+ if (button <= 48) {
+ /*
+ * Grip buttons [17 .. 44] are mapped to
+ * KEY_MACRO1 .. KEY_MACRO28;
+ * button numbers [45 .. 48] are not used.
+ */
+ return (button - 17) + KEY_MACRO1;
+ } else {
+ /*
+ * Grip buttons [49 .. 64] are mapped to
+ * BTN_TRIGGER_HAPPY1 .. BTN_TRIGGER_HAPPY16
+ */
+ return (button - 49) + BTN_TRIGGER_HAPPY1;
+ }
+ } else {
+ if (button <= 32) {
+ /*
+ * Grip buttons [17 .. 32] are mapped to
+ * BTN_TRIGGER_HAPPY1 .. BTN_TRIGGER_HAPPY16
+ */
+ return (button - 17) + BTN_TRIGGER_HAPPY1;
+ }
+ /* Not mapping button numbers [33 .. 64] */
+ }
+
+ return KEY_RESERVED;
+}
+
+static int winwing_input_mapping(struct hid_device *hdev,
+ struct hid_input *hi, struct hid_field *field, struct hid_usage *usage,
+ unsigned long **bit, int *max)
+{
+ struct winwing_drv_data *data;
+ int code = KEY_RESERVED;
+ int button = 0;
+
+ data = hid_get_drvdata(hdev);
+
+ if (!data)
+ return -EINVAL;
+
+ if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
+ return 0;
+
+ if (field->application != HID_GD_JOYSTICK)
+ return 0;
+
+ /* Button numbers start with 1 */
+ button = usage->hid & HID_USAGE;
+
+ code = winwing_map_button(button, data->too_many_buttons);
+
+ hid_map_usage(hi, usage, bit, max, EV_KEY, code);
+
+ return 1;
+}
+
static int winwing_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
+ struct winwing_drv_data *data;
+ size_t data_size = struct_size(data, leds, 3);
int ret;
ret = hid_parse(hdev);
@@ -130,6 +212,15 @@ static int winwing_probe(struct hid_device *hdev,
return ret;
}
+ data = devm_kzalloc(&hdev->dev, data_size, GFP_KERNEL);
+
+ if (!data)
+ return -ENOMEM;
+
+ data->too_many_buttons = id->driver_data;
+
+ hid_set_drvdata(hdev, data);
+
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hw start failed\n");
@@ -152,64 +243,11 @@ static int winwing_input_configured(struct hid_device *hdev,
return ret;
}
-static const __u8 original_rdesc_buttons[] = {
- 0x05, 0x09, 0x19, 0x01, 0x29, 0x6F,
- 0x15, 0x00, 0x25, 0x01, 0x35, 0x00,
- 0x45, 0x01, 0x75, 0x01, 0x95, 0x6F,
- 0x81, 0x02, 0x75, 0x01, 0x95, 0x01,
- 0x81, 0x01
-};
-
-/*
- * HID report descriptor shows 111 buttons, which exceeds maximum
- * number of buttons (80) supported by Linux kernel HID subsystem.
- *
- * This module skips numbers 32-63, unused on some throttle grips.
- */
-
-static const __u8 *winwing_report_fixup(struct hid_device *hdev, __u8 *rdesc,
- unsigned int *rsize)
-{
- int sig_length = sizeof(original_rdesc_buttons);
- int unused_button_numbers = 32;
-
- if (*rsize < 34)
- return rdesc;
-
- if (memcmp(rdesc + 8, original_rdesc_buttons, sig_length) == 0) {
-
- /* Usage Maximum */
- rdesc[13] -= unused_button_numbers;
-
- /* Report Count for buttons */
- rdesc[25] -= unused_button_numbers;
-
- /* Report Count for padding [HID1_11, 6.2.2.9] */
- rdesc[31] += unused_button_numbers;
-
- hid_info(hdev, "winwing descriptor fixed\n");
- }
-
- return rdesc;
-}
-
-static int winwing_raw_event(struct hid_device *hdev,
- struct hid_report *report, u8 *raw_data, int size)
-{
- if (size >= 15) {
- /* Skip buttons 32 .. 63 */
- memmove(raw_data + 5, raw_data + 9, 6);
-
- /* Clear the padding */
- memset(raw_data + 11, 0, 4);
- }
-
- return 0;
-}
-
static const struct hid_device_id winwing_devices[] = {
- { HID_USB_DEVICE(0x4098, 0xbe62) }, /* TGRIP-18 */
- { HID_USB_DEVICE(0x4098, 0xbe68) }, /* TGRIP-16EX */
+ { HID_USB_DEVICE(0x4098, 0xbd65), .driver_data = 1 }, /* TGRIP-15E */
+ { HID_USB_DEVICE(0x4098, 0xbd64), .driver_data = 1 }, /* TGRIP-15EX */
+ { HID_USB_DEVICE(0x4098, 0xbe68), .driver_data = 0 }, /* TGRIP-16EX */
+ { HID_USB_DEVICE(0x4098, 0xbe62), .driver_data = 0 }, /* TGRIP-18 */
{}
};
@@ -218,10 +256,9 @@ MODULE_DEVICE_TABLE(hid, winwing_devices);
static struct hid_driver winwing_driver = {
.name = "winwing",
.id_table = winwing_devices,
+ .input_mapping = winwing_input_mapping,
.probe = winwing_probe,
.input_configured = winwing_input_configured,
- .report_fixup = winwing_report_fixup,
- .raw_event = winwing_raw_event,
};
module_hid_driver(winwing_driver);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH V0 1/2] hyper-v: Add CONFIG_HYPERV_VMBUS option
From: Mukesh R @ 2025-08-29 20:59 UTC (permalink / raw)
To: Nuno Das Neves, dri-devel, linux-kernel, linux-input,
linux-hyperv, netdev, linux-pci, linux-scsi, linux-fbdev,
linux-arch, virtualization
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, jikos,
bentiss, kys, haiyangz, wei.liu, decui, dmitry.torokhov,
andrew+netdev, davem, edumazet, kuba, pabeni, bhelgaas,
James.Bottomley, martin.petersen, gregkh, deller, arnd, sgarzare,
horms
In-Reply-To: <5003d5e8-a025-4827-b8a0-6fe11877421b@linux.microsoft.com>
On 8/28/25 17:29, Nuno Das Neves wrote:
> On 8/27/2025 5:59 PM, Mukesh Rathor wrote:
>> Somehow vmbus driver is hinged on CONFIG_HYPERV. It appears this is initial
>> code that did not get addressed when the scope of CONFIG_HYPERV went beyond
>> vmbus. This commit creates a fine grained HYPERV_VMBUS option and updates
>> drivers that depend on VMBUS.
>>
>
> The commit message can be improved. The docs are helpful here:
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
>
> In particular, some clearer reasons for the change.
> e.g.
> - CONFIG_HYPERV encompasses too much right now. It's not always clear what
> depends on builtin hyperv code and what depends on vmbus.
>
> - Since there is so much builtin hyperv code, building CONFIG_HYPERV as a
> module doesn't make intuitive sense. Building vmbus support as a module does.
>
> - There are actually some real scenarios someone may want to compile with
> CONFIG_HYPERV but without vmbus, like baremetal root partition.
>
> FWIW I think it's a good idea, interested to hear what others think.
Sorry, you had mentioned it and I expanded the cover letter and forgot the
commit message here. You said it better than I could above, so I can just use
that in V1 next week if no other comments.
Thanks,
-Mukesh
> Nuno
>
>> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
>> ---
>> drivers/gpu/drm/Kconfig | 2 +-
>> drivers/hid/Kconfig | 2 +-
>> drivers/hv/Kconfig | 12 +++++++++---
>> drivers/hv/Makefile | 2 +-
>> drivers/input/serio/Kconfig | 4 ++--
>> drivers/net/hyperv/Kconfig | 2 +-
>> drivers/pci/Kconfig | 2 +-
>> drivers/scsi/Kconfig | 2 +-
>> drivers/uio/Kconfig | 2 +-
>> drivers/video/fbdev/Kconfig | 2 +-
>> include/asm-generic/mshyperv.h | 8 +++++---
>> net/vmw_vsock/Kconfig | 2 +-
>> 12 files changed, 25 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index f7ea8e895c0c..58f34da061c6 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -398,7 +398,7 @@ source "drivers/gpu/drm/imagination/Kconfig"
>>
>> config DRM_HYPERV
>> tristate "DRM Support for Hyper-V synthetic video device"
>> - depends on DRM && PCI && HYPERV
>> + depends on DRM && PCI && HYPERV_VMBUS
>> select DRM_CLIENT_SELECTION
>> select DRM_KMS_HELPER
>> select DRM_GEM_SHMEM_HELPER
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index a57901203aeb..fe3dc8c0db99 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -1162,7 +1162,7 @@ config GREENASIA_FF
>>
>> config HID_HYPERV_MOUSE
>> tristate "Microsoft Hyper-V mouse driver"
>> - depends on HYPERV
>> + depends on HYPERV_VMBUS
>> help
>> Select this option to enable the Hyper-V mouse driver.
>>
>> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
>> index 2e8df09db599..08c4ed005137 100644
>> --- a/drivers/hv/Kconfig
>> +++ b/drivers/hv/Kconfig
>> @@ -44,18 +44,24 @@ config HYPERV_TIMER
>>
>> config HYPERV_UTILS
>> tristate "Microsoft Hyper-V Utilities driver"
>> - depends on HYPERV && CONNECTOR && NLS
>> + depends on HYPERV_VMBUS && CONNECTOR && NLS
>> depends on PTP_1588_CLOCK_OPTIONAL
>> help
>> Select this option to enable the Hyper-V Utilities.
>>
>> config HYPERV_BALLOON
>> tristate "Microsoft Hyper-V Balloon driver"
>> - depends on HYPERV
>> + depends on HYPERV_VMBUS
>> select PAGE_REPORTING
>> help
>> Select this option to enable Hyper-V Balloon driver.
>>
>> +config HYPERV_VMBUS
>> + tristate "Microsoft Hyper-V Vmbus driver"
>> + depends on HYPERV
>> + help
>> + Select this option to enable Hyper-V Vmbus driver.
>> +
>> config MSHV_ROOT
>> tristate "Microsoft Hyper-V root partition support"
>> depends on HYPERV && (X86_64 || ARM64)
>> @@ -75,7 +81,7 @@ config MSHV_ROOT
>>
>> config MSHV_VTL
>> tristate "Microsoft Hyper-V VTL driver"
>> - depends on X86_64 && HYPERV_VTL_MODE
>> + depends on X86_64 && HYPERV_VTL_MODE && HYPERV_VMBUS
>> # Mapping VTL0 memory to a userspace process in VTL2 is supported in OpenHCL.
>> # VTL2 for OpenHCL makes use of Huge Pages to improve performance on VMs,
>> # specially with large memory requirements.
>> diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
>> index c53a0df746b7..050517756a82 100644
>> --- a/drivers/hv/Makefile
>> +++ b/drivers/hv/Makefile
>> @@ -1,5 +1,5 @@
>> # SPDX-License-Identifier: GPL-2.0
>> -obj-$(CONFIG_HYPERV) += hv_vmbus.o
>> +obj-$(CONFIG_HYPERV_VMBUS) += hv_vmbus.o
>> obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
>> obj-$(CONFIG_HYPERV_BALLOON) += hv_balloon.o
>> obj-$(CONFIG_MSHV_ROOT) += mshv_root.o
>> diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
>> index 17edc1597446..c7ef347a4dff 100644
>> --- a/drivers/input/serio/Kconfig
>> +++ b/drivers/input/serio/Kconfig
>> @@ -276,8 +276,8 @@ config SERIO_OLPC_APSP
>>
>> config HYPERV_KEYBOARD
>> tristate "Microsoft Synthetic Keyboard driver"
>> - depends on HYPERV
>> - default HYPERV
>> + depends on HYPERV_VMBUS
>> + default HYPERV_VMBUS
>> help
>> Select this option to enable the Hyper-V Keyboard driver.
>>
>> diff --git a/drivers/net/hyperv/Kconfig b/drivers/net/hyperv/Kconfig
>> index c8cbd85adcf9..982964c1a9fb 100644
>> --- a/drivers/net/hyperv/Kconfig
>> +++ b/drivers/net/hyperv/Kconfig
>> @@ -1,7 +1,7 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> config HYPERV_NET
>> tristate "Microsoft Hyper-V virtual network driver"
>> - depends on HYPERV
>> + depends on HYPERV_VMBUS
>> select UCS2_STRING
>> select NLS
>> help
>> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
>> index 9a249c65aedc..7065a8e5f9b1 100644
>> --- a/drivers/pci/Kconfig
>> +++ b/drivers/pci/Kconfig
>> @@ -221,7 +221,7 @@ config PCI_LABEL
>>
>> config PCI_HYPERV
>> tristate "Hyper-V PCI Frontend"
>> - depends on ((X86 && X86_64) || ARM64) && HYPERV && PCI_MSI && SYSFS
>> + depends on ((X86 && X86_64) || ARM64) && HYPERV_VMBUS && PCI_MSI && SYSFS
>> select PCI_HYPERV_INTERFACE
>> select IRQ_MSI_LIB
>> help
>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>> index 5522310bab8d..19d0884479a2 100644
>> --- a/drivers/scsi/Kconfig
>> +++ b/drivers/scsi/Kconfig
>> @@ -589,7 +589,7 @@ config XEN_SCSI_FRONTEND
>>
>> config HYPERV_STORAGE
>> tristate "Microsoft Hyper-V virtual storage driver"
>> - depends on SCSI && HYPERV
>> + depends on SCSI && HYPERV_VMBUS
>> depends on m || SCSI_FC_ATTRS != m
>> default HYPERV
>> help
>> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
>> index b060dcd7c635..6f86a61231e6 100644
>> --- a/drivers/uio/Kconfig
>> +++ b/drivers/uio/Kconfig
>> @@ -140,7 +140,7 @@ config UIO_MF624
>>
>> config UIO_HV_GENERIC
>> tristate "Generic driver for Hyper-V VMBus"
>> - depends on HYPERV
>> + depends on HYPERV_VMBUS
>> help
>> Generic driver that you can bind, dynamically, to any
>> Hyper-V VMBus device. It is useful to provide direct access
>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>> index c21484d15f0c..72c63eaeb983 100644
>> --- a/drivers/video/fbdev/Kconfig
>> +++ b/drivers/video/fbdev/Kconfig
>> @@ -1774,7 +1774,7 @@ config FB_BROADSHEET
>>
>> config FB_HYPERV
>> tristate "Microsoft Hyper-V Synthetic Video support"
>> - depends on FB && HYPERV
>> + depends on FB && HYPERV_VMBUS
>> select DMA_CMA if HAVE_DMA_CONTIGUOUS && CMA
>> select FB_IOMEM_HELPERS_DEFERRED
>> help
>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>> index 1d2ad1304ad4..66c58c91b530 100644
>> --- a/include/asm-generic/mshyperv.h
>> +++ b/include/asm-generic/mshyperv.h
>> @@ -165,6 +165,7 @@ static inline u64 hv_generate_guest_id(u64 kernel_version)
>>
>> void __init hv_mark_resources(void);
>>
>> +#if IS_ENABLED(CONFIG_HYPERV_VMBUS)
>> /* Free the message slot and signal end-of-message if required */
>> static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
>> {
>> @@ -200,6 +201,10 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
>> }
>> }
>>
>> +extern int vmbus_interrupt;
>> +extern int vmbus_irq;
>> +#endif /* CONFIG_HYPERV_VMBUS */
>> +
>> int hv_get_hypervisor_version(union hv_hypervisor_version_info *info);
>>
>> void hv_setup_vmbus_handler(void (*handler)(void));
>> @@ -213,9 +218,6 @@ void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs));
>> void hv_remove_crash_handler(void);
>> void hv_setup_mshv_handler(void (*handler)(void));
>>
>> -extern int vmbus_interrupt;
>> -extern int vmbus_irq;
>> -
>> #if IS_ENABLED(CONFIG_HYPERV)
>> /*
>> * Hypervisor's notion of virtual processor ID is different from
>> diff --git a/net/vmw_vsock/Kconfig b/net/vmw_vsock/Kconfig
>> index 56356d2980c8..8e803c4828c4 100644
>> --- a/net/vmw_vsock/Kconfig
>> +++ b/net/vmw_vsock/Kconfig
>> @@ -72,7 +72,7 @@ config VIRTIO_VSOCKETS_COMMON
>>
>> config HYPERV_VSOCKETS
>> tristate "Hyper-V transport for Virtual Sockets"
>> - depends on VSOCKETS && HYPERV
>> + depends on VSOCKETS && HYPERV_VMBUS
>> help
>> This module implements a Hyper-V transport for Virtual Sockets.
>>
^ permalink raw reply
* [PATCH v3 5/7] dt-bindings: mfd: fsl,mc13xxx: convert txt to DT schema
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
In-Reply-To: <20250829201517.15374-1-akurz@blala.de>
Convert the txt mc13xxx bindings to DT schema attempting to keep most
information. The nodes codec and touchscreen are not part of the new
schema since it was only briefly mentioned before.
Following the convention, rename led-control to fsl,led-control.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
.../devicetree/bindings/mfd/fsl,mc13xxx.yaml | 218 ++++++++++++++++++
.../devicetree/bindings/mfd/mc13xxx.txt | 156 -------------
2 files changed, 218 insertions(+), 156 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/mc13xxx.txt
diff --git a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
new file mode 100644
index 000000000000..007c2e3eee91
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
@@ -0,0 +1,218 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/fsl,mc13xxx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale MC13xxx Power Management Integrated Circuits (PMIC)
+
+maintainers:
+ - Alexander Kurz <akurz@blala.de>
+
+description: >
+ The MC13xxx PMIC series consists of the three models MC13783, MC13892
+ and MC34708 and provide regulators and other features like RTC, ADC,
+ LED, touchscreen, codec and input buttons.
+
+ Link to datasheets
+ https://www.nxp.com/docs/en/data-sheet/MC13783.pdf
+ https://www.nxp.com/docs/en/data-sheet/MC13892.pdf
+ https://www.nxp.com/docs/en/data-sheet/MC34708.pdf
+
+properties:
+ compatible:
+ enum:
+ - fsl,mc13783
+ - fsl,mc13892
+ - fsl,mc34708
+
+ reg:
+ description: I2C slave address or SPI chip select number.
+ maxItems: 1
+
+ spi-max-frequency: true
+
+ spi-cs-high: true
+
+ system-power-controller: true
+
+ interrupts:
+ maxItems: 1
+
+ leds:
+ type: object
+ $ref: /schemas/leds/common.yaml#
+
+ properties:
+ reg:
+ description: |
+ One of
+ MC13783 LED IDs
+ 0: Main display
+ 1: AUX display
+ 2: Keypad
+ 3: Red 1
+ 4: Green 1
+ 5: Blue 1
+ 6: Red 2
+ 7: Green 2
+ 8: Blue 2
+ 9: Red 3
+ 10: Green 3
+ 11: Blue 3
+
+ MC13892 LED IDs
+ 0: Main display
+ 1: AUX display
+ 2: Keypad
+ 3: Red
+ 4: Green
+ 5: Blue
+
+ MC34708 LED IDs
+ 0: Charger Red
+ 1: Charger Green
+ maxItems: 1
+
+ led-control:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ Setting for LED-Control register array length depends on model,
+ mc13783: 6, mc13892: 4, mc34708: 1
+
+ regulators:
+ type: object
+
+ additionalProperties:
+ type: object
+
+ description: |
+ List of child nodes specifying the regulators, depending on chip variant.
+ Each child node is defined using the standard binding for regulators and
+ the optional regulator properties defined below.
+
+ fsl,mc13xxx-uses-adc:
+ type: boolean
+ description: Indicate the ADC is being used
+
+ fsl,mc13xxx-uses-codec:
+ type: boolean
+ description: Indicate the Audio Codec is being used
+
+ fsl,mc13xxx-uses-rtc:
+ type: boolean
+ description: Indicate the RTC is being used
+
+ fsl,mc13xxx-uses-touch:
+ type: boolean
+ description: Indicate the touchscreen controller is being used
+
+required:
+ - compatible
+ - reg
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: fsl,mc13783
+ then:
+ properties:
+ leds:
+ properties:
+ led-control:
+ minItems: 6
+ maxItems: 6
+ regulators:
+ patternProperties:
+ "^gpo[1-4]|pwgt[12]spi|sw[12][ab]|sw3|vaudio|vcam|vdig|vesim|vgen|viohi|violo|vmmc[12]|vrf[12]|vrfbg|vrfcp|vrfdig|vrfref|vsim|vvib$":
+ type: object
+ $ref: /schemas/regulator/regulator.yaml#
+
+ unevaluatedProperties: false
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: fsl,mc13892
+ then:
+ properties:
+ leds:
+ properties:
+ led-control:
+ minItems: 4
+ maxItems: 4
+ regulators:
+ patternProperties:
+ "^gpo[1-4]|pwgt[12]spi|sw[1-4]|swbst|vaudio|vcam|vcoincell|vdig|vgen[1-3]|viohi|vpll|vsd|vusb|vusb2|vvideo$":
+ type: object
+ $ref: /schemas/regulator/regulator.yaml#
+
+ unevaluatedProperties: false
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: fsl,mc34708
+ then:
+ properties:
+ leds:
+ properties:
+ led-control:
+ minItems: 1
+ maxItems: 1
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/leds/common.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic: mc13892@0 {
+ compatible = "fsl,mc13892";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ spi-cs-high;
+ interrupt-parent = <&gpio0>;
+ interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
+ fsl,mc13xxx-uses-rtc;
+ fsl,mc13xxx-uses-adc;
+
+ leds {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ led-control = <0x000 0x000 0x0e0 0x000>;
+
+ sysled@3 {
+ reg = <3>;
+ label = "system:red:live";
+ linux,default-trigger = "heartbeat";
+ };
+ };
+
+ regulators {
+ sw1_reg: sw1 {
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <1375000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ sw2_reg: sw2 {
+ regulator-min-microvolt = <900000>;
+ regulator-max-microvolt = <1850000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/mfd/mc13xxx.txt b/Documentation/devicetree/bindings/mfd/mc13xxx.txt
deleted file mode 100644
index 8261ea73278a..000000000000
--- a/Documentation/devicetree/bindings/mfd/mc13xxx.txt
+++ /dev/null
@@ -1,156 +0,0 @@
-* Freescale MC13783/MC13892 Power Management Integrated Circuit (PMIC)
-
-Required properties:
-- compatible : Should be "fsl,mc13783" or "fsl,mc13892"
-
-Optional properties:
-- fsl,mc13xxx-uses-adc : Indicate the ADC is being used
-- fsl,mc13xxx-uses-codec : Indicate the Audio Codec is being used
-- fsl,mc13xxx-uses-rtc : Indicate the RTC is being used
-- fsl,mc13xxx-uses-touch : Indicate the touchscreen controller is being used
-
-Sub-nodes:
-- codec: Contain the Audio Codec node.
- - adc-port: Contain PMIC SSI port number used for ADC.
- - dac-port: Contain PMIC SSI port number used for DAC.
-- leds : Contain the led nodes and initial register values in property
- "led-control". Number of register depends of used IC, for MC13783 is 6,
- for MC13892 is 4, for MC34708 is 1. See datasheet for bits definitions of
- these registers.
- - #address-cells: Must be 1.
- - #size-cells: Must be 0.
- Each led node should contain "reg", which used as LED ID (described below).
- Optional properties "label" and "linux,default-trigger" is described in
- Documentation/devicetree/bindings/leds/common.txt.
-- regulators : Contain the regulator nodes. The regulators are bound using
- their names as listed below with their registers and bits for enabling.
-
-MC13783 LED IDs:
- 0 : Main display
- 1 : AUX display
- 2 : Keypad
- 3 : Red 1
- 4 : Green 1
- 5 : Blue 1
- 6 : Red 2
- 7 : Green 2
- 8 : Blue 2
- 9 : Red 3
- 10 : Green 3
- 11 : Blue 3
-
-MC13892 LED IDs:
- 0 : Main display
- 1 : AUX display
- 2 : Keypad
- 3 : Red
- 4 : Green
- 5 : Blue
-
-MC34708 LED IDs:
- 0 : Charger Red
- 1 : Charger Green
-
-MC13783 regulators:
- sw1a : regulator SW1A (register 24, bit 0)
- sw1b : regulator SW1B (register 25, bit 0)
- sw2a : regulator SW2A (register 26, bit 0)
- sw2b : regulator SW2B (register 27, bit 0)
- sw3 : regulator SW3 (register 29, bit 20)
- vaudio : regulator VAUDIO (register 32, bit 0)
- viohi : regulator VIOHI (register 32, bit 3)
- violo : regulator VIOLO (register 32, bit 6)
- vdig : regulator VDIG (register 32, bit 9)
- vgen : regulator VGEN (register 32, bit 12)
- vrfdig : regulator VRFDIG (register 32, bit 15)
- vrfref : regulator VRFREF (register 32, bit 18)
- vrfcp : regulator VRFCP (register 32, bit 21)
- vsim : regulator VSIM (register 33, bit 0)
- vesim : regulator VESIM (register 33, bit 3)
- vcam : regulator VCAM (register 33, bit 6)
- vrfbg : regulator VRFBG (register 33, bit 9)
- vvib : regulator VVIB (register 33, bit 11)
- vrf1 : regulator VRF1 (register 33, bit 12)
- vrf2 : regulator VRF2 (register 33, bit 15)
- vmmc1 : regulator VMMC1 (register 33, bit 18)
- vmmc2 : regulator VMMC2 (register 33, bit 21)
- gpo1 : regulator GPO1 (register 34, bit 6)
- gpo2 : regulator GPO2 (register 34, bit 8)
- gpo3 : regulator GPO3 (register 34, bit 10)
- gpo4 : regulator GPO4 (register 34, bit 12)
- pwgt1spi : regulator PWGT1SPI (register 34, bit 15)
- pwgt2spi : regulator PWGT2SPI (register 34, bit 16)
-
-MC13892 regulators:
- vcoincell : regulator VCOINCELL (register 13, bit 23)
- sw1 : regulator SW1 (register 24, bit 0)
- sw2 : regulator SW2 (register 25, bit 0)
- sw3 : regulator SW3 (register 26, bit 0)
- sw4 : regulator SW4 (register 27, bit 0)
- swbst : regulator SWBST (register 29, bit 20)
- vgen1 : regulator VGEN1 (register 32, bit 0)
- viohi : regulator VIOHI (register 32, bit 3)
- vdig : regulator VDIG (register 32, bit 9)
- vgen2 : regulator VGEN2 (register 32, bit 12)
- vpll : regulator VPLL (register 32, bit 15)
- vusb2 : regulator VUSB2 (register 32, bit 18)
- vgen3 : regulator VGEN3 (register 33, bit 0)
- vcam : regulator VCAM (register 33, bit 6)
- vvideo : regulator VVIDEO (register 33, bit 12)
- vaudio : regulator VAUDIO (register 33, bit 15)
- vsd : regulator VSD (register 33, bit 18)
- gpo1 : regulator GPO1 (register 34, bit 6)
- gpo2 : regulator GPO2 (register 34, bit 8)
- gpo3 : regulator GPO3 (register 34, bit 10)
- gpo4 : regulator GPO4 (register 34, bit 12)
- pwgt1spi : regulator PWGT1SPI (register 34, bit 15)
- pwgt2spi : regulator PWGT2SPI (register 34, bit 16)
- vusb : regulator VUSB (register 50, bit 3)
-
- The bindings details of individual regulator device can be found in:
- Documentation/devicetree/bindings/regulator/regulator.txt
-
-Examples:
-
-ecspi@70010000 { /* ECSPI1 */
- cs-gpios = <&gpio4 24 0>, /* GPIO4_24 */
- <&gpio4 25 0>; /* GPIO4_25 */
-
- pmic: mc13892@0 {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "fsl,mc13892";
- spi-max-frequency = <6000000>;
- reg = <0>;
- interrupt-parent = <&gpio0>;
- interrupts = <8>;
-
- leds {
- #address-cells = <1>;
- #size-cells = <0>;
- led-control = <0x000 0x000 0x0e0 0x000>;
-
- sysled@3 {
- reg = <3>;
- label = "system:red:live";
- linux,default-trigger = "heartbeat";
- };
- };
-
- regulators {
- sw1_reg: mc13892__sw1 {
- regulator-min-microvolt = <600000>;
- regulator-max-microvolt = <1375000>;
- regulator-boot-on;
- regulator-always-on;
- };
-
- sw2_reg: mc13892__sw2 {
- regulator-min-microvolt = <900000>;
- regulator-max-microvolt = <1850000>;
- regulator-boot-on;
- regulator-always-on;
- };
- };
- };
-};
--
2.39.5
^ permalink raw reply related
* [PATCH v3 4/7] Input: mc13783-pwrbutton: enable other mc13xxx PMIC
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
In-Reply-To: <20250829201517.15374-1-akurz@blala.de>
All three mc13xxx types do feature two common power buttons referred as
ONOFD[12] (mc13783) and PWRON[12] (mc13892/mc34708) in the SoC reference
manuals. Add support for PWRON[12] (mc13892/mc34708) but skip support for
button PWRON3 (mc13892) for sake of simplicity.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
drivers/input/misc/Kconfig | 4 +--
drivers/input/misc/mc13783-pwrbutton.c | 44 +++++++++++++++++++++++---
include/linux/mfd/mc13783.h | 4 +--
include/linux/mfd/mc13xxx.h | 2 ++
4 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 0fb21c99a5e3..b66e920369f2 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -276,8 +276,8 @@ config INPUT_MC13783_PWRBUTTON
tristate "MC13783 ON buttons"
depends on MFD_MC13XXX
help
- Support the ON buttons of MC13783 PMIC as an input device
- reporting power button status.
+ Support the ON buttons of MC13783/MC13892/MC34708 PMIC as an input
+ device reporting power button status.
To compile this driver as a module, choose M here: the module
will be called mc13783-pwrbutton.
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index ace9f286fd24..c9eea57ceedd 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -30,16 +30,21 @@
#include <linux/sched.h>
#include <linux/slab.h>
+struct mc13xxx_button_devtype {
+ int button_id_max;
+};
+
struct mc13783_pwrb {
struct input_dev *pwr;
struct mc13xxx *mc13783;
-#define MC13783_PWRB_B1_POL_INVERT (1 << 0)
-#define MC13783_PWRB_B2_POL_INVERT (1 << 1)
-#define MC13783_PWRB_B3_POL_INVERT (1 << 2)
int flags;
unsigned short keymap[3];
};
+#define MC13783_PWRB_B1_POL_INVERT (1 << 0)
+#define MC13783_PWRB_B2_POL_INVERT (1 << 1)
+#define MC13783_PWRB_B3_POL_INVERT (1 << 2)
+
#define MC13783_REG_INTERRUPT_SENSE_1 5
#define MC13783_IRQSENSE1_ONOFD1S (1 << 3)
#define MC13783_IRQSENSE1_ONOFD2S (1 << 4)
@@ -108,6 +113,8 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
{
const struct mc13xxx_buttons_platform_data *pdata;
struct mc13xxx *mc13783 = dev_get_drvdata(pdev->dev.parent);
+ struct mc13xxx_button_devtype *devtype =
+ (struct mc13xxx_button_devtype *)pdev->id_entry->driver_data;
struct input_dev *pwr;
struct mc13783_pwrb *priv;
int err = 0;
@@ -127,6 +134,11 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
+ if (devtype->button_id_max < 2 && pdata->b_on_flags[2] & 0x3) {
+ dev_err(&pdev->dev, "button not supported\n");
+ return -ENODEV;
+ }
+
reg |= (pdata->b_on_flags[0] & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
reg |= (pdata->b_on_flags[1] & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
reg |= (pdata->b_on_flags[2] & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
@@ -239,12 +251,15 @@ static void mc13783_pwrbutton_remove(struct platform_device *pdev)
{
struct mc13783_pwrb *priv = platform_get_drvdata(pdev);
const struct mc13xxx_buttons_platform_data *pdata;
+ struct mc13xxx_button_devtype *devtype =
+ (struct mc13xxx_button_devtype *)pdev->id_entry->driver_data;
pdata = dev_get_platdata(&pdev->dev);
mc13xxx_lock(priv->mc13783);
- if (pdata->b_on_flags[2] & MC13783_BUTTON_ENABLE)
+ if (devtype->button_id_max >= 2 &&
+ pdata->b_on_flags[2] & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD3, priv);
if (pdata->b_on_flags[1] & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD2, priv);
@@ -254,7 +269,28 @@ static void mc13783_pwrbutton_remove(struct platform_device *pdev)
mc13xxx_unlock(priv->mc13783);
}
+static const struct mc13xxx_button_devtype mc13783_button_devtype = {
+ .button_id_max = 2,
+};
+
+static const struct mc13xxx_button_devtype mc13892_button_devtype = {
+ /* PWRON3 is not supported yet. */
+ .button_id_max = 1,
+};
+
+static const struct mc13xxx_button_devtype mc34708_button_devtype = {
+ .button_id_max = 1,
+};
+
+static const struct platform_device_id mc13xxx_pwrbutton_idtable[] = {
+ { "mc13783-pwrbutton", (kernel_ulong_t)&mc13783_button_devtype },
+ { "mc13892-pwrbutton", (kernel_ulong_t)&mc13892_button_devtype },
+ { "mc34708-pwrbutton", (kernel_ulong_t)&mc34708_button_devtype },
+ { /* sentinel */ }
+};
+
static struct platform_driver mc13783_pwrbutton_driver = {
+ .id_table = mc13xxx_pwrbutton_idtable,
.probe = mc13783_pwrbutton_probe,
.remove = mc13783_pwrbutton_remove,
.driver = {
diff --git a/include/linux/mfd/mc13783.h b/include/linux/mfd/mc13783.h
index c25b1676741b..ab6db774e1fa 100644
--- a/include/linux/mfd/mc13783.h
+++ b/include/linux/mfd/mc13783.h
@@ -65,8 +65,8 @@
#define MC13783_IRQ_UDM 23
#define MC13783_IRQ_1HZ MC13XXX_IRQ_1HZ
#define MC13783_IRQ_TODA MC13XXX_IRQ_TODA
-#define MC13783_IRQ_ONOFD1 27
-#define MC13783_IRQ_ONOFD2 28
+#define MC13783_IRQ_ONOFD1 MC13XXX_IRQ_PWRON1
+#define MC13783_IRQ_ONOFD2 MC13XXX_IRQ_PWRON2
#define MC13783_IRQ_ONOFD3 29
#define MC13783_IRQ_SYSRST MC13XXX_IRQ_SYSRST
#define MC13783_IRQ_RTCRST MC13XXX_IRQ_RTCRST
diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h
index 0393083af28a..36e5e7de7cb2 100644
--- a/include/linux/mfd/mc13xxx.h
+++ b/include/linux/mfd/mc13xxx.h
@@ -67,6 +67,8 @@ int mc13xxx_irq_unmask(struct mc13xxx *mc13xxx, int irq);
#define MC13XXX_IRQ_LOBATH 14
#define MC13XXX_IRQ_1HZ 24
#define MC13XXX_IRQ_TODA 25
+#define MC13XXX_IRQ_PWRON1 27
+#define MC13XXX_IRQ_PWRON2 28
#define MC13XXX_IRQ_SYSRST 30
#define MC13XXX_IRQ_RTCRST 31
#define MC13XXX_IRQ_PC 32
--
2.39.5
^ permalink raw reply related
* [PATCH v3 7/7] Input: mc13783-pwrbutton: add OF support
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
In-Reply-To: <20250829201517.15374-1-akurz@blala.de>
Add OF support for the mc13783-pwrbutton so that it can be used with
modern DT based systems.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
drivers/input/misc/mc13783-pwrbutton.c | 94 +++++++++++++++++++++++---
1 file changed, 86 insertions(+), 8 deletions(-)
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index c9eea57ceedd..a20236b19103 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -27,6 +27,7 @@
#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/mfd/mc13783.h>
+#include <linux/property.h>
#include <linux/sched.h>
#include <linux/slab.h>
@@ -109,8 +110,82 @@ static irqreturn_t button3_irq(int irq, void *_priv)
return button_irq(MC13783_IRQ_ONOFD3, _priv);
}
-static int mc13783_pwrbutton_probe(struct platform_device *pdev)
+#ifdef CONFIG_OF
+static struct mc13xxx_buttons_platform_data __init *mc13xxx_pwrbutton_probe_dt(
+ struct platform_device *pdev)
{
+ struct mc13xxx_buttons_platform_data *pdata;
+ struct fwnode_handle *child;
+ struct device *dev = &pdev->dev;
+ struct mc13xxx_button_devtype *devtype =
+ (struct mc13xxx_button_devtype *)platform_get_device_id(pdev)->driver_data;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ struct fwnode_handle *parent __free(fwnode_handle) =
+ device_get_named_child_node(dev->parent, "buttons");
+ if (!parent)
+ return ERR_PTR(-ENODATA);
+
+ fwnode_for_each_named_child_node(parent, child, "onkey") {
+ u32 idx;
+ u8 dbnc = MC13783_BUTTON_DBNC_30MS;
+ u16 dbnc_ms;
+
+ if (fwnode_property_read_u32(child, "reg", &idx))
+ continue;
+
+ if (idx > devtype->button_id_max) {
+ dev_warn(dev, "reg out of range\n");
+ continue;
+ }
+
+ fwnode_property_read_u16(child, "debounce-delay-ms", &dbnc_ms);
+ switch (dbnc_ms) {
+ case 0:
+ dbnc = MC13783_BUTTON_DBNC_0MS;
+ break;
+ case 30:
+ dbnc = MC13783_BUTTON_DBNC_30MS;
+ break;
+ case 150:
+ dbnc = MC13783_BUTTON_DBNC_150MS;
+ break;
+ case 750:
+ dbnc = MC13783_BUTTON_DBNC_750MS;
+ break;
+ default:
+ dev_warn(dev, "invalid debounce-delay-ms value\n");
+ continue;
+ }
+
+ if (fwnode_property_read_u32(child, "linux,code", &pdata->b_on_key[idx]))
+ continue;
+
+ if (fwnode_property_read_bool(child, "active-low"))
+ pdata->b_on_flags[idx] |= MC13783_BUTTON_POL_INVERT;
+
+ if (fwnode_property_read_bool(child, "fsl,enable-reset"))
+ pdata->b_on_flags[idx] |= MC13783_BUTTON_RESET_EN;
+
+ pdata->b_on_flags[idx] |= MC13783_BUTTON_ENABLE | dbnc;
+ }
+
+ return pdata;
+}
+#else
+static inline struct mc13xxx_buttons_platform_data __init *mc13xxx_pwrbutton_probe_dt(
+ struct platform_device *pdev)
+{
+ return ERR_PTR(-ENODEV);
+}
+#endif
+
+static int __init mc13783_pwrbutton_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
const struct mc13xxx_buttons_platform_data *pdata;
struct mc13xxx *mc13783 = dev_get_drvdata(pdev->dev.parent);
struct mc13xxx_button_devtype *devtype =
@@ -121,9 +196,13 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
int reg = 0;
pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
- dev_err(&pdev->dev, "missing platform data\n");
- return -ENODEV;
+ if (dev->parent->of_node) {
+ pdata = mc13xxx_pwrbutton_probe_dt(pdev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ } else if (!pdata) {
+ dev_err(dev, "missing platform data\n");
+ return -ENODATA;
}
pwr = devm_input_allocate_device(&pdev->dev);
@@ -290,15 +369,14 @@ static const struct platform_device_id mc13xxx_pwrbutton_idtable[] = {
};
static struct platform_driver mc13783_pwrbutton_driver = {
- .id_table = mc13xxx_pwrbutton_idtable,
- .probe = mc13783_pwrbutton_probe,
- .remove = mc13783_pwrbutton_remove,
.driver = {
.name = "mc13783-pwrbutton",
},
+ .id_table = mc13xxx_pwrbutton_idtable,
+ .remove = mc13783_pwrbutton_remove,
};
-module_platform_driver(mc13783_pwrbutton_driver);
+module_platform_driver_probe(mc13783_pwrbutton_driver, mc13783_pwrbutton_probe);
MODULE_ALIAS("platform:mc13783-pwrbutton");
MODULE_DESCRIPTION("MC13783 Power Button");
--
2.39.5
^ permalink raw reply related
* [PATCH v3 0/7] Fix, extend and support OF to mc13xxx pwrbutton
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
Goal of this patch series is to make the mc13892 PWRON1 button usable,
found e.g. on amazon kindle D01100/D01200/EY21 readers.
A ten-year-old IRQ issue needed a fix, mc13783-pwrbutton had to be
extended to the other to mc13xxx PMIC as well (keeping the mc13892
PWRON3 key unsupported for simplicity) and adding OF support.
The implementation has been tested on amazon kindle D01100 and D01200
readers using PWRON1 of a mc13892.
Changes in v3:
- Link to v2: https://lore.kernel.org/linux-input/20250823144441.12654-1-akurz@blala.de/
- Undo all changes to led-control (rename to fsl,led-control), thanks Rob
- Restructured the new buttons node for unevaluatedProperties: false
- Various other remarks from Rob
- Rebase to current state
Changes in v2:
- Link to v1: https://lore.kernel.org/linux-input/20250817102751.29709-1-akurz@blala.de/
- Convert dt-bindings from txt to fsl,mc13xxx.yaml and add vendor prefix
to led-control property, causing changes in dts and driver.
- Change node name from pwrbuttons to buttons
- Change property debounce-delay-value to debounce-delay-ms
- Fixed a section mismatch error
- Fixed https://lore.kernel.org/r/202508210551.VzAtE5re-lkp@intel.com/
(wrong index used when converting to array access)
- Usage of generic device properties API in mc13783-pwrbutton.c
- Provide chip-specific max button id via platform_device_id, therefore
swap patches 3 and 4.
Alexander Kurz (7):
Input: mc13783-pwrbutton: fix irq mixup
Input: mc13783-pwrbutton: use managed resources
Input: mc13783-pwrbutton: convert pdata members to array
Input: mc13783-pwrbutton: enable other mc13xxx PMIC
dt-bindings: mfd: fsl,mc13xxx: convert txt to DT schema
dt-bindings: mfd: fsl,mc13xxx: add buttons node
Input: mc13783-pwrbutton: add OF support
.../devicetree/bindings/mfd/fsl,mc13xxx.yaml | 288 ++++++++++++++++++
.../devicetree/bindings/mfd/mc13xxx.txt | 156 ----------
drivers/input/misc/Kconfig | 4 +-
drivers/input/misc/mc13783-pwrbutton.c | 235 ++++++++++----
include/linux/mfd/mc13783.h | 4 +-
include/linux/mfd/mc13xxx.h | 10 +-
6 files changed, 472 insertions(+), 225 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/mc13xxx.txt
--
2.39.5
^ permalink raw reply
* [PATCH v3 2/7] Input: mc13783-pwrbutton: use managed resources
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
In-Reply-To: <20250829201517.15374-1-akurz@blala.de>
Use devres functionality to simplify resource freeing, dev.parent will
be set by devm_input_allocate_device().
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
drivers/input/misc/mc13783-pwrbutton.c | 28 ++++++++------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index 4765b25bc9f6..9fd84b8d163d 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -21,6 +21,7 @@
#include <linux/module.h>
#include <linux/kernel.h>
+#include <linux/device.h>
#include <linux/errno.h>
#include <linux/input.h>
#include <linux/interrupt.h>
@@ -118,18 +119,13 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
return -ENODEV;
}
- pwr = input_allocate_device();
- if (!pwr) {
- dev_dbg(&pdev->dev, "Can't allocate power button\n");
+ pwr = devm_input_allocate_device(&pdev->dev);
+ if (!pwr)
return -ENOMEM;
- }
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv) {
- err = -ENOMEM;
- dev_dbg(&pdev->dev, "Can't allocate power button\n");
- goto free_input_dev;
- }
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
reg |= (pdata->b1on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
reg |= (pdata->b2on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
@@ -155,7 +151,7 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
button1_irq, "b1on", priv);
if (err) {
dev_dbg(&pdev->dev, "Can't request irq\n");
- goto free_priv;
+ goto free_mc13xxx_lock;
}
}
@@ -203,7 +199,6 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
pwr->name = "mc13783_pwrbutton";
pwr->phys = "mc13783_pwrbutton/input0";
- pwr->dev.parent = &pdev->dev;
pwr->keycode = priv->keymap;
pwr->keycodemax = ARRAY_SIZE(priv->keymap);
@@ -234,12 +229,8 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
if (pdata->b1on_flags & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD1, priv);
-free_priv:
+free_mc13xxx_lock:
mc13xxx_unlock(mc13783);
- kfree(priv);
-
-free_input_dev:
- input_free_device(pwr);
return err;
}
@@ -261,9 +252,6 @@ static void mc13783_pwrbutton_remove(struct platform_device *pdev)
mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD1, priv);
mc13xxx_unlock(priv->mc13783);
-
- input_unregister_device(priv->pwr);
- kfree(priv);
}
static struct platform_driver mc13783_pwrbutton_driver = {
--
2.39.5
^ permalink raw reply related
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