* [PATCHv3 0/4] led-backlight cleanups & fixes
@ 2020-04-22 6:51 ` Tomi Valkeinen
0 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2020-04-22 6:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev
Cc: Tomi Valkeinen
Hi,
Changes in v3:
- "backlight: led_bl: fix led -> backlight brightness mapping": Simplify
the for loop as suggested by Daniel
Changes in v2:
- Drop "backlight: led_bl: rewrite led_bl_parse_levels()". The patch
changed the behavior, and the new behavior may not be wanted. So lets
drop this for now.
- "backlight: led_bl: fix led -> backlight brightness mapping" will now
use max brightness if LED's brightness is higher than highest
backlight brightness level.
- Added reviewed-bys.
Tomi
Tomi Valkeinen (4):
backlight: led_bl: fix cosmetic issues
backlight: led_bl: drop useless NULL initialization
backlight: led_bl: add led_access locking
backlight: led_bl: fix led -> backlight brightness mapping
drivers/video/backlight/led_bl.c | 37 +++++++++++++++++---------------
1 file changed, 20 insertions(+), 17 deletions(-)
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv3 1/4] backlight: led_bl: fix cosmetic issues
2020-04-22 6:51 ` Tomi Valkeinen
@ 2020-04-22 6:51 ` Tomi Valkeinen
-1 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2020-04-22 6:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev
Cc: Tomi Valkeinen
Fix issues reported by checkpatch. No functional changes.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
drivers/video/backlight/led_bl.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index 3f66549997c8..d4e1ce684366 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -126,7 +126,7 @@ static int led_bl_get_leds(struct device *dev,
}
static int led_bl_parse_levels(struct device *dev,
- struct led_bl_data *priv)
+ struct led_bl_data *priv)
{
struct device_node *node = dev->of_node;
int num_levels;
@@ -148,8 +148,8 @@ static int led_bl_parse_levels(struct device *dev,
return -ENOMEM;
ret = of_property_read_u32_array(node, "brightness-levels",
- levels,
- num_levels);
+ levels,
+ num_levels);
if (ret < 0)
return ret;
@@ -159,14 +159,15 @@ static int led_bl_parse_levels(struct device *dev,
*/
db = priv->default_brightness;
for (i = 0 ; i < num_levels; i++) {
- if ((i && db > levels[i-1]) && db <= levels[i])
+ if ((i && db > levels[i - 1]) && db <= levels[i])
break;
}
priv->default_brightness = i;
priv->max_brightness = num_levels - 1;
priv->levels = levels;
- } else if (num_levels >= 0)
+ } else if (num_levels >= 0) {
dev_warn(dev, "Not enough levels defined\n");
+ }
ret = of_property_read_u32(node, "default-brightness-level", &value);
if (!ret && value <= priv->max_brightness)
@@ -208,7 +209,8 @@ static int led_bl_probe(struct platform_device *pdev)
props.power = (priv->default_brightness > 0) ? FB_BLANK_POWERDOWN :
FB_BLANK_UNBLANK;
priv->bl_dev = backlight_device_register(dev_name(&pdev->dev),
- &pdev->dev, priv, &led_bl_ops, &props);
+ &pdev->dev, priv, &led_bl_ops,
+ &props);
if (IS_ERR(priv->bl_dev)) {
dev_err(&pdev->dev, "Failed to register backlight\n");
return PTR_ERR(priv->bl_dev);
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCHv3 1/4] backlight: led_bl: fix cosmetic issues
@ 2020-04-22 6:51 ` Tomi Valkeinen
0 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2020-04-22 6:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev
Cc: Tomi Valkeinen
Fix issues reported by checkpatch. No functional changes.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
drivers/video/backlight/led_bl.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index 3f66549997c8..d4e1ce684366 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -126,7 +126,7 @@ static int led_bl_get_leds(struct device *dev,
}
static int led_bl_parse_levels(struct device *dev,
- struct led_bl_data *priv)
+ struct led_bl_data *priv)
{
struct device_node *node = dev->of_node;
int num_levels;
@@ -148,8 +148,8 @@ static int led_bl_parse_levels(struct device *dev,
return -ENOMEM;
ret = of_property_read_u32_array(node, "brightness-levels",
- levels,
- num_levels);
+ levels,
+ num_levels);
if (ret < 0)
return ret;
@@ -159,14 +159,15 @@ static int led_bl_parse_levels(struct device *dev,
*/
db = priv->default_brightness;
for (i = 0 ; i < num_levels; i++) {
- if ((i && db > levels[i-1]) && db <= levels[i])
+ if ((i && db > levels[i - 1]) && db <= levels[i])
break;
}
priv->default_brightness = i;
priv->max_brightness = num_levels - 1;
priv->levels = levels;
- } else if (num_levels >= 0)
+ } else if (num_levels >= 0) {
dev_warn(dev, "Not enough levels defined\n");
+ }
ret = of_property_read_u32(node, "default-brightness-level", &value);
if (!ret && value <= priv->max_brightness)
@@ -208,7 +209,8 @@ static int led_bl_probe(struct platform_device *pdev)
props.power = (priv->default_brightness > 0) ? FB_BLANK_POWERDOWN :
FB_BLANK_UNBLANK;
priv->bl_dev = backlight_device_register(dev_name(&pdev->dev),
- &pdev->dev, priv, &led_bl_ops, &props);
+ &pdev->dev, priv, &led_bl_ops,
+ &props);
if (IS_ERR(priv->bl_dev)) {
dev_err(&pdev->dev, "Failed to register backlight\n");
return PTR_ERR(priv->bl_dev);
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv3 2/4] backlight: led_bl: drop useless NULL initialization
2020-04-22 6:51 ` Tomi Valkeinen
@ 2020-04-22 6:51 ` Tomi Valkeinen
-1 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2020-04-22 6:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev
Cc: Tomi Valkeinen
There's no need to set 'levels' to NULL.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
drivers/video/backlight/led_bl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index d4e1ce684366..c46ecdfe8b0a 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -140,7 +140,7 @@ static int led_bl_parse_levels(struct device *dev,
if (num_levels > 1) {
int i;
unsigned int db;
- u32 *levels = NULL;
+ u32 *levels;
levels = devm_kzalloc(dev, sizeof(u32) * num_levels,
GFP_KERNEL);
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCHv3 2/4] backlight: led_bl: drop useless NULL initialization
@ 2020-04-22 6:51 ` Tomi Valkeinen
0 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2020-04-22 6:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev
Cc: Tomi Valkeinen
There's no need to set 'levels' to NULL.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
drivers/video/backlight/led_bl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index d4e1ce684366..c46ecdfe8b0a 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -140,7 +140,7 @@ static int led_bl_parse_levels(struct device *dev,
if (num_levels > 1) {
int i;
unsigned int db;
- u32 *levels = NULL;
+ u32 *levels;
levels = devm_kzalloc(dev, sizeof(u32) * num_levels,
GFP_KERNEL);
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv3 3/4] backlight: led_bl: add led_access locking
2020-04-22 6:51 ` Tomi Valkeinen
@ 2020-04-22 6:51 ` Tomi Valkeinen
-1 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2020-04-22 6:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev
Cc: Tomi Valkeinen
led_bl does not lock 'led_access' when calling led_sysfs_disable and
led_sysfs_enable, causing the below WARN. Add the locking.
WARNING: CPU: 0 PID: 223 at drivers/leds/led-core.c:353 led_sysfs_disable+0x4c/0x5c
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
drivers/video/backlight/led_bl.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index c46ecdfe8b0a..63693c4f0883 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -216,8 +216,11 @@ static int led_bl_probe(struct platform_device *pdev)
return PTR_ERR(priv->bl_dev);
}
- for (i = 0; i < priv->nb_leds; i++)
+ for (i = 0; i < priv->nb_leds; i++) {
+ mutex_lock(&priv->leds[i]->led_access);
led_sysfs_disable(priv->leds[i]);
+ mutex_unlock(&priv->leds[i]->led_access);
+ }
backlight_update_status(priv->bl_dev);
@@ -233,8 +236,11 @@ static int led_bl_remove(struct platform_device *pdev)
backlight_device_unregister(bl);
led_bl_power_off(priv);
- for (i = 0; i < priv->nb_leds; i++)
+ for (i = 0; i < priv->nb_leds; i++) {
+ mutex_lock(&priv->leds[i]->led_access);
led_sysfs_enable(priv->leds[i]);
+ mutex_unlock(&priv->leds[i]->led_access);
+ }
return 0;
}
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCHv3 3/4] backlight: led_bl: add led_access locking
@ 2020-04-22 6:51 ` Tomi Valkeinen
0 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2020-04-22 6:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev
Cc: Tomi Valkeinen
led_bl does not lock 'led_access' when calling led_sysfs_disable and
led_sysfs_enable, causing the below WARN. Add the locking.
WARNING: CPU: 0 PID: 223 at drivers/leds/led-core.c:353 led_sysfs_disable+0x4c/0x5c
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
---
drivers/video/backlight/led_bl.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index c46ecdfe8b0a..63693c4f0883 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -216,8 +216,11 @@ static int led_bl_probe(struct platform_device *pdev)
return PTR_ERR(priv->bl_dev);
}
- for (i = 0; i < priv->nb_leds; i++)
+ for (i = 0; i < priv->nb_leds; i++) {
+ mutex_lock(&priv->leds[i]->led_access);
led_sysfs_disable(priv->leds[i]);
+ mutex_unlock(&priv->leds[i]->led_access);
+ }
backlight_update_status(priv->bl_dev);
@@ -233,8 +236,11 @@ static int led_bl_remove(struct platform_device *pdev)
backlight_device_unregister(bl);
led_bl_power_off(priv);
- for (i = 0; i < priv->nb_leds; i++)
+ for (i = 0; i < priv->nb_leds; i++) {
+ mutex_lock(&priv->leds[i]->led_access);
led_sysfs_enable(priv->leds[i]);
+ mutex_unlock(&priv->leds[i]->led_access);
+ }
return 0;
}
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv3 4/4] backlight: led_bl: fix led -> backlight brightness mapping
2020-04-22 6:51 ` Tomi Valkeinen
@ 2020-04-22 6:51 ` Tomi Valkeinen
-1 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2020-04-22 6:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev
Cc: Tomi Valkeinen
The code that maps the LED default brightness to backlight levels has
two issues: 1) if the default brightness is the first backlight level
(usually 0), the code fails to find it, and 2) when the code fails to
find a backlight level, it ends up using max_brightness + 1 as the
default brightness.
Fix these two issues.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/backlight/led_bl.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index 63693c4f0883..699424b111ec 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -139,7 +139,6 @@ static int led_bl_parse_levels(struct device *dev,
num_levels = of_property_count_u32_elems(node, "brightness-levels");
if (num_levels > 1) {
int i;
- unsigned int db;
u32 *levels;
levels = devm_kzalloc(dev, sizeof(u32) * num_levels,
@@ -153,15 +152,11 @@ static int led_bl_parse_levels(struct device *dev,
if (ret < 0)
return ret;
- /*
- * Try to map actual LED brightness to backlight brightness
- * level
- */
- db = priv->default_brightness;
- for (i = 0 ; i < num_levels; i++) {
- if ((i && db > levels[i - 1]) && db <= levels[i])
+ /* Map LED brightness to backlight brightness level */
+ for (i = 0; i < num_levels - 1; i++)
+ if (levels[i] >= priv->default_brightness)
break;
- }
+
priv->default_brightness = i;
priv->max_brightness = num_levels - 1;
priv->levels = levels;
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCHv3 4/4] backlight: led_bl: fix led -> backlight brightness mapping
@ 2020-04-22 6:51 ` Tomi Valkeinen
0 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2020-04-22 6:51 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev
Cc: Tomi Valkeinen
The code that maps the LED default brightness to backlight levels has
two issues: 1) if the default brightness is the first backlight level
(usually 0), the code fails to find it, and 2) when the code fails to
find a backlight level, it ends up using max_brightness + 1 as the
default brightness.
Fix these two issues.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/backlight/led_bl.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index 63693c4f0883..699424b111ec 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -139,7 +139,6 @@ static int led_bl_parse_levels(struct device *dev,
num_levels = of_property_count_u32_elems(node, "brightness-levels");
if (num_levels > 1) {
int i;
- unsigned int db;
u32 *levels;
levels = devm_kzalloc(dev, sizeof(u32) * num_levels,
@@ -153,15 +152,11 @@ static int led_bl_parse_levels(struct device *dev,
if (ret < 0)
return ret;
- /*
- * Try to map actual LED brightness to backlight brightness
- * level
- */
- db = priv->default_brightness;
- for (i = 0 ; i < num_levels; i++) {
- if ((i && db > levels[i - 1]) && db <= levels[i])
+ /* Map LED brightness to backlight brightness level */
+ for (i = 0; i < num_levels - 1; i++)
+ if (levels[i] >= priv->default_brightness)
break;
- }
+
priv->default_brightness = i;
priv->max_brightness = num_levels - 1;
priv->levels = levels;
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCHv3 4/4] backlight: led_bl: fix led -> backlight brightness mapping
2020-04-22 6:51 ` Tomi Valkeinen
@ 2020-04-22 8:30 ` Daniel Thompson
-1 siblings, 0 replies; 12+ messages in thread
From: Daniel Thompson @ 2020-04-22 8:30 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jingoo Han, linux-fbdev, Lee Jones, dri-devel, Bartlomiej Zolnierkiewicz
On Wed, Apr 22, 2020 at 09:51:14AM +0300, Tomi Valkeinen wrote:
> The code that maps the LED default brightness to backlight levels has
> two issues: 1) if the default brightness is the first backlight level
> (usually 0), the code fails to find it, and 2) when the code fails to
> find a backlight level, it ends up using max_brightness + 1 as the
> default brightness.
>
> Fix these two issues.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 4/4] backlight: led_bl: fix led -> backlight brightness mapping
@ 2020-04-22 8:30 ` Daniel Thompson
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Thompson @ 2020-04-22 8:30 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jingoo Han, linux-fbdev, Lee Jones, dri-devel, Bartlomiej Zolnierkiewicz
On Wed, Apr 22, 2020 at 09:51:14AM +0300, Tomi Valkeinen wrote:
> The code that maps the LED default brightness to backlight levels has
> two issues: 1) if the default brightness is the first backlight level
> (usually 0), the code fails to find it, and 2) when the code fails to
> find a backlight level, it ends up using max_brightness + 1 as the
> default brightness.
>
> Fix these two issues.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread