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
next prev parent 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®