[PATCH 3/4] HID: lamparray: transfer control when use_leds_uapi changes

[Cristian Mazzotta]

Autonomous mode is disabled once in lamparray_register() and never
changed again. Writing 0 to use_leds_uapi unregisters the LED class
device but leaves the device under host control, and writing 1
re-registers the class device without taking the device back, so lamp
updates sent afterwards are ignored.

On an Acer Predator PT14-52T (USB keyboard 05AF:767A) the device stays
with whatever last drove it: after writing 0, running a userspace tool
that sets a hardware effect, then writing 1, the LED class device
reports the cached values despite the hardware still running an effect.
Reading AutonomousMode back returns the last value written by the host
rather than the state the device is actually in, so the driver cannot
detect this.

Disable autonomous mode before registering the LED class device and
re-enable it after unregistering, so control is handed over in both
directions. Unregister the class device before handing the hardware
back, so userspace cannot write lamp updates to a device that is no
longer accepting them.

Failure to re-enable autonomous mode on the disable path is logged but
not propagated since the class device is already gone and the write
has otherwise succeeded. The restore failure path drops back to
hid_warn() for the same reason: it is recoverable, and the attribute
reverts to its previous value.

Signed-off-by: Cristian Mazzotta <cmmazzo@xxxxxxxxxx>
---
 drivers/hid/hid-lamparray.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-lamparray.c b/drivers/hid/hid-lamparray.c
index 70c596f292a2..bbde006d119b 100644
--- a/drivers/hid/hid-lamparray.c
+++ b/drivers/hid/hid-lamparray.c
@@ -659,25 +659,43 @@ static ssize_t use_leds_uapi_store(struct device *dev,
 	ldev->use_leds_uapi = val;
 	mutex_unlock(&ldev->dev_lock);
 
+	/*
+	 * Take the device out of autonomous mode before exposing the LED
+	 * class device, and hand it back afterwards, so that control is
+	 * transferred in both directions rather than only at probe.
+	 */
 	if (val == 1) {
+		ret = lamparray_hw_set_autonomous(ldev, false);
+		if (ret)
+			goto err_revert;
 		ret = lamparray_register_led(ldev);
 		if (ret) {
-			mutex_lock(&ldev->dev_lock);
-			ldev->use_leds_uapi = old_val;
-			mutex_unlock(&ldev->dev_lock);
-			return ret;
+			lamparray_hw_set_autonomous(ldev, true);
+			goto err_revert;
 		}
 		ret = lamparray_restore_state(ldev);
 		if (ret) {
-			hid_err(ldev->hdev, "Could not restore state: %d\n", ret);
-			return ret;
+			hid_warn(ldev->hdev, "Could not restore state: %d\n", ret);
+			lamparray_unregister_led(ldev);
+			lamparray_hw_set_autonomous(ldev, true);
+			goto err_revert;
 		}
-
 	} else {
 		lamparray_unregister_led(ldev);
+		ret = lamparray_hw_set_autonomous(ldev, true);
+		if (ret) {
+			hid_warn(ldev->hdev, "Could not enable autonomous mode: %d\n", ret);
+			return count;
+		}
 	}
 
 	return count;
+
+err_revert:
+	mutex_lock(&ldev->dev_lock);
+	ldev->use_leds_uapi = old_val;
+	mutex_unlock(&ldev->dev_lock);
+	return ret;
 }
 static DEVICE_ATTR_RW(use_leds_uapi);
 
-- 
2.55.0