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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022-03-16 21:19, Heiner Kallweit wrote:
On 16.03.2022 01:38, Robin Murphy wrote:
On 2022-02-25 21:09, Heiner Kallweit 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.

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.

FWIW I gave this a go on my Beelink A1, which has an AiP1618 and a clock display which would mapped like so:

     titanmec,segment-mapping = /bits/ 8 <1 2 3 13 12 5 4>;
     titanmec,grid = /bits/ 8 <5 4 2 1>;

(grid 3 segment 2 is used for a colon in the middle)

If I bodge around the lack of support for non-contiguous grids, it does otherwise work fairly well, other than being 6-segment displays because it needs to be in display mode 1 to drive SEG13 rather than GRID6. I wonder if we could be a bit cleverer about picking a display mode based on the grid/segment numbers used?

Definitely this could be one future extension. It could also consider that there's a number of more or less
identical chips from other vendors that differ primarily in the supported display modes.

I also have a couple of those TM1638 breakout boards with 8 digits, 8 single LEDs and 8 buttons that I might have a go with too. Have you given any thought to how the DT binding might support inputs as well? (The best time to be future-proof is before it's merged...)

With regards to inputs at least I have no plans because I have no hw supporting input.

FWIW, if you've got a board with exposed GPIO/SPI headers, searching "TM1638" on ebay/aliexpress/etc. should find the cheapo breakout boards. I believe they're quite popular with the Arduino crowd, so I expect that may well carry over to the Raspberry Pi crowd once they get wind of a kernel driver that can be driven by DT overlays.

Since the first attempts to support this LED driver hw two years have been passed w/o any tangible (mainline) result.
Therefore I want to keep the initial version a MVP. Wanting to have too many features in an initial version
may result in longer discussions until maintainer or I give up.

Unfortunately the principle is that DT bindings describe the device, not whatever the current level of Linux driver support for it might be. Perhaps I'm a little sensitised since I'm currently feeling the pain of extending a decade-old binding with functionality that was overlooked at the time, and not breaking compatibility is now rather awkward.

I'm not suggesting that there needs to be any support implemented in the driver, just to be certain that we're not painting ourselves into a corner with the binding.

Important is that user space interface / DT bindings are flexible enough so that future extensions don't have to break
existing users. And I think that's the case.

May I ask what you have in mind? I figure that inputs would most likely want to be described individually, similarly to the gpio-keys binding, which would lend itself to having them as child nodes, except that doesn't fit with the current scheme of child nodes having to be LEDs addressed by (grid,segment). I suppose there is a possible escape hatch of abusing unused addresses, e.g. saying a node at address (0,n) is input n rather than an LED segment, but that seems pretty horrid (and I'm not sure how well schema could validate it). Or possibly pretending to also be a GPIO controller to reference from a separate gpio-keys node, but again that seems ugly and more like something to only do if there's no other option.

IMO it would be cleanest just to have an extra level of hierarchy, e.g.:


	led-controller@0 {
		compatible = "titanmec,tm1628";
		...

		leds {
			#address-cells = <2>;
			#size-cells = <0>;

			alarm@5,4 {
				...
			};
		};
	};

That way there's clearly almost no risk of breakage if an additional "inputs" node with its own children turns up later. Plus it should also be a trivial change to the current driver, compared to having to implement trick special cases or whole other APIs down the line - of course bindings should not be designed expressly for ease of driver implementation, but if they do work out that way it's usually a good sign :)

Thanks,
Robin.



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux