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

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

[Nicolas Belin]

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

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

[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

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

[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     | 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 <nbelin@baylibre.com>
+
+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

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

[Nicolas Belin]

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

Reviewed-by: Corentin Labbe <clabbe@baylibre.com>
Signed-off-by: Nicolas Belin <nbelin@baylibre.com>
---
 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 <nbelin@baylibre.com>
+ */
+
+/*
+ *  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 <nbelin@baylibre.com>");
+MODULE_DESCRIPTION("apa102c LED driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spi:apa102c");
-- 
2.7.4

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

[Jacek Anaszewski]

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 <clabbe@baylibre.com>
> Signed-off-by: Nicolas Belin <nbelin@baylibre.com>
> ---
>  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

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

[Rob Herring]

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 <nbelin@baylibre.com>
> ---
>  .../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.

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

[Nicolas Belin]

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
<jacek.anaszewski@gmail.com> 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 <clabbe@baylibre.com>
> > Signed-off-by: Nicolas Belin <nbelin@baylibre.com>
> > ---
> >  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
www.baylibre.com

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

[Rob Herring]

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 <nbelin@baylibre.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

[Jacek Anaszewski]

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
> <jacek.anaszewski@gmail.com> 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 <clabbe@baylibre.com>
>>> Signed-off-by: Nicolas Belin <nbelin@baylibre.com>
>>> ---
>>>  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
> 
> 
>