linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Device Tree support for LP5562 predefined patterns
@ 2021-05-11 20:48 Doug Zobel
  2021-05-11 20:48 ` [PATCH 1/2] leds: lp55xx: support predefined pattern in Device Tree Doug Zobel
  2021-05-11 20:48 ` [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern Doug Zobel
  0 siblings, 2 replies; 9+ messages in thread
From: Doug Zobel @ 2021-05-11 20:48 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Jacek Anaszewski, linux-leds,
	devicetree, linux-kernel
  Cc: Doug Zobel

Add support for the LP5562 pre-defined LED patterns in Device Tree.

Doug Zobel (2):
  leds: lp55xx: support predefined pattern in Device Tree
  dt: bindings: lp55xx: Add predefined LED pattern

 .../devicetree/bindings/leds/leds-lp55xx.yaml | 103 +++++++++++++++++-
 drivers/leds/leds-lp55xx-common.c             |  94 ++++++++++++++--
 2 files changed, 186 insertions(+), 11 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] leds: lp55xx: support predefined pattern in Device Tree
  2021-05-11 20:48 [PATCH 0/2] Device Tree support for LP5562 predefined patterns Doug Zobel
@ 2021-05-11 20:48 ` Doug Zobel
  2021-05-11 20:48 ` [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern Doug Zobel
  1 sibling, 0 replies; 9+ messages in thread
From: Doug Zobel @ 2021-05-11 20:48 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Jacek Anaszewski, linux-leds,
	devicetree, linux-kernel
  Cc: Doug Zobel

The predefined LED pattern supported by the lp5562 can only
be passed via the antiquated platform data structure.  This
patch allows the patterns to be defined in the Device Tree.

Signed-off-by: Doug Zobel <dougdev334@gmail.com>
---
 drivers/leds/leds-lp55xx-common.c | 94 +++++++++++++++++++++++++++----
 1 file changed, 84 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
index 81de1346bf5d..227577ab4a16 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -659,15 +659,25 @@ struct lp55xx_platform_data *lp55xx_of_populate_pdata(struct device *dev,
 	struct device_node *child;
 	struct lp55xx_platform_data *pdata;
 	struct lp55xx_led_config *cfg;
-	int num_channels;
-	int i = 0;
+	struct lp55xx_predef_pattern *pat = NULL;
+	int num_channels = 0;
+	int cfg_num = 0;
+	int num_patterns = 0;
+	int pat_num = 0;
+	int pat_size;
 	int ret;
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return ERR_PTR(-ENOMEM);
 
-	num_channels = of_get_available_child_count(np);
+	for_each_available_child_of_node(np, child) {
+		if (of_property_read_bool(child, "chan-name"))
+			num_channels++;
+		else if (of_property_read_bool(child, "pat-name"))
+			num_patterns++;
+	}
+
 	if (num_channels == 0) {
 		dev_err(dev, "no LED channels\n");
 		return ERR_PTR(-EINVAL);
@@ -677,19 +687,83 @@ struct lp55xx_platform_data *lp55xx_of_populate_pdata(struct device *dev,
 	if (!cfg)
 		return ERR_PTR(-ENOMEM);
 
-	pdata->led_config = &cfg[0];
-	pdata->num_channels = num_channels;
+	if (num_patterns) {
+		pat = devm_kcalloc(dev, num_patterns, sizeof(*pat), GFP_KERNEL);
+		if (!pat)
+			return ERR_PTR(-ENOMEM);
+	}
+
 	cfg->max_channel = chip->cfg->max_channel;
 
 	for_each_available_child_of_node(np, child) {
-		ret = lp55xx_parse_logical_led(child, cfg, i);
-		if (ret) {
-			of_node_put(child);
-			return ERR_PTR(-EINVAL);
+		if (of_property_read_bool(child, "chan-name")) {
+			ret = lp55xx_parse_logical_led(child, cfg, cfg_num);
+			if (ret) {
+				of_node_put(child);
+				return ERR_PTR(-EINVAL);
+			}
+			cfg_num++;
+		} else if (of_property_read_bool(child, "pat-name")) {
+			pat_size = of_property_count_elems_of_size(child,
+								   "pat-r",
+						sizeof((*pat[pat_num].r)));
+			if (pat_size <= 0) {
+				pat[pat_num].size_r = 0;
+			} else {
+				char *program = devm_kcalloc(dev, pat_size,
+					sizeof(*(pat[pat_num].r)), GFP_KERNEL);
+				if (!program)
+					return ERR_PTR(-ENOMEM);
+				of_property_read_u8_array(child, "pat-r",
+							  program,
+							  pat_size);
+				pat[pat_num].r = program;
+				pat[pat_num].size_r = pat_size;
+			}
+
+			pat_size = of_property_count_elems_of_size(child,
+								   "pat-g",
+						sizeof((*pat[pat_num].g)));
+			if (pat_size <= 0) {
+				pat[pat_num].size_g = 0;
+			} else {
+				char *program = devm_kcalloc(dev, pat_size,
+					sizeof(*(pat[pat_num].g)), GFP_KERNEL);
+				if (!program)
+					return ERR_PTR(-ENOMEM);
+				of_property_read_u8_array(child, "pat-g",
+							  program,
+							  pat_size);
+				pat[pat_num].g = program;
+				pat[pat_num].size_g = pat_size;
+			}
+
+			pat_size = of_property_count_elems_of_size(child,
+								   "pat-b",
+						sizeof((*pat[pat_num].b)));
+			if (pat_size <= 0) {
+				pat[pat_num].size_b = 0;
+			} else {
+				char *program = devm_kcalloc(dev, pat_size,
+					sizeof(*(pat[pat_num].b)), GFP_KERNEL);
+				if (!program)
+					return ERR_PTR(-ENOMEM);
+				of_property_read_u8_array(child, "pat-b",
+							  program,
+							  pat_size);
+				pat[pat_num].b = program;
+				pat[pat_num].size_b = pat_size;
+			}
+
+			pat_num++;
 		}
-		i++;
 	}
 
+	pdata->led_config = &cfg[0];
+	pdata->num_channels = num_channels;
+	pdata->patterns = &pat[0];
+	pdata->num_patterns = num_patterns;
+
 	of_property_read_string(np, "label", &pdata->label);
 	of_property_read_u8(np, "clock-mode", &pdata->clock_mode);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern
  2021-05-11 20:48 [PATCH 0/2] Device Tree support for LP5562 predefined patterns Doug Zobel
  2021-05-11 20:48 ` [PATCH 1/2] leds: lp55xx: support predefined pattern in Device Tree Doug Zobel
@ 2021-05-11 20:48 ` Doug Zobel
  2021-05-12 18:35   ` Rob Herring
                     ` (2 more replies)
  1 sibling, 3 replies; 9+ messages in thread
From: Doug Zobel @ 2021-05-11 20:48 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring, Jacek Anaszewski, linux-leds,
	devicetree, linux-kernel
  Cc: Doug Zobel

Add a new device tree object for LP5562 predfined led patterns.

Signed-off-by: Doug Zobel <dougdev334@gmail.com>
---
 .../devicetree/bindings/leds/leds-lp55xx.yaml | 103 +++++++++++++++++-
 1 file changed, 102 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
index f552cd143d5b..2524a84fe688 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
@@ -100,6 +100,31 @@ patternProperties:
         $ref: /schemas/types.yaml#/definitions/string
         description: name of channel
 
+  "(^pattern@[0-9a-f]$|pattern)":
+    type: object
+    $ref: common.yaml#
+    description: |
+      LP5562 sepcific object.  LED pattern program saved to and run on LP5562.
+    properties:
+      pat-name:
+        $ref: /schemas/types.yaml#/definitions/string
+        description: Name of pattern program
+
+      pat-r:
+        $ref: /schemas/types.yaml#/definitions/uint8-array
+        description: |
+          Program data for red channel.  See LP5562 datasheet for program format specification.
+
+      pat-g:
+        $ref: /schemas/types.yaml#/definitions/uint8-array
+        description: |
+          Program data for green channel.  See LP5562 datasheet for program format specification.
+
+      pat-b:
+        $ref: /schemas/types.yaml#/definitions/uint8-array
+        description: |
+          Program data for blue channel.  See LP5562 datasheet for program format specification.
+
 required:
   - compatible
   - reg
@@ -223,6 +248,82 @@ examples:
                };
             };
         };
-    };
 
+        led-controller@30 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            compatible = "ti,lp5562";
+            reg = <0x30>;
+
+            led@0 {
+                reg = <0>;
+                chan-name = "red";
+                color = <LED_COLOR_ID_RED>;
+            };
+
+            led@1 {
+                reg = <1>;
+                chan-name = "green";
+                color = <LED_COLOR_ID_GREEN>;
+            };
+
+            led@2 {
+                reg = <2>;
+                chan-name = "blue";
+                color = <LED_COLOR_ID_BLUE>;
+            };
+
+            pattern@1 {
+                /* Pulsing blue pattern
+                 *   Blue:
+                 *     027F: Ramp up 50%
+                 *     027F: Ramp up 50%
+                 *     4600: Wait 100ms
+                 *     02FF: Ramp down 50%
+                 *     02FF: Ramp down 50%
+                 *     4600: Wait 100ms
+                 *     0000: Goto start
+                 */
+                pat-name = "Pulsing Blue";
+                pat-b = [02 7f 02 7f 46 00 02 ff 02 ff 46 00 00 00];
+            };
+
+            pattern@2 {
+                /*
+                 * HSV rainbow
+                 *   Red:
+                 *     40FF: Set PWM 255
+                 *     41FF: Ramp down 50%
+                 *     41FF: Ramp down 50%
+                 *     41FF: Wait 1/2 ramp time
+                 *     41FF: Wait 1/2 ramp time
+                 *     417F: Ramp up 50%
+                 *     417F: Ramp up 50%
+                 *     0000: Goto start
+                 *   Green:
+                 *     4000: Set PWM 0
+                 *     417F: Ramp up 50%
+                 *     417F: Ramp up 50%
+                 *     41FF: Ramp down 50%
+                 *     41FF: Ramp down 50%
+                 *     41FF: Wait 1/2 ramp time
+                 *     41FF: Wait 1/2 ramp time
+                 *     0000: Goto start
+                 *   Blue:
+                 *     4000: Set PWM 0
+                 *     41FF: Wait 1/2 ramp time
+                 *     41FF: Wait 1/2 ramp time
+                 *     417F: Ramp up 50%
+                 *     417F: Ramp up 50%
+                 *     41FF: Ramp down 50%
+                 *     41FF: Ramp down 50%
+                 *     0000: Goto start
+                 */
+                pat-name = "HSV Rainbow";
+                pat-r = [40 ff 41 FF 41 FF 41 FF 41 FF 41 7F 41 7F 00 00];
+                pat-g = [40 00 41 7F 41 7F 41 FF 41 FF 41 FF 41 FF 00 00];
+                pat-b = [40 00 41 FF 41 FF 41 7F 41 7F 41 FF 41 FF 00 00];
+            };
+        };
+    };
 ...
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern
  2021-05-11 20:48 ` [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern Doug Zobel
@ 2021-05-12 18:35   ` Rob Herring
  2021-05-13  2:20   ` Rob Herring
  2021-05-16 18:22   ` Pavel Machek
  2 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2021-05-12 18:35 UTC (permalink / raw)
  To: Doug Zobel
  Cc: Rob Herring, devicetree, Jacek Anaszewski, linux-kernel,
	linux-leds, Pavel Machek

On Tue, 11 May 2021 15:48:34 -0500, Doug Zobel wrote:
> Add a new device tree object for LP5562 predfined led patterns.
> 
> Signed-off-by: Doug Zobel <dougdev334@gmail.com>
> ---
>  .../devicetree/bindings/leds/leds-lp55xx.yaml | 103 +++++++++++++++++-
>  1 file changed, 102 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/leds/leds-lp55xx.example.dts:159.28-172.20: Warning (unit_address_vs_reg): /example-0/i2c/led-controller@30/pattern@1: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/leds/leds-lp55xx.example.dts:174.28-209.20: Warning (unit_address_vs_reg): /example-0/i2c/led-controller@30/pattern@2: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/leds/leds-lp55xx.example.dts:147.24-151.20: Warning (unique_unit_address): /example-0/i2c/led-controller@30/led@1: duplicate unit-address (also used in node /example-0/i2c/led-controller@30/pattern@1)
Documentation/devicetree/bindings/leds/leds-lp55xx.example.dts:153.24-157.20: Warning (unique_unit_address): /example-0/i2c/led-controller@30/led@2: duplicate unit-address (also used in node /example-0/i2c/led-controller@30/pattern@2)

See https://patchwork.ozlabs.org/patch/1477300

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern
  2021-05-11 20:48 ` [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern Doug Zobel
  2021-05-12 18:35   ` Rob Herring
@ 2021-05-13  2:20   ` Rob Herring
  2021-05-13 20:47     ` Doug Zobel
  2021-05-16 18:22   ` Pavel Machek
  2 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2021-05-13  2:20 UTC (permalink / raw)
  To: Doug Zobel
  Cc: Pavel Machek, Jacek Anaszewski, linux-leds, devicetree, linux-kernel

On Tue, May 11, 2021 at 03:48:34PM -0500, Doug Zobel wrote:
> Add a new device tree object for LP5562 predfined led patterns.

If you are going to define something generic looking, put it in a 
generic binding.

I don't know that this belongs in DT though. Won't a user want to create 
their own patterns? That means there should be a sysfs interface (which 
we either already have or has been attempted IIRC).

> 
> Signed-off-by: Doug Zobel <dougdev334@gmail.com>
> ---
>  .../devicetree/bindings/leds/leds-lp55xx.yaml | 103 +++++++++++++++++-
>  1 file changed, 102 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
> index f552cd143d5b..2524a84fe688 100644
> --- a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
> +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
> @@ -100,6 +100,31 @@ patternProperties:
>          $ref: /schemas/types.yaml#/definitions/string
>          description: name of channel
>  
> +  "(^pattern@[0-9a-f]$|pattern)":
> +    type: object
> +    $ref: common.yaml#
> +    description: |
> +      LP5562 sepcific object.  LED pattern program saved to and run on LP5562.
> +    properties:
> +      pat-name:
> +        $ref: /schemas/types.yaml#/definitions/string
> +        description: Name of pattern program
> +
> +      pat-r:
> +        $ref: /schemas/types.yaml#/definitions/uint8-array
> +        description: |
> +          Program data for red channel.  See LP5562 datasheet for program format specification.
> +
> +      pat-g:
> +        $ref: /schemas/types.yaml#/definitions/uint8-array
> +        description: |
> +          Program data for green channel.  See LP5562 datasheet for program format specification.
> +
> +      pat-b:
> +        $ref: /schemas/types.yaml#/definitions/uint8-array
> +        description: |
> +          Program data for blue channel.  See LP5562 datasheet for program format specification.
> +
>  required:
>    - compatible
>    - reg
> @@ -223,6 +248,82 @@ examples:
>                 };
>              };
>          };
> -    };
>  
> +        led-controller@30 {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            compatible = "ti,lp5562";
> +            reg = <0x30>;
> +
> +            led@0 {
> +                reg = <0>;
> +                chan-name = "red";
> +                color = <LED_COLOR_ID_RED>;
> +            };
> +
> +            led@1 {
> +                reg = <1>;
> +                chan-name = "green";
> +                color = <LED_COLOR_ID_GREEN>;
> +            };
> +
> +            led@2 {
> +                reg = <2>;
> +                chan-name = "blue";
> +                color = <LED_COLOR_ID_BLUE>;
> +            };
> +
> +            pattern@1 {
> +                /* Pulsing blue pattern
> +                 *   Blue:
> +                 *     027F: Ramp up 50%
> +                 *     027F: Ramp up 50%
> +                 *     4600: Wait 100ms
> +                 *     02FF: Ramp down 50%
> +                 *     02FF: Ramp down 50%
> +                 *     4600: Wait 100ms
> +                 *     0000: Goto start
> +                 */
> +                pat-name = "Pulsing Blue";
> +                pat-b = [02 7f 02 7f 46 00 02 ff 02 ff 46 00 00 00];
> +            };
> +
> +            pattern@2 {
> +                /*
> +                 * HSV rainbow
> +                 *   Red:
> +                 *     40FF: Set PWM 255
> +                 *     41FF: Ramp down 50%
> +                 *     41FF: Ramp down 50%
> +                 *     41FF: Wait 1/2 ramp time
> +                 *     41FF: Wait 1/2 ramp time
> +                 *     417F: Ramp up 50%
> +                 *     417F: Ramp up 50%
> +                 *     0000: Goto start
> +                 *   Green:
> +                 *     4000: Set PWM 0
> +                 *     417F: Ramp up 50%
> +                 *     417F: Ramp up 50%
> +                 *     41FF: Ramp down 50%
> +                 *     41FF: Ramp down 50%
> +                 *     41FF: Wait 1/2 ramp time
> +                 *     41FF: Wait 1/2 ramp time
> +                 *     0000: Goto start
> +                 *   Blue:
> +                 *     4000: Set PWM 0
> +                 *     41FF: Wait 1/2 ramp time
> +                 *     41FF: Wait 1/2 ramp time
> +                 *     417F: Ramp up 50%
> +                 *     417F: Ramp up 50%
> +                 *     41FF: Ramp down 50%
> +                 *     41FF: Ramp down 50%
> +                 *     0000: Goto start
> +                 */
> +                pat-name = "HSV Rainbow";
> +                pat-r = [40 ff 41 FF 41 FF 41 FF 41 FF 41 7F 41 7F 00 00];
> +                pat-g = [40 00 41 7F 41 7F 41 FF 41 FF 41 FF 41 FF 00 00];
> +                pat-b = [40 00 41 FF 41 FF 41 7F 41 7F 41 FF 41 FF 00 00];
> +            };
> +        };
> +    };
>  ...
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern
  2021-05-13  2:20   ` Rob Herring
@ 2021-05-13 20:47     ` Doug Zobel
  2021-05-16 18:31       ` Pavel Machek
  0 siblings, 1 reply; 9+ messages in thread
From: Doug Zobel @ 2021-05-13 20:47 UTC (permalink / raw)
  To: Rob Herring
  Cc: Pavel Machek, Jacek Anaszewski, linux-leds, devicetree, linux-kernel

Sorry for the re-send.  I didn't realize gmail was sending everything
HTML encoded.

On Wed, May 12, 2021 at 9:20 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, May 11, 2021 at 03:48:34PM -0500, Doug Zobel wrote:
> > Add a new device tree object for LP5562 predfined led patterns.
>
> If you are going to define something generic looking, put it in a
> generic binding.

I'm not clear on what this would be.  After re-running the
dt_binding_check with DT_CHECKER_FLAGS=-m, I see that the pattern
object definitions I have won't work since the numbering conflicts
with the existing led objects.  I was going to move these patterns
under a new 'predefine-patterns' object.  Is this what you mean by a
'generic binding'?

> I don't know that this belongs in DT though. Won't a user want to create
> their own patterns? That means there should be a sysfs interface (which
> we either already have or has been attempted IIRC).

Yes, there is a sysfs interface for running patterns via the firmware
loading interface.  The firmware loading interface doesn't seem well
suited for constantly changing the pattern that the LED driver runs.
I found it to be slow and unreliable when quickly changing the LED
pattern.  The existing predef pattern functionality works much better.
Unfortunately the only way to define the patterns for it is via the
platform data structure.  Adding the predef patterns to the device
tree seemed like a good way to make use of the existing functionality
in the driver.

> >
> > Signed-off-by: Doug Zobel <dougdev334@gmail.com>
> > ---
> >  .../devicetree/bindings/leds/leds-lp55xx.yaml | 103 +++++++++++++++++-
> >  1 file changed, 102 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
> > index f552cd143d5b..2524a84fe688 100644
> > --- a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
> > +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
> > @@ -100,6 +100,31 @@ patternProperties:
> >          $ref: /schemas/types.yaml#/definitions/string
> >          description: name of channel
> >
> > +  "(^pattern@[0-9a-f]$|pattern)":
> > +    type: object
> > +    $ref: common.yaml#
> > +    description: |
> > +      LP5562 sepcific object.  LED pattern program saved to and run on LP5562.
> > +    properties:
> > +      pat-name:
> > +        $ref: /schemas/types.yaml#/definitions/string
> > +        description: Name of pattern program
> > +
> > +      pat-r:
> > +        $ref: /schemas/types.yaml#/definitions/uint8-array
> > +        description: |
> > +          Program data for red channel.  See LP5562 datasheet for program format specification.
> > +
> > +      pat-g:
> > +        $ref: /schemas/types.yaml#/definitions/uint8-array
> > +        description: |
> > +          Program data for green channel.  See LP5562 datasheet for program format specification.
> > +
> > +      pat-b:
> > +        $ref: /schemas/types.yaml#/definitions/uint8-array
> > +        description: |
> > +          Program data for blue channel.  See LP5562 datasheet for program format specification.
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -223,6 +248,82 @@ examples:
> >                 };
> >              };
> >          };
> > -    };
> >
> > +        led-controller@30 {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +            compatible = "ti,lp5562";
> > +            reg = <0x30>;
> > +
> > +            led@0 {
> > +                reg = <0>;
> > +                chan-name = "red";
> > +                color = <LED_COLOR_ID_RED>;
> > +            };
> > +
> > +            led@1 {
> > +                reg = <1>;
> > +                chan-name = "green";
> > +                color = <LED_COLOR_ID_GREEN>;
> > +            };
> > +
> > +            led@2 {
> > +                reg = <2>;
> > +                chan-name = "blue";
> > +                color = <LED_COLOR_ID_BLUE>;
> > +            };
> > +
> > +            pattern@1 {
> > +                /* Pulsing blue pattern
> > +                 *   Blue:
> > +                 *     027F: Ramp up 50%
> > +                 *     027F: Ramp up 50%
> > +                 *     4600: Wait 100ms
> > +                 *     02FF: Ramp down 50%
> > +                 *     02FF: Ramp down 50%
> > +                 *     4600: Wait 100ms
> > +                 *     0000: Goto start
> > +                 */
> > +                pat-name = "Pulsing Blue";
> > +                pat-b = [02 7f 02 7f 46 00 02 ff 02 ff 46 00 00 00];
> > +            };
> > +
> > +            pattern@2 {
> > +                /*
> > +                 * HSV rainbow
> > +                 *   Red:
> > +                 *     40FF: Set PWM 255
> > +                 *     41FF: Ramp down 50%
> > +                 *     41FF: Ramp down 50%
> > +                 *     41FF: Wait 1/2 ramp time
> > +                 *     41FF: Wait 1/2 ramp time
> > +                 *     417F: Ramp up 50%
> > +                 *     417F: Ramp up 50%
> > +                 *     0000: Goto start
> > +                 *   Green:
> > +                 *     4000: Set PWM 0
> > +                 *     417F: Ramp up 50%
> > +                 *     417F: Ramp up 50%
> > +                 *     41FF: Ramp down 50%
> > +                 *     41FF: Ramp down 50%
> > +                 *     41FF: Wait 1/2 ramp time
> > +                 *     41FF: Wait 1/2 ramp time
> > +                 *     0000: Goto start
> > +                 *   Blue:
> > +                 *     4000: Set PWM 0
> > +                 *     41FF: Wait 1/2 ramp time
> > +                 *     41FF: Wait 1/2 ramp time
> > +                 *     417F: Ramp up 50%
> > +                 *     417F: Ramp up 50%
> > +                 *     41FF: Ramp down 50%
> > +                 *     41FF: Ramp down 50%
> > +                 *     0000: Goto start
> > +                 */
> > +                pat-name = "HSV Rainbow";
> > +                pat-r = [40 ff 41 FF 41 FF 41 FF 41 FF 41 7F 41 7F 00 00];
> > +                pat-g = [40 00 41 7F 41 7F 41 FF 41 FF 41 FF 41 FF 00 00];
> > +                pat-b = [40 00 41 FF 41 FF 41 7F 41 7F 41 FF 41 FF 00 00];
> > +            };
> > +        };
> > +    };
> >  ...
> > --
> > 2.20.1
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern
  2021-05-11 20:48 ` [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern Doug Zobel
  2021-05-12 18:35   ` Rob Herring
  2021-05-13  2:20   ` Rob Herring
@ 2021-05-16 18:22   ` Pavel Machek
  2 siblings, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2021-05-16 18:22 UTC (permalink / raw)
  To: Doug Zobel
  Cc: Rob Herring, Jacek Anaszewski, linux-leds, devicetree, linux-kernel

Hi!

> Add a new device tree object for LP5562 predfined led patterns.

typo.

> +  "(^pattern@[0-9a-f]$|pattern)":
> +    type: object
> +    $ref: common.yaml#
> +    description: |
> +      LP5562 sepcific object.  LED pattern program saved to and run
> on LP5562.

typo.

But.. no, not like this.

We have generic pattern trigger. Take a look at its interface. Extend
it
to get parameters from device tree.

Then use existing 'default-trigger' functionality.

Best regards,
								Pavel

-- 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern
  2021-05-13 20:47     ` Doug Zobel
@ 2021-05-16 18:31       ` Pavel Machek
  2021-05-17 20:07         ` Doug Zobel
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Machek @ 2021-05-16 18:31 UTC (permalink / raw)
  To: Doug Zobel
  Cc: Rob Herring, Jacek Anaszewski, linux-leds, devicetree, linux-kernel

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

Hi!

> > I don't know that this belongs in DT though. Won't a user want to create
> > their own patterns? That means there should be a sysfs interface (which
> > we either already have or has been attempted IIRC).
> 
> Yes, there is a sysfs interface for running patterns via the firmware
> loading interface.  The firmware loading interface doesn't seem well
> suited for constantly changing the pattern that the LED driver runs.
> I found it to be slow and unreliable when quickly changing the LED
> pattern.  The existing predef pattern functionality works much better.
> Unfortunately the only way to define the patterns for it is via the
> platform data structure.  Adding the predef patterns to the device
> tree seemed like a good way to make use of the existing functionality
> in the driver.

Take a look at the pattern trigger. That's the way to change patterns
at runtime, no need for firmware loading.

I may even have compiler from that interface to the bytecode lp55xx
uses. Some assembly will be required. Doing so with the RGB LED will
be even more fun. 

We'll want to deprecate the firmware loading interface at some point.

Forget the device tree, that will not help you.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern
  2021-05-16 18:31       ` Pavel Machek
@ 2021-05-17 20:07         ` Doug Zobel
  0 siblings, 0 replies; 9+ messages in thread
From: Doug Zobel @ 2021-05-17 20:07 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Rob Herring, Jacek Anaszewski, linux-leds, devicetree, linux-kernel

> Take a look at the pattern trigger. That's the way to change patterns
> at runtime, no need for firmware loading.

Thanks for the pointer.  That looks like it could work.  I would need
to add support for the lp5562 to run the patterns in hardware.  The
only problem I see is in synchronizing the 3 color channels.  Since
the pattern triggers are associated with each individual channel, I
don't see a clean way to run a multi-chanel (RGB) pattern and keep it
in sync.  I was thinking I could restart all 3 channels' programs
anytime a channel program is changed.  This would assure they start in
sync.  However that would cause glitches in the patterns if they are
being used as 3 independent (non-RGB) patterns.

> I may even have compiler from that interface to the bytecode lp55xx
> uses. Some assembly will be required. Doing so with the RGB LED will
> be even more fun.

If you have some previous work on this, I could use it.  Otherwise
I'll just write my own bytecode generator.  As far as I know the
lp5562 is the only lp55xx controller which supports on chip
programming.  So I would add the support in the lp5562 driver.

> We'll want to deprecate the firmware loading interface at some point.

If I add support for running the pattern triggers in lp5562 hardware,
then this will now be the 3rd method (firmware &
lp5562_run_predef_led_pattern() are the other two) for loading custom
patterns onto the chip.  Will this be a problem?

-Doug

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-05-17 20:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-11 20:48 [PATCH 0/2] Device Tree support for LP5562 predefined patterns Doug Zobel
2021-05-11 20:48 ` [PATCH 1/2] leds: lp55xx: support predefined pattern in Device Tree Doug Zobel
2021-05-11 20:48 ` [PATCH 2/2] dt: bindings: lp55xx: Add predefined LED pattern Doug Zobel
2021-05-12 18:35   ` Rob Herring
2021-05-13  2:20   ` Rob Herring
2021-05-13 20:47     ` Doug Zobel
2021-05-16 18:31       ` Pavel Machek
2021-05-17 20:07         ` Doug Zobel
2021-05-16 18:22   ` Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).