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

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

[Lee Jones]

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

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

[Björn Persson]

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

Lee Jones wrote:
> On Fri, 24 Apr 2026, Björn Persson wrote:
> 
> > Lee Jones wrote:  
> > > On Thu, 02 Apr 2026, Björn Persson wrote:
> > >   
> > > > +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.

That's technically true, as I wrote my program in C++. It's far from my
favorite, but I had to use a language that can include C header files
and use C types, because /dev/uleds is a very C-centric interface.

If API documentation isn't allowed to name a type, then I withdraw the
patch. It's pointless to continue. The next programmer will also have to
read the code to find out what the true API is, like I did.

Björn Persson

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