Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755877AbaFPUkc (ORCPT ); Mon, 16 Jun 2014 16:40:32 -0400 Received: from smtp206.alice.it ([82.57.200.102]:2281 "EHLO smtp206.alice.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752640AbaFPUkb (ORCPT ); Mon, 16 Jun 2014 16:40:31 -0400 Date: Mon, 16 Jun 2014 22:40:07 +0200 From: Antonio Ospite To: Jamie Lentin Cc: Jiri Kosina , Alexander Clouter , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 1/2] Rename hid-lenovo-tpkbd to hid-lenovo, so we can add other keyboards Message-Id: <20140616224007.cef4138915c567aa6500e477@ao2.it> In-Reply-To: <1402864790-7478-2-git-send-email-jm@lentin.co.uk> References: <1402864790-7478-1-git-send-email-jm@lentin.co.uk> <1402864790-7478-2-git-send-email-jm@lentin.co.uk> X-Mailer: Sylpheed 3.5.0beta1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) X-Face: z*RaLf`X<@C75u6Ig9}{oW$H;1_\2t5)({*|jhM/Vb;]yA5\I~93>J<_`<4)A{':UrE Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 15 Jun 2014 21:39:49 +0100 Jamie Lentin wrote: Almost there :) > Signed-off-by: Jamie Lentin > --- > drivers/hid/Kconfig | 14 +- > drivers/hid/Makefile | 2 +- > drivers/hid/hid-core.c | 2 +- > drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} | 233 +++++++++++++---------- > 4 files changed, 142 insertions(+), 109 deletions(-) > rename drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} (59%) > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index f722001..dd07d59 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -322,18 +322,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. > Try to wrap text in Kconfig at 80 chars. > - 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: Maybe don't mention keyboards just yet on the line above since the driver is now supposed to support other devices too. > + - ThinkPad USB Keyboard with TrackPoint (supports extra LEDs and trackpoint > + configuration) > Wrap at 80 chars here too. > config HID_LOGITECH > tristate "Logitech devices" if EXPERT > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index 30e4431..384f981 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -58,7 +58,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 8a5384c..e8ce932 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1736,7 +1736,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 59% > rename from drivers/hid/hid-lenovo-tpkbd.c > rename to drivers/hid/hid-lenovo.c > index 2d25b6c..aabf084 100644 > --- a/drivers/hid/hid-lenovo-tpkbd.c > +++ b/drivers/hid/hid-lenovo.c > @@ -1,5 +1,6 @@ > /* > - * HID driver for Lenovo ThinkPad USB Keyboard with TrackPoint > + * HID driver for Lenovo:- The dash before the colon is not needed (really not a big deal I mentioned that again just because there are other fixes too). > + * - ThinkPad USB Keyboard with TrackPoint (tpkbd) > * This hunk could go into a preparatory patch separated from the rename one, see below. > * Copyright (c) 2012 Bernhard Seibold > */ > @@ -20,8 +21,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 +35,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, I'd name this just lenovo_input_mapping() for now. > struct hid_input *hi, struct hid_field *field, > struct hid_usage *usage, unsigned long **bit, int *max) > { > @@ -48,12 +48,26 @@ static int tpkbd_input_mapping(struct hid_device *hdev, > return 0; > } > > +static int lenovo_input_mapping(struct hid_device *hdev, And add this separation in a preparatory patch, more or less equal to your previous patch 1, which would become a following patch on top of the rename one. > + struct hid_input *hi, struct hid_field *field, > + struct hid_usage *usage, unsigned long **bit, int *max) > +{ > + switch (hdev->product) { > + case USB_DEVICE_ID_LENOVO_TPKBD: > + return lenovo_input_mapping_tpkbd(hdev, hi, field, > + usage, bit, max); > + break; break after return is not needed. I also like explicit default cases, but I guess it's just personal preference. > + } > + > + return 0; > +} > + > #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 +83,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, I have no definitive opinion on these renames, I guess they are OK; maybe having these functions end in _store and _show is slightly more readable, but this is not a big deal. > 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 +108,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 +138,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 +168,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 +198,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 +321,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,13 +344,24 @@ 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, > + const struct hid_device_id *id) Just curios, why did the function change the signature too here? The "id" argument is not used in either of the "internal" probe routines. > { > 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; > + int i, ret; > + > + /* > + * If this is the pointer half of the keyboard, input_mapping should > + * have set drvdata to 1. Otherwise, it's the keyboard which needs > + * nothing special doing to it. > + */ > + if (!hid_get_drvdata(hdev)) > + return 0; > + > + hid_set_drvdata(hdev, NULL); > This last part would also go in the preparatory patch. > /* Validate required reports. */ > for (i = 0; i < 4; i++) { > @@ -346,13 +371,12 @@ static int tpkbd_probe_tp(struct hid_device *hdev) > if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 3, 0, 2)) > return -ENODEV; > > - if (sysfs_create_group(&hdev->dev.kobj, > - &tpkbd_attr_group_pointer)) { > + ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_tpkbd); > + if (ret) > hid_warn(hdev, "Could not create sysfs group\n"); > - } This is a cleanup of existing code, this ideally would go alone in a dedicated "cleanup" patch. > > 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 +399,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; > @@ -408,12 +434,13 @@ static int tpkbd_probe(struct hid_device *hdev, > goto err; > } > > - if (hid_get_drvdata(hdev)) { > - hid_set_drvdata(hdev, NULL); > - ret = tpkbd_probe_tp(hdev); > - if (ret) > - goto err_hid; > + switch (hdev->product) { > + case USB_DEVICE_ID_LENOVO_TPKBD: > + ret = lenovo_probe_tpkbd(hdev, id); > + break; > } > + if (ret) > + goto err_hid; > This hunk would also go in the preparatory patch. > return 0; > err_hid: > @@ -422,12 +449,15 @@ 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); > + > + if (data_pointer == NULL) > + return; > And this too. > led_classdev_unregister(&data_pointer->led_micmute); > led_classdev_unregister(&data_pointer->led_mute); > @@ -435,28 +465,31 @@ 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); > + switch (hdev->product) { > + case USB_DEVICE_ID_LENOVO_TPKBD: > + lenovo_remove_tpkbd(hdev); > + break; > + } > And this. > 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 = { > +static struct hid_driver lenovo_driver = { > .name = "lenovo_tpkbd", The name could now become just "lenovo". > - .id_table = tpkbd_devices, > - .input_mapping = tpkbd_input_mapping, > - .probe = tpkbd_probe, > - .remove = tpkbd_remove, > + .id_table = lenovo_devices, > + .input_mapping = lenovo_input_mapping, > + .probe = lenovo_probe, > + .remove = lenovo_remove, > }; > -module_hid_driver(tpkbd_driver); > +module_hid_driver(lenovo_driver); > > MODULE_LICENSE("GPL"); > -- > 2.0.0.rc2 > > -- Antonio Ospite http://ao2.it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/