* turris omnia leds again: question @ 2020-03-10 17:38 Marek Behun 2020-03-10 21:48 ` Jacek Anaszewski 0 siblings, 1 reply; 6+ messages in thread From: Marek Behun @ 2020-03-10 17:38 UTC (permalink / raw) To: linux-leds; +Cc: Jacek Anaszewski, Pavel Machek Hi, I am going to try to send driver for Omnia LEDs again. The last time there was a problem: on 05/01/2019 Jacek wrote: > I wonder if we're doing right merging this driver in this form. > We break the rule one-led-class-device-per-one-channel. We don't > have LED multi color support yet, so this should support RGB LEDs > in the old manner. Or switch to using LED multi color class. > Once we will have LED multi color class, we will be able to add the > support for it to the driver and make the driver configurable to be > able to expose old interface or the LED multi color one. > Moreover, the bindings should use led-sources property for grouping > three channels under single LED class device. This is certainly to be > fixed. So I am going to try to modify the driver so that each channel creates one LED class device. Do I understand this correctly then, that this way when there are three channels (RGB) on one LED, all the 3 device tree nodes for should have the same reg property, but different led-sources property? Eg: led@0,0 { reg = <0>; led-sources = <0>; label = "omnia::heartbeat::red"; }; led@0,1 { reg = <0>; led-sources = <1>; label = "omnia::heartbeat::green"; }; led@0,2 { reg = <0>; led-sources = <2>; label = "omnia::heartbeat::blue"; }; Or did I misinterpret the led-sources property? Thanks, Marek. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question 2020-03-10 17:38 turris omnia leds again: question Marek Behun @ 2020-03-10 21:48 ` Jacek Anaszewski 2020-03-10 22:23 ` Marek Behun 0 siblings, 1 reply; 6+ messages in thread From: Jacek Anaszewski @ 2020-03-10 21:48 UTC (permalink / raw) To: Marek Behun, linux-leds; +Cc: Pavel Machek Hi Marek, On 3/10/20 6:38 PM, Marek Behun wrote: > Hi, > > I am going to try to send driver for Omnia LEDs again. The last time > there was a problem: on 05/01/2019 Jacek wrote: > >> I wonder if we're doing right merging this driver in this form. >> We break the rule one-led-class-device-per-one-channel. We don't >> have LED multi color support yet, so this should support RGB LEDs >> in the old manner. Or switch to using LED multi color class. > >> Once we will have LED multi color class, we will be able to add the >> support for it to the driver and make the driver configurable to be >> able to expose old interface or the LED multi color one. > >> Moreover, the bindings should use led-sources property for grouping >> three channels under single LED class device. This is certainly to be >> fixed. > > So I am going to try to modify the driver so that each channel creates > one LED class device. Do I understand this correctly then, that this > way when there are three channels (RGB) on one LED, all the 3 device > tree nodes for should have the same reg property, but different > led-sources property? Eg: > > led@0,0 { > reg = <0>; > led-sources = <0>; > label = "omnia::heartbeat::red"; > }; > > led@0,1 { > reg = <0>; > led-sources = <1>; > label = "omnia::heartbeat::green"; > }; > > led@0,2 { > reg = <0>; > led-sources = <2>; > label = "omnia::heartbeat::blue"; > }; > > Or did I misinterpret the led-sources property? This is what I proposed back then, strangely that message wasn't archived by bots, or maybe it resides only in my outbox... -------------- LED sub-node properties: - reg : Must be from 0x0 to 0xb, since there are 12 RGB LEDs on this controller. - label : (optional) see Documentation/devicetree/bindings/leds/common.txt - linux,default-trigger : (optional) see Documentation/devicetree/bindings/leds/common.txt - led-sources : Each child node should describe RGB LED it controls, by listing corresponding iout identifiers: 0 - RGB LED 0: red 1 - RGB LED 0: green 2 - RGB LED 0: blue 3 - RGB LED 1: red 4 - RGB LED 1: green 5 - RGB LED 1: blue 6 - RGB LED 2: red 7 - RGB LED 2: green 8 - RGB LED 2: blue 9 - RGB LED 3: red 10 - RGB LED 3: green 11 - RGB LED 3: blue ... and list all the iouts, maybe other names will be more appropriate for this device, feel free to propose something Example: led-controller@2b { compatible = "cznic,turris-omnia-leds"; reg = <0x2b>; #address-cells = <1>; #size-cells = <0>; led@0 { reg = <0x0>; label = "userB"; linux,default-trigger = "heartbeat"; led-sources = <0 1 2>; }; led@1 { reg = <0x1>; label = "userA"; led-sources = <3 4 5>; }; led@2 { reg = <0x2>; label = "pci3"; led-sources = <6 7 8>; }; led@3 { reg = <0x3>; label = "pci2"; led-sources = <9 10 11>; }; ... -------------- Of course now label should be replaced with color and function properties. I've just reviewed that patch set and realized that we agreed upon setting max_brightness to 1 for all LEDs, right? -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question 2020-03-10 21:48 ` Jacek Anaszewski @ 2020-03-10 22:23 ` Marek Behun 2020-03-11 10:59 ` Jacek Anaszewski 0 siblings, 1 reply; 6+ messages in thread From: Marek Behun @ 2020-03-10 22:23 UTC (permalink / raw) To: Jacek Anaszewski; +Cc: linux-leds, Pavel Machek On Tue, 10 Mar 2020 22:48:09 +0100 Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > Hi Marek, > > On 3/10/20 6:38 PM, Marek Behun wrote: > > Hi, > > > > I am going to try to send driver for Omnia LEDs again. The last time > > there was a problem: on 05/01/2019 Jacek wrote: > > > >> I wonder if we're doing right merging this driver in this form. > >> We break the rule one-led-class-device-per-one-channel. We don't > >> have LED multi color support yet, so this should support RGB LEDs > >> in the old manner. Or switch to using LED multi color class. > > > >> Once we will have LED multi color class, we will be able to add the > >> support for it to the driver and make the driver configurable to be > >> able to expose old interface or the LED multi color one. > > > >> Moreover, the bindings should use led-sources property for grouping > >> three channels under single LED class device. This is certainly to be > >> fixed. > > > > So I am going to try to modify the driver so that each channel creates > > one LED class device. Do I understand this correctly then, that this > > way when there are three channels (RGB) on one LED, all the 3 device > > tree nodes for should have the same reg property, but different > > led-sources property? Eg: > > > > led@0,0 { > > reg = <0>; > > led-sources = <0>; > > label = "omnia::heartbeat::red"; > > }; > > > > led@0,1 { > > reg = <0>; > > led-sources = <1>; > > label = "omnia::heartbeat::green"; > > }; > > > > led@0,2 { > > reg = <0>; > > led-sources = <2>; > > label = "omnia::heartbeat::blue"; > > }; > > > > Or did I misinterpret the led-sources property? > > This is what I proposed back then, strangely that message wasn't > archived by bots, or maybe it resides only in my outbox... > > -------------- > > LED sub-node properties: > - reg : Must be from 0x0 to 0xb, since there are 12 RGB > LEDs on this > controller. > - label : (optional) > see Documentation/devicetree/bindings/leds/common.txt > - linux,default-trigger : (optional) > see Documentation/devicetree/bindings/leds/common.txt > - led-sources : Each child node should describe RGB LED it controls, > by listing corresponding iout identifiers: > 0 - RGB LED 0: red > 1 - RGB LED 0: green > 2 - RGB LED 0: blue > 3 - RGB LED 1: red > 4 - RGB LED 1: green > 5 - RGB LED 1: blue > 6 - RGB LED 2: red > 7 - RGB LED 2: green > 8 - RGB LED 2: blue > 9 - RGB LED 3: red > 10 - RGB LED 3: green > 11 - RGB LED 3: blue > ... and list all the iouts, maybe other names will be more > appropriate for this device, feel free to propose something > > > > Example: > > led-controller@2b { > compatible = "cznic,turris-omnia-leds"; > reg = <0x2b>; > #address-cells = <1>; > #size-cells = <0>; > > led@0 { > reg = <0x0>; > label = "userB"; > linux,default-trigger = "heartbeat"; > led-sources = <0 1 2>; > }; > > led@1 { > reg = <0x1>; > label = "userA"; > led-sources = <3 4 5>; > }; > > led@2 { > reg = <0x2>; > label = "pci3"; > led-sources = <6 7 8>; > }; > > led@3 { > reg = <0x3>; > label = "pci2"; > led-sources = <9 10 11>; > }; > ... > -------------- > > > Of course now label should be replaced with color and function > properties. I've just reviewed that patch set and realized that > we agreed upon setting max_brightness to 1 for all LEDs, right? > No, there were subsequent patches which added support for max_brightness = 255. So the driver will register one LED class device per node, with color ID = WHITE. Once RGB LED class is merged, the driver can be remade, but the device tree won't need to be changed. Marek ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question 2020-03-10 22:23 ` Marek Behun @ 2020-03-11 10:59 ` Jacek Anaszewski 2020-03-19 4:34 ` Marek Behun 2020-03-19 15:27 ` Marek Behun 0 siblings, 2 replies; 6+ messages in thread From: Jacek Anaszewski @ 2020-03-11 10:59 UTC (permalink / raw) To: Marek Behun; +Cc: linux-leds, Pavel Machek On 3/10/20 11:23 PM, Marek Behun wrote: > On Tue, 10 Mar 2020 22:48:09 +0100 > Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > >> Hi Marek, >> >> On 3/10/20 6:38 PM, Marek Behun wrote: >>> Hi, >>> >>> I am going to try to send driver for Omnia LEDs again. The last time >>> there was a problem: on 05/01/2019 Jacek wrote: >>> >>>> I wonder if we're doing right merging this driver in this form. >>>> We break the rule one-led-class-device-per-one-channel. We don't >>>> have LED multi color support yet, so this should support RGB LEDs >>>> in the old manner. Or switch to using LED multi color class. >>> >>>> Once we will have LED multi color class, we will be able to add the >>>> support for it to the driver and make the driver configurable to be >>>> able to expose old interface or the LED multi color one. >>> >>>> Moreover, the bindings should use led-sources property for grouping >>>> three channels under single LED class device. This is certainly to be >>>> fixed. >>> >>> So I am going to try to modify the driver so that each channel creates >>> one LED class device. Do I understand this correctly then, that this >>> way when there are three channels (RGB) on one LED, all the 3 device >>> tree nodes for should have the same reg property, but different >>> led-sources property? Eg: >>> >>> led@0,0 { >>> reg = <0>; >>> led-sources = <0>; >>> label = "omnia::heartbeat::red"; >>> }; >>> >>> led@0,1 { >>> reg = <0>; >>> led-sources = <1>; >>> label = "omnia::heartbeat::green"; >>> }; >>> >>> led@0,2 { >>> reg = <0>; >>> led-sources = <2>; >>> label = "omnia::heartbeat::blue"; >>> }; >>> >>> Or did I misinterpret the led-sources property? >> >> This is what I proposed back then, strangely that message wasn't >> archived by bots, or maybe it resides only in my outbox... >> >> -------------- >> >> LED sub-node properties: >> - reg : Must be from 0x0 to 0xb, since there are 12 RGB >> LEDs on this >> controller. >> - label : (optional) >> see Documentation/devicetree/bindings/leds/common.txt >> - linux,default-trigger : (optional) >> see Documentation/devicetree/bindings/leds/common.txt >> - led-sources : Each child node should describe RGB LED it controls, >> by listing corresponding iout identifiers: >> 0 - RGB LED 0: red >> 1 - RGB LED 0: green >> 2 - RGB LED 0: blue >> 3 - RGB LED 1: red >> 4 - RGB LED 1: green >> 5 - RGB LED 1: blue >> 6 - RGB LED 2: red >> 7 - RGB LED 2: green >> 8 - RGB LED 2: blue >> 9 - RGB LED 3: red >> 10 - RGB LED 3: green >> 11 - RGB LED 3: blue >> ... and list all the iouts, maybe other names will be more >> appropriate for this device, feel free to propose something >> >> >> >> Example: >> >> led-controller@2b { >> compatible = "cznic,turris-omnia-leds"; >> reg = <0x2b>; >> #address-cells = <1>; >> #size-cells = <0>; >> >> led@0 { >> reg = <0x0>; >> label = "userB"; >> linux,default-trigger = "heartbeat"; >> led-sources = <0 1 2>; >> }; >> >> led@1 { >> reg = <0x1>; >> label = "userA"; >> led-sources = <3 4 5>; >> }; >> >> led@2 { >> reg = <0x2>; >> label = "pci3"; >> led-sources = <6 7 8>; >> }; >> >> led@3 { >> reg = <0x3>; >> label = "pci2"; >> led-sources = <9 10 11>; >> }; >> ... >> -------------- >> >> >> Of course now label should be replaced with color and function >> properties. I've just reviewed that patch set and realized that >> we agreed upon setting max_brightness to 1 for all LEDs, right? >> > > No, there were subsequent patches which added support for > max_brightness = 255. Right. > So the driver will register one LED class device per node, with color > ID = WHITE. Once RGB LED class is merged, the driver can be remade, but > the device tree won't need to be changed. Device Tree will need to be changed to LED mc specific bindings, which at current state introduces one more level or nesting and LED_COLOR_ID_MULTI for the top level DT node. And the driver will need to still support this approach as well as the new LED mc class. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question 2020-03-11 10:59 ` Jacek Anaszewski @ 2020-03-19 4:34 ` Marek Behun 2020-03-19 15:27 ` Marek Behun 1 sibling, 0 replies; 6+ messages in thread From: Marek Behun @ 2020-03-19 4:34 UTC (permalink / raw) To: Jacek Anaszewski; +Cc: linux-leds, Pavel Machek On Wed, 11 Mar 2020 11:59:07 +0100 Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > On 3/10/20 11:23 PM, Marek Behun wrote: > > On Tue, 10 Mar 2020 22:48:09 +0100 > > Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > > > >> Hi Marek, > >> > >> On 3/10/20 6:38 PM, Marek Behun wrote: > >>> Hi, > >>> > >>> I am going to try to send driver for Omnia LEDs again. The last time > >>> there was a problem: on 05/01/2019 Jacek wrote: > >>> > >>>> I wonder if we're doing right merging this driver in this form. > >>>> We break the rule one-led-class-device-per-one-channel. We don't > >>>> have LED multi color support yet, so this should support RGB LEDs > >>>> in the old manner. Or switch to using LED multi color class. > >>> > >>>> Once we will have LED multi color class, we will be able to add the > >>>> support for it to the driver and make the driver configurable to be > >>>> able to expose old interface or the LED multi color one. > >>> > >>>> Moreover, the bindings should use led-sources property for grouping > >>>> three channels under single LED class device. This is certainly to be > >>>> fixed. > >>> > >>> So I am going to try to modify the driver so that each channel creates > >>> one LED class device. Do I understand this correctly then, that this > >>> way when there are three channels (RGB) on one LED, all the 3 device > >>> tree nodes for should have the same reg property, but different > >>> led-sources property? Eg: > >>> > >>> led@0,0 { > >>> reg = <0>; > >>> led-sources = <0>; > >>> label = "omnia::heartbeat::red"; > >>> }; > >>> > >>> led@0,1 { > >>> reg = <0>; > >>> led-sources = <1>; > >>> label = "omnia::heartbeat::green"; > >>> }; > >>> > >>> led@0,2 { > >>> reg = <0>; > >>> led-sources = <2>; > >>> label = "omnia::heartbeat::blue"; > >>> }; > >>> > >>> Or did I misinterpret the led-sources property? > >> > >> This is what I proposed back then, strangely that message wasn't > >> archived by bots, or maybe it resides only in my outbox... > >> > >> -------------- > >> > >> LED sub-node properties: > >> - reg : Must be from 0x0 to 0xb, since there are 12 RGB > >> LEDs on this > >> controller. > >> - label : (optional) > >> see Documentation/devicetree/bindings/leds/common.txt > >> - linux,default-trigger : (optional) > >> see Documentation/devicetree/bindings/leds/common.txt > >> - led-sources : Each child node should describe RGB LED it controls, > >> by listing corresponding iout identifiers: > >> 0 - RGB LED 0: red > >> 1 - RGB LED 0: green > >> 2 - RGB LED 0: blue > >> 3 - RGB LED 1: red > >> 4 - RGB LED 1: green > >> 5 - RGB LED 1: blue > >> 6 - RGB LED 2: red > >> 7 - RGB LED 2: green > >> 8 - RGB LED 2: blue > >> 9 - RGB LED 3: red > >> 10 - RGB LED 3: green > >> 11 - RGB LED 3: blue > >> ... and list all the iouts, maybe other names will be more > >> appropriate for this device, feel free to propose something > >> > >> > >> > >> Example: > >> > >> led-controller@2b { > >> compatible = "cznic,turris-omnia-leds"; > >> reg = <0x2b>; > >> #address-cells = <1>; > >> #size-cells = <0>; > >> > >> led@0 { > >> reg = <0x0>; > >> label = "userB"; > >> linux,default-trigger = "heartbeat"; > >> led-sources = <0 1 2>; > >> }; > >> > >> led@1 { > >> reg = <0x1>; > >> label = "userA"; > >> led-sources = <3 4 5>; > >> }; > >> > >> led@2 { > >> reg = <0x2>; > >> label = "pci3"; > >> led-sources = <6 7 8>; > >> }; > >> > >> led@3 { > >> reg = <0x3>; > >> label = "pci2"; > >> led-sources = <9 10 11>; > >> }; > >> ... > >> -------------- > >> > >> > >> Of course now label should be replaced with color and function > >> properties. I've just reviewed that patch set and realized that > >> we agreed upon setting max_brightness to 1 for all LEDs, right? > >> > > > > No, there were subsequent patches which added support for > > max_brightness = 255. > > Right. > > > So the driver will register one LED class device per node, with color > > ID = WHITE. Once RGB LED class is merged, the driver can be remade, but > > the device tree won't need to be changed. > > Device Tree will need to be changed to LED mc specific bindings, > which at current state introduces one more level or nesting > and LED_COLOR_ID_MULTI for the top level DT node. > > And the driver will need to still support this approach as well > as the new LED mc class. > Hi Jacek, I have used the led-sources in such a way that the user can either set led-sources = <0 1 2>; color = <LED_COLOR_ID_WHITE>; in which case all three channels will be grouped into one led cdev, or the user can use just one led-source, for example led-sources = <0>; color = <LED_COLOR_ID_RED>; and in this case they can have one led cdev per channel. Is this acceptable? Or should I just go with the WHITE approach? In case that this is acceptable I wonder what should be the suggested device-tree node naming and reg property, when using one led cdev per channel, for example: led@1,0 { reg = <1>; led-sources = <3>; color = <LED_COLOR_ID_RED>; }; led@1,1 { reg = <1>; led-sources = <4>; color = <LED_COLOR_ID_GREEN>; }; led@1,2 { reg = <1>; led-sources = <5>; color = <LED_COLOR_ID_BLUE>; }; I don't think different nodes should have the same reg property. Should in this case the reg property have two values? Marek ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: turris omnia leds again: question 2020-03-11 10:59 ` Jacek Anaszewski 2020-03-19 4:34 ` Marek Behun @ 2020-03-19 15:27 ` Marek Behun 1 sibling, 0 replies; 6+ messages in thread From: Marek Behun @ 2020-03-19 15:27 UTC (permalink / raw) To: Jacek Anaszewski; +Cc: linux-leds, Pavel Machek On Wed, 11 Mar 2020 11:59:07 +0100 Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > Device Tree will need to be changed to LED mc specific bindings, > which at current state introduces one more level or nesting > and LED_COLOR_ID_MULTI for the top level DT node. > > And the driver will need to still support this approach as well > as the new LED mc class. > Hi Jacek, I have used the led-sources in such a way that the user can either set led-sources = <0 1 2>; color = <LED_COLOR_ID_WHITE>; in which case all three channels will be grouped into one led cdev, or the user can use just one led-source, for example led-sources = <0>; color = <LED_COLOR_ID_RED>; and in this case they can have one led cdev per channel. Is this acceptable? Or should I just go with the WHITE approach? In case that this is acceptable I wonder what should be the suggested device-tree node naming and reg property, when using one led cdev per channel, for example: led@1,0 { reg = <1>; led-sources = <3>; color = <LED_COLOR_ID_RED>; }; led@1,1 { reg = <1>; led-sources = <4>; color = <LED_COLOR_ID_GREEN>; }; led@1,2 { reg = <1>; led-sources = <5>; color = <LED_COLOR_ID_BLUE>; }; I don't think different nodes should have the same reg property. Should in this case the reg property have two values? Marek ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-19 15:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-03-10 17:38 turris omnia leds again: question Marek Behun 2020-03-10 21:48 ` Jacek Anaszewski 2020-03-10 22:23 ` Marek Behun 2020-03-11 10:59 ` Jacek Anaszewski 2020-03-19 4:34 ` Marek Behun 2020-03-19 15:27 ` Marek Behun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox