[PATCH 00/25] Make charlcd device independent

[PATCH 00/25] Make charlcd device independent

[poeschel]

From: Lars Poeschel <poeschel@lemonage.de>

This tries to make charlcd device independent. At the moment hd44780
device specific code is contained deep in charlcd. This moves this out
into a hd44780_common module, where the two hd44780 drivers we have at
the moment (hd44780 and panel) can use this from. The goal is that at
the end other drivers can use the charlcd interface.
I add one such driver for a modtronix lcd displau  with the last patch.
I submitted this already some time ago, where the wish was so split
this into smaller chunks what I try to do with this patchset.

This is v5 of the patchset. I address a few review comments with this.
I fixed some typos, but more importantly Miguel spotted that I reverted
commit 3f03b6498 ("auxdisplay: charlcd: Reuse hex_to_bin() instead of
custom code") during rebasing. This is corrected now.

As a note to patch 23:
This might slightly change behaviour.
On hd44780 displays with one or two lines the previous implementation
did still write characters to the buffer of the display even if they are
currently not visible. The shift_display command could be used so set
the "viewing window" to a new position in the buffer and then you could
see the characters previously written.
This described behaviour does not work for hd44780 displays with more
than two display lines. There simply is not enough buffer.
So the behaviour was a bit inconsistens across different displays.
The new behaviour is to stop writing character at the end of a visible
line, even if there would be room in the buffer. This allows us to have
an easy implementation, that should behave equal on all supported
displays. This is not hd44780 hardware dependent anymore.

Link: https://lore.kernel.org/lkml/20191016082430.5955-1-poeschel@lemonage.de/
Link: https://lore.kernel.org/lkml/CANiq72kS-u_Xd_m+2CQVh-JCncPf1XNXrXAZ=4z+mze8fwv2kw@mail.gmail.com/

Changes in v5:
- patch 1: Fix a commit message typo: of -> on
- patch 2: Remove some unnecessary newlines
- patch 8: Fix some typos
- patch 14: Fix commit message typo: it's -> its
- patch 15: this patch is squashed together from the former individual
  hd44780_common_ function patches
- patch 16: combined two cleanup patches
- patch 17: I did previously undo commit 3f03b6498 which was a mistake.
  This is now corrected.
- patch 24: Picked up Robs Reviewed-by
- patch 25: use hex_to_bin like in commit 3f03b6498 but for the lcd2s.c
  file

Changes in v4:
- modtronix -> Modtronix in new lcd2s driver
- Kconfig: remove "default n" in new lcd2s driver

Changes in v3:
- Fix some typos in Kconfig stuff
- Fix kernel test robot reported error: Missed EXPORT_SYMBOL_GPL
- new patch to reduce display timeout
- better patch description to why not move cursor beyond end of a line
- Fixed make dt_binding_doc errors

Changes in v2:
- split whole patch into many smaller chunks
- device tree doc in yaml

Lars Poeschel (25):
  auxdisplay: Use an enum for charlcd  backlight on/off ops
  auxdisplay: Introduce hd44780_common.[ch]
  auxdisplay: Move hwidth and bwidth to struct hd44780_common
  auxdisplay: Move ifwidth to struct hd44780_common
  auxdisplay: Move write_data pointer to hd44780_common
  auxdisplay: Move write_cmd pointers to hd44780 drivers
  auxdisplay: Move addr out of charlcd_priv
  auxdisplay: hd44780_common_print
  auxdisplay: provide hd44780_common_gotoxy
  auxdisplay: add home to charlcd_ops
  auxdisplay: Move clear_display to hd44780_common
  auxdisplay: make charlcd_backlight visible to hd44780_common
  auxdisplay: Make use of enum for backlight on / off
  auxdisplay: Move init_display to hd44780_common
  auxdisplay: implement various hd44780_common_ functions
  auxdisplay: cleanup unnecessary hd44780 code in charlcd
  auxdisplay: Move char redefine code to hd44780_common
  auxdisplay: Call charlcd_backlight in place
  auxdisplay: hd44780_common: Reduce clear_display timeout
  auxdisplay: hd44780: Remove clear_fast
  auxdisplay: charlcd: replace last device specific stuff
  auxdisplay: Change gotoxy calling interface
  auxdisplay: charlcd: Do not print chars at end of line
  auxdisplay: lcd2s DT binding doc
  auxdisplay: add a driver for lcd2s character display

 .../bindings/auxdisplay/modtronix,lcd2s.yaml  |  58 +++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/auxdisplay/Kconfig                    |  30 ++
 drivers/auxdisplay/Makefile                   |   2 +
 drivers/auxdisplay/charlcd.c                  | 412 +++++-------------
 drivers/auxdisplay/charlcd.h                  |  86 +++-
 drivers/auxdisplay/hd44780.c                  | 120 +++--
 drivers/auxdisplay/hd44780_common.c           | 361 +++++++++++++++
 drivers/auxdisplay/hd44780_common.h           |  33 ++
 drivers/auxdisplay/lcd2s.c                    | 403 +++++++++++++++++
 drivers/auxdisplay/panel.c                    | 180 ++++----
 11 files changed, 1237 insertions(+), 450 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/modtronix,lcd2s.yaml
 create mode 100644 drivers/auxdisplay/hd44780_common.c
 create mode 100644 drivers/auxdisplay/hd44780_common.h
 create mode 100644 drivers/auxdisplay/lcd2s.c

-- 
2.28.0

[PATCH 01/25] auxdisplay: Use an enum for charlcd backlight on/off ops

[poeschel]

From: Lars Poeschel <poeschel@lemonage.de>

We use an enum for calling the functions in charlcd, that turn the
backlight on or off. This enum is generic and can be used for other
charlcd turn on / turn off operations as well.

Reviewed-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v5:
- Fix a commit message typo: of -> on
---
 drivers/auxdisplay/charlcd.c | 2 +-
 drivers/auxdisplay/charlcd.h | 7 ++++++-
 drivers/auxdisplay/hd44780.c | 2 +-
 drivers/auxdisplay/panel.c   | 2 +-
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index 5aee0f546351..8aaee0fea9ab 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -101,7 +101,7 @@ static void long_sleep(int ms)
 }
 
 /* turn the backlight on or off */
-static void charlcd_backlight(struct charlcd *lcd, int on)
+static void charlcd_backlight(struct charlcd *lcd, enum charlcd_onoff on)
 {
 	struct charlcd_priv *priv = charlcd_to_priv(lcd);
 
diff --git a/drivers/auxdisplay/charlcd.h b/drivers/auxdisplay/charlcd.h
index 00911ad0f3de..c66f038e5d2b 100644
--- a/drivers/auxdisplay/charlcd.h
+++ b/drivers/auxdisplay/charlcd.h
@@ -9,6 +9,11 @@
 #ifndef _CHARLCD_H
 #define _CHARLCD_H
 
+enum charlcd_onoff {
+	CHARLCD_OFF = 0,
+	CHARLCD_ON,
+};
+
 struct charlcd {
 	const struct charlcd_ops *ops;
 	const unsigned char *char_conv;	/* Optional */
@@ -30,7 +35,7 @@ struct charlcd_ops {
 	/* Optional */
 	void (*write_cmd_raw4)(struct charlcd *lcd, int cmd);	/* 4-bit only */
 	void (*clear_fast)(struct charlcd *lcd);
-	void (*backlight)(struct charlcd *lcd, int on);
+	void (*backlight)(struct charlcd *lcd, enum charlcd_onoff on);
 };
 
 struct charlcd *charlcd_alloc(unsigned int drvdata_size);
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index bcbe13092327..5982158557bb 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -37,7 +37,7 @@ struct hd44780 {
 	struct gpio_desc *pins[PIN_NUM];
 };
 
-static void hd44780_backlight(struct charlcd *lcd, int on)
+static void hd44780_backlight(struct charlcd *lcd, enum charlcd_onoff on)
 {
 	struct hd44780 *hd = lcd->drvdata;
 
diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index 1c82d824ae00..de623ae219f1 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -708,7 +708,7 @@ static void lcd_send_serial(int byte)
 }
 
 /* turn the backlight on or off */
-static void lcd_backlight(struct charlcd *charlcd, int on)
+static void lcd_backlight(struct charlcd *charlcd, enum charlcd_onoff on)
 {
 	if (lcd.pins.bl == PIN_NONE)
 		return;
-- 
2.28.0

[PATCH 02/25] auxdisplay: Introduce hd44780_common.[ch]

[poeschel]

From: Lars Poeschel <poeschel@lemonage.de>

There is some hd44780 specific code in charlcd and this code is used by
multiple drivers. To make charlcd independent from this device specific
code this has to be moved to a place where the multiple drivers can
share their common code. This common place is now introduced as
hd44780_common.

Reviewed-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v5:
- Remove some unnecessary newlines

Changes in v3:
- Fix some typos
---
 drivers/auxdisplay/Kconfig          | 20 ++++++++++++++
 drivers/auxdisplay/Makefile         |  1 +
 drivers/auxdisplay/hd44780.c        | 43 +++++++++++++++++++----------
 drivers/auxdisplay/hd44780_common.c | 19 +++++++++++++
 drivers/auxdisplay/hd44780_common.h |  7 +++++
 drivers/auxdisplay/panel.c          | 18 ++++++++++--
 6 files changed, 91 insertions(+), 17 deletions(-)
 create mode 100644 drivers/auxdisplay/hd44780_common.c
 create mode 100644 drivers/auxdisplay/hd44780_common.h

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 81757eeded68..a56171d1a1ba 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -14,12 +14,31 @@ menuconfig AUXDISPLAY
 
 	  If you say N, all options in this submenu will be skipped and disabled.
 
+config CHARLCD
+	tristate "Character LCD core support" if COMPILE_TEST
+	help
+	  This is the base system for character-based LCD displays.
+	  It makes no sense to have this alone, you select your display driver
+	  and if it needs the charlcd core, it will select it automatically.
+	  This is some character LCD core interface that multiple drivers can
+	  use.
+
+config HD44780_COMMON
+	tristate "Common functions for HD44780 (and compatibles) LCD displays" if COMPILE_TEST
+	help
+	  This is a module with the common symbols for HD44780 (and compatibles)
+	  displays. This is the code that multiple other modules use. It is not
+	  useful alone. If you have some sort of HD44780 compatible display,
+	  you very likely use this. It is selected automatically by selecting
+	  your concrete display.
+
 if AUXDISPLAY
 
 config HD44780
 	tristate "HD44780 Character LCD support"
 	depends on GPIOLIB || COMPILE_TEST
 	select CHARLCD
+	select HD44780_COMMON
 	help
 	  Enable support for Character LCDs using a HD44780 controller.
 	  The LCD is accessible through the /dev/lcd char device (10, 156).
@@ -168,6 +187,7 @@ menuconfig PARPORT_PANEL
 	tristate "Parallel port LCD/Keypad Panel support"
 	depends on PARPORT
 	select CHARLCD
+	select HD44780_COMMON
 	help
 	  Say Y here if you have an HD44780 or KS-0074 LCD connected to your
 	  parallel port. This driver also features 4 and 6-key keypads. The LCD
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index cf54b5efb07e..7e8a8c3eb3c3 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -4,6 +4,7 @@
 #
 
 obj-$(CONFIG_CHARLCD)		+= charlcd.o
+obj-$(CONFIG_HD44780_COMMON)	+= hd44780_common.o
 obj-$(CONFIG_ARM_CHARLCD)	+= arm-charlcd.o
 obj-$(CONFIG_KS0108)		+= ks0108.o
 obj-$(CONFIG_CFAG12864B)	+= cfag12864b.o cfag12864bfb.o
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index 5982158557bb..271dba9cd108 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 
 #include "charlcd.h"
+#include "hd44780_common.h"
 
 enum hd44780_pin {
 	/* Order does matter due to writing to GPIO array subsets! */
@@ -179,8 +180,9 @@ static int hd44780_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	unsigned int i, base;
 	struct charlcd *lcd;
+	struct hd44780_common *hdc;
 	struct hd44780 *hd;
-	int ifwidth, ret;
+	int ifwidth, ret = -ENOMEM;
 
 	/* Required pins */
 	ifwidth = gpiod_count(dev, "data");
@@ -198,31 +200,39 @@ static int hd44780_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	hdc = hd44780_common_alloc();
+	if (!hdc)
+		return -ENOMEM;
+
 	lcd = charlcd_alloc(sizeof(struct hd44780));
 	if (!lcd)
-		return -ENOMEM;
+		goto fail1;
 
-	hd = lcd->drvdata;
+	hd = kzalloc(sizeof(struct hd44780), GFP_KERNEL);
+	if (!hd)
+		goto fail2;
 
+	hdc->hd44780 = hd;
+	lcd->drvdata = hdc;
 	for (i = 0; i < ifwidth; i++) {
 		hd->pins[base + i] = devm_gpiod_get_index(dev, "data", i,
 							  GPIOD_OUT_LOW);
 		if (IS_ERR(hd->pins[base + i])) {
 			ret = PTR_ERR(hd->pins[base + i]);
-			goto fail;
+			goto fail3;
 		}
 	}
 
 	hd->pins[PIN_CTRL_E] = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
 	if (IS_ERR(hd->pins[PIN_CTRL_E])) {
 		ret = PTR_ERR(hd->pins[PIN_CTRL_E]);
-		goto fail;
+		goto fail3;
 	}
 
 	hd->pins[PIN_CTRL_RS] = devm_gpiod_get(dev, "rs", GPIOD_OUT_HIGH);
 	if (IS_ERR(hd->pins[PIN_CTRL_RS])) {
 		ret = PTR_ERR(hd->pins[PIN_CTRL_RS]);
-		goto fail;
+		goto fail3;
 	}
 
 	/* Optional pins */
@@ -230,24 +240,24 @@ static int hd44780_probe(struct platform_device *pdev)
 							GPIOD_OUT_LOW);
 	if (IS_ERR(hd->pins[PIN_CTRL_RW])) {
 		ret = PTR_ERR(hd->pins[PIN_CTRL_RW]);
-		goto fail;
+		goto fail3;
 	}
 
 	hd->pins[PIN_CTRL_BL] = devm_gpiod_get_optional(dev, "backlight",
 							GPIOD_OUT_LOW);
 	if (IS_ERR(hd->pins[PIN_CTRL_BL])) {
 		ret = PTR_ERR(hd->pins[PIN_CTRL_BL]);
-		goto fail;
+		goto fail3;
 	}
 
 	/* Required properties */
 	ret = device_property_read_u32(dev, "display-height-chars",
 				       &lcd->height);
 	if (ret)
-		goto fail;
+		goto fail3;
 	ret = device_property_read_u32(dev, "display-width-chars", &lcd->width);
 	if (ret)
-		goto fail;
+		goto fail3;
 
 	/*
 	 * On displays with more than two rows, the internal buffer width is
@@ -264,13 +274,17 @@ static int hd44780_probe(struct platform_device *pdev)
 
 	ret = charlcd_register(lcd);
 	if (ret)
-		goto fail;
+		goto fail3;
 
 	platform_set_drvdata(pdev, lcd);
 	return 0;
 
-fail:
-	charlcd_free(lcd);
+fail3:
+	kfree(hd);
+fail2:
+	kfree(lcd);
+fail1:
+	kfree(hdc);
 	return ret;
 }
 
@@ -278,9 +292,10 @@ static int hd44780_remove(struct platform_device *pdev)
 {
 	struct charlcd *lcd = platform_get_drvdata(pdev);
 
+	kfree(lcd->drvdata);
 	charlcd_unregister(lcd);
 
-	charlcd_free(lcd);
+	kfree(lcd);
 	return 0;
 }
 
diff --git a/drivers/auxdisplay/hd44780_common.c b/drivers/auxdisplay/hd44780_common.c
new file mode 100644
index 000000000000..2fdea29d6a8f
--- /dev/null
+++ b/drivers/auxdisplay/hd44780_common.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "hd44780_common.h"
+
+struct hd44780_common *hd44780_common_alloc(void)
+{
+	struct hd44780_common *hd;
+
+	hd = kzalloc(sizeof(*hd), GFP_KERNEL);
+	if (!hd)
+		return NULL;
+
+	return hd;
+}
+EXPORT_SYMBOL_GPL(hd44780_common_alloc);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/auxdisplay/hd44780_common.h b/drivers/auxdisplay/hd44780_common.h
new file mode 100644
index 000000000000..974868f7529c
--- /dev/null
+++ b/drivers/auxdisplay/hd44780_common.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+struct hd44780_common {
+	void *hd44780;
+};
+
+struct hd44780_common *hd44780_common_alloc(void);
diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index de623ae219f1..c3a60e190a7a 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -56,6 +56,7 @@
 #include <linux/uaccess.h>
 
 #include "charlcd.h"
+#include "hd44780_common.h"
 
 #define LCD_MAXBYTES		256	/* max burst write */
 
@@ -895,10 +896,20 @@ static const struct charlcd_ops charlcd_tilcd_ops = {
 static void lcd_init(void)
 {
 	struct charlcd *charlcd;
+	struct hd44780_common *hdc;
+
+	hdc = hd44780_common_alloc();
+	if (!hdc)
+		return;
 
 	charlcd = charlcd_alloc(0);
-	if (!charlcd)
+	if (!charlcd) {
+		kfree(hdc);
 		return;
+	}
+
+	hdc->hd44780 = &lcd;
+	charlcd->drvdata = hdc;
 
 	/*
 	 * Init lcd struct with load-time values to preserve exact
@@ -1620,7 +1631,7 @@ static void panel_attach(struct parport *port)
 	if (lcd.enabled)
 		charlcd_unregister(lcd.charlcd);
 err_unreg_device:
-	charlcd_free(lcd.charlcd);
+	kfree(lcd.charlcd);
 	lcd.charlcd = NULL;
 	parport_unregister_device(pprt);
 	pprt = NULL;
@@ -1647,7 +1658,8 @@ static void panel_detach(struct parport *port)
 	if (lcd.enabled) {
 		charlcd_unregister(lcd.charlcd);
 		lcd.initialized = false;
-		charlcd_free(lcd.charlcd);
+		kfree(lcd.charlcd->drvdata);
+		kfree(lcd.charlcd);
 		lcd.charlcd = NULL;
 	}
 
-- 
2.28.0

Re: [PATCH 00/25] Make charlcd device independent

[Lars Poeschel]

This series was sent wrong. Should be v5. Drop this one.

Sorry and thanks,
Lars

On Thu, Oct 29, 2020 at 10:50:07AM +0100, poeschel@lemonage.de wrote:
> From: Lars Poeschel <poeschel@lemonage.de>
> 
> This tries to make charlcd device independent. At the moment hd44780
> device specific code is contained deep in charlcd. This moves this out
> into a hd44780_common module, where the two hd44780 drivers we have at
> the moment (hd44780 and panel) can use this from. The goal is that at
> the end other drivers can use the charlcd interface.
> I add one such driver for a modtronix lcd displau  with the last patch.
> I submitted this already some time ago, where the wish was so split
> this into smaller chunks what I try to do with this patchset.
> 
> This is v5 of the patchset. I address a few review comments with this.
> I fixed some typos, but more importantly Miguel spotted that I reverted
> commit 3f03b6498 ("auxdisplay: charlcd: Reuse hex_to_bin() instead of
> custom code") during rebasing. This is corrected now.
> 
> As a note to patch 23:
> This might slightly change behaviour.
> On hd44780 displays with one or two lines the previous implementation
> did still write characters to the buffer of the display even if they are
> currently not visible. The shift_display command could be used so set
> the "viewing window" to a new position in the buffer and then you could
> see the characters previously written.
> This described behaviour does not work for hd44780 displays with more
> than two display lines. There simply is not enough buffer.
> So the behaviour was a bit inconsistens across different displays.
> The new behaviour is to stop writing character at the end of a visible
> line, even if there would be room in the buffer. This allows us to have
> an easy implementation, that should behave equal on all supported
> displays. This is not hd44780 hardware dependent anymore.
> 
> Link: https://lore.kernel.org/lkml/20191016082430.5955-1-poeschel@lemonage.de/
> Link: https://lore.kernel.org/lkml/CANiq72kS-u_Xd_m+2CQVh-JCncPf1XNXrXAZ=4z+mze8fwv2kw@mail.gmail.com/
> 
> Changes in v5:
> - patch 1: Fix a commit message typo: of -> on
> - patch 2: Remove some unnecessary newlines
> - patch 8: Fix some typos
> - patch 14: Fix commit message typo: it's -> its
> - patch 15: this patch is squashed together from the former individual
>   hd44780_common_ function patches
> - patch 16: combined two cleanup patches
> - patch 17: I did previously undo commit 3f03b6498 which was a mistake.
>   This is now corrected.
> - patch 24: Picked up Robs Reviewed-by
> - patch 25: use hex_to_bin like in commit 3f03b6498 but for the lcd2s.c
>   file
> 
> Changes in v4:
> - modtronix -> Modtronix in new lcd2s driver
> - Kconfig: remove "default n" in new lcd2s driver
> 
> Changes in v3:
> - Fix some typos in Kconfig stuff
> - Fix kernel test robot reported error: Missed EXPORT_SYMBOL_GPL
> - new patch to reduce display timeout
> - better patch description to why not move cursor beyond end of a line
> - Fixed make dt_binding_doc errors
> 
> Changes in v2:
> - split whole patch into many smaller chunks
> - device tree doc in yaml
> 
> Lars Poeschel (25):
>   auxdisplay: Use an enum for charlcd  backlight on/off ops
>   auxdisplay: Introduce hd44780_common.[ch]
>   auxdisplay: Move hwidth and bwidth to struct hd44780_common
>   auxdisplay: Move ifwidth to struct hd44780_common
>   auxdisplay: Move write_data pointer to hd44780_common
>   auxdisplay: Move write_cmd pointers to hd44780 drivers
>   auxdisplay: Move addr out of charlcd_priv
>   auxdisplay: hd44780_common_print
>   auxdisplay: provide hd44780_common_gotoxy
>   auxdisplay: add home to charlcd_ops
>   auxdisplay: Move clear_display to hd44780_common
>   auxdisplay: make charlcd_backlight visible to hd44780_common
>   auxdisplay: Make use of enum for backlight on / off
>   auxdisplay: Move init_display to hd44780_common
>   auxdisplay: implement various hd44780_common_ functions
>   auxdisplay: cleanup unnecessary hd44780 code in charlcd
>   auxdisplay: Move char redefine code to hd44780_common
>   auxdisplay: Call charlcd_backlight in place
>   auxdisplay: hd44780_common: Reduce clear_display timeout
>   auxdisplay: hd44780: Remove clear_fast
>   auxdisplay: charlcd: replace last device specific stuff
>   auxdisplay: Change gotoxy calling interface
>   auxdisplay: charlcd: Do not print chars at end of line
>   auxdisplay: lcd2s DT binding doc
>   auxdisplay: add a driver for lcd2s character display
> 
>  .../bindings/auxdisplay/modtronix,lcd2s.yaml  |  58 +++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  drivers/auxdisplay/Kconfig                    |  30 ++
>  drivers/auxdisplay/Makefile                   |   2 +
>  drivers/auxdisplay/charlcd.c                  | 412 +++++-------------
>  drivers/auxdisplay/charlcd.h                  |  86 +++-
>  drivers/auxdisplay/hd44780.c                  | 120 +++--
>  drivers/auxdisplay/hd44780_common.c           | 361 +++++++++++++++
>  drivers/auxdisplay/hd44780_common.h           |  33 ++
>  drivers/auxdisplay/lcd2s.c                    | 403 +++++++++++++++++
>  drivers/auxdisplay/panel.c                    | 180 ++++----
>  11 files changed, 1237 insertions(+), 450 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/modtronix,lcd2s.yaml
>  create mode 100644 drivers/auxdisplay/hd44780_common.c
>  create mode 100644 drivers/auxdisplay/hd44780_common.h
>  create mode 100644 drivers/auxdisplay/lcd2s.c
> 
> -- 
> 2.28.0
> 

Re: [PATCH 00/25] Make charlcd device independent

[Miguel Ojeda]

Hi Lars,

On Thu, Oct 29, 2020 at 10:52 AM <poeschel@lemonage.de> wrote:
>
> Changes in v5:
> - patch 1: Fix a commit message typo: of -> on
> - patch 2: Remove some unnecessary newlines
> - patch 8: Fix some typos
> - patch 14: Fix commit message typo: it's -> its
> - patch 15: this patch is squashed together from the former individual
>   hd44780_common_ function patches
> - patch 16: combined two cleanup patches
> - patch 17: I did previously undo commit 3f03b6498 which was a mistake.
>   This is now corrected.
> - patch 24: Picked up Robs Reviewed-by
> - patch 25: use hex_to_bin like in commit 3f03b6498 but for the lcd2s.c
>   file

Thanks a lot for all that! Please take a look at my other two messages
for v5. We are almost there...

Cheers,
Miguel

[PATCH 00/25] Make charlcd device independent

[poeschel]

From: Lars Poeschel <poeschel@lemonage.de>

This tries to make charlcd device independent. At the moment hd44780
device specific code is contained deep in charlcd. This moves this out
into a hd44780_common module, where the two hd44780 drivers we have at
the moment (hd44780 and panel) can use this from. The goal is that at
the end other drivers can use the charlcd interface.
I add one such driver for a modtronix lcd displau  with the last patch.
I submitted this already some time ago, where the wish was so split
this into smaller chunks what I try to do with this patchset.

This is v5 of the patchset. I address a few review comments with this.
I fixed some typos, but more importantly Miguel spotted that I reverted
commit 3f03b6498 ("auxdisplay: charlcd: Reuse hex_to_bin() instead of
custom code") during rebasing. This is corrected now.

As a note to patch 23:
This might slightly change behaviour.
On hd44780 displays with one or two lines the previous implementation
did still write characters to the buffer of the display even if they are
currently not visible. The shift_display command could be used so set
the "viewing window" to a new position in the buffer and then you could
see the characters previously written.
This described behaviour does not work for hd44780 displays with more
than two display lines. There simply is not enough buffer.
So the behaviour was a bit inconsistens across different displays.
The new behaviour is to stop writing character at the end of a visible
line, even if there would be room in the buffer. This allows us to have
an easy implementation, that should behave equal on all supported
displays. This is not hd44780 hardware dependent anymore.

Link: https://lore.kernel.org/lkml/20191016082430.5955-1-poeschel@lemonage.de/
Link: https://lore.kernel.org/lkml/CANiq72kS-u_Xd_m+2CQVh-JCncPf1XNXrXAZ=4z+mze8fwv2kw@mail.gmail.com/

Changes in v5:
- patch 1: Fix a commit message typo: of -> on
- patch 2: Remove some unnecessary newlines
- patch 8: Fix some typos
- patch 14: Fix commit message typo: it's -> its
- patch 15: this patch is squashed together from the former individual
  hd44780_common_ function patches
- patch 16: combined two cleanup patches
- patch 17: I did previously undo commit 3f03b6498 which was a mistake.
  This is now corrected.
- patch 24: Picked up Robs Reviewed-by
- patch 25: use hex_to_bin like in commit 3f03b6498 but for the lcd2s.c
  file

Changes in v4:
- modtronix -> Modtronix in new lcd2s driver
- Kconfig: remove "default n" in new lcd2s driver

Changes in v3:
- Fix some typos in Kconfig stuff
- Fix kernel test robot reported error: Missed EXPORT_SYMBOL_GPL
- new patch to reduce display timeout
- better patch description to why not move cursor beyond end of a line
- Fixed make dt_binding_doc errors

Changes in v2:
- split whole patch into many smaller chunks
- device tree doc in yaml

Lars Poeschel (25):
  auxdisplay: Use an enum for charlcd  backlight on/off ops
  auxdisplay: Introduce hd44780_common.[ch]
  auxdisplay: Move hwidth and bwidth to struct hd44780_common
  auxdisplay: Move ifwidth to struct hd44780_common
  auxdisplay: Move write_data pointer to hd44780_common
  auxdisplay: Move write_cmd pointers to hd44780 drivers
  auxdisplay: Move addr out of charlcd_priv
  auxdisplay: hd44780_common_print
  auxdisplay: provide hd44780_common_gotoxy
  auxdisplay: add home to charlcd_ops
  auxdisplay: Move clear_display to hd44780_common
  auxdisplay: make charlcd_backlight visible to hd44780_common
  auxdisplay: Make use of enum for backlight on / off
  auxdisplay: Move init_display to hd44780_common
  auxdisplay: implement various hd44780_common_ functions
  auxdisplay: cleanup unnecessary hd44780 code in charlcd
  auxdisplay: Move char redefine code to hd44780_common
  auxdisplay: Call charlcd_backlight in place
  auxdisplay: hd44780_common: Reduce clear_display timeout
  auxdisplay: hd44780: Remove clear_fast
  auxdisplay: charlcd: replace last device specific stuff
  auxdisplay: Change gotoxy calling interface
  auxdisplay: charlcd: Do not print chars at end of line
  auxdisplay: lcd2s DT binding doc
  auxdisplay: add a driver for lcd2s character display

 .../bindings/auxdisplay/modtronix,lcd2s.yaml  |  58 +++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/auxdisplay/Kconfig                    |  30 ++
 drivers/auxdisplay/Makefile                   |   2 +
 drivers/auxdisplay/charlcd.c                  | 412 +++++-------------
 drivers/auxdisplay/charlcd.h                  |  86 +++-
 drivers/auxdisplay/hd44780.c                  | 120 +++--
 drivers/auxdisplay/hd44780_common.c           | 361 +++++++++++++++
 drivers/auxdisplay/hd44780_common.h           |  33 ++
 drivers/auxdisplay/lcd2s.c                    | 403 +++++++++++++++++
 drivers/auxdisplay/panel.c                    | 180 ++++----
 11 files changed, 1237 insertions(+), 450 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/modtronix,lcd2s.yaml
 create mode 100644 drivers/auxdisplay/hd44780_common.c
 create mode 100644 drivers/auxdisplay/hd44780_common.h
 create mode 100644 drivers/auxdisplay/lcd2s.c

-- 
2.28.0