linux-kernel - Re: [PATCH v5 1/5] dt-bindings: leds: add TI/National Semiconductor LP5812 LED Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250415095358.8044-1-trannamatk@gmail.com>
Date: Tue, 15 Apr 2025 16:53:58 +0700
From: Nam Tran <trannamatk@...il.com>
To: krzk+dt@...nel.org
Cc: pavel@...nel.org,
	lee@...nel.org,
	robh@...nel.org,
	conor+dt@...nel.org,
	corbet@....net,
	devicetree@...r.kernel.org,
	linux-leds@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/5] dt-bindings: leds: add TI/National Semiconductor LP5812 LED Driver

On Mon, 14 Apr 2025, Krzysztof Kozlowski wrote:

> On 14/04/2025 16:57, Nam Tran wrote:
> > +
> > +description: |
> > +  The LP5812 is an I2C LED Driver that can support LED matrix 4x3.
> > +  For more product information please see the link below:
> > +  https://www.ti.com/product/LP5812#tech-docs
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,lp5812
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> 
> No need for supply?

Since the hardware uses an external power supply,
we decide not to include the supply property in the binding.

> > +
> > +patternProperties:
> > +  "^led@[0-9a-b]$":
> > +    type: object
> > +    $ref: common.yaml#
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        minimum: 0
> > +        maximum: 0xb
> > +
> > +      chan-name:
> > +        $ref: /schemas/types.yaml#/definitions/string
> > +        description: LED channel name
> 
> My comment stay valid. I don't think LEDs have channels, datasheet also
> has nothing about channels, so again - use existing properties. Or
> better drop it - I don't see any point in the name. The reg already
> defines it.

The channel was named for the output channel to each LED, not the LED channels.
They are not required properties because we can control entirely the LEDs of LP5812 through the indexes (regs property),
but the person who wants to develop LP5812's matrix-related features can use the "channels" for easy mapping.

> 
> However after dropping this, your example has nodes with only reg -
> what's the point of them? Why no properties from common.yaml are
> applicable? If they are not applicable, then the entire subnode should
> be dropped - you don't need them to describe the hardware.

Actually, the "color" property can be applied, but the LP5812 is a matrix LED,
so specifying a particular LED color is not necessary when developing LP5812 features.

Best regards,
Nam Tran