Hello,
this is sort of a response to Simon Wood's recent G27 LED's patch but since the patch touches
a completely different part of the Logitech driver I submitted it separately.
The main goal of this patch is to allow for some custom device specific properties like the rotation
range to be stored as private driver data. Up until now the hid-lg treated the pointer to driver_data
as unsigned long as stored some device quirks there. Because of the scope of the changes, Simon's
implementation of the G27 LEDs code is unfortunately incompatible with this patch
because it uses the linked list I'm trying to get rid of.
The patch also introduces some spinlocking to hid-lg and hid-lg4ff to (hopefully) prevent any nasty
race conditions you were concerned about.
Signed-off-by: Michal Malý <madcatxster@xxxxxxxxx>
---
diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index e7a7bd1..69a5ed0 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -109,23 +109,25 @@ static __u8 dfp_rdesc_fixed[] = {
static __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
{
- unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
+ unsigned long flags;
+ struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
- if ((quirks & LG_RDESC) && *rsize >= 90 && rdesc[83] == 0x26 &&
+ spin_lock_irqsave(&drv_data->lock, flags);
+ if ((drv_data->quirks & LG_RDESC) && *rsize >= 90 && rdesc[83] == 0x26 &&
rdesc[84] == 0x8c && rdesc[85] == 0x02) {
hid_info(hdev,
"fixing up Logitech keyboard report descriptor\n");
rdesc[84] = rdesc[89] = 0x4d;
rdesc[85] = rdesc[90] = 0x10;
}
- if ((quirks & LG_RDESC_REL_ABS) && *rsize >= 50 &&
+ if ((drv_data->quirks & LG_RDESC_REL_ABS) && *rsize >= 50 &&
rdesc[32] == 0x81 && rdesc[33] == 0x06 &&
rdesc[49] == 0x81 && rdesc[50] == 0x06) {
hid_info(hdev,
"fixing up rel/abs in Logitech report descriptor\n");
rdesc[33] = rdesc[50] = 0x02;
}
- if ((quirks & LG_FF4) && *rsize >= 101 &&
+ if ((drv_data->quirks & LG_FF4) && *rsize >= 101 &&
rdesc[41] == 0x95 && rdesc[42] == 0x0B &&
rdesc[47] == 0x05 && rdesc[48] == 0x09) {
hid_info(hdev, "fixing up Logitech Speed Force Wireless button descriptor\n");
@@ -134,6 +136,7 @@ static __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
rdesc[47] = 0x95;
rdesc[48] = 0x0B;
}
+ spin_unlock_irqrestore(&drv_data->lock, flags);
switch (hdev->product) {
case USB_DEVICE_ID_LOGITECH_DFP_WHEEL:
@@ -278,7 +281,8 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
0, 0, 0, 0, 0,183,184,185,186,187,
188,189,190,191,192,193,194, 0, 0, 0
};
- unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
+ unsigned long flags;
+ struct lg_drv_data* drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
unsigned int hid = usage->hid;
if (hdev->product == USB_DEVICE_ID_LOGITECH_RECEIVER &&
@@ -289,28 +293,37 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
lg_dinovo_mapping(hi, usage, bit, max))
return 1;
- if ((quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
+ spin_lock_irqsave(&drv_data->lock, flags);
+ if ((drv_data->quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max)) {
+ spin_unlock_irqrestore(&drv_data->lock, flags);
return 1;
+ }
- if ((hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
+ if ((hid & HID_USAGE_PAGE) != HID_UP_BUTTON) {
+ spin_unlock_irqrestore(&drv_data->lock, flags);
return 0;
+ }
hid &= HID_USAGE;
/* Special handling for Logitech Cordless Desktop */
if (field->application == HID_GD_MOUSE) {
- if ((quirks & LG_IGNORE_DOUBLED_WHEEL) &&
- (hid == 7 || hid == 8))
+ if ((drv_data->quirks & LG_IGNORE_DOUBLED_WHEEL) &&
+ (hid == 7 || hid == 8)) {
+ spin_unlock_irqrestore(&drv_data->lock, flags);
return -1;
+ }
} else {
- if ((quirks & LG_EXPANDED_KEYMAP) &&
+ if ((drv_data->quirks & LG_EXPANDED_KEYMAP) &&
hid < ARRAY_SIZE(e_keymap) &&
e_keymap[hid] != 0) {
hid_map_usage(hi, usage, bit, max, EV_KEY,
e_keymap[hid]);
+ spin_unlock_irqrestore(&drv_data->lock, flags);
return 1;
}
}
+ spin_unlock_irqrestore(&drv_data->lock, flags);
return 0;
}
@@ -319,15 +332,22 @@ static int lg_input_mapped(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
unsigned long **bit, int *max)
{
- unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
+ unsigned long flags;
+ struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
- if ((quirks & LG_BAD_RELATIVE_KEYS) && usage->type == EV_KEY &&
- (field->flags & HID_MAIN_ITEM_RELATIVE))
+ spin_lock_irqsave(&drv_data->lock, flags);
+ if ((drv_data->quirks & LG_BAD_RELATIVE_KEYS) && usage->type == EV_KEY &&
+ (field->flags & HID_MAIN_ITEM_RELATIVE)) {
+ spin_unlock_irqrestore(&drv_data->lock, flags);
field->flags &= ~HID_MAIN_ITEM_RELATIVE;
+ }
- if ((quirks & LG_DUPLICATE_USAGES) && (usage->type == EV_KEY ||
- usage->type == EV_REL || usage->type == EV_ABS))
+ if ((drv_data->quirks & LG_DUPLICATE_USAGES) && (usage->type == EV_KEY ||
+ usage->type == EV_REL || usage->type == EV_ABS)) {
+ spin_unlock_irqrestore(&drv_data->lock, flags);
clear_bit(usage->code, *bit);
+ }
+ spin_unlock_irqrestore(&drv_data->lock, flags);
return 0;
}
@@ -335,26 +355,39 @@ static int lg_input_mapped(struct hid_device *hdev, struct hid_input *hi,
static int lg_event(struct hid_device *hdev, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{
- unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
+ unsigned long flags;
+ struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
- if ((quirks & LG_INVERT_HWHEEL) && usage->code == REL_HWHEEL) {
+ spin_lock_irqsave(&drv_data->lock, flags);
+ if ((drv_data->quirks & LG_INVERT_HWHEEL) && usage->code == REL_HWHEEL) {
input_event(field->hidinput->input, usage->type, usage->code,
-value);
+ spin_unlock_irqrestore(&drv_data->lock, flags);
return 1;
}
+ spin_unlock_irqrestore(&drv_data->lock, flags);
return 0;
}
static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
- unsigned long quirks = id->driver_data;
unsigned int connect_mask = HID_CONNECT_DEFAULT;
int ret;
+
+ /* Allocate memory for driver data */
+ struct lg_drv_data *drv_data = kzalloc(sizeof(struct lg_drv_data), GFP_KERNEL);
+ if (!drv_data) {
+ hid_err(hdev, "Cannot initialize device, insufficient memory.");
+ ret = -ENOMEM;
+ goto err_free;
+ }
+ spin_lock_init(&drv_data->lock);
+ drv_data->quirks = id->driver_data;
- hid_set_drvdata(hdev, (void *)quirks);
+ hid_set_drvdata(hdev, (void *)drv_data);
- if (quirks & LG_NOGET)
+ if (drv_data->quirks & LG_NOGET)
hdev->quirks |= HID_QUIRK_NOGET;
ret = hid_parse(hdev);
@@ -363,7 +396,7 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto err_free;
}
- if (quirks & (LG_FF | LG_FF2 | LG_FF3 | LG_FF4))
+ if (drv_data->quirks & (LG_FF | LG_FF2 | LG_FF3 | LG_FF4))
connect_mask &= ~HID_CONNECT_FF;
ret = hid_hw_start(hdev, connect_mask);
@@ -392,14 +425,14 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
}
- if (quirks & LG_FF)
+ if (drv_data->quirks & LG_FF)
lgff_init(hdev);
- if (quirks & LG_FF2)
+ if (drv_data->quirks & LG_FF2)
lg2ff_init(hdev);
- if (quirks & LG_FF3)
+ if (drv_data->quirks & LG_FF3)
lg3ff_init(hdev);
- if (quirks & LG_FF4)
- lg4ff_init(hdev);
+ if (drv_data->quirks & LG_FF4)
+ return lg4ff_init(hdev);
return 0;
err_free:
@@ -408,10 +441,17 @@ err_free:
static void lg_remove(struct hid_device *hdev)
{
- unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
- if(quirks & LG_FF4)
+ unsigned long flags;
+ struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
+
+ spin_lock_irqsave(&drv_data->lock, flags);
+ if (drv_data->quirks & LG_FF4)
lg4ff_deinit(hdev);
+ spin_unlock_irqrestore(&drv_data->lock, flags);
+ kfree(drv_data);
+
hid_hw_stop(hdev);
}
diff --git a/drivers/hid/hid-lg.h b/drivers/hid/hid-lg.h
index 4b09728..7b10cdc 100644
--- a/drivers/hid/hid-lg.h
+++ b/drivers/hid/hid-lg.h
@@ -1,6 +1,15 @@
#ifndef __HID_LG_H
#define __HID_LG_H
+#include <linux/spinlock.h>
+
+/* Driver data struct */
+struct lg_drv_data {
+ spinlock_t lock;
+ unsigned long quirks;
+ void *dev_props;
+};
+
#ifdef CONFIG_LOGITECH_FF
int lgff_init(struct hid_device *hdev);
#else
diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 6ecc9e2..98d83fe 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -51,20 +51,14 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
static DEVICE_ATTR(range, S_IRWXU | S_IRWXG | S_IRWXO, lg4ff_range_show, lg4ff_range_store);
-static bool list_inited;
-
struct lg4ff_device_entry {
- char *device_id; /* Use name in respective kobject structure's address as the ID */
__u16 range;
__u16 min_range;
__u16 max_range;
__u8 leds;
- struct list_head list;
void (*set_range)(struct hid_device *hid, u16 range);
};
-static struct lg4ff_device_entry device_list;
-
static const signed short lg4ff_wheel_effects[] = {
FF_CONSTANT,
FF_AUTOCENTER,
@@ -91,13 +85,13 @@ static const struct lg4ff_wheel lg4ff_devices[] = {
};
struct lg4ff_native_cmd {
- const __u8 cmd_num; /* Number of commands to send */
- const __u8 cmd[];
+ const u8 cmd_num; /* Number of commands to send */
+ const u8 cmd[];
};
struct lg4ff_usb_revision {
- const __u16 rev_maj;
- const __u16 rev_min;
+ const u16 rev_maj;
+ const u16 rev_min;
const struct lg4ff_native_cmd *command;
};
@@ -285,22 +279,30 @@ static void hid_lg4ff_switch_native(struct hid_device *hid, const struct lg4ff_n
/* Read current range and display it in terminal */
static ssize_t lg4ff_range_show(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct lg4ff_device_entry *uninitialized_var(entry);
- struct list_head *h;
- struct hid_device *hid = to_hid_device(dev);
size_t count;
+ struct hid_device *hid = to_hid_device(dev);
+ struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+ struct lg4ff_device_entry *uninitialized_var(entry);
+ __u16 out_rng;
- list_for_each(h, &device_list.list) {
- entry = list_entry(h, struct lg4ff_device_entry, list);
- if (strcmp(entry->device_id, (&hid->dev)->kobj.name) == 0)
- break;
+
+ if (!drv_data) {
+ hid_err(hid, "Device data not found.");
+ return 0;
}
- if (h == &device_list.list) {
- dbg_hid("Device not found!");
+
+ spin_lock(&drv_data->lock);
+ entry = (struct lg4ff_device_entry *)drv_data->dev_props;
+ if (!entry) {
+ hid_err(hid, "Device properties not found.");
+ spin_unlock(&drv_data->lock);
return 0;
}
- count = scnprintf(buf, PAGE_SIZE, "%u\n", entry->range);
+ out_rng = entry->range;
+ spin_unlock(&drv_data->lock);
+ count = scnprintf(buf, PAGE_SIZE, "%u\n", out_rng);
+
return count;
}
@@ -308,18 +310,21 @@ static ssize_t lg4ff_range_show(struct device *dev, struct device_attribute *att
* according to the type of the wheel */
static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
{
- struct lg4ff_device_entry *uninitialized_var(entry);
- struct list_head *h;
struct hid_device *hid = to_hid_device(dev);
+ struct lg_drv_data* drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+ struct lg4ff_device_entry *uninitialized_var(entry);
__u16 range = simple_strtoul(buf, NULL, 10);
- list_for_each(h, &device_list.list) {
- entry = list_entry(h, struct lg4ff_device_entry, list);
- if (strcmp(entry->device_id, (&hid->dev)->kobj.name) == 0)
- break;
+ if (!drv_data) {
+ hid_err(hid, "Device data not found.");
+ return count;
}
- if (h == &device_list.list) {
- dbg_hid("Device not found!");
+
+ spin_lock(&drv_data->lock);
+ entry = (struct lg4ff_device_entry *)drv_data->dev_props;
+ if (!entry) {
+ hid_err(hid, "Device properties not found.");
+ spin_unlock(&drv_data->lock);
return count;
}
@@ -332,6 +337,7 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
entry->set_range(hid, range);
entry->range = range;
}
+ spin_unlock(&drv_data->lock);
return count;
}
@@ -339,10 +345,12 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
int lg4ff_init(struct hid_device *hid)
{
struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
+ unsigned long flags;
struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
struct input_dev *dev = hidinput->input;
struct hid_report *report;
struct hid_field *field;
+ struct lg_drv_data *drv_data;
struct lg4ff_device_entry *entry;
struct usb_device_descriptor *udesc;
int error, i, j;
@@ -423,28 +431,26 @@ int lg4ff_init(struct hid_device *hid)
dev->ff->set_autocenter(dev, 0);
}
- /* Initialize device_list if this is the first device to handle by lg4ff */
- if (!list_inited) {
- INIT_LIST_HEAD(&device_list.list);
- list_inited = 1;
- }
-
/* Add the device to device_list */
entry = kzalloc(sizeof(struct lg4ff_device_entry), GFP_KERNEL);
if (!entry) {
hid_err(hid, "Cannot add device, insufficient memory.\n");
return -ENOMEM;
}
- entry->device_id = kstrdup((&hid->dev)->kobj.name, GFP_KERNEL);
- if (!entry->device_id) {
- hid_err(hid, "Cannot set device_id, insufficient memory.\n");
- kfree(entry);
- return -ENOMEM;
- }
entry->min_range = lg4ff_devices[i].min_range;
entry->max_range = lg4ff_devices[i].max_range;
entry->set_range = lg4ff_devices[i].set_range;
- list_add(&entry->list, &device_list.list);
+
+ drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+ spin_lock_irqsave(&drv_data->lock, flags);
+ drv_data->dev_props = entry;
+
+ /* Set the maximum range to start with */
+ entry->range = entry->max_range;
+ if (entry->set_range != NULL)
+ entry->set_range(hid, entry->range);
+
+ spin_unlock_irqrestore(&drv_data->lock, flags);
/* Create sysfs interface */
error = device_create_file(&hid->dev, &dev_attr_range);
@@ -452,34 +458,17 @@ int lg4ff_init(struct hid_device *hid)
return error;
dbg_hid("sysfs interface created\n");
- /* Set the maximum range to start with */
- entry->range = entry->max_range;
- if (entry->set_range != NULL)
- entry->set_range(hid, entry->range);
-
hid_info(hid, "Force feedback for Logitech Speed Force Wireless by Simon Wood <simon@xxxxxxxxxxxxx>\n");
return 0;
}
int lg4ff_deinit(struct hid_device *hid)
{
- bool found = 0;
- struct lg4ff_device_entry *entry;
- struct list_head *h, *g;
- list_for_each_safe(h, g, &device_list.list) {
- entry = list_entry(h, struct lg4ff_device_entry, list);
- if (strcmp(entry->device_id, (&hid->dev)->kobj.name) == 0) {
- list_del(h);
- kfree(entry->device_id);
- kfree(entry);
- found = 1;
- break;
- }
- }
-
- if (!found) {
- dbg_hid("Device entry not found!\n");
- return -1;
+ /* This is called from lg_remove with a spinlock held */
+ struct lg_drv_data *drv_data = (struct lg_drv_data *)drv_data;
+ if (drv_data) {
+ if (drv_data->dev_props)
+ kfree(drv_data->dev_props);
}
device_remove_file(&hid->dev, &dev_attr_range);
Attachment:
signature.asc
Description: This is a digitally signed message part.
