| Message ID | 20210511204834.2675271-3-dougdev334@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Device Tree support for LP5562 predefined patterns | expand |
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.
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 >
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 > >
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 --
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
> 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
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]; + }; + }; + }; ...
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(-)