Hello,
This patch series implements the LED controllers found in some RTD1295 based
TV set-top boxes.
Ever since I've had mainline Linux kernels booting on my Zidoo X9S TV box,
it's been bugging me that it kept displaying "boot" on its front display.
A hot lead was a TM1628 chip on the front display's daughterboard, which
English and Chinese datasheets were available for. The biggest hurdle
to overcome was whether and how this chip was connected to the SoC.
Confusingly the datasheet talks about "Serial Interface" and shows pins
DIO, SCLK and STB; whereas neither UART nor SPI DT nodes seemed to be in use
for this, no mention of such chipset in the binary vendor DT, and only one
seemingly unrelated exported GPIO. Sadly Zidoo have refused to share GPL
sourcecode with me, and the public GPL code drops from NAS and SBC vendors
didn't seem to include drivers for this chip. Last weekend, review of vendor
DT pinctrl nodes revealed a "spi@1" pinctrl node in use by the pinctrl node
itself, despite there being only one GSPI block on the SoC. debugfs under
Android revealed GPIO pins named "fp_stb", "fp_data" and "fp_clk" (on X5:
3x "vfdtest", unhelpfully). So I hereby present my first 3-wire SPI slave,
using standard spi-gpio driver.
This required to extend the spi-gpio driver with Little Endian support.
TM1628 and related chipsets have an internal Display RAM, from which they
control a two-dimensional array of LED components, often used for
seven-segment displays, i.e. clock display, but also for indicators.
Individual LEDs can be turned on/off, but brightness is applied globally.
Some chipsets also support polling a two-dimensional key pad.
This initial RFC implements a SPI slave driver within Linux leds subsystem
and lets DT expose individual LED components as two-state LEDs, allowing
to assign standard Linux LED triggers and to control them via sysfs.
It goes on to add a "text" attribute to the driver that enables DT-configured
seven-segment displays; I was expecting to find precedence in auxdisplay
subsystem but came up empty. So my driver currently integrates its own
generic (but incomplete) character-to-8-segments mapping, as well as in a
second step a combined-characters-to-8-segments mapping, which then gets
mapped to the chipset's available output lines. Doing this as sysfs device
attribute had the advantage of being able to test it quickly; it also leaves
timezone management to userspace and lets it choose between wall clock and
playback time as needed. LED triggers appeared to be per-LED; otherwise an
RTC-implemented interrupt based LED trigger would've been nice for RTD1195+,
since my pending irqchip driver exposes interrupts down to half-second that
would seem ideal for accurately driving such a display, with blinking colon.
Finally, it sketches how keypad handling could be integrated into the leds
driver, but I am lacking a test case for that functionality.
Distinguishing LEDs and key inputs in DT may get difficult...
For brightness control I am still investigating the backlight API and
defaulting to the chipset's default (lowest) brightness.
Prepended is a new DT for Xnano X5 OTT TV Box, featuring an FD628 display.
Displays connected to these controllers didn't have any model or vendor
usually, and for the lengthy numbers from my X9S, Google found no hits.
Therefore I've been unable to come up with compatible strings for those
displays and need to configure it per .dts, even though some may be using
the same, e.g., "88:88" type display model.
Whereas the same display might be connected to different LED controllers,
thus is orthogonal to the controller's compatible string.
Another aspect here is that the leds binding expects to have child nodes
per LED directly on the LED controller node. So I've gone to lengths to
shoehorn my display child node into that scheme via wildcard reg property.
The alternative would be to define some special child node, as done for the
SPI controller's "slave" node, to use as display. But in theory there might
be multiple displays connected to one controller (which is neglected here).
And in theory the same display might be wired up differently, so at most
the display model could tell us about layout and availability of LEDs, but
we'd still need a mapping from the LED controller's to the display's pins.
So far neither of the two displays tested actually use the segment lines
for the segments, but rather switch segment and grid lines.
So in theory we might consider the display as LED controller and implement
binding/driver on that level (moving it to DT root node like gpio-leds),
if we can hook it up to the actual LED controller in this case on SPI bus?
Assuming we can actually identify the display with some compatible string,
that is.
However, update efficiency has been a concern, with clock display in mind.
Thus, forcing two SPI commands (three SPI transfers) per LED segment, as the
the current LED API would entail, should better be avoided. This led to the
current design of having everything in tm1628 driver, so that we can easily
determine the scope of an update operation there (one per LED; all for text,
to be optimized through bit field of dirtied bytes).
Locking is completely missing still. We'll need at least a mutex to avoid,
e.g., a heartbeat LED trigger and a text update conflicting on SPI bus or
"hazards" becoming visible on the display during conflicting byte updates.
Module remove support is missing, too.
We may also need to revisit my error checking and either inline functions
or drop checks on the LED bit level, if it becomes a performance bottleneck.
On the cosmetic side, some lines are still beyond 80 characters.
Some more notes:
* Public TM1628 V1.1 datasheet is in Chinese only and differs from the
unversioned English version found elsewhere on datasheet sites by
documenting more display modes, included here (guessed from Arabic numbers).
* Public FD628 datasheet is Chinese only (guesses based on Arabic numbers).
FD623 appears to have more output lines, which would fit current data types.
* AiP1618 links were all broken (404); try Google "site:szfdwdz.com" search
to actually find the documents available on their site.
* Princeton PT6964 is another related LED controller with public datasheet
that I did not encounter in my TV boxes yet, thus not included here.
Datasheets are linked only for PT6959 and PT6967, but PT6964 V1.3 and V1.4
are available elsewhere. PT6967 has more output lines, which my current
data types could barely hold. Maybe bump them all to u32 type right away?
* TM1628 is also found on MeLE V9 TV box, to be tested.
* FD628 is also found on Amlogic S905X2 based Vontar X96 Max TV box,
to be tested (once UART is soldered).
* AiP1618 was found on Ava and Lake I TV boxes, to be tested.
* It remained unclear to me which of these many similar chipsets was first.
My driver name is therefore based on the chip I encountered first.
This series is based on my not-yet-posted RTD1295 pinctrl and GPIO drivers.
Latest experimental patches at:
https://github.com/afaerber/linux/commits/rtd1295-next
Have a lot of fun!
Cheers,
Andreas
Cc: [email protected]
Cc: Jacek Anaszewski <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Dan Murphy <[email protected]>
Cc: [email protected]
Cc: Alessandro Zummo <[email protected]>
Cc: Alexandre Belloni <[email protected]>
Cc: [email protected]
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: Mark Brown <[email protected]>
Cc: [email protected]
Cc: Roc He <[email protected]>
# No email for Xnano
Cc: [email protected]
Cc: [email protected]
# No email for szfdwdz.com
Cc: [email protected]
Andreas Färber (25):
dt-bindings: vendor-prefixes: Add Xnano
dt-bindings: arm: realtek: Add Xnano X5
arm64: dts: realtek: rtd1295: Add Xnano X5
spi: gpio: Implement LSB First bitbang support
dt-bindings: vendor-prefixes: Add Titan Micro Electronics
dt-bindings: leds: Add Titan Micro Electronics TM1628
leds: Add Titan Micro Electronics TM1628
arm64: dts: realtek: rtd129x-zidoo-x9s: Add TM1628 LED controller
arm64: dts: realtek: rtd1295-zidoo-x9s: Add regular LEDs to TM1628
dt-bindings: vendor-prefixes: Add Fuda Hisi Microelectronics
dt-bindings: leds: tm1628: Add Fuda Hisi Microelectronics FD628
leds: tm1628: Add Fuda Hisi Microelectronics FD628
arm64: dts: realtek: rtd1295-xnano-x5: Add FD628 LED controller
arm64: dts: realtek: rtd1295-xnano-x5: Add regular LEDs to FD628
dt-bindings: vendor-prefixes: Add Fude Microelectronics
dt-bindings: leds: tm1628: Add Fude Microelectronics AiP1618
leds: tm1628: Prepare Fude Microelectronics AiP1618
dt-bindings: leds: tm1628: Define display child nodes
leds: tm1628: Add 7-segment display support
arm64: dts: realtek: rtd1295-zidoo-x9s: Add display to TM1628
arm64: dts: realtek: rtd1295-xnano-x5: Add display to FD628
leds: tm1826: Add combined glyph support
WIP: leds: tm1628: Prepare TM1628 keys
WIP: leds: tm1628: Prepare FD628 keys
WIP: leds: tm1628: Prepare AiP1618 keys
Documentation/devicetree/bindings/arm/realtek.yaml | 1 +
.../devicetree/bindings/leds/titanmec,tm1628.yaml | 134 ++++
.../devicetree/bindings/vendor-prefixes.yaml | 8 +
arch/arm64/boot/dts/realtek/Makefile | 1 +
arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts | 108 +++
arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts | 36 +-
drivers/leds/Kconfig | 12 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-tm1628.c | 727 +++++++++++++++++++++
drivers/spi/spi-bitbang-txrx.h | 68 +-
drivers/spi/spi-gpio.c | 42 +-
11 files changed, 1126 insertions(+), 12 deletions(-)
create mode 100644 Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
create mode 100644 arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts
create mode 100644 drivers/leds/leds-tm1628.c
--
2.16.4
Add a child node for 4-digit 7-segment display.
Cc: Roc He <[email protected]>
Signed-off-by: Andreas Färber <[email protected]>
---
arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts b/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts
index 219b4987412c..6c2337e9282f 100644
--- a/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts
+++ b/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts
@@ -64,6 +64,10 @@
#address-cells = <2>;
#size-cells = <0>;
+ display@0,10 {
+ reg = <0 10>, <0 9>, <0 8>, <0 7>;
+ };
+
colon@5,4 {
reg = <5 4>;
color = <LED_COLOR_ID_WHITE>;
--
2.16.4
Define a compatible string for AiP1618 chipset.
Signed-off-by: Andreas Färber <[email protected]>
---
@Rob: This one would conditionally need to further restrict reg ranges then.
Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
index 3f2cc4cec04d..50205e3c3624 100644
--- a/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
+++ b/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
@@ -13,6 +13,7 @@ properties:
compatible:
enum:
- fdhisi,fd628
+ - szfdwdz,aip1618
- titanmec,tm1628
reg:
--
2.16.4
Assign vendor prefix "szfdwdz", based on their domain name.
Signed-off-by: Andreas Färber <[email protected]>
---
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 34b6c298c9bd..1f44e45971ab 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -927,6 +927,8 @@ patternProperties:
description: Synaptics Inc.
"^synology,.*":
description: Synology, Inc.
+ "^szfdwdz,.*":
+ description: Shenzhen Fude Microelectronics Co., Ltd.
"^tbs,.*":
description: TBS Technologies
"^tbs-biometrics,.*":
--
2.16.4
Add child nodes for individual LED indicators plus the half-second colon.
Signed-off-by: Andreas Färber <[email protected]>
---
arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts | 52 ++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts b/arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts
index ea9e9a96f1d3..9d678b300c7b 100644
--- a/arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts
+++ b/arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts
@@ -6,6 +6,7 @@
/dts-v1/;
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
#include "rtd1295.dtsi"
@@ -43,6 +44,57 @@
spi-rx-delay-us = <1>;
spi-max-frequency = <500000>;
#grids = <7>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ apps@1,4 {
+ reg = <1 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <1>;
+ };
+
+ setup@2,4 {
+ reg = <2 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <2>;
+ };
+
+ usb@3,4 {
+ reg = <3 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <3>;
+ };
+
+ card@4,4 {
+ reg = <4 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <4>;
+ };
+
+ colon@5,4 {
+ reg = <5 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_STATUS;
+ function-enumerator = <5>;
+ };
+
+ hdmi@6,4 {
+ reg = <6 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <6>;
+ };
+
+ cvbs@7,4 {
+ reg = <7 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <7>;
+ };
};
};
};
--
2.16.4
Extend TM1628 definition with key input lines.
Cc: [email protected]
Signed-off-by: Andreas Färber <[email protected]>
---
drivers/leds/leds-tm1628.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/leds/leds-tm1628.c b/drivers/leds/leds-tm1628.c
index 0c0d099de8b0..410f34234811 100644
--- a/drivers/leds/leds-tm1628.c
+++ b/drivers/leds/leds-tm1628.c
@@ -43,6 +43,8 @@ struct tm1628_info {
u16 seg_mask;
const struct tm1628_mode *modes;
int default_mode;
+ u8 k_mask;
+ u16 ks_mask;
const struct pwm_capture *pwm_map;
int default_pwm;
};
@@ -656,6 +658,8 @@ static const struct tm1628_info tm1628_info = {
.seg_mask = GENMASK(14, 12) | GENMASK(10, 1),
.modes = tm1628_modes,
.default_mode = 3,
+ .k_mask = GENMASK(2, 1),
+ .ks_mask = GENMASK(10, 1),
.pwm_map = tm1628_pwm_map,
.default_pwm = 0,
};
--
2.16.4
Assign vendor prefix "fdhisi", based on their domain name.
Cc: [email protected]
Signed-off-by: Andreas Färber <[email protected]>
---
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 31d0c4772b96..34b6c298c9bd 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -331,6 +331,8 @@ patternProperties:
description: Fastrax Oy
"^fcs,.*":
description: Fairchild Semiconductor
+ "^fdhisi,.*":
+ description: Fuzhou Fuda Hisi Microelectronics Co., Ltd.
"^feiyang,.*":
description: Shenzhen Fly Young Technology Co.,LTD.
"^firefly,.*":
--
2.16.4
Add a child node for the half-second colon.
Cc: Roc He <[email protected]>
Signed-off-by: Andreas Färber <[email protected]>
---
arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts b/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts
index 5183acd851a8..219b4987412c 100644
--- a/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts
+++ b/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts
@@ -6,6 +6,7 @@
/dts-v1/;
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
#include "rtd1295.dtsi"
@@ -60,6 +61,15 @@
spi-rx-delay-us = <1>;
spi-max-frequency = <500000>;
#grids = <7>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ colon@5,4 {
+ reg = <5 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_STATUS;
+ function-enumerator = <5>;
+ };
};
};
};
--
2.16.4
Add a YAML schema binding for TM1628 LED controller.
Cc: [email protected]
Signed-off-by: Andreas Färber <[email protected]>
---
@Rob: How could we express constraints on two-cell reg value ranges here?
Should we also model constraints on reg range by #grids property?
.../devicetree/bindings/leds/titanmec,tm1628.yaml | 80 ++++++++++++++++++++++
1 file changed, 80 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
diff --git a/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
new file mode 100644
index 000000000000..024875656e79
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/titanmec,tm1628.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Titan Micro Electronics TM1628 LED controller
+
+maintainers:
+ - Andreas Färber <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - titanmec,tm1628
+
+ reg:
+ maxItems: 1
+
+ "#grids":
+ description: |
+ Number of GRID output lines to use.
+ This limits the number of available SEG output lines.
+ minimum: 4
+ maximum: 7
+
+ "#address-cells":
+ const: 2
+
+ "#size-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+
+patternProperties:
+ "^.*@[1-7],([1-9]|1[02-4])$":
+ type: object
+ description: |
+ Properties for a single LED.
+
+ properties:
+ reg:
+ description: |
+ 1-based grid number, followed by 1-based segment number.
+ maxItems: 1
+
+ linux,default-trigger: true
+
+ required:
+ - reg
+
+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>;
+ #grids = <7>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ colon@5,4 {
+ reg = <5 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_INDICATOR;
+ linux,default-trigger = "heartbeat";
+ };
+ };
+ };
+...
--
2.16.4
Assign vendor prefix "titanmec", matching their domain name.
Cc: [email protected]
Signed-off-by: Andreas Färber <[email protected]>
---
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 84a76efad2a8..31d0c4772b96 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -951,6 +951,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.16.4
The Xnano X5 is an OTT TV Box.
Signed-off-by: Andreas Färber <[email protected]>
---
arch/arm64/boot/dts/realtek/Makefile | 1 +
arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts | 30 ++++++++++++++++++++++++
2 files changed, 31 insertions(+)
create mode 100644 arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts
diff --git a/arch/arm64/boot/dts/realtek/Makefile b/arch/arm64/boot/dts/realtek/Makefile
index 54bd02d11c02..ef8d8fcbaa05 100644
--- a/arch/arm64/boot/dts/realtek/Makefile
+++ b/arch/arm64/boot/dts/realtek/Makefile
@@ -4,6 +4,7 @@ dtb-$(CONFIG_ARCH_REALTEK) += rtd1293-ds418j.dtb
dtb-$(CONFIG_ARCH_REALTEK) += rtd1295-mele-v9.dtb
dtb-$(CONFIG_ARCH_REALTEK) += rtd1295-probox2-ava.dtb
+dtb-$(CONFIG_ARCH_REALTEK) += rtd1295-xnano-x5.dtb
dtb-$(CONFIG_ARCH_REALTEK) += rtd1295-zidoo-x9s.dtb
dtb-$(CONFIG_ARCH_REALTEK) += rtd1296-ds418.dtb
diff --git a/arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts b/arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts
new file mode 100644
index 000000000000..d7878ff942e6
--- /dev/null
+++ b/arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+/*
+ * Copyright (c) 2017-2019 Andreas Färber
+ */
+
+/dts-v1/;
+
+#include "rtd1295.dtsi"
+
+/ {
+ compatible = "xnano,x5", "realtek,rtd1295";
+ model = "Xnano X5";
+
+ memory@1f000 {
+ device_type = "memory";
+ reg = <0x1f000 0x3ffe1000>; /* boot ROM to 1 GiB or 2 GiB */
+ };
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
--
2.16.4
Add support for slave DT property spi-lsb-first, i.e., SPI_LSB_FIRST mode.
Duplicate the inline helpers bitbang_txrx_be_cpha{0,1} as LE versions.
Make checkpatch.pl happy by changing "unsigned" to "unsigned int".
Conditionally call them from all the spi-gpio txrx_word callbacks.
Signed-off-by: Andreas Färber <[email protected]>
---
drivers/spi/spi-bitbang-txrx.h | 68 ++++++++++++++++++++++++++++++++++++++++--
drivers/spi/spi-gpio.c | 42 ++++++++++++++++++++------
2 files changed, 99 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h
index ae61d72c7d28..999a89325325 100644
--- a/drivers/spi/spi-bitbang-txrx.h
+++ b/drivers/spi/spi-bitbang-txrx.h
@@ -45,7 +45,7 @@
static inline u32
bitbang_txrx_be_cpha0(struct spi_device *spi,
- unsigned nsecs, unsigned cpol, unsigned flags,
+ unsigned int nsecs, unsigned int cpol, unsigned int flags,
u32 word, u8 bits)
{
/* if (cpol == 0) this is SPI_MODE_0; else this is SPI_MODE_2 */
@@ -77,7 +77,7 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,
static inline u32
bitbang_txrx_be_cpha1(struct spi_device *spi,
- unsigned nsecs, unsigned cpol, unsigned flags,
+ unsigned int nsecs, unsigned int cpol, unsigned int flags,
u32 word, u8 bits)
{
/* if (cpol == 0) this is SPI_MODE_1; else this is SPI_MODE_3 */
@@ -106,3 +106,67 @@ bitbang_txrx_be_cpha1(struct spi_device *spi,
}
return word;
}
+
+static inline u32
+bitbang_txrx_le_cpha0(struct spi_device *spi,
+ unsigned int nsecs, unsigned int cpol, unsigned int flags,
+ u32 word, u8 bits)
+{
+ /* if (cpol == 0) this is SPI_MODE_0; else this is SPI_MODE_2 */
+
+ u32 oldbit = !(word & 1);
+ /* clock starts at inactive polarity */
+ for (; likely(bits); bits--) {
+
+ /* setup LSB (to slave) on trailing edge */
+ if ((flags & SPI_MASTER_NO_TX) == 0) {
+ if ((word & 1) != oldbit) {
+ setmosi(spi, word & 1);
+ oldbit = word & 1;
+ }
+ }
+ spidelay(nsecs); /* T(setup) */
+
+ setsck(spi, !cpol);
+ spidelay(nsecs);
+
+ /* sample LSB (from slave) on leading edge */
+ word >>= 1;
+ if ((flags & SPI_MASTER_NO_RX) == 0)
+ word |= getmiso(spi) << (bits - 1);
+ setsck(spi, cpol);
+ }
+ return word;
+}
+
+static inline u32
+bitbang_txrx_le_cpha1(struct spi_device *spi,
+ unsigned int nsecs, unsigned int cpol, unsigned int flags,
+ u32 word, u8 bits)
+{
+ /* if (cpol == 0) this is SPI_MODE_1; else this is SPI_MODE_3 */
+
+ u32 oldbit = !(word & 1);
+ /* clock starts at inactive polarity */
+ for (; likely(bits); bits--) {
+
+ /* setup LSB (to slave) on leading edge */
+ setsck(spi, !cpol);
+ if ((flags & SPI_MASTER_NO_TX) == 0) {
+ if ((word & 1) != oldbit) {
+ setmosi(spi, word & 1);
+ oldbit = word & 1;
+ }
+ }
+ spidelay(nsecs); /* T(setup) */
+
+ setsck(spi, cpol);
+ spidelay(nsecs);
+
+ /* sample LSB (from slave) on trailing edge */
+ word >>= 1;
+ if ((flags & SPI_MASTER_NO_RX) == 0)
+ word |= getmiso(spi) << (bits - 1);
+ }
+ return word;
+}
diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c
index 7ceb0ba27b75..493723eda844 100644
--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -135,25 +135,37 @@ static inline int getmiso(const struct spi_device *spi)
static u32 spi_gpio_txrx_word_mode0(struct spi_device *spi,
unsigned nsecs, u32 word, u8 bits, unsigned flags)
{
- return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
+ if (unlikely(spi->mode & SPI_LSB_FIRST))
+ return bitbang_txrx_le_cpha0(spi, nsecs, 0, flags, word, bits);
+ else
+ return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
}
static u32 spi_gpio_txrx_word_mode1(struct spi_device *spi,
unsigned nsecs, u32 word, u8 bits, unsigned flags)
{
- return bitbang_txrx_be_cpha1(spi, nsecs, 0, flags, word, bits);
+ if (unlikely(spi->mode & SPI_LSB_FIRST))
+ return bitbang_txrx_le_cpha1(spi, nsecs, 0, flags, word, bits);
+ else
+ return bitbang_txrx_be_cpha1(spi, nsecs, 0, flags, word, bits);
}
static u32 spi_gpio_txrx_word_mode2(struct spi_device *spi,
unsigned nsecs, u32 word, u8 bits, unsigned flags)
{
- return bitbang_txrx_be_cpha0(spi, nsecs, 1, flags, word, bits);
+ if (unlikely(spi->mode & SPI_LSB_FIRST))
+ return bitbang_txrx_le_cpha0(spi, nsecs, 1, flags, word, bits);
+ else
+ return bitbang_txrx_be_cpha0(spi, nsecs, 1, flags, word, bits);
}
static u32 spi_gpio_txrx_word_mode3(struct spi_device *spi,
unsigned nsecs, u32 word, u8 bits, unsigned flags)
{
- return bitbang_txrx_be_cpha1(spi, nsecs, 1, flags, word, bits);
+ if (unlikely(spi->mode & SPI_LSB_FIRST))
+ return bitbang_txrx_le_cpha1(spi, nsecs, 1, flags, word, bits);
+ else
+ return bitbang_txrx_be_cpha1(spi, nsecs, 1, flags, word, bits);
}
/*
@@ -170,28 +182,40 @@ static u32 spi_gpio_spec_txrx_word_mode0(struct spi_device *spi,
unsigned nsecs, u32 word, u8 bits, unsigned flags)
{
flags = spi->master->flags;
- return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
+ if (unlikely(spi->mode & SPI_LSB_FIRST))
+ return bitbang_txrx_le_cpha0(spi, nsecs, 0, flags, word, bits);
+ else
+ return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
}
static u32 spi_gpio_spec_txrx_word_mode1(struct spi_device *spi,
unsigned nsecs, u32 word, u8 bits, unsigned flags)
{
flags = spi->master->flags;
- return bitbang_txrx_be_cpha1(spi, nsecs, 0, flags, word, bits);
+ if (unlikely(spi->mode & SPI_LSB_FIRST))
+ return bitbang_txrx_le_cpha1(spi, nsecs, 0, flags, word, bits);
+ else
+ return bitbang_txrx_be_cpha1(spi, nsecs, 0, flags, word, bits);
}
static u32 spi_gpio_spec_txrx_word_mode2(struct spi_device *spi,
unsigned nsecs, u32 word, u8 bits, unsigned flags)
{
flags = spi->master->flags;
- return bitbang_txrx_be_cpha0(spi, nsecs, 1, flags, word, bits);
+ if (unlikely(spi->mode & SPI_LSB_FIRST))
+ return bitbang_txrx_le_cpha0(spi, nsecs, 1, flags, word, bits);
+ else
+ return bitbang_txrx_be_cpha0(spi, nsecs, 1, flags, word, bits);
}
static u32 spi_gpio_spec_txrx_word_mode3(struct spi_device *spi,
unsigned nsecs, u32 word, u8 bits, unsigned flags)
{
flags = spi->master->flags;
- return bitbang_txrx_be_cpha1(spi, nsecs, 1, flags, word, bits);
+ if (unlikely(spi->mode & SPI_LSB_FIRST))
+ return bitbang_txrx_le_cpha1(spi, nsecs, 1, flags, word, bits);
+ else
+ return bitbang_txrx_be_cpha1(spi, nsecs, 1, flags, word, bits);
}
/*----------------------------------------------------------------------*/
@@ -389,7 +413,7 @@ static int spi_gpio_probe(struct platform_device *pdev)
master->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 32);
master->mode_bits = SPI_3WIRE | SPI_3WIRE_HIZ | SPI_CPHA | SPI_CPOL |
- SPI_CS_HIGH;
+ SPI_CS_HIGH | SPI_LSB_FIRST;
if (!spi_gpio->mosi) {
/* HW configuration without MOSI pin
*
--
2.16.4
Allow to squeeze the text "HEllO" into a 4-digit display,
as seen on MeLE V9 TV box.
Enable this combining mode only if the text would overflow.
Signed-off-by: Andreas Färber <[email protected]>
---
drivers/leds/leds-tm1628.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/leds/leds-tm1628.c b/drivers/leds/leds-tm1628.c
index e2c2a5d91596..0c0d099de8b0 100644
--- a/drivers/leds/leds-tm1628.c
+++ b/drivers/leds/leds-tm1628.c
@@ -347,6 +347,28 @@ static u8 tm1628_get_char_ssd_map(char ch)
return 0x0;
}
+struct tm1628_ssd_glyph {
+ char *str;
+ u8 segs;
+};
+
+static const struct tm1628_ssd_glyph tm1628_glyph_ssd_map[] = {
+ { "ll", SSD_TOP_LEFT | SSD_BOTTOM_LEFT |
+ SSD_TOP_RIGHT | SSD_BOTTOM_RIGHT },
+};
+
+static u8 tm1628_get_glyph_ssd_map(const char *str)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(tm1628_glyph_ssd_map); i++) {
+ if (!strncmp(tm1628_glyph_ssd_map[i].str, str, 2))
+ return tm1628_glyph_ssd_map[i].segs;
+ }
+
+ return 0x0;
+}
+
static int tm1628_display_apply_map(struct tm1628 *s,
struct tm1628_display *display, u8 map)
{
@@ -366,7 +388,7 @@ static ssize_t text_store(struct device *dev,
{
struct tm1628 *s = dev_get_drvdata(dev);
size_t offset, len = count;
- u8 map;
+ u8 map, glyph_map;
int i, ret;
if (len > 0 && buf[len - 1] == '\n')
@@ -375,6 +397,13 @@ static ssize_t text_store(struct device *dev,
for (i = 0, offset = 0; i < s->num_displays; i++) {
if (offset < len) {
map = tm1628_get_char_ssd_map(buf[offset]);
+ if (offset + 1 < len && len > s->num_displays) {
+ glyph_map = tm1628_get_glyph_ssd_map(buf + offset);
+ if (glyph_map) {
+ map = glyph_map;
+ offset++;
+ }
+ }
offset++;
} else
map = 0x0;
--
2.16.4
Define a compatible string for Xnano X5 OTT TV Box.
Signed-off-by: Andreas Färber <[email protected]>
---
Documentation/devicetree/bindings/arm/realtek.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/arm/realtek.yaml b/Documentation/devicetree/bindings/arm/realtek.yaml
index db43dc3ab7ac..e11616883736 100644
--- a/Documentation/devicetree/bindings/arm/realtek.yaml
+++ b/Documentation/devicetree/bindings/arm/realtek.yaml
@@ -32,6 +32,7 @@ properties:
- enum:
- mele,v9 # MeLE V9
- probox2,ava # ProBox2 AVA
+ - xnano,x5 # Xnano X5
- zidoo,x9s # Zidoo X9S
- const: realtek,rtd1295
--
2.16.4
Detect DT child nodes covering more than one LED component and implement
a text to 7-segment display mapping.
Signed-off-by: Andreas Färber <[email protected]>
---
drivers/leds/leds-tm1628.c | 230 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 229 insertions(+), 1 deletion(-)
diff --git a/drivers/leds/leds-tm1628.c b/drivers/leds/leds-tm1628.c
index 8a8fd1562853..e2c2a5d91596 100644
--- a/drivers/leds/leds-tm1628.c
+++ b/drivers/leds/leds-tm1628.c
@@ -47,6 +47,15 @@ struct tm1628_info {
int default_pwm;
};
+struct tm1628_segment {
+ u32 grid;
+ u32 seg;
+};
+
+struct tm1628_display {
+ struct tm1628_segment segments[8];
+};
+
struct tm1628_led {
struct led_classdev leddev;
struct tm1628 *ctrl;
@@ -62,6 +71,8 @@ struct tm1628 {
int mode_index;
int pwm_index;
u8 data[14];
+ unsigned int num_displays;
+ struct tm1628_display *displays;
unsigned int num_leds;
struct tm1628_led leds[];
};
@@ -253,6 +264,215 @@ static int tm1628_register_led(struct tm1628 *s,
return devm_led_classdev_register_ext(dev, &led->leddev, &init_data);
}
+#define SSD_TOP BIT(0)
+#define SSD_TOP_RIGHT BIT(1)
+#define SSD_BOTTOM_RIGHT BIT(2)
+#define SSD_BOTTOM BIT(3)
+#define SSD_BOTTOM_LEFT BIT(4)
+#define SSD_TOP_LEFT BIT(5)
+#define SSD_MIDDLE BIT(6)
+#define SSD_DOT BIT(7)
+
+struct tm1628_ssd_char {
+ char ch;
+ u8 segs;
+};
+
+static const struct tm1628_ssd_char tm1628_char_ssd_map[] = {
+ { '0', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { '1', SSD_TOP_RIGHT | SSD_BOTTOM_RIGHT },
+ { '2', SSD_TOP | SSD_TOP_RIGHT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM },
+ { '3', SSD_TOP | SSD_TOP_RIGHT | SSD_MIDDLE | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { '4', SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE | SSD_BOTTOM_RIGHT },
+ { '5', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { '6', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { '7', SSD_TOP | SSD_TOP_RIGHT | SSD_BOTTOM_RIGHT },
+ { '8', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { '9', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
+ SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { 'A', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT },
+ { 'B', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { 'C', SSD_TOP | SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM },
+ { 'D', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { 'E', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM },
+ { 'F', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT },
+ { 'G', SSD_TOP | SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
+ { 'H', SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT },
+ { 'I', SSD_TOP_LEFT | SSD_BOTTOM_LEFT },
+ { 'J', SSD_TOP | SSD_TOP_RIGHT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { 'K', SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT },
+ { 'L', SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM },
+ { 'O', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { 'P', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
+ SSD_BOTTOM_LEFT },
+ { 'Q', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { 'R', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
+ SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT },
+ { 'S', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
+ { 'T', SSD_TOP | SSD_TOP_LEFT | SSD_BOTTOM_LEFT },
+ { 'U', SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT | SSD_TOP_RIGHT },
+ { 'V', SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT | SSD_TOP_RIGHT },
+ { 'W', SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT | SSD_TOP_RIGHT },
+ { 'Z', SSD_TOP | SSD_TOP_RIGHT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM },
+ { 'b', SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
+ { 'l', SSD_TOP_LEFT | SSD_BOTTOM_LEFT },
+ { 'o', SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
+ { 't', SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM },
+ { 'u', SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
+ { 'v', SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
+ { 'w', SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
+ { '-', SSD_MIDDLE },
+ { '_', SSD_BOTTOM },
+ { '.', SSD_DOT },
+};
+
+static u8 tm1628_get_char_ssd_map(char ch)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(tm1628_char_ssd_map); i++) {
+ if (tm1628_char_ssd_map[i].ch == ch)
+ return tm1628_char_ssd_map[i].segs;
+ }
+
+ return 0x0;
+}
+
+static int tm1628_display_apply_map(struct tm1628 *s,
+ struct tm1628_display *display, u8 map)
+{
+ struct tm1628_segment *segment;
+ int i;
+
+ for (i = 0; i < 8; i++) {
+ segment = &display->segments[i];
+ tm1628_set_led(s, segment->grid, segment->seg, map & BIT(i));
+ }
+
+ return 0;
+}
+
+static ssize_t text_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct tm1628 *s = dev_get_drvdata(dev);
+ size_t offset, len = count;
+ u8 map;
+ int i, ret;
+
+ if (len > 0 && buf[len - 1] == '\n')
+ len--;
+
+ for (i = 0, offset = 0; i < s->num_displays; i++) {
+ if (offset < len) {
+ map = tm1628_get_char_ssd_map(buf[offset]);
+ offset++;
+ } else
+ map = 0x0;
+
+ tm1628_display_apply_map(s, &s->displays[i], map);
+ }
+
+ ret = tm1628_set_address(s->spi, 0x0);
+ if (ret)
+ return ret;
+
+ ret = tm1628_write_data(s->spi, s->data, 14);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static struct device_attribute tm1628_attr =
+ __ATTR_WO(text);
+
+static int tm1628_register_display(struct tm1628 *s,
+ struct fwnode_handle *node)
+{
+ struct device *dev = &s->spi->dev;
+ struct tm1628_display *display;
+ u32 *reg;
+ u32 grid, seg;
+ int i, j, ret, reg_count;
+
+ reg_count = fwnode_property_count_u32(node, "reg");
+ if (reg_count < 0)
+ return reg_count;
+
+ if (reg_count % 2) {
+ dev_warn(dev, "Ignoring extra cell in %s reg property\n",
+ fwnode_get_name(node));
+ reg_count--;
+ }
+
+ if (s->displays) {
+ dev_warn(dev, "Only one display supported\n");
+ return -EINVAL;
+ }
+
+ s->num_displays = reg_count >> 1;
+
+ reg = devm_kzalloc(dev, reg_count * sizeof(*reg), GFP_KERNEL);
+ if (!reg)
+ return -ENOMEM;
+
+ ret = fwnode_property_read_u32_array(node, "reg", reg, reg_count);
+ if (ret) {
+ dev_err(dev, "Reading %s reg property failed (%d)\n",
+ fwnode_get_name(node), ret);
+ return ret;
+ }
+
+ s->displays = devm_kzalloc(dev, s->num_displays * sizeof(*s->displays), GFP_KERNEL);
+ if (!s->displays)
+ return -ENOMEM;
+
+ for (i = 0; i < s->num_displays; i++) {
+ display = &s->displays[i];
+ grid = reg[i * 2];
+ seg = reg[i * 2 + 1];
+ if (grid == 0 && seg != 0) {
+ if (!tm1628_is_valid_seg(s, seg)) {
+ dev_warn(dev, "%s reg out of range\n", fwnode_get_name(node));
+ return -EINVAL;
+ }
+ grid = s->info->modes[s->mode_index].grid_mask;
+ for (j = 0; grid && j < 7; j++) {
+ display->segments[j].seg = seg;
+ display->segments[j].grid = __ffs(grid);
+ grid &= ~BIT(display->segments[j].grid);
+ }
+ } else if (grid != 0 && seg == 0) {
+ if (!tm1628_is_valid_grid(s, grid)) {
+ dev_warn(dev, "%s reg out of range\n", fwnode_get_name(node));
+ return -EINVAL;
+ }
+ seg = s->info->modes[s->mode_index].seg_mask;
+ for (j = 0; seg && j < 8; j++) {
+ display->segments[j].grid = grid;
+ display->segments[j].seg = __ffs(seg);
+ seg &= ~BIT(display->segments[j].seg);
+ }
+ }
+ }
+
+ devm_kfree(dev, reg);
+
+ device_create_file(dev, &tm1628_attr);
+
+ return 0;
+}
+
/* Work around __builtin_popcount() */
static u32 tm1628_grid_popcount(u8 grid_mask)
{
@@ -325,7 +545,7 @@ static int tm1628_spi_probe(struct spi_device *spi)
return ret;
}
- if (fwnode_property_count_u32(child, "reg") == 2) {
+ if (reg[0] != 0 && reg[1] != 0 && fwnode_property_count_u32(child, "reg") == 2) {
ret = tm1628_register_led(s, child, reg[0], reg[1], &s->leds[i++]);
if (ret && ret != -EINVAL) {
dev_err(&spi->dev, "Failed to register LED %s (%d)\n",
@@ -334,6 +554,14 @@ static int tm1628_spi_probe(struct spi_device *spi)
return ret;
}
s->num_leds++;
+ } else {
+ ret = tm1628_register_display(s, child);
+ if (ret) {
+ dev_err(&spi->dev, "Failed to register display %s (%d)\n",
+ fwnode_get_name(child), ret);
+ fwnode_handle_put(child);
+ return ret;
+ }
}
}
--
2.16.4
Amend the individual LEDs by allowing to specify one special child node
with more than one reg entry, with value zero having wildcard meaning.
Signed-off-by: Andreas Färber <[email protected]>
---
Read the cover letter for further discussion of concept and alternatives.
.../devicetree/bindings/leds/titanmec,tm1628.yaml | 52 ++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
index 50205e3c3624..cf6c8d81e68c 100644
--- a/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
+++ b/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
@@ -53,6 +53,24 @@ patternProperties:
required:
- reg
+ "^display@([1-7],0|0,([1-9]|1[02-4]))$":
+ type: object
+ description: |
+ Properties for a sequence of 7-segment digits composed of multiple LEDs.
+
+ properties:
+ reg:
+ description: |
+ One or more tuples of grid number and segment number in visual order.
+ A segment of zero indicates that the corresponding grid output lines
+ represent the individual segments; a grid of zero indicates that the
+ corresponding segment output lines represent the individual segments.
+ minItems: 1
+ maxItems: 7
+
+ required:
+ - reg
+
examples:
- |
#include <dt-bindings/leds/common.h>
@@ -71,6 +89,40 @@ examples:
#address-cells = <2>;
#size-cells = <0>;
+ display@0,8 {
+ reg = <0 8>, <0 7>, <0 6>, <0 5>;
+ };
+
+ colon@5,4 {
+ reg = <5 4>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_INDICATOR;
+ linux,default-trigger = "heartbeat";
+ };
+ };
+ };
+
+ - |
+ #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>;
+ #grids = <6>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ display@1,0 {
+ reg = <1 0>, <2 0>, <3 0>, <4 0>;
+ };
+
colon@5,4 {
reg = <5 4>;
color = <LED_COLOR_ID_WHITE>;
--
2.16.4
Xnano is a Chinese TV box brand, but not much more is publicly known.
Signed-off-by: Andreas Färber <[email protected]>
---
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 6046f4555852..84a76efad2a8 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1058,6 +1058,8 @@ patternProperties:
description: Xillybus Ltd.
"^xlnx,.*":
description: Xilinx
+ "^xnano,.*":
+ description: Xnano
"^xunlong,.*":
description: Shenzhen Xunlong Software CO.,Limited
"^yones-toptech,.*":
--
2.16.4
Hi Andreas,
On Thu, Dec 12, 2019 at 4:42 AM Andreas Färber <[email protected]> wrote:
> Detect DT child nodes covering more than one LED component and implement
> a text to 7-segment display mapping.
>
> Signed-off-by: Andreas Färber <[email protected]>
> --- a/drivers/leds/leds-tm1628.c
> +++ b/drivers/leds/leds-tm1628.c
> +static const struct tm1628_ssd_char tm1628_char_ssd_map[] = {
> + { '0', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
Please use include/uapi/linux/map_to_7segment.h
Yes, it is well hidden ;-).
Gr{oetje,eeting}s,
Geert
--
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
Hi Andreas,
On Thu, Dec 12, 2019 at 4:41 AM Andreas Färber <[email protected]> wrote:
> Add support for slave DT property spi-lsb-first, i.e., SPI_LSB_FIRST mode.
>
> Duplicate the inline helpers bitbang_txrx_be_cpha{0,1} as LE versions.
> Make checkpatch.pl happy by changing "unsigned" to "unsigned int".
>
> Conditionally call them from all the spi-gpio txrx_word callbacks.
>
> Signed-off-by: Andreas Färber <[email protected]>
Thanks for your patch!
> --- a/drivers/spi/spi-gpio.c
> +++ b/drivers/spi/spi-gpio.c
> @@ -135,25 +135,37 @@ static inline int getmiso(const struct spi_device *spi)
> static u32 spi_gpio_txrx_word_mode0(struct spi_device *spi,
> unsigned nsecs, u32 word, u8 bits, unsigned flags)
> {
> - return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
> + if (unlikely(spi->mode & SPI_LSB_FIRST))
> + return bitbang_txrx_le_cpha0(spi, nsecs, 0, flags, word, bits);
> + else
> + return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
> }
Duplicating all functions sounds a bit wasteful to me.
What about reverting the word first, and calling the normal functions?
if (unlikely(spi->mode & SPI_LSB_FIRST)) {
if (bits <= 8)
word = bitrev8(word) >> (bits - 8);
else if (bits <= 16)
word = bitrev16(word) >> (bits - 16);
else
word = bitrev32(word) >> (bits - 32);
}
return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
Gr{oetje,eeting}s,
Geert
--
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
Hi Andreas,
On 12/12/2019 3:39 am, Andreas Färber wrote:
> Hello,
>
> This patch series implements the LED controllers found in some RTD1295 based
> TV set-top boxes.
>
> Ever since I've had mainline Linux kernels booting on my Zidoo X9S TV box,
> it's been bugging me that it kept displaying "boot" on its front display.
> A hot lead was a TM1628 chip on the front display's daughterboard, which
> English and Chinese datasheets were available for. The biggest hurdle
> to overcome was whether and how this chip was connected to the SoC.
> Confusingly the datasheet talks about "Serial Interface" and shows pins
> DIO, SCLK and STB; whereas neither UART nor SPI DT nodes seemed to be in use
> for this, no mention of such chipset in the binary vendor DT, and only one
> seemingly unrelated exported GPIO. Sadly Zidoo have refused to share GPL
> sourcecode with me, and the public GPL code drops from NAS and SBC vendors
> didn't seem to include drivers for this chip. Last weekend, review of vendor
> DT pinctrl nodes revealed a "spi@1" pinctrl node in use by the pinctrl node
> itself, despite there being only one GSPI block on the SoC. debugfs under
> Android revealed GPIO pins named "fp_stb", "fp_data" and "fp_clk" (on X5:
> 3x "vfdtest", unhelpfully). So I hereby present my first 3-wire SPI slave,
> using standard spi-gpio driver.
>
> This required to extend the spi-gpio driver with Little Endian support.
>
> TM1628 and related chipsets have an internal Display RAM, from which they
> control a two-dimensional array of LED components, often used for
> seven-segment displays, i.e. clock display, but also for indicators.
> Individual LEDs can be turned on/off, but brightness is applied globally.
> Some chipsets also support polling a two-dimensional key pad.
>
> This initial RFC implements a SPI slave driver within Linux leds subsystem
> and lets DT expose individual LED components as two-state LEDs, allowing
> to assign standard Linux LED triggers and to control them via sysfs.
>
> It goes on to add a "text" attribute to the driver that enables DT-configured
> seven-segment displays; I was expecting to find precedence in auxdisplay
> subsystem but came up empty. So my driver currently integrates its own
> generic (but incomplete) character-to-8-segments mapping, as well as in a
> second step a combined-characters-to-8-segments mapping, which then gets
> mapped to the chipset's available output lines. Doing this as sysfs device
> attribute had the advantage of being able to test it quickly; it also leaves
> timezone management to userspace and lets it choose between wall clock and
> playback time as needed. LED triggers appeared to be per-LED; otherwise an
> RTC-implemented interrupt based LED trigger would've been nice for RTD1195+,
> since my pending irqchip driver exposes interrupts down to half-second that
> would seem ideal for accurately driving such a display, with blinking colon.
>
> Finally, it sketches how keypad handling could be integrated into the leds
> driver, but I am lacking a test case for that functionality.
> Distinguishing LEDs and key inputs in DT may get difficult...
>
> For brightness control I am still investigating the backlight API and
> defaulting to the chipset's default (lowest) brightness.
>
> Prepended is a new DT for Xnano X5 OTT TV Box, featuring an FD628 display.
>
> Displays connected to these controllers didn't have any model or vendor
> usually, and for the lengthy numbers from my X9S, Google found no hits.
> Therefore I've been unable to come up with compatible strings for those
> displays and need to configure it per .dts, even though some may be using
> the same, e.g., "88:88" type display model.
> Whereas the same display might be connected to different LED controllers,
> thus is orthogonal to the controller's compatible string.
>
> Another aspect here is that the leds binding expects to have child nodes
> per LED directly on the LED controller node. So I've gone to lengths to
> shoehorn my display child node into that scheme via wildcard reg property.
>
> The alternative would be to define some special child node, as done for the
> SPI controller's "slave" node, to use as display. But in theory there might
> be multiple displays connected to one controller (which is neglected here).
> And in theory the same display might be wired up differently, so at most
> the display model could tell us about layout and availability of LEDs, but
> we'd still need a mapping from the LED controller's to the display's pins.
> So far neither of the two displays tested actually use the segment lines
> for the segments, but rather switch segment and grid lines.
>
> So in theory we might consider the display as LED controller and implement
> binding/driver on that level (moving it to DT root node like gpio-leds),
> if we can hook it up to the actual LED controller in this case on SPI bus?
> Assuming we can actually identify the display with some compatible string,
> that is.
> However, update efficiency has been a concern, with clock display in mind.
> Thus, forcing two SPI commands (three SPI transfers) per LED segment, as the
> the current LED API would entail, should better be avoided. This led to the
> current design of having everything in tm1628 driver, so that we can easily
> determine the scope of an update operation there (one per LED; all for text,
> to be optimized through bit field of dirtied bytes).
>
> Locking is completely missing still. We'll need at least a mutex to avoid,
> e.g., a heartbeat LED trigger and a text update conflicting on SPI bus or
> "hazards" becoming visible on the display during conflicting byte updates.
>
> Module remove support is missing, too.
>
> We may also need to revisit my error checking and either inline functions
> or drop checks on the LED bit level, if it becomes a performance bottleneck.
>
> On the cosmetic side, some lines are still beyond 80 characters.
>
> Some more notes:
> * Public TM1628 V1.1 datasheet is in Chinese only and differs from the
> unversioned English version found elsewhere on datasheet sites by
> documenting more display modes, included here (guessed from Arabic numbers).
> * Public FD628 datasheet is Chinese only (guesses based on Arabic numbers).
> FD623 appears to have more output lines, which would fit current data types.
> * AiP1618 links were all broken (404); try Google "site:szfdwdz.com" search
> to actually find the documents available on their site.
> * Princeton PT6964 is another related LED controller with public datasheet
> that I did not encounter in my TV boxes yet, thus not included here.
> Datasheets are linked only for PT6959 and PT6967, but PT6964 V1.3 and V1.4
> are available elsewhere. PT6967 has more output lines, which my current
> data types could barely hold. Maybe bump them all to u32 type right away?
> * TM1628 is also found on MeLE V9 TV box, to be tested.
> * FD628 is also found on Amlogic S905X2 based Vontar X96 Max TV box,
> to be tested (once UART is soldered).
> * AiP1618 was found on Ava and Lake I TV boxes, to be tested.
> * It remained unclear to me which of these many similar chipsets was first.
> My driver name is therefore based on the chip I encountered first.
It's pretty cool to see this! My Rockchip box has an AiP1618-driven
display that I've also spent a bit of time hacking around with - I did
get some way into writing an LED driver, but ultimately gave up and
wrote a simple thing to bit-bang the GPIO chardev from userspace (and
since there are enough clocks in my house, I now have a cpufreq display!)
In case it helps, in my research I found that ARTSCHIP are another
vendor of these things with accessible datasheets[1], and as far as I
could tell the command set appears to derive from (or is at least common
to) some old Holtek VFD controllers.
If I can figure out the DT parts (which was one of the areas that
stalled my attempt) I'll try to have a play with this series over the
holidays. One thought to ponder is that I have an "88:88" display where
the entire middle grid is reserved for the colon (which is wired to just
one segment) - I'm not sure how that could be sanely described :/
Robin.
[1]
http://www.artschip.com/products.asp?lx=small&anid=779&ParentName=Signal%20management%20_I_O%20Extender
> This series is based on my not-yet-posted RTD1295 pinctrl and GPIO drivers.
>
> Latest experimental patches at:
> https://github.com/afaerber/linux/commits/rtd1295-next
>
> Have a lot of fun!
>
> Cheers,
> Andreas
>
> Cc: [email protected]
> Cc: Jacek Anaszewski <[email protected]>
> Cc: Pavel Machek <[email protected]>
> Cc: Dan Murphy <[email protected]>
>
> Cc: [email protected]
> Cc: Alessandro Zummo <[email protected]>
> Cc: Alexandre Belloni <[email protected]>
>
> Cc: [email protected]
> Cc: Rob Herring <[email protected]>
>
> Cc: [email protected]
> Cc: Mark Brown <[email protected]>
>
> Cc: [email protected]
>
> Cc: Roc He <[email protected]>
> # No email for Xnano
>
> Cc: [email protected]
> Cc: [email protected]
> # No email for szfdwdz.com
> Cc: [email protected]
>
> Andreas Färber (25):
> dt-bindings: vendor-prefixes: Add Xnano
> dt-bindings: arm: realtek: Add Xnano X5
> arm64: dts: realtek: rtd1295: Add Xnano X5
> spi: gpio: Implement LSB First bitbang support
> dt-bindings: vendor-prefixes: Add Titan Micro Electronics
> dt-bindings: leds: Add Titan Micro Electronics TM1628
> leds: Add Titan Micro Electronics TM1628
> arm64: dts: realtek: rtd129x-zidoo-x9s: Add TM1628 LED controller
> arm64: dts: realtek: rtd1295-zidoo-x9s: Add regular LEDs to TM1628
> dt-bindings: vendor-prefixes: Add Fuda Hisi Microelectronics
> dt-bindings: leds: tm1628: Add Fuda Hisi Microelectronics FD628
> leds: tm1628: Add Fuda Hisi Microelectronics FD628
> arm64: dts: realtek: rtd1295-xnano-x5: Add FD628 LED controller
> arm64: dts: realtek: rtd1295-xnano-x5: Add regular LEDs to FD628
> dt-bindings: vendor-prefixes: Add Fude Microelectronics
> dt-bindings: leds: tm1628: Add Fude Microelectronics AiP1618
> leds: tm1628: Prepare Fude Microelectronics AiP1618
> dt-bindings: leds: tm1628: Define display child nodes
> leds: tm1628: Add 7-segment display support
> arm64: dts: realtek: rtd1295-zidoo-x9s: Add display to TM1628
> arm64: dts: realtek: rtd1295-xnano-x5: Add display to FD628
> leds: tm1826: Add combined glyph support
> WIP: leds: tm1628: Prepare TM1628 keys
> WIP: leds: tm1628: Prepare FD628 keys
> WIP: leds: tm1628: Prepare AiP1618 keys
>
> Documentation/devicetree/bindings/arm/realtek.yaml | 1 +
> .../devicetree/bindings/leds/titanmec,tm1628.yaml | 134 ++++
> .../devicetree/bindings/vendor-prefixes.yaml | 8 +
> arch/arm64/boot/dts/realtek/Makefile | 1 +
> arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts | 108 +++
> arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts | 36 +-
> drivers/leds/Kconfig | 12 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-tm1628.c | 727 +++++++++++++++++++++
> drivers/spi/spi-bitbang-txrx.h | 68 +-
> drivers/spi/spi-gpio.c | 42 +-
> 11 files changed, 1126 insertions(+), 12 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
> create mode 100644 arch/arm64/boot/dts/realtek/rtd1295-xnano-x5.dts
> create mode 100644 drivers/leds/leds-tm1628.c
>
Hi Geert,
Am 12.12.19 um 09:33 schrieb Geert Uytterhoeven:
> On Thu, Dec 12, 2019 at 4:42 AM Andreas Färber <[email protected]> wrote:
>> Detect DT child nodes covering more than one LED component and implement
>> a text to 7-segment display mapping.
>>
>> Signed-off-by: Andreas Färber <[email protected]>
>
>> --- a/drivers/leds/leds-tm1628.c
>> +++ b/drivers/leds/leds-tm1628.c
>
>> +static const struct tm1628_ssd_char tm1628_char_ssd_map[] = {
>> + { '0', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
>> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
>
> Please use include/uapi/linux/map_to_7segment.h
> Yes, it is well hidden ;-).
Many thanks for that pointer! Even matches the segment/bit order of my
datasheets/defines.
The small t may be different (T?), and I don't see it mapping combined
characters (patch 22/25). The recommended sysfs map_7seg attribute may
help deal with that, but seems terribly unhandy for my busybox initrd.
Hope you also read the cover letter, where not finding this and many
other issues were discussed. I was expecting some .c file in either
drivers/leds or auxdisplay - input is indeed unexpected for output.
Still wondering whether we can somehow describe those "VFD" displays in
a more reusable way, like we do for MIPI DSI panels.
Cheers,
Andreas
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)
Hi Geert,
Am 12.12.19 um 09:40 schrieb Geert Uytterhoeven:
> On Thu, Dec 12, 2019 at 4:41 AM Andreas Färber <[email protected]> wrote:
>> Add support for slave DT property spi-lsb-first, i.e., SPI_LSB_FIRST mode.
>>
>> Duplicate the inline helpers bitbang_txrx_be_cpha{0,1} as LE versions.
>> Make checkpatch.pl happy by changing "unsigned" to "unsigned int".
>>
>> Conditionally call them from all the spi-gpio txrx_word callbacks.
>>
>> Signed-off-by: Andreas Färber <[email protected]>
>
> Thanks for your patch!
NP. I prefer fixing issues at the source over awkward workarounds. :)
>> --- a/drivers/spi/spi-gpio.c
>> +++ b/drivers/spi/spi-gpio.c
>> @@ -135,25 +135,37 @@ static inline int getmiso(const struct spi_device *spi)
>> static u32 spi_gpio_txrx_word_mode0(struct spi_device *spi,
>> unsigned nsecs, u32 word, u8 bits, unsigned flags)
>> {
>> - return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
>> + if (unlikely(spi->mode & SPI_LSB_FIRST))
>> + return bitbang_txrx_le_cpha0(spi, nsecs, 0, flags, word, bits);
>> + else
>> + return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
>> }
>
> Duplicating all functions sounds a bit wasteful to me.
Two functions duplicated, eight function calls duplicated.
> What about reverting the word first, and calling the normal functions?
>
> if (unlikely(spi->mode & SPI_LSB_FIRST)) {
> if (bits <= 8)
> word = bitrev8(word) >> (bits - 8);
> else if (bits <= 16)
> word = bitrev16(word) >> (bits - 16);
> else
> word = bitrev32(word) >> (bits - 32);
> }
> return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits);
Hm, wasn't aware of those helpers, so I opted not to loop over the bits
for reversing myself, as Thomas Gleixner disliked bit loops in irqchip.
Performance appeared to be a concern here, too.
Eight functions duplicated then. I don't like that - at least we should
pack that into an inline helper or macro, to not copy&paste even more
lines around. Who knows, maybe we'll get 64-bit support at one point?
Do you think it would be acceptable to instead encapsulate this inside
the _be inline helpers? That would lead the name ad absurdum, of course,
but we would then need to do it only twice, not eight times.
However, either way would seem to make the LSB code path slower than MSB
due to the prepended reversal.
Delays are already stubbed out, with open TODOs for further inlining
functions that are being dispatched today.
So from that angle I don't see a better way than either duplicating the
functions or using some macro magic to #include the header twice. If we
wanted to go down that path, we could probably de-duplicate the existing
two functions, too, but I was trying to err on the cautious side, since
I don't have setups to test all four code paths myself (and a ton of
more relevant but less fun patches to flush out ;)).
Regards,
Andreas
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)
On Thu, Dec 12, 2019 at 04:14:59PM +0100, Andreas F?rber wrote:
> Am 12.12.19 um 09:40 schrieb Geert Uytterhoeven:
> > On Thu, Dec 12, 2019 at 4:41 AM Andreas F?rber <[email protected]> wrote:
> >> Add support for slave DT property spi-lsb-first, i.e., SPI_LSB_FIRST mode.
> >> Duplicate the inline helpers bitbang_txrx_be_cpha{0,1} as LE versions.
> >> Make checkpatch.pl happy by changing "unsigned" to "unsigned int".
Separate patch for this?
> So from that angle I don't see a better way than either duplicating the
> functions or using some macro magic to #include the header twice. If we
> wanted to go down that path, we could probably de-duplicate the existing
> two functions, too, but I was trying to err on the cautious side, since
> I don't have setups to test all four code paths myself (and a ton of
> more relevant but less fun patches to flush out ;)).
Yeah, I don't think there's any great options here with the potential
performance issues - probably the nicest thing would be to autogenerate
lots of variants but I think that's far more trouble than it's worth.
Hi Robin,
[- Roc He, + linux-rockchip]
Am 12.12.19 um 14:14 schrieb Robin Murphy:
> On 12/12/2019 3:39 am, Andreas Färber wrote:
>> This patch series implements the LED controllers found in some RTD1295
>> based
>> TV set-top boxes.
[...]
>> TM1628 and related chipsets have an internal Display RAM, from which they
>> control a two-dimensional array of LED components, often used for
>> seven-segment displays, i.e. clock display, but also for indicators.
>> Individual LEDs can be turned on/off, but brightness is applied globally.
>> Some chipsets also support polling a two-dimensional key pad.
[...]
>> Some more notes:
>> * Public TM1628 V1.1 datasheet is in Chinese only and differs from the
>> unversioned English version found elsewhere on datasheet sites by
>> documenting more display modes, included here (guessed from Arabic
>> numbers).
>> * Public FD628 datasheet is Chinese only (guesses based on Arabic
>> numbers).
>> FD623 appears to have more output lines, which would fit current
>> data types.
>> * AiP1618 links were all broken (404); try Google "site:szfdwdz.com"
>> search
>> to actually find the documents available on their site.
>> * Princeton PT6964 is another related LED controller with public
>> datasheet
>> that I did not encounter in my TV boxes yet, thus not included here.
>> Datasheets are linked only for PT6959 and PT6967, but PT6964 V1.3
>> and V1.4
>> are available elsewhere. PT6967 has more output lines, which my
>> current
>> data types could barely hold. Maybe bump them all to u32 type right
>> away?
>> * TM1628 is also found on MeLE V9 TV box, to be tested.
>> * FD628 is also found on Amlogic S905X2 based Vontar X96 Max TV box,
>> to be tested (once UART is soldered).
>> * AiP1618 was found on Ava and Lake I TV boxes, to be tested.
>> * It remained unclear to me which of these many similar chipsets was
>> first.
>> My driver name is therefore based on the chip I encountered first.
>
> It's pretty cool to see this!
Glad someone else finds it useful. :)
> My Rockchip box has an AiP1618-driven
> display [...]
You don't mention the model: Does it have a mainline .dts we can extend?
If not, I'd ask you to get that merged into -next, then I can happily
pick up patches adding the LED controller for your TV box into this
series as it evolves. (I'm expecting at least two more RFC iterations.)
Similarly, I'm planning to drop Xnano X5 in v2, if it doesn't require a
respin, so that no Realtek-specific parts other than .dts node additions
remain here.
> In case it helps, in my research I found that ARTSCHIP are another
> vendor of these things with accessible datasheets[1],
Thanks, their HT1628 indeed looks compatible.
Sunmoon Microelectronics SM1628 also looks compatible.
http://www.chinaasic.com/product.jsp#item=other#style=27#id=138
> and as far as I
> could tell the command set appears to derive from (or is at least common
> to) some old Holtek VFD controllers.
Hmm, HT16515 looks similar and has more lines, RAM and mode bits than I
prepared here.
https://www.holtek.com/productdetail/-/vg/ht16515
So I'd need to make more numbers model-dependent and allocate the
Display RAM buffer dynamically.
Whereas HT16D35A seems incompatible command-wise, and HT16528 appears to
be out of scope, for dot displays and with fancy embedded character map.
No Holtek email alias that I can quickly spot.
But given that I'm proposing vendor-specific compatibles just in case,
the main decisions will be the Kconfig symbol and module name. The
driver code itself we could always refactor after merging, and renaming
the schema file (as opposed to compatible) should also be possible.
> If I can figure out the DT parts (which was one of the areas that
> stalled my attempt) I'll try to have a play with this series over the
> holidays.
That reminded me that I forgot to push - done in the meantime. :)
> One thought to ponder is that I have an "88:88" display where
> the entire middle grid is reserved for the colon (which is wired to just
> one segment) - I'm not sure how that could be sanely described :/
Well, that sounds exactly like my bindings example and X9S. You'll find
the colon configured as LED, separate from the four digits, which don't
need to be contiguous due to separate reg entries per digit.
While it may be possible to put more cleverness into text_store() to set
the colon as part of five-char "88:88" text, we'd likely want to blink
it every half second, which we should better do without updating the
full display text from "88:88" to "88 88". "8888" updated every minute
sounds less problematic.
Ugly with the colon LED is that the redone LED bindings don't yet have a
function defined for this, so I'm currently misusing whatever was there.
I should prepare a bindings addition, if we want to use an LED node.
Regards,
Andreas
> [1]
> http://www.artschip.com/products.asp?lx=small&anid=779&ParentName=Signal%20management%20_I_O%20Extender
>
>> This series is based on my not-yet-posted RTD1295 pinctrl and GPIO
>> drivers.
>>
>> Latest experimental patches at:
>> https://github.com/afaerber/linux/commits/rtd1295-next
[snip]
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)
Am 12.12.19 um 18:19 schrieb Mark Brown:
> On Thu, Dec 12, 2019 at 04:14:59PM +0100, Andreas F?rber wrote:
>> Am 12.12.19 um 09:40 schrieb Geert Uytterhoeven:
>>> On Thu, Dec 12, 2019 at 4:41 AM Andreas F?rber <[email protected]> wrote:
>>>> Add support for slave DT property spi-lsb-first, i.e., SPI_LSB_FIRST mode.
>
>>>> Duplicate the inline helpers bitbang_txrx_be_cpha{0,1} as LE versions.
>>>> Make checkpatch.pl happy by changing "unsigned" to "unsigned int".
>
> Separate patch for this?
For the checkpatch cleanup? Or helpers preparation vs. spi-gpio.c usage?
>> So from that angle I don't see a better way than either duplicating the
>> functions or using some macro magic to #include the header twice. If we
>> wanted to go down that path, we could probably de-duplicate the existing
>> two functions, too, but I was trying to err on the cautious side, since
>> I don't have setups to test all four code paths myself (and a ton of
>> more relevant but less fun patches to flush out ;)).
>
> Yeah, I don't think there's any great options here with the potential
> performance issues - probably the nicest thing would be to autogenerate
> lots of variants but I think that's far more trouble than it's worth.
Maybe add another code comment to revisit that idea later then?
Thanks,
Andreas
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer
HRB 36809 (AG N?rnberg)
On Thu, Dec 12, 2019 at 10:08:24PM +0100, Andreas F?rber wrote:
> Am 12.12.19 um 18:19 schrieb Mark Brown:
> >>>> Duplicate the inline helpers bitbang_txrx_be_cpha{0,1} as LE versions.
> >>>> Make checkpatch.pl happy by changing "unsigned" to "unsigned int".
> > Separate patch for this?
> For the checkpatch cleanup? Or helpers preparation vs. spi-gpio.c usage?
At least the checkpatch cleanup.
> > Yeah, I don't think there's any great options here with the potential
> > performance issues - probably the nicest thing would be to autogenerate
> > lots of variants but I think that's far more trouble than it's worth.
> Maybe add another code comment to revisit that idea later then?
Sure, if you like.
On 12/12/2019 8:55 pm, Andreas Färber wrote:
> Hi Robin,
>
> [- Roc He, + linux-rockchip]
>
> Am 12.12.19 um 14:14 schrieb Robin Murphy:
>> On 12/12/2019 3:39 am, Andreas Färber wrote:
>>> This patch series implements the LED controllers found in some RTD1295
>>> based
>>> TV set-top boxes.
> [...]
>>> TM1628 and related chipsets have an internal Display RAM, from which they
>>> control a two-dimensional array of LED components, often used for
>>> seven-segment displays, i.e. clock display, but also for indicators.
>>> Individual LEDs can be turned on/off, but brightness is applied globally.
>>> Some chipsets also support polling a two-dimensional key pad.
> [...]
>>> Some more notes:
>>> * Public TM1628 V1.1 datasheet is in Chinese only and differs from the
>>> unversioned English version found elsewhere on datasheet sites by
>>> documenting more display modes, included here (guessed from Arabic
>>> numbers).
>>> * Public FD628 datasheet is Chinese only (guesses based on Arabic
>>> numbers).
>>> FD623 appears to have more output lines, which would fit current
>>> data types.
>>> * AiP1618 links were all broken (404); try Google "site:szfdwdz.com"
>>> search
>>> to actually find the documents available on their site.
>>> * Princeton PT6964 is another related LED controller with public
>>> datasheet
>>> that I did not encounter in my TV boxes yet, thus not included here.
>>> Datasheets are linked only for PT6959 and PT6967, but PT6964 V1.3
>>> and V1.4
>>> are available elsewhere. PT6967 has more output lines, which my
>>> current
>>> data types could barely hold. Maybe bump them all to u32 type right
>>> away?
>>> * TM1628 is also found on MeLE V9 TV box, to be tested.
>>> * FD628 is also found on Amlogic S905X2 based Vontar X96 Max TV box,
>>> to be tested (once UART is soldered).
>>> * AiP1618 was found on Ava and Lake I TV boxes, to be tested.
>>> * It remained unclear to me which of these many similar chipsets was
>>> first.
>>> My driver name is therefore based on the chip I encountered first.
>>
>> It's pretty cool to see this!
>
> Glad someone else finds it useful. :)
>
>> My Rockchip box has an AiP1618-driven
>> display [...]
>
> You don't mention the model: Does it have a mainline .dts we can extend?
> If not, I'd ask you to get that merged into -next, then I can happily
> pick up patches adding the LED controller for your TV box into this
> series as it evolves. (I'm expecting at least two more RFC iterations.)
It's the Beelink A1, which we have indeed just landed a DT for - I'll
certainly share whatever patch I come up with. I also have one of the
H96 Max boxes (which I picked up out of curiosity for the mysterious
RK3318) with an FD6551, although I've not attacked that one with the
logic analyser yet to see how similar it is.
> Similarly, I'm planning to drop Xnano X5 in v2, if it doesn't require a
> respin, so that no Realtek-specific parts other than .dts node additions
> remain here.
>
>> In case it helps, in my research I found that ARTSCHIP are another
>> vendor of these things with accessible datasheets[1],
>
> Thanks, their HT1628 indeed looks compatible.
>
> Sunmoon Microelectronics SM1628 also looks compatible.
> http://www.chinaasic.com/product.jsp#item=other#style=27#id=138
>
>> and as far as I
>> could tell the command set appears to derive from (or is at least common
>> to) some old Holtek VFD controllers.
>
> Hmm, HT16515 looks similar and has more lines, RAM and mode bits than I
> prepared here.
> https://www.holtek.com/productdetail/-/vg/ht16515
>
> So I'd need to make more numbers model-dependent and allocate the
> Display RAM buffer dynamically.
>
> Whereas HT16D35A seems incompatible command-wise, and HT16528 appears to
> be out of scope, for dot displays and with fancy embedded character map.
>
> No Holtek email alias that I can quickly spot.
>
> But given that I'm proposing vendor-specific compatibles just in case,
> the main decisions will be the Kconfig symbol and module name. The
> driver code itself we could always refactor after merging, and renaming
> the schema file (as opposed to compatible) should also be possible.
Yeah, I'm not sure that it really matters, as I doubt there are many
Linux-capable devices with a real VFD anyway; it just seemed like an
interesting datapoint that fell out of scouring the web trying to find
any evidence for which the "canonical" 1618 might be (and the Holtek
connection was actually a coincidence from a misidentification of the
ARTSCHIP part number).
>> If I can figure out the DT parts (which was one of the areas that
>> stalled my attempt) I'll try to have a play with this series over the
>> holidays.
>
> That reminded me that I forgot to push - done in the meantime. :)
>
>> One thought to ponder is that I have an "88:88" display where
>> the entire middle grid is reserved for the colon (which is wired to just
>> one segment) - I'm not sure how that could be sanely described :/
>
> Well, that sounds exactly like my bindings example and X9S. You'll find
> the colon configured as LED, separate from the four digits, which don't
> need to be contiguous due to separate reg entries per digit.
Aha, yes, I should have engaged the brain a bit more on that one :)
> While it may be possible to put more cleverness into text_store() to set
> the colon as part of five-char "88:88" text, we'd likely want to blink
> it every half second, which we should better do without updating the
> full display text from "88:88" to "88 88". "8888" updated every minute
> sounds less problematic.
Sure - perhaps at that point text_store() could also grow some caching
and partial update logic to decide if writing individual grids is
cheaper than clocking out the whole display for a given change, but this
initial approach does seem good enough to start with. Lumping colons in
with the other miscellaneous indicators many of these displays have does
at least have a self-consistent logic in terms of "things that aren't
7-segment grids".
Thanks,
Robin.
> Ugly with the colon LED is that the redone LED bindings don't yet have a
> function defined for this, so I'm currently misusing whatever was there.
> I should prepare a bindings addition, if we want to use an LED node.
>
> Regards,
> Andreas
>
>> [1]
>> http://www.artschip.com/products.asp?lx=small&anid=779&ParentName=Signal%20management%20_I_O%20Extender
>>
>>> This series is based on my not-yet-posted RTD1295 pinctrl and GPIO
>>> drivers.
>>>
>>> Latest experimental patches at:
>>> https://github.com/afaerber/linux/commits/rtd1295-next
> [snip]
>
Hi Robin,
On Fri, Dec 13, 2019 at 3:08 PM Robin Murphy <[email protected]> wrote:
> On 12/12/2019 8:55 pm, Andreas Färber wrote:
> > Am 12.12.19 um 14:14 schrieb Robin Murphy:
> >> and as far as I
> >> could tell the command set appears to derive from (or is at least common
> >> to) some old Holtek VFD controllers.
> >
> > Hmm, HT16515 looks similar and has more lines, RAM and mode bits than I
> > prepared here.
> > https://www.holtek.com/productdetail/-/vg/ht16515
> >
> > So I'd need to make more numbers model-dependent and allocate the
> > Display RAM buffer dynamically.
> >
> > Whereas HT16D35A seems incompatible command-wise, and HT16528 appears to
> > be out of scope, for dot displays and with fancy embedded character map.
> >
> > No Holtek email alias that I can quickly spot.
> >
> > But given that I'm proposing vendor-specific compatibles just in case,
> > the main decisions will be the Kconfig symbol and module name. The
> > driver code itself we could always refactor after merging, and renaming
> > the schema file (as opposed to compatible) should also be possible.
>
> Yeah, I'm not sure that it really matters, as I doubt there are many
> Linux-capable devices with a real VFD anyway; it just seemed like an
> interesting datapoint that fell out of scouring the web trying to find
> any evidence for which the "canonical" 1618 might be (and the Holtek
> connection was actually a coincidence from a misidentification of the
> ARTSCHIP part number).
My Sony Blu-Ray/Home Theatre has a nice one (14-segment!), also driven
by an HT16515. While the specific model predates the arrival of Linux
in the next stepping of the hardware, it should be sufficiently powerful
to run Linux.
Unfortunately it's in active use, so hacking experiments would be vetoed by
the family ;-)
Gr{oetje,eeting}s,
Geert
--
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
On Thu, 12 Dec 2019 04:39:28 +0100, =?UTF-8?q?Andreas=20F=C3=A4rber?= wrote:
> Xnano is a Chinese TV box brand, but not much more is publicly known.
>
> Signed-off-by: Andreas F?rber <[email protected]>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Thu, 12 Dec 2019 04:39:29 +0100, =?UTF-8?q?Andreas=20F=C3=A4rber?= wrote:
> Define a compatible string for Xnano X5 OTT TV Box.
>
> Signed-off-by: Andreas F?rber <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/realtek.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <[email protected]>
On Thu, 12 Dec 2019 04:39:32 +0100, =?UTF-8?q?Andreas=20F=C3=A4rber?= wrote:
> Assign vendor prefix "titanmec", matching their domain name.
>
> Cc: [email protected]
> Signed-off-by: Andreas F?rber <[email protected]>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Thu, 12 Dec 2019 04:39:37 +0100, =?UTF-8?q?Andreas=20F=C3=A4rber?= wrote:
> Assign vendor prefix "fdhisi", based on their domain name.
>
> Cc: [email protected]
> Signed-off-by: Andreas F?rber <[email protected]>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Thu, Dec 12, 2019 at 04:39:33AM +0100, Andreas F?rber wrote:
> Add a YAML schema binding for TM1628 LED controller.
>
> Cc: [email protected]
> Signed-off-by: Andreas F?rber <[email protected]>
> ---
> @Rob: How could we express constraints on two-cell reg value ranges here?
It's encoded as a matrix, so something like this:
reg:
items:
items:
- # constraints on 1st cell value
- # constraints on 2nd cell value
> Should we also model constraints on reg range by #grids property?
So the value of #grid defines the max values in reg? Unfortunately,
we can't yet for json-schema. There's been some proposals, but nothing
final I think.
> .../devicetree/bindings/leds/titanmec,tm1628.yaml | 80 ++++++++++++++++++++++
> 1 file changed, 80 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
> new file mode 100644
> index 000000000000..024875656e79
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/titanmec,tm1628.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Titan Micro Electronics TM1628 LED controller
> +
> +maintainers:
> + - Andreas F?rber <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - titanmec,tm1628
> +
> + reg:
> + maxItems: 1
> +
> + "#grids":
I'd be more a fan of using '#' prefix if we'd been better at using it
when appropriate, so I'd probably drop the '#'.
> + description: |
> + Number of GRID output lines to use.
> + This limits the number of available SEG output lines.
> + minimum: 4
> + maximum: 7
> +
> + "#address-cells":
> + const: 2
> +
> + "#size-cells":
> + const: 0
> +
> +required:
> + - compatible
> + - reg
> +
> +patternProperties:
> + "^.*@[1-7],([1-9]|1[02-4])$":
> + type: object
> + description: |
> + Properties for a single LED.
Please describe the unit-address format. I assume it's <grid>,<segment>.
> +
> + properties:
> + reg:
> + description: |
> + 1-based grid number, followed by 1-based segment number.
> + maxItems: 1
> +
> + linux,default-trigger: true
> +
> + required:
> + - reg
> +
> +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>;
> + #grids = <7>;
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + colon@5,4 {
> + reg = <5 4>;
> + color = <LED_COLOR_ID_WHITE>;
> + function = LED_FUNCTION_INDICATOR;
> + linux,default-trigger = "heartbeat";
> + };
> + };
> + };
> +...
> --
> 2.16.4
>
On Thu, 12 Dec 2019 04:39:42 +0100, =?UTF-8?q?Andreas=20F=C3=A4rber?= wrote:
> Assign vendor prefix "szfdwdz", based on their domain name.
>
> Signed-off-by: Andreas F?rber <[email protected]>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Thu, 12 Dec 2019 04:39:43 +0100, =?UTF-8?q?Andreas=20F=C3=A4rber?= wrote:
> Define a compatible string for AiP1618 chipset.
>
> Signed-off-by: Andreas F?rber <[email protected]>
> ---
> @Rob: This one would conditionally need to further restrict reg ranges then.
>
> Documentation/devicetree/bindings/leds/titanmec,tm1628.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <[email protected]>
Hi!
> This patch series implements the LED controllers found in some RTD1295 based
> TV set-top boxes.
>
> Ever since I've had mainline Linux kernels booting on my Zidoo X9S TV box,
> it's been bugging me that it kept displaying "boot" on its front display.
> A hot lead was a TM1628 chip on the front display's daughterboard, which
> English and Chinese datasheets were available for. The biggest
> hurdle
Fun :-).
> It goes on to add a "text" attribute to the driver that enables DT-configured
> seven-segment displays; I was expecting to find precedence in auxdisplay
> subsystem but came up empty. So my driver currently integrates its own
> generic (but incomplete) character-to-8-segments mapping, as well as in a
> second step a combined-characters-to-8-segments mapping, which then gets
> mapped to the chipset's available output lines. Doing this as sysfs
> device
I did not investigate this in great detail; but if it is displaying
characters, auxdisplay is probably right subsystem to handle that. I
guess LEDs can still take the low-level parts...
Oh, and common dimming for many LEDs is seen on other hardware, too
(Turris routers). Not sure how to handle that, either :-(.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> Add child nodes for individual LED indicators plus the half-second colon.
>
> Signed-off-by: Andreas F?rber <[email protected]>
> ---
> + apps@1,4 {
> + reg = <1 4>;
> + color = <LED_COLOR_ID_WHITE>;
> + function = LED_FUNCTION_INDICATOR;
> + function-enumerator = <1>;
> + };
> +
> + setup@2,4 {
> + reg = <2 4>;
> + color = <LED_COLOR_ID_WHITE>;
> + function = LED_FUNCTION_INDICATOR;
> + function-enumerator = <2>;
> + };
having 10 "indicator" leds is not going to be exactly useful.
So, what does the display look like? "apps", "setup", "usb", "card",
... texts?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> Detect DT child nodes covering more than one LED component and implement
> a text to 7-segment display mapping.
Up to here, I seen nothing totally crazy.
But I'd prefer not to have this in the LED subsystem.
Best regards,
Pavel
> Signed-off-by: Andreas F?rber <[email protected]>
> ---
> drivers/leds/leds-tm1628.c | 230 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 229 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/leds/leds-tm1628.c b/drivers/leds/leds-tm1628.c
> index 8a8fd1562853..e2c2a5d91596 100644
> --- a/drivers/leds/leds-tm1628.c
> +++ b/drivers/leds/leds-tm1628.c
> @@ -47,6 +47,15 @@ struct tm1628_info {
> int default_pwm;
> };
>
> +struct tm1628_segment {
> + u32 grid;
> + u32 seg;
> +};
> +
> +struct tm1628_display {
> + struct tm1628_segment segments[8];
> +};
> +
> struct tm1628_led {
> struct led_classdev leddev;
> struct tm1628 *ctrl;
> @@ -62,6 +71,8 @@ struct tm1628 {
> int mode_index;
> int pwm_index;
> u8 data[14];
> + unsigned int num_displays;
> + struct tm1628_display *displays;
> unsigned int num_leds;
> struct tm1628_led leds[];
> };
> @@ -253,6 +264,215 @@ static int tm1628_register_led(struct tm1628 *s,
> return devm_led_classdev_register_ext(dev, &led->leddev, &init_data);
> }
>
> +#define SSD_TOP BIT(0)
> +#define SSD_TOP_RIGHT BIT(1)
> +#define SSD_BOTTOM_RIGHT BIT(2)
> +#define SSD_BOTTOM BIT(3)
> +#define SSD_BOTTOM_LEFT BIT(4)
> +#define SSD_TOP_LEFT BIT(5)
> +#define SSD_MIDDLE BIT(6)
> +#define SSD_DOT BIT(7)
> +
> +struct tm1628_ssd_char {
> + char ch;
> + u8 segs;
> +};
> +
> +static const struct tm1628_ssd_char tm1628_char_ssd_map[] = {
> + { '0', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { '1', SSD_TOP_RIGHT | SSD_BOTTOM_RIGHT },
> + { '2', SSD_TOP | SSD_TOP_RIGHT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM },
> + { '3', SSD_TOP | SSD_TOP_RIGHT | SSD_MIDDLE | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { '4', SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE | SSD_BOTTOM_RIGHT },
> + { '5', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { '6', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { '7', SSD_TOP | SSD_TOP_RIGHT | SSD_BOTTOM_RIGHT },
> + { '8', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { '9', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
> + SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { 'A', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT },
> + { 'B', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { 'C', SSD_TOP | SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM },
> + { 'D', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { 'E', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM },
> + { 'F', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT },
> + { 'G', SSD_TOP | SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
> + { 'H', SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT },
> + { 'I', SSD_TOP_LEFT | SSD_BOTTOM_LEFT },
> + { 'J', SSD_TOP | SSD_TOP_RIGHT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { 'K', SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT },
> + { 'L', SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM },
> + { 'O', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { 'P', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
> + SSD_BOTTOM_LEFT },
> + { 'Q', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { 'R', SSD_TOP | SSD_TOP_LEFT | SSD_TOP_RIGHT | SSD_MIDDLE |
> + SSD_BOTTOM_LEFT | SSD_BOTTOM_RIGHT },
> + { 'S', SSD_TOP | SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_RIGHT | SSD_BOTTOM },
> + { 'T', SSD_TOP | SSD_TOP_LEFT | SSD_BOTTOM_LEFT },
> + { 'U', SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT | SSD_TOP_RIGHT },
> + { 'V', SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT | SSD_TOP_RIGHT },
> + { 'W', SSD_TOP_LEFT | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT | SSD_TOP_RIGHT },
> + { 'Z', SSD_TOP | SSD_TOP_RIGHT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM },
> + { 'b', SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
> + { 'l', SSD_TOP_LEFT | SSD_BOTTOM_LEFT },
> + { 'o', SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
> + { 't', SSD_TOP_LEFT | SSD_MIDDLE | SSD_BOTTOM_LEFT | SSD_BOTTOM },
> + { 'u', SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
> + { 'v', SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
> + { 'w', SSD_BOTTOM_LEFT | SSD_BOTTOM | SSD_BOTTOM_RIGHT },
> + { '-', SSD_MIDDLE },
> + { '_', SSD_BOTTOM },
> + { '.', SSD_DOT },
> +};
> +
> +static u8 tm1628_get_char_ssd_map(char ch)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(tm1628_char_ssd_map); i++) {
> + if (tm1628_char_ssd_map[i].ch == ch)
> + return tm1628_char_ssd_map[i].segs;
> + }
> +
> + return 0x0;
> +}
> +
> +static int tm1628_display_apply_map(struct tm1628 *s,
> + struct tm1628_display *display, u8 map)
> +{
> + struct tm1628_segment *segment;
> + int i;
> +
> + for (i = 0; i < 8; i++) {
> + segment = &display->segments[i];
> + tm1628_set_led(s, segment->grid, segment->seg, map & BIT(i));
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t text_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct tm1628 *s = dev_get_drvdata(dev);
> + size_t offset, len = count;
> + u8 map;
> + int i, ret;
> +
> + if (len > 0 && buf[len - 1] == '\n')
> + len--;
> +
> + for (i = 0, offset = 0; i < s->num_displays; i++) {
> + if (offset < len) {
> + map = tm1628_get_char_ssd_map(buf[offset]);
> + offset++;
> + } else
> + map = 0x0;
> +
> + tm1628_display_apply_map(s, &s->displays[i], map);
> + }
> +
> + ret = tm1628_set_address(s->spi, 0x0);
> + if (ret)
> + return ret;
> +
> + ret = tm1628_write_data(s->spi, s->data, 14);
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +static struct device_attribute tm1628_attr =
> + __ATTR_WO(text);
> +
> +static int tm1628_register_display(struct tm1628 *s,
> + struct fwnode_handle *node)
> +{
> + struct device *dev = &s->spi->dev;
> + struct tm1628_display *display;
> + u32 *reg;
> + u32 grid, seg;
> + int i, j, ret, reg_count;
> +
> + reg_count = fwnode_property_count_u32(node, "reg");
> + if (reg_count < 0)
> + return reg_count;
> +
> + if (reg_count % 2) {
> + dev_warn(dev, "Ignoring extra cell in %s reg property\n",
> + fwnode_get_name(node));
> + reg_count--;
> + }
> +
> + if (s->displays) {
> + dev_warn(dev, "Only one display supported\n");
> + return -EINVAL;
> + }
> +
> + s->num_displays = reg_count >> 1;
> +
> + reg = devm_kzalloc(dev, reg_count * sizeof(*reg), GFP_KERNEL);
> + if (!reg)
> + return -ENOMEM;
> +
> + ret = fwnode_property_read_u32_array(node, "reg", reg, reg_count);
> + if (ret) {
> + dev_err(dev, "Reading %s reg property failed (%d)\n",
> + fwnode_get_name(node), ret);
> + return ret;
> + }
> +
> + s->displays = devm_kzalloc(dev, s->num_displays * sizeof(*s->displays), GFP_KERNEL);
> + if (!s->displays)
> + return -ENOMEM;
> +
> + for (i = 0; i < s->num_displays; i++) {
> + display = &s->displays[i];
> + grid = reg[i * 2];
> + seg = reg[i * 2 + 1];
> + if (grid == 0 && seg != 0) {
> + if (!tm1628_is_valid_seg(s, seg)) {
> + dev_warn(dev, "%s reg out of range\n", fwnode_get_name(node));
> + return -EINVAL;
> + }
> + grid = s->info->modes[s->mode_index].grid_mask;
> + for (j = 0; grid && j < 7; j++) {
> + display->segments[j].seg = seg;
> + display->segments[j].grid = __ffs(grid);
> + grid &= ~BIT(display->segments[j].grid);
> + }
> + } else if (grid != 0 && seg == 0) {
> + if (!tm1628_is_valid_grid(s, grid)) {
> + dev_warn(dev, "%s reg out of range\n", fwnode_get_name(node));
> + return -EINVAL;
> + }
> + seg = s->info->modes[s->mode_index].seg_mask;
> + for (j = 0; seg && j < 8; j++) {
> + display->segments[j].grid = grid;
> + display->segments[j].seg = __ffs(seg);
> + seg &= ~BIT(display->segments[j].seg);
> + }
> + }
> + }
> +
> + devm_kfree(dev, reg);
> +
> + device_create_file(dev, &tm1628_attr);
> +
> + return 0;
> +}
> +
> /* Work around __builtin_popcount() */
> static u32 tm1628_grid_popcount(u8 grid_mask)
> {
> @@ -325,7 +545,7 @@ static int tm1628_spi_probe(struct spi_device *spi)
> return ret;
> }
>
> - if (fwnode_property_count_u32(child, "reg") == 2) {
> + if (reg[0] != 0 && reg[1] != 0 && fwnode_property_count_u32(child, "reg") == 2) {
> ret = tm1628_register_led(s, child, reg[0], reg[1], &s->leds[i++]);
> if (ret && ret != -EINVAL) {
> dev_err(&spi->dev, "Failed to register LED %s (%d)\n",
> @@ -334,6 +554,14 @@ static int tm1628_spi_probe(struct spi_device *spi)
> return ret;
> }
> s->num_leds++;
> + } else {
> + ret = tm1628_register_display(s, child);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to register display %s (%d)\n",
> + fwnode_get_name(child), ret);
> + fwnode_handle_put(child);
> + return ret;
> + }
> }
> }
>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thu 2019-12-12 04:39:49, Andreas F?rber wrote:
> Allow to squeeze the text "HEllO" into a 4-digit display,
> as seen on MeLE V9 TV box.
>
> Enable this combining mode only if the text would overflow.
"HEll,nO"!
:-)
Ok, it is kind of cool, but... Can you take a look at
drivers/auxdisplay/charlcd.c ? It seems to support some kind of text
displays...
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Pavel,
Am 21.12.19 um 21:27 schrieb Pavel Machek:
> On Thu 2019-12-12 04:39:49, Andreas F?rber wrote:
>> Allow to squeeze the text "HEllO" into a 4-digit display,
>> as seen on MeLE V9 TV box.
>>
>> Enable this combining mode only if the text would overflow.
>
> "HEll,nO"!
>
> :-)
>
> Ok, it is kind of cool, but... Can you take a look at
> drivers/auxdisplay/charlcd.c ? It seems to support some kind of text
> displays...
Why don't you look at it before making such a suggestion? ;) It is in no
way useful, as I pointed out in my cover letter. The only thing related
today, as Geert pointed out, is in the input subsystem.
If you don't want this in leds, you'll have to help make leds subsystem
more useful to external users - the latest function refactoring has been
anything but helpful here, as you've seen with the indicators, and we're
completely lacking any indexing or bulk operations on the LED controller
level, since you treat each LED as a standalone device. That's precisely
why this code is here in leds although - as I pointed out - it shouldn't
belong here.
Regards,
Andreas
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer
HRB 36809 (AG N?rnberg)
Hi!
> >>Allow to squeeze the text "HEllO" into a 4-digit display,
> >>as seen on MeLE V9 TV box.
> >>
> >>Enable this combining mode only if the text would overflow.
> >
> >"HEll,nO"!
> >
> >:-)
> >
> >Ok, it is kind of cool, but... Can you take a look at
> >drivers/auxdisplay/charlcd.c ? It seems to support some kind of text
> >displays...
>
> Why don't you look at it before making such a suggestion? ;) It is in no way
> useful, as I pointed out in my cover letter. The only thing related today,
> as Geert pointed out, is in the input subsystem.
Okay, so maybe we should get
AUXILIARY DISPLAY DRIVERS
M: Miguel Ojeda Sandonis <[email protected]>
on the Cc: list? What you really have is a display, not a bunch of LEDs.
> If you don't want this in leds, you'll have to help make leds subsystem more
> useful to external users - the latest function refactoring has been anything
> but helpful here, as you've seen with the indicators, and we're completely
> lacking any indexing or bulk operations on the LED controller level, since
> you treat each LED as a standalone device. That's precisely why this code is
> here in leds although - as I pointed out - it shouldn't belong here.
Well, your introduction mail was kind of long :-).
If someone wants to do heartbeat on
--
| | <- this segment
--
| |
--
they are probably crazy. We may not want to support that. What about
doing it as auxdisplay driver, and then exporting the indicators
around that as LEDs?
Having USB activity trigger on 'USB' icon makes sense, on the other
hand. That would still be supported.
Hmm?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
[- Roc He, - chipset vendors]
Am 21.12.19 um 19:20 schrieb Pavel Machek:
>> It goes on to add a "text" attribute to the driver that enables DT-configured
>> seven-segment displays; I was expecting to find precedence in auxdisplay
>> subsystem but came up empty. So my driver currently integrates its own
>> generic (but incomplete) character-to-8-segments mapping, as well as in a
>> second step a combined-characters-to-8-segments mapping, which then gets
>> mapped to the chipset's available output lines. Doing this as sysfs
>> device
>
> I did not investigate this in great detail; but if it is displaying
> characters, auxdisplay is probably right subsystem to handle that.
ausdisplay does not have any common API AFAICS. Most of them are
high-level displays with some parallel interface to set text and
metadata. Half of them hardcode the text to Linux or maybe offer a
Kconfig option to override it; the other half implements their own
character device file with ABI specific to that driver.
> I
> guess LEDs can still take the low-level parts...
I'd hope so, but I believe we're missing multiple things there:
1) A bulk-update API for setting multiple LEDs at once.
.brightness_set[_blocking]() is all we have on the device side, which
here results in two SPI commands. led_set_brightness[_sync]() is all I
see on the API side. We'd need an API that takes an array of LEDs and
brightness values and allows a common driver rather than individual
devices to update the Display RAM via SPI from an internal buffer.
2) DT is currently limited to one node per LED device. We'd need
#led-cells, with current LED nodes defaulting to zero. That way we could
address LEDs from an external, e.g., auxdisplay driver via a two-cell
index for these LED controllers, without needing to have DT nodes for
each and every display segment.
3) Better LED device names. More "function" values, or a reversal of the
label deprecation. Or an alternative API to register LEDs with manual name.
4) LED triggers controlling more than one LED. linux,default-trigger
seems to assign one per LED, so that two heartbeats are quickly out of
sync. Doing it from code would probably be simpler than finding a way to
model this in DT, but I don't yet see how.
Alternatively we could expose those LED output lines as a gpiochip,
which we can already index in DT, and consider the display GPIO-based,
but then we're in the situation again that GregKH was telling people to
either go screw themselves in userspace or move things into leds, which
now you're against.
Also, if you don't allow displays in leds, then we can't have LED
triggers for them either.
>
> Oh, and common dimming for many LEDs is seen on other hardware, too
> (Turris routers). Not sure how to handle that, either :-(.
That part I have indeed successfully solved with a backlight device.
My current problem (WIP blocking a push) is the key input handling - not
sure how to model both LEDs and keys as DT child nodes - do we need a
compatible to distinguish between them? Unit addresses and reg values
would be in different ranges, making this awkward, not to mention the
problem of naming a compatible, given the incredible diverse chipsets.
Regards,
Andreas
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer
HRB 36809 (AG N?rnberg)
Hi,
Am 21.12.19 um 22:04 schrieb Pavel Machek:
>>>> Allow to squeeze the text "HEllO" into a 4-digit display,
>>>> as seen on MeLE V9 TV box.
>>>>
>>>> Enable this combining mode only if the text would overflow.
>>>
>>> "HEll,nO"!
>>>
>>> :-)
>>>
>>> Ok, it is kind of cool, but... Can you take a look at
>>> drivers/auxdisplay/charlcd.c ? It seems to support some kind of text
>>> displays...
>>
>> Why don't you look at it before making such a suggestion? ;) It is in no way
>> useful, as I pointed out in my cover letter. The only thing related today,
>> as Geert pointed out, is in the input subsystem.
>
> Okay, so maybe we should get
>
> AUXILIARY DISPLAY DRIVERS
> M: Miguel Ojeda Sandonis <[email protected]>
>
> on the Cc: list?
Let's see if that email still exists - the code looked ancient, full of
platform_data and driver-specific exported functions...
(the Yealink input driver was from 2005, too)
> What you really have is a display, not a bunch of LEDs.
We have an LED Controller connected to zero, one or more displays.
They are most certainly _not_ the same thing.
>> If you don't want this in leds, you'll have to help make leds subsystem more
>> useful to external users - the latest function refactoring has been anything
>> but helpful here, as you've seen with the indicators, and we're completely
>> lacking any indexing or bulk operations on the LED controller level, since
>> you treat each LED as a standalone device. That's precisely why this code is
>> here in leds although - as I pointed out - it shouldn't belong here.
>
> Well, your introduction mail was kind of long :-).
>
> If someone wants to do heartbeat on
>
> --
> | | <- this segment
> --
> | |
> --
>
> they are probably crazy. We may not want to support that. What about
> doing it as auxdisplay driver, and then exporting the indicators
> around that as LEDs?
You're really just discussing which directory to put this file into -
moving it around is the easiest thing...
>
> Having USB activity trigger on 'USB' icon makes sense, on the other
> hand. That would still be supported.
Actually I disagree about those indicators - that was the reason they're
indicators and not, e.g., "usb". IMO people would go crazy if large text
like that blinked during USB transfers. I assumed the meaning of those
LEDs were to indicate whether a USB/SD medium is connected, which I did
not see any better function for, and I'm not aware of us having such
triggers today.
Maybe you also overread that with trigger I was referring to using RTC
as trigger for a) the colon blinking every half-second and b) the text
getting updated based on avsilable RTC interrupts?
You could also think of GPIO-connected LEDs that you may want to animate
without two different heartbeats/timers getting out of sync. Or think of
an RGB LED that today we sadly need to model as multiple GPIO LEDs
instead of as one with a color property we can change (and hardcoding a
color in DT/name is not helping that use case either).
auxdisplay offers no API that I could register with to drive output, nor
any triggers to automate such output - that's unique to LEDs. Like I
said, we can place this spi_driver file into auxdisplay/, but that
doesn't solve the driver design. So I really think we need to decouple
the two and keep the LED Controller driver in leds and the display logic
elsewhere, with suitable new APIs to connect them. We're lacking
suggestions for the how, on DT and API levels - see my response on the
cover letter.
Regards,
Andreas
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer
HRB 36809 (AG N?rnberg)
Hi Miguel,
Am 22.12.19 um 00:12 schrieb Miguel Ojeda:
> On Sat, 21 Dec 2019 at 22:49 Andreas Färber <[email protected]
> <mailto:[email protected]>> wrote:
>
> Hi,
>
> Am 21.12.19 um 22:04 schrieb Pavel Machek:
> >>>> Allow to squeeze the text "HEllO" into a 4-digit display,
> >>>> as seen on MeLE V9 TV box.
> >>>>
> >>>> Enable this combining mode only if the text would overflow.
> >>>
> >>> "HEll,nO"!
> >>>
> >>> :-)
> >>>
> >>> Ok, it is kind of cool, but... Can you take a look at
> >>> drivers/auxdisplay/charlcd.c ? It seems to support some kind of
> text
> >>> displays...
> >>
> >> Why don't you look at it before making such a suggestion? ;) It
> is in no way
> >> useful, as I pointed out in my cover letter. The only thing
> related today,
> >> as Geert pointed out, is in the input subsystem.
> >
> > Okay, so maybe we should get
> >
> > AUXILIARY DISPLAY DRIVERS
> > M: Miguel Ojeda Sandonis <[email protected]
> <mailto:[email protected]>>
> >
> > on the Cc: list?
>
> Let's see if that email still exists - the code looked ancient, full of
> platform_data and driver-specific exported functions...
>
> (the Yealink input driver was from 2005, too)
>
>
> The code may look ancient, but the email surely exists ;)
>
>
>
> > What you really have is a display, not a bunch of LEDs.
>
> We have an LED Controller connected to zero, one or more displays.
> They are most certainly _not_ the same thing.
>
> >> If you don't want this in leds, you'll have to help make leds
> subsystem more
> >> useful to external users - the latest function refactoring has
> been anything
> >> but helpful here, as you've seen with the indicators, and we're
> completely
> >> lacking any indexing or bulk operations on the LED controller
> level, since
> >> you treat each LED as a standalone device. That's precisely why
> this code is
> >> here in leds although - as I pointed out - it shouldn't belong here.
> >
> > Well, your introduction mail was kind of long :-).
> >
> > If someone wants to do heartbeat on
> >
> > --
> > | | <- this segment
> > --
> > | |
> > --
> >
> > they are probably crazy. We may not want to support that. What about
> > doing it as auxdisplay driver, and then exporting the indicators
> > around that as LEDs?
>
> You're really just discussing which directory to put this file into -
> moving it around is the easiest thing...
>
> >
> > Having USB activity trigger on 'USB' icon makes sense, on the other
> > hand. That would still be supported.
>
> Actually I disagree about those indicators - that was the reason
> they're
> indicators and not, e.g., "usb". IMO people would go crazy if large
> text
> like that blinked during USB transfers. I assumed the meaning of those
> LEDs were to indicate whether a USB/SD medium is connected, which I did
> not see any better function for, and I'm not aware of us having such
> triggers today.
>
> Maybe you also overread that with trigger I was referring to using RTC
> as trigger for a) the colon blinking every half-second and b) the text
> getting updated based on avsilable RTC interrupts?
>
> You could also think of GPIO-connected LEDs that you may want to
> animate
> without two different heartbeats/timers getting out of sync. Or
> think of
> an RGB LED that today we sadly need to model as multiple GPIO LEDs
> instead of as one with a color property we can change (and hardcoding a
> color in DT/name is not helping that use case either).
>
> auxdisplay offers no API that I could register with to drive output,
> nor
> any triggers to automate such output - that's unique to LEDs. Like I
> said, we can place this spi_driver file into auxdisplay/, but that
> doesn't solve the driver design.
>
>
> I think it would be alright to put it in auxdisplay.
Please find the full series on, e.g., LAKML:
https://patchwork.kernel.org/cover/11286939/
> So I really think we need to decouple
> the two and keep the LED Controller driver in leds and the display
> logic
> elsewhere, with suitable new APIs to connect them. We're lacking
> suggestions for the how, on DT and API levels - see my response on
> [...]the
> cover letter.
As explained in the cover letter, the problem here is that I don't know
the model or manufacturer of these unmarked white-plastic-frame LED
displays. So we have neither a filename to use in auxdisplay/ nor a DT
compatible string to use for those displays.
Cheers,
Andreas
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)
Am 12.12.19 um 04:39 schrieb Andreas Färber:
> Prepended is a new DT for Xnano X5 OTT TV Box, featuring an FD628 display.
[...]
> Andreas Färber (25):
> dt-bindings: vendor-prefixes: Add Xnano
> dt-bindings: arm: realtek: Add Xnano X5
> arm64: dts: realtek: rtd1295: Add Xnano X5
[snip]
Applied these three to linux-realtek.git v5.6/dt:
https://git.kernel.org/pub/scm/linux/kernel/git/afaerber/linux-realtek.git/log/?h=v5.6/dt
Thanks,
Andreas
--
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)
Hi Andreas,
On Sun, Dec 22, 2019 at 4:14 AM Andreas Färber <[email protected]> wrote:
>
> Please find the full series on, e.g., LAKML:
>
> https://patchwork.kernel.org/cover/11286939/
Sorry for the late reply. Thanks for the pointer!
> As explained in the cover letter, the problem here is that I don't know
> the model or manufacturer of these unmarked white-plastic-frame LED
> displays. So we have neither a filename to use in auxdisplay/ nor a DT
> compatible string to use for those displays.
For the filename, I don't think it is a big deal. Just use whatever
you think would fit best, even if it is a generic name. The most
important bit is having a clear Kconfig (and being disabled by
default).
Cheers,
Miguel
Hi Robin,
Hi Andreas,
> On 13 Dec 2019, at 15:07, Robin Murphy <[email protected]> wrote:
>
> I also have one of the H96 Max boxes (which I picked up out of curiosity
> for the mysterious RK3318) with an FD6551, although I've not attacked
> that one with the logic analyser yet to see how similar it is.
I have a T9 (RK3328) TV box with the same chip in it. The FD6551 uses an
I2C-like protocol. Every digit (and the symbols) have an I2C address,
but, the display does not signal ACK. AFAIK the FD650 and FD655 which
are used in other boxes (Amlogic) are very similar.
So far, I have whipped up a proof-of-cocept driver that uses i2c-gpio.
The digits seem to be rotated by 180 degrees. So, in order to use
map_to_7segment.h I had to define the BIT_SEG7_* constants differently.
My display also has multiple symbols (WIFI, network, pause, play, USB,
alarm) that are controlled by writing to the same address as for the
colon.
I’d love to work on a driver (similar to Andreas’ SPI based driver) for
these I2C connected chips.
Cheers,
Ezra.
Hi!
> > On 13 Dec 2019, at 15:07, Robin Murphy <[email protected]> wrote:
> >
> > I also have one of the H96 Max boxes (which I picked up out of curiosity
> > for the mysterious RK3318) with an FD6551, although I've not attacked
> > that one with the logic analyser yet to see how similar it is.
>
> I have a T9 (RK3328) TV box with the same chip in it. The FD6551 uses an
> I2C-like protocol. Every digit (and the symbols) have an I2C address,
> but, the display does not signal ACK. AFAIK the FD650 and FD655 which
> are used in other boxes (Amlogic) are very similar.
>
> So far, I have whipped up a proof-of-cocept driver that uses i2c-gpio.
> The digits seem to be rotated by 180 degrees. So, in order to use
> map_to_7segment.h I had to define the BIT_SEG7_* constants differently.
> My display also has multiple symbols (WIFI, network, pause, play, USB,
> alarm) that are controlled by writing to the same address as for the
> colon.
>
> I’d love to work on a driver (similar to Andreas’ SPI based driver) for
> these I2C connected chips.
Create a driver in drivers/auxdisplay for alphanumeric parts. You can
then export any remaining symbols as LEDs if it will provide benefits.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html