Re: [PATCH v3 2/4] mfd: aat2870: Derive brightness steps from max current

[Svyatoslav Ryhel]

пт, 16 жовт. 2026 р. о 18:30 Daniel Thompson <danielt@xxxxxxxxxx> пише:
>
> On Tue, Oct 13, 2026 at 09:09:31PM +0300, Svyatoslav Ryhel wrote:
> > @@ -69,10 +57,11 @@ static int aat2870_bl_update_status(struct backlight_device *bd)
> >       dev_dbg(&bd->dev, "brightness=%d, power=%d, state=%d\n",
> >                bd->props.brightness, bd->props.power, bd->props.state);
> >
> > -     ret = aat2870->write(aat2870, AAT2870_BLM,
> > -                          (u8)aat2870_brightness(aat2870_bl, brightness));
> > -     if (ret < 0)
> > -             return ret;
> > +     if (brightness) {
> > +             ret = aat2870->write(aat2870, AAT2870_BLM, (u8)brightness - 1);
> > +             if (ret < 0)
> > +                     return ret;
> > +     }
> >
> >       if (brightness == 0) {
>
> I think this could be spelled "else" ;-)
>
> However, other than this this looks good. With that change applied:
> Reviewed-by: Daniel Thompson (RISCstar) <danielt@xxxxxxxxxx>
>

Patch cuts part of code, this is the full context

if ((brightness < 0) || (bd->props.max_brightness < brightness)) {
        dev_err(&bd->dev, "invalid brightness, %d\n", brightness);
        return -EINVAL;
}

if (brightness) {
        ret = aat2870->write(aat2870, AAT2870_BLM, (u8)brightness - 1);
        if (ret < 0)
                return ret;
}

if (brightness == 0) {
        ret = aat2870_bl_disable(aat2870_bl);
        if (ret < 0)
                return ret;
} else if (aat2870_bl->brightness == 0) {
        ret = aat2870_bl_enable(aat2870_bl);
        if (ret < 0)
                return ret;
}

aat2870 cannot read main backlight register nor channel enable
register to check prev backlight status. Hence this logic is used. If
new brightness > 0, update main bl register; if brightness == 0,
disable all active channels, if it was 0 and now is >0 enable
supported channels.

>
> Daniel