auxdisplay: Add TM16xx and compatible LED display controllers driver

Assign vendor prefix "princeton", based on their domain name.

Quoted 1 line · Jean-François Lessard

Assign vendor prefix "princeton", based on their domain name.

princeton.com gives me consultants, although I expected known university, so same questions or problem in commit msg as in other cases.

Assign vendor prefix "winrise", matching their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

Documentation/devicetree/bindings/vendor-prefixes.yaml +0 -0

index f03ab02afe..a3bf93e5dc 100644

Quoted 7 lines · Jean-François Lessard

Assign vendor prefix "winrise", matching their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++

1 file changed, 2 insertions(+)

Documentation/devicetree/bindings/vendor-prefixes.yaml patternProperties: · line 1715

Quoted 10 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0

index f03ab02afe..a3bf93e5dc 100644
@@ -1711,6 +1711,8 @@ patternProperties:
    description: Wingtech Technology Co., Ltd.
  "^winlink,.*":
    description: WinLink Co., Ltd
+  "^winrise,.*":
+    description: Shenzhen Winrise Technology Co., Ltd.

Hm? https://winrise.com/ redirects to https://amdaluminum.com/ for fences and other alu products.

Quoted 10 lines · Krzysztof Kozlowski

Quoted 7 lines · Jean-François Lessard

Assign vendor prefix "winrise", matching their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++

1 file changed, 2 insertions(+)

Quoted 12 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml patternProperties: · line 1715

Quoted 10 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0

index f03ab02afe..a3bf93e5dc 100644
@@ -1711,6 +1711,8 @@ patternProperties:
    description: Wingtech Technology Co., Ltd.
  "^winlink,.*":
    description: WinLink Co., Ltd
+  "^winrise,.*":
+    description: Shenzhen Winrise Technology Co., Ltd.

Hm? https://winrise.com/ redirects to https://amdaluminum.com/ for fences and other alu products.

According to multiple Chinese chip-buying/trade websites [0],[1] and the internet archive [2] their domain is winrise.cn (not .com). There is currently no active website despite whois entries showing that the domain registration is still active/alive.

[0] http://www.84878.tradebig.com/
[1] https://www.tradeeasy.com/supplier/714703/products
[2] https://web.archive.org/web/20160312143416/http://winrise.cn/

If you’d prefer “Assign vendor prefix based on their name” as the patch description? we can change that for the next iteration.

Quoted 28 lines

Quoted 10 lines · Krzysztof Kozlowski

Quoted 7 lines · Jean-François Lessard

Assign vendor prefix "winrise", matching their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++

1 file changed, 2 insertions(+)

Quoted 12 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml patternProperties: · line 1715

Quoted 10 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0

index f03ab02afe..a3bf93e5dc 100644
@@ -1711,6 +1711,8 @@ patternProperties:
    description: Wingtech Technology Co., Ltd.
  "^winlink,.*":
    description: WinLink Co., Ltd
+  "^winrise,.*":
+    description: Shenzhen Winrise Technology Co., Ltd.

Hm? https://winrise.com/ redirects to https://amdaluminum.com/ for fences and other alu products.

According to multiple Chinese chip-buying/trade websites [0],[1] and the internet archive [2] their domain is winrise.cn (not .com). There is currently no active website despite whois entries showing that the domain registration is still active/alive.

Quoted 6 lines

[0] http://www.84878.tradebig.com/
[1] https://www.tradeeasy.com/supplier/714703/products
[2] https://web.archive.org/web/20160312143416/http://winrise.cn/

If you’d prefer “Assign vendor prefix based on their name” as the patch description? we can change that for the next iteration.

If commit msg says "domain name" as an argument and it turns out it is not matching domain name, then that other domain name least needs to be in commit msg. The rule of domain name comes from US tickers, so only .com should be considered. If there is no conflict and no .com manufacturer it is fine to use whatever other name, but the commit msg is not then correct.

Best regards,
Krzysztof

Assign vendor prefix "icore", based on their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

---

Assign vendor prefix "icore", based on their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

Hi Jean-François,

Quoted 3 lines · Jean-François Lessard

Assign vendor prefix "icore", based on their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

Thanks for your patch!

Documentation/devicetree/bindings/vendor-prefixes.yaml patternProperties: · line 698

Quoted 8 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0

@@ -694,6 +694,8 @@ patternProperties:
    description: International Business Machines (IBM)
  "^ibm,.*":
    description: International Business Machines (IBM)
+  "^icore,.*":
+    description: Wuxi i-Core Electronics Co., Ltd.

This sounds a bit too generic to me. What is the domain name?

Documentation/devicetree/bindings/vendor-prefixes.yaml patternProperties:

Quoted 4 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml +0 -1

  "^icplus,.*":
    description: IC Plus Corp.
  "^idt,.*":
--

Gr{oetje,eeting}s,

Geert

Signature · 5 lines

Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                               -- Linus Torvalds

Quoted 9 lines · Geert Uytterhoeven

Quoted 3 lines · Jean-François Lessard

Assign vendor prefix "icore", based on their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

Thanks for your patch!

Quoted 10 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml patternProperties: · line 698

Quoted 8 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0

@@ -694,6 +694,8 @@ patternProperties:
    description: International Business Machines (IBM)
  "^ibm,.*":
    description: International Business Machines (IBM)
+  "^icore,.*":
+    description: Wuxi i-Core Electronics Co., Ltd.

This sounds a bit too generic to me. What is the domain name?

Their domain/website is http://www.i-core.cn/en/ and i-Core is used as the watermark in their datasheets [0]. We’ve thought to drop the hyphen and use plain ‘icore’ to avoid future typos.

Hi Jean-François,

Quoted 3 lines · Jean-François Lessard

Assign vendor prefix "icore", based on their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

Thanks for your patch!

Documentation/devicetree/bindings/vendor-prefixes.yaml patternProperties: · line 698

Quoted 8 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0

@@ -694,6 +694,8 @@ patternProperties:
     description: International Business Machines (IBM)
   "^ibm,.*":
     description: International Business Machines (IBM)
+  "^icore,.*":
+    description: Wuxi i-Core Electronics Co., Ltd.

This sounds a bit too generic to me. What is the domain name?

Quoted 10 lines · Geert Uytterhoeven

Hi Jean-François,

Quoted 3 lines · Jean-François Lessard

Assign vendor prefix "icore", based on their domain name.

Signed-off-by Jean-François Lessard <[email protected]>

Thanks for your patch!

Quoted 10 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml patternProperties: · line 698

Quoted 8 lines

Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0

@@ -694,6 +694,8 @@ patternProperties:
    description: International Business Machines (IBM)
  "^ibm,.*":
    description: International Business Machines (IBM)
+  "^icore,.*":
+    description: Wuxi i-Core Electronics Co., Ltd.

This sounds a bit too generic to me. What is the domain name?

Good point. icore.com should be but obviously it points somewhere else, so this does not follow ticker / domain name style.

Add documentation for Titanmec TM16XX and compatible LED display controllers.

This patch is inspired by previous work from Andreas Färber and Heiner Kallweit.

Co-developed-by Andreas Färber <[email protected]>

Co-developed-by Heiner Kallweit <[email protected]>

Signed-off-by Jean-François Lessard <[email protected]>

---
 .../bindings/auxdisplay/titanmec,tm16xx.yaml  | 210 ++++++++++++++++++
 1 file changed, 210 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +0 -0

new file mode 100644
index 0000000000..65c43e3ba4

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +22 -0

+  board design.
+
+  Typical display example:
+
+           ---    ---       ---    ---
+    WIFI  |   |  |   |  -  |   |  |   |  USB  PLAY
+           ---    ---       ---    ---
+    LAN   |   |  |   |  -  |   |  |   |  BT   PAUSE
+           ---    ---       ---    ---
+
+  The controller itself is agnostic of the display layout. The specific arrangement
+  (which grids and segments drive which digits or icons) is determined by the board-level
+  wiring. Therefore, these bindings describe hardware configuration at the PCB level
+  to enable support of multiple display implementations using these LED controllers.
+
+properties:
+  compatible:
+    enum:
+      - titanmec,tm1618
+      - titanmec,tm1620
+      - titanmec,tm1628
+      - titanmec,tm1650

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +11 -0

+    $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.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +100 -0

+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      display-controller@24 {
+        reg = <0x24>;
+        compatible = "fdhisi,fd655";
+        titanmec,digits = [01 02 03 04];
+        titanmec,segment-mapping = [03 04 05 00 01 02 06];
+        #address-cells = <2>;
+        #size-cells = <0>;
+
+        led@0,0 {
+          reg = <0 0>;
+          function = LED_FUNCTION_ALARM;
+        };
+
+        led@0,1 {
+          reg = <0 1>;
+          function = LED_FUNCTION_USB;
+        };
+
+        led@0,2 {
+          reg = <0 2>;
+          function = "play";
+        };
+
+        led@0,3 {
+          reg = <0 3>;
+          function = "pause";
+        };
+
+        led@0,4 {
+          reg = <0 4>;
+          function = "colon";
+        };
+
+        led@0,5 {
+          reg = <0 5>;
+          function = LED_FUNCTION_LAN;
+        };
+
+        led@0,6 {
+          reg = <0 6>;
+          function = LED_FUNCTION_WLAN;
+        };
+      };
+    };
+
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      display-controller@0 {
+        reg = <0x0>;
+        compatible = "fdhisi,fd628";
+        titanmec,transposed;
+        titanmec,digits = [00 01 02 03];
+        titanmec,segment-mapping = [00 01 02 03 04 05 06];
+        spi-3wire;
+        spi-lsb-first;
+        spi-rx-delay-us = <1>;
+        spi-max-frequency = <500000>;
+        #address-cells = <2>;
+        #size-cells = <0>;
+
+        led@4,0 {
+          reg = <4 0>;
+          function = "apps";
+        };
+
+        led@4,1 {
+          reg = <4 1>;
+          function = "setup";
+        };
+
+        led@4,2 {
+          reg = <4 2>;
+          function = LED_FUNCTION_USB;
+        };
+
+        led@4,3 {
+          reg = <4 3>;
+          function = LED_FUNCTION_SD;
+        };
+
+        led@4,4 {
+          reg = <4 4>;
+          function = "colon";
+        };
+
+        led@4,5 {

Quoted 3 lines

Add documentation for Titanmec TM16XX and compatible LED display controllers.

This patch is inspired by previous work from Andreas Färber and Heiner Kallweit.

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

Please do not use "This commit/patch/change", but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

Quoted 9 lines

Co-developed-by Andreas Färber <[email protected]>

Co-developed-by Heiner Kallweit <[email protected]>

Signed-off-by Jean-François Lessard <[email protected]>

---
 .../bindings/auxdisplay/titanmec,tm16xx.yaml  | 210 ++++++++++++++++++
 1 file changed, 210 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 4

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +4 -0

new file mode 100644
index 0000000000..65c43e3ba4
@@ -0,0 +1,210 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm16xx.yaml#

Why isn't this in leds directory? Everything below describes it as LED controller.

Quoted 4 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +4 -0

+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Auxiliary displays based on TM16xx and compatible LED controllers
+

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 14

Quoted 6 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +6 -0

+maintainers:
+  - Jean-François Lessard <[email protected]>
+
+description: |
+  TM16xx controllers manage a matrix of LEDs organized in grids (rows) and segments (columns).
+  Each grid or segment can be wired to drive either a digit or individual icons, depending on the

Wrap according to Linux coding style, so at 80.

Quoted 27 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +27 -0

+  board design.
+
+  Typical display example:
+
+           ---    ---       ---    ---
+    WIFI  |   |  |   |  -  |   |  |   |  USB  PLAY
+           ---    ---       ---    ---
+    LAN   |   |  |   |  -  |   |  |   |  BT   PAUSE
+           ---    ---       ---    ---
+
+  The controller itself is agnostic of the display layout. The specific arrangement
+  (which grids and segments drive which digits or icons) is determined by the board-level
+  wiring. Therefore, these bindings describe hardware configuration at the PCB level
+  to enable support of multiple display implementations using these LED controllers.
+
+properties:
+  compatible:
+    enum:
+      - titanmec,tm1618
+      - titanmec,tm1620
+      - titanmec,tm1628
+      - titanmec,tm1650
+      - fdhisi,fd620
+      - fdhisi,fd628
+      - fdhisi,fd650
+      - fdhisi,fd6551
+      - fdhisi,fd655

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 46

Quoted 5 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +5 -0

+      - icore,aip650
+      - icore,aip1618
+      - icore,aip1628
+      - princeton,pt6964
+      - winrise,hbs658

Several devices are compatible, so express it here and drop redundant entries in the driver.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 53

Quoted 7 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +7 -0

+
+  reg:
+    maxItems: 1
+
+  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.

Wrap according to Linux coding style, so at 80.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 63

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +10 -0

+      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.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 65

Quoted 2 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +2 -0

+      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.

Quoted 16 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +15 -0

+    $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

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 86

Quoted 6 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +6 -0

+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^led@[0-7],[0-7]$":

Why do you have two addresses? It's not used in your example.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 90

Quoted 4 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +4 -0

+    $ref: /schemas/leds/common.yaml#
+    properties:
+      reg:
+        description: Grid (row) and segment (column) index in the matrix of this individual LED icon

Missing constraints.

Quoted 3 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +3 -0

+    required:
+      - reg
+

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 100

Quoted 7 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +7 -0

+required:
+  - compatible
+  - reg
+  - titanmec,digits
+  - titanmec,segment-mapping
+
+additionalProperties: true

No, this cannot be true. Look at any other binding, look at example-schema.

Quoted 106 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +106 -0

+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      display-controller@24 {
+        reg = <0x24>;
+        compatible = "fdhisi,fd655";
+        titanmec,digits = [01 02 03 04];
+        titanmec,segment-mapping = [03 04 05 00 01 02 06];
+        #address-cells = <2>;
+        #size-cells = <0>;
+
+        led@0,0 {
+          reg = <0 0>;
+          function = LED_FUNCTION_ALARM;
+        };
+
+        led@0,1 {
+          reg = <0 1>;
+          function = LED_FUNCTION_USB;
+        };
+
+        led@0,2 {
+          reg = <0 2>;
+          function = "play";
+        };
+
+        led@0,3 {
+          reg = <0 3>;
+          function = "pause";
+        };
+
+        led@0,4 {
+          reg = <0 4>;
+          function = "colon";
+        };
+
+        led@0,5 {
+          reg = <0 5>;
+          function = LED_FUNCTION_LAN;
+        };
+
+        led@0,6 {
+          reg = <0 6>;
+          function = LED_FUNCTION_WLAN;
+        };
+      };
+    };
+
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      display-controller@0 {
+        reg = <0x0>;
+        compatible = "fdhisi,fd628";
+        titanmec,transposed;
+        titanmec,digits = [00 01 02 03];
+        titanmec,segment-mapping = [00 01 02 03 04 05 06];
+        spi-3wire;
+        spi-lsb-first;
+        spi-rx-delay-us = <1>;
+        spi-max-frequency = <500000>;
+        #address-cells = <2>;
+        #size-cells = <0>;
+
+        led@4,0 {
+          reg = <4 0>;
+          function = "apps";
+        };
+
+        led@4,1 {
+          reg = <4 1>;
+          function = "setup";
+        };
+
+        led@4,2 {
+          reg = <4 2>;
+          function = LED_FUNCTION_USB;
+        };
+
+        led@4,3 {
+          reg = <4 3>;
+          function = LED_FUNCTION_SD;
+        };
+
+        led@4,4 {
+          reg = <4 4>;
+          function = "colon";
+        };
+
+        led@4,5 {
+          reg = <4 5>;
+          function = "hdmi";
+        };
+
+        led@4,6 {
+          reg = <4 6>;

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 210

Quoted 4 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +4 -0

+          function = "video";
+        };
+      };
+    };

Best regards,
Krzysztof

Le 30 juin 2025 02 h 19 min 11 s HAE, Krzysztof Kozlowski <[email protected]> a écrit :

Quoted 22 lines · Jean-François Lessard

Quoted 3 lines

Add documentation for Titanmec TM16XX and compatible LED display controllers.

This patch is inspired by previous work from Andreas Färber and Heiner Kallweit.

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

Please do not use "This commit/patch/change", but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

Quoted 9 lines

Co-developed-by Andreas Färber <[email protected]>

Co-developed-by Heiner Kallweit <[email protected]>

Signed-off-by Jean-François Lessard <[email protected]>

---
 .../bindings/auxdisplay/titanmec,tm16xx.yaml  | 210 ++++++++++++++++++
 1 file changed, 210 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml

Quoted 14 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 4

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +4 -0

new file mode 100644
index 0000000000..65c43e3ba4
@@ -0,0 +1,210 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm16xx.yaml#

Why isn't this in leds directory? Everything below describes it as LED controller.

TM16XX controllers are commonly used as auxiliary display drivers in consumer Android-based TV boxes for driving 7-segment and icon displays, rather than for generic LEDs.

Previous attempts to place TM1628 drivers under LED subsystem were NAK’ed by LED maintainers, with Pavel Machek recommending drivers/auxdisplay instead (see https://lore.kernel.org/linux-devicetree/[email protected]/).

Quoted 17 lines

Quoted 16 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +16 -0

+  compatible:
+    enum:
+      - titanmec,tm1618
+      - titanmec,tm1620
+      - titanmec,tm1628
+      - titanmec,tm1650
+      - fdhisi,fd620
+      - fdhisi,fd628
+      - fdhisi,fd650
+      - fdhisi,fd6551
+      - fdhisi,fd655
+      - icore,aip650
+      - icore,aip1618
+      - icore,aip1628
+      - princeton,pt6964
+      - winrise,hbs658

Quoted 3 lines

Several devices are compatible, so express it here and drop redundant entries in the driver.

I understand the concern. I would appreciate your guidance since these are not always direct aliases. E.g.:
- tm1620 and fd620 varies on which bit is used for the 8th segment
- fd655 and fd650 have no titanmec counterpart
- hbs658 is similar to tm1628, yet distinct

We could keep only known distinct implementations, that would yield to:
- titanmec,tm1618
- titanmec,tm1620
- titanmec,tm1628
- titanmec,tm1650
- fdhisi,fd620
- fdhisi,fd650
- fdhisi,fd6551
- fdhisi,fd655
- winrise,hbs658

Which would be inconsistent for cases where vendors appear for another variant. e.g. fdhisi,fd628 now being aliased by titanmec,tm1628 while other fdhisi variants are listed.

Therefore I would suggest to keep fdhisi,fd628 even if implementation is the same as titanmec,tm1628.

icore and princeton could be dropped in favor of titanmec equivalents, at least for the variants I am aware of. Specific implementation for other variants can be let for future extension, if ever needed.

How would you approach this?

Quoted 11 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 53

Quoted 8 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +7 -0

+
+  reg:
+    maxItems: 1
+
+  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)

Quoted 13 lines

Wrap according to Linux coding style, so at 80.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 63

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +10 -0

+      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: |

Quoted 9 lines

Do not need '|' unless you need to preserve formatting.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 65

Quoted 2 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +2 -0

+      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

Quoted 16 lines

Quoted 16 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +15 -0

+    $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

Quoted 9 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 86

Quoted 6 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +6 -0

+
+  "#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.

Quoted 11 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 90

Quoted 5 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +4 -0

+    $ref: /schemas/leds/common.yaml#
+    properties:
+      reg:
+        description: Grid (row) and segment (column) index in the matrix of this individual LED icon

Missing constraints.

Quoted 3 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +3 -0

+    required:
+      - reg
+

Well noted.

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 100

Quoted 7 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +7 -0

+required:
+  - compatible
+  - reg
+  - titanmec,digits
+  - titanmec,segment-mapping
+
+additionalProperties: true

No, this cannot be true. Look at any other binding, look at example-schema.

I got misled by "spi-3wire" which is not defined in spi-peripheral-props.yaml but only as a child node property of spi-controller.yaml.

I wasn't sure how to implement this correctly. I have reviewed existing examples and will replace with:

if:
properties:
compatible:
contains:
enum:
- titanmec,tm1618
- titanmec,tm1620
- titanmec,tm1628
- fdhisi,fd620
- fdhisi,fd628
- winrise,hbs658
then:
allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

properties:
spi-3wire: true
required:
- spi-3wire

Quoted 15 lines

if:
properties:
compatible:
contains:
enum:
- titanmec,tm1618
- titanmec,tm1620
- titanmec,tm1628
- fdhisi,fd620
- fdhisi,fd628
- winrise,hbs658
then:
allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

Why is this conditional? Are these devices with entirely different programming model? Then they usually should not be in the same binding, although depends on differences.

Quoted 38 lines

Le 30 juin 2025 02 h 19 min 11 s HAE, Krzysztof Kozlowski <[email protected]> a écrit :

Quoted 36 lines · Jean-François Lessard

Quoted 3 lines

Add documentation for Titanmec TM16XX and compatible LED display controllers.

This patch is inspired by previous work from Andreas Färber and Heiner Kallweit.

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

Please do not use "This commit/patch/change", but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

Quoted 15 lines

Co-developed-by Andreas Färber <[email protected]>

Co-developed-by Heiner Kallweit <[email protected]>

Signed-off-by Jean-François Lessard <[email protected]>

---
 .../bindings/auxdisplay/titanmec,tm16xx.yaml  | 210 ++++++++++++++++++
 1 file changed, 210 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +0 -0

new file mode 100644
index 0000000000..65c43e3ba4

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 4

Quoted 4 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +4 -0

@@ -0,0 +1,210 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm16xx.yaml#

Why isn't this in leds directory? Everything below describes it as LED controller.

Quoted 3 lines

TM16XX controllers are commonly used as auxiliary display drivers in consumer Android-based TV boxes for driving 7-segment and icon displays, rather than for generic LEDs.

Previous attempts to place TM1628 drivers under LED subsystem were NAK’ed by LED maintainers, with Pavel Machek recommending drivers/auxdisplay instead (see https://lore.kernel.org/linux-devicetree/[email protected]/).

OK, it's fine. If you want to avoid the same question at v3, v4 and v5, please mention this in the patch changelog.

...

Quoted 21 lines

Quoted 20 lines

Quoted 6 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +6 -0

+  compatible:
+    enum:
+      - titanmec,tm1618
+      - titanmec,tm1620
+      - titanmec,tm1628
+      - titanmec,tm1650

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 46

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +10 -0

+      - fdhisi,fd620
+      - fdhisi,fd628
+      - fdhisi,fd650
+      - fdhisi,fd6551
+      - fdhisi,fd655
+      - icore,aip650
+      - icore,aip1618
+      - icore,aip1628
+      - princeton,pt6964
+      - winrise,hbs658

Several devices are compatible, so express it here and drop redundant entries in the driver.

Quoted 4 lines

I understand the concern. I would appreciate your guidance since these are not always direct aliases. E.g.:
- tm1620 and fd620 varies on which bit is used for the 8th segment
- fd655 and fd650 have no titanmec counterpart
- hbs658 is similar to tm1628, yet distinct

You did not get the point. I did not ask to make incompatible devices as compatible. I asked to make compatible devices compatible.

Also wrap your emails to mailing list style. It's very difficult to read and respond to them.

Quoted 12 lines

We could keep only known distinct implementations, that would yield to:
- titanmec,tm1618
- titanmec,tm1620
- titanmec,tm1628
- titanmec,tm1650
- fdhisi,fd620
- fdhisi,fd650
- fdhisi,fd6551
- fdhisi,fd655
- winrise,hbs658

I do not see compatibility expressed. You need front compatible and fallback.

Quoted 3 lines

Which would be inconsistent for cases where vendors appear for another variant. e.g. fdhisi,fd628 now being aliased by titanmec,tm1628 while other fdhisi variants are listed.

I don't understand what that means. I don't understand what aliased means. There is no such term in DT bindings.

Quoted 2 lines

Therefore I would suggest to keep fdhisi,fd628 even if implementation is the same as titanmec,tm1628.

I don't understand the problem.

Quoted 3 lines

icore and princeton could be dropped in favor of titanmec equivalents, at least for the variants I am aware of. Specific implementation for other variants can be let for future extension, if ever needed.

No clue what you are saying here.

Quoted 2 lines

How would you approach this?

You have compatible devices, yes? If so, you drop irrelevant entries in device ID tables and use fallbacks in the bindings, just like roughly 80% of devices in the bindings.

Quoted 75 lines

Quoted 11 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 53

Quoted 8 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +7 -0

+
+  reg:
+    maxItems: 1
+
+  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)

Quoted 13 lines

Wrap according to Linux coding style, so at 80.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 63

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +10 -0

+      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: |

Quoted 9 lines

Do not need '|' unless you need to preserve formatting.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 65

Quoted 2 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +2 -0

+      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

Quoted 25 lines

Quoted 12 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +11 -0

+    $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.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 86

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +10 -0

+    $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.

Quoted 1 line

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.

Quoted 29 lines

Quoted 11 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 90

Quoted 5 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +4 -0

+    $ref: /schemas/leds/common.yaml#
+    properties:
+      reg:
+        description: Grid (row) and segment (column) index in the matrix of this individual LED icon

Missing constraints.

Quoted 3 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +3 -0

+    required:
+      - reg
+

Well noted.

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 100

Quoted 7 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +7 -0

+required:
+  - compatible
+  - reg
+  - titanmec,digits
+  - titanmec,segment-mapping
+
+additionalProperties: true

No, this cannot be true. Look at any other binding, look at example-schema.

I got misled by "spi-3wire" which is not defined in spi-peripheral-props.yaml but only as a child node property of spi-controller.yaml.

I wasn't sure how to implement this correctly. I have reviewed existing examples and will replace with:

Quoted 14 lines

if:
properties:
compatible:
contains:
enum:
- titanmec,tm1618
- titanmec,tm1620
- titanmec,tm1628
- fdhisi,fd620
- fdhisi,fd628
- winrise,hbs658
then:
allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

Why is this conditional? Are these devices with entirely different programming model? Then they usually should not be in the same binding, although depends on differences.

Quoted 4 lines

properties:
spi-3wire: true
required:
- spi-3wire

Best regards,
Krzysztof

Quoted 70 lines

Quoted 70 lines

Quoted 6 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 53

Quoted 3 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +3 -0

+  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)

Quoted 13 lines

Wrap according to Linux coding style, so at 80.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 63

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +10 -0

+      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: |

Quoted 9 lines

Do not need '|' unless you need to preserve formatting.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 65

Quoted 2 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +2 -0

+      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

Quoted 25 lines

Quoted 12 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +11 -0

+    $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.

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 86

Quoted 10 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +10 -0

+    $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.

Quoted 5 lines

Quoted 1 line

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.

Quoted 14 lines

Quoted 14 lines

Quoted 11 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml line 90

Quoted 5 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +4 -0

+    $ref: /schemas/leds/common.yaml#
+    properties:
+      reg:
+        description: Grid (row) and segment (column) index in the matrix of this individual LED icon

Missing constraints.

Quoted 3 lines

Documentation/devicetree/bindings/auxdisplay/titanmec,tm16xx.yaml new file +3 -0

+    required:
+      - reg
+

Well noted.

Quoted 4 lines

Best regards,
Krzysztof

Thanks for your time, patience and guidance,
Jean-François Lessard

Add documentation for Titanmec TM16XX and compatible LED display controllers.

This patch is inspired by previous work from Andreas Färber and Heiner Kallweit.

Co-developed-by Andreas Färber <[email protected]>

Co-developed-by Heiner Kallweit <[email protected]>

Signed-off-by Jean-François Lessard <[email protected]>

This patch introduces a new auxiliary display driver for TM16XX family LED controllers and compatible chips:

- Shenzhen Titan Micro Electronics: TM1618, TM1620, TM1628, TM1650
- Fuzhou Fuda Hisi Microelectronics: FD620, FD628, FD650, FD655, FD6551
- Wuxi i-Core Electronics: AiP650, AiP1618, AiP1628
- Princeton Technology: PT6964
- Shenzhen Winrise Technology: HBS658

+ * tm16xx_parse_dt - Parse device tree data
+ * @dev: Pointer to device structure
+ * @display: Pointer to tm16xx_display structure
+ *
+ * Return: 0 on success, negative error code on failure
+ */

drivers/auxdisplay/tm16xx.c new file +138 -0

+
+	ret = device_property_count_u8(dev, "titanmec,digits");
+	if (ret < 0) {
+		dev_err(dev, "Failed to count 'titanmec,digits' property: %d\n", ret);
+		return ret;
+	}
+
+	display->num_digits = ret;
+	dev_dbg(dev, "Number of digits: %d\n", display->num_digits);
+
+	digits = devm_kcalloc(dev, display->num_digits, sizeof(*digits), GFP_KERNEL);
+	if (!digits)
+		return -ENOMEM;
+
+	ret = device_property_read_u8_array(dev, "titanmec,digits", digits,
+					    display->num_digits);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read 'titanmec,digits' property: %d\n", ret);
+		return ret;
+	}
+
+	display->digits = devm_kcalloc(dev, display->num_digits, sizeof(*display->digits),
+				       GFP_KERNEL);
+	if (!display->digits)
+		return -ENOMEM;
+
+	for (i = 0; i < display->num_digits; i++) {
+		if (digits[i] >= display->controller->max_grids) {
+			dev_err(dev, "Digit grid %d exceeds controller max_grids %d\n",
+				digits[i], display->controller->max_grids);
+			return -EINVAL;
+		}
+
+		display->digits[i].grid = digits[i];
+		max_grid = umax(max_grid, digits[i]);
+	}
+
+	devm_kfree(dev, digits);
+
+	ret = device_property_read_u8_array(dev, "titanmec,segment-mapping",
+					    display->segment_mapping, DIGIT_SEGMENTS);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read 'titanmec,segment-mapping' property: %d\n",
+			ret);
+		return ret;
+	}
+
+	display->digit_bitmask = 0;
+	for (i = 0; i < DIGIT_SEGMENTS; i++) {
+		if (display->segment_mapping[i] < MIN_SEGMENT ||
+		    display->segment_mapping[i] > MAX_SEGMENT) {
+			dev_err(dev,
+				"Invalid 'titanmec,segment-mapping' value: %d (must be between %d and %d)\n",
+				display->segment_mapping[i], MIN_SEGMENT, MAX_SEGMENT);
+			return -EINVAL;
+		}
+
+		display->digit_bitmask |= BIT(display->segment_mapping[i]);
+	}
+
+	display->num_leds = 0;
+	device_for_each_child_node(dev, child) {
+		u32 reg[2];
+
+		ret = fwnode_property_read_u32_array(child, "reg", reg, 2);
+		if (ret < 0) {
+			dev_err(dev, "Failed to read 'reg' property of led node: %d\n",
+				ret);
+			fwnode_handle_put(child);
+			return ret;
+		}
+
+		if (reg[0] >= display->controller->max_grids) {
+			dev_err(dev, "LED grid %d exceeds controller max_grids %d\n",
+				reg[0], display->controller->max_grids);
+			fwnode_handle_put(child);
+			return -EINVAL;
+		}
+
+		if (reg[1] < MIN_SEGMENT || reg[1] > MAX_SEGMENT) {
+			dev_err(dev,
+				"LED segment %d is invalid (must be between %d and %d)\n",
+				reg[1], MIN_SEGMENT, MAX_SEGMENT);
+			fwnode_handle_put(child);
+			return -EINVAL;
+		}
+
+		max_grid = umax(max_grid, reg[0]);
+		display->num_leds++;
+	}
+
+	dev_dbg(dev, "Number of LEDs: %d\n", display->num_leds);
+
+	display->display_data_len = max_grid + 1;
+	dev_dbg(dev, "Number of display grids: %zu\n", display->display_data_len);
+
+	display->display_data = devm_kcalloc(dev, display->display_data_len,
+					     sizeof(*display->display_data), GFP_KERNEL);
+	if (!display->display_data)
+		return -ENOMEM;
+
+	return 0;
+}
+
+/**
+ * tm16xx_probe - Probe function for tm16xx devices
+ * @display: Pointer to tm16xx_display structure
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int tm16xx_probe(struct tm16xx_display *display)
+{
+	struct device *dev = display->dev;
+	struct fwnode_handle *child;
+	int ret, i;
+
+	ret = tm16xx_parse_dt(dev, display);
+	if (ret < 0) {
+		dev_err(dev, "Failed to parse device tree: %d\n", ret);
+		return ret;
+	}
+
+	mutex_init(&display->lock);
+	INIT_WORK(&display->flush_init, tm16xx_display_flush_init);
+
+	/* Initialize work structure with appropriate flush function */
+	if (display->transpose_display_data) {
+		INIT_WORK(&display->flush_display, tm16xx_display_flush_data_transposed);
+		dev_info(display->dev, "Operating in transposed mode\n");
+	} else {
+		INIT_WORK(&display->flush_display, tm16xx_display_flush_data);
+	}
+
+	display->main_led.name = TM16XX_DEVICE_NAME;
+	display->main_led.brightness = display->controller->max_brightness;
+	display->main_led.max_brightness = display->controller->max_brightness;
+	display->main_led.brightness_set = tm16xx_brightness_set;
+	display->main_led.groups = tm16xx_main_led_groups;

drivers/auxdisplay/tm16xx.c new file +35 -0

+	if (!display->leds)
+		return -ENOMEM;
+
+	i = 0;
+	device_for_each_child_node(dev, child) {
+		struct tm16xx_led *led = &display->leds[i];
+		struct led_init_data led_init = {
+			.fwnode = child,
+			.devicename = display->main_led.name,
+			.devname_mandatory = true,
+		};
+		u32 reg[2];
+
+		ret = fwnode_property_read_u32_array(child, "reg", reg, 2);
+		if (ret < 0) {
+			fwnode_handle_put(child);
+			dev_err(dev, "Failed to read LED reg property: %d\n", ret);
+			return ret;
+		}
+
+		led->grid = reg[0];
+		led->segment = reg[1];
+
+		led->cdev.max_brightness = 1;
+		led->cdev.brightness_set = tm16xx_led_set;
+		led->cdev.flags = LED_RETAIN_AT_SHUTDOWN | LED_CORE_SUSPENDRESUME;
+
+		ret = devm_led_classdev_register_ext(dev, &led->cdev, &led_init);
+		if (ret < 0) {
+			fwnode_handle_put(child);
+			dev_err(dev, "Failed to register LED %s: %d\n", led->cdev.name,
+				ret);
+			return ret;
+		}
+

drivers/auxdisplay/tm16xx.c new file +29 -0

+	return 0;
+}
+
+/* SPI specific code */
+static int tm16xx_spi_probe(struct spi_device *spi)
+{
+	const struct tm16xx_controller *controller;
+	struct tm16xx_display *display;
+	int ret;
+
+	controller = of_device_get_match_data(&spi->dev);
+	if (!controller)
+		return -EINVAL;
+
+	display = devm_kzalloc(&spi->dev, sizeof(*display), GFP_KERNEL);
+	if (!display)
+		return -ENOMEM;
+
+	display->client.spi = spi;
+	display->dev = &spi->dev;
+	display->controller = controller;
+
+	spi_set_drvdata(spi, display);
+
+	ret = tm16xx_probe(display);
+	if (ret)
+		return ret;
+
+	return 0;

drivers/auxdisplay/tm16xx.c new file +18 -0

+static const struct of_device_id tm16xx_spi_of_match[] = {
+	{ .compatible = "titanmec,tm1618", .data = &tm1618_controller },
+	{ .compatible = "titanmec,tm1620", .data = &tm1628_controller },
+	{ .compatible = "titanmec,tm1628", .data = &tm1628_controller },
+	{ .compatible = "fdhisi,fd620", .data = &tm1628_controller },
+	{ .compatible = "fdhisi,fd628", .data = &tm1628_controller },
+	{ .compatible = "icore,aip1618", .data = &tm1618_controller },
+	{ .compatible = "icore,aip1628", .data = &tm1628_controller },
+	{ .compatible = "princeton,pt6964", .data = &tm1628_controller },
+	{ .compatible = "winrise,hbs658", .data = &hbs658_controller },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tm16xx_spi_of_match);
+
+static const struct spi_device_id tm16xx_spi_id[] = {
+	{ "tm1618", 0 },
+	{ "tm1620", 0 },
+	{ "tm1628", 0 },

drivers/auxdisplay/tm16xx.c new file +38 -0

+
+static struct spi_driver tm16xx_spi_driver = {
+	.driver = {
+		.name = TM16XX_DRIVER_NAME,
+		.of_match_table = tm16xx_spi_of_match,
+	},
+	.probe = tm16xx_spi_probe,
+	.remove = tm16xx_spi_remove,
+	.shutdown = tm16xx_spi_remove,
+	.id_table = tm16xx_spi_id,
+};
+
+/* I2C specific code */
+static int tm16xx_i2c_probe(struct i2c_client *client)
+{
+	const struct tm16xx_controller *controller;
+	struct tm16xx_display *display;
+	int ret;
+
+	controller = of_device_get_match_data(&client->dev);
+	if (!controller)
+		return -EINVAL;
+
+	display = devm_kzalloc(&client->dev, sizeof(*display), GFP_KERNEL);
+	if (!display)
+		return -ENOMEM;
+
+	display->client.i2c = client;
+	display->dev = &client->dev;
+	display->controller = controller;
+
+	i2c_set_clientdata(client, display);
+
+	ret = tm16xx_probe(display);
+	if (ret)
+		return ret;
+
+	return 0;

drivers/auxdisplay/tm16xx.c new file +10 -0

+static const struct of_device_id tm16xx_i2c_of_match[] = {
+	{ .compatible = "titanmec,tm1650", .data = &tm1650_controller },
+	{ .compatible = "icore,aip650", .data = &tm1650_controller },
+	{ .compatible = "fdhisi,fd650", .data = &tm1650_controller },
+	{ .compatible = "fdhisi,fd6551", .data = &fd6551_controller },
+	{ .compatible = "fdhisi,fd655", .data = &fd655_controller },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tm16xx_i2c_of_match);
+

drivers/auxdisplay/tm16xx.c new file +35 -0

+
+static struct i2c_driver tm16xx_i2c_driver = {
+	.driver = {
+		.name = TM16XX_DRIVER_NAME,
+		.of_match_table = tm16xx_i2c_of_match,
+	},
+	.probe = tm16xx_i2c_probe,
+	.remove = tm16xx_i2c_remove,
+	.shutdown = tm16xx_i2c_remove,
+	.id_table = tm16xx_i2c_id,
+};
+
+static int __init tm16xx_init(void)
+{
+	int ret;
+
+	ret = spi_register_driver(&tm16xx_spi_driver);
+	if (ret)
+		return ret;
+
+	ret = i2c_add_driver(&tm16xx_i2c_driver);
+	if (ret) {
+		spi_unregister_driver(&tm16xx_spi_driver);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void __exit tm16xx_exit(void)
+{
+	i2c_del_driver(&tm16xx_i2c_driver);
+	spi_unregister_driver(&tm16xx_spi_driver);
+}
+

On 29/06/2025 15:18, Jean-François Lessard wrote:

drivers/auxdisplay/tm16xx.c line 1082

Quoted 2 lines

drivers/auxdisplay/tm16xx.c new file +2 -0

+	display->leds =
+		devm_kcalloc(dev, display->num_leds, sizeof(*display->leds), GFP_KERNEL);

Wrong wrapping. Use kernel style, not clang style.

Review

Quoted 2 lines

(quoted diff) +2 -0

+	if (!display->leds)
+		return -ENOMEM;

Quoted 1 line · Krzysztof Kozlowski

On 29/06/2025 15:18, Jean-François Lessard wrote:

...

Quoted 6 lines

drivers/auxdisplay/tm16xx.c line 1082

Quoted 2 lines

drivers/auxdisplay/tm16xx.c new file +2 -0

+	display->leds =
+		devm_kcalloc(dev, display->num_leds, sizeof(*display->leds), GFP_KERNEL);

Wrong wrapping. Use kernel style, not clang style.

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+	if (!display->leds)
+		return -ENOMEM;

Just wondering how .clang-format is official? Note some of the maintainers even

prefer (ugly in some cases in my opinion) style because it's generated by the clang-format.

Quoted 2 lines · Andy Shevchenko

Quoted 1 line · Krzysztof Kozlowski

On 29/06/2025 15:18, Jean-François Lessard wrote:

Quoted 6 lines

Quoted 6 lines

drivers/auxdisplay/tm16xx.c line 1082

Quoted 2 lines

drivers/auxdisplay/tm16xx.c new file +2 -0

+	display->leds =
+		devm_kcalloc(dev, display->num_leds, sizeof(*display->leds), GFP_KERNEL);

Wrong wrapping. Use kernel style, not clang style.

Quoted 4 lines

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+	if (!display->leds)
+		return -ENOMEM;

Just wondering how .clang-format is official? Note some of the maintainers even

First time I hear above clang style is preferred. Where is it expected?

I assume clang-format is half-working and should not be used blindly, but fixed to match actual kernel coding style.

Quoted 2 lines

prefer (ugly in some cases in my opinion) style because it's generated by the clang-format.

Quoted 3 lines · Krzysztof Kozlowski

Quoted 2 lines · Andy Shevchenko

Quoted 1 line · Krzysztof Kozlowski

On 29/06/2025 15:18, Jean-François Lessard wrote:

...

Quoted 11 lines

Quoted 10 lines

Quoted 8 lines

drivers/auxdisplay/tm16xx.c line 1082

Quoted 2 lines

drivers/auxdisplay/tm16xx.c new file +2 -0

+	display->leds =
+		devm_kcalloc(dev, display->num_leds, sizeof(*display->leds), GFP_KERNEL);

Wrong wrapping. Use kernel style, not clang style.

Review

Quoted 2 lines

(quoted diff) +2 -0

+	if (!display->leds)
+		return -ENOMEM;

Just wondering how .clang-format is official? Note some of the maintainers even

Quoted 1 line

First time I hear above clang style is preferred. Where is it expected?

Documented here:
https://www.kernel.org/doc/html/latest/process/coding-style.html#you-ve-made-a-mess-of-it

For example, discussed here
https://lore.kernel.org/lkml/CAPcyv4ij3s+9uO0f9aLHGj3=ACG7hAjZ0Rf=tyFmpt3+uQyymw@mail.gmail.com/

or here
https://lore.kernel.org/lkml/64dbeffcf243a_47b5729487@dwillia2-mobl3.amr.corp.intel.com.notmuch/

or
...

Quoted 2 lines

I assume clang-format is half-working and should not be used blindly, but fixed to match actual kernel coding style.

That sounds like the case, at least in accordance with Miguel.

Quoted 2 lines

Quoted 2 lines

prefer (ugly in some cases in my opinion) style because it's generated by the clang-format.

Quoted 4 lines · Andy Shevchenko

Quoted 3 lines · Krzysztof Kozlowski

Quoted 2 lines · Andy Shevchenko

Quoted 1 line · Krzysztof Kozlowski

On 29/06/2025 15:18, Jean-François Lessard wrote:

Quoted 11 lines

Quoted 11 lines

Quoted 10 lines

Quoted 8 lines

drivers/auxdisplay/tm16xx.c line 1082

Quoted 2 lines

drivers/auxdisplay/tm16xx.c new file +2 -0

+	display->leds =
+		devm_kcalloc(dev, display->num_leds, sizeof(*display->leds), GFP_KERNEL);

Wrong wrapping. Use kernel style, not clang style.

Review

Quoted 2 lines

(quoted diff) +2 -0

+	if (!display->leds)
+		return -ENOMEM;

Just wondering how .clang-format is official? Note some of the maintainers even

Quoted 4 lines

Quoted 1 line

First time I hear above clang style is preferred. Where is it expected?

Documented here:
https://www.kernel.org/doc/html/latest/process/coding-style.html#you-ve-made-a-mess-of-it

I mean, which maintainers prefer such style of wrapping. Above I know, but it does not solve the discussion we have here - above line wrapping preferred by clang and opposite to most of the kernel code.

Quoted 2 lines

For example, discussed here
https://lore.kernel.org/lkml/CAPcyv4ij3s+9uO0f9aLHGj3=ACG7hAjZ0Rf=tyFmpt3+uQyymw@mail.gmail.com/

Heh, I read it and two emails earlier and still could not get they prefer to wrap at assignment instead of standard checkpatch-preferred wrapping at arguments.

Quoted 2 lines

or here
https://lore.kernel.org/lkml/64dbeffcf243a_47b5729487@dwillia2-mobl3.amr.corp.intel.com.notmuch/

This is line length, so not the problem discussed here.

Quoted 10 lines

or
...

Quoted 2 lines

I assume clang-format is half-working and should not be used blindly, but fixed to match actual kernel coding style.

That sounds like the case, at least in accordance with Miguel.

Quoted 2 lines

Quoted 2 lines

prefer (ugly in some cases in my opinion) style because it's generated by the clang-format.

Quoted 5 lines · Krzysztof Kozlowski

Quoted 4 lines · Andy Shevchenko

Quoted 3 lines · Krzysztof Kozlowski

Quoted 2 lines · Andy Shevchenko

Quoted 1 line · Krzysztof Kozlowski

On 29/06/2025 15:18, Jean-François Lessard wrote:

...

Quoted 13 lines

Quoted 13 lines

Quoted 12 lines

Quoted 10 lines

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+	display->leds =
+		devm_kcalloc(dev, display->num_leds, sizeof(*display->leds), GFP_KERNEL);

Wrong wrapping. Use kernel style, not clang style.

Review

Quoted 2 lines

(quoted diff) +2 -0

+	if (!display->leds)
+		return -ENOMEM;

Just wondering how .clang-format is official? Note some of the maintainers even

First time I hear above clang style is preferred. Where is it expected?

Quoted 6 lines

Quoted 2 lines

Documented here:
https://www.kernel.org/doc/html/latest/process/coding-style.html#you-ve-made-a-mess-of-it

I mean, which maintainers prefer such style of wrapping. Above I know, but it does not solve the discussion we have here - above line wrapping preferred by clang and opposite to most of the kernel code.

IIRC Dan Williams (as you might have deduced already from the links).

Quoted 6 lines · Andy Shevchenko

Quoted 5 lines · Krzysztof Kozlowski

Quoted 4 lines · Andy Shevchenko

Quoted 3 lines · Krzysztof Kozlowski

Quoted 2 lines · Andy Shevchenko

Quoted 1 line · Krzysztof Kozlowski

On 29/06/2025 15:18, Jean-François Lessard wrote:

Quoted 16 lines

Quoted 16 lines

Quoted 15 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+	display->leds =
+		devm_kcalloc(dev, display->num_leds, sizeof(*display->leds), GFP_KERNEL);

Wrong wrapping. Use kernel style, not clang style.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+	if (!display->leds)
+		return -ENOMEM;

Just wondering how .clang-format is official? Note some of the maintainers even

First time I hear above clang style is preferred. Where is it expected?

Documented here:
https://www.kernel.org/doc/html/latest/process/coding-style.html#you-ve-made-a-mess-of-it

Quoted 5 lines

Quoted 3 lines

I mean, which maintainers prefer such style of wrapping. Above I know, but it does not solve the discussion we have here - above line wrapping preferred by clang and opposite to most of the kernel code.

IIRC Dan Williams (as you might have deduced already from the links).

BTW, if that's your preference, then obviously it is perfectly fine. It's your subsystem.

It's not my preference for the record, but I wanted to know more about an application of the clang-format. And perhaps some docs (besides .clang-format) should be fixed rather sooner?

Quoted 4 lines · Andy Shevchenko

It's not my preference for the record, but I wanted to know more about an application of the clang-format. And perhaps some docs (besides .clang-format) should be fixed rather sooner?

There are some docs at:

https://docs.kernel.org/dev-tools/clang-format.html

You just pasted the same what we talk about. Did you read the thread?

Quoted 1 line

This patch introduces a new auxiliary display driver for TM16XX family LED controllers and compatible chips:

Please do not use "This commit/patch/change", but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

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

Quoted 7 lines

- Shenzhen Titan Micro Electronics: TM1618, TM1620, TM1628, TM1650
- Fuzhou Fuda Hisi Microelectronics: FD620, FD628, FD650, FD655, FD6551
- Wuxi i-Core Electronics: AiP650, AiP1618, AiP1628
- Princeton Technology: PT6964
- Shenzhen Winrise Technology: HBS658

...

Quoted 6 lines

+ * tm16xx_parse_dt - Parse device tree data
+ * @dev: Pointer to device structure
+ * @display: Pointer to tm16xx_display structure
+ *
+ * Return: 0 on success, negative error code on failure
+ */

drivers/auxdisplay/tm16xx.c line 934

Quoted 8 lines

drivers/auxdisplay/tm16xx.c new file +8 -0

+static int tm16xx_parse_dt(struct device *dev, struct tm16xx_display *display)
+{
+	struct fwnode_handle *child;
+	int ret, i, max_grid = 0;
+	u8 *digits;
+
+	display->transpose_display_data =
+		device_property_read_bool(dev, "titanmec,transposed");

Wrong wrapping.

Quoted 142 lines

drivers/auxdisplay/tm16xx.c new file +142 -0

+
+	ret = device_property_count_u8(dev, "titanmec,digits");
+	if (ret < 0) {
+		dev_err(dev, "Failed to count 'titanmec,digits' property: %d\n", ret);
+		return ret;
+	}
+
+	display->num_digits = ret;
+	dev_dbg(dev, "Number of digits: %d\n", display->num_digits);
+
+	digits = devm_kcalloc(dev, display->num_digits, sizeof(*digits), GFP_KERNEL);
+	if (!digits)
+		return -ENOMEM;
+
+	ret = device_property_read_u8_array(dev, "titanmec,digits", digits,
+					    display->num_digits);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read 'titanmec,digits' property: %d\n", ret);
+		return ret;
+	}
+
+	display->digits = devm_kcalloc(dev, display->num_digits, sizeof(*display->digits),
+				       GFP_KERNEL);
+	if (!display->digits)
+		return -ENOMEM;
+
+	for (i = 0; i < display->num_digits; i++) {
+		if (digits[i] >= display->controller->max_grids) {
+			dev_err(dev, "Digit grid %d exceeds controller max_grids %d\n",
+				digits[i], display->controller->max_grids);
+			return -EINVAL;
+		}
+
+		display->digits[i].grid = digits[i];
+		max_grid = umax(max_grid, digits[i]);
+	}
+
+	devm_kfree(dev, digits);
+
+	ret = device_property_read_u8_array(dev, "titanmec,segment-mapping",
+					    display->segment_mapping, DIGIT_SEGMENTS);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read 'titanmec,segment-mapping' property: %d\n",
+			ret);
+		return ret;
+	}
+
+	display->digit_bitmask = 0;
+	for (i = 0; i < DIGIT_SEGMENTS; i++) {
+		if (display->segment_mapping[i] < MIN_SEGMENT ||
+		    display->segment_mapping[i] > MAX_SEGMENT) {
+			dev_err(dev,
+				"Invalid 'titanmec,segment-mapping' value: %d (must be between %d and %d)\n",
+				display->segment_mapping[i], MIN_SEGMENT, MAX_SEGMENT);
+			return -EINVAL;
+		}
+
+		display->digit_bitmask |= BIT(display->segment_mapping[i]);
+	}
+
+	display->num_leds = 0;
+	device_for_each_child_node(dev, child) {
+		u32 reg[2];
+
+		ret = fwnode_property_read_u32_array(child, "reg", reg, 2);
+		if (ret < 0) {
+			dev_err(dev, "Failed to read 'reg' property of led node: %d\n",
+				ret);
+			fwnode_handle_put(child);
+			return ret;
+		}
+
+		if (reg[0] >= display->controller->max_grids) {
+			dev_err(dev, "LED grid %d exceeds controller max_grids %d\n",
+				reg[0], display->controller->max_grids);
+			fwnode_handle_put(child);
+			return -EINVAL;
+		}
+
+		if (reg[1] < MIN_SEGMENT || reg[1] > MAX_SEGMENT) {
+			dev_err(dev,
+				"LED segment %d is invalid (must be between %d and %d)\n",
+				reg[1], MIN_SEGMENT, MAX_SEGMENT);
+			fwnode_handle_put(child);
+			return -EINVAL;
+		}
+
+		max_grid = umax(max_grid, reg[0]);
+		display->num_leds++;
+	}
+
+	dev_dbg(dev, "Number of LEDs: %d\n", display->num_leds);
+
+	display->display_data_len = max_grid + 1;
+	dev_dbg(dev, "Number of display grids: %zu\n", display->display_data_len);
+
+	display->display_data = devm_kcalloc(dev, display->display_data_len,
+					     sizeof(*display->display_data), GFP_KERNEL);
+	if (!display->display_data)
+		return -ENOMEM;
+
+	return 0;
+}
+
+/**
+ * tm16xx_probe - Probe function for tm16xx devices
+ * @display: Pointer to tm16xx_display structure
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int tm16xx_probe(struct tm16xx_display *display)
+{
+	struct device *dev = display->dev;
+	struct fwnode_handle *child;
+	int ret, i;
+
+	ret = tm16xx_parse_dt(dev, display);
+	if (ret < 0) {
+		dev_err(dev, "Failed to parse device tree: %d\n", ret);
+		return ret;
+	}
+
+	mutex_init(&display->lock);
+	INIT_WORK(&display->flush_init, tm16xx_display_flush_init);
+
+	/* Initialize work structure with appropriate flush function */
+	if (display->transpose_display_data) {
+		INIT_WORK(&display->flush_display, tm16xx_display_flush_data_transposed);
+		dev_info(display->dev, "Operating in transposed mode\n");
+	} else {
+		INIT_WORK(&display->flush_display, tm16xx_display_flush_data);
+	}
+
+	display->main_led.name = TM16XX_DEVICE_NAME;
+	display->main_led.brightness = display->controller->max_brightness;
+	display->main_led.max_brightness = display->controller->max_brightness;
+	display->main_led.brightness_set = tm16xx_brightness_set;
+	display->main_led.groups = tm16xx_main_led_groups;
+	display->main_led.flags = LED_RETAIN_AT_SHUTDOWN | LED_CORE_SUSPENDRESUME;
+
+	ret = devm_led_classdev_register(dev, &display->main_led);
+	if (ret < 0) {

drivers/auxdisplay/tm16xx.c line 1082

Quoted 6 lines

drivers/auxdisplay/tm16xx.c new file +6 -0

+		dev_err(dev, "Failed to register main LED: %d\n", ret);
+		return ret;
+	}
+
+	display->leds =
+		devm_kcalloc(dev, display->num_leds, sizeof(*display->leds), GFP_KERNEL);

Wrong wrapping. Use kernel style, not clang style.

Quoted 40 lines

drivers/auxdisplay/tm16xx.c new file +39 -0

+	if (!display->leds)
+		return -ENOMEM;
+
+	i = 0;
+	device_for_each_child_node(dev, child) {
+		struct tm16xx_led *led = &display->leds[i];
+		struct led_init_data led_init = {
+			.fwnode = child,
+			.devicename = display->main_led.name,
+			.devname_mandatory = true,
+		};
+		u32 reg[2];
+
+		ret = fwnode_property_read_u32_array(child, "reg", reg, 2);
+		if (ret < 0) {
+			fwnode_handle_put(child);
+			dev_err(dev, "Failed to read LED reg property: %d\n", ret);
+			return ret;
+		}
+
+		led->grid = reg[0];
+		led->segment = reg[1];
+
+		led->cdev.max_brightness = 1;
+		led->cdev.brightness_set = tm16xx_led_set;
+		led->cdev.flags = LED_RETAIN_AT_SHUTDOWN | LED_CORE_SUSPENDRESUME;
+
+		ret = devm_led_classdev_register_ext(dev, &led->cdev, &led_init);
+		if (ret < 0) {
+			fwnode_handle_put(child);
+			dev_err(dev, "Failed to register LED %s: %d\n", led->cdev.name,
+				ret);
+			return ret;
+		}
+
+		i++;
+	}
+
+	ret = tm16xx_display_init(display);

drivers/auxdisplay/tm16xx.c line 1127

Quoted 6 lines

drivers/auxdisplay/tm16xx.c new file +6 -0

+	if (ret < 0) {
+		dev_err(display->dev, "Failed to initialize display: %d\n", ret);
+		return ret;
+	}
+
+	dev_info(display->dev, "Display initialized successfully\n");

Drop, drivers should be silent on success. See coding style.

Quoted 29 lines

drivers/auxdisplay/tm16xx.c new file +29 -0

+	return 0;
+}
+
+/* SPI specific code */
+static int tm16xx_spi_probe(struct spi_device *spi)
+{
+	const struct tm16xx_controller *controller;
+	struct tm16xx_display *display;
+	int ret;
+
+	controller = of_device_get_match_data(&spi->dev);
+	if (!controller)
+		return -EINVAL;
+
+	display = devm_kzalloc(&spi->dev, sizeof(*display), GFP_KERNEL);
+	if (!display)
+		return -ENOMEM;
+
+	display->client.spi = spi;
+	display->dev = &spi->dev;
+	display->controller = controller;
+
+	spi_set_drvdata(spi, display);
+
+	ret = tm16xx_probe(display);
+	if (ret)
+		return ret;
+
+	return 0;

drivers/auxdisplay/tm16xx.c line 1166

Quoted 10 lines

drivers/auxdisplay/tm16xx.c new file +10 -0

+}
+
+static void tm16xx_spi_remove(struct spi_device *spi)
+{
+	struct tm16xx_display *display = spi_get_drvdata(spi);
+
+	tm16xx_display_remove(display);
+}
+
+// clang-format off

Drop

Quoted 18 lines

drivers/auxdisplay/tm16xx.c new file +18 -0

+static const struct of_device_id tm16xx_spi_of_match[] = {
+	{ .compatible = "titanmec,tm1618", .data = &tm1618_controller },
+	{ .compatible = "titanmec,tm1620", .data = &tm1628_controller },
+	{ .compatible = "titanmec,tm1628", .data = &tm1628_controller },
+	{ .compatible = "fdhisi,fd620", .data = &tm1628_controller },
+	{ .compatible = "fdhisi,fd628", .data = &tm1628_controller },
+	{ .compatible = "icore,aip1618", .data = &tm1618_controller },
+	{ .compatible = "icore,aip1628", .data = &tm1628_controller },
+	{ .compatible = "princeton,pt6964", .data = &tm1628_controller },
+	{ .compatible = "winrise,hbs658", .data = &hbs658_controller },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tm16xx_spi_of_match);
+
+static const struct spi_device_id tm16xx_spi_id[] = {
+	{ "tm1618", 0 },
+	{ "tm1620", 0 },
+	{ "tm1628", 0 },

drivers/auxdisplay/tm16xx.c line 1194

Quoted 10 lines

drivers/auxdisplay/tm16xx.c new file +10 -0

+	{ "fd620", 0 },
+	{ "fd628", 0 },
+	{ "aip1618", 0 },
+	{ "aip1628", 0 },
+	{ "pt6964", 0 },
+	{ "hbs658", 0 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, tm16xx_spi_id);
+// clang-format on

Drop

Quoted 38 lines

drivers/auxdisplay/tm16xx.c new file +38 -0

+
+static struct spi_driver tm16xx_spi_driver = {
+	.driver = {
+		.name = TM16XX_DRIVER_NAME,
+		.of_match_table = tm16xx_spi_of_match,
+	},
+	.probe = tm16xx_spi_probe,
+	.remove = tm16xx_spi_remove,
+	.shutdown = tm16xx_spi_remove,
+	.id_table = tm16xx_spi_id,
+};
+
+/* I2C specific code */
+static int tm16xx_i2c_probe(struct i2c_client *client)
+{
+	const struct tm16xx_controller *controller;
+	struct tm16xx_display *display;
+	int ret;
+
+	controller = of_device_get_match_data(&client->dev);
+	if (!controller)
+		return -EINVAL;
+
+	display = devm_kzalloc(&client->dev, sizeof(*display), GFP_KERNEL);
+	if (!display)
+		return -ENOMEM;
+
+	display->client.i2c = client;
+	display->dev = &client->dev;
+	display->controller = controller;
+
+	i2c_set_clientdata(client, display);
+
+	ret = tm16xx_probe(display);
+	if (ret)
+		return ret;
+
+	return 0;

drivers/auxdisplay/tm16xx.c line 1242

Quoted 10 lines

drivers/auxdisplay/tm16xx.c new file +10 -0

+}
+
+static void tm16xx_i2c_remove(struct i2c_client *client)
+{
+	struct tm16xx_display *display = i2c_get_clientdata(client);
+
+	tm16xx_display_remove(display);
+}
+
+// clang-format off

Drop

Quoted 10 lines

drivers/auxdisplay/tm16xx.c new file +10 -0

+static const struct of_device_id tm16xx_i2c_of_match[] = {
+	{ .compatible = "titanmec,tm1650", .data = &tm1650_controller },
+	{ .compatible = "icore,aip650", .data = &tm1650_controller },
+	{ .compatible = "fdhisi,fd650", .data = &tm1650_controller },
+	{ .compatible = "fdhisi,fd6551", .data = &fd6551_controller },
+	{ .compatible = "fdhisi,fd655", .data = &fd655_controller },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tm16xx_i2c_of_match);
+

drivers/auxdisplay/tm16xx.c line 1262

Quoted 10 lines

drivers/auxdisplay/tm16xx.c new file +10 -0

+static const struct i2c_device_id tm16xx_i2c_id[] = {
+	{ "tm1650", 0 },
+	{ "aip650", 0 },
+	{ "fd650", 0 },
+	{ "fd6551", 0 },
+	{ "fd655", 0 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, tm16xx_i2c_id);
+// clang-format on

Drop.

Quoted 37 lines

drivers/auxdisplay/tm16xx.c new file +37 -0

+
+static struct i2c_driver tm16xx_i2c_driver = {
+	.driver = {
+		.name = TM16XX_DRIVER_NAME,
+		.of_match_table = tm16xx_i2c_of_match,
+	},
+	.probe = tm16xx_i2c_probe,
+	.remove = tm16xx_i2c_remove,
+	.shutdown = tm16xx_i2c_remove,
+	.id_table = tm16xx_i2c_id,
+};
+
+static int __init tm16xx_init(void)
+{
+	int ret;
+
+	ret = spi_register_driver(&tm16xx_spi_driver);
+	if (ret)
+		return ret;
+
+	ret = i2c_add_driver(&tm16xx_i2c_driver);
+	if (ret) {
+		spi_unregister_driver(&tm16xx_spi_driver);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void __exit tm16xx_exit(void)
+{
+	i2c_del_driver(&tm16xx_i2c_driver);
+	spi_unregister_driver(&tm16xx_spi_driver);
+}
+
+module_init(tm16xx_init);
+module_exit(tm16xx_exit);

drivers/auxdisplay/tm16xx.c line 1305

Quoted 6 lines

drivers/auxdisplay/tm16xx.c new file +6 -0

+
+MODULE_AUTHOR("Jean-François Lessard");
+MODULE_DESCRIPTION("TM16XX Compatible LED Display Controllers");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:tm16xx");
+MODULE_ALIAS("i2c:tm16xx");

Drop these two.

Best regards,
Krzysztof