Re: [PATCH 16/17] auxdisplay: ht16k33: Add support for segment displays
On 2021-03-29 09:15, Geert Uytterhoeven wrote:
Hoi Robin,
On Mon, Mar 29, 2021 at 9:09 AM Robin van der Gracht
wrote:
On 2021-03-22 15:48, Geert Uytterhoeven wrote:
> The Holtek HT16K33 LED controller is not only used for driving
> dot-matrix displays, but also for driving segment displays.
>
> Add support for 4-digit 7-segment and quad 14-segment alphanumeric
> displays, like the Adafruit 7-segment and 14-segment display backpack
> and FeatherWing expansion boards. Use the character line display core
> support to display a message, which will be scrolled if it doesn't fit.
>
> Signed-off-by: Geert Uytterhoeven
> ---
> The 7-segment support is based on schematics, and untested on actual
> hardware.
> ---
> drivers/auxdisplay/ht16k33.c | 198 +--
> 1 file changed, 191 insertions(+), 7 deletions(-)
>
...
>
> +static int ht16k33_seg_probe(struct i2c_client *client,
> + struct ht16k33_priv *priv, uint32_t brightness)
> +{
> + struct ht16k33_seg *seg = >seg;
> + struct device *dev = >dev;
> + int err;
> +
> + err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
> + if (err)
> + return err;
> +
> + switch (priv->type) {
> + case DISP_MATRIX:
> + /* not handled here */
> + break;
This 'case' shouldn't happen. Having said that, the break here will
still
cause the linedisp_register() function to be called for the
DISP_MATRIX
type.
If you'd like to handle this case, a return (or setting 'err') should
prevent this.
This function is never called if priv->type == DISP_MATRIX, so this
cannot happen. However, gcc complains if not all enum values are
handled in a switch() statement, hence the dummy case.
I see. But if it's there it should work right? :P
Is there a better way to handle this?
How about adding the default case:
default:
/* not handled */
err = -EINVAL;
Groetjes/Kind regards,
Robin van der Gracht
--
Protonic Holland
Factorij 36
1689AL Zwaag
+31 (0)229 212928
https://www.protonic.nl
Re: [PATCH 16/17] auxdisplay: ht16k33: Add support for segment displays
Hoi Robin,
On Mon, Mar 29, 2021 at 9:09 AM Robin van der Gracht wrote:
> On 2021-03-22 15:48, Geert Uytterhoeven wrote:
> > The Holtek HT16K33 LED controller is not only used for driving
> > dot-matrix displays, but also for driving segment displays.
> >
> > Add support for 4-digit 7-segment and quad 14-segment alphanumeric
> > displays, like the Adafruit 7-segment and 14-segment display backpack
> > and FeatherWing expansion boards. Use the character line display core
> > support to display a message, which will be scrolled if it doesn't fit.
> >
> > Signed-off-by: Geert Uytterhoeven
> > ---
> > The 7-segment support is based on schematics, and untested on actual
> > hardware.
> > ---
> > drivers/auxdisplay/ht16k33.c | 198 +--
> > 1 file changed, 191 insertions(+), 7 deletions(-)
> >
> ...
> >
> > +static int ht16k33_seg_probe(struct i2c_client *client,
> > + struct ht16k33_priv *priv, uint32_t brightness)
> > +{
> > + struct ht16k33_seg *seg = >seg;
> > + struct device *dev = >dev;
> > + int err;
> > +
> > + err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
> > + if (err)
> > + return err;
> > +
> > + switch (priv->type) {
> > + case DISP_MATRIX:
> > + /* not handled here */
> > + break;
>
> This 'case' shouldn't happen. Having said that, the break here will
> still
> cause the linedisp_register() function to be called for the DISP_MATRIX
> type.
> If you'd like to handle this case, a return (or setting 'err') should
> prevent this.
This function is never called if priv->type == DISP_MATRIX, so this
cannot happen. However, gcc complains if not all enum values are
handled in a switch() statement, hence the dummy case.
Is there a better way to handle this?
> > + case DISP_QUAD_7SEG:
> > + INIT_DELAYED_WORK(>work, ht16k33_seg7_update);
> > + seg->map.seg7 = initial_map_seg7;
> > + seg->map_size = sizeof(seg->map.seg7);
> > + err = device_create_file(dev, _attr_map_seg7);
> > + break;
> > +
> > + case DISP_QUAD_14SEG:
> > + INIT_DELAYED_WORK(>work, ht16k33_seg14_update);
> > + seg->map.seg14 = initial_map_seg14;
> > + seg->map_size = sizeof(seg->map.seg14);
> > + err = device_create_file(dev, _attr_map_seg14);
> > + break;
> > + }
> > + if (err)
> > + return err;
> > +
> > + err = linedisp_register(>linedisp, dev, 4, seg->curr,
> > + ht16k33_linedisp_update);
> > + if (err)
> > + goto err_remove_map_file;
> > +
> > + return 0;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 16/17] auxdisplay: ht16k33: Add support for segment displays
Hello Geert,
On 2021-03-22 15:48, Geert Uytterhoeven wrote:
The Holtek HT16K33 LED controller is not only used for driving
dot-matrix displays, but also for driving segment displays.
Add support for 4-digit 7-segment and quad 14-segment alphanumeric
displays, like the Adafruit 7-segment and 14-segment display backpack
and FeatherWing expansion boards. Use the character line display core
support to display a message, which will be scrolled if it doesn't fit.
Signed-off-by: Geert Uytterhoeven
---
The 7-segment support is based on schematics, and untested on actual
hardware.
---
drivers/auxdisplay/ht16k33.c | 198 +--
1 file changed, 191 insertions(+), 7 deletions(-)
...
+static int ht16k33_seg_probe(struct i2c_client *client,
+struct ht16k33_priv *priv, uint32_t brightness)
+{
+ struct ht16k33_seg *seg = >seg;
+ struct device *dev = >dev;
+ int err;
+
+ err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
+ if (err)
+ return err;
+
+ switch (priv->type) {
+ case DISP_MATRIX:
+ /* not handled here */
+ break;
This 'case' shouldn't happen. Having said that, the break here will
still
cause the linedisp_register() function to be called for the DISP_MATRIX
type.
If you'd like to handle this case, a return (or setting 'err') should
prevent this.
+
+ case DISP_QUAD_7SEG:
+ INIT_DELAYED_WORK(>work, ht16k33_seg7_update);
+ seg->map.seg7 = initial_map_seg7;
+ seg->map_size = sizeof(seg->map.seg7);
+ err = device_create_file(dev, _attr_map_seg7);
+ break;
+
+ case DISP_QUAD_14SEG:
+ INIT_DELAYED_WORK(>work, ht16k33_seg14_update);
+ seg->map.seg14 = initial_map_seg14;
+ seg->map_size = sizeof(seg->map.seg14);
+ err = device_create_file(dev, _attr_map_seg14);
+ break;
+ }
+ if (err)
+ return err;
+
+ err = linedisp_register(>linedisp, dev, 4, seg->curr,
+ ht16k33_linedisp_update);
+ if (err)
+ goto err_remove_map_file;
+
+ return 0;
Groetjes/Kind regards,
Robin van der Gracht
[PATCH 16/17] auxdisplay: ht16k33: Add support for segment displays
The Holtek HT16K33 LED controller is not only used for driving
dot-matrix displays, but also for driving segment displays.
Add support for 4-digit 7-segment and quad 14-segment alphanumeric
displays, like the Adafruit 7-segment and 14-segment display backpack
and FeatherWing expansion boards. Use the character line display core
support to display a message, which will be scrolled if it doesn't fit.
Signed-off-by: Geert Uytterhoeven
---
The 7-segment support is based on schematics, and untested on actual
hardware.
---
drivers/auxdisplay/ht16k33.c | 198 +--
1 file changed, 191 insertions(+), 7 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 11d86960fbc5d8c1..0b502a6039f89c6b 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -5,6 +5,7 @@
* Author: Robin van der Gracht
*
* Copyright: (C) 2016 Protonic Holland.
+ * Copyright (C) 2021 Glider bv
*/
#include
@@ -17,9 +18,18 @@
#include
#include
#include
+#include
+#include
#include
#include
+#include
+#include
+
+#include
+
+#include "line-display.h"
+
/* Registers */
#define REG_SYSTEM_SETUP 0x20
#define REG_SYSTEM_SETUP_OSC_ONBIT(0)
@@ -47,6 +57,12 @@
#define BYTES_PER_ROW (HT16K33_MATRIX_LED_MAX_ROWS / 8)
#define HT16K33_FB_SIZE(HT16K33_MATRIX_LED_MAX_COLS *
BYTES_PER_ROW)
+enum display_type {
+ DISP_MATRIX = 0,
+ DISP_QUAD_7SEG,
+ DISP_QUAD_14SEG,
+};
+
struct ht16k33_keypad {
struct i2c_client *client;
struct input_dev *dev;
@@ -67,11 +83,25 @@ struct ht16k33_fbdev {
uint8_t *cache;
};
+struct ht16k33_seg {
+ struct linedisp linedisp;
+ union {
+ struct seg7_conversion_map seg7;
+ struct seg14_conversion_map seg14;
+ } map;
+ unsigned int map_size;
+ char curr[4];
+};
+
struct ht16k33_priv {
struct i2c_client *client;
struct delayed_work work;
struct ht16k33_keypad keypad;
- struct ht16k33_fbdev fbdev;
+ union {
+ struct ht16k33_fbdev fbdev;
+ struct ht16k33_seg seg;
+ };
+ enum display_type type;
};
static const struct fb_fix_screeninfo ht16k33_fb_fix = {
@@ -101,6 +131,33 @@ static const struct fb_var_screeninfo ht16k33_fb_var = {
.vmode = FB_VMODE_NONINTERLACED,
};
+static const SEG7_DEFAULT_MAP(initial_map_seg7);
+static const SEG14_DEFAULT_MAP(initial_map_seg14);
+
+static ssize_t map_seg_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct ht16k33_priv *priv = dev_get_drvdata(dev);
+
+ memcpy(buf, >seg.map, priv->seg.map_size);
+ return priv->seg.map_size;
+}
+
+static ssize_t map_seg_store(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t cnt)
+{
+ struct ht16k33_priv *priv = dev_get_drvdata(dev);
+
+ if (cnt != priv->seg.map_size)
+ return -EINVAL;
+
+ memcpy(>seg.map, buf, cnt);
+ return cnt;
+}
+
+static DEVICE_ATTR(map_seg7, 0644, map_seg_show, map_seg_store);
+static DEVICE_ATTR(map_seg14, 0644, map_seg_show, map_seg_store);
+
static int ht16k33_display_on(struct ht16k33_priv *priv)
{
uint8_t data = REG_DISPLAY_SETUP | REG_DISPLAY_SETUP_ON;
@@ -325,6 +382,51 @@ static void ht16k33_keypad_stop(struct input_dev *dev)
disable_irq(keypad->client->irq);
}
+static void ht16k33_linedisp_update(struct linedisp *linedisp)
+{
+ struct ht16k33_priv *priv = container_of(linedisp, struct ht16k33_priv,
+seg.linedisp);
+
+ schedule_delayed_work(>work, 0);
+}
+
+static void ht16k33_seg7_update(struct work_struct *work)
+{
+ struct ht16k33_priv *priv = container_of(work, struct ht16k33_priv,
+work.work);
+ struct ht16k33_seg *seg = >seg;
+ char *s = seg->curr;
+ uint8_t buf[9];
+
+ buf[0] = map_to_seg7(>map.seg7, *s++);
+ buf[1] = 0;
+ buf[2] = map_to_seg7(>map.seg7, *s++);
+ buf[3] = 0;
+ buf[4] = 0;
+ buf[5] = 0;
+ buf[6] = map_to_seg7(>map.seg7, *s++);
+ buf[7] = 0;
+ buf[8] = map_to_seg7(>map.seg7, *s++);
+
+ i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
+}
+
+static void ht16k33_seg14_update(struct work_struct *work)
+{
+ struct ht16k33_priv *priv = container_of(work, struct ht16k33_priv,
+work.work);
+ struct ht16k33_seg *seg = >seg;
+ char *s = seg->curr;
+ uint8_t buf[8];
+
+ put_unaligned_le16(map_to_seg14(>map.seg14, *s++), buf);
+ put_unaligned_le16(map_to_seg14(>map.seg14, *s++), buf + 2);
+ put_unaligned_le16(map_to_seg14(>map.seg14, *s++), buf + 4);
+
