mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Jean-François Lessard" <jefflessard3@gmail.com>
To: Krzysztof Kozlowski <krzk@kernel.org>,
	Andy Shevchenko <andy@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>
Cc: "Geert Uytterhoeven" <geert@linux-m68k.org>,
	devicetree@vger.kernel.org, linux-leds@vger.kernel.org,
	linux-kernel@vger.kernel.org, "Andreas Färber" <afaerber@suse.de>,
	"Boris Gjenero" <boris.gjenero@gmail.com>,
	"Christian Hewitt" <christianshewitt@gmail.com>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"Paolo Sabatino" <paolo.sabatino@gmail.com>
Subject: Re: [PATCH v2 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX
Date: Wed, 02 Jul 2025 20:33:20 -0400	[thread overview]
Message-ID: <9133F5BC-7F4E-4732-9649-178E5A698273@gmail.com> (raw)
In-Reply-To: <B57E9DD6-52F5-4522-AB80-EF0AEEAED5C0@gmail.com>

Le 2 juillet 2025 13 h 30 min 58 s HAE, "Jean-François Lessard" <jefflessard3@gmail.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

  reply	other threads:[~2025-07-03  0:33 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-29 12:59 [PATCH v2 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
2025-06-29 12:59 ` Jean-François Lessard
2025-06-29 12:59 ` [PATCH v2 1/8] dt-bindings: vendor-prefixes: Add Fuda Hisi Microelectronics Jean-François Lessard
2025-06-29 12:59 ` [PATCH v2 2/8] dt-bindings: vendor-prefixes: Add Titan Micro Electronics Jean-François Lessard
2025-06-29 12:59 ` [PATCH v2 3/8] dt-bindings: vendor-prefixes: Add Princeton Technology Corp Jean-François Lessard
2025-07-03  7:32   ` Krzysztof Kozlowski
2025-07-03  8:13     ` Geert Uytterhoeven
2025-06-29 12:59 ` [PATCH v2 4/8] dt-bindings: vendor-prefixes: Add Winrise Technology Jean-François Lessard
2025-06-30 12:25   ` Krzysztof Kozlowski
2025-06-30 13:51     ` Christian Hewitt
2025-07-02 20:14       ` Krzysztof Kozlowski
2025-07-03  0:50         ` Jean-François Lessard
2025-06-29 12:59 ` [PATCH v2 5/8] dt-bindings: vendor-prefixes: Add Wuxi i-Core Electronics Jean-François Lessard
2025-06-30  6:07   ` Krzysztof Kozlowski
2025-06-30  8:19   ` Geert Uytterhoeven
2025-06-30  8:31     ` Christian Hewitt
2025-06-30  8:38       ` Geert Uytterhoeven
2025-06-30 12:24     ` Krzysztof Kozlowski
2025-06-30 13:53       ` Christian Hewitt
2025-06-29 12:59 ` [PATCH v2 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX Jean-François Lessard
2025-06-30  6:19   ` Krzysztof Kozlowski
2025-07-01  3:22     ` Jean-François Lessard
2025-07-02 15:02       ` Krzysztof Kozlowski
2025-07-02 15:07         ` Krzysztof Kozlowski
2025-07-02 17:30         ` Jean-François Lessard
2025-07-03  0:33           ` Jean-François Lessard [this message]
2025-07-03  7:33   ` Krzysztof Kozlowski
2025-06-29 13:18 ` [PATCH v2 7/8] auxdisplay: Add Titanmec TM16xx 7-segment display controllers driver Jean-François Lessard
2025-06-30  6:12   ` Krzysztof Kozlowski
2025-06-30  7:27     ` Andy Shevchenko
2025-06-30  9:27       ` Krzysztof Kozlowski
2025-06-30  9:54         ` Andy Shevchenko
2025-06-30 11:39           ` Krzysztof Kozlowski
2025-06-30 14:17             ` Andy Shevchenko
2025-07-02 15:05               ` Krzysztof Kozlowski
2025-07-02 15:19                 ` Andy Shevchenko
2025-07-03 20:49                   ` Miguel Ojeda
2025-07-04  8:23                     ` Krzysztof Kozlowski
2025-07-04  9:26                       ` Miguel Ojeda
2025-07-01  1:02     ` Jean-François Lessard
2025-07-01  1:32   ` kernel test robot
2025-06-29 13:19 ` [PATCH v2 8/8] MAINTAINERS: Add entry for TM16xx driver Jean-François Lessard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9133F5BC-7F4E-4732-9649-178E5A698273@gmail.com \
    --to=jefflessard3@gmail.com \
    --cc=afaerber@suse.de \
    --cc=andy@kernel.org \
    --cc=boris.gjenero@gmail.com \
    --cc=christianshewitt@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=geert@linux-m68k.org \
    --cc=hkallweit1@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=paolo.sabatino@gmail.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®