Add a new initial driver for Maxim MAX6958/6959 chips.
While developing that driver I realised that there is a lot
of duplication between ht16k33 and a new one. Hence set of
cleanups and refactorings.
Note, the new driver has minimum support of the hardware and
I have plans to cover more features in the future.
Andy Shevchenko (15):
auxdisplay: img-ascii-lcd: Make container_of() no-op for struct
linedisp
auxdisplay: linedisp: Free allocated resources in ->release()
auxdisplay: linedisp: Use unique number for id
auxdisplay: linedisp: Unshadow error codes in ->store()
auxdisplay: linedisp: Add missing header(s)
auxdisplay: linedisp: Move exported symbols to a namespace
auxdisplay: linedisp: Group line display drivers together
auxdisplay: linedisp: Provide struct linedisp_ops for future extension
auxdisplay: linedisp: Add support for overriding character mapping
auxdisplay: linedisp: Provide a small buffer in the struct linedisp
auxdisplay: ht16k33: Move ht16k33_linedisp_ops down
auxdisplay: ht16k33: Switch to use line display character mapping
auxdisplay: ht16k33: Use buffer from struct linedisp
dt-bindings: auxdisplay: Add Maxim MAX6958/6959
auxdisplay: Add driver for MAX695x 7-segment LED controllers
.../bindings/auxdisplay/maxim,max6959.yaml | 35 +++
drivers/auxdisplay/Kconfig | 40 ++--
drivers/auxdisplay/Makefile | 13 +-
drivers/auxdisplay/ht16k33.c | 145 +++++--------
drivers/auxdisplay/img-ascii-lcd.c | 24 ++-
drivers/auxdisplay/line-display.c | 162 ++++++++++++--
drivers/auxdisplay/line-display.h | 57 ++++-
drivers/auxdisplay/max6959.c | 200 ++++++++++++++++++
8 files changed, 530 insertions(+), 146 deletions(-)
create mode 100644 Documentation/devicetree/bindings/auxdisplay/maxim,max6959.yaml
create mode 100644 drivers/auxdisplay/max6959.c
--
2.43.0.rc1.1.gbec44491f096
Move embedded struct linedisp member to make container_of() no-op.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/img-ascii-lcd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/auxdisplay/img-ascii-lcd.c b/drivers/auxdisplay/img-ascii-lcd.c
index 56efda0740fb..09014ada38bd 100644
--- a/drivers/auxdisplay/img-ascii-lcd.c
+++ b/drivers/auxdisplay/img-ascii-lcd.c
@@ -32,21 +32,21 @@ struct img_ascii_lcd_config {
/**
* struct img_ascii_lcd_ctx - Private data structure
+ * @linedisp: line display structure
* @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
- * @linedisp: line display structure
* @curr: the string currently displayed on the LCD
*/
struct img_ascii_lcd_ctx {
+ struct linedisp linedisp;
union {
void __iomem *base;
struct regmap *regmap;
};
u32 offset;
const struct img_ascii_lcd_config *cfg;
- struct linedisp linedisp;
char curr[] __aligned(8);
};
--
2.43.0.rc1.1.gbec44491f096
While there is no issue currently with the resources allocation,
the code may still be made more robust by deallocating message
in the ->release() callback.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/line-display.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 03e7f104aa1a..310e9bfb41ae 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -188,8 +188,16 @@ static struct attribute *linedisp_attrs[] = {
};
ATTRIBUTE_GROUPS(linedisp);
+static void linedisp_release(struct device *dev)
+{
+ struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+
+ kfree(linedisp->message);
+}
+
static const struct device_type linedisp_type = {
.groups = linedisp_groups,
+ .release = linedisp_release,
};
/**
@@ -253,7 +261,6 @@ 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);
--
2.43.0.rc1.1.gbec44491f096
For better usability group the line display drivers together in Kconfig
and Makefile.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/Kconfig | 32 ++++++++++++++++----------------
drivers/auxdisplay/Makefile | 12 ++++++------
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index d944d5298eca..a34a9a52158f 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -25,12 +25,6 @@ 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
@@ -52,6 +46,16 @@ config HD44780
kernel and started at boot.
If you don't understand what all this is about, say N.
+config LCD2S
+ tristate "lcd2s 20x4 character display over I2C console"
+ depends on I2C
+ select CHARLCD
+ help
+ This is a driver that lets you use the lcd2s 20x4 character display
+ from Modtronix engineering as a console output device. The display
+ is a simple single color character display. You have to connect it
+ to an I2C bus.
+
config KS0108
tristate "KS0108 LCD Controller"
depends on PARPORT_PC
@@ -153,6 +157,12 @@ config CFAG12864B_RATE
If you compile this as a module, you can still override this
value using the module parameters.
+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 IMG_ASCII_LCD
tristate "Imagination Technologies ASCII LCD Display"
depends on HAS_IOMEM
@@ -177,16 +187,6 @@ config HT16K33
Say yes here to add support for Holtek HT16K33, RAM mapping 16*8
LED controller driver with keyscan.
-config LCD2S
- tristate "lcd2s 20x4 character display over I2C console"
- depends on I2C
- select CHARLCD
- help
- This is a driver that lets you use the lcd2s 20x4 character display
- from Modtronix engineering as a console output device. The display
- is a simple single color character display. You have to connect it
- to an I2C bus.
-
config ARM_CHARLCD
bool "ARM Ltd. Character LCD Driver"
depends on PLAT_VERSATILE
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index 6968ed4d3f0a..43bad850481c 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -5,12 +5,12 @@
obj-$(CONFIG_CHARLCD) += charlcd.o
obj-$(CONFIG_HD44780_COMMON) += hd44780_common.o
-obj-$(CONFIG_ARM_CHARLCD) += arm-charlcd.o
+obj-$(CONFIG_HD44780) += hd44780.o
+obj-$(CONFIG_LCD2S) += lcd2s.o
obj-$(CONFIG_KS0108) += ks0108.o
obj-$(CONFIG_CFAG12864B) += cfag12864b.o cfag12864bfb.o
-obj-$(CONFIG_IMG_ASCII_LCD) += img-ascii-lcd.o
-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
+obj-$(CONFIG_IMG_ASCII_LCD) += img-ascii-lcd.o
+obj-$(CONFIG_HT16K33) += ht16k33.o
+obj-$(CONFIG_ARM_CHARLCD) += arm-charlcd.o
+obj-$(CONFIG_PARPORT_PANEL) += panel.o
--
2.43.0.rc1.1.gbec44491f096
Currently the line display library doesn't scale in case we want to
provide more operations. Prepare the library to take a newly created
struct linedisp_ops that scales.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/ht16k33.c | 7 +++++--
drivers/auxdisplay/img-ascii-lcd.c | 19 ++++++++++++-------
drivers/auxdisplay/line-display.c | 11 +++++------
drivers/auxdisplay/line-display.h | 17 +++++++++++++----
4 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index c6a42c5c128f..0cdf3fbdf81e 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -448,6 +448,10 @@ static void ht16k33_linedisp_update(struct linedisp *linedisp)
schedule_delayed_work(&priv->work, 0);
}
+static const struct linedisp_ops ht16k33_linedisp_ops = {
+ .update = ht16k33_linedisp_update,
+};
+
static void ht16k33_seg7_update(struct work_struct *work)
{
struct ht16k33_priv *priv = container_of(work, struct ht16k33_priv,
@@ -696,8 +700,7 @@ static int ht16k33_seg_probe(struct device *dev, struct ht16k33_priv *priv,
if (err)
return err;
- err = linedisp_register(&seg->linedisp, dev, 4, seg->curr,
- ht16k33_linedisp_update);
+ err = linedisp_register(&seg->linedisp, dev, 4, seg->curr, &ht16k33_linedisp_ops);
if (err)
goto err_remove_map_file;
diff --git a/drivers/auxdisplay/img-ascii-lcd.c b/drivers/auxdisplay/img-ascii-lcd.c
index c571e54d9eb5..c0ec7d9a1c62 100644
--- a/drivers/auxdisplay/img-ascii-lcd.c
+++ b/drivers/auxdisplay/img-ascii-lcd.c
@@ -22,12 +22,12 @@ struct img_ascii_lcd_ctx;
* struct img_ascii_lcd_config - Configuration information about an LCD model
* @num_chars: the number of characters the LCD can display
* @external_regmap: true if registers are in a system controller, else false
- * @update: function called to update the LCD
+ * @ops: character line display operations
*/
struct img_ascii_lcd_config {
unsigned int num_chars;
bool external_regmap;
- void (*update)(struct linedisp *linedisp);
+ const struct linedisp_ops ops;
};
/**
@@ -75,7 +75,9 @@ static void boston_update(struct linedisp *linedisp)
static struct img_ascii_lcd_config boston_config = {
.num_chars = 8,
- .update = boston_update,
+ .ops = {
+ .update = boston_update,
+ },
};
/*
@@ -103,7 +105,9 @@ static void malta_update(struct linedisp *linedisp)
static struct img_ascii_lcd_config malta_config = {
.num_chars = 8,
.external_regmap = true,
- .update = malta_update,
+ .ops = {
+ .update = malta_update,
+ },
};
/*
@@ -203,7 +207,9 @@ static void sead3_update(struct linedisp *linedisp)
static struct img_ascii_lcd_config sead3_config = {
.num_chars = 16,
.external_regmap = true,
- .update = sead3_update,
+ .ops = {
+ .update = sead3_update,
+ },
};
static const struct of_device_id img_ascii_lcd_matches[] = {
@@ -247,8 +253,7 @@ static int img_ascii_lcd_probe(struct platform_device *pdev)
return PTR_ERR(ctx->base);
}
- err = linedisp_register(&ctx->linedisp, dev, cfg->num_chars, ctx->curr,
- cfg->update);
+ err = linedisp_register(&ctx->linedisp, dev, cfg->num_chars, ctx->curr, &cfg->ops);
if (err)
return err;
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 851b2c0f9443..62e8a911bb12 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -50,7 +50,7 @@ static void linedisp_scroll(struct timer_list *t)
}
/* update the display */
- linedisp->update(linedisp);
+ linedisp->ops->update(linedisp);
/* move on to the next character */
linedisp->scroll_pos++;
@@ -94,7 +94,7 @@ static int linedisp_display(struct linedisp *linedisp, const char *msg,
linedisp->message = NULL;
linedisp->message_len = 0;
memset(linedisp->buf, ' ', linedisp->num_chars);
- linedisp->update(linedisp);
+ linedisp->ops->update(linedisp);
return 0;
}
@@ -216,20 +216,19 @@ static const struct device_type linedisp_type = {
* @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!
+ * @ops: character line display operations
*
* 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))
+ unsigned int num_chars, char *buf, const struct linedisp_ops *ops)
{
int err;
memset(linedisp, 0, sizeof(*linedisp));
linedisp->dev.parent = parent;
linedisp->dev.type = &linedisp_type;
- linedisp->update = update;
+ linedisp->ops = ops;
linedisp->buf = buf;
linedisp->num_chars = num_chars;
linedisp->scroll_rate = DEFAULT_SCROLL_RATE;
diff --git a/drivers/auxdisplay/line-display.h b/drivers/auxdisplay/line-display.h
index d72c1899dc50..a4f0d1bf5848 100644
--- a/drivers/auxdisplay/line-display.h
+++ b/drivers/auxdisplay/line-display.h
@@ -14,12 +14,22 @@
#include <linux/device.h>
#include <linux/timer_types.h>
+struct linedisp;
+
+/**
+ * struct linedisp_ops - character line display operations
+ * @update: Function called to update the display. This must not sleep!
+ */
+struct linedisp_ops {
+ void (*update)(struct linedisp *linedisp);
+};
+
/**
* struct linedisp - character line display private data structure
* @dev: the line display device
* @id: instance id of this display
* @timer: timer used to implement scrolling
- * @update: function called to update the display
+ * @ops: character line display operations
* @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
@@ -31,7 +41,7 @@ struct linedisp {
struct device dev;
unsigned int id;
struct timer_list timer;
- void (*update)(struct linedisp *linedisp);
+ const struct linedisp_ops *ops;
char *buf;
char *message;
unsigned int num_chars;
@@ -41,8 +51,7 @@ struct linedisp {
};
int linedisp_register(struct linedisp *linedisp, struct device *parent,
- unsigned int num_chars, char *buf,
- void (*update)(struct linedisp *linedisp));
+ unsigned int num_chars, char *buf, const struct linedisp_ops *ops);
void linedisp_unregister(struct linedisp *linedisp);
#endif /* LINEDISP_H */
--
2.43.0.rc1.1.gbec44491f096
On Thu, Feb 08, 2024 at 06:58:43PM +0200, Andy Shevchenko wrote:
> Add a new initial driver for Maxim MAX6958/6959 chips.
> While developing that driver I realised that there is a lot
> of duplication between ht16k33 and a new one. Hence set of
> cleanups and refactorings.
>
> Note, the new driver has minimum support of the hardware and
> I have plans to cover more features in the future.
>
> Andy Shevchenko (15):
> auxdisplay: img-ascii-lcd: Make container_of() no-op for struct
> linedisp
> auxdisplay: linedisp: Free allocated resources in ->release()
> auxdisplay: linedisp: Use unique number for id
> auxdisplay: linedisp: Unshadow error codes in ->store()
> auxdisplay: linedisp: Add missing header(s)
> auxdisplay: linedisp: Move exported symbols to a namespace
> auxdisplay: linedisp: Group line display drivers together
> auxdisplay: linedisp: Provide struct linedisp_ops for future extension
> auxdisplay: linedisp: Add support for overriding character mapping
> auxdisplay: linedisp: Provide a small buffer in the struct linedisp
> auxdisplay: ht16k33: Move ht16k33_linedisp_ops down
> auxdisplay: ht16k33: Switch to use line display character mapping
> auxdisplay: ht16k33: Use buffer from struct linedisp
> dt-bindings: auxdisplay: Add Maxim MAX6958/6959
> auxdisplay: Add driver for MAX695x 7-segment LED controllers
Not all of these patches have made their way to the lists FYI:
2024-02-08 16:58 Andy Shevchenko [this message]
2024-02-08 16:58 ` [PATCH v1 01/15] auxdisplay: img-ascii-lcd: Make container_of() no-op for struct linedisp Andy Shevchenko
2024-02-08 16:58 ` [PATCH v1 02/15] auxdisplay: linedisp: Free allocated resources in ->release() Andy Shevchenko
2024-02-08 16:58 ` [PATCH v1 03/15] auxdisplay: linedisp: Use unique number for id Andy Shevchenko
2024-02-08 16:58 ` [PATCH v1 06/15] auxdisplay: linedisp: Move exported symbols to a namespace Andy Shevchenko
2024-02-08 16:58 ` [PATCH v1 07/15] auxdisplay: linedisp: Group line display drivers together Andy Shevchenko
2024-02-08 16:58 ` [PATCH v1 08/15] auxdisplay: linedisp: Provide struct linedisp_ops for future extension Andy Shevchenko
2024-02-08 16:58 ` [PATCH v1 09/15] auxdisplay: linedisp: Add support for overriding character mapping Andy Shevchenko
2024-02-08 16:58 ` [PATCH v1 10/15] auxdisplay: linedisp: Provide a small buffer in the struct linedisp Andy Shevchenko
2024-02-08 16:58 ` [PATCH v1 14/15] dt-bindings: auxdisplay: Add Maxim MAX6958/6959 Andy Shevchenko
2024-02-08 16:58 ` [PATCH v1 15/15] auxdisplay: Add driver for MAX695x 7-segment LED controllers Andy Shevchenko
https://lore.kernel.org/all/[email protected]/
Cheers,
Conor.
On Thu, Feb 8, 2024 at 6:52 PM Conor Dooley <[email protected]> wrote:
> On Thu, Feb 08, 2024 at 06:58:43PM +0200, Andy Shevchenko wrote:
> > Add a new initial driver for Maxim MAX6958/6959 chips.
> > While developing that driver I realised that there is a lot
> > of duplication between ht16k33 and a new one. Hence set of
> > cleanups and refactorings.
> >
> > Note, the new driver has minimum support of the hardware and
> > I have plans to cover more features in the future.
> >
> > Andy Shevchenko (15):
> > auxdisplay: img-ascii-lcd: Make container_of() no-op for struct
> > linedisp
> > auxdisplay: linedisp: Free allocated resources in ->release()
> > auxdisplay: linedisp: Use unique number for id
> > auxdisplay: linedisp: Unshadow error codes in ->store()
> > auxdisplay: linedisp: Add missing header(s)
> > auxdisplay: linedisp: Move exported symbols to a namespace
> > auxdisplay: linedisp: Group line display drivers together
> > auxdisplay: linedisp: Provide struct linedisp_ops for future extension
> > auxdisplay: linedisp: Add support for overriding character mapping
> > auxdisplay: linedisp: Provide a small buffer in the struct linedisp
> > auxdisplay: ht16k33: Move ht16k33_linedisp_ops down
> > auxdisplay: ht16k33: Switch to use line display character mapping
> > auxdisplay: ht16k33: Use buffer from struct linedisp
> > dt-bindings: auxdisplay: Add Maxim MAX6958/6959
> > auxdisplay: Add driver for MAX695x 7-segment LED controllers
>
> Not all of these patches have made their way to the lists FYI:
> 2024-02-08 16:58 Andy Shevchenko [this message]
> 2024-02-08 16:58 ` [PATCH v1 01/15] auxdisplay: img-ascii-lcd: Make container_of() no-op for struct linedisp Andy Shevchenko
> 2024-02-08 16:58 ` [PATCH v1 02/15] auxdisplay: linedisp: Free allocated resources in ->release() Andy Shevchenko
> 2024-02-08 16:58 ` [PATCH v1 03/15] auxdisplay: linedisp: Use unique number for id Andy Shevchenko
> 2024-02-08 16:58 ` [PATCH v1 06/15] auxdisplay: linedisp: Move exported symbols to a namespace Andy Shevchenko
> 2024-02-08 16:58 ` [PATCH v1 07/15] auxdisplay: linedisp: Group line display drivers together Andy Shevchenko
> 2024-02-08 16:58 ` [PATCH v1 08/15] auxdisplay: linedisp: Provide struct linedisp_ops for future extension Andy Shevchenko
> 2024-02-08 16:58 ` [PATCH v1 09/15] auxdisplay: linedisp: Add support for overriding character mapping Andy Shevchenko
> 2024-02-08 16:58 ` [PATCH v1 10/15] auxdisplay: linedisp: Provide a small buffer in the struct linedisp Andy Shevchenko
> 2024-02-08 16:58 ` [PATCH v1 14/15] dt-bindings: auxdisplay: Add Maxim MAX6958/6959 Andy Shevchenko
> 2024-02-08 16:58 ` [PATCH v1 15/15] auxdisplay: Add driver for MAX695x 7-segment LED controllers Andy Shevchenko
> https://lore.kernel.org/all/[email protected]/
Same for my mailbox.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
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
Add initial driver for the MAX6958 and MAX6959 7-segment LED
controllers.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/Kconfig | 14 +++
drivers/auxdisplay/Makefile | 1 +
drivers/auxdisplay/max6959.c | 200 +++++++++++++++++++++++++++++++++++
3 files changed, 215 insertions(+)
create mode 100644 drivers/auxdisplay/max6959.c
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index a34a9a52158f..079d58bb0293 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -187,6 +187,20 @@ config HT16K33
Say yes here to add support for Holtek HT16K33, RAM mapping 16*8
LED controller driver with keyscan.
+config MAX6959
+ tristate "Maxim MAX6958/6959 7-segment LED controller with keyscan"
+ depends on I2C
+ select REGMAP_I2C
+ select LINEDISP
+ help
+ If you say yes here you get support for the following Maxim chips
+ (I2C 7-segment LED display controller with keyscan):
+ - MAX6958
+ - MAX6959 (debounce support)
+
+ This driver can also be built as a module. If so, the module
+ will be called max6959.
+
config ARM_CHARLCD
bool "ARM Ltd. Character LCD Driver"
depends on PLAT_VERSATILE
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index 43bad850481c..f62a258809ef 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -12,5 +12,6 @@ obj-$(CONFIG_CFAG12864B) += cfag12864b.o cfag12864bfb.o
obj-$(CONFIG_LINEDISP) += line-display.o
obj-$(CONFIG_IMG_ASCII_LCD) += img-ascii-lcd.o
obj-$(CONFIG_HT16K33) += ht16k33.o
+obj-$(CONFIG_MAX6959) += max6959.o
obj-$(CONFIG_ARM_CHARLCD) += arm-charlcd.o
obj-$(CONFIG_PARPORT_PANEL) += panel.o
diff --git a/drivers/auxdisplay/max6959.c b/drivers/auxdisplay/max6959.c
new file mode 100644
index 000000000000..0c5cbd16c3fe
--- /dev/null
+++ b/drivers/auxdisplay/max6959.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MAX6958/6959 7-segment LED display controller with keyscan
+ * Datasheet:
+ * https://www.analog.com/media/en/technical-documentation/data-sheets/MAX6958-MAX6959.pdf
+ *
+ * Copyright (c) 2024, Intel Corporation.
+ * Author: Andy Shevchenko <[email protected]>
+ */
+#include <linux/array_size.h>
+#include <linux/bitrev.h>
+#include <linux/bits.h>
+#include <linux/container_of.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+#include <linux/map_to_7segment.h>
+
+#include "line-display.h"
+
+/* Registers */
+#define REG_DECODE_MODE 0x01
+#define REG_INTENSITY 0x02
+#define REG_SCAN_LIMIT 0x03
+#define REG_CONFIGURATION 0x04
+#define REG_CONFIGURATION_S_BIT BIT(0)
+
+#define REG_DIGIT(x) (0x20 + (x))
+#define REG_DIGIT0 0x20
+#define REG_DIGIT1 0x21
+#define REG_DIGIT2 0x22
+#define REG_DIGIT3 0x23
+
+#define REG_SEGMENTS 0x24
+#define REG_MAX REG_SEGMENTS
+
+/* Defines */
+#define MIN_BRIGHTNESS 0x01
+#define MAX_BRIGHTNESS 0x40
+
+struct max6959_priv {
+ struct linedisp linedisp;
+
+ struct delayed_work work;
+
+ struct regmap *regmap;
+};
+
+static void max6959_disp_update(struct work_struct *work)
+{
+ struct max6959_priv *priv = container_of(work, struct max6959_priv, work.work);
+ struct linedisp *linedisp = &priv->linedisp;
+ struct linedisp_map *map = linedisp->map;
+ char *s = linedisp->curr;
+ u8 buf[4];
+
+ /* Map segments according to datasheet */
+ buf[0] = bitrev8(map_to_seg7(&map->map.seg7, *s++)) >> 1;
+ buf[1] = bitrev8(map_to_seg7(&map->map.seg7, *s++)) >> 1;
+ buf[2] = bitrev8(map_to_seg7(&map->map.seg7, *s++)) >> 1;
+ buf[3] = bitrev8(map_to_seg7(&map->map.seg7, *s++)) >> 1;
+
+ regmap_bulk_write(priv->regmap, REG_DIGIT(0), buf, ARRAY_SIZE(buf));
+}
+
+static int max6959_linedisp_get_map_type(struct linedisp *linedisp)
+{
+ struct max6959_priv *priv = container_of(linedisp, struct max6959_priv, linedisp);
+
+ INIT_DELAYED_WORK(&priv->work, max6959_disp_update);
+ return LINEDISP_MAP_SEG7;
+}
+
+static void max6959_linedisp_update(struct linedisp *linedisp)
+{
+ struct max6959_priv *priv = container_of(linedisp, struct max6959_priv, linedisp);
+
+ schedule_delayed_work(&priv->work, 0);
+}
+
+static const struct linedisp_ops max6959_linedisp_ops = {
+ .get_map_type = max6959_linedisp_get_map_type,
+ .update = max6959_linedisp_update,
+};
+
+static int max6959_enable(struct max6959_priv *priv, bool enable)
+{
+ u8 mask = REG_CONFIGURATION_S_BIT;
+ u8 value = enable ? mask : 0;
+
+ return regmap_update_bits(priv->regmap, REG_CONFIGURATION, mask, value);
+}
+
+static void max6959_power_off(void *priv)
+{
+ max6959_enable(priv, false);
+}
+
+static int max6959_power_on(struct max6959_priv *priv)
+{
+ struct device *dev = regmap_get_device(priv->regmap);
+ int ret;
+
+ ret = max6959_enable(priv, true);
+ if (ret)
+ return ret;
+
+ return devm_add_action_or_reset(dev, max6959_power_off, priv);
+}
+
+static const struct regmap_config max6959_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = REG_MAX,
+ .cache_type = REGCACHE_MAPLE,
+};
+
+static int max6959_i2c_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct max6959_priv *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->regmap = devm_regmap_init_i2c(client, &max6959_regmap_config);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
+
+ ret = max6959_power_on(priv);
+ if (ret)
+ return ret;
+
+ ret = linedisp_register(&priv->linedisp, dev, 4, NULL, &max6959_linedisp_ops);
+ if (ret)
+ return ret;
+
+ i2c_set_clientdata(client, priv);
+
+ return 0;
+}
+
+static void max6959_i2c_remove(struct i2c_client *client)
+{
+ struct max6959_priv *priv = i2c_get_clientdata(client);
+
+ cancel_delayed_work_sync(&priv->work);
+ linedisp_unregister(&priv->linedisp);
+}
+
+static int max6959_suspend(struct device *dev)
+{
+ return max6959_enable(dev_get_drvdata(dev), false);
+}
+
+static int max6959_resume(struct device *dev)
+{
+ return max6959_enable(dev_get_drvdata(dev), true);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(max6959_pm_ops, max6959_suspend, max6959_resume);
+
+static const struct i2c_device_id max6959_i2c_id[] = {
+ { "max6959" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max6959_i2c_id);
+
+static const struct of_device_id max6959_of_table[] = {
+ { .compatible = "maxim,max6959" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max6959_of_table);
+
+static struct i2c_driver max6959_i2c_driver = {
+ .driver = {
+ .name = "max6959",
+ .pm = pm_sleep_ptr(&max6959_pm_ops),
+ .of_match_table = max6959_of_table,
+ },
+ .probe = max6959_i2c_probe,
+ .remove = max6959_i2c_remove,
+ .id_table = max6959_i2c_id,
+};
+module_i2c_driver(max6959_i2c_driver);
+
+MODULE_DESCRIPTION("MAX6958/6959 7-segment LED controller with keyscan");
+MODULE_AUTHOR("Andy Shevchenko <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LINEDISP);
--
2.43.0.rc1.1.gbec44491f096
struct linedips embedds a small buffer for the string that we may reuse.
Update the driver accordingly.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/ht16k33.c | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index b104f08252dd..08cc05b9d216 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -85,11 +85,6 @@ struct ht16k33_fbdev {
uint8_t *cache;
};
-struct ht16k33_seg {
- struct linedisp linedisp;
- char curr[4];
-};
-
struct ht16k33_priv {
struct i2c_client *client;
struct delayed_work work;
@@ -97,7 +92,7 @@ struct ht16k33_priv {
struct ht16k33_keypad keypad;
union {
struct ht16k33_fbdev fbdev;
- struct ht16k33_seg seg;
+ struct linedisp linedisp;
};
enum display_type type;
uint8_t blink;
@@ -412,10 +407,9 @@ 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;
- struct linedisp *linedisp = &seg->linedisp;
+ struct linedisp *linedisp = &priv->linedisp;
struct linedisp_map *map = linedisp->map;
- char *s = seg->curr;
+ char *s = linedisp->curr;
uint8_t buf[9];
buf[0] = map_to_seg7(&map->map.seg7, *s++);
@@ -435,10 +429,9 @@ 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;
- struct linedisp *linedisp = &seg->linedisp;
+ struct linedisp *linedisp = &priv->linedisp;
struct linedisp_map *map = linedisp->map;
- char *s = seg->curr;
+ char *s = linedisp->curr;
uint8_t buf[8];
put_unaligned_le16(map_to_seg14(&map->map.seg14, *s++), buf + 0);
@@ -451,8 +444,7 @@ static void ht16k33_seg14_update(struct work_struct *work)
static int ht16k33_linedisp_get_map_type(struct linedisp *linedisp)
{
- struct ht16k33_priv *priv = container_of(linedisp, struct ht16k33_priv,
- seg.linedisp);
+ struct ht16k33_priv *priv = container_of(linedisp, struct ht16k33_priv, linedisp);
switch (priv->type) {
case DISP_MATRIX:
@@ -471,8 +463,7 @@ static int ht16k33_linedisp_get_map_type(struct linedisp *linedisp)
static void ht16k33_linedisp_update(struct linedisp *linedisp)
{
- struct ht16k33_priv *priv = container_of(linedisp, struct ht16k33_priv,
- seg.linedisp);
+ struct ht16k33_priv *priv = container_of(linedisp, struct ht16k33_priv, linedisp);
schedule_delayed_work(&priv->work, 0);
}
@@ -663,14 +654,13 @@ static int ht16k33_fbdev_probe(struct device *dev, struct ht16k33_priv *priv,
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, brightness);
if (err)
return err;
- return linedisp_register(&seg->linedisp, dev, 4, seg->curr, &ht16k33_linedisp_ops);
+ return linedisp_register(&priv->linedisp, dev, 4, NULL, &ht16k33_linedisp_ops);
}
static int ht16k33_probe(struct i2c_client *client)
@@ -754,7 +744,7 @@ static void ht16k33_remove(struct i2c_client *client)
case DISP_QUAD_7SEG:
case DISP_QUAD_14SEG:
- linedisp_unregister(&priv->seg.linedisp);
+ linedisp_unregister(&priv->linedisp);
break;
}
}
--
2.43.0.rc1.1.gbec44491f096
We will need the update functions to be defined before
ht16k33_linedisp_ops. Move the latter down in the code.
No functional change intended.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/ht16k33.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 0cdf3fbdf81e..75c4a8d31642 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -440,18 +440,6 @@ 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 const struct linedisp_ops ht16k33_linedisp_ops = {
- .update = ht16k33_linedisp_update,
-};
-
static void ht16k33_seg7_update(struct work_struct *work)
{
struct ht16k33_priv *priv = container_of(work, struct ht16k33_priv,
@@ -489,6 +477,18 @@ static void ht16k33_seg14_update(struct work_struct *work)
i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
}
+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 const struct linedisp_ops ht16k33_linedisp_ops = {
+ .update = ht16k33_linedisp_update,
+};
+
static int ht16k33_led_probe(struct device *dev, struct led_classdev *led,
unsigned int brightness)
{
--
2.43.0.rc1.1.gbec44491f096
On Thu, Feb 08, 2024 at 07:10:40PM +0100, Geert Uytterhoeven wrote:
> On Thu, Feb 8, 2024 at 6:52 PM Conor Dooley <[email protected]> wrote:
> > On Thu, Feb 08, 2024 at 06:58:43PM +0200, Andy Shevchenko wrote:
> > > Add a new initial driver for Maxim MAX6958/6959 chips.
> > > While developing that driver I realised that there is a lot
> > > of duplication between ht16k33 and a new one. Hence set of
> > > cleanups and refactorings.
> > >
> > > Note, the new driver has minimum support of the hardware and
> > > I have plans to cover more features in the future.
> > >
> > > Andy Shevchenko (15):
> > > auxdisplay: img-ascii-lcd: Make container_of() no-op for struct
> > > linedisp
> > > auxdisplay: linedisp: Free allocated resources in ->release()
> > > auxdisplay: linedisp: Use unique number for id
> > > auxdisplay: linedisp: Unshadow error codes in ->store()
> > > auxdisplay: linedisp: Add missing header(s)
> > > auxdisplay: linedisp: Move exported symbols to a namespace
> > > auxdisplay: linedisp: Group line display drivers together
> > > auxdisplay: linedisp: Provide struct linedisp_ops for future extension
> > > auxdisplay: linedisp: Add support for overriding character mapping
> > > auxdisplay: linedisp: Provide a small buffer in the struct linedisp
> > > auxdisplay: ht16k33: Move ht16k33_linedisp_ops down
> > > auxdisplay: ht16k33: Switch to use line display character mapping
> > > auxdisplay: ht16k33: Use buffer from struct linedisp
> > > dt-bindings: auxdisplay: Add Maxim MAX6958/6959
> > > auxdisplay: Add driver for MAX695x 7-segment LED controllers
> >
> > Not all of these patches have made their way to the lists FYI:
> > 2024-02-08 16:58 Andy Shevchenko [this message]
> > 2024-02-08 16:58 ` [PATCH v1 01/15] auxdisplay: img-ascii-lcd: Make container_of() no-op for struct linedisp Andy Shevchenko
> > 2024-02-08 16:58 ` [PATCH v1 02/15] auxdisplay: linedisp: Free allocated resources in ->release() Andy Shevchenko
> > 2024-02-08 16:58 ` [PATCH v1 03/15] auxdisplay: linedisp: Use unique number for id Andy Shevchenko
> > 2024-02-08 16:58 ` [PATCH v1 06/15] auxdisplay: linedisp: Move exported symbols to a namespace Andy Shevchenko
> > 2024-02-08 16:58 ` [PATCH v1 07/15] auxdisplay: linedisp: Group line display drivers together Andy Shevchenko
> > 2024-02-08 16:58 ` [PATCH v1 08/15] auxdisplay: linedisp: Provide struct linedisp_ops for future extension Andy Shevchenko
> > 2024-02-08 16:58 ` [PATCH v1 09/15] auxdisplay: linedisp: Add support for overriding character mapping Andy Shevchenko
> > 2024-02-08 16:58 ` [PATCH v1 10/15] auxdisplay: linedisp: Provide a small buffer in the struct linedisp Andy Shevchenko
> > 2024-02-08 16:58 ` [PATCH v1 14/15] dt-bindings: auxdisplay: Add Maxim MAX6958/6959 Andy Shevchenko
> > 2024-02-08 16:58 ` [PATCH v1 15/15] auxdisplay: Add driver for MAX695x 7-segment LED controllers Andy Shevchenko
> > https://lore.kernel.org/all/[email protected]/
>
> Same for my mailbox.
I just resent it, hopefully without missing parts now.
--
With Best Regards,
Andy Shevchenko
Avoid unnecessary pollution of the global symbol namespace by
moving library functions in to a specific namespace and import
that into the drivers that make use of the functions.
For more info: https://lwn.net/Articles/760045/
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/ht16k33.c | 1 +
drivers/auxdisplay/img-ascii-lcd.c | 1 +
drivers/auxdisplay/line-display.c | 4 ++--
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index a90430b7d07b..c6a42c5c128f 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -831,4 +831,5 @@ module_i2c_driver(ht16k33_driver);
MODULE_DESCRIPTION("Holtek HT16K33 driver");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LINEDISP);
MODULE_AUTHOR("Robin van der Gracht <[email protected]>");
diff --git a/drivers/auxdisplay/img-ascii-lcd.c b/drivers/auxdisplay/img-ascii-lcd.c
index 09014ada38bd..c571e54d9eb5 100644
--- a/drivers/auxdisplay/img-ascii-lcd.c
+++ b/drivers/auxdisplay/img-ascii-lcd.c
@@ -298,3 +298,4 @@ module_platform_driver(img_ascii_lcd_driver);
MODULE_DESCRIPTION("Imagination Technologies ASCII LCD Display");
MODULE_AUTHOR("Paul Burton <[email protected]>");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LINEDISP);
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 6b3d25e20eeb..851b2c0f9443 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -263,7 +263,7 @@ int linedisp_register(struct linedisp *linedisp, struct device *parent,
put_device(&linedisp->dev);
return err;
}
-EXPORT_SYMBOL_GPL(linedisp_register);
+EXPORT_SYMBOL_NS_GPL(linedisp_register, LINEDISP);
/**
* linedisp_unregister - unregister a character line display
@@ -276,6 +276,6 @@ void linedisp_unregister(struct linedisp *linedisp)
del_timer_sync(&linedisp->timer);
put_device(&linedisp->dev);
}
-EXPORT_SYMBOL_GPL(linedisp_unregister);
+EXPORT_SYMBOL_NS_GPL(linedisp_unregister, LINEDISP);
MODULE_LICENSE("GPL");
--
2.43.0.rc1.1.gbec44491f096
There is already the driver using character mapping table for
7 or 14 segment display. It is possible to override it. Make
the similar in the line display library to allow other drivers
to utilise the same functionality.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/line-display.c | 111 +++++++++++++++++++++++++++++-
drivers/auxdisplay/line-display.h | 31 +++++++++
2 files changed, 140 insertions(+), 2 deletions(-)
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 62e8a911bb12..da47fc59f6cb 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -22,6 +22,9 @@
#include <linux/sysfs.h>
#include <linux/timer.h>
+#include <linux/map_to_7segment.h>
+#include <linux/map_to_14segment.h>
+
#include "line-display.h"
#define DEFAULT_SCROLL_RATE (HZ / 2)
@@ -188,12 +191,71 @@ static ssize_t scroll_step_ms_store(struct device *dev,
static DEVICE_ATTR_RW(scroll_step_ms);
+static ssize_t map_seg_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+ struct linedisp_map *map = linedisp->map;
+
+ memcpy(buf, &map->map, map->size);
+ return map->size;
+}
+
+static ssize_t map_seg_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+ struct linedisp_map *map = linedisp->map;
+
+ if (count != map->size)
+ return -EINVAL;
+
+ memcpy(&map->map, buf, count);
+ return count;
+}
+
+static const SEG7_DEFAULT_MAP(initial_map_seg7);
+static DEVICE_ATTR(map_seg7, 0644, map_seg_show, map_seg_store);
+
+static const SEG14_DEFAULT_MAP(initial_map_seg14);
+static DEVICE_ATTR(map_seg14, 0644, map_seg_show, map_seg_store);
+
static struct attribute *linedisp_attrs[] = {
&dev_attr_message.attr,
&dev_attr_scroll_step_ms.attr,
- NULL,
+ &dev_attr_map_seg7.attr,
+ &dev_attr_map_seg14.attr,
+ NULL
};
-ATTRIBUTE_GROUPS(linedisp);
+
+static umode_t linedisp_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+ struct linedisp_map *map = linedisp->map;
+ umode_t mode = attr->mode;
+
+ if (attr == &dev_attr_map_seg7.attr) {
+ if (!map)
+ return 0;
+ if (map->type != LINEDISP_MAP_SEG7)
+ return 0;
+ }
+
+ if (attr == &dev_attr_map_seg14.attr) {
+ if (!map)
+ return 0;
+ if (map->type != LINEDISP_MAP_SEG14)
+ return 0;
+ }
+
+ return mode;
+};
+
+static const struct attribute_group linedisp_group = {
+ .is_visible = linedisp_attr_is_visible,
+ .attrs = linedisp_attrs,
+};
+__ATTRIBUTE_GROUPS(linedisp);
static DEFINE_IDA(linedisp_id);
@@ -201,6 +263,7 @@ static void linedisp_release(struct device *dev)
{
struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+ kfree(linedisp->map);
kfree(linedisp->message);
ida_free(&linedisp_id, linedisp->id);
}
@@ -210,6 +273,44 @@ static const struct device_type linedisp_type = {
.release = linedisp_release,
};
+static int linedisp_init_map(struct linedisp *linedisp)
+{
+ struct linedisp_map *map;
+ int err;
+
+ if (!linedisp->ops->get_map_type)
+ return 0;
+
+ err = linedisp->ops->get_map_type(linedisp);
+ if (err < 0)
+ return err;
+
+ map = kmalloc(sizeof(*map), GFP_KERNEL);
+ if (!map)
+ return -ENOMEM;
+
+ map->type = err;
+
+ /* assign initial mapping */
+ switch (map->type) {
+ case LINEDISP_MAP_SEG7:
+ map->map.seg7 = initial_map_seg7;
+ map->size = sizeof(map->map.seg7);
+ break;
+ case LINEDISP_MAP_SEG14:
+ map->map.seg14 = initial_map_seg14;
+ map->size = sizeof(map->map.seg14);
+ break;
+ default:
+ kfree(map);
+ return -EINVAL;
+ }
+
+ linedisp->map = map;
+
+ return 0;
+}
+
/**
* linedisp_register - register a character line display
* @linedisp: pointer to character line display structure
@@ -241,6 +342,11 @@ int linedisp_register(struct linedisp *linedisp, struct device *parent,
device_initialize(&linedisp->dev);
dev_set_name(&linedisp->dev, "linedisp.%u", linedisp->id);
+ /* initialise a character mapping, if required */
+ err = linedisp_init_map(linedisp);
+ if (err)
+ goto out_put_device;
+
/* initialise a timer for scrolling the message */
timer_setup(&linedisp->timer, linedisp_scroll, 0);
@@ -259,6 +365,7 @@ int linedisp_register(struct linedisp *linedisp, struct device *parent,
device_del(&linedisp->dev);
out_del_timer:
del_timer_sync(&linedisp->timer);
+out_put_device:
put_device(&linedisp->dev);
return err;
}
diff --git a/drivers/auxdisplay/line-display.h b/drivers/auxdisplay/line-display.h
index a4f0d1bf5848..65d782111f53 100644
--- a/drivers/auxdisplay/line-display.h
+++ b/drivers/auxdisplay/line-display.h
@@ -14,13 +14,43 @@
#include <linux/device.h>
#include <linux/timer_types.h>
+#include <linux/map_to_7segment.h>
+#include <linux/map_to_14segment.h>
+
struct linedisp;
+/**
+ * enum linedisp_map_type - type of the character mapping
+ * @LINEDISP_MAP_SEG7: Map characters to 7 segment display
+ * @LINEDISP_MAP_SEG14: Map characters to 14 segment display
+ */
+enum linedisp_map_type {
+ LINEDISP_MAP_SEG7,
+ LINEDISP_MAP_SEG14,
+};
+
+/**
+ * struct linedisp_map - character mapping
+ * @type: type of the character mapping
+ * @map: conversion character mapping
+ * @size: size of the @map
+ */
+struct linedisp_map {
+ enum linedisp_map_type type;
+ union {
+ struct seg7_conversion_map seg7;
+ struct seg14_conversion_map seg14;
+ } map;
+ unsigned int size;
+};
+
/**
* struct linedisp_ops - character line display operations
+ * @get_map_type: Function called to get the character mapping, if required
* @update: Function called to update the display. This must not sleep!
*/
struct linedisp_ops {
+ int (*get_map_type)(struct linedisp *linedisp);
void (*update)(struct linedisp *linedisp);
};
@@ -42,6 +72,7 @@ struct linedisp {
unsigned int id;
struct timer_list timer;
const struct linedisp_ops *ops;
+ struct linedisp_map *map;
char *buf;
char *message;
unsigned int num_chars;
--
2.43.0.rc1.1.gbec44491f096
Since line display library supports necessary bits to map the characters
(if required), switch this driver to use that.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/ht16k33.c | 109 +++++++++++------------------------
1 file changed, 34 insertions(+), 75 deletions(-)
diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 75c4a8d31642..b104f08252dd 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -87,11 +87,6 @@ struct ht16k33_fbdev {
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];
};
@@ -135,33 +130,6 @@ 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 | priv->blink;
@@ -445,18 +413,20 @@ 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;
+ struct linedisp *linedisp = &seg->linedisp;
+ struct linedisp_map *map = linedisp->map;
char *s = seg->curr;
uint8_t buf[9];
- buf[0] = map_to_seg7(&seg->map.seg7, *s++);
+ buf[0] = map_to_seg7(&map->map.seg7, *s++);
buf[1] = 0;
- buf[2] = map_to_seg7(&seg->map.seg7, *s++);
+ buf[2] = map_to_seg7(&map->map.seg7, *s++);
buf[3] = 0;
buf[4] = 0;
buf[5] = 0;
- buf[6] = map_to_seg7(&seg->map.seg7, *s++);
+ buf[6] = map_to_seg7(&map->map.seg7, *s++);
buf[7] = 0;
- buf[8] = map_to_seg7(&seg->map.seg7, *s++);
+ buf[8] = map_to_seg7(&map->map.seg7, *s++);
i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
}
@@ -466,17 +436,39 @@ 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;
+ struct linedisp *linedisp = &seg->linedisp;
+ struct linedisp_map *map = linedisp->map;
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);
+ put_unaligned_le16(map_to_seg14(&map->map.seg14, *s++), buf + 0);
+ put_unaligned_le16(map_to_seg14(&map->map.seg14, *s++), buf + 2);
+ put_unaligned_le16(map_to_seg14(&map->map.seg14, *s++), buf + 4);
+ put_unaligned_le16(map_to_seg14(&map->map.seg14, *s++), buf + 6);
i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf);
}
+static int ht16k33_linedisp_get_map_type(struct linedisp *linedisp)
+{
+ struct ht16k33_priv *priv = container_of(linedisp, struct ht16k33_priv,
+ seg.linedisp);
+
+ switch (priv->type) {
+ case DISP_MATRIX:
+ /* not handled here */
+ return -EINVAL;
+
+ case DISP_QUAD_7SEG:
+ INIT_DELAYED_WORK(&priv->work, ht16k33_seg7_update);
+ return LINEDISP_MAP_SEG7;
+
+ case DISP_QUAD_14SEG:
+ INIT_DELAYED_WORK(&priv->work, ht16k33_seg14_update);
+ return LINEDISP_MAP_SEG14;
+ }
+}
+
static void ht16k33_linedisp_update(struct linedisp *linedisp)
{
struct ht16k33_priv *priv = container_of(linedisp, struct ht16k33_priv,
@@ -486,6 +478,7 @@ static void ht16k33_linedisp_update(struct linedisp *linedisp)
}
static const struct linedisp_ops ht16k33_linedisp_ops = {
+ .get_map_type = ht16k33_linedisp_get_map_type,
.update = ht16k33_linedisp_update,
};
@@ -677,39 +670,7 @@ static int ht16k33_seg_probe(struct device *dev, struct ht16k33_priv *priv,
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_ops);
- 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;
+ return linedisp_register(&seg->linedisp, dev, 4, seg->curr, &ht16k33_linedisp_ops);
}
static int ht16k33_probe(struct i2c_client *client)
@@ -794,8 +755,6 @@ static void ht16k33_remove(struct i2c_client *client)
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;
}
}
--
2.43.0.rc1.1.gbec44491f096
Do not imply that some of the generic headers may be always included.
Instead, include explicitly what we are direct user of.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/line-display.c | 3 +++
drivers/auxdisplay/line-display.h | 3 +++
2 files changed, 6 insertions(+)
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 4b92ae7781f1..6b3d25e20eeb 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -10,8 +10,11 @@
#include <generated/utsrelease.h>
+#include <linux/container_of.h>
#include <linux/device.h>
+#include <linux/export.h>
#include <linux/idr.h>
+#include <linux/jiffies.h>
#include <linux/kstrtox.h>
#include <linux/module.h>
#include <linux/slab.h>
diff --git a/drivers/auxdisplay/line-display.h b/drivers/auxdisplay/line-display.h
index 1fbe57fdc4cb..d72c1899dc50 100644
--- a/drivers/auxdisplay/line-display.h
+++ b/drivers/auxdisplay/line-display.h
@@ -11,6 +11,9 @@
#ifndef _LINEDISP_H
#define _LINEDISP_H
+#include <linux/device.h>
+#include <linux/timer_types.h>
+
/**
* struct linedisp - character line display private data structure
* @dev: the line display device
--
2.43.0.rc1.1.gbec44491f096
kstrtox() may return different error codes.
Unshadow them in the ->store() callback to give better error report.
While at it, add missing kstrtox.h inclusion.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/auxdisplay/line-display.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index c4dbb13293d1..4b92ae7781f1 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -12,6 +12,7 @@
#include <linux/device.h>
#include <linux/idr.h>
+#include <linux/kstrtox.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/string.h>
@@ -166,9 +167,11 @@ static ssize_t scroll_step_ms_store(struct device *dev,
{
struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
unsigned int ms;
+ int ret;
- if (kstrtouint(buf, 10, &ms) != 0)
- return -EINVAL;
+ ret = kstrtouint(buf, 10, &ms);
+ if (ret)
+ return ret;
linedisp->scroll_rate = msecs_to_jiffies(ms);
if (linedisp->message && linedisp->message_len > linedisp->num_chars) {
--
2.43.0.rc1.1.gbec44491f096