Skip to contentSkip to conversation

HID: hid-lg, hid-lg4ff: Mostly cleanup patches

PATCH In review Michal Malý 2 participants 18 messages Apr 8, 2015 – May 7, 2015 HID

lore
Share & tools
lore

Michal Malý

author
#0 lore raw

v2 cover HID: hid-lg, hid-lg4ff: Mostly cleanup patches

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 patches 8 through 10 which introduce 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. Patches 8 and 9 contain the necessary prep work, patch 10 adds the spinlocking.

v2:
- Split previous patch 7 into three smaller patches
- Split the long line of text in patch 5 (now patch 6)
- Add another cleanup patch that removes unnecessary casts from void *

Michal Malý (15):
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: (Cleanup) Explicit casts from void * are not necessary
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: Make lg4ff_wheel_data a separate structure stored
within lg4ff_device_entry. Adjust the initialization process
accordingly.
HID: hid-lg4ff: Stop the hid device from lg4ff.
HID: hid-lg4ff: Protect concurrent access to output HID report 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                            | 458 ++++++++++++++-------
 drivers/hid/hid-lg4ff.h                            |   4 +-
 4 files changed, 318 insertions(+), 173 deletions(-)
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#1 lore raw

PATCH v2 1/15 HID: hid-lg4ff: (Cleanup) Remove double underscore prefix from numeric types.

Remove double underscore prefix from numeric types. This code will never be used outside the kernel so the prefixes are unnecessary.

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 52 ++++++++++++++++++++++++-------------------------
 drivers/hid/hid-lg4ff.h |  4 ++--
 2 files changed, 28 insertions(+), 28 deletions(-)
drivers/hid/hid-lg4ff.c +26 -26
index 1232210..70e5ae6 100644
@@ -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;
drivers/hid/hid-lg4ff.h +2 -2
index 5b6a508..66201af 100644
@@ -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
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#2 lore raw

PATCH v2 2/15 HID: hid-lg4ff: (Cleanup) Remove "hid_" prefix from some functions' names.

In reply to Michal Malý

Remove "hid_" prefix from some functions' names. All internal functions of hid-lg4ff should be prefixed with just "lg4ff_"

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
drivers/hid/hid-lg4ff.c +14 -14
index 70e5ae6..9a0c0e5 100644
@@ -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);
 	}
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#3 lore raw

PATCH v2 3/15 HID: hid-lg4ff: (Cleanup) Replace DEVICE_ATTR_RW with DEVICE_ATTR to have all internal functions prefixed with "lg4ff_"

In reply to Michal Malý

Replace DEVICE_ATTR_RW with DEVICE_ATTR to have all internal functions prefixed with "lg4ff_". 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ý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/hid/hid-lg4ff.c +6 -6
index 9a0c0e5..cec84a5 100644
@@ -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)
 {
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#4 lore raw

PATCH v2 4/15 HID: hid-lg4ff: (Cleanup) Remove unused variable from the "lg4ff_device_entry" struct.

In reply to Michal Malý

Remove unused variable from the "lg4ff_device_entry" struct. 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ý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 1 -
 1 file changed, 1 deletion(-)
drivers/hid/hid-lg4ff.c +0 -1
index cec84a5..d1d5d45 100644
@@ -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);
 };
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#5 lore raw

PATCH v2 5/15 HID: hid-lg4ff: (Cleanup) Explicit casts from void * are not necessary

In reply to Michal Malý

Explicit casts from void * are not necessary.

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/hid/hid-lg4ff.c +2 -2
index d1d5d45..5a3b3be 100644
@@ -847,7 +847,7 @@ static void lg4ff_led_set_brightness(struct led_classdev *led_cdev,
 		return;
 	}
 
-	entry = (struct lg4ff_device_entry *)drv_data->device_props;
+	entry = drv_data->device_props;
 
 	if (!entry) {
 		hid_err(hid, "Device properties not found.");

@@ -882,7 +882,7 @@ static enum led_brightness lg4ff_led_get_brightness(struct led_classdev *led_cde
 		return LED_OFF;
 	}
 
-	entry = (struct lg4ff_device_entry *)drv_data->device_props;
+	entry = drv_data->device_props;
 
 	if (!entry) {
 		hid_err(hid, "Device properties not found.");
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#6 lore raw

PATCH v2 6/15 HID: hid-lg4ff: Update a warning message for a case where device is incorrectly flagged to be handled by hid-lg4ff in hid-lg.

In reply to Michal Malý

Update a warning message for a case where device is incorrectly flagged to be handled by hid-lg4ff in hid-lg. 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ý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/hid/hid-lg4ff.c +3 -2
index 5a3b3be..04822fb 100644
@@ -1021,8 +1021,9 @@ 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 <[email protected]> or Michal Maly <[email protected]>\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 <[email protected]> or "
+			     "Michal Maly <[email protected]>\n");
 		return -1;
 	}
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#7 lore raw

PATCH v2 7/15 HID: hid-lg: Check return values from lg[N]ff_init()

In reply to Michal Malý

Check return values from lg[N]ff_init(). 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ý <[email protected]>
---
 drivers/hid/hid-lg.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/hid/hid-lg.c +7 -4
index b86c18e..c797781 100644
@@ -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:
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#8 lore raw

PATCH v2 8/15 HID: hid-lg4ff: Make lg4ff_wheel_data a separate structure stored within lg4ff_device_entry. Adjust the initialization process accordingly.

In reply to Michal Malý

Make lg4ff_wheel_data a separate structure stored within lg4ff_device_entry. Adjust the initialization process accordingly.

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 166 +++++++++++++++++++++++++-----------------------
 1 file changed, 88 insertions(+), 78 deletions(-)
drivers/hid/hid-lg4ff.c +88 -78
index 04822fb..0f8d726 100644
@@ -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,14 @@ 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 {
+	struct lg4ff_wheel_data wdata;
+};
+
 static const signed short lg4ff_wheel_effects[] = {
 	FF_CONSTANT,
 	FF_AUTOCENTER,

@@ -278,11 +283,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:

@@ -383,7 +388,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;

@@ -605,23 +610,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 +679,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 +697,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 +746,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 +773,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 +805,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;
 }
 

@@ -855,15 +860,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 +895,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 +1007,16 @@ 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;
+	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 +1026,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++) {

@@ -1024,7 +1044,8 @@ int lg4ff_init(struct hid_device *hid)
 		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 <[email protected]> or "
 			     "Michal Maly <[email protected]>\n");
-		return -1;
+		error = -1;
+		goto err_init;
 	}
 
 	if (mmode_ret == LG4FF_MMODE_IS_MULTIMODE) {

@@ -1035,7 +1056,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;
 		}
 	}
 

@@ -1046,33 +1068,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

@@ -1091,27 +1098,28 @@ 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;
+			goto err_init;
+
 		error = device_create_file(&hid->dev, &dev_attr_alternate_modes);
 		if (error)
-			return error;
+			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;

@@ -1126,7 +1134,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]);

@@ -1137,16 +1145,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);

@@ -1160,6 +1168,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)

@@ -1176,14 +1189,13 @@ 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);
 #ifdef CONFIG_LEDS_CLASS
 	{
 		int j;

@@ -1192,8 +1204,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);

@@ -1202,9 +1214,7 @@ int lg4ff_deinit(struct hid_device *hid)
 	}
 #endif
 
-	/* Deallocate memory */
 	kfree(entry);
-
 out:
 	dbg_hid("Device successfully unregistered\n");
 	return 0;
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#9 lore raw

PATCH v2 9/15 HID: hid-lg4ff: Stop the hid device from lg4ff.

In reply to Michal Malý

Stop the hid device from lg4ff. Originally hid-lg was responsible for stopping of the device.

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg.c    | 4 ++--
 drivers/hid/hid-lg4ff.c | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)
drivers/hid/hid-lg.c +2 -2
index c797781..c3981da 100644
@@ -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);
 }
drivers/hid/hid-lg4ff.c +2 -0
index 0f8d726..031b8ab 100644
@@ -1213,6 +1213,8 @@ int lg4ff_deinit(struct hid_device *hid)
 		}
 	}
 #endif
+	hid_hw_stop(hid);
+	drv_data->device_props = NULL;
 
 	kfree(entry);
 out:
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#10 lore raw

PATCH v2 10/15 HID: hid-lg4ff: Protect concurrent access to output HID report with a spinlock

In reply to Michal Malý

Protect concurrent access to output HID report with a spinlock. The HID report structure used to submit commands to wheels is shared amongst all functions that need to do so and some of these functions can be executed in parallel. Additionally, lg4ff_play() can be called from interrupt context by ff-memless module. Locking is necessary to prevent sending bogus data to the wheels.

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)
drivers/hid/hid-lg4ff.c +110 -0
index 031b8ab..f7f4eb6 100644
@@ -89,6 +89,7 @@ struct lg4ff_wheel_data {
 };
 
 struct lg4ff_device_entry {
+	spinlock_t report_lock; /* Protect output HID report */
 	struct lg4ff_wheel_data wdata;
 };
 

@@ -303,9 +304,24 @@ 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;
+	unsigned long flags;
 	s32 *value = report->field[0]->value;
 	int x;
 
+	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)
 
 	switch (effect->type) {

@@ -313,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;

@@ -324,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;
 		}
 

@@ -336,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;

@@ -352,6 +371,7 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 	u32 expand_a, expand_b;
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
+	unsigned long flags;
 
 	drv_data = hid_get_drvdata(hid);
 	if (!drv_data) {

@@ -366,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;

@@ -376,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;
 	}
 

@@ -417,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 */

@@ -425,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;

@@ -437,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 */

@@ -444,10 +484,26 @@ 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;
+	unsigned long flags;
 	s32 *value = report->field[0]->value;
 
+	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;

@@ -457,6 +513,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 */

@@ -465,11 +522,27 @@ 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);
 	int start_left, start_right, full_range;
+	struct lg4ff_device_entry *entry;
+	struct lg_drv_data *drv_data;
+	unsigned long flags;
 	s32 *value = report->field[0]->value;
 
+	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);
 
 	/* Prepare "coarse" limit command */
+	spin_lock_irqsave(&entry->report_lock, flags);
 	value[0] = 0xf8;
 	value[1] = 0x00;	/* Set later */
 	value[2] = 0x00;

@@ -498,6 +571,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;
 	}
 

@@ -512,6 +586,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)

@@ -575,9 +650,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;
+	unsigned long flags;
 	s32 *value = report->field[0]->value;
 	u8 i;
 
+	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;
 

@@ -586,6 +677,7 @@ 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;
 }

@@ -826,8 +918,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;
+	struct lg4ff_device_entry *entry;
+	unsigned long flags;
 	s32 *value = report->field[0]->value;
 
+	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;

@@ -836,6 +944,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,

@@ -1015,6 +1124,7 @@ int lg4ff_init(struct hid_device *hid)
 	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
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#11 lore raw

PATCH v2 11/15 HID: hid-lg4ff: Store pointer to the output HID report struct in the device entry struct.

In reply to Michal Malý

Store pointer to the output HID report struct in the device entry struct. This eliminates the need to look the HID report struct up every time it is needed.

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 67 +++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 35 deletions(-)
drivers/hid/hid-lg4ff.c +32 -35
index f7f4eb6..d27b210 100644
@@ -90,6 +90,7 @@ struct lg4ff_wheel_data {
 
 struct lg4ff_device_entry {
 	spinlock_t report_lock; /* Protect output HID report */
+	struct hid_report *report;
 	struct lg4ff_wheel_data wdata;
 };
 

@@ -302,12 +303,10 @@ 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;
 	unsigned long flags;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 	int x;
 
 	drv_data = hid_get_drvdata(hid);

@@ -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;
 	}

@@ -384,6 +384,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 +397,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 +428,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 +439,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 +447,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 +464,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,19 +475,17 @@ 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;
 	unsigned long flags;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 
 	drv_data = hid_get_drvdata(hid);
 	if (!drv_data) {

@@ -500,7 +498,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);

@@ -512,20 +510,18 @@ 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);
-	int start_left, start_right, full_range;
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
 	unsigned long flags;
-	s32 *value = report->field[0]->value;
+	int start_left, start_right, full_range;
+	s32 *value;
 
 	drv_data = hid_get_drvdata(hid);
 	if (!drv_data) {

@@ -538,7 +534,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);
 
 	/* Prepare "coarse" limit command */

@@ -552,13 +548,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;

@@ -570,7 +566,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;
 	}

@@ -585,7 +581,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);
 }
 

@@ -648,12 +644,10 @@ 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;
 	unsigned long flags;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 	u8 i;
 
 	drv_data = hid_get_drvdata(hid);

@@ -667,6 +661,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++) {

@@ -675,7 +670,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);
 	hid_hw_wait(hid);

@@ -916,12 +911,10 @@ 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;
 	struct lg4ff_device_entry *entry;
 	unsigned long flags;
-	s32 *value = report->field[0]->value;
+	s32 *value;
 
 	drv_data = hid_get_drvdata(hid);
 	if (!drv_data) {

@@ -934,6 +927,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;

@@ -943,7 +937,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);
 }
 

@@ -1104,6 +1098,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;

@@ -1125,6 +1121,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
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#12 lore raw

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

In reply to Michal Malý

Constify those members of lg4ff_device_entry struct whose value is not supposed to change.

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 54 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 15 deletions(-)
drivers/hid/hid-lg4ff.c +39 -15
index d27b210..dcaf481 100644
@@ -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);

@@ -1102,6 +1130,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;

@@ -1177,17 +1206,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 */
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#13 lore raw

PATCH v2 13/15 HID: hid-lg4ff: Allow the driver to continue without sysfs interface.

In reply to Michal Malý

Allow the driver to continue without sysfs interface. Instead of bailing out allow the driver to continue in a degraded mode.

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg4ff.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/hid/hid-lg4ff.c +3 -4
index dcaf481..02cec83 100644
@@ -1229,15 +1229,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)
-			goto err_init;
-
+			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)
-			goto err_init;
+			hid_warn(hid, "Unable to create sysfs interface for \"alternate_modes\", errno %d\n", error);
 	}
 	dbg_hid("sysfs interface created\n");
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#14 lore raw

PATCH v2 14/15 HID: hid-lg4ff: Update respective sysfs interface documentation

In reply to Michal Malý

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

Signed-off-by Michal Malý <[email protected]>
---
 Documentation/ABI/testing/sysfs-driver-hid-logitech-lg4ff | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Documentation/ABI/testing/sysfs-driver-hid-logitech-lg4ff +4 -4
index b3f6a2a..db197a8 100644
@@ -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ý <[email protected]>
+Contact:	Michal Malý <[email protected]>
 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ý <[email protected]>
+Contact:	Michal Malý <[email protected]>
 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ý <[email protected]>
+Contact:	Michal Malý <[email protected]>
 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.
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#15 lore raw

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

In reply to Michal Malý

Only one of LG_FF flags can be set for a given device.

Signed-off-by Michal Malý <[email protected]>
---
 drivers/hid/hid-lg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/hid/hid-lg.c +3 -3
index c3981da..f1e92bf 100644
@@ -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)
Signature · 6 lines
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#16 lore raw
In reply to Michal Malý
Quoted 3 lines · Michal Malý

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.

Hi Michal,

thanks for the respin. It's on my radar, but due to its nature and due to the merge window coming, I'll treat it as 4.2 material (IOW, please don't expect my response to this before 4.1-rc1 is released).

Thanks,

Signature · 6 lines
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#17 lore raw
In reply to Michal Malý
Quoted 12 lines · 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 patches 8 through 10 which introduce 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. Patches 8 and 9 contain the necessary prep work, patch 10 adds the spinlocking.

Quoted 4 lines

v2:
- Split previous patch 7 into three smaller patches
- Split the long line of text in patch 5 (now patch 6)
- Add another cleanup patch that removes unnecessary casts from void *

This is a nice cleanup. I've applied it to for-4.2/logitech.

Thanks,

Signature · 6 lines
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html