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
Document Shiji Lighting APA102C LED driver device tree bindings.
Signed-off-by: Nicolas Belin <[email protected]>
---
.../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 <[email protected]>
+
+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
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
Initilial commit in order to support the apa102c RGB leds.
Signed-off-by: Nicolas Belin <[email protected]>
---
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 <[email protected]>
+ */
+
+#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 <[email protected]>");
+MODULE_DESCRIPTION("apa102c LED driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spi:apa102c");
--
2.7.4
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 <[email protected]>
> ---
You need to add the devicetree mailing list and Rob Herring to this for
their review of the dt bindings patches
Dan
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
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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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
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 <[email protected]>
> ---
> 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 <[email protected]>
> + */
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 <[email protected]>");
> +MODULE_DESCRIPTION("apa102c LED driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("spi:apa102c");
>
--
Best regards,
Jacek Anaszewski
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
Hi Dan,
Le mar. 18 févr. 2020 à 13:47, Dan Murphy <[email protected]> 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
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
<[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
> > + */
>
> 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 <[email protected]>");
> > +MODULE_DESCRIPTION("apa102c LED driver");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_ALIAS("spi:apa102c");
> >
>
> --
> Best regards,
> Jacek Anaszewski
Hi Nicolas,
CC devicetree, Lukas
On Tue, Feb 18, 2020 at 10:39 AM Nicolas Belin <[email protected]> wrote:
> Document Shiji Lighting APA102C LED driver device tree bindings.
>
> Signed-off-by: Nicolas Belin <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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 -- [email protected]
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
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 <[email protected]> wrote:
>> Document Shiji Lighting APA102C LED driver device tree bindings.
>>
>> Signed-off-by: Nicolas Belin <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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
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