linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver
@ 2025-06-28 16:18 Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 1/8] dt-bindings: vendor-prefixes: Add Fuda Hisi Microelectronics Jean-François Lessard
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Jean-François Lessard @ 2025-06-28 16:18 UTC (permalink / raw)
  To: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Geert Uytterhoeven, devicetree, linux-leds, linux-kernel,
	Andreas Färber, Boris Gjenero, Christian Hewitt,
	Heiner Kallweit, Paolo Sabatino

This patch series introduces a new auxiliary display driver for the TM16xx family of LED controllers and compatible chips, widely used in TV boxes and embedded devices.

Many consumer devices, particularly TV boxes, use auxiliary displays based on TM16xx LED controllers to show status information such as time, network connectivity, and system state. Currently, there is no mainline kernel support for these displays, forcing users to rely on out-of-tree drivers or userspace solutions that directly manipulate GPIO pins.

This driver provides a unified interface for TM16xx-based auxiliary displays through the Linux LED subsystem. It supports both I2C and SPI communication protocols and integrates with the existing LED class framework, allowing displays to be controlled via standard sysfs interfaces and LED triggers.

Upstreaming this driver will eliminate reliance on out-of-tree drivers and enable standardized auxiliary display support across devices using these controllers.

It is compatible with multiple LED controller families:
- Titan Micro Electronics: TM1618, TM1620, TM1628, TM1650
- FUDA HISI Microelectronics: FD620, FD628, FD650, FD655, FD6551
- i-Core Electronics: AiP650, AiP1618, AiP1628
- Princeton Technology: PT6964
- Winrise Technology: HBS658

Key features:
- Write-only display support: This initial submission implements display output functionality. Most devices do not wire key scanning lines for input, so key input is left for potential future extensions if needed.
- 7-segment display support: Full integration with kernel segment mapping helpers for driving standard 7-segment digit displays.
- Flexible display configuration: Device tree bindings allow board-specific configuration of digit grids, segment mappings, and matrix orientation to accommodate different PCB layouts and wiring designs.
- LED subsystem integration: Individual display elements (icons) are exposed as LED devices, enabling use of LED triggers for automatic control based on system events (network activity, USB connections, etc.).
- Dual protocol support: Supports both I2C and SPI communication, with the protocol selected based on device tree configuration.

The device tree bindings provide properties to describe board-specific wiring and display layout, as the controller itself is agnostic to the display configuration:
- tm16xx,digits: Array defining which controller grids drive digit displays.
- tm16xx,segment-mapping: Mapping of 7-segment display elements to controller outputs.
- tm16xx,transposed: Flag for displays with swapped grid/segment orientation.
- Individual LED definitions for icons and status indicators.

Tested platforms:
- Multiple TV boxes with Amlogic, Rockchip and Allwinner SoCs.
- Various display configurations and controller variants.
- Both I2C and SPI communication modes.
- LED trigger integration for automatic status indication.

Dependencies:
- Requires CONFIG_NEW_LEDS=y and CONFIG_LEDS_CLASS=y

Optional LED trigger modules for advanced functionality:
- CONFIG_LEDS_TRIGGER_TIMER for blinking elements.
- CONFIG_LEDS_TRIGGER_NETDEV for network activity indication.
- CONFIG_USB_LEDS_TRIGGER_USBPORT for USB activity indication.

User space clients, including display-service and display-utils for testing and integration, are available at: https://github.com/jefflessard/tm16xx-display

Andreas Färber (2):
  dt-bindings: vendor-prefixes: Add Fuda Hisi Microelectronics
  dt-bindings: vendor-prefixes: Add Titan Micro Electronics

Jean-François Lessard (6):
  dt-bindings: vendor-prefixes: Add Princeton Technology Corp
  dt-bindings: vendor-prefixes: Add Winrise Technology
  dt-bindings: vendor-prefixes: Add Wuxi i-Core Electronics
  dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX
  auxdisplay: Add support for Titanmec TM16xx 7 segment display
    controller
  MAINTAINERS: Add entry for TM16xx driver

 .../bindings/auxdisplay/tm16xx.yaml           |  153 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   10 +
 MAINTAINERS                                   |    6 +
 drivers/auxdisplay/Kconfig                    |   18 +
 drivers/auxdisplay/Makefile                   |    1 +
 drivers/auxdisplay/tm16xx.c                   | 1305 +++++++++++++++++
 6 files changed, 1493 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/tm16xx.yaml
 create mode 100644 drivers/auxdisplay/tm16xx.c

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/8] dt-bindings: vendor-prefixes: Add Fuda Hisi Microelectronics
  2025-06-28 16:18 [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
@ 2025-06-28 16:18 ` Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 2/8] dt-bindings: vendor-prefixes: Add Titan Micro Electronics Jean-François Lessard
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jean-François Lessard @ 2025-06-28 16:18 UTC (permalink / raw)
  To: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Geert Uytterhoeven, devicetree, linux-leds, linux-kernel,
	Andreas Färber, Boris Gjenero, Christian Hewitt,
	Heiner Kallweit, Paolo Sabatino, sales

From: Andreas Färber <afaerber@suse.de>

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

Cc: sales@fdhisi.com
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 5d2a7a8d3a..9e4d251e71 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -531,6 +531,8 @@ patternProperties:
     description: Fastrax Oy
   "^fcs,.*":
     description: Fairchild Semiconductor
+  "^fdhisi,.*":
+    description: Fuzhou Fuda Hisi Microelectronics Co., Ltd.
   "^feixin,.*":
     description: Shenzhen Feixin Photoelectic Co., Ltd
   "^feiyang,.*":
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/8] dt-bindings: vendor-prefixes: Add Titan Micro Electronics
  2025-06-28 16:18 [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 1/8] dt-bindings: vendor-prefixes: Add Fuda Hisi Microelectronics Jean-François Lessard
@ 2025-06-28 16:18 ` Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 3/8] dt-bindings: vendor-prefixes: Add Princeton Technology Corp Jean-François Lessard
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jean-François Lessard @ 2025-06-28 16:18 UTC (permalink / raw)
  To: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Geert Uytterhoeven, devicetree, linux-leds, linux-kernel,
	Andreas Färber, Boris Gjenero, Christian Hewitt,
	Heiner Kallweit, Paolo Sabatino, zypeng

From: Andreas Färber <afaerber@suse.de>

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

Cc: zypeng@titanmec.com
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 9e4d251e71..d02615496b 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1550,6 +1550,8 @@ patternProperties:
     description: Texas Instruments
   "^tianma,.*":
     description: Tianma Micro-electronics Co., Ltd.
+  "^titanmec,.*":
+    description: Shenzhen Titan Micro Electronics Co., Ltd.
   "^tlm,.*":
     description: Trusted Logic Mobility
   "^tmt,.*":
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/8] dt-bindings: vendor-prefixes: Add Princeton Technology Corp
  2025-06-28 16:18 [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 1/8] dt-bindings: vendor-prefixes: Add Fuda Hisi Microelectronics Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 2/8] dt-bindings: vendor-prefixes: Add Titan Micro Electronics Jean-François Lessard
@ 2025-06-28 16:18 ` Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 4/8] dt-bindings: vendor-prefixes: Add Winrise Technology Jean-François Lessard
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jean-François Lessard @ 2025-06-28 16:18 UTC (permalink / raw)
  To: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Geert Uytterhoeven, devicetree, linux-leds, linux-kernel,
	Andreas Färber, Boris Gjenero, Christian Hewitt,
	Heiner Kallweit, Paolo Sabatino

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

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index d02615496b..f03ab02afe 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1218,6 +1218,8 @@ patternProperties:
     description: Prime View International (PVI)
   "^primux,.*":
     description: Primux Trading, S.L.
+  "^princeton,.*":
+    description: Princeton Technology Corp.
   "^probox2,.*":
     description: PROBOX2 (by W2COMP Co., Ltd.)
   "^pri,.*":
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/8] dt-bindings: vendor-prefixes: Add Winrise Technology
  2025-06-28 16:18 [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
                   ` (2 preceding siblings ...)
  2025-06-28 16:18 ` [PATCH 3/8] dt-bindings: vendor-prefixes: Add Princeton Technology Corp Jean-François Lessard
@ 2025-06-28 16:18 ` Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 5/8] dt-bindings: vendor-prefixes: Add Wuxi i-Core Electronics Jean-François Lessard
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jean-François Lessard @ 2025-06-28 16:18 UTC (permalink / raw)
  To: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Geert Uytterhoeven, devicetree, linux-leds, linux-kernel,
	Andreas Färber, Boris Gjenero, Christian Hewitt,
	Heiner Kallweit, Paolo Sabatino

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

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index f03ab02afe..a3bf93e5dc 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1711,6 +1711,8 @@ patternProperties:
     description: Wingtech Technology Co., Ltd.
   "^winlink,.*":
     description: WinLink Co., Ltd
+  "^winrise,.*":
+    description: Shenzhen Winrise Technology Co., Ltd.
   "^winsen,.*":
     description: Winsen Corp.
   "^winstar,.*":
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/8] dt-bindings: vendor-prefixes: Add Wuxi i-Core Electronics
  2025-06-28 16:18 [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
                   ` (3 preceding siblings ...)
  2025-06-28 16:18 ` [PATCH 4/8] dt-bindings: vendor-prefixes: Add Winrise Technology Jean-François Lessard
@ 2025-06-28 16:18 ` Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX Jean-François Lessard
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jean-François Lessard @ 2025-06-28 16:18 UTC (permalink / raw)
  To: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Geert Uytterhoeven, devicetree, linux-leds, linux-kernel,
	Andreas Färber, Boris Gjenero, Christian Hewitt,
	Heiner Kallweit, Paolo Sabatino

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

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index a3bf93e5dc..def3530099 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -694,6 +694,8 @@ patternProperties:
     description: International Business Machines (IBM)
   "^ibm,.*":
     description: International Business Machines (IBM)
+  "^icore,.*":
+    description: Wuxi i-Core Electronics Co., Ltd.
   "^icplus,.*":
     description: IC Plus Corp.
   "^idt,.*":
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX
  2025-06-28 16:18 [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
                   ` (4 preceding siblings ...)
  2025-06-28 16:18 ` [PATCH 5/8] dt-bindings: vendor-prefixes: Add Wuxi i-Core Electronics Jean-François Lessard
@ 2025-06-28 16:18 ` Jean-François Lessard
  2025-06-28 17:55   ` Rob Herring (Arm)
  2025-06-28 16:18 ` [PATCH 7/8] auxdisplay: Add support for Titanmec TM16xx 7 segment display controller Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 8/8] MAINTAINERS: Add entry for TM16xx driver Jean-François Lessard
  7 siblings, 1 reply; 11+ messages in thread
From: Jean-François Lessard @ 2025-06-28 16:18 UTC (permalink / raw)
  To: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Geert Uytterhoeven, devicetree, linux-leds, linux-kernel,
	Andreas Färber, Boris Gjenero, Christian Hewitt,
	Heiner Kallweit, Paolo Sabatino

Add documentation for 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 <afaerber@suse.de>
Co-developed-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 .../bindings/auxdisplay/tm16xx.yaml           | 153 ++++++++++++++++++
 1 file changed, 153 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/tm16xx.yaml

diff --git a/Documentation/devicetree/bindings/auxdisplay/tm16xx.yaml b/Documentation/devicetree/bindings/auxdisplay/tm16xx.yaml
new file mode 100644
index 0000000000..eba3d3f3f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/tm16xx.yaml
@@ -0,0 +1,153 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/auxdisplay/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@gmail.com>
+
+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
+  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
+      - icore,aip650
+      - icore,aip1618
+      - icore,aip1628
+      - princeton,pt6964
+      - winrise,hbs658
+
+  reg:
+    maxItems: 1
+
+  tm16xx,digits:
+    description: |
+      Array of grid (row) indexes corresponding to specific wiring of digits in the display matrix.
+      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
+
+  tm16xx,segment-mapping:
+    description: |
+      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).
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    items:
+      minimum: 0
+      maximum: 7
+    minItems: 7
+    maxItems: 7
+
+  tm16xx,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]$":
+    $ref: /schemas/leds/common.yaml#
+    properties:
+      reg:
+        description: Grid (row) and segment (column) index in the matrix of this individual LED icon
+    required:
+      - reg
+
+required:
+  - compatible
+  - reg
+  - tm16xx,digits
+  - tm16xx,segment-mapping
+
+additionalProperties: true
+
+examples:
+  - |
+    display_client: i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        display@24 {
+            compatible = "titanmec,tm1650";
+            reg = <0x24>;
+            tm16xx,digits = /bits/ 8 <0 1 2 3>;
+            tm16xx,segment-mapping = /bits/ 8 <0 1 2 3 4 5 6>;
+
+            #address-cells = <2>;
+            #size-cells = <0>;
+
+            led@4,0 {
+                reg = <4 0>;
+                function = "lan";
+            };
+
+            led@4,1 {
+                reg = <4 1>;
+                function = "wlan";
+            };
+        };
+    };
+  - |
+    display_client: spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        display@0 {
+            compatible = "titanmec,tm1628";
+            reg = <0>;
+            tm16xx,transposed;
+            tm16xx,digits = /bits/ 8 <1 2 3 4>;
+            tm16xx,segment-mapping = /bits/ 8 <0 1 2 3 4 5 6>;
+
+            #address-cells = <2>;
+            #size-cells = <0>;
+
+            led@0,2 {
+                reg = <0 2>;
+                function = "usb";
+            };
+
+            led@0,3 {
+                reg = <0 3>;
+                function = "power";
+            };
+        };
+    };
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/8] auxdisplay: Add support for Titanmec TM16xx 7 segment display controller
  2025-06-28 16:18 [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
                   ` (5 preceding siblings ...)
  2025-06-28 16:18 ` [PATCH 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX Jean-François Lessard
@ 2025-06-28 16:18 ` Jean-François Lessard
  2025-06-28 16:18 ` [PATCH 8/8] MAINTAINERS: Add entry for TM16xx driver Jean-François Lessard
  7 siblings, 0 replies; 11+ messages in thread
From: Jean-François Lessard @ 2025-06-28 16:18 UTC (permalink / raw)
  To: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Geert Uytterhoeven, devicetree, linux-leds, linux-kernel,
	Andreas Färber, Boris Gjenero, Christian Hewitt,
	Heiner Kallweit, Paolo Sabatino

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

Known to be successfully tested on:
- H96 Max (I2C, rockchip XY_RK3328)
- Magicsee N5 (I2C, amlogic s905x)
- Tanix TX3 Mini (SPI, amlogic s905w)
- Tanix TX6 (I2C, allwinner h6)
- X92 (SPI)
- X96 Max (SPI, amlogic s905x2)

Acked-by: Paolo Sabatino <paolo.sabatino@gmail.com> # As primary user, integrated tm16xx into Armbian rockchip64
Acked-by: Christian Hewitt <christianshewitt@gmail.com> # As primary user, integrated tm16xx into LibreElec
Tested-by: Boris Gjenero <boris.gjenero@gmail.com> # Tested on X92 (transposed)
Tested-by: Paolo Sabatino <paolo.sabatino@gmail.com> # Tested on H96 Max (XY_RK3328)
Tested-by: Christian Hewitt <christianshewitt@gmail.com> # Tested on X96 Max, Tanix TX3 Mini
Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 drivers/auxdisplay/Kconfig  |   18 +
 drivers/auxdisplay/Makefile |    1 +
 drivers/auxdisplay/tm16xx.c | 1305 +++++++++++++++++++++++++++++++++++
 3 files changed, 1324 insertions(+)
 create mode 100644 drivers/auxdisplay/tm16xx.c

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index bedc6133f9..0195db15f6 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -526,6 +526,24 @@ config SEG_LED_GPIO
 	  This driver can also be built as a module. If so, the module
 	  will be called seg-led-gpio.
 
+config TM16XX
+	tristate "TM16XX-compatible LED controllers display"
+	depends on SPI || I2C
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	help
+	  This driver supports the following TM16XX compatible
+	  (I2C and SPI 7-Segment LED display controller) chips:
+	  - Titanmec: TM1618, TM1620, TM1628, TM1650
+	  - Fuda Hisi: FD620, FD628, FD650, FD655, FD6551
+	  - i-Core: AiP650, AiP1618, AiP1628
+	  - Princeton: PT6964
+	  - Winrise: HBS658
+
+	  This driver can also be built as a module. If so, the module
+	  will be called tm16xx.
+
 #
 # Character LCD with non-conforming interface section
 #
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index f5c13ed1cd..a2444cfb7a 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_LINEDISP)		+= line-display.o
 obj-$(CONFIG_MAX6959)		+= max6959.o
 obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
 obj-$(CONFIG_SEG_LED_GPIO)	+= seg-led-gpio.o
+obj-$(CONFIG_TM16XX)		+= tm16xx.o
diff --git a/drivers/auxdisplay/tm16xx.c b/drivers/auxdisplay/tm16xx.c
new file mode 100644
index 0000000000..6099686531
--- /dev/null
+++ b/drivers/auxdisplay/tm16xx.c
@@ -0,0 +1,1305 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Auxiliary Display Driver for TM16XX and compatible LED controllers
+ *
+ * Copyright (C) 2024 Jean-François Lessard
+ *
+ * This driver supports various LED controller chips, including TM16XX family,
+ * FD6XX family, PT6964, and HBS658. It provides support for both I2C and SPI interfaces.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/spi/spi.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/leds.h>
+#include <linux/delay.h>
+#include <linux/workqueue.h>
+#include <linux/map_to_7segment.h>
+
+#define TM16XX_DRIVER_NAME "tm16xx"
+#define TM16XX_DEVICE_NAME "display"
+#define DIGIT_SEGMENTS 7
+#define MIN_SEGMENT 0
+#define MAX_SEGMENT 7 /* data stored as 8 bits (u8) */
+
+/* Common bit field definitions */
+// clang-format off
+/* Command type bits (bits 7-6) */
+#define TM16XX_CMD_TYPE_MASK    GENMASK(7, 6)
+#define TM16XX_CMD_MODE         0
+#define TM16XX_CMD_DATA         BIT(6)
+#define TM16XX_CMD_CTRL         BIT(7)
+#define TM16XX_CMD_ADDR         (BIT(7) | BIT(6))
+
+/* Mode command grid settings (bits 1-0) */
+#define TM16XX_MODE_GRID_MASK   GENMASK(1, 0)
+#define TM16XX_MODE_4GRIDS      0
+#define TM16XX_MODE_5GRIDS      BIT(0)
+#define TM16XX_MODE_6GRIDS      BIT(1)
+#define TM16XX_MODE_7GRIDS      (BIT(1) | BIT(0))
+
+/* Data command settings */
+#define TM16XX_DATA_ADDR_MASK   BIT(2)
+#define TM16XX_DATA_ADDR_AUTO   0
+#define TM16XX_DATA_ADDR_FIXED  BIT(2)
+#define TM16XX_DATA_MODE_MASK   GENMASK(1, 0)
+#define TM16XX_DATA_WRITE       0
+#define TM16XX_DATA_READ        BIT(1)
+
+/* Control command settings */
+#define TM16XX_CTRL_ON          BIT(3)
+#define TM16XX_CTRL_BR_MASK     GENMASK(2, 0)
+
+/* TM1618 specific constants */
+#define TM1618_BYTE1_MASK       GENMASK(4, 0)
+#define TM1618_BYTE1_RSHIFT     0
+#define TM1618_BYTE2_MASK       (~TM1618_BYTE1_MASK)
+#define TM1618_BYTE2_RSHIFT     2
+
+/* I2C controller addresses and control settings */
+#define TM1650_CMD_CTRL         0x48
+#define TM1650_CMD_ADDR         0x68
+#define TM1650_CTRL_BR_MASK     GENMASK(6, 4)
+#define TM1650_CTRL_ON          BIT(0)
+#define TM1650_CTRL_SEG_MASK    BIT(3)
+#define TM1650_CTRL_SEG8_MODE   0
+#define TM1650_CTRL_SEG7_MODE   BIT(3)
+
+#define FD655_CMD_CTRL          0x48
+#define FD655_CMD_ADDR          0x66
+#define FD655_CTRL_BR_MASK      GENMASK(6, 5)
+#define FD655_CTRL_ON           BIT(0)
+
+#define FD6551_CMD_CTRL         0x48
+#define FD6551_CTRL_BR_MASK     GENMASK(3, 1)
+#define FD6551_CTRL_ON          BIT(0)
+
+#define TM16XX_CTRL_BRIGHTNESS(enabled, value, prefix) \
+	((enabled) ? \
+	 (FIELD_PREP(prefix##_CTRL_BR_MASK, (value)) | prefix##_CTRL_ON) : \
+	 0)
+// clang-format on
+
+static char *default_value;
+module_param(default_value, charp, 0644);
+MODULE_PARM_DESC(default_value, "Default display value to initialize");
+
+static SEG7_CONVERSION_MAP(map_seg7, MAP_ASCII7SEG_ALPHANUM);
+
+/* Forward declarations */
+struct tm16xx_display;
+
+/**
+ * DOC: struct tm16xx_controller - Controller-specific operations
+ * @max_grids: Maximum number of grids supported by the controller
+ * @max_brightness: Maximum brightness level supported by the controller
+ * @init: Configures the controller mode and brightness
+ * @data: Writes display data to the controller
+ *
+ * This structure holds function pointers for controller-specific operations.
+ */
+struct tm16xx_controller {
+	const u8 max_grids;
+	const u8 max_brightness;
+	int (* const init)(struct tm16xx_display *display);
+	int (* const data)(struct tm16xx_display *display, u8 index, u8 data);
+};
+
+/**
+ * struct tm16xx_led - LED information
+ * @cdev: LED class device
+ * @grid: Grid index of the LED
+ * @segment: Segment index of the LED
+ */
+struct tm16xx_led {
+	struct led_classdev cdev;
+	u8 grid;
+	u8 segment;
+};
+
+/**
+ * struct tm16xx_digit - Digit information
+ * @grid: Grid index of the digit
+ * @value: Current value of the digit
+ */
+struct tm16xx_digit {
+	u8 grid;
+	char value;
+};
+
+/**
+ * struct tm16xx_display - Main driver structure
+ * @dev: Pointer to device structure
+ * @controller: Pointer to controller-specific operations
+ * @client: Union of I2C and SPI client structures
+ * @main_led: LED class device for the entire display
+ * @leds: Array of individual LEDs
+ * @num_leds: Number of LEDs
+ * @digits: Array of digits
+ * @num_digits: Number of digits
+ * @segment_mapping: Segment mapping array
+ * @digit_bitmask: Bitmask for setting digit values
+ * @display_data: Display data buffer
+ * @display_data_len: Length of display data buffer
+ * @lock: Mutex for concurrent access protection
+ * @flush_init: Work structure for brightness update
+ * @flush_display: Work structure for display update
+ * @flush_status: Result of the last flush work
+ * @transpose_display_data: Flag indicating if segments and grids should be transposed when writing data
+ */
+struct tm16xx_display {
+	struct device *dev;
+	const struct tm16xx_controller *controller;
+	union {
+		struct i2c_client *i2c;
+		struct spi_device *spi;
+	} client;
+	struct led_classdev main_led;
+	struct tm16xx_led *leds;
+	int num_leds;
+	struct tm16xx_digit *digits;
+	int num_digits;
+	u8 segment_mapping[DIGIT_SEGMENTS];
+	u8 digit_bitmask;
+	u8 *display_data;
+	size_t display_data_len;
+	struct mutex lock; /* protect shared display state in work functions */
+	struct work_struct flush_init;
+	struct work_struct flush_display;
+	int flush_status;
+	bool transpose_display_data;
+};
+
+/**
+ * tm16xx_i2c_write - Write data to I2C client
+ * @display: Pointer to tm16xx_display structure
+ * @data: Data to write
+ * @len: Length of data
+ *
+ * Return: Number of bytes written or negative error code
+ */
+static int tm16xx_i2c_write(struct tm16xx_display *display, u8 *data, size_t len)
+{
+	dev_dbg(display->dev, "i2c_write %*ph", (char)len, data);
+
+	struct i2c_msg msg = {
+		.addr = data[0] >> 1,
+		.flags = 0,
+		.len = len - 1,
+		.buf = &data[1],
+	};
+	int ret;
+
+	ret = i2c_transfer(display->client.i2c->adapter, &msg, 1);
+	if (ret < 0)
+		return ret;
+
+	return (ret == 1) ? len : -EIO;
+}
+
+/**
+ * tm16xx_spi_write - Write data to SPI client
+ * @display: Pointer to tm16xx_display structure
+ * @data: Data to write
+ * @len: Length of data
+ *
+ * Return: Number of bytes written or negative error code
+ */
+static int tm16xx_spi_write(struct tm16xx_display *display, u8 *data, size_t len)
+{
+	dev_dbg(display->dev, "spi_write %*ph", (char)len, data);
+
+	struct spi_device *spi = display->client.spi;
+
+	return spi_write(spi, data, len);
+}
+
+/* Controller-specific functions */
+static int tm1628_init(struct tm16xx_display *display)
+{
+	const enum led_brightness brightness = display->main_led.brightness;
+	const u8 num_grids = display->transpose_display_data ? DIGIT_SEGMENTS :
+							       display->display_data_len;
+	u8 cmd;
+	int ret;
+
+	/* Set mode command based on grid count */
+	cmd = TM16XX_CMD_MODE;
+	if (num_grids <= 4)
+		cmd |= TM16XX_MODE_4GRIDS;
+	else if (num_grids == 5)
+		cmd |= TM16XX_MODE_5GRIDS;
+	else if (num_grids == 6)
+		cmd |= TM16XX_MODE_6GRIDS;
+	else
+		cmd |= TM16XX_MODE_7GRIDS;
+
+	ret = tm16xx_spi_write(display, &cmd, 1);
+	if (ret < 0)
+		return ret;
+
+	/* Set data command */
+	cmd = TM16XX_CMD_DATA | TM16XX_DATA_ADDR_AUTO | TM16XX_DATA_WRITE;
+	ret = tm16xx_spi_write(display, &cmd, 1);
+	if (ret < 0)
+		return ret;
+
+	/* Set control command with brightness */
+	cmd = TM16XX_CMD_CTRL |
+	      TM16XX_CTRL_BRIGHTNESS(brightness, brightness - 1, TM16XX);
+	ret = tm16xx_spi_write(display, &cmd, 1);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int tm1618_data(struct tm16xx_display *display, u8 index, u8 data)
+{
+	u8 cmds[3];
+
+	cmds[0] = TM16XX_CMD_ADDR + index * 2;
+	cmds[1] = (data & TM1618_BYTE1_MASK) >> TM1618_BYTE1_RSHIFT;
+	cmds[2] = (data & TM1618_BYTE2_MASK) >> TM1618_BYTE2_RSHIFT;
+
+	return tm16xx_spi_write(display, cmds, ARRAY_SIZE(cmds));
+}
+
+static int tm1628_data(struct tm16xx_display *display, u8 index, u8 data)
+{
+	u8 cmds[3];
+
+	cmds[0] = TM16XX_CMD_ADDR + index * 2;
+	cmds[1] = data; // SEG 1 to 8
+	cmds[2] = 0; // SEG 9 to 14
+
+	return tm16xx_spi_write(display, cmds, ARRAY_SIZE(cmds));
+}
+
+static int tm1650_init(struct tm16xx_display *display)
+{
+	u8 cmds[2];
+	const enum led_brightness brightness = display->main_led.brightness;
+
+	cmds[0] = TM1650_CMD_CTRL;
+	cmds[1] = TM16XX_CTRL_BRIGHTNESS(brightness, brightness, TM1650) |
+		  TM1650_CTRL_SEG8_MODE;
+
+	return tm16xx_i2c_write(display, cmds, ARRAY_SIZE(cmds));
+}
+
+static int tm1650_data(struct tm16xx_display *display, u8 index, u8 data)
+{
+	u8 cmds[2];
+
+	cmds[0] = TM1650_CMD_ADDR + index * 2;
+	cmds[1] = data; // SEG 1 to 8
+
+	return tm16xx_i2c_write(display, cmds, ARRAY_SIZE(cmds));
+}
+
+static int fd655_init(struct tm16xx_display *display)
+{
+	u8 cmds[2];
+	const enum led_brightness brightness = display->main_led.brightness;
+
+	cmds[0] = FD655_CMD_CTRL;
+	cmds[1] = TM16XX_CTRL_BRIGHTNESS(brightness, brightness % 3, FD655);
+
+	return tm16xx_i2c_write(display, cmds, ARRAY_SIZE(cmds));
+}
+
+static int fd655_data(struct tm16xx_display *display, u8 index, u8 data)
+{
+	u8 cmds[2];
+
+	cmds[0] = FD655_CMD_ADDR + index * 2;
+	cmds[1] = data; // SEG 1 to 8
+
+	return tm16xx_i2c_write(display, cmds, ARRAY_SIZE(cmds));
+}
+
+static int fd6551_init(struct tm16xx_display *display)
+{
+	u8 cmds[2];
+	const enum led_brightness brightness = display->main_led.brightness;
+
+	cmds[0] = FD6551_CMD_CTRL;
+	cmds[1] = TM16XX_CTRL_BRIGHTNESS(brightness, ~(brightness - 1), FD6551);
+
+	return tm16xx_i2c_write(display, cmds, ARRAY_SIZE(cmds));
+}
+
+static void hbs658_swap_nibbles(u8 *data, size_t len)
+{
+	for (size_t i = 0; i < len; i++)
+		data[i] = (data[i] << 4) | (data[i] >> 4);
+}
+
+static int hbs658_init(struct tm16xx_display *display)
+{
+	const enum led_brightness brightness = display->main_led.brightness;
+	u8 cmd;
+	int ret;
+
+	/* Set data command */
+	cmd = TM16XX_CMD_DATA | TM16XX_DATA_ADDR_AUTO | TM16XX_DATA_WRITE;
+	hbs658_swap_nibbles(&cmd, 1);
+	ret = tm16xx_spi_write(display, &cmd, 1);
+	if (ret < 0)
+		return ret;
+
+	/* Set control command with brightness */
+	cmd = TM16XX_CMD_CTRL |
+	      TM16XX_CTRL_BRIGHTNESS(brightness, brightness - 1, TM16XX);
+	hbs658_swap_nibbles(&cmd, 1);
+	ret = tm16xx_spi_write(display, &cmd, 1);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int hbs658_data(struct tm16xx_display *display, u8 index, u8 data)
+{
+	u8 cmds[2];
+
+	cmds[0] = TM16XX_CMD_ADDR + index * 2;
+	cmds[1] = data;
+
+	hbs658_swap_nibbles(cmds, ARRAY_SIZE(cmds));
+	return tm16xx_spi_write(display, cmds, ARRAY_SIZE(cmds));
+}
+
+/* Controller definitions */
+static const struct tm16xx_controller tm1618_controller = {
+	.max_grids = 7,
+	.max_brightness = 8,
+	.init = tm1628_init,
+	.data = tm1618_data,
+};
+
+static const struct tm16xx_controller tm1628_controller = {
+	.max_grids = 7,
+	.max_brightness = 8,
+	.init = tm1628_init,
+	.data = tm1628_data,
+};
+
+static const struct tm16xx_controller tm1650_controller = {
+	.max_grids = 4,
+	.max_brightness = 8,
+	.init = tm1650_init,
+	.data = tm1650_data,
+};
+
+static const struct tm16xx_controller fd655_controller = {
+	.max_grids = 5,
+	.max_brightness = 3,
+	.init = fd655_init,
+	.data = fd655_data,
+};
+
+static const struct tm16xx_controller fd6551_controller = {
+	.max_grids = 5,
+	.max_brightness = 8,
+	.init = fd6551_init,
+	.data = fd655_data,
+};
+
+static const struct tm16xx_controller hbs658_controller = {
+	.max_grids = 5,
+	.max_brightness = 8,
+	.init = hbs658_init,
+	.data = hbs658_data,
+};
+
+/**
+ * tm16xx_ascii_to_segments - Convert ASCII character to segment pattern
+ * @display: Pointer to tm16xx_display structure
+ * @c: ASCII character to convert
+ *
+ * Return: Segment pattern for the given ASCII character
+ */
+static u8 tm16xx_ascii_to_segments(struct tm16xx_display *display, char c)
+{
+	u8 standard_segments, mapped_segments = 0;
+	int i;
+
+	standard_segments = map_to_seg7(&map_seg7, c);
+
+	for (i = 0; i < DIGIT_SEGMENTS; i++) {
+		if (standard_segments & BIT(i))
+			mapped_segments |= BIT(display->segment_mapping[i]);
+	}
+
+	return mapped_segments;
+}
+
+/**
+ * tm16xx_display_flush_init - Work function to update controller configuration (mode and brightness)
+ * @work: Pointer to work_struct
+ */
+static void tm16xx_display_flush_init(struct work_struct *work)
+{
+	struct tm16xx_display *display =
+		container_of(work, struct tm16xx_display, flush_init);
+	int ret;
+
+	if (display->controller->init) {
+		mutex_lock(&display->lock);
+		ret = display->controller->init(display);
+		display->flush_status = ret;
+		mutex_unlock(&display->lock);
+		if (ret < 0)
+			dev_err(display->dev, "Failed to set brightness: %d\n", ret);
+	}
+}
+
+/**
+ * tm16xx_display_flush_data - Work function to update display data
+ * @work: Pointer to work_struct
+ */
+static void tm16xx_display_flush_data(struct work_struct *work)
+{
+	struct tm16xx_display *display =
+		container_of(work, struct tm16xx_display, flush_display);
+	int i, ret = 0;
+
+	mutex_lock(&display->lock);
+
+	if (display->controller->data) {
+		for (i = 0; i < display->display_data_len; i++) {
+			ret = display->controller->data(display, i,
+							display->display_data[i]);
+			if (ret < 0) {
+				dev_err(display->dev,
+					"Failed to write display data: %d\n", ret);
+				break;
+			}
+		}
+	}
+
+	display->flush_status = ret;
+	mutex_unlock(&display->lock);
+}
+
+/**
+ * tm16xx_display_flush_data_transposed - Transposed flush work function
+ * @work: Pointer to work_struct
+ */
+static void tm16xx_display_flush_data_transposed(struct work_struct *work)
+{
+	struct tm16xx_display *display =
+		container_of(work, struct tm16xx_display, flush_display);
+	int i, j, ret = 0;
+	u8 transposed_data;
+
+	mutex_lock(&display->lock);
+
+	if (display->controller->data) {
+		/* Write operations based on number of segments */
+		for (i = MIN_SEGMENT; i <= MAX_SEGMENT; i++) {
+			/* Gather bits from each grid for this segment */
+			transposed_data = 0;
+			for (j = 0; j < display->display_data_len; j++) {
+				if (display->display_data[j] & BIT(i))
+					transposed_data |= BIT(MAX_SEGMENT - j);
+			}
+
+			ret = display->controller->data(display, i, transposed_data);
+			if (ret < 0) {
+				dev_err(display->dev,
+					"Failed to write transposed data: %d\n", ret);
+				break;
+			}
+		}
+	}
+
+	display->flush_status = ret;
+	mutex_unlock(&display->lock);
+}
+
+/**
+ * tm16xx_display_remove - Remove the display
+ * @display: Pointer to tm16xx_display structure
+ */
+static void tm16xx_display_remove(struct tm16xx_display *display)
+{
+	memset(display->display_data, 0x00, display->display_data_len);
+	schedule_work(&display->flush_display);
+	flush_work(&display->flush_display);
+
+	display->main_led.brightness = LED_OFF;
+	schedule_work(&display->flush_init);
+	flush_work(&display->flush_init);
+
+	dev_info(display->dev, "Display turned off\n");
+}
+
+/**
+ * tm16xx_brightness_set - Set brightness of the display
+ * @led_cdev: Pointer to led_classdev
+ * @brightness: Brightness value to set
+ */
+static void tm16xx_brightness_set(struct led_classdev *led_cdev,
+				  enum led_brightness brightness)
+{
+	struct tm16xx_display *display = dev_get_drvdata(led_cdev->dev->parent);
+
+	led_cdev->brightness = brightness;
+	schedule_work(&display->flush_init);
+}
+
+/**
+ * tm16xx_led_set - Set state of an individual LED
+ * @led_cdev: Pointer to led_classdev
+ * @value: Value to set (on/off)
+ */
+static void tm16xx_led_set(struct led_classdev *led_cdev, enum led_brightness value)
+{
+	struct tm16xx_led *led = container_of(led_cdev, struct tm16xx_led, cdev);
+	struct tm16xx_display *display = dev_get_drvdata(led_cdev->dev->parent);
+
+	if (value)
+		display->display_data[led->grid] |= (1 << led->segment);
+	else
+		display->display_data[led->grid] &= ~(1 << led->segment);
+
+	schedule_work(&display->flush_display);
+}
+
+/**
+ * tm16xx_value_show - Show current display value
+ * @dev: Pointer to device structure
+ * @attr: Pointer to device attribute structure
+ * @buf: Buffer to write the display value
+ *
+ * Return: Number of bytes written to buffer
+ */
+static ssize_t tm16xx_value_show(struct device *dev, struct device_attribute *attr,
+				 char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct tm16xx_display *display = dev_get_drvdata(led_cdev->dev->parent);
+	int i;
+
+	for (i = 0; i < display->num_digits && i < PAGE_SIZE - 1; i++)
+		buf[i] = display->digits[i].value;
+
+	buf[i++] = '\n';
+
+	return i;
+}
+
+/**
+ * tm16xx_value_store - Store new display value
+ * @dev: Pointer to device structure
+ * @attr: Pointer to device attribute structure
+ * @buf: Buffer containing the new display value
+ * @count: Number of bytes in buffer
+ *
+ * Return: Number of bytes written or negative error code
+ */
+static ssize_t tm16xx_value_store(struct device *dev, struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct tm16xx_display *display = dev_get_drvdata(led_cdev->dev->parent);
+	struct tm16xx_digit *digit;
+	int i;
+	u8 data;
+
+	for (i = 0; i < display->num_digits; i++) {
+		digit = &display->digits[i];
+
+		if (i < count && buf[i] != '\n') {
+			digit->value = buf[i];
+			data = tm16xx_ascii_to_segments(display, digit->value);
+		} else {
+			digit->value = 0;
+			data = 0;
+		}
+
+		display->display_data[digit->grid] =
+			(display->display_data[digit->grid] & ~display->digit_bitmask) |
+			(data & display->digit_bitmask);
+	}
+
+	schedule_work(&display->flush_display);
+
+	return count;
+}
+
+/**
+ * tm16xx_num_digits_show - Show the number of digits in the display
+ * @dev: The device struct
+ * @attr: The device_attribute struct
+ * @buf: The output buffer
+ *
+ * This function returns the number of digits in the display.
+ *
+ * Return: Number of bytes written to buf
+ */
+static ssize_t tm16xx_num_digits_show(struct device *dev, struct device_attribute *attr,
+				      char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct tm16xx_display *display = dev_get_drvdata(led_cdev->dev->parent);
+
+	return sprintf(buf, "%d\n", display->num_digits);
+}
+
+static int tm16xx_parse_int_array(const char *buf, int **array)
+{
+	int *values, value, count = 0, len;
+	const char *ptr = buf;
+
+	while (sscanf(ptr, "%d %n", &value, &len) == 1) {
+		count++;
+		ptr += len;
+	}
+
+	if (count == 0) {
+		*array = NULL;
+		return 0;
+	}
+
+	values = kmalloc(count * sizeof(*values), GFP_KERNEL);
+	if (!values)
+		return -ENOMEM;
+
+	ptr = buf;
+	count = 0;
+	while (sscanf(ptr, "%d %n", &value, &len) == 1) {
+		values[count++] = value;
+		ptr += len;
+	}
+
+	*array = values;
+	return count;
+}
+
+/**
+ * tm16xx_segments_show - Show the current segment mapping
+ * @dev: The device struct
+ * @attr: The device_attribute struct
+ * @buf: The output buffer
+ *
+ * This function returns the current segment mapping of the display.
+ *
+ * Return: Number of bytes written to buf
+ */
+static ssize_t tm16xx_segments_show(struct device *dev, struct device_attribute *attr,
+				    char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct tm16xx_display *display = dev_get_drvdata(led_cdev->dev->parent);
+	int i, count = 0;
+
+	for (i = 0; i < DIGIT_SEGMENTS; i++)
+		count += sprintf(buf + count, "%d ", display->segment_mapping[i]);
+
+	count += sprintf(buf + count, "\n");
+
+	return count;
+}
+
+/**
+ * tm16xx_segments_store - Set a new segment mapping
+ * @dev: The device struct
+ * @attr: The device_attribute struct
+ * @buf: The input buffer
+ * @count: Number of bytes in buf
+ *
+ * This function sets a new segment mapping for the display.
+ * It validates that each value is within the valid range, and that
+ * the number of received values matches the number of segments.
+ *
+ * Return: count on success, negative errno on failure
+ */
+static ssize_t tm16xx_segments_store(struct device *dev, struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct tm16xx_display *display = dev_get_drvdata(led_cdev->dev->parent);
+	int *array, ret, i;
+
+	ret = tm16xx_parse_int_array(buf, &array);
+	if (ret < 0)
+		return ret;
+
+	if (ret != DIGIT_SEGMENTS) {
+		kfree(array);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < DIGIT_SEGMENTS; i++) {
+		if (array[i] < MIN_SEGMENT || array[i] > MAX_SEGMENT) {
+			kfree(array);
+			return -EINVAL;
+		}
+	}
+
+	display->digit_bitmask = 0;
+	for (i = 0; i < DIGIT_SEGMENTS; i++) {
+		display->segment_mapping[i] = (u8)array[i];
+		display->digit_bitmask |= BIT(display->segment_mapping[i]);
+	}
+
+	kfree(array);
+	return count;
+}
+
+/**
+ * tm16xx_digits_show - Show the current digit ordering
+ * @dev: The device struct
+ * @attr: The device_attribute struct
+ * @buf: The output buffer
+ *
+ * This function returns the current ordering of digits in the display.
+ *
+ * Return: Number of bytes written to buf
+ */
+static ssize_t tm16xx_digits_show(struct device *dev, struct device_attribute *attr,
+				  char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct tm16xx_display *display = dev_get_drvdata(led_cdev->dev->parent);
+	int i, count = 0;
+
+	for (i = 0; i < display->num_digits; i++)
+		count += sprintf(buf + count, "%d ", display->digits[i].grid);
+
+	count += sprintf(buf + count, "\n");
+
+	return count;
+}
+
+/**
+ * tm16xx_digits_store - Set a new digit ordering
+ * @dev: The device struct
+ * @attr: The device_attribute struct
+ * @buf: The input buffer
+ * @count: Number of bytes in buf
+ *
+ * This function sets a new ordering of digits for the display.
+ * It validates that all values match the original digit grid indexes,
+ * and that the number of received values matches the number of digits.
+ *
+ * Return: count on success, negative errno on failure
+ */
+static ssize_t tm16xx_digits_store(struct device *dev, struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct tm16xx_display *display = dev_get_drvdata(led_cdev->dev->parent);
+	int *array, ret, i, j;
+	bool found;
+
+	ret = tm16xx_parse_int_array(buf, &array);
+	if (ret < 0)
+		return ret;
+
+	if (ret != display->num_digits) {
+		kfree(array);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < display->num_digits; i++) {
+		found = false;
+
+		for (j = 0; j < display->num_digits; j++) {
+			if (display->digits[i].grid == array[j]) {
+				found = true;
+				break;
+			}
+		}
+
+		if (!found) {
+			kfree(array);
+			return -EINVAL;
+		}
+	}
+
+	for (i = 0; i < display->num_digits; i++)
+		display->digits[i].grid = (u8)array[i];
+
+	kfree(array);
+	return count;
+}
+
+/**
+ * tm16xx_map_seg7_show - Show the current 7-segment character map
+ * @dev: The device struct
+ * @attr: The device_attribute struct
+ * @buf: The output buffer
+ *
+ * This function returns the current 7-segment character map.
+ *
+ * Return: Number of bytes written to buf
+ */
+static ssize_t tm16xx_map_seg7_show(struct device *dev, struct device_attribute *attr,
+				    char *buf)
+{
+	memcpy(buf, &map_seg7, sizeof(map_seg7));
+	return sizeof(map_seg7);
+}
+
+/**
+ * tm16xx_map_seg7_store - Set a new 7-segment character map
+ * @dev: The device struct
+ * @attr: The device_attribute struct
+ * @buf: The input buffer
+ * @cnt: Number of bytes in buf
+ *
+ * This function sets a new 7-segment character map.
+ *
+ * Return: cnt on success, negative errno on failure
+ */
+static ssize_t tm16xx_map_seg7_store(struct device *dev, struct device_attribute *attr,
+				     const char *buf, size_t cnt)
+{
+	if (cnt != sizeof(map_seg7))
+		return -EINVAL;
+	memcpy(&map_seg7, buf, cnt);
+	return cnt;
+}
+
+// clang-format off
+static DEVICE_ATTR(value, 0644, tm16xx_value_show, tm16xx_value_store);
+static DEVICE_ATTR(num_digits, 0444, tm16xx_num_digits_show, NULL);
+static DEVICE_ATTR(segments, 0644, tm16xx_segments_show, tm16xx_segments_store);
+static DEVICE_ATTR(digits, 0644, tm16xx_digits_show, tm16xx_digits_store);
+static DEVICE_ATTR(map_seg7, 0644, tm16xx_map_seg7_show, tm16xx_map_seg7_store);
+
+static struct attribute *tm16xx_main_led_attrs[] = {
+	&dev_attr_value.attr,
+	&dev_attr_num_digits.attr,
+	&dev_attr_segments.attr,
+	&dev_attr_digits.attr,
+	&dev_attr_map_seg7.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(tm16xx_main_led);
+// clang-format on
+
+/**
+ * tm16xx_display_init - Initialize the display
+ * @display: Pointer to tm16xx_display structure
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int tm16xx_display_init(struct tm16xx_display *display)
+{
+	schedule_work(&display->flush_init);
+	flush_work(&display->flush_init);
+	if (display->flush_status < 0)
+		return display->flush_status;
+
+	if (default_value && strlen(default_value) > 0) {
+		tm16xx_value_store(display->main_led.dev, NULL, default_value,
+				   strlen(default_value));
+	} else {
+		memset(display->display_data, 0xFF, display->display_data_len);
+		schedule_work(&display->flush_display);
+		flush_work(&display->flush_display);
+		memset(display->display_data, 0x00, display->display_data_len);
+		if (display->flush_status < 0)
+			return display->flush_status;
+	}
+
+	return 0;
+}
+
+/**
+ * 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
+ */
+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, "tm16xx,transposed");
+
+	ret = device_property_count_u8(dev, "tm16xx,digits");
+	if (ret < 0) {
+		dev_err(dev, "Failed to count 'tm16xx,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, "tm16xx,digits", digits,
+					    display->num_digits);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read 'tm16xx,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, "tm16xx,segment-mapping",
+					    display->segment_mapping, DIGIT_SEGMENTS);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read 'tm16xx,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 'tm16xx,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) {
+		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);
+	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);
+	if (ret < 0) {
+		dev_err(display->dev, "Failed to initialize display: %d\n", ret);
+		return ret;
+	}
+
+	dev_info(display->dev, "Display initialized successfully\n");
+	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;
+}
+
+static void tm16xx_spi_remove(struct spi_device *spi)
+{
+	struct tm16xx_display *display = spi_get_drvdata(spi);
+
+	tm16xx_display_remove(display);
+}
+
+// clang-format off
+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 },
+	{ "fd620", 0 },
+	{ "fd628", 0 },
+	{ "aip1618", 0 },
+	{ "aip1628", 0 },
+	{ "pt6964", 0 },
+	{ "hbs658", 0 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, tm16xx_spi_id);
+// clang-format on
+
+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;
+}
+
+static void tm16xx_i2c_remove(struct i2c_client *client)
+{
+	struct tm16xx_display *display = i2c_get_clientdata(client);
+
+	tm16xx_display_remove(display);
+}
+
+// clang-format off
+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);
+
+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
+
+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);
+
+MODULE_AUTHOR("Jean-François Lessard");
+MODULE_DESCRIPTION("TM16XX Compatible LED Display Controllers");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:tm16xx");
+MODULE_ALIAS("i2c:tm16xx");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 8/8] MAINTAINERS: Add entry for TM16xx driver
  2025-06-28 16:18 [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
                   ` (6 preceding siblings ...)
  2025-06-28 16:18 ` [PATCH 7/8] auxdisplay: Add support for Titanmec TM16xx 7 segment display controller Jean-François Lessard
@ 2025-06-28 16:18 ` Jean-François Lessard
  7 siblings, 0 replies; 11+ messages in thread
From: Jean-François Lessard @ 2025-06-28 16:18 UTC (permalink / raw)
  To: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Geert Uytterhoeven, devicetree, linux-leds, linux-kernel,
	Andreas Färber, Boris Gjenero, Christian Hewitt,
	Heiner Kallweit, Paolo Sabatino

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index efb51ee926..eee24ad0a8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25047,6 +25047,12 @@ F:	mm/memfd.c
 F:	mm/shmem.c
 F:	mm/shmem_quota.c
 
+TM16XX-COMPATIBLE LED CONTROLLERS DISPLAY DRIVER
+M:  Jean-François Lessard <jefflessard3@gmail.com>
+S:  Maintained
+F:  drivers/auxdisplay/tm16xx.c
+F:  Documentation/devicetree/bindings/auxdisplay/tm16xx.yaml
+
 TOMOYO SECURITY MODULE
 M:	Kentaro Takeda <takedakn@nttdata.co.jp>
 M:	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX
  2025-06-28 16:18 ` [PATCH 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX Jean-François Lessard
@ 2025-06-28 17:55   ` Rob Herring (Arm)
  2025-06-28 21:06     ` Jean-Francois Lessard
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring (Arm) @ 2025-06-28 17:55 UTC (permalink / raw)
  To: Jean-François Lessard
  Cc: Andy Shevchenko, linux-kernel, Krzysztof Kozlowski, Boris Gjenero,
	Heiner Kallweit, linux-leds, Christian Hewitt,
	Andreas Färber, Paolo Sabatino, devicetree,
	Geert Uytterhoeven, Conor Dooley


On Sat, 28 Jun 2025 12:18:43 -0400, Jean-François Lessard wrote:
> Add documentation for 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 <afaerber@suse.de>
> Co-developed-by: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
> ---
>  .../bindings/auxdisplay/tm16xx.yaml           | 153 ++++++++++++++++++
>  1 file changed, 153 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/tm16xx.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/auxdisplay/tm16xx.example.dts:59.29-83.11: ERROR (duplicate_label): /example-1/spi: Duplicate label 'display_client' on /example-1/spi and /example-0/i2c
ERROR: Input tree has errors, aborting (use -f to force output)
make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/auxdisplay/tm16xx.example.dtb] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1519: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250628161850.38865-7-jefflessard3@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX
  2025-06-28 17:55   ` Rob Herring (Arm)
@ 2025-06-28 21:06     ` Jean-Francois Lessard
  0 siblings, 0 replies; 11+ messages in thread
From: Jean-Francois Lessard @ 2025-06-28 21:06 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Andy Shevchenko, linux-kernel, Krzysztof Kozlowski, Boris Gjenero,
	Heiner Kallweit, linux-leds, Christian Hewitt,
	Andreas Färber, Paolo Sabatino, devicetree,
	Geert Uytterhoeven, Conor Dooley

On Sat, Jun 28, 2025 at 1:55 PM Rob Herring (Arm) <robh@kernel.org> wrote:
>
>
> On Sat, 28 Jun 2025 12:18:43 -0400, Jean-François Lessard wrote:
> > Add documentation for 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 <afaerber@suse.de>
> > Co-developed-by: Heiner Kallweit <hkallweit1@gmail.com>
> > Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
> > ---
> >  .../bindings/auxdisplay/tm16xx.yaml           | 153 ++++++++++++++++++
> >  1 file changed, 153 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/auxdisplay/tm16xx.yaml
> >
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/auxdisplay/tm16xx.example.dts:59.29-83.11: ERROR (duplicate_label): /example-1/spi: Duplicate label 'display_client' on /example-1/spi and /example-0/i2c
> ERROR: Input tree has errors, aborting (use -f to force output)
> make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/auxdisplay/tm16xx.example.dtb] Error 2
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1519: dt_binding_check] Error 2
> make: *** [Makefile:248: __sub-make] Error 2
>

Thank you for the dt_binding_check feedback – I’ll prepare v2 shortly
fixing the duplicate label issue.

> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250628161850.38865-7-jefflessard3@gmail.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>

Before I send v2, I’d like your guidance on one specific design question:

Currently, the binding uses a 'tm16xx,' prefix for properties
describing hardware layout (e.g. tm16xx,digits). TM16xx being the
family name for these compatible controllers (Titanmec, Fuda Hisi,
Princeton, Winrise, i-Core).

1. Is using 'tm16xx,' as a class prefix appropriate here, or should I
instead use a vendor-specific prefix (e.g. 'titanmec,') despite
multiple vendors implementing this hardware design?

2. Should 'tm16xx' also be formally added to vendor-prefixes.yaml or
handled differently?

Your advice will ensure the next revision aligns with DT conventions.

Thanks again for your review and guidance.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-06-28 21:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-28 16:18 [PATCH 0/8] auxdisplay: Add TM16xx and compatible LED display controllers driver Jean-François Lessard
2025-06-28 16:18 ` [PATCH 1/8] dt-bindings: vendor-prefixes: Add Fuda Hisi Microelectronics Jean-François Lessard
2025-06-28 16:18 ` [PATCH 2/8] dt-bindings: vendor-prefixes: Add Titan Micro Electronics Jean-François Lessard
2025-06-28 16:18 ` [PATCH 3/8] dt-bindings: vendor-prefixes: Add Princeton Technology Corp Jean-François Lessard
2025-06-28 16:18 ` [PATCH 4/8] dt-bindings: vendor-prefixes: Add Winrise Technology Jean-François Lessard
2025-06-28 16:18 ` [PATCH 5/8] dt-bindings: vendor-prefixes: Add Wuxi i-Core Electronics Jean-François Lessard
2025-06-28 16:18 ` [PATCH 6/8] dt-bindings: auxdisplay: add Titan Micro Electronics TM16XX Jean-François Lessard
2025-06-28 17:55   ` Rob Herring (Arm)
2025-06-28 21:06     ` Jean-Francois Lessard
2025-06-28 16:18 ` [PATCH 7/8] auxdisplay: Add support for Titanmec TM16xx 7 segment display controller Jean-François Lessard
2025-06-28 16:18 ` [PATCH 8/8] MAINTAINERS: Add entry for TM16xx driver Jean-François Lessard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).