On Dienstag, 20. Februar 2024 15:11:07 CET Daniel Thompson wrote: > On Tue, Feb 20, 2024 at 12:11:21AM +0100, Luca Weiss wrote: > > As per documentation "drivers are expected to use this function in their > > update_status() operation to get the brightness value.". > > > > With this we can also drop the manual backlight_is_blank() handling > > since backlight_get_brightness() is already handling this correctly. > > > > Signed-off-by: Luca Weiss <luca@xxxxxxxxx> > > Reviewed-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx> > > However... > > > --- > > /* disable sleep */ > > @@ -201,9 +202,9 @@ static int lm3630a_bank_a_update_status(struct backlight_device *bl) > > goto out_i2c_err; > > usleep_range(1000, 2000); > > /* minimum brightness is 0x04 */ > > - ret = lm3630a_write(pchip, REG_BRT_A, bl->props.brightness); > > + ret = lm3630a_write(pchip, REG_BRT_A, brightness); > > ... then handling of the minimum brightness looks weird in this driver. > > The range of the backlight is 0..max_brightness. Sadly the drivers > are inconsistant regarding whether zero means off or just minimum, > however three certainly isn't supposed to mean off! In other words the > offsetting should be handled by driver rather than hoping userspace has > some magic LM3630A mode. I could also try and fix that.. 1. Treat 1..4 as 4, so have backlight on at that minimum level? Probably wouldn't be noticable that brightness 1=2=3=4. And the backlight will be on compared to off as it is now. 2. Decrease max_brightness by 4 values, so probably 0..251 and shift the values up in the driver so we get 4..255? Or would you have some other idea here? Regards Luca > > You didn't introduce this so this patch still has my R-b ... > > > Daniel. >
