[PATCH v2 21/33] HID: logitech-hidpp: execute G920 range sysfs store synchronously

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Writing to the G920 range sysfs attribute currently queues an
asynchronous work item to issue the HIDPP_FF_SET_APERTURE command on
the force-feedback workqueue. Because the sysfs store returns success
immediately without waiting for the hardware transfer to complete, user
space cannot know whether the setting succeeded and a subsequent read of
the range attribute may return stale data before the workqueue drains.

Execute the aperture setting command synchronously within the range
sysfs store method. Open and close the underlying transport around the
command, clamp the input range between 180 and 900 degrees, update the
cached range on success, and return errors directly to user space. Read
and write the cached range with READ_ONCE and WRITE_ONCE to prevent
torn reads.

Because the sysfs store now sends the command directly using the HID++
device handle, the backpointer to the force-feedback private data is no
longer needed in struct g920_data. Drop the unused pointer and simplify
the attribute visibility check.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/hid/hid-logitech-hidpp.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index f24fa70d272e..f7fefff0aed4 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2348,7 +2348,6 @@ struct hidpp_ff_private_data {
 };
 
 struct g920_data {
-	struct hidpp_ff_private_data *ff;
 	u8 feature_index;
 	u8 slot_autocenter;
 	u8 num_effects;
@@ -2473,9 +2472,6 @@ static void hidpp_ff_work_handler(struct work_struct *w)
 	case HIDPP_FF_SET_GLOBAL_GAINS:
 		g920->gain = (wd->params[0] << 8) + wd->params[1];
 		break;
-	case HIDPP_FF_SET_APERTURE:
-		g920->range = (wd->params[0] << 8) + wd->params[1];
-		break;
 	default:
 		/* no action needed */
 		break;
@@ -2724,7 +2720,7 @@ static ssize_t hidpp_ff_range_show(struct device *dev, struct device_attribute *
 	struct hidpp_device *hidpp = hid_get_drvdata(hid);
 	struct g920_data *g920 = hidpp->private_data;
 
-	return sysfs_emit(buf, "%u\n", g920->range);
+	return sysfs_emit(buf, "%u\n", READ_ONCE(g920->range));
 }
 
 static ssize_t hidpp_ff_range_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
@@ -2732,15 +2728,34 @@ static ssize_t hidpp_ff_range_store(struct device *dev, struct device_attribute
 	struct hid_device *hid = to_hid_device(dev);
 	struct hidpp_device *hidpp = hid_get_drvdata(hid);
 	struct g920_data *g920 = hidpp->private_data;
+	struct hidpp_report response;
 	u8 params[2];
-	int range = simple_strtoul(buf, NULL, 10);
+	u16 range;
+	int error;
 
-	range = clamp(range, 180, 900);
+	error = kstrtou16(buf, 10, &range);
+	if (error)
+		return error;
+
+	range = clamp_val(range, 180, 900);
 
 	params[0] = range >> 8;
 	params[1] = range & 0x00FF;
 
-	hidpp_ff_queue_work(g920->ff, -1, HIDPP_FF_SET_APERTURE, params, ARRAY_SIZE(params));
+	error = hid_hw_open(hid);
+	if (error)
+		return error;
+
+	error = hidpp_send_fap_command_sync(hidpp, g920->feature_index,
+					    HIDPP_FF_SET_APERTURE,
+					    params, ARRAY_SIZE(params),
+					    &response);
+	hid_hw_close(hid);
+
+	if (error)
+		return error;
+
+	WRITE_ONCE(g920->range, range);
 
 	return count;
 }
@@ -2764,7 +2779,7 @@ static umode_t g920_range_attr_is_visible(struct kobject *kobj,
 		return 0;
 
 	g920 = hidpp->private_data;
-	if (!g920 || !g920->ff)
+	if (!g920)
 		return 0;
 
 	return attr->mode;
@@ -2784,12 +2799,10 @@ static void hidpp_ff_destroy(struct ff_device *ff)
 {
 	struct hidpp_ff_private_data *data = ff->private;
 	struct hidpp_device *hidpp = data->hidpp;
-	struct g920_data *g920 = hidpp->private_data;
 
 	hid_info(hidpp->hid_dev, "Unloading HID++ force feedback.\n");
 
 	destroy_workqueue(data->wq);
-	g920->ff = NULL;
 }
 
 static int g920_input_open(struct input_dev *dev)
@@ -2861,7 +2874,6 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, struct input_dev *dev)
 	}
 
 	ff = dev->ff;
-	g920->ff = data;
 	ff->private = no_free_ptr(data);
 
 	ff->upload = hidpp_ff_upload_effect;

-- 
2.56.0.542.g3ef89b7020-goog