v9 EL15203000

v9 EL15203000

[Oleh Kravchenko]

[PATCH v9 1/2] dt-bindings: Add docs for EL15203000
[PATCH v9 2/2] leds: add LED driver for EL15203000 board

Changes what was made:
- updated Documentation/devicetree/bindings/leds/leds-el15203000.txt
  after code review from Dan Murphy.

[PATCH v9 1/2] dt-bindings: Add docs for EL15203000

[Oleh Kravchenko]

Add documentation and example for dt-bindings EL15203000.
LED board (aka RED LED board) from Crane Merchandising Systems.

Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
---
 .../bindings/leds/leds-el15203000.txt         | 147 ++++++++++++++++++
 1 file changed, 147 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-el15203000.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-el15203000.txt b/Documentation/devicetree/bindings/leds/leds-el15203000.txt
new file mode 100644
index 000000000000..4a9b29cc9f46
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt
@@ -0,0 +1,147 @@
+Crane Merchandising System - EL15203000 LED driver
+--------------------------------------------------
+
+This LED Board (aka RED LEDs board) is widely used in
+coffee vending machines produced by Crane Merchandising Systems.
+The board manages 3 LEDs and supports predefined blinking patterns
+for specific leds.
+
+Vending area LED encoded with symbol 'V' (hex code 0x56).
+Doesn't have any hardware blinking pattern.
+
+Screen light tube LED which surrounds vending machine screen and
+encoded with symbol 'S' (hex code 0x53). Supports blinking breathing pattern:
+
+    ^
+    |
+Max >_____***___________**
+    |    *   *         *
+    |   *     *       *
+    |  *       *     *
+    | *         *   *
+Min >*___________***______
+    |
+    +------^------^------> time (sec)
+    0      4      8
+
+
+Water Pipe LED actually consists from 5 LEDs
+that exposed by protocol like one LED. Supports next patterns:
+
+- cascade pattern
+
+     ^
+     |
+LED0 >*****____________________*****____________________*****
+     |
+LED1 >_____*****____________________*****____________________
+     |
+LED2 >__________*****____________________*****_______________
+     |
+LED3 >_______________*****____________________*****__________
+     |
+LED4 >____________________*****____________________*****_____
+     |
+     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
+     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
+
+- inversed cascade pattern
+
+     ^
+     |
+LED0 >_____********************_____********************_____
+     |
+LED1 >*****_____********************_____********************
+     |
+LED2 >**********_____********************_____***************
+     |
+LED3 >***************_____********************_____**********
+     |
+LED4 >********************_____********************_____*****
+     |
+     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
+     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
+
+- bounce pattern
+
+     ^
+     |
+LED0 >*****________________________________________*****_____
+     |
+LED1 >_____*****______________________________*****_____*****
+     |
+LED2 >__________*****____________________*****_______________
+     |
+LED3 >_______________*****__________*****____________________
+     |
+LED4 >____________________**********_________________________
+     |
+     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
+     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
+
+- inversed bounce pattern
+
+     ^
+     |
+LED0 >_____****************************************_____*****
+     |
+LED1 >*****_____******************************_____*****_____
+     |
+LED2 >**********_____********************_____***************
+     |
+LED3 >***************_____**********_____********************
+     |
+LED4 >********************__________*************************
+     |
+     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
+     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
+
+Required properties:
+- compatible : "crane,el15203000"
+- #address-cells : must be 1
+- #size-cells : must be 0
+
+Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
+apply. In particular, "reg" and "spi-max-frequency" properties must be given.
+
+Optional LED sub-node properties:
+- function:
+	see Documentation/devicetree/bindings/leds/common.txt
+- color:
+	see Documentation/devicetree/bindings/leds/common.txt
+- label:
+	see Documentation/devicetree/bindings/leds/common.txt (deprecated)
+
+Example
+-------
+
+#include <dt-bindings/leds/common.h>
+
+led-controller@0 {
+	compatible = "crane,el15203000";
+	reg = <0>;
+	spi-max-frequency = <50000>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	/* water pipe */
+	led@50 {
+		reg = <0x50>;
+		function = "pipe";
+		color = <LED_COLOR_ID_RED>;
+	};
+
+	/* screen frame */
+	led@53 {
+		reg = <0x53>;
+		function = "screen";
+		color = <LED_COLOR_ID_RED>;
+	};
+
+	/* vending area */
+	led@56 {
+		reg = <0x56>;
+		function = "vend";
+		color = <LED_COLOR_ID_RED>;
+	};
+};
-- 
2.21.0

[PATCH v9 2/2] leds: add LED driver for EL15203000 board

[Oleh Kravchenko]

This patch adds a LED class driver for the LEDs found on
the Crane Merchandising System EL15203000 LEDs board
(aka RED LEDs board).

Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
Reviewed-by: Dan Murphy <dmurphy@ti.com>
---
 .../testing/sysfs-class-led-driver-el15203000 |  32 ++
 drivers/leds/Kconfig                          |  13 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-el15203000.c                | 357 ++++++++++++++++++
 4 files changed, 403 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-el15203000
 create mode 100644 drivers/leds/leds-el15203000.c

diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-el15203000 b/Documentation/ABI/testing/sysfs-class-led-driver-el15203000
new file mode 100644
index 000000000000..5e9cbf49da59
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-led-driver-el15203000
@@ -0,0 +1,32 @@
+What:		/sys/class/leds/<led>/hw_pattern
+Date:		September 2019
+KernelVersion:	5.5
+Description:
+		Specify a hardware pattern for the EL15203000 LED.
+		The LEDs board supports only predefined patterns by firmware
+		for specific LEDs.
+
+		Breathing mode for Screen frame light tube:
+		"0 4000 1 4000"
+
+		Cascade mode for Pipe LED:
+		"1 800 2 800 4 800 8 800 16 800"
+
+		Inverted cascade mode for Pipe LED:
+		"30 800 29 800 27 800 23 800 15 800"
+
+		Bounce mode for Pipe LED:
+		"1 800 2 800 4 800 8 800 16 800 16 800 8 800 4 800 2 800 1 800"
+
+		Inverted bounce mode for Pipe LED:
+		"30 800 29 800 27 800 23 800 15 800 15 800 23 800 27 800 29 800 30 800"
+
+What:		/sys/class/leds/<led>/repeat
+Date:		September 2019
+KernelVersion:	5.5
+Description:
+		EL15203000 supports only indefinitely patterns,
+		so this file should always store -1.
+
+		For more info, please see:
+		Documentation/ABI/testing/sysfs-class-led-trigger-pattern
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1988de1d64c0..6e7703fd03d0 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -132,6 +132,19 @@ config LEDS_CR0014114
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-cr0014114.
 
+config LEDS_EL15203000
+	tristate "LED Support for Crane EL15203000"
+	depends on LEDS_CLASS
+	depends on SPI
+	depends on OF
+	help
+	  This option enables support for EL15203000 LED Board
+	  (aka RED LED board) which is widely used in coffee vending
+	  machines produced by Crane Merchandising Systems.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called leds-el15203000.
+
 config LEDS_LM3530
 	tristate "LCD Backlight driver for LM3530"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 41fb073a39c1..2da39e896ce8 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_LEDS_LM36274)		+= leds-lm36274.o
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
 obj-$(CONFIG_LEDS_DAC124S085)		+= leds-dac124s085.o
+obj-$(CONFIG_LEDS_EL15203000)		+= leds-el15203000.o
 
 # LED Userspace Drivers
 obj-$(CONFIG_LEDS_USER)			+= uleds.o
diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
new file mode 100644
index 000000000000..298b13e4807a
--- /dev/null
+++ b/drivers/leds/leds-el15203000.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Crane Merchandising Systems. All rights reserved.
+// Copyright (C) 2019 Oleh Kravchenko <oleg@kaa.org.ua>
+
+#include <linux/delay.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi.h>
+
+/*
+ * EL15203000 SPI protocol description:
+ * +-----+---------+
+ * | LED | COMMAND |
+ * +-----+---------+
+ * |  1  |    1    |
+ * +-----+---------+
+ * (*) LEDs MCU board expects 20 msec delay per byte.
+ *
+ * LEDs:
+ * +----------+--------------+-------------------------------------------+
+ * |    ID    |     NAME     |         DESCRIPTION                       |
+ * +----------+--------------+-------------------------------------------+
+ * | 'P' 0x50 |     Pipe     | Consists from 5 LEDs, controlled by board |
+ * +----------+--------------+-------------------------------------------+
+ * | 'S' 0x53 | Screen frame | Light tube around the screen              |
+ * +----------+--------------+-------------------------------------------+
+ * | 'V' 0x56 | Vending area | Highlights a cup of coffee                |
+ * +----------+--------------+-------------------------------------------+
+ *
+ * COMMAND:
+ * +----------+-----------------+--------------+--------------+
+ * |  VALUES  |       PIPE      | SCREEN FRAME | VENDING AREA |
+ * +----------+-----------------+--------------+--------------+
+ * | '0' 0x30 |                      Off                      |
+ * +----------+-----------------------------------------------+
+ * | '1' 0x31 |                      On                       |
+ * +----------+-----------------+--------------+--------------+
+ * | '2' 0x32 |     Cascade     |   Breathing  |
+ * +----------+-----------------+--------------+
+ * | '3' 0x33 | Inverse cascade |
+ * +----------+-----------------+
+ * | '4' 0x34 |     Bounce      |
+ * +----------+-----------------+
+ * | '5' 0x35 | Inverse bounce  |
+ * +----------+-----------------+
+ */
+
+/* EL15203000 default settings */
+#define EL_FW_DELAY_USEC	20000ul
+#define EL_PATTERN_DELAY_MSEC	800u
+#define EL_PATTERN_LEN		10u
+#define EL_PATTERN_HALF_LEN	(EL_PATTERN_LEN / 2)
+
+enum el15203000_command {
+	/* for all LEDs */
+	EL_OFF			= '0',
+	EL_ON			= '1',
+
+	/* for Screen LED */
+	EL_SCREEN_BREATHING	= '2',
+
+	/* for Pipe LED */
+	EL_PIPE_CASCADE		= '2',
+	EL_PIPE_INV_CASCADE	= '3',
+	EL_PIPE_BOUNCE		= '4',
+	EL_PIPE_INV_BOUNCE	= '5',
+};
+
+struct el15203000_led {
+	struct el15203000	*priv;
+	struct led_classdev	ldev;
+	u32			reg;
+};
+
+struct el15203000 {
+	struct device		*dev;
+	struct mutex		lock;
+	struct spi_device	*spi;
+	unsigned long		delay;
+	size_t			count;
+	struct el15203000_led	leds[];
+};
+
+static int el15203000_cmd(struct el15203000_led *led, u8 brightness)
+{
+	int		ret;
+	u8		cmd[2];
+	size_t		i;
+
+	mutex_lock(&led->priv->lock);
+
+	dev_dbg(led->priv->dev, "Set brightness of 0x%02x(%c) to 0x%02x(%c)",
+		led->reg, led->reg, brightness, brightness);
+
+	/* to avoid SPI mistiming with firmware we should wait some time */
+	if (time_after(led->priv->delay, jiffies)) {
+		dev_dbg(led->priv->dev, "Wait %luus to sync",
+			EL_FW_DELAY_USEC);
+
+		usleep_range(EL_FW_DELAY_USEC,
+			     EL_FW_DELAY_USEC + 1);
+	}
+
+	cmd[0] = led->reg;
+	cmd[1] = brightness;
+
+	for (i = 0; i < ARRAY_SIZE(cmd); i++) {
+		if (i)
+			usleep_range(EL_FW_DELAY_USEC,
+				     EL_FW_DELAY_USEC + 1);
+
+		ret = spi_write(led->priv->spi, &cmd[i], sizeof(cmd[i]));
+		if (ret) {
+			dev_err(led->priv->dev,
+				"spi_write() error %d", ret);
+			break;
+		}
+	}
+
+	led->priv->delay = jiffies + usecs_to_jiffies(EL_FW_DELAY_USEC);
+
+	mutex_unlock(&led->priv->lock);
+
+	return ret;
+}
+
+static int el15203000_set_blocking(struct led_classdev *ldev,
+				   enum led_brightness brightness)
+{
+	struct el15203000_led *led = container_of(ldev,
+						  struct el15203000_led,
+						  ldev);
+
+	return el15203000_cmd(led, brightness == LED_OFF ? EL_OFF : EL_ON);
+}
+
+static int el15203000_pattern_set_S(struct led_classdev *ldev,
+				    struct led_pattern *pattern,
+				    u32 len, int repeat)
+{
+	struct el15203000_led *led = container_of(ldev,
+						  struct el15203000_led,
+						  ldev);
+
+	if (repeat > 0 || len != 2 ||
+	    pattern[0].delta_t != 4000 || pattern[0].brightness != 0 ||
+	    pattern[1].delta_t != 4000 || pattern[1].brightness != 1)
+		return -EINVAL;
+
+	dev_dbg(led->priv->dev, "Breathing mode for 0x%02x(%c)",
+		led->reg, led->reg);
+
+	return el15203000_cmd(led, EL_SCREEN_BREATHING);
+}
+
+static bool is_cascade(const struct led_pattern *pattern, u32 len,
+		       bool inv, bool right)
+{
+	int val, t;
+	u32 i;
+
+	if (len != EL_PATTERN_HALF_LEN)
+		return false;
+
+	val = right ? BIT(4) : BIT(0);
+
+	for (i = 0; i < len; i++) {
+		t = inv ? ~val & GENMASK(4, 0) : val;
+
+		if (pattern[i].delta_t != EL_PATTERN_DELAY_MSEC ||
+		    pattern[i].brightness != t)
+			return false;
+
+		val = right ? val >> 1 : val << 1;
+	}
+
+	return true;
+}
+
+static bool is_bounce(const struct led_pattern *pattern, u32 len, bool inv)
+{
+	if (len != EL_PATTERN_LEN)
+		return false;
+
+	return is_cascade(pattern, EL_PATTERN_HALF_LEN, inv, false) &&
+	       is_cascade(pattern + EL_PATTERN_HALF_LEN,
+			  EL_PATTERN_HALF_LEN, inv, true);
+}
+
+static int el15203000_pattern_set_P(struct led_classdev *ldev,
+				    struct led_pattern *pattern,
+				    u32 len, int repeat)
+{
+	u8			cmd;
+	struct el15203000_led	*led = container_of(ldev,
+						    struct el15203000_led,
+						    ldev);
+
+	if (repeat > 0)
+		return -EINVAL;
+
+	if (is_cascade(pattern, len, false, false)) {
+		dev_dbg(led->priv->dev, "Cascade mode for 0x%02x(%c)",
+			led->reg, led->reg);
+
+		cmd = EL_PIPE_CASCADE;
+	} else if (is_cascade(pattern, len, true, false)) {
+		dev_dbg(led->priv->dev, "Inverse cascade mode for 0x%02x(%c)",
+			led->reg, led->reg);
+
+		cmd = EL_PIPE_INV_CASCADE;
+	} else if (is_bounce(pattern, len, false)) {
+		dev_dbg(led->priv->dev, "Bounce mode for 0x%02x(%c)",
+			led->reg, led->reg);
+
+		cmd = EL_PIPE_BOUNCE;
+	} else if (is_bounce(pattern, len, true)) {
+		dev_dbg(led->priv->dev, "Inverse bounce mode for 0x%02x(%c)",
+			led->reg, led->reg);
+
+		cmd = EL_PIPE_INV_BOUNCE;
+	} else {
+		dev_err(led->priv->dev, "Invalid hw_pattern for 0x%02x(%c)!",
+			led->reg, led->reg);
+
+		return -EINVAL;
+	}
+
+	return el15203000_cmd(led, cmd);
+}
+
+static int el15203000_pattern_clear(struct led_classdev *ldev)
+{
+	struct el15203000_led	*led = container_of(ldev,
+						    struct el15203000_led,
+						    ldev);
+
+	return el15203000_cmd(led, EL_OFF);
+}
+
+static int el15203000_probe_dt(struct el15203000 *priv)
+{
+	struct el15203000_led	*led = priv->leds;
+	struct fwnode_handle	*child;
+	int			ret;
+
+	device_for_each_child_node(priv->dev, child) {
+		struct led_init_data init_data = {};
+
+		ret = fwnode_property_read_u32(child, "reg", &led->reg);
+		if (ret) {
+			dev_err(priv->dev, "LED without ID number");
+			fwnode_handle_put(child);
+
+			break;
+		}
+
+		if (led->reg > U8_MAX) {
+			dev_err(priv->dev, "LED value %d is invalid", led->reg);
+			fwnode_handle_put(child);
+
+			return -EINVAL;
+		}
+
+		fwnode_property_read_string(child, "linux,default-trigger",
+					    &led->ldev.default_trigger);
+
+		led->priv			  = priv;
+		led->ldev.max_brightness	  = LED_ON;
+		led->ldev.brightness_set_blocking = el15203000_set_blocking;
+
+		if (led->reg == 'S') {
+			led->ldev.pattern_set	= el15203000_pattern_set_S;
+			led->ldev.pattern_clear	= el15203000_pattern_clear;
+		} else if (led->reg == 'P') {
+			led->ldev.pattern_set	= el15203000_pattern_set_P;
+			led->ldev.pattern_clear	= el15203000_pattern_clear;
+		}
+
+		init_data.fwnode = child;
+		ret = devm_led_classdev_register_ext(priv->dev, &led->ldev,
+						     &init_data);
+		if (ret) {
+			dev_err(priv->dev,
+				"failed to register LED device %s, err %d",
+				led->ldev.name, ret);
+			fwnode_handle_put(child);
+
+			break;
+		}
+
+		led++;
+	}
+
+	return ret;
+}
+
+static int el15203000_probe(struct spi_device *spi)
+{
+	struct el15203000	*priv;
+	size_t			count;
+
+	count = device_get_child_node_count(&spi->dev);
+	if (!count) {
+		dev_err(&spi->dev, "LEDs are not defined in device tree!");
+		return -ENODEV;
+	}
+
+	priv = devm_kzalloc(&spi->dev, struct_size(priv, leds, count),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_init(&priv->lock);
+	priv->count	= count;
+	priv->dev	= &spi->dev;
+	priv->spi	= spi;
+	priv->delay	= jiffies -
+			  usecs_to_jiffies(EL_FW_DELAY_USEC);
+
+	spi_set_drvdata(spi, priv);
+
+	return el15203000_probe_dt(priv);
+}
+
+static int el15203000_remove(struct spi_device *spi)
+{
+	struct el15203000 *priv = spi_get_drvdata(spi);
+
+	mutex_destroy(&priv->lock);
+
+	return 0;
+}
+
+static const struct of_device_id el15203000_dt_ids[] = {
+	{ .compatible = "crane,el15203000", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, el15203000_dt_ids);
+
+static struct spi_driver el15203000_driver = {
+	.probe		= el15203000_probe,
+	.remove		= el15203000_remove,
+	.driver = {
+		.name		= KBUILD_MODNAME,
+		.of_match_table	= el15203000_dt_ids,
+	},
+};
+
+module_spi_driver(el15203000_driver);
+
+MODULE_AUTHOR("Oleh Kravchenko <oleg@kaa.org.ua>");
+MODULE_DESCRIPTION("el15203000 LED driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spi:el15203000");
-- 
2.21.0

Re: [PATCH v9 1/2] dt-bindings: Add docs for EL15203000

[Jacek Anaszewski]

Hi Oleh,

Thank you for the update.

I have some comments below. Please take a look.

On 9/18/19 12:52 PM, Oleh Kravchenko wrote:
> Add documentation and example for dt-bindings EL15203000.
> LED board (aka RED LED board) from Crane Merchandising Systems.
> 
> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
> ---
>  .../bindings/leds/leds-el15203000.txt         | 147 ++++++++++++++++++
>  1 file changed, 147 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-el15203000.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-el15203000.txt b/Documentation/devicetree/bindings/leds/leds-el15203000.txt
> new file mode 100644
> index 000000000000..4a9b29cc9f46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt
> @@ -0,0 +1,147 @@
> +Crane Merchandising System - EL15203000 LED driver
> +--------------------------------------------------
> +
> +This LED Board (aka RED LEDs board) is widely used in
> +coffee vending machines produced by Crane Merchandising Systems.
> +The board manages 3 LEDs and supports predefined blinking patterns
> +for specific leds.
> +
> +Vending area LED encoded with symbol 'V' (hex code 0x56).
> +Doesn't have any hardware blinking pattern.
> +
> +Screen light tube LED which surrounds vending machine screen and
> +encoded with symbol 'S' (hex code 0x53). Supports blinking breathing pattern:
> +
> +    ^
> +    |
> +Max >_____***___________**
> +    |    *   *         *
> +    |   *     *       *
> +    |  *       *     *
> +    | *         *   *
> +Min >*___________***______
> +    |
> +    +------^------^------> time (sec)
> +    0      4      8
> +
> +
> +Water Pipe LED actually consists from 5 LEDs

"(hex code 0x50)" is here missing if you want to be consistent.

> +that exposed by protocol like one LED. Supports next patterns:
> +
> +- cascade pattern
> +
> +     ^
> +     |
> +LED0 >*****____________________*****____________________*****
> +     |
> +LED1 >_____*****____________________*****____________________
> +     |
> +LED2 >__________*****____________________*****_______________
> +     |
> +LED3 >_______________*****____________________*****__________
> +     |
> +LED4 >____________________*****____________________*****_____
> +     |
> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
> +
> +- inversed cascade pattern
> +
> +     ^
> +     |
> +LED0 >_____********************_____********************_____
> +     |
> +LED1 >*****_____********************_____********************
> +     |
> +LED2 >**********_____********************_____***************
> +     |
> +LED3 >***************_____********************_____**********
> +     |
> +LED4 >********************_____********************_____*****
> +     |
> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
> +
> +- bounce pattern
> +
> +     ^
> +     |
> +LED0 >*****________________________________________*****_____
> +     |
> +LED1 >_____*****______________________________*****_____*****
> +     |
> +LED2 >__________*****____________________*****_______________
> +     |
> +LED3 >_______________*****__________*****____________________
> +     |
> +LED4 >____________________**********_________________________
> +     |
> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
> +
> +- inversed bounce pattern
> +
> +     ^
> +     |
> +LED0 >_____****************************************_____*****
> +     |
> +LED1 >*****_____******************************_____*****_____
> +     |
> +LED2 >**********_____********************_____***************
> +     |
> +LED3 >***************_____**********_____********************
> +     |
> +LED4 >********************__________*************************
> +     |
> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8

Regarding this ASCII art - I presume you wanted to follow
Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt.

It was added to bindings because we support 'pattern' value
for linux,default-trigger, which in turn looks for 'led-pattern'
property, whose format needs to be documented in the LED bindings.

leds-trigger-pattern.txt documents only common syntax for software
pattern engine. Currently we don't have a means to setup hw_pattern
for the pattern trigger from DT, which is obvious omission and your
patch just brings it to light.

That said, I propose to fix it alongside and introduce led-hw-pattern
property. When present, ledtrig-pattern would setup the pattern
using pattern_set op, similarly as if it was set via sysfs hw_pattern
file.

Only in this case placing the pattern depiction here would be justified.
Otherwise, it would have to land in the ABI documentation.

> +
> +Required properties:
> +- compatible : "crane,el15203000"
> +- #address-cells : must be 1
> +- #size-cells : must be 0
> +
> +Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
> +apply. In particular, "reg" and "spi-max-frequency" properties must be given.
> +
> +Optional LED sub-node properties:
> +- function:
> +	see Documentation/devicetree/bindings/leds/common.txt
> +- color:
> +	see Documentation/devicetree/bindings/leds/common.txt
> +- label:
> +	see Documentation/devicetree/bindings/leds/common.txt (deprecated)
> +
> +Example
> +-------
> +
> +#include <dt-bindings/leds/common.h>
> +
> +led-controller@0 {
> +	compatible = "crane,el15203000";
> +	reg = <0>;
> +	spi-max-frequency = <50000>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	/* water pipe */
> +	led@50 {
> +		reg = <0x50>;
> +		function = "pipe";
> +		color = <LED_COLOR_ID_RED>;
> +	};
> +
> +	/* screen frame */
> +	led@53 {
> +		reg = <0x53>;
> +		function = "screen";
> +		color = <LED_COLOR_ID_RED>;
> +	};
> +
> +	/* vending area */
> +	led@56 {
> +		reg = <0x56>;
> +		function = "vend";
> +		color = <LED_COLOR_ID_RED>;
> +	};
> +};
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v9 1/2] dt-bindings: Add docs for EL15203000

[Oleh Kravchenko]

Hello Jacek,

19.09.19 00:02, Jacek Anaszewski пише:
> Hi Oleh,
> 
> Thank you for the update.
> 
> I have some comments below. Please take a look.
> 
> On 9/18/19 12:52 PM, Oleh Kravchenko wrote:
>> Add documentation and example for dt-bindings EL15203000.
>> LED board (aka RED LED board) from Crane Merchandising Systems.
>>
>> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
>> ---
>>  .../bindings/leds/leds-el15203000.txt         | 147 ++++++++++++++++++
>>  1 file changed, 147 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/leds/leds-el15203000.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-el15203000.txt b/Documentation/devicetree/bindings/leds/leds-el15203000.txt
>> new file mode 100644
>> index 000000000000..4a9b29cc9f46
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt
>> @@ -0,0 +1,147 @@
>> +Crane Merchandising System - EL15203000 LED driver
>> +--------------------------------------------------
>> +
>> +This LED Board (aka RED LEDs board) is widely used in
>> +coffee vending machines produced by Crane Merchandising Systems.
>> +The board manages 3 LEDs and supports predefined blinking patterns
>> +for specific leds.
>> +
>> +Vending area LED encoded with symbol 'V' (hex code 0x56).
>> +Doesn't have any hardware blinking pattern.
>> +
>> +Screen light tube LED which surrounds vending machine screen and
>> +encoded with symbol 'S' (hex code 0x53). Supports blinking breathing pattern:
>> +
>> +    ^
>> +    |
>> +Max >_____***___________**
>> +    |    *   *         *
>> +    |   *     *       *
>> +    |  *       *     *
>> +    | *         *   *
>> +Min >*___________***______
>> +    |
>> +    +------^------^------> time (sec)
>> +    0      4      8
>> +
>> +
>> +Water Pipe LED actually consists from 5 LEDs
> 
> "(hex code 0x50)" is here missing if you want to be consistent.
> 
>> +that exposed by protocol like one LED. Supports next patterns:
>> +
>> +- cascade pattern
>> +
>> +     ^
>> +     |
>> +LED0 >*****____________________*****____________________*****
>> +     |
>> +LED1 >_____*****____________________*****____________________
>> +     |
>> +LED2 >__________*****____________________*****_______________
>> +     |
>> +LED3 >_______________*****____________________*****__________
>> +     |
>> +LED4 >____________________*****____________________*****_____
>> +     |
>> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
>> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
>> +
>> +- inversed cascade pattern
>> +
>> +     ^
>> +     |
>> +LED0 >_____********************_____********************_____
>> +     |
>> +LED1 >*****_____********************_____********************
>> +     |
>> +LED2 >**********_____********************_____***************
>> +     |
>> +LED3 >***************_____********************_____**********
>> +     |
>> +LED4 >********************_____********************_____*****
>> +     |
>> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
>> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
>> +
>> +- bounce pattern
>> +
>> +     ^
>> +     |
>> +LED0 >*****________________________________________*****_____
>> +     |
>> +LED1 >_____*****______________________________*****_____*****
>> +     |
>> +LED2 >__________*****____________________*****_______________
>> +     |
>> +LED3 >_______________*****__________*****____________________
>> +     |
>> +LED4 >____________________**********_________________________
>> +     |
>> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
>> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
>> +
>> +- inversed bounce pattern
>> +
>> +     ^
>> +     |
>> +LED0 >_____****************************************_____*****
>> +     |
>> +LED1 >*****_____******************************_____*****_____
>> +     |
>> +LED2 >**********_____********************_____***************
>> +     |
>> +LED3 >***************_____**********_____********************
>> +     |
>> +LED4 >********************__________*************************
>> +     |
>> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
>> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
> 
> Regarding this ASCII art - I presume you wanted to follow
> Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt.
> 
> It was added to bindings because we support 'pattern' value
> for linux,default-trigger, which in turn looks for 'led-pattern'
> property, whose format needs to be documented in the LED bindings.
> 
> leds-trigger-pattern.txt documents only common syntax for software
> pattern engine. Currently we don't have a means to setup hw_pattern
> for the pattern trigger from DT, which is obvious omission and your
> patch just brings it to light.
> 
> That said, I propose to fix it alongside and introduce led-hw-pattern
> property. When present, ledtrig-pattern would setup the pattern
> using pattern_set op, similarly as if it was set via sysfs hw_pattern
> file.
> 
> Only in this case placing the pattern depiction here would be justified.
> Otherwise, it would have to land in the ABI documentation.

You are okay, if I move it to Documentation/ABI/testing/sysfs-class-led-driver-el15203000 ?

-- 
Best regards,
Oleh Kravchenko

Re: [PATCH v9 1/2] dt-bindings: Add docs for EL15203000

[Jacek Anaszewski]

On 9/18/19 11:17 PM, Oleh Kravchenko wrote:
> Hello Jacek,
> 
> 19.09.19 00:02, Jacek Anaszewski пише:
>> Hi Oleh,
>>
>> Thank you for the update.
>>
>> I have some comments below. Please take a look.
>>
>> On 9/18/19 12:52 PM, Oleh Kravchenko wrote:
>>> Add documentation and example for dt-bindings EL15203000.
>>> LED board (aka RED LED board) from Crane Merchandising Systems.
>>>
>>> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
>>> ---
>>>  .../bindings/leds/leds-el15203000.txt         | 147 ++++++++++++++++++
>>>  1 file changed, 147 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/leds/leds-el15203000.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/leds-el15203000.txt b/Documentation/devicetree/bindings/leds/leds-el15203000.txt
>>> new file mode 100644
>>> index 000000000000..4a9b29cc9f46
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/leds/leds-el15203000.txt
>>> @@ -0,0 +1,147 @@
>>> +Crane Merchandising System - EL15203000 LED driver
>>> +--------------------------------------------------
>>> +
>>> +This LED Board (aka RED LEDs board) is widely used in
>>> +coffee vending machines produced by Crane Merchandising Systems.
>>> +The board manages 3 LEDs and supports predefined blinking patterns
>>> +for specific leds.
>>> +
>>> +Vending area LED encoded with symbol 'V' (hex code 0x56).
>>> +Doesn't have any hardware blinking pattern.
>>> +
>>> +Screen light tube LED which surrounds vending machine screen and
>>> +encoded with symbol 'S' (hex code 0x53). Supports blinking breathing pattern:
>>> +
>>> +    ^
>>> +    |
>>> +Max >_____***___________**
>>> +    |    *   *         *
>>> +    |   *     *       *
>>> +    |  *       *     *
>>> +    | *         *   *
>>> +Min >*___________***______
>>> +    |
>>> +    +------^------^------> time (sec)
>>> +    0      4      8
>>> +
>>> +
>>> +Water Pipe LED actually consists from 5 LEDs
>>
>> "(hex code 0x50)" is here missing if you want to be consistent.
>>
>>> +that exposed by protocol like one LED. Supports next patterns:
>>> +
>>> +- cascade pattern
>>> +
>>> +     ^
>>> +     |
>>> +LED0 >*****____________________*****____________________*****
>>> +     |
>>> +LED1 >_____*****____________________*****____________________
>>> +     |
>>> +LED2 >__________*****____________________*****_______________
>>> +     |
>>> +LED3 >_______________*****____________________*****__________
>>> +     |
>>> +LED4 >____________________*****____________________*****_____
>>> +     |
>>> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
>>> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
>>> +
>>> +- inversed cascade pattern
>>> +
>>> +     ^
>>> +     |
>>> +LED0 >_____********************_____********************_____
>>> +     |
>>> +LED1 >*****_____********************_____********************
>>> +     |
>>> +LED2 >**********_____********************_____***************
>>> +     |
>>> +LED3 >***************_____********************_____**********
>>> +     |
>>> +LED4 >********************_____********************_____*****
>>> +     |
>>> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
>>> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
>>> +
>>> +- bounce pattern
>>> +
>>> +     ^
>>> +     |
>>> +LED0 >*****________________________________________*****_____
>>> +     |
>>> +LED1 >_____*****______________________________*****_____*****
>>> +     |
>>> +LED2 >__________*****____________________*****_______________
>>> +     |
>>> +LED3 >_______________*****__________*****____________________
>>> +     |
>>> +LED4 >____________________**********_________________________
>>> +     |
>>> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
>>> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
>>> +
>>> +- inversed bounce pattern
>>> +
>>> +     ^
>>> +     |
>>> +LED0 >_____****************************************_____*****
>>> +     |
>>> +LED1 >*****_____******************************_____*****_____
>>> +     |
>>> +LED2 >**********_____********************_____***************
>>> +     |
>>> +LED3 >***************_____**********_____********************
>>> +     |
>>> +LED4 >********************__________*************************
>>> +     |
>>> +     +----^----^----^----^----^----^----^----^----^----^----> time (sec)
>>> +     0   0.8  1.6  2.4  3.2   4   4.8  5.6  6.4  7.2   8
>>
>> Regarding this ASCII art - I presume you wanted to follow
>> Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt.
>>
>> It was added to bindings because we support 'pattern' value
>> for linux,default-trigger, which in turn looks for 'led-pattern'
>> property, whose format needs to be documented in the LED bindings.
>>
>> leds-trigger-pattern.txt documents only common syntax for software
>> pattern engine. Currently we don't have a means to setup hw_pattern
>> for the pattern trigger from DT, which is obvious omission and your
>> patch just brings it to light.
>>
>> That said, I propose to fix it alongside and introduce led-hw-pattern
>> property. When present, ledtrig-pattern would setup the pattern
>> using pattern_set op, similarly as if it was set via sysfs hw_pattern
>> file.
>>
>> Only in this case placing the pattern depiction here would be justified.
>> Otherwise, it would have to land in the ABI documentation.
> 
> You are okay, if I move it to Documentation/ABI/testing/sysfs-class-led-driver-el15203000 ?
> 

Yes, we can cover led-hw-pattern property later.

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v9 1/2] dt-bindings: Add docs for EL15203000

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]

Hi!

> > Regarding this ASCII art - I presume you wanted to follow
> > Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt.
> > 
> > It was added to bindings because we support 'pattern' value
> > for linux,default-trigger, which in turn looks for 'led-pattern'
> > property, whose format needs to be documented in the LED bindings.
> > 
> > leds-trigger-pattern.txt documents only common syntax for software
> > pattern engine. Currently we don't have a means to setup hw_pattern
> > for the pattern trigger from DT, which is obvious omission and your
> > patch just brings it to light.
> > 
> > That said, I propose to fix it alongside and introduce led-hw-pattern
> > property. When present, ledtrig-pattern would setup the pattern
> > using pattern_set op, similarly as if it was set via sysfs hw_pattern
> > file.
> > 
> > Only in this case placing the pattern depiction here would be justified.
> > Otherwise, it would have to land in the ABI documentation.
> 
> You are okay, if I move it to Documentation/ABI/testing/sysfs-class-led-driver-el15203000 ?

I don't see if this got a reply. Yes,
Documentation/ABI/testing/sysfs-class-led-driver-el15203000 sounds
like a right place for the sysfs description.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v9 1/2] dt-bindings: Add docs for EL15203000

[Jacek Anaszewski]

Hi Pavel.

On 10/13/19 2:11 PM, Pavel Machek wrote:
> Hi!
> 
>>> Regarding this ASCII art - I presume you wanted to follow
>>> Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt.
>>>
>>> It was added to bindings because we support 'pattern' value
>>> for linux,default-trigger, which in turn looks for 'led-pattern'
>>> property, whose format needs to be documented in the LED bindings.
>>>
>>> leds-trigger-pattern.txt documents only common syntax for software
>>> pattern engine. Currently we don't have a means to setup hw_pattern
>>> for the pattern trigger from DT, which is obvious omission and your
>>> patch just brings it to light.
>>>
>>> That said, I propose to fix it alongside and introduce led-hw-pattern
>>> property. When present, ledtrig-pattern would setup the pattern
>>> using pattern_set op, similarly as if it was set via sysfs hw_pattern
>>> file.
>>>
>>> Only in this case placing the pattern depiction here would be justified.
>>> Otherwise, it would have to land in the ABI documentation.
>>
>> You are okay, if I move it to Documentation/ABI/testing/sysfs-class-led-driver-el15203000 ?
> 
> I don't see if this got a reply. Yes,
> Documentation/ABI/testing/sysfs-class-led-driver-el15203000 sounds
> like a right place for the sysfs description.

There was no explicit reply because Oleh sent the update with this
change before I managed to respond. And the patch is already in the
for-next branch with the file in the discussed location. So the reply
was in the form of my later confirmation that the patch was applied.

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v9 1/2] dt-bindings: Add docs for EL15203000

[Oleh Kravchenko]

[-- Attachment #1.1: Type: text/plain, Size: 1772 bytes --]

Hello Pavel,

13.10.19 19:47, Jacek Anaszewski пише:
> Hi Pavel.
> 
> On 10/13/19 2:11 PM, Pavel Machek wrote:
>> Hi!
>>
>>>> Regarding this ASCII art - I presume you wanted to follow
>>>> Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt.
>>>>
>>>> It was added to bindings because we support 'pattern' value
>>>> for linux,default-trigger, which in turn looks for 'led-pattern'
>>>> property, whose format needs to be documented in the LED bindings.
>>>>
>>>> leds-trigger-pattern.txt documents only common syntax for software
>>>> pattern engine. Currently we don't have a means to setup hw_pattern
>>>> for the pattern trigger from DT, which is obvious omission and your
>>>> patch just brings it to light.
>>>>
>>>> That said, I propose to fix it alongside and introduce led-hw-pattern
>>>> property. When present, ledtrig-pattern would setup the pattern
>>>> using pattern_set op, similarly as if it was set via sysfs hw_pattern
>>>> file.
>>>>
>>>> Only in this case placing the pattern depiction here would be justified.
>>>> Otherwise, it would have to land in the ABI documentation.
>>>
>>> You are okay, if I move it to Documentation/ABI/testing/sysfs-class-led-driver-el15203000 ?
>>
>> I don't see if this got a reply. Yes,
>> Documentation/ABI/testing/sysfs-class-led-driver-el15203000 sounds
>> like a right place for the sysfs description.
> 
> There was no explicit reply because Oleh sent the update with this
> change before I managed to respond. And the patch is already in the
> for-next branch with the file in the discussed location. So the reply
> was in the form of my later confirmation that the patch was applied.
> 

Anyway thanks for reply.

-- 
Best regards,
Oleh Kravchenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]