Hoi Robin, On Mon, Oct 4, 2021 at 10:26 AM Robin van der Gracht <robin@xxxxxxxxxxx> wrote: > On 2021-10-01 17:51, Geert Uytterhoeven wrote: > > On Thu, Sep 30, 2021 at 12:57 PM Robin van der Gracht <robin@xxxxxxxxxxx> > > wrote: > >> On 2021-09-14 16:38, Geert Uytterhoeven wrote: > >> > Instantiate a single LED based on the "led" subnode in DT. > >> > This allows the user to control display brightness and blinking (backed > >> > by hardware support) through the LED class API and triggers, and exposes > >> > the display color. The LED will be named > >> > "auxdisplay:<color>:<function>". > >> > > >> > When running in dot-matrix mode and if no "led" subnode is found, the > >> > driver falls back to the traditional backlight mode, to preserve > >> > backwards compatibility. > >> > > >> > Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> > >> > + > >> > + err = devm_led_classdev_register_ext(dev, led, &init_data); > >> > + if (err) > >> > + dev_err(dev, "Failed to register LED\n"); > >> > >> You might want to call ht16k33_brightness_set(priv, brightness) here to get > >> a > >> know value into the display setup register (0x80). > >> > >> Right now if I enable hardware blinking and (soft)reboot my board it keeps > >> on > >> blinking even after a re-probe. > > > > I don't have that issue. > > Aha, ht16k33_seg_probe() calls ht16k33_brightness_set(), but > > ht16k33_fbdev_probe() doesn't. The latter should do that, too, > > when not using backwards compatibility mode. > > Ack. I have hardware which uses the ht16k33 in dot matrix mode and I tested > both the backlight and led setup. I ran into this with the fbdev + led setup. > > I noticed ht16k33_bl_update_status() is called in ht16k33_fbdev_probe() > before the fbdev device is registered. Which is fine right now, but in theory > the fbdev blank state can influence the backlight setting (nitpick since > the fbdev device is unblanked by default). > > The point: Maybe ht16k33_brightness_set() (or ht16k33_bl_update_status() for > backlight device) should be called in one central place (i.e at the end of > the > main probe function). That would mean the main function need to know more about which mode is being used, so I think it's better to leave it to the individual display sub-drivers. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds