2020-02-26 14:34:01

by Nicolas Belin

[permalink] [raw]
Subject: [PATCH RFC v2 0/3] leds: add support for apa102c leds

This patch series adds the driver and its related documentation
for the APA102C RGB Leds. It is based on the Multicolor Framework and
must be applied on top of the Multicolor framework patches located at
https://lore.kernel.org/patchwork/project/lkml/list/?series=427513 .

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.

Updates since v1:
- Moved the apa102c line in the Makefile to the LED SPI Drivers part
- The driver is now based on the Multicolor Framework.

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 | 154 +++++++++++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
drivers/leds/Kconfig | 7 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-apa102c.c | 291 +++++++++++++++++++++
5 files changed, 455 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-apa102c.yaml
create mode 100644 drivers/leds/leds-apa102c.c

--
2.7.4


2020-02-26 14:34:02

by Nicolas Belin

[permalink] [raw]
Subject: [PATCH RFC v2 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver

Document Shiji Lighting APA102C LED driver device tree bindings.

Signed-off-by: Nicolas Belin <[email protected]>
---
.../devicetree/bindings/leds/leds-apa102c.yaml | 154 +++++++++++++++++++++
1 file changed, 154 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..90c827ab5a5a
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-apa102c.yaml
@@ -0,0 +1,154 @@
+# 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 <[email protected]>
+
+description:
+ Each RGB LED is represented as a multi-led 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
+
+patternProperties:
+ "^multi-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
+
+ color:
+ const: 8
+
+ linux,default-trigger: true
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ patternProperties:
+ "^led@[0-9]+$":
+ type: object
+ description: |
+ Properties for the LED color components.
+
+ properties:
+ reg:
+ maxItems: 1
+
+ color:
+ oneOf:
+ - enum: [ 1, 2, 3 ]
+
+ required:
+ - reg
+ - color
+
+ required:
+ - reg
+ - color
+ - '#address-cells'
+ - '#size-cells'
+
+required:
+ - compatible
+ - reg
+ - spi-max-frequency
+ - '#address-cells'
+ - '#size-cells'
+
+examples:
+ - |
+
+ #include <dt-bindings/leds/common.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ led-controller@0 {
+ compatible = "shiji,apa102c";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ multi-led@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ color = <LED_COLOR_ID_MULTI>;
+ label = "rgb_led1";
+
+ led@0 {
+ reg = <0>;
+ color = <LED_COLOR_ID_RED>;
+ };
+
+ led@1 {
+ reg = <1>;
+ color = <LED_COLOR_ID_GREEN>;
+ };
+
+ led@2 {
+ reg = <2>;
+ color = <LED_COLOR_ID_BLUE>;
+ };
+ };
+ multi-led@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ color = <LED_COLOR_ID_MULTI>;
+ label = "rgb_led2";
+
+ led@3 {
+ reg = <3>;
+ color = <LED_COLOR_ID_RED>;
+ };
+
+ led@4 {
+ reg = <4>;
+ color = <LED_COLOR_ID_GREEN>;
+ };
+
+ led@5 {
+ reg = <5>;
+ color = <LED_COLOR_ID_BLUE>;
+ };
+ };
+ };
+ };
--
2.7.4

2020-02-26 14:34:05

by Nicolas Belin

[permalink] [raw]
Subject: [PATCH RFC v2 3/3] drivers: leds: add support for apa102c leds

Initilial commit in order to support the apa102c RGB leds. This
is based on the Multicolor Framework.

Reviewed-by: Corentin Labbe <[email protected]>
Signed-off-by: Nicolas Belin <[email protected]>
---
drivers/leds/Kconfig | 7 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-apa102c.c | 291 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 299 insertions(+)
create mode 100644 drivers/leds/leds-apa102c.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 5dc6535a88ef..71e29727c6ec 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -79,6 +79,13 @@ 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 SPI
+ depends on LEDS_CLASS_MULTI_COLOR
+ help
+ This option enables support for APA102C LEDs.
+
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 b5305b7d43fb..8334cb6dc7e8 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -90,6 +90,7 @@ obj-$(CONFIG_LEDS_LM36274) += leds-lm36274.o
obj-$(CONFIG_LEDS_TPS6105X) += leds-tps6105x.o

# LED SPI Drivers
+obj-$(CONFIG_LEDS_APA102C) += leds-apa102c.o
obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
obj-$(CONFIG_LEDS_EL15203000) += leds-el15203000.o
diff --git a/drivers/leds/leds-apa102c.c b/drivers/leds/leds-apa102c.c
new file mode 100644
index 000000000000..b46db0db7501
--- /dev/null
+++ b/drivers/leds/leds-apa102c.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi.h>
+#include <linux/led-class-multicolor.h>
+
+/*
+ * Copyright (C) 2020 BayLibre, SAS
+ * Author: Nicolas Belin <[email protected]>
+ */
+
+/*
+ * 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 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)
+
+#define IS_RGB (1<<LED_COLOR_ID_RED \
+ | 1<<LED_COLOR_ID_GREEN \
+ | 1<<LED_COLOR_ID_BLUE)
+
+#define APA_DEV_NAME "apa102c"
+
+struct apa102c_led {
+ struct apa102c *priv;
+ struct led_classdev ldev;
+ struct led_classdev_mc mc_cdev;
+};
+
+struct apa102c {
+ size_t led_count;
+ struct device *dev;
+ struct mutex lock;
+ struct spi_device *spi;
+ u8 *buf;
+ struct apa102c_led leds[];
+};
+
+static void apa102c_set_rgb_bytes(u8 *bgr_buf, struct list_head *color_list)
+{
+ struct led_mc_color_entry *color_data;
+
+ /* Looking for the RGB values and putting them in the buffer in
+ * BGR order
+ */
+ list_for_each_entry(color_data, color_list, list) {
+ switch (color_data->led_color_id) {
+ case LED_COLOR_ID_RED:
+ bgr_buf[2] = color_data->intensity;
+ break;
+ case LED_COLOR_ID_GREEN:
+ bgr_buf[1] = color_data->intensity;
+ break;
+ case LED_COLOR_ID_BLUE:
+ bgr_buf[0] = color_data->intensity;
+ break;
+ }
+ }
+}
+
+static int apa102c_sync(struct apa102c *priv)
+{
+ int ret;
+ size_t i;
+ struct apa102c_led *leds = priv->leds;
+ u8 *buf = priv->buf;
+
+ /* creating the data that will be sent to all the leds at once */
+ for (i = 0; i < 4; i++)
+ *buf++ = START_BYTE;
+
+ /* looping on each multicolor led getting the luma and the RGB values */
+ for (i = 0; i < priv->led_count; i++) {
+ *buf++ = LED_FRAME_HEADER | leds[i].ldev.brightness;
+ apa102c_set_rgb_bytes(buf, &leds[i].mc_cdev.color_list);
+ buf += 3;
+ }
+
+ for (i = 0; i < 4; i++)
+ *buf++ = END_BYTE;
+
+ ret = spi_write(priv->spi, priv->buf, priv->led_count*4 + 8);
+
+ return ret;
+}
+
+static int apa102c_brightness_set(struct led_classdev *ldev,
+ enum led_brightness brightness)
+{
+ int ret;
+ struct apa102c_led *led = container_of(ldev,
+ struct apa102c_led,
+ ldev);
+
+ mutex_lock(&led->priv->lock);
+ /* updating the brightness then synching all the leds */
+ ldev->brightness = brightness;
+ ret = apa102c_sync(led->priv);
+ mutex_unlock(&led->priv->lock);
+
+ return ret;
+}
+
+static int apa102c_probe_dt(struct apa102c *priv)
+{
+ int ret = 0;
+ int num_colors;
+ u32 color_id, i;
+ struct apa102c_led *led;
+ struct fwnode_handle *child, *grandchild;
+ struct led_init_data init_data;
+
+ device_for_each_child_node(priv->dev, child) {
+
+ ret = fwnode_property_read_u32(child, "reg", &i);
+ if (ret) {
+ dev_err(priv->dev, "coudld not read reg %d\n", ret);
+ goto child_out;
+ }
+
+ if (i >= priv->led_count) {
+ ret = -EINVAL;
+ dev_err(priv->dev, "reg value too big\n");
+ goto child_out;
+ }
+
+ led = &priv->leds[i];
+ /* checking if this led config is already used */
+ if (led->mc_cdev.led_cdev) {
+ ret = -EINVAL;
+ dev_err(priv->dev, "using the same reg value twice\n");
+ goto child_out;
+ }
+
+ led->priv = priv;
+ led->ldev.max_brightness = MAX_BRIGHTNESS;
+ fwnode_property_read_string(child, "linux,default-trigger",
+ &led->ldev.default_trigger);
+
+ init_data.fwnode = child;
+ init_data.devicename = APA_DEV_NAME;
+ init_data.default_label = ":";
+
+ num_colors = 0;
+ fwnode_for_each_child_node(child, grandchild) {
+ ret = fwnode_property_read_u32(grandchild, "color",
+ &color_id);
+ if (ret) {
+ dev_err(priv->dev, "Cannot read color\n");
+ goto child_out;
+ }
+
+ set_bit(color_id, &led->mc_cdev.available_colors);
+ num_colors++;
+ }
+
+ if (num_colors != 3) {
+ ret = -EINVAL;
+ dev_err(priv->dev, "There should be 3 colors\n");
+ goto child_out;
+ }
+
+ if (led->mc_cdev.available_colors != IS_RGB) {
+ ret = -EINVAL;
+ dev_err(priv->dev, "The led is expected to be RGB\n");
+ goto child_out;
+ }
+
+ led->mc_cdev.num_leds = num_colors;
+ led->mc_cdev.led_cdev = &led->ldev;
+ led->ldev.brightness_set_blocking = apa102c_brightness_set;
+ ret = devm_led_classdev_multicolor_register_ext(priv->dev,
+ &led->mc_cdev,
+ &init_data);
+ if (ret) {
+ dev_err(priv->dev, "led register err: %d\n", ret);
+ fwnode_handle_put(child);
+ goto child_out;
+ }
+ }
+
+child_out:
+ return ret;
+}
+
+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),
+ GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->buf = devm_kzalloc(&spi->dev, (led_count + 2) * 4, 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 <[email protected]>");
+MODULE_DESCRIPTION("apa102c LED driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spi:apa102c");
--
2.7.4

2020-02-26 14:34:32

by Nicolas Belin

[permalink] [raw]
Subject: [PATCH RFC v2 1/3] dt-bindings: Document shiji vendor-prefix

Shenzhen Shiji Lighting Co.,Ltd is a LED manufacturer.

Signed-off-by: Nicolas Belin <[email protected]>
---
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

2020-02-26 20:14:36

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH RFC v2 3/3] drivers: leds: add support for apa102c leds

Hi Nicolas,

Regardless of the fact that LED mc framework in current shape
will probably not materialize in mainline, I have single
remark regarding LED initialization. Please take a look below.

On 2/26/20 3:33 PM, Nicolas Belin wrote:
> Initilial commit in order to support the apa102c RGB leds. This
> is based on the Multicolor Framework.
>
> Reviewed-by: Corentin Labbe <[email protected]>
> Signed-off-by: Nicolas Belin <[email protected]>
> ---
> drivers/leds/Kconfig | 7 ++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-apa102c.c | 291 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 299 insertions(+)
> create mode 100644 drivers/leds/leds-apa102c.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 5dc6535a88ef..71e29727c6ec 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -79,6 +79,13 @@ 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 SPI
> + depends on LEDS_CLASS_MULTI_COLOR
> + help
> + This option enables support for APA102C LEDs.
> +
> 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 b5305b7d43fb..8334cb6dc7e8 100644
[...]
> +
> + led->priv = priv;
> + led->ldev.max_brightness = MAX_BRIGHTNESS;
> + fwnode_property_read_string(child, "linux,default-trigger",
> + &led->ldev.default_trigger);
> +
> + init_data.fwnode = child;
> + init_data.devicename = APA_DEV_NAME;
> + init_data.default_label = ":";

devicename property should be filled in new drivers only in case
devname_mandatory is set to true.
default_label property is for legacy drivers, for backward compatibility
with old LED naming convention.

For more information please refer to:
- Documentation/leds/leds-class.rst, "LED Device Naming" section
- struct led_init_data documention in linux/leds.h

In effect you need only fwnode here,

> +
> + num_colors = 0;
> + fwnode_for_each_child_node(child, grandchild) {
> + ret = fwnode_property_read_u32(grandchild, "color",
> + &color_id);
> + if (ret) {
> + dev_err(priv->dev, "Cannot read color\n");
> + goto child_out;
> + }
> +
> + set_bit(color_id, &led->mc_cdev.available_colors);
> + num_colors++;
> + }
> +
> + if (num_colors != 3) {
> + ret = -EINVAL;
> + dev_err(priv->dev, "There should be 3 colors\n");
> + goto child_out;
> + }
> +
> + if (led->mc_cdev.available_colors != IS_RGB) {
> + ret = -EINVAL;
> + dev_err(priv->dev, "The led is expected to be RGB\n");
> + goto child_out;
> + }
> +
> + led->mc_cdev.num_leds = num_colors;
> + led->mc_cdev.led_cdev = &led->ldev;
> + led->ldev.brightness_set_blocking = apa102c_brightness_set;
> + ret = devm_led_classdev_multicolor_register_ext(priv->dev,

--
Best regards,
Jacek Anaszewski

2020-02-26 21:57:15

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH RFC v2 2/3] dt-bindings: leds: Shiji Lighting APA102C LED driver

On Wed, 26 Feb 2020 15:33:11 +0100, Nicolas Belin wrote:
> Document Shiji Lighting APA102C LED driver device tree bindings.
>
> Signed-off-by: Nicolas Belin <[email protected]>
> ---
> .../devicetree/bindings/leds/leds-apa102c.yaml | 154 +++++++++++++++++++++
> 1 file changed, 154 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-apa102c.yaml
>

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
Error: Documentation/devicetree/bindings/leds/leds-apa102c.example.dts:33.30-31 syntax error
FATAL ERROR: Unable to parse input tree
scripts/Makefile.lib:300: recipe for target 'Documentation/devicetree/bindings/leds/leds-apa102c.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/leds/leds-apa102c.example.dt.yaml] Error 1
Makefile:1263: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1245095
Please check and re-submit.

2020-03-03 10:31:23

by Nicolas Belin

[permalink] [raw]
Subject: Re: [PATCH RFC v2 3/3] drivers: leds: add support for apa102c leds

Hi Jacek,

That's a shame that it is not going to be upstreamed soon as it making
my led driver much nicer :)
So what's the plan with the multicolor framework?
I am happy to send a new version to fix your remark and then adapt my
driver to the future changes in the Framework.

Let me know what you think.

Thanks,

Regards,

Nicolas

Le mer. 26 févr. 2020 à 21:14, Jacek Anaszewski
<[email protected]> a écrit :
>
> Hi Nicolas,
>
> Regardless of the fact that LED mc framework in current shape
> will probably not materialize in mainline, I have single
> remark regarding LED initialization. Please take a look below.
>
> On 2/26/20 3:33 PM, Nicolas Belin wrote:
> > Initilial commit in order to support the apa102c RGB leds. This
> > is based on the Multicolor Framework.
> >
> > Reviewed-by: Corentin Labbe <[email protected]>
> > Signed-off-by: Nicolas Belin <[email protected]>
> > ---
> > drivers/leds/Kconfig | 7 ++
> > drivers/leds/Makefile | 1 +
> > drivers/leds/leds-apa102c.c | 291 ++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 299 insertions(+)
> > create mode 100644 drivers/leds/leds-apa102c.c
> >
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index 5dc6535a88ef..71e29727c6ec 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -79,6 +79,13 @@ 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 SPI
> > + depends on LEDS_CLASS_MULTI_COLOR
> > + help
> > + This option enables support for APA102C LEDs.
> > +
> > 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 b5305b7d43fb..8334cb6dc7e8 100644
> [...]
> > +
> > + led->priv = priv;
> > + led->ldev.max_brightness = MAX_BRIGHTNESS;
> > + fwnode_property_read_string(child, "linux,default-trigger",
> > + &led->ldev.default_trigger);
> > +
> > + init_data.fwnode = child;
> > + init_data.devicename = APA_DEV_NAME;
> > + init_data.default_label = ":";
>
> devicename property should be filled in new drivers only in case
> devname_mandatory is set to true.
> default_label property is for legacy drivers, for backward compatibility
> with old LED naming convention.
>
> For more information please refer to:
> - Documentation/leds/leds-class.rst, "LED Device Naming" section
> - struct led_init_data documention in linux/leds.h
>
> In effect you need only fwnode here,
>
> > +
> > + num_colors = 0;
> > + fwnode_for_each_child_node(child, grandchild) {
> > + ret = fwnode_property_read_u32(grandchild, "color",
> > + &color_id);
> > + if (ret) {
> > + dev_err(priv->dev, "Cannot read color\n");
> > + goto child_out;
> > + }
> > +
> > + set_bit(color_id, &led->mc_cdev.available_colors);
> > + num_colors++;
> > + }
> > +
> > + if (num_colors != 3) {
> > + ret = -EINVAL;
> > + dev_err(priv->dev, "There should be 3 colors\n");
> > + goto child_out;
> > + }
> > +
> > + if (led->mc_cdev.available_colors != IS_RGB) {
> > + ret = -EINVAL;
> > + dev_err(priv->dev, "The led is expected to be RGB\n");
> > + goto child_out;
> > + }
> > +
> > + led->mc_cdev.num_leds = num_colors;
> > + led->mc_cdev.led_cdev = &led->ldev;
> > + led->ldev.brightness_set_blocking = apa102c_brightness_set;
> > + ret = devm_led_classdev_multicolor_register_ext(priv->dev,
>
> --
> Best regards,
> Jacek Anaszewski



--
Nicolas Belin
Software design engineer
BayLibre
http://www.baylibre.com

2020-03-03 14:29:48

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH RFC v2 1/3] dt-bindings: Document shiji vendor-prefix

On Wed, 26 Feb 2020 15:33:10 +0100, Nicolas Belin wrote:
> Shenzhen Shiji Lighting Co.,Ltd is a LED manufacturer.
>
> Signed-off-by: Nicolas Belin <[email protected]>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>

Acked-by: Rob Herring <[email protected]>

2020-03-03 21:44:32

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH RFC v2 3/3] drivers: leds: add support for apa102c leds

Hi Nicolas,

On 3/3/20 11:30 AM, Nicolas Belin wrote:
> Hi Jacek,
>
> That's a shame that it is not going to be upstreamed soon as it making
> my led driver much nicer :)

Now when we clarified interface related issues I hope it will
be rather weeks than months when it is ready.

> So what's the plan with the multicolor framework?
> I am happy to send a new version to fix your remark and then adapt my
> driver to the future changes in the Framework.

Just rework the driver to create one LED class device per LED color.
After LED mc framework is upstream you will be free to add the
support for it to your driver.

Best regards,
Jacek Anaszewski

> Let me know what you think.
>
> Thanks,
>
> Regards,
>
> Nicolas
>
> Le mer. 26 févr. 2020 à 21:14, Jacek Anaszewski
> <[email protected]> a écrit :
>>
>> Hi Nicolas,
>>
>> Regardless of the fact that LED mc framework in current shape
>> will probably not materialize in mainline, I have single
>> remark regarding LED initialization. Please take a look below.
>>
>> On 2/26/20 3:33 PM, Nicolas Belin wrote:
>>> Initilial commit in order to support the apa102c RGB leds. This
>>> is based on the Multicolor Framework.
>>>
>>> Reviewed-by: Corentin Labbe <[email protected]>
>>> Signed-off-by: Nicolas Belin <[email protected]>
>>> ---
>>> drivers/leds/Kconfig | 7 ++
>>> drivers/leds/Makefile | 1 +
>>> drivers/leds/leds-apa102c.c | 291 ++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 299 insertions(+)
>>> create mode 100644 drivers/leds/leds-apa102c.c
>>>
>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>> index 5dc6535a88ef..71e29727c6ec 100644
>>> --- a/drivers/leds/Kconfig
>>> +++ b/drivers/leds/Kconfig
>>> @@ -79,6 +79,13 @@ 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 SPI
>>> + depends on LEDS_CLASS_MULTI_COLOR
>>> + help
>>> + This option enables support for APA102C LEDs.
>>> +
>>> 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 b5305b7d43fb..8334cb6dc7e8 100644
>> [...]
>>> +
>>> + led->priv = priv;
>>> + led->ldev.max_brightness = MAX_BRIGHTNESS;
>>> + fwnode_property_read_string(child, "linux,default-trigger",
>>> + &led->ldev.default_trigger);
>>> +
>>> + init_data.fwnode = child;
>>> + init_data.devicename = APA_DEV_NAME;
>>> + init_data.default_label = ":";
>>
>> devicename property should be filled in new drivers only in case
>> devname_mandatory is set to true.
>> default_label property is for legacy drivers, for backward compatibility
>> with old LED naming convention.
>>
>> For more information please refer to:
>> - Documentation/leds/leds-class.rst, "LED Device Naming" section
>> - struct led_init_data documention in linux/leds.h
>>
>> In effect you need only fwnode here,
>>
>>> +
>>> + num_colors = 0;
>>> + fwnode_for_each_child_node(child, grandchild) {
>>> + ret = fwnode_property_read_u32(grandchild, "color",
>>> + &color_id);
>>> + if (ret) {
>>> + dev_err(priv->dev, "Cannot read color\n");
>>> + goto child_out;
>>> + }
>>> +
>>> + set_bit(color_id, &led->mc_cdev.available_colors);
>>> + num_colors++;
>>> + }
>>> +
>>> + if (num_colors != 3) {
>>> + ret = -EINVAL;
>>> + dev_err(priv->dev, "There should be 3 colors\n");
>>> + goto child_out;
>>> + }
>>> +
>>> + if (led->mc_cdev.available_colors != IS_RGB) {
>>> + ret = -EINVAL;
>>> + dev_err(priv->dev, "The led is expected to be RGB\n");
>>> + goto child_out;
>>> + }
>>> +
>>> + led->mc_cdev.num_leds = num_colors;
>>> + led->mc_cdev.led_cdev = &led->ldev;
>>> + led->ldev.brightness_set_blocking = apa102c_brightness_set;
>>> + ret = devm_led_classdev_multicolor_register_ext(priv->dev,
>>
>> --
>> Best regards,
>> Jacek Anaszewski
>
>
>