| Message ID | 2671e6e3-8f18-8b70-244b-9e1415bfdf8f@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | auxdisplay: Add support for the Titanmec TM1628 7 segment display controller | expand |
On 2022-02-25 21:13, Heiner Kallweit wrote: > Add a YAML schema binding for TM1628 auxdisplay > (7/11-segment LED) controller. > > This patch is partially based on previous work from > Andreas Färber <afaerber@suse.de>. > > Co-developed-by: Andreas Färber <afaerber@suse.de> > Signed-off-by: Andreas Färber <afaerber@suse.de> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > v5: > - add vendor prefix to driver-specific properties > --- > .../bindings/auxdisplay/titanmec,tm1628.yaml | 92 +++++++++++++++++++ > 1 file changed, 92 insertions(+) > create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml > > diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml > new file mode 100644 > index 000000000..2a1ef692c > --- /dev/null > +++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml > @@ -0,0 +1,92 @@ > +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Titan Micro Electronics TM1628 LED controller > + > +maintainers: > + - Andreas Färber <afaerber@suse.de> > + - Heiner Kallweit <hkallweit1@gmail.com> > + > +allOf: > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + const: titanmec,tm1628 > + > + reg: > + maxItems: 1 > + > + titanmec,grid: > + description: > + Mapping of display digit position to grid number. > + This implicitly defines the display size. > + $ref: /schemas/types.yaml#/definitions/uint8-array > + minItems: 1 > + maxItems: 7 > + > + titanmec,segment-mapping: > + description: > + Mapping of 7 segment display segments A-G to bit numbers 1-12. > + $ref: /schemas/types.yaml#/definitions/uint8-array > + minItems: 7 > + maxItems: 7 > + > + "#address-cells": > + const: 2 > + > + "#size-cells": > + const: 0 > + > +required: > + - compatible > + - reg Would it be fair to say that "spi-lsb-first" and "spi-3wire" are also required? The chips aren't configurable so won't exactly be usable any other way. Furthermore I believe the transmission format actually works out equivalent to SPI mode 3, so should warrant "spi-cpha" and "spi-cpol" as well. > + > +patternProperties: > + "^.*@[1-7],([1-9]|1[0-6])$": > + type: object > + $ref: /schemas/leds/common.yaml# > + unevaluatedProperties: false > + description: | > + Properties for a single LED. > + > + properties: > + reg: > + description: | > + 1-based grid number, followed by 1-based segment bit number. > + maxItems: 1 > + > + required: > + - reg I'm concerned that this leaves us no room to support the additional keypad functionality in future. Having now double-checked a datasheet, the inputs are also a two-dimensional mux (sharing the segment lines), so the device effectively has two distinct but numerically-overlapping child address spaces - one addressed by (grid,segment) and the other by (segment,key). Rob, Krysztof, any thoughts on the best DT idiom to leave accommodation for that? I'm thinking either require an intermediate node to contain each notional address space, or perhaps add another leading address cell to select between them? I don't believe any of these things have further functionality beyond that. Thanks, Robin. > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/leds/common.h> > + > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + led-controller@0 { > + compatible = "titanmec,tm1628"; > + reg = <0>; > + spi-3-wire; > + spi-lsb-first; > + spi-max-frequency = <500000>; > + titanmec,grid = /bits/ 8 <4 3 2 1>; > + titanmec,segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>; > + #address-cells = <2>; > + #size-cells = <0>; > + > + alarm@5,4 { > + reg = <5 4>; > + function = LED_FUNCTION_ALARM; > + }; > + }; > + }; > +...
On 2022-03-21 08:12, Geert Uytterhoeven wrote: > Hi Robin, > > On Fri, Mar 18, 2022 at 9:50 PM Robin Murphy <robin.murphy@arm.com> wrote: >> On 2022-02-25 21:13, Heiner Kallweit wrote: >>> Add a YAML schema binding for TM1628 auxdisplay >>> (7/11-segment LED) controller. >>> >>> This patch is partially based on previous work from >>> Andreas Färber <afaerber@suse.de>. >>> >>> Co-developed-by: Andreas Färber <afaerber@suse.de> >>> Signed-off-by: Andreas Färber <afaerber@suse.de> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml > >>> + >>> +patternProperties: >>> + "^.*@[1-7],([1-9]|1[0-6])$": >>> + type: object >>> + $ref: /schemas/leds/common.yaml# >>> + unevaluatedProperties: false >>> + description: | >>> + Properties for a single LED. >>> + >>> + properties: >>> + reg: >>> + description: | >>> + 1-based grid number, followed by 1-based segment bit number. >>> + maxItems: 1 >>> + >>> + required: >>> + - reg >> >> I'm concerned that this leaves us no room to support the additional >> keypad functionality in future. Having now double-checked a datasheet, >> the inputs are also a two-dimensional mux (sharing the segment lines), >> so the device effectively has two distinct but numerically-overlapping >> child address spaces - one addressed by (grid,segment) and the other by >> (segment,key). > > Sounds similar to HT16K33? /me searches up a datasheet... Keypad-wise, it appears so, however the display side of this 1618/1628/1638 family is very much tuned for 7-segment displays rather than arbitrary dot-matrix ones. I do recall when I was digging a few years ago, turning up an old Holtek VFD driver which looked suspiciously like it might be the origin of the particular 3-wire protocol and command set (including weird non-linear brightness scale) which all these LED driver clones seem to borrow from, but I can't now remember the part number :( >> Rob, Krysztof, any thoughts on the best DT idiom to leave accommodation >> for that? I'm thinking either require an intermediate node to contain >> each notional address space, or perhaps add another leading address cell >> to select between them? I don't believe any of these things have further >> functionality beyond that. > > The problem with these devices is that there are thousands of different > ways to wire them, and coming up with a generic wiring description in > DT and writing code to handle that can be very hard. > > For HT16K33 non-dot-matrix wirings, I just added extra compatible > values matching the wiring of a few known devices[1]. That way the > driver can handle them efficiently. > It does have the disadvantage that adding support for new devices > means introducing more compatible values, and adding more code. > > Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml I think the display side of Heiner's binding is fine for what these chips can do - I've finally had a bit more time to play with this series, and (with minor driver hacks) it works just fine to describe my TM1638 breakout board with an 8-digit display, where segments 8 and 9 of each grid are respectively a decimal point and a discrete indicator LED, managed as separate LED nodes. However, I think you've indirectly addressed my outstanding concern there - I wasn't aware of the "linux,keymap" property, but since that brings its own implicit (row,column) addresses distinct from the DT address space, it looks like that might be sufficient as a neat standard way to extend this binding in future *without* any other changes. Thanks, Robin.
diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml new file mode 100644 index 000000000..2a1ef692c --- /dev/null +++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml @@ -0,0 +1,92 @@ +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Titan Micro Electronics TM1628 LED controller + +maintainers: + - Andreas Färber <afaerber@suse.de> + - Heiner Kallweit <hkallweit1@gmail.com> + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + const: titanmec,tm1628 + + reg: + maxItems: 1 + + titanmec,grid: + description: + Mapping of display digit position to grid number. + This implicitly defines the display size. + $ref: /schemas/types.yaml#/definitions/uint8-array + minItems: 1 + maxItems: 7 + + titanmec,segment-mapping: + description: + Mapping of 7 segment display segments A-G to bit numbers 1-12. + $ref: /schemas/types.yaml#/definitions/uint8-array + minItems: 7 + maxItems: 7 + + "#address-cells": + const: 2 + + "#size-cells": + const: 0 + +required: + - compatible + - reg + +patternProperties: + "^.*@[1-7],([1-9]|1[0-6])$": + type: object + $ref: /schemas/leds/common.yaml# + unevaluatedProperties: false + description: | + Properties for a single LED. + + properties: + reg: + description: | + 1-based grid number, followed by 1-based segment bit number. + maxItems: 1 + + required: + - reg + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/leds/common.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@0 { + compatible = "titanmec,tm1628"; + reg = <0>; + spi-3-wire; + spi-lsb-first; + spi-max-frequency = <500000>; + titanmec,grid = /bits/ 8 <4 3 2 1>; + titanmec,segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>; + #address-cells = <2>; + #size-cells = <0>; + + alarm@5,4 { + reg = <5 4>; + function = LED_FUNCTION_ALARM; + }; + }; + }; +...