Message-ID: <9133F5BC-7F4E-4732-9649-178E5A698273@gmail.com>
Date: Wed, 02 Jul 2025 20:33:20 -0400
From: Jean-François Lessard <jefflessard3@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Andy Shevchenko <andy@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>
CC: Geert Uytterhoeven <geert@...ux-m68k.org>, devicetree@...r.kernel.org,
 linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org,
 Andreas Färber <afaerber@...e.de>,
 Boris Gjenero <boris.gjenero@...il.com>,
 Christian Hewitt <christianshewitt@...il.com>,
 Heiner Kallweit <hkallweit1@...il.com>,
 Paolo Sabatino <paolo.sabatino@...il.com>
Subject: Re: [PATCH v2 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX

Le 2 juillet 2025 13 h 30 min 58 s HAE, "Jean-François Lessard" <jefflessard3@...il.com> a écrit :
>>>>> +  titanmec,digits:
>>>>> +    description: |
>>>>> +      Array of grid (row) indexes corresponding to specific wiring of digits in the display matrix.
>>>>
>>>> What is wiring of digits? This and other descriptions don't tell me much.
>>>>
>>> 
>>> Controllers use a matrix to drive LEDs. Terminology used in datasheets is:
>>> - grids: matrix rows
>>> - segments: matrix columns
>>> 
>>> Board manufacturers wires display panels differently, including LEDs which are parts of 7-segments:
>>> - “digits” refers to the ordered list of GRID indices wired to the physical 7-segment digit displays (arranged right to left)
>>> - “segment-mapping” defines how each SEGMENT index within these grids maps to the standard 7-segment elements (a-g)
>>> 
>>>> Wrap according to Linux coding style, so at 80.
>>>>
>>>>> +      Defines which grid lines are connected to digit elements.
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>>>>> +    items:
>>>>> +      minimum: 0
>>>>> +      maximum: 7
>>>>> +    minItems: 1
>>>>> +    maxItems: 8
>>>>> +
>>>>> +  titanmec,segment-mapping:
>>>>> +    description: |
>>>>
>>>> Do not need '|' unless you need to preserve formatting.
>>>>
>>>>> +      Array of segment (column) indexes specifying the hardware layout mapping used for digit display.
>>>>> +      Each entry gives the segment index corresponding to a standard 7-segment element (a-g).
>>>>
>>>> Wrap according to Linux coding style, so at 80.
>>>>
>>>> This looks like duplicating the reg property.
>>>>
>>> 
>>> While related, this is not replicating the reg property of led child nodes.
>>> 
>>> Each (grid,segment) combination might have a distinct role:
>>> - part of a 7-segment: described using digits and segment-mapping properties
>>> - individual led: described using led child nodes
>>> 
>>>>
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>>>>> +    items:
>>>>> +      minimum: 0
>>>>> +      maximum: 7
>>>>> +    minItems: 7
>>>>> +    maxItems: 7
>>>>> +
>>>>> +  titanmec,transposed:
>>>>> +    description: |
>>>>> +      Optional flag indicating if grids and segments are swapped compared to standard matrix orientation.
>>>>> +      This accommodates devices where segments are wired to rows and grids to columns.
>>>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>>>> +
>>>>> +  "#address-cells":
>>>>> +    const: 2
>>>>> +
>>>>> +  "#size-cells":
>>>>> +    const: 0
>>>>> +
>>>>> +patternProperties:
>>>>> +  "^led@[0-7],[0-7]$":
>>>>
>>>> Why do you have two addresses? It's not used in your example.
>>>>
>>> 
>>> First is for the grid index, second of for the segment index.
>>
>>But it is not used. I really do not get why this is different than other
>>matrix LED controllers.
>>
>
>You are right, addresses of child led nodes are not used by the driver.
>But the 2 cells of the reg property are used by the driver.
>Isn't it a common practice to match node addresses the reg property?
>
>I will thoroughly review other matrix LED controllers again to better capture what I am missing here.
>

I have reviewed other matrix LED controllers again and it doesn't not match.
Some controllers linearize the matrix using a single address, but that doesn't fit current usage.

However, I see the confusion with the two-address led@x,y nodes used for icons.
These are for individual LEDs wired at specific grid/segment pairs (e.g. WiFi, USB indicators)
while digits are driven as ordered groups.

The current bindings use
- "titanmec,digits"
- "titanmec,segment-mapping"
- "titanmec,transposed"
to concisely describe 7-segment digit groups without enumerating each grid/segment individually.
The idea was to simplify definitions since most displays wire segments consistently across digits.

For clarity and extendability, I am considering an alternative bindings structure like:

auxdisplay@24 {
  compatible = "...";
  reg = <0x24>;

  leds {
    #address-cells = <2>;
    #size-cells = <0>;

    wifi_led: led@0,1 {
      reg = <0 1>;
      function = "wifi";
    };
  };

  digits {
    #address-cells = <1>;
    #size-cells = <0>;

    digit@0 {
      reg = <0>;
      segments = <1 0>, <1 1>, <1 2>, <1 3>, <1 4>, <1 5>, <1 6>; /* a-g */
    };

    digit@1 {
      reg = <1>;
      segments = <2 0>, <2 1>, <2 2>, <2 3>, <2 4>, <2 5>, <2 6>; /* a-g */
    };
  };
};

This explicitly separates icons (leds) and 7-segment digit definitions (digits),
avoiding ambiguity with generic LED matrix drivers.

Would you prefer this approach for v3?

>>> 
>>>>> +    $ref: /schemas/leds/common.yaml#
>>>>> +    properties:
>>>>> +      reg:
>>>>> +        description: Grid (row) and segment (column) index in the matrix of this individual LED icon
>>>>
>>>> Missing constraints.
>>>>
>>>>> +    required:
>>>>> +      - reg
>>>>> +
>>> 
>>> Well noted.
>>> 
...
>>
>>
>>Best regards,
>>Krzysztof
>
>Thanks for your time, patience and guidance,
>Jean-François Lessard
>

Thanks for your guidance.

Best regards,
Jean-François Lessard

Powered by blists - more mailing lists