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

["Jean-François Lessard" <jefflessard3@gmail.com>]

Le 25 août 2025 21 h 33 min 58 s HAE, "Jean-François Lessard" <jefflessard3@gmail.com> a écrit :
>Le 25 août 2025 14 h 26 min 57 s HAE, Rob Herring <robh@kernel.org> a écrit :
>>On Sun, Aug 24, 2025 at 11:32:28PM -0400, Jean-François Lessard wrote:
>>> Add documentation for TM16xx-compatible 7-segment LED display controllers
>>> with keyscan.
>>> 
>>> Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
>>> ---
>>>
...
>>>  .../bindings/auxdisplay/titanmec,tm16xx.yaml  | 477 ++++++++++++++++++
>>>  MAINTAINERS                                   |   5 +
>>>  2 files changed, 482 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml
>>> 
>>> diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml
>>> new file mode 100644
>>> index 000000000..c94556d95
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml
>>> @@ -0,0 +1,477 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm16xx.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Auxiliary displays based on TM16xx and compatible LED controllers
>>> +
...
>>> +
>>> +  digits:
>>> +    type: object
>>> +    description: Container for 7-segment digit group definitions
>>> +    additionalProperties: false
>>> +
>>> +    properties:
>>> +      "#address-cells":
>>> +        const: 1
>>> +      "#size-cells":
>>> +        const: 0
>>> +
>>> +    patternProperties:
>>> +      "^digit@[0-9]+$":
>>> +        type: object
>>> +        unevaluatedProperties: false
>>> +
>>> +        properties:
>>> +          reg:
>>> +            description: Digit position identifier
>>
>>Position is right to left (0 on right)? Please clarify.
>> 
>
>I'll clarify: digit positions are numbered sequentially left-to-right, 
>with reg=0 representing the leftmost digit position as displayed to the user.
>
>>> +            maxItems: 1
>>> +
>>> +          segments:
>>> +            $ref: /schemas/types.yaml#/definitions/uint32-matrix
>>> +            description: |
>>> +              Array of grid/segment coordinate pairs for each 7-segment position.
>>> +              Each entry is <grid segment> mapping to standard 7-segment positions
>>> +              in order: a, b, c, d, e, f, g
>>> +
>>> +              Standard 7-segment layout:
>>> +                 aaa
>>> +                f   b
>>> +                f   b
>>> +                 ggg
>>> +                e   c
>>> +                e   c
>>> +                 ddd
>>> +            items:
>>> +              items:
>>> +                - description: Grid index
>>> +                - description: Segment index
>>
>>Can't you do an array instead and make the array index be the grid or 
>>segment index?
>>
>
>Original design was array-based:
>- titanmec,digits: array of grid indices
>- titanmec,segment-mapping: array of segment indices for a,b,c,d,e,f,g
>- titanmec,transposed: boolean for matrix-transposed cases
>
>The current explicit coordinate approach was adopted based on v2 feedback and
>handles both standard and transposed wiring patterns effectively, where
>manufacturers swap grid/segment relationships:
>- Standard: digit segments use same grid, different segments  
>- Transposed: digit segments use same segment, different grids
>It also future-proofs potential irregular wiring patterns where individual
>digits might have different grid/segment relationships.
>
>Unless you have strong objections, I prefer to keep this approach to avoid
>further churn, as it's proven to handle all the real-world board layouts
>encountered.
>
>See 
>ttps://lore.kernel.org/linux-devicetree/9133F5BC-7F4E-4732-9649-178E5A698273@gmail.com/
>

Diving deeper on your suggestion of using arrays, would this revised design be
acceptable?

properties:
  digits:
    patternProperties:
      "^digit@[0-9]+$":
        properties:
          reg:
            maxItems: 1
            
          grids:
            $ref: /schemas/types.yaml#/definitions/uint32-array
            description: Grid indices for segments a,b,c,d,e,f,g in order
            minItems: 7
            maxItems: 7
            
          segments:
            $ref: /schemas/types.yaml#/definitions/uint32-array  
            description: Segment indices for segments a,b,c,d,e,f,g in order
            minItems: 7
            maxItems: 7

This approach:
- Uses arrays as you suggested, indexed by segment position
- Maintains flexibility for both standard and transpose layouts
- Keeps the semantic clarity that Krzysztof requested

Example usage would be:

digit@0 {
    reg = <0>;
    grids = <4 4 4 4 4 4 4>;     // Standard: all segments use same grid
    segments = <3 4 5 0 1 2 6>;   // Different segment indices
};

// vs transpose case:
digit@0 {
    reg = <0>;
    grids = <0 1 2 3 4 5 6>;     // Transpose: different grids
    segments = <3 3 3 3 3 3 3>;   // Same segment index
};

Would this better align with your preference for array-based approaches?

If so, the remaining question is if these needs to be vendor prefixed
or if they are still generic enough hardware description concept
applicable to any 7-segment display controller.

>>> +            minItems: 7
>>> +            maxItems: 7
>>> +
>>> +        required:
>>> +          - reg
>>> +          - segments
>>> +
...

Best Regards
Jean-François Lessard