2020-10-05 12:31:24

by Lars Poeschel

[permalink] [raw]
Subject: [PATCH v4 00/32] Make charlcd device independent

From: Lars Poeschel <[email protected]>

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 at the end 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 new patchset.
Most of the patches pick one specific function in charlcd and move the
device specific code into hd44780_common.

As a note to patch 30:
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 dependents anymore.

Link: https://lore.kernel.org/lkml/[email protected]/
Link: https://lore.kernel.org/lkml/CANiq72kS-u_Xd_m+2CQVh-JCncPf1XNXrXAZ=4z+mze8fwv2kw@mail.gmail.com/

Lars Poeschel (32):
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 hd44780_common_shift_cursor
auxdisplay: Implement hd44780_common_display_shift
auxdisplay: Implement a hd44780_common_display
auxdisplay: Implement hd44780_common_cursor
auxdisplay: Implement hd44780_common_blink
auxdisplay: cleanup unnecessary hd44780 code in charlcd
auxdisplay: Implement hd44780_common_fontsize
auxdisplay: Implement hd44780_common_lines
auxdisplay: Remove unnecessary hd44780 from 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 | 368 ++++++++++++++++
drivers/auxdisplay/hd44780_common.h | 34 ++
drivers/auxdisplay/lcd2s.c | 409 +++++++++++++++++
drivers/auxdisplay/panel.c | 180 ++++----
11 files changed, 1251 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


2020-10-05 12:32:22

by Lars Poeschel

[permalink] [raw]
Subject: [PATCH v4 05/32] auxdisplay: Move write_data pointer to hd44780_common

From: Lars Poeschel <[email protected]>

This moves the write_data function pointer from struct charlcd_ops to
struct hd44780_common. This is the function that actually writes the
character to the display. This hd44780 hardware specific function is
used by two drivers at the moment.

Reviewed-by: Willy Tarreau <[email protected]>
Signed-off-by: Lars Poeschel <[email protected]>
---
drivers/auxdisplay/charlcd.c | 12 ++++++------
drivers/auxdisplay/charlcd.h | 1 -
drivers/auxdisplay/hd44780.c | 16 +++++++++-------
drivers/auxdisplay/hd44780_common.h | 1 +
drivers/auxdisplay/panel.c | 12 ++++++------
5 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index 59e0a815bf3d..df54078656c1 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -182,7 +182,7 @@ static void charlcd_print(struct charlcd *lcd, char c)
if (priv->addr.x < hdc->bwidth) {
if (lcd->char_conv)
c = lcd->char_conv[(unsigned char)c];
- lcd->ops->write_data(lcd, c);
+ hdc->write_data(hdc, c);
priv->addr.x++;

/* prevents the cursor from wrapping onto the next line */
@@ -202,7 +202,7 @@ static void charlcd_clear_fast(struct charlcd *lcd)
lcd->ops->clear_fast(lcd);
else
for (pos = 0; pos < min(2, lcd->height) * hdc->hwidth; pos++)
- lcd->ops->write_data(lcd, ' ');
+ hdc->write_data(hdc, ' ');

charlcd_home(lcd);
}
@@ -446,7 +446,7 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
int x;

for (x = priv->addr.x; x < hdc->bwidth; x++)
- lcd->ops->write_data(lcd, ' ');
+ hdc->write_data(hdc, ' ');

/* restore cursor position */
charlcd_gotoxy(lcd);
@@ -505,7 +505,7 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)

lcd->ops->write_cmd(lcd, LCD_CMD_SET_CGRAM_ADDR | (cgaddr * 8));
for (addr = 0; addr < cgoffset; addr++)
- lcd->ops->write_data(lcd, cgbytes[addr]);
+ hdc->write_data(hdc, cgbytes[addr]);

/* ensures that we stop writing to CGRAM */
charlcd_gotoxy(lcd);
@@ -587,7 +587,7 @@ static void charlcd_write_char(struct charlcd *lcd, char c)
priv->addr.x--;
}
/* replace with a space */
- lcd->ops->write_data(lcd, ' ');
+ hdc->write_data(hdc, ' ');
/* back one char again */
lcd->ops->write_cmd(lcd, LCD_CMD_SHIFT);
break;
@@ -601,7 +601,7 @@ static void charlcd_write_char(struct charlcd *lcd, char c)
* go to the beginning of the next line
*/
for (; priv->addr.x < hdc->bwidth; priv->addr.x++)
- lcd->ops->write_data(lcd, ' ');
+ hdc->write_data(hdc, ' ');
priv->addr.x = 0;
priv->addr.y = (priv->addr.y + 1) % lcd->height;
charlcd_gotoxy(lcd);
diff --git a/drivers/auxdisplay/charlcd.h b/drivers/auxdisplay/charlcd.h
index 5dce9dd36562..fba4f2cd42c4 100644
--- a/drivers/auxdisplay/charlcd.h
+++ b/drivers/auxdisplay/charlcd.h
@@ -27,7 +27,6 @@ struct charlcd {
struct charlcd_ops {
/* Required */
void (*write_cmd)(struct charlcd *lcd, int cmd);
- void (*write_data)(struct charlcd *lcd, int data);

/* Optional */
void (*write_cmd_raw4)(struct charlcd *lcd, int cmd); /* 4-bit only */
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index f6786239c36f..922f0e0d2e6d 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -115,9 +115,8 @@ static void hd44780_write_cmd_gpio8(struct charlcd *lcd, int cmd)
}

/* Send data to the LCD panel in 8 bit GPIO mode */
-static void hd44780_write_data_gpio8(struct charlcd *lcd, int data)
+static void hd44780_write_data_gpio8(struct hd44780_common *hdc, int data)
{
- struct hd44780_common *hdc = lcd->drvdata;
struct hd44780 *hd = hdc->hd44780;

hd44780_write_gpio8(hd, data, 1);
@@ -128,7 +127,6 @@ static void hd44780_write_data_gpio8(struct charlcd *lcd, int data)

static const struct charlcd_ops hd44780_ops_gpio8 = {
.write_cmd = hd44780_write_cmd_gpio8,
- .write_data = hd44780_write_data_gpio8,
.backlight = hd44780_backlight,
};

@@ -163,9 +161,8 @@ static void hd44780_write_cmd_raw_gpio4(struct charlcd *lcd, int cmd)
}

/* Send data to the LCD panel in 4 bit GPIO mode */
-static void hd44780_write_data_gpio4(struct charlcd *lcd, int data)
+static void hd44780_write_data_gpio4(struct hd44780_common *hdc, int data)
{
- struct hd44780_common *hdc = lcd->drvdata;
struct hd44780 *hd = hdc->hd44780;

hd44780_write_gpio4(hd, data, 1);
@@ -177,7 +174,6 @@ static void hd44780_write_data_gpio4(struct charlcd *lcd, int data)
static const struct charlcd_ops hd44780_ops_gpio4 = {
.write_cmd = hd44780_write_cmd_gpio4,
.write_cmd_raw4 = hd44780_write_cmd_raw_gpio4,
- .write_data = hd44780_write_data_gpio4,
.backlight = hd44780_backlight,
};

@@ -276,7 +272,13 @@ static int hd44780_probe(struct platform_device *pdev)
device_property_read_u32(dev, "internal-buffer-width", &hdc->bwidth);

hdc->ifwidth = ifwidth;
- lcd->ops = ifwidth == 8 ? &hd44780_ops_gpio8 : &hd44780_ops_gpio4;
+ if (ifwidth == 8) {
+ lcd->ops = &hd44780_ops_gpio8;
+ hdc->write_data = hd44780_write_data_gpio8;
+ } else {
+ lcd->ops = &hd44780_ops_gpio4;
+ hdc->write_data = hd44780_write_data_gpio4;
+ }

ret = charlcd_register(lcd);
if (ret)
diff --git a/drivers/auxdisplay/hd44780_common.h b/drivers/auxdisplay/hd44780_common.h
index 1100e0a32394..1d686c99b2c1 100644
--- a/drivers/auxdisplay/hd44780_common.h
+++ b/drivers/auxdisplay/hd44780_common.h
@@ -7,6 +7,7 @@ struct hd44780_common {
int ifwidth; /* 4-bit or 8-bit (default) */
int bwidth; /* Default set by hd44780_alloc() */
int hwidth; /* Default set by hd44780_alloc() */
+ void (*write_data)(struct hd44780_common *hdc, int data);
void *hd44780;
};

diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index cec6b729d668..15100d21a6e9 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -734,7 +734,7 @@ static void lcd_write_cmd_s(struct charlcd *charlcd, int cmd)
}

/* send data to the LCD panel in serial mode */
-static void lcd_write_data_s(struct charlcd *charlcd, int data)
+static void lcd_write_data_s(struct hd44780_common *hdc, int data)
{
spin_lock_irq(&pprt_lock);
lcd_send_serial(0x5F); /* R/W=W, RS=1 */
@@ -767,7 +767,7 @@ static void lcd_write_cmd_p8(struct charlcd *charlcd, int cmd)
}

/* send data to the LCD panel in 8 bits parallel mode */
-static void lcd_write_data_p8(struct charlcd *charlcd, int data)
+static void lcd_write_data_p8(struct hd44780_common *hdc, int data)
{
spin_lock_irq(&pprt_lock);
/* present the data to the data port */
@@ -799,7 +799,7 @@ static void lcd_write_cmd_tilcd(struct charlcd *charlcd, int cmd)
}

/* send data to the TI LCD panel */
-static void lcd_write_data_tilcd(struct charlcd *charlcd, int data)
+static void lcd_write_data_tilcd(struct hd44780_common *hdc, int data)
{
spin_lock_irq(&pprt_lock);
/* present the data to the data port */
@@ -874,21 +874,18 @@ static void lcd_clear_fast_tilcd(struct charlcd *charlcd)

static const struct charlcd_ops charlcd_serial_ops = {
.write_cmd = lcd_write_cmd_s,
- .write_data = lcd_write_data_s,
.clear_fast = lcd_clear_fast_s,
.backlight = lcd_backlight,
};

static const struct charlcd_ops charlcd_parallel_ops = {
.write_cmd = lcd_write_cmd_p8,
- .write_data = lcd_write_data_p8,
.clear_fast = lcd_clear_fast_p8,
.backlight = lcd_backlight,
};

static const struct charlcd_ops charlcd_tilcd_ops = {
.write_cmd = lcd_write_cmd_tilcd,
- .write_data = lcd_write_data_tilcd,
.clear_fast = lcd_clear_fast_tilcd,
.backlight = lcd_backlight,
};
@@ -1019,6 +1016,7 @@ static void lcd_init(void)

if (lcd.proto == LCD_PROTO_SERIAL) { /* SERIAL */
charlcd->ops = &charlcd_serial_ops;
+ hdc->write_data = lcd_write_data_s;

if (lcd.pins.cl == PIN_NOT_SET)
lcd.pins.cl = DEFAULT_LCD_PIN_SCL;
@@ -1027,6 +1025,7 @@ static void lcd_init(void)

} else if (lcd.proto == LCD_PROTO_PARALLEL) { /* PARALLEL */
charlcd->ops = &charlcd_parallel_ops;
+ hdc->write_data = lcd_write_data_p8;

if (lcd.pins.e == PIN_NOT_SET)
lcd.pins.e = DEFAULT_LCD_PIN_E;
@@ -1036,6 +1035,7 @@ static void lcd_init(void)
lcd.pins.rw = DEFAULT_LCD_PIN_RW;
} else {
charlcd->ops = &charlcd_tilcd_ops;
+ hdc->write_data = lcd_write_data_tilcd;
}

if (lcd.pins.bl == PIN_NOT_SET)
--
2.28.0

2020-10-05 12:32:34

by Lars Poeschel

[permalink] [raw]
Subject: [PATCH v4 01/32] auxdisplay: Use an enum for charlcd backlight on/off ops

From: Lars Poeschel <[email protected]>

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 of / turn off operations as well.

Reviewed-by: Willy Tarreau <[email protected]>
Signed-off-by: Lars Poeschel <[email protected]>
---
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

2020-10-05 12:32:41

by Lars Poeschel

[permalink] [raw]
Subject: [PATCH v4 06/32] auxdisplay: Move write_cmd pointers to hd44780 drivers

From: Lars Poeschel <[email protected]>

The write_cmd function is used to send commands to hd44780 displays.
The individual hd44780 drivers then implement their appropriate way of
doing this with their supported displays. So we move this pointer so
hd44780_common.

Reviewed-by: Willy Tarreau <[email protected]>
Signed-off-by: Lars Poeschel <[email protected]>
---
drivers/auxdisplay/charlcd.c | 45 +++++++++++++++--------------
drivers/auxdisplay/charlcd.h | 5 ----
drivers/auxdisplay/hd44780.c | 15 ++++------
drivers/auxdisplay/hd44780_common.h | 3 ++
drivers/auxdisplay/panel.c | 12 ++++----
5 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index df54078656c1..ce6622f71c34 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -162,7 +162,7 @@ static void charlcd_gotoxy(struct charlcd *lcd)
addr += hdc->hwidth;
if (priv->addr.y & 2)
addr += hdc->bwidth;
- lcd->ops->write_cmd(lcd, LCD_CMD_SET_DDRAM_ADDR | addr);
+ hdc->write_cmd(hdc, LCD_CMD_SET_DDRAM_ADDR | addr);
}

static void charlcd_home(struct charlcd *lcd)
@@ -211,8 +211,9 @@ static void charlcd_clear_fast(struct charlcd *lcd)
static void charlcd_clear_display(struct charlcd *lcd)
{
struct charlcd_priv *priv = charlcd_to_priv(lcd);
+ struct hd44780_common *hdc = lcd->drvdata;

- lcd->ops->write_cmd(lcd, LCD_CMD_DISPLAY_CLEAR);
+ hdc->write_cmd(hdc, LCD_CMD_DISPLAY_CLEAR);
priv->addr.x = 0;
priv->addr.y = 0;
/* we must wait a few milliseconds (15) */
@@ -221,7 +222,7 @@ static void charlcd_clear_display(struct charlcd *lcd)

static int charlcd_init_display(struct charlcd *lcd)
{
- void (*write_cmd_raw)(struct charlcd *lcd, int cmd);
+ void (*write_cmd_raw)(struct hd44780_common *hdc, int cmd);
struct charlcd_priv *priv = charlcd_to_priv(lcd);
struct hd44780_common *hdc = lcd->drvdata;
u8 init;
@@ -241,25 +242,25 @@ static int charlcd_init_display(struct charlcd *lcd)
init = LCD_CMD_FUNCTION_SET | LCD_CMD_DATA_LEN_8BITS;
if (hdc->ifwidth == 4) {
init >>= 4;
- write_cmd_raw = lcd->ops->write_cmd_raw4;
+ write_cmd_raw = hdc->write_cmd_raw4;
} else {
- write_cmd_raw = lcd->ops->write_cmd;
+ write_cmd_raw = hdc->write_cmd;
}
- write_cmd_raw(lcd, init);
+ write_cmd_raw(hdc, init);
long_sleep(10);
- write_cmd_raw(lcd, init);
+ write_cmd_raw(hdc, init);
long_sleep(10);
- write_cmd_raw(lcd, init);
+ write_cmd_raw(hdc, init);
long_sleep(10);

if (hdc->ifwidth == 4) {
/* Switch to 4-bit mode, 1 line, small fonts */
- lcd->ops->write_cmd_raw4(lcd, LCD_CMD_FUNCTION_SET >> 4);
+ hdc->write_cmd_raw4(hdc, LCD_CMD_FUNCTION_SET >> 4);
long_sleep(10);
}

/* set font height and lines number */
- lcd->ops->write_cmd(lcd,
+ hdc->write_cmd(hdc,
LCD_CMD_FUNCTION_SET |
((hdc->ifwidth == 8) ? LCD_CMD_DATA_LEN_8BITS : 0) |
((priv->flags & LCD_FLAG_F) ? LCD_CMD_FONT_5X10_DOTS : 0) |
@@ -267,10 +268,10 @@ static int charlcd_init_display(struct charlcd *lcd)
long_sleep(10);

/* display off, cursor off, blink off */
- lcd->ops->write_cmd(lcd, LCD_CMD_DISPLAY_CTRL);
+ hdc->write_cmd(hdc, LCD_CMD_DISPLAY_CTRL);
long_sleep(10);

- lcd->ops->write_cmd(lcd,
+ hdc->write_cmd(hdc,
LCD_CMD_DISPLAY_CTRL | /* set display mode */
((priv->flags & LCD_FLAG_D) ? LCD_CMD_DISPLAY_ON : 0) |
((priv->flags & LCD_FLAG_C) ? LCD_CMD_CURSOR_ON : 0) |
@@ -281,7 +282,7 @@ static int charlcd_init_display(struct charlcd *lcd)
long_sleep(10);

/* entry mode set : increment, cursor shifting */
- lcd->ops->write_cmd(lcd, LCD_CMD_ENTRY_MODE | LCD_CMD_CURSOR_INC);
+ hdc->write_cmd(hdc, LCD_CMD_ENTRY_MODE | LCD_CMD_CURSOR_INC);

charlcd_clear_display(lcd);
return 0;
@@ -417,7 +418,7 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
if (priv->addr.x > 0) {
/* back one char if not at end of line */
if (priv->addr.x < hdc->bwidth)
- lcd->ops->write_cmd(lcd, LCD_CMD_SHIFT);
+ hdc->write_cmd(hdc, LCD_CMD_SHIFT);
priv->addr.x--;
}
processed = 1;
@@ -426,18 +427,18 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
if (priv->addr.x < lcd->width) {
/* allow the cursor to pass the end of the line */
if (priv->addr.x < (hdc->bwidth - 1))
- lcd->ops->write_cmd(lcd,
+ hdc->write_cmd(hdc,
LCD_CMD_SHIFT | LCD_CMD_SHIFT_RIGHT);
priv->addr.x++;
}
processed = 1;
break;
case 'L': /* shift display left */
- lcd->ops->write_cmd(lcd, LCD_CMD_SHIFT | LCD_CMD_DISPLAY_SHIFT);
+ hdc->write_cmd(hdc, LCD_CMD_SHIFT | LCD_CMD_DISPLAY_SHIFT);
processed = 1;
break;
case 'R': /* shift display right */
- lcd->ops->write_cmd(lcd,
+ hdc->write_cmd(hdc,
LCD_CMD_SHIFT | LCD_CMD_DISPLAY_SHIFT |
LCD_CMD_SHIFT_RIGHT);
processed = 1;
@@ -503,7 +504,7 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
}
}

- lcd->ops->write_cmd(lcd, LCD_CMD_SET_CGRAM_ADDR | (cgaddr * 8));
+ hdc->write_cmd(hdc, LCD_CMD_SET_CGRAM_ADDR | (cgaddr * 8));
for (addr = 0; addr < cgoffset; addr++)
hdc->write_data(hdc, cgbytes[addr]);

@@ -535,14 +536,14 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
if ((oldflags ^ priv->flags) &
(LCD_FLAG_B | LCD_FLAG_C | LCD_FLAG_D))
/* set display mode */
- lcd->ops->write_cmd(lcd,
+ hdc->write_cmd(hdc,
LCD_CMD_DISPLAY_CTRL |
((priv->flags & LCD_FLAG_D) ? LCD_CMD_DISPLAY_ON : 0) |
((priv->flags & LCD_FLAG_C) ? LCD_CMD_CURSOR_ON : 0) |
((priv->flags & LCD_FLAG_B) ? LCD_CMD_BLINK_ON : 0));
/* check whether one of F,N flags was changed */
else if ((oldflags ^ priv->flags) & (LCD_FLAG_F | LCD_FLAG_N))
- lcd->ops->write_cmd(lcd,
+ hdc->write_cmd(hdc,
LCD_CMD_FUNCTION_SET |
((hdc->ifwidth == 8) ? LCD_CMD_DATA_LEN_8BITS : 0) |
((priv->flags & LCD_FLAG_F) ? LCD_CMD_FONT_5X10_DOTS : 0) |
@@ -583,13 +584,13 @@ static void charlcd_write_char(struct charlcd *lcd, char c)
*/
if (priv->addr.x < hdc->bwidth)
/* back one char */
- lcd->ops->write_cmd(lcd, LCD_CMD_SHIFT);
+ hdc->write_cmd(hdc, LCD_CMD_SHIFT);
priv->addr.x--;
}
/* replace with a space */
hdc->write_data(hdc, ' ');
/* back one char again */
- lcd->ops->write_cmd(lcd, LCD_CMD_SHIFT);
+ hdc->write_cmd(hdc, LCD_CMD_SHIFT);
break;
case '\f':
/* quickly clear the display */
diff --git a/drivers/auxdisplay/charlcd.h b/drivers/auxdisplay/charlcd.h
index fba4f2cd42c4..ad6fd2733523 100644
--- a/drivers/auxdisplay/charlcd.h
+++ b/drivers/auxdisplay/charlcd.h
@@ -25,11 +25,6 @@ struct charlcd {
};

struct charlcd_ops {
- /* Required */
- void (*write_cmd)(struct charlcd *lcd, int cmd);
-
- /* Optional */
- void (*write_cmd_raw4)(struct charlcd *lcd, int cmd); /* 4-bit only */
void (*clear_fast)(struct charlcd *lcd);
void (*backlight)(struct charlcd *lcd, enum charlcd_onoff on);
};
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index 922f0e0d2e6d..dc4738563298 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -103,9 +103,8 @@ static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
}

/* Send a command to the LCD panel in 8 bit GPIO mode */
-static void hd44780_write_cmd_gpio8(struct charlcd *lcd, int cmd)
+static void hd44780_write_cmd_gpio8(struct hd44780_common *hdc, int cmd)
{
- struct hd44780_common *hdc = lcd->drvdata;
struct hd44780 *hd = hdc->hd44780;

hd44780_write_gpio8(hd, cmd, 0);
@@ -126,14 +125,12 @@ static void hd44780_write_data_gpio8(struct hd44780_common *hdc, int data)
}

static const struct charlcd_ops hd44780_ops_gpio8 = {
- .write_cmd = hd44780_write_cmd_gpio8,
.backlight = hd44780_backlight,
};

/* Send a command to the LCD panel in 4 bit GPIO mode */
-static void hd44780_write_cmd_gpio4(struct charlcd *lcd, int cmd)
+static void hd44780_write_cmd_gpio4(struct hd44780_common *hdc, int cmd)
{
- struct hd44780_common *hdc = lcd->drvdata;
struct hd44780 *hd = hdc->hd44780;

hd44780_write_gpio4(hd, cmd, 0);
@@ -143,10 +140,9 @@ static void hd44780_write_cmd_gpio4(struct charlcd *lcd, int cmd)
}

/* Send 4-bits of a command to the LCD panel in raw 4 bit GPIO mode */
-static void hd44780_write_cmd_raw_gpio4(struct charlcd *lcd, int cmd)
+static void hd44780_write_cmd_raw_gpio4(struct hd44780_common *hdc, int cmd)
{
DECLARE_BITMAP(values, 6); /* for DATA[4-7], RS, RW */
- struct hd44780_common *hdc = lcd->drvdata;
struct hd44780 *hd = hdc->hd44780;
unsigned int n;

@@ -172,8 +168,6 @@ static void hd44780_write_data_gpio4(struct hd44780_common *hdc, int data)
}

static const struct charlcd_ops hd44780_ops_gpio4 = {
- .write_cmd = hd44780_write_cmd_gpio4,
- .write_cmd_raw4 = hd44780_write_cmd_raw_gpio4,
.backlight = hd44780_backlight,
};

@@ -275,9 +269,12 @@ static int hd44780_probe(struct platform_device *pdev)
if (ifwidth == 8) {
lcd->ops = &hd44780_ops_gpio8;
hdc->write_data = hd44780_write_data_gpio8;
+ hdc->write_cmd = hd44780_write_cmd_gpio8;
} else {
lcd->ops = &hd44780_ops_gpio4;
hdc->write_data = hd44780_write_data_gpio4;
+ hdc->write_cmd = hd44780_write_cmd_gpio4;
+ hdc->write_cmd_raw4 = hd44780_write_cmd_raw_gpio4;
}

ret = charlcd_register(lcd);
diff --git a/drivers/auxdisplay/hd44780_common.h b/drivers/auxdisplay/hd44780_common.h
index 1d686c99b2c1..6c38ddf8f2ce 100644
--- a/drivers/auxdisplay/hd44780_common.h
+++ b/drivers/auxdisplay/hd44780_common.h
@@ -8,6 +8,9 @@ struct hd44780_common {
int bwidth; /* Default set by hd44780_alloc() */
int hwidth; /* Default set by hd44780_alloc() */
void (*write_data)(struct hd44780_common *hdc, int data);
+ void (*write_cmd)(struct hd44780_common *hdc, int cmd);
+ /* write_cmd_raw4 is for 4-bit connected displays only */
+ void (*write_cmd_raw4)(struct hd44780_common *hdc, int cmd);
void *hd44780;
};

diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index 15100d21a6e9..7ef9bc7188ca 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -723,7 +723,7 @@ static void lcd_backlight(struct charlcd *charlcd, enum charlcd_onoff on)
}

/* send a command to the LCD panel in serial mode */
-static void lcd_write_cmd_s(struct charlcd *charlcd, int cmd)
+static void lcd_write_cmd_s(struct hd44780_common *hdc, int cmd)
{
spin_lock_irq(&pprt_lock);
lcd_send_serial(0x1F); /* R/W=W, RS=0 */
@@ -745,7 +745,7 @@ static void lcd_write_data_s(struct hd44780_common *hdc, int data)
}

/* send a command to the LCD panel in 8 bits parallel mode */
-static void lcd_write_cmd_p8(struct charlcd *charlcd, int cmd)
+static void lcd_write_cmd_p8(struct hd44780_common *hdc, int cmd)
{
spin_lock_irq(&pprt_lock);
/* present the data to the data port */
@@ -789,7 +789,7 @@ static void lcd_write_data_p8(struct hd44780_common *hdc, int data)
}

/* send a command to the TI LCD panel */
-static void lcd_write_cmd_tilcd(struct charlcd *charlcd, int cmd)
+static void lcd_write_cmd_tilcd(struct hd44780_common *hdc, int cmd)
{
spin_lock_irq(&pprt_lock);
/* present the data to the control port */
@@ -873,19 +873,16 @@ static void lcd_clear_fast_tilcd(struct charlcd *charlcd)
}

static const struct charlcd_ops charlcd_serial_ops = {
- .write_cmd = lcd_write_cmd_s,
.clear_fast = lcd_clear_fast_s,
.backlight = lcd_backlight,
};

static const struct charlcd_ops charlcd_parallel_ops = {
- .write_cmd = lcd_write_cmd_p8,
.clear_fast = lcd_clear_fast_p8,
.backlight = lcd_backlight,
};

static const struct charlcd_ops charlcd_tilcd_ops = {
- .write_cmd = lcd_write_cmd_tilcd,
.clear_fast = lcd_clear_fast_tilcd,
.backlight = lcd_backlight,
};
@@ -1017,6 +1014,7 @@ static void lcd_init(void)
if (lcd.proto == LCD_PROTO_SERIAL) { /* SERIAL */
charlcd->ops = &charlcd_serial_ops;
hdc->write_data = lcd_write_data_s;
+ hdc->write_cmd = lcd_write_cmd_s;

if (lcd.pins.cl == PIN_NOT_SET)
lcd.pins.cl = DEFAULT_LCD_PIN_SCL;
@@ -1026,6 +1024,7 @@ static void lcd_init(void)
} else if (lcd.proto == LCD_PROTO_PARALLEL) { /* PARALLEL */
charlcd->ops = &charlcd_parallel_ops;
hdc->write_data = lcd_write_data_p8;
+ hdc->write_cmd = lcd_write_cmd_p8;

if (lcd.pins.e == PIN_NOT_SET)
lcd.pins.e = DEFAULT_LCD_PIN_E;
@@ -1036,6 +1035,7 @@ static void lcd_init(void)
} else {
charlcd->ops = &charlcd_tilcd_ops;
hdc->write_data = lcd_write_data_tilcd;
+ hdc->write_cmd = lcd_write_cmd_tilcd;
}

if (lcd.pins.bl == PIN_NOT_SET)
--
2.28.0

2020-10-05 15:35:52

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v4 00/32] Make charlcd device independent

Hi Lars,

On Mon, Oct 5, 2020 at 2:27 PM <[email protected]> wrote:
>
> This tries to make charlcd device independent.

Thanks a lot for the work!

I see you have written the differences between versions in each patch,
but given there are 32 patches, it'd be nice to comment which ones have
changed so that folks that already did a review can take a look at
those.

Cheers,
Miguel

2020-10-06 08:41:23

by Lars Poeschel

[permalink] [raw]
Subject: Re: [PATCH v4 00/32] Make charlcd device independent

On Mon, Oct 05, 2020 at 05:30:59PM +0200, Miguel Ojeda wrote:
> Hi Lars,
>
> On Mon, Oct 5, 2020 at 2:27 PM <[email protected]> wrote:
> >
> > This tries to make charlcd device independent.
>
> Thanks a lot for the work!
>
> I see you have written the differences between versions in each patch,
> but given there are 32 patches, it'd be nice to comment which ones have
> changed so that folks that already did a review can take a look at
> those.

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

Regards,
Lars

2020-10-16 04:00:10

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v4 01/32] auxdisplay: Use an enum for charlcd backlight on/off ops

On Mon, Oct 5, 2020 at 2:27 PM <[email protected]> wrote:
>
> 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 of / turn off operations as well.

Typo: of -> on

(Already corrected in my queue)

Cheers,
Miguel

2020-10-16 04:00:26

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v4 00/32] Make charlcd device independent

Hi Lars,

On Tue, Oct 6, 2020 at 10:38 AM Lars Poeschel <[email protected]> wrote:
>
> 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

Picking these for linux-next (including Rob's Reviewed-by). I have
spotted a few typos that I corrected -- I will note them by email.

Cheers,
Miguel

2020-10-16 04:47:48

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v4 00/32] Make charlcd device independent

On Fri, Oct 16, 2020 at 4:33 AM Miguel Ojeda
<[email protected]> wrote:
>
> Picking these for linux-next (including Rob's Reviewed-by). I have
> spotted a few typos that I corrected -- I will note them by email.

Hmm, I think we should do another round instead, since I found what
looks to be an unintended revert of a previous commit in patch 24.
Lars, can you please take a look?

Also, please take the chance to apply my comments given we have a new
round (and Rob's Reviewed-by too).

By the way, I think you could simplify by squashing the "implement
hd44780_*" commits together (i.e. from 15 to 22 except 20), and the
two cleanups together too (i.e. 20 and 23). I know we asked you to
split things up before, but those two sets are quite similar
(including in their commit message) and easy to understand all
together.

Cheers,
Miguel

2020-10-22 08:52:43

by Lars Poeschel

[permalink] [raw]
Subject: Re: [PATCH v4 00/32] Make charlcd device independent

On Fri, Oct 16, 2020 at 05:59:04AM +0200, Miguel Ojeda wrote:
> On Fri, Oct 16, 2020 at 4:33 AM Miguel Ojeda
> <[email protected]> wrote:
> >
> > Picking these for linux-next (including Rob's Reviewed-by). I have
> > spotted a few typos that I corrected -- I will note them by email.
>
> Hmm, I think we should do another round instead, since I found what
> looks to be an unintended revert of a previous commit in patch 24.
> Lars, can you please take a look?

I will work on this soon.

> Also, please take the chance to apply my comments given we have a new
> round (and Rob's Reviewed-by too).

Yes, of course.

> By the way, I think you could simplify by squashing the "implement
> hd44780_*" commits together (i.e. from 15 to 22 except 20), and the
> two cleanups together too (i.e. 20 and 23). I know we asked you to
> split things up before, but those two sets are quite similar
> (including in their commit message) and easy to understand all
> together.

No problem. I will do this as well.

Regards,
Lars