[PATCH dt-schema] schemas: chosen: Add OpenWrt LEDs properties for system states

[Krzysztof Kozlowski]

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