On Thu, Jul 20, 2023 at 2:27 PM Daniel Thompson <daniel.thompson@xxxxxxxxxx> wrote: > > On Thu, Jul 20, 2023 at 06:06:27AM +0000, Ying Liu wrote: > > Bootloader may leave gpio direction as input and gpio value as logical low. > > It hints that initial backlight power state should be FB_BLANK_POWERDOWN > > since the gpio value is literally logical low. > > To be honest this probably "hints" that the bootloader simply didn't > consider the backlight at all :-) . I'd rather the patch description > focus on what circumstances lead to the current code making a bad > decision. More like: > > If the GPIO pin is in the input state but the backlight is currently > off due to default pull downs then ... > > > So, let's drop output gpio > > direction check and only check gpio value to set the initial power state. > > This check was specifically added by Bartosz so I'd be interested in his > opinion of this change (especially since he is now a GPIO maintainer)! > > What motivates (or motivated) the need to check the direction rather > than just read that current logic level on the pin? ... > > - else if (gpiod_get_direction(gbl->gpiod) == 0 && > > - gpiod_get_value_cansleep(gbl->gpiod) == 0) > > + else if (gpiod_get_value_cansleep(gbl->gpiod) == 0) > > bl->props.power = FB_BLANK_POWERDOWN; The code before this patch needs a bit of elaboration. There is no prohibition on reading value for the pin that is in any direction. I.o.w. if the direction here is a problem it should have been configured beforehand. -- With Best Regards, Andy Shevchenko