| Message ID | 1405236262-8070-2-git-send-email-jm@lentin.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Sun, 13 Jul 2014 08:24:19 +0100 Jamie Lentin <jm@lentin.co.uk> wrote: > Rename module and all functions within so we can add support for other > keyboards in the same file. Rename the _tp postfix to _tpkbd, to > signify functions relevant to the TP USB keyboard. > > Signed-off-by: Jamie Lentin <jm@lentin.co.uk> Reviewed-by: Antonio Ospite <ao2@ao2.it> Patch looks good to me, easier to validate now :) A few ideas below for some possible _future_ cleanups, but no reason to hold this one any longer IMHO. Ciao, Antonio > --- > ...er-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} | 0 > drivers/hid/Kconfig | 14 +- > drivers/hid/Makefile | 2 +- > drivers/hid/hid-core.c | 2 +- > drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} | 185 +++++++++++---------- > 5 files changed, 102 insertions(+), 101 deletions(-) > rename Documentation/ABI/testing/{sysfs-driver-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} (100%) > rename drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} (64%) > > diff --git a/Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd b/Documentation/ABI/testing/sysfs-driver-hid-lenovo > similarity index 100% > rename from Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd > rename to Documentation/ABI/testing/sysfs-driver-hid-lenovo > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index 800c8b6..9b7acfc 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -331,18 +331,18 @@ config HID_LCPOWER > ---help--- > Support for LC-Power RC1000MCE RF remote control. > > -config HID_LENOVO_TPKBD > - tristate "Lenovo ThinkPad USB Keyboard with TrackPoint" > +config HID_LENOVO > + tristate "Lenovo / Thinkpad devices" > depends on HID > select NEW_LEDS > select LEDS_CLASS > ---help--- > - Support for the Lenovo ThinkPad USB Keyboard with TrackPoint. > + Support for Lenovo devices that are not fully compliant with HID standard. > > - Say Y here if you have a Lenovo ThinkPad USB Keyboard with TrackPoint > - and would like to use device-specific features like changing the > - sensitivity of the trackpoint, using the microphone mute button or > - controlling the mute and microphone mute LEDs. > + Say Y if you want support for the non-compliant features of the Lenovo > + Thinkpad standalone keyboards, e.g: This could become: Say Y if you want support for the non-HID-compliant features of Lenovo devices, e.g: ... because, in principle, the driver is not only about keyboards anymore. > > + - ThinkPad USB Keyboard with TrackPoint (supports extra LEDs and trackpoint > + configuration) > > config HID_LOGITECH > tristate "Logitech devices" if EXPERT > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index a6fa6ba..5e96be3 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -59,7 +59,7 @@ obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o > obj-$(CONFIG_HID_KEYTOUCH) += hid-keytouch.o > obj-$(CONFIG_HID_KYE) += hid-kye.o > obj-$(CONFIG_HID_LCPOWER) += hid-lcpower.o > -obj-$(CONFIG_HID_LENOVO_TPKBD) += hid-lenovo-tpkbd.o > +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_MAGICMOUSE) += hid-magicmouse.o > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 8ed66fd..55841bd 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1796,7 +1796,7 @@ static const struct hid_device_id hid_have_special_driver[] = { > { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LABTEC, USB_DEVICE_ID_LABTEC_WIRELESS_KEYBOARD) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LCPOWER, USB_DEVICE_ID_LCPOWER_LC1000 ) }, > -#if IS_ENABLED(CONFIG_HID_LENOVO_TPKBD) > +#if IS_ENABLED(CONFIG_HID_LENOVO) > { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) }, > #endif > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) }, > diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo.c > similarity index 64% > rename from drivers/hid/hid-lenovo-tpkbd.c > rename to drivers/hid/hid-lenovo.c > index 2d25b6c..0320b96 100644 > --- a/drivers/hid/hid-lenovo-tpkbd.c > +++ b/drivers/hid/hid-lenovo.c > @@ -20,8 +20,7 @@ > > #include "hid-ids.h" > > -/* This is only used for the trackpoint part of the driver, hence _tp */ > -struct tpkbd_data_pointer { > +struct lenovo_drvdata_tpkbd { > int led_state; > struct led_classdev led_mute; > struct led_classdev led_micmute; > @@ -35,7 +34,7 @@ struct tpkbd_data_pointer { > > #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c)) > > -static int tpkbd_input_mapping(struct hid_device *hdev, > +static int lenovo_input_mapping_tpkbd(struct hid_device *hdev, > struct hid_input *hi, struct hid_field *field, > struct hid_usage *usage, unsigned long **bit, int *max) > { > @@ -50,10 +49,10 @@ static int tpkbd_input_mapping(struct hid_device *hdev, > > #undef map_key_clear > > -static int tpkbd_features_set(struct hid_device *hdev) > +static int lenovo_features_set_tpkbd(struct hid_device *hdev) > { > struct hid_report *report; > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); I think the word "pointer" here is misleading, sometimes in other parts of the driver it is used as a synonym of trackpoint, but lenovo_drvdata_tpkbd also contains data about leds, so here it looks like it refers to C pointers. I'd call the variable tpkbd_data or tpkbd_drvdata, throughout the driver. What do you think? And the driver could standardize on the word "trackpoint" where appropriate in order to eliminate any ambiguity. I know, these are not your faults :) > > report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[4]; > > @@ -69,23 +68,23 @@ static int tpkbd_features_set(struct hid_device *hdev) > return 0; > } > > -static ssize_t pointer_press_to_select_show(struct device *dev, > +static ssize_t attr_press_to_select_show_tpkbd(struct device *dev, > struct device_attribute *attr, > char *buf) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > > return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->press_to_select); > } > > -static ssize_t pointer_press_to_select_store(struct device *dev, > +static ssize_t attr_press_to_select_store_tpkbd(struct device *dev, > struct device_attribute *attr, > const char *buf, > size_t count) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > int value; > > if (kstrtoint(buf, 10, &value)) > @@ -94,28 +93,28 @@ static ssize_t pointer_press_to_select_store(struct device *dev, > return -EINVAL; > > data_pointer->press_to_select = value; > - tpkbd_features_set(hdev); > + lenovo_features_set_tpkbd(hdev); > > return count; > } > > -static ssize_t pointer_dragging_show(struct device *dev, > +static ssize_t attr_dragging_show_tpkbd(struct device *dev, > struct device_attribute *attr, > char *buf) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > > return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->dragging); > } > > -static ssize_t pointer_dragging_store(struct device *dev, > +static ssize_t attr_dragging_store_tpkbd(struct device *dev, > struct device_attribute *attr, > const char *buf, > size_t count) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > int value; > > if (kstrtoint(buf, 10, &value)) > @@ -124,28 +123,28 @@ static ssize_t pointer_dragging_store(struct device *dev, > return -EINVAL; > > data_pointer->dragging = value; > - tpkbd_features_set(hdev); > + lenovo_features_set_tpkbd(hdev); > > return count; > } > > -static ssize_t pointer_release_to_select_show(struct device *dev, > +static ssize_t attr_release_to_select_show_tpkbd(struct device *dev, > struct device_attribute *attr, > char *buf) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > > return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->release_to_select); > } > > -static ssize_t pointer_release_to_select_store(struct device *dev, > +static ssize_t attr_release_to_select_store_tpkbd(struct device *dev, > struct device_attribute *attr, > const char *buf, > size_t count) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > int value; > > if (kstrtoint(buf, 10, &value)) > @@ -154,28 +153,28 @@ static ssize_t pointer_release_to_select_store(struct device *dev, > return -EINVAL; > > data_pointer->release_to_select = value; > - tpkbd_features_set(hdev); > + lenovo_features_set_tpkbd(hdev); > > return count; > } > > -static ssize_t pointer_select_right_show(struct device *dev, > +static ssize_t attr_select_right_show_tpkbd(struct device *dev, > struct device_attribute *attr, > char *buf) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > > return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->select_right); > } > > -static ssize_t pointer_select_right_store(struct device *dev, > +static ssize_t attr_select_right_store_tpkbd(struct device *dev, > struct device_attribute *attr, > const char *buf, > size_t count) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > int value; > > if (kstrtoint(buf, 10, &value)) > @@ -184,119 +183,119 @@ static ssize_t pointer_select_right_store(struct device *dev, > return -EINVAL; > > data_pointer->select_right = value; > - tpkbd_features_set(hdev); > + lenovo_features_set_tpkbd(hdev); > > return count; > } > > -static ssize_t pointer_sensitivity_show(struct device *dev, > +static ssize_t attr_sensitivity_show_tpkbd(struct device *dev, > struct device_attribute *attr, > char *buf) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > > return snprintf(buf, PAGE_SIZE, "%u\n", > data_pointer->sensitivity); > } > > -static ssize_t pointer_sensitivity_store(struct device *dev, > +static ssize_t attr_sensitivity_store_tpkbd(struct device *dev, > struct device_attribute *attr, > const char *buf, > size_t count) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > int value; > > if (kstrtoint(buf, 10, &value) || value < 1 || value > 255) > return -EINVAL; > > data_pointer->sensitivity = value; > - tpkbd_features_set(hdev); > + lenovo_features_set_tpkbd(hdev); > > return count; > } > > -static ssize_t pointer_press_speed_show(struct device *dev, > +static ssize_t attr_press_speed_show_tpkbd(struct device *dev, > struct device_attribute *attr, > char *buf) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > > return snprintf(buf, PAGE_SIZE, "%u\n", > data_pointer->press_speed); > } > > -static ssize_t pointer_press_speed_store(struct device *dev, > +static ssize_t attr_press_speed_store_tpkbd(struct device *dev, > struct device_attribute *attr, > const char *buf, > size_t count) > { > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > int value; > > if (kstrtoint(buf, 10, &value) || value < 1 || value > 255) > return -EINVAL; > > data_pointer->press_speed = value; > - tpkbd_features_set(hdev); > + lenovo_features_set_tpkbd(hdev); > > return count; > } > > -static struct device_attribute dev_attr_pointer_press_to_select = > +static struct device_attribute dev_attr_press_to_select_tpkbd = > __ATTR(press_to_select, S_IWUSR | S_IRUGO, > - pointer_press_to_select_show, > - pointer_press_to_select_store); > + attr_press_to_select_show_tpkbd, > + attr_press_to_select_store_tpkbd); > > -static struct device_attribute dev_attr_pointer_dragging = > +static struct device_attribute dev_attr_dragging_tpkbd = > __ATTR(dragging, S_IWUSR | S_IRUGO, > - pointer_dragging_show, > - pointer_dragging_store); > + attr_dragging_show_tpkbd, > + attr_dragging_store_tpkbd); > > -static struct device_attribute dev_attr_pointer_release_to_select = > +static struct device_attribute dev_attr_release_to_select_tpkbd = > __ATTR(release_to_select, S_IWUSR | S_IRUGO, > - pointer_release_to_select_show, > - pointer_release_to_select_store); > + attr_release_to_select_show_tpkbd, > + attr_release_to_select_store_tpkbd); > > -static struct device_attribute dev_attr_pointer_select_right = > +static struct device_attribute dev_attr_select_right_tpkbd = > __ATTR(select_right, S_IWUSR | S_IRUGO, > - pointer_select_right_show, > - pointer_select_right_store); > + attr_select_right_show_tpkbd, > + attr_select_right_store_tpkbd); > > -static struct device_attribute dev_attr_pointer_sensitivity = > +static struct device_attribute dev_attr_sensitivity_tpkbd = > __ATTR(sensitivity, S_IWUSR | S_IRUGO, > - pointer_sensitivity_show, > - pointer_sensitivity_store); > + attr_sensitivity_show_tpkbd, > + attr_sensitivity_store_tpkbd); > > -static struct device_attribute dev_attr_pointer_press_speed = > +static struct device_attribute dev_attr_press_speed_tpkbd = > __ATTR(press_speed, S_IWUSR | S_IRUGO, > - pointer_press_speed_show, > - pointer_press_speed_store); > - > -static struct attribute *tpkbd_attributes_pointer[] = { > - &dev_attr_pointer_press_to_select.attr, > - &dev_attr_pointer_dragging.attr, > - &dev_attr_pointer_release_to_select.attr, > - &dev_attr_pointer_select_right.attr, > - &dev_attr_pointer_sensitivity.attr, > - &dev_attr_pointer_press_speed.attr, > + attr_press_speed_show_tpkbd, > + attr_press_speed_store_tpkbd); > + > +static struct attribute *lenovo_attributes_tpkbd[] = { > + &dev_attr_press_to_select_tpkbd.attr, > + &dev_attr_dragging_tpkbd.attr, > + &dev_attr_release_to_select_tpkbd.attr, > + &dev_attr_select_right_tpkbd.attr, > + &dev_attr_sensitivity_tpkbd.attr, > + &dev_attr_press_speed_tpkbd.attr, > NULL > }; > > -static const struct attribute_group tpkbd_attr_group_pointer = { > - .attrs = tpkbd_attributes_pointer, > +static const struct attribute_group lenovo_attr_group_tpkbd = { > + .attrs = lenovo_attributes_tpkbd, > }; > > -static enum led_brightness tpkbd_led_brightness_get( > +static enum led_brightness lenovo_led_brightness_get_tpkbd( > struct led_classdev *led_cdev) > { > struct device *dev = led_cdev->dev->parent; > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > int led_nr = 0; > > if (led_cdev == &data_pointer->led_micmute) > @@ -307,12 +306,12 @@ static enum led_brightness tpkbd_led_brightness_get( > : LED_OFF; > } > > -static void tpkbd_led_brightness_set(struct led_classdev *led_cdev, > +static void lenovo_led_brightness_set_tpkbd(struct led_classdev *led_cdev, > enum led_brightness value) > { > struct device *dev = led_cdev->dev->parent; > struct hid_device *hdev = container_of(dev, struct hid_device, dev); > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > struct hid_report *report; > int led_nr = 0; > > @@ -330,10 +329,10 @@ static void tpkbd_led_brightness_set(struct led_classdev *led_cdev, > hid_hw_request(hdev, report, HID_REQ_SET_REPORT); > } > > -static int tpkbd_probe_tp(struct hid_device *hdev) > +static int lenovo_probe_tpkbd(struct hid_device *hdev) > { > struct device *dev = &hdev->dev; > - struct tpkbd_data_pointer *data_pointer; > + struct lenovo_drvdata_tpkbd *data_pointer; > size_t name_sz = strlen(dev_name(dev)) + 16; > char *name_mute, *name_micmute; > int i; > @@ -347,12 +346,12 @@ static int tpkbd_probe_tp(struct hid_device *hdev) > return -ENODEV; > > if (sysfs_create_group(&hdev->dev.kobj, > - &tpkbd_attr_group_pointer)) { > + &lenovo_attr_group_tpkbd)) { > hid_warn(hdev, "Could not create sysfs group\n"); > } > > data_pointer = devm_kzalloc(&hdev->dev, > - sizeof(struct tpkbd_data_pointer), > + sizeof(struct lenovo_drvdata_tpkbd), > GFP_KERNEL); > if (data_pointer == NULL) { > hid_err(hdev, "Could not allocate memory for driver data\n"); > @@ -375,23 +374,25 @@ static int tpkbd_probe_tp(struct hid_device *hdev) > hid_set_drvdata(hdev, data_pointer); > > data_pointer->led_mute.name = name_mute; > - data_pointer->led_mute.brightness_get = tpkbd_led_brightness_get; > - data_pointer->led_mute.brightness_set = tpkbd_led_brightness_set; > + data_pointer->led_mute.brightness_get = lenovo_led_brightness_get_tpkbd; > + data_pointer->led_mute.brightness_set = lenovo_led_brightness_set_tpkbd; > data_pointer->led_mute.dev = dev; > led_classdev_register(dev, &data_pointer->led_mute); > > data_pointer->led_micmute.name = name_micmute; > - data_pointer->led_micmute.brightness_get = tpkbd_led_brightness_get; > - data_pointer->led_micmute.brightness_set = tpkbd_led_brightness_set; > + data_pointer->led_micmute.brightness_get = > + lenovo_led_brightness_get_tpkbd; > + data_pointer->led_micmute.brightness_set = > + lenovo_led_brightness_set_tpkbd; > data_pointer->led_micmute.dev = dev; > led_classdev_register(dev, &data_pointer->led_micmute); > > - tpkbd_features_set(hdev); > + lenovo_features_set_tpkbd(hdev); > > return 0; > } > > -static int tpkbd_probe(struct hid_device *hdev, > +static int lenovo_probe(struct hid_device *hdev, > const struct hid_device_id *id) > { > int ret; > @@ -410,7 +411,7 @@ static int tpkbd_probe(struct hid_device *hdev, > > if (hid_get_drvdata(hdev)) { > hid_set_drvdata(hdev, NULL); > - ret = tpkbd_probe_tp(hdev); > + ret = lenovo_probe_tpkbd(hdev); > if (ret) > goto err_hid; > } > @@ -422,12 +423,12 @@ err: > return ret; > } > > -static void tpkbd_remove_tp(struct hid_device *hdev) > +static void lenovo_remove_tpkbd(struct hid_device *hdev) > { > - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > > sysfs_remove_group(&hdev->dev.kobj, > - &tpkbd_attr_group_pointer); > + &lenovo_attr_group_tpkbd); > > led_classdev_unregister(&data_pointer->led_micmute); > led_classdev_unregister(&data_pointer->led_mute); > @@ -435,28 +436,28 @@ static void tpkbd_remove_tp(struct hid_device *hdev) > hid_set_drvdata(hdev, NULL); > } > > -static void tpkbd_remove(struct hid_device *hdev) > +static void lenovo_remove(struct hid_device *hdev) > { > if (hid_get_drvdata(hdev)) > - tpkbd_remove_tp(hdev); > + lenovo_remove_tpkbd(hdev); > > hid_hw_stop(hdev); > } > > -static const struct hid_device_id tpkbd_devices[] = { > +static const struct hid_device_id lenovo_devices[] = { > { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) }, > { } > }; > > -MODULE_DEVICE_TABLE(hid, tpkbd_devices); > +MODULE_DEVICE_TABLE(hid, lenovo_devices); > > -static struct hid_driver tpkbd_driver = { > - .name = "lenovo_tpkbd", > - .id_table = tpkbd_devices, > - .input_mapping = tpkbd_input_mapping, > - .probe = tpkbd_probe, > - .remove = tpkbd_remove, > +static struct hid_driver lenovo_driver = { > + .name = "lenovo", > + .id_table = lenovo_devices, > + .input_mapping = lenovo_input_mapping_tpkbd, > + .probe = lenovo_probe, > + .remove = lenovo_remove, > }; > -module_hid_driver(tpkbd_driver); > +module_hid_driver(lenovo_driver); > > MODULE_LICENSE("GPL"); > -- > 2.0.0 >
On Mon, 14 Jul 2014, Antonio Ospite wrote: > On Sun, 13 Jul 2014 08:24:19 +0100 > Jamie Lentin <jm@lentin.co.uk> wrote: > >> Rename module and all functions within so we can add support for other >> keyboards in the same file. Rename the _tp postfix to _tpkbd, to >> signify functions relevant to the TP USB keyboard. >> >> Signed-off-by: Jamie Lentin <jm@lentin.co.uk> > > Reviewed-by: Antonio Ospite <ao2@ao2.it> > > Patch looks good to me, easier to validate now :) > > A few ideas below for some possible _future_ cleanups, but no reason to > hold this one any longer IMHO. > > Ciao, > Antonio > >> --- >> ...er-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} | 0 >> drivers/hid/Kconfig | 14 +- >> drivers/hid/Makefile | 2 +- >> drivers/hid/hid-core.c | 2 +- >> drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} | 185 +++++++++++---------- >> 5 files changed, 102 insertions(+), 101 deletions(-) >> rename Documentation/ABI/testing/{sysfs-driver-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} (100%) >> rename drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} (64%) >> >> diff --git a/Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd b/Documentation/ABI/testing/sysfs-driver-hid-lenovo >> similarity index 100% >> rename from Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd >> rename to Documentation/ABI/testing/sysfs-driver-hid-lenovo >> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig >> index 800c8b6..9b7acfc 100644 >> --- a/drivers/hid/Kconfig >> +++ b/drivers/hid/Kconfig >> @@ -331,18 +331,18 @@ config HID_LCPOWER >> ---help--- >> Support for LC-Power RC1000MCE RF remote control. >> >> -config HID_LENOVO_TPKBD >> - tristate "Lenovo ThinkPad USB Keyboard with TrackPoint" >> +config HID_LENOVO >> + tristate "Lenovo / Thinkpad devices" >> depends on HID >> select NEW_LEDS >> select LEDS_CLASS >> ---help--- >> - Support for the Lenovo ThinkPad USB Keyboard with TrackPoint. >> + Support for Lenovo devices that are not fully compliant with HID standard. >> >> - Say Y here if you have a Lenovo ThinkPad USB Keyboard with TrackPoint >> - and would like to use device-specific features like changing the >> - sensitivity of the trackpoint, using the microphone mute button or >> - controlling the mute and microphone mute LEDs. >> + Say Y if you want support for the non-compliant features of the Lenovo >> + Thinkpad standalone keyboards, e.g: > > This could become: > > Say Y if you want support for the non-HID-compliant features of Lenovo > devices, e.g: > ... > > because, in principle, the driver is not only about keyboards anymore. True, although I'd be very surprised if anything else came up :) >> >> + - ThinkPad USB Keyboard with TrackPoint (supports extra LEDs and trackpoint >> + configuration) >> >> config HID_LOGITECH >> tristate "Logitech devices" if EXPERT >> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile >> index a6fa6ba..5e96be3 100644 >> --- a/drivers/hid/Makefile >> +++ b/drivers/hid/Makefile >> @@ -59,7 +59,7 @@ obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o >> obj-$(CONFIG_HID_KEYTOUCH) += hid-keytouch.o >> obj-$(CONFIG_HID_KYE) += hid-kye.o >> obj-$(CONFIG_HID_LCPOWER) += hid-lcpower.o >> -obj-$(CONFIG_HID_LENOVO_TPKBD) += hid-lenovo-tpkbd.o >> +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_MAGICMOUSE) += hid-magicmouse.o >> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c >> index 8ed66fd..55841bd 100644 >> --- a/drivers/hid/hid-core.c >> +++ b/drivers/hid/hid-core.c >> @@ -1796,7 +1796,7 @@ static const struct hid_device_id hid_have_special_driver[] = { >> { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X) }, >> { HID_USB_DEVICE(USB_VENDOR_ID_LABTEC, USB_DEVICE_ID_LABTEC_WIRELESS_KEYBOARD) }, >> { HID_USB_DEVICE(USB_VENDOR_ID_LCPOWER, USB_DEVICE_ID_LCPOWER_LC1000 ) }, >> -#if IS_ENABLED(CONFIG_HID_LENOVO_TPKBD) >> +#if IS_ENABLED(CONFIG_HID_LENOVO) >> { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) }, >> #endif >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) }, >> diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo.c >> similarity index 64% >> rename from drivers/hid/hid-lenovo-tpkbd.c >> rename to drivers/hid/hid-lenovo.c >> index 2d25b6c..0320b96 100644 >> --- a/drivers/hid/hid-lenovo-tpkbd.c >> +++ b/drivers/hid/hid-lenovo.c >> @@ -20,8 +20,7 @@ >> >> #include "hid-ids.h" >> >> -/* This is only used for the trackpoint part of the driver, hence _tp */ >> -struct tpkbd_data_pointer { >> +struct lenovo_drvdata_tpkbd { >> int led_state; >> struct led_classdev led_mute; >> struct led_classdev led_micmute; >> @@ -35,7 +34,7 @@ struct tpkbd_data_pointer { >> >> #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c)) >> >> -static int tpkbd_input_mapping(struct hid_device *hdev, >> +static int lenovo_input_mapping_tpkbd(struct hid_device *hdev, >> struct hid_input *hi, struct hid_field *field, >> struct hid_usage *usage, unsigned long **bit, int *max) >> { >> @@ -50,10 +49,10 @@ static int tpkbd_input_mapping(struct hid_device *hdev, >> >> #undef map_key_clear >> >> -static int tpkbd_features_set(struct hid_device *hdev) >> +static int lenovo_features_set_tpkbd(struct hid_device *hdev) >> { >> struct hid_report *report; >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > > > I think the word "pointer" here is misleading, sometimes in other parts > of the driver it is used as a synonym of trackpoint, but > lenovo_drvdata_tpkbd also contains data about leds, so here it looks > like it refers to C pointers. I'd call the variable tpkbd_data or > tpkbd_drvdata, throughout the driver. What do you think? Yes, the rationale behind the name really isn't obvious but all meanings are sorta correct, so it's not that bad :P I like tpkbd_drvdata better though, and can correct it in the compact keyboard code too. I wanted to make the variable names match, but didn't get around to it, so will do it next time around. > And the driver could standardize on the word "trackpoint" where > appropriate in order to eliminate any ambiguity. The key other place I can think of that this happens is for the USB compact keyboard, for it's HID_TYPE_USBMOUSE half (versus the keyboard half). Saying trackpoint there is a mis-truth, since it's also where all the extra keys hide. But I'll have a look through anyway. > I know, these are not your faults :) > >> >> report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[4]; >> >> @@ -69,23 +68,23 @@ static int tpkbd_features_set(struct hid_device *hdev) >> return 0; >> } >> >> -static ssize_t pointer_press_to_select_show(struct device *dev, >> +static ssize_t attr_press_to_select_show_tpkbd(struct device *dev, >> struct device_attribute *attr, >> char *buf) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> >> return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->press_to_select); >> } >> >> -static ssize_t pointer_press_to_select_store(struct device *dev, >> +static ssize_t attr_press_to_select_store_tpkbd(struct device *dev, >> struct device_attribute *attr, >> const char *buf, >> size_t count) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> int value; >> >> if (kstrtoint(buf, 10, &value)) >> @@ -94,28 +93,28 @@ static ssize_t pointer_press_to_select_store(struct device *dev, >> return -EINVAL; >> >> data_pointer->press_to_select = value; >> - tpkbd_features_set(hdev); >> + lenovo_features_set_tpkbd(hdev); >> >> return count; >> } >> >> -static ssize_t pointer_dragging_show(struct device *dev, >> +static ssize_t attr_dragging_show_tpkbd(struct device *dev, >> struct device_attribute *attr, >> char *buf) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> >> return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->dragging); >> } >> >> -static ssize_t pointer_dragging_store(struct device *dev, >> +static ssize_t attr_dragging_store_tpkbd(struct device *dev, >> struct device_attribute *attr, >> const char *buf, >> size_t count) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> int value; >> >> if (kstrtoint(buf, 10, &value)) >> @@ -124,28 +123,28 @@ static ssize_t pointer_dragging_store(struct device *dev, >> return -EINVAL; >> >> data_pointer->dragging = value; >> - tpkbd_features_set(hdev); >> + lenovo_features_set_tpkbd(hdev); >> >> return count; >> } >> >> -static ssize_t pointer_release_to_select_show(struct device *dev, >> +static ssize_t attr_release_to_select_show_tpkbd(struct device *dev, >> struct device_attribute *attr, >> char *buf) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> >> return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->release_to_select); >> } >> >> -static ssize_t pointer_release_to_select_store(struct device *dev, >> +static ssize_t attr_release_to_select_store_tpkbd(struct device *dev, >> struct device_attribute *attr, >> const char *buf, >> size_t count) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> int value; >> >> if (kstrtoint(buf, 10, &value)) >> @@ -154,28 +153,28 @@ static ssize_t pointer_release_to_select_store(struct device *dev, >> return -EINVAL; >> >> data_pointer->release_to_select = value; >> - tpkbd_features_set(hdev); >> + lenovo_features_set_tpkbd(hdev); >> >> return count; >> } >> >> -static ssize_t pointer_select_right_show(struct device *dev, >> +static ssize_t attr_select_right_show_tpkbd(struct device *dev, >> struct device_attribute *attr, >> char *buf) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> >> return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->select_right); >> } >> >> -static ssize_t pointer_select_right_store(struct device *dev, >> +static ssize_t attr_select_right_store_tpkbd(struct device *dev, >> struct device_attribute *attr, >> const char *buf, >> size_t count) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> int value; >> >> if (kstrtoint(buf, 10, &value)) >> @@ -184,119 +183,119 @@ static ssize_t pointer_select_right_store(struct device *dev, >> return -EINVAL; >> >> data_pointer->select_right = value; >> - tpkbd_features_set(hdev); >> + lenovo_features_set_tpkbd(hdev); >> >> return count; >> } >> >> -static ssize_t pointer_sensitivity_show(struct device *dev, >> +static ssize_t attr_sensitivity_show_tpkbd(struct device *dev, >> struct device_attribute *attr, >> char *buf) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> >> return snprintf(buf, PAGE_SIZE, "%u\n", >> data_pointer->sensitivity); >> } >> >> -static ssize_t pointer_sensitivity_store(struct device *dev, >> +static ssize_t attr_sensitivity_store_tpkbd(struct device *dev, >> struct device_attribute *attr, >> const char *buf, >> size_t count) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> int value; >> >> if (kstrtoint(buf, 10, &value) || value < 1 || value > 255) >> return -EINVAL; >> >> data_pointer->sensitivity = value; >> - tpkbd_features_set(hdev); >> + lenovo_features_set_tpkbd(hdev); >> >> return count; >> } >> >> -static ssize_t pointer_press_speed_show(struct device *dev, >> +static ssize_t attr_press_speed_show_tpkbd(struct device *dev, >> struct device_attribute *attr, >> char *buf) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> >> return snprintf(buf, PAGE_SIZE, "%u\n", >> data_pointer->press_speed); >> } >> >> -static ssize_t pointer_press_speed_store(struct device *dev, >> +static ssize_t attr_press_speed_store_tpkbd(struct device *dev, >> struct device_attribute *attr, >> const char *buf, >> size_t count) >> { >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> int value; >> >> if (kstrtoint(buf, 10, &value) || value < 1 || value > 255) >> return -EINVAL; >> >> data_pointer->press_speed = value; >> - tpkbd_features_set(hdev); >> + lenovo_features_set_tpkbd(hdev); >> >> return count; >> } >> >> -static struct device_attribute dev_attr_pointer_press_to_select = >> +static struct device_attribute dev_attr_press_to_select_tpkbd = >> __ATTR(press_to_select, S_IWUSR | S_IRUGO, >> - pointer_press_to_select_show, >> - pointer_press_to_select_store); >> + attr_press_to_select_show_tpkbd, >> + attr_press_to_select_store_tpkbd); >> >> -static struct device_attribute dev_attr_pointer_dragging = >> +static struct device_attribute dev_attr_dragging_tpkbd = >> __ATTR(dragging, S_IWUSR | S_IRUGO, >> - pointer_dragging_show, >> - pointer_dragging_store); >> + attr_dragging_show_tpkbd, >> + attr_dragging_store_tpkbd); >> >> -static struct device_attribute dev_attr_pointer_release_to_select = >> +static struct device_attribute dev_attr_release_to_select_tpkbd = >> __ATTR(release_to_select, S_IWUSR | S_IRUGO, >> - pointer_release_to_select_show, >> - pointer_release_to_select_store); >> + attr_release_to_select_show_tpkbd, >> + attr_release_to_select_store_tpkbd); >> >> -static struct device_attribute dev_attr_pointer_select_right = >> +static struct device_attribute dev_attr_select_right_tpkbd = >> __ATTR(select_right, S_IWUSR | S_IRUGO, >> - pointer_select_right_show, >> - pointer_select_right_store); >> + attr_select_right_show_tpkbd, >> + attr_select_right_store_tpkbd); >> >> -static struct device_attribute dev_attr_pointer_sensitivity = >> +static struct device_attribute dev_attr_sensitivity_tpkbd = >> __ATTR(sensitivity, S_IWUSR | S_IRUGO, >> - pointer_sensitivity_show, >> - pointer_sensitivity_store); >> + attr_sensitivity_show_tpkbd, >> + attr_sensitivity_store_tpkbd); >> >> -static struct device_attribute dev_attr_pointer_press_speed = >> +static struct device_attribute dev_attr_press_speed_tpkbd = >> __ATTR(press_speed, S_IWUSR | S_IRUGO, >> - pointer_press_speed_show, >> - pointer_press_speed_store); >> - >> -static struct attribute *tpkbd_attributes_pointer[] = { >> - &dev_attr_pointer_press_to_select.attr, >> - &dev_attr_pointer_dragging.attr, >> - &dev_attr_pointer_release_to_select.attr, >> - &dev_attr_pointer_select_right.attr, >> - &dev_attr_pointer_sensitivity.attr, >> - &dev_attr_pointer_press_speed.attr, >> + attr_press_speed_show_tpkbd, >> + attr_press_speed_store_tpkbd); >> + >> +static struct attribute *lenovo_attributes_tpkbd[] = { >> + &dev_attr_press_to_select_tpkbd.attr, >> + &dev_attr_dragging_tpkbd.attr, >> + &dev_attr_release_to_select_tpkbd.attr, >> + &dev_attr_select_right_tpkbd.attr, >> + &dev_attr_sensitivity_tpkbd.attr, >> + &dev_attr_press_speed_tpkbd.attr, >> NULL >> }; >> >> -static const struct attribute_group tpkbd_attr_group_pointer = { >> - .attrs = tpkbd_attributes_pointer, >> +static const struct attribute_group lenovo_attr_group_tpkbd = { >> + .attrs = lenovo_attributes_tpkbd, >> }; >> >> -static enum led_brightness tpkbd_led_brightness_get( >> +static enum led_brightness lenovo_led_brightness_get_tpkbd( >> struct led_classdev *led_cdev) >> { >> struct device *dev = led_cdev->dev->parent; >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> int led_nr = 0; >> >> if (led_cdev == &data_pointer->led_micmute) >> @@ -307,12 +306,12 @@ static enum led_brightness tpkbd_led_brightness_get( >> : LED_OFF; >> } >> >> -static void tpkbd_led_brightness_set(struct led_classdev *led_cdev, >> +static void lenovo_led_brightness_set_tpkbd(struct led_classdev *led_cdev, >> enum led_brightness value) >> { >> struct device *dev = led_cdev->dev->parent; >> struct hid_device *hdev = container_of(dev, struct hid_device, dev); >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> struct hid_report *report; >> int led_nr = 0; >> >> @@ -330,10 +329,10 @@ static void tpkbd_led_brightness_set(struct led_classdev *led_cdev, >> hid_hw_request(hdev, report, HID_REQ_SET_REPORT); >> } >> >> -static int tpkbd_probe_tp(struct hid_device *hdev) >> +static int lenovo_probe_tpkbd(struct hid_device *hdev) >> { >> struct device *dev = &hdev->dev; >> - struct tpkbd_data_pointer *data_pointer; >> + struct lenovo_drvdata_tpkbd *data_pointer; >> size_t name_sz = strlen(dev_name(dev)) + 16; >> char *name_mute, *name_micmute; >> int i; >> @@ -347,12 +346,12 @@ static int tpkbd_probe_tp(struct hid_device *hdev) >> return -ENODEV; >> >> if (sysfs_create_group(&hdev->dev.kobj, >> - &tpkbd_attr_group_pointer)) { >> + &lenovo_attr_group_tpkbd)) { >> hid_warn(hdev, "Could not create sysfs group\n"); >> } >> >> data_pointer = devm_kzalloc(&hdev->dev, >> - sizeof(struct tpkbd_data_pointer), >> + sizeof(struct lenovo_drvdata_tpkbd), >> GFP_KERNEL); >> if (data_pointer == NULL) { >> hid_err(hdev, "Could not allocate memory for driver data\n"); >> @@ -375,23 +374,25 @@ static int tpkbd_probe_tp(struct hid_device *hdev) >> hid_set_drvdata(hdev, data_pointer); >> >> data_pointer->led_mute.name = name_mute; >> - data_pointer->led_mute.brightness_get = tpkbd_led_brightness_get; >> - data_pointer->led_mute.brightness_set = tpkbd_led_brightness_set; >> + data_pointer->led_mute.brightness_get = lenovo_led_brightness_get_tpkbd; >> + data_pointer->led_mute.brightness_set = lenovo_led_brightness_set_tpkbd; >> data_pointer->led_mute.dev = dev; >> led_classdev_register(dev, &data_pointer->led_mute); >> >> data_pointer->led_micmute.name = name_micmute; >> - data_pointer->led_micmute.brightness_get = tpkbd_led_brightness_get; >> - data_pointer->led_micmute.brightness_set = tpkbd_led_brightness_set; >> + data_pointer->led_micmute.brightness_get = >> + lenovo_led_brightness_get_tpkbd; >> + data_pointer->led_micmute.brightness_set = >> + lenovo_led_brightness_set_tpkbd; >> data_pointer->led_micmute.dev = dev; >> led_classdev_register(dev, &data_pointer->led_micmute); >> >> - tpkbd_features_set(hdev); >> + lenovo_features_set_tpkbd(hdev); >> >> return 0; >> } >> >> -static int tpkbd_probe(struct hid_device *hdev, >> +static int lenovo_probe(struct hid_device *hdev, >> const struct hid_device_id *id) >> { >> int ret; >> @@ -410,7 +411,7 @@ static int tpkbd_probe(struct hid_device *hdev, >> >> if (hid_get_drvdata(hdev)) { >> hid_set_drvdata(hdev, NULL); >> - ret = tpkbd_probe_tp(hdev); >> + ret = lenovo_probe_tpkbd(hdev); >> if (ret) >> goto err_hid; >> } >> @@ -422,12 +423,12 @@ err: >> return ret; >> } >> >> -static void tpkbd_remove_tp(struct hid_device *hdev) >> +static void lenovo_remove_tpkbd(struct hid_device *hdev) >> { >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); >> >> sysfs_remove_group(&hdev->dev.kobj, >> - &tpkbd_attr_group_pointer); >> + &lenovo_attr_group_tpkbd); >> >> led_classdev_unregister(&data_pointer->led_micmute); >> led_classdev_unregister(&data_pointer->led_mute); >> @@ -435,28 +436,28 @@ static void tpkbd_remove_tp(struct hid_device *hdev) >> hid_set_drvdata(hdev, NULL); >> } >> >> -static void tpkbd_remove(struct hid_device *hdev) >> +static void lenovo_remove(struct hid_device *hdev) >> { >> if (hid_get_drvdata(hdev)) >> - tpkbd_remove_tp(hdev); >> + lenovo_remove_tpkbd(hdev); >> >> hid_hw_stop(hdev); >> } >> >> -static const struct hid_device_id tpkbd_devices[] = { >> +static const struct hid_device_id lenovo_devices[] = { >> { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) }, >> { } >> }; >> >> -MODULE_DEVICE_TABLE(hid, tpkbd_devices); >> +MODULE_DEVICE_TABLE(hid, lenovo_devices); >> >> -static struct hid_driver tpkbd_driver = { >> - .name = "lenovo_tpkbd", >> - .id_table = tpkbd_devices, >> - .input_mapping = tpkbd_input_mapping, >> - .probe = tpkbd_probe, >> - .remove = tpkbd_remove, >> +static struct hid_driver lenovo_driver = { >> + .name = "lenovo", >> + .id_table = lenovo_devices, >> + .input_mapping = lenovo_input_mapping_tpkbd, >> + .probe = lenovo_probe, >> + .remove = lenovo_remove, >> }; >> -module_hid_driver(tpkbd_driver); >> +module_hid_driver(lenovo_driver); >> >> MODULE_LICENSE("GPL"); >> -- >> 2.0.0 >> > > >
On Fri, 18 Jul 2014 10:35:35 +0100 (BST) Jamie Lentin <jm@lentin.co.uk> wrote: > On Mon, 14 Jul 2014, Antonio Ospite wrote: > > > On Sun, 13 Jul 2014 08:24:19 +0100 > > Jamie Lentin <jm@lentin.co.uk> wrote: > > > >> Rename module and all functions within so we can add support for other > >> keyboards in the same file. Rename the _tp postfix to _tpkbd, to > >> signify functions relevant to the TP USB keyboard. > >> > >> Signed-off-by: Jamie Lentin <jm@lentin.co.uk> > > > > Reviewed-by: Antonio Ospite <ao2@ao2.it> > > > > Patch looks good to me, easier to validate now :) > > > > A few ideas below for some possible _future_ cleanups, but no reason to > > hold this one any longer IMHO. > > > > Ciao, > > Antonio > > > >> --- > >> ...er-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} | 0 > >> drivers/hid/Kconfig | 14 +- > >> drivers/hid/Makefile | 2 +- > >> drivers/hid/hid-core.c | 2 +- > >> drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} | 185 +++++++++++---------- > >> 5 files changed, 102 insertions(+), 101 deletions(-) > >> rename Documentation/ABI/testing/{sysfs-driver-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} (100%) > >> rename drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} (64%) > >> [...] > >> -static int tpkbd_features_set(struct hid_device *hdev) > >> +static int lenovo_features_set_tpkbd(struct hid_device *hdev) > >> { > >> struct hid_report *report; > >> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); > >> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); > > > > > > I think the word "pointer" here is misleading, sometimes in other parts > > of the driver it is used as a synonym of trackpoint, but > > lenovo_drvdata_tpkbd also contains data about leds, so here it looks > > like it refers to C pointers. I'd call the variable tpkbd_data or > > tpkbd_drvdata, throughout the driver. What do you think? > > Yes, the rationale behind the name really isn't obvious but all meanings > are sorta correct, so it's not that bad :P > > I like tpkbd_drvdata better though, and can correct it in the compact > keyboard code too. I wanted to make the variable names match, but didn't > get around to it, so will do it next time around. > IMHO you can very well use data_pointer for now so the names are consistent with the current ones; a cleanup can be done later to make the naming more straightforward too. > > And the driver could standardize on the word "trackpoint" where > > appropriate in order to eliminate any ambiguity. > > The key other place I can think of that this happens is for the USB > compact keyboard, for it's HID_TYPE_USBMOUSE half (versus the keyboard > half). Saying trackpoint there is a mis-truth, since it's also where all > the extra keys hide. But I'll have a look through anyway. > I meant: where "pointer" means trackpoint let's just use "trackpoint", where pointer means "not keyboard" (like in data_pointer apparently) something else can be used if "trackpoint" is too specific. But again, this is cleanup: it'd be nice to have it eventually but not worth blocking this patchset. Thanks, Antonio
diff --git a/Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd b/Documentation/ABI/testing/sysfs-driver-hid-lenovo similarity index 100% rename from Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd rename to Documentation/ABI/testing/sysfs-driver-hid-lenovo diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 800c8b6..9b7acfc 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -331,18 +331,18 @@ config HID_LCPOWER ---help--- Support for LC-Power RC1000MCE RF remote control. -config HID_LENOVO_TPKBD - tristate "Lenovo ThinkPad USB Keyboard with TrackPoint" +config HID_LENOVO + tristate "Lenovo / Thinkpad devices" depends on HID select NEW_LEDS select LEDS_CLASS ---help--- - Support for the Lenovo ThinkPad USB Keyboard with TrackPoint. + Support for Lenovo devices that are not fully compliant with HID standard. - Say Y here if you have a Lenovo ThinkPad USB Keyboard with TrackPoint - and would like to use device-specific features like changing the - sensitivity of the trackpoint, using the microphone mute button or - controlling the mute and microphone mute LEDs. + Say Y if you want support for the non-compliant features of the Lenovo + Thinkpad standalone keyboards, e.g: + - ThinkPad USB Keyboard with TrackPoint (supports extra LEDs and trackpoint + configuration) config HID_LOGITECH tristate "Logitech devices" if EXPERT diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index a6fa6ba..5e96be3 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -59,7 +59,7 @@ obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o obj-$(CONFIG_HID_KEYTOUCH) += hid-keytouch.o obj-$(CONFIG_HID_KYE) += hid-kye.o obj-$(CONFIG_HID_LCPOWER) += hid-lcpower.o -obj-$(CONFIG_HID_LENOVO_TPKBD) += hid-lenovo-tpkbd.o +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_MAGICMOUSE) += hid-magicmouse.o diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 8ed66fd..55841bd 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1796,7 +1796,7 @@ static const struct hid_device_id hid_have_special_driver[] = { { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X) }, { HID_USB_DEVICE(USB_VENDOR_ID_LABTEC, USB_DEVICE_ID_LABTEC_WIRELESS_KEYBOARD) }, { HID_USB_DEVICE(USB_VENDOR_ID_LCPOWER, USB_DEVICE_ID_LCPOWER_LC1000 ) }, -#if IS_ENABLED(CONFIG_HID_LENOVO_TPKBD) +#if IS_ENABLED(CONFIG_HID_LENOVO) { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) }, #endif { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) }, diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo.c similarity index 64% rename from drivers/hid/hid-lenovo-tpkbd.c rename to drivers/hid/hid-lenovo.c index 2d25b6c..0320b96 100644 --- a/drivers/hid/hid-lenovo-tpkbd.c +++ b/drivers/hid/hid-lenovo.c @@ -20,8 +20,7 @@ #include "hid-ids.h" -/* This is only used for the trackpoint part of the driver, hence _tp */ -struct tpkbd_data_pointer { +struct lenovo_drvdata_tpkbd { int led_state; struct led_classdev led_mute; struct led_classdev led_micmute; @@ -35,7 +34,7 @@ struct tpkbd_data_pointer { #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c)) -static int tpkbd_input_mapping(struct hid_device *hdev, +static int lenovo_input_mapping_tpkbd(struct hid_device *hdev, struct hid_input *hi, struct hid_field *field, struct hid_usage *usage, unsigned long **bit, int *max) { @@ -50,10 +49,10 @@ static int tpkbd_input_mapping(struct hid_device *hdev, #undef map_key_clear -static int tpkbd_features_set(struct hid_device *hdev) +static int lenovo_features_set_tpkbd(struct hid_device *hdev) { struct hid_report *report; - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[4]; @@ -69,23 +68,23 @@ static int tpkbd_features_set(struct hid_device *hdev) return 0; } -static ssize_t pointer_press_to_select_show(struct device *dev, +static ssize_t attr_press_to_select_show_tpkbd(struct device *dev, struct device_attribute *attr, char *buf) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->press_to_select); } -static ssize_t pointer_press_to_select_store(struct device *dev, +static ssize_t attr_press_to_select_store_tpkbd(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); int value; if (kstrtoint(buf, 10, &value)) @@ -94,28 +93,28 @@ static ssize_t pointer_press_to_select_store(struct device *dev, return -EINVAL; data_pointer->press_to_select = value; - tpkbd_features_set(hdev); + lenovo_features_set_tpkbd(hdev); return count; } -static ssize_t pointer_dragging_show(struct device *dev, +static ssize_t attr_dragging_show_tpkbd(struct device *dev, struct device_attribute *attr, char *buf) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->dragging); } -static ssize_t pointer_dragging_store(struct device *dev, +static ssize_t attr_dragging_store_tpkbd(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); int value; if (kstrtoint(buf, 10, &value)) @@ -124,28 +123,28 @@ static ssize_t pointer_dragging_store(struct device *dev, return -EINVAL; data_pointer->dragging = value; - tpkbd_features_set(hdev); + lenovo_features_set_tpkbd(hdev); return count; } -static ssize_t pointer_release_to_select_show(struct device *dev, +static ssize_t attr_release_to_select_show_tpkbd(struct device *dev, struct device_attribute *attr, char *buf) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->release_to_select); } -static ssize_t pointer_release_to_select_store(struct device *dev, +static ssize_t attr_release_to_select_store_tpkbd(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); int value; if (kstrtoint(buf, 10, &value)) @@ -154,28 +153,28 @@ static ssize_t pointer_release_to_select_store(struct device *dev, return -EINVAL; data_pointer->release_to_select = value; - tpkbd_features_set(hdev); + lenovo_features_set_tpkbd(hdev); return count; } -static ssize_t pointer_select_right_show(struct device *dev, +static ssize_t attr_select_right_show_tpkbd(struct device *dev, struct device_attribute *attr, char *buf) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->select_right); } -static ssize_t pointer_select_right_store(struct device *dev, +static ssize_t attr_select_right_store_tpkbd(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); int value; if (kstrtoint(buf, 10, &value)) @@ -184,119 +183,119 @@ static ssize_t pointer_select_right_store(struct device *dev, return -EINVAL; data_pointer->select_right = value; - tpkbd_features_set(hdev); + lenovo_features_set_tpkbd(hdev); return count; } -static ssize_t pointer_sensitivity_show(struct device *dev, +static ssize_t attr_sensitivity_show_tpkbd(struct device *dev, struct device_attribute *attr, char *buf) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->sensitivity); } -static ssize_t pointer_sensitivity_store(struct device *dev, +static ssize_t attr_sensitivity_store_tpkbd(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); int value; if (kstrtoint(buf, 10, &value) || value < 1 || value > 255) return -EINVAL; data_pointer->sensitivity = value; - tpkbd_features_set(hdev); + lenovo_features_set_tpkbd(hdev); return count; } -static ssize_t pointer_press_speed_show(struct device *dev, +static ssize_t attr_press_speed_show_tpkbd(struct device *dev, struct device_attribute *attr, char *buf) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->press_speed); } -static ssize_t pointer_press_speed_store(struct device *dev, +static ssize_t attr_press_speed_store_tpkbd(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); int value; if (kstrtoint(buf, 10, &value) || value < 1 || value > 255) return -EINVAL; data_pointer->press_speed = value; - tpkbd_features_set(hdev); + lenovo_features_set_tpkbd(hdev); return count; } -static struct device_attribute dev_attr_pointer_press_to_select = +static struct device_attribute dev_attr_press_to_select_tpkbd = __ATTR(press_to_select, S_IWUSR | S_IRUGO, - pointer_press_to_select_show, - pointer_press_to_select_store); + attr_press_to_select_show_tpkbd, + attr_press_to_select_store_tpkbd); -static struct device_attribute dev_attr_pointer_dragging = +static struct device_attribute dev_attr_dragging_tpkbd = __ATTR(dragging, S_IWUSR | S_IRUGO, - pointer_dragging_show, - pointer_dragging_store); + attr_dragging_show_tpkbd, + attr_dragging_store_tpkbd); -static struct device_attribute dev_attr_pointer_release_to_select = +static struct device_attribute dev_attr_release_to_select_tpkbd = __ATTR(release_to_select, S_IWUSR | S_IRUGO, - pointer_release_to_select_show, - pointer_release_to_select_store); + attr_release_to_select_show_tpkbd, + attr_release_to_select_store_tpkbd); -static struct device_attribute dev_attr_pointer_select_right = +static struct device_attribute dev_attr_select_right_tpkbd = __ATTR(select_right, S_IWUSR | S_IRUGO, - pointer_select_right_show, - pointer_select_right_store); + attr_select_right_show_tpkbd, + attr_select_right_store_tpkbd); -static struct device_attribute dev_attr_pointer_sensitivity = +static struct device_attribute dev_attr_sensitivity_tpkbd = __ATTR(sensitivity, S_IWUSR | S_IRUGO, - pointer_sensitivity_show, - pointer_sensitivity_store); + attr_sensitivity_show_tpkbd, + attr_sensitivity_store_tpkbd); -static struct device_attribute dev_attr_pointer_press_speed = +static struct device_attribute dev_attr_press_speed_tpkbd = __ATTR(press_speed, S_IWUSR | S_IRUGO, - pointer_press_speed_show, - pointer_press_speed_store); - -static struct attribute *tpkbd_attributes_pointer[] = { - &dev_attr_pointer_press_to_select.attr, - &dev_attr_pointer_dragging.attr, - &dev_attr_pointer_release_to_select.attr, - &dev_attr_pointer_select_right.attr, - &dev_attr_pointer_sensitivity.attr, - &dev_attr_pointer_press_speed.attr, + attr_press_speed_show_tpkbd, + attr_press_speed_store_tpkbd); + +static struct attribute *lenovo_attributes_tpkbd[] = { + &dev_attr_press_to_select_tpkbd.attr, + &dev_attr_dragging_tpkbd.attr, + &dev_attr_release_to_select_tpkbd.attr, + &dev_attr_select_right_tpkbd.attr, + &dev_attr_sensitivity_tpkbd.attr, + &dev_attr_press_speed_tpkbd.attr, NULL }; -static const struct attribute_group tpkbd_attr_group_pointer = { - .attrs = tpkbd_attributes_pointer, +static const struct attribute_group lenovo_attr_group_tpkbd = { + .attrs = lenovo_attributes_tpkbd, }; -static enum led_brightness tpkbd_led_brightness_get( +static enum led_brightness lenovo_led_brightness_get_tpkbd( struct led_classdev *led_cdev) { struct device *dev = led_cdev->dev->parent; struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); int led_nr = 0; if (led_cdev == &data_pointer->led_micmute) @@ -307,12 +306,12 @@ static enum led_brightness tpkbd_led_brightness_get( : LED_OFF; } -static void tpkbd_led_brightness_set(struct led_classdev *led_cdev, +static void lenovo_led_brightness_set_tpkbd(struct led_classdev *led_cdev, enum led_brightness value) { struct device *dev = led_cdev->dev->parent; struct hid_device *hdev = container_of(dev, struct hid_device, dev); - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); struct hid_report *report; int led_nr = 0; @@ -330,10 +329,10 @@ static void tpkbd_led_brightness_set(struct led_classdev *led_cdev, hid_hw_request(hdev, report, HID_REQ_SET_REPORT); } -static int tpkbd_probe_tp(struct hid_device *hdev) +static int lenovo_probe_tpkbd(struct hid_device *hdev) { struct device *dev = &hdev->dev; - struct tpkbd_data_pointer *data_pointer; + struct lenovo_drvdata_tpkbd *data_pointer; size_t name_sz = strlen(dev_name(dev)) + 16; char *name_mute, *name_micmute; int i; @@ -347,12 +346,12 @@ static int tpkbd_probe_tp(struct hid_device *hdev) return -ENODEV; if (sysfs_create_group(&hdev->dev.kobj, - &tpkbd_attr_group_pointer)) { + &lenovo_attr_group_tpkbd)) { hid_warn(hdev, "Could not create sysfs group\n"); } data_pointer = devm_kzalloc(&hdev->dev, - sizeof(struct tpkbd_data_pointer), + sizeof(struct lenovo_drvdata_tpkbd), GFP_KERNEL); if (data_pointer == NULL) { hid_err(hdev, "Could not allocate memory for driver data\n"); @@ -375,23 +374,25 @@ static int tpkbd_probe_tp(struct hid_device *hdev) hid_set_drvdata(hdev, data_pointer); data_pointer->led_mute.name = name_mute; - data_pointer->led_mute.brightness_get = tpkbd_led_brightness_get; - data_pointer->led_mute.brightness_set = tpkbd_led_brightness_set; + data_pointer->led_mute.brightness_get = lenovo_led_brightness_get_tpkbd; + data_pointer->led_mute.brightness_set = lenovo_led_brightness_set_tpkbd; data_pointer->led_mute.dev = dev; led_classdev_register(dev, &data_pointer->led_mute); data_pointer->led_micmute.name = name_micmute; - data_pointer->led_micmute.brightness_get = tpkbd_led_brightness_get; - data_pointer->led_micmute.brightness_set = tpkbd_led_brightness_set; + data_pointer->led_micmute.brightness_get = + lenovo_led_brightness_get_tpkbd; + data_pointer->led_micmute.brightness_set = + lenovo_led_brightness_set_tpkbd; data_pointer->led_micmute.dev = dev; led_classdev_register(dev, &data_pointer->led_micmute); - tpkbd_features_set(hdev); + lenovo_features_set_tpkbd(hdev); return 0; } -static int tpkbd_probe(struct hid_device *hdev, +static int lenovo_probe(struct hid_device *hdev, const struct hid_device_id *id) { int ret; @@ -410,7 +411,7 @@ static int tpkbd_probe(struct hid_device *hdev, if (hid_get_drvdata(hdev)) { hid_set_drvdata(hdev, NULL); - ret = tpkbd_probe_tp(hdev); + ret = lenovo_probe_tpkbd(hdev); if (ret) goto err_hid; } @@ -422,12 +423,12 @@ err: return ret; } -static void tpkbd_remove_tp(struct hid_device *hdev) +static void lenovo_remove_tpkbd(struct hid_device *hdev) { - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev); + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev); sysfs_remove_group(&hdev->dev.kobj, - &tpkbd_attr_group_pointer); + &lenovo_attr_group_tpkbd); led_classdev_unregister(&data_pointer->led_micmute); led_classdev_unregister(&data_pointer->led_mute); @@ -435,28 +436,28 @@ static void tpkbd_remove_tp(struct hid_device *hdev) hid_set_drvdata(hdev, NULL); } -static void tpkbd_remove(struct hid_device *hdev) +static void lenovo_remove(struct hid_device *hdev) { if (hid_get_drvdata(hdev)) - tpkbd_remove_tp(hdev); + lenovo_remove_tpkbd(hdev); hid_hw_stop(hdev); } -static const struct hid_device_id tpkbd_devices[] = { +static const struct hid_device_id lenovo_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) }, { } }; -MODULE_DEVICE_TABLE(hid, tpkbd_devices); +MODULE_DEVICE_TABLE(hid, lenovo_devices); -static struct hid_driver tpkbd_driver = { - .name = "lenovo_tpkbd", - .id_table = tpkbd_devices, - .input_mapping = tpkbd_input_mapping, - .probe = tpkbd_probe, - .remove = tpkbd_remove, +static struct hid_driver lenovo_driver = { + .name = "lenovo", + .id_table = lenovo_devices, + .input_mapping = lenovo_input_mapping_tpkbd, + .probe = lenovo_probe, + .remove = lenovo_remove, }; -module_hid_driver(tpkbd_driver); +module_hid_driver(lenovo_driver); MODULE_LICENSE("GPL");
Rename module and all functions within so we can add support for other keyboards in the same file. Rename the _tp postfix to _tpkbd, to signify functions relevant to the TP USB keyboard. Signed-off-by: Jamie Lentin <jm@lentin.co.uk> --- ...er-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} | 0 drivers/hid/Kconfig | 14 +- drivers/hid/Makefile | 2 +- drivers/hid/hid-core.c | 2 +- drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} | 185 +++++++++++---------- 5 files changed, 102 insertions(+), 101 deletions(-) rename Documentation/ABI/testing/{sysfs-driver-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} (100%) rename drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} (64%)