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

From: Heiner Kallweit <hkallweit1@gmail.com>
To: "Andreas Färber" <afaerber@suse.de>,
	"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com>,
	"Pavel Machek" <pavel@ucw.cz>
Cc: Mark Brown <broonie@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Miguel Ojeda <ojeda@kernel.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	linux-amlogic@lists.infradead.org
Subject: Re: [PATCH 0/6] auxdisplay: Add support for the Titanmec TM1628 7 segment display controller
Date: Sat, 19 Feb 2022 18:16:32 +0100	[thread overview]
Message-ID: <09bf3d8a-2902-723b-80d2-0c4d1c24f53d@gmail.com> (raw)
In-Reply-To: <862fc0e3-6c76-8dea-6725-a6c45ade1ecd@suse.de>

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.

> as was suggested, as you will find in my tree - 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.

> 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? 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.

> Regards,
> Andreas
> 

Heiner