On Mon, 2018-03-05 at 18:26 +0100, Benjamin Tissoires wrote: > Hi Florent, Hi Benjamin, > > On Mon, Mar 5, 2018 at 10:31 AM, Nestor Lopez Casado > <nlopezcasad@xxxxxxxxxxxx> wrote: > > Hello Florent, > > > > In my view, this driver may not be a good idea. The default > > behaviour > > of K290 is 'send multimedia keycodes' with the user given the > > choice > > to change that behaviour via vendor commands. Putting a driver that > > will unconditionally change that behaviour without the user's > > consent > > might bother other users that prefer the multimedia keycodes by > > default. > > > > Besides, I'd argue that instead of a kernel module this would be > > best > > achieved from a user space application. Something in the lines of > > Solaar (github pwr/solaar) or libratbag (there's an issue open to > > support keyboards) or even a specific application built for the > > purpose. Anyways, please collect the input from Benjamin and Jiri > > as > > they as they best placed to advise than myself. > > On top of what Nestor said, this type of functionality, if we want to > have them in the kernel should probably be integrated in > hid-logitech-hidpp, in order not having some magic reports to send. > > Things like reconnect of the device would be handled far more easily > in hid-logitech-hidpp while you would be reinventing the wheel here. > > One other thing I do not like in this submission of the driver is the > direct use of USB while we have a full transport agnostic layer > called > HID. Fair enough, I didn't have a look at how hid-logitech-hidpp is working yet. I'll dig into that to see if this driver can me implemented more elegantly. Regards, Florent > Cheers, > Benjamin > > > > > Cheers, > > -nestor > > > > On Sun, Mar 4, 2018 at 11:14 PM, Florent Flament > > <contact@xxxxxxxxxxxxxxxxxx> wrote: > > > With the generic HID driver, K290 keyboards' F1 to F12 keys send > > > multimedia events by default, and standard keycodes when the > > > function > > > key is pressed. This driver allows to configure K290 keyboards, > > > so > > > that F1 to F12 have a standard behavior and send multimedia > > > events > > > when the function key is pressed. The keyboard mode is set > > > through the > > > fn_mode module parameter: when set to 1 (default setting) the > > > keyboard > > > behaves as with the generic HID driver, when set to 0 the > > > keyboard is > > > configured to work as standard keyboards. > > > > > > Signed-off-by: Florent Flament <contact@xxxxxxxxxxxxxxxxxx> > > > --- > > > drivers/hid/Kconfig | 18 ++++++++ > > > drivers/hid/Makefile | 1 + > > > drivers/hid/hid-ids.h | 1 + > > > drivers/hid/hid-logitech-k290.c | 100 > > > ++++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 120 insertions(+) > > > create mode 100644 drivers/hid/hid-logitech-k290.c > > > > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > > index 19c499f5623d..6686da8daac6 100644 > > > --- a/drivers/hid/Kconfig > > > +++ b/drivers/hid/Kconfig > > > @@ -488,6 +488,24 @@ config HID_LOGITECH_HIDPP > > > T651, TK820), some mice (Zone Touch mouse), or even > > > keyboards (Solar > > > Keyboard). > > > > > > +config HID_LOGITECH_K290 > > > + tristate "Logitech K290 Keyboard support" > > > + depends on USB_HID > > > + ---help--- > > > + This enhances support of Logitech K290 keyboards. > > > + > > > + With the generic HID driver, K290 keyboards' F1 to F12 > > > keys > > > + send multimedia events by default, and standard keycodes > > > when > > > + the function key is pressed. This driver allows to > > > configure > > > + K290 keyboards, so that F1 to F12 have a standard > > > behavior and > > > + send multimedia events when the function key is pressed. > > > The > > > + keyboard mode is set through the fn_mode module > > > parameter: > > > + when set to 1 (default setting) the keyboard behaves as > > > with > > > + the generic HID driver, when set to 0 the keyboard is > > > + configured to work as standard keyboards. > > > + > > > + Say Y if you have a Logitech K290 keyboard. > > > + > > > config LOGITECH_FF > > > bool "Logitech force feedback support" > > > depends on HID_LOGITECH > > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > > > index eb13b9e92d85..78079d3a5d58 100644 > > > --- a/drivers/hid/Makefile > > > +++ b/drivers/hid/Makefile > > > @@ -61,6 +61,7 @@ obj-$(CONFIG_HID_LENOVO) += hid-lenovo.o > > > obj-$(CONFIG_HID_LOGITECH) += hid-logitech.o > > > obj-$(CONFIG_HID_LOGITECH_DJ) += hid-logitech-dj.o > > > obj-$(CONFIG_HID_LOGITECH_HIDPP) += hid-logitech-hidpp.o > > > +obj-$(CONFIG_HID_LOGITECH_K290) += hid-logitech-k290.o > > > obj-$(CONFIG_HID_MAGICMOUSE) += hid-magicmouse.o > > > obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o > > > obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > > index 9454ac134ce2..68caba7e666c 100644 > > > --- a/drivers/hid/hid-ids.h > > > +++ b/drivers/hid/hid-ids.h > > > @@ -693,6 +693,7 @@ > > > #define USB_DEVICE_ID_LOGITECH_HARMONY_LAST 0xc14f > > > #define USB_DEVICE_ID_LOGITECH_HARMONY_PS3 0x0306 > > > #define USB_DEVICE_ID_LOGITECH_KEYBOARD_G710_PLUS 0xc24d > > > +#define USB_DEVICE_ID_LOGITECH_KEYBOARD_K290 0xc31f > > > #define USB_DEVICE_ID_LOGITECH_MOUSE_C01A 0xc01a > > > #define USB_DEVICE_ID_LOGITECH_MOUSE_C05A 0xc05a > > > #define USB_DEVICE_ID_LOGITECH_MOUSE_C06A 0xc06a > > > diff --git a/drivers/hid/hid-logitech-k290.c b/drivers/hid/hid- > > > logitech-k290.c > > > new file mode 100644 > > > index 000000000000..36fdb5838842 > > > --- /dev/null > > > +++ b/drivers/hid/hid-logitech-k290.c > > > @@ -0,0 +1,100 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * HID driver for Logitech K290 keyboard > > > + * > > > + * Copyright (c) 2018 Florent Flament > > > + * > > > + * This drivers allows to configure the K290 keyboard's function > > > key > > > + * behaviour (whether function mode is activated or not by > > > default). > > > + * > > > + * Logitech custom commands taken from Marcus Ilgner k290- > > > fnkeyctl > > > + * (https://github.com/milgner/k290-fnkeyctl): > > > + * K290_SET_FUNCTION_CMD > > > + * K290_SET_FUNCTION_VAL > > > + * K290_SET_FUNCTION_OFF > > > + * K290_SET_FUNCTION_ON > > > + * > > > + * Based on hid-accutouch.c and hid-elo.c > > > + */ > > > + > > > +#include <linux/device.h> > > > +#include <linux/hid.h> > > > +#include <linux/module.h> > > > +#include <linux/stat.h> > > > +#include <linux/types.h> > > > +#include <linux/usb.h> > > > + > > > +#include "hid-ids.h" > > > +#include "usbhid/usbhid.h" > > > + > > > +// Logitech K290 custom USB command and value to setup function > > > key > > > +#define K290_SET_FUNCTION_CMD 0x02 > > > +#define K290_SET_FUNCTION_VAL 0x001a > > > + > > > +// Have function mode turned off (as with standard keyboards) > > > +#define K290_SET_FUNCTION_OFF 0x0001 > > > +// Have function mode turned on (default k290 behavior) > > > +#define K290_SET_FUNCTION_ON 0x0000 > > > + > > > +// Function key default mode is set at module load time for > > > every K290 > > > +// keyboards plugged on the machine. By default fn_mode = 1, i.e > > > +// sending K290_SET_FUNCTION_ON (default K290 behavior). > > > +static bool fn_mode = 1; > > > +module_param(fn_mode, bool, 0444); > > > +MODULE_PARM_DESC(fn_mode, "Logitech K290 function key mode > > > (default = 1)"); > > > + > > > +static void k290_set_function(struct usb_device *dev, uint16_t > > > function_mode) > > > +{ > > > + int ret; > > > + > > > + ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), > > > + K290_SET_FUNCTION_CMD, > > > + USB_DIR_OUT | USB_TYPE_VENDOR | > > > USB_RECIP_DEVICE, > > > + K290_SET_FUNCTION_VAL, > > > + function_mode, 0, 0, > > > USB_CTRL_SET_TIMEOUT); > > > + > > > + if (ret < 0) > > > + dev_err(&dev->dev, > > > + "Failed to setup K290 function key, error > > > %d\n", ret); > > > +} > > > + > > > +static int k290_set_function_hid_device(struct hid_device *hid) > > > +{ > > > + struct usb_device *usb_dev = hid_to_usb_dev(hid); > > > + > > > + k290_set_function(usb_dev, > > > + fn_mode ? K290_SET_FUNCTION_ON : > > > K290_SET_FUNCTION_OFF); > > > + return 0; > > > +} > > > + > > > +static int k290_input_configured(struct hid_device *hid, > > > + struct hid_input *hidinput) > > > +{ > > > + return k290_set_function_hid_device(hid); > > > +} > > > + > > > +static int k290_resume(struct hid_device *hid) > > > +{ > > > + return k290_set_function_hid_device(hid); > > > +} > > > + > > > +static const struct hid_device_id k290_devices[] = { > > > + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, > > > + USB_DEVICE_ID_LOGITECH_KEYBOARD_K290) }, > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(hid, k290_devices); > > > + > > > +static struct hid_driver k290_driver = { > > > + .name = "hid-logitech-k290", > > > + .id_table = k290_devices, > > > + .input_configured = k290_input_configured, > > > + .resume = k290_resume, > > > + .reset_resume = k290_resume, > > > +}; > > > + > > > +module_hid_driver(k290_driver); > > > + > > > +MODULE_AUTHOR("Florent Flament <contact@xxxxxxxxxxxxxxxxxx>"); > > > +MODULE_DESCRIPTION("Logitech K290 keyboard driver"); > > > +MODULE_LICENSE("GPL v2"); > > > -- > > > 2.14.3 > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux- > > > input" in > > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > > More majordomo info at http://vger.kernel.org/majordomo-info.htm > > > l -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
