| Message ID | 20200421124629.20977-5-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | led-backlight cleanups & fixes | expand |
On Tue, Apr 21, 2020 at 03:46:29PM +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> > --- > drivers/video/backlight/led_bl.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c > index 63693c4f0883..43a5302f163a 100644 > --- a/drivers/video/backlight/led_bl.c > +++ b/drivers/video/backlight/led_bl.c > @@ -159,10 +159,11 @@ 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 == 0 || db > levels[i - 1]) && db <= levels[i]) > break; > } I looked at this loop again and realized the entire check of levels[i-1] is pointless anyway: we already know that db is greater then levels[i-1] otherwise the loop would have exited on its previous iteration. > - priv->default_brightness = i; > + > + priv->default_brightness = min(i, num_levels - 1); Perhaps this min() also tells us the loop exit condition is wrong as well... and whilst we are at it the final comparison is arguably not in the best order (since to describe what the loop does we have to a complex clauses like "such that"). In natural English what the code is trying to do is "find the first value in the lookup table that is larger than or equal to db or, if that does not exist, choose the brightest value". In other words: for (i=0; i<num_levels-1; i++) if (levels[i] >= db) break; Daniel. > - if ((i && db > levels[i - 1]) && db <= > levels[i]) > + if ((i == 0 || db > levels[i - 1]) && db <= > levels[i]) > break; > } > priv->max_brightness = num_levels - 1; > priv->levels = levels; > } else if (num_levels >= 0) { > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c index 63693c4f0883..43a5302f163a 100644 --- a/drivers/video/backlight/led_bl.c +++ b/drivers/video/backlight/led_bl.c @@ -159,10 +159,11 @@ 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 == 0 || db > levels[i - 1]) && db <= levels[i]) break; } - priv->default_brightness = i; + + priv->default_brightness = min(i, num_levels - 1); priv->max_brightness = num_levels - 1; priv->levels = levels; } else if (num_levels >= 0) {
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 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)