On Tue, Feb 20, 2024 at 05:43:32PM +0100, Luca Weiss wrote: > 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? I think #2 is the right option but shouldn't it be decrease max_brightness by 3, yielding 0..252 . Only nitpicking on that because, given how old this driver is I'd like to be conservative. I don't expect there to be userspaces with magic LM3630A modes but there could be some that assume #0 is off! Hence I wanted to make sure we are on the same page. Daniel.
