Hi, On Mon, Jun 21, 2021 at 11:38 AM Sam Ravnborg <sam@xxxxxxxxxxxx> wrote: > > > > I cannot see why you need the extra check on ->enabled? > > > Would it be sufficient to check backlight_is_blank() only? > > > > This extra check on bl->enabled flag is added to avoid enabling/disabling > > backlight again if it is already enabled/disabled. > > Using this flag way can know the transition between backlight blank and > > un-blank, and decide when to enable/disable the backlight. > > My point is that this should really not be needed, as it would cover up > for some other bug whaere we try to do something twice that is not > needed. But I am less certain here so if you think it is needed, keep > it as is. I haven't tested this myself, but I believe that it is needed. I don't think the backlight update_status() function is like an enable/disable function. I believe it can be called more than one time even while the backlight is disabled. For instance, you can see that backlight_update_status() just blindly calls through to update the status. That function can be called for a number of reasons. Perhaps Rajeev can put some printouts to confirm but I think that if the backlight is "blanked" for whatever reason and you write to sysfs and change the backlight level you'll still get called again even though the backlight is still "disabled". -Doug
