| Message ID | 20240109082312.9989-1-zajec5@gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Rafał Miłecki |
| Headers | show |
| Series | [dt-schema] schemas: chosen: Add OpenWrt LEDs properties for system states | expand |
On 09/01/2024 09:23, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > OpenWrt project provides downstream support for thousands of embedded > home network devices. Its custom requirement for DT is to provide info > about LEDs roles. Currently it does it by using custom non-documented > aliases. While formally valid (aliases.yaml doesn't limit names or > purposes of aliases) it's quite a loose solution. > > Document 4 precise "chosen" biding properties with clearly documented > OpenWrt usage. This will allow upstreaming tons of DTS files that noone typo: none > cared about so far as those would need to be patched downstream anyway. From all this description I understand why you want to add it, but I don't understand what are you exactly adding and how it is being used by the users/OS. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- > A few weeks ago I was seeking for a help regarding OpenWrt's need for > specifing LEDs roles in DT, see: > > Describing LEDs roles in device tree? > https://lore.kernel.org/linux-devicetree/ee912a89-4fd7-43c3-a79b-16659a035fe1@gmail.com/T/#u > > I DON'T think OpenWrt's current solution with aliases is good enough: > * It's not clearly documented > * It may vary from other projects usa case > * It may be refused by random maintainers I think > > I decided to suggest 4 OpenWrt-prefixed properties for "chosen". I'm > hoping this small custom binding is sth we could go with. I'm really > looking forward to upstreaming OpenWrt's downstream DTS files so other > projects (e.g. Buildroot) can use them. > > If you have any better fitting solution in mind please let me know. I > should be fine with anything that lets me solve this downstream mess > situation. > > dtschema/schemas/chosen.yaml | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/dtschema/schemas/chosen.yaml b/dtschema/schemas/chosen.yaml > index 6d5c3f1..96d0db7 100644 > --- a/dtschema/schemas/chosen.yaml > +++ b/dtschema/schemas/chosen.yaml > @@ -264,4 +264,13 @@ properties: > patternProperties: > "^framebuffer": true > > + "^openwrt,led-(boot|failsafe|running|upgrade)$": > + $ref: types.yaml#/definitions/string > + description: > + OpenWrt choice of LED for a given role. Neither this explains it. What is "OpenWrt choice"? Choice like what? What are the valid choices? > Value must be a full path (encoded > + as a string) to a relevant LED node. What do you mean by full path? DT node path? Then no, use phandles. Anyway, we have existing properties for this - LED functions. Just choose LED with given function (from sysfs) and you are done. If function is missing in the header: feel free to go, least for me. Also: please Cc LED maintainers on all future submissions of this. Best regards, Krzysztof
On 9.01.2024 10:02, Krzysztof Kozlowski wrote: > On 09/01/2024 09:23, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> OpenWrt project provides downstream support for thousands of embedded >> home network devices. Its custom requirement for DT is to provide info >> about LEDs roles. Currently it does it by using custom non-documented >> aliases. While formally valid (aliases.yaml doesn't limit names or >> purposes of aliases) it's quite a loose solution. >> >> Document 4 precise "chosen" biding properties with clearly documented >> OpenWrt usage. This will allow upstreaming tons of DTS files that noone > > typo: none typo: no one ;) >> cared about so far as those would need to be patched downstream anyway. > > From all this description I understand why you want to add it, but I > don't understand what are you exactly adding and how it is being used by > the users/OS. In OpenWrt we have user space script that reads LED full path: cat /proc/device-tree/aliases/led-<foo> And then sets LED to state matching current boot stage: echo 1 > /sys/class/leds/<bar>/brightness >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >> --- >> A few weeks ago I was seeking for a help regarding OpenWrt's need for >> specifing LEDs roles in DT, see: >> >> Describing LEDs roles in device tree? >> https://lore.kernel.org/linux-devicetree/ee912a89-4fd7-43c3-a79b-16659a035fe1@gmail.com/T/#u >> >> I DON'T think OpenWrt's current solution with aliases is good enough: >> * It's not clearly documented >> * It may vary from other projects usa case >> * It may be refused by random maintainers I think >> >> I decided to suggest 4 OpenWrt-prefixed properties for "chosen". I'm >> hoping this small custom binding is sth we could go with. I'm really >> looking forward to upstreaming OpenWrt's downstream DTS files so other >> projects (e.g. Buildroot) can use them. >> >> If you have any better fitting solution in mind please let me know. I >> should be fine with anything that lets me solve this downstream mess >> situation. >> >> dtschema/schemas/chosen.yaml | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/dtschema/schemas/chosen.yaml b/dtschema/schemas/chosen.yaml >> index 6d5c3f1..96d0db7 100644 >> --- a/dtschema/schemas/chosen.yaml >> +++ b/dtschema/schemas/chosen.yaml >> @@ -264,4 +264,13 @@ properties: >> patternProperties: >> "^framebuffer": true >> >> + "^openwrt,led-(boot|failsafe|running|upgrade)$": >> + $ref: types.yaml#/definitions/string >> + description: >> + OpenWrt choice of LED for a given role. > > Neither this explains it. What is "OpenWrt choice"? Choice like what? > What are the valid choices? > >> Value must be a full path (encoded >> + as a string) to a relevant LED node. > > What do you mean by full path? DT node path? Then no, use phandles. > > Anyway, we have existing properties for this - LED functions. Just > choose LED with given function (from sysfs) and you are done. If > function is missing in the header: feel free to go, least for me. In "Describing LEDs roles in device tree?" e-mail I wrote: " Please note that "function" on its own is not sufficient as multiple LEDs my share the same function name but its meaning may vary depending on color. " Let me elaborate here. Values of "function" property match LEDs descriptions (usually it's a physical label). That is OK and makes sense but doesn't allow OpenWrt to automatically pick proper LED to use during given boot stage. Some devices may have multiple color of a the same LED function. OpenWrt developer needs to choose which color to use for failsafe more and which boot fully booted state (and others too). Devices also differ in available LEDs by their functions. Some may have LED_FUNCTION_POWER that OpenWrt needs to use. Some may have LED_FUNCTION_STATUS. Or both. There are some more (less common) functions like LED_FUNCTION_ACTIVITY. We failed at OpenWrt to develop a single generic script to rule all devices and all their LEDs combinations. We ended up with developers *choosing* what LED to use for a given system state. > Also: please Cc LED maintainers on all future submissions of this. Included them (apart from linux-leds@ already present) now, thanks.
On 09/01/2024 17:38, Rafał Miłecki wrote: > On 9.01.2024 10:02, Krzysztof Kozlowski wrote: >> On 09/01/2024 09:23, Rafał Miłecki wrote: >>> From: Rafał Miłecki <rafal@milecki.pl> >>> >>> OpenWrt project provides downstream support for thousands of embedded >>> home network devices. Its custom requirement for DT is to provide info >>> about LEDs roles. Currently it does it by using custom non-documented >>> aliases. While formally valid (aliases.yaml doesn't limit names or >>> purposes of aliases) it's quite a loose solution. >>> >>> Document 4 precise "chosen" biding properties with clearly documented >>> OpenWrt usage. This will allow upstreaming tons of DTS files that noone >> >> typo: none > > typo: no one ;) > >>> cared about so far as those would need to be patched downstream anyway. >> >> From all this description I understand why you want to add it, but I >> don't understand what are you exactly adding and how it is being used by >> the users/OS. > > In OpenWrt we have user space script that reads LED full path: > cat /proc/device-tree/aliases/led-<foo> > > And then sets LED to state matching current boot stage: > echo 1 > /sys/class/leds/<bar>/brightness OK, it's nowhere close to a hardware property. You now instruct OS what to do. It's software or software policy. > > >>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >>> --- >>> A few weeks ago I was seeking for a help regarding OpenWrt's need for >>> specifing LEDs roles in DT, see: >>> >>> Describing LEDs roles in device tree? >>> https://lore.kernel.org/linux-devicetree/ee912a89-4fd7-43c3-a79b-16659a035fe1@gmail.com/T/#u >>> >>> I DON'T think OpenWrt's current solution with aliases is good enough: >>> * It's not clearly documented >>> * It may vary from other projects usa case >>> * It may be refused by random maintainers I think >>> >>> I decided to suggest 4 OpenWrt-prefixed properties for "chosen". I'm >>> hoping this small custom binding is sth we could go with. I'm really >>> looking forward to upstreaming OpenWrt's downstream DTS files so other >>> projects (e.g. Buildroot) can use them. >>> >>> If you have any better fitting solution in mind please let me know. I >>> should be fine with anything that lets me solve this downstream mess >>> situation. >>> >>> dtschema/schemas/chosen.yaml | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/dtschema/schemas/chosen.yaml b/dtschema/schemas/chosen.yaml >>> index 6d5c3f1..96d0db7 100644 >>> --- a/dtschema/schemas/chosen.yaml >>> +++ b/dtschema/schemas/chosen.yaml >>> @@ -264,4 +264,13 @@ properties: >>> patternProperties: >>> "^framebuffer": true >>> >>> + "^openwrt,led-(boot|failsafe|running|upgrade)$": >>> + $ref: types.yaml#/definitions/string >>> + description: >>> + OpenWrt choice of LED for a given role. >> >> Neither this explains it. What is "OpenWrt choice"? Choice like what? >> What are the valid choices? >> >>> Value must be a full path (encoded >>> + as a string) to a relevant LED node. >> >> What do you mean by full path? DT node path? Then no, use phandles. >> >> Anyway, we have existing properties for this - LED functions. Just >> choose LED with given function (from sysfs) and you are done. If >> function is missing in the header: feel free to go, least for me. > > In "Describing LEDs roles in device tree?" e-mail I wrote: > > " > Please note that "function" on its own is not sufficient as multiple > LEDs my share the same function name but its meaning may vary depending > on color. > " > > Let me elaborate here. > > Values of "function" property match LEDs descriptions (usually it's a > physical label). That is OK and makes sense but doesn't allow OpenWrt to > automatically pick proper LED to use during given boot stage. > > Some devices may have multiple color of a the same LED function. OpenWrt > developer needs to choose which color to use for failsafe more and which > boot fully booted state (and others too). > > Devices also differ in available LEDs by their functions. Some may have > LED_FUNCTION_POWER that OpenWrt needs to use. Some may have > LED_FUNCTION_STATUS. Or both. There are some more (less common) > functions like LED_FUNCTION_ACTIVITY. > > We failed at OpenWrt to develop a single generic script to rule all > devices and all their LEDs combinations. We ended up with developers > *choosing* what LED to use for a given system state. So this all is because you want to write easier OS. That's abuse of Devicetree. You can define which LEDs have different meaning, e.g. physical label or icon attached to them. That's a hardware property. You can also define how pieces of hardware are wired together and create entire system, e.g. connect one LED to disk activity. However what you are proposing here is to dynamically configure one, given OS. I don't think it is suitable. The problem of OS to nicely figure out which LED to blink when, is not a problem of Devicetree. It is a problem of OS and its configuration. Best regards, Krzysztof
Hi Krzysztof, On Tue, Jan 9, 2024 at 8:10 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 09/01/2024 17:38, Rafał Miłecki wrote: > > On 9.01.2024 10:02, Krzysztof Kozlowski wrote: > >> On 09/01/2024 09:23, Rafał Miłecki wrote: > >>> From: Rafał Miłecki <rafal@milecki.pl> > >>> > >>> OpenWrt project provides downstream support for thousands of embedded > >>> home network devices. Its custom requirement for DT is to provide info > >>> about LEDs roles. Currently it does it by using custom non-documented > >>> aliases. While formally valid (aliases.yaml doesn't limit names or > >>> purposes of aliases) it's quite a loose solution. > >>> > >>> Document 4 precise "chosen" biding properties with clearly documented > >>> OpenWrt usage. This will allow upstreaming tons of DTS files that noone > >> > >> typo: none > > > > typo: no one ;) > > > >>> cared about so far as those would need to be patched downstream anyway. > >> > >> From all this description I understand why you want to add it, but I > >> don't understand what are you exactly adding and how it is being used by > >> the users/OS. > > > > In OpenWrt we have user space script that reads LED full path: > > cat /proc/device-tree/aliases/led-<foo> > > > > And then sets LED to state matching current boot stage: > > echo 1 > /sys/class/leds/<bar>/brightness > > OK, it's nowhere close to a hardware property. You now instruct OS what > to do. It's software or software policy. > >> Anyway, we have existing properties for this - LED functions. Just > >> choose LED with given function (from sysfs) and you are done. If > >> function is missing in the header: feel free to go, least for me. > > > > In "Describing LEDs roles in device tree?" e-mail I wrote: > > > > " > > Please note that "function" on its own is not sufficient as multiple > > LEDs my share the same function name but its meaning may vary depending > > on color. > > " > > > > Let me elaborate here. > > > > Values of "function" property match LEDs descriptions (usually it's a > > physical label). That is OK and makes sense but doesn't allow OpenWrt to > > automatically pick proper LED to use during given boot stage. > > > > Some devices may have multiple color of a the same LED function. OpenWrt > > developer needs to choose which color to use for failsafe more and which > > boot fully booted state (and others too). > > > > Devices also differ in available LEDs by their functions. Some may have > > LED_FUNCTION_POWER that OpenWrt needs to use. Some may have > > LED_FUNCTION_STATUS. Or both. There are some more (less common) > > functions like LED_FUNCTION_ACTIVITY. > > > > We failed at OpenWrt to develop a single generic script to rule all > > devices and all their LEDs combinations. We ended up with developers > > *choosing* what LED to use for a given system state. > > So this all is because you want to write easier OS. That's abuse of > Devicetree. You can define which LEDs have different meaning, e.g. > physical label or icon attached to them. That's a hardware property. > > You can also define how pieces of hardware are wired together and create > entire system, e.g. connect one LED to disk activity. > > However what you are proposing here is to dynamically configure one, > given OS. I don't think it is suitable. > > The problem of OS to nicely figure out which LED to blink when, is not a > problem of Devicetree. It is a problem of OS and its configuration. I'd say it's a grey area... What if the colors are printed on the case, next to the LED? Like these multi-color LEDs indicating port speed on network switches? Then it suddenly becomes hardware description, just like "aliases/serial2 = &...;" referring to serial port 2. Next, what if the colors are not printed on the case, but documented in the product's manual? What if there is no paper product manual, but just the OpenWRT online documentation? Gr{oetje,eeting}s, Geert
On 9.01.2024 20:10, Krzysztof Kozlowski wrote: > On 09/01/2024 17:38, Rafał Miłecki wrote: >> On 9.01.2024 10:02, Krzysztof Kozlowski wrote: >>> On 09/01/2024 09:23, Rafał Miłecki wrote: >>>> From: Rafał Miłecki <rafal@milecki.pl> >>>> >>>> OpenWrt project provides downstream support for thousands of embedded >>>> home network devices. Its custom requirement for DT is to provide info >>>> about LEDs roles. Currently it does it by using custom non-documented >>>> aliases. While formally valid (aliases.yaml doesn't limit names or >>>> purposes of aliases) it's quite a loose solution. >>>> >>>> Document 4 precise "chosen" biding properties with clearly documented >>>> OpenWrt usage. This will allow upstreaming tons of DTS files that noone >>> >>> typo: none >> >> typo: no one ;) >> >>>> cared about so far as those would need to be patched downstream anyway. >>> >>> From all this description I understand why you want to add it, but I >>> don't understand what are you exactly adding and how it is being used by >>> the users/OS. >> >> In OpenWrt we have user space script that reads LED full path: >> cat /proc/device-tree/aliases/led-<foo> >> >> And then sets LED to state matching current boot stage: >> echo 1 > /sys/class/leds/<bar>/brightness > > OK, it's nowhere close to a hardware property. You now instruct OS what > to do. It's software or software policy. That's the reason I targeted "chosen" node which seemed the best option given it does not describe real hardware. >>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >>>> --- >>>> A few weeks ago I was seeking for a help regarding OpenWrt's need for >>>> specifing LEDs roles in DT, see: >>>> >>>> Describing LEDs roles in device tree? >>>> https://lore.kernel.org/linux-devicetree/ee912a89-4fd7-43c3-a79b-16659a035fe1@gmail.com/T/#u >>>> >>>> I DON'T think OpenWrt's current solution with aliases is good enough: >>>> * It's not clearly documented >>>> * It may vary from other projects usa case >>>> * It may be refused by random maintainers I think >>>> >>>> I decided to suggest 4 OpenWrt-prefixed properties for "chosen". I'm >>>> hoping this small custom binding is sth we could go with. I'm really >>>> looking forward to upstreaming OpenWrt's downstream DTS files so other >>>> projects (e.g. Buildroot) can use them. >>>> >>>> If you have any better fitting solution in mind please let me know. I >>>> should be fine with anything that lets me solve this downstream mess >>>> situation. >>>> >>>> dtschema/schemas/chosen.yaml | 9 +++++++++ >>>> 1 file changed, 9 insertions(+) >>>> >>>> diff --git a/dtschema/schemas/chosen.yaml b/dtschema/schemas/chosen.yaml >>>> index 6d5c3f1..96d0db7 100644 >>>> --- a/dtschema/schemas/chosen.yaml >>>> +++ b/dtschema/schemas/chosen.yaml >>>> @@ -264,4 +264,13 @@ properties: >>>> patternProperties: >>>> "^framebuffer": true >>>> >>>> + "^openwrt,led-(boot|failsafe|running|upgrade)$": >>>> + $ref: types.yaml#/definitions/string >>>> + description: >>>> + OpenWrt choice of LED for a given role. >>> >>> Neither this explains it. What is "OpenWrt choice"? Choice like what? >>> What are the valid choices? >>> >>>> Value must be a full path (encoded >>>> + as a string) to a relevant LED node. >>> >>> What do you mean by full path? DT node path? Then no, use phandles. >>> >>> Anyway, we have existing properties for this - LED functions. Just >>> choose LED with given function (from sysfs) and you are done. If >>> function is missing in the header: feel free to go, least for me. >> >> In "Describing LEDs roles in device tree?" e-mail I wrote: >> >> " >> Please note that "function" on its own is not sufficient as multiple >> LEDs my share the same function name but its meaning may vary depending >> on color. >> " >> >> Let me elaborate here. >> >> Values of "function" property match LEDs descriptions (usually it's a >> physical label). That is OK and makes sense but doesn't allow OpenWrt to >> automatically pick proper LED to use during given boot stage. >> >> Some devices may have multiple color of a the same LED function. OpenWrt >> developer needs to choose which color to use for failsafe more and which >> boot fully booted state (and others too). >> >> Devices also differ in available LEDs by their functions. Some may have >> LED_FUNCTION_POWER that OpenWrt needs to use. Some may have >> LED_FUNCTION_STATUS. Or both. There are some more (less common) >> functions like LED_FUNCTION_ACTIVITY. >> >> We failed at OpenWrt to develop a single generic script to rule all >> devices and all their LEDs combinations. We ended up with developers >> *choosing* what LED to use for a given system state. > > So this all is because you want to write easier OS. That's abuse of > Devicetree. You can define which LEDs have different meaning, e.g. > physical label or icon attached to them. That's a hardware property. > > You can also define how pieces of hardware are wired together and create > entire system, e.g. connect one LED to disk activity. > > However what you are proposing here is to dynamically configure one, > given OS. I don't think it is suitable. > > The problem of OS to nicely figure out which LED to blink when, is not a > problem of Devicetree. It is a problem of OS and its configuration. I'd say it's a thin line. Or just a grey idea as Geert said. What is a LED "function" after all? How exactly are: LED_FUNCTION_STATUS LED_FUNCTION_ACTIVITY LED_FUNCTION_BOOT LED_FUNCTION_HEARTBEAT different from each other? I can imagine OpenWrt seeing a different role for LED_FUNCTION_ACTIVITY or LED_FUNCTION_BOOT than other projects. Proposed properties "openwrt,led-<foo>" don't exactly describe hardware per se but are still designed to deal with hardware differences. From a practical point of view it's much easier to put such OS configuration info in DT since it's closely related to LEDs defined there and it helps a lot with maintenance. If at some point we change DT due to previous mistake (e.g. we fix LED color from amber to red) that would mean breaking user space of Linux system (changing LED name). Having DT binding for LEDs roles would prevent that. I was hoping that vendor prefixed "chosen" properties may somehow get accepted as a reasonable solution for dealing with hardware differences even if they don't strictly describe hardware itself. Is there any other DT solution you think would be better and could be accepted? Given my hesitation about "function" meaning would something like openwrt,function = "(boot|failsafe|running|upgrade)" be any better? Any other ideas?
On 09/01/2024 22:08, Geert Uytterhoeven wrote: > Hi Krzysztof, > > On Tue, Jan 9, 2024 at 8:10 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> On 09/01/2024 17:38, Rafał Miłecki wrote: >>> On 9.01.2024 10:02, Krzysztof Kozlowski wrote: >>>> On 09/01/2024 09:23, Rafał Miłecki wrote: >>>>> From: Rafał Miłecki <rafal@milecki.pl> >>>>> >>>>> OpenWrt project provides downstream support for thousands of embedded >>>>> home network devices. Its custom requirement for DT is to provide info >>>>> about LEDs roles. Currently it does it by using custom non-documented >>>>> aliases. While formally valid (aliases.yaml doesn't limit names or >>>>> purposes of aliases) it's quite a loose solution. >>>>> >>>>> Document 4 precise "chosen" biding properties with clearly documented >>>>> OpenWrt usage. This will allow upstreaming tons of DTS files that noone >>>> >>>> typo: none >>> >>> typo: no one ;) >>> >>>>> cared about so far as those would need to be patched downstream anyway. >>>> >>>> From all this description I understand why you want to add it, but I >>>> don't understand what are you exactly adding and how it is being used by >>>> the users/OS. >>> >>> In OpenWrt we have user space script that reads LED full path: >>> cat /proc/device-tree/aliases/led-<foo> >>> >>> And then sets LED to state matching current boot stage: >>> echo 1 > /sys/class/leds/<bar>/brightness >> >> OK, it's nowhere close to a hardware property. You now instruct OS what >> to do. It's software or software policy. > >>>> Anyway, we have existing properties for this - LED functions. Just >>>> choose LED with given function (from sysfs) and you are done. If >>>> function is missing in the header: feel free to go, least for me. >>> >>> In "Describing LEDs roles in device tree?" e-mail I wrote: >>> >>> " >>> Please note that "function" on its own is not sufficient as multiple >>> LEDs my share the same function name but its meaning may vary depending >>> on color. >>> " >>> >>> Let me elaborate here. >>> >>> Values of "function" property match LEDs descriptions (usually it's a >>> physical label). That is OK and makes sense but doesn't allow OpenWrt to >>> automatically pick proper LED to use during given boot stage. >>> >>> Some devices may have multiple color of a the same LED function. OpenWrt >>> developer needs to choose which color to use for failsafe more and which >>> boot fully booted state (and others too). >>> >>> Devices also differ in available LEDs by their functions. Some may have >>> LED_FUNCTION_POWER that OpenWrt needs to use. Some may have >>> LED_FUNCTION_STATUS. Or both. There are some more (less common) >>> functions like LED_FUNCTION_ACTIVITY. >>> >>> We failed at OpenWrt to develop a single generic script to rule all >>> devices and all their LEDs combinations. We ended up with developers >>> *choosing* what LED to use for a given system state. >> >> So this all is because you want to write easier OS. That's abuse of >> Devicetree. You can define which LEDs have different meaning, e.g. >> physical label or icon attached to them. That's a hardware property. >> >> You can also define how pieces of hardware are wired together and create >> entire system, e.g. connect one LED to disk activity. >> >> However what you are proposing here is to dynamically configure one, >> given OS. I don't think it is suitable. >> >> The problem of OS to nicely figure out which LED to blink when, is not a >> problem of Devicetree. It is a problem of OS and its configuration. > > I'd say it's a grey area... > > What if the colors are printed on the case, next to the LED? > Like these multi-color LEDs indicating port speed on network switches? > Then it suddenly becomes hardware description, just like > "aliases/serial2 = &...;" referring to serial port 2. > > Next, what if the colors are not printed on the case, but documented > in the product's manual? > What if there is no paper product manual, but just the OpenWRT online > documentation? Mapping between colors and logical meanings is subjective. I don't think it is the same case as LED with a radio-signal or power-plug label. Anyway I also agree that this distinction is a bit blurred. Best regards, Krzysztof
On 09/01/2024 22:48, Rafał Miłecki wrote: >> >> You can also define how pieces of hardware are wired together and create >> entire system, e.g. connect one LED to disk activity. >> >> However what you are proposing here is to dynamically configure one, >> given OS. I don't think it is suitable. >> >> The problem of OS to nicely figure out which LED to blink when, is not a >> problem of Devicetree. It is a problem of OS and its configuration. > > I'd say it's a thin line. Or just a grey idea as Geert said. > > What is a LED "function" after all? How exactly are: > LED_FUNCTION_STATUS > LED_FUNCTION_ACTIVITY > LED_FUNCTION_BOOT > LED_FUNCTION_HEARTBEAT > different from each other? > > I can imagine OpenWrt seeing a different role for LED_FUNCTION_ACTIVITY > or LED_FUNCTION_BOOT than other projects. ...which is not a problem. The meaning of these, except quite obvious heartbeat, is defined by the OS or system configurators. > > Proposed properties "openwrt,led-<foo>" don't exactly describe hardware > per se but are still designed to deal with hardware differences. > > From a practical point of view it's much easier to put such OS > configuration info in DT since it's closely related to LEDs defined > there and it helps a lot with maintenance. If at some point we change I agree, however this is an abuse of DT and therefore it is not an argument to put something into DT. And this was told many, many times on the lists: just because it is easier to instantiate each Linux struct device from DT (with 1-1 mapping between devices and device nodes), does not mean you should do it. Same here. Just because it is easier for OpenWRT, does not mean this is the solution. This is the most frequent argument used in all of such DT abuses. Another example: I want to boot some virtual machine and doing ACPI is too difficult, so I will just use DT as way to pass from host to guest. There were several examples of this. I understand why DT is the easiest for the job... > DT due to previous mistake (e.g. we fix LED color from amber to red) > that would mean breaking user space of Linux system (changing LED name). > Having DT binding for LEDs roles would prevent that. I can argue that LEDs "label" can be un-deprecated and used for that purpose as well. It will provide you stable sysfs entry, regardless of the "color" property. In your case you could also use to solve the actual problem: just label each LED accordingly, e.g. "phase:boot", "phase:upgrade". It might be not the best solution though, because we put one's OS expectations inside DT device node... > I was hoping that vendor prefixed "chosen" properties may somehow get > accepted as a reasonable solution for dealing with hardware differences > even if they don't strictly describe hardware itself. It's actually not the worst idea considering above "OS expectations inside DT device node" when using "label"... > > Is there any other DT solution you think would be better and could be > accepted? > Given my hesitation about "function" meaning would something like > openwrt,function = "(boot|failsafe|running|upgrade)" > be any better? Your problem is not really that specific to OpenWRT - several embedded systems want to do the same, including Android. Some of the LEDs must be active before the user-space comes up, so it is the job for kernel and/or DT. Therefore let's go with generic solutions? I still wonder why we cannot define new LED FUNCTION constants and use them? You need them only for the pre-userspace phase, so do you expect one LED would have two functions? But if you do not have user-space how this aliases are being handled? By how? If you have user-space, then it's not a job for kernel. Best regards, Krzysztof
diff --git a/dtschema/schemas/chosen.yaml b/dtschema/schemas/chosen.yaml index 6d5c3f1..96d0db7 100644 --- a/dtschema/schemas/chosen.yaml +++ b/dtschema/schemas/chosen.yaml @@ -264,4 +264,13 @@ properties: patternProperties: "^framebuffer": true + "^openwrt,led-(boot|failsafe|running|upgrade)$": + $ref: types.yaml#/definitions/string + description: + OpenWrt choice of LED for a given role. Value must be a full path (encoded + as a string) to a relevant LED node. + + Property user may use specified path to control proper LED during current + system boot phase. + additionalProperties: false