[PATCH stable v4.4+ 1/3] leds: call led_pwm_set() in leds-pwm to enforce default LED_OFF

[PATCH stable v4.4+ 1/3] leds: call led_pwm_set() in leds-pwm to enforce default LED_OFF

[Krzysztof Kozlowski]

From: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>

commit f16703360da7731a057df2ffa902306819c22398 upstream.

Some PWMs are disabled by default or the default pin setting
does not match the LED_OFF state (e.g., active-low leds).
Hence, the driver may end up reporting 0 brightness, but
the leds are actually on using full brightness, because
it never enforces its default configuration.
So enforce it by calling led_pwm_set() after successfully
registering the device.

Tested on a Phytec phyFLEX i.MX6Q board based on kernel
v3.19.5.

Signed-off-by: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>
Tested-by: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/leds/leds-pwm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 1d07e3e83d29..3149dbece146 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -132,6 +132,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
 	ret = led_classdev_register(dev, &led_data->cdev);
 	if (ret == 0) {
 		priv->num_leds++;
+		led_pwm_set(&led_data->cdev, led_data->cdev.brightness);
 	} else {
 		dev_err(dev, "failed to register PWM led for %s: %d\n",
 			led->name, ret);
-- 
2.7.4

[PATCH stable v4.4+ 2/3] leds: turn off the LED and wait for completion on unregistering LED class device

[Krzysztof Kozlowski]

From: Milo Kim <milo.kim@ti.com>

commit d1aa577f5e191d77d3ad62da93729b5af9532bb4 upstream.

Workqueue, 'set_brightness_work' is used for scheduling brightness control.
This workqueue is canceled when the LED class device is unregistered.
Currently, LED subsystem handles like below.

  cancel_work_sync(&led_cdev->set_brightness_work)
  led_set_brightness(led_cdev, LED_OFF)

However, this could be a problem.
Workqueue is going to be canceled but LED device needs to be off.
The worst case is null pointer access due to scheduling a workqueue.

LED module is loaded.
  LED driver private data is allocated by using devm_zalloc().

LED module is unloaded.
  led_classdev_unregister() is called.
    cancel_work_sync()
      led_set_brightness(led_cdev, LED_OFF)
        schedule_work() if LED driver uses brightness_set_blocking()
        In the meantime, driver private data will be freed.

        ..scheduling..

        brightness_set_blocking() callback is invoked.
          For the brightness control, LED driver tries to access private
          data but resource is removed!

To avoid this problem, LED subsystem should turn off the brightness first
and wait for completion.

  led_set_brightness(led_cdev, LED_OFF)
  flush_work(&led_cdev->set_brightness_work)

It guarantees that LED driver turns off the brightness prior to
resource management.

Cc: linux-leds@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Milo Kim <milo.kim@ti.com>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/leds/led-class.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 7385f98dd54b..51a5b51ec467 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -247,12 +247,13 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
 	up_write(&led_cdev->trigger_lock);
 #endif
 
-	cancel_work_sync(&led_cdev->set_brightness_work);
-
 	/* Stop blinking */
 	led_stop_software_blink(led_cdev);
+
 	led_set_brightness(led_cdev, LED_OFF);
 
+	flush_work(&led_cdev->set_brightness_work);
+
 	device_unregister(led_cdev->dev);
 
 	down_write(&leds_list_lock);
-- 
2.7.4

[PATCH stable v4.4+ 3/3] leds: leds-gpio: Fix return value check in create_gpio_led()

[Krzysztof Kozlowski]

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

commit 2d88a331e48095cf60ad9bdf3177bd401bf99727 upstream.

In case of error, the function gpio_to_desc() returns NULL
pointer not ERR_PTR(). The IS_ERR() test in the return value
check should be replaced with NULL test.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/leds/leds-gpio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 5db4515a4fd7..df186b38da78 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -118,8 +118,8 @@ static int create_gpio_led(const struct gpio_led *template,
 			return ret;
 
 		led_dat->gpiod = gpio_to_desc(template->gpio);
-		if (IS_ERR(led_dat->gpiod))
-			return PTR_ERR(led_dat->gpiod);
+		if (!led_dat->gpiod)
+			return -EINVAL;
 	}
 
 	led_dat->cdev.name = template->name;
-- 
2.7.4

Re: [PATCH stable v4.4+ 3/3] leds: leds-gpio: Fix return value check in create_gpio_led()

[Greg KH]

On Thu, Dec 06, 2018 at 10:47:02AM +0100, Krzysztof Kozlowski wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> commit 2d88a331e48095cf60ad9bdf3177bd401bf99727 upstream.
> 
> In case of error, the function gpio_to_desc() returns NULL
> pointer not ERR_PTR(). The IS_ERR() test in the return value
> check should be replaced with NULL test.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  drivers/leds/leds-gpio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

All 3 now queued up, thanks.

greg k-h
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index 5db4515a4fd7..df186b38da78 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -118,8 +118,8 @@ static int create_gpio_led(const struct gpio_led *template,
>  			return ret;
>  
>  		led_dat->gpiod = gpio_to_desc(template->gpio);
> -		if (IS_ERR(led_dat->gpiod))
> -			return PTR_ERR(led_dat->gpiod);
> +		if (!led_dat->gpiod)
> +			return -EINVAL;
>  	}
>  
>  	led_dat->cdev.name = template->name;
> -- 
> 2.7.4
>