Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1574847 > unrolled thread

[PATCH 00/13] Add HD44780 Character LCD support

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/13] Add HD44780 Character LCD support Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-06 15:50 +0100
    [PATCH 03/13] misc: panel: Remove unused LCD_FLAG_S and LCD_FLAG_ID Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-06 15:50 +0100
    [PATCH 05/13] misc: panel: Move all suboptions into a big if section Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-06 15:50 +0100
    [PATCH 04/13] misc: panel: Remove reference to misc device support Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-06 15:50 +0100
    [PATCH 06/13] misc: panel: Remove always-true check from panel_detach() Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-06 15:50 +0100
    [PATCH 11/13] auxdisplay: charlcd: Add support for displays with more than two lines Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-06 15:50 +0100
    [PATCH 02/13] misc: panel: Remove PANEL_VERSION Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-06 15:50 +0100
    Re: [PATCH 09/13] auxdisplay: charlcd: Extract character LCD core  from misc/panel Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-06 16:20 +0100
    Re: [PATCH 09/13] auxdisplay: charlcd: Extract character LCD core  from misc/panel Arnd Bergmann <arnd@arndb.de> - 2017-02-06 16:20 +0100
    Re: [PATCH 00/13] Add HD44780 Character LCD support Willy TARREAU <wtarreau@haproxy.com> - 2017-02-06 16:50 +0100
    Re: [PATCH 09/13] auxdisplay: charlcd: Extract character LCD core  from misc/panel Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-08 13:30 +0100
    Re: [PATCH 00/13] Add HD44780 Character LCD support Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-02-10 15:40 +0100

#1574847 — [PATCH 00/13] Add HD44780 Character LCD support

	Hi all,

Character LCDs are currently typically driven from userspace, either
directly through bit-banging, or via some kind of serial (UART/i2c)
interface.

This patch series adds kernel support for character LCDs using an
Hitachi HD44780 Character LCD Controller where its M6800 bus interface
is connected to GPIOs.  It does so after extracting the character LCD
core support from the existing Parallel port LCD/Keypad Panel driver
into its own subdriver.

  - Patch 1 fixes a bug in the existing panel driver,
  - Patches 2-8 perform clean ups and improvements to the panel driver,
  - Patch 9 extract the character LCD core support from the panel driver
    into an auxdisplay charlcd subdriver,
  - Patches 10-11 add new features to the character LCD core driver
    (4-bit interface support and support for displays with more than 2
    lines),
  - Patch 12 adds DT bindings for describing HD44780 Character LCDs,
  - Patch 13 adds a driver for HD44780 Character LCDs connected to
    GPIOs, using the new character LCD core.

Note that Linux has other support for HD44780 Character LCDs, but none of
it is generic:
  - drivers/misc/arm-charlcd.c involves a custom IP block for interfacing
    to the LCD controller,
  - drivers/parisc/led.c is tightly coupled to LED handling on various HP
    PA-RISC machines.

This has been tested using a 20x4 character LCD with backlight, using both
8-bit and 4-bit wiring to one or two 74HC595 shift registers connected to
an SPI master controller.

Regression testing on original "panel" hardware would be appreciated.

Thanks for your comments!

Geert Uytterhoeven (13):
  misc: panel: Fix LCD_FLAG_F/LCD_FLAG_N exchange
  misc: panel: Remove PANEL_VERSION
  misc: panel: Remove unused LCD_FLAG_S and LCD_FLAG_ID
  misc: panel: Remove reference to misc device support
  misc: panel: Move all suboptions into a big if section
  misc: panel: Remove always-true check from panel_detach()
  misc: panel: Add lcd_home() helper
  misc: panel: Abstract temporary backlight handling
  auxdisplay: charlcd: Extract character LCD core from misc/panel
  auxdisplay: charlcd: Add support for 4-bit interfaces
  auxdisplay: charlcd: Add support for displays with more than two lines
  dt-bindings: auxdisplay: Add bindings for Hitachi HD44780
  auxdisplay: Add HD44780 Character LCD support

 .../devicetree/bindings/auxdisplay/hit,hd44780.txt |  44 ++
 drivers/auxdisplay/Kconfig                         |  14 +
 drivers/auxdisplay/Makefile                        |   2 +
 drivers/auxdisplay/charlcd.c                       | 818 ++++++++++++++++++++
 drivers/auxdisplay/hd44780.c                       | 326 ++++++++
 drivers/misc/Kconfig                               |  45 +-
 drivers/misc/panel.c                               | 852 +++------------------
 include/misc/charlcd.h                             |  42 +
 8 files changed, 1374 insertions(+), 769 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/hit,hd44780.txt
 create mode 100644 drivers/auxdisplay/charlcd.c
 create mode 100644 drivers/auxdisplay/hd44780.c
 create mode 100644 include/misc/charlcd.h

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[toc] | [next] | [standalone]

#1574849 — [PATCH 03/13] misc: panel: Remove unused LCD_FLAG_S and LCD_FLAG_ID

These definitions were never used in any publicly available version
since (at least) 2004.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/misc/panel.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/misc/panel.c b/drivers/misc/panel.c
index fa4c12768a161eaa..8af500ecaaaf6634 100644
--- a/drivers/misc/panel.c
+++ b/drivers/misc/panel.c
@@ -119,8 +119,6 @@
 #define PIN_SELECP		17
 #define PIN_NOT_SET		127
 
-#define LCD_FLAG_S		0x0001
-#define LCD_FLAG_ID		0x0002
 #define LCD_FLAG_B		0x0004	/* blink on */
 #define LCD_FLAG_C		0x0008	/* cursor on */
 #define LCD_FLAG_D		0x0010	/* display on */
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1574852 — [PATCH 05/13] misc: panel: Move all suboptions into a big if section

All 18 suboptions related to the panel driver have individual
dependencies on PANEL.

Replace them by a single "if PANEL / endif # PANEL" section for easier
dependency management.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/misc/Kconfig | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 2687ecf3e9e673fa..c290990d73edf87e 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -503,9 +503,10 @@ config PANEL
 	  compiled as a module, or linked into the kernel and started at boot.
 	  If you don't understand what all this is about, say N.
 
+if PANEL
+
 config PANEL_PARPORT
 	int "Default parallel port number (0=LPT1)"
-	depends on PANEL
 	range 0 255
 	default "0"
 	---help---
@@ -517,7 +518,6 @@ config PANEL_PARPORT
 
 config PANEL_PROFILE
 	int "Default panel profile (0-5, 0=custom)"
-	depends on PANEL
 	range 0 5
 	default "5"
 	---help---
@@ -538,7 +538,7 @@ config PANEL_PROFILE
 	  for experts.
 
 config PANEL_KEYPAD
-	depends on PANEL && PANEL_PROFILE="0"
+	depends on PANEL_PROFILE="0"
 	int "Keypad type (0=none, 1=old 6 keys, 2=new 6 keys, 3=Nexcom 4 keys)"
 	range 0 3
 	default 0
@@ -555,7 +555,7 @@ config PANEL_KEYPAD
 	  supports simultaneous keys pressed when the keypad supports them.
 
 config PANEL_LCD
-	depends on PANEL && PANEL_PROFILE="0"
+	depends on PANEL_PROFILE="0"
 	int "LCD type (0=none, 1=custom, 2=old //, 3=ks0074, 4=hantronix, 5=Nexcom)"
 	range 0 5
 	default 0
@@ -578,7 +578,7 @@ config PANEL_LCD
 	   that those values changed from the 2.4 driver for better consistency.
 
 config PANEL_LCD_HEIGHT
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1"
 	int "Number of lines on the LCD (1-2)"
 	range 1 2
 	default 2
@@ -587,7 +587,7 @@ config PANEL_LCD_HEIGHT
 	  It can either be 1 or 2.
 
 config PANEL_LCD_WIDTH
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1"
 	int "Number of characters per line on the LCD (1-40)"
 	range 1 40
 	default 40
@@ -596,7 +596,7 @@ config PANEL_LCD_WIDTH
 	  Common values are 16,20,24,40.
 
 config PANEL_LCD_BWIDTH
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1"
 	int "Internal LCD line width (1-40, 40 by default)"
 	range 1 40
 	default 40
@@ -612,7 +612,7 @@ config PANEL_LCD_BWIDTH
 	  If you don't know, put '40' here.
 
 config PANEL_LCD_HWIDTH
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1"
 	int "Hardware LCD line width (1-64, 64 by default)"
 	range 1 64
 	default 64
@@ -626,7 +626,7 @@ config PANEL_LCD_HWIDTH
 	  64 here for a 2x40.
 
 config PANEL_LCD_CHARSET
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1"
 	int "LCD character set (0=normal, 1=KS0074)"
 	range 0 1
 	default 0
@@ -642,7 +642,7 @@ config PANEL_LCD_CHARSET
 	  If you don't know, use the normal one (0).
 
 config PANEL_LCD_PROTO
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1"
 	int "LCD communication mode (0=parallel 8 bits, 1=serial)"
 	range 0 1
 	default 0
@@ -655,7 +655,7 @@ config PANEL_LCD_PROTO
 	  parallel LCD, and 1 for a serial LCD.
 
 config PANEL_LCD_PIN_E
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO="0"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO="0"
         int "Parallel port pin number & polarity connected to the LCD E signal (-17...17) "
 	range -17 17
 	default 14
@@ -670,7 +670,7 @@ config PANEL_LCD_PIN_E
 	  Default for the 'E' pin in custom profile is '14' (AUTOFEED).
 
 config PANEL_LCD_PIN_RS
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO="0"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO="0"
         int "Parallel port pin number & polarity connected to the LCD RS signal (-17...17) "
 	range -17 17
 	default 17
@@ -685,7 +685,7 @@ config PANEL_LCD_PIN_RS
 	  Default for the 'RS' pin in custom profile is '17' (SELECT IN).
 
 config PANEL_LCD_PIN_RW
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO="0"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO="0"
         int "Parallel port pin number & polarity connected to the LCD RW signal (-17...17) "
 	range -17 17
 	default 16
@@ -700,7 +700,7 @@ config PANEL_LCD_PIN_RW
 	  Default for the 'RW' pin in custom profile is '16' (INIT).
 
 config PANEL_LCD_PIN_SCL
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO!="0"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO!="0"
         int "Parallel port pin number & polarity connected to the LCD SCL signal (-17...17) "
 	range -17 17
 	default 1
@@ -715,7 +715,7 @@ config PANEL_LCD_PIN_SCL
 	  Default for the 'SCL' pin in custom profile is '1' (STROBE).
 
 config PANEL_LCD_PIN_SDA
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO!="0"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1" && PANEL_LCD_PROTO!="0"
         int "Parallel port pin number & polarity connected to the LCD SDA signal (-17...17) "
 	range -17 17
 	default 2
@@ -730,7 +730,7 @@ config PANEL_LCD_PIN_SDA
 	  Default for the 'SDA' pin in custom profile is '2' (D0).
 
 config PANEL_LCD_PIN_BL
-	depends on PANEL && PANEL_PROFILE="0" && PANEL_LCD="1"
+	depends on PANEL_PROFILE="0" && PANEL_LCD="1"
         int "Parallel port pin number & polarity connected to the LCD backlight signal (-17...17) "
 	range -17 17
 	default 0
@@ -745,7 +745,6 @@ config PANEL_LCD_PIN_BL
 	  Default for the 'BL' pin in custom profile is '0' (uncontrolled).
 
 config PANEL_CHANGE_MESSAGE
-	depends on PANEL
 	bool "Change LCD initialization message ?"
 	default "n"
 	---help---
@@ -758,7 +757,7 @@ config PANEL_CHANGE_MESSAGE
 	  say 'N' and keep the default message with the version.
 
 config PANEL_BOOT_MESSAGE
-	depends on PANEL && PANEL_CHANGE_MESSAGE="y"
+	depends on PANEL_CHANGE_MESSAGE="y"
 	string "New initialization message"
 	default ""
 	---help---
@@ -770,6 +769,8 @@ config PANEL_BOOT_MESSAGE
 	  An empty message will only clear the display at driver init time. Any other
 	  printf()-formatted message is valid with newline and escape codes.
 
+endif # PANEL
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1574855 — [PATCH 04/13] misc: panel: Remove reference to misc device support

As of commit 7c5763b8453a9487 ("drivers: misc: Remove MISC_DEVICES
config option"), misc device support no longer needs to be enabled
manually.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/misc/Kconfig | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b4d6aac838244d85..2687ecf3e9e673fa 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -499,10 +499,9 @@ config PANEL
 	  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
 	  is accessible through the /dev/lcd char device (10, 156), and the
-	  keypad through /dev/keypad (10, 185). Both require misc device to be
-	  enabled. This code can either be compiled as a module, or linked into
-	  the kernel and started at boot. If you don't understand what all this
-	  is about, say N.
+	  keypad through /dev/keypad (10, 185). This code can either be
+	  compiled as a module, or linked into the kernel and started at boot.
+	  If you don't understand what all this is about, say N.
 
 config PANEL_PARPORT
 	int "Default parallel port number (0=LPT1)"
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1574858 — [PATCH 06/13] misc: panel: Remove always-true check from panel_detach()

panel_detach() already verified that pptr is a valid pointer.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/misc/panel.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/panel.c b/drivers/misc/panel.c
index 8af500ecaaaf6634..ed19b3e399b08d1f 100644
--- a/drivers/misc/panel.c
+++ b/drivers/misc/panel.c
@@ -2266,24 +2266,22 @@ static void panel_detach(struct parport *port)
 	if (scan_timer.function)
 		del_timer_sync(&scan_timer);
 
-	if (pprt) {
-		if (keypad.enabled) {
-			misc_deregister(&keypad_dev);
-			keypad_initialized = 0;
-		}
-
-		if (lcd.enabled) {
-			panel_lcd_print("\x0cLCD driver unloaded.\x1b[Lc\x1b[Lb\x1b[L-");
-			misc_deregister(&lcd_dev);
-			lcd.initialized = false;
-		}
+	if (keypad.enabled) {
+		misc_deregister(&keypad_dev);
+		keypad_initialized = 0;
+	}
 
-		/* TODO: free all input signals */
-		parport_release(pprt);
-		parport_unregister_device(pprt);
-		pprt = NULL;
-		unregister_reboot_notifier(&panel_notifier);
+	if (lcd.enabled) {
+		panel_lcd_print("\x0cLCD driver unloaded.\x1b[Lc\x1b[Lb\x1b[L-");
+		misc_deregister(&lcd_dev);
+		lcd.initialized = false;
 	}
+
+	/* TODO: free all input signals */
+	parport_release(pprt);
+	parport_unregister_device(pprt);
+	pprt = NULL;
+	unregister_reboot_notifier(&panel_notifier);
 }
 
 static struct parport_driver panel_driver = {
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1574860 — [PATCH 11/13] auxdisplay: charlcd: Add support for displays with more than two lines

On displays with more than two lines, the additional lines are stored in
the buffers used for the first two lines, but beyond the visible parts.
Adjust the DDRAM address calculation to cater for this.

When clearing the display, avoid writing more spaces than the actual
size of the physical buffer.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/auxdisplay/charlcd.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index 6d27d9363c9baaf2..6aea90086e3695c5 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -159,15 +159,19 @@ void charlcd_poke(struct charlcd *lcd)
 static void charlcd_gotoxy(struct charlcd *lcd)
 {
 	struct charlcd_priv *priv = to_priv(lcd);
+	unsigned int addr;
 
-	lcd->ops->write_cmd(lcd,
-		LCD_CMD_SET_DDRAM_ADDR | (priv->addr.y ? lcd->hwidth : 0) |
-		/*
-		 * we force the cursor to stay at the end of the
-		 * line if it wants to go farther
-		 */
-		((priv->addr.x < lcd->bwidth) ? priv->addr.x & (lcd->hwidth - 1)
-					      : lcd->bwidth - 1));
+	/*
+	 * we force the cursor to stay at the end of the
+	 * line if it wants to go farther
+	 */
+	addr = priv->addr.x < lcd->bwidth ? priv->addr.x & (lcd->hwidth - 1)
+					  : lcd->bwidth - 1;
+	if (priv->addr.y & 1)
+		addr += lcd->hwidth;
+	if (priv->addr.y & 2)
+		addr += lcd->bwidth;
+	lcd->ops->write_cmd(lcd, LCD_CMD_SET_DDRAM_ADDR | addr);
 }
 
 static void charlcd_home(struct charlcd *lcd)
@@ -203,7 +207,7 @@ static void charlcd_clear_fast(struct charlcd *lcd)
 	if (lcd->ops->clear_fast)
 		lcd->ops->clear_fast(lcd);
 	else
-		for (pos = 0; pos < lcd->height * lcd->hwidth; pos++)
+		for (pos = 0; pos < min(2, lcd->height) * lcd->hwidth; pos++)
 			lcd->ops->write_data(lcd, ' ');
 
 	charlcd_home(lcd);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1574861 — [PATCH 02/13] misc: panel: Remove PANEL_VERSION

Hardcoded driver versions are so pre-git.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/misc/panel.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/misc/panel.c b/drivers/misc/panel.c
index cac7ac62ce5b71d7..fa4c12768a161eaa 100644
--- a/drivers/misc/panel.c
+++ b/drivers/misc/panel.c
@@ -64,8 +64,6 @@
 #define LCD_MINOR		156
 #define KEYPAD_MINOR		185
 
-#define PANEL_VERSION		"0.9.5"
-
 #define LCD_MAXBYTES		256	/* max burst write */
 
 #define KEYPAD_BUFFER		64
@@ -1656,8 +1654,7 @@ static void lcd_init(void)
 	panel_lcd_print("\x1b[Lc\x1b[Lb\x1b[L*" CONFIG_PANEL_BOOT_MESSAGE);
 #endif
 #else
-	panel_lcd_print("\x1b[Lc\x1b[Lb\x1b[L*Linux-" UTS_RELEASE "\nPanel-"
-			PANEL_VERSION);
+	panel_lcd_print("\x1b[Lc\x1b[Lb\x1b[L*Linux-" UTS_RELEASE);
 #endif
 	lcd.addr.x = 0;
 	lcd.addr.y = 0;
@@ -2278,8 +2275,7 @@ static void panel_detach(struct parport *port)
 		}
 
 		if (lcd.enabled) {
-			panel_lcd_print("\x0cLCD driver " PANEL_VERSION
-					"\nunloaded.\x1b[Lc\x1b[Lb\x1b[L-");
+			panel_lcd_print("\x0cLCD driver unloaded.\x1b[Lc\x1b[Lb\x1b[L-");
 			misc_deregister(&lcd_dev);
 			lcd.initialized = false;
 		}
@@ -2401,7 +2397,7 @@ static int __init panel_init_module(void)
 
 	if (!lcd.enabled && !keypad.enabled) {
 		/* no device enabled, let's exit */
-		pr_err("driver version " PANEL_VERSION " disabled.\n");
+		pr_err("panel driver disabled.\n");
 		return -ENODEV;
 	}
 
@@ -2412,12 +2408,10 @@ static int __init panel_init_module(void)
 	}
 
 	if (pprt)
-		pr_info("driver version " PANEL_VERSION
-			" registered on parport%d (io=0x%lx).\n", parport,
-			pprt->port->base);
+		pr_info("panel driver registered on parport%d (io=0x%lx).\n",
+			parport, pprt->port->base);
 	else
-		pr_info("driver version " PANEL_VERSION
-			" not yet registered\n");
+		pr_info("panel driver not yet registered\n");
 	return 0;
 }
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1574881 — Re: [PATCH 09/13] auxdisplay: charlcd: Extract character LCD core from misc/panel

Hi Arnd,

On Mon, Feb 6, 2017 at 4:09 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Feb 6, 2017 at 3:38 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> Extract the character LCD core from the Parallel port LCD/Keypad Panel
>> driver in the misc subsystem, and convert it into a subdriver in the
>> auxdisplay subsystem.  This allows the character LCD core to be used by
>> other drivers later.
>>
>> Compilation is controlled by its own Kconfig symbol CHARLCD, which is to
>> be selected by its users, but can be enabled manually for
>> compile-testing.
>>
>> All functions changed their prefix from "lcd_" to "charlcd_", and gained
>> a "struct charlcd *" parameter to operate on a specific instance.
>> While the driver API thus is ready to support multiple instances, the
>> current limitation of a single display (/dev/lcd has a single misc minor
>> assigned) is retained.
>>
>> No functional changes intended.
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>>  drivers/auxdisplay/Kconfig   |   3 +
>>  drivers/auxdisplay/Makefile  |   1 +
>>  drivers/auxdisplay/charlcd.c | 790 +++++++++++++++++++++++++++++++++++++++++
>>  drivers/misc/Kconfig         |   1 +
>>  drivers/misc/panel.c         | 827 +++++--------------------------------------
>>  include/misc/charlcd.h       |  40 +++
>>  6 files changed, 927 insertions(+), 735 deletions(-)
>>  create mode 100644 drivers/auxdisplay/charlcd.c
>>  create mode 100644 include/misc/charlcd.h
>
> Looks good overall, and I like the entire series, but I wonder if we should move
> the panel.c file into drivers/auxdisplay/ as well. If I understand it
> right, what's
> left now is a parport driver that hooks into the charlcd framework. I see that
> we already have another parport driver in drivers/auxdisplay/ks0108.c, so why
> not move both halves of the misc/panel.c driver into modules in
> drivers/auxdisplay?

What's left in panel.c is:
  1. a parport driver that hooks into the charlcd framework,
  2. a parport driver that does keypad handling.

I didn't move all of it because of 2.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1574892 — Re: [PATCH 09/13] auxdisplay: charlcd: Extract character LCD core from misc/panel

On Mon, Feb 6, 2017 at 3:38 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Extract the character LCD core from the Parallel port LCD/Keypad Panel
> driver in the misc subsystem, and convert it into a subdriver in the
> auxdisplay subsystem.  This allows the character LCD core to be used by
> other drivers later.
>
> Compilation is controlled by its own Kconfig symbol CHARLCD, which is to
> be selected by its users, but can be enabled manually for
> compile-testing.
>
> All functions changed their prefix from "lcd_" to "charlcd_", and gained
> a "struct charlcd *" parameter to operate on a specific instance.
> While the driver API thus is ready to support multiple instances, the
> current limitation of a single display (/dev/lcd has a single misc minor
> assigned) is retained.
>
> No functional changes intended.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  drivers/auxdisplay/Kconfig   |   3 +
>  drivers/auxdisplay/Makefile  |   1 +
>  drivers/auxdisplay/charlcd.c | 790 +++++++++++++++++++++++++++++++++++++++++
>  drivers/misc/Kconfig         |   1 +
>  drivers/misc/panel.c         | 827 +++++--------------------------------------
>  include/misc/charlcd.h       |  40 +++
>  6 files changed, 927 insertions(+), 735 deletions(-)
>  create mode 100644 drivers/auxdisplay/charlcd.c
>  create mode 100644 include/misc/charlcd.h

Looks good overall, and I like the entire series, but I wonder if we should move
the panel.c file into drivers/auxdisplay/ as well. If I understand it
right, what's
left now is a parport driver that hooks into the charlcd framework. I see that
we already have another parport driver in drivers/auxdisplay/ks0108.c, so why
not move both halves of the misc/panel.c driver into modules in
drivers/auxdisplay?

    Arnd

[toc] | [prev] | [next] | [standalone]

#1574927

Hi Geert,

On Mon, Feb 06, 2017 at 03:38:02PM +0100, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> Character LCDs are currently typically driven from userspace, either
> directly through bit-banging, or via some kind of serial (UART/i2c)
> interface.
> 
> This patch series adds kernel support for character LCDs using an
> Hitachi HD44780 Character LCD Controller where its M6800 bus interface
> is connected to GPIOs.  It does so after extracting the character LCD
> core support from the existing Parallel port LCD/Keypad Panel driver
> into its own subdriver.

That's excellent, I was dreaming about it a few days ago ;-)
Not reviewed yet but I like the idea!

Willy

[toc] | [prev] | [next] | [standalone]

#1576508 — Re: [PATCH 09/13] auxdisplay: charlcd: Extract character LCD core from misc/panel

On Mon, Feb 6, 2017 at 3:38 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Extract the character LCD core from the Parallel port LCD/Keypad Panel
> driver in the misc subsystem, and convert it into a subdriver in the
> auxdisplay subsystem.  This allows the character LCD core to be used by
> other drivers later.
>
> Compilation is controlled by its own Kconfig symbol CHARLCD, which is to
> be selected by its users, but can be enabled manually for
> compile-testing.
>
> All functions changed their prefix from "lcd_" to "charlcd_", and gained
> a "struct charlcd *" parameter to operate on a specific instance.
> While the driver API thus is ready to support multiple instances, the
> current limitation of a single display (/dev/lcd has a single misc minor
> assigned) is retained.
>
> No functional changes intended.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

> --- /dev/null
> +++ b/drivers/auxdisplay/charlcd.c

> +/* initialize the LCD driver */
> +static int charlcd_init(struct charlcd *lcd)
> +{
> +       struct charlcd_priv *priv = to_priv(lcd);
> +       int ret;
> +
> +       /*
> +        * before this line, we must NOT send anything to the display.
> +        * Since charlcd_init_display() needs to write data, we have to
> +        * enable mark the LCD initialized just before.
> +        */
> +       ret = charlcd_init_display(lcd);

0day reported:

| INFO: trying to register non-static key.
| the code is fine but needs lockdep annotation.
| turning off the locking correctness validator.

However, the code is not fine.
charlcd_init_display() calls charlcd_backlight(), which acquires the
mutex that is only initialized below.

> +       if (ret)
> +               return ret;
> +
> +       if (lcd->ops->backlight) {
> +               mutex_init(&priv->bl_tempo_lock);
> +               INIT_DELAYED_WORK(&priv->bl_work, charlcd_bl_off);
> +       }

Moving the block above before the call to charlcd_init_display() fixes this.
Will do so in v2.

Note that "misc: panel: Abstract temporary backlight handling" doesn't
suffer from this bug.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1578510

On Mon, Feb 06, 2017 at 03:38:02PM +0100, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> Character LCDs are currently typically driven from userspace, either
> directly through bit-banging, or via some kind of serial (UART/i2c)
> interface.
> 
> This patch series adds kernel support for character LCDs using an
> Hitachi HD44780 Character LCD Controller where its M6800 bus interface
> is connected to GPIOs.  It does so after extracting the character LCD
> core support from the existing Parallel port LCD/Keypad Panel driver
> into its own subdriver.
> 
>   - Patch 1 fixes a bug in the existing panel driver,
>   - Patches 2-8 perform clean ups and improvements to the panel driver,

I've applied the series up to here, I'm guessing the rest will take some
reworks based on review comments.

thanks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web