On Tue, Sep 07, 2021 at 02:47:51PM +0200, Thomas Wei?schuh wrote:
> backlight.h documents "struct backlight_ops->get_brightness()" to return
> a negative errno on failure.
> So far these errors have not been handled in the backlight core.
> This leads to negative values being exposed through sysfs although only
> positive values are documented to be reported.
>
> Signed-off-by: Thomas Wei?schuh <[email protected]>
Reviewed-by: Daniel Thompson <[email protected]>
> ---
>
> v1: https://lore.kernel.org/dri-devel/[email protected]/
>
> v1 -> v2:
> * use dev_err() instead of dev_warn() (Daniel Thompson)
> * Finish logging format string with newline (Daniel Thompson)
> * Log errno via dedicated error pointer format (Daniel Thompson)
>
> drivers/video/backlight/backlight.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 537fe1b376ad..4658cfb75aa2 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -292,10 +292,13 @@ static ssize_t actual_brightness_show(struct device *dev,
> struct backlight_device *bd = to_backlight_device(dev);
>
> mutex_lock(&bd->ops_lock);
> - if (bd->ops && bd->ops->get_brightness)
> - rc = sprintf(buf, "%d\n", bd->ops->get_brightness(bd));
> - else
> + if (bd->ops && bd->ops->get_brightness) {
> + rc = bd->ops->get_brightness(bd);
> + if (rc >= 0)
> + rc = sprintf(buf, "%d\n", rc);
> + } else {
> rc = sprintf(buf, "%d\n", bd->props.brightness);
> + }
> mutex_unlock(&bd->ops_lock);
>
> return rc;
> @@ -381,9 +384,18 @@ ATTRIBUTE_GROUPS(bl_device);
> void backlight_force_update(struct backlight_device *bd,
> enum backlight_update_reason reason)
> {
> + int brightness;
> +
> mutex_lock(&bd->ops_lock);
> - if (bd->ops && bd->ops->get_brightness)
> - bd->props.brightness = bd->ops->get_brightness(bd);
> + if (bd->ops && bd->ops->get_brightness) {
> + brightness = bd->ops->get_brightness(bd);
> + if (brightness >= 0)
> + bd->props.brightness = brightness;
> + else
> + dev_err(&bd->dev,
> + "Could not update brightness from device: %pe\n",
> + ERR_PTR(brightness));
> + }
> mutex_unlock(&bd->ops_lock);
> backlight_generate_event(bd, reason);
> }
>
> base-commit: 79fad92f2e596f5a8dd085788a24f540263ef887
> --
> 2.33.0
>
On 2021-09-07T14:10+0100, Daniel Thompson wrote:
> On Tue, Sep 07, 2021 at 02:47:51PM +0200, Thomas Weißschuh wrote:
> > backlight.h documents "struct backlight_ops->get_brightness()" to return
> > a negative errno on failure.
> > So far these errors have not been handled in the backlight core.
> > This leads to negative values being exposed through sysfs although only
> > positive values are documented to be reported.
> >
> > Signed-off-by: Thomas Weißschuh <[email protected]>
>
> Reviewed-by: Daniel Thompson <[email protected]>
Thanks!
Thomas