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
| ||
|
Date: Fri, 2 Dec 2022 20:55:32 +0800 From: Chuanhong Guo <gch981213@...il.com> To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> Cc: linux-leds@...r.kernel.org, Pavel Machek <pavel@....cz>, Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Stanislav Jakubek <stano.jakubek@...il.com>, Linus Walleij <linus.walleij@...aro.org>, Shawn Guo <shawnguo@...nel.org>, Johan Hovold <johan+linaro@...nel.org>, Bjorn Andersson <andersson@...nel.org>, Marijn Suijten <marijn.suijten@...ainline.org>, Sven Schwermer <sven.schwermer@...ruptive-technologies.com>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>, open list <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v2 2/3] dt-bindings: leds: add dt schema for worldsemi,ws2812b-spi Hi! On Fri, Dec 2, 2022 at 7:14 PM Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote: > > On 02/12/2022 04:42, Chuanhong Guo wrote: > > This patch adds dt binding schema for WorldSemi WS2812B driven using SPI > > bus. > > Do not use "This commit/patch". > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 OK. > [...] > > + > > + default-brightness: > > + description: > > + The default brightness that should be applied to the LED by the operating > > + system on start-up. The brightness should not exceed the brightness the > > + LED can provide. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 0 > > + maximum: 255 > > + default: 0 > > + > > + default-intensity: > > + description: | > > + An array of 3 integer specifying the default intensity of each color > > + components in this LED. <255 255 255> if unspecified. > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > I am still not convinced these two properties are correct. Why this LED > is special and defines default brightness and intensity and other LEDs > do not? You explained you are doing it for user-space which is usually > not a valid reason for changes specific to one binding. Either all > bindings should support it or none. There's already a default-state for simple LEDs without brightness control so I think it makes sense to add default-brightness for LEDs with brightness control and default-intensity for colored LEDs. The default-state seems to be implemented in various LED drivers, so I implemented these two properties in my LED driver. There's nothing device-specific about these two properties. > > > + maxItems: 3 > > + items: > > + minimum: 0 > > + maximum: 255 > > + > > + reg: > > + description: | > > + Which LED this node represents. The reg of the first LED on the chain > > + is 0. > > + maxItems: 1 > > + > > + required: > > + - reg > > + - color > > + - function > > + > > +required: > > + - compatible > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/leds/common.h> > > + spi { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + leds@0 { > > git grep leds@ -- Documentation/devicetree/ | wc -l > 1 > git grep led@ -- Documentation/devicetree/ | wc -l > 165 > > so rather not the first one ("leds"). As you can see, this node describes a chain of LEDs, not a single LED, so the plural form is more appropriate than the singular form. > > There is also: > git grep led-controller@ -- Documentation/devicetree/ | wc -l > 30 This also isn't appropriate. WS2812B is a single LED package of 3 diodes and a microcontroller. If we treat every package as a LED, the SPI MOSI is connected directly to the LED packages themselves with no controller in between. If we treat the microcontroller as a led-controller, every LED contains its own controller, instead of one controller controlling all LEDs, and the parent node still shouldn't be called a led-controller. Here's a picture of the WS2812B LED package: https://cdn-shop.adafruit.com/970x728/1655-00.jpg and a chain of them: https://cdn-shop.adafruit.com/970x728/1463-00.jpg -- Regards, Chuanhong Guo
Powered by blists - more mailing lists