Re: [RFC 22/25] leds: tm1826: Add combined glyph support

[Andreas FÃrber]

Hi Miguel,

Am 22.12.19 um 00:12 schrieb Miguel Ojeda:
> On Sat, 21 Dec 2019 at 22:49 Andreas FÃrber <afaerber@xxxxxxx 
> <mailto:afaerber@xxxxxxx>> wrote:
>
>     Hi,
>
>     Am 21.12.19 um 22:04 schrieb Pavel Machek:
>      >>>> Allow to squeeze the text "HEllO" into a 4-digit display,
>      >>>> as seen on MeLE V9 TV box.
>      >>>>
>      >>>> Enable this combining mode only if the text would overflow.
>      >>>
>      >>> "HEll,nO"!
>      >>>
>      >>> :-)
>      >>>
>      >>> Ok, it is kind of cool, but... Can you take a look at
>      >>> drivers/auxdisplay/charlcd.c ? It seems to support some kind of
>     text
>      >>> displays...
>      >>
>      >> Why don't you look at it before making such a suggestion? ;) It
>     is in no way
>      >> useful, as I pointed out in my cover letter. The only thing
>     related today,
>      >> as Geert pointed out, is in the input subsystem.
>      >
>      > Okay, so maybe we should get
>      >
>      > AUXILIARY DISPLAY DRIVERS
>      > M:Â Â Â Miguel Ojeda Sandonis <miguel.ojeda.sandonis@xxxxxxxxx
>     <mailto:miguel.ojeda.sandonis@xxxxxxxxx>>
>      >
>      > on the Cc: list?
>
>     Let's see if that email still exists - the code looked ancient, full of
>     platform_data and driver-specific exported functions...
>
>     (the Yealink input driver was from 2005, too)
>
>
> The code may look ancient, but the email surely exists ;)
>
>
>
>      > What you really have is a display, not a bunch of LEDs.
>
>     We have an LED Controller connected to zero, one or more displays.
>     They are most certainly _not_ the same thing.
>
>      >> If you don't want this in leds, you'll have to help make leds
>     subsystem more
>      >> useful to external users - the latest function refactoring has
>     been anything
>      >> but helpful here, as you've seen with the indicators, and we're
>     completely
>      >> lacking any indexing or bulk operations on the LED controller
>     level, since
>      >> you treat each LED as a standalone device. That's precisely why
>     this code is
>      >> here in leds although - as I pointed out - it shouldn't belong here.
>      >
>      > Well, your introduction mail was kind of long :-).
>      >
>      > If someone wants to do heartbeat on
>      >
>      >Â Â--
>      > |Â | <- this segment
>      >Â Â--
>      > |Â |
>      >Â Â--
>      >
>      > they are probably crazy. We may not want to support that. What about
>      > doing it as auxdisplay driver, and then exporting the indicators
>      > around that as LEDs?
>
>     You're really just discussing which directory to put this file into -
>     moving it around is the easiest thing...
>
>      >
>      > Having USB activity trigger on 'USB' icon makes sense, on the other
>      > hand. That would still be supported.
>
>     Actually I disagree about those indicators - that was the reason
>     they're
>     indicators and not, e.g., "usb". IMO people would go crazy if large
>     text
>     like that blinked during USB transfers. I assumed the meaning of those
>     LEDs were to indicate whether a USB/SD medium is connected, which I did
>     not see any better function for, and I'm not aware of us having such
>     triggers today.
>
>     Maybe you also overread that with trigger I was referring to using RTC
>     as trigger for a) the colon blinking every half-second and b) the text
>     getting updated based on avsilable RTC interrupts?
>
>     You could also think of GPIO-connected LEDs that you may want to
>     animate
>     without two different heartbeats/timers getting out of sync. Or
>     think of
>     an RGB LED that today we sadly need to model as multiple GPIO LEDs
>     instead of as one with a color property we can change (and hardcoding a
>     color in DT/name is not helping that use case either).
>
>     auxdisplay offers no API that I could register with to drive output,
>     nor
>     any triggers to automate such output - that's unique to LEDs. Like I
>     said, we can place this spi_driver file into auxdisplay/, but that
>     doesn't solve the driver design.
>
>
> I think it would be alright to put it in auxdisplay.

Please find the full series on, e.g., LAKML:

https://patchwork.kernel.org/cover/11286939/

>     So I really think we need to decouple
>     the two and keep the LED Controller driver in leds and the display
>     logic
>     elsewhere, with suitable new APIs to connect them. We're lacking
>     suggestions for the how, on DT and API levels - see my response on
>     [...]the
>     cover letter.

As explained in the cover letter, the problem here is that I don't know 
the model or manufacturer of these unmarked white-plastic-frame LED 
displays. So we have neither a filename to use in auxdisplay/ nor a DT 
compatible string to use for those displays.

Cheers,
Andreas

--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 NÃrnberg, Germany
GF: Felix ImendÃrffer
HRB 36809 (AG NÃrnberg)