* Re: [PATCH] docs: leds: uleds: Make the documentation match the code.
[not found] ` <20260424194714.71de0ef6@tag.xn--rombobjrn-67a.se>
@ 2026-05-07 13:11 ` Lee Jones
2026-05-10 19:43 ` Björn Persson
0 siblings, 1 reply; 2+ messages in thread
From: Lee Jones @ 2026-05-07 13:11 UTC (permalink / raw)
To: Björn Persson
Cc: Pavel Machek, Jonathan Corbet, Shuah Khan, linux-leds, linux-doc,
linux-kernel
On Fri, 24 Apr 2026, Björn Persson wrote:
> Lee Jones wrote:
> > On Thu, 02 Apr 2026, Björn Persson wrote:
> >
> > > From: Björn Persson <Bjorn@Rombobjörn.se>
> > >
> > > · max_brightness must be set. Leaving it uninitialized or just omitting it
> > > won't work.
> >
> > What are these points? How do you even type one of those?
>
> The bullet point is the character U+00B7 middle dot. I type it with
> AltGr-period on a Swedish keyboard in Fedora. I don't know what keymap
> your distro uses for your keyboard. I hear some keyboards lack an AltGr
> key.
Please use ASCII or you'll freak out the Luddites (like me)!
- or * is fine.
> > Anyway, proper sentences / paragraphs is better.
>
> You mean you dislike bulleted lists?
>
> Or if you mean that the first sentence doesn't begin with a capital M,
> that's because identifiers are case-sensitive in C. There is no
> Max_brightness, and if it were defined, it would be different from
> max_brightness.
>
> Otherwise I don't understand what you mean with "proper sentences", as
> I don't see any grammatical errors.
By all means use bullet points for lists, but a slab list of changes in
place of sentences is a little odd. Simply describe your changes as you
would speak to another human.
> > > -A new LED class device will be created with the name given. The name can be
> > > -any valid sysfs device node name, but consider using the LED class naming
> > > -convention of "devicename:color:function".
> > > +A new LED class device will be created with the given name and maximum
> >
> > Did you mean to revers "name given"? A "given name" usually means
> > something else.
>
> I felt that "the name and maximum brightness given" would be
> grammatically awkward.
>
> To prevent misinterpretation, how about replacing "given" with a
> synonym? Perhaps "the specified name and maximum brightness"? Another
This is nice.
> option is "the given maximum brightness and name", but it feels a
> little odd to mention the brightness before the name.
>
> > > +Although max_brightness is a signed int, only positive values are valid:
> > > +1 to INT_MAX.
> >
> > What about 0?
>
> That will get you an EINVAL from uleds.c – presumably because a
> brightness interval from 0 to 0 would be pointless. That LED would never
> be lit.
Ah, this is MAX brightness.
Okay so it's impossible to set a LED to always off.
> > > +The current brightness is found by reading a whole int from the character
> >
> > Try not to shorten names in documentation "integer".
>
> The type is named "int" in C. There are many integer types, but it would
> be wrong to try to read a uint16_t or a size_t or any other integer
> type. The document needs to use the actual type name to make it clear to
> the reader that they must read sizeof(int) bytes.
Right, but you're not writing in C.
> > Why do we need to specify "whole"?
>
> Because you can't read it piecemeal. Usually when you read from a disk
> file, a pipe, a TCP socket or some other bytestream, the system call
> will let you read one byte at a time if you want. A reader might assume
> that /dev/uleds works the same way.
>
> From a datagram socket you can read the beginning of a datagram and
> discard the part that doesn't fit in your buffer. To a reader with a
> little-endian system and max_brightness ≤ 255, it might seem logical
> that they'd be able to read the first byte and discard the bits that
> will always be zero.
>
> I thought "whole" would communicate to the reader that they must read
> sizeof(int) bytes in a single system call.
>
> It seems this wording wasn't enough to get the point across that it's
> necessary to read an int, a whole int, and nothing but an int. Do you
> think the document needs to expound that point more?
I think it needs rephrasing a little.
"Current brightness is obtained from the character device. It is
read in as an integer and must be done so in one go.
Or words to that effect.
--
Lee Jones
^ permalink raw reply [flat|nested] 2+ messages in thread