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

[Andreas Färber <afaerber@xxxxxxx>]

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@xxxxxxxxx> 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@xxxxxxxxxx/
>>
>> 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@xxxxxxxxxx/

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

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)