[PATCH v2] Input: applespi - turn the keyboard backlight off across suspend

[Michal Szpakowski <michi.szpakowski@gmail.com>]

The keyboard backlight of the SPI topcase stays lit through s2idle: the
driver's suspend handler only switches the caps-lock LED off, and the
backlight LED is registered without LED_CORE_SUSPENDRESUME, so the LED
core never gets to blank it. Under deep (S3) the rails go down anyway,
which is why it only shows on machines that default to s2idle.

Set the flag, and deal with the two things that follow from the LED
core now sending a command of its own right before applespi_suspend():

- The LED is a child of the SPI device, so its "off" command is queued
  just before applespi_suspend() runs. The driver keeps one command in
  flight, so the caps-lock command was skipped while that one was still
  out, and then dropped by the drain. Wait for the write to finish first;
  the completion now wakes the drain queue unconditionally so that this
  wait has something to wake it.

- The level the LED core blanks to would be what poweroff_late and
  shutdown save to EFI for the next boot. Keep the last level the user
  asked for in its own field, updated only while the LED is not
  suspended, and save that instead.

Tested on a MacBookPro13,1 (2016): 30 s of s2idle with the backlight at
200/255 leaves it lit on the stock kernel; with this patch it goes dark
at suspend and is back at 200 on wake, and a caps-lock LED that was on
at suspend is off during the sleep and on again afterwards. A
hibernation with the backlight at 200 leaves the level for 200 in the
EFI variable instead of the blanked minimum.

Fixes: 038b1a05eae6 ("Input: add Apple SPI keyboard and trackpad driver")
Cc: stable@vger.kernel.org
Signed-off-by: Michal Szpakowski <michi.szpakowski@gmail.com>
---
v2: address the two points from the automated review on v1 - the
    caps-lock command racing the LED core's backlight command at
    suspend, and the blanked level being saved to EFI at power-off.

 drivers/input/keyboard/applespi.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -407,6 +407,7 @@
 	bool				have_cl_led_on;
 	unsigned int			want_bl_level;
 	unsigned int			have_bl_level;
+	unsigned int			bl_level_persist;
 	unsigned int			cmd_msg_cntr;
 	/* lock to protect the above parameters and flags below */
 	spinlock_t			cmd_msg_lock;
@@ -724,7 +725,7 @@
 	if (is_write_msg)
 		applespi->write_active = false;
 
-	if (applespi->drain && !applespi->write_active)
+	if (!applespi->write_active)
 		wake_up_all(&applespi->drain_complete);
 
 	if (is_write_msg) {
@@ -923,6 +924,14 @@
 			 KBD_BL_LEVEL_MIN);
 	}
 
+	/*
+	 * The LED core blanks the backlight around suspend; that level is
+	 * not what the user wants restored after a power-off, so keep the
+	 * last one they asked for separately.
+	 */
+	if (!(led_cdev->flags & LED_SUSPENDED))
+		applespi->bl_level_persist = applespi->want_bl_level;
+
 	applespi_send_cmd_msg(applespi);
 }
 
@@ -1772,6 +1781,7 @@
 	applespi->backlight_info.name            = "spi::kbd_backlight";
 	applespi->backlight_info.default_trigger = "kbd-backlight";
 	applespi->backlight_info.brightness_set  = applespi_set_bl_level;
+	applespi->backlight_info.flags           = LED_CORE_SUSPENDRESUME;
 
 	sts = devm_led_classdev_register(&spi->dev, &applespi->backlight_info);
 	if (sts)
@@ -1791,6 +1801,14 @@
 	return 0;
 }
 
+static void applespi_wait_for_write(struct applespi_data *applespi)
+{
+	guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
+
+	wait_event_lock_irq(applespi->drain_complete, !applespi->write_active,
+			    applespi->cmd_msg_lock);
+}
+
 static void applespi_drain_writes(struct applespi_data *applespi)
 {
 	guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
@@ -1829,7 +1847,7 @@
 {
 	struct applespi_data *applespi = spi_get_drvdata(spi);
 
-	applespi_save_bl_level(applespi, applespi->have_bl_level);
+	applespi_save_bl_level(applespi, applespi->bl_level_persist);
 }
 
 static int applespi_poweroff_late(struct device *dev)
@@ -1837,7 +1855,7 @@
 	struct spi_device *spi = to_spi_device(dev);
 	struct applespi_data *applespi = spi_get_drvdata(spi);
 
-	applespi_save_bl_level(applespi, applespi->have_bl_level);
+	applespi_save_bl_level(applespi, applespi->bl_level_persist);
 
 	return 0;
 }
@@ -1849,6 +1867,14 @@
 	acpi_status acpi_sts;
 	int sts;
 
+	/*
+	 * The backlight LED is a child of this device, so the LED core has
+	 * just sent its "off" command; wait for it, or the caps-lock command
+	 * is skipped (one command in flight at a time) and then dropped by the
+	 * drain below.
+	 */
+	applespi_wait_for_write(applespi);
+
 	/* turn off caps-lock - it'll stay on otherwise */
 	sts = applespi_set_capsl_led(applespi, false);
 	if (sts)