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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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