Re: [PATCH 0/6] auxdisplay: Add support for the Titanmec TM1628 7 segment display controller

[Neil Armstrong <narmstrong@baylibre.com>]

On 22/02/2022 13:12, Andreas Färber wrote:
> On 19.02.22 18:16, Heiner Kallweit wrote:
>> On 19.02.2022 17:07, Andreas Färber wrote:
>>> Hi,
>>>
>>> On 19.02.22 14:37, Heiner Kallweit wrote:
>>>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>>>
>>>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>>>
>>>>> Could you please link to the discussion and/or summarize the rationale
>>>>> behind the NAK?
>>>>>
>>>>
>>>> +Pavel
>>>>
>>>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>>>> one that makes use of the LED subsystem, but doesn't belong to it.
>>>> In the following mail he's expressing his opinion that the driver should
>>>> be best placed under auxdisplay.
>>>>
>>>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
>>>
>>> And I disagreed. It does not fit with the other drivers in auxdisplay
>>> that were operating on a much higher level.
>>>
>>
>> We need to find a place. And if Pavel has good reasons that it doesn't
>> fit into the LED subsystem, and Miguel should be fine with having
>> it in auxdisplay, then I'd see no reason to not go this way.
>>
>>> I'd also like to point out that I did implement the map_to_7segment API,
>>
>> Looking at the history of include/uapi/linux/map_to_7segment.h I see no
>> commit from you. Seems I'm missing something here.
> 
> You're replying inline too early:
> 
>>> as was suggested, as you will find in my tree
> 
> As I said, I implemented it in my driver:
> 
> https://github.com/afaerber/linux/commit/bbecf951348c7de8ba922c6c002a09369b717d82
> 
> Thus me saying you are unnecessarily duplicating work that I already
> did, without ping'ing the thread or me and claiming the credit for an
> implementation change which I already did myself.
> 
>>> - which you may have
>>> missed, referencing only the RFC patchset and putting your authorship on
>>> it exclusively? A move from one directory to another should not warrant
>>> my author and SoB getting removed from the actual driver.
>>>
>> The driver includes major changes and I mentioned your work in the commit
>> message. Also your still listed as MODULE_AUTHOR. My intention is to
>> get a driver upstream, not to earn credits for something.
>> So sure, your SoB can be (re-)added.
> 
> https://github.com/afaerber/linux/commits/rtd1295-next
> 
> Also note this 5-in-4 optimization:
> 
> https://github.com/afaerber/linux/commit/ff8284b6ed9dc1e354c35840afdaf50b1cd97fea
> 
> And several more chipsets being covered.
> 
>>> Given that we need to manage a buffer with bits per segment or LED
>>> symbol, one idea that I haven't found time for yet was to implement it
>>> as framebuffer or drm device instead. (And most Realtek platforms got
>>> broken by removing the adjustable text base defines.)
>>>
>> I'm not aware of the Realtek platform issue, do you have a link to a
>> related discussion?
> 
> Realtek has a boot ROM at the beginning of memory space, which has been
> a problem from the first RFC and for most bootloaders required to tweak
> the kernel's text offset for successful boot. (Some not Open Source (LK)
> and/or not openly flashable.)
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html
> 
> In 2020 that arm64 feature got removed without any further discussion:
> 
> https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/

Note the TEXT_OFFSET is only an issue with Amlogic vendor bootloader,
it has never been an issue with mainline U-Boot.

Neil

> 
> I've tried to revert it, but that's been a pain:
> 
> https://github.com/afaerber/linux/commit/0d2c647781bc89ee95bfa7b80d71237c7ebea230
> 
>> And wouldn't you think it's overengineering to
>> write a DRM driver for a 7 segment display with 4 digits?
>> Framebuffer seems to be deprecated based on my experience with
>> pygame / SDL2.
> 
> Is there any other API that would allow userspace to write to the buffer
> and bitblt parts to the SPI device?
> 
> Thinking of some optimizations I implemented in my driver to avoid
> unnecessary SPI transfers:
> 
> https://github.com/afaerber/linux/commit/46c40209db163a81474c6894ebbd90b5e238ce60
> 
> Regards,
> Andreas
>