| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7CD0DD74-EB4A-414B-B22E-E9248ECF8CBC@gmail.com>
Date: Thu, 21 Aug 2025 11:16:15 -0400
From: Jean-François Lessard <jefflessard3@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
CC: Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
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>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Subject: Re: [PATCH v3 2/4] dt-bindings: auxdisplay: add Titan Micro Electronics TM16xx
Le 21 août 2025 03 h 48 min 21 s HAE, Krzysztof Kozlowski <krzk@...nel.org> a écrit :
>On Wed, Aug 20, 2025 at 12:31:15PM -0400, Jean-François Lessard wrote:
>> Add documentation for TM16xx-compatible 7-segment LED display controllers with
>> keyscan.
>
>Here and other patches - this is not wrapped.
>
>Please wrap commit message according to Linux coding style / submission
>process (neither too early nor over the limit):
>https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
Well received. Will wrap at 75 instead of 80 for commit messages.
>>
>> Signed-off-by: Jean-François Lessard <jefflessard3@...il.com>
>> ---
>>
>> Note: The 'segments' property is intentionally not vendor-prefixed as it defines
>> a generic hardware description concept applicable to any 7-segment display
>> controller. The property describes the fundamental grid/segment coordinate
>> mapping that is controller-agnostic and could be reused by other LED matrix
>> display bindings. Similar to how 'gpios' describes GPIO connections generically,
>> 'segments' describes segment connections in a standardized way using
>> uint32-matrix format.
>>
>> .../bindings/auxdisplay/titanmec,tm16xx.yaml | 471 ++++++++++++++++++
>> 1 file changed, 471 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..b563c6e1e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml
>> @@ -0,0 +1,471 @@
>> +# 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
>> +
>> +maintainers:
>> + - Jean-François Lessard <jefflessard3@...il.com>
>> +
>> +description: |
>> + LED matrix controllers used in auxiliary display devices that drive individual
>> + LED icons and 7-segment digit groups through a grid/segment addressing scheme.
>> + Controllers manage a matrix of LEDs organized as grids (columns/banks in
>> + vendor datasheets) and segments (rows/bit positions in vendor datasheets).
>> + Maximum grid and segment indices are controller-specific.
>> +
>> + The controller is agnostic of the display layout. Board-specific LED wiring is
>> + described through child nodes that specify grid/segment coordinates for
>> + individual icons and segment mapping for 7-segment digits.
>> +
>> + The bindings use separate 'leds' and 'digits' containers to accommodate
>> + different addressing schemes:
>> + - LEDs use 2-cell addressing (grid, segment) for matrix coordinates
>> + - Digits use 1-cell addressing with explicit segment mapping
>> +
>> + Optional keypad scanning is supported when both 'linux,keymap' and
>> + 'poll-interval' properties are specified.
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + # Controllers with titanmec,tm1628 fallback
>
>Drop comment, obvious. Schema tells that.
>
Well received.
>> + - items:
>> + - enum:
>> + - fdhisi,fd628
>> + - princeton,pt6964
>> + - wxicore,aip1628
>> + - const: titanmec,tm1628
>> + - const: titanmec,tm1628
>
>This is part of one enum
>
>> + # Controllers with titanmec,tm1618 fallback
>> + - items:
>> + - enum:
>> + - wxicore,aip1618
>> + - const: titanmec,tm1618
>> + - const: titanmec,tm1618
>
>Enum...
>
>> + # 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.
>> +
>> + 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.
If helpful, I can add a $ref to /schemas/leds/common.yaml#/properties/label
or include its description explicitly.
>> +
>> + default-brightness:
>> + description:
>> + Brightness to be set if LED's default state is on. Used only during
>> + initialization. If the option is not set then max brightness is used.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + max-brightness:
>> + description:
>> + Normally the maximum brightness is determined by the hardware and this
>> + property is not required. This property is used to put a software limit
>> + on the brightness apart from what the driver says, as it could happen
>> + that a LED can be made so bright that it gets damaged or causes damage
>> + due to restrictions in a specific system, such as mounting conditions.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + linux,keymap:
>> + $ref: /schemas/input/matrix-keymap.yaml#/properties/linux,keymap
>> +
>> + poll-interval:
>> + $ref: /schemas/input/input.yaml#/properties/poll-interval
>> +
>> + autorepeat:
>> + $ref: /schemas/input/input.yaml#/properties/autorepeat
>
>You rather miss some reference to input or touchscreen.
>
Well received. I will replace the individual references with:
allOf:
- $ref: /schemas/input/input.yaml#
- $ref: /schemas/input/matrix-keymap.yaml#
>> +
>> + digits:
>> + type: object
>> + description: Container for 7-segment digit group definitions
>
>additionalProperties go here
>
>and blank line
>
>> + properties:
>> + "#address-cells":
>> + const: 1
>> + "#size-cells":
>> + const: 0
>> +
>> + patternProperties:
>> + "^digit@[0-9]+$":
>> + type: object
>
>unevaluatedProperties
>
>Blank line
>
>> + properties:
>> + reg:
>> + description: Digit position identifier
>> + maxItems: 1
>
>Blank line
>
Well received.
>> + 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
>> + minItems: 7
>> + maxItems: 7
>> + required:
>> + - reg
>> + - segments
>> + unevaluatedProperties: false
>> +
>> + additionalProperties: false
>> +
>> + leds:
>> + type: object
>> + description: Container for individual LED icon definitions
>> + properties:
>> + "#address-cells":
>> + const: 2
>> + "#size-cells":
>> + const: 0
>> +
>> + patternProperties:
>> + "^led@[0-9]+,[0-9]+$":
>> + type: object
>> + $ref: /schemas/leds/common.yaml#
>> + properties:
>> + reg:
>> + description:
>> + Grid and segment indices as <grid segment> of this individual LED icon
>> + required:
>> + - reg
>> + unevaluatedProperties: false
>> +
>> + additionalProperties: false
>
>Best regards,
>Krzysztof
>
Thanks for your detailed feedback.
Best regards,
Jean-François Lessard
Powered by blists - more mailing lists