Hi all,
The Holtek HT16K33 LED controller is not only used for driving
dot-matrix displays, but also for driving segment displays.
The current auxdisplay driver is limited to dot-matrix displays, which
are exposed as a frame buffer device.
This patch series extends the driver to 4-digit 7-segment and quad
14-segment alphanumeric displays, allowing the user to display and
scroll text messages.
List of patches:
- Patch 1 provides font data for displaying ASCII characters on
14-segment displays,
- Patch 2 updates the HT16K33 DT bindings for segment displays,
- Patches 3-5 contain a bug fix and small improvements for the
Imagination Technologies ASCII LCD Display driver,
- Patch 6 extracts the character line display core support from the
Imagination Technologies ASCII LCD Display driver, for reuse,
- Patches 7-8 contain cleanups and improvements for the character line
display core driver,
- Patches 9-16 contain a bug fix, cleanups and improvements for the
HT16K33 driver, to prepare for segment display support,
- Patch 17 adds support for 7/14-segment displays to the HT16K33
driver,
- Patch 18 updates the HT16K33 DT bindings to document an LED subnode,
- Patch 19 adds segment display LED support to the HT16K33 driver,
to make use of hardware blinking, and to expose display color.
Changes compared to v5[1]:
- Add Reviewed-by,
- Reorder operations in ht16k33_led_probe() to ease future conversion
to device properties.
Changes compared to v4[2]:
- Add Reviewed-by,
- Add missing select NEW_LEDS.
Changes compared to v3[3]:
- Combine compatible values for 7/14 segment displays into an enum,
- Add Reviewed-by,
- Add missing select LEDS_CLASS.
Changes compared to v2[4]:
- Drop color property from display node,
- Use compat_only_sysfs_link_entry_to_kobj() instead of cooking our
own helper on top of kernfs_create_link(),
- Use "err" instead of "error" to be consistent with existing driver
naming style,
- Pass "dev" instead of "client" to ht16k33_fbdev_probe() and
ht16k33_seg_probe(),
- Drop local variable "node",
- Remove unneeded inclusion of <linux/leds.h> and <linux/of_device.h>,
- Document LED subnode,
- Remove unneeded C++ comment,
- Make the creation of the LED device dependent on the presence of the
"led" subnode in DT, so it can be used in dot-matrix mode too.
- Use led_init_data() and devm_led_classdev_register_ext() to retrieve
all LED properties from DT, instead of manual LED name construction
based on just the "color" property.
Changes compared to v1[5]:
- Fix type of color to uint32,
- "refresh-rate-hz" is still required for dot-matrix displays.
- Move "select LINEDISP" for HT16K33 symbol to correct patch,
- Add backwards compatibility "message" symlink to img-ascii-lcd,
- Connect backlight to fbdev in ht16k33 dot-matrix mode,
- Set "err = -EINVAL" in switch() case that cannot happen,
- Use "auxdisplay" instead of DRIVER_NAME in LED name.
This series has been tested using an Adafruit 0.54" Quad Alphanumeric
Red FeatherWing Display, connected to an OrangeCrab ECP5 FPGA board
running a 64 MHz VexRiscv RISC-V softcore.
7-segment display support is based purely on schematics, and has not
been tested on actual hardware. The changes to img-ascii-lcd.c are also
untested, due to lack of hardware.
Thanks for applying!
[1] "[PATCH v6 00/19] auxdisplay: ht16k33: Add character display support"
https://lore.kernel.org/r/[email protected]
[2] "[PATCH v4 00/19] auxdisplay: ht16k33: Add character display support"
https://lore.kernel.org/r/[email protected]/
[3] "[PATCH v3 00/19] auxdisplay: ht16k33: Add character display support"
https://lore.kernel.org/r/[email protected]/
[4] "[PATCH v2 00/18] auxdisplay: ht16k33: Add character display support"
https://lore.kernel.org/r/[email protected]/
[5] "[PATCH 00/17] auxdisplay: ht16k33: Add character display support"
https://lore.kernel.org/r/[email protected]/
Geert Uytterhoeven (19):
uapi: Add <linux/map_to_14segment.h>
dt-bindings: auxdisplay: ht16k33: Document Adafruit segment displays
auxdisplay: img-ascii-lcd: Fix lock-up when displaying empty string
auxdisplay: img-ascii-lcd: Add helper variable dev
auxdisplay: img-ascii-lcd: Convert device attribute to sysfs_emit()
auxdisplay: Extract character line display core support
auxdisplay: linedisp: Use kmemdup_nul() helper
auxdisplay: linedisp: Add support for changing scroll rate
auxdisplay: ht16k33: Connect backlight to fbdev
auxdisplay: ht16k33: Use HT16K33_FB_SIZE in ht16k33_initialize()
auxdisplay: ht16k33: Remove unneeded error check in keypad probe()
auxdisplay: ht16k33: Convert to simple i2c probe function
auxdisplay: ht16k33: Add helper variable dev
auxdisplay: ht16k33: Move delayed work
auxdisplay: ht16k33: Extract ht16k33_brightness_set()
auxdisplay: ht16k33: Extract frame buffer probing
auxdisplay: ht16k33: Add support for segment displays
dt-bindings: auxdisplay: ht16k33: Document LED subnode
auxdisplay: ht16k33: Add LED support
.../bindings/auxdisplay/holtek,ht16k33.yaml | 31 +-
drivers/auxdisplay/Kconfig | 10 +
drivers/auxdisplay/Makefile | 1 +
drivers/auxdisplay/ht16k33.c | 473 ++++++++++++++----
drivers/auxdisplay/img-ascii-lcd.c | 205 ++------
drivers/auxdisplay/line-display.c | 261 ++++++++++
drivers/auxdisplay/line-display.h | 43 ++
include/uapi/linux/map_to_14segment.h | 239 +++++++++
8 files changed, 996 insertions(+), 267 deletions(-)
create mode 100644 drivers/auxdisplay/line-display.c
create mode 100644 drivers/auxdisplay/line-display.h
create mode 100644 include/uapi/linux/map_to_14segment.h
--
2.25.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Extract the character line display core support from the simple ASCII
LCD driver for the MIPS Boston, Malta & SEAD3 development boards into
its own subdriver, so it can be reused for other displays.
As this moves the "message" device attribute in sysfs in a "linedisp.N"
subdirectory, a symlink is added to preserve backwards compatibility.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Changes to img-ascii-lcd.c are untested due to lack of hardware.
The backwards compatibility symlink has been tested by adding similar
code to drivers/auxdisplay/ht16k33.c.
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- Use compat_only_sysfs_link_entry_to_kobj() instead of cooking our
own helper on top of kernfs_create_link(),
v2:
- Drop "select LINEDISP" from HT16K33 symbol,
- Add backwards compatibility symlink.
---
drivers/auxdisplay/Kconfig | 7 +
drivers/auxdisplay/Makefile | 1 +
drivers/auxdisplay/img-ascii-lcd.c | 204 ++++---------------------
drivers/auxdisplay/line-display.c | 231 +++++++++++++++++++++++++++++
drivers/auxdisplay/line-display.h | 43 ++++++
5 files changed, 312 insertions(+), 174 deletions(-)
create mode 100644 drivers/auxdisplay/line-display.c
create mode 100644 drivers/auxdisplay/line-display.h
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 1509cb74705a30ad..b41fdc5f7bf72a1b 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -25,6 +25,12 @@ config CHARLCD
This is some character LCD core interface that multiple drivers can
use.
+config LINEDISP
+ tristate "Character line display core support" if COMPILE_TEST
+ help
+ This is the core support for single-line character displays, to be
+ selected by drivers that use it.
+
config HD44780_COMMON
tristate "Common functions for HD44780 (and compatibles) LCD displays" if COMPILE_TEST
select CHARLCD
@@ -155,6 +161,7 @@ config IMG_ASCII_LCD
depends on HAS_IOMEM
default y if MIPS_MALTA
select MFD_SYSCON
+ select LINEDISP
help
Enable this to support the simple ASCII LCD displays found on
development boards such as the MIPS Boston, MIPS Malta & MIPS SEAD3
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index 307771027c893a5b..6968ed4d3f0a8930 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_HD44780) += hd44780.o
obj-$(CONFIG_HT16K33) += ht16k33.o
obj-$(CONFIG_PARPORT_PANEL) += panel.o
obj-$(CONFIG_LCD2S) += lcd2s.o
+obj-$(CONFIG_LINEDISP) += line-display.o
diff --git a/drivers/auxdisplay/img-ascii-lcd.c b/drivers/auxdisplay/img-ascii-lcd.c
index 2b5640b638900a90..fa23e415f260ec56 100644
--- a/drivers/auxdisplay/img-ascii-lcd.c
+++ b/drivers/auxdisplay/img-ascii-lcd.c
@@ -4,7 +4,6 @@
* Author: Paul Burton <[email protected]>
*/
-#include <generated/utsrelease.h>
#include <linux/kernel.h>
#include <linux/io.h>
#include <linux/mfd/syscon.h>
@@ -14,7 +13,8 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/slab.h>
-#include <linux/sysfs.h>
+
+#include "line-display.h"
struct img_ascii_lcd_ctx;
@@ -27,36 +27,26 @@ struct img_ascii_lcd_ctx;
struct img_ascii_lcd_config {
unsigned int num_chars;
bool external_regmap;
- void (*update)(struct img_ascii_lcd_ctx *ctx);
+ void (*update)(struct linedisp *linedisp);
};
/**
* struct img_ascii_lcd_ctx - Private data structure
- * @pdev: the ASCII LCD platform device
* @base: the base address of the LCD registers
* @regmap: the regmap through which LCD registers are accessed
* @offset: the offset within regmap to the start of the LCD registers
* @cfg: pointer to the LCD model configuration
- * @message: the full message to display or scroll on the LCD
- * @message_len: the length of the @message string
- * @scroll_pos: index of the first character of @message currently displayed
- * @scroll_rate: scroll interval in jiffies
- * @timer: timer used to implement scrolling
+ * @linedisp: line display structure
* @curr: the string currently displayed on the LCD
*/
struct img_ascii_lcd_ctx {
- struct platform_device *pdev;
union {
void __iomem *base;
struct regmap *regmap;
};
u32 offset;
const struct img_ascii_lcd_config *cfg;
- char *message;
- unsigned int message_len;
- unsigned int scroll_pos;
- unsigned int scroll_rate;
- struct timer_list timer;
+ struct linedisp linedisp;
char curr[] __aligned(8);
};
@@ -64,8 +54,10 @@ struct img_ascii_lcd_ctx {
* MIPS Boston development board
*/
-static void boston_update(struct img_ascii_lcd_ctx *ctx)
+static void boston_update(struct linedisp *linedisp)
{
+ struct img_ascii_lcd_ctx *ctx =
+ container_of(linedisp, struct img_ascii_lcd_ctx, linedisp);
ulong val;
#if BITS_PER_LONG == 64
@@ -90,12 +82,14 @@ static struct img_ascii_lcd_config boston_config = {
* MIPS Malta development board
*/
-static void malta_update(struct img_ascii_lcd_ctx *ctx)
+static void malta_update(struct linedisp *linedisp)
{
+ struct img_ascii_lcd_ctx *ctx =
+ container_of(linedisp, struct img_ascii_lcd_ctx, linedisp);
unsigned int i;
int err = 0;
- for (i = 0; i < ctx->cfg->num_chars; i++) {
+ for (i = 0; i < linedisp->num_chars; i++) {
err = regmap_write(ctx->regmap,
ctx->offset + (i * 8), ctx->curr[i]);
if (err)
@@ -173,12 +167,14 @@ static int sead3_wait_lcd_idle(struct img_ascii_lcd_ctx *ctx)
return 0;
}
-static void sead3_update(struct img_ascii_lcd_ctx *ctx)
+static void sead3_update(struct linedisp *linedisp)
{
+ struct img_ascii_lcd_ctx *ctx =
+ container_of(linedisp, struct img_ascii_lcd_ctx, linedisp);
unsigned int i;
int err = 0;
- for (i = 0; i < ctx->cfg->num_chars; i++) {
+ for (i = 0; i < linedisp->num_chars; i++) {
err = sead3_wait_lcd_idle(ctx);
if (err)
break;
@@ -218,140 +214,6 @@ static const struct of_device_id img_ascii_lcd_matches[] = {
};
MODULE_DEVICE_TABLE(of, img_ascii_lcd_matches);
-/**
- * img_ascii_lcd_scroll() - scroll the display by a character
- * @t: really a pointer to the private data structure
- *
- * Scroll the current message along the LCD by one character, rearming the
- * timer if required.
- */
-static void img_ascii_lcd_scroll(struct timer_list *t)
-{
- struct img_ascii_lcd_ctx *ctx = from_timer(ctx, t, timer);
- unsigned int i, ch = ctx->scroll_pos;
- unsigned int num_chars = ctx->cfg->num_chars;
-
- /* update the current message string */
- for (i = 0; i < num_chars;) {
- /* copy as many characters from the string as possible */
- for (; i < num_chars && ch < ctx->message_len; i++, ch++)
- ctx->curr[i] = ctx->message[ch];
-
- /* wrap around to the start of the string */
- ch = 0;
- }
-
- /* update the LCD */
- ctx->cfg->update(ctx);
-
- /* move on to the next character */
- ctx->scroll_pos++;
- ctx->scroll_pos %= ctx->message_len;
-
- /* rearm the timer */
- if (ctx->message_len > ctx->cfg->num_chars)
- mod_timer(&ctx->timer, jiffies + ctx->scroll_rate);
-}
-
-/**
- * img_ascii_lcd_display() - set the message to be displayed
- * @ctx: pointer to the private data structure
- * @msg: the message to display
- * @count: length of msg, or -1
- *
- * Display a new message @msg on the LCD. @msg can be longer than the number of
- * characters the LCD can display, in which case it will begin scrolling across
- * the LCD display.
- *
- * Return: 0 on success, -ENOMEM on memory allocation failure
- */
-static int img_ascii_lcd_display(struct img_ascii_lcd_ctx *ctx,
- const char *msg, ssize_t count)
-{
- char *new_msg;
-
- /* stop the scroll timer */
- del_timer_sync(&ctx->timer);
-
- if (count == -1)
- count = strlen(msg);
-
- /* if the string ends with a newline, trim it */
- if (msg[count - 1] == '\n')
- count--;
-
- if (!count) {
- /* clear the LCD */
- devm_kfree(&ctx->pdev->dev, ctx->message);
- ctx->message = NULL;
- ctx->message_len = 0;
- memset(ctx->curr, ' ', ctx->cfg->num_chars);
- ctx->cfg->update(ctx);
- return 0;
- }
-
- new_msg = devm_kmalloc(&ctx->pdev->dev, count + 1, GFP_KERNEL);
- if (!new_msg)
- return -ENOMEM;
-
- memcpy(new_msg, msg, count);
- new_msg[count] = 0;
-
- if (ctx->message)
- devm_kfree(&ctx->pdev->dev, ctx->message);
-
- ctx->message = new_msg;
- ctx->message_len = count;
- ctx->scroll_pos = 0;
-
- /* update the LCD */
- img_ascii_lcd_scroll(&ctx->timer);
-
- return 0;
-}
-
-/**
- * message_show() - read message via sysfs
- * @dev: the LCD device
- * @attr: the LCD message attribute
- * @buf: the buffer to read the message into
- *
- * Read the current message being displayed or scrolled across the LCD display
- * into @buf, for reads from sysfs.
- *
- * Return: the number of characters written to @buf
- */
-static ssize_t message_show(struct device *dev, struct device_attribute *attr,
- char *buf)
-{
- struct img_ascii_lcd_ctx *ctx = dev_get_drvdata(dev);
-
- return sysfs_emit(buf, "%s\n", ctx->message);
-}
-
-/**
- * message_store() - write a new message via sysfs
- * @dev: the LCD device
- * @attr: the LCD message attribute
- * @buf: the buffer containing the new message
- * @count: the size of the message in @buf
- *
- * Write a new message to display or scroll across the LCD display from sysfs.
- *
- * Return: the size of the message on success, else -ERRNO
- */
-static ssize_t message_store(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct img_ascii_lcd_ctx *ctx = dev_get_drvdata(dev);
- int err;
-
- err = img_ascii_lcd_display(ctx, buf, count);
- return err ?: count;
-}
-
-static DEVICE_ATTR_RW(message);
-
/**
* img_ascii_lcd_probe() - probe an LCD display device
* @pdev: the LCD platform device
@@ -391,29 +253,23 @@ static int img_ascii_lcd_probe(struct platform_device *pdev)
return PTR_ERR(ctx->base);
}
- ctx->pdev = pdev;
- ctx->cfg = cfg;
- ctx->message = NULL;
- ctx->scroll_pos = 0;
- ctx->scroll_rate = HZ / 2;
-
- /* initialise a timer for scrolling the message */
- timer_setup(&ctx->timer, img_ascii_lcd_scroll, 0);
-
- platform_set_drvdata(pdev, ctx);
-
- /* display a default message */
- err = img_ascii_lcd_display(ctx, "Linux " UTS_RELEASE " ", -1);
+ err = linedisp_register(&ctx->linedisp, dev, cfg->num_chars, ctx->curr,
+ cfg->update);
if (err)
- goto out_del_timer;
+ return err;
- err = device_create_file(dev, &dev_attr_message);
+ /* for backwards compatibility */
+ err = compat_only_sysfs_link_entry_to_kobj(&dev->kobj,
+ &ctx->linedisp.dev.kobj,
+ "message", NULL);
if (err)
- goto out_del_timer;
+ goto err_unregister;
+ platform_set_drvdata(pdev, ctx);
return 0;
-out_del_timer:
- del_timer_sync(&ctx->timer);
+
+err_unregister:
+ linedisp_unregister(&ctx->linedisp);
return err;
}
@@ -430,8 +286,8 @@ static int img_ascii_lcd_remove(struct platform_device *pdev)
{
struct img_ascii_lcd_ctx *ctx = platform_get_drvdata(pdev);
- device_remove_file(&pdev->dev, &dev_attr_message);
- del_timer_sync(&ctx->timer);
+ sysfs_remove_link(&pdev->dev.kobj, "message");
+ linedisp_unregister(&ctx->linedisp);
return 0;
}
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
new file mode 100644
index 0000000000000000..4b97c20ac0b381ee
--- /dev/null
+++ b/drivers/auxdisplay/line-display.c
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Character line display core support
+ *
+ * Copyright (C) 2016 Imagination Technologies
+ * Author: Paul Burton <[email protected]>
+ *
+ * Copyright (C) 2021 Glider bv
+ */
+
+#include <generated/utsrelease.h>
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/timer.h>
+
+#include "line-display.h"
+
+/**
+ * linedisp_scroll() - scroll the display by a character
+ * @t: really a pointer to the private data structure
+ *
+ * Scroll the current message along the display by one character, rearming the
+ * timer if required.
+ */
+static void linedisp_scroll(struct timer_list *t)
+{
+ struct linedisp *linedisp = from_timer(linedisp, t, timer);
+ unsigned int i, ch = linedisp->scroll_pos;
+ unsigned int num_chars = linedisp->num_chars;
+
+ /* update the current message string */
+ for (i = 0; i < num_chars;) {
+ /* copy as many characters from the string as possible */
+ for (; i < num_chars && ch < linedisp->message_len; i++, ch++)
+ linedisp->buf[i] = linedisp->message[ch];
+
+ /* wrap around to the start of the string */
+ ch = 0;
+ }
+
+ /* update the display */
+ linedisp->update(linedisp);
+
+ /* move on to the next character */
+ linedisp->scroll_pos++;
+ linedisp->scroll_pos %= linedisp->message_len;
+
+ /* rearm the timer */
+ if (linedisp->message_len > num_chars)
+ mod_timer(&linedisp->timer, jiffies + linedisp->scroll_rate);
+}
+
+/**
+ * linedisp_display() - set the message to be displayed
+ * @linedisp: pointer to the private data structure
+ * @msg: the message to display
+ * @count: length of msg, or -1
+ *
+ * Display a new message @msg on the display. @msg can be longer than the
+ * number of characters the display can display, in which case it will begin
+ * scrolling across the display.
+ *
+ * Return: 0 on success, -ENOMEM on memory allocation failure
+ */
+static int linedisp_display(struct linedisp *linedisp, const char *msg,
+ ssize_t count)
+{
+ char *new_msg;
+
+ /* stop the scroll timer */
+ del_timer_sync(&linedisp->timer);
+
+ if (count == -1)
+ count = strlen(msg);
+
+ /* if the string ends with a newline, trim it */
+ if (msg[count - 1] == '\n')
+ count--;
+
+ if (!count) {
+ /* Clear the display */
+ kfree(linedisp->message);
+ linedisp->message = NULL;
+ linedisp->message_len = 0;
+ memset(linedisp->buf, ' ', linedisp->num_chars);
+ linedisp->update(linedisp);
+ return 0;
+ }
+
+ new_msg = kmalloc(count + 1, GFP_KERNEL);
+ if (!new_msg)
+ return -ENOMEM;
+
+ memcpy(new_msg, msg, count);
+ new_msg[count] = 0;
+
+ kfree(linedisp->message);
+
+ linedisp->message = new_msg;
+ linedisp->message_len = count;
+ linedisp->scroll_pos = 0;
+
+ /* update the display */
+ linedisp_scroll(&linedisp->timer);
+
+ return 0;
+}
+
+/**
+ * message_show() - read message via sysfs
+ * @dev: the display device
+ * @attr: the display message attribute
+ * @buf: the buffer to read the message into
+ *
+ * Read the current message being displayed or scrolled across the display into
+ * @buf, for reads from sysfs.
+ *
+ * Return: the number of characters written to @buf
+ */
+static ssize_t message_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+
+ return sysfs_emit(buf, "%s\n", linedisp->message);
+}
+
+/**
+ * message_store() - write a new message via sysfs
+ * @dev: the display device
+ * @attr: the display message attribute
+ * @buf: the buffer containing the new message
+ * @count: the size of the message in @buf
+ *
+ * Write a new message to display or scroll across the display from sysfs.
+ *
+ * Return: the size of the message on success, else -ERRNO
+ */
+static ssize_t message_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+ int err;
+
+ err = linedisp_display(linedisp, buf, count);
+ return err ?: count;
+}
+
+static DEVICE_ATTR_RW(message);
+
+static struct attribute *linedisp_attrs[] = {
+ &dev_attr_message.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(linedisp);
+
+static const struct device_type linedisp_type = {
+ .groups = linedisp_groups,
+};
+
+/**
+ * linedisp_register - register a character line display
+ * @linedisp: pointer to character line display structure
+ * @parent: parent device
+ * @num_chars: the number of characters that can be displayed
+ * @buf: pointer to a buffer that can hold @num_chars characters
+ * @update: Function called to update the display. This must not sleep!
+ *
+ * Return: zero on success, else a negative error code.
+ */
+int linedisp_register(struct linedisp *linedisp, struct device *parent,
+ unsigned int num_chars, char *buf,
+ void (*update)(struct linedisp *linedisp))
+{
+ static atomic_t linedisp_id = ATOMIC_INIT(-1);
+ int err;
+
+ memset(linedisp, 0, sizeof(*linedisp));
+ linedisp->dev.parent = parent;
+ linedisp->dev.type = &linedisp_type;
+ linedisp->update = update;
+ linedisp->buf = buf;
+ linedisp->num_chars = num_chars;
+ linedisp->scroll_rate = HZ / 2;
+
+ device_initialize(&linedisp->dev);
+ dev_set_name(&linedisp->dev, "linedisp.%lu",
+ (unsigned long)atomic_inc_return(&linedisp_id));
+
+ /* initialise a timer for scrolling the message */
+ timer_setup(&linedisp->timer, linedisp_scroll, 0);
+
+ err = device_add(&linedisp->dev);
+ if (err)
+ goto out_del_timer;
+
+ /* display a default message */
+ err = linedisp_display(linedisp, "Linux " UTS_RELEASE " ", -1);
+ if (err)
+ goto out_del_dev;
+
+ return 0;
+
+out_del_dev:
+ device_del(&linedisp->dev);
+out_del_timer:
+ del_timer_sync(&linedisp->timer);
+ put_device(&linedisp->dev);
+ return err;
+}
+EXPORT_SYMBOL_GPL(linedisp_register);
+
+/**
+ * linedisp_unregister - unregister a character line display
+ * @linedisp: pointer to character line display structure registered previously
+ * with linedisp_register()
+ */
+void linedisp_unregister(struct linedisp *linedisp)
+{
+ device_del(&linedisp->dev);
+ del_timer_sync(&linedisp->timer);
+ kfree(linedisp->message);
+ put_device(&linedisp->dev);
+}
+EXPORT_SYMBOL_GPL(linedisp_unregister);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/auxdisplay/line-display.h b/drivers/auxdisplay/line-display.h
new file mode 100644
index 0000000000000000..0f5891d34c485871
--- /dev/null
+++ b/drivers/auxdisplay/line-display.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Character line display core support
+ *
+ * Copyright (C) 2016 Imagination Technologies
+ * Author: Paul Burton <[email protected]>
+ *
+ * Copyright (C) 2021 Glider bv
+ */
+
+#ifndef _LINEDISP_H
+#define _LINEDISP_H
+
+/**
+ * struct linedisp - character line display private data structure
+ * @dev: the line display device
+ * @timer: timer used to implement scrolling
+ * @update: function called to update the display
+ * @buf: pointer to the buffer for the string currently displayed
+ * @message: the full message to display or scroll on the display
+ * @num_chars: the number of characters that can be displayed
+ * @message_len: the length of the @message string
+ * @scroll_pos: index of the first character of @message currently displayed
+ * @scroll_rate: scroll interval in jiffies
+ */
+struct linedisp {
+ struct device dev;
+ struct timer_list timer;
+ void (*update)(struct linedisp *linedisp);
+ char *buf;
+ char *message;
+ unsigned int num_chars;
+ unsigned int message_len;
+ unsigned int scroll_pos;
+ unsigned int scroll_rate;
+};
+
+int linedisp_register(struct linedisp *linedisp, struct device *parent,
+ unsigned int num_chars, char *buf,
+ void (*update)(struct linedisp *linedisp));
+void linedisp_unregister(struct linedisp *linedisp);
+
+#endif /* LINEDISP_H */
--
2.25.1
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 <[email protected]>
---
The 7-segment support is based on schematics, and untested on actual
hardware.
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- Pass "dev" instead of "client" to ht16k33_seg_probe(),
- Remove unneeded inclusion of <linux/leds.h> and <linux/of_device.h>,
v2:
- Move "select LINEDISP" to this patch,
- Set "err = -EINVAL" in switch() case that cannot happen.
---
drivers/auxdisplay/Kconfig | 1 +
drivers/auxdisplay/ht16k33.c | 196 +++++++++++++++++++++++++++++++++--
2 files changed, 190 insertions(+), 7 deletions(-)
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index b41fdc5f7bf72a1b..42fc7b155de09dbc 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -176,6 +176,7 @@ config HT16K33
select FB_SYS_IMAGEBLIT
select INPUT_MATRIXKMAP
select FB_BACKLIGHT
+ select LINEDISP
help
Say yes here to add support for Holtek HT16K33, RAM mapping 16*8
LED controller driver with keyscan.
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 27ac167dae74bd82..3b555e119e326cec 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -5,6 +5,7 @@
* Author: Robin van der Gracht <[email protected]>
*
* Copyright: (C) 2016 Protonic Holland.
+ * Copyright (C) 2021 Glider bv
*/
#include <linux/kernel.h>
@@ -20,6 +21,13 @@
#include <linux/workqueue.h>
#include <linux/mm.h>
+#include <linux/map_to_7segment.h>
+#include <linux/map_to_14segment.h>
+
+#include <asm/unaligned.h>
+
+#include "line-display.h"
+
/* Registers */
#define REG_SYSTEM_SETUP 0x20
#define REG_SYSTEM_SETUP_OSC_ON BIT(0)
@@ -47,6 +55,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 +81,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 +129,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, &priv->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(&priv->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 +380,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(&priv->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 = &priv->seg;
+ char *s = seg->curr;
+ uint8_t buf[9];
+
+ buf[0] = map_to_seg7(&seg->map.seg7, *s++);
+ buf[1] = 0;
+ buf[2] = map_to_seg7(&seg->map.seg7, *s++);
+ buf[3] = 0;
+ buf[4] = 0;
+ buf[5] = 0;
+ buf[6] = map_to_seg7(&seg->map.seg7, *s++);
+ buf[7] = 0;
+ buf[8] = map_to_seg7(&seg->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 = &priv->seg;
+ char *s = seg->curr;
+ uint8_t buf[8];
+
+ put_unaligned_le16(map_to_seg14(&seg->map.seg14, *s++), buf);
+ put_unaligned_le16(map_to_seg14(&seg->map.seg14, *s++), buf + 2);
+ put_unaligned_le16(map_to_seg14(&seg->map.seg14, *s++), buf + 4);
+ put_unaligned_le16(map_to_seg14(&seg->map.seg14, *s++), buf + 6);
+
+ i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
+}
+
static int ht16k33_keypad_probe(struct i2c_client *client,
struct ht16k33_keypad *keypad)
{
@@ -469,9 +569,56 @@ static int ht16k33_fbdev_probe(struct device *dev, struct ht16k33_priv *priv,
return err;
}
+static int ht16k33_seg_probe(struct device *dev, struct ht16k33_priv *priv,
+ uint32_t brightness)
+{
+ struct ht16k33_seg *seg = &priv->seg;
+ int err;
+
+ err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
+ if (err)
+ return err;
+
+ switch (priv->type) {
+ case DISP_MATRIX:
+ /* not handled here */
+ err = -EINVAL;
+ break;
+
+ case DISP_QUAD_7SEG:
+ INIT_DELAYED_WORK(&priv->work, ht16k33_seg7_update);
+ seg->map.seg7 = initial_map_seg7;
+ seg->map_size = sizeof(seg->map.seg7);
+ err = device_create_file(dev, &dev_attr_map_seg7);
+ break;
+
+ case DISP_QUAD_14SEG:
+ INIT_DELAYED_WORK(&priv->work, ht16k33_seg14_update);
+ seg->map.seg14 = initial_map_seg14;
+ seg->map_size = sizeof(seg->map.seg14);
+ err = device_create_file(dev, &dev_attr_map_seg14);
+ break;
+ }
+ if (err)
+ return err;
+
+ err = linedisp_register(&seg->linedisp, dev, 4, seg->curr,
+ ht16k33_linedisp_update);
+ if (err)
+ goto err_remove_map_file;
+
+ return 0;
+
+err_remove_map_file:
+ device_remove_file(dev, &dev_attr_map_seg7);
+ device_remove_file(dev, &dev_attr_map_seg14);
+ return err;
+}
+
static int ht16k33_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
+ const struct of_device_id *id;
struct ht16k33_priv *priv;
uint32_t dft_brightness;
int err;
@@ -486,6 +633,9 @@ static int ht16k33_probe(struct i2c_client *client)
return -ENOMEM;
priv->client = client;
+ id = i2c_of_match_device(dev->driver->of_match_table, client);
+ if (id)
+ priv->type = (uintptr_t)id->data;
i2c_set_clientdata(client, priv);
err = ht16k33_initialize(priv);
@@ -510,8 +660,19 @@ static int ht16k33_probe(struct i2c_client *client)
return err;
}
- /* Frame Buffer Display */
- return ht16k33_fbdev_probe(dev, priv, dft_brightness);
+ switch (priv->type) {
+ case DISP_MATRIX:
+ /* Frame Buffer Display */
+ err = ht16k33_fbdev_probe(dev, priv, dft_brightness);
+ break;
+
+ case DISP_QUAD_7SEG:
+ case DISP_QUAD_14SEG:
+ /* Segment Display */
+ err = ht16k33_seg_probe(dev, priv, dft_brightness);
+ break;
+ }
+ return err;
}
static int ht16k33_remove(struct i2c_client *client)
@@ -520,9 +681,21 @@ static int ht16k33_remove(struct i2c_client *client)
struct ht16k33_fbdev *fbdev = &priv->fbdev;
cancel_delayed_work_sync(&priv->work);
- unregister_framebuffer(fbdev->info);
- framebuffer_release(fbdev->info);
- free_page((unsigned long) fbdev->buffer);
+
+ switch (priv->type) {
+ case DISP_MATRIX:
+ unregister_framebuffer(fbdev->info);
+ framebuffer_release(fbdev->info);
+ free_page((unsigned long)fbdev->buffer);
+ break;
+
+ case DISP_QUAD_7SEG:
+ case DISP_QUAD_14SEG:
+ linedisp_unregister(&priv->seg.linedisp);
+ device_remove_file(&client->dev, &dev_attr_map_seg7);
+ device_remove_file(&client->dev, &dev_attr_map_seg14);
+ break;
+ }
return 0;
}
@@ -534,7 +707,16 @@ static const struct i2c_device_id ht16k33_i2c_match[] = {
MODULE_DEVICE_TABLE(i2c, ht16k33_i2c_match);
static const struct of_device_id ht16k33_of_match[] = {
- { .compatible = "holtek,ht16k33", },
+ {
+ /* 0.56" 4-Digit 7-Segment FeatherWing Display (Red) */
+ .compatible = "adafruit,3108", .data = (void *)DISP_QUAD_7SEG,
+ }, {
+ /* 0.54" Quad Alphanumeric FeatherWing Display (Red) */
+ .compatible = "adafruit,3130", .data = (void *)DISP_QUAD_14SEG,
+ }, {
+ /* Generic, assumed Dot-Matrix Display */
+ .compatible = "holtek,ht16k33", .data = (void *)DISP_MATRIX,
+ },
{ }
};
MODULE_DEVICE_TABLE(of, ht16k33_of_match);
--
2.25.1
Use the existing HT16K33_FB_SIZE definition instead of open-coding the
same calculation using an hardcoded value.
While at it, restore reverse Christmas tree variable declaration order.
Signed-off-by: Geert Uytterhoeven <[email protected]>
Acked-by: Robin van der Gracht <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- No changes,
v2:
- Add Acked-by.
---
drivers/auxdisplay/ht16k33.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 2b630e194570f6e5..99daf1974980b435 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -168,9 +168,9 @@ static void ht16k33_fb_update(struct work_struct *work)
static int ht16k33_initialize(struct ht16k33_priv *priv)
{
+ uint8_t data[HT16K33_FB_SIZE];
uint8_t byte;
int err;
- uint8_t data[HT16K33_MATRIX_LED_MAX_COLS * 2];
/* Clear RAM (8 * 16 bits) */
memset(data, 0, sizeof(data));
--
2.25.1
Convert the "message" device attribute from sprintf() to sysfs_emit(),
as the latter is aware of the PAGE_SIZE buffer.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- No changes,
v2:
- No changes.
---
drivers/auxdisplay/img-ascii-lcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/auxdisplay/img-ascii-lcd.c b/drivers/auxdisplay/img-ascii-lcd.c
index 2b6e41ec57544faa..2b5640b638900a90 100644
--- a/drivers/auxdisplay/img-ascii-lcd.c
+++ b/drivers/auxdisplay/img-ascii-lcd.c
@@ -326,7 +326,7 @@ static ssize_t message_show(struct device *dev, struct device_attribute *attr,
{
struct img_ascii_lcd_ctx *ctx = dev_get_drvdata(dev);
- return sprintf(buf, "%s\n", ctx->message);
+ return sysfs_emit(buf, "%s\n", ctx->message);
}
/**
--
2.25.1
Currently /sys/class/graphics/fb0/bl_curve is not accessible (-ENODEV),
as the driver does not connect the backlight to the frame buffer device.
Fix this moving backlight initialization up, and filling in
fb_info.bl_dev.
Fixes: 8992da44c6805d53 ("auxdisplay: ht16k33: Driver for LED controller")
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- No changes,
v2:
- New.
---
drivers/auxdisplay/ht16k33.c | 56 ++++++++++++++++++------------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 1e69cc6d21a0dca2..2b630e194570f6e5 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -413,6 +413,33 @@ static int ht16k33_probe(struct i2c_client *client,
if (err)
return err;
+ /* Backlight */
+ memset(&bl_props, 0, sizeof(struct backlight_properties));
+ bl_props.type = BACKLIGHT_RAW;
+ bl_props.max_brightness = MAX_BRIGHTNESS;
+
+ bl = devm_backlight_device_register(&client->dev, DRIVER_NAME"-bl",
+ &client->dev, priv,
+ &ht16k33_bl_ops, &bl_props);
+ if (IS_ERR(bl)) {
+ dev_err(&client->dev, "failed to register backlight\n");
+ return PTR_ERR(bl);
+ }
+
+ err = of_property_read_u32(node, "default-brightness-level",
+ &dft_brightness);
+ if (err) {
+ dft_brightness = MAX_BRIGHTNESS;
+ } else if (dft_brightness > MAX_BRIGHTNESS) {
+ dev_warn(&client->dev,
+ "invalid default brightness level: %u, using %u\n",
+ dft_brightness, MAX_BRIGHTNESS);
+ dft_brightness = MAX_BRIGHTNESS;
+ }
+
+ bl->props.brightness = dft_brightness;
+ ht16k33_bl_update_status(bl);
+
/* Framebuffer (2 bytes per column) */
BUILD_BUG_ON(PAGE_SIZE < HT16K33_FB_SIZE);
fbdev->buffer = (unsigned char *) get_zeroed_page(GFP_KERNEL);
@@ -445,6 +472,7 @@ static int ht16k33_probe(struct i2c_client *client,
fbdev->info->screen_size = HT16K33_FB_SIZE;
fbdev->info->fix = ht16k33_fb_fix;
fbdev->info->var = ht16k33_fb_var;
+ fbdev->info->bl_dev = bl;
fbdev->info->pseudo_palette = NULL;
fbdev->info->flags = FBINFO_FLAG_DEFAULT;
fbdev->info->par = priv;
@@ -460,34 +488,6 @@ static int ht16k33_probe(struct i2c_client *client,
goto err_fbdev_unregister;
}
- /* Backlight */
- memset(&bl_props, 0, sizeof(struct backlight_properties));
- bl_props.type = BACKLIGHT_RAW;
- bl_props.max_brightness = MAX_BRIGHTNESS;
-
- bl = devm_backlight_device_register(&client->dev, DRIVER_NAME"-bl",
- &client->dev, priv,
- &ht16k33_bl_ops, &bl_props);
- if (IS_ERR(bl)) {
- dev_err(&client->dev, "failed to register backlight\n");
- err = PTR_ERR(bl);
- goto err_fbdev_unregister;
- }
-
- err = of_property_read_u32(node, "default-brightness-level",
- &dft_brightness);
- if (err) {
- dft_brightness = MAX_BRIGHTNESS;
- } else if (dft_brightness > MAX_BRIGHTNESS) {
- dev_warn(&client->dev,
- "invalid default brightness level: %u, using %u\n",
- dft_brightness, MAX_BRIGHTNESS);
- dft_brightness = MAX_BRIGHTNESS;
- }
-
- bl->props.brightness = dft_brightness;
- ht16k33_bl_update_status(bl);
-
ht16k33_fb_queue(priv);
return 0;
--
2.25.1
Extract brightness handling into a helper function, so it can be called
from multiple places.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- Use "err" instead of "error" to be consistent with existing driver
naming style,
v2:
- No changes.
---
drivers/auxdisplay/ht16k33.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index c7a3a0e1fbb5d03e..928ac9722c142855 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -113,6 +113,22 @@ static int ht16k33_display_off(struct ht16k33_priv *priv)
return i2c_smbus_write_byte(priv->client, REG_DISPLAY_SETUP);
}
+static int ht16k33_brightness_set(struct ht16k33_priv *priv,
+ unsigned int brightness)
+{
+ int err;
+
+ if (brightness == 0)
+ return ht16k33_display_off(priv);
+
+ err = ht16k33_display_on(priv);
+ if (err)
+ return err;
+
+ return i2c_smbus_write_byte(priv->client,
+ REG_BRIGHTNESS | (brightness - 1));
+}
+
static void ht16k33_fb_queue(struct ht16k33_priv *priv)
{
struct ht16k33_fbdev *fbdev = &priv->fbdev;
@@ -197,13 +213,10 @@ static int ht16k33_bl_update_status(struct backlight_device *bl)
if (bl->props.power != FB_BLANK_UNBLANK ||
bl->props.fb_blank != FB_BLANK_UNBLANK ||
- bl->props.state & BL_CORE_FBBLANK || brightness == 0) {
- return ht16k33_display_off(priv);
- }
+ bl->props.state & BL_CORE_FBBLANK)
+ brightness = 0;
- ht16k33_display_on(priv);
- return i2c_smbus_write_byte(priv->client,
- REG_BRIGHTNESS | (brightness - 1));
+ return ht16k33_brightness_set(priv, brightness);
}
static int ht16k33_bl_check_fb(struct backlight_device *bl, struct fb_info *fi)
--
2.25.1
Move delayed_work from ht16k33_fbdev to ht16k33_priv, as it is not
specific to dot-matrix displays, but common to all display types.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- No changes,
v2:
- No changes.
---
drivers/auxdisplay/ht16k33.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 75d326a823543898..c7a3a0e1fbb5d03e 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -65,11 +65,11 @@ struct ht16k33_fbdev {
uint32_t refresh_rate;
uint8_t *buffer;
uint8_t *cache;
- struct delayed_work work;
};
struct ht16k33_priv {
struct i2c_client *client;
+ struct delayed_work work;
struct ht16k33_keypad keypad;
struct ht16k33_fbdev fbdev;
};
@@ -117,7 +117,7 @@ static void ht16k33_fb_queue(struct ht16k33_priv *priv)
{
struct ht16k33_fbdev *fbdev = &priv->fbdev;
- schedule_delayed_work(&fbdev->work, HZ / fbdev->refresh_rate);
+ schedule_delayed_work(&priv->work, HZ / fbdev->refresh_rate);
}
/*
@@ -125,10 +125,9 @@ static void ht16k33_fb_queue(struct ht16k33_priv *priv)
*/
static void ht16k33_fb_update(struct work_struct *work)
{
- struct ht16k33_fbdev *fbdev =
- container_of(work, struct ht16k33_fbdev, work.work);
- struct ht16k33_priv *priv =
- container_of(fbdev, struct ht16k33_priv, fbdev);
+ struct ht16k33_priv *priv = container_of(work, struct ht16k33_priv,
+ work.work);
+ struct ht16k33_fbdev *fbdev = &priv->fbdev;
uint8_t *p1, *p2;
int len, pos = 0, first = -1;
@@ -462,7 +461,7 @@ static int ht16k33_probe(struct i2c_client *client)
}
fb_bl_default_curve(fbdev->info, 0, MIN_BRIGHTNESS, MAX_BRIGHTNESS);
- INIT_DELAYED_WORK(&fbdev->work, ht16k33_fb_update);
+ INIT_DELAYED_WORK(&priv->work, ht16k33_fb_update);
fbdev->info->fbops = &ht16k33_fb_ops;
fbdev->info->screen_base = (char __iomem *) fbdev->buffer;
fbdev->info->screen_size = HT16K33_FB_SIZE;
@@ -502,7 +501,7 @@ static int ht16k33_remove(struct i2c_client *client)
struct ht16k33_priv *priv = i2c_get_clientdata(client);
struct ht16k33_fbdev *fbdev = &priv->fbdev;
- cancel_delayed_work_sync(&fbdev->work);
+ cancel_delayed_work_sync(&priv->work);
unregister_framebuffer(fbdev->info);
framebuffer_release(fbdev->info);
free_page((unsigned long) fbdev->buffer);
--
2.25.1
Use the existing kmemdup_nul() helper instead of open-coding the same
operation.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- No changes,
v2:
- No changes.
---
drivers/auxdisplay/line-display.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 4b97c20ac0b381ee..3f35199bc39f511f 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -91,13 +91,10 @@ static int linedisp_display(struct linedisp *linedisp, const char *msg,
return 0;
}
- new_msg = kmalloc(count + 1, GFP_KERNEL);
+ new_msg = kmemdup_nul(msg, count, GFP_KERNEL);
if (!new_msg)
return -ENOMEM;
- memcpy(new_msg, msg, count);
- new_msg[count] = 0;
-
kfree(linedisp->message);
linedisp->message = new_msg;
--
2.25.1
The Holtek HT16K33 LED controller is not only used for driving
dot-matrix displays, but also for driving segment displays.
Document compatible values for the Adafruit 7-segment[1] and
14-segment[2] FeatherWing expansion boards with red displays. According
to the schematics, all other Adafruit 7-segment and 14-segment display
backpack and FeatherWing expansion boards (including bare boards and
boards fitted with displays) are compatible with these two boards.
[1] https://www.adafruit.com/product/3108
[2] https://www.adafruit.com/product/3130
Signed-off-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
v6:
- No changes,
v5:
- Add Reviewed-by,
v4:
- Combine compatible values for 7/14 segment displays into an enum,
v3:
- Drop color property,
v2:
- Fix type of color to uint32,
- "refresh-rate-hz" is still required for dot-matrix displays.
Alternatives I considered:
1. Document the attached display type in a child node.
I.e. specify segment type, number of characters, and wiring.
Especially the latter would become really complex, due to the sheer
amount of possible wiring combinations.
Using this method, you also loose the ability to just connect a
display to an i2c bus, and instantiate the device from sysfs,
without using DT:
echo adafruit,3130 0x70 > /sys/class/i2c/i2c-adapter/.../new_device
2. Document all Adafruit 7-segment and 14-segment display backpack and
FeatherWing expansion boards.
This would lead to a myriad of compatible values:
- items:
- enum:
- adafruit,878 # 0.56" 4-Digit 7-Segment Display Backpack (Red)
- adafruit,879 # 0.56" 4-Digit 7-Segment Display Backpack (Yellow)
- adafruit,880 # 0.56" 4-Digit 7-Segment Display Backpack (Green)
- adafruit,881 # 0.56" 4-Digit 7-Segment Display Backpack (Blue)
- adafruit,1002 # 0.56" 4-Digit 7-Segment Display Backpack (White)
- const: adafruit,877 # 0.56" 4-Digit 7-Segment Backpack
- const: holtek,ht16k33
- items:
- enum:
- adafruit,1268 # 1.2" 4-Digit 7-Segment Display Backpack (Green)
- adafruit,1269 # 1.2" 4-Digit 7-Segment Display Backpack (Yellow)
- adafruit,1270 # 1.2" 4-Digit 7-Segment Display Backpack (Red)
- const: adafruit,1271 # 1.2" 4-Digit 7-Segment Backpack
- const: holtek,ht16k33
- items:
- enum:
- adafruit,1911 # 0.54" Quad Alphanumeric Display Backpack (Red)
- adafruit,1912 # 0.54" Quad Alphanumeric Display Backpack (Blue)
- adafruit,2157 # 0.54" Quad Alphanumeric Display Backpack (White)
- adafruit,2158 # 0.54" Quad Alphanumeric Display Backpack (Yellow)
- adafruit,2159 # 0.54" Quad Alphanumeric Display Backpack (Yellow-Green)
- adafruit,2160 # 0.54" Quad Alphanumeric Display Backpack (Green)
- const: adafruit,1910 # 0.54" Quad 14-segment Alphanumeric Backpack
- const: holtek,ht16k33
- items:
- enum:
- adafruit,3106 # 0.56" 4-Digit 7-Segment FeatherWing Display (Blue)
- adafruit,3107 # 0.56" 4-Digit 7-Segment FeatherWing Display (Green)
- adafruit,3108 # 0.56" 4-Digit 7-Segment FeatherWing Display (Red)
- adafruit,3109 # 0.56" 4-Digit 7-Segment FeatherWing Display (White)
- adafruit,3110 # 0.56" 4-Digit 7-Segment FeatherWing Display (Yellow)
- const: adafruit,3088 # 0.56" 4-Digit 7-Segment FeatherWing
- const: holtek,ht16k33
- items:
- enum:
- adafruit,3127 # 0.54" Quad Alphanumeric FeatherWing Display (White)
- adafruit,3128 # 0.54" Quad Alphanumeric FeatherWing Display (Blue)
- adafruit,3129 # 0.54" Quad Alphanumeric FeatherWing Display (Green)
- adafruit,3130 # 0.54" Quad Alphanumeric FeatherWing Display (Red)
- adafruit,3131 # 0.54" Quad Alphanumeric FeatherWing Display (Yellow)
- adafruit,3132 # 0.54" Quad Alphanumeric FeatherWing Display (Yellow-Green)
- const: adafruit,3089 # 0.54" Quad 14-segment Alphanumeric FeatherWing
- const: holtek,ht16k33
---
.../bindings/auxdisplay/holtek,ht16k33.yaml | 20 ++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml b/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
index 64ffff460026040f..6baf1200c9d8488e 100644
--- a/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
+++ b/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
@@ -14,14 +14,21 @@ allOf:
properties:
compatible:
- const: holtek,ht16k33
+ oneOf:
+ - items:
+ - enum:
+ - adafruit,3108 # 0.56" 4-Digit 7-Segment FeatherWing Display (Red)
+ - adafruit,3130 # 0.54" Quad Alphanumeric FeatherWing Display (Red)
+ - const: holtek,ht16k33
+
+ - const: holtek,ht16k33 # Generic 16*8 LED controller with dot-matrix display
reg:
maxItems: 1
refresh-rate-hz:
maxItems: 1
- description: Display update interval in Hertz
+ description: Display update interval in Hertz for dot-matrix displays
interrupts:
maxItems: 1
@@ -44,7 +51,14 @@ properties:
required:
- compatible
- reg
- - refresh-rate-hz
+
+if:
+ properties:
+ compatible:
+ const: holtek,ht16k33
+then:
+ required:
+ - refresh-rate-hz
additionalProperties: false
--
2.25.1
This driver has many users of "client->dev". Add shorthands to simplify
the code.
Signed-off-by: Geert Uytterhoeven <[email protected]>
Acked-by: Robin van der Gracht <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- No changes,
v2:
- Add Acked-by,
- Rebased.
---
drivers/auxdisplay/ht16k33.c | 43 ++++++++++++++++++------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 8c1689b77db95676..75d326a823543898 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -316,7 +316,8 @@ static void ht16k33_keypad_stop(struct input_dev *dev)
static int ht16k33_keypad_probe(struct i2c_client *client,
struct ht16k33_keypad *keypad)
{
- struct device_node *node = client->dev.of_node;
+ struct device *dev = &client->dev;
+ struct device_node *node = dev->of_node;
u32 rows = HT16K33_MATRIX_KEYPAD_MAX_ROWS;
u32 cols = HT16K33_MATRIX_KEYPAD_MAX_COLS;
int err;
@@ -324,7 +325,7 @@ static int ht16k33_keypad_probe(struct i2c_client *client,
keypad->client = client;
init_waitqueue_head(&keypad->wait);
- keypad->dev = devm_input_allocate_device(&client->dev);
+ keypad->dev = devm_input_allocate_device(dev);
if (!keypad->dev)
return -ENOMEM;
@@ -341,17 +342,17 @@ static int ht16k33_keypad_probe(struct i2c_client *client,
err = of_property_read_u32(node, "debounce-delay-ms",
&keypad->debounce_ms);
if (err) {
- dev_err(&client->dev, "key debounce delay not specified\n");
+ dev_err(dev, "key debounce delay not specified\n");
return err;
}
- err = matrix_keypad_parse_of_params(&client->dev, &rows, &cols);
+ err = matrix_keypad_parse_of_params(dev, &rows, &cols);
if (err)
return err;
if (rows > HT16K33_MATRIX_KEYPAD_MAX_ROWS ||
cols > HT16K33_MATRIX_KEYPAD_MAX_COLS) {
- dev_err(&client->dev, "%u rows or %u cols out of range in DT\n",
- rows, cols);
+ dev_err(dev, "%u rows or %u cols out of range in DT\n", rows,
+ cols);
return -ERANGE;
}
@@ -362,17 +363,17 @@ static int ht16k33_keypad_probe(struct i2c_client *client,
err = matrix_keypad_build_keymap(NULL, NULL, rows, cols, NULL,
keypad->dev);
if (err) {
- dev_err(&client->dev, "failed to build keymap\n");
+ dev_err(dev, "failed to build keymap\n");
return err;
}
- err = devm_request_threaded_irq(&client->dev, client->irq,
- NULL, ht16k33_keypad_irq_thread,
+ err = devm_request_threaded_irq(dev, client->irq, NULL,
+ ht16k33_keypad_irq_thread,
IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
DRIVER_NAME, keypad);
if (err) {
- dev_err(&client->dev, "irq request failed %d, error %d\n",
- client->irq, err);
+ dev_err(dev, "irq request failed %d, error %d\n", client->irq,
+ err);
return err;
}
@@ -389,14 +390,15 @@ static int ht16k33_probe(struct i2c_client *client)
struct backlight_properties bl_props;
struct ht16k33_priv *priv;
struct ht16k33_fbdev *fbdev;
- struct device_node *node = client->dev.of_node;
+ struct device *dev = &client->dev;
+ struct device_node *node = dev->of_node;
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
- dev_err(&client->dev, "i2c_check_functionality error\n");
+ dev_err(dev, "i2c_check_functionality error\n");
return -EIO;
}
- priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -413,11 +415,10 @@ static int ht16k33_probe(struct i2c_client *client)
bl_props.type = BACKLIGHT_RAW;
bl_props.max_brightness = MAX_BRIGHTNESS;
- bl = devm_backlight_device_register(&client->dev, DRIVER_NAME"-bl",
- &client->dev, priv,
+ bl = devm_backlight_device_register(dev, DRIVER_NAME"-bl", dev, priv,
&ht16k33_bl_ops, &bl_props);
if (IS_ERR(bl)) {
- dev_err(&client->dev, "failed to register backlight\n");
+ dev_err(dev, "failed to register backlight\n");
return PTR_ERR(bl);
}
@@ -426,7 +427,7 @@ static int ht16k33_probe(struct i2c_client *client)
if (err) {
dft_brightness = MAX_BRIGHTNESS;
} else if (dft_brightness > MAX_BRIGHTNESS) {
- dev_warn(&client->dev,
+ dev_warn(dev,
"invalid default brightness level: %u, using %u\n",
dft_brightness, MAX_BRIGHTNESS);
dft_brightness = MAX_BRIGHTNESS;
@@ -441,13 +442,13 @@ static int ht16k33_probe(struct i2c_client *client)
if (!fbdev->buffer)
return -ENOMEM;
- fbdev->cache = devm_kmalloc(&client->dev, HT16K33_FB_SIZE, GFP_KERNEL);
+ fbdev->cache = devm_kmalloc(dev, HT16K33_FB_SIZE, GFP_KERNEL);
if (!fbdev->cache) {
err = -ENOMEM;
goto err_fbdev_buffer;
}
- fbdev->info = framebuffer_alloc(0, &client->dev);
+ fbdev->info = framebuffer_alloc(0, dev);
if (!fbdev->info) {
err = -ENOMEM;
goto err_fbdev_buffer;
@@ -456,7 +457,7 @@ static int ht16k33_probe(struct i2c_client *client)
err = of_property_read_u32(node, "refresh-rate-hz",
&fbdev->refresh_rate);
if (err) {
- dev_err(&client->dev, "refresh rate not specified\n");
+ dev_err(dev, "refresh rate not specified\n");
goto err_fbdev_info;
}
fb_bl_default_curve(fbdev->info, 0, MIN_BRIGHTNESS, MAX_BRIGHTNESS);
--
2.25.1
Extract all frame buffer (including backlight) probing into
ht16k33_fbdev_probe().
Call ht16k33_fbdev_probe() after ht16k33_keypad_probe(), as the latter
does not need any manual cleanup in the probe error path.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- Pass "dev" instead of "client" to ht16k33_fbdev_probe(),
- Drop local variable "node",
v2:
- Rebased.
---
drivers/auxdisplay/ht16k33.c | 101 ++++++++++++++++++-----------------
1 file changed, 53 insertions(+), 48 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 928ac9722c142855..27ac167dae74bd82 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -394,33 +394,13 @@ static int ht16k33_keypad_probe(struct i2c_client *client,
return input_register_device(keypad->dev);
}
-static int ht16k33_probe(struct i2c_client *client)
+static int ht16k33_fbdev_probe(struct device *dev, struct ht16k33_priv *priv,
+ uint32_t brightness)
{
- int err;
- uint32_t dft_brightness;
- struct backlight_device *bl;
+ struct ht16k33_fbdev *fbdev = &priv->fbdev;
struct backlight_properties bl_props;
- struct ht16k33_priv *priv;
- struct ht16k33_fbdev *fbdev;
- struct device *dev = &client->dev;
- struct device_node *node = dev->of_node;
-
- if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
- dev_err(dev, "i2c_check_functionality error\n");
- return -EIO;
- }
-
- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
- priv->client = client;
- i2c_set_clientdata(client, priv);
- fbdev = &priv->fbdev;
-
- err = ht16k33_initialize(priv);
- if (err)
- return err;
+ struct backlight_device *bl;
+ int err;
/* Backlight */
memset(&bl_props, 0, sizeof(struct backlight_properties));
@@ -434,18 +414,7 @@ static int ht16k33_probe(struct i2c_client *client)
return PTR_ERR(bl);
}
- err = of_property_read_u32(node, "default-brightness-level",
- &dft_brightness);
- if (err) {
- dft_brightness = MAX_BRIGHTNESS;
- } else if (dft_brightness > MAX_BRIGHTNESS) {
- dev_warn(dev,
- "invalid default brightness level: %u, using %u\n",
- dft_brightness, MAX_BRIGHTNESS);
- dft_brightness = MAX_BRIGHTNESS;
- }
-
- bl->props.brightness = dft_brightness;
+ bl->props.brightness = brightness;
ht16k33_bl_update_status(bl);
/* Framebuffer (2 bytes per column) */
@@ -466,8 +435,8 @@ static int ht16k33_probe(struct i2c_client *client)
goto err_fbdev_buffer;
}
- err = of_property_read_u32(node, "refresh-rate-hz",
- &fbdev->refresh_rate);
+ err = of_property_read_u32(dev->of_node, "refresh-rate-hz",
+ &fbdev->refresh_rate);
if (err) {
dev_err(dev, "refresh rate not specified\n");
goto err_fbdev_info;
@@ -489,18 +458,9 @@ static int ht16k33_probe(struct i2c_client *client)
if (err)
goto err_fbdev_info;
- /* Keypad */
- if (client->irq > 0) {
- err = ht16k33_keypad_probe(client, &priv->keypad);
- if (err)
- goto err_fbdev_unregister;
- }
-
ht16k33_fb_queue(priv);
return 0;
-err_fbdev_unregister:
- unregister_framebuffer(fbdev->info);
err_fbdev_info:
framebuffer_release(fbdev->info);
err_fbdev_buffer:
@@ -509,6 +469,51 @@ static int ht16k33_probe(struct i2c_client *client)
return err;
}
+static int ht16k33_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct ht16k33_priv *priv;
+ uint32_t dft_brightness;
+ int err;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ dev_err(dev, "i2c_check_functionality error\n");
+ return -EIO;
+ }
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->client = client;
+ i2c_set_clientdata(client, priv);
+
+ err = ht16k33_initialize(priv);
+ if (err)
+ return err;
+
+ err = of_property_read_u32(dev->of_node, "default-brightness-level",
+ &dft_brightness);
+ if (err) {
+ dft_brightness = MAX_BRIGHTNESS;
+ } else if (dft_brightness > MAX_BRIGHTNESS) {
+ dev_warn(dev,
+ "invalid default brightness level: %u, using %u\n",
+ dft_brightness, MAX_BRIGHTNESS);
+ dft_brightness = MAX_BRIGHTNESS;
+ }
+
+ /* Keypad */
+ if (client->irq > 0) {
+ err = ht16k33_keypad_probe(client, &priv->keypad);
+ if (err)
+ return err;
+ }
+
+ /* Frame Buffer Display */
+ return ht16k33_fbdev_probe(dev, priv, dft_brightness);
+}
+
static int ht16k33_remove(struct i2c_client *client)
{
struct ht16k33_priv *priv = i2c_get_clientdata(client);
--
2.25.1
Instantiate a single LED based on the "led" subnode in DT.
This allows the user to control display brightness and blinking (backed
by hardware support) through the LED class API and triggers, and exposes
the display color. The LED will be named
"auxdisplay:<color>:<function>".
When running in dot-matrix mode and if no "led" subnode is found, the
driver falls back to the traditional backlight mode, to preserve
backwards compatibility.
Signed-off-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Marek Behún <[email protected]>
---
v6:
- Add Reviewed-by,
- Reorder operations in ht16k33_led_probe() to ease future conversion
to device properties,
v5:
- Add missing select NEW_LEDS,
v4:
- Add missing select LEDS_CLASS,
v3:
- Remove unneeded C++ comment,
- Use "err" instead of "error" to be consistent with existing driver
naming style,
- Make the creation of the LED device dependent on the presence of the
"led" subnode in DT, so it can be used in dot-matrix mode too.
- Use led_init_data() and devm_led_classdev_register_ext() to retrieve
all LED properties from DT, instead of manual LED name construction
based on just the "color" property,
v2:
- Use "auxdisplay" instead of DRIVER_NAME in LED name.
---
drivers/auxdisplay/Kconfig | 2 +
drivers/auxdisplay/ht16k33.c | 124 ++++++++++++++++++++++++++++++-----
2 files changed, 109 insertions(+), 17 deletions(-)
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 42fc7b155de09dbc..e32ef7f9945d49b2 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -176,6 +176,8 @@ config HT16K33
select FB_SYS_IMAGEBLIT
select INPUT_MATRIXKMAP
select FB_BACKLIGHT
+ select NEW_LEDS
+ select LEDS_CLASS
select LINEDISP
help
Say yes here to add support for Holtek HT16K33, RAM mapping 16*8
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 3b555e119e326cec..89ee5b4b3dfccb68 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -18,6 +18,7 @@
#include <linux/backlight.h>
#include <linux/input.h>
#include <linux/input/matrix_keypad.h>
+#include <linux/leds.h>
#include <linux/workqueue.h>
#include <linux/mm.h>
@@ -34,6 +35,10 @@
#define REG_DISPLAY_SETUP 0x80
#define REG_DISPLAY_SETUP_ON BIT(0)
+#define REG_DISPLAY_SETUP_BLINK_OFF (0 << 1)
+#define REG_DISPLAY_SETUP_BLINK_2HZ (1 << 1)
+#define REG_DISPLAY_SETUP_BLINK_1HZ (2 << 1)
+#define REG_DISPLAY_SETUP_BLINK_0HZ5 (3 << 1)
#define REG_ROWINT_SET 0xA0
#define REG_ROWINT_SET_INT_EN BIT(0)
@@ -94,12 +99,14 @@ struct ht16k33_seg {
struct ht16k33_priv {
struct i2c_client *client;
struct delayed_work work;
+ struct led_classdev led;
struct ht16k33_keypad keypad;
union {
struct ht16k33_fbdev fbdev;
struct ht16k33_seg seg;
};
enum display_type type;
+ uint8_t blink;
};
static const struct fb_fix_screeninfo ht16k33_fb_fix = {
@@ -158,7 +165,7 @@ 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;
+ uint8_t data = REG_DISPLAY_SETUP | REG_DISPLAY_SETUP_ON | priv->blink;
return i2c_smbus_write_byte(priv->client, data);
}
@@ -173,8 +180,10 @@ static int ht16k33_brightness_set(struct ht16k33_priv *priv,
{
int err;
- if (brightness == 0)
+ if (brightness == 0) {
+ priv->blink = REG_DISPLAY_SETUP_BLINK_OFF;
return ht16k33_display_off(priv);
+ }
err = ht16k33_display_on(priv);
if (err)
@@ -184,6 +193,49 @@ static int ht16k33_brightness_set(struct ht16k33_priv *priv,
REG_BRIGHTNESS | (brightness - 1));
}
+static int ht16k33_brightness_set_blocking(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct ht16k33_priv *priv = container_of(led_cdev, struct ht16k33_priv,
+ led);
+
+ return ht16k33_brightness_set(priv, brightness);
+}
+
+static int ht16k33_blink_set(struct led_classdev *led_cdev,
+ unsigned long *delay_on, unsigned long *delay_off)
+{
+ struct ht16k33_priv *priv = container_of(led_cdev, struct ht16k33_priv,
+ led);
+ unsigned int delay;
+ uint8_t blink;
+ int err;
+
+ if (!*delay_on && !*delay_off) {
+ blink = REG_DISPLAY_SETUP_BLINK_1HZ;
+ delay = 1000;
+ } else if (*delay_on <= 750) {
+ blink = REG_DISPLAY_SETUP_BLINK_2HZ;
+ delay = 500;
+ } else if (*delay_on <= 1500) {
+ blink = REG_DISPLAY_SETUP_BLINK_1HZ;
+ delay = 1000;
+ } else {
+ blink = REG_DISPLAY_SETUP_BLINK_0HZ5;
+ delay = 2000;
+ }
+
+ err = i2c_smbus_write_byte(priv->client,
+ REG_DISPLAY_SETUP | REG_DISPLAY_SETUP_ON |
+ blink);
+ if (err)
+ return err;
+
+ priv->blink = blink;
+ *delay_on = *delay_off = delay;
+ return 0;
+}
+
static void ht16k33_fb_queue(struct ht16k33_priv *priv)
{
struct ht16k33_fbdev *fbdev = &priv->fbdev;
@@ -425,6 +477,35 @@ static void ht16k33_seg14_update(struct work_struct *work)
i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
}
+static int ht16k33_led_probe(struct device *dev, struct led_classdev *led,
+ unsigned int brightness)
+{
+ struct led_init_data init_data = {};
+ struct device_node *node;
+ int err;
+
+ /* The LED is optional */
+ node = of_get_child_by_name(dev->of_node, "led");
+ if (!node)
+ return 0;
+
+ init_data.fwnode = of_fwnode_handle(node);
+ init_data.devicename = "auxdisplay";
+ init_data.devname_mandatory = true;
+
+ led->brightness_set_blocking = ht16k33_brightness_set_blocking;
+ led->blink_set = ht16k33_blink_set;
+ led->flags = LED_CORE_SUSPENDRESUME;
+ led->brightness = brightness;
+ led->max_brightness = MAX_BRIGHTNESS;
+
+ err = devm_led_classdev_register_ext(dev, led, &init_data);
+ if (err)
+ dev_err(dev, "Failed to register LED\n");
+
+ return err;
+}
+
static int ht16k33_keypad_probe(struct i2c_client *client,
struct ht16k33_keypad *keypad)
{
@@ -498,24 +579,28 @@ static int ht16k33_fbdev_probe(struct device *dev, struct ht16k33_priv *priv,
uint32_t brightness)
{
struct ht16k33_fbdev *fbdev = &priv->fbdev;
- struct backlight_properties bl_props;
- struct backlight_device *bl;
+ struct backlight_device *bl = NULL;
int err;
- /* Backlight */
- memset(&bl_props, 0, sizeof(struct backlight_properties));
- bl_props.type = BACKLIGHT_RAW;
- bl_props.max_brightness = MAX_BRIGHTNESS;
+ if (!priv->led.dev) {
+ /* backwards compatibility with DT lacking an led subnode */
+ struct backlight_properties bl_props;
- bl = devm_backlight_device_register(dev, DRIVER_NAME"-bl", dev, priv,
- &ht16k33_bl_ops, &bl_props);
- if (IS_ERR(bl)) {
- dev_err(dev, "failed to register backlight\n");
- return PTR_ERR(bl);
- }
+ memset(&bl_props, 0, sizeof(struct backlight_properties));
+ bl_props.type = BACKLIGHT_RAW;
+ bl_props.max_brightness = MAX_BRIGHTNESS;
+
+ bl = devm_backlight_device_register(dev, DRIVER_NAME"-bl", dev,
+ priv, &ht16k33_bl_ops,
+ &bl_props);
+ if (IS_ERR(bl)) {
+ dev_err(dev, "failed to register backlight\n");
+ return PTR_ERR(bl);
+ }
- bl->props.brightness = brightness;
- ht16k33_bl_update_status(bl);
+ bl->props.brightness = brightness;
+ ht16k33_bl_update_status(bl);
+ }
/* Framebuffer (2 bytes per column) */
BUILD_BUG_ON(PAGE_SIZE < HT16K33_FB_SIZE);
@@ -575,7 +660,7 @@ static int ht16k33_seg_probe(struct device *dev, struct ht16k33_priv *priv,
struct ht16k33_seg *seg = &priv->seg;
int err;
- err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
+ err = ht16k33_brightness_set(priv, brightness);
if (err)
return err;
@@ -653,6 +738,11 @@ static int ht16k33_probe(struct i2c_client *client)
dft_brightness = MAX_BRIGHTNESS;
}
+ /* LED */
+ err = ht16k33_led_probe(dev, &priv->led, dft_brightness);
+ if (err)
+ return err;
+
/* Keypad */
if (client->irq > 0) {
err = ht16k33_keypad_probe(client, &priv->keypad);
--
2.25.1
There is no need to check the return code of input_register_device(),
just propagate it to the caller.
Signed-off-by: Geert Uytterhoeven <[email protected]>
Acked-by: Robin van der Gracht <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- No changes,
v3:
- No changes,
v2:
- Add Acked-by.
---
drivers/auxdisplay/ht16k33.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 99daf1974980b435..8c9acc4800bc94e0 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -378,11 +378,7 @@ static int ht16k33_keypad_probe(struct i2c_client *client,
ht16k33_keypad_stop(keypad->dev);
- err = input_register_device(keypad->dev);
- if (err)
- return err;
-
- return 0;
+ return input_register_device(keypad->dev);
}
static int ht16k33_probe(struct i2c_client *client,
--
2.25.1
Extend the Holtek HT16K33 LED controller Device Tree bindings with an
LED subnode, conforming to the standard LED bindings.
This allows the user to exert more control, like specifying LED color,
function, and/or trigger, to extend LED functionality beyond a simple
display backlight.
Signed-off-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
v6:
- No changes,
v5:
- No changes,
v4:
- Add Reviewed-by,
v3:
- New.
---
.../bindings/auxdisplay/holtek,ht16k33.yaml | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml b/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
index 6baf1200c9d8488e..0f1414185fefc1a9 100644
--- a/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
+++ b/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
@@ -48,6 +48,11 @@ properties:
default: 16
description: Initial brightness level
+ led:
+ type: object
+ $ref: /schemas/leds/common.yaml#
+ unevaluatedProperties: false
+
required:
- compatible
- reg
@@ -66,6 +71,7 @@ examples:
- |
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/input/input.h>
+ #include <dt-bindings/leds/common.h>
i2c1 {
#address-cells = <1>;
#size-cells = <0>;
@@ -87,5 +93,10 @@ examples:
<MATRIX_KEY(4, 1, KEY_F9)>,
<MATRIX_KEY(5, 1, KEY_F3)>,
<MATRIX_KEY(6, 1, KEY_F1)>;
+
+ led {
+ color = <LED_COLOR_ID_RED>;
+ function = LED_FUNCTION_BACKLIGHT;
+ };
};
};
--
2.25.1
Reviewed-by: Robin van der Gracht <[email protected]>
On 2021-09-14 16:38, Geert Uytterhoeven wrote:
> Currently /sys/class/graphics/fb0/bl_curve is not accessible (-ENODEV),
> as the driver does not connect the backlight to the frame buffer device.
> Fix this moving backlight initialization up, and filling in
> fb_info.bl_dev.
>
> Fixes: 8992da44c6805d53 ("auxdisplay: ht16k33: Driver for LED controller")
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> v6:
> - No changes,
>
> v5:
> - No changes,
>
> v4:
> - No changes,
>
> v3:
> - No changes,
>
> v2:
> - New.
> ---
> drivers/auxdisplay/ht16k33.c | 56 ++++++++++++++++++------------------
> 1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
> index 1e69cc6d21a0dca2..2b630e194570f6e5 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -413,6 +413,33 @@ static int ht16k33_probe(struct i2c_client *client,
> if (err)
> return err;
>
> + /* Backlight */
> + memset(&bl_props, 0, sizeof(struct backlight_properties));
> + bl_props.type = BACKLIGHT_RAW;
> + bl_props.max_brightness = MAX_BRIGHTNESS;
> +
> + bl = devm_backlight_device_register(&client->dev, DRIVER_NAME"-bl",
> + &client->dev, priv,
> + &ht16k33_bl_ops, &bl_props);
> + if (IS_ERR(bl)) {
> + dev_err(&client->dev, "failed to register backlight\n");
> + return PTR_ERR(bl);
> + }
> +
> + err = of_property_read_u32(node, "default-brightness-level",
> + &dft_brightness);
> + if (err) {
> + dft_brightness = MAX_BRIGHTNESS;
> + } else if (dft_brightness > MAX_BRIGHTNESS) {
> + dev_warn(&client->dev,
> + "invalid default brightness level: %u, using %u\n",
> + dft_brightness, MAX_BRIGHTNESS);
> + dft_brightness = MAX_BRIGHTNESS;
> + }
> +
> + bl->props.brightness = dft_brightness;
> + ht16k33_bl_update_status(bl);
> +
> /* Framebuffer (2 bytes per column) */
> BUILD_BUG_ON(PAGE_SIZE < HT16K33_FB_SIZE);
> fbdev->buffer = (unsigned char *) get_zeroed_page(GFP_KERNEL);
> @@ -445,6 +472,7 @@ static int ht16k33_probe(struct i2c_client *client,
> fbdev->info->screen_size = HT16K33_FB_SIZE;
> fbdev->info->fix = ht16k33_fb_fix;
> fbdev->info->var = ht16k33_fb_var;
> + fbdev->info->bl_dev = bl;
> fbdev->info->pseudo_palette = NULL;
> fbdev->info->flags = FBINFO_FLAG_DEFAULT;
> fbdev->info->par = priv;
> @@ -460,34 +488,6 @@ static int ht16k33_probe(struct i2c_client *client,
> goto err_fbdev_unregister;
> }
>
> - /* Backlight */
> - memset(&bl_props, 0, sizeof(struct backlight_properties));
> - bl_props.type = BACKLIGHT_RAW;
> - bl_props.max_brightness = MAX_BRIGHTNESS;
> -
> - bl = devm_backlight_device_register(&client->dev, DRIVER_NAME"-bl",
> - &client->dev, priv,
> - &ht16k33_bl_ops, &bl_props);
> - if (IS_ERR(bl)) {
> - dev_err(&client->dev, "failed to register backlight\n");
> - err = PTR_ERR(bl);
> - goto err_fbdev_unregister;
> - }
> -
> - err = of_property_read_u32(node, "default-brightness-level",
> - &dft_brightness);
> - if (err) {
> - dft_brightness = MAX_BRIGHTNESS;
> - } else if (dft_brightness > MAX_BRIGHTNESS) {
> - dev_warn(&client->dev,
> - "invalid default brightness level: %u, using %u\n",
> - dft_brightness, MAX_BRIGHTNESS);
> - dft_brightness = MAX_BRIGHTNESS;
> - }
> -
> - bl->props.brightness = dft_brightness;
> - ht16k33_bl_update_status(bl);
> -
> ht16k33_fb_queue(priv);
> return 0;
Reviewed-by: Robin van der Gracht <[email protected]>
On 2021-09-14 16:38, Geert Uytterhoeven wrote:
> Extract brightness handling into a helper function, so it can be called
> from multiple places.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> v6:
> - No changes,
>
> v5:
> - No changes,
>
> v4:
> - No changes,
>
> v3:
> - Use "err" instead of "error" to be consistent with existing driver
> naming style,
>
> v2:
> - No changes.
> ---
> drivers/auxdisplay/ht16k33.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
> index c7a3a0e1fbb5d03e..928ac9722c142855 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -113,6 +113,22 @@ static int ht16k33_display_off(struct ht16k33_priv
> *priv)
> return i2c_smbus_write_byte(priv->client, REG_DISPLAY_SETUP);
> }
>
> +static int ht16k33_brightness_set(struct ht16k33_priv *priv,
> + unsigned int brightness)
> +{
> + int err;
> +
> + if (brightness == 0)
> + return ht16k33_display_off(priv);
> +
> + err = ht16k33_display_on(priv);
> + if (err)
> + return err;
> +
> + return i2c_smbus_write_byte(priv->client,
> + REG_BRIGHTNESS | (brightness - 1));
> +}
> +
> static void ht16k33_fb_queue(struct ht16k33_priv *priv)
> {
> struct ht16k33_fbdev *fbdev = &priv->fbdev;
> @@ -197,13 +213,10 @@ static int ht16k33_bl_update_status(struct
> backlight_device *bl)
>
> if (bl->props.power != FB_BLANK_UNBLANK ||
> bl->props.fb_blank != FB_BLANK_UNBLANK ||
> - bl->props.state & BL_CORE_FBBLANK || brightness == 0) {
> - return ht16k33_display_off(priv);
> - }
> + bl->props.state & BL_CORE_FBBLANK)
> + brightness = 0;
>
> - ht16k33_display_on(priv);
> - return i2c_smbus_write_byte(priv->client,
> - REG_BRIGHTNESS | (brightness - 1));
> + return ht16k33_brightness_set(priv, brightness);
> }
>
> static int ht16k33_bl_check_fb(struct backlight_device *bl, struct fb_info
> *fi)
Met vriendelijke groet,
Robin van der Gracht
--
Protonic Holland
Factorij 36
1689AL Zwaag
+31 (0)229 212928
https://www.protonic.nl
Acked-by: Robin van der Gracht <[email protected]>
On 2021-09-14 16:38, 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 <[email protected]>
> ---
> The 7-segment support is based on schematics, and untested on actual
> hardware.
>
> v6:
> - No changes,
>
> v5:
> - No changes,
>
> v4:
> - No changes,
>
> v3:
> - Pass "dev" instead of "client" to ht16k33_seg_probe(),
> - Remove unneeded inclusion of <linux/leds.h> and <linux/of_device.h>,
>
> v2:
> - Move "select LINEDISP" to this patch,
> - Set "err = -EINVAL" in switch() case that cannot happen.
> ---
> drivers/auxdisplay/Kconfig | 1 +
> drivers/auxdisplay/ht16k33.c | 196 +++++++++++++++++++++++++++++++++--
> 2 files changed, 190 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> index b41fdc5f7bf72a1b..42fc7b155de09dbc 100644
> --- a/drivers/auxdisplay/Kconfig
> +++ b/drivers/auxdisplay/Kconfig
> @@ -176,6 +176,7 @@ config HT16K33
> select FB_SYS_IMAGEBLIT
> select INPUT_MATRIXKMAP
> select FB_BACKLIGHT
> + select LINEDISP
> help
> Say yes here to add support for Holtek HT16K33, RAM mapping 16*8
> LED controller driver with keyscan.
> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
> index 27ac167dae74bd82..3b555e119e326cec 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -5,6 +5,7 @@
> * Author: Robin van der Gracht <[email protected]>
> *
> * Copyright: (C) 2016 Protonic Holland.
> + * Copyright (C) 2021 Glider bv
> */
>
> #include <linux/kernel.h>
> @@ -20,6 +21,13 @@
> #include <linux/workqueue.h>
> #include <linux/mm.h>
>
> +#include <linux/map_to_7segment.h>
> +#include <linux/map_to_14segment.h>
> +
> +#include <asm/unaligned.h>
> +
> +#include "line-display.h"
> +
> /* Registers */
> #define REG_SYSTEM_SETUP 0x20
> #define REG_SYSTEM_SETUP_OSC_ON BIT(0)
> @@ -47,6 +55,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 +81,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 +129,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, &priv->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(&priv->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 +380,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(&priv->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 = &priv->seg;
> + char *s = seg->curr;
> + uint8_t buf[9];
> +
> + buf[0] = map_to_seg7(&seg->map.seg7, *s++);
> + buf[1] = 0;
> + buf[2] = map_to_seg7(&seg->map.seg7, *s++);
> + buf[3] = 0;
> + buf[4] = 0;
> + buf[5] = 0;
> + buf[6] = map_to_seg7(&seg->map.seg7, *s++);
> + buf[7] = 0;
> + buf[8] = map_to_seg7(&seg->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 = &priv->seg;
> + char *s = seg->curr;
> + uint8_t buf[8];
> +
> + put_unaligned_le16(map_to_seg14(&seg->map.seg14, *s++), buf);
> + put_unaligned_le16(map_to_seg14(&seg->map.seg14, *s++), buf + 2);
> + put_unaligned_le16(map_to_seg14(&seg->map.seg14, *s++), buf + 4);
> + put_unaligned_le16(map_to_seg14(&seg->map.seg14, *s++), buf + 6);
> +
> + i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
> +}
> +
> static int ht16k33_keypad_probe(struct i2c_client *client,
> struct ht16k33_keypad *keypad)
> {
> @@ -469,9 +569,56 @@ static int ht16k33_fbdev_probe(struct device *dev,
> struct ht16k33_priv *priv,
> return err;
> }
>
> +static int ht16k33_seg_probe(struct device *dev, struct ht16k33_priv *priv,
> + uint32_t brightness)
> +{
> + struct ht16k33_seg *seg = &priv->seg;
> + int err;
> +
> + err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
> + if (err)
> + return err;
> +
> + switch (priv->type) {
> + case DISP_MATRIX:
> + /* not handled here */
> + err = -EINVAL;
> + break;
> +
> + case DISP_QUAD_7SEG:
> + INIT_DELAYED_WORK(&priv->work, ht16k33_seg7_update);
> + seg->map.seg7 = initial_map_seg7;
> + seg->map_size = sizeof(seg->map.seg7);
> + err = device_create_file(dev, &dev_attr_map_seg7);
> + break;
> +
> + case DISP_QUAD_14SEG:
> + INIT_DELAYED_WORK(&priv->work, ht16k33_seg14_update);
> + seg->map.seg14 = initial_map_seg14;
> + seg->map_size = sizeof(seg->map.seg14);
> + err = device_create_file(dev, &dev_attr_map_seg14);
> + break;
> + }
> + if (err)
> + return err;
> +
> + err = linedisp_register(&seg->linedisp, dev, 4, seg->curr,
> + ht16k33_linedisp_update);
> + if (err)
> + goto err_remove_map_file;
> +
> + return 0;
> +
> +err_remove_map_file:
> + device_remove_file(dev, &dev_attr_map_seg7);
> + device_remove_file(dev, &dev_attr_map_seg14);
> + return err;
> +}
> +
> static int ht16k33_probe(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> + const struct of_device_id *id;
> struct ht16k33_priv *priv;
> uint32_t dft_brightness;
> int err;
> @@ -486,6 +633,9 @@ static int ht16k33_probe(struct i2c_client *client)
> return -ENOMEM;
>
> priv->client = client;
> + id = i2c_of_match_device(dev->driver->of_match_table, client);
> + if (id)
> + priv->type = (uintptr_t)id->data;
> i2c_set_clientdata(client, priv);
>
> err = ht16k33_initialize(priv);
> @@ -510,8 +660,19 @@ static int ht16k33_probe(struct i2c_client *client)
> return err;
> }
>
> - /* Frame Buffer Display */
> - return ht16k33_fbdev_probe(dev, priv, dft_brightness);
> + switch (priv->type) {
> + case DISP_MATRIX:
> + /* Frame Buffer Display */
> + err = ht16k33_fbdev_probe(dev, priv, dft_brightness);
> + break;
> +
> + case DISP_QUAD_7SEG:
> + case DISP_QUAD_14SEG:
> + /* Segment Display */
> + err = ht16k33_seg_probe(dev, priv, dft_brightness);
> + break;
> + }
> + return err;
> }
>
> static int ht16k33_remove(struct i2c_client *client)
> @@ -520,9 +681,21 @@ static int ht16k33_remove(struct i2c_client *client)
> struct ht16k33_fbdev *fbdev = &priv->fbdev;
>
> cancel_delayed_work_sync(&priv->work);
> - unregister_framebuffer(fbdev->info);
> - framebuffer_release(fbdev->info);
> - free_page((unsigned long) fbdev->buffer);
> +
> + switch (priv->type) {
> + case DISP_MATRIX:
> + unregister_framebuffer(fbdev->info);
> + framebuffer_release(fbdev->info);
> + free_page((unsigned long)fbdev->buffer);
> + break;
> +
> + case DISP_QUAD_7SEG:
> + case DISP_QUAD_14SEG:
> + linedisp_unregister(&priv->seg.linedisp);
> + device_remove_file(&client->dev, &dev_attr_map_seg7);
> + device_remove_file(&client->dev, &dev_attr_map_seg14);
> + break;
> + }
>
> return 0;
> }
> @@ -534,7 +707,16 @@ static const struct i2c_device_id ht16k33_i2c_match[] =
> {
> MODULE_DEVICE_TABLE(i2c, ht16k33_i2c_match);
>
> static const struct of_device_id ht16k33_of_match[] = {
> - { .compatible = "holtek,ht16k33", },
> + {
> + /* 0.56" 4-Digit 7-Segment FeatherWing Display (Red) */
> + .compatible = "adafruit,3108", .data = (void *)DISP_QUAD_7SEG,
> + }, {
> + /* 0.54" Quad Alphanumeric FeatherWing Display (Red) */
> + .compatible = "adafruit,3130", .data = (void *)DISP_QUAD_14SEG,
> + }, {
> + /* Generic, assumed Dot-Matrix Display */
> + .compatible = "holtek,ht16k33", .data = (void *)DISP_MATRIX,
> + },
> { }
> };
> MODULE_DEVICE_TABLE(of, ht16k33_of_match);
Acked-by: Robin van der Gracht <[email protected]>
On 2021-09-14 16:38, Geert Uytterhoeven wrote:
> Move delayed_work from ht16k33_fbdev to ht16k33_priv, as it is not
> specific to dot-matrix displays, but common to all display types.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> v6:
> - No changes,
>
> v5:
> - No changes,
>
> v4:
> - No changes,
>
> v3:
> - No changes,
>
> v2:
> - No changes.
> ---
> drivers/auxdisplay/ht16k33.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
> index 75d326a823543898..c7a3a0e1fbb5d03e 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -65,11 +65,11 @@ struct ht16k33_fbdev {
> uint32_t refresh_rate;
> uint8_t *buffer;
> uint8_t *cache;
> - struct delayed_work work;
> };
>
> struct ht16k33_priv {
> struct i2c_client *client;
> + struct delayed_work work;
> struct ht16k33_keypad keypad;
> struct ht16k33_fbdev fbdev;
> };
> @@ -117,7 +117,7 @@ static void ht16k33_fb_queue(struct ht16k33_priv *priv)
> {
> struct ht16k33_fbdev *fbdev = &priv->fbdev;
>
> - schedule_delayed_work(&fbdev->work, HZ / fbdev->refresh_rate);
> + schedule_delayed_work(&priv->work, HZ / fbdev->refresh_rate);
> }
>
> /*
> @@ -125,10 +125,9 @@ static void ht16k33_fb_queue(struct ht16k33_priv *priv)
> */
> static void ht16k33_fb_update(struct work_struct *work)
> {
> - struct ht16k33_fbdev *fbdev =
> - container_of(work, struct ht16k33_fbdev, work.work);
> - struct ht16k33_priv *priv =
> - container_of(fbdev, struct ht16k33_priv, fbdev);
> + struct ht16k33_priv *priv = container_of(work, struct ht16k33_priv,
> + work.work);
> + struct ht16k33_fbdev *fbdev = &priv->fbdev;
>
> uint8_t *p1, *p2;
> int len, pos = 0, first = -1;
> @@ -462,7 +461,7 @@ static int ht16k33_probe(struct i2c_client *client)
> }
> fb_bl_default_curve(fbdev->info, 0, MIN_BRIGHTNESS, MAX_BRIGHTNESS);
>
> - INIT_DELAYED_WORK(&fbdev->work, ht16k33_fb_update);
> + INIT_DELAYED_WORK(&priv->work, ht16k33_fb_update);
> fbdev->info->fbops = &ht16k33_fb_ops;
> fbdev->info->screen_base = (char __iomem *) fbdev->buffer;
> fbdev->info->screen_size = HT16K33_FB_SIZE;
> @@ -502,7 +501,7 @@ static int ht16k33_remove(struct i2c_client *client)
> struct ht16k33_priv *priv = i2c_get_clientdata(client);
> struct ht16k33_fbdev *fbdev = &priv->fbdev;
>
> - cancel_delayed_work_sync(&fbdev->work);
> + cancel_delayed_work_sync(&priv->work);
> unregister_framebuffer(fbdev->info);
> framebuffer_release(fbdev->info);
> free_page((unsigned long) fbdev->buffer);
Reviewed-by: Robin van der Gracht <[email protected]>
On 2021-09-14 16:38, Geert Uytterhoeven wrote:
> Extract all frame buffer (including backlight) probing into
> ht16k33_fbdev_probe().
>
> Call ht16k33_fbdev_probe() after ht16k33_keypad_probe(), as the latter
> does not need any manual cleanup in the probe error path.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> v6:
> - No changes,
>
> v5:
> - No changes,
>
> v4:
> - No changes,
>
> v3:
> - Pass "dev" instead of "client" to ht16k33_fbdev_probe(),
> - Drop local variable "node",
>
> v2:
> - Rebased.
> ---
> drivers/auxdisplay/ht16k33.c | 101 ++++++++++++++++++-----------------
> 1 file changed, 53 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
> index 928ac9722c142855..27ac167dae74bd82 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -394,33 +394,13 @@ static int ht16k33_keypad_probe(struct i2c_client
> *client,
> return input_register_device(keypad->dev);
> }
>
> -static int ht16k33_probe(struct i2c_client *client)
> +static int ht16k33_fbdev_probe(struct device *dev, struct ht16k33_priv
> *priv,
> + uint32_t brightness)
> {
> - int err;
> - uint32_t dft_brightness;
> - struct backlight_device *bl;
> + struct ht16k33_fbdev *fbdev = &priv->fbdev;
> struct backlight_properties bl_props;
> - struct ht16k33_priv *priv;
> - struct ht16k33_fbdev *fbdev;
> - struct device *dev = &client->dev;
> - struct device_node *node = dev->of_node;
> -
> - if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> - dev_err(dev, "i2c_check_functionality error\n");
> - return -EIO;
> - }
> -
> - priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> - if (!priv)
> - return -ENOMEM;
> -
> - priv->client = client;
> - i2c_set_clientdata(client, priv);
> - fbdev = &priv->fbdev;
> -
> - err = ht16k33_initialize(priv);
> - if (err)
> - return err;
> + struct backlight_device *bl;
> + int err;
>
> /* Backlight */
> memset(&bl_props, 0, sizeof(struct backlight_properties));
> @@ -434,18 +414,7 @@ static int ht16k33_probe(struct i2c_client *client)
> return PTR_ERR(bl);
> }
>
> - err = of_property_read_u32(node, "default-brightness-level",
> - &dft_brightness);
> - if (err) {
> - dft_brightness = MAX_BRIGHTNESS;
> - } else if (dft_brightness > MAX_BRIGHTNESS) {
> - dev_warn(dev,
> - "invalid default brightness level: %u, using %u\n",
> - dft_brightness, MAX_BRIGHTNESS);
> - dft_brightness = MAX_BRIGHTNESS;
> - }
> -
> - bl->props.brightness = dft_brightness;
> + bl->props.brightness = brightness;
> ht16k33_bl_update_status(bl);
>
> /* Framebuffer (2 bytes per column) */
> @@ -466,8 +435,8 @@ static int ht16k33_probe(struct i2c_client *client)
> goto err_fbdev_buffer;
> }
>
> - err = of_property_read_u32(node, "refresh-rate-hz",
> - &fbdev->refresh_rate);
> + err = of_property_read_u32(dev->of_node, "refresh-rate-hz",
> + &fbdev->refresh_rate);
> if (err) {
> dev_err(dev, "refresh rate not specified\n");
> goto err_fbdev_info;
> @@ -489,18 +458,9 @@ static int ht16k33_probe(struct i2c_client *client)
> if (err)
> goto err_fbdev_info;
>
> - /* Keypad */
> - if (client->irq > 0) {
> - err = ht16k33_keypad_probe(client, &priv->keypad);
> - if (err)
> - goto err_fbdev_unregister;
> - }
> -
> ht16k33_fb_queue(priv);
> return 0;
>
> -err_fbdev_unregister:
> - unregister_framebuffer(fbdev->info);
> err_fbdev_info:
> framebuffer_release(fbdev->info);
> err_fbdev_buffer:
> @@ -509,6 +469,51 @@ static int ht16k33_probe(struct i2c_client *client)
> return err;
> }
>
> +static int ht16k33_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct ht16k33_priv *priv;
> + uint32_t dft_brightness;
> + int err;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> + dev_err(dev, "i2c_check_functionality error\n");
> + return -EIO;
> + }
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->client = client;
> + i2c_set_clientdata(client, priv);
> +
> + err = ht16k33_initialize(priv);
> + if (err)
> + return err;
> +
> + err = of_property_read_u32(dev->of_node, "default-brightness-level",
> + &dft_brightness);
> + if (err) {
> + dft_brightness = MAX_BRIGHTNESS;
> + } else if (dft_brightness > MAX_BRIGHTNESS) {
> + dev_warn(dev,
> + "invalid default brightness level: %u, using %u\n",
> + dft_brightness, MAX_BRIGHTNESS);
> + dft_brightness = MAX_BRIGHTNESS;
> + }
> +
> + /* Keypad */
> + if (client->irq > 0) {
> + err = ht16k33_keypad_probe(client, &priv->keypad);
> + if (err)
> + return err;
> + }
> +
> + /* Frame Buffer Display */
> + return ht16k33_fbdev_probe(dev, priv, dft_brightness);
> +}
> +
> static int ht16k33_remove(struct i2c_client *client)
> {
> struct ht16k33_priv *priv = i2c_get_clientdata(client);
Met vriendelijke groet,
Robin van der Gracht
--
Protonic Holland
Factorij 36
1689AL Zwaag
+31 (0)229 212928
https://www.protonic.nl
Hi Geert,
On 2021-09-14 16:38, Geert Uytterhoeven wrote:
> Instantiate a single LED based on the "led" subnode in DT.
> This allows the user to control display brightness and blinking (backed
> by hardware support) through the LED class API and triggers, and exposes
> the display color. The LED will be named
> "auxdisplay:<color>:<function>".
>
> When running in dot-matrix mode and if no "led" subnode is found, the
> driver falls back to the traditional backlight mode, to preserve
> backwards compatibility.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> Reviewed-by: Marek Behún <[email protected]>
> ---
> v6:
> - Add Reviewed-by,
> - Reorder operations in ht16k33_led_probe() to ease future conversion
> to device properties,
>
> v5:
> - Add missing select NEW_LEDS,
>
> v4:
> - Add missing select LEDS_CLASS,
>
> v3:
> - Remove unneeded C++ comment,
> - Use "err" instead of "error" to be consistent with existing driver
> naming style,
> - Make the creation of the LED device dependent on the presence of the
> "led" subnode in DT, so it can be used in dot-matrix mode too.
> - Use led_init_data() and devm_led_classdev_register_ext() to retrieve
> all LED properties from DT, instead of manual LED name construction
> based on just the "color" property,
>
> v2:
> - Use "auxdisplay" instead of DRIVER_NAME in LED name.
> ---
> drivers/auxdisplay/Kconfig | 2 +
> drivers/auxdisplay/ht16k33.c | 124 ++++++++++++++++++++++++++++++-----
> 2 files changed, 109 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> index 42fc7b155de09dbc..e32ef7f9945d49b2 100644
> --- a/drivers/auxdisplay/Kconfig
> +++ b/drivers/auxdisplay/Kconfig
> @@ -176,6 +176,8 @@ config HT16K33
> select FB_SYS_IMAGEBLIT
> select INPUT_MATRIXKMAP
> select FB_BACKLIGHT
> + select NEW_LEDS
> + select LEDS_CLASS
> select LINEDISP
> help
> Say yes here to add support for Holtek HT16K33, RAM mapping 16*8
> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
> index 3b555e119e326cec..89ee5b4b3dfccb68 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -18,6 +18,7 @@
> #include <linux/backlight.h>
> #include <linux/input.h>
> #include <linux/input/matrix_keypad.h>
> +#include <linux/leds.h>
> #include <linux/workqueue.h>
> #include <linux/mm.h>
>
> @@ -34,6 +35,10 @@
>
> #define REG_DISPLAY_SETUP 0x80
> #define REG_DISPLAY_SETUP_ON BIT(0)
> +#define REG_DISPLAY_SETUP_BLINK_OFF (0 << 1)
> +#define REG_DISPLAY_SETUP_BLINK_2HZ (1 << 1)
> +#define REG_DISPLAY_SETUP_BLINK_1HZ (2 << 1)
> +#define REG_DISPLAY_SETUP_BLINK_0HZ5 (3 << 1)
>
> #define REG_ROWINT_SET 0xA0
> #define REG_ROWINT_SET_INT_EN BIT(0)
> @@ -94,12 +99,14 @@ struct ht16k33_seg {
> struct ht16k33_priv {
> struct i2c_client *client;
> struct delayed_work work;
> + struct led_classdev led;
> struct ht16k33_keypad keypad;
> union {
> struct ht16k33_fbdev fbdev;
> struct ht16k33_seg seg;
> };
> enum display_type type;
> + uint8_t blink;
> };
>
> static const struct fb_fix_screeninfo ht16k33_fb_fix = {
> @@ -158,7 +165,7 @@ 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;
> + uint8_t data = REG_DISPLAY_SETUP | REG_DISPLAY_SETUP_ON | priv->blink;
>
> return i2c_smbus_write_byte(priv->client, data);
> }
> @@ -173,8 +180,10 @@ static int ht16k33_brightness_set(struct ht16k33_priv
> *priv,
> {
> int err;
>
> - if (brightness == 0)
> + if (brightness == 0) {
> + priv->blink = REG_DISPLAY_SETUP_BLINK_OFF;
> return ht16k33_display_off(priv);
> + }
>
> err = ht16k33_display_on(priv);
> if (err)
> @@ -184,6 +193,49 @@ static int ht16k33_brightness_set(struct ht16k33_priv
> *priv,
> REG_BRIGHTNESS | (brightness - 1));
> }
>
> +static int ht16k33_brightness_set_blocking(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + struct ht16k33_priv *priv = container_of(led_cdev, struct ht16k33_priv,
> + led);
> +
> + return ht16k33_brightness_set(priv, brightness);
> +}
> +
> +static int ht16k33_blink_set(struct led_classdev *led_cdev,
> + unsigned long *delay_on, unsigned long *delay_off)
> +{
> + struct ht16k33_priv *priv = container_of(led_cdev, struct ht16k33_priv,
> + led);
> + unsigned int delay;
> + uint8_t blink;
> + int err;
> +
> + if (!*delay_on && !*delay_off) {
> + blink = REG_DISPLAY_SETUP_BLINK_1HZ;
> + delay = 1000;
> + } else if (*delay_on <= 750) {
> + blink = REG_DISPLAY_SETUP_BLINK_2HZ;
> + delay = 500;
> + } else if (*delay_on <= 1500) {
> + blink = REG_DISPLAY_SETUP_BLINK_1HZ;
> + delay = 1000;
> + } else {
> + blink = REG_DISPLAY_SETUP_BLINK_0HZ5;
> + delay = 2000;
> + }
> +
> + err = i2c_smbus_write_byte(priv->client,
> + REG_DISPLAY_SETUP | REG_DISPLAY_SETUP_ON |
> + blink);
> + if (err)
> + return err;
> +
> + priv->blink = blink;
> + *delay_on = *delay_off = delay;
> + return 0;
> +}
> +
> static void ht16k33_fb_queue(struct ht16k33_priv *priv)
> {
> struct ht16k33_fbdev *fbdev = &priv->fbdev;
> @@ -425,6 +477,35 @@ static void ht16k33_seg14_update(struct work_struct
> *work)
> i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
> }
>
> +static int ht16k33_led_probe(struct device *dev, struct led_classdev *led,
> + unsigned int brightness)
> +{
> + struct led_init_data init_data = {};
> + struct device_node *node;
> + int err;
> +
> + /* The LED is optional */
> + node = of_get_child_by_name(dev->of_node, "led");
> + if (!node)
> + return 0;
> +
> + init_data.fwnode = of_fwnode_handle(node);
> + init_data.devicename = "auxdisplay";
> + init_data.devname_mandatory = true;
> +
> + led->brightness_set_blocking = ht16k33_brightness_set_blocking;
> + led->blink_set = ht16k33_blink_set;
> + led->flags = LED_CORE_SUSPENDRESUME;
> + led->brightness = brightness;
> + led->max_brightness = MAX_BRIGHTNESS;
What do you think about adding a default trigger and making it 'backlight'?
led->default_trigger = "blacklight";
Or as an alternative, suggesting linux,default-trigger = "backlight" in the
docs? Since the led class won't respond to blank events by just making it's
function LED_FUNCTION_BACKLIGHT.
led {
function = LED_FUNCTION_BACKLIGHT;
color = <LED_COLOR_ID_GREEN>;
linux,default-trigger = "backlight";
};
I noticed blanking is broken. The backlight device (or LED device with
backlight trigger) doens't get notified when the framebuffer is blanked since
the driver doesn't implement fb_blank.
Right now:
echo 1 > /sys/class/graphics/fb0/blank
|
sh: write error: Invalid argument
Due to:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/fbdev/core/fbmem.c?h=v5.15-rc3#n1078
Something like this fixes it.
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 89ee5b4b3dfc..0883d5252c81 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -346,6 +346,15 @@ static int ht16k33_mmap(struct fb_info *info, struct
vm_area_struct *vma)
return vm_map_pages_zero(vma, &pages, 1);
}
+/*
+ * Blank events will be passed to the backlight device (or the LED device if
+ * it's trigger is 'backlight') when we return 0 here.
+ */
+static int ht16k33_blank(int blank, struct fb_info *info)
+{
+ return 0;
+}
+
static const struct fb_ops ht16k33_fb_ops = {
.owner = THIS_MODULE,
.fb_read = fb_sys_read,
@@ -354,6 +363,7 @@ static const struct fb_ops ht16k33_fb_ops = {
.fb_copyarea = sys_copyarea,
.fb_imageblit = sys_imageblit,
.fb_mmap = ht16k33_mmap,
+ .fb_blank = ht16k33_blank,
};
/*
Feel free to include (something like) this in the patch stack.
> +
> + err = devm_led_classdev_register_ext(dev, led, &init_data);
> + if (err)
> + dev_err(dev, "Failed to register LED\n");
You might want to call ht16k33_brightness_set(priv, brightness) here to get a
know value into the display setup register (0x80).
Right now if I enable hardware blinking and (soft)reboot my board it keeps on
blinking even after a re-probe.
> +
> + return err;
> +}
> +
> static int ht16k33_keypad_probe(struct i2c_client *client,
> struct ht16k33_keypad *keypad)
> {
> @@ -498,24 +579,28 @@ static int ht16k33_fbdev_probe(struct device *dev,
> struct ht16k33_priv *priv,
> uint32_t brightness)
> {
> struct ht16k33_fbdev *fbdev = &priv->fbdev;
> - struct backlight_properties bl_props;
> - struct backlight_device *bl;
> + struct backlight_device *bl = NULL;
> int err;
>
> - /* Backlight */
> - memset(&bl_props, 0, sizeof(struct backlight_properties));
> - bl_props.type = BACKLIGHT_RAW;
> - bl_props.max_brightness = MAX_BRIGHTNESS;
> + if (!priv->led.dev) {
> + /* backwards compatibility with DT lacking an led subnode */
> + struct backlight_properties bl_props;
>
> - bl = devm_backlight_device_register(dev, DRIVER_NAME"-bl", dev, priv,
> - &ht16k33_bl_ops, &bl_props);
> - if (IS_ERR(bl)) {
> - dev_err(dev, "failed to register backlight\n");
> - return PTR_ERR(bl);
> - }
> + memset(&bl_props, 0, sizeof(struct backlight_properties));
> + bl_props.type = BACKLIGHT_RAW;
> + bl_props.max_brightness = MAX_BRIGHTNESS;
> +
> + bl = devm_backlight_device_register(dev, DRIVER_NAME"-bl", dev,
> + priv, &ht16k33_bl_ops,
> + &bl_props);
> + if (IS_ERR(bl)) {
> + dev_err(dev, "failed to register backlight\n");
> + return PTR_ERR(bl);
> + }
>
> - bl->props.brightness = brightness;
> - ht16k33_bl_update_status(bl);
> + bl->props.brightness = brightness;
> + ht16k33_bl_update_status(bl);
> + }
>
> /* Framebuffer (2 bytes per column) */
> BUILD_BUG_ON(PAGE_SIZE < HT16K33_FB_SIZE);
> @@ -575,7 +660,7 @@ static int ht16k33_seg_probe(struct device *dev, struct
> ht16k33_priv *priv,
> struct ht16k33_seg *seg = &priv->seg;
> int err;
>
> - err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
> + err = ht16k33_brightness_set(priv, brightness);
This looks like a bugfix for patch 17, maybe move this change there?
> if (err)
> return err;
>
> @@ -653,6 +738,11 @@ static int ht16k33_probe(struct i2c_client *client)
> dft_brightness = MAX_BRIGHTNESS;
> }
>
> + /* LED */
> + err = ht16k33_led_probe(dev, &priv->led, dft_brightness);
> + if (err)
> + return err;
> +
> /* Keypad */
> if (client->irq > 0) {
> err = ht16k33_keypad_probe(client, &priv->keypad);
Gr{oetje,eeting}s,
Robin
Hoi Robin,
On Thu, Sep 30, 2021 at 12:57 PM Robin van der Gracht <[email protected]> wrote:
> On 2021-09-14 16:38, Geert Uytterhoeven wrote:
> > Instantiate a single LED based on the "led" subnode in DT.
> > This allows the user to control display brightness and blinking (backed
> > by hardware support) through the LED class API and triggers, and exposes
> > the display color. The LED will be named
> > "auxdisplay:<color>:<function>".
> >
> > When running in dot-matrix mode and if no "led" subnode is found, the
> > driver falls back to the traditional backlight mode, to preserve
> > backwards compatibility.
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > Reviewed-by: Marek Behún <[email protected]>
> > ---
> > v6:
> > - Add Reviewed-by,
> > - Reorder operations in ht16k33_led_probe() to ease future conversion
> > to device properties,
> > --- a/drivers/auxdisplay/ht16k33.c
> > +++ b/drivers/auxdisplay/ht16k33.c
> > @@ -425,6 +477,35 @@ static void ht16k33_seg14_update(struct work_struct
> > *work)
> > i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
> > }
> >
> > +static int ht16k33_led_probe(struct device *dev, struct led_classdev *led,
> > + unsigned int brightness)
> > +{
> > + struct led_init_data init_data = {};
> > + struct device_node *node;
> > + int err;
> > +
> > + /* The LED is optional */
> > + node = of_get_child_by_name(dev->of_node, "led");
> > + if (!node)
> > + return 0;
> > +
> > + init_data.fwnode = of_fwnode_handle(node);
> > + init_data.devicename = "auxdisplay";
> > + init_data.devname_mandatory = true;
> > +
> > + led->brightness_set_blocking = ht16k33_brightness_set_blocking;
> > + led->blink_set = ht16k33_blink_set;
> > + led->flags = LED_CORE_SUSPENDRESUME;
> > + led->brightness = brightness;
> > + led->max_brightness = MAX_BRIGHTNESS;
>
> What do you think about adding a default trigger and making it 'backlight'?
>
> led->default_trigger = "blacklight";
>
> Or as an alternative, suggesting linux,default-trigger = "backlight" in the
> docs? Since the led class won't respond to blank events by just making it's
> function LED_FUNCTION_BACKLIGHT.
>
> led {
> function = LED_FUNCTION_BACKLIGHT;
> color = <LED_COLOR_ID_GREEN>;
> linux,default-trigger = "backlight";
> };
The latter makes perfect sense to me. Will do.
> I noticed blanking is broken. The backlight device (or LED device with
> backlight trigger) doens't get notified when the framebuffer is blanked since
> the driver doesn't implement fb_blank.
>
> Right now:
>
> echo 1 > /sys/class/graphics/fb0/blank
> |
> sh: write error: Invalid argument
>
> Due to:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/fbdev/core/fbmem.c?h=v5.15-rc3#n1078
That's a pre-existing problem, righ? ;-)
> Something like this fixes it.
>
> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
> index 89ee5b4b3dfc..0883d5252c81 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -346,6 +346,15 @@ static int ht16k33_mmap(struct fb_info *info, struct
> vm_area_struct *vma)
> return vm_map_pages_zero(vma, &pages, 1);
> }
>
> +/*
> + * Blank events will be passed to the backlight device (or the LED device if
> + * it's trigger is 'backlight') when we return 0 here.
> + */
> +static int ht16k33_blank(int blank, struct fb_info *info)
> +{
> + return 0;
> +}
> +
> static const struct fb_ops ht16k33_fb_ops = {
> .owner = THIS_MODULE,
> .fb_read = fb_sys_read,
> @@ -354,6 +363,7 @@ static const struct fb_ops ht16k33_fb_ops = {
> .fb_copyarea = sys_copyarea,
> .fb_imageblit = sys_imageblit,
> .fb_mmap = ht16k33_mmap,
> + .fb_blank = ht16k33_blank,
> };
>
> /*
>
> Feel free to include (something like) this in the patch stack.
Thanks, will do.
> > +
> > + err = devm_led_classdev_register_ext(dev, led, &init_data);
> > + if (err)
> > + dev_err(dev, "Failed to register LED\n");
>
> You might want to call ht16k33_brightness_set(priv, brightness) here to get a
> know value into the display setup register (0x80).
>
> Right now if I enable hardware blinking and (soft)reboot my board it keeps on
> blinking even after a re-probe.
I don't have that issue.
Aha, ht16k33_seg_probe() calls ht16k33_brightness_set(), but
ht16k33_fbdev_probe() doesn't. The latter should do that, too,
when not using backwards compatibility mode.
> > @@ -575,7 +660,7 @@ static int ht16k33_seg_probe(struct device *dev, struct
> > ht16k33_priv *priv,
> > struct ht16k33_seg *seg = &priv->seg;
> > int err;
> >
> > - err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
> > + err = ht16k33_brightness_set(priv, brightness);
>
> This looks like a bugfix for patch 17, maybe move this change there?
Indeed. Bad rebase. Will move.
Thanks a lot for your comments!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On 2021-10-01 17:51, Geert Uytterhoeven wrote:
> Hoi Robin,
>
> On Thu, Sep 30, 2021 at 12:57 PM Robin van der Gracht <[email protected]>
> wrote:
>> On 2021-09-14 16:38, Geert Uytterhoeven wrote:
>> > Instantiate a single LED based on the "led" subnode in DT.
>> > This allows the user to control display brightness and blinking (backed
>> > by hardware support) through the LED class API and triggers, and exposes
>> > the display color. The LED will be named
>> > "auxdisplay:<color>:<function>".
>> >
>> > When running in dot-matrix mode and if no "led" subnode is found, the
>> > driver falls back to the traditional backlight mode, to preserve
>> > backwards compatibility.
>> >
>> > Signed-off-by: Geert Uytterhoeven <[email protected]>
>> > Reviewed-by: Marek Behún <[email protected]>
>> > ---
>> > v6:
>> > - Add Reviewed-by,
>> > - Reorder operations in ht16k33_led_probe() to ease future conversion
>> > to device properties,
>
>> > --- a/drivers/auxdisplay/ht16k33.c
>> > +++ b/drivers/auxdisplay/ht16k33.c
>> > @@ -425,6 +477,35 @@ static void ht16k33_seg14_update(struct work_struct
>> > *work)
>> > i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
>> > }
>> >
>> > +static int ht16k33_led_probe(struct device *dev, struct led_classdev *led,
>> > + unsigned int brightness)
>> > +{
>> > + struct led_init_data init_data = {};
>> > + struct device_node *node;
>> > + int err;
>> > +
>> > + /* The LED is optional */
>> > + node = of_get_child_by_name(dev->of_node, "led");
>> > + if (!node)
>> > + return 0;
>> > +
>> > + init_data.fwnode = of_fwnode_handle(node);
>> > + init_data.devicename = "auxdisplay";
>> > + init_data.devname_mandatory = true;
>> > +
>> > + led->brightness_set_blocking = ht16k33_brightness_set_blocking;
>> > + led->blink_set = ht16k33_blink_set;
>> > + led->flags = LED_CORE_SUSPENDRESUME;
>> > + led->brightness = brightness;
>> > + led->max_brightness = MAX_BRIGHTNESS;
>>
>> What do you think about adding a default trigger and making it 'backlight'?
>>
>> led->default_trigger = "blacklight";
>>
>> Or as an alternative, suggesting linux,default-trigger = "backlight" in the
>> docs? Since the led class won't respond to blank events by just making it's
>> function LED_FUNCTION_BACKLIGHT.
>>
>> led {
>> function = LED_FUNCTION_BACKLIGHT;
>> color = <LED_COLOR_ID_GREEN>;
>> linux,default-trigger = "backlight";
>> };
>
> The latter makes perfect sense to me. Will do.
Ack.
>
>> I noticed blanking is broken. The backlight device (or LED device with
>> backlight trigger) doens't get notified when the framebuffer is blanked
>> since
>> the driver doesn't implement fb_blank.
>>
>> Right now:
>>
>> echo 1 > /sys/class/graphics/fb0/blank
>> |
>> sh: write error: Invalid argument
>>
>> Due to:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/fbdev/core/fbmem.c?h=v5.15-rc3#n1078
>
> That's a pre-existing problem, righ? ;-)
Yes it is. The fix is easy and since we're making major driver changes now...
I needed the fix to properly test the changes we're making, so I thought we
might as well include it.
>
>> Something like this fixes it.
>>
>> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
>> index 89ee5b4b3dfc..0883d5252c81 100644
>> --- a/drivers/auxdisplay/ht16k33.c
>> +++ b/drivers/auxdisplay/ht16k33.c
>> @@ -346,6 +346,15 @@ static int ht16k33_mmap(struct fb_info *info, struct
>> vm_area_struct *vma)
>> return vm_map_pages_zero(vma, &pages, 1);
>> }
>>
>> +/*
>> + * Blank events will be passed to the backlight device (or the LED device
>> if
>> + * it's trigger is 'backlight') when we return 0 here.
>> + */
>> +static int ht16k33_blank(int blank, struct fb_info *info)
>> +{
>> + return 0;
>> +}
>> +
>> static const struct fb_ops ht16k33_fb_ops = {
>> .owner = THIS_MODULE,
>> .fb_read = fb_sys_read,
>> @@ -354,6 +363,7 @@ static const struct fb_ops ht16k33_fb_ops = {
>> .fb_copyarea = sys_copyarea,
>> .fb_imageblit = sys_imageblit,
>> .fb_mmap = ht16k33_mmap,
>> + .fb_blank = ht16k33_blank,
>> };
>>
>> /*
>>
>> Feel free to include (something like) this in the patch stack.
>
> Thanks, will do.
Ack.
>
>> > +
>> > + err = devm_led_classdev_register_ext(dev, led, &init_data);
>> > + if (err)
>> > + dev_err(dev, "Failed to register LED\n");
>>
>> You might want to call ht16k33_brightness_set(priv, brightness) here to get
>> a
>> know value into the display setup register (0x80).
>>
>> Right now if I enable hardware blinking and (soft)reboot my board it keeps
>> on
>> blinking even after a re-probe.
>
> I don't have that issue.
> Aha, ht16k33_seg_probe() calls ht16k33_brightness_set(), but
> ht16k33_fbdev_probe() doesn't. The latter should do that, too,
> when not using backwards compatibility mode.
Ack. I have hardware which uses the ht16k33 in dot matrix mode and I tested
both the backlight and led setup. I ran into this with the fbdev + led setup.
I noticed ht16k33_bl_update_status() is called in ht16k33_fbdev_probe()
before the fbdev device is registered. Which is fine right now, but in theory
the fbdev blank state can influence the backlight setting (nitpick since
the fbdev device is unblanked by default).
The point: Maybe ht16k33_brightness_set() (or ht16k33_bl_update_status() for
backlight device) should be called in one central place (i.e at the end of
the
main probe function).
>
>> > @@ -575,7 +660,7 @@ static int ht16k33_seg_probe(struct device *dev, struct
>> > ht16k33_priv *priv,
>> > struct ht16k33_seg *seg = &priv->seg;
>> > int err;
>> >
>> > - err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
>> > + err = ht16k33_brightness_set(priv, brightness);
>>
>> This looks like a bugfix for patch 17, maybe move this change there?
>
> Indeed. Bad rebase. Will move.
>
> Thanks a lot for your comments!
:)
- Robin
Hoi Robin,
On Mon, Oct 4, 2021 at 10:26 AM Robin van der Gracht <[email protected]> wrote:
> On 2021-10-01 17:51, Geert Uytterhoeven wrote:
> > On Thu, Sep 30, 2021 at 12:57 PM Robin van der Gracht <[email protected]>
> > wrote:
> >> On 2021-09-14 16:38, Geert Uytterhoeven wrote:
> >> > Instantiate a single LED based on the "led" subnode in DT.
> >> > This allows the user to control display brightness and blinking (backed
> >> > by hardware support) through the LED class API and triggers, and exposes
> >> > the display color. The LED will be named
> >> > "auxdisplay:<color>:<function>".
> >> >
> >> > When running in dot-matrix mode and if no "led" subnode is found, the
> >> > driver falls back to the traditional backlight mode, to preserve
> >> > backwards compatibility.
> >> >
> >> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> >> > +
> >> > + err = devm_led_classdev_register_ext(dev, led, &init_data);
> >> > + if (err)
> >> > + dev_err(dev, "Failed to register LED\n");
> >>
> >> You might want to call ht16k33_brightness_set(priv, brightness) here to get
> >> a
> >> know value into the display setup register (0x80).
> >>
> >> Right now if I enable hardware blinking and (soft)reboot my board it keeps
> >> on
> >> blinking even after a re-probe.
> >
> > I don't have that issue.
> > Aha, ht16k33_seg_probe() calls ht16k33_brightness_set(), but
> > ht16k33_fbdev_probe() doesn't. The latter should do that, too,
> > when not using backwards compatibility mode.
>
> Ack. I have hardware which uses the ht16k33 in dot matrix mode and I tested
> both the backlight and led setup. I ran into this with the fbdev + led setup.
>
> I noticed ht16k33_bl_update_status() is called in ht16k33_fbdev_probe()
> before the fbdev device is registered. Which is fine right now, but in theory
> the fbdev blank state can influence the backlight setting (nitpick since
> the fbdev device is unblanked by default).
>
> The point: Maybe ht16k33_brightness_set() (or ht16k33_bl_update_status() for
> backlight device) should be called in one central place (i.e at the end of
> the
> main probe function).
That would mean the main function need to know more about which mode
is being used, so I think it's better to leave it to the individual display
sub-drivers.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds