[PATCH] platform/x86: toshiba_acpi: use brightness_set_blocking for LED callbacks

[PATCH] platform/x86: toshiba_acpi: use brightness_set_blocking for LED callbacks

[Runyu Xiao]

The Toshiba illumination, eco mode, and keyboard backlight callbacks all
go through ACPI/HCI/SCI helpers that may sleep, but the driver still
registers them as brightness_set callbacks.

This issue was found by our static analysis tool and then manually
reviewed against the current tree.

A minimal Lockdep reproducer that keeps the original registration and
call chains is enough to trigger the warning in all three cases:

  toshiba_illumination_set() -> sci_open()/sci_write() -> tci_raw()
  toshiba_kbd_backlight_set() -> hci_write() -> tci_raw()
  toshiba_eco_mode_set_status() -> tci_raw()

All three paths reach ACPI object evaluation while
led_trigger_event_atomic() is still holding spin_lock_irqsave(),
and Lockdep reports sleeping function called from invalid context with
acpi_os_wait_semaphore() on the stack.

Convert the three callbacks to brightness_set_blocking and return proper
status codes from the firmware transactions.

Fixes: 360f0f39d0c5 ("toshiba_acpi: Add keyboard backlight support")
Fixes: 6c3f6e6c575a ("toshiba-acpi: Add support for Toshiba Illumination.")
Fixes: def6c4e25d31 ("toshiba_acpi: Add ECO mode led support")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
Notes:
  - Not tested on Toshiba hardware.

 drivers/platform/x86/toshiba_acpi.c | 42 ++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 5ad3a7183d33..22a831c9e8c4 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -486,8 +486,8 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev)
 	dev->illumination_supported = 1;
 }
 
-static void toshiba_illumination_set(struct led_classdev *cdev,
-				     enum led_brightness brightness)
+static int toshiba_illumination_set(struct led_classdev *cdev,
+				    enum led_brightness brightness)
 {
 	struct toshiba_acpi_dev *dev = container_of(cdev,
 			struct toshiba_acpi_dev, led_dev);
@@ -496,14 +496,20 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
 
 	/* First request : initialize communication. */
 	if (!sci_open(dev))
-		return;
+		return -EIO;
 
 	/* Switch the illumination on/off */
 	state = brightness ? 1 : 0;
 	result = sci_write(dev, SCI_ILLUMINATION, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE)
+	if (result == TOS_FAILURE) {
 		pr_err("ACPI call for illumination failed\n");
+		return -EIO;
+	}
+	if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+
+	return result == TOS_SUCCESS ? 0 : -EIO;
 }
 
 static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
@@ -624,7 +630,7 @@ static enum led_brightness toshiba_kbd_backlight_get(struct led_classdev *cdev)
 	return state ? LED_FULL : LED_OFF;
 }
 
-static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
+static int toshiba_kbd_backlight_set(struct led_classdev *cdev,
 				     enum led_brightness brightness)
 {
 	struct toshiba_acpi_dev *dev = container_of(cdev,
@@ -635,8 +641,14 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
 	/* Set the keyboard backlight state */
 	state = brightness ? 1 : 0;
 	result = hci_write(dev, HCI_KBD_ILLUMINATION, state);
-	if (result == TOS_FAILURE)
+	if (result == TOS_FAILURE) {
 		pr_err("ACPI call to set KBD Illumination mode failed\n");
+		return -EIO;
+	}
+	if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+
+	return result == TOS_SUCCESS ? 0 : -EIO;
 }
 
 /* TouchPad support */
@@ -737,8 +749,8 @@ toshiba_eco_mode_get_status(struct led_classdev *cdev)
 	return out[2] ? LED_FULL : LED_OFF;
 }
 
-static void toshiba_eco_mode_set_status(struct led_classdev *cdev,
-				     enum led_brightness brightness)
+static int toshiba_eco_mode_set_status(struct led_classdev *cdev,
+				       enum led_brightness brightness)
 {
 	struct toshiba_acpi_dev *dev = container_of(cdev,
 			struct toshiba_acpi_dev, eco_led);
@@ -749,8 +761,14 @@ static void toshiba_eco_mode_set_status(struct led_classdev *cdev,
 	/* Switch the Eco Mode led on/off */
 	in[2] = (brightness) ? 1 : 0;
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status))
+	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to set ECO led failed\n");
+		return -EIO;
+	}
+	if (out[0] == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+
+	return out[0] == TOS_SUCCESS ? 0 : -EIO;
 }
 
 /* Accelerometer support */
@@ -3366,7 +3384,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 	if (dev->illumination_supported) {
 		dev->led_dev.name = "toshiba::illumination";
 		dev->led_dev.max_brightness = 1;
-		dev->led_dev.brightness_set = toshiba_illumination_set;
+		dev->led_dev.brightness_set_blocking = toshiba_illumination_set;
 		dev->led_dev.brightness_get = toshiba_illumination_get;
 		led_classdev_register(&acpi_dev->dev, &dev->led_dev);
 	}
@@ -3375,7 +3393,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 	if (dev->eco_supported) {
 		dev->eco_led.name = "toshiba::eco_mode";
 		dev->eco_led.max_brightness = 1;
-		dev->eco_led.brightness_set = toshiba_eco_mode_set_status;
+		dev->eco_led.brightness_set_blocking = toshiba_eco_mode_set_status;
 		dev->eco_led.brightness_get = toshiba_eco_mode_get_status;
 		led_classdev_register(&dev->acpi_dev->dev, &dev->eco_led);
 	}
@@ -3391,7 +3409,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 		dev->kbd_led.name = "toshiba::kbd_backlight";
 		dev->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
 		dev->kbd_led.max_brightness = 1;
-		dev->kbd_led.brightness_set = toshiba_kbd_backlight_set;
+		dev->kbd_led.brightness_set_blocking = toshiba_kbd_backlight_set;
 		dev->kbd_led.brightness_get = toshiba_kbd_backlight_get;
 		led_classdev_register(&dev->acpi_dev->dev, &dev->kbd_led);
 	}
-- 
2.34.1

Re: [PATCH] platform/x86: toshiba_acpi: use brightness_set_blocking for LED callbacks

[Ilpo Järvinen]

On Thu, 18 Jun 2026, Runyu Xiao wrote:

> The Toshiba illumination, eco mode, and keyboard backlight callbacks all
> go through ACPI/HCI/SCI helpers that may sleep, but the driver still
> registers them as brightness_set callbacks.
> 
> This issue was found by our static analysis tool and then manually
> reviewed against the current tree.
> 
> A minimal Lockdep reproducer that keeps the original registration and
> call chains is enough to trigger the warning in all three cases:
> 
>   toshiba_illumination_set() -> sci_open()/sci_write() -> tci_raw()
>   toshiba_kbd_backlight_set() -> hci_write() -> tci_raw()
>   toshiba_eco_mode_set_status() -> tci_raw()
> 
> All three paths reach ACPI object evaluation while
> led_trigger_event_atomic() is still holding spin_lock_irqsave(),
> and Lockdep reports sleeping function called from invalid context with
> acpi_os_wait_semaphore() on the stack.
> 
> Convert the three callbacks to brightness_set_blocking and return proper
> status codes from the firmware transactions.
> 
> Fixes: 360f0f39d0c5 ("toshiba_acpi: Add keyboard backlight support")
> Fixes: 6c3f6e6c575a ("toshiba-acpi: Add support for Toshiba Illumination.")
> Fixes: def6c4e25d31 ("toshiba_acpi: Add ECO mode led support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> Notes:
>   - Not tested on Toshiba hardware.
> 
>  drivers/platform/x86/toshiba_acpi.c | 42 ++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 5ad3a7183d33..22a831c9e8c4 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -486,8 +486,8 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev)
>  	dev->illumination_supported = 1;
>  }
>  
> -static void toshiba_illumination_set(struct led_classdev *cdev,
> -				     enum led_brightness brightness)
> +static int toshiba_illumination_set(struct led_classdev *cdev,
> +				    enum led_brightness brightness)
>  {
>  	struct toshiba_acpi_dev *dev = container_of(cdev,
>  			struct toshiba_acpi_dev, led_dev);
> @@ -496,14 +496,20 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
>  
>  	/* First request : initialize communication. */
>  	if (!sci_open(dev))
> -		return;
> +		return -EIO;
>  
>  	/* Switch the illumination on/off */
>  	state = brightness ? 1 : 0;
>  	result = sci_write(dev, SCI_ILLUMINATION, state);
>  	sci_close(dev);
> -	if (result == TOS_FAILURE)
> +	if (result == TOS_FAILURE) {
>  		pr_err("ACPI call for illumination failed\n");
> +		return -EIO;
> +	}
> +	if (result == TOS_NOT_SUPPORTED)
> +		return -ENODEV;

So this has not been observed on some HW after 
toshiba_illumination_available() finds it supported. Adding the check
looks just extra churn to me and IMO it would be fine to return -EIO if 
it suddenly stops working.

> +
> +	return result == TOS_SUCCESS ? 0 : -EIO;
>  }
>  
>  static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
> @@ -624,7 +630,7 @@ static enum led_brightness toshiba_kbd_backlight_get(struct led_classdev *cdev)
>  	return state ? LED_FULL : LED_OFF;
>  }
>  
> -static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
> +static int toshiba_kbd_backlight_set(struct led_classdev *cdev,
>  				     enum led_brightness brightness)
>  {
>  	struct toshiba_acpi_dev *dev = container_of(cdev,
> @@ -635,8 +641,14 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
>  	/* Set the keyboard backlight state */
>  	state = brightness ? 1 : 0;
>  	result = hci_write(dev, HCI_KBD_ILLUMINATION, state);
> -	if (result == TOS_FAILURE)
> +	if (result == TOS_FAILURE) {
>  		pr_err("ACPI call to set KBD Illumination mode failed\n");
> +		return -EIO;
> +	}
> +	if (result == TOS_NOT_SUPPORTED)
> +		return -ENODEV;
> +
> +	return result == TOS_SUCCESS ? 0 : -EIO;
>  }
>  
>  /* TouchPad support */
> @@ -737,8 +749,8 @@ toshiba_eco_mode_get_status(struct led_classdev *cdev)
>  	return out[2] ? LED_FULL : LED_OFF;
>  }
>  
> -static void toshiba_eco_mode_set_status(struct led_classdev *cdev,
> -				     enum led_brightness brightness)
> +static int toshiba_eco_mode_set_status(struct led_classdev *cdev,
> +				       enum led_brightness brightness)
>  {
>  	struct toshiba_acpi_dev *dev = container_of(cdev,
>  			struct toshiba_acpi_dev, eco_led);
> @@ -749,8 +761,14 @@ static void toshiba_eco_mode_set_status(struct led_classdev *cdev,
>  	/* Switch the Eco Mode led on/off */
>  	in[2] = (brightness) ? 1 : 0;
>  	status = tci_raw(dev, in, out);
> -	if (ACPI_FAILURE(status))
> +	if (ACPI_FAILURE(status)) {
>  		pr_err("ACPI call to set ECO led failed\n");
> +		return -EIO;
> +	}
> +	if (out[0] == TOS_NOT_SUPPORTED)
> +		return -ENODEV;

Same note here.

I suggest you just do the callback changes and leave these error code 
plays out of the patch unless some hw actually needs them.

> +
> +	return out[0] == TOS_SUCCESS ? 0 : -EIO;
>  }
>  
>  /* Accelerometer support */
> @@ -3366,7 +3384,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>  	if (dev->illumination_supported) {
>  		dev->led_dev.name = "toshiba::illumination";
>  		dev->led_dev.max_brightness = 1;
> -		dev->led_dev.brightness_set = toshiba_illumination_set;
> +		dev->led_dev.brightness_set_blocking = toshiba_illumination_set;
>  		dev->led_dev.brightness_get = toshiba_illumination_get;
>  		led_classdev_register(&acpi_dev->dev, &dev->led_dev);
>  	}
> @@ -3375,7 +3393,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>  	if (dev->eco_supported) {
>  		dev->eco_led.name = "toshiba::eco_mode";
>  		dev->eco_led.max_brightness = 1;
> -		dev->eco_led.brightness_set = toshiba_eco_mode_set_status;
> +		dev->eco_led.brightness_set_blocking = toshiba_eco_mode_set_status;
>  		dev->eco_led.brightness_get = toshiba_eco_mode_get_status;
>  		led_classdev_register(&dev->acpi_dev->dev, &dev->eco_led);
>  	}
> @@ -3391,7 +3409,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>  		dev->kbd_led.name = "toshiba::kbd_backlight";
>  		dev->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
>  		dev->kbd_led.max_brightness = 1;
> -		dev->kbd_led.brightness_set = toshiba_kbd_backlight_set;
> +		dev->kbd_led.brightness_set_blocking = toshiba_kbd_backlight_set;
>  		dev->kbd_led.brightness_get = toshiba_kbd_backlight_get;
>  		led_classdev_register(&dev->acpi_dev->dev, &dev->kbd_led);
>  	}
> 

-- 
 i.