Re: [PATCH v3 2/4] dt-bindings: auxdisplay: add Titan Micro Electronics TM16xx

[Jean-François Lessard]

Le 22 août 2025 02 h 44 min 26 s HAE, Krzysztof Kozlowski <krzk@xxxxxxxxxx> a écrit :
>On 21/08/2025 17:16, Jean-François Lessard wrote:
>>>
>>>> +      # Controllers with titanmec,tm1650 fallback
>>>> +      - items:
>>>> +          - enum:
>>>> +              - fdhisi,fd650
>>>> +              - wxicore,aip650
>>>> +          - const: titanmec,tm1650
>>>> +      - const: titanmec,tm1650
>>>> +      # Canonical standalone controllers
>>>
>>> Drop
>>>
>>>> +      - const: fdhisi,fd620
>>>> +      - const: fdhisi,fd655
>>>> +      - const: fdhisi,fd6551
>>>> +      - const: titanmec,tm1620
>>>> +      - const: titanmec,tm1638
>>>> +      - const: winrise,hbs658
>>>
>>> This is one enum
>>>
>> 
>> Well received. I followed the older style and will adopt the modern approach:
>> 
>> properties:
>>   compatible:
>>     items:
>>       - enum:
>>           - fdhisi,fd628
>>           - princeton,pt6964
>>           - wxicore,aip1628
>>           - wxicore,aip1618
>>           - fdhisi,fd650
>>           - wxicore,aip650
>>           - fdhisi,fd620
>>           - fdhisi,fd655
>>           - fdhisi,fd6551
>>           - titanmec,tm1620
>>           - titanmec,tm1638
>>           - winrise,hbs658
>>       - enum:
>>           - titanmec,tm1628
>>           - titanmec,tm1618
>>           - titanmec,tm1650
>>     minItems: 1
>>     maxItems: 2
>> 
>> Fallback relationships will be documented explicitly in the binding’s description.
>
>Sorry, but what? I commented under specific lines. Why are you changing
>other things?
>
>

I apologize for misunderstanding. I hope this fits your specific comments:

properties:
  compatible:
    oneOf:
      - items:
          - enum:
              - fdhisi,fd628
              - princeton,pt6964
              - wxicore,aip1628
          - const: titanmec,tm1628  
      - items:
          - enum:
              - wxicore,aip1618
          - const: titanmec,tm1618
      - items:
          - enum:
              - fdhisi,fd650
              - wxicore,aip650
          - const: titanmec,tm1650
      - enum:
          - titanmec,tm1628
          - titanmec,tm1618
          - titanmec,tm1650
          - fdhisi,fd620
          - fdhisi,fd655
          - fdhisi,fd6551
          - titanmec,tm1620
          - titanmec,tm1638
          - winrise,hbs658

>> 
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  label:
>>>> +    description: Name of the entire device
>>>> +    default: display
>>>
>>> Huh? Why do you need label? Are you sure auxdisplays have labels?
>>>
>>>
>> 
>> Display integrates with the LED subsystem (/sys/class/leds), where label is
>> a standard property per leds/common.yaml. It ensures predictable LED class
>> device naming when multiple display instances are present, following established
>> LED subsystem conventions rather than general DT naming rules.
>
>You want to say that top level node is like LED? Then probably it misses
>common.yaml reference. Although I am still puzzled... you have sub nodes
>for actual LEDs, no?
>

The top-level device node is registered as a pseudo-LED device to control the
entire display (brightness 0-7, digits) via /sys/class/leds/{label}.
Individual LED icons are separate LED devices at
/sys/class/leds/{label}::{function} with on/off status control (brightness 0-1).

However, the top-level pseudo-LED shouldn't support all LED properties
(no color, function, trigger-sources, etc.), so I'll reference specific
properties from leds/common.yaml rather than the entire schema:

  label:
    $ref: /schemas/leds/common.yaml#/properties/label
  max-brightness:
    $ref: /schemas/leds/common.yaml#/properties/max-brightness

This approach provides LED subsystem consistency while avoiding inappropriate 
properties for the entire display.

>> 
>> If helpful, I can add a $ref to /schemas/leds/common.yaml#/properties/label
>> or include its description explicitly.
>> 

Best Regards,
Jean-François Lessard