Re: [PATCH] docs: leds: uleds: Make the documentation match the code.

["Björn Persson" <Bjorn@xn--rombobjrn-67a.se>]

[-- Attachment #1: Type: text/plain, Size: 3298 bytes --]

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.

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

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

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

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

Björn Persson

[-- Attachment #2: OpenPGP digital signatur --]
[-- Type: application/pgp-signature, Size: 833 bytes --]