| Message ID | 20221212045558.69602-1-gch981213@gmail.com |
|---|---|
| Headers | show |
| Series | leds: add driver for SPI driven WorldSemi WS2812B RGB LEDs | expand |
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.
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
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 :)
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
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.
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
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.
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?
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
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.
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
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.