* [PATCH 1/3] HID: lenovo-tpkbd: devm conversion
2013-09-11 20:12 [PATCH 0/3] Few cleanups Benjamin Tissoires
@ 2013-09-11 20:12 ` Benjamin Tissoires
2013-09-11 20:12 ` [PATCH 2/3] HID: lenovo-tpkbd: remove usb dependency Benjamin Tissoires
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Benjamin Tissoires @ 2013-09-11 20:12 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
We can use the devres API in hid modules, so use it to avoid some kfree
and potential leaks.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-lenovo-tpkbd.c | 30 ++++++++----------------------
1 file changed, 8 insertions(+), 22 deletions(-)
diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c
index 31cf29a..0d9a276 100644
--- a/drivers/hid/hid-lenovo-tpkbd.c
+++ b/drivers/hid/hid-lenovo-tpkbd.c
@@ -339,7 +339,7 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
struct tpkbd_data_pointer *data_pointer;
size_t name_sz = strlen(dev_name(dev)) + 16;
char *name_mute, *name_micmute;
- int i, ret;
+ int i;
/* Validate required reports. */
for (i = 0; i < 4; i++) {
@@ -354,7 +354,9 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
hid_warn(hdev, "Could not create sysfs group\n");
}
- data_pointer = kzalloc(sizeof(struct tpkbd_data_pointer), GFP_KERNEL);
+ data_pointer = devm_kzalloc(&hdev->dev,
+ sizeof(struct tpkbd_data_pointer),
+ GFP_KERNEL);
if (data_pointer == NULL) {
hid_err(hdev, "Could not allocate memory for driver data\n");
return -ENOMEM;
@@ -364,20 +366,13 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
data_pointer->sensitivity = 0xa0;
data_pointer->press_speed = 0x38;
- name_mute = kzalloc(name_sz, GFP_KERNEL);
- if (name_mute == NULL) {
+ name_mute = devm_kzalloc(&hdev->dev, name_sz, GFP_KERNEL);
+ name_micmute = devm_kzalloc(&hdev->dev, name_sz, GFP_KERNEL);
+ if (name_mute == NULL || name_micmute == NULL) {
hid_err(hdev, "Could not allocate memory for led data\n");
- ret = -ENOMEM;
- goto err;
+ return -ENOMEM;
}
snprintf(name_mute, name_sz, "%s:amber:mute", dev_name(dev));
-
- name_micmute = kzalloc(name_sz, GFP_KERNEL);
- if (name_micmute == NULL) {
- hid_err(hdev, "Could not allocate memory for led data\n");
- ret = -ENOMEM;
- goto err2;
- }
snprintf(name_micmute, name_sz, "%s:amber:micmute", dev_name(dev));
hid_set_drvdata(hdev, data_pointer);
@@ -397,12 +392,6 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
tpkbd_features_set(hdev);
return 0;
-
-err2:
- kfree(name_mute);
-err:
- kfree(data_pointer);
- return ret;
}
static int tpkbd_probe(struct hid_device *hdev,
@@ -449,9 +438,6 @@ static void tpkbd_remove_tp(struct hid_device *hdev)
led_classdev_unregister(&data_pointer->led_mute);
hid_set_drvdata(hdev, NULL);
- kfree(data_pointer->led_micmute.name);
- kfree(data_pointer->led_mute.name);
- kfree(data_pointer);
}
static void tpkbd_remove(struct hid_device *hdev)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] HID: lenovo-tpkbd: remove usb dependency
2013-09-11 20:12 [PATCH 0/3] Few cleanups Benjamin Tissoires
2013-09-11 20:12 ` [PATCH 1/3] HID: lenovo-tpkbd: devm conversion Benjamin Tissoires
@ 2013-09-11 20:12 ` Benjamin Tissoires
2013-09-11 20:12 ` [PATCH 3/3] HID: sony: use hid_get_raw_report() instead of a direct call to usb Benjamin Tissoires
2013-09-24 9:48 ` [PATCH 0/3] Few cleanups Jiri Kosina
3 siblings, 0 replies; 5+ messages in thread
From: Benjamin Tissoires @ 2013-09-11 20:12 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
lenovo tpkbd currently relies on the usb interface number to detect
if it is dealing with the touchpad interface or not.
As the report descriptors of the interface 0 does not contain the
button 3, we can use this to remove the need to check for usb.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
I am not 100% happy with the trick required to differenciate the 2 interfaces.
It is working, but I don't really like setting "hid_set_drvdata(hdev, (void *)1);"
to avoid a kzalloc (and some more indirections later in the led part).
Any comments welcome :)
Cheers,
Benjamin
drivers/hid/Kconfig | 2 +-
drivers/hid/hid-lenovo-tpkbd.c | 20 ++++++--------------
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 3d7c9f6..4b70800 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -322,7 +322,7 @@ config HID_LCPOWER
config HID_LENOVO_TPKBD
tristate "Lenovo ThinkPad USB Keyboard with TrackPoint"
- depends on USB_HID
+ depends on HID
select NEW_LEDS
select LEDS_CLASS
---help---
diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c
index 0d9a276..2d25b6c 100644
--- a/drivers/hid/hid-lenovo-tpkbd.c
+++ b/drivers/hid/hid-lenovo-tpkbd.c
@@ -14,11 +14,9 @@
#include <linux/module.h>
#include <linux/sysfs.h>
#include <linux/device.h>
-#include <linux/usb.h>
#include <linux/hid.h>
#include <linux/input.h>
#include <linux/leds.h>
-#include "usbhid/usbhid.h"
#include "hid-ids.h"
@@ -41,10 +39,9 @@ static int tpkbd_input_mapping(struct hid_device *hdev,
struct hid_input *hi, struct hid_field *field,
struct hid_usage *usage, unsigned long **bit, int *max)
{
- struct usbhid_device *uhdev;
-
- uhdev = (struct usbhid_device *) hdev->driver_data;
- if (uhdev->ifnum == 1 && usage->hid == (HID_UP_BUTTON | 0x0010)) {
+ if (usage->hid == (HID_UP_BUTTON | 0x0010)) {
+ /* mark the device as pointer */
+ hid_set_drvdata(hdev, (void *)1);
map_key_clear(KEY_MICMUTE);
return 1;
}
@@ -398,7 +395,6 @@ static int tpkbd_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
int ret;
- struct usbhid_device *uhdev;
ret = hid_parse(hdev);
if (ret) {
@@ -412,9 +408,8 @@ static int tpkbd_probe(struct hid_device *hdev,
goto err;
}
- uhdev = (struct usbhid_device *) hdev->driver_data;
-
- if (uhdev->ifnum == 1) {
+ if (hid_get_drvdata(hdev)) {
+ hid_set_drvdata(hdev, NULL);
ret = tpkbd_probe_tp(hdev);
if (ret)
goto err_hid;
@@ -442,10 +437,7 @@ static void tpkbd_remove_tp(struct hid_device *hdev)
static void tpkbd_remove(struct hid_device *hdev)
{
- struct usbhid_device *uhdev;
-
- uhdev = (struct usbhid_device *) hdev->driver_data;
- if (uhdev->ifnum == 1)
+ if (hid_get_drvdata(hdev))
tpkbd_remove_tp(hdev);
hid_hw_stop(hdev);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] HID: sony: use hid_get_raw_report() instead of a direct call to usb
2013-09-11 20:12 [PATCH 0/3] Few cleanups Benjamin Tissoires
2013-09-11 20:12 ` [PATCH 1/3] HID: lenovo-tpkbd: devm conversion Benjamin Tissoires
2013-09-11 20:12 ` [PATCH 2/3] HID: lenovo-tpkbd: remove usb dependency Benjamin Tissoires
@ 2013-09-11 20:12 ` Benjamin Tissoires
2013-09-24 9:48 ` [PATCH 0/3] Few cleanups Jiri Kosina
3 siblings, 0 replies; 5+ messages in thread
From: Benjamin Tissoires @ 2013-09-11 20:12 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
The usb packets are exactly the same, but it makes it a little bit more
independent of the transport layer.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-sony.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index b18320d..bc37a18 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -419,21 +419,14 @@ static int sixaxis_usb_output_raw_report(struct hid_device *hid, __u8 *buf,
*/
static int sixaxis_set_operational_usb(struct hid_device *hdev)
{
- struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
- struct usb_device *dev = interface_to_usbdev(intf);
- __u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
int ret;
char *buf = kmalloc(18, GFP_KERNEL);
if (!buf)
return -ENOMEM;
- ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
- HID_REQ_GET_REPORT,
- USB_DIR_IN | USB_TYPE_CLASS |
- USB_RECIP_INTERFACE,
- (3 << 8) | 0xf2, ifnum, buf, 17,
- USB_CTRL_GET_TIMEOUT);
+ ret = hdev->hid_get_raw_report(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT);
+
if (ret < 0)
hid_err(hdev, "can't set operational mode\n");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/3] Few cleanups
2013-09-11 20:12 [PATCH 0/3] Few cleanups Benjamin Tissoires
` (2 preceding siblings ...)
2013-09-11 20:12 ` [PATCH 3/3] HID: sony: use hid_get_raw_report() instead of a direct call to usb Benjamin Tissoires
@ 2013-09-24 9:48 ` Jiri Kosina
3 siblings, 0 replies; 5+ messages in thread
From: Jiri Kosina @ 2013-09-24 9:48 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Benjamin Tissoires, linux-input, linux-kernel
On Wed, 11 Sep 2013, Benjamin Tissoires wrote:
> Well, while debugging hid-lenovo-tpkbd, I also cleaned it up a little (so this
> goes on top of the CVE series):
> - use devres API
> - remove usb references (so that the regressions tests through uhid are working)
>
> This has also been tested by https://bugzilla.redhat.com/show_bug.cgi?id=1003998
>
> And I also had a patch for sony that I wanted to send, but I was waiting for a
> complete drop of usb, which I still did not managed to do.
It took me some time, as I wanted to think a little bit more about
cleaning up 2/3, but didn't really come up with something that'd be
substantially nicer.
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 5+ messages in thread