All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] backlight: led_bl: fix initial power state
@ 2023-07-04 16:19 ` Mans Rullgard
  0 siblings, 0 replies; 4+ messages in thread
From: Mans Rullgard @ 2023-07-04 16:19 UTC (permalink / raw)
  To: Lee Jones, Daniel Thompson, Jingoo Han
  Cc: Helge Deller, dri-devel, linux-fbdev, linux-kernel

The condition for the initial power state based on the default
brightness value is reversed.  Fix it.

Furthermore, use the actual state of the LEDs rather than the default
brightness specified in the devicetree as the latter should not cause
the backlight to be automatically turned on.

If the backlight device is not linked to any display, set the initial
power to on unconditionally.

Fixes: ae232e45acf9 ("backlight: add led-backlight driver")
Signed-off-by: Mans Rullgard <mans@mansr.com>
---
Changes in v2:
- Use the reported LED state to set initial power state
- Always power on if no phandle in DT
---
 drivers/video/backlight/led_bl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index 3259292fda76..bbf1673b1fb0 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -176,6 +176,7 @@ static int led_bl_probe(struct platform_device *pdev)
 {
 	struct backlight_properties props;
 	struct led_bl_data *priv;
+	int init_brightness;
 	int ret, i;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -190,6 +191,8 @@ static int led_bl_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	init_brightness = priv->default_brightness;
+
 	ret = led_bl_parse_levels(&pdev->dev, priv);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to parse DT data\n");
@@ -200,8 +203,8 @@ static int led_bl_probe(struct platform_device *pdev)
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = priv->max_brightness;
 	props.brightness = priv->default_brightness;
-	props.power = (priv->default_brightness > 0) ? FB_BLANK_POWERDOWN :
-		      FB_BLANK_UNBLANK;
+	props.power = (init_brightness || !pdev->dev.of_node->phandle) ?
+		FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
 	priv->bl_dev = backlight_device_register(dev_name(&pdev->dev),
 			&pdev->dev, priv, &led_bl_ops, &props);
 	if (IS_ERR(priv->bl_dev)) {
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH v2] backlight: led_bl: fix initial power state
@ 2023-07-04 16:19 ` Mans Rullgard
  0 siblings, 0 replies; 4+ messages in thread
From: Mans Rullgard @ 2023-07-04 16:19 UTC (permalink / raw)
  To: Lee Jones, Daniel Thompson, Jingoo Han
  Cc: Helge Deller, linux-fbdev, dri-devel, linux-kernel

The condition for the initial power state based on the default
brightness value is reversed.  Fix it.

Furthermore, use the actual state of the LEDs rather than the default
brightness specified in the devicetree as the latter should not cause
the backlight to be automatically turned on.

If the backlight device is not linked to any display, set the initial
power to on unconditionally.

Fixes: ae232e45acf9 ("backlight: add led-backlight driver")
Signed-off-by: Mans Rullgard <mans@mansr.com>
---
Changes in v2:
- Use the reported LED state to set initial power state
- Always power on if no phandle in DT
---
 drivers/video/backlight/led_bl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index 3259292fda76..bbf1673b1fb0 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -176,6 +176,7 @@ static int led_bl_probe(struct platform_device *pdev)
 {
 	struct backlight_properties props;
 	struct led_bl_data *priv;
+	int init_brightness;
 	int ret, i;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -190,6 +191,8 @@ static int led_bl_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	init_brightness = priv->default_brightness;
+
 	ret = led_bl_parse_levels(&pdev->dev, priv);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to parse DT data\n");
@@ -200,8 +203,8 @@ static int led_bl_probe(struct platform_device *pdev)
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = priv->max_brightness;
 	props.brightness = priv->default_brightness;
-	props.power = (priv->default_brightness > 0) ? FB_BLANK_POWERDOWN :
-		      FB_BLANK_UNBLANK;
+	props.power = (init_brightness || !pdev->dev.of_node->phandle) ?
+		FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
 	priv->bl_dev = backlight_device_register(dev_name(&pdev->dev),
 			&pdev->dev, priv, &led_bl_ops, &props);
 	if (IS_ERR(priv->bl_dev)) {
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v2] backlight: led_bl: fix initial power state
  2023-07-04 16:19 ` Mans Rullgard
@ 2023-07-05 14:09   ` Daniel Thompson
  -1 siblings, 0 replies; 4+ messages in thread
From: Daniel Thompson @ 2023-07-05 14:09 UTC (permalink / raw)
  To: Mans Rullgard
  Cc: Lee Jones, Jingoo Han, Helge Deller, dri-devel, linux-fbdev,
	linux-kernel

On Tue, Jul 04, 2023 at 05:19:52PM +0100, Mans Rullgard wrote:
> The condition for the initial power state based on the default
> brightness value is reversed.  Fix it.
>
> Furthermore, use the actual state of the LEDs rather than the default
> brightness specified in the devicetree as the latter should not cause
> the backlight to be automatically turned on.
>
> If the backlight device is not linked to any display, set the initial
> power to on unconditionally.
>
> Fixes: ae232e45acf9 ("backlight: add led-backlight driver")
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
> Changes in v2:
> - Use the reported LED state to set initial power state
> - Always power on if no phandle in DT
> ---
>  drivers/video/backlight/led_bl.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
> index 3259292fda76..bbf1673b1fb0 100644
> --- a/drivers/video/backlight/led_bl.c
> +++ b/drivers/video/backlight/led_bl.c
> @@ -176,6 +176,7 @@ static int led_bl_probe(struct platform_device *pdev)
>  {
>  	struct backlight_properties props;
>  	struct led_bl_data *priv;
> +	int init_brightness;
>  	int ret, i;
>
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> @@ -190,6 +191,8 @@ static int led_bl_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> +	init_brightness = priv->default_brightness;
> +
>  	ret = led_bl_parse_levels(&pdev->dev, priv);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "Failed to parse DT data\n");
> @@ -200,8 +203,8 @@ static int led_bl_probe(struct platform_device *pdev)
>  	props.type = BACKLIGHT_RAW;
>  	props.max_brightness = priv->max_brightness;
>  	props.brightness = priv->default_brightness;
> -	props.power = (priv->default_brightness > 0) ? FB_BLANK_POWERDOWN :
> -		      FB_BLANK_UNBLANK;
> +	props.power = (init_brightness || !pdev->dev.of_node->phandle) ?
> +		FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;

I was rather expecting to see a comment in the code here... it's super
hard to figure out the purpose of the phandle check otherwise.


Daniel.

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v2] backlight: led_bl: fix initial power state
@ 2023-07-05 14:09   ` Daniel Thompson
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Thompson @ 2023-07-05 14:09 UTC (permalink / raw)
  To: Mans Rullgard
  Cc: linux-fbdev, Jingoo Han, Helge Deller, Lee Jones, linux-kernel,
	dri-devel

On Tue, Jul 04, 2023 at 05:19:52PM +0100, Mans Rullgard wrote:
> The condition for the initial power state based on the default
> brightness value is reversed.  Fix it.
>
> Furthermore, use the actual state of the LEDs rather than the default
> brightness specified in the devicetree as the latter should not cause
> the backlight to be automatically turned on.
>
> If the backlight device is not linked to any display, set the initial
> power to on unconditionally.
>
> Fixes: ae232e45acf9 ("backlight: add led-backlight driver")
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
> Changes in v2:
> - Use the reported LED state to set initial power state
> - Always power on if no phandle in DT
> ---
>  drivers/video/backlight/led_bl.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
> index 3259292fda76..bbf1673b1fb0 100644
> --- a/drivers/video/backlight/led_bl.c
> +++ b/drivers/video/backlight/led_bl.c
> @@ -176,6 +176,7 @@ static int led_bl_probe(struct platform_device *pdev)
>  {
>  	struct backlight_properties props;
>  	struct led_bl_data *priv;
> +	int init_brightness;
>  	int ret, i;
>
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> @@ -190,6 +191,8 @@ static int led_bl_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> +	init_brightness = priv->default_brightness;
> +
>  	ret = led_bl_parse_levels(&pdev->dev, priv);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "Failed to parse DT data\n");
> @@ -200,8 +203,8 @@ static int led_bl_probe(struct platform_device *pdev)
>  	props.type = BACKLIGHT_RAW;
>  	props.max_brightness = priv->max_brightness;
>  	props.brightness = priv->default_brightness;
> -	props.power = (priv->default_brightness > 0) ? FB_BLANK_POWERDOWN :
> -		      FB_BLANK_UNBLANK;
> +	props.power = (init_brightness || !pdev->dev.of_node->phandle) ?
> +		FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;

I was rather expecting to see a comment in the code here... it's super
hard to figure out the purpose of the phandle check otherwise.


Daniel.

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-07-05 14:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-04 16:19 [PATCH v2] backlight: led_bl: fix initial power state Mans Rullgard
2023-07-04 16:19 ` Mans Rullgard
2023-07-05 14:09 ` Daniel Thompson
2023-07-05 14:09   ` Daniel Thompson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.