[PATCH 00/12] HID: hid-lg, hid-lg4ff: Mostly cleanup patches

[PATCH 00/12] HID: hid-lg, hid-lg4ff: Mostly cleanup patches

[Michal Malý]

Hello everyone,

this is a mostly boring series that deals with a few inconsistencies in the
code that have accumulated over the years. Besides that it patches up a handful
of problems such a return values not being checked etc.

The only significant change comes in patch 0007 which introduces a spinlock to
handle concurrent access to the HID report that is used by the driver to send
data to the wheel. I would appreciate some comments on this one, particularly
on the way it handles deinitialization.

Michal Malý (12):
  HID: hid-lg4ff: (Cleanup) Remove double underscore prefix from numeric
    types.
  HID: hid-lg4ff: (Cleanup) Remove "hid_" prefix from some functions
    names.
  HID: hid-lg4ff: (Cleanup) Replace DEVICE_ATTR_RW with DEVICE_ATTR to
    have all internal functions prefixed with "lg4ff_"
  HID: hid-lg4ff: (Cleanup) Remove unused variable from the
    "lg4ff_device_entry" struct.
  HID: hid-lg4ff: Update a warning message for a case where device is
    incorrectly flagged to be handled by hid-lg4ff in hid-lg.
  HID: hid-lg: Check return values from lg[N]ff_init()
  HID: hid-lg4ff: Protect concurrent access to the output HID report
    values with a spinlock.
  HID: hid-lg4ff: Store pointer to the output HID report struct in the
    device entry struct.
  HID: hid-lg4ff: Constify those members of lg4ff_device_entry struct
    whose value is not supposed to change.
  HID: hid-lg4ff: Allow the driver to continue without sysfs interface.
  HID: hid-lg4ff: Update respective sysfs interface documentation
  HID: hid-lg: Only one of LG_FF flags can be set for a given device.

 .../ABI/testing/sysfs-driver-hid-logitech-lg4ff    |   8 +-
 drivers/hid/hid-lg.c                               |  21 +-
 drivers/hid/hid-lg4ff.c                            | 459 ++++++++++++++-------
 drivers/hid/hid-lg4ff.h                            |   4 +-
 4 files changed, 319 insertions(+), 173 deletions(-)

-- 
2.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/12] HID: hid-lg4ff: (Cleanup) Remove double underscore prefix from numeric types.

[Michal Malý]

This code will never be used outside the kernel so the prefixes are
unnecessary.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg4ff.c | 52 ++++++++++++++++++++++++-------------------------
 drivers/hid/hid-lg4ff.h |  4 ++--
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 1232210..70e5ae6 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -72,12 +72,12 @@ static void hid_lg4ff_set_range_dfp(struct hid_device *hid, u16 range);
 static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range);
 
 struct lg4ff_device_entry {
-	__u32 product_id;
-	__u16 range;
-	__u16 min_range;
-	__u16 max_range;
+	u32 product_id;
+	u16 range;
+	u16 min_range;
+	u16 max_range;
 #ifdef CONFIG_LEDS_CLASS
-	__u8  led_state;
+	u8  led_state;
 	struct led_classdev *led[5];
 #endif
 	u32 alternate_modes;
@@ -95,16 +95,16 @@ static const signed short lg4ff_wheel_effects[] = {
 };
 
 struct lg4ff_wheel {
-	const __u32 product_id;
+	const u32 product_id;
 	const signed short *ff_effects;
-	const __u16 min_range;
-	const __u16 max_range;
+	const u16 min_range;
+	const u16 max_range;
 	void (*set_range)(struct hid_device *hid, u16 range);
 };
 
 struct lg4ff_compat_mode_switch {
-	const __u8 cmd_count;	/* Number of commands to send */
-	const __u8 cmd[];
+	const u8 cmd_count;	/* Number of commands to send */
+	const u8 cmd[];
 };
 
 struct lg4ff_wheel_ident_info {
@@ -245,10 +245,10 @@ static const struct lg4ff_compat_mode_switch lg4ff_mode_switch_ext16_g25 = {
 };
 
 /* Recalculates X axis value accordingly to currently selected range */
-static __s32 lg4ff_adjust_dfp_x_axis(__s32 value, __u16 range)
+static s32 lg4ff_adjust_dfp_x_axis(s32 value, u16 range)
 {
-	__u16 max_range;
-	__s32 new_value;
+	u16 max_range;
+	s32 new_value;
 
 	if (range == 900)
 		return value;
@@ -269,10 +269,10 @@ static __s32 lg4ff_adjust_dfp_x_axis(__s32 value, __u16 range)
 }
 
 int lg4ff_adjust_input_event(struct hid_device *hid, struct hid_field *field,
-			     struct hid_usage *usage, __s32 value, struct lg_drv_data *drv_data)
+			     struct hid_usage *usage, s32 value, struct lg_drv_data *drv_data)
 {
 	struct lg4ff_device_entry *entry = drv_data->device_props;
-	__s32 new_value = 0;
+	s32 new_value = 0;
 
 	if (!entry) {
 		hid_err(hid, "Device properties not found");
@@ -299,7 +299,7 @@ static int hid_lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *e
 	struct hid_device *hid = input_get_drvdata(dev);
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
-	__s32 *value = report->field[0]->value;
+	s32 *value = report->field[0]->value;
 	int x;
 
 #define CLAMP(x) do { if (x < 0) x = 0; else if (x > 0xff) x = 0xff; } while (0)
@@ -344,8 +344,8 @@ static void hid_lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitud
 	struct hid_device *hid = input_get_drvdata(dev);
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
-	__s32 *value = report->field[0]->value;
-	__u32 expand_a, expand_b;
+	s32 *value = report->field[0]->value;
+	u32 expand_a, expand_b;
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
 
@@ -421,7 +421,7 @@ static void hid_lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
 	struct hid_device *hid = input_get_drvdata(dev);
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
-	__s32 *value = report->field[0]->value;
+	s32 *value = report->field[0]->value;
 	magnitude = magnitude * 90 / 65535;
 
 	value[0] = 0xfe;
@@ -440,7 +440,7 @@ static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
-	__s32 *value = report->field[0]->value;
+	s32 *value = report->field[0]->value;
 
 	dbg_hid("G25/G27/DFGT: setting range to %u\n", range);
 
@@ -456,12 +456,12 @@ static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 }
 
 /* Sends commands to set range compatible with Driving Force Pro wheel */
-static void hid_lg4ff_set_range_dfp(struct hid_device *hid, __u16 range)
+static void hid_lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	int start_left, start_right, full_range;
-	__s32 *value = report->field[0]->value;
+	s32 *value = report->field[0]->value;
 
 	dbg_hid("Driving Force Pro: setting range to %u\n", range);
 
@@ -571,7 +571,7 @@ static int lg4ff_switch_compatibility_mode(struct hid_device *hid, const struct
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
-	__s32 *value = report->field[0]->value;
+	s32 *value = report->field[0]->value;
 	u8 i;
 
 	for (i = 0; i < s->cmd_count; i++) {
@@ -754,7 +754,7 @@ static ssize_t range_store(struct device *dev, struct device_attribute *attr,
 	struct hid_device *hid = to_hid_device(dev);
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
-	__u16 range = simple_strtoul(buf, NULL, 10);
+	u16 range = simple_strtoul(buf, NULL, 10);
 
 	drv_data = hid_get_drvdata(hid);
 	if (!drv_data) {
@@ -818,11 +818,11 @@ static ssize_t lg4ff_real_id_store(struct device *dev, struct device_attribute *
 static DEVICE_ATTR(real_id, S_IRUGO, lg4ff_real_id_show, lg4ff_real_id_store);
 
 #ifdef CONFIG_LEDS_CLASS
-static void lg4ff_set_leds(struct hid_device *hid, __u8 leds)
+static void lg4ff_set_leds(struct hid_device *hid, u8 leds)
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
-	__s32 *value = report->field[0]->value;
+	s32 *value = report->field[0]->value;
 
 	value[0] = 0xf8;
 	value[1] = 0x12;
diff --git a/drivers/hid/hid-lg4ff.h b/drivers/hid/hid-lg4ff.h
index 5b6a508..66201af 100644
--- a/drivers/hid/hid-lg4ff.h
+++ b/drivers/hid/hid-lg4ff.h
@@ -5,12 +5,12 @@
 extern int lg4ff_no_autoswitch; /* From hid-lg.c */
 
 int lg4ff_adjust_input_event(struct hid_device *hid, struct hid_field *field,
-			     struct hid_usage *usage, __s32 value, struct lg_drv_data *drv_data);
+			     struct hid_usage *usage, s32 value, struct lg_drv_data *drv_data);
 int lg4ff_init(struct hid_device *hdev);
 int lg4ff_deinit(struct hid_device *hdev);
 #else
 static inline int lg4ff_adjust_input_event(struct hid_device *hid, struct hid_field *field,
-					   struct hid_usage *usage, __s32 value, struct lg_drv_data *drv_data) { return 0; }
+					   struct hid_usage *usage, s32 value, struct lg_drv_data *drv_data) { return 0; }
 static inline int lg4ff_init(struct hid_device *hdev) { return -1; }
 static inline int lg4ff_deinit(struct hid_device *hdev) { return -1; }
 #endif
-- 
2.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/12] HID: hid-lg4ff: (Cleanup) Remove "hid_" prefix from some functions names.

[Michal Malý]

All internal functions of hid-lg4ff should be prefixed with just
"lg4ff_"

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg4ff.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 70e5ae6..9a0c0e5 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -68,8 +68,8 @@
 #define LG4FF_FFEX_REV_MAJ 0x21
 #define LG4FF_FFEX_REV_MIN 0x00
 
-static void hid_lg4ff_set_range_dfp(struct hid_device *hid, u16 range);
-static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range);
+static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range);
+static void lg4ff_set_range_g25(struct hid_device *hid, u16 range);
 
 struct lg4ff_device_entry {
 	u32 product_id;
@@ -134,10 +134,10 @@ struct lg4ff_alternate_mode {
 static const struct lg4ff_wheel lg4ff_devices[] = {
 	{USB_DEVICE_ID_LOGITECH_WHEEL,       lg4ff_wheel_effects, 40, 270, NULL},
 	{USB_DEVICE_ID_LOGITECH_MOMO_WHEEL,  lg4ff_wheel_effects, 40, 270, NULL},
-	{USB_DEVICE_ID_LOGITECH_DFP_WHEEL,   lg4ff_wheel_effects, 40, 900, hid_lg4ff_set_range_dfp},
-	{USB_DEVICE_ID_LOGITECH_G25_WHEEL,   lg4ff_wheel_effects, 40, 900, hid_lg4ff_set_range_g25},
-	{USB_DEVICE_ID_LOGITECH_DFGT_WHEEL,  lg4ff_wheel_effects, 40, 900, hid_lg4ff_set_range_g25},
-	{USB_DEVICE_ID_LOGITECH_G27_WHEEL,   lg4ff_wheel_effects, 40, 900, hid_lg4ff_set_range_g25},
+	{USB_DEVICE_ID_LOGITECH_DFP_WHEEL,   lg4ff_wheel_effects, 40, 900, lg4ff_set_range_dfp},
+	{USB_DEVICE_ID_LOGITECH_G25_WHEEL,   lg4ff_wheel_effects, 40, 900, lg4ff_set_range_g25},
+	{USB_DEVICE_ID_LOGITECH_DFGT_WHEEL,  lg4ff_wheel_effects, 40, 900, lg4ff_set_range_g25},
+	{USB_DEVICE_ID_LOGITECH_G27_WHEEL,   lg4ff_wheel_effects, 40, 900, lg4ff_set_range_g25},
 	{USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2, lg4ff_wheel_effects, 40, 270, NULL},
 	{USB_DEVICE_ID_LOGITECH_WII_WHEEL,   lg4ff_wheel_effects, 40, 270, NULL}
 };
@@ -294,7 +294,7 @@ int lg4ff_adjust_input_event(struct hid_device *hid, struct hid_field *field,
 	}
 }
 
-static int hid_lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effect)
+static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effect)
 {
 	struct hid_device *hid = input_get_drvdata(dev);
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
@@ -339,7 +339,7 @@ static int hid_lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *e
 
 /* Sends default autocentering command compatible with
  * all wheels except Formula Force EX */
-static void hid_lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
+static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 {
 	struct hid_device *hid = input_get_drvdata(dev);
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
@@ -416,7 +416,7 @@ static void hid_lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitud
 }
 
 /* Sends autocentering command compatible with Formula Force EX */
-static void hid_lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
+static void lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
 {
 	struct hid_device *hid = input_get_drvdata(dev);
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
@@ -436,7 +436,7 @@ static void hid_lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
 }
 
 /* Sends command to set range compatible with G25/G27/Driving Force GT */
-static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range)
+static void lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
@@ -456,7 +456,7 @@ static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 }
 
 /* Sends commands to set range compatible with Driving Force Pro wheel */
-static void hid_lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
+static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
@@ -1043,7 +1043,7 @@ int lg4ff_init(struct hid_device *hid)
 	for (j = 0; lg4ff_devices[i].ff_effects[j] >= 0; j++)
 		set_bit(lg4ff_devices[i].ff_effects[j], dev->ffbit);
 
-	error = input_ff_create_memless(dev, NULL, hid_lg4ff_play);
+	error = input_ff_create_memless(dev, NULL, lg4ff_play);
 
 	if (error)
 		return error;
@@ -1081,9 +1081,9 @@ int lg4ff_init(struct hid_device *hid)
 		/* Formula Force EX expects different autocentering command */
 		if ((bcdDevice >> 8) == LG4FF_FFEX_REV_MAJ &&
 		    (bcdDevice & 0xff) == LG4FF_FFEX_REV_MIN)
-			dev->ff->set_autocenter = hid_lg4ff_set_autocenter_ffex;
+			dev->ff->set_autocenter = lg4ff_set_autocenter_ffex;
 		else
-			dev->ff->set_autocenter = hid_lg4ff_set_autocenter_default;
+			dev->ff->set_autocenter = lg4ff_set_autocenter_default;
 
 		dev->ff->set_autocenter(dev, 0);
 	}
-- 
2.3.3

[PATCH 03/12] HID: hid-lg4ff: (Cleanup) Replace DEVICE_ATTR_RW with DEVICE_ATTR to have all internal functions prefixed with "lg4ff_"

[Michal Malý]

All internal functions should be prefixed with just "lg4ff_". Usage of
DEVICE_ATTR_RW breaks this scheme because it expects the functions'
names to match the name of the respective sysfs entry.
This partially reverts "2f1cec3250e38609bf9252db52dbbe61603c04a7"

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg4ff.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 9a0c0e5..cec84a5 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -721,9 +721,9 @@ static ssize_t lg4ff_alternate_modes_store(struct device *dev, struct device_att
 }
 static DEVICE_ATTR(alternate_modes, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH, lg4ff_alternate_modes_show, lg4ff_alternate_modes_store);
 
-/* Read current range and display it in terminal */
-static ssize_t range_show(struct device *dev, struct device_attribute *attr,
-			  char *buf)
+/* Export the currently set range of the wheel */
+static ssize_t lg4ff_range_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
 {
 	struct hid_device *hid = to_hid_device(dev);
 	struct lg4ff_device_entry *entry;
@@ -748,8 +748,8 @@ static ssize_t range_show(struct device *dev, struct device_attribute *attr,
 
 /* Set range to user specified value, call appropriate function
  * according to the type of the wheel */
-static ssize_t range_store(struct device *dev, struct device_attribute *attr,
-			   const char *buf, size_t count)
+static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *attr,
+				 const char *buf, size_t count)
 {
 	struct hid_device *hid = to_hid_device(dev);
 	struct lg4ff_device_entry *entry;
@@ -780,7 +780,7 @@ static ssize_t range_store(struct device *dev, struct device_attribute *attr,
 
 	return count;
 }
-static DEVICE_ATTR_RW(range);
+static DEVICE_ATTR(range, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH, lg4ff_range_show, lg4ff_range_store);
 
 static ssize_t lg4ff_real_id_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
-- 
2.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/12] HID: hid-lg4ff: (Cleanup) Remove unused variable from the "lg4ff_device_entry" struct.

[Michal Malý]

This is a leftover from times where hid_get|set_drvdata() was not
available to hid-lg4ff so it had to keep track of the devices it managed
by itself.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg4ff.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index cec84a5..d1d5d45 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -84,7 +84,6 @@ struct lg4ff_device_entry {
 	const char *real_tag;
 	const char *real_name;
 	u16 real_product_id;
-	struct list_head list;
 	void (*set_range)(struct hid_device *hid, u16 range);
 };
 
-- 
2.3.3

[PATCH 05/12] HID: hid-lg4ff: Update a warning message for a case where device is incorrectly flagged to be handled by hid-lg4ff in hid-lg.

[Michal Malý]

The original warning message was highly misleading. This warning can be
triggered only if a device is flagged to be handled by hid-lg4ff in
hid-lg but hid-lg4ff lacks support for such device.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg4ff.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index d1d5d45..a2f47ee 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -1021,8 +1021,7 @@ int lg4ff_init(struct hid_device *hid)
 	}
 
 	if (i == ARRAY_SIZE(lg4ff_devices)) {
-		hid_err(hid, "Device is not supported by lg4ff driver. If you think it should be, consider reporting a bug to"
-			     "LKML, Simon Wood <simon@mungewell.org> or Michal Maly <madcatxster@gmail.com>\n");
+		hid_err(hid, "This device is flagged to be handled by the lg4ff module but this module does not know how to handle it. Please report this as a bug to LKML, Simon Wood <simon@mungewell.org> or Michal Maly <madcatxster@devoid-pointer.net>\n");
 		return -1;
 	}
 
-- 
2.3.3

[PATCH 06/12] HID: hid-lg: Check return values from lg[N]ff_init()

[Michal Malý]

hid-lg did not check return values from the lg[N]_init() functions,
possibly trying to work with a device whose initialization has failed.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index b86c18e..c797781 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -712,13 +712,16 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (drv_data->quirks & LG_FF)
-		lgff_init(hdev);
+		ret = lgff_init(hdev);
 	if (drv_data->quirks & LG_FF2)
-		lg2ff_init(hdev);
+		ret = lg2ff_init(hdev);
 	if (drv_data->quirks & LG_FF3)
-		lg3ff_init(hdev);
+		ret = lg3ff_init(hdev);
 	if (drv_data->quirks & LG_FF4)
-		lg4ff_init(hdev);
+		ret = lg4ff_init(hdev);
+
+	if (ret)
+		goto err_free;
 
 	return 0;
 err_free:
-- 
2.3.3

[PATCH 07/12] HID: hid-lg4ff: Protect concurrent access to the output HID report values with a spinlock.

[Michal Malý]

Since all functions that need to send some data to the device they
manage share the same HID report some synchronization is needed to
prevent sending bogus data to the device.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg.c    |   4 +-
 drivers/hid/hid-lg4ff.c | 293 ++++++++++++++++++++++++++++++++++--------------
 2 files changed, 213 insertions(+), 84 deletions(-)

diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index c797781..c3981da 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -734,8 +734,8 @@ static void lg_remove(struct hid_device *hdev)
 	struct lg_drv_data *drv_data = hid_get_drvdata(hdev);
 	if (drv_data->quirks & LG_FF4)
 		lg4ff_deinit(hdev);
-
-	hid_hw_stop(hdev);
+	else
+		hid_hw_stop(hdev);
 	kfree(drv_data);
 }
 
diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index a2f47ee..0bbdeea 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -71,7 +71,7 @@
 static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range);
 static void lg4ff_set_range_g25(struct hid_device *hid, u16 range);
 
-struct lg4ff_device_entry {
+struct lg4ff_wheel_data {
 	u32 product_id;
 	u16 range;
 	u16 min_range;
@@ -84,9 +84,15 @@ struct lg4ff_device_entry {
 	const char *real_tag;
 	const char *real_name;
 	u16 real_product_id;
+
 	void (*set_range)(struct hid_device *hid, u16 range);
 };
 
+struct lg4ff_device_entry {
+	spinlock_t report_lock;
+	struct lg4ff_wheel_data wdata;
+};
+
 static const signed short lg4ff_wheel_effects[] = {
 	FF_CONSTANT,
 	FF_AUTOCENTER,
@@ -278,11 +284,11 @@ int lg4ff_adjust_input_event(struct hid_device *hid, struct hid_field *field,
 		return 0;
 	}
 
-	switch (entry->product_id) {
+	switch (entry->wdata.product_id) {
 	case USB_DEVICE_ID_LOGITECH_DFP_WHEEL:
 		switch (usage->code) {
 		case ABS_X:
-			new_value = lg4ff_adjust_dfp_x_axis(value, entry->range);
+			new_value = lg4ff_adjust_dfp_x_axis(value, entry->wdata.range);
 			input_event(field->hidinput->input, usage->type, usage->code, new_value);
 			return 1;
 		default:
@@ -298,8 +304,23 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 	struct hid_device *hid = input_get_drvdata(dev);
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
+	struct lg4ff_device_entry *entry;
+	struct lg_drv_data *drv_data;
 	s32 *value = report->field[0]->value;
 	int x;
+	unsigned long flags;
+
+	drv_data = hid_get_drvdata(hid);
+	if (!drv_data) {
+		hid_err(hid, "Private driver data not found!\n");
+		return -EINVAL;
+	}
+
+	entry = drv_data->device_props;
+	if (!entry) {
+		hid_err(hid, "Device properties not found!\n");
+		return -EINVAL;
+	}
 
 #define CLAMP(x) do { if (x < 0) x = 0; else if (x > 0xff) x = 0xff; } while (0)
 
@@ -308,6 +329,7 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 		x = effect->u.ramp.start_level + 0x80;	/* 0x80 is no force */
 		CLAMP(x);
 
+		spin_lock_irqsave(&entry->report_lock, flags);
 		if (x == 0x80) {
 			/* De-activate force in slot-1*/
 			value[0] = 0x13;
@@ -319,6 +341,7 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 			value[6] = 0x00;
 
 			hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+			spin_unlock_irqrestore(&entry->report_lock, flags);
 			return 0;
 		}
 
@@ -331,6 +354,7 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 		value[6] = 0x00;
 
 		hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+		spin_unlock_irqrestore(&entry->report_lock, flags);
 		break;
 	}
 	return 0;
@@ -343,10 +367,11 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 	struct hid_device *hid = input_get_drvdata(dev);
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
-	s32 *value = report->field[0]->value;
-	u32 expand_a, expand_b;
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
+	s32 *value = report->field[0]->value;
+	u32 expand_a, expand_b;
+	unsigned long flags;
 
 	drv_data = hid_get_drvdata(hid);
 	if (!drv_data) {
@@ -361,6 +386,7 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 	}
 
 	/* De-activate Auto-Center */
+	spin_lock_irqsave(&entry->report_lock, flags);
 	if (magnitude == 0) {
 		value[0] = 0xf5;
 		value[1] = 0x00;
@@ -371,6 +397,7 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 		value[6] = 0x00;
 
 		hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+		spin_unlock_irqrestore(&entry->report_lock, flags);
 		return;
 	}
 
@@ -383,7 +410,7 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 	}
 
 	/* Adjust for non-MOMO wheels */
-	switch (entry->product_id) {
+	switch (entry->wdata.product_id) {
 	case USB_DEVICE_ID_LOGITECH_MOMO_WHEEL:
 	case USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2:
 		break;
@@ -412,6 +439,7 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 	value[6] = 0x00;
 
 	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
 /* Sends autocentering command compatible with Formula Force EX */
@@ -420,9 +448,25 @@ static void lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
 	struct hid_device *hid = input_get_drvdata(dev);
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
+	struct lg4ff_device_entry *entry;
+	struct lg_drv_data *drv_data;
+	unsigned long flags;
 	s32 *value = report->field[0]->value;
 	magnitude = magnitude * 90 / 65535;
 
+	drv_data = hid_get_drvdata(hid);
+	if (!drv_data) {
+		hid_err(hid, "Private driver data not found!\n");
+		return;
+	}
+
+	entry = drv_data->device_props;
+	if (!entry) {
+		hid_err(hid, "Device properties not found!\n");
+		return;
+	}
+
+	spin_lock_irqsave(&entry->report_lock, flags);
 	value[0] = 0xfe;
 	value[1] = 0x03;
 	value[2] = magnitude >> 14;
@@ -432,6 +476,7 @@ static void lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
 	value[6] = 0x00;
 
 	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
 /* Sends command to set range compatible with G25/G27/Driving Force GT */
@@ -439,10 +484,25 @@ static void lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
+	struct lg4ff_device_entry *entry;
+	struct lg_drv_data *drv_data;
 	s32 *value = report->field[0]->value;
+	unsigned long flags;
 
+	drv_data = hid_get_drvdata(hid);
+	if (!drv_data) {
+		hid_err(hid, "Private driver data not found!\n");
+		return;
+	}
+
+	entry = drv_data->device_props;
+	if (!entry) {
+		hid_err(hid, "Device properties not found!\n");
+		return;
+	}
 	dbg_hid("G25/G27/DFGT: setting range to %u\n", range);
 
+	spin_lock_irqsave(&entry->report_lock, flags);
 	value[0] = 0xf8;
 	value[1] = 0x81;
 	value[2] = range & 0x00ff;
@@ -452,6 +512,7 @@ static void lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 	value[6] = 0x00;
 
 	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
 /* Sends commands to set range compatible with Driving Force Pro wheel */
@@ -459,11 +520,26 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
+	struct lg4ff_device_entry *entry;
+	struct lg_drv_data *drv_data;
 	int start_left, start_right, full_range;
 	s32 *value = report->field[0]->value;
+	unsigned long flags;
 
+	drv_data = hid_get_drvdata(hid);
+	if (!drv_data) {
+		hid_err(hid, "Private driver data not found!\n");
+		return;
+	}
+
+	entry = drv_data->device_props;
+	if (!entry) {
+		hid_err(hid, "Device properties not found!\n");
+		return;
+	}
 	dbg_hid("Driving Force Pro: setting range to %u\n", range);
 
+	spin_lock_irqsave(&entry->report_lock, flags);
 	/* Prepare "coarse" limit command */
 	value[0] = 0xf8;
 	value[1] = 0x00;	/* Set later */
@@ -493,6 +569,7 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 
 	if (range == 200 || range == 900) {	/* Do not apply any fine limit */
 		hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+		spin_unlock_irqrestore(&entry->report_lock, flags);
 		return;
 	}
 
@@ -507,6 +584,7 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 	value[6] = 0xff;
 
 	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
 static const struct lg4ff_compat_mode_switch *lg4ff_get_mode_switch_command(const u16 real_product_id, const u16 target_product_id)
@@ -570,9 +648,25 @@ static int lg4ff_switch_compatibility_mode(struct hid_device *hid, const struct
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
+	struct lg4ff_device_entry *entry;
+	struct lg_drv_data *drv_data;
 	s32 *value = report->field[0]->value;
 	u8 i;
+	unsigned long flags;
+
+	drv_data = hid_get_drvdata(hid);
+	if (!drv_data) {
+		hid_err(hid, "Private driver data not found!\n");
+		return -EINVAL;
+	}
+
+	entry = drv_data->device_props;
+	if (!entry) {
+		hid_err(hid, "Device properties not found!\n");
+		return -EINVAL;
+	}
 
+	spin_lock_irqsave(&entry->report_lock, flags);
 	for (i = 0; i < s->cmd_count; i++) {
 		u8 j;
 
@@ -581,6 +675,8 @@ static int lg4ff_switch_compatibility_mode(struct hid_device *hid, const struct
 
 		hid_hw_request(hid, report, HID_REQ_SET_REPORT);
 	}
+	spin_unlock_irqrestore(&entry->report_lock, flags);
+
 	hid_hw_wait(hid);
 	return 0;
 }
@@ -605,23 +701,23 @@ static ssize_t lg4ff_alternate_modes_show(struct device *dev, struct device_attr
 		return 0;
 	}
 
-	if (!entry->real_name) {
+	if (!entry->wdata.real_name) {
 		hid_err(hid, "NULL pointer to string\n");
 		return 0;
 	}
 
 	for (i = 0; i < LG4FF_MODE_MAX_IDX; i++) {
-		if (entry->alternate_modes & BIT(i)) {
+		if (entry->wdata.alternate_modes & BIT(i)) {
 			/* Print tag and full name */
 			count += scnprintf(buf + count, PAGE_SIZE - count, "%s: %s",
 					   lg4ff_alternate_modes[i].tag,
-					   !lg4ff_alternate_modes[i].product_id ? entry->real_name : lg4ff_alternate_modes[i].name);
+					   !lg4ff_alternate_modes[i].product_id ? entry->wdata.real_name : lg4ff_alternate_modes[i].name);
 			if (count >= PAGE_SIZE - 1)
 				return count;
 
 			/* Mark the currently active mode with an asterisk */
-			if (lg4ff_alternate_modes[i].product_id == entry->product_id ||
-			    (lg4ff_alternate_modes[i].product_id == 0 && entry->product_id == entry->real_product_id))
+			if (lg4ff_alternate_modes[i].product_id == entry->wdata.product_id ||
+			    (lg4ff_alternate_modes[i].product_id == 0 && entry->wdata.product_id == entry->wdata.real_product_id))
 				count += scnprintf(buf + count, PAGE_SIZE - count, " *\n");
 			else
 				count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
@@ -674,10 +770,10 @@ static ssize_t lg4ff_alternate_modes_store(struct device *dev, struct device_att
 		const u16 mode_product_id = lg4ff_alternate_modes[i].product_id;
 		const char *tag = lg4ff_alternate_modes[i].tag;
 
-		if (entry->alternate_modes & BIT(i)) {
+		if (entry->wdata.alternate_modes & BIT(i)) {
 			if (!strcmp(tag, lbuf)) {
 				if (!mode_product_id)
-					target_product_id = entry->real_product_id;
+					target_product_id = entry->wdata.real_product_id;
 				else
 					target_product_id = mode_product_id;
 				break;
@@ -692,24 +788,24 @@ static ssize_t lg4ff_alternate_modes_store(struct device *dev, struct device_att
 	}
 	kfree(lbuf); /* Not needed anymore */
 
-	if (target_product_id == entry->product_id) /* Nothing to do */
+	if (target_product_id == entry->wdata.product_id) /* Nothing to do */
 		return count;
 
 	/* Automatic switching has to be disabled for the switch to DF-EX mode to work correctly */
 	if (target_product_id == USB_DEVICE_ID_LOGITECH_WHEEL && !lg4ff_no_autoswitch) {
 		hid_info(hid, "\"%s\" cannot be switched to \"DF-EX\" mode. Load the \"hid_logitech\" module with \"lg4ff_no_autoswitch=1\" parameter set and try again\n",
-			 entry->real_name);
+			 entry->wdata.real_name);
 		return -EINVAL;
 	}
 
 	/* Take care of hardware limitations */
-	if ((entry->real_product_id == USB_DEVICE_ID_LOGITECH_DFP_WHEEL || entry->real_product_id == USB_DEVICE_ID_LOGITECH_G25_WHEEL) &&
-	    entry->product_id > target_product_id) {
-		hid_info(hid, "\"%s\" cannot be switched back into \"%s\" mode\n", entry->real_name, lg4ff_alternate_modes[i].name);
+	if ((entry->wdata.real_product_id == USB_DEVICE_ID_LOGITECH_DFP_WHEEL || entry->wdata.real_product_id == USB_DEVICE_ID_LOGITECH_G25_WHEEL) &&
+	    entry->wdata.product_id > target_product_id) {
+		hid_info(hid, "\"%s\" cannot be switched back into \"%s\" mode\n", entry->wdata.real_name, lg4ff_alternate_modes[i].name);
 		return -EINVAL;
 	}
 
-	s = lg4ff_get_mode_switch_command(entry->real_product_id, target_product_id);
+	s = lg4ff_get_mode_switch_command(entry->wdata.real_product_id, target_product_id);
 	if (!s) {
 		hid_err(hid, "Invalid target product ID %X\n", target_product_id);
 		return -EINVAL;
@@ -741,7 +837,7 @@ static ssize_t lg4ff_range_show(struct device *dev, struct device_attribute *att
 		return 0;
 	}
 
-	count = scnprintf(buf, PAGE_SIZE, "%u\n", entry->range);
+	count = scnprintf(buf, PAGE_SIZE, "%u\n", entry->wdata.range);
 	return count;
 }
 
@@ -768,13 +864,13 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
 	}
 
 	if (range == 0)
-		range = entry->max_range;
+		range = entry->wdata.max_range;
 
 	/* Check if the wheel supports range setting
 	 * and that the range is within limits for the wheel */
-	if (entry->set_range != NULL && range >= entry->min_range && range <= entry->max_range) {
-		entry->set_range(hid, range);
-		entry->range = range;
+	if (entry->wdata.set_range && range >= entry->wdata.min_range && range <= entry->wdata.max_range) {
+		entry->wdata.set_range(hid, range);
+		entry->wdata.range = range;
 	}
 
 	return count;
@@ -800,12 +896,12 @@ static ssize_t lg4ff_real_id_show(struct device *dev, struct device_attribute *a
 		return 0;
 	}
 
-	if (!entry->real_tag || !entry->real_name) {
+	if (!entry->wdata.real_tag || !entry->wdata.real_name) {
 		hid_err(hid, "NULL pointer to string\n");
 		return 0;
 	}
 
-	count = scnprintf(buf, PAGE_SIZE, "%s: %s\n", entry->real_tag, entry->real_name);
+	count = scnprintf(buf, PAGE_SIZE, "%s: %s\n", entry->wdata.real_tag, entry->wdata.real_name);
 	return count;
 }
 
@@ -821,8 +917,24 @@ static void lg4ff_set_leds(struct hid_device *hid, u8 leds)
 {
 	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
+	struct lg_drv_data *drv_data = hid_get_drvdata(hid);
+	struct lg4ff_device_entry *entry;
 	s32 *value = report->field[0]->value;
+	unsigned long flags;
+
+	drv_data = hid_get_drvdata(hid);
+	if (!drv_data) {
+		hid_err(hid, "Private driver data not found!\n");
+		return;
+	}
 
+	entry = drv_data->device_props;
+	if (!entry) {
+		hid_err(hid, "Device properties not found!\n");
+		return;
+	}
+
+	spin_lock_irqsave(&entry->report_lock, flags);
 	value[0] = 0xf8;
 	value[1] = 0x12;
 	value[2] = leds;
@@ -831,6 +943,7 @@ static void lg4ff_set_leds(struct hid_device *hid, u8 leds)
 	value[5] = 0x00;
 	value[6] = 0x00;
 	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
 static void lg4ff_led_set_brightness(struct led_classdev *led_cdev,
@@ -855,15 +968,15 @@ static void lg4ff_led_set_brightness(struct led_classdev *led_cdev,
 	}
 
 	for (i = 0; i < 5; i++) {
-		if (led_cdev != entry->led[i])
+		if (led_cdev != entry->wdata.led[i])
 			continue;
-		state = (entry->led_state >> i) & 1;
+		state = (entry->wdata.led_state >> i) & 1;
 		if (value == LED_OFF && state) {
-			entry->led_state &= ~(1 << i);
-			lg4ff_set_leds(hid, entry->led_state);
+			entry->wdata.led_state &= ~(1 << i);
+			lg4ff_set_leds(hid, entry->wdata.led_state);
 		} else if (value != LED_OFF && !state) {
-			entry->led_state |= 1 << i;
-			lg4ff_set_leds(hid, entry->led_state);
+			entry->wdata.led_state |= 1 << i;
+			lg4ff_set_leds(hid, entry->wdata.led_state);
 		}
 		break;
 	}
@@ -890,8 +1003,8 @@ static enum led_brightness lg4ff_led_get_brightness(struct led_classdev *led_cde
 	}
 
 	for (i = 0; i < 5; i++)
-		if (led_cdev == entry->led[i]) {
-			value = (entry->led_state >> i) & 1;
+		if (led_cdev == entry->wdata.led[i]) {
+			value = (entry->wdata.led_state >> i) & 1;
 			break;
 		}
 
@@ -1002,6 +1115,17 @@ int lg4ff_init(struct hid_device *hid)
 	if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7))
 		return -1;
 
+	drv_data = hid_get_drvdata(hid);
+	if (!drv_data) {
+		hid_err(hid, "Cannot add device, private driver data not allocated\n");
+		return -1;
+	}
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+	spin_lock_init(&entry->report_lock);
+	drv_data->device_props = entry;
+
 	/* Check if a multimode wheel has been connected and
 	 * handle it appropriately */
 	mmode_ret = lg4ff_handle_multimode_wheel(hid, &real_product_id, bcdDevice);
@@ -1011,6 +1135,11 @@ int lg4ff_init(struct hid_device *hid)
 	 */
 	if (mmode_ret == LG4FF_MMODE_SWITCHED)
 		return 0;
+	else if (mmode_ret < 0) {
+		hid_err(hid, "Unable to switch device mode during initialization, errno %d\n", mmode_ret);
+		error = mmode_ret;
+		goto err_init;
+	}
 
 	/* Check what wheel has been connected */
 	for (i = 0; i < ARRAY_SIZE(lg4ff_devices); i++) {
@@ -1022,7 +1151,8 @@ int lg4ff_init(struct hid_device *hid)
 
 	if (i == ARRAY_SIZE(lg4ff_devices)) {
 		hid_err(hid, "This device is flagged to be handled by the lg4ff module but this module does not know how to handle it. Please report this as a bug to LKML, Simon Wood <simon@mungewell.org> or Michal Maly <madcatxster@devoid-pointer.net>\n");
-		return -1;
+		error = -1;
+		goto err_init;
 	}
 
 	if (mmode_ret == LG4FF_MMODE_IS_MULTIMODE) {
@@ -1033,7 +1163,8 @@ int lg4ff_init(struct hid_device *hid)
 
 		if (mmode_idx == ARRAY_SIZE(lg4ff_multimode_wheels)) {
 			hid_err(hid, "Device product ID %X is not listed as a multimode wheel", real_product_id);
-			return -1;
+			error = -1;
+			goto err_init;
 		}
 	}
 
@@ -1044,33 +1175,18 @@ int lg4ff_init(struct hid_device *hid)
 	error = input_ff_create_memless(dev, NULL, lg4ff_play);
 
 	if (error)
-		return error;
-
-	/* Get private driver data */
-	drv_data = hid_get_drvdata(hid);
-	if (!drv_data) {
-		hid_err(hid, "Cannot add device, private driver data not allocated\n");
-		return -1;
-	}
+		goto err_init;
 
-	/* Initialize device properties */
-	entry = kzalloc(sizeof(struct lg4ff_device_entry), GFP_KERNEL);
-	if (!entry) {
-		hid_err(hid, "Cannot add device, insufficient memory to allocate device properties.\n");
-		return -ENOMEM;
-	}
-	drv_data->device_props = entry;
-
-	entry->product_id = lg4ff_devices[i].product_id;
-	entry->real_product_id = real_product_id;
-	entry->min_range = lg4ff_devices[i].min_range;
-	entry->max_range = lg4ff_devices[i].max_range;
-	entry->set_range = lg4ff_devices[i].set_range;
+	entry->wdata.product_id = lg4ff_devices[i].product_id;
+	entry->wdata.real_product_id = real_product_id;
+	entry->wdata.min_range = lg4ff_devices[i].min_range;
+	entry->wdata.max_range = lg4ff_devices[i].max_range;
+	entry->wdata.set_range = lg4ff_devices[i].set_range;
 	if (mmode_ret == LG4FF_MMODE_IS_MULTIMODE) {
 		BUG_ON(mmode_idx == -1);
-		entry->alternate_modes = lg4ff_multimode_wheels[mmode_idx].alternate_modes;
-		entry->real_tag = lg4ff_multimode_wheels[mmode_idx].real_tag;
-		entry->real_name = lg4ff_multimode_wheels[mmode_idx].real_name;
+		entry->wdata.alternate_modes = lg4ff_multimode_wheels[mmode_idx].alternate_modes;
+		entry->wdata.real_tag = lg4ff_multimode_wheels[mmode_idx].real_tag;
+		entry->wdata.real_name = lg4ff_multimode_wheels[mmode_idx].real_name;
 	}
 
 	/* Check if autocentering is available and
@@ -1089,27 +1205,32 @@ int lg4ff_init(struct hid_device *hid)
 	/* Create sysfs interface */
 	error = device_create_file(&hid->dev, &dev_attr_range);
 	if (error)
-		return error;
+		goto err_init;
 	if (mmode_ret == LG4FF_MMODE_IS_MULTIMODE) {
 		error = device_create_file(&hid->dev, &dev_attr_real_id);
-		if (error)
-			return error;
+		if (error) {
+			device_remove_file(&hid->dev, &dev_attr_range);
+			goto err_init;
+		}
 		error = device_create_file(&hid->dev, &dev_attr_alternate_modes);
-		if (error)
-			return error;
+		if (error) {
+			device_remove_file(&hid->dev, &dev_attr_real_id);
+			device_remove_file(&hid->dev, &dev_attr_range);
+			goto err_init;
+		}
 	}
 	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);
+	entry->wdata.range = entry->wdata.max_range;
+	if (entry->wdata.set_range)
+		entry->wdata.set_range(hid, entry->wdata.range);
 
 #ifdef CONFIG_LEDS_CLASS
 	/* register led subsystem - G27 only */
-	entry->led_state = 0;
+	entry->wdata.led_state = 0;
 	for (j = 0; j < 5; j++)
-		entry->led[j] = NULL;
+		entry->wdata.led[j] = NULL;
 
 	if (lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_G27_WHEEL) {
 		struct led_classdev *led;
@@ -1124,7 +1245,7 @@ int lg4ff_init(struct hid_device *hid)
 			led = kzalloc(sizeof(struct led_classdev)+name_sz, GFP_KERNEL);
 			if (!led) {
 				hid_err(hid, "can't allocate memory for LED %d\n", j);
-				goto err;
+				goto err_leds;
 			}
 
 			name = (void *)(&led[1]);
@@ -1135,16 +1256,16 @@ int lg4ff_init(struct hid_device *hid)
 			led->brightness_get = lg4ff_led_get_brightness;
 			led->brightness_set = lg4ff_led_set_brightness;
 
-			entry->led[j] = led;
+			entry->wdata.led[j] = led;
 			error = led_classdev_register(&hid->dev, led);
 
 			if (error) {
 				hid_err(hid, "failed to register LED %d. Aborting.\n", j);
-err:
+err_leds:
 				/* Deregister LEDs (if any) */
 				for (j = 0; j < 5; j++) {
-					led = entry->led[j];
-					entry->led[j] = NULL;
+					led = entry->wdata.led[j];
+					entry->wdata.led[j] = NULL;
 					if (!led)
 						continue;
 					led_classdev_unregister(led);
@@ -1158,6 +1279,11 @@ out:
 #endif
 	hid_info(hid, "Force feedback support for Logitech Gaming Wheels\n");
 	return 0;
+
+err_init:
+	drv_data->device_props = NULL;
+	kfree(entry);
+	return error;
 }
 
 int lg4ff_deinit(struct hid_device *hid)
@@ -1174,14 +1300,19 @@ int lg4ff_deinit(struct hid_device *hid)
 	if (!entry)
 		goto out; /* Nothing more to do */
 
-	device_remove_file(&hid->dev, &dev_attr_range);
-
 	/* Multimode devices will have at least the "MODE_NATIVE" bit set */
-	if (entry->alternate_modes) {
+	if (entry->wdata.alternate_modes) {
 		device_remove_file(&hid->dev, &dev_attr_real_id);
 		device_remove_file(&hid->dev, &dev_attr_alternate_modes);
 	}
 
+	device_remove_file(&hid->dev, &dev_attr_range);
+
+	/* Make sure that nothing will try to touch the device while it is
+	 * being removed */
+	hid_hw_stop(hid);
+	drv_data->device_props = NULL;
+
 #ifdef CONFIG_LEDS_CLASS
 	{
 		int j;
@@ -1190,8 +1321,8 @@ int lg4ff_deinit(struct hid_device *hid)
 		/* Deregister LEDs (if any) */
 		for (j = 0; j < 5; j++) {
 
-			led = entry->led[j];
-			entry->led[j] = NULL;
+			led = entry->wdata.led[j];
+			entry->wdata.led[j] = NULL;
 			if (!led)
 				continue;
 			led_classdev_unregister(led);
@@ -1200,9 +1331,7 @@ int lg4ff_deinit(struct hid_device *hid)
 	}
 #endif
 
-	/* Deallocate memory */
 	kfree(entry);
-
 out:
 	dbg_hid("Device successfully unregistered\n");
 	return 0;
-- 
2.3.3

[PATCH 08/12] HID: hid-lg4ff: Store pointer to the output HID report struct in the device entry struct.

[Michal Malý]

This eliminates the need to look the HID report struct up every time it is needed.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg4ff.c | 67 +++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 0bbdeea..fd6f140 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -90,6 +90,7 @@ struct lg4ff_wheel_data {
 
 struct lg4ff_device_entry {
 	spinlock_t report_lock;
+	struct hid_report *report;
 	struct lg4ff_wheel_data wdata;
 };
 
@@ -302,11 +303,9 @@ int lg4ff_adjust_input_event(struct hid_device *hid, struct hid_field *field,
 static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effect)
 {
 	struct hid_device *hid = input_get_drvdata(dev);
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
-	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 	int x;
 	unsigned long flags;
 
@@ -321,6 +320,7 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 		hid_err(hid, "Device properties not found!\n");
 		return -EINVAL;
 	}
+	value = entry->report->field[0]->value;
 
 #define CLAMP(x) do { if (x < 0) x = 0; else if (x > 0xff) x = 0xff; } while (0)
 
@@ -340,7 +340,7 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 			value[5] = 0x00;
 			value[6] = 0x00;
 
-			hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+			hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 			spin_unlock_irqrestore(&entry->report_lock, flags);
 			return 0;
 		}
@@ -353,7 +353,7 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 		value[5] = 0x00;
 		value[6] = 0x00;
 
-		hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 		spin_unlock_irqrestore(&entry->report_lock, flags);
 		break;
 	}
@@ -365,11 +365,9 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 {
 	struct hid_device *hid = input_get_drvdata(dev);
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
-	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 	u32 expand_a, expand_b;
 	unsigned long flags;
 
@@ -384,6 +382,7 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 		hid_err(hid, "Device properties not found!\n");
 		return;
 	}
+	value = entry->report->field[0]->value;
 
 	/* De-activate Auto-Center */
 	spin_lock_irqsave(&entry->report_lock, flags);
@@ -396,7 +395,7 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 		value[5] = 0x00;
 		value[6] = 0x00;
 
-		hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 		spin_unlock_irqrestore(&entry->report_lock, flags);
 		return;
 	}
@@ -427,7 +426,7 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 	value[5] = 0x00;
 	value[6] = 0x00;
 
-	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 
 	/* Activate Auto-Center */
 	value[0] = 0x14;
@@ -438,7 +437,7 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 	value[5] = 0x00;
 	value[6] = 0x00;
 
-	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
@@ -446,12 +445,10 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 static void lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
 {
 	struct hid_device *hid = input_get_drvdata(dev);
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
-	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
 	unsigned long flags;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 	magnitude = magnitude * 90 / 65535;
 
 	drv_data = hid_get_drvdata(hid);
@@ -465,6 +462,7 @@ static void lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
 		hid_err(hid, "Device properties not found!\n");
 		return;
 	}
+	value = entry->report->field[0]->value;
 
 	spin_lock_irqsave(&entry->report_lock, flags);
 	value[0] = 0xfe;
@@ -475,18 +473,16 @@ static void lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
 	value[5] = 0x00;
 	value[6] = 0x00;
 
-	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
 /* Sends command to set range compatible with G25/G27/Driving Force GT */
 static void lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 {
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
-	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 	unsigned long flags;
 
 	drv_data = hid_get_drvdata(hid);
@@ -500,6 +496,7 @@ static void lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 		hid_err(hid, "Device properties not found!\n");
 		return;
 	}
+	value = entry->report->field[0]->value;
 	dbg_hid("G25/G27/DFGT: setting range to %u\n", range);
 
 	spin_lock_irqsave(&entry->report_lock, flags);
@@ -511,19 +508,17 @@ static void lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 	value[5] = 0x00;
 	value[6] = 0x00;
 
-	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
 /* Sends commands to set range compatible with Driving Force Pro wheel */
 static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 {
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
-	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
 	int start_left, start_right, full_range;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 	unsigned long flags;
 
 	drv_data = hid_get_drvdata(hid);
@@ -537,6 +532,7 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 		hid_err(hid, "Device properties not found!\n");
 		return;
 	}
+	value = entry->report->field[0]->value;
 	dbg_hid("Driving Force Pro: setting range to %u\n", range);
 
 	spin_lock_irqsave(&entry->report_lock, flags);
@@ -550,13 +546,13 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 	value[6] = 0x00;
 
 	if (range > 200) {
-		report->field[0]->value[1] = 0x03;
+		value[1] = 0x03;
 		full_range = 900;
 	} else {
-		report->field[0]->value[1] = 0x02;
+		value[1] = 0x02;
 		full_range = 200;
 	}
-	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 
 	/* Prepare "fine" limit command */
 	value[0] = 0x81;
@@ -568,7 +564,7 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 	value[6] = 0x00;
 
 	if (range == 200 || range == 900) {	/* Do not apply any fine limit */
-		hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 		spin_unlock_irqrestore(&entry->report_lock, flags);
 		return;
 	}
@@ -583,7 +579,7 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 	value[5] = (start_right & 0xe) << 4 | (start_left & 0xe);
 	value[6] = 0xff;
 
-	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
@@ -646,11 +642,9 @@ static const struct lg4ff_compat_mode_switch *lg4ff_get_mode_switch_command(cons
 
 static int lg4ff_switch_compatibility_mode(struct hid_device *hid, const struct lg4ff_compat_mode_switch *s)
 {
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
-	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 	u8 i;
 	unsigned long flags;
 
@@ -665,6 +659,7 @@ static int lg4ff_switch_compatibility_mode(struct hid_device *hid, const struct
 		hid_err(hid, "Device properties not found!\n");
 		return -EINVAL;
 	}
+	value = entry->report->field[0]->value;
 
 	spin_lock_irqsave(&entry->report_lock, flags);
 	for (i = 0; i < s->cmd_count; i++) {
@@ -673,7 +668,7 @@ static int lg4ff_switch_compatibility_mode(struct hid_device *hid, const struct
 		for (j = 0; j < 7; j++)
 			value[j] = s->cmd[j + (7*i)];
 
-		hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	}
 	spin_unlock_irqrestore(&entry->report_lock, flags);
 
@@ -915,11 +910,9 @@ static DEVICE_ATTR(real_id, S_IRUGO, lg4ff_real_id_show, lg4ff_real_id_store);
 #ifdef CONFIG_LEDS_CLASS
 static void lg4ff_set_leds(struct hid_device *hid, u8 leds)
 {
-	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
-	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	struct lg_drv_data *drv_data = hid_get_drvdata(hid);
 	struct lg4ff_device_entry *entry;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 	unsigned long flags;
 
 	drv_data = hid_get_drvdata(hid);
@@ -933,6 +926,7 @@ static void lg4ff_set_leds(struct hid_device *hid, u8 leds)
 		hid_err(hid, "Device properties not found!\n");
 		return;
 	}
+	value = entry->report->field[0]->value;
 
 	spin_lock_irqsave(&entry->report_lock, flags);
 	value[0] = 0xf8;
@@ -942,7 +936,7 @@ static void lg4ff_set_leds(struct hid_device *hid, u8 leds)
 	value[4] = 0x00;
 	value[5] = 0x00;
 	value[6] = 0x00;
-	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
 }
 
@@ -1103,6 +1097,8 @@ int lg4ff_init(struct hid_device *hid)
 {
 	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
 	struct input_dev *dev = hidinput->input;
+	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
+	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	const struct usb_device_descriptor *udesc = &(hid_to_usb_dev(hid)->descriptor);
 	const u16 bcdDevice = le16_to_cpu(udesc->bcdDevice);
 	struct lg4ff_device_entry *entry;
@@ -1124,6 +1120,7 @@ int lg4ff_init(struct hid_device *hid)
 	if (!entry)
 		return -ENOMEM;
 	spin_lock_init(&entry->report_lock);
+	entry->report = report;
 	drv_data->device_props = entry;
 
 	/* Check if a multimode wheel has been connected and
-- 
2.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/12] HID: hid-lg4ff: Constify those members of lg4ff_device_entry struct whose value is not supposed to change.

[Michal Malý]

Prevent accidental modifications of read-only members of the lg4ff_device_entry
struct.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg4ff.c | 54 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index fd6f140..ad959e2 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -72,18 +72,18 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range);
 static void lg4ff_set_range_g25(struct hid_device *hid, u16 range);
 
 struct lg4ff_wheel_data {
-	u32 product_id;
+	const u32 product_id;
 	u16 range;
-	u16 min_range;
-	u16 max_range;
+	const u16 min_range;
+	const u16 max_range;
 #ifdef CONFIG_LEDS_CLASS
 	u8  led_state;
 	struct led_classdev *led[5];
 #endif
-	u32 alternate_modes;
-	const char *real_tag;
-	const char *real_name;
-	u16 real_product_id;
+	const u32 alternate_modes;
+	const char * const real_tag;
+	const char * const real_name;
+	const u16 real_product_id;
 
 	void (*set_range)(struct hid_device *hid, u16 range);
 };
@@ -300,6 +300,34 @@ int lg4ff_adjust_input_event(struct hid_device *hid, struct hid_field *field,
 	}
 }
 
+static void lg4ff_init_wheel_data(struct lg4ff_wheel_data * const wdata, const struct lg4ff_wheel *wheel,
+				  const struct lg4ff_multimode_wheel *mmode_wheel,
+				  const u16 real_product_id)
+{
+	u32 alternate_modes = 0;
+	const char *real_tag = NULL;
+	const char *real_name = NULL;
+
+	if (mmode_wheel) {
+		alternate_modes = mmode_wheel->alternate_modes;
+		real_tag = mmode_wheel->real_tag;
+		real_name = mmode_wheel->real_name;
+	}
+
+	{
+		struct lg4ff_wheel_data t_wdata =  { .product_id = wheel->product_id,
+						     .real_product_id = real_product_id,
+						     .min_range = wheel->min_range,
+						     .max_range = wheel->max_range,
+						     .set_range = wheel->set_range,
+						     .alternate_modes = alternate_modes,
+						     .real_tag = real_tag,
+						     .real_name = real_name };
+
+		memcpy(wdata, &t_wdata, sizeof(t_wdata));
+	}
+}
+
 static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effect)
 {
 	struct hid_device *hid = input_get_drvdata(dev);
@@ -1101,6 +1129,7 @@ int lg4ff_init(struct hid_device *hid)
 	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
 	const struct usb_device_descriptor *udesc = &(hid_to_usb_dev(hid)->descriptor);
 	const u16 bcdDevice = le16_to_cpu(udesc->bcdDevice);
+	const struct lg4ff_multimode_wheel *mmode_wheel = NULL;
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
 	int error, i, j;
@@ -1174,17 +1203,12 @@ int lg4ff_init(struct hid_device *hid)
 	if (error)
 		goto err_init;
 
-	entry->wdata.product_id = lg4ff_devices[i].product_id;
-	entry->wdata.real_product_id = real_product_id;
-	entry->wdata.min_range = lg4ff_devices[i].min_range;
-	entry->wdata.max_range = lg4ff_devices[i].max_range;
-	entry->wdata.set_range = lg4ff_devices[i].set_range;
+	/* Initialize device properties */
 	if (mmode_ret == LG4FF_MMODE_IS_MULTIMODE) {
 		BUG_ON(mmode_idx == -1);
-		entry->wdata.alternate_modes = lg4ff_multimode_wheels[mmode_idx].alternate_modes;
-		entry->wdata.real_tag = lg4ff_multimode_wheels[mmode_idx].real_tag;
-		entry->wdata.real_name = lg4ff_multimode_wheels[mmode_idx].real_name;
+		mmode_wheel = &lg4ff_multimode_wheels[mmode_idx];
 	}
+	lg4ff_init_wheel_data(&entry->wdata, &lg4ff_devices[i], mmode_wheel, real_product_id);
 
 	/* Check if autocentering is available and
 	 * set the centering force to zero by default */
-- 
2.3.3

[PATCH 10/12] HID: hid-lg4ff: Allow the driver to continue without sysfs interface.

[Michal Malý]

Instead of aborting the initialization allow the driver to continue in
a degraded mode.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg4ff.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index ad959e2..5543728 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -1226,19 +1226,14 @@ int lg4ff_init(struct hid_device *hid)
 	/* Create sysfs interface */
 	error = device_create_file(&hid->dev, &dev_attr_range);
 	if (error)
-		goto err_init;
+		hid_warn(hid, "Unable to create sysfs interface for \"range\", errno %d\n", error);
 	if (mmode_ret == LG4FF_MMODE_IS_MULTIMODE) {
 		error = device_create_file(&hid->dev, &dev_attr_real_id);
-		if (error) {
-			device_remove_file(&hid->dev, &dev_attr_range);
-			goto err_init;
-		}
+		if (error)
+			hid_warn(hid, "Unable to create sysfs interface for \"real_id\", errno %d\n", error);
 		error = device_create_file(&hid->dev, &dev_attr_alternate_modes);
-		if (error) {
-			device_remove_file(&hid->dev, &dev_attr_real_id);
-			device_remove_file(&hid->dev, &dev_attr_range);
-			goto err_init;
-		}
+		if (error)
+			hid_warn(hid, "Unable to create sysfs interface for \"alternate_modes\", errno %d\n", error);
 	}
 	dbg_hid("sysfs interface created\n");
 
-- 
2.3.3

[PATCH 11/12] HID: hid-lg4ff: Update respective sysfs interface documentation

[Michal Malý]

- Refer to the sysfs interface for "range" using "/sys/bus/..."
- Update contact email address

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 Documentation/ABI/testing/sysfs-driver-hid-logitech-lg4ff | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-hid-logitech-lg4ff b/Documentation/ABI/testing/sysfs-driver-hid-logitech-lg4ff
index b3f6a2a..db197a8 100644
--- a/Documentation/ABI/testing/sysfs-driver-hid-logitech-lg4ff
+++ b/Documentation/ABI/testing/sysfs-driver-hid-logitech-lg4ff
@@ -1,7 +1,7 @@
-What:		/sys/module/hid_logitech/drivers/hid:logitech/<dev>/range.
+What:		/sys/bus/hid/drivers/logitech/<dev>/range
 Date:		July 2011
 KernelVersion:	3.2
-Contact:	Michal Malý <madcatxster@gmail.com>
+Contact:	Michal Malý <madcatxster@devoid-pointer.net>
 Description:	Display minimum, maximum and current range of the steering
 		wheel. Writing a value within min and max boundaries sets the
 		range of the wheel.
@@ -9,7 +9,7 @@ Description:	Display minimum, maximum and current range of the steering
 What:		/sys/bus/hid/drivers/logitech/<dev>/alternate_modes
 Date:		Feb 2015
 KernelVersion:	4.1
-Contact:	Michal Malý <madcatxster@gmail.com>
+Contact:	Michal Malý <madcatxster@devoid-pointer.net>
 Description:	Displays a set of alternate modes supported by a wheel. Each
 		mode is listed as follows:
 		  Tag: Mode Name
@@ -45,7 +45,7 @@ Description:	Displays a set of alternate modes supported by a wheel. Each
 What:		/sys/bus/hid/drivers/logitech/<dev>/real_id
 Date:		Feb 2015
 KernelVersion:	4.1
-Contact:	Michal Malý <madcatxster@gmail.com>
+Contact:	Michal Malý <madcatxster@devoid-pointer.net>
 Description:	Displays the real model of the wheel regardless of any
 		alternate mode the wheel might be switched to.
 		It is a read-only value.
-- 
2.3.3

[PATCH 12/12] HID: hid-lg: Only one of LG_FF flags can be set for a given device.

[Michal Malý]

Reflect the mutual exclusivity of the LG_FF flags in the code.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/hid/hid-lg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index c3981da..f1e92bf 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -713,11 +713,11 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	if (drv_data->quirks & LG_FF)
 		ret = lgff_init(hdev);
-	if (drv_data->quirks & LG_FF2)
+	else if (drv_data->quirks & LG_FF2)
 		ret = lg2ff_init(hdev);
-	if (drv_data->quirks & LG_FF3)
+	else if (drv_data->quirks & LG_FF3)
 		ret = lg3ff_init(hdev);
-	if (drv_data->quirks & LG_FF4)
+	else if (drv_data->quirks & LG_FF4)
 		ret = lg4ff_init(hdev);
 
 	if (ret)
-- 
2.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/12] HID: hid-lg, hid-lg4ff: Mostly cleanup patches

[Elias Vanderstuyft]

On Sat, Mar 21, 2015 at 12:47 PM, Michal Malý
<madcatxster@devoid-pointer.net> wrote:
> Hello everyone,
>
> this is a mostly boring series that deals with a few inconsistencies in the
> code that have accumulated over the years. Besides that it patches up a handful
> of problems such a return values not being checked etc.
>
> The only significant change comes in patch 0007 which introduces a spinlock to
> handle concurrent access to the HID report that is used by the driver to send
> data to the wheel. I would appreciate some comments on this one, particularly
> on the way it handles deinitialization.
>
> Michal Malý (12):
>   HID: hid-lg4ff: (Cleanup) Remove double underscore prefix from numeric
>     types.
>   HID: hid-lg4ff: (Cleanup) Remove "hid_" prefix from some functions
>     names.
>   HID: hid-lg4ff: (Cleanup) Replace DEVICE_ATTR_RW with DEVICE_ATTR to
>     have all internal functions prefixed with "lg4ff_"
>   HID: hid-lg4ff: (Cleanup) Remove unused variable from the
>     "lg4ff_device_entry" struct.
>   HID: hid-lg4ff: Update a warning message for a case where device is
>     incorrectly flagged to be handled by hid-lg4ff in hid-lg.
>   HID: hid-lg: Check return values from lg[N]ff_init()
>   HID: hid-lg4ff: Protect concurrent access to the output HID report
>     values with a spinlock.
>   HID: hid-lg4ff: Store pointer to the output HID report struct in the
>     device entry struct.
>   HID: hid-lg4ff: Constify those members of lg4ff_device_entry struct
>     whose value is not supposed to change.
>   HID: hid-lg4ff: Allow the driver to continue without sysfs interface.
>   HID: hid-lg4ff: Update respective sysfs interface documentation
>   HID: hid-lg: Only one of LG_FF flags can be set for a given device.
>
>  .../ABI/testing/sysfs-driver-hid-logitech-lg4ff    |   8 +-
>  drivers/hid/hid-lg.c                               |  21 +-
>  drivers/hid/hid-lg4ff.c                            | 459 ++++++++++++++-------
>  drivers/hid/hid-lg4ff.h                            |   4 +-
>  4 files changed, 319 insertions(+), 173 deletions(-)
>
> --
> 2.3.3
>

Tested on kernel 3.18.6 (I had to apply some uptream patches first).
I tried to crash the kernel by simultaneously running many instances
of FF applications (which made use of effects with nonzero attack
times, thus the ff-memless timer's timeout got modified often), and
then unplugging the device while in use.
Instead of crashing, the kernel behaved like it should, no panics or
lockups were found.

Tested with to following devices:
- Logitech MOMO (Black) : PID 0xCA03
- Logitech Formula Vibration : PID 0xCA04

Tested-by: Elias Vanderstuyft <elias.vds@gmail.com>

Regards,
Elias
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/12] HID: hid-lg4ff: Update a warning message for a case where device is incorrectly flagged to be handled by hid-lg4ff in hid-lg.

[Jiri Kosina]

On Sat, 21 Mar 2015, Michal Malý wrote:

> The original warning message was highly misleading. This warning can be
> triggered only if a device is flagged to be handled by hid-lg4ff in
> hid-lg but hid-lg4ff lacks support for such device.
> 
> Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
> ---
>  drivers/hid/hid-lg4ff.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
> index d1d5d45..a2f47ee 100644
> --- a/drivers/hid/hid-lg4ff.c
> +++ b/drivers/hid/hid-lg4ff.c
> @@ -1021,8 +1021,7 @@ int lg4ff_init(struct hid_device *hid)
>  	}
>  
>  	if (i == ARRAY_SIZE(lg4ff_devices)) {
> -		hid_err(hid, "Device is not supported by lg4ff driver. If you think it should be, consider reporting a bug to"
> -			     "LKML, Simon Wood <simon@mungewell.org> or Michal Maly <madcatxster@gmail.com>\n");
> +		hid_err(hid, "This device is flagged to be handled by the lg4ff module but this module does not know how to handle it. Please report this as a bug to LKML, Simon Wood <simon@mungewell.org> or Michal Maly <madcatxster@devoid-pointer.net>\n");

Could you please split this so that we don't have this ugly overly long 
line, and resend this patch as v2? Thanks.

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/12] HID: hid-lg4ff: Protect concurrent access to the output HID report values with a spinlock.

[Jiri Kosina]

On Sat, 21 Mar 2015, Michal Malý wrote:

> Since all functions that need to send some data to the device they
> manage share the same HID report some synchronization is needed to
> prevent sending bogus data to the device.

This patch is doing much more than just adding a mutual exclusion 
mechanisms (for example the wdata indirection), so more descriptive 
changelog is needed.

Looking at the patch it's actually not clear at all what are all the 
things it's trying to do, so please resend v2 of this patch with much more 
verbose changelog, so that I am able to review it properly. Thanks.

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html