* [PATCH v4 0/3] leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs @ 2022-12-12 4:55 Chuanhong Guo 2022-12-12 4:55 ` [PATCH v4 1/3] dt-bindings: vendor-prefixes: add an entry for WorldSemi Chuanhong Guo ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Chuanhong Guo @ 2022-12-12 4:55 UTC (permalink / raw) To: linux-leds Cc: Chuanhong Guo, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list This patch adds support for driving a chain of WS2812B LED chips using SPI bus. WorldSemi WS2812B is a individually addressable LED chip that can be chained together and controlled individually using a single wire. The chip recognize a long pulse as a bit of 1 and a short pulse as a bit of 0. Host sends a continuous stream of 24-bits color values, each LED chip takes the first 3 byte it receives as its color value and passes the leftover bytes to the next LED on the chain. This driver simulates this protocol using SPI bus by sending a long pulse as 3'b110 and a short pulse as 3'b100. The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct and the controller needs to transfer all the bytes continuously. Changes since v1: various dt binding fixes add support for default-brightness Changes since v2: more dt binding fixes drop default-brightness and default-intensity Changes since v3: 1. add more comments 2. rename reg to cascade 3. redo some line breaking 4. move duplicated pointer calculation into ws2812b_set_byte 5. reword error message 6. get ws2812b_priv from led cdev->dev->parent Chuanhong Guo (3): dt-bindings: vendor-prefixes: add an entry for WorldSemi dt-bindings: leds: add worldsemi,ws2812b leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs .../bindings/leds/worldsemi,ws2812b.yaml | 116 +++++++++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + drivers/leds/rgb/Kconfig | 11 + drivers/leds/rgb/Makefile | 1 + drivers/leds/rgb/leds-ws2812b.c | 231 ++++++++++++++++++ 5 files changed, 361 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml create mode 100644 drivers/leds/rgb/leds-ws2812b.c -- 2.38.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 1/3] dt-bindings: vendor-prefixes: add an entry for WorldSemi 2022-12-12 4:55 [PATCH v4 0/3] leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs Chuanhong Guo @ 2022-12-12 4:55 ` Chuanhong Guo 2022-12-12 4:55 ` [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b Chuanhong Guo 2022-12-12 4:55 ` [PATCH v4 3/3] leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs Chuanhong Guo 2 siblings, 0 replies; 17+ messages in thread From: Chuanhong Guo @ 2022-12-12 4:55 UTC (permalink / raw) To: linux-leds Cc: Chuanhong Guo, Krzysztof Kozlowski, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list Add vendor prefix for WorldSemi that makes WS2812B individually-addressable RGB LEDs. Signed-off-by: Chuanhong Guo <gch981213@gmail.com> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Change since v1: reword commit message Change since v2: none Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 10c178d97b02..32274d894664 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -1462,6 +1462,8 @@ patternProperties: description: Wondermedia Technologies, Inc. "^wobo,.*": description: Wobo + "^worldsemi,.*": + description: WorldSemi Co., Limited "^wanchanglong,.*": description: Wanchanglong Electronics Technology(SHENZHEN)Co.,Ltd. "^x-powers,.*": -- 2.38.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2022-12-12 4:55 [PATCH v4 0/3] leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs Chuanhong Guo 2022-12-12 4:55 ` [PATCH v4 1/3] dt-bindings: vendor-prefixes: add an entry for WorldSemi Chuanhong Guo @ 2022-12-12 4:55 ` Chuanhong Guo 2022-12-12 8:56 ` Krzysztof Kozlowski 2022-12-12 4:55 ` [PATCH v4 3/3] leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs Chuanhong Guo 2 siblings, 1 reply; 17+ messages in thread From: Chuanhong Guo @ 2022-12-12 4:55 UTC (permalink / raw) To: linux-leds Cc: Chuanhong Guo, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list Add dt binding schema for WorldSemi WS2812B driven using SPI bus. Signed-off-by: Chuanhong Guo <gch981213@gmail.com> --- Changes since v1: remove linux driver reference from description remove some obvious descriptions fix unit address regex in multi-led property drop various minItems add maxItems = 1 to reg fix node names and property orders in binding example drop -spi from compatible string add default-brightness Change since v2: drop "this patch" from commit message rename leds to led-controller drop default-brightness and default-intensity Change since v3: reword commit title .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml new file mode 100644 index 000000000000..548c05ac3d31 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml @@ -0,0 +1,116 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: WS2812B LEDs driven using SPI + +maintainers: + - Chuanhong Guo <gch981213@gmail.com> + +description: | + WorldSemi WS2812B is a individually addressable LED chip that can be chained + together and controlled individually using a single wire. + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. + Typical setups includes connecting the data pin of the LED chain to MOSI as + the only device or using CS and MOSI with a tri-state voltage-level shifter + for the data pin. + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct + and the controller needs to send all the bytes continuously. + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + const: worldsemi,ws2812b + + reg: + maxItems: 1 + + spi-max-frequency: + minimum: 2105000 + maximum: 2850000 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +patternProperties: + "^multi-led@[0-9a-f]+$": + type: object + $ref: leds-class-multicolor.yaml# + unevaluatedProperties: false + + properties: + color-index: + description: | + A 3-item array specifying color of each components in this LED. It + should be one of the LED_COLOR_ID_* prefixed definitions from the + header include/dt-bindings/leds/common.h. Defaults to + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> + if unspecified. + $ref: /schemas/types.yaml#/definitions/uint32-array + maxItems: 3 + + 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>; + + led-controller@0 { + compatible = "worldsemi,ws2812b"; + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + spi-max-frequency = <2850000>; + multi-led@0 { + reg = <0>; + color-index = <LED_COLOR_ID_RED LED_COLOR_ID_GREEN LED_COLOR_ID_BLUE>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_STATUS; + function-enumerator = <0>; + }; + multi-led@1 { + reg = <1>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_STATUS; + function-enumerator = <1>; + }; + multi-led@2 { + reg = <2>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_STATUS; + function-enumerator = <2>; + }; + multi-led@3 { + reg = <3>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_STATUS; + function-enumerator = <3>; + }; + }; + }; + +... -- 2.38.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2022-12-12 4:55 ` [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b Chuanhong Guo @ 2022-12-12 8:56 ` Krzysztof Kozlowski 2022-12-23 12:48 ` Lee Jones 2022-12-23 17:19 ` Pavel Machek 0 siblings, 2 replies; 17+ messages in thread From: Krzysztof Kozlowski @ 2022-12-12 8:56 UTC (permalink / raw) To: Chuanhong Guo, linux-leds, Pavel Machek, Lee Jones Cc: Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On 12/12/2022 05:55, Chuanhong Guo wrote: > Add dt binding schema for WorldSemi WS2812B driven using SPI > bus. > > Signed-off-by: Chuanhong Guo <gch981213@gmail.com> > --- > Changes since v1: > remove linux driver reference from description > remove some obvious descriptions > fix unit address regex in multi-led property > drop various minItems > add maxItems = 1 to reg > fix node names and property orders in binding example > drop -spi from compatible string > add default-brightness > > Change since v2: > drop "this patch" from commit message > rename leds to led-controller > drop default-brightness and default-intensity > > Change since v3: > reword commit title > > .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ > 1 file changed, 116 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > new file mode 100644 > index 000000000000..548c05ac3d31 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > @@ -0,0 +1,116 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: WS2812B LEDs driven using SPI > + > +maintainers: > + - Chuanhong Guo <gch981213@gmail.com> > + > +description: | > + WorldSemi WS2812B is a individually addressable LED chip that can be chained > + together and controlled individually using a single wire. > + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. > + Typical setups includes connecting the data pin of the LED chain to MOSI as > + the only device or using CS and MOSI with a tri-state voltage-level shifter > + for the data pin. > + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct > + and the controller needs to send all the bytes continuously. > + > +allOf: > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + const: worldsemi,ws2812b > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + minimum: 2105000 > + maximum: 2850000 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > +patternProperties: > + "^multi-led@[0-9a-f]+$": > + type: object > + $ref: leds-class-multicolor.yaml# > + unevaluatedProperties: false > + > + properties: > + color-index: > + description: | > + A 3-item array specifying color of each components in this LED. It > + should be one of the LED_COLOR_ID_* prefixed definitions from the > + header include/dt-bindings/leds/common.h. Defaults to > + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> > + if unspecified. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + maxItems: 3 In general I am fine with it, although there is still question for adding more multi-color defines in binding headers to replace this property - GRB/RBG/GBR and even more for RGBW. Pavel, Lee, any thoughts from your side? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2022-12-12 8:56 ` Krzysztof Kozlowski @ 2022-12-23 12:48 ` Lee Jones 2022-12-23 17:19 ` Pavel Machek 1 sibling, 0 replies; 17+ messages in thread From: Lee Jones @ 2022-12-23 12:48 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Chuanhong Guo, linux-leds, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On Mon, 12 Dec 2022, Krzysztof Kozlowski wrote: > On 12/12/2022 05:55, Chuanhong Guo wrote: > > Add dt binding schema for WorldSemi WS2812B driven using SPI > > bus. > > > > Signed-off-by: Chuanhong Guo <gch981213@gmail.com> > > --- > > Changes since v1: > > remove linux driver reference from description > > remove some obvious descriptions > > fix unit address regex in multi-led property > > drop various minItems > > add maxItems = 1 to reg > > fix node names and property orders in binding example > > drop -spi from compatible string > > add default-brightness > > > > Change since v2: > > drop "this patch" from commit message > > rename leds to led-controller > > drop default-brightness and default-intensity > > > > Change since v3: > > reword commit title > > > > .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ > > 1 file changed, 116 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > > > diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > new file mode 100644 > > index 000000000000..548c05ac3d31 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > @@ -0,0 +1,116 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: WS2812B LEDs driven using SPI > > + > > +maintainers: > > + - Chuanhong Guo <gch981213@gmail.com> > > + > > +description: | > > + WorldSemi WS2812B is a individually addressable LED chip that can be chained > > + together and controlled individually using a single wire. > > + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. > > + Typical setups includes connecting the data pin of the LED chain to MOSI as > > + the only device or using CS and MOSI with a tri-state voltage-level shifter > > + for the data pin. > > + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct > > + and the controller needs to send all the bytes continuously. > > + > > +allOf: > > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > > + > > +properties: > > + compatible: > > + const: worldsemi,ws2812b > > + > > + reg: > > + maxItems: 1 > > + > > + spi-max-frequency: > > + minimum: 2105000 > > + maximum: 2850000 > > + > > + "#address-cells": > > + const: 1 > > + > > + "#size-cells": > > + const: 0 > > + > > +patternProperties: > > + "^multi-led@[0-9a-f]+$": > > + type: object > > + $ref: leds-class-multicolor.yaml# > > + unevaluatedProperties: false > > + > > + properties: > > + color-index: > > + description: | > > + A 3-item array specifying color of each components in this LED. It > > + should be one of the LED_COLOR_ID_* prefixed definitions from the > > + header include/dt-bindings/leds/common.h. Defaults to > > + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> > > + if unspecified. > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + maxItems: 3 > > In general I am fine with it, although there is still question for > adding more multi-color defines in binding headers to replace this > property - GRB/RBG/GBR and even more for RGBW. > > Pavel, Lee, any thoughts from your side? Nothing from me yet. If Pavel doesn't respond soon, I'll get around to doing a full sweep after the merge-window has closed. Actually, most likely the new year now. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2022-12-12 8:56 ` Krzysztof Kozlowski 2022-12-23 12:48 ` Lee Jones @ 2022-12-23 17:19 ` Pavel Machek 2022-12-24 5:52 ` Chuanhong Guo 2022-12-24 12:53 ` Krzysztof Kozlowski 1 sibling, 2 replies; 17+ messages in thread From: Pavel Machek @ 2022-12-23 17:19 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Chuanhong Guo, linux-leds, Lee Jones, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list [-- Attachment #1: Type: text/plain, Size: 3717 bytes --] Hi! > > Add dt binding schema for WorldSemi WS2812B driven using SPI > > bus. > > > > Signed-off-by: Chuanhong Guo <gch981213@gmail.com> > > --- > > Changes since v1: > > remove linux driver reference from description > > remove some obvious descriptions > > fix unit address regex in multi-led property > > drop various minItems > > add maxItems = 1 to reg > > fix node names and property orders in binding example > > drop -spi from compatible string > > add default-brightness > > > > Change since v2: > > drop "this patch" from commit message > > rename leds to led-controller > > drop default-brightness and default-intensity > > > > Change since v3: > > reword commit title > > > > .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ > > 1 file changed, 116 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > > > diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > new file mode 100644 > > index 000000000000..548c05ac3d31 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > @@ -0,0 +1,116 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: WS2812B LEDs driven using SPI > > + > > +maintainers: > > + - Chuanhong Guo <gch981213@gmail.com> > > + > > +description: | > > + WorldSemi WS2812B is a individually addressable LED chip that can be chained > > + together and controlled individually using a single wire. > > + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. > > + Typical setups includes connecting the data pin of the LED chain to MOSI as > > + the only device or using CS and MOSI with a tri-state voltage-level shifter > > + for the data pin. > > + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct > > + and the controller needs to send all the bytes continuously. > > + > > +allOf: > > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > > + > > +properties: > > + compatible: > > + const: worldsemi,ws2812b > > + > > + reg: > > + maxItems: 1 > > + > > + spi-max-frequency: > > + minimum: 2105000 > > + maximum: 2850000 > > + > > + "#address-cells": > > + const: 1 > > + > > + "#size-cells": > > + const: 0 > > + > > +patternProperties: > > + "^multi-led@[0-9a-f]+$": > > + type: object > > + $ref: leds-class-multicolor.yaml# > > + unevaluatedProperties: false > > + > > + properties: > > + color-index: > > + description: | > > + A 3-item array specifying color of each components in this LED. It > > + should be one of the LED_COLOR_ID_* prefixed definitions from the > > + header include/dt-bindings/leds/common.h. Defaults to > > + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> > > + if unspecified. > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + maxItems: 3 > > In general I am fine with it, although there is still question for > adding more multi-color defines in binding headers to replace this > property - GRB/RBG/GBR and even more for RGBW. > > Pavel, Lee, any thoughts from your side? This really needs to mention the name this hardware is known as -- I believe it is NeoPixel. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2022-12-23 17:19 ` Pavel Machek @ 2022-12-24 5:52 ` Chuanhong Guo 2022-12-24 12:53 ` Krzysztof Kozlowski 1 sibling, 0 replies; 17+ messages in thread From: Chuanhong Guo @ 2022-12-24 5:52 UTC (permalink / raw) To: Pavel Machek Cc: Krzysztof Kozlowski, linux-leds, Lee Jones, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list Hi! On Sat, Dec 24, 2022 at 1:19 AM Pavel Machek <pavel@ucw.cz> wrote: > > In general I am fine with it, although there is still question for > > adding more multi-color defines in binding headers to replace this > > property - GRB/RBG/GBR and even more for RGBW. > > > > Pavel, Lee, any thoughts from your side? > > This really needs to mention the name this hardware is known as -- I > believe it is NeoPixel. I'll do so in the next version. I'm actually waiting for an answer to the comment from Krzysztof (whether I should change color-index) before sending v5 :) -- Regards, Chuanhong Guo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2022-12-23 17:19 ` Pavel Machek 2022-12-24 5:52 ` Chuanhong Guo @ 2022-12-24 12:53 ` Krzysztof Kozlowski 2023-01-09 16:52 ` Lee Jones 1 sibling, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2022-12-24 12:53 UTC (permalink / raw) To: Pavel Machek Cc: Chuanhong Guo, linux-leds, Lee Jones, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On 23/12/2022 18:19, Pavel Machek wrote: > Hi! > >>> Add dt binding schema for WorldSemi WS2812B driven using SPI >>> bus. >>> >>> Signed-off-by: Chuanhong Guo <gch981213@gmail.com> >>> --- >>> Changes since v1: >>> remove linux driver reference from description >>> remove some obvious descriptions >>> fix unit address regex in multi-led property >>> drop various minItems >>> add maxItems = 1 to reg >>> fix node names and property orders in binding example >>> drop -spi from compatible string >>> add default-brightness >>> >>> Change since v2: >>> drop "this patch" from commit message >>> rename leds to led-controller >>> drop default-brightness and default-intensity >>> >>> Change since v3: >>> reword commit title >>> >>> .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ >>> 1 file changed, 116 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml >>> new file mode 100644 >>> index 000000000000..548c05ac3d31 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml >>> @@ -0,0 +1,116 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: WS2812B LEDs driven using SPI >>> + >>> +maintainers: >>> + - Chuanhong Guo <gch981213@gmail.com> >>> + >>> +description: | >>> + WorldSemi WS2812B is a individually addressable LED chip that can be chained >>> + together and controlled individually using a single wire. >>> + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. >>> + Typical setups includes connecting the data pin of the LED chain to MOSI as >>> + the only device or using CS and MOSI with a tri-state voltage-level shifter >>> + for the data pin. >>> + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct >>> + and the controller needs to send all the bytes continuously. >>> + >>> +allOf: >>> + - $ref: /schemas/spi/spi-peripheral-props.yaml# >>> + >>> +properties: >>> + compatible: >>> + const: worldsemi,ws2812b >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + spi-max-frequency: >>> + minimum: 2105000 >>> + maximum: 2850000 >>> + >>> + "#address-cells": >>> + const: 1 >>> + >>> + "#size-cells": >>> + const: 0 >>> + >>> +patternProperties: >>> + "^multi-led@[0-9a-f]+$": >>> + type: object >>> + $ref: leds-class-multicolor.yaml# >>> + unevaluatedProperties: false >>> + >>> + properties: >>> + color-index: >>> + description: | >>> + A 3-item array specifying color of each components in this LED. It >>> + should be one of the LED_COLOR_ID_* prefixed definitions from the >>> + header include/dt-bindings/leds/common.h. Defaults to >>> + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> >>> + if unspecified. >>> + $ref: /schemas/types.yaml#/definitions/uint32-array >>> + maxItems: 3 >> >> In general I am fine with it, although there is still question for >> adding more multi-color defines in binding headers to replace this >> property - GRB/RBG/GBR and even more for RGBW. >> >> Pavel, Lee, any thoughts from your side? > > This really needs to mention the name this hardware is known as -- I > believe it is NeoPixel. We wait here for feedback on colors... The binding is re-implementing color, just because of combinations GRB/RBG/GBR, which could be achieved with new color defines. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2022-12-24 12:53 ` Krzysztof Kozlowski @ 2023-01-09 16:52 ` Lee Jones 2023-01-10 9:24 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: Lee Jones @ 2023-01-09 16:52 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Pavel Machek, Chuanhong Guo, linux-leds, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On Sat, 24 Dec 2022, Krzysztof Kozlowski wrote: > On 23/12/2022 18:19, Pavel Machek wrote: > > Hi! > > > >>> Add dt binding schema for WorldSemi WS2812B driven using SPI > >>> bus. > >>> > >>> Signed-off-by: Chuanhong Guo <gch981213@gmail.com> > >>> --- > >>> Changes since v1: > >>> remove linux driver reference from description > >>> remove some obvious descriptions > >>> fix unit address regex in multi-led property > >>> drop various minItems > >>> add maxItems = 1 to reg > >>> fix node names and property orders in binding example > >>> drop -spi from compatible string > >>> add default-brightness > >>> > >>> Change since v2: > >>> drop "this patch" from commit message > >>> rename leds to led-controller > >>> drop default-brightness and default-intensity > >>> > >>> Change since v3: > >>> reword commit title > >>> > >>> .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ > >>> 1 file changed, 116 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > >>> new file mode 100644 > >>> index 000000000000..548c05ac3d31 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > >>> @@ -0,0 +1,116 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: WS2812B LEDs driven using SPI > >>> + > >>> +maintainers: > >>> + - Chuanhong Guo <gch981213@gmail.com> > >>> + > >>> +description: | > >>> + WorldSemi WS2812B is a individually addressable LED chip that can be chained > >>> + together and controlled individually using a single wire. > >>> + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. > >>> + Typical setups includes connecting the data pin of the LED chain to MOSI as > >>> + the only device or using CS and MOSI with a tri-state voltage-level shifter > >>> + for the data pin. > >>> + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct > >>> + and the controller needs to send all the bytes continuously. > >>> + > >>> +allOf: > >>> + - $ref: /schemas/spi/spi-peripheral-props.yaml# > >>> + > >>> +properties: > >>> + compatible: > >>> + const: worldsemi,ws2812b > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> + spi-max-frequency: > >>> + minimum: 2105000 > >>> + maximum: 2850000 > >>> + > >>> + "#address-cells": > >>> + const: 1 > >>> + > >>> + "#size-cells": > >>> + const: 0 > >>> + > >>> +patternProperties: > >>> + "^multi-led@[0-9a-f]+$": > >>> + type: object > >>> + $ref: leds-class-multicolor.yaml# > >>> + unevaluatedProperties: false > >>> + > >>> + properties: > >>> + color-index: Why "index"? Isn't it just an array of colours rather than an index into something? > >>> + description: | > >>> + A 3-item array specifying color of each components in this LED. It Why are you forcing this to 3? Surely there are multi-colour LEDs containing more or less colours? > >>> + should be one of the LED_COLOR_ID_* prefixed definitions from the > >>> + header include/dt-bindings/leds/common.h. Defaults to Isn't "include" a Linuxisum? > >>> + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> > >>> + if unspecified. > >>> + $ref: /schemas/types.yaml#/definitions/uint32-array > >>> + maxItems: 3 > >> > >> In general I am fine with it, although there is still question for > >> adding more multi-color defines in binding headers to replace this > >> property - GRB/RBG/GBR and even more for RGBW. > >> > >> Pavel, Lee, any thoughts from your side? > > > > This really needs to mention the name this hardware is known as -- I > > believe it is NeoPixel. > > We wait here for feedback on colors... The binding is re-implementing > color, just because of combinations GRB/RBG/GBR, which could be achieved > with new color defines. Sure, but where does that end? How many permutations are there likely to be? An unlimited array has more of a chance of standing the test of time. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2023-01-09 16:52 ` Lee Jones @ 2023-01-10 9:24 ` Krzysztof Kozlowski 2023-01-10 10:21 ` Lee Jones 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-01-10 9:24 UTC (permalink / raw) To: Lee Jones Cc: Pavel Machek, Chuanhong Guo, linux-leds, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On 09/01/2023 17:52, Lee Jones wrote: > On Sat, 24 Dec 2022, Krzysztof Kozlowski wrote: > >> On 23/12/2022 18:19, Pavel Machek wrote: >>> Hi! >>> >>>>> Add dt binding schema for WorldSemi WS2812B driven using SPI >>>>> bus. >>>>> >>>>> Signed-off-by: Chuanhong Guo <gch981213@gmail.com> >>>>> --- >>>>> Changes since v1: >>>>> remove linux driver reference from description >>>>> remove some obvious descriptions >>>>> fix unit address regex in multi-led property >>>>> drop various minItems >>>>> add maxItems = 1 to reg >>>>> fix node names and property orders in binding example >>>>> drop -spi from compatible string >>>>> add default-brightness >>>>> >>>>> Change since v2: >>>>> drop "this patch" from commit message >>>>> rename leds to led-controller >>>>> drop default-brightness and default-intensity >>>>> >>>>> Change since v3: >>>>> reword commit title >>>>> >>>>> .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ >>>>> 1 file changed, 116 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml >>>>> new file mode 100644 >>>>> index 000000000000..548c05ac3d31 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml >>>>> @@ -0,0 +1,116 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>> +%YAML 1.2 >>>>> +--- >>>>> +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>> + >>>>> +title: WS2812B LEDs driven using SPI >>>>> + >>>>> +maintainers: >>>>> + - Chuanhong Guo <gch981213@gmail.com> >>>>> + >>>>> +description: | >>>>> + WorldSemi WS2812B is a individually addressable LED chip that can be chained >>>>> + together and controlled individually using a single wire. >>>>> + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. >>>>> + Typical setups includes connecting the data pin of the LED chain to MOSI as >>>>> + the only device or using CS and MOSI with a tri-state voltage-level shifter >>>>> + for the data pin. >>>>> + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct >>>>> + and the controller needs to send all the bytes continuously. >>>>> + >>>>> +allOf: >>>>> + - $ref: /schemas/spi/spi-peripheral-props.yaml# >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + const: worldsemi,ws2812b >>>>> + >>>>> + reg: >>>>> + maxItems: 1 >>>>> + >>>>> + spi-max-frequency: >>>>> + minimum: 2105000 >>>>> + maximum: 2850000 >>>>> + >>>>> + "#address-cells": >>>>> + const: 1 >>>>> + >>>>> + "#size-cells": >>>>> + const: 0 >>>>> + >>>>> +patternProperties: >>>>> + "^multi-led@[0-9a-f]+$": >>>>> + type: object >>>>> + $ref: leds-class-multicolor.yaml# >>>>> + unevaluatedProperties: false >>>>> + >>>>> + properties: >>>>> + color-index: > > Why "index"? > > Isn't it just an array of colours rather than an index into something? Yeah. > >>>>> + description: | >>>>> + A 3-item array specifying color of each components in this LED. It > > Why are you forcing this to 3? > > Surely there are multi-colour LEDs containing more or less colours? For this device, because it has only tuples of three. > >>>>> + should be one of the LED_COLOR_ID_* prefixed definitions from the >>>>> + header include/dt-bindings/leds/common.h. Defaults to > > Isn't "include" a Linuxisum? No, better to have full paths, so automated tools can validate them. If we ever decide to drop it, we can also make a easier search&replace for the pattern starting with include/. > >>>>> + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> >>>>> + if unspecified. >>>>> + $ref: /schemas/types.yaml#/definitions/uint32-array >>>>> + maxItems: 3 >>>> >>>> In general I am fine with it, although there is still question for >>>> adding more multi-color defines in binding headers to replace this >>>> property - GRB/RBG/GBR and even more for RGBW. >>>> >>>> Pavel, Lee, any thoughts from your side? >>> >>> This really needs to mention the name this hardware is known as -- I >>> believe it is NeoPixel. >> >> We wait here for feedback on colors... The binding is re-implementing >> color, just because of combinations GRB/RBG/GBR, which could be achieved >> with new color defines. > > Sure, but where does that end? > > How many permutations are there likely to be? For light emitting devices, RGB seems to be used for so long, that I don't expect more permutations (e.g. CMY). On the other hand, someone might create LED strip with whatever colors, so maybe indeed better to allow any variations as in array. > An unlimited array has more of a chance of standing the test of time. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2023-01-10 9:24 ` Krzysztof Kozlowski @ 2023-01-10 10:21 ` Lee Jones 2023-01-11 18:53 ` Chuanhong Guo 0 siblings, 1 reply; 17+ messages in thread From: Lee Jones @ 2023-01-10 10:21 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Pavel Machek, Chuanhong Guo, linux-leds, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On Tue, 10 Jan 2023, Krzysztof Kozlowski wrote: > On 09/01/2023 17:52, Lee Jones wrote: > > On Sat, 24 Dec 2022, Krzysztof Kozlowski wrote: > > > >> On 23/12/2022 18:19, Pavel Machek wrote: > >>> Hi! > >>> > >>>>> Add dt binding schema for WorldSemi WS2812B driven using SPI > >>>>> bus. > >>>>> > >>>>> Signed-off-by: Chuanhong Guo <gch981213@gmail.com> > >>>>> --- > >>>>> Changes since v1: > >>>>> remove linux driver reference from description > >>>>> remove some obvious descriptions > >>>>> fix unit address regex in multi-led property > >>>>> drop various minItems > >>>>> add maxItems = 1 to reg > >>>>> fix node names and property orders in binding example > >>>>> drop -spi from compatible string > >>>>> add default-brightness > >>>>> > >>>>> Change since v2: > >>>>> drop "this patch" from commit message > >>>>> rename leds to led-controller > >>>>> drop default-brightness and default-intensity > >>>>> > >>>>> Change since v3: > >>>>> reword commit title > >>>>> > >>>>> .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ > >>>>> 1 file changed, 116 insertions(+) > >>>>> create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > >>>>> new file mode 100644 > >>>>> index 000000000000..548c05ac3d31 > >>>>> --- /dev/null > >>>>> +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > >>>>> @@ -0,0 +1,116 @@ > >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>>> +%YAML 1.2 > >>>>> +--- > >>>>> +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# > >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>>> + > >>>>> +title: WS2812B LEDs driven using SPI > >>>>> + > >>>>> +maintainers: > >>>>> + - Chuanhong Guo <gch981213@gmail.com> > >>>>> + > >>>>> +description: | > >>>>> + WorldSemi WS2812B is a individually addressable LED chip that can be chained > >>>>> + together and controlled individually using a single wire. > >>>>> + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. > >>>>> + Typical setups includes connecting the data pin of the LED chain to MOSI as > >>>>> + the only device or using CS and MOSI with a tri-state voltage-level shifter > >>>>> + for the data pin. > >>>>> + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct > >>>>> + and the controller needs to send all the bytes continuously. > >>>>> + > >>>>> +allOf: > >>>>> + - $ref: /schemas/spi/spi-peripheral-props.yaml# > >>>>> + > >>>>> +properties: > >>>>> + compatible: > >>>>> + const: worldsemi,ws2812b > >>>>> + > >>>>> + reg: > >>>>> + maxItems: 1 > >>>>> + > >>>>> + spi-max-frequency: > >>>>> + minimum: 2105000 > >>>>> + maximum: 2850000 > >>>>> + > >>>>> + "#address-cells": > >>>>> + const: 1 > >>>>> + > >>>>> + "#size-cells": > >>>>> + const: 0 > >>>>> + > >>>>> +patternProperties: > >>>>> + "^multi-led@[0-9a-f]+$": > >>>>> + type: object > >>>>> + $ref: leds-class-multicolor.yaml# > >>>>> + unevaluatedProperties: false > >>>>> + > >>>>> + properties: > >>>>> + color-index: > > > > Why "index"? > > > > Isn't it just an array of colours rather than an index into something? > > Yeah. > > > > >>>>> + description: | > >>>>> + A 3-item array specifying color of each components in this LED. It > > > > Why are you forcing this to 3? > > > > Surely there are multi-colour LEDs containing more or less colours? > > For this device, because it has only tuples of three. This doesn't looks like a device specific property to me. If this is not going to be used by any other device, shouldn't it contain a prefix? > >>>>> + should be one of the LED_COLOR_ID_* prefixed definitions from the > >>>>> + header include/dt-bindings/leds/common.h. Defaults to > > > > Isn't "include" a Linuxisum? > > No, better to have full paths, so automated tools can validate them. If > we ever decide to drop it, we can also make a easier search&replace for > the pattern starting with include/. Very well. It's your train set. :) > >>>>> + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> > >>>>> + if unspecified. > >>>>> + $ref: /schemas/types.yaml#/definitions/uint32-array > >>>>> + maxItems: 3 > >>>> > >>>> In general I am fine with it, although there is still question for > >>>> adding more multi-color defines in binding headers to replace this > >>>> property - GRB/RBG/GBR and even more for RGBW. > >>>> > >>>> Pavel, Lee, any thoughts from your side? > >>> > >>> This really needs to mention the name this hardware is known as -- I > >>> believe it is NeoPixel. > >> > >> We wait here for feedback on colors... The binding is re-implementing > >> color, just because of combinations GRB/RBG/GBR, which could be achieved > >> with new color defines. > > > > Sure, but where does that end? > > > > How many permutations are there likely to be? > > For light emitting devices, RGB seems to be used for so long, that I > don't expect more permutations (e.g. CMY). On the other hand, someone > might create LED strip with whatever colors, so maybe indeed better to > allow any variations as in array. Even you suggested variation: "even more for RGBW". Caveat: as you are well aware, "I'm new here", so my input is no more informed or valuable as yours at this point. I'm just calling it as I see it. If you have strong opinions and they differ wildly from mine, we may have to take intervention from Pavel. As it stands, the property, although slightly restricted IMHO, appears fine. > > An unlimited array has more of a chance of standing the test of time. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2023-01-10 10:21 ` Lee Jones @ 2023-01-11 18:53 ` Chuanhong Guo 2023-01-12 9:16 ` Krzysztof Kozlowski 2023-01-13 14:56 ` Lee Jones 0 siblings, 2 replies; 17+ messages in thread From: Chuanhong Guo @ 2023-01-11 18:53 UTC (permalink / raw) To: Lee Jones Cc: Krzysztof Kozlowski, Pavel Machek, linux-leds, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list Hi! On Tue, Jan 10, 2023 at 6:21 PM Lee Jones <lee@kernel.org> wrote: > > On Tue, 10 Jan 2023, Krzysztof Kozlowski wrote: > > > On 09/01/2023 17:52, Lee Jones wrote: > > > On Sat, 24 Dec 2022, Krzysztof Kozlowski wrote: > > > > > >> On 23/12/2022 18:19, Pavel Machek wrote: > > >>> Hi! > > >>> > > >>>>> Add dt binding schema for WorldSemi WS2812B driven using SPI > > >>>>> bus. > > >>>>> > > >>>>> Signed-off-by: Chuanhong Guo <gch981213@gmail.com> > > >>>>> --- > > >>>>> Changes since v1: > > >>>>> remove linux driver reference from description > > >>>>> remove some obvious descriptions > > >>>>> fix unit address regex in multi-led property > > >>>>> drop various minItems > > >>>>> add maxItems = 1 to reg > > >>>>> fix node names and property orders in binding example > > >>>>> drop -spi from compatible string > > >>>>> add default-brightness > > >>>>> > > >>>>> Change since v2: > > >>>>> drop "this patch" from commit message > > >>>>> rename leds to led-controller > > >>>>> drop default-brightness and default-intensity > > >>>>> > > >>>>> Change since v3: > > >>>>> reword commit title > > >>>>> > > >>>>> .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ > > >>>>> 1 file changed, 116 insertions(+) > > >>>>> create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > >>>>> > > >>>>> diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > >>>>> new file mode 100644 > > >>>>> index 000000000000..548c05ac3d31 > > >>>>> --- /dev/null > > >>>>> +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > >>>>> @@ -0,0 +1,116 @@ > > >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > >>>>> +%YAML 1.2 > > >>>>> +--- > > >>>>> +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# > > >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > > >>>>> + > > >>>>> +title: WS2812B LEDs driven using SPI > > >>>>> + > > >>>>> +maintainers: > > >>>>> + - Chuanhong Guo <gch981213@gmail.com> > > >>>>> + > > >>>>> +description: | > > >>>>> + WorldSemi WS2812B is a individually addressable LED chip that can be chained > > >>>>> + together and controlled individually using a single wire. > > >>>>> + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. > > >>>>> + Typical setups includes connecting the data pin of the LED chain to MOSI as > > >>>>> + the only device or using CS and MOSI with a tri-state voltage-level shifter > > >>>>> + for the data pin. > > >>>>> + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct > > >>>>> + and the controller needs to send all the bytes continuously. > > >>>>> + > > >>>>> +allOf: > > >>>>> + - $ref: /schemas/spi/spi-peripheral-props.yaml# > > >>>>> + > > >>>>> +properties: > > >>>>> + compatible: > > >>>>> + const: worldsemi,ws2812b > > >>>>> + > > >>>>> + reg: > > >>>>> + maxItems: 1 > > >>>>> + > > >>>>> + spi-max-frequency: > > >>>>> + minimum: 2105000 > > >>>>> + maximum: 2850000 > > >>>>> + > > >>>>> + "#address-cells": > > >>>>> + const: 1 > > >>>>> + > > >>>>> + "#size-cells": > > >>>>> + const: 0 > > >>>>> + > > >>>>> +patternProperties: > > >>>>> + "^multi-led@[0-9a-f]+$": > > >>>>> + type: object > > >>>>> + $ref: leds-class-multicolor.yaml# > > >>>>> + unevaluatedProperties: false > > >>>>> + > > >>>>> + properties: > > >>>>> + color-index: > > > > > > Why "index"? > > > > > > Isn't it just an array of colours rather than an index into something? > > > > Yeah. The corresponding sysfs interface is called 'multi_index' so I called it this way. > > > > > > > >>>>> + description: | > > >>>>> + A 3-item array specifying color of each components in this LED. It > > > > > > Why are you forcing this to 3? > > > > > > Surely there are multi-colour LEDs containing more or less colours? > > > > For this device, because it has only tuples of three. WS2812B has 3 colors per chip. There are chips using a similar protocol with 4 colors but my current driver is hard-coded to support exactly 3 colors. > This doesn't looks like a device specific property to me. > > If this is not going to be used by any other device, shouldn't it > contain a prefix? > > > >>>>> + should be one of the LED_COLOR_ID_* prefixed definitions from the > > >>>>> + header include/dt-bindings/leds/common.h. Defaults to > > > > > > Isn't "include" a Linuxisum? > > > > No, better to have full paths, so automated tools can validate them. If > > we ever decide to drop it, we can also make a easier search&replace for > > the pattern starting with include/. > > Very well. It's your train set. :) > > > >>>>> + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> > > >>>>> + if unspecified. > > >>>>> + $ref: /schemas/types.yaml#/definitions/uint32-array > > >>>>> + maxItems: 3 > > >>>> > > >>>> In general I am fine with it, although there is still question for > > >>>> adding more multi-color defines in binding headers to replace this > > >>>> property - GRB/RBG/GBR and even more for RGBW. > > >>>> > > >>>> Pavel, Lee, any thoughts from your side? > > >>> > > >>> This really needs to mention the name this hardware is known as -- I > > >>> believe it is NeoPixel. > > >> > > >> We wait here for feedback on colors... The binding is re-implementing > > >> color, just because of combinations GRB/RBG/GBR, which could be achieved > > >> with new color defines. > > > > > > Sure, but where does that end? > > > > > > How many permutations are there likely to be? > > > > For light emitting devices, RGB seems to be used for so long, that I > > don't expect more permutations (e.g. CMY). On the other hand, someone > > might create LED strip with whatever colors, so maybe indeed better to > > allow any variations as in array. > > Even you suggested variation: "even more for RGBW". > > Caveat: as you are well aware, "I'm new here", so my input is no more > informed or valuable as yours at this point. I'm just calling it as I > see it. If you have strong opinions and they differ wildly from mine, > we may have to take intervention from Pavel. As it stands, the > property, although slightly restricted IMHO, appears fine. > > > > An unlimited array has more of a chance of standing the test of time. I have another idea that avoids this whole conversation: Abandon color-index completely and determine colors with compatible string and platform data. My original idea of this property is to support WS2812B and its clones with different colors under the same compatible string. Technically genuine WS2812Bs only come with GRB colors and the clones should have their own chip names (e.g. xiaomi,hm0807a for an RGB clone and <unknown vendor>,sk6812 for an RGBW one.). It's reasonable to have one compatible string per chip for colors, a chip-specific property. Also, adding more compatible devices to a driver is less invasive than adding more definitions to leds/common.h What do you think? -- Regards, Chuanhong Guo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2023-01-11 18:53 ` Chuanhong Guo @ 2023-01-12 9:16 ` Krzysztof Kozlowski 2023-01-13 14:56 ` Lee Jones 1 sibling, 0 replies; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-01-12 9:16 UTC (permalink / raw) To: Chuanhong Guo, Lee Jones Cc: Pavel Machek, linux-leds, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On 11/01/2023 19:53, Chuanhong Guo wrote: > Hi! > > On Tue, Jan 10, 2023 at 6:21 PM Lee Jones <lee@kernel.org> wrote: >> >> On Tue, 10 Jan 2023, Krzysztof Kozlowski wrote: >> >>> On 09/01/2023 17:52, Lee Jones wrote: >>>> On Sat, 24 Dec 2022, Krzysztof Kozlowski wrote: >>>> >>>>> On 23/12/2022 18:19, Pavel Machek wrote: >>>>>> Hi! >>>>>> >>>>>>>> Add dt binding schema for WorldSemi WS2812B driven using SPI >>>>>>>> bus. >>>>>>>> >>>>>>>> Signed-off-by: Chuanhong Guo <gch981213@gmail.com> >>>>>>>> --- >>>>>>>> Changes since v1: >>>>>>>> remove linux driver reference from description >>>>>>>> remove some obvious descriptions >>>>>>>> fix unit address regex in multi-led property >>>>>>>> drop various minItems >>>>>>>> add maxItems = 1 to reg >>>>>>>> fix node names and property orders in binding example >>>>>>>> drop -spi from compatible string >>>>>>>> add default-brightness >>>>>>>> >>>>>>>> Change since v2: >>>>>>>> drop "this patch" from commit message >>>>>>>> rename leds to led-controller >>>>>>>> drop default-brightness and default-intensity >>>>>>>> >>>>>>>> Change since v3: >>>>>>>> reword commit title >>>>>>>> >>>>>>>> .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ >>>>>>>> 1 file changed, 116 insertions(+) >>>>>>>> create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml >>>>>>>> >>>>>>>> diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml >>>>>>>> new file mode 100644 >>>>>>>> index 000000000000..548c05ac3d31 >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml >>>>>>>> @@ -0,0 +1,116 @@ >>>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>>>>> +%YAML 1.2 >>>>>>>> +--- >>>>>>>> +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# >>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>>>>> + >>>>>>>> +title: WS2812B LEDs driven using SPI >>>>>>>> + >>>>>>>> +maintainers: >>>>>>>> + - Chuanhong Guo <gch981213@gmail.com> >>>>>>>> + >>>>>>>> +description: | >>>>>>>> + WorldSemi WS2812B is a individually addressable LED chip that can be chained >>>>>>>> + together and controlled individually using a single wire. >>>>>>>> + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. >>>>>>>> + Typical setups includes connecting the data pin of the LED chain to MOSI as >>>>>>>> + the only device or using CS and MOSI with a tri-state voltage-level shifter >>>>>>>> + for the data pin. >>>>>>>> + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct >>>>>>>> + and the controller needs to send all the bytes continuously. >>>>>>>> + >>>>>>>> +allOf: >>>>>>>> + - $ref: /schemas/spi/spi-peripheral-props.yaml# >>>>>>>> + >>>>>>>> +properties: >>>>>>>> + compatible: >>>>>>>> + const: worldsemi,ws2812b >>>>>>>> + >>>>>>>> + reg: >>>>>>>> + maxItems: 1 >>>>>>>> + >>>>>>>> + spi-max-frequency: >>>>>>>> + minimum: 2105000 >>>>>>>> + maximum: 2850000 >>>>>>>> + >>>>>>>> + "#address-cells": >>>>>>>> + const: 1 >>>>>>>> + >>>>>>>> + "#size-cells": >>>>>>>> + const: 0 >>>>>>>> + >>>>>>>> +patternProperties: >>>>>>>> + "^multi-led@[0-9a-f]+$": >>>>>>>> + type: object >>>>>>>> + $ref: leds-class-multicolor.yaml# >>>>>>>> + unevaluatedProperties: false >>>>>>>> + >>>>>>>> + properties: >>>>>>>> + color-index: >>>> >>>> Why "index"? >>>> >>>> Isn't it just an array of colours rather than an index into something? >>> >>> Yeah. > > The corresponding sysfs interface is called 'multi_index' so I called > it this way. > >>> >>>> >>>>>>>> + description: | >>>>>>>> + A 3-item array specifying color of each components in this LED. It >>>> >>>> Why are you forcing this to 3? >>>> >>>> Surely there are multi-colour LEDs containing more or less colours? >>> >>> For this device, because it has only tuples of three. > > WS2812B has 3 colors per chip. There are chips using a similar protocol > with 4 colors but my current driver is hard-coded to support exactly 3 colors. > >> This doesn't looks like a device specific property to me. >> >> If this is not going to be used by any other device, shouldn't it >> contain a prefix? >> >>>>>>>> + should be one of the LED_COLOR_ID_* prefixed definitions from the >>>>>>>> + header include/dt-bindings/leds/common.h. Defaults to >>>> >>>> Isn't "include" a Linuxisum? >>> >>> No, better to have full paths, so automated tools can validate them. If >>> we ever decide to drop it, we can also make a easier search&replace for >>> the pattern starting with include/. >> >> Very well. It's your train set. :) >> >>>>>>>> + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> >>>>>>>> + if unspecified. >>>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32-array >>>>>>>> + maxItems: 3 >>>>>>> >>>>>>> In general I am fine with it, although there is still question for >>>>>>> adding more multi-color defines in binding headers to replace this >>>>>>> property - GRB/RBG/GBR and even more for RGBW. >>>>>>> >>>>>>> Pavel, Lee, any thoughts from your side? >>>>>> >>>>>> This really needs to mention the name this hardware is known as -- I >>>>>> believe it is NeoPixel. >>>>> >>>>> We wait here for feedback on colors... The binding is re-implementing >>>>> color, just because of combinations GRB/RBG/GBR, which could be achieved >>>>> with new color defines. >>>> >>>> Sure, but where does that end? >>>> >>>> How many permutations are there likely to be? >>> >>> For light emitting devices, RGB seems to be used for so long, that I >>> don't expect more permutations (e.g. CMY). On the other hand, someone >>> might create LED strip with whatever colors, so maybe indeed better to >>> allow any variations as in array. >> >> Even you suggested variation: "even more for RGBW". >> >> Caveat: as you are well aware, "I'm new here", so my input is no more >> informed or valuable as yours at this point. I'm just calling it as I >> see it. If you have strong opinions and they differ wildly from mine, >> we may have to take intervention from Pavel. As it stands, the >> property, although slightly restricted IMHO, appears fine. >> >>>> An unlimited array has more of a chance of standing the test of time. > > I have another idea that avoids this whole conversation: Abandon > color-index completely and determine colors with compatible string > and platform data. > My original idea of this property is to support WS2812B and its clones > with different colors under the same compatible string. Technically > genuine WS2812Bs only come with GRB colors and the clones > should have their own chip names (e.g. xiaomi,hm0807a for an RGB > clone and <unknown vendor>,sk6812 for an RGBW one.). It's > reasonable to have one compatible string per chip for colors, a > chip-specific property. Also, adding more compatible devices to a > driver is less invasive than adding more definitions to leds/common.h > What do you think? This sounds good. However it should not lead to compatibles like worldsemi,ws2812b-rgb, worldsemi,ws2812b-bgr etc. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2023-01-11 18:53 ` Chuanhong Guo 2023-01-12 9:16 ` Krzysztof Kozlowski @ 2023-01-13 14:56 ` Lee Jones 2023-01-14 12:29 ` Chuanhong Guo 1 sibling, 1 reply; 17+ messages in thread From: Lee Jones @ 2023-01-13 14:56 UTC (permalink / raw) To: Chuanhong Guo Cc: Krzysztof Kozlowski, Pavel Machek, linux-leds, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On Thu, 12 Jan 2023, Chuanhong Guo wrote: > Hi! > > On Tue, Jan 10, 2023 at 6:21 PM Lee Jones <lee@kernel.org> wrote: > > > > On Tue, 10 Jan 2023, Krzysztof Kozlowski wrote: > > > > > On 09/01/2023 17:52, Lee Jones wrote: > > > > On Sat, 24 Dec 2022, Krzysztof Kozlowski wrote: > > > > > > > >> On 23/12/2022 18:19, Pavel Machek wrote: > > > >>> Hi! > > > >>> > > > >>>>> Add dt binding schema for WorldSemi WS2812B driven using SPI > > > >>>>> bus. > > > >>>>> > > > >>>>> Signed-off-by: Chuanhong Guo <gch981213@gmail.com> > > > >>>>> --- > > > >>>>> Changes since v1: > > > >>>>> remove linux driver reference from description > > > >>>>> remove some obvious descriptions > > > >>>>> fix unit address regex in multi-led property > > > >>>>> drop various minItems > > > >>>>> add maxItems = 1 to reg > > > >>>>> fix node names and property orders in binding example > > > >>>>> drop -spi from compatible string > > > >>>>> add default-brightness > > > >>>>> > > > >>>>> Change since v2: > > > >>>>> drop "this patch" from commit message > > > >>>>> rename leds to led-controller > > > >>>>> drop default-brightness and default-intensity > > > >>>>> > > > >>>>> Change since v3: > > > >>>>> reword commit title > > > >>>>> > > > >>>>> .../bindings/leds/worldsemi,ws2812b.yaml | 116 ++++++++++++++++++ > > > >>>>> 1 file changed, 116 insertions(+) > > > >>>>> create mode 100644 Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > > >>>>> > > > >>>>> diff --git a/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > > >>>>> new file mode 100644 > > > >>>>> index 000000000000..548c05ac3d31 > > > >>>>> --- /dev/null > > > >>>>> +++ b/Documentation/devicetree/bindings/leds/worldsemi,ws2812b.yaml > > > >>>>> @@ -0,0 +1,116 @@ > > > >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > >>>>> +%YAML 1.2 > > > >>>>> +--- > > > >>>>> +$id: http://devicetree.org/schemas/leds/worldsemi,ws2812b.yaml# > > > >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > >>>>> + > > > >>>>> +title: WS2812B LEDs driven using SPI > > > >>>>> + > > > >>>>> +maintainers: > > > >>>>> + - Chuanhong Guo <gch981213@gmail.com> > > > >>>>> + > > > >>>>> +description: | > > > >>>>> + WorldSemi WS2812B is a individually addressable LED chip that can be chained > > > >>>>> + together and controlled individually using a single wire. > > > >>>>> + This binding describes a chain of WS2812B LEDs connected to the SPI MOSI pin. > > > >>>>> + Typical setups includes connecting the data pin of the LED chain to MOSI as > > > >>>>> + the only device or using CS and MOSI with a tri-state voltage-level shifter > > > >>>>> + for the data pin. > > > >>>>> + The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct > > > >>>>> + and the controller needs to send all the bytes continuously. > > > >>>>> + > > > >>>>> +allOf: > > > >>>>> + - $ref: /schemas/spi/spi-peripheral-props.yaml# > > > >>>>> + > > > >>>>> +properties: > > > >>>>> + compatible: > > > >>>>> + const: worldsemi,ws2812b > > > >>>>> + > > > >>>>> + reg: > > > >>>>> + maxItems: 1 > > > >>>>> + > > > >>>>> + spi-max-frequency: > > > >>>>> + minimum: 2105000 > > > >>>>> + maximum: 2850000 > > > >>>>> + > > > >>>>> + "#address-cells": > > > >>>>> + const: 1 > > > >>>>> + > > > >>>>> + "#size-cells": > > > >>>>> + const: 0 > > > >>>>> + > > > >>>>> +patternProperties: > > > >>>>> + "^multi-led@[0-9a-f]+$": > > > >>>>> + type: object > > > >>>>> + $ref: leds-class-multicolor.yaml# > > > >>>>> + unevaluatedProperties: false > > > >>>>> + > > > >>>>> + properties: > > > >>>>> + color-index: > > > > > > > > Why "index"? > > > > > > > > Isn't it just an array of colours rather than an index into something? > > > > > > Yeah. > > The corresponding sysfs interface is called 'multi_index' so I called > it this way. DT nomenclature should not be affected by Linuxisums. Please err towards data-sheet and H/W nomenclature. > > > >>>>> + description: | > > > >>>>> + A 3-item array specifying color of each components in this LED. It > > > > > > > > Why are you forcing this to 3? > > > > > > > > Surely there are multi-colour LEDs containing more or less colours? > > > > > > For this device, because it has only tuples of three. > > WS2812B has 3 colors per chip. There are chips using a similar protocol > with 4 colors but my current driver is hard-coded to support exactly 3 colors. So the description is for 'this device' rather than any re-use. And the handling of this property is only contained in your driver? In which case, my understanding is that you should use a prefix. > > This doesn't looks like a device specific property to me. > > > > If this is not going to be used by any other device, shouldn't it > > contain a prefix? > > > > > >>>>> + should be one of the LED_COLOR_ID_* prefixed definitions from the > > > >>>>> + header include/dt-bindings/leds/common.h. Defaults to > > > > > > > > Isn't "include" a Linuxisum? > > > > > > No, better to have full paths, so automated tools can validate them. If > > > we ever decide to drop it, we can also make a easier search&replace for > > > the pattern starting with include/. > > > > Very well. It's your train set. :) > > > > > >>>>> + <LED_COLOR_ID_GREEN LED_COLOR_ID_RED LED_COLOR_ID_BLUE> > > > >>>>> + if unspecified. > > > >>>>> + $ref: /schemas/types.yaml#/definitions/uint32-array > > > >>>>> + maxItems: 3 > > > >>>> > > > >>>> In general I am fine with it, although there is still question for > > > >>>> adding more multi-color defines in binding headers to replace this > > > >>>> property - GRB/RBG/GBR and even more for RGBW. > > > >>>> > > > >>>> Pavel, Lee, any thoughts from your side? > > > >>> > > > >>> This really needs to mention the name this hardware is known as -- I > > > >>> believe it is NeoPixel. > > > >> > > > >> We wait here for feedback on colors... The binding is re-implementing > > > >> color, just because of combinations GRB/RBG/GBR, which could be achieved > > > >> with new color defines. > > > > > > > > Sure, but where does that end? > > > > > > > > How many permutations are there likely to be? > > > > > > For light emitting devices, RGB seems to be used for so long, that I > > > don't expect more permutations (e.g. CMY). On the other hand, someone > > > might create LED strip with whatever colors, so maybe indeed better to > > > allow any variations as in array. > > > > Even you suggested variation: "even more for RGBW". > > > > Caveat: as you are well aware, "I'm new here", so my input is no more > > informed or valuable as yours at this point. I'm just calling it as I > > see it. If you have strong opinions and they differ wildly from mine, > > we may have to take intervention from Pavel. As it stands, the > > property, although slightly restricted IMHO, appears fine. > > > > > > An unlimited array has more of a chance of standing the test of time. > > I have another idea that avoids this whole conversation: Abandon > color-index completely and determine colors with compatible string > and platform data. > My original idea of this property is to support WS2812B and its clones > with different colors under the same compatible string. Technically > genuine WS2812Bs only come with GRB colors and the clones > should have their own chip names (e.g. xiaomi,hm0807a for an RGB > clone and <unknown vendor>,sk6812 for an RGBW one.). It's > reasonable to have one compatible string per chip for colors, a > chip-specific property. Also, adding more compatible devices to a > driver is less invasive than adding more definitions to leds/common.h > What do you think? Yes it's reasonable to have a compatible string per device and yes, you can do matching (in C) based on compatible string, so this sounds reasonable to me. It also sounds reasonable to describe the H/W (inc. colours along with any ordering, if that's important) inside the DT. Your call I guess. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2023-01-13 14:56 ` Lee Jones @ 2023-01-14 12:29 ` Chuanhong Guo 2023-01-19 14:46 ` Lee Jones 0 siblings, 1 reply; 17+ messages in thread From: Chuanhong Guo @ 2023-01-14 12:29 UTC (permalink / raw) To: Lee Jones Cc: Krzysztof Kozlowski, Pavel Machek, linux-leds, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list Hi! On Fri, Jan 13, 2023 at 10:57 PM Lee Jones <lee@kernel.org> wrote: > [...] > So the description is for 'this device' rather than any re-use. > > And the handling of this property is only contained in your driver? > > In which case, my understanding is that you should use a prefix. (Just out of curiosity. I don't want this property now.) My understanding is that a vendor prefix means this property is for devices by this vendor. However color-index is for supporting clones, which are chips not made by this vendor. Does a vendor prefix really apply in this case? -- Regards, Chuanhong Guo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b 2023-01-14 12:29 ` Chuanhong Guo @ 2023-01-19 14:46 ` Lee Jones 0 siblings, 0 replies; 17+ messages in thread From: Lee Jones @ 2023-01-19 14:46 UTC (permalink / raw) To: Chuanhong Guo Cc: Krzysztof Kozlowski, Pavel Machek, linux-leds, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On Sat, 14 Jan 2023, Chuanhong Guo wrote: > Hi! > > On Fri, Jan 13, 2023 at 10:57 PM Lee Jones <lee@kernel.org> wrote: > > [...] > > So the description is for 'this device' rather than any re-use. > > > > And the handling of this property is only contained in your driver? > > > > In which case, my understanding is that you should use a prefix. > > (Just out of curiosity. I don't want this property now.) > > My understanding is that a vendor prefix means this property > is for devices by this vendor. However color-index is for supporting > clones, which are chips not made by this vendor. Does a vendor > prefix really apply in this case? No idea. Sounds like a grey area. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 3/3] leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs 2022-12-12 4:55 [PATCH v4 0/3] leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs Chuanhong Guo 2022-12-12 4:55 ` [PATCH v4 1/3] dt-bindings: vendor-prefixes: add an entry for WorldSemi Chuanhong Guo 2022-12-12 4:55 ` [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b Chuanhong Guo @ 2022-12-12 4:55 ` Chuanhong Guo 2 siblings, 0 replies; 17+ messages in thread From: Chuanhong Guo @ 2022-12-12 4:55 UTC (permalink / raw) To: linux-leds Cc: Chuanhong Guo, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Stanislav Jakubek, Linus Walleij, Shawn Guo, Johan Hovold, Bjorn Andersson, Marijn Suijten, Sven Schwermer, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list This patch adds support for driving a chain of WS2812B LED chips using SPI bus. WorldSemi WS2812B is a individually addressable LED chip that can be chained together and controlled individually using a single wire. The chip recognize a long pulse as a bit of 1 and a short pulse as a bit of 0. Host sends a continuous stream of 24-bits color values, each LED chip takes the first 3 byte it receives as its color value and passes the leftover bytes to the next LED on the chain. This driver simulates this protocol using SPI bus by sending a long pulse as 3'b110 and a short pulse as 3'b100. The SPI frequency needs to be 2.105MHz~2.85MHz for the timing to be correct and the controller needs to transfer all the bytes continuously. Signed-off-by: Chuanhong Guo <gch981213@gmail.com> --- Changes since v1: rename the driver to drop -spi suffix add support for default-brightness use fwnode apis for properties Changes since v2: drop default-brightness and default-intensity Changes since v3: 1. add more comments 2. rename reg to cascade 3. redo some line breaking 4. move duplicated pointer calculation into ws2812b_set_byte 5. reword error message 6. get ws2812b_priv from led cdev->dev->parent drivers/leds/rgb/Kconfig | 11 ++ drivers/leds/rgb/Makefile | 1 + drivers/leds/rgb/leds-ws2812b.c | 231 ++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+) create mode 100644 drivers/leds/rgb/leds-ws2812b.c diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig index 204cf470beae..5c2081852f01 100644 --- a/drivers/leds/rgb/Kconfig +++ b/drivers/leds/rgb/Kconfig @@ -26,4 +26,15 @@ config LEDS_QCOM_LPG If compiled as a module, the module will be named leds-qcom-lpg. +config LEDS_WS2812B + tristate "SPI driven WS2812B RGB LED support" + depends on OF + depends on SPI + help + This option enables support for driving daisy-chained WS2812B RGB + LED chips using SPI bus. This driver simulates the single-wire + protocol by sending bits over the SPI MOSI pin. For this to work, + the SPI frequency should be 2.105MHz~2.85MHz and the controller + needs to transfer all the bytes continuously. + endif # LEDS_CLASS_MULTICOLOR diff --git a/drivers/leds/rgb/Makefile b/drivers/leds/rgb/Makefile index 0675bc0f6e18..a6f855eaeb14 100644 --- a/drivers/leds/rgb/Makefile +++ b/drivers/leds/rgb/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_LEDS_PWM_MULTICOLOR) += leds-pwm-multicolor.o obj-$(CONFIG_LEDS_QCOM_LPG) += leds-qcom-lpg.o +obj-$(CONFIG_LEDS_WS2812B) += leds-ws2812b.o diff --git a/drivers/leds/rgb/leds-ws2812b.c b/drivers/leds/rgb/leds-ws2812b.c new file mode 100644 index 000000000000..ca9c5caabe48 --- /dev/null +++ b/drivers/leds/rgb/leds-ws2812b.c @@ -0,0 +1,231 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * WorldSemi WS2812B individually-addressable LED driver using SPI + * + * Copyright 2022 Chuanhong Guo <gch981213@gmail.com> + * + * This driver simulates WS2812B protocol using SPI MOSI pin. A one pulse + * is transferred as 3'b110 and a zero pulse is 3'b100. For this driver to + * work properly, the SPI frequency should be 2.105MHz~2.85MHz and it needs + * to transfer all the bytes continuously. + */ + +#include <linux/led-class-multicolor.h> +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/property.h> +#include <linux/spi/spi.h> +#include <linux/mutex.h> + +#define WS2812B_BYTES_PER_COLOR 3 +#define WS2812B_NUM_COLORS 3 +/* A continuous 0 for 50us+ as the 'reset' signal */ +#define WS2812B_RESET_LEN 18 + +struct ws2812b_led { + struct led_classdev_mc mc_cdev; + struct mc_subled subled[WS2812B_NUM_COLORS]; + int cascade; +}; + +struct ws2812b_priv { + struct led_classdev ldev; + struct spi_device *spi; + struct mutex mutex; + int num_leds; + size_t data_len; + u8 *data_buf; + struct ws2812b_led leds[]; +}; + +/** + * ws2812b_set_byte - convert a byte of data to 3-byte SPI data for pulses + * @priv: pointer to the private data structure + * @offset: offset of the target byte in the data stream + * @val: 1-byte data to be set + * + * WS2812B receives a stream of bytes from DI, takes the first 3 byte as LED + * brightness and pases the rest to the next LED through the DO pin. + * This function assembles a single byte of data to the LED: + * A bit is represented with a pulse of specific length. A long pulse is a 1 + * and a short pulse is a 0. + * SPI transfers data continuously, MSB first. We can send 3'b100 to create a + * 0 pulse and 3'b110 for a 1 pulse. In this way, a byte of data takes up 3 + * bytes in a SPI transfer: + * 1x0 1x0 1x0 1x0 1x0 1x0 1x0 1x0 + * Let's rearrange it in 8 bits: + * 1x01x01x 01x01x01 x01x01x0 + * The higher 3 bits, middle 2 bits and lower 3 bits are represented with the + * 1st, 2nd and 3rd byte in the SPI transfer respectively. + * There are only 8 combinations for 3 bits and 4 for 2 bits, so we can create + * a lookup table for the 3 bytes. + * e.g. For 0x6b -> 2'b01101011: + * Bit 7-5: 3'b011 -> 10011011 -> 0x9b + * Bit 4-3: 2'b01 -> 01001101 -> 0x4d + * Bit 2-0: 3'b011 -> 00110110 -> 0x36 + */ +static void ws2812b_set_byte(struct ws2812b_priv *priv, size_t offset, u8 val) +{ + /* The lookup table for Bit 7-5 4-3 2-0 */ + const u8 h3b[] = { 0x92, 0x93, 0x9a, 0x9b, 0xd2, 0xd3, 0xda, 0xdb }; + const u8 m2b[] = { 0x49, 0x4d, 0x69, 0x6d }; + const u8 l3b[] = { 0x24, 0x26, 0x34, 0x36, 0xa4, 0xa6, 0xb4, 0xb6 }; + u8 *p = priv->data_buf + WS2812B_RESET_LEN + (offset * WS2812B_BYTES_PER_COLOR); + + p[0] = h3b[val >> 5]; /* Bit 7-5 */ + p[1] = m2b[(val >> 3) & 0x3]; /* Bit 4-3 */ + p[2] = l3b[val & 0x7]; /* Bit 2-0 */ +} + +static int ws2812b_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); + struct ws2812b_led *led = + container_of(mc_cdev, struct ws2812b_led, mc_cdev); + struct ws2812b_priv *priv = dev_get_drvdata(cdev->dev->parent); + int ret; + int i; + + led_mc_calc_color_components(mc_cdev, brightness); + + mutex_lock(&priv->mutex); + for (i = 0; i < WS2812B_NUM_COLORS; i++) + ws2812b_set_byte(priv, led->cascade * WS2812B_NUM_COLORS + i, + led->subled[i].brightness); + ret = spi_write(priv->spi, priv->data_buf, priv->data_len); + mutex_unlock(&priv->mutex); + + return ret; +} + +static int ws2812b_probe(struct spi_device *spi) +{ + struct device *dev = &spi->dev; + int cur_led = 0; + struct ws2812b_priv *priv; + struct fwnode_handle *led_node; + int num_leds, i, cnt, ret; + + num_leds = device_get_child_node_count(dev); + + priv = devm_kzalloc(dev, struct_size(priv, leds, num_leds), GFP_KERNEL); + if (!priv) + return -ENOMEM; + priv->data_len = + num_leds * WS2812B_BYTES_PER_COLOR * WS2812B_NUM_COLORS + + WS2812B_RESET_LEN; + priv->data_buf = kzalloc(priv->data_len, GFP_KERNEL); + if (!priv->data_buf) + return -ENOMEM; + + for (i = 0; i < num_leds * WS2812B_NUM_COLORS; i++) + ws2812b_set_byte(priv, i, 0); + + mutex_init(&priv->mutex); + priv->num_leds = num_leds; + priv->spi = spi; + + device_for_each_child_node(dev, led_node) { + struct led_init_data init_data = { + .fwnode = led_node, + }; + /* WS2812B LEDs usually come with GRB color */ + u32 color_idx[WS2812B_NUM_COLORS] = { + LED_COLOR_ID_GREEN, + LED_COLOR_ID_RED, + LED_COLOR_ID_BLUE, + }; + u32 cascade; + + ret = fwnode_property_read_u32(led_node, "reg", &cascade); + if (ret) { + dev_err(dev, "failed to obtain numerical LED index for %s", + fwnode_get_name(led_node)); + goto ERR_UNREG_LEDS; + } + if (cascade >= num_leds) { + dev_err(dev, "LED index of %s is larger than the number of LEDs.", + fwnode_get_name(led_node)); + ret = -EINVAL; + goto ERR_UNREG_LEDS; + } + + cnt = fwnode_property_count_u32(led_node, "color-index"); + if (cnt > 0 && cnt <= WS2812B_NUM_COLORS) + fwnode_property_read_u32_array(led_node, "color-index", + color_idx, (size_t)cnt); + + priv->leds[cur_led].mc_cdev.subled_info = + priv->leds[cur_led].subled; + priv->leds[cur_led].mc_cdev.num_colors = WS2812B_NUM_COLORS; + priv->leds[cur_led].mc_cdev.led_cdev.max_brightness = 255; + priv->leds[cur_led].mc_cdev.led_cdev.brightness_set_blocking = ws2812b_set; + + for (i = 0; i < WS2812B_NUM_COLORS; i++) { + priv->leds[cur_led].subled[i].color_index = color_idx[i]; + priv->leds[cur_led].subled[i].intensity = 255; + } + + priv->leds[cur_led].cascade = cascade; + + ret = led_classdev_multicolor_register_ext( + dev, &priv->leds[cur_led].mc_cdev, &init_data); + if (ret) { + dev_err(dev, "registration of %s failed.", + fwnode_get_name(led_node)); + goto ERR_UNREG_LEDS; + } + cur_led++; + } + + spi_set_drvdata(spi, priv); + + return 0; +ERR_UNREG_LEDS: + for (; cur_led >= 0; cur_led--) + led_classdev_multicolor_unregister(&priv->leds[cur_led].mc_cdev); + mutex_destroy(&priv->mutex); + kfree(priv->data_buf); + return ret; +} + +static void ws2812b_remove(struct spi_device *spi) +{ + struct ws2812b_priv *priv = spi_get_drvdata(spi); + int cur_led; + + for (cur_led = priv->num_leds - 1; cur_led >= 0; cur_led--) + led_classdev_multicolor_unregister(&priv->leds[cur_led].mc_cdev); + kfree(priv->data_buf); + mutex_destroy(&priv->mutex); +} + +static const struct spi_device_id ws2812b_spi_ids[] = { + { "ws2812b" }, + {}, +}; +MODULE_DEVICE_TABLE(spi, ws2812b_spi_ids); + +static const struct of_device_id ws2812b_dt_ids[] = { + { .compatible = "worldsemi,ws2812b" }, + {}, +}; +MODULE_DEVICE_TABLE(of, ws2812b_dt_ids); + +static struct spi_driver ws2812b_driver = { + .probe = ws2812b_probe, + .remove = ws2812b_remove, + .id_table = ws2812b_spi_ids, + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = ws2812b_dt_ids, + }, +}; + +module_spi_driver(ws2812b_driver); + +MODULE_AUTHOR("Chuanhong Guo <gch981213@gmail.com>"); +MODULE_DESCRIPTION("WS2812B LED driver using SPI"); +MODULE_LICENSE("GPL"); -- 2.38.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-01-20 5:07 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-12 4:55 [PATCH v4 0/3] leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs Chuanhong Guo 2022-12-12 4:55 ` [PATCH v4 1/3] dt-bindings: vendor-prefixes: add an entry for WorldSemi Chuanhong Guo 2022-12-12 4:55 ` [PATCH v4 2/3] dt-bindings: leds: add worldsemi,ws2812b Chuanhong Guo 2022-12-12 8:56 ` Krzysztof Kozlowski 2022-12-23 12:48 ` Lee Jones 2022-12-23 17:19 ` Pavel Machek 2022-12-24 5:52 ` Chuanhong Guo 2022-12-24 12:53 ` Krzysztof Kozlowski 2023-01-09 16:52 ` Lee Jones 2023-01-10 9:24 ` Krzysztof Kozlowski 2023-01-10 10:21 ` Lee Jones 2023-01-11 18:53 ` Chuanhong Guo 2023-01-12 9:16 ` Krzysztof Kozlowski 2023-01-13 14:56 ` Lee Jones 2023-01-14 12:29 ` Chuanhong Guo 2023-01-19 14:46 ` Lee Jones 2022-12-12 4:55 ` [PATCH v4 3/3] leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs Chuanhong Guo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.