Re: [PATCH v2 1/2] dt-bindings: leds: Add default-intensity property

[Conor Dooley <conor.dooley@microchip.com>]

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

On Fri, Jun 05, 2026 at 10:45:12AM +0200, Jonas Rebmann wrote:
> Hello,
> 
> On 2026-06-05 09:38, sashiko-bot@kernel.org wrote:
> > > @@ -73,6 +73,17 @@ properties:
> > >         - keep
> > >       default: off
> > > +  default-intensity:
> > > +    description:
> > > +      The initial intensity of the LED color component. Used only during
> > > +      initialization. Defaults to 0. As the intensity of each sub-LED is

I'd drop sentence two here, it just duplicates sentence one.

> > > +      multiplied with the overall brightness, without this property on a
> > > +      sub-LED, the leds-pwm-multicolor-LED is effectively initialized at
> > > +      minimum brightness regardless of its linux,default-trigger and
> > > +      default-brightness properties.
> > 
> > [Severity: Low]
> > This isn't a bug, but the description for default-intensity explicitly
> > mentions the specific driver name leds-pwm-multicolor-LED.
> > 
> > Since common.yaml is intended to be a hardware-agnostic specification
> > for all LED bindings, could this wording be updated to refer generically
> > to the multicolor LED or simply the LED to avoid leaking an
> > implementation detail into the generic schema?
> 
> I'll be leaving this as-is until human feedback.
> 
> I think documenting the property in common.yaml is correct because the
> default-intensity itself is applicable to other multicolor-LED drivers,

I think this is fine.

> I just only implemented it for pwm-multicolor because that's what I use.
> But we could just as well move it to leds-pwm-multicolor.yaml for as
> long as that's the only driver supporting it.

But this is probably also fine and it can be moved if other users crop
up.

> I intentionally included the "pwm" in that sentence because the
> described issue that default-intensity is effectively required when
> using linux,default-trigger may be pwm-multicolor specific.

I don't think this is fine. However, it is almost certainly fine if you
just do s/the leds-pwm-multicolour-LED/it/ because you're no longer
tying the description to the name of one driver.

> 
> I would not like to omit the sentence either because this situation is
> the reason why one might actually need (not just want)
> default-intensity (for -pwm-).
> 
> Regards,
> Jonas
> 
> -- 
> Pengutronix e.K.                           | Jonas Rebmann               |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]