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

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

[Heiner Kallweit]

This series adds support for the Titanmec TM1628 7 segment display
controller. It's based on previous RFC work from Andreas Färber.
The RFC version placed the driver in the LED subsystem, but this was
NAK'ed by the LED maintainer. Therefore I moved the driver to
/drivers/auxdisplay what seems most reasonable to me.

To be decided is through which tree this series should go.
I'd think SPI would be most suited, but that's a decision I
leave up to the respective maintainers.

Further changes to the RFC version:
- Driver can be built also w/o LED class support, for displays that
  don't have any symbols to be exposed as LED's.
- Simplified the code and rewrote a lot of it.
- Driver is now kind of a MVP, but functionality should be sufficient
  for most use cases.
- Use the existing 7 segment support in uapi/linux/map_to_7segment.h
  as suggested by Geert Uytterhoeven.

Note: There's a number of chips from other manufacturers that are
      almost identical, e.g. FD628, SM1628. Only difference I saw so
      far is that they partially support other display modes.
      TM1628: 6x12, 7x11
      SM1628C: 4x13, 5x12, 6x11, 7x10
      For typical displays on devices using these chips this
      difference shouldn't matter.

Successfully tested on a TX3 Mini TV box that has an SM1628C and a
display with 4 digits and 7 symbols.

Andreas Färber (2):
  spi: gpio: Implement LSB First bitbang support
  dt-bindings: vendor-prefixes: Add Titan Micro Electronics

Heiner Kallweit (4):
  dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628
  docs: ABI: document tm1628 attribute display-text
  auxdisplay: add support for Titanmec TM1628 7 segment display
    controller
  arm64: dts: meson-gxl-s905w-tx3-mini: add support for the 7 segment
    display

 .../testing/sysfs-devices-auxdisplay-tm1628   |   7 +
 .../bindings/auxdisplay/titanmec,tm1628.yaml  |  82 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 .../dts/amlogic/meson-gxl-s905w-tx3-mini.dts  |  59 +++
 drivers/auxdisplay/Kconfig                    |  10 +
 drivers/auxdisplay/Makefile                   |   1 +
 drivers/auxdisplay/tm1628.c                   | 368 ++++++++++++++++++
 drivers/spi/spi-bitbang-txrx.h                |  66 ++++
 drivers/spi/spi-gpio.c                        |  42 +-
 9 files changed, 628 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
 create mode 100644 drivers/auxdisplay/tm1628.c

-- 
2.35.1

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

[Heiner Kallweit]

This series adds support for the Titanmec TM1628 7 segment display
controller. It's based on previous RFC work from Andreas Färber.
The RFC version placed the driver in the LED subsystem, but this was
NAK'ed by the LED maintainer. Therefore I moved the driver to
/drivers/auxdisplay what seems most reasonable to me.

To be decided is through which tree this series should go.
I'd think SPI would be most suited, but that's a decision I
leave up to the respective maintainers.

Further changes to the RFC version:
- Driver can be built also w/o LED class support, for displays that
  don't have any symbols to be exposed as LED's.
- Simplified the code and rewrote a lot of it.
- Driver is now kind of a MVP, but functionality should be sufficient
  for most use cases.
- Use the existing 7 segment support in uapi/linux/map_to_7segment.h
  as suggested by Geert Uytterhoeven.

Note: There's a number of chips from other manufacturers that are
      almost identical, e.g. FD628, SM1628. Only difference I saw so
      far is that they partially support other display modes.
      TM1628: 6x12, 7x11
      SM1628C: 4x13, 5x12, 6x11, 7x10
      For typical displays on devices using these chips this
      difference shouldn't matter.

Successfully tested on a TX3 Mini TV box that has an SM1628C and a
display with 4 digits and 7 symbols.

Andreas Färber (2):
  spi: gpio: Implement LSB First bitbang support
  dt-bindings: vendor-prefixes: Add Titan Micro Electronics

Heiner Kallweit (4):
  dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628
  docs: ABI: document tm1628 attribute display-text
  auxdisplay: add support for Titanmec TM1628 7 segment display
    controller
  arm64: dts: meson-gxl-s905w-tx3-mini: add support for the 7 segment
    display

 .../testing/sysfs-devices-auxdisplay-tm1628   |   7 +
 .../bindings/auxdisplay/titanmec,tm1628.yaml  |  82 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 .../dts/amlogic/meson-gxl-s905w-tx3-mini.dts  |  59 +++
 drivers/auxdisplay/Kconfig                    |  10 +
 drivers/auxdisplay/Makefile                   |   1 +
 drivers/auxdisplay/tm1628.c                   | 368 ++++++++++++++++++
 drivers/spi/spi-bitbang-txrx.h                |  66 ++++
 drivers/spi/spi-gpio.c                        |  42 +-
 9 files changed, 628 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
 create mode 100644 drivers/auxdisplay/tm1628.c

-- 
2.35.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Heiner Kallweit]

This series adds support for the Titanmec TM1628 7 segment display
controller. It's based on previous RFC work from Andreas Färber.
The RFC version placed the driver in the LED subsystem, but this was
NAK'ed by the LED maintainer. Therefore I moved the driver to
/drivers/auxdisplay what seems most reasonable to me.

To be decided is through which tree this series should go.
I'd think SPI would be most suited, but that's a decision I
leave up to the respective maintainers.

Further changes to the RFC version:
- Driver can be built also w/o LED class support, for displays that
  don't have any symbols to be exposed as LED's.
- Simplified the code and rewrote a lot of it.
- Driver is now kind of a MVP, but functionality should be sufficient
  for most use cases.
- Use the existing 7 segment support in uapi/linux/map_to_7segment.h
  as suggested by Geert Uytterhoeven.

Note: There's a number of chips from other manufacturers that are
      almost identical, e.g. FD628, SM1628. Only difference I saw so
      far is that they partially support other display modes.
      TM1628: 6x12, 7x11
      SM1628C: 4x13, 5x12, 6x11, 7x10
      For typical displays on devices using these chips this
      difference shouldn't matter.

Successfully tested on a TX3 Mini TV box that has an SM1628C and a
display with 4 digits and 7 symbols.

Andreas Färber (2):
  spi: gpio: Implement LSB First bitbang support
  dt-bindings: vendor-prefixes: Add Titan Micro Electronics

Heiner Kallweit (4):
  dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628
  docs: ABI: document tm1628 attribute display-text
  auxdisplay: add support for Titanmec TM1628 7 segment display
    controller
  arm64: dts: meson-gxl-s905w-tx3-mini: add support for the 7 segment
    display

 .../testing/sysfs-devices-auxdisplay-tm1628   |   7 +
 .../bindings/auxdisplay/titanmec,tm1628.yaml  |  82 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 .../dts/amlogic/meson-gxl-s905w-tx3-mini.dts  |  59 +++
 drivers/auxdisplay/Kconfig                    |  10 +
 drivers/auxdisplay/Makefile                   |   1 +
 drivers/auxdisplay/tm1628.c                   | 368 ++++++++++++++++++
 drivers/spi/spi-bitbang-txrx.h                |  66 ++++
 drivers/spi/spi-gpio.c                        |  42 +-
 9 files changed, 628 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
 create mode 100644 drivers/auxdisplay/tm1628.c

-- 
2.35.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] spi: gpio: Implement LSB First bitbang support

[Heiner Kallweit]

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

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.
Conditionally call them from all the spi-gpio txrx_word callbacks.

Some alternatives to this implementation approach were discussed back
then [0], but eventually it was considered reasonable.

[0] https://lore.kernel.org/linux-arm-kernel/20191212033952.5967-8-afaerber@suse.de/

Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/spi/spi-bitbang-txrx.h | 66 ++++++++++++++++++++++++++++++++++
 drivers/spi/spi-gpio.c         | 42 +++++++++++++++++-----
 2 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h
index ae61d72c7..267342dfa 100644
--- a/drivers/spi/spi-bitbang-txrx.h
+++ b/drivers/spi/spi-bitbang-txrx.h
@@ -41,6 +41,8 @@
  * chips need ... there may be several reasons you'd need to tweak timings
  * in these routines, not just to make it faster or slower to match a
  * particular CPU clock rate.
+ *
+ * ToDo: Maybe the bitrev macros can be used to improve the code?
  */
 
 static inline u32
@@ -106,3 +108,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 0584f4d2f..4b12c4964 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);
 }
 
 /*----------------------------------------------------------------------*/
@@ -378,7 +402,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.35.1

[PATCH 1/6] spi: gpio: Implement LSB First bitbang support

[Heiner Kallweit]

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

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.
Conditionally call them from all the spi-gpio txrx_word callbacks.

Some alternatives to this implementation approach were discussed back
then [0], but eventually it was considered reasonable.

[0] https://lore.kernel.org/linux-arm-kernel/20191212033952.5967-8-afaerber@suse.de/

Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/spi/spi-bitbang-txrx.h | 66 ++++++++++++++++++++++++++++++++++
 drivers/spi/spi-gpio.c         | 42 +++++++++++++++++-----
 2 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h
index ae61d72c7..267342dfa 100644
--- a/drivers/spi/spi-bitbang-txrx.h
+++ b/drivers/spi/spi-bitbang-txrx.h
@@ -41,6 +41,8 @@
  * chips need ... there may be several reasons you'd need to tweak timings
  * in these routines, not just to make it faster or slower to match a
  * particular CPU clock rate.
+ *
+ * ToDo: Maybe the bitrev macros can be used to improve the code?
  */
 
 static inline u32
@@ -106,3 +108,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 0584f4d2f..4b12c4964 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);
 }
 
 /*----------------------------------------------------------------------*/
@@ -378,7 +402,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.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 1/6] spi: gpio: Implement LSB First bitbang support

[Heiner Kallweit]

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

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.
Conditionally call them from all the spi-gpio txrx_word callbacks.

Some alternatives to this implementation approach were discussed back
then [0], but eventually it was considered reasonable.

[0] https://lore.kernel.org/linux-arm-kernel/20191212033952.5967-8-afaerber@suse.de/

Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/spi/spi-bitbang-txrx.h | 66 ++++++++++++++++++++++++++++++++++
 drivers/spi/spi-gpio.c         | 42 +++++++++++++++++-----
 2 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h
index ae61d72c7..267342dfa 100644
--- a/drivers/spi/spi-bitbang-txrx.h
+++ b/drivers/spi/spi-bitbang-txrx.h
@@ -41,6 +41,8 @@
  * chips need ... there may be several reasons you'd need to tweak timings
  * in these routines, not just to make it faster or slower to match a
  * particular CPU clock rate.
+ *
+ * ToDo: Maybe the bitrev macros can be used to improve the code?
  */
 
 static inline u32
@@ -106,3 +108,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 0584f4d2f..4b12c4964 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);
 }
 
 /*----------------------------------------------------------------------*/
@@ -378,7 +402,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.35.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] dt-bindings: vendor-prefixes: Add Titan Micro Electronics

[Heiner Kallweit]

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

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

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index ebe294516..38bfb462c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1229,6 +1229,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.35.1

[PATCH 2/6] dt-bindings: vendor-prefixes: Add Titan Micro Electronics

[Heiner Kallweit]

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

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

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index ebe294516..38bfb462c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1229,6 +1229,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.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 2/6] dt-bindings: vendor-prefixes: Add Titan Micro Electronics

[Heiner Kallweit]

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

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

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index ebe294516..38bfb462c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1229,6 +1229,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.35.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

[Heiner Kallweit]

Add a YAML schema binding for TM1628 auxdisplay
(7/11-segment LED) controller.

This patch is partially based on previous RFC work from
Andreas Färber <afaerber@suse.de>.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 .../bindings/auxdisplay/titanmec,tm1628.yaml  | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml

diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
new file mode 100644
index 000000000..d9cbbc950
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
@@ -0,0 +1,82 @@
+# 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
+
+properties:
+  compatible:
+    enum:
+    - titanmec,tm1628
+
+  reg:
+    maxItems: 1
+
+  grid:
+    description:
+      Mapping of display digit position to grid number.
+      This implicitly defines the display size.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 1
+    maxItems: 7
+
+  segment-mapping:
+    description:
+      Mapping of 7 segment display segments A-G to bit numbers 1-12.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 7
+    maxItems: 7
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+patternProperties:
+  "^.*@[1-7],([1-9]|1[0-6])$":
+    type: object
+    description: |
+      Properties for a single LED.
+
+    properties:
+      reg:
+        description: |
+          1-based grid number, followed by 1-based segment bit number.
+        maxItems: 1
+
+    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>;
+            grid = /bits/ 8 <4 3 2 1>;
+            segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>;
+            #address-cells = <2>;
+            #size-cells = <0>;
+
+            alarmn@5,4 {
+                reg = <5 4>;
+                function = LED_FUNCTION_ALARM;
+            };
+        };
+    };
+...
-- 
2.35.1

[PATCH 3/6] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

[Heiner Kallweit]

Add a YAML schema binding for TM1628 auxdisplay
(7/11-segment LED) controller.

This patch is partially based on previous RFC work from
Andreas Färber <afaerber@suse.de>.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 .../bindings/auxdisplay/titanmec,tm1628.yaml  | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml

diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
new file mode 100644
index 000000000..d9cbbc950
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
@@ -0,0 +1,82 @@
+# 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
+
+properties:
+  compatible:
+    enum:
+    - titanmec,tm1628
+
+  reg:
+    maxItems: 1
+
+  grid:
+    description:
+      Mapping of display digit position to grid number.
+      This implicitly defines the display size.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 1
+    maxItems: 7
+
+  segment-mapping:
+    description:
+      Mapping of 7 segment display segments A-G to bit numbers 1-12.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 7
+    maxItems: 7
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+patternProperties:
+  "^.*@[1-7],([1-9]|1[0-6])$":
+    type: object
+    description: |
+      Properties for a single LED.
+
+    properties:
+      reg:
+        description: |
+          1-based grid number, followed by 1-based segment bit number.
+        maxItems: 1
+
+    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>;
+            grid = /bits/ 8 <4 3 2 1>;
+            segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>;
+            #address-cells = <2>;
+            #size-cells = <0>;
+
+            alarmn@5,4 {
+                reg = <5 4>;
+                function = LED_FUNCTION_ALARM;
+            };
+        };
+    };
+...
-- 
2.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 3/6] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

[Heiner Kallweit]

Add a YAML schema binding for TM1628 auxdisplay
(7/11-segment LED) controller.

This patch is partially based on previous RFC work from
Andreas Färber <afaerber@suse.de>.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 .../bindings/auxdisplay/titanmec,tm1628.yaml  | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml

diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
new file mode 100644
index 000000000..d9cbbc950
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
@@ -0,0 +1,82 @@
+# 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
+
+properties:
+  compatible:
+    enum:
+    - titanmec,tm1628
+
+  reg:
+    maxItems: 1
+
+  grid:
+    description:
+      Mapping of display digit position to grid number.
+      This implicitly defines the display size.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 1
+    maxItems: 7
+
+  segment-mapping:
+    description:
+      Mapping of 7 segment display segments A-G to bit numbers 1-12.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 7
+    maxItems: 7
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+patternProperties:
+  "^.*@[1-7],([1-9]|1[0-6])$":
+    type: object
+    description: |
+      Properties for a single LED.
+
+    properties:
+      reg:
+        description: |
+          1-based grid number, followed by 1-based segment bit number.
+        maxItems: 1
+
+    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>;
+            grid = /bits/ 8 <4 3 2 1>;
+            segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>;
+            #address-cells = <2>;
+            #size-cells = <0>;
+
+            alarmn@5,4 {
+                reg = <5 4>;
+                function = LED_FUNCTION_ALARM;
+            };
+        };
+    };
+...
-- 
2.35.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] docs: ABI: document tm1628 attribute display-text

[Heiner Kallweit]

Document the attribute for reading / writing the text to be displayed on
the 7 segment display.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628 | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628

diff --git a/Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628 b/Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628
new file mode 100644
index 000000000..382757e72
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628
@@ -0,0 +1,7 @@
+What:		/sys/devices/.../display-text
+Date:		February 2022
+Contact:	Heiner Kallweit <hkallweit1@gmail.com>
+Description:
+		The text to be displayed on the 7 segment display.
+		Any printable character is allowed as input, but some
+		can not be displayed in a readable way with 7 segments.
-- 
2.35.1

[PATCH 4/6] docs: ABI: document tm1628 attribute display-text

[Heiner Kallweit]

Document the attribute for reading / writing the text to be displayed on
the 7 segment display.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628 | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628

diff --git a/Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628 b/Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628
new file mode 100644
index 000000000..382757e72
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628
@@ -0,0 +1,7 @@
+What:		/sys/devices/.../display-text
+Date:		February 2022
+Contact:	Heiner Kallweit <hkallweit1@gmail.com>
+Description:
+		The text to be displayed on the 7 segment display.
+		Any printable character is allowed as input, but some
+		can not be displayed in a readable way with 7 segments.
-- 
2.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 4/6] docs: ABI: document tm1628 attribute display-text

[Heiner Kallweit]

Document the attribute for reading / writing the text to be displayed on
the 7 segment display.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628 | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628

diff --git a/Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628 b/Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628
new file mode 100644
index 000000000..382757e72
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-auxdisplay-tm1628
@@ -0,0 +1,7 @@
+What:		/sys/devices/.../display-text
+Date:		February 2022
+Contact:	Heiner Kallweit <hkallweit1@gmail.com>
+Description:
+		The text to be displayed on the 7 segment display.
+		Any printable character is allowed as input, but some
+		can not be displayed in a readable way with 7 segments.
-- 
2.35.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] auxdisplay: add support for Titanmec TM1628 7 segment display controller

[Heiner Kallweit]

This patch adds support for the Titanmec TM1628 7 segment display
controller. It's based on previous RFC work from Andreas Färber.
The RFC version placed the driver in the LED subsystem, but this was
NAK'ed by the LED maintainer. Therefore I moved the driver to
/drivers/auxdisplay what seems most reasonable to me.

Further changes to the RFC version:
- Driver can be built also w/o LED class support, for displays that
  don't have any symbols to be exposed as LED's.
- Simplified the code and rewrote a lot of it.
- Driver is now kind of a MVP, but functionality should be sufficient
  for most use cases.
- Use the existing 7 segment support in uapi/linux/map_to_7segment.h
  as suggested by Geert Uytterhoeven.

Note: There's a number of chips from other manufacturers that are
      almost identical, e.g. FD628, SM1628. Only difference I saw so
      far is that they partially support other display modes.
      TM1628: 6x12, 7x11
      SM1628C: 4x13, 5x12, 6x11, 7x10
      For typical displays on devices using these chips this
      difference shouldn't matter.

Successfully tested on a TX3 Mini TV box that has an SM1628C and a
display with 4 digits and 7 symbols.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/auxdisplay/Kconfig  |  10 +
 drivers/auxdisplay/Makefile |   1 +
 drivers/auxdisplay/tm1628.c | 368 ++++++++++++++++++++++++++++++++++++
 3 files changed, 379 insertions(+)
 create mode 100644 drivers/auxdisplay/tm1628.c

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 64012cda4..25ef2e452 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -203,6 +203,16 @@ config ARM_CHARLCD
 	  line and the Linux version on the second line, but that's
 	  still useful.
 
+config TM1628
+	tristate "TM1628 driver for LED 7/11 segment displays"
+	depends on SPI
+	depends on OF || COMPILE_TEST
+	help
+	  Say Y to enable support for Titan Micro Electronics TM1628
+	  LED controller.
+	  It's a 3-wire SPI device controlling a two-dimensional grid of
+	  LEDs. Dimming is applied to all outputs through an internal PWM.
+
 menuconfig PARPORT_PANEL
 	tristate "Parallel port LCD/Keypad Panel support"
 	depends on PARPORT
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index 6968ed4d3..7728e17e1 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_HT16K33)		+= ht16k33.o
 obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
 obj-$(CONFIG_LCD2S)		+= lcd2s.o
 obj-$(CONFIG_LINEDISP)		+= line-display.o
+obj-$(CONFIG_TM1628)		+= tm1628.o
diff --git a/drivers/auxdisplay/tm1628.c b/drivers/auxdisplay/tm1628.c
new file mode 100644
index 000000000..a39b63828
--- /dev/null
+++ b/drivers/auxdisplay/tm1628.c
@@ -0,0 +1,368 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Titan Micro Electronics TM1628 LED controller
+ *
+ * Copyright (c) 2019 Andreas Färber
+ */
+
+#include <linux/delay.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/spi/spi.h>
+#include <uapi/linux/map_to_7segment.h>
+
+#define TM1628_CMD_DISPLAY_MODE		(0 << 6)
+#define TM1628_DISPLAY_MODE_6_12	0x02
+#define TM1628_DISPLAY_MODE_7_11	0x03
+
+#define TM1628_CMD_DATA			(1 << 6)
+#define TM1628_DATA_TEST_MODE		BIT(3)
+#define TM1628_DATA_FIXED_ADDR		BIT(2)
+#define TM1628_DATA_WRITE_DATA		0x00
+#define TM1628_DATA_READ_DATA		0x02
+
+#define TM1628_CMD_DISPLAY_CTRL		(2 << 6)
+#define TM1628_DISPLAY_CTRL_DISPLAY_ON	BIT(3)
+
+#define TM1628_CMD_SET_ADDRESS		(3 << 6)
+
+#define TM1628_BRIGHTNESS_MAX		7
+
+/* Physical limits, depending on the mode the chip may support less */
+#define MAX_GRID_SIZE			7
+#define MAX_SEGMENT_NUM			16
+
+struct tm1628_led {
+	struct led_classdev	leddev;
+	struct tm1628		*ctrl;
+	u32			grid;
+	u32			seg;
+};
+
+struct tm1628 {
+	struct spi_device		*spi;
+	__le16				data[MAX_GRID_SIZE];
+	struct mutex			disp_lock;
+	char				text[MAX_GRID_SIZE + 1];
+	u8				segment_mapping[7];
+	u8				grid[MAX_GRID_SIZE];
+	int				grid_size;
+	struct tm1628_led		leds[];
+};
+
+/* Command 1: Display Mode Setting */
+static int tm1628_set_display_mode(struct spi_device *spi, u8 grid_mode)
+{
+	u8 cmd = TM1628_CMD_DISPLAY_MODE | grid_mode;
+
+	return spi_write(spi, &cmd, 1);
+}
+
+/* Command 3: Address Setting */
+static int tm1628_set_address(struct spi_device *spi, u8 offset)
+{
+	u8 cmd = TM1628_CMD_SET_ADDRESS | (offset * sizeof(__le16));
+
+	return spi_write(spi, &cmd, 1);
+}
+
+/* Command 2: Data Setting */
+static int tm1628_write_data(struct spi_device *spi, unsigned int offset,
+			     unsigned int len)
+{
+	struct tm1628 *s = spi_get_drvdata(spi);
+	u8 cmd = TM1628_CMD_DATA | TM1628_DATA_WRITE_DATA;
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &cmd,
+			.len = 1,
+		},
+		{
+			.tx_buf = (__force void *)(s->data + offset),
+			.len = len * sizeof(__le16),
+		},
+	};
+
+	if (offset + len > MAX_GRID_SIZE) {
+		dev_err(&spi->dev, "Invalid data address offset %u len %u\n",
+			offset, len);
+		return -EINVAL;
+	}
+
+	tm1628_set_address(spi, offset);
+
+	return spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
+}
+
+/* Command 4: Display Control */
+static int tm1628_set_display_ctrl(struct spi_device *spi, bool on)
+{
+	u8 cmd = TM1628_CMD_DISPLAY_CTRL | TM1628_BRIGHTNESS_MAX;
+
+	if (on)
+		cmd |= TM1628_DISPLAY_CTRL_DISPLAY_ON;
+
+	return spi_write(spi, &cmd, 1);
+}
+
+static int tm1628_show_text(struct tm1628 *s)
+{
+	static SEG7_CONVERSION_MAP(map_seg7, MAP_ASCII7SEG_ALPHANUM);
+	int i, ret;
+
+	int msg_len = strlen(s->text);
+
+	mutex_lock(&s->disp_lock);
+
+	for (i = 0; i < s->grid_size; i++) {
+		int pos = s->grid[i] - 1;
+
+		if (i < msg_len) {
+			int char7_raw = map_to_seg7(&map_seg7, s->text[i]);
+			int j, char7;
+
+			for (j = 0, char7 = 0; j < 7; j++) {
+				if (char7_raw & BIT(j))
+					char7 |= BIT(s->segment_mapping[j] - 1);
+			}
+
+			s->data[pos] = cpu_to_le16(char7);
+		} else {
+			s->data[pos] = 0;
+		}
+	}
+
+	ret = tm1628_write_data(s->spi, 0, s->grid_size);
+
+	mutex_unlock(&s->disp_lock);
+
+	return ret;
+}
+
+static int tm1628_led_set_brightness(struct led_classdev *led_cdev,
+				     enum led_brightness brightness)
+{
+	struct tm1628_led *led = container_of(led_cdev, struct tm1628_led, leddev);
+	struct tm1628 *s = led->ctrl;
+	int offset, ret;
+	__le16 bit;
+
+	offset = led->grid - 1;
+	bit = cpu_to_le16(BIT(led->seg - 1));
+
+	mutex_lock(&s->disp_lock);
+
+	if (brightness == LED_OFF)
+		s->data[offset] &= ~bit;
+	else
+		s->data[offset] |= bit;
+
+	ret = tm1628_write_data(s->spi, offset, 1);
+
+	mutex_unlock(&s->disp_lock);
+
+	return ret;
+}
+
+static enum led_brightness tm1628_led_get_brightness(struct led_classdev *led_cdev)
+{
+	struct tm1628_led *led = container_of(led_cdev, struct tm1628_led, leddev);
+	struct tm1628 *s = led->ctrl;
+	int offset;
+	__le16 bit;
+	bool on;
+
+	offset = led->grid - 1;
+	bit = cpu_to_le16(BIT(led->seg - 1));
+
+	mutex_lock(&s->disp_lock);
+	on = s->data[offset] & bit;
+	mutex_unlock(&s->disp_lock);
+
+	return on ? LED_ON : LED_OFF;
+}
+
+static int tm1628_register_led(struct tm1628 *s, struct fwnode_handle *node,
+			       u32 grid, u32 seg, struct tm1628_led *led)
+{
+	struct device *dev = &s->spi->dev;
+	struct led_init_data init_data = { .fwnode = node };
+
+	led->ctrl = s;
+	led->grid = grid;
+	led->seg  = seg;
+	led->leddev.max_brightness = LED_ON;
+	led->leddev.brightness_set_blocking = tm1628_led_set_brightness;
+	led->leddev.brightness_get = tm1628_led_get_brightness;
+
+	return devm_led_classdev_register_ext(dev, &led->leddev, &init_data);
+}
+
+static ssize_t display_text_show(struct device *dev, struct device_attribute *attr,
+				 char *buf)
+{
+	struct tm1628 *s = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%s\n", s->text);
+}
+
+static ssize_t display_text_store(struct device *dev, struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct tm1628 *s = dev_get_drvdata(dev);
+	int ret, i;
+
+	if (count > s->grid_size + 1) /* consider trailing newline */
+		return -E2BIG;
+
+	for (i = 0; i < count && isprint(buf[i]); i++)
+		s->text[i] = buf[i];
+
+	s->text[i] = '\0';
+
+	ret = tm1628_show_text(s);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static const DEVICE_ATTR_RW(display_text);
+
+static int tm1628_spi_probe(struct spi_device *spi)
+{
+	struct fwnode_handle *child;
+	struct tm1628_led *leds;
+	unsigned int num_leds;
+	struct tm1628 *s;
+	int ret, i;
+
+	num_leds = device_get_child_node_count(&spi->dev);
+
+	s = devm_kzalloc(&spi->dev, struct_size(s, leds, num_leds), GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	s->spi = spi;
+	spi_set_drvdata(spi, s);
+
+	mutex_init(&s->disp_lock);
+
+	msleep(200); /* according to TM1628 datasheet */
+
+	/* clear screen */
+	ret = tm1628_write_data(spi, 0, MAX_GRID_SIZE);
+	if (ret)
+		return ret;
+	/* Assume that subsequent SPI transfers will be ok if first was ok */
+
+	/* For now we support 6x12 mode only. This should be sufficient for most use cases */
+	tm1628_set_display_mode(spi, TM1628_DISPLAY_MODE_6_12);
+
+	tm1628_set_display_ctrl(spi, true);
+
+	if (!IS_REACHABLE(CONFIG_LEDS_CLASS))
+		goto no_leds;
+
+	leds = s->leds;
+
+	device_for_each_child_node(&spi->dev, child) {
+		u32 reg[2];
+
+		ret = fwnode_property_read_u32_array(child, "reg", reg, 2);
+		if (ret) {
+			dev_err(&spi->dev, "Reading %s reg property failed (%d)\n",
+				fwnode_get_name(child), ret);
+			continue;
+		}
+
+		if (reg[0] == 0 || reg[0] > MAX_GRID_SIZE) {
+			dev_err(&spi->dev, "Invalid grid %u at %s\n",
+				reg[0], fwnode_get_name(child));
+			continue;
+		}
+
+		if (reg[1] == 0 || reg[1] > MAX_SEGMENT_NUM) {
+			dev_err(&spi->dev, "Invalid segment %u at %s\n",
+				reg[1], fwnode_get_name(child));
+			continue;
+		}
+
+		ret = tm1628_register_led(s, child, reg[0], reg[1], leds++);
+		if (ret) {
+			dev_err(&spi->dev, "Failed to register LED %s (%d)\n",
+				fwnode_get_name(child), ret);
+			continue;
+		}
+	}
+
+no_leds:
+	ret = device_property_count_u8(&spi->dev, "grid");
+	if (ret < 1 || ret > MAX_GRID_SIZE) {
+		dev_err(&spi->dev, "Invalid display length (%d)\n", ret);
+		return -EINVAL;
+	}
+
+	s->grid_size = ret;
+
+	ret = device_property_read_u8_array(&spi->dev, "grid", s->grid, s->grid_size);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < s->grid_size; i++) {
+		if (s->grid[i] < 1 || s->grid[i] > s->grid_size)
+			return -EINVAL;
+	}
+
+	ret = device_property_read_u8_array(&spi->dev, "segment-mapping", s->segment_mapping, 7);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < 7; i++) {
+		if (s->segment_mapping[i] < 1 || s->segment_mapping[i] > MAX_SEGMENT_NUM)
+			return -EINVAL;
+	}
+
+	return device_create_file(&spi->dev, &dev_attr_display_text);
+}
+
+static void tm1628_spi_remove(struct spi_device *spi)
+{
+	device_remove_file(&spi->dev, &dev_attr_display_text);
+	tm1628_set_display_ctrl(spi, false);
+}
+
+static void tm1628_spi_shutdown(struct spi_device *spi)
+{
+	tm1628_set_display_ctrl(spi, false);
+}
+
+static const struct of_device_id tm1628_spi_of_matches[] = {
+	{ .compatible = "titanmec,tm1628" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tm1628_spi_of_matches);
+
+static const struct spi_device_id tm1628_spi_id_table[] = {
+	{ "tm1628" },
+	{},
+};
+MODULE_DEVICE_TABLE(spi, tm1628_spi_id_table);
+
+static struct spi_driver tm1628_spi_driver = {
+	.probe = tm1628_spi_probe,
+	.remove = tm1628_spi_remove,
+	.shutdown = tm1628_spi_shutdown,
+	.id_table = tm1628_spi_id_table,
+
+	.driver = {
+		.name = "tm1628",
+		.of_match_table = tm1628_spi_of_matches,
+	},
+};
+module_spi_driver(tm1628_spi_driver);
+
+MODULE_DESCRIPTION("TM1628 LED controller driver");
+MODULE_AUTHOR("Andreas Färber");
+MODULE_LICENSE("GPL");
-- 
2.35.1

[PATCH 5/6] auxdisplay: add support for Titanmec TM1628 7 segment display controller

[Heiner Kallweit]

This patch adds support for the Titanmec TM1628 7 segment display
controller. It's based on previous RFC work from Andreas Färber.
The RFC version placed the driver in the LED subsystem, but this was
NAK'ed by the LED maintainer. Therefore I moved the driver to
/drivers/auxdisplay what seems most reasonable to me.

Further changes to the RFC version:
- Driver can be built also w/o LED class support, for displays that
  don't have any symbols to be exposed as LED's.
- Simplified the code and rewrote a lot of it.
- Driver is now kind of a MVP, but functionality should be sufficient
  for most use cases.
- Use the existing 7 segment support in uapi/linux/map_to_7segment.h
  as suggested by Geert Uytterhoeven.

Note: There's a number of chips from other manufacturers that are
      almost identical, e.g. FD628, SM1628. Only difference I saw so
      far is that they partially support other display modes.
      TM1628: 6x12, 7x11
      SM1628C: 4x13, 5x12, 6x11, 7x10
      For typical displays on devices using these chips this
      difference shouldn't matter.

Successfully tested on a TX3 Mini TV box that has an SM1628C and a
display with 4 digits and 7 symbols.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/auxdisplay/Kconfig  |  10 +
 drivers/auxdisplay/Makefile |   1 +
 drivers/auxdisplay/tm1628.c | 368 ++++++++++++++++++++++++++++++++++++
 3 files changed, 379 insertions(+)
 create mode 100644 drivers/auxdisplay/tm1628.c

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 64012cda4..25ef2e452 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -203,6 +203,16 @@ config ARM_CHARLCD
 	  line and the Linux version on the second line, but that's
 	  still useful.
 
+config TM1628
+	tristate "TM1628 driver for LED 7/11 segment displays"
+	depends on SPI
+	depends on OF || COMPILE_TEST
+	help
+	  Say Y to enable support for Titan Micro Electronics TM1628
+	  LED controller.
+	  It's a 3-wire SPI device controlling a two-dimensional grid of
+	  LEDs. Dimming is applied to all outputs through an internal PWM.
+
 menuconfig PARPORT_PANEL
 	tristate "Parallel port LCD/Keypad Panel support"
 	depends on PARPORT
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index 6968ed4d3..7728e17e1 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_HT16K33)		+= ht16k33.o
 obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
 obj-$(CONFIG_LCD2S)		+= lcd2s.o
 obj-$(CONFIG_LINEDISP)		+= line-display.o
+obj-$(CONFIG_TM1628)		+= tm1628.o
diff --git a/drivers/auxdisplay/tm1628.c b/drivers/auxdisplay/tm1628.c
new file mode 100644
index 000000000..a39b63828
--- /dev/null
+++ b/drivers/auxdisplay/tm1628.c
@@ -0,0 +1,368 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Titan Micro Electronics TM1628 LED controller
+ *
+ * Copyright (c) 2019 Andreas Färber
+ */
+
+#include <linux/delay.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/spi/spi.h>
+#include <uapi/linux/map_to_7segment.h>
+
+#define TM1628_CMD_DISPLAY_MODE		(0 << 6)
+#define TM1628_DISPLAY_MODE_6_12	0x02
+#define TM1628_DISPLAY_MODE_7_11	0x03
+
+#define TM1628_CMD_DATA			(1 << 6)
+#define TM1628_DATA_TEST_MODE		BIT(3)
+#define TM1628_DATA_FIXED_ADDR		BIT(2)
+#define TM1628_DATA_WRITE_DATA		0x00
+#define TM1628_DATA_READ_DATA		0x02
+
+#define TM1628_CMD_DISPLAY_CTRL		(2 << 6)
+#define TM1628_DISPLAY_CTRL_DISPLAY_ON	BIT(3)
+
+#define TM1628_CMD_SET_ADDRESS		(3 << 6)
+
+#define TM1628_BRIGHTNESS_MAX		7
+
+/* Physical limits, depending on the mode the chip may support less */
+#define MAX_GRID_SIZE			7
+#define MAX_SEGMENT_NUM			16
+
+struct tm1628_led {
+	struct led_classdev	leddev;
+	struct tm1628		*ctrl;
+	u32			grid;
+	u32			seg;
+};
+
+struct tm1628 {
+	struct spi_device		*spi;
+	__le16				data[MAX_GRID_SIZE];
+	struct mutex			disp_lock;
+	char				text[MAX_GRID_SIZE + 1];
+	u8				segment_mapping[7];
+	u8				grid[MAX_GRID_SIZE];
+	int				grid_size;
+	struct tm1628_led		leds[];
+};
+
+/* Command 1: Display Mode Setting */
+static int tm1628_set_display_mode(struct spi_device *spi, u8 grid_mode)
+{
+	u8 cmd = TM1628_CMD_DISPLAY_MODE | grid_mode;
+
+	return spi_write(spi, &cmd, 1);
+}
+
+/* Command 3: Address Setting */
+static int tm1628_set_address(struct spi_device *spi, u8 offset)
+{
+	u8 cmd = TM1628_CMD_SET_ADDRESS | (offset * sizeof(__le16));
+
+	return spi_write(spi, &cmd, 1);
+}
+
+/* Command 2: Data Setting */
+static int tm1628_write_data(struct spi_device *spi, unsigned int offset,
+			     unsigned int len)
+{
+	struct tm1628 *s = spi_get_drvdata(spi);
+	u8 cmd = TM1628_CMD_DATA | TM1628_DATA_WRITE_DATA;
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &cmd,
+			.len = 1,
+		},
+		{
+			.tx_buf = (__force void *)(s->data + offset),
+			.len = len * sizeof(__le16),
+		},
+	};
+
+	if (offset + len > MAX_GRID_SIZE) {
+		dev_err(&spi->dev, "Invalid data address offset %u len %u\n",
+			offset, len);
+		return -EINVAL;
+	}
+
+	tm1628_set_address(spi, offset);
+
+	return spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
+}
+
+/* Command 4: Display Control */
+static int tm1628_set_display_ctrl(struct spi_device *spi, bool on)
+{
+	u8 cmd = TM1628_CMD_DISPLAY_CTRL | TM1628_BRIGHTNESS_MAX;
+
+	if (on)
+		cmd |= TM1628_DISPLAY_CTRL_DISPLAY_ON;
+
+	return spi_write(spi, &cmd, 1);
+}
+
+static int tm1628_show_text(struct tm1628 *s)
+{
+	static SEG7_CONVERSION_MAP(map_seg7, MAP_ASCII7SEG_ALPHANUM);
+	int i, ret;
+
+	int msg_len = strlen(s->text);
+
+	mutex_lock(&s->disp_lock);
+
+	for (i = 0; i < s->grid_size; i++) {
+		int pos = s->grid[i] - 1;
+
+		if (i < msg_len) {
+			int char7_raw = map_to_seg7(&map_seg7, s->text[i]);
+			int j, char7;
+
+			for (j = 0, char7 = 0; j < 7; j++) {
+				if (char7_raw & BIT(j))
+					char7 |= BIT(s->segment_mapping[j] - 1);
+			}
+
+			s->data[pos] = cpu_to_le16(char7);
+		} else {
+			s->data[pos] = 0;
+		}
+	}
+
+	ret = tm1628_write_data(s->spi, 0, s->grid_size);
+
+	mutex_unlock(&s->disp_lock);
+
+	return ret;
+}
+
+static int tm1628_led_set_brightness(struct led_classdev *led_cdev,
+				     enum led_brightness brightness)
+{
+	struct tm1628_led *led = container_of(led_cdev, struct tm1628_led, leddev);
+	struct tm1628 *s = led->ctrl;
+	int offset, ret;
+	__le16 bit;
+
+	offset = led->grid - 1;
+	bit = cpu_to_le16(BIT(led->seg - 1));
+
+	mutex_lock(&s->disp_lock);
+
+	if (brightness == LED_OFF)
+		s->data[offset] &= ~bit;
+	else
+		s->data[offset] |= bit;
+
+	ret = tm1628_write_data(s->spi, offset, 1);
+
+	mutex_unlock(&s->disp_lock);
+
+	return ret;
+}
+
+static enum led_brightness tm1628_led_get_brightness(struct led_classdev *led_cdev)
+{
+	struct tm1628_led *led = container_of(led_cdev, struct tm1628_led, leddev);
+	struct tm1628 *s = led->ctrl;
+	int offset;
+	__le16 bit;
+	bool on;
+
+	offset = led->grid - 1;
+	bit = cpu_to_le16(BIT(led->seg - 1));
+
+	mutex_lock(&s->disp_lock);
+	on = s->data[offset] & bit;
+	mutex_unlock(&s->disp_lock);
+
+	return on ? LED_ON : LED_OFF;
+}
+
+static int tm1628_register_led(struct tm1628 *s, struct fwnode_handle *node,
+			       u32 grid, u32 seg, struct tm1628_led *led)
+{
+	struct device *dev = &s->spi->dev;
+	struct led_init_data init_data = { .fwnode = node };
+
+	led->ctrl = s;
+	led->grid = grid;
+	led->seg  = seg;
+	led->leddev.max_brightness = LED_ON;
+	led->leddev.brightness_set_blocking = tm1628_led_set_brightness;
+	led->leddev.brightness_get = tm1628_led_get_brightness;
+
+	return devm_led_classdev_register_ext(dev, &led->leddev, &init_data);
+}
+
+static ssize_t display_text_show(struct device *dev, struct device_attribute *attr,
+				 char *buf)
+{
+	struct tm1628 *s = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%s\n", s->text);
+}
+
+static ssize_t display_text_store(struct device *dev, struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct tm1628 *s = dev_get_drvdata(dev);
+	int ret, i;
+
+	if (count > s->grid_size + 1) /* consider trailing newline */
+		return -E2BIG;
+
+	for (i = 0; i < count && isprint(buf[i]); i++)
+		s->text[i] = buf[i];
+
+	s->text[i] = '\0';
+
+	ret = tm1628_show_text(s);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static const DEVICE_ATTR_RW(display_text);
+
+static int tm1628_spi_probe(struct spi_device *spi)
+{
+	struct fwnode_handle *child;
+	struct tm1628_led *leds;
+	unsigned int num_leds;
+	struct tm1628 *s;
+	int ret, i;
+
+	num_leds = device_get_child_node_count(&spi->dev);
+
+	s = devm_kzalloc(&spi->dev, struct_size(s, leds, num_leds), GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	s->spi = spi;
+	spi_set_drvdata(spi, s);
+
+	mutex_init(&s->disp_lock);
+
+	msleep(200); /* according to TM1628 datasheet */
+
+	/* clear screen */
+	ret = tm1628_write_data(spi, 0, MAX_GRID_SIZE);
+	if (ret)
+		return ret;
+	/* Assume that subsequent SPI transfers will be ok if first was ok */
+
+	/* For now we support 6x12 mode only. This should be sufficient for most use cases */
+	tm1628_set_display_mode(spi, TM1628_DISPLAY_MODE_6_12);
+
+	tm1628_set_display_ctrl(spi, true);
+
+	if (!IS_REACHABLE(CONFIG_LEDS_CLASS))
+		goto no_leds;
+
+	leds = s->leds;
+
+	device_for_each_child_node(&spi->dev, child) {
+		u32 reg[2];
+
+		ret = fwnode_property_read_u32_array(child, "reg", reg, 2);
+		if (ret) {
+			dev_err(&spi->dev, "Reading %s reg property failed (%d)\n",
+				fwnode_get_name(child), ret);
+			continue;
+		}
+
+		if (reg[0] == 0 || reg[0] > MAX_GRID_SIZE) {
+			dev_err(&spi->dev, "Invalid grid %u at %s\n",
+				reg[0], fwnode_get_name(child));
+			continue;
+		}
+
+		if (reg[1] == 0 || reg[1] > MAX_SEGMENT_NUM) {
+			dev_err(&spi->dev, "Invalid segment %u at %s\n",
+				reg[1], fwnode_get_name(child));
+			continue;
+		}
+
+		ret = tm1628_register_led(s, child, reg[0], reg[1], leds++);
+		if (ret) {
+			dev_err(&spi->dev, "Failed to register LED %s (%d)\n",
+				fwnode_get_name(child), ret);
+			continue;
+		}
+	}
+
+no_leds:
+	ret = device_property_count_u8(&spi->dev, "grid");
+	if (ret < 1 || ret > MAX_GRID_SIZE) {
+		dev_err(&spi->dev, "Invalid display length (%d)\n", ret);
+		return -EINVAL;
+	}
+
+	s->grid_size = ret;
+
+	ret = device_property_read_u8_array(&spi->dev, "grid", s->grid, s->grid_size);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < s->grid_size; i++) {
+		if (s->grid[i] < 1 || s->grid[i] > s->grid_size)
+			return -EINVAL;
+	}
+
+	ret = device_property_read_u8_array(&spi->dev, "segment-mapping", s->segment_mapping, 7);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < 7; i++) {
+		if (s->segment_mapping[i] < 1 || s->segment_mapping[i] > MAX_SEGMENT_NUM)
+			return -EINVAL;
+	}
+
+	return device_create_file(&spi->dev, &dev_attr_display_text);
+}
+
+static void tm1628_spi_remove(struct spi_device *spi)
+{
+	device_remove_file(&spi->dev, &dev_attr_display_text);
+	tm1628_set_display_ctrl(spi, false);
+}
+
+static void tm1628_spi_shutdown(struct spi_device *spi)
+{
+	tm1628_set_display_ctrl(spi, false);
+}
+
+static const struct of_device_id tm1628_spi_of_matches[] = {
+	{ .compatible = "titanmec,tm1628" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tm1628_spi_of_matches);
+
+static const struct spi_device_id tm1628_spi_id_table[] = {
+	{ "tm1628" },
+	{},
+};
+MODULE_DEVICE_TABLE(spi, tm1628_spi_id_table);
+
+static struct spi_driver tm1628_spi_driver = {
+	.probe = tm1628_spi_probe,
+	.remove = tm1628_spi_remove,
+	.shutdown = tm1628_spi_shutdown,
+	.id_table = tm1628_spi_id_table,
+
+	.driver = {
+		.name = "tm1628",
+		.of_match_table = tm1628_spi_of_matches,
+	},
+};
+module_spi_driver(tm1628_spi_driver);
+
+MODULE_DESCRIPTION("TM1628 LED controller driver");
+MODULE_AUTHOR("Andreas Färber");
+MODULE_LICENSE("GPL");
-- 
2.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 5/6] auxdisplay: add support for Titanmec TM1628 7 segment display controller

[Heiner Kallweit]

This patch adds support for the Titanmec TM1628 7 segment display
controller. It's based on previous RFC work from Andreas Färber.
The RFC version placed the driver in the LED subsystem, but this was
NAK'ed by the LED maintainer. Therefore I moved the driver to
/drivers/auxdisplay what seems most reasonable to me.

Further changes to the RFC version:
- Driver can be built also w/o LED class support, for displays that
  don't have any symbols to be exposed as LED's.
- Simplified the code and rewrote a lot of it.
- Driver is now kind of a MVP, but functionality should be sufficient
  for most use cases.
- Use the existing 7 segment support in uapi/linux/map_to_7segment.h
  as suggested by Geert Uytterhoeven.

Note: There's a number of chips from other manufacturers that are
      almost identical, e.g. FD628, SM1628. Only difference I saw so
      far is that they partially support other display modes.
      TM1628: 6x12, 7x11
      SM1628C: 4x13, 5x12, 6x11, 7x10
      For typical displays on devices using these chips this
      difference shouldn't matter.

Successfully tested on a TX3 Mini TV box that has an SM1628C and a
display with 4 digits and 7 symbols.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/auxdisplay/Kconfig  |  10 +
 drivers/auxdisplay/Makefile |   1 +
 drivers/auxdisplay/tm1628.c | 368 ++++++++++++++++++++++++++++++++++++
 3 files changed, 379 insertions(+)
 create mode 100644 drivers/auxdisplay/tm1628.c

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 64012cda4..25ef2e452 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -203,6 +203,16 @@ config ARM_CHARLCD
 	  line and the Linux version on the second line, but that's
 	  still useful.
 
+config TM1628
+	tristate "TM1628 driver for LED 7/11 segment displays"
+	depends on SPI
+	depends on OF || COMPILE_TEST
+	help
+	  Say Y to enable support for Titan Micro Electronics TM1628
+	  LED controller.
+	  It's a 3-wire SPI device controlling a two-dimensional grid of
+	  LEDs. Dimming is applied to all outputs through an internal PWM.
+
 menuconfig PARPORT_PANEL
 	tristate "Parallel port LCD/Keypad Panel support"
 	depends on PARPORT
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index 6968ed4d3..7728e17e1 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_HT16K33)		+= ht16k33.o
 obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
 obj-$(CONFIG_LCD2S)		+= lcd2s.o
 obj-$(CONFIG_LINEDISP)		+= line-display.o
+obj-$(CONFIG_TM1628)		+= tm1628.o
diff --git a/drivers/auxdisplay/tm1628.c b/drivers/auxdisplay/tm1628.c
new file mode 100644
index 000000000..a39b63828
--- /dev/null
+++ b/drivers/auxdisplay/tm1628.c
@@ -0,0 +1,368 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Titan Micro Electronics TM1628 LED controller
+ *
+ * Copyright (c) 2019 Andreas Färber
+ */
+
+#include <linux/delay.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/spi/spi.h>
+#include <uapi/linux/map_to_7segment.h>
+
+#define TM1628_CMD_DISPLAY_MODE		(0 << 6)
+#define TM1628_DISPLAY_MODE_6_12	0x02
+#define TM1628_DISPLAY_MODE_7_11	0x03
+
+#define TM1628_CMD_DATA			(1 << 6)
+#define TM1628_DATA_TEST_MODE		BIT(3)
+#define TM1628_DATA_FIXED_ADDR		BIT(2)
+#define TM1628_DATA_WRITE_DATA		0x00
+#define TM1628_DATA_READ_DATA		0x02
+
+#define TM1628_CMD_DISPLAY_CTRL		(2 << 6)
+#define TM1628_DISPLAY_CTRL_DISPLAY_ON	BIT(3)
+
+#define TM1628_CMD_SET_ADDRESS		(3 << 6)
+
+#define TM1628_BRIGHTNESS_MAX		7
+
+/* Physical limits, depending on the mode the chip may support less */
+#define MAX_GRID_SIZE			7
+#define MAX_SEGMENT_NUM			16
+
+struct tm1628_led {
+	struct led_classdev	leddev;
+	struct tm1628		*ctrl;
+	u32			grid;
+	u32			seg;
+};
+
+struct tm1628 {
+	struct spi_device		*spi;
+	__le16				data[MAX_GRID_SIZE];
+	struct mutex			disp_lock;
+	char				text[MAX_GRID_SIZE + 1];
+	u8				segment_mapping[7];
+	u8				grid[MAX_GRID_SIZE];
+	int				grid_size;
+	struct tm1628_led		leds[];
+};
+
+/* Command 1: Display Mode Setting */
+static int tm1628_set_display_mode(struct spi_device *spi, u8 grid_mode)
+{
+	u8 cmd = TM1628_CMD_DISPLAY_MODE | grid_mode;
+
+	return spi_write(spi, &cmd, 1);
+}
+
+/* Command 3: Address Setting */
+static int tm1628_set_address(struct spi_device *spi, u8 offset)
+{
+	u8 cmd = TM1628_CMD_SET_ADDRESS | (offset * sizeof(__le16));
+
+	return spi_write(spi, &cmd, 1);
+}
+
+/* Command 2: Data Setting */
+static int tm1628_write_data(struct spi_device *spi, unsigned int offset,
+			     unsigned int len)
+{
+	struct tm1628 *s = spi_get_drvdata(spi);
+	u8 cmd = TM1628_CMD_DATA | TM1628_DATA_WRITE_DATA;
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &cmd,
+			.len = 1,
+		},
+		{
+			.tx_buf = (__force void *)(s->data + offset),
+			.len = len * sizeof(__le16),
+		},
+	};
+
+	if (offset + len > MAX_GRID_SIZE) {
+		dev_err(&spi->dev, "Invalid data address offset %u len %u\n",
+			offset, len);
+		return -EINVAL;
+	}
+
+	tm1628_set_address(spi, offset);
+
+	return spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
+}
+
+/* Command 4: Display Control */
+static int tm1628_set_display_ctrl(struct spi_device *spi, bool on)
+{
+	u8 cmd = TM1628_CMD_DISPLAY_CTRL | TM1628_BRIGHTNESS_MAX;
+
+	if (on)
+		cmd |= TM1628_DISPLAY_CTRL_DISPLAY_ON;
+
+	return spi_write(spi, &cmd, 1);
+}
+
+static int tm1628_show_text(struct tm1628 *s)
+{
+	static SEG7_CONVERSION_MAP(map_seg7, MAP_ASCII7SEG_ALPHANUM);
+	int i, ret;
+
+	int msg_len = strlen(s->text);
+
+	mutex_lock(&s->disp_lock);
+
+	for (i = 0; i < s->grid_size; i++) {
+		int pos = s->grid[i] - 1;
+
+		if (i < msg_len) {
+			int char7_raw = map_to_seg7(&map_seg7, s->text[i]);
+			int j, char7;
+
+			for (j = 0, char7 = 0; j < 7; j++) {
+				if (char7_raw & BIT(j))
+					char7 |= BIT(s->segment_mapping[j] - 1);
+			}
+
+			s->data[pos] = cpu_to_le16(char7);
+		} else {
+			s->data[pos] = 0;
+		}
+	}
+
+	ret = tm1628_write_data(s->spi, 0, s->grid_size);
+
+	mutex_unlock(&s->disp_lock);
+
+	return ret;
+}
+
+static int tm1628_led_set_brightness(struct led_classdev *led_cdev,
+				     enum led_brightness brightness)
+{
+	struct tm1628_led *led = container_of(led_cdev, struct tm1628_led, leddev);
+	struct tm1628 *s = led->ctrl;
+	int offset, ret;
+	__le16 bit;
+
+	offset = led->grid - 1;
+	bit = cpu_to_le16(BIT(led->seg - 1));
+
+	mutex_lock(&s->disp_lock);
+
+	if (brightness == LED_OFF)
+		s->data[offset] &= ~bit;
+	else
+		s->data[offset] |= bit;
+
+	ret = tm1628_write_data(s->spi, offset, 1);
+
+	mutex_unlock(&s->disp_lock);
+
+	return ret;
+}
+
+static enum led_brightness tm1628_led_get_brightness(struct led_classdev *led_cdev)
+{
+	struct tm1628_led *led = container_of(led_cdev, struct tm1628_led, leddev);
+	struct tm1628 *s = led->ctrl;
+	int offset;
+	__le16 bit;
+	bool on;
+
+	offset = led->grid - 1;
+	bit = cpu_to_le16(BIT(led->seg - 1));
+
+	mutex_lock(&s->disp_lock);
+	on = s->data[offset] & bit;
+	mutex_unlock(&s->disp_lock);
+
+	return on ? LED_ON : LED_OFF;
+}
+
+static int tm1628_register_led(struct tm1628 *s, struct fwnode_handle *node,
+			       u32 grid, u32 seg, struct tm1628_led *led)
+{
+	struct device *dev = &s->spi->dev;
+	struct led_init_data init_data = { .fwnode = node };
+
+	led->ctrl = s;
+	led->grid = grid;
+	led->seg  = seg;
+	led->leddev.max_brightness = LED_ON;
+	led->leddev.brightness_set_blocking = tm1628_led_set_brightness;
+	led->leddev.brightness_get = tm1628_led_get_brightness;
+
+	return devm_led_classdev_register_ext(dev, &led->leddev, &init_data);
+}
+
+static ssize_t display_text_show(struct device *dev, struct device_attribute *attr,
+				 char *buf)
+{
+	struct tm1628 *s = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%s\n", s->text);
+}
+
+static ssize_t display_text_store(struct device *dev, struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct tm1628 *s = dev_get_drvdata(dev);
+	int ret, i;
+
+	if (count > s->grid_size + 1) /* consider trailing newline */
+		return -E2BIG;
+
+	for (i = 0; i < count && isprint(buf[i]); i++)
+		s->text[i] = buf[i];
+
+	s->text[i] = '\0';
+
+	ret = tm1628_show_text(s);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static const DEVICE_ATTR_RW(display_text);
+
+static int tm1628_spi_probe(struct spi_device *spi)
+{
+	struct fwnode_handle *child;
+	struct tm1628_led *leds;
+	unsigned int num_leds;
+	struct tm1628 *s;
+	int ret, i;
+
+	num_leds = device_get_child_node_count(&spi->dev);
+
+	s = devm_kzalloc(&spi->dev, struct_size(s, leds, num_leds), GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	s->spi = spi;
+	spi_set_drvdata(spi, s);
+
+	mutex_init(&s->disp_lock);
+
+	msleep(200); /* according to TM1628 datasheet */
+
+	/* clear screen */
+	ret = tm1628_write_data(spi, 0, MAX_GRID_SIZE);
+	if (ret)
+		return ret;
+	/* Assume that subsequent SPI transfers will be ok if first was ok */
+
+	/* For now we support 6x12 mode only. This should be sufficient for most use cases */
+	tm1628_set_display_mode(spi, TM1628_DISPLAY_MODE_6_12);
+
+	tm1628_set_display_ctrl(spi, true);
+
+	if (!IS_REACHABLE(CONFIG_LEDS_CLASS))
+		goto no_leds;
+
+	leds = s->leds;
+
+	device_for_each_child_node(&spi->dev, child) {
+		u32 reg[2];
+
+		ret = fwnode_property_read_u32_array(child, "reg", reg, 2);
+		if (ret) {
+			dev_err(&spi->dev, "Reading %s reg property failed (%d)\n",
+				fwnode_get_name(child), ret);
+			continue;
+		}
+
+		if (reg[0] == 0 || reg[0] > MAX_GRID_SIZE) {
+			dev_err(&spi->dev, "Invalid grid %u at %s\n",
+				reg[0], fwnode_get_name(child));
+			continue;
+		}
+
+		if (reg[1] == 0 || reg[1] > MAX_SEGMENT_NUM) {
+			dev_err(&spi->dev, "Invalid segment %u at %s\n",
+				reg[1], fwnode_get_name(child));
+			continue;
+		}
+
+		ret = tm1628_register_led(s, child, reg[0], reg[1], leds++);
+		if (ret) {
+			dev_err(&spi->dev, "Failed to register LED %s (%d)\n",
+				fwnode_get_name(child), ret);
+			continue;
+		}
+	}
+
+no_leds:
+	ret = device_property_count_u8(&spi->dev, "grid");
+	if (ret < 1 || ret > MAX_GRID_SIZE) {
+		dev_err(&spi->dev, "Invalid display length (%d)\n", ret);
+		return -EINVAL;
+	}
+
+	s->grid_size = ret;
+
+	ret = device_property_read_u8_array(&spi->dev, "grid", s->grid, s->grid_size);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < s->grid_size; i++) {
+		if (s->grid[i] < 1 || s->grid[i] > s->grid_size)
+			return -EINVAL;
+	}
+
+	ret = device_property_read_u8_array(&spi->dev, "segment-mapping", s->segment_mapping, 7);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < 7; i++) {
+		if (s->segment_mapping[i] < 1 || s->segment_mapping[i] > MAX_SEGMENT_NUM)
+			return -EINVAL;
+	}
+
+	return device_create_file(&spi->dev, &dev_attr_display_text);
+}
+
+static void tm1628_spi_remove(struct spi_device *spi)
+{
+	device_remove_file(&spi->dev, &dev_attr_display_text);
+	tm1628_set_display_ctrl(spi, false);
+}
+
+static void tm1628_spi_shutdown(struct spi_device *spi)
+{
+	tm1628_set_display_ctrl(spi, false);
+}
+
+static const struct of_device_id tm1628_spi_of_matches[] = {
+	{ .compatible = "titanmec,tm1628" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tm1628_spi_of_matches);
+
+static const struct spi_device_id tm1628_spi_id_table[] = {
+	{ "tm1628" },
+	{},
+};
+MODULE_DEVICE_TABLE(spi, tm1628_spi_id_table);
+
+static struct spi_driver tm1628_spi_driver = {
+	.probe = tm1628_spi_probe,
+	.remove = tm1628_spi_remove,
+	.shutdown = tm1628_spi_shutdown,
+	.id_table = tm1628_spi_id_table,
+
+	.driver = {
+		.name = "tm1628",
+		.of_match_table = tm1628_spi_of_matches,
+	},
+};
+module_spi_driver(tm1628_spi_driver);
+
+MODULE_DESCRIPTION("TM1628 LED controller driver");
+MODULE_AUTHOR("Andreas Färber");
+MODULE_LICENSE("GPL");
-- 
2.35.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] arm64: dts: meson-gxl-s905w-tx3-mini: add support for the 7 segment display

[Heiner Kallweit]

This patch adds support for the 7 segment display of the device.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 .../dts/amlogic/meson-gxl-s905w-tx3-mini.dts  | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
index 6705c2082..20bbd931e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
@@ -10,6 +10,7 @@
 
 #include "meson-gxl-s905x.dtsi"
 #include "meson-gx-p23x-q20x.dtsi"
+#include <dt-bindings/leds/common.h>
 
 / {
 	compatible = "oranth,tx3-mini", "amlogic,s905w", "amlogic,meson-gxl";
@@ -19,6 +20,64 @@ memory@0 {
 		device_type = "memory";
 		reg = <0x0 0x0 0x0 0x40000000>; /* 1 GiB or 2 GiB */
 	};
+
+	spi {
+		compatible = "spi-gpio";
+		sck-gpios = <&gpio GPIODV_27  GPIO_ACTIVE_HIGH>;
+		mosi-gpios = <&gpio GPIODV_26 GPIO_ACTIVE_HIGH>;
+		cs-gpios = <&gpio_ao GPIOAO_4 GPIO_ACTIVE_LOW>;
+		num-chipselects = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		tm1628: led-controller@0 {
+			compatible = "titanmec,tm1628";
+			reg = <0>;
+			spi-3wire;
+			spi-lsb-first;
+			spi-rx-delay-us = <1>;
+			spi-max-frequency = <500000>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+
+			segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>;
+			grid = /bits/ 8 <4 3 2 1>;
+
+			alarm@5,1 {
+				reg = <5 1>;
+				function = LED_FUNCTION_ALARM;
+			};
+
+			usb@5,2 {
+				reg = <5 2>;
+				function = LED_FUNCTION_USB;
+			};
+			play@5,3 {
+				reg = <5 3>;
+				function = "play";
+			};
+
+			pause@5,4 {
+				reg = <5 4>;
+				function = "pause";
+			};
+
+			colon@5,5 {
+				reg = <5 5>;
+				function = "colon";
+			};
+
+			lan@5,6 {
+				reg = <5 6>;
+				function = LED_FUNCTION_LAN;
+			};
+
+			wlan@5,7 {
+				reg = <5 7>;
+				function = LED_FUNCTION_WLAN;
+			};
+		};
+	};
 };
 
 &ir {
-- 
2.35.1

[PATCH 6/6] arm64: dts: meson-gxl-s905w-tx3-mini: add support for the 7 segment display

[Heiner Kallweit]

This patch adds support for the 7 segment display of the device.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 .../dts/amlogic/meson-gxl-s905w-tx3-mini.dts  | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
index 6705c2082..20bbd931e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
@@ -10,6 +10,7 @@
 
 #include "meson-gxl-s905x.dtsi"
 #include "meson-gx-p23x-q20x.dtsi"
+#include <dt-bindings/leds/common.h>
 
 / {
 	compatible = "oranth,tx3-mini", "amlogic,s905w", "amlogic,meson-gxl";
@@ -19,6 +20,64 @@ memory@0 {
 		device_type = "memory";
 		reg = <0x0 0x0 0x0 0x40000000>; /* 1 GiB or 2 GiB */
 	};
+
+	spi {
+		compatible = "spi-gpio";
+		sck-gpios = <&gpio GPIODV_27  GPIO_ACTIVE_HIGH>;
+		mosi-gpios = <&gpio GPIODV_26 GPIO_ACTIVE_HIGH>;
+		cs-gpios = <&gpio_ao GPIOAO_4 GPIO_ACTIVE_LOW>;
+		num-chipselects = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		tm1628: led-controller@0 {
+			compatible = "titanmec,tm1628";
+			reg = <0>;
+			spi-3wire;
+			spi-lsb-first;
+			spi-rx-delay-us = <1>;
+			spi-max-frequency = <500000>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+
+			segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>;
+			grid = /bits/ 8 <4 3 2 1>;
+
+			alarm@5,1 {
+				reg = <5 1>;
+				function = LED_FUNCTION_ALARM;
+			};
+
+			usb@5,2 {
+				reg = <5 2>;
+				function = LED_FUNCTION_USB;
+			};
+			play@5,3 {
+				reg = <5 3>;
+				function = "play";
+			};
+
+			pause@5,4 {
+				reg = <5 4>;
+				function = "pause";
+			};
+
+			colon@5,5 {
+				reg = <5 5>;
+				function = "colon";
+			};
+
+			lan@5,6 {
+				reg = <5 6>;
+				function = LED_FUNCTION_LAN;
+			};
+
+			wlan@5,7 {
+				reg = <5 7>;
+				function = LED_FUNCTION_WLAN;
+			};
+		};
+	};
 };
 
 &ir {
-- 
2.35.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 6/6] arm64: dts: meson-gxl-s905w-tx3-mini: add support for the 7 segment display

[Heiner Kallweit]

This patch adds support for the 7 segment display of the device.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 .../dts/amlogic/meson-gxl-s905w-tx3-mini.dts  | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
index 6705c2082..20bbd931e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
@@ -10,6 +10,7 @@
 
 #include "meson-gxl-s905x.dtsi"
 #include "meson-gx-p23x-q20x.dtsi"
+#include <dt-bindings/leds/common.h>
 
 / {
 	compatible = "oranth,tx3-mini", "amlogic,s905w", "amlogic,meson-gxl";
@@ -19,6 +20,64 @@ memory@0 {
 		device_type = "memory";
 		reg = <0x0 0x0 0x0 0x40000000>; /* 1 GiB or 2 GiB */
 	};
+
+	spi {
+		compatible = "spi-gpio";
+		sck-gpios = <&gpio GPIODV_27  GPIO_ACTIVE_HIGH>;
+		mosi-gpios = <&gpio GPIODV_26 GPIO_ACTIVE_HIGH>;
+		cs-gpios = <&gpio_ao GPIOAO_4 GPIO_ACTIVE_LOW>;
+		num-chipselects = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		tm1628: led-controller@0 {
+			compatible = "titanmec,tm1628";
+			reg = <0>;
+			spi-3wire;
+			spi-lsb-first;
+			spi-rx-delay-us = <1>;
+			spi-max-frequency = <500000>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+
+			segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>;
+			grid = /bits/ 8 <4 3 2 1>;
+
+			alarm@5,1 {
+				reg = <5 1>;
+				function = LED_FUNCTION_ALARM;
+			};
+
+			usb@5,2 {
+				reg = <5 2>;
+				function = LED_FUNCTION_USB;
+			};
+			play@5,3 {
+				reg = <5 3>;
+				function = "play";
+			};
+
+			pause@5,4 {
+				reg = <5 4>;
+				function = "pause";
+			};
+
+			colon@5,5 {
+				reg = <5 5>;
+				function = "colon";
+			};
+
+			lan@5,6 {
+				reg = <5 6>;
+				function = LED_FUNCTION_LAN;
+			};
+
+			wlan@5,7 {
+				reg = <5 7>;
+				function = LED_FUNCTION_WLAN;
+			};
+		};
+	};
 };
 
 &ir {
-- 
2.35.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Miguel Ojeda]

On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> This series adds support for the Titanmec TM1628 7 segment display
> controller. It's based on previous RFC work from Andreas Färber.
> The RFC version placed the driver in the LED subsystem, but this was
> NAK'ed by the LED maintainer. Therefore I moved the driver to
> /drivers/auxdisplay what seems most reasonable to me.

Could you please link to the discussion and/or summarize the rationale
behind the NAK?

Cheers,
Miguel

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

[Miguel Ojeda]

On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> This series adds support for the Titanmec TM1628 7 segment display
> controller. It's based on previous RFC work from Andreas Färber.
> The RFC version placed the driver in the LED subsystem, but this was
> NAK'ed by the LED maintainer. Therefore I moved the driver to
> /drivers/auxdisplay what seems most reasonable to me.

Could you please link to the discussion and/or summarize the rationale
behind the NAK?

Cheers,
Miguel

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Miguel Ojeda]

On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> This series adds support for the Titanmec TM1628 7 segment display
> controller. It's based on previous RFC work from Andreas Färber.
> The RFC version placed the driver in the LED subsystem, but this was
> NAK'ed by the LED maintainer. Therefore I moved the driver to
> /drivers/auxdisplay what seems most reasonable to me.

Could you please link to the discussion and/or summarize the rationale
behind the NAK?

Cheers,
Miguel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Heiner Kallweit]

On 19.02.2022 14:27, Miguel Ojeda wrote:
> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> This series adds support for the Titanmec TM1628 7 segment display
>> controller. It's based on previous RFC work from Andreas Färber.
>> The RFC version placed the driver in the LED subsystem, but this was
>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>> /drivers/auxdisplay what seems most reasonable to me.
> 
> Could you please link to the discussion and/or summarize the rationale
> behind the NAK?
> 

+Pavel

I didn't find an explicit reason, but I suppose Pavel sees this driver as
one that makes use of the LED subsystem, but doesn't belong to it.
In the following mail he's expressing his opinion that the driver should
be best placed under auxdisplay.

https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/

> Cheers,
> Miguel

Heiner

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

[Heiner Kallweit]

On 19.02.2022 14:27, Miguel Ojeda wrote:
> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> This series adds support for the Titanmec TM1628 7 segment display
>> controller. It's based on previous RFC work from Andreas Färber.
>> The RFC version placed the driver in the LED subsystem, but this was
>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>> /drivers/auxdisplay what seems most reasonable to me.
> 
> Could you please link to the discussion and/or summarize the rationale
> behind the NAK?
> 

+Pavel

I didn't find an explicit reason, but I suppose Pavel sees this driver as
one that makes use of the LED subsystem, but doesn't belong to it.
In the following mail he's expressing his opinion that the driver should
be best placed under auxdisplay.

https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/

> Cheers,
> Miguel

Heiner



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Heiner Kallweit]

On 19.02.2022 14:27, Miguel Ojeda wrote:
> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> This series adds support for the Titanmec TM1628 7 segment display
>> controller. It's based on previous RFC work from Andreas Färber.
>> The RFC version placed the driver in the LED subsystem, but this was
>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>> /drivers/auxdisplay what seems most reasonable to me.
> 
> Could you please link to the discussion and/or summarize the rationale
> behind the NAK?
> 

+Pavel

I didn't find an explicit reason, but I suppose Pavel sees this driver as
one that makes use of the LED subsystem, but doesn't belong to it.
In the following mail he's expressing his opinion that the driver should
be best placed under auxdisplay.

https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/

> Cheers,
> Miguel

Heiner



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Miguel Ojeda]

On Sat, Feb 19, 2022 at 2:37 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> I didn't find an explicit reason, but I suppose Pavel sees this driver as
> one that makes use of the LED subsystem, but doesn't belong to it.
> In the following mail he's expressing his opinion that the driver should
> be best placed under auxdisplay.

Ah, OK -- thanks!

Cheers,
Miguel

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

[Miguel Ojeda]

On Sat, Feb 19, 2022 at 2:37 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> I didn't find an explicit reason, but I suppose Pavel sees this driver as
> one that makes use of the LED subsystem, but doesn't belong to it.
> In the following mail he's expressing his opinion that the driver should
> be best placed under auxdisplay.

Ah, OK -- thanks!

Cheers,
Miguel

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Miguel Ojeda]

On Sat, Feb 19, 2022 at 2:37 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> I didn't find an explicit reason, but I suppose Pavel sees this driver as
> one that makes use of the LED subsystem, but doesn't belong to it.
> In the following mail he's expressing his opinion that the driver should
> be best placed under auxdisplay.

Ah, OK -- thanks!

Cheers,
Miguel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Andreas Färber]

Hi,

On 19.02.22 14:37, Heiner Kallweit wrote:
> On 19.02.2022 14:27, Miguel Ojeda wrote:
>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>
>>> This series adds support for the Titanmec TM1628 7 segment display
>>> controller. It's based on previous RFC work from Andreas Färber.
>>> The RFC version placed the driver in the LED subsystem, but this was
>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>> /drivers/auxdisplay what seems most reasonable to me.
>>
>> Could you please link to the discussion and/or summarize the rationale
>> behind the NAK?
>>
> 
> +Pavel
> 
> I didn't find an explicit reason, but I suppose Pavel sees this driver as
> one that makes use of the LED subsystem, but doesn't belong to it.
> In the following mail he's expressing his opinion that the driver should
> be best placed under auxdisplay.
> 
> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/

And I disagreed. It does not fit with the other drivers in auxdisplay
that were operating on a much higher level.

I'd also like to point out that I did implement the map_to_7segment API,
as was suggested, as you will find in my tree - which you may have
missed, referencing only the RFC patchset and putting your authorship on
it exclusively? A move from one directory to another should not warrant
my author and SoB getting removed from the actual driver.

Given that we need to manage a buffer with bits per segment or LED
symbol, one idea that I haven't found time for yet was to implement it
as framebuffer or drm device instead. (And most Realtek platforms got
broken by removing the adjustable text base defines.)

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)

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

[Andreas Färber]

Hi,

On 19.02.22 14:37, Heiner Kallweit wrote:
> On 19.02.2022 14:27, Miguel Ojeda wrote:
>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>
>>> This series adds support for the Titanmec TM1628 7 segment display
>>> controller. It's based on previous RFC work from Andreas Färber.
>>> The RFC version placed the driver in the LED subsystem, but this was
>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>> /drivers/auxdisplay what seems most reasonable to me.
>>
>> Could you please link to the discussion and/or summarize the rationale
>> behind the NAK?
>>
> 
> +Pavel
> 
> I didn't find an explicit reason, but I suppose Pavel sees this driver as
> one that makes use of the LED subsystem, but doesn't belong to it.
> In the following mail he's expressing his opinion that the driver should
> be best placed under auxdisplay.
> 
> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/

And I disagreed. It does not fit with the other drivers in auxdisplay
that were operating on a much higher level.

I'd also like to point out that I did implement the map_to_7segment API,
as was suggested, as you will find in my tree - which you may have
missed, referencing only the RFC patchset and putting your authorship on
it exclusively? A move from one directory to another should not warrant
my author and SoB getting removed from the actual driver.

Given that we need to manage a buffer with bits per segment or LED
symbol, one idea that I haven't found time for yet was to implement it
as framebuffer or drm device instead. (And most Realtek platforms got
broken by removing the adjustable text base defines.)

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Andreas Färber]

Hi,

On 19.02.22 14:37, Heiner Kallweit wrote:
> On 19.02.2022 14:27, Miguel Ojeda wrote:
>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>
>>> This series adds support for the Titanmec TM1628 7 segment display
>>> controller. It's based on previous RFC work from Andreas Färber.
>>> The RFC version placed the driver in the LED subsystem, but this was
>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>> /drivers/auxdisplay what seems most reasonable to me.
>>
>> Could you please link to the discussion and/or summarize the rationale
>> behind the NAK?
>>
> 
> +Pavel
> 
> I didn't find an explicit reason, but I suppose Pavel sees this driver as
> one that makes use of the LED subsystem, but doesn't belong to it.
> In the following mail he's expressing his opinion that the driver should
> be best placed under auxdisplay.
> 
> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/

And I disagreed. It does not fit with the other drivers in auxdisplay
that were operating on a much higher level.

I'd also like to point out that I did implement the map_to_7segment API,
as was suggested, as you will find in my tree - which you may have
missed, referencing only the RFC patchset and putting your authorship on
it exclusively? A move from one directory to another should not warrant
my author and SoB getting removed from the actual driver.

Given that we need to manage a buffer with bits per segment or LED
symbol, one idea that I haven't found time for yet was to implement it
as framebuffer or drm device instead. (And most Realtek platforms got
broken by removing the adjustable text base defines.)

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Heiner Kallweit]

On 19.02.2022 17:07, Andreas Färber wrote:
> Hi,
> 
> On 19.02.22 14:37, Heiner Kallweit wrote:
>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>
>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>
>>> Could you please link to the discussion and/or summarize the rationale
>>> behind the NAK?
>>>
>>
>> +Pavel
>>
>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>> one that makes use of the LED subsystem, but doesn't belong to it.
>> In the following mail he's expressing his opinion that the driver should
>> be best placed under auxdisplay.
>>
>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
> 
> And I disagreed. It does not fit with the other drivers in auxdisplay
> that were operating on a much higher level.
> 

We need to find a place. And if Pavel has good reasons that it doesn't
fit into the LED subsystem, and Miguel should be fine with having
it in auxdisplay, then I'd see no reason to not go this way.

> I'd also like to point out that I did implement the map_to_7segment API,

Looking at the history of include/uapi/linux/map_to_7segment.h I see no
commit from you. Seems I'm missing something here.

> as was suggested, as you will find in my tree - which you may have
> missed, referencing only the RFC patchset and putting your authorship on
> it exclusively? A move from one directory to another should not warrant
> my author and SoB getting removed from the actual driver.
> 
The driver includes major changes and I mentioned your work in the commit
message. Also your still listed as MODULE_AUTHOR. My intention is to
get a driver upstream, not to earn credits for something.
So sure, your SoB can be (re-)added.

> Given that we need to manage a buffer with bits per segment or LED
> symbol, one idea that I haven't found time for yet was to implement it
> as framebuffer or drm device instead. (And most Realtek platforms got
> broken by removing the adjustable text base defines.)
> 
I'm not aware of the Realtek platform issue, do you have a link to a
related discussion? And wouldn't you think it's overengineering to
write a DRM driver for a 7 segment display with 4 digits?
Framebuffer seems to be deprecated based on my experience with
pygame / SDL2.

> Regards,
> Andreas
> 

Heiner

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

[Heiner Kallweit]

On 19.02.2022 17:07, Andreas Färber wrote:
> Hi,
> 
> On 19.02.22 14:37, Heiner Kallweit wrote:
>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>
>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>
>>> Could you please link to the discussion and/or summarize the rationale
>>> behind the NAK?
>>>
>>
>> +Pavel
>>
>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>> one that makes use of the LED subsystem, but doesn't belong to it.
>> In the following mail he's expressing his opinion that the driver should
>> be best placed under auxdisplay.
>>
>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
> 
> And I disagreed. It does not fit with the other drivers in auxdisplay
> that were operating on a much higher level.
> 

We need to find a place. And if Pavel has good reasons that it doesn't
fit into the LED subsystem, and Miguel should be fine with having
it in auxdisplay, then I'd see no reason to not go this way.

> I'd also like to point out that I did implement the map_to_7segment API,

Looking at the history of include/uapi/linux/map_to_7segment.h I see no
commit from you. Seems I'm missing something here.

> as was suggested, as you will find in my tree - which you may have
> missed, referencing only the RFC patchset and putting your authorship on
> it exclusively? A move from one directory to another should not warrant
> my author and SoB getting removed from the actual driver.
> 
The driver includes major changes and I mentioned your work in the commit
message. Also your still listed as MODULE_AUTHOR. My intention is to
get a driver upstream, not to earn credits for something.
So sure, your SoB can be (re-)added.

> Given that we need to manage a buffer with bits per segment or LED
> symbol, one idea that I haven't found time for yet was to implement it
> as framebuffer or drm device instead. (And most Realtek platforms got
> broken by removing the adjustable text base defines.)
> 
I'm not aware of the Realtek platform issue, do you have a link to a
related discussion? And wouldn't you think it's overengineering to
write a DRM driver for a 7 segment display with 4 digits?
Framebuffer seems to be deprecated based on my experience with
pygame / SDL2.

> Regards,
> Andreas
> 

Heiner

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Heiner Kallweit]

On 19.02.2022 17:07, Andreas Färber wrote:
> Hi,
> 
> On 19.02.22 14:37, Heiner Kallweit wrote:
>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>
>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>
>>> Could you please link to the discussion and/or summarize the rationale
>>> behind the NAK?
>>>
>>
>> +Pavel
>>
>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>> one that makes use of the LED subsystem, but doesn't belong to it.
>> In the following mail he's expressing his opinion that the driver should
>> be best placed under auxdisplay.
>>
>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
> 
> And I disagreed. It does not fit with the other drivers in auxdisplay
> that were operating on a much higher level.
> 

We need to find a place. And if Pavel has good reasons that it doesn't
fit into the LED subsystem, and Miguel should be fine with having
it in auxdisplay, then I'd see no reason to not go this way.

> I'd also like to point out that I did implement the map_to_7segment API,

Looking at the history of include/uapi/linux/map_to_7segment.h I see no
commit from you. Seems I'm missing something here.

> as was suggested, as you will find in my tree - which you may have
> missed, referencing only the RFC patchset and putting your authorship on
> it exclusively? A move from one directory to another should not warrant
> my author and SoB getting removed from the actual driver.
> 
The driver includes major changes and I mentioned your work in the commit
message. Also your still listed as MODULE_AUTHOR. My intention is to
get a driver upstream, not to earn credits for something.
So sure, your SoB can be (re-)added.

> Given that we need to manage a buffer with bits per segment or LED
> symbol, one idea that I haven't found time for yet was to implement it
> as framebuffer or drm device instead. (And most Realtek platforms got
> broken by removing the adjustable text base defines.)
> 
I'm not aware of the Realtek platform issue, do you have a link to a
related discussion? And wouldn't you think it's overengineering to
write a DRM driver for a 7 segment display with 4 digits?
Framebuffer seems to be deprecated based on my experience with
pygame / SDL2.

> Regards,
> Andreas
> 

Heiner

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

[Rob Herring]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]

On Sat, 19 Feb 2022 14:17:54 +0100, Heiner Kallweit wrote:
> Add a YAML schema binding for TM1628 auxdisplay
> (7/11-segment LED) controller.
> 
> This patch is partially based on previous RFC work from
> Andreas Färber <afaerber@suse.de>.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 82 +++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml:12:5: [warning] wrong indentation: expected 6 but found 4 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'maintainers' is a required property
	hint: Metaschema for devicetree binding documentation
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'additionalProperties' is a required property
	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: ignoring, error in schema: 
Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.example.dt.yaml:0:0: /example-0/spi/led-controller@0: failed to match any schema with compatible: ['titanmec,tm1628']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1595016

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 3/6] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

[Rob Herring]

[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]

On Sat, 19 Feb 2022 14:17:54 +0100, Heiner Kallweit wrote:
> Add a YAML schema binding for TM1628 auxdisplay
> (7/11-segment LED) controller.
> 
> This patch is partially based on previous RFC work from
> Andreas Färber <afaerber@suse.de>.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 82 +++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml:12:5: [warning] wrong indentation: expected 6 but found 4 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'maintainers' is a required property
	hint: Metaschema for devicetree binding documentation
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'additionalProperties' is a required property
	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: ignoring, error in schema: 
Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.example.dt.yaml:0:0: /example-0/spi/led-controller@0: failed to match any schema with compatible: ['titanmec,tm1628']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1595016

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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

pip3 install dtschema --upgrade

Please check and re-submit.



[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 3/6] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

[Rob Herring]

[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]

On Sat, 19 Feb 2022 14:17:54 +0100, Heiner Kallweit wrote:
> Add a YAML schema binding for TM1628 auxdisplay
> (7/11-segment LED) controller.
> 
> This patch is partially based on previous RFC work from
> Andreas Färber <afaerber@suse.de>.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 82 +++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml:12:5: [warning] wrong indentation: expected 6 but found 4 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'maintainers' is a required property
	hint: Metaschema for devicetree binding documentation
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'additionalProperties' is a required property
	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: ignoring, error in schema: 
Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.example.dt.yaml:0:0: /example-0/spi/led-controller@0: failed to match any schema with compatible: ['titanmec,tm1628']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1595016

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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

pip3 install dtschema --upgrade

Please check and re-submit.



[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Christian Hewitt]

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

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

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

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

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

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

static void tm1628_spi_shutdown(struct spi_device *spi)

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

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

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

Tested-by: Christian Hewitt <christianshewitt@gmail.com>

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

[Christian Hewitt]

resend from correct mail account:

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

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

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

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

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

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

static void tm1628_spi_shutdown(struct spi_device *spi)

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

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

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

Tested-by: Christian Hewitt <christianshewitt@gmail.com>

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

[Christian Hewitt]

resend from correct mail account:

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

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

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

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

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

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

static void tm1628_spi_shutdown(struct spi_device *spi)

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

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

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

Tested-by: Christian Hewitt <christianshewitt@gmail.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Christian Hewitt]

resend from correct mail account:

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

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

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

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

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

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

static void tm1628_spi_shutdown(struct spi_device *spi)

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

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

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

Tested-by: Christian Hewitt <christianshewitt@gmail.com>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 3/6] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

[Heiner Kallweit]

On 21.02.2022 03:36, Rob Herring wrote:
> On Sat, 19 Feb 2022 14:17:54 +0100, Heiner Kallweit wrote:
>> Add a YAML schema binding for TM1628 auxdisplay
>> (7/11-segment LED) controller.
>>
>> This patch is partially based on previous RFC work from
>> Andreas Färber <afaerber@suse.de>.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 82 +++++++++++++++++++
>>  1 file changed, 82 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
>>
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml:12:5: [warning] wrong indentation: expected 6 but found 4 (indentation)
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'maintainers' is a required property
> 	hint: Metaschema for devicetree binding documentation
> 	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'additionalProperties' is a required property
> 	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
> 	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
> ./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: $id: relative path/filename doesn't match actual path or filename
> 	expected: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: ignoring, error in schema: 
> Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.example.dt.yaml:0:0: /example-0/spi/led-controller@0: failed to match any schema with compatible: ['titanmec,tm1628']
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/patch/1595016
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
> 
Thanks for the feedback. Will check and consider it in v2.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 3/6] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

[Heiner Kallweit]

On 21.02.2022 03:36, Rob Herring wrote:
> On Sat, 19 Feb 2022 14:17:54 +0100, Heiner Kallweit wrote:
>> Add a YAML schema binding for TM1628 auxdisplay
>> (7/11-segment LED) controller.
>>
>> This patch is partially based on previous RFC work from
>> Andreas Färber <afaerber@suse.de>.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 82 +++++++++++++++++++
>>  1 file changed, 82 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
>>
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml:12:5: [warning] wrong indentation: expected 6 but found 4 (indentation)
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'maintainers' is a required property
> 	hint: Metaschema for devicetree binding documentation
> 	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'additionalProperties' is a required property
> 	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
> 	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
> ./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: $id: relative path/filename doesn't match actual path or filename
> 	expected: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: ignoring, error in schema: 
> Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.example.dt.yaml:0:0: /example-0/spi/led-controller@0: failed to match any schema with compatible: ['titanmec,tm1628']
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/patch/1595016
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
> 
Thanks for the feedback. Will check and consider it in v2.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

[Heiner Kallweit]

On 21.02.2022 03:36, Rob Herring wrote:
> On Sat, 19 Feb 2022 14:17:54 +0100, Heiner Kallweit wrote:
>> Add a YAML schema binding for TM1628 auxdisplay
>> (7/11-segment LED) controller.
>>
>> This patch is partially based on previous RFC work from
>> Andreas Färber <afaerber@suse.de>.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 82 +++++++++++++++++++
>>  1 file changed, 82 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
>>
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml:12:5: [warning] wrong indentation: expected 6 but found 4 (indentation)
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'maintainers' is a required property
> 	hint: Metaschema for devicetree binding documentation
> 	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: 'additionalProperties' is a required property
> 	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
> 	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
> ./Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: $id: relative path/filename doesn't match actual path or filename
> 	expected: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml: ignoring, error in schema: 
> Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.example.dt.yaml:0:0: /example-0/spi/led-controller@0: failed to match any schema with compatible: ['titanmec,tm1628']
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/patch/1595016
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
> 
Thanks for the feedback. Will check and consider it in v2.

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

[Heiner Kallweit]

On 21.02.2022 07:32, Christian Hewitt wrote:
> resend from correct mail account:
> 
>> On 19 Feb 2022, at 5:13 pm, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> This series adds support for the Titanmec TM1628 7 segment display
>> controller. It's based on previous RFC work from Andreas Färber.
>> The RFC version placed the driver in the LED subsystem, but this was
>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>> /drivers/auxdisplay what seems most reasonable to me.
>>
>> To be decided is through which tree this series should go.
>> I'd think SPI would be most suited, but that's a decision I
>> leave up to the respective maintainers.
>>
>> Further changes to the RFC version:
>> - Driver can be built also w/o LED class support, for displays that
>> don't have any symbols to be exposed as LED's.
>> - Simplified the code and rewrote a lot of it.
>> - Driver is now kind of a MVP, but functionality should be sufficient
>> for most use cases.
>> - Use the existing 7 segment support in uapi/linux/map_to_7segment.h
>> as suggested by Geert Uytterhoeven.
>>
>> Note: There's a number of chips from other manufacturers that are
>>     almost identical, e.g. FD628, SM1628. Only difference I saw so
>>     far is that they partially support other display modes.
>>     TM1628: 6x12, 7x11
>>     SM1628C: 4x13, 5x12, 6x11, 7x10
>>     For typical displays on devices using these chips this
>>     difference shouldn't matter.
>>
>> Successfully tested on a TX3 Mini TV box that has an SM1628C and a
>> display with 4 digits and 7 symbols.
> 
> Thanks for dusting off sources and working on this! - it’s another piece
> of the upstream puzzle for distros that install on Android boxes.
> 
> I needed the following patch to address compile issues (missing include,
> and the recent void/int change in linux-next (I’m using 5.17.y):
> 
> diff --git a/drivers/auxdisplay/tm1628.c b/drivers/auxdisplay/tm1628.c
> index a39b638282c1..ab3557f8b330 100644
> --- a/drivers/auxdisplay/tm1628.c
> +++ b/drivers/auxdisplay/tm1628.c
> @@ -5,6 +5,7 @@
> * Copyright (c) 2019 Andreas Färber
> */
> 
> +#include <linux/ctype.h>
> #include <linux/delay.h>
> #include <linux/leds.h>
> #include <linux/module.h>
> @@ -327,10 +328,11 @@ static int tm1628_spi_probe(struct spi_device *spi)
>       return device_create_file(&spi->dev, &dev_attr_display_text);
> }
> 
> -static void tm1628_spi_remove(struct spi_device *spi)
> +static int tm1628_spi_remove(struct spi_device *spi)
> {
>       device_remove_file(&spi->dev, &dev_attr_display_text);
>       tm1628_set_display_ctrl(spi, false);
> +       return 0;
> }
> 
> static void tm1628_spi_shutdown(struct spi_device *spi)
> 
> I also needed CONFIG_SPI_GPIO=y in kernel config. With this added the
> driver probes on my TX3 mini box and the display goes dark overwriting
> the default ‘boot’ text. The following systemd service and script sets
> the clock and flashes the colon separator on/off to count seconds:
> 
> https://github.com/chewitt/LibreELEC.tv/commit/c8f1ebe6f6c366188f18f9d2b401de6c2979fdd7
> 
> With the include fixup and maybe a Kconfig tweak, for the series:
> 
> Tested-by: Christian Hewitt <christianshewitt@gmail.com>

Thanks for testing! 
On some systems the display controller may be connected to a HW SPI
interface not using GPIO's. Therefore I'd prefer to not make
the driver dependent on CONFIG_SPI_GPIO.

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

[Heiner Kallweit]

On 21.02.2022 07:32, Christian Hewitt wrote:
> resend from correct mail account:
> 
>> On 19 Feb 2022, at 5:13 pm, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> This series adds support for the Titanmec TM1628 7 segment display
>> controller. It's based on previous RFC work from Andreas Färber.
>> The RFC version placed the driver in the LED subsystem, but this was
>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>> /drivers/auxdisplay what seems most reasonable to me.
>>
>> To be decided is through which tree this series should go.
>> I'd think SPI would be most suited, but that's a decision I
>> leave up to the respective maintainers.
>>
>> Further changes to the RFC version:
>> - Driver can be built also w/o LED class support, for displays that
>> don't have any symbols to be exposed as LED's.
>> - Simplified the code and rewrote a lot of it.
>> - Driver is now kind of a MVP, but functionality should be sufficient
>> for most use cases.
>> - Use the existing 7 segment support in uapi/linux/map_to_7segment.h
>> as suggested by Geert Uytterhoeven.
>>
>> Note: There's a number of chips from other manufacturers that are
>>     almost identical, e.g. FD628, SM1628. Only difference I saw so
>>     far is that they partially support other display modes.
>>     TM1628: 6x12, 7x11
>>     SM1628C: 4x13, 5x12, 6x11, 7x10
>>     For typical displays on devices using these chips this
>>     difference shouldn't matter.
>>
>> Successfully tested on a TX3 Mini TV box that has an SM1628C and a
>> display with 4 digits and 7 symbols.
> 
> Thanks for dusting off sources and working on this! - it’s another piece
> of the upstream puzzle for distros that install on Android boxes.
> 
> I needed the following patch to address compile issues (missing include,
> and the recent void/int change in linux-next (I’m using 5.17.y):
> 
> diff --git a/drivers/auxdisplay/tm1628.c b/drivers/auxdisplay/tm1628.c
> index a39b638282c1..ab3557f8b330 100644
> --- a/drivers/auxdisplay/tm1628.c
> +++ b/drivers/auxdisplay/tm1628.c
> @@ -5,6 +5,7 @@
> * Copyright (c) 2019 Andreas Färber
> */
> 
> +#include <linux/ctype.h>
> #include <linux/delay.h>
> #include <linux/leds.h>
> #include <linux/module.h>
> @@ -327,10 +328,11 @@ static int tm1628_spi_probe(struct spi_device *spi)
>       return device_create_file(&spi->dev, &dev_attr_display_text);
> }
> 
> -static void tm1628_spi_remove(struct spi_device *spi)
> +static int tm1628_spi_remove(struct spi_device *spi)
> {
>       device_remove_file(&spi->dev, &dev_attr_display_text);
>       tm1628_set_display_ctrl(spi, false);
> +       return 0;
> }
> 
> static void tm1628_spi_shutdown(struct spi_device *spi)
> 
> I also needed CONFIG_SPI_GPIO=y in kernel config. With this added the
> driver probes on my TX3 mini box and the display goes dark overwriting
> the default ‘boot’ text. The following systemd service and script sets
> the clock and flashes the colon separator on/off to count seconds:
> 
> https://github.com/chewitt/LibreELEC.tv/commit/c8f1ebe6f6c366188f18f9d2b401de6c2979fdd7
> 
> With the include fixup and maybe a Kconfig tweak, for the series:
> 
> Tested-by: Christian Hewitt <christianshewitt@gmail.com>

Thanks for testing! 
On some systems the display controller may be connected to a HW SPI
interface not using GPIO's. Therefore I'd prefer to not make
the driver dependent on CONFIG_SPI_GPIO.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Heiner Kallweit]

On 21.02.2022 07:32, Christian Hewitt wrote:
> resend from correct mail account:
> 
>> On 19 Feb 2022, at 5:13 pm, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> This series adds support for the Titanmec TM1628 7 segment display
>> controller. It's based on previous RFC work from Andreas Färber.
>> The RFC version placed the driver in the LED subsystem, but this was
>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>> /drivers/auxdisplay what seems most reasonable to me.
>>
>> To be decided is through which tree this series should go.
>> I'd think SPI would be most suited, but that's a decision I
>> leave up to the respective maintainers.
>>
>> Further changes to the RFC version:
>> - Driver can be built also w/o LED class support, for displays that
>> don't have any symbols to be exposed as LED's.
>> - Simplified the code and rewrote a lot of it.
>> - Driver is now kind of a MVP, but functionality should be sufficient
>> for most use cases.
>> - Use the existing 7 segment support in uapi/linux/map_to_7segment.h
>> as suggested by Geert Uytterhoeven.
>>
>> Note: There's a number of chips from other manufacturers that are
>>     almost identical, e.g. FD628, SM1628. Only difference I saw so
>>     far is that they partially support other display modes.
>>     TM1628: 6x12, 7x11
>>     SM1628C: 4x13, 5x12, 6x11, 7x10
>>     For typical displays on devices using these chips this
>>     difference shouldn't matter.
>>
>> Successfully tested on a TX3 Mini TV box that has an SM1628C and a
>> display with 4 digits and 7 symbols.
> 
> Thanks for dusting off sources and working on this! - it’s another piece
> of the upstream puzzle for distros that install on Android boxes.
> 
> I needed the following patch to address compile issues (missing include,
> and the recent void/int change in linux-next (I’m using 5.17.y):
> 
> diff --git a/drivers/auxdisplay/tm1628.c b/drivers/auxdisplay/tm1628.c
> index a39b638282c1..ab3557f8b330 100644
> --- a/drivers/auxdisplay/tm1628.c
> +++ b/drivers/auxdisplay/tm1628.c
> @@ -5,6 +5,7 @@
> * Copyright (c) 2019 Andreas Färber
> */
> 
> +#include <linux/ctype.h>
> #include <linux/delay.h>
> #include <linux/leds.h>
> #include <linux/module.h>
> @@ -327,10 +328,11 @@ static int tm1628_spi_probe(struct spi_device *spi)
>       return device_create_file(&spi->dev, &dev_attr_display_text);
> }
> 
> -static void tm1628_spi_remove(struct spi_device *spi)
> +static int tm1628_spi_remove(struct spi_device *spi)
> {
>       device_remove_file(&spi->dev, &dev_attr_display_text);
>       tm1628_set_display_ctrl(spi, false);
> +       return 0;
> }
> 
> static void tm1628_spi_shutdown(struct spi_device *spi)
> 
> I also needed CONFIG_SPI_GPIO=y in kernel config. With this added the
> driver probes on my TX3 mini box and the display goes dark overwriting
> the default ‘boot’ text. The following systemd service and script sets
> the clock and flashes the colon separator on/off to count seconds:
> 
> https://github.com/chewitt/LibreELEC.tv/commit/c8f1ebe6f6c366188f18f9d2b401de6c2979fdd7
> 
> With the include fixup and maybe a Kconfig tweak, for the series:
> 
> Tested-by: Christian Hewitt <christianshewitt@gmail.com>

Thanks for testing! 
On some systems the display controller may be connected to a HW SPI
interface not using GPIO's. Therefore I'd prefer to not make
the driver dependent on CONFIG_SPI_GPIO.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Andreas Färber]

On 19.02.22 18:16, Heiner Kallweit wrote:
> On 19.02.2022 17:07, Andreas Färber wrote:
>> Hi,
>>
>> On 19.02.22 14:37, Heiner Kallweit wrote:
>>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>>
>>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>>
>>>> Could you please link to the discussion and/or summarize the rationale
>>>> behind the NAK?
>>>>
>>>
>>> +Pavel
>>>
>>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>>> one that makes use of the LED subsystem, but doesn't belong to it.
>>> In the following mail he's expressing his opinion that the driver should
>>> be best placed under auxdisplay.
>>>
>>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
>>
>> And I disagreed. It does not fit with the other drivers in auxdisplay
>> that were operating on a much higher level.
>>
> 
> We need to find a place. And if Pavel has good reasons that it doesn't
> fit into the LED subsystem, and Miguel should be fine with having
> it in auxdisplay, then I'd see no reason to not go this way.
> 
>> I'd also like to point out that I did implement the map_to_7segment API,
> 
> Looking at the history of include/uapi/linux/map_to_7segment.h I see no
> commit from you. Seems I'm missing something here.

You're replying inline too early:

>> as was suggested, as you will find in my tree

As I said, I implemented it in my driver:

https://github.com/afaerber/linux/commit/bbecf951348c7de8ba922c6c002a09369b717d82

Thus me saying you are unnecessarily duplicating work that I already
did, without ping'ing the thread or me and claiming the credit for an
implementation change which I already did myself.

>> - which you may have
>> missed, referencing only the RFC patchset and putting your authorship on
>> it exclusively? A move from one directory to another should not warrant
>> my author and SoB getting removed from the actual driver.
>>
> The driver includes major changes and I mentioned your work in the commit
> message. Also your still listed as MODULE_AUTHOR. My intention is to
> get a driver upstream, not to earn credits for something.
> So sure, your SoB can be (re-)added.

https://github.com/afaerber/linux/commits/rtd1295-next

Also note this 5-in-4 optimization:

https://github.com/afaerber/linux/commit/ff8284b6ed9dc1e354c35840afdaf50b1cd97fea

And several more chipsets being covered.

>> Given that we need to manage a buffer with bits per segment or LED
>> symbol, one idea that I haven't found time for yet was to implement it
>> as framebuffer or drm device instead. (And most Realtek platforms got
>> broken by removing the adjustable text base defines.)
>>
> I'm not aware of the Realtek platform issue, do you have a link to a
> related discussion?

Realtek has a boot ROM at the beginning of memory space, which has been
a problem from the first RFC and for most bootloaders required to tweak
the kernel's text offset for successful boot. (Some not Open Source (LK)
and/or not openly flashable.)

http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html

In 2020 that arm64 feature got removed without any further discussion:

https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/

I've tried to revert it, but that's been a pain:

https://github.com/afaerber/linux/commit/0d2c647781bc89ee95bfa7b80d71237c7ebea230

> And wouldn't you think it's overengineering to
> write a DRM driver for a 7 segment display with 4 digits?
> Framebuffer seems to be deprecated based on my experience with
> pygame / SDL2.

Is there any other API that would allow userspace to write to the buffer
and bitblt parts to the SPI device?

Thinking of some optimizations I implemented in my driver to avoid
unnecessary SPI transfers:

https://github.com/afaerber/linux/commit/46c40209db163a81474c6894ebbd90b5e238ce60

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)

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

[Andreas Färber]

On 19.02.22 18:16, Heiner Kallweit wrote:
> On 19.02.2022 17:07, Andreas Färber wrote:
>> Hi,
>>
>> On 19.02.22 14:37, Heiner Kallweit wrote:
>>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>>
>>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>>
>>>> Could you please link to the discussion and/or summarize the rationale
>>>> behind the NAK?
>>>>
>>>
>>> +Pavel
>>>
>>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>>> one that makes use of the LED subsystem, but doesn't belong to it.
>>> In the following mail he's expressing his opinion that the driver should
>>> be best placed under auxdisplay.
>>>
>>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
>>
>> And I disagreed. It does not fit with the other drivers in auxdisplay
>> that were operating on a much higher level.
>>
> 
> We need to find a place. And if Pavel has good reasons that it doesn't
> fit into the LED subsystem, and Miguel should be fine with having
> it in auxdisplay, then I'd see no reason to not go this way.
> 
>> I'd also like to point out that I did implement the map_to_7segment API,
> 
> Looking at the history of include/uapi/linux/map_to_7segment.h I see no
> commit from you. Seems I'm missing something here.

You're replying inline too early:

>> as was suggested, as you will find in my tree

As I said, I implemented it in my driver:

https://github.com/afaerber/linux/commit/bbecf951348c7de8ba922c6c002a09369b717d82

Thus me saying you are unnecessarily duplicating work that I already
did, without ping'ing the thread or me and claiming the credit for an
implementation change which I already did myself.

>> - which you may have
>> missed, referencing only the RFC patchset and putting your authorship on
>> it exclusively? A move from one directory to another should not warrant
>> my author and SoB getting removed from the actual driver.
>>
> The driver includes major changes and I mentioned your work in the commit
> message. Also your still listed as MODULE_AUTHOR. My intention is to
> get a driver upstream, not to earn credits for something.
> So sure, your SoB can be (re-)added.

https://github.com/afaerber/linux/commits/rtd1295-next

Also note this 5-in-4 optimization:

https://github.com/afaerber/linux/commit/ff8284b6ed9dc1e354c35840afdaf50b1cd97fea

And several more chipsets being covered.

>> Given that we need to manage a buffer with bits per segment or LED
>> symbol, one idea that I haven't found time for yet was to implement it
>> as framebuffer or drm device instead. (And most Realtek platforms got
>> broken by removing the adjustable text base defines.)
>>
> I'm not aware of the Realtek platform issue, do you have a link to a
> related discussion?

Realtek has a boot ROM at the beginning of memory space, which has been
a problem from the first RFC and for most bootloaders required to tweak
the kernel's text offset for successful boot. (Some not Open Source (LK)
and/or not openly flashable.)

http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html

In 2020 that arm64 feature got removed without any further discussion:

https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/

I've tried to revert it, but that's been a pain:

https://github.com/afaerber/linux/commit/0d2c647781bc89ee95bfa7b80d71237c7ebea230

> And wouldn't you think it's overengineering to
> write a DRM driver for a 7 segment display with 4 digits?
> Framebuffer seems to be deprecated based on my experience with
> pygame / SDL2.

Is there any other API that would allow userspace to write to the buffer
and bitblt parts to the SPI device?

Thinking of some optimizations I implemented in my driver to avoid
unnecessary SPI transfers:

https://github.com/afaerber/linux/commit/46c40209db163a81474c6894ebbd90b5e238ce60

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Andreas Färber]

On 19.02.22 18:16, Heiner Kallweit wrote:
> On 19.02.2022 17:07, Andreas Färber wrote:
>> Hi,
>>
>> On 19.02.22 14:37, Heiner Kallweit wrote:
>>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>>
>>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>>
>>>> Could you please link to the discussion and/or summarize the rationale
>>>> behind the NAK?
>>>>
>>>
>>> +Pavel
>>>
>>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>>> one that makes use of the LED subsystem, but doesn't belong to it.
>>> In the following mail he's expressing his opinion that the driver should
>>> be best placed under auxdisplay.
>>>
>>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
>>
>> And I disagreed. It does not fit with the other drivers in auxdisplay
>> that were operating on a much higher level.
>>
> 
> We need to find a place. And if Pavel has good reasons that it doesn't
> fit into the LED subsystem, and Miguel should be fine with having
> it in auxdisplay, then I'd see no reason to not go this way.
> 
>> I'd also like to point out that I did implement the map_to_7segment API,
> 
> Looking at the history of include/uapi/linux/map_to_7segment.h I see no
> commit from you. Seems I'm missing something here.

You're replying inline too early:

>> as was suggested, as you will find in my tree

As I said, I implemented it in my driver:

https://github.com/afaerber/linux/commit/bbecf951348c7de8ba922c6c002a09369b717d82

Thus me saying you are unnecessarily duplicating work that I already
did, without ping'ing the thread or me and claiming the credit for an
implementation change which I already did myself.

>> - which you may have
>> missed, referencing only the RFC patchset and putting your authorship on
>> it exclusively? A move from one directory to another should not warrant
>> my author and SoB getting removed from the actual driver.
>>
> The driver includes major changes and I mentioned your work in the commit
> message. Also your still listed as MODULE_AUTHOR. My intention is to
> get a driver upstream, not to earn credits for something.
> So sure, your SoB can be (re-)added.

https://github.com/afaerber/linux/commits/rtd1295-next

Also note this 5-in-4 optimization:

https://github.com/afaerber/linux/commit/ff8284b6ed9dc1e354c35840afdaf50b1cd97fea

And several more chipsets being covered.

>> Given that we need to manage a buffer with bits per segment or LED
>> symbol, one idea that I haven't found time for yet was to implement it
>> as framebuffer or drm device instead. (And most Realtek platforms got
>> broken by removing the adjustable text base defines.)
>>
> I'm not aware of the Realtek platform issue, do you have a link to a
> related discussion?

Realtek has a boot ROM at the beginning of memory space, which has been
a problem from the first RFC and for most bootloaders required to tweak
the kernel's text offset for successful boot. (Some not Open Source (LK)
and/or not openly flashable.)

http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html

In 2020 that arm64 feature got removed without any further discussion:

https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/

I've tried to revert it, but that's been a pain:

https://github.com/afaerber/linux/commit/0d2c647781bc89ee95bfa7b80d71237c7ebea230

> And wouldn't you think it's overengineering to
> write a DRM driver for a 7 segment display with 4 digits?
> Framebuffer seems to be deprecated based on my experience with
> pygame / SDL2.

Is there any other API that would allow userspace to write to the buffer
and bitblt parts to the SPI device?

Thinking of some optimizations I implemented in my driver to avoid
unnecessary SPI transfers:

https://github.com/afaerber/linux/commit/46c40209db163a81474c6894ebbd90b5e238ce60

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Neil Armstrong]

On 22/02/2022 13:12, Andreas Färber wrote:
> On 19.02.22 18:16, Heiner Kallweit wrote:
>> On 19.02.2022 17:07, Andreas Färber wrote:
>>> Hi,
>>>
>>> On 19.02.22 14:37, Heiner Kallweit wrote:
>>>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>>>
>>>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>>>
>>>>> Could you please link to the discussion and/or summarize the rationale
>>>>> behind the NAK?
>>>>>
>>>>
>>>> +Pavel
>>>>
>>>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>>>> one that makes use of the LED subsystem, but doesn't belong to it.
>>>> In the following mail he's expressing his opinion that the driver should
>>>> be best placed under auxdisplay.
>>>>
>>>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
>>>
>>> And I disagreed. It does not fit with the other drivers in auxdisplay
>>> that were operating on a much higher level.
>>>
>>
>> We need to find a place. And if Pavel has good reasons that it doesn't
>> fit into the LED subsystem, and Miguel should be fine with having
>> it in auxdisplay, then I'd see no reason to not go this way.
>>
>>> I'd also like to point out that I did implement the map_to_7segment API,
>>
>> Looking at the history of include/uapi/linux/map_to_7segment.h I see no
>> commit from you. Seems I'm missing something here.
> 
> You're replying inline too early:
> 
>>> as was suggested, as you will find in my tree
> 
> As I said, I implemented it in my driver:
> 
> https://github.com/afaerber/linux/commit/bbecf951348c7de8ba922c6c002a09369b717d82
> 
> Thus me saying you are unnecessarily duplicating work that I already
> did, without ping'ing the thread or me and claiming the credit for an
> implementation change which I already did myself.
> 
>>> - which you may have
>>> missed, referencing only the RFC patchset and putting your authorship on
>>> it exclusively? A move from one directory to another should not warrant
>>> my author and SoB getting removed from the actual driver.
>>>
>> The driver includes major changes and I mentioned your work in the commit
>> message. Also your still listed as MODULE_AUTHOR. My intention is to
>> get a driver upstream, not to earn credits for something.
>> So sure, your SoB can be (re-)added.
> 
> https://github.com/afaerber/linux/commits/rtd1295-next
> 
> Also note this 5-in-4 optimization:
> 
> https://github.com/afaerber/linux/commit/ff8284b6ed9dc1e354c35840afdaf50b1cd97fea
> 
> And several more chipsets being covered.
> 
>>> Given that we need to manage a buffer with bits per segment or LED
>>> symbol, one idea that I haven't found time for yet was to implement it
>>> as framebuffer or drm device instead. (And most Realtek platforms got
>>> broken by removing the adjustable text base defines.)
>>>
>> I'm not aware of the Realtek platform issue, do you have a link to a
>> related discussion?
> 
> Realtek has a boot ROM at the beginning of memory space, which has been
> a problem from the first RFC and for most bootloaders required to tweak
> the kernel's text offset for successful boot. (Some not Open Source (LK)
> and/or not openly flashable.)
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html
> 
> In 2020 that arm64 feature got removed without any further discussion:
> 
> https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/

Note the TEXT_OFFSET is only an issue with Amlogic vendor bootloader,
it has never been an issue with mainline U-Boot.

Neil

> 
> I've tried to revert it, but that's been a pain:
> 
> https://github.com/afaerber/linux/commit/0d2c647781bc89ee95bfa7b80d71237c7ebea230
> 
>> And wouldn't you think it's overengineering to
>> write a DRM driver for a 7 segment display with 4 digits?
>> Framebuffer seems to be deprecated based on my experience with
>> pygame / SDL2.
> 
> Is there any other API that would allow userspace to write to the buffer
> and bitblt parts to the SPI device?
> 
> Thinking of some optimizations I implemented in my driver to avoid
> unnecessary SPI transfers:
> 
> https://github.com/afaerber/linux/commit/46c40209db163a81474c6894ebbd90b5e238ce60
> 
> Regards,
> Andreas
> 

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

[Neil Armstrong]

On 22/02/2022 13:12, Andreas Färber wrote:
> On 19.02.22 18:16, Heiner Kallweit wrote:
>> On 19.02.2022 17:07, Andreas Färber wrote:
>>> Hi,
>>>
>>> On 19.02.22 14:37, Heiner Kallweit wrote:
>>>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>>>
>>>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>>>
>>>>> Could you please link to the discussion and/or summarize the rationale
>>>>> behind the NAK?
>>>>>
>>>>
>>>> +Pavel
>>>>
>>>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>>>> one that makes use of the LED subsystem, but doesn't belong to it.
>>>> In the following mail he's expressing his opinion that the driver should
>>>> be best placed under auxdisplay.
>>>>
>>>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
>>>
>>> And I disagreed. It does not fit with the other drivers in auxdisplay
>>> that were operating on a much higher level.
>>>
>>
>> We need to find a place. And if Pavel has good reasons that it doesn't
>> fit into the LED subsystem, and Miguel should be fine with having
>> it in auxdisplay, then I'd see no reason to not go this way.
>>
>>> I'd also like to point out that I did implement the map_to_7segment API,
>>
>> Looking at the history of include/uapi/linux/map_to_7segment.h I see no
>> commit from you. Seems I'm missing something here.
> 
> You're replying inline too early:
> 
>>> as was suggested, as you will find in my tree
> 
> As I said, I implemented it in my driver:
> 
> https://github.com/afaerber/linux/commit/bbecf951348c7de8ba922c6c002a09369b717d82
> 
> Thus me saying you are unnecessarily duplicating work that I already
> did, without ping'ing the thread or me and claiming the credit for an
> implementation change which I already did myself.
> 
>>> - which you may have
>>> missed, referencing only the RFC patchset and putting your authorship on
>>> it exclusively? A move from one directory to another should not warrant
>>> my author and SoB getting removed from the actual driver.
>>>
>> The driver includes major changes and I mentioned your work in the commit
>> message. Also your still listed as MODULE_AUTHOR. My intention is to
>> get a driver upstream, not to earn credits for something.
>> So sure, your SoB can be (re-)added.
> 
> https://github.com/afaerber/linux/commits/rtd1295-next
> 
> Also note this 5-in-4 optimization:
> 
> https://github.com/afaerber/linux/commit/ff8284b6ed9dc1e354c35840afdaf50b1cd97fea
> 
> And several more chipsets being covered.
> 
>>> Given that we need to manage a buffer with bits per segment or LED
>>> symbol, one idea that I haven't found time for yet was to implement it
>>> as framebuffer or drm device instead. (And most Realtek platforms got
>>> broken by removing the adjustable text base defines.)
>>>
>> I'm not aware of the Realtek platform issue, do you have a link to a
>> related discussion?
> 
> Realtek has a boot ROM at the beginning of memory space, which has been
> a problem from the first RFC and for most bootloaders required to tweak
> the kernel's text offset for successful boot. (Some not Open Source (LK)
> and/or not openly flashable.)
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html
> 
> In 2020 that arm64 feature got removed without any further discussion:
> 
> https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/

Note the TEXT_OFFSET is only an issue with Amlogic vendor bootloader,
it has never been an issue with mainline U-Boot.

Neil

> 
> I've tried to revert it, but that's been a pain:
> 
> https://github.com/afaerber/linux/commit/0d2c647781bc89ee95bfa7b80d71237c7ebea230
> 
>> And wouldn't you think it's overengineering to
>> write a DRM driver for a 7 segment display with 4 digits?
>> Framebuffer seems to be deprecated based on my experience with
>> pygame / SDL2.
> 
> Is there any other API that would allow userspace to write to the buffer
> and bitblt parts to the SPI device?
> 
> Thinking of some optimizations I implemented in my driver to avoid
> unnecessary SPI transfers:
> 
> https://github.com/afaerber/linux/commit/46c40209db163a81474c6894ebbd90b5e238ce60
> 
> Regards,
> Andreas
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Neil Armstrong]

On 22/02/2022 13:12, Andreas Färber wrote:
> On 19.02.22 18:16, Heiner Kallweit wrote:
>> On 19.02.2022 17:07, Andreas Färber wrote:
>>> Hi,
>>>
>>> On 19.02.22 14:37, Heiner Kallweit wrote:
>>>> On 19.02.2022 14:27, Miguel Ojeda wrote:
>>>>> On Sat, Feb 19, 2022 at 2:13 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>>>
>>>>>> This series adds support for the Titanmec TM1628 7 segment display
>>>>>> controller. It's based on previous RFC work from Andreas Färber.
>>>>>> The RFC version placed the driver in the LED subsystem, but this was
>>>>>> NAK'ed by the LED maintainer. Therefore I moved the driver to
>>>>>> /drivers/auxdisplay what seems most reasonable to me.
>>>>>
>>>>> Could you please link to the discussion and/or summarize the rationale
>>>>> behind the NAK?
>>>>>
>>>>
>>>> +Pavel
>>>>
>>>> I didn't find an explicit reason, but I suppose Pavel sees this driver as
>>>> one that makes use of the LED subsystem, but doesn't belong to it.
>>>> In the following mail he's expressing his opinion that the driver should
>>>> be best placed under auxdisplay.
>>>>
>>>> https://lore.kernel.org/linux-arm-kernel/20200226130300.GB2800@duo.ucw.cz/
>>>
>>> And I disagreed. It does not fit with the other drivers in auxdisplay
>>> that were operating on a much higher level.
>>>
>>
>> We need to find a place. And if Pavel has good reasons that it doesn't
>> fit into the LED subsystem, and Miguel should be fine with having
>> it in auxdisplay, then I'd see no reason to not go this way.
>>
>>> I'd also like to point out that I did implement the map_to_7segment API,
>>
>> Looking at the history of include/uapi/linux/map_to_7segment.h I see no
>> commit from you. Seems I'm missing something here.
> 
> You're replying inline too early:
> 
>>> as was suggested, as you will find in my tree
> 
> As I said, I implemented it in my driver:
> 
> https://github.com/afaerber/linux/commit/bbecf951348c7de8ba922c6c002a09369b717d82
> 
> Thus me saying you are unnecessarily duplicating work that I already
> did, without ping'ing the thread or me and claiming the credit for an
> implementation change which I already did myself.
> 
>>> - which you may have
>>> missed, referencing only the RFC patchset and putting your authorship on
>>> it exclusively? A move from one directory to another should not warrant
>>> my author and SoB getting removed from the actual driver.
>>>
>> The driver includes major changes and I mentioned your work in the commit
>> message. Also your still listed as MODULE_AUTHOR. My intention is to
>> get a driver upstream, not to earn credits for something.
>> So sure, your SoB can be (re-)added.
> 
> https://github.com/afaerber/linux/commits/rtd1295-next
> 
> Also note this 5-in-4 optimization:
> 
> https://github.com/afaerber/linux/commit/ff8284b6ed9dc1e354c35840afdaf50b1cd97fea
> 
> And several more chipsets being covered.
> 
>>> Given that we need to manage a buffer with bits per segment or LED
>>> symbol, one idea that I haven't found time for yet was to implement it
>>> as framebuffer or drm device instead. (And most Realtek platforms got
>>> broken by removing the adjustable text base defines.)
>>>
>> I'm not aware of the Realtek platform issue, do you have a link to a
>> related discussion?
> 
> Realtek has a boot ROM at the beginning of memory space, which has been
> a problem from the first RFC and for most bootloaders required to tweak
> the kernel's text offset for successful boot. (Some not Open Source (LK)
> and/or not openly flashable.)
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html
> 
> In 2020 that arm64 feature got removed without any further discussion:
> 
> https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/

Note the TEXT_OFFSET is only an issue with Amlogic vendor bootloader,
it has never been an issue with mainline U-Boot.

Neil

> 
> I've tried to revert it, but that's been a pain:
> 
> https://github.com/afaerber/linux/commit/0d2c647781bc89ee95bfa7b80d71237c7ebea230
> 
>> And wouldn't you think it's overengineering to
>> write a DRM driver for a 7 segment display with 4 digits?
>> Framebuffer seems to be deprecated based on my experience with
>> pygame / SDL2.
> 
> Is there any other API that would allow userspace to write to the buffer
> and bitblt parts to the SPI device?
> 
> Thinking of some optimizations I implemented in my driver to avoid
> unnecessary SPI transfers:
> 
> https://github.com/afaerber/linux/commit/46c40209db163a81474c6894ebbd90b5e238ce60
> 
> Regards,
> Andreas
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Andreas Färber]

On 22.02.22 15:48, Neil Armstrong wrote:
> On 22/02/2022 13:12, Andreas Färber wrote:
>> On 19.02.22 18:16, Heiner Kallweit wrote:
>>> On 19.02.2022 17:07, Andreas Färber wrote:
>>>> [...] (And most Realtek platforms got
>>>> broken by removing the adjustable text base defines.)
>>>>
>>> I'm not aware of the Realtek platform issue, do you have a link to a
>>> related discussion?
>>
>> Realtek has a boot ROM at the beginning of memory space, which has been
>> a problem from the first RFC and for most bootloaders required to tweak
>> the kernel's text offset for successful boot. (Some not Open Source (LK)
>> and/or not openly flashable.)
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html
>>
>>
>> In 2020 that arm64 feature got removed without any further discussion:
>>
>> https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/
> 
> Note the TEXT_OFFSET is only an issue with Amlogic vendor bootloader,
> it has never been an issue with mainline U-Boot.

There is no mainline U-Boot for Realtek DHC (!= Amlogic Meson) though!

More important drivers than LED got blocked here, too, like MMC and USB
and pinctrl and clk.

And as hinted above, some Realtek boards come with a vendor LK that I
can't even patch a downstream version of.

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)

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

[Andreas Färber]

On 22.02.22 15:48, Neil Armstrong wrote:
> On 22/02/2022 13:12, Andreas Färber wrote:
>> On 19.02.22 18:16, Heiner Kallweit wrote:
>>> On 19.02.2022 17:07, Andreas Färber wrote:
>>>> [...] (And most Realtek platforms got
>>>> broken by removing the adjustable text base defines.)
>>>>
>>> I'm not aware of the Realtek platform issue, do you have a link to a
>>> related discussion?
>>
>> Realtek has a boot ROM at the beginning of memory space, which has been
>> a problem from the first RFC and for most bootloaders required to tweak
>> the kernel's text offset for successful boot. (Some not Open Source (LK)
>> and/or not openly flashable.)
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html
>>
>>
>> In 2020 that arm64 feature got removed without any further discussion:
>>
>> https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/
> 
> Note the TEXT_OFFSET is only an issue with Amlogic vendor bootloader,
> it has never been an issue with mainline U-Boot.

There is no mainline U-Boot for Realtek DHC (!= Amlogic Meson) though!

More important drivers than LED got blocked here, too, like MMC and USB
and pinctrl and clk.

And as hinted above, some Realtek boards come with a vendor LK that I
can't even patch a downstream version of.

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Andreas Färber]

On 22.02.22 15:48, Neil Armstrong wrote:
> On 22/02/2022 13:12, Andreas Färber wrote:
>> On 19.02.22 18:16, Heiner Kallweit wrote:
>>> On 19.02.2022 17:07, Andreas Färber wrote:
>>>> [...] (And most Realtek platforms got
>>>> broken by removing the adjustable text base defines.)
>>>>
>>> I'm not aware of the Realtek platform issue, do you have a link to a
>>> related discussion?
>>
>> Realtek has a boot ROM at the beginning of memory space, which has been
>> a problem from the first RFC and for most bootloaders required to tweak
>> the kernel's text offset for successful boot. (Some not Open Source (LK)
>> and/or not openly flashable.)
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/487718.html
>>
>>
>> In 2020 that arm64 feature got removed without any further discussion:
>>
>> https://lore.kernel.org/all/20200825135440.11288-1-ardb@kernel.org/
> 
> Note the TEXT_OFFSET is only an issue with Amlogic vendor bootloader,
> it has never been an issue with mainline U-Boot.

There is no mainline U-Boot for Realtek DHC (!= Amlogic Meson) though!

More important drivers than LED got blocked here, too, like MMC and USB
and pinctrl and clk.

And as hinted above, some Realtek boards come with a vendor LK that I
can't even patch a downstream version of.

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Ivo Totev
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Mark Brown]

On Sat, 19 Feb 2022 14:13:12 +0100, Heiner Kallweit wrote:
> This series adds support for the Titanmec TM1628 7 segment display
> controller. It's based on previous RFC work from Andreas Färber.
> The RFC version placed the driver in the LED subsystem, but this was
> NAK'ed by the LED maintainer. Therefore I moved the driver to
> /drivers/auxdisplay what seems most reasonable to me.
> 
> To be decided is through which tree this series should go.
> I'd think SPI would be most suited, but that's a decision I
> leave up to the respective maintainers.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/6] spi: gpio: Implement LSB First bitbang support
      commit: 1847e3046c528bd85bd51e2860f4139bd9052d6c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Mark Brown]

On Sat, 19 Feb 2022 14:13:12 +0100, Heiner Kallweit wrote:
> This series adds support for the Titanmec TM1628 7 segment display
> controller. It's based on previous RFC work from Andreas Färber.
> The RFC version placed the driver in the LED subsystem, but this was
> NAK'ed by the LED maintainer. Therefore I moved the driver to
> /drivers/auxdisplay what seems most reasonable to me.
> 
> To be decided is through which tree this series should go.
> I'd think SPI would be most suited, but that's a decision I
> leave up to the respective maintainers.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/6] spi: gpio: Implement LSB First bitbang support
      commit: 1847e3046c528bd85bd51e2860f4139bd9052d6c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel