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

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

[Björn Persson]

From: Björn Persson <Bjorn@Rombobjörn.se>

· max_brightness must be set. Leaving it uninitialized or just omitting it
  won't work.

· The maximum brightness is not 255 but the value given to max_brightness.

· Brightness values must be read as ints, not bytes.

· The ints are signed, so the word "unsigned" is misleading.

Signed-off-by: Björn Persson <Bjorn@Rombobjörn.se>
---
 Documentation/leds/uleds.rst | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/Documentation/leds/uleds.rst b/Documentation/leds/uleds.rst
index 83221098009c..9875a0fa4185 100644
--- a/Documentation/leds/uleds.rst
+++ b/Documentation/leds/uleds.rst
@@ -17,16 +17,20 @@ structure to it (found in kernel public header file linux/uleds.h)::
 
     struct uleds_user_dev {
 	char name[LED_MAX_NAME_SIZE];
+	int max_brightness;
     };
 
-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
+brightness. The name can be any valid sysfs device node name, but consider
+using the LED class naming convention of "devicename:color:function".
 
-The current brightness is found by reading a single byte from the character
-device. Values are unsigned: 0 to 255. Reading will block until the brightness
-changes. The device node can also be polled to notify when the brightness value
-changes.
+Although max_brightness is a signed int, only positive values are valid:
+1 to INT_MAX.
+
+The current brightness is found by reading a whole int from the character
+device. The possible values are 0 to max_brightness. Reading will block until
+the brightness changes. The device node can also be polled to notify when the
+brightness value changes.
 
 The LED class device will be removed when the open file handle to /dev/uleds
 is closed.
-- 
2.53.0

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

[Lee Jones]

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?

Anyway, proper sentences / paragraphs is better.

> · The maximum brightness is not 255 but the value given to max_brightness.
> 
> · Brightness values must be read as ints, not bytes.
> 
> · The ints are signed, so the word "unsigned" is misleading.
> 
> Signed-off-by: Björn Persson <Bjorn@Rombobjörn.se>
> ---
>  Documentation/leds/uleds.rst | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/leds/uleds.rst b/Documentation/leds/uleds.rst
> index 83221098009c..9875a0fa4185 100644
> --- a/Documentation/leds/uleds.rst
> +++ b/Documentation/leds/uleds.rst
> @@ -17,16 +17,20 @@ structure to it (found in kernel public header file linux/uleds.h)::
>  
>      struct uleds_user_dev {
>  	char name[LED_MAX_NAME_SIZE];
> +	int max_brightness;
>      };
>  
> -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.

> +brightness. The name can be any valid sysfs device node name, but consider
> +using the LED class naming convention of "devicename:color:function".
>  
> -The current brightness is found by reading a single byte from the character
> -device. Values are unsigned: 0 to 255. Reading will block until the brightness
> -changes. The device node can also be polled to notify when the brightness value
> -changes.
> +Although max_brightness is a signed int, only positive values are valid:
> +1 to INT_MAX.

What about 0?

> +The current brightness is found by reading a whole int from the character

Try not to shorten names in documentation "integer".

Why do we need to specify "whole"?

> +device. The possible values are 0 to max_brightness. Reading will block until
> +the brightness changes. The device node can also be polled to notify when the
> +brightness value changes.
>  
>  The LED class device will be removed when the open file handle to /dev/uleds
>  is closed.
> -- 
> 2.53.0
> 

-- 
Lee Jones [李琼斯]

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

[Björn Persson]

[-- 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 --]

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