* [PATCH 0/3] leds: add support for apa102c leds
@ 2020-02-18 9:37 Nicolas Belin
2020-02-18 9:37 ` [PATCH 1/3] dt-bindings: Document shiji vendor-prefix Nicolas Belin
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Nicolas Belin @ 2020-02-18 9:37 UTC (permalink / raw)
To: linux-kernel, linux-leds, jacek.anaszewski, pavel, dmurphy; +Cc: Nicolas Belin
This patch series adds the driver and its related documentation
for the APA102C RGB Leds.
Patch 1 adds the APA102C led manufacturer to the vendor-prefixes list.
Patch 2 Documents the APA102C led driver.
Patch 3 contains the actual driver code and modifications in the Kconfig
and the Makefile.
Nicolas Belin (3):
dt-bindings: Document shiji vendor-prefix
dt-bindings: leds: Shiji Lighting APA102C LED driver
drivers: leds: add support for apa102c leds
.../devicetree/bindings/leds/leds-apa102c.yaml | 91 +++++++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
drivers/leds/Kconfig | 11 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-apa102c.c | 268 +++++++++++++++++++++
5 files changed, 373 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-apa102c.yaml
create mode 100644 drivers/leds/leds-apa102c.c
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] dt-bindings: Document shiji vendor-prefix 2020-02-18 9:37 [PATCH 0/3] leds: add support for apa102c leds Nicolas Belin @ 2020-02-18 9:37 ` Nicolas Belin 2020-02-18 12:40 ` Dan Murphy 2020-02-18 9:37 ` [PATCH 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver Nicolas Belin ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Nicolas Belin @ 2020-02-18 9:37 UTC (permalink / raw) To: linux-kernel, linux-leds, jacek.anaszewski, pavel, dmurphy; +Cc: Nicolas Belin Shenzhen Shiji Lighting Co.,Ltd is a LED manufacturer. Signed-off-by: Nicolas Belin <nbelin@baylibre.com> --- 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 9e67944bec9c..a463286c3960 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -863,6 +863,8 @@ patternProperties: description: SGX Sensortech "^sharp,.*": description: Sharp Corporation + "^shiji,.*": + description: Shenzhen Shiji Lighting Co.,Ltd "^shimafuji,.*": description: Shimafuji Electric, Inc. "^si-en,.*": -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: Document shiji vendor-prefix 2020-02-18 9:37 ` [PATCH 1/3] dt-bindings: Document shiji vendor-prefix Nicolas Belin @ 2020-02-18 12:40 ` Dan Murphy 0 siblings, 0 replies; 14+ messages in thread From: Dan Murphy @ 2020-02-18 12:40 UTC (permalink / raw) To: Nicolas Belin, linux-kernel, linux-leds, jacek.anaszewski, pavel Nicolas On 2/18/20 3:37 AM, Nicolas Belin wrote: > Shenzhen Shiji Lighting Co.,Ltd is a LED manufacturer. > > Signed-off-by: Nicolas Belin <nbelin@baylibre.com> > --- You need to add the devicetree mailing list and Rob Herring to this for their review of the dt bindings patches Dan ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver 2020-02-18 9:37 [PATCH 0/3] leds: add support for apa102c leds Nicolas Belin 2020-02-18 9:37 ` [PATCH 1/3] dt-bindings: Document shiji vendor-prefix Nicolas Belin @ 2020-02-18 9:37 ` Nicolas Belin 2020-02-18 20:28 ` Jacek Anaszewski 2020-02-20 10:30 ` Geert Uytterhoeven 2020-02-18 9:37 ` [PATCH 3/3] drivers: leds: add support for apa102c leds Nicolas Belin 2020-02-18 12:43 ` [PATCH 0/3] " Dan Murphy 3 siblings, 2 replies; 14+ messages in thread From: Nicolas Belin @ 2020-02-18 9:37 UTC (permalink / raw) To: linux-kernel, linux-leds, jacek.anaszewski, pavel, dmurphy; +Cc: Nicolas Belin Document Shiji Lighting APA102C LED driver device tree bindings. Signed-off-by: Nicolas Belin <nbelin@baylibre.com> --- .../devicetree/bindings/leds/leds-apa102c.yaml | 91 ++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-apa102c.yaml diff --git a/Documentation/devicetree/bindings/leds/leds-apa102c.yaml b/Documentation/devicetree/bindings/leds/leds-apa102c.yaml new file mode 100644 index 000000000000..24bc2fc19fcb --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-apa102c.yaml @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/leds-apa102c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LED driver for Shiji Lighting - APA102C + +maintainers: + - Nicolas Belin <nbelin@baylibre.com> + +description: + Each LED is represented as a sub-node of the leds-apa102c device. Each LED + is a three color RGB LED with 32 levels brightness adjustment that can be + cascaded so that multiple LEDs can be set with a single command. + +properties: + compatible: + const: shiji,apa102c + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 1000000 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +required: + - compatible + - reg + - spi-max-frequency + - '#address-cells' + - '#size-cells' + +patternProperties: + "^led@[0-9]+$": + type: object + description: | + Properties for an array of connected LEDs. + + properties: + reg: + description: | + This property corresponds to the led index. It has to be between 0 + and the number of managed leds minus 1 + maxItems: 1 + + label: + description: | + This property corresponds to the name of the led. If not set, + the led index will be used to create the led name instead + maxItems: 1 + + linux,default-trigger: true + + required: + - reg + +examples: + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@0 { + compatible = "shiji,apa102c"; + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + spi-max-frequency = <1000000>; + led@0 { + reg = <0>; + label = "led1"; + }; + + led@1 { + reg = <1>; + label = "led2"; + }; + + led@2 { + reg = <2>; + label = "led3"; + }; + }; + }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver 2020-02-18 9:37 ` [PATCH 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver Nicolas Belin @ 2020-02-18 20:28 ` Jacek Anaszewski 2020-02-20 10:30 ` Geert Uytterhoeven 1 sibling, 0 replies; 14+ messages in thread From: Jacek Anaszewski @ 2020-02-18 20:28 UTC (permalink / raw) To: Nicolas Belin, linux-kernel, linux-leds, pavel, dmurphy Hi Nicolas, Thank you for the patch set. On 2/18/20 10:37 AM, Nicolas Belin wrote: > Document Shiji Lighting APA102C LED driver device tree bindings. > > Signed-off-by: Nicolas Belin <nbelin@baylibre.com> > --- > .../devicetree/bindings/leds/leds-apa102c.yaml | 91 ++++++++++++++++++++++ > 1 file changed, 91 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-apa102c.yaml > > diff --git a/Documentation/devicetree/bindings/leds/leds-apa102c.yaml b/Documentation/devicetree/bindings/leds/leds-apa102c.yaml > new file mode 100644 > index 000000000000..24bc2fc19fcb > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-apa102c.yaml > @@ -0,0 +1,91 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/leds-apa102c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: LED driver for Shiji Lighting - APA102C > + > +maintainers: > + - Nicolas Belin <nbelin@baylibre.com> > + > +description: > + Each LED is represented as a sub-node of the leds-apa102c device. Each LED > + is a three color RGB LED with 32 levels brightness adjustment that can be > + cascaded so that multiple LEDs can be set with a single command. > + > +properties: > + compatible: > + const: shiji,apa102c > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 1000000 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > +required: > + - compatible > + - reg > + - spi-max-frequency > + - '#address-cells' > + - '#size-cells' > + > +patternProperties: > + "^led@[0-9]+$": > + type: object > + description: | > + Properties for an array of connected LEDs. > + > + properties: > + reg: > + description: | > + This property corresponds to the led index. It has to be between 0 > + and the number of managed leds minus 1 > + maxItems: 1 > + > + label: label property has been deprecated, please refer to common LED bindings [0] and use function and color properties instead. > + description: | > + This property corresponds to the name of the led. If not set, > + the led index will be used to create the led name instead > + maxItems: 1 > + > + linux,default-trigger: true > + > + required: > + - reg > + > +examples: > + - | > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + led-controller@0 { > + compatible = "shiji,apa102c"; > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + spi-max-frequency = <1000000>; > + led@0 { > + reg = <0>; > + label = "led1"; > + }; > + > + led@1 { > + reg = <1>; > + label = "led2"; > + }; > + > + led@2 { > + reg = <2>; > + label = "led3"; > + }; > + }; > + }; > [0] Documentation/devicetree/bindings/leds/common.txt -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver 2020-02-18 9:37 ` [PATCH 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver Nicolas Belin 2020-02-18 20:28 ` Jacek Anaszewski @ 2020-02-20 10:30 ` Geert Uytterhoeven 2020-02-20 19:41 ` Jacek Anaszewski 1 sibling, 1 reply; 14+ messages in thread From: Geert Uytterhoeven @ 2020-02-20 10:30 UTC (permalink / raw) To: Nicolas Belin Cc: Linux Kernel Mailing List, linux-leds, Jacek Anaszewski, Pavel Machek, Dan Murphy, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Lukas Wunner Hi Nicolas, CC devicetree, Lukas On Tue, Feb 18, 2020 at 10:39 AM Nicolas Belin <nbelin@baylibre.com> wrote: > Document Shiji Lighting APA102C LED driver device tree bindings. > > Signed-off-by: Nicolas Belin <nbelin@baylibre.com> > --- > .../devicetree/bindings/leds/leds-apa102c.yaml | 91 ++++++++++++++++++++++ > 1 file changed, 91 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-apa102c.yaml > > diff --git a/Documentation/devicetree/bindings/leds/leds-apa102c.yaml b/Documentation/devicetree/bindings/leds/leds-apa102c.yaml > new file mode 100644 > index 000000000000..24bc2fc19fcb > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-apa102c.yaml > @@ -0,0 +1,91 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/leds-apa102c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: LED driver for Shiji Lighting - APA102C > + > +maintainers: > + - Nicolas Belin <nbelin@baylibre.com> > + > +description: > + Each LED is represented as a sub-node of the leds-apa102c device. Each LED > + is a three color RGB LED with 32 levels brightness adjustment that can be > + cascaded so that multiple LEDs can be set with a single command. > + > +properties: > + compatible: > + const: shiji,apa102c > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 1000000 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > +required: > + - compatible > + - reg > + - spi-max-frequency > + - '#address-cells' > + - '#size-cells' > + > +patternProperties: > + "^led@[0-9]+$": > + type: object > + description: | > + Properties for an array of connected LEDs. > + > + properties: > + reg: > + description: | > + This property corresponds to the led index. It has to be between 0 > + and the number of managed leds minus 1 > + maxItems: 1 > + > + label: > + description: | > + This property corresponds to the name of the led. If not set, > + the led index will be used to create the led name instead > + maxItems: 1 > + > + linux,default-trigger: true > + > + required: > + - reg > + > +examples: > + - | > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + led-controller@0 { > + compatible = "shiji,apa102c"; > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + spi-max-frequency = <1000000>; > + led@0 { > + reg = <0>; > + label = "led1"; > + }; > + > + led@1 { > + reg = <1>; > + label = "led2"; > + }; > + > + led@2 { > + reg = <2>; > + label = "led3"; > + }; > + }; > + }; Perhaps this should use "#daisy-chained-devices" instead of listing all LEDs explicitly? Or would that cause problems w.r.t. LED labeling? Documentation/devicetree/bindings/common-properties.txt Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver 2020-02-20 10:30 ` Geert Uytterhoeven @ 2020-02-20 19:41 ` Jacek Anaszewski 0 siblings, 0 replies; 14+ messages in thread From: Jacek Anaszewski @ 2020-02-20 19:41 UTC (permalink / raw) To: Geert Uytterhoeven, Nicolas Belin Cc: Linux Kernel Mailing List, linux-leds, Pavel Machek, Dan Murphy, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Lukas Wunner Hi Geert, On 2/20/20 11:30 AM, Geert Uytterhoeven wrote: > Hi Nicolas, > > CC devicetree, Lukas > > On Tue, Feb 18, 2020 at 10:39 AM Nicolas Belin <nbelin@baylibre.com> wrote: >> Document Shiji Lighting APA102C LED driver device tree bindings. >> >> Signed-off-by: Nicolas Belin <nbelin@baylibre.com> >> --- >> .../devicetree/bindings/leds/leds-apa102c.yaml | 91 ++++++++++++++++++++++ >> 1 file changed, 91 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-apa102c.yaml >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-apa102c.yaml b/Documentation/devicetree/bindings/leds/leds-apa102c.yaml >> new file mode 100644 >> index 000000000000..24bc2fc19fcb >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-apa102c.yaml >> @@ -0,0 +1,91 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/leds/leds-apa102c.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: LED driver for Shiji Lighting - APA102C >> + >> +maintainers: >> + - Nicolas Belin <nbelin@baylibre.com> >> + >> +description: >> + Each LED is represented as a sub-node of the leds-apa102c device. Each LED >> + is a three color RGB LED with 32 levels brightness adjustment that can be >> + cascaded so that multiple LEDs can be set with a single command. >> + >> +properties: >> + compatible: >> + const: shiji,apa102c >> + >> + reg: >> + maxItems: 1 >> + >> + spi-max-frequency: >> + maximum: 1000000 >> + >> + "#address-cells": >> + const: 1 >> + >> + "#size-cells": >> + const: 0 >> + >> +required: >> + - compatible >> + - reg >> + - spi-max-frequency >> + - '#address-cells' >> + - '#size-cells' >> + >> +patternProperties: >> + "^led@[0-9]+$": >> + type: object >> + description: | >> + Properties for an array of connected LEDs. >> + >> + properties: >> + reg: >> + description: | >> + This property corresponds to the led index. It has to be between 0 >> + and the number of managed leds minus 1 >> + maxItems: 1 >> + >> + label: >> + description: | >> + This property corresponds to the name of the led. If not set, >> + the led index will be used to create the led name instead >> + maxItems: 1 >> + >> + linux,default-trigger: true >> + >> + required: >> + - reg >> + >> +examples: >> + - | >> + spi { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + led-controller@0 { >> + compatible = "shiji,apa102c"; >> + reg = <0>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + spi-max-frequency = <1000000>; >> + led@0 { >> + reg = <0>; >> + label = "led1"; >> + }; >> + >> + led@1 { >> + reg = <1>; >> + label = "led2"; >> + }; >> + >> + led@2 { >> + reg = <2>; >> + label = "led3"; >> + }; >> + }; >> + }; > > Perhaps this should use "#daisy-chained-devices" instead of listing all LEDs > explicitly? > Or would that cause problems w.r.t. LED labeling? It would be against common LED bindings. Besides, this daisy-chain-devices property is not well documented. I.e. I don't see how it facilitates conveying information on output ID (here reg is used for that), and as you already mentioned - LED labeling. Generally speaking, I don't think it is applicable to the LED subsystem. > Documentation/devicetree/bindings/common-properties.txt > > Gr{oetje,eeting}s, > > Geert > -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] drivers: leds: add support for apa102c leds 2020-02-18 9:37 [PATCH 0/3] leds: add support for apa102c leds Nicolas Belin 2020-02-18 9:37 ` [PATCH 1/3] dt-bindings: Document shiji vendor-prefix Nicolas Belin 2020-02-18 9:37 ` [PATCH 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver Nicolas Belin @ 2020-02-18 9:37 ` Nicolas Belin 2020-02-18 21:13 ` Jacek Anaszewski 2020-02-18 12:43 ` [PATCH 0/3] " Dan Murphy 3 siblings, 1 reply; 14+ messages in thread From: Nicolas Belin @ 2020-02-18 9:37 UTC (permalink / raw) To: linux-kernel, linux-leds, jacek.anaszewski, pavel, dmurphy; +Cc: Nicolas Belin Initilial commit in order to support the apa102c RGB leds. Signed-off-by: Nicolas Belin <nbelin@baylibre.com> --- drivers/leds/Kconfig | 11 ++ drivers/leds/Makefile | 1 + drivers/leds/leds-apa102c.c | 268 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 280 insertions(+) create mode 100644 drivers/leds/leds-apa102c.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index d82f1dea3711..4fafeaaf6ee8 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -69,6 +69,17 @@ config LEDS_AN30259A To compile this driver as a module, choose M here: the module will be called leds-an30259a. +config LEDS_APA102C + tristate "LED Support for Shiji APA102C" + depends on LEDS_CLASS + depends on SPI + help + This option enables support for the Shiji Lighthing APA102C RGB full color + LEDs. + + To compile this driver as a module, choose M here: the module + will be called leds-apa102c. + config LEDS_APU tristate "Front panel LED support for PC Engines APU/APU2/APU3 boards" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index d7e1107753fb..ab17f90347cb 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o # LED Platform Drivers obj-$(CONFIG_LEDS_88PM860X) += leds-88pm860x.o obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o +obj-$(CONFIG_LEDS_APA102C) += leds-apa102c.o obj-$(CONFIG_LEDS_APU) += leds-apu.o obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o obj-$(CONFIG_LEDS_AN30259A) += leds-an30259a.o diff --git a/drivers/leds/leds-apa102c.c b/drivers/leds/leds-apa102c.c new file mode 100644 index 000000000000..e7abe3f5b7c2 --- /dev/null +++ b/drivers/leds/leds-apa102c.c @@ -0,0 +1,268 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Copyright (C) 2020 BayLibre, SAS + * Author: Nicolas Belin <nbelin@baylibre.com> + */ + +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/spi/spi.h> +#include <uapi/linux/uleds.h> + +/* + * APA102C SPI protocol description: + * +------+----------------------------------------+------+ + * |START | DATA FIELD: | END | + * |FRAME | N LED FRAMES |FRAME | + * +------+------+------+------+------+-----+------+------+ + * | 0*32 | LED1 | LED2 | LED3 | LED4 | --- | LEDN | 1*32 | + * +------+------+------+------+------+-----+------+------+ + * + * +-----------------------------------+ + * |START FRAME 32bits | + * +--------+--------+--------+--------+ + * |00000000|00000000|00000000|00000000| + * +--------+--------+--------+--------+ + * + * +------------------------------------+ + * |LED FRAME 32bits | + * +---+-----+--------+--------+--------+ + * |111|LUMA | BLUE | GREEN | RED | + * +---+-----+--------+--------+--------+ + * |3b |5bits| 8bits | 8bits | 8bits | + * +---+-----+--------+--------+--------+ + * |MSB LSB|MSB LSB|MSB LSB|MSB LSB| + * +---+-----+--------+--------+--------+ + * + * +-----------------------------------+ + * |END FRAME 32bits | + * +--------+--------+--------+--------+ + * |11111111|11111111|11111111|11111111| + * +--------+--------+--------+--------+ + */ + +/* apa102c default settings */ +#define CR_MAX_BRIGHTNESS GENMASK(7, 0) +#define LM_MAX_BRIGHTNESS GENMASK(4, 0) +#define CH_NUM 4 +#define START_BYTE 0 +#define END_BYTE GENMASK(7, 0) +#define LED_FRAME_HEADER GENMASK(7, 5) + +enum led_channels { + RED, + GREEN, + BLUE, + LUMA, +}; + +struct apa102c_led { + char name[LED_MAX_NAME_SIZE]; + struct apa102c *priv; + struct led_classdev ldev; + u8 brightness; +}; + +struct apa102c { + size_t led_count; + struct device *dev; + struct mutex lock; + struct spi_device *spi; + u8 *buf; + struct apa102c_led leds[]; +}; + +static int apa102c_sync(struct apa102c *priv) +{ + int ret; + size_t i; + size_t bytes = 0; + + for (i = 0; i < 4; i++) + priv->buf[bytes++] = START_BYTE; + + for (i = 0; i < priv->led_count; i++) { + priv->buf[bytes++] = LED_FRAME_HEADER | + priv->leds[i * CH_NUM + LUMA].brightness; + priv->buf[bytes++] = priv->leds[i * CH_NUM + BLUE].brightness; + priv->buf[bytes++] = priv->leds[i * CH_NUM + GREEN].brightness; + priv->buf[bytes++] = priv->leds[i * CH_NUM + RED].brightness; + } + + for (i = 0; i < 4; i++) + priv->buf[bytes++] = END_BYTE; + + ret = spi_write(priv->spi, priv->buf, bytes); + + return ret; +} + +static int apa102c_set_sync(struct led_classdev *ldev, + enum led_brightness brightness) +{ + int ret; + struct apa102c_led *led = container_of(ldev, + struct apa102c_led, + ldev); + + dev_dbg(led->priv->dev, "Set brightness of %s to %d\n", + led->name, brightness); + + mutex_lock(&led->priv->lock); + led->brightness = (u8)brightness; + ret = apa102c_sync(led->priv); + mutex_unlock(&led->priv->lock); + + return ret; +} + +static int apa102c_probe_dt(struct apa102c *priv) +{ + u32 i = 0; + int j = 0; + struct apa102c_led *led; + struct fwnode_handle *child; + struct device_node *np; + int ret; + int use_index; + const char *str; + static const char * const rgb_name[] = {"red", + "green", + "blue", + "luma"}; + + device_for_each_child_node(priv->dev, child) { + np = to_of_node(child); + + ret = fwnode_property_read_u32(child, "reg", &i); + if (ret) + return ret; + + if (i >= priv->led_count) + return -EINVAL; + + /* use the index to create the name if the label is not set */ + use_index = fwnode_property_read_string(child, "label", &str); + + /* for each physical LED, 4 LEDs are created representing + * the 4 components: red, green, blue and global luma. + */ + for (j = 0; j < CH_NUM; j++) { + led = &priv->leds[i * CH_NUM + j]; + + if (use_index) + snprintf(led->name, sizeof(led->name), + "apa102c:%s:%d", rgb_name[j], i); + else + snprintf(led->name, sizeof(led->name), + "apa102c:%s:%s", rgb_name[j], str); + + fwnode_property_read_string(child, + "linux,default-trigger", + &led->ldev.default_trigger); + + led->priv = priv; + led->ldev.name = led->name; + if (j == LUMA) { + led->ldev.brightness = led->brightness + = LM_MAX_BRIGHTNESS; + led->ldev.max_brightness = LM_MAX_BRIGHTNESS; + } else { + led->ldev.brightness = led->brightness + = 0; + led->ldev.max_brightness = CR_MAX_BRIGHTNESS; + } + + led->ldev.brightness_set_blocking = apa102c_set_sync; + + ret = devm_led_classdev_register(priv->dev, &led->ldev); + if (ret) { + dev_err(priv->dev, + "failed to register LED %s, err %d", + led->name, ret); + fwnode_handle_put(child); + return ret; + } + + led->ldev.dev->of_node = np; + + } + } + + return 0; +} + +static int apa102c_probe(struct spi_device *spi) +{ + struct apa102c *priv; + size_t led_count; + int ret; + + led_count = device_get_child_node_count(&spi->dev); + if (!led_count) { + dev_err(&spi->dev, "No LEDs defined in device tree!"); + return -ENODEV; + } + + priv = devm_kzalloc(&spi->dev, + struct_size(priv, leds, led_count * CH_NUM), + GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->buf = devm_kzalloc(&spi->dev, led_count * CH_NUM + 8, GFP_KERNEL); + if (!priv->buf) + return -ENOMEM; + + mutex_init(&priv->lock); + priv->led_count = led_count; + priv->dev = &spi->dev; + priv->spi = spi; + + ret = apa102c_probe_dt(priv); + if (ret) + return ret; + + /* Set the LEDs with default values at start */ + apa102c_sync(priv); + if (ret) + return ret; + + spi_set_drvdata(spi, priv); + + return 0; +} + +static int apa102c_remove(struct spi_device *spi) +{ + struct apa102c *priv = spi_get_drvdata(spi); + + mutex_destroy(&priv->lock); + + return 0; +} + +static const struct of_device_id apa102c_dt_ids[] = { + { .compatible = "shiji,apa102c", }, + {}, +}; + +MODULE_DEVICE_TABLE(of, apa102c_dt_ids); + +static struct spi_driver apa102c_driver = { + .probe = apa102c_probe, + .remove = apa102c_remove, + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = apa102c_dt_ids, + }, +}; + +module_spi_driver(apa102c_driver); + +MODULE_AUTHOR("Nicolas Belin <nbelin@baylibre.com>"); +MODULE_DESCRIPTION("apa102c LED driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("spi:apa102c"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drivers: leds: add support for apa102c leds 2020-02-18 9:37 ` [PATCH 3/3] drivers: leds: add support for apa102c leds Nicolas Belin @ 2020-02-18 21:13 ` Jacek Anaszewski 2020-02-20 10:25 ` Nicolas Belin 0 siblings, 1 reply; 14+ messages in thread From: Jacek Anaszewski @ 2020-02-18 21:13 UTC (permalink / raw) To: Nicolas Belin, linux-kernel, linux-leds, pavel, dmurphy Hi Nicolas, On 2/18/20 10:37 AM, Nicolas Belin wrote: > Initilial commit in order to support the apa102c RGB leds. > > Signed-off-by: Nicolas Belin <nbelin@baylibre.com> > --- > drivers/leds/Kconfig | 11 ++ > drivers/leds/Makefile | 1 + > drivers/leds/leds-apa102c.c | 268 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 280 insertions(+) > create mode 100644 drivers/leds/leds-apa102c.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index d82f1dea3711..4fafeaaf6ee8 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -69,6 +69,17 @@ config LEDS_AN30259A > To compile this driver as a module, choose M here: the module > will be called leds-an30259a. > > +config LEDS_APA102C > + tristate "LED Support for Shiji APA102C" > + depends on LEDS_CLASS > + depends on SPI > + help > + This option enables support for the Shiji Lighthing APA102C RGB full color > + LEDs. > + > + To compile this driver as a module, choose M here: the module > + will be called leds-apa102c. > + > config LEDS_APU > tristate "Front panel LED support for PC Engines APU/APU2/APU3 boards" > depends on LEDS_CLASS > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index d7e1107753fb..ab17f90347cb 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o > # LED Platform Drivers > obj-$(CONFIG_LEDS_88PM860X) += leds-88pm860x.o > obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o > +obj-$(CONFIG_LEDS_APA102C) += leds-apa102c.o > obj-$(CONFIG_LEDS_APU) += leds-apu.o > obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o > obj-$(CONFIG_LEDS_AN30259A) += leds-an30259a.o > diff --git a/drivers/leds/leds-apa102c.c b/drivers/leds/leds-apa102c.c > new file mode 100644 > index 000000000000..e7abe3f5b7c2 > --- /dev/null > +++ b/drivers/leds/leds-apa102c.c > @@ -0,0 +1,268 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * Copyright (C) 2020 BayLibre, SAS > + * Author: Nicolas Belin <nbelin@baylibre.com> > + */ Please use "//" comment style for all the above lines. > + > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/spi/spi.h> > +#include <uapi/linux/uleds.h> > + > +/* > + * APA102C SPI protocol description: > + * +------+----------------------------------------+------+ > + * |START | DATA FIELD: | END | > + * |FRAME | N LED FRAMES |FRAME | > + * +------+------+------+------+------+-----+------+------+ > + * | 0*32 | LED1 | LED2 | LED3 | LED4 | --- | LEDN | 1*32 | > + * +------+------+------+------+------+-----+------+------+ > + * > + * +-----------------------------------+ > + * |START FRAME 32bits | > + * +--------+--------+--------+--------+ > + * |00000000|00000000|00000000|00000000| > + * +--------+--------+--------+--------+ > + * > + * +------------------------------------+ > + * |LED FRAME 32bits | > + * +---+-----+--------+--------+--------+ > + * |111|LUMA | BLUE | GREEN | RED | > + * +---+-----+--------+--------+--------+ > + * |3b |5bits| 8bits | 8bits | 8bits | > + * +---+-----+--------+--------+--------+ > + * |MSB LSB|MSB LSB|MSB LSB|MSB LSB| > + * +---+-----+--------+--------+--------+ > + * > + * +-----------------------------------+ > + * |END FRAME 32bits | > + * +--------+--------+--------+--------+ > + * |11111111|11111111|11111111|11111111| > + * +--------+--------+--------+--------+ > + */ > + > +/* apa102c default settings */ > +#define CR_MAX_BRIGHTNESS GENMASK(7, 0) > +#define LM_MAX_BRIGHTNESS GENMASK(4, 0) > +#define CH_NUM 4 > +#define START_BYTE 0 > +#define END_BYTE GENMASK(7, 0) > +#define LED_FRAME_HEADER GENMASK(7, 5) > + > +enum led_channels { > + RED, > + GREEN, > + BLUE, > + LUMA, > +}; > + > +struct apa102c_led { > + char name[LED_MAX_NAME_SIZE]; > + struct apa102c *priv; > + struct led_classdev ldev; > + u8 brightness; Please drop this one, struct led_classdev already holds brightness value. > +}; > + > +struct apa102c { > + size_t led_count; > + struct device *dev; > + struct mutex lock; > + struct spi_device *spi; > + u8 *buf; > + struct apa102c_led leds[]; > +}; > + > +static int apa102c_sync(struct apa102c *priv) > +{ > + int ret; > + size_t i; > + size_t bytes = 0; > + > + for (i = 0; i < 4; i++) > + priv->buf[bytes++] = START_BYTE; > + > + for (i = 0; i < priv->led_count; i++) { > + priv->buf[bytes++] = LED_FRAME_HEADER | > + priv->leds[i * CH_NUM + LUMA].brightness; > + priv->buf[bytes++] = priv->leds[i * CH_NUM + BLUE].brightness; > + priv->buf[bytes++] = priv->leds[i * CH_NUM + GREEN].brightness; > + priv->buf[bytes++] = priv->leds[i * CH_NUM + RED].brightness; This is odd. You create separate LED class device for each color anyway, so this seems pointless. We have pending LED multi color framework patch set, as Dan mentioned, so you could try to use it. If you want to have the patch set accepted quicker then just set brightness for one LED at a time. You will be able to add LED multicolor class support later when it will be ready. > + } > + > + for (i = 0; i < 4; i++) > + priv->buf[bytes++] = END_BYTE; > + > + ret = spi_write(priv->spi, priv->buf, bytes); > + > + return ret; > +} > + > +static int apa102c_set_sync(struct led_classdev *ldev, > + enum led_brightness brightness) > +{ > + int ret; > + struct apa102c_led *led = container_of(ldev, > + struct apa102c_led, > + ldev); > + > + dev_dbg(led->priv->dev, "Set brightness of %s to %d\n", > + led->name, brightness); > + > + mutex_lock(&led->priv->lock); > + led->brightness = (u8)brightness; > + ret = apa102c_sync(led->priv); > + mutex_unlock(&led->priv->lock); > + > + return ret; > +} > + > +static int apa102c_probe_dt(struct apa102c *priv) > +{ > + u32 i = 0; > + int j = 0; > + struct apa102c_led *led; > + struct fwnode_handle *child; > + struct device_node *np; > + int ret; > + int use_index; > + const char *str; > + static const char * const rgb_name[] = {"red", > + "green", > + "blue", > + "luma"}; We have LED_COLOR_ID* definitions in dt-bindings/leds/common.h for red, green and blue. And regarding "luma" - what is specificity of that one? If neither of existing definitions fits for it then you are welcome to submit a patch adding LED_COLOR_ID_LUMA. > + > + device_for_each_child_node(priv->dev, child) { > + np = to_of_node(child); > + > + ret = fwnode_property_read_u32(child, "reg", &i); > + if (ret) > + return ret; > + > + if (i >= priv->led_count) > + return -EINVAL; > + > + /* use the index to create the name if the label is not set */ > + use_index = fwnode_property_read_string(child, "label", &str); > + > + /* for each physical LED, 4 LEDs are created representing > + * the 4 components: red, green, blue and global luma. > + */ > + for (j = 0; j < CH_NUM; j++) { > + led = &priv->leds[i * CH_NUM + j]; > + > + if (use_index) > + snprintf(led->name, sizeof(led->name), > + "apa102c:%s:%d", rgb_name[j], i); > + else > + snprintf(led->name, sizeof(led->name), > + "apa102c:%s:%s", rgb_name[j], str); LED core already handles LED name composition. Please refer to existing LED class drivers that use devm_led_classdev_register_ext() API and use it in your driver. > + > + fwnode_property_read_string(child, > + "linux,default-trigger", > + &led->ldev.default_trigger); > + > + led->priv = priv; > + led->ldev.name = led->name; > + if (j == LUMA) { > + led->ldev.brightness = led->brightness What do you want to achieve here? > + = LM_MAX_BRIGHTNESS; > + led->ldev.max_brightness = LM_MAX_BRIGHTNESS; > + } else { > + led->ldev.brightness = led->brightness > + = 0; > + led->ldev.max_brightness = CR_MAX_BRIGHTNESS; > + } > + > + led->ldev.brightness_set_blocking = apa102c_set_sync; > + > + ret = devm_led_classdev_register(priv->dev, &led->ldev); As mentioned above - new *ext API will make your life easier. > + if (ret) { > + dev_err(priv->dev, > + "failed to register LED %s, err %d", > + led->name, ret); > + fwnode_handle_put(child); > + return ret; > + } > + > + led->ldev.dev->of_node = np; > + > + } > + } > + > + return 0; > +} > + > +static int apa102c_probe(struct spi_device *spi) > +{ > + struct apa102c *priv; > + size_t led_count; > + int ret; > + > + led_count = device_get_child_node_count(&spi->dev); > + if (!led_count) { > + dev_err(&spi->dev, "No LEDs defined in device tree!"); > + return -ENODEV; > + } > + > + priv = devm_kzalloc(&spi->dev, > + struct_size(priv, leds, led_count * CH_NUM), > + GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->buf = devm_kzalloc(&spi->dev, led_count * CH_NUM + 8, GFP_KERNEL); > + if (!priv->buf) > + return -ENOMEM; > + > + mutex_init(&priv->lock); > + priv->led_count = led_count; > + priv->dev = &spi->dev; > + priv->spi = spi; > + > + ret = apa102c_probe_dt(priv); > + if (ret) > + return ret; > + > + /* Set the LEDs with default values at start */ > + apa102c_sync(priv); > + if (ret) > + return ret; > + > + spi_set_drvdata(spi, priv); > + > + return 0; > +} > + > +static int apa102c_remove(struct spi_device *spi) > +{ > + struct apa102c *priv = spi_get_drvdata(spi); > + > + mutex_destroy(&priv->lock); > + > + return 0; > +} > + > +static const struct of_device_id apa102c_dt_ids[] = { > + { .compatible = "shiji,apa102c", }, > + {}, > +}; > + > +MODULE_DEVICE_TABLE(of, apa102c_dt_ids); > + > +static struct spi_driver apa102c_driver = { > + .probe = apa102c_probe, > + .remove = apa102c_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = apa102c_dt_ids, > + }, > +}; > + > +module_spi_driver(apa102c_driver); > + > +MODULE_AUTHOR("Nicolas Belin <nbelin@baylibre.com>"); > +MODULE_DESCRIPTION("apa102c LED driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("spi:apa102c"); > -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drivers: leds: add support for apa102c leds 2020-02-18 21:13 ` Jacek Anaszewski @ 2020-02-20 10:25 ` Nicolas Belin 0 siblings, 0 replies; 14+ messages in thread From: Nicolas Belin @ 2020-02-20 10:25 UTC (permalink / raw) To: Jacek Anaszewski; +Cc: linux-kernel, linux-leds, pavel, dmurphy Hi Jacek, Thanks for you feedback. I am going to use multicolor framework as Dan mentioned, and fix the issues you pointed out. Regards, Nicolas Le mar. 18 févr. 2020 à 22:13, Jacek Anaszewski <jacek.anaszewski@gmail.com> a écrit : > > Hi Nicolas, > > On 2/18/20 10:37 AM, Nicolas Belin wrote: > > Initilial commit in order to support the apa102c RGB leds. > > > > Signed-off-by: Nicolas Belin <nbelin@baylibre.com> > > --- > > drivers/leds/Kconfig | 11 ++ > > drivers/leds/Makefile | 1 + > > drivers/leds/leds-apa102c.c | 268 ++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 280 insertions(+) > > create mode 100644 drivers/leds/leds-apa102c.c > > > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > > index d82f1dea3711..4fafeaaf6ee8 100644 > > --- a/drivers/leds/Kconfig > > +++ b/drivers/leds/Kconfig > > @@ -69,6 +69,17 @@ config LEDS_AN30259A > > To compile this driver as a module, choose M here: the module > > will be called leds-an30259a. > > > > +config LEDS_APA102C > > + tristate "LED Support for Shiji APA102C" > > + depends on LEDS_CLASS > > + depends on SPI > > + help > > + This option enables support for the Shiji Lighthing APA102C RGB full color > > + LEDs. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called leds-apa102c. > > + > > config LEDS_APU > > tristate "Front panel LED support for PC Engines APU/APU2/APU3 boards" > > depends on LEDS_CLASS > > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > > index d7e1107753fb..ab17f90347cb 100644 > > --- a/drivers/leds/Makefile > > +++ b/drivers/leds/Makefile > > @@ -9,6 +9,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o > > # LED Platform Drivers > > obj-$(CONFIG_LEDS_88PM860X) += leds-88pm860x.o > > obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o > > +obj-$(CONFIG_LEDS_APA102C) += leds-apa102c.o > > obj-$(CONFIG_LEDS_APU) += leds-apu.o > > obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o > > obj-$(CONFIG_LEDS_AN30259A) += leds-an30259a.o > > diff --git a/drivers/leds/leds-apa102c.c b/drivers/leds/leds-apa102c.c > > new file mode 100644 > > index 000000000000..e7abe3f5b7c2 > > --- /dev/null > > +++ b/drivers/leds/leds-apa102c.c > > @@ -0,0 +1,268 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +/* > > + * Copyright (C) 2020 BayLibre, SAS > > + * Author: Nicolas Belin <nbelin@baylibre.com> > > + */ > > Please use "//" comment style for all the above lines. > > > + > > +#include <linux/leds.h> > > +#include <linux/module.h> > > +#include <linux/of_device.h> > > +#include <linux/spi/spi.h> > > +#include <uapi/linux/uleds.h> > > + > > +/* > > + * APA102C SPI protocol description: > > + * +------+----------------------------------------+------+ > > + * |START | DATA FIELD: | END | > > + * |FRAME | N LED FRAMES |FRAME | > > + * +------+------+------+------+------+-----+------+------+ > > + * | 0*32 | LED1 | LED2 | LED3 | LED4 | --- | LEDN | 1*32 | > > + * +------+------+------+------+------+-----+------+------+ > > + * > > + * +-----------------------------------+ > > + * |START FRAME 32bits | > > + * +--------+--------+--------+--------+ > > + * |00000000|00000000|00000000|00000000| > > + * +--------+--------+--------+--------+ > > + * > > + * +------------------------------------+ > > + * |LED FRAME 32bits | > > + * +---+-----+--------+--------+--------+ > > + * |111|LUMA | BLUE | GREEN | RED | > > + * +---+-----+--------+--------+--------+ > > + * |3b |5bits| 8bits | 8bits | 8bits | > > + * +---+-----+--------+--------+--------+ > > + * |MSB LSB|MSB LSB|MSB LSB|MSB LSB| > > + * +---+-----+--------+--------+--------+ > > + * > > + * +-----------------------------------+ > > + * |END FRAME 32bits | > > + * +--------+--------+--------+--------+ > > + * |11111111|11111111|11111111|11111111| > > + * +--------+--------+--------+--------+ > > + */ > > + > > +/* apa102c default settings */ > > +#define CR_MAX_BRIGHTNESS GENMASK(7, 0) > > +#define LM_MAX_BRIGHTNESS GENMASK(4, 0) > > +#define CH_NUM 4 > > +#define START_BYTE 0 > > +#define END_BYTE GENMASK(7, 0) > > +#define LED_FRAME_HEADER GENMASK(7, 5) > > + > > +enum led_channels { > > + RED, > > + GREEN, > > + BLUE, > > + LUMA, > > +}; > > + > > +struct apa102c_led { > > + char name[LED_MAX_NAME_SIZE]; > > + struct apa102c *priv; > > + struct led_classdev ldev; > > + u8 brightness; > > Please drop this one, struct led_classdev already holds brightness > value. > > > +}; > > + > > +struct apa102c { > > + size_t led_count; > > + struct device *dev; > > + struct mutex lock; > > + struct spi_device *spi; > > + u8 *buf; > > + struct apa102c_led leds[]; > > +}; > > + > > +static int apa102c_sync(struct apa102c *priv) > > +{ > > + int ret; > > + size_t i; > > + size_t bytes = 0; > > + > > + for (i = 0; i < 4; i++) > > + priv->buf[bytes++] = START_BYTE; > > + > > + for (i = 0; i < priv->led_count; i++) { > > + priv->buf[bytes++] = LED_FRAME_HEADER | > > + priv->leds[i * CH_NUM + LUMA].brightness; > > + priv->buf[bytes++] = priv->leds[i * CH_NUM + BLUE].brightness; > > + priv->buf[bytes++] = priv->leds[i * CH_NUM + GREEN].brightness; > > + priv->buf[bytes++] = priv->leds[i * CH_NUM + RED].brightness; > > This is odd. You create separate LED class device for each color anyway, > so this seems pointless. We have pending LED multi color framework patch > set, as Dan mentioned, so you could try to use it. If you want to have > the patch set accepted quicker then just set brightness for one LED at > a time. You will be able to add LED multicolor class support later when > it will be ready. > > > + } > > + > > + for (i = 0; i < 4; i++) > > + priv->buf[bytes++] = END_BYTE; > > + > > + ret = spi_write(priv->spi, priv->buf, bytes); > > + > > + return ret; > > +} > > + > > +static int apa102c_set_sync(struct led_classdev *ldev, > > + enum led_brightness brightness) > > +{ > > + int ret; > > + struct apa102c_led *led = container_of(ldev, > > + struct apa102c_led, > > + ldev); > > + > > + dev_dbg(led->priv->dev, "Set brightness of %s to %d\n", > > + led->name, brightness); > > + > > + mutex_lock(&led->priv->lock); > > + led->brightness = (u8)brightness; > > + ret = apa102c_sync(led->priv); > > + mutex_unlock(&led->priv->lock); > > + > > + return ret; > > +} > > + > > +static int apa102c_probe_dt(struct apa102c *priv) > > +{ > > + u32 i = 0; > > + int j = 0; > > + struct apa102c_led *led; > > + struct fwnode_handle *child; > > + struct device_node *np; > > + int ret; > > + int use_index; > > + const char *str; > > + static const char * const rgb_name[] = {"red", > > + "green", > > + "blue", > > + "luma"}; > > We have LED_COLOR_ID* definitions in dt-bindings/leds/common.h > for red, green and blue. And regarding "luma" - what is specificity > of that one? If neither of existing definitions fits for it then > you are welcome to submit a patch adding LED_COLOR_ID_LUMA. > > > + > > + device_for_each_child_node(priv->dev, child) { > > + np = to_of_node(child); > > + > > + ret = fwnode_property_read_u32(child, "reg", &i); > > + if (ret) > > + return ret; > > + > > + if (i >= priv->led_count) > > + return -EINVAL; > > + > > + /* use the index to create the name if the label is not set */ > > + use_index = fwnode_property_read_string(child, "label", &str); > > + > > + /* for each physical LED, 4 LEDs are created representing > > + * the 4 components: red, green, blue and global luma. > > + */ > > + for (j = 0; j < CH_NUM; j++) { > > + led = &priv->leds[i * CH_NUM + j]; > > + > > + if (use_index) > > + snprintf(led->name, sizeof(led->name), > > + "apa102c:%s:%d", rgb_name[j], i); > > + else > > + snprintf(led->name, sizeof(led->name), > > + "apa102c:%s:%s", rgb_name[j], str); > > LED core already handles LED name composition. Please refer to existing > LED class drivers that use devm_led_classdev_register_ext() API and use > it in your driver. > > > + > > + fwnode_property_read_string(child, > > + "linux,default-trigger", > > + &led->ldev.default_trigger); > > + > > + led->priv = priv; > > + led->ldev.name = led->name; > > + if (j == LUMA) { > > + led->ldev.brightness = led->brightness > > What do you want to achieve here? > > > + = LM_MAX_BRIGHTNESS; > > + led->ldev.max_brightness = LM_MAX_BRIGHTNESS; > > + } else { > > + led->ldev.brightness = led->brightness > > + = 0; > > + led->ldev.max_brightness = CR_MAX_BRIGHTNESS; > > + } > > + > > + led->ldev.brightness_set_blocking = apa102c_set_sync; > > + > > + ret = devm_led_classdev_register(priv->dev, &led->ldev); > > As mentioned above - new *ext API will make your life easier. > > > + if (ret) { > > + dev_err(priv->dev, > > + "failed to register LED %s, err %d", > > + led->name, ret); > > + fwnode_handle_put(child); > > + return ret; > > + } > > + > > + led->ldev.dev->of_node = np; > > + > > + } > > + } > > + > > + return 0; > > +} > > + > > +static int apa102c_probe(struct spi_device *spi) > > +{ > > + struct apa102c *priv; > > + size_t led_count; > > + int ret; > > + > > + led_count = device_get_child_node_count(&spi->dev); > > + if (!led_count) { > > + dev_err(&spi->dev, "No LEDs defined in device tree!"); > > + return -ENODEV; > > + } > > + > > + priv = devm_kzalloc(&spi->dev, > > + struct_size(priv, leds, led_count * CH_NUM), > > + GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->buf = devm_kzalloc(&spi->dev, led_count * CH_NUM + 8, GFP_KERNEL); > > + if (!priv->buf) > > + return -ENOMEM; > > + > > + mutex_init(&priv->lock); > > + priv->led_count = led_count; > > + priv->dev = &spi->dev; > > + priv->spi = spi; > > + > > + ret = apa102c_probe_dt(priv); > > + if (ret) > > + return ret; > > + > > + /* Set the LEDs with default values at start */ > > + apa102c_sync(priv); > > + if (ret) > > + return ret; > > + > > + spi_set_drvdata(spi, priv); > > + > > + return 0; > > +} > > + > > +static int apa102c_remove(struct spi_device *spi) > > +{ > > + struct apa102c *priv = spi_get_drvdata(spi); > > + > > + mutex_destroy(&priv->lock); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id apa102c_dt_ids[] = { > > + { .compatible = "shiji,apa102c", }, > > + {}, > > +}; > > + > > +MODULE_DEVICE_TABLE(of, apa102c_dt_ids); > > + > > +static struct spi_driver apa102c_driver = { > > + .probe = apa102c_probe, > > + .remove = apa102c_remove, > > + .driver = { > > + .name = KBUILD_MODNAME, > > + .of_match_table = apa102c_dt_ids, > > + }, > > +}; > > + > > +module_spi_driver(apa102c_driver); > > + > > +MODULE_AUTHOR("Nicolas Belin <nbelin@baylibre.com>"); > > +MODULE_DESCRIPTION("apa102c LED driver"); > > +MODULE_LICENSE("GPL v2"); > > +MODULE_ALIAS("spi:apa102c"); > > > > -- > Best regards, > Jacek Anaszewski ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] leds: add support for apa102c leds 2020-02-18 9:37 [PATCH 0/3] leds: add support for apa102c leds Nicolas Belin ` (2 preceding siblings ...) 2020-02-18 9:37 ` [PATCH 3/3] drivers: leds: add support for apa102c leds Nicolas Belin @ 2020-02-18 12:43 ` Dan Murphy 2020-02-18 21:26 ` Jacek Anaszewski 2020-02-20 10:19 ` Nicolas Belin 3 siblings, 2 replies; 14+ messages in thread From: Dan Murphy @ 2020-02-18 12:43 UTC (permalink / raw) To: Nicolas Belin, linux-kernel, linux-leds, jacek.anaszewski, pavel Hellp On 2/18/20 3:37 AM, Nicolas Belin wrote: > This patch series adds the driver and its related documentation > for the APA102C RGB Leds. > > Patch 1 adds the APA102C led manufacturer to the vendor-prefixes list. > > Patch 2 Documents the APA102C led driver. > > Patch 3 contains the actual driver code and modifications in the Kconfig > and the Makefile. Is this something that can benefit from the Multicolor framework patches? https://lore.kernel.org/patchwork/project/lkml/list/?series=427513 Can you RFC the APA102C driver on top of the Multicolor FW to see how it blends? Dan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] leds: add support for apa102c leds 2020-02-18 12:43 ` [PATCH 0/3] " Dan Murphy @ 2020-02-18 21:26 ` Jacek Anaszewski 2020-02-20 10:19 ` Nicolas Belin 1 sibling, 0 replies; 14+ messages in thread From: Jacek Anaszewski @ 2020-02-18 21:26 UTC (permalink / raw) To: Dan Murphy, Nicolas Belin, linux-kernel, linux-leds, pavel Hi Dan, On 2/18/20 1:43 PM, Dan Murphy wrote: > Hellp > > On 2/18/20 3:37 AM, Nicolas Belin wrote: >> This patch series adds the driver and its related documentation >> for the APA102C RGB Leds. >> >> Patch 1 adds the APA102C led manufacturer to the vendor-prefixes list. >> >> Patch 2 Documents the APA102C led driver. >> >> Patch 3 contains the actual driver code and modifications in the Kconfig >> and the Makefile. > > Is this something that can benefit from the Multicolor framework patches? > > https://lore.kernel.org/patchwork/project/lkml/list/?series=427513 > > Can you RFC the APA102C driver on top of the Multicolor FW to see how it > blends? Just to let you know - I am currently playing with Samsung Galaxy S3, which has mainline support, and also with a driver for its RGB LED: leds-an30259a.c. I am adjusting that driver to LED multicolor class and I will have certainly some more remarks (also to the variable naming I proposed myself, which after few months feels awkward to me). Generally speaking - we have to do everything to make the addition of LED multicolor support to the driver way easier and more straightforward since currently it is painful. I would go for a separate compilation unit for multicolor specifc part of LED drivers. But I will be able to tell more after few more days. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] leds: add support for apa102c leds 2020-02-18 12:43 ` [PATCH 0/3] " Dan Murphy 2020-02-18 21:26 ` Jacek Anaszewski @ 2020-02-20 10:19 ` Nicolas Belin 2020-02-26 14:19 ` Pavel Machek 1 sibling, 1 reply; 14+ messages in thread From: Nicolas Belin @ 2020-02-20 10:19 UTC (permalink / raw) To: Dan Murphy; +Cc: linux-kernel, linux-leds, jacek.anaszewski, pavel Hi Dan, Le mar. 18 févr. 2020 à 13:47, Dan Murphy <dmurphy@ti.com> a écrit : > > Hellp > > On 2/18/20 3:37 AM, Nicolas Belin wrote: > > This patch series adds the driver and its related documentation > > for the APA102C RGB Leds. > > > > Patch 1 adds the APA102C led manufacturer to the vendor-prefixes list. > > > > Patch 2 Documents the APA102C led driver. > > > > Patch 3 contains the actual driver code and modifications in the Kconfig > > and the Makefile. > > Is this something that can benefit from the Multicolor framework patches? > > https://lore.kernel.org/patchwork/project/lkml/list/?series=427513 > > Can you RFC the APA102C driver on top of the Multicolor FW to see how it > blends? Sure, the Multicolor framework will probably improve my driver ! I'll send you a new version once I have tested it. > > Dan > Thanks, Nicolas ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] leds: add support for apa102c leds 2020-02-20 10:19 ` Nicolas Belin @ 2020-02-26 14:19 ` Pavel Machek 0 siblings, 0 replies; 14+ messages in thread From: Pavel Machek @ 2020-02-26 14:19 UTC (permalink / raw) To: Nicolas Belin; +Cc: Dan Murphy, linux-kernel, linux-leds, jacek.anaszewski [-- Attachment #1: Type: text/plain, Size: 684 bytes --] Hi! > > Is this something that can benefit from the Multicolor framework patches? > > > > https://lore.kernel.org/patchwork/project/lkml/list/?series=427513 > > > > Can you RFC the APA102C driver on top of the Multicolor FW to see how it > > blends? > > Sure, the Multicolor framework will probably improve my driver ! > I'll send you a new version once I have tested it. If you want to submit basic version of your driver that does _not_ support RGB, that may help get the driver merged early. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-02-26 14:19 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-18 9:37 [PATCH 0/3] leds: add support for apa102c leds Nicolas Belin 2020-02-18 9:37 ` [PATCH 1/3] dt-bindings: Document shiji vendor-prefix Nicolas Belin 2020-02-18 12:40 ` Dan Murphy 2020-02-18 9:37 ` [PATCH 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver Nicolas Belin 2020-02-18 20:28 ` Jacek Anaszewski 2020-02-20 10:30 ` Geert Uytterhoeven 2020-02-20 19:41 ` Jacek Anaszewski 2020-02-18 9:37 ` [PATCH 3/3] drivers: leds: add support for apa102c leds Nicolas Belin 2020-02-18 21:13 ` Jacek Anaszewski 2020-02-20 10:25 ` Nicolas Belin 2020-02-18 12:43 ` [PATCH 0/3] " Dan Murphy 2020-02-18 21:26 ` Jacek Anaszewski 2020-02-20 10:19 ` Nicolas Belin 2020-02-26 14:19 ` Pavel Machek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).