Re: [PATCH 2/7] dt-bindings: leds: pca995x: describe output-gain property

[Nora Schiffer <nora.schiffer@ew.tq-group.com>]

On Wed, 2026-09-02 at 12:31 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Low] The custom, device-specific property `output-gain` lacks a mandatory vendor prefix.

Probably makes sense to add the vendor prefix, will do in v2.

> - [Low] The `output-gain` property incorrectly exposes a raw hardware register value directly to the Device Tree instead of describing physical units.

According to the datasheet, the LED current is calculated as follows:

    Iled = output_gain * 900mV / Rext / 4

If we want to fully describe the output gain using physical units, something
like output-current-microamps and external-resistor-ohms would work. There
wouldn't be a meaningful default for these properties to describe in the binding
docs however, which is possible for "output-gain" (as the driver just sets that
to 127.)

Best,
Nora


> - [Low] The commit message description is not capitalized, violating LED subsystem naming conventions.
> --
> 
> commit 09d8712f125575e995a97c0a819c386e17eb766a
> Author: Nora Schiffer <nora.schiffer@ew.tq-group.com>
> 
> dt-bindings: leds: pca995x: describe output-gain property
> 
> This commit adds the output-gain property to the pca995x device tree binding,
> allowing configuration of the output current to adjust for different hardware
> designs.
> 
> > dt-bindings: leds: pca995x: describe output-gain property
> 
> [Severity: Low]
> This isn't a bug, but the subsystem guidelines for LED bindings specify
> that the description after the prefix should be capitalized. Should
> this be changed to "Describe output-gain property"?
> 
> > diff --git a/Documentation/devicetree/bindings/leds/nxp,pca995x.yaml b/Documentation/devicetree/bindings/leds/nxp,pca995x.yaml
> > --- a/Documentation/devicetree/bindings/leds/nxp,pca995x.yaml
> > +++ b/Documentation/devicetree/bindings/leds/nxp,pca995x.yaml
> [ ... ]
> > @@ -36,6 +36,14 @@ properties:
> >    "#size-cells":
> >      const: 0
> >  
> > +  output-gain:
> 
> [Severity: Low]
> Because this is a custom, device-specific property, does it require a
> mandatory vendor prefix (such as nxp,output-gain)? Exposing it without a
> prefix could pollute the global device tree namespace and potentially conflict
> with future standard properties or other bindings.
> 
> > +    description:
> > +      Set the output gain controlling the LED current as described in the
> > +      PCA995x datasheets.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    default: 128
> > +    maximum: 255
> 
> [Severity: Low]
> Do these constraints expose a raw 8-bit hardware register value directly
> to the device tree? Usually, device tree properties describe hardware neutrally
> using standard physical units (such as microamps) rather than tying the value
> to the internal register layout of the chip. Could this be abstracted to use
> standard physical units instead?
> 

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/