* [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-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
* 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
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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.