Re: [PATCH 0/6] auxdisplay: Add support for the Titanmec TM1628 7 segment display controller

[Christian Hewitt <andrew@xxxxxxxxxxxxxxxxxxx>]

> On 19 Feb 2022, at 5:13 pm, Heiner Kallweit <hkallweit1@xxxxxxxxx> wrote:
> 
> This series adds support for the Titanmec TM1628 7 segment display
> controller. It's based on previous RFC work from Andreas Färber.
> The RFC version placed the driver in the LED subsystem, but this was
> NAK'ed by the LED maintainer. Therefore I moved the driver to
> /drivers/auxdisplay what seems most reasonable to me.
> 
> To be decided is through which tree this series should go.
> I'd think SPI would be most suited, but that's a decision I
> leave up to the respective maintainers.
> 
> Further changes to the RFC version:
> - Driver can be built also w/o LED class support, for displays that
>  don't have any symbols to be exposed as LED's.
> - Simplified the code and rewrote a lot of it.
> - Driver is now kind of a MVP, but functionality should be sufficient
>  for most use cases.
> - Use the existing 7 segment support in uapi/linux/map_to_7segment.h
>  as suggested by Geert Uytterhoeven.
> 
> Note: There's a number of chips from other manufacturers that are
>      almost identical, e.g. FD628, SM1628. Only difference I saw so
>      far is that they partially support other display modes.
>      TM1628: 6x12, 7x11
>      SM1628C: 4x13, 5x12, 6x11, 7x10
>      For typical displays on devices using these chips this
>      difference shouldn't matter.
> 
> Successfully tested on a TX3 Mini TV box that has an SM1628C and a
> display with 4 digits and 7 symbols.

Thanks for dusting off sources and working on this! - it’s another piece
of the upstream puzzle for distros that install on Android boxes.

I needed the following patch to address compile issues (missing include,
and the recent void/int change in linux-next (I’m using 5.17.y):

diff --git a/drivers/auxdisplay/tm1628.c b/drivers/auxdisplay/tm1628.c
index a39b638282c1..ab3557f8b330 100644
--- a/drivers/auxdisplay/tm1628.c
+++ b/drivers/auxdisplay/tm1628.c
@@ -5,6 +5,7 @@
 * Copyright (c) 2019 Andreas Färber
 */

+#include <linux/ctype.h>
#include <linux/delay.h>
#include <linux/leds.h>
#include <linux/module.h>
@@ -327,10 +328,11 @@ static int tm1628_spi_probe(struct spi_device *spi)
       return device_create_file(&spi->dev, &dev_attr_display_text);
}

-static void tm1628_spi_remove(struct spi_device *spi)
+static int tm1628_spi_remove(struct spi_device *spi)
{
       device_remove_file(&spi->dev, &dev_attr_display_text);
       tm1628_set_display_ctrl(spi, false);
+       return 0;
}

static void tm1628_spi_shutdown(struct spi_device *spi)

I also needed CONFIG_SPI_GPIO=y in kernel config. With this added the
driver probes on my TX3 mini box and the display goes dark overwriting
the default ‘boot’ text. The following systemd service and script sets
the clock and flashes the colon separator on/off to count seconds:

https://github.com/chewitt/LibreELEC.tv/commit/c8f1ebe6f6c366188f18f9d2b401de6c2979fdd7

With the include fixup and maybe a Kconfig tweak, for the series:

Tested-by: Christian Hewitt <christianshewitt@xxxxxxxxx>