[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <E12E0A91-8B9A-4326-96DD-10078BEA81F1@gmail.com>
Date: Sat, 30 Aug 2025 12:57:13 -0400
From: Jean-François Lessard <jefflessard3@...il.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>,
Geert Uytterhoeven <geert@...ux-m68k.org>
CC: Andy Shevchenko <andy@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] auxdisplay: line-display: support attribute attachment to existing device
Le 30 août 2025 12 h 18 min 27 s HAE, Andy Shevchenko <andy.shevchenko@...il.com> a écrit :
>On Sat, Aug 30, 2025 at 6:21 PM Jean-François Lessard
><jefflessard3@...il.com> wrote:
>> Le 30 août 2025 10 h 39 min 20 s HAE, Andy Shevchenko <andy.shevchenko@...il.com> a écrit :
>> >On Sat, Aug 30, 2025 at 4:55 PM Jean-François Lessard
>> ><jefflessard3@...il.com> wrote:
>> >> Le 30 août 2025 05 h 15 min 23 s HAE, Andy Shevchenko <andy.shevchenko@...il.com> a écrit :
>> >> >On Fri, Aug 29, 2025 at 4:03 AM Jean-François Lessard
>> >> ><jefflessard3@...il.com> wrote:
>
>...
>
>> >> If needed to be split into multiple patches, it could be:
>> >> 1. Add attach/detach capability
>> >> 2. Add num_chars sysfs attribute + ABI doc
>> >
>> >Yeah, the commit message was a list of 5 things, hence 5 patches. I
>> >haven't read closely to map each listed feature to the possible
>> >change. The description of to_linedisp() sounds like it can be split
>> >to a separate patch. I really want to see the attachment/detachment
>> >patch separate with the explanation "why?". I am not sure I see the
>> >point.
>>
>> I will shorten commit message and clarify the "why":
>>
>> Enable linedisp library integration into existing kernel devices
>> (like LED class) to provide uniform 7-segment userspace API without creating
>
>a uniform
>
Acknowledged.
>> separate child devices, meeting consistent interface while maintaining
>
>the consistent
>
Acknowledged.
>> coherent device hierarchies.
>>
>> This allows uniform 7-segment API across all drivers while solving device
>> proliferation and fragmented userspace interfaces.
>
>Yes, this is a good start.
>
Great. I will submit the attach capability in a separate patch clarifying this.
>> Note that there is no point in introducing to_linedisp() based on attachments
>> without attach/detach capability. Current implementation is actually correct
>> if only supporting registration of child devices.
>
>This can be made in a series, so the order will suggest the
>dependencies immediately.
>
Agreed. Series would then be:
1. attachment list + to_linedisp()
2. attach to existing device capability
3. num_digits attribute + ABI docs
I guess this should be an independent patch series than the tm16xx patch series.
Isn't it an issue to submit code without a real user? I mean, tm16xx would be
the first attach/detach consumer.
>...
>
>> >> I am also thinking to add a 3rd one to pad the message when length is smaller
>> >> than num_chars. Current behavior is to wrap around which seems weird to me.
>> >> E.g. for 4 chars:
>> >>
>> >> Current behavior:
>> >> cat "123" > message
>> >> results in "1231" being displayed
>> >>
>> >> Padded behavior:
>> >> cat "123" > message
>> >> would result in "123<blank>" being displayed
>> >>
>> >> Any thoughts on that?
>> >
>> >It's basically the question of circular vs. one time message exposure.
>> >When it's one time, the padding makes sense, otherwise the current
>> >(circular) behaviour seems normal to me.
>>
>> I get your point but I should have noted that current behavior is to wrap but
>> does NOT scroll. That's why it seems wrong to me. It should either wrap AND
>> scroll, or otherwise simply pad.
>
>Ah, that's an interesting observation. I think we need to ask Geert
>and others about their opinions on this. To me it seems like you have
>a point.
>
Let's see Geert and others feedback on this. If no feedback, I will submit a 4th
patch to the series that would display static (non-scrolling) padded message
when length <= num_chars.
>...
>
>> >> >> + &dev_attr_num_chars.attr,
>> >> >
>> >> >This needs a Documentation update for a new ABI.
>> >>
>> >> Agreed. I think it's also worth documenting the other ABIs along the way since
>> >> they are not documented yet. Then, what Contact should be documented?
>> >> Is there an auxdisplay mailing list?
>> >
>> >Your or no-one's? Is it a mandatory field? In any case the ABI must be
>> >documented, w.o. doing that any good patch is simply NAK
>> >automatically.
>>
>> I thought contact was mandatory but after looking closer, there are multiple
>> existing precedents where there is no documented contact. I think it would be
>> appropriate to add my contact to the added num_chars property documentation,
>> but to add documentation of the other existing properties with no contact.
>
>Good for me.
>
Will do so.
Powered by blists - more mailing lists