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

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

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

On Fri, Jun 05, 2026 at 11:29:31AM +0100, Conor Dooley wrote:
> 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.

Also drop "Defaults to 0." since that duplicates the default: 0.

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