2014-11-27 21:37:24

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 0/9] staging: panel: Refactor panel initialization

This set of patches focuses on making current initialization
process easier to understand - especially it tries to emphasize
what are the priorities of the params coming from different
sources (Kconfig values, device profiles and module param values
set on loading). I paid attention not to change to behaviour of
the code itself (at least for now), so all hacky places are kept.

The whole patchset is already:
Acked-by: Willy Tarreau <[email protected]>

v2: Don't introduce new macros for param value check
v3: Resend, no other changes

Mariusz Gorski (9):
staging: panel: Set default parport module param value
staging: panel: Call init function directly
staging: panel: Remove magic numbers
staging: panel: Use defined value or checking module params state
staging: panel: Start making module params read-only
staging: panel: Make two more module params read-only
staging: panel: Refactor LCD init code
staging: panel: Remove more magic number comparison
staging: panel: Move LCD-related state into struct lcd

drivers/staging/panel/panel.c | 669 ++++++++++++++++++++++--------------------
1 file changed, 354 insertions(+), 315 deletions(-)

--
2.1.3


2014-11-27 21:37:26

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 1/9] staging: panel: Set default parport module param value

Set default parport module param value to DEFAULT_PARPORT so that
a if-block can be avoided.

Signed-off-by: Mariusz Gorski <[email protected]>
Acked-by: Willy Tarreau <[email protected]>
---
drivers/staging/panel/panel.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index c6eeddf..3dd318a 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -429,7 +429,7 @@ static struct timer_list scan_timer;

MODULE_DESCRIPTION("Generic parallel port LCD/Keypad driver");

-static int parport = -1;
+static int parport = DEFAULT_PARPORT;
module_param(parport, int, 0000);
MODULE_PARM_DESC(parport, "Parallel port index (0=lpt1, 1=lpt2, ...)");

@@ -2231,9 +2231,6 @@ static int panel_init(void)
if (lcd_type < 0)
lcd_type = lcd_enabled;

- if (parport < 0)
- parport = DEFAULT_PARPORT;
-
/* take care of an eventual profile */
switch (profile) {
case PANEL_PROFILE_CUSTOM:
--
2.1.3

2014-11-27 21:37:30

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 3/9] staging: panel: Remove magic numbers

Get rid of magic numbers indicating that the value of a module param
is not set. Use a defined value instead.

Signed-off-by: Mariusz Gorski <[email protected]>
Acked-by: Willy Tarreau <[email protected]>
---
drivers/staging/panel/panel.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 0d3f09e..1b4a211 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -133,6 +133,8 @@
#define LCD_ESCAPE_LEN 24 /* max chars for LCD escape command */
#define LCD_ESCAPE_CHAR 27 /* use char 27 for escape command */

+#define NOT_SET -1
+
/* macros to simplify use of the parallel port */
#define r_ctr(x) (parport_read_control((x)->port))
#define r_dtr(x) (parport_read_data((x)->port))
@@ -439,37 +441,37 @@ MODULE_PARM_DESC(profile,
"1=16x2 old kp; 2=serial 16x2, new kp; 3=16x2 hantronix; "
"4=16x2 nexcom; default=40x2, old kp");

-static int keypad_type = -1;
+static int keypad_type = NOT_SET;
module_param(keypad_type, int, 0000);
MODULE_PARM_DESC(keypad_type,
"Keypad type: 0=none, 1=old 6 keys, 2=new 6+1 keys, 3=nexcom 4 keys");

-static int lcd_type = -1;
+static int lcd_type = NOT_SET;
module_param(lcd_type, int, 0000);
MODULE_PARM_DESC(lcd_type,
"LCD type: 0=none, 1=old //, 2=serial ks0074, 3=hantronix //, 4=nexcom //, 5=compiled-in");

-static int lcd_height = -1;
+static int lcd_height = NOT_SET;
module_param(lcd_height, int, 0000);
MODULE_PARM_DESC(lcd_height, "Number of lines on the LCD");

-static int lcd_width = -1;
+static int lcd_width = NOT_SET;
module_param(lcd_width, int, 0000);
MODULE_PARM_DESC(lcd_width, "Number of columns on the LCD");

-static int lcd_bwidth = -1; /* internal buffer width (usually 40) */
+static int lcd_bwidth = NOT_SET; /* internal buffer width (usually 40) */
module_param(lcd_bwidth, int, 0000);
MODULE_PARM_DESC(lcd_bwidth, "Internal LCD line width (40)");

-static int lcd_hwidth = -1; /* hardware buffer width (usually 64) */
+static int lcd_hwidth = NOT_SET; /* hardware buffer width (usually 64) */
module_param(lcd_hwidth, int, 0000);
MODULE_PARM_DESC(lcd_hwidth, "LCD line hardware address (64)");

-static int lcd_charset = -1;
+static int lcd_charset = NOT_SET;
module_param(lcd_charset, int, 0000);
MODULE_PARM_DESC(lcd_charset, "LCD character set: 0=standard, 1=KS0074");

-static int lcd_proto = -1;
+static int lcd_proto = NOT_SET;
module_param(lcd_proto, int, 0000);
MODULE_PARM_DESC(lcd_proto,
"LCD communication: 0=parallel (//), 1=serial, 2=TI LCD Interface");
@@ -515,11 +517,11 @@ MODULE_PARM_DESC(lcd_bl_pin,

/* Deprecated module parameters - consider not using them anymore */

-static int lcd_enabled = -1;
+static int lcd_enabled = NOT_SET;
module_param(lcd_enabled, int, 0000);
MODULE_PARM_DESC(lcd_enabled, "Deprecated option, use lcd_type instead");

-static int keypad_enabled = -1;
+static int keypad_enabled = NOT_SET;
module_param(keypad_enabled, int, 0000);
MODULE_PARM_DESC(keypad_enabled, "Deprecated option, use keypad_type instead");

--
2.1.3

2014-11-27 21:37:36

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 7/9] staging: panel: Refactor LCD init code

Rework lcd_init method to make it a little bit more clear about
the precedence of the params, move LCD geometry and pins layout
to the LCD struct and thus make the LCD-related module params
effectively read-only.

Signed-off-by: Mariusz Gorski <[email protected]>
Acked-by: Willy Tarreau <[email protected]>
---
drivers/staging/panel/panel.c | 304 ++++++++++++++++++++++--------------------
1 file changed, 163 insertions(+), 141 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 5868a28..0bdb447 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -225,6 +225,21 @@ static wait_queue_head_t keypad_read_wait;
/* lcd-specific variables */
static struct {
bool enabled;
+ int height;
+ int width;
+ int bwidth;
+ int hwidth;
+ int charset;
+ int proto;
+ /* TODO: use union here? */
+ struct {
+ int e;
+ int rs;
+ int rw;
+ int cl;
+ int da;
+ int bl;
+ } pins;
} lcd;

/* Needed only for init */
@@ -766,7 +781,7 @@ static void lcd_send_serial(int byte)
/* turn the backlight on or off */
static void lcd_backlight(int on)
{
- if (lcd_bl_pin == PIN_NONE)
+ if (lcd.pins.bl == PIN_NONE)
return;

/* The backlight is activated by setting the AUTOFEED line to +5V */
@@ -865,23 +880,23 @@ static void lcd_write_data_tilcd(int data)
static void lcd_gotoxy(void)
{
lcd_write_cmd(0x80 /* set DDRAM address */
- | (lcd_addr_y ? lcd_hwidth : 0)
+ | (lcd_addr_y ? lcd.hwidth : 0)
/* we force the cursor to stay at the end of the
line if it wants to go farther */
- | ((lcd_addr_x < lcd_bwidth) ? lcd_addr_x &
- (lcd_hwidth - 1) : lcd_bwidth - 1));
+ | ((lcd_addr_x < lcd.bwidth) ? lcd_addr_x &
+ (lcd.hwidth - 1) : lcd.bwidth - 1));
}

static void lcd_print(char c)
{
- if (lcd_addr_x < lcd_bwidth) {
+ if (lcd_addr_x < lcd.bwidth) {
if (lcd_char_conv != NULL)
c = lcd_char_conv[(unsigned char)c];
lcd_write_data(c);
lcd_addr_x++;
}
/* prevents the cursor from wrapping onto the next line */
- if (lcd_addr_x == lcd_bwidth)
+ if (lcd_addr_x == lcd.bwidth)
lcd_gotoxy();
}

@@ -895,7 +910,7 @@ static void lcd_clear_fast_s(void)
lcd_gotoxy();

spin_lock_irq(&pprt_lock);
- for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
+ for (pos = 0; pos < lcd.height * lcd.hwidth; pos++) {
lcd_send_serial(0x5F); /* R/W=W, RS=1 */
lcd_send_serial(' ' & 0x0F);
lcd_send_serial((' ' >> 4) & 0x0F);
@@ -918,7 +933,7 @@ static void lcd_clear_fast_p8(void)
lcd_gotoxy();

spin_lock_irq(&pprt_lock);
- for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
+ for (pos = 0; pos < lcd.height * lcd.hwidth; pos++) {
/* present the data to the data port */
w_dtr(pprt, ' ');

@@ -956,7 +971,7 @@ static void lcd_clear_fast_tilcd(void)
lcd_gotoxy();

spin_lock_irq(&pprt_lock);
- for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
+ for (pos = 0; pos < lcd.height * lcd.hwidth; pos++) {
/* present the data to the data port */
w_dtr(pprt, ' ');
udelay(60);
@@ -981,7 +996,7 @@ static void lcd_clear_display(void)

static void lcd_init_display(void)
{
- lcd_flags = ((lcd_height > 1) ? LCD_FLAG_N : 0)
+ lcd_flags = ((lcd.height > 1) ? LCD_FLAG_N : 0)
| LCD_FLAG_D | LCD_FLAG_C | LCD_FLAG_B;

long_sleep(20); /* wait 20 ms after power-up for the paranoid */
@@ -1095,17 +1110,17 @@ static inline int handle_lcd_special_code(void)
case 'l': /* Shift Cursor Left */
if (lcd_addr_x > 0) {
/* back one char if not at end of line */
- if (lcd_addr_x < lcd_bwidth)
+ if (lcd_addr_x < lcd.bwidth)
lcd_write_cmd(0x10);
lcd_addr_x--;
}
processed = 1;
break;
case 'r': /* shift cursor right */
- if (lcd_addr_x < lcd_width) {
+ if (lcd_addr_x < lcd.width) {
/* allow the cursor to pass the end of the line */
if (lcd_addr_x <
- (lcd_bwidth - 1))
+ (lcd.bwidth - 1))
lcd_write_cmd(0x14);
lcd_addr_x++;
}
@@ -1124,7 +1139,7 @@ static inline int handle_lcd_special_code(void)
case 'k': { /* kill end of line */
int x;

- for (x = lcd_addr_x; x < lcd_bwidth; x++)
+ for (x = lcd_addr_x; x < lcd.bwidth; x++)
lcd_write_data(' ');

/* restore cursor position */
@@ -1272,7 +1287,7 @@ static void lcd_write_char(char c)
if (lcd_addr_x > 0) {
/* check if we're not at the
end of the line */
- if (lcd_addr_x < lcd_bwidth)
+ if (lcd_addr_x < lcd.bwidth)
/* back one char */
lcd_write_cmd(0x10);
lcd_addr_x--;
@@ -1289,10 +1304,10 @@ static void lcd_write_char(char c)
case '\n':
/* flush the remainder of the current line and
go to the beginning of the next line */
- for (; lcd_addr_x < lcd_bwidth; lcd_addr_x++)
+ for (; lcd_addr_x < lcd.bwidth; lcd_addr_x++)
lcd_write_data(' ');
lcd_addr_x = 0;
- lcd_addr_y = (lcd_addr_y + 1) % lcd_height;
+ lcd_addr_y = (lcd_addr_y + 1) % lcd.height;
lcd_gotoxy();
break;
case '\r':
@@ -1421,174 +1436,164 @@ static void lcd_init(void)
switch (selected_lcd_type) {
case LCD_TYPE_OLD:
/* parallel mode, 8 bits */
- if (lcd_proto == NOT_SET)
- lcd_proto = LCD_PROTO_PARALLEL;
- if (lcd_charset == NOT_SET)
- lcd_charset = LCD_CHARSET_NORMAL;
- if (lcd_e_pin == PIN_NOT_SET)
- lcd_e_pin = PIN_STROBE;
- if (lcd_rs_pin == PIN_NOT_SET)
- lcd_rs_pin = PIN_AUTOLF;
-
- if (lcd_width == NOT_SET)
- lcd_width = 40;
- if (lcd_bwidth == NOT_SET)
- lcd_bwidth = 40;
- if (lcd_hwidth == NOT_SET)
- lcd_hwidth = 64;
- if (lcd_height == NOT_SET)
- lcd_height = 2;
+ lcd.proto = LCD_PROTO_PARALLEL;
+ lcd.charset = LCD_CHARSET_NORMAL;
+ lcd.pins.e = PIN_STROBE;
+ lcd.pins.rs = PIN_AUTOLF;
+
+ lcd.width = 40;
+ lcd.bwidth = 40;
+ lcd.hwidth = 64;
+ lcd.height = 2;
break;
case LCD_TYPE_KS0074:
/* serial mode, ks0074 */
- if (lcd_proto == NOT_SET)
- lcd_proto = LCD_PROTO_SERIAL;
- if (lcd_charset == NOT_SET)
- lcd_charset = LCD_CHARSET_KS0074;
- if (lcd_bl_pin == PIN_NOT_SET)
- lcd_bl_pin = PIN_AUTOLF;
- if (lcd_cl_pin == PIN_NOT_SET)
- lcd_cl_pin = PIN_STROBE;
- if (lcd_da_pin == PIN_NOT_SET)
- lcd_da_pin = PIN_D0;
-
- if (lcd_width == NOT_SET)
- lcd_width = 16;
- if (lcd_bwidth == NOT_SET)
- lcd_bwidth = 40;
- if (lcd_hwidth == NOT_SET)
- lcd_hwidth = 16;
- if (lcd_height == NOT_SET)
- lcd_height = 2;
+ lcd.proto = LCD_PROTO_SERIAL;
+ lcd.charset = LCD_CHARSET_KS0074;
+ lcd.pins.bl = PIN_AUTOLF;
+ lcd.pins.cl = PIN_STROBE;
+ lcd.pins.da = PIN_D0;
+
+ lcd.width = 16;
+ lcd.bwidth = 40;
+ lcd.hwidth = 16;
+ lcd.height = 2;
break;
case LCD_TYPE_NEXCOM:
/* parallel mode, 8 bits, generic */
- if (lcd_proto == NOT_SET)
- lcd_proto = LCD_PROTO_PARALLEL;
- if (lcd_charset == NOT_SET)
- lcd_charset = LCD_CHARSET_NORMAL;
- if (lcd_e_pin == PIN_NOT_SET)
- lcd_e_pin = PIN_AUTOLF;
- if (lcd_rs_pin == PIN_NOT_SET)
- lcd_rs_pin = PIN_SELECP;
- if (lcd_rw_pin == PIN_NOT_SET)
- lcd_rw_pin = PIN_INITP;
-
- if (lcd_width == NOT_SET)
- lcd_width = 16;
- if (lcd_bwidth == NOT_SET)
- lcd_bwidth = 40;
- if (lcd_hwidth == NOT_SET)
- lcd_hwidth = 64;
- if (lcd_height == NOT_SET)
- lcd_height = 2;
+ lcd.proto = LCD_PROTO_PARALLEL;
+ lcd.charset = LCD_CHARSET_NORMAL;
+ lcd.pins.e = PIN_AUTOLF;
+ lcd.pins.rs = PIN_SELECP;
+ lcd.pins.rw = PIN_INITP;
+
+ lcd.width = 16;
+ lcd.bwidth = 40;
+ lcd.hwidth = 64;
+ lcd.height = 2;
break;
case LCD_TYPE_CUSTOM:
/* customer-defined */
- if (lcd_proto == NOT_SET)
- lcd_proto = DEFAULT_LCD_PROTO;
- if (lcd_charset == NOT_SET)
- lcd_charset = DEFAULT_LCD_CHARSET;
+ lcd.proto = DEFAULT_LCD_PROTO;
+ lcd.charset = DEFAULT_LCD_CHARSET;
/* default geometry will be set later */
break;
case LCD_TYPE_HANTRONIX:
/* parallel mode, 8 bits, hantronix-like */
default:
- if (lcd_proto == NOT_SET)
- lcd_proto = LCD_PROTO_PARALLEL;
- if (lcd_charset == NOT_SET)
- lcd_charset = LCD_CHARSET_NORMAL;
- if (lcd_e_pin == PIN_NOT_SET)
- lcd_e_pin = PIN_STROBE;
- if (lcd_rs_pin == PIN_NOT_SET)
- lcd_rs_pin = PIN_SELECP;
-
- if (lcd_width == NOT_SET)
- lcd_width = 16;
- if (lcd_bwidth == NOT_SET)
- lcd_bwidth = 40;
- if (lcd_hwidth == NOT_SET)
- lcd_hwidth = 64;
- if (lcd_height == NOT_SET)
- lcd_height = 2;
+ lcd.proto = LCD_PROTO_PARALLEL;
+ lcd.charset = LCD_CHARSET_NORMAL;
+ lcd.pins.e = PIN_STROBE;
+ lcd.pins.rs = PIN_SELECP;
+
+ lcd.width = 16;
+ lcd.bwidth = 40;
+ lcd.hwidth = 64;
+ lcd.height = 2;
break;
}

+ /* Overwrite with module params set on loading */
+ if (lcd_height > -1)
+ lcd.height = lcd_height;
+ if (lcd_width > -1)
+ lcd.width = lcd_width;
+ if (lcd_bwidth > -1)
+ lcd.bwidth = lcd_bwidth;
+ if (lcd_hwidth > -1)
+ lcd.hwidth = lcd_hwidth;
+ if (lcd_charset > -1)
+ lcd.charset = lcd_charset;
+ if (lcd_proto > -1)
+ lcd.proto = lcd_proto;
+ if (lcd_e_pin != PIN_NOT_SET)
+ lcd.pins.e = lcd_e_pin;
+ if (lcd_rs_pin != PIN_NOT_SET)
+ lcd.pins.rs = lcd_rs_pin;
+ if (lcd_rw_pin != PIN_NOT_SET)
+ lcd.pins.rw = lcd_rw_pin;
+ if (lcd_cl_pin != PIN_NOT_SET)
+ lcd.pins.cl = lcd_cl_pin;
+ if (lcd_da_pin != PIN_NOT_SET)
+ lcd.pins.da = lcd_da_pin;
+ if (lcd_bl_pin != PIN_NOT_SET)
+ lcd.pins.bl = lcd_bl_pin;
+
/* this is used to catch wrong and default values */
- if (lcd_width <= 0)
- lcd_width = DEFAULT_LCD_WIDTH;
- if (lcd_bwidth <= 0)
- lcd_bwidth = DEFAULT_LCD_BWIDTH;
- if (lcd_hwidth <= 0)
- lcd_hwidth = DEFAULT_LCD_HWIDTH;
- if (lcd_height <= 0)
- lcd_height = DEFAULT_LCD_HEIGHT;
-
- if (lcd_proto == LCD_PROTO_SERIAL) { /* SERIAL */
+ if (lcd.width <= 0)
+ lcd.width = DEFAULT_LCD_WIDTH;
+ if (lcd.bwidth <= 0)
+ lcd.bwidth = DEFAULT_LCD_BWIDTH;
+ if (lcd.hwidth <= 0)
+ lcd.hwidth = DEFAULT_LCD_HWIDTH;
+ if (lcd.height <= 0)
+ lcd.height = DEFAULT_LCD_HEIGHT;
+
+ if (lcd.proto == LCD_PROTO_SERIAL) { /* SERIAL */
lcd_write_cmd = lcd_write_cmd_s;
lcd_write_data = lcd_write_data_s;
lcd_clear_fast = lcd_clear_fast_s;

- if (lcd_cl_pin == PIN_NOT_SET)
- lcd_cl_pin = DEFAULT_LCD_PIN_SCL;
- if (lcd_da_pin == PIN_NOT_SET)
- lcd_da_pin = DEFAULT_LCD_PIN_SDA;
+ if (lcd.pins.cl == PIN_NOT_SET)
+ lcd.pins.cl = DEFAULT_LCD_PIN_SCL;
+ if (lcd.pins.da == PIN_NOT_SET)
+ lcd.pins.da = DEFAULT_LCD_PIN_SDA;

- } else if (lcd_proto == LCD_PROTO_PARALLEL) { /* PARALLEL */
+ } else if (lcd.proto == LCD_PROTO_PARALLEL) { /* PARALLEL */
lcd_write_cmd = lcd_write_cmd_p8;
lcd_write_data = lcd_write_data_p8;
lcd_clear_fast = lcd_clear_fast_p8;

- if (lcd_e_pin == PIN_NOT_SET)
- lcd_e_pin = DEFAULT_LCD_PIN_E;
- if (lcd_rs_pin == PIN_NOT_SET)
- lcd_rs_pin = DEFAULT_LCD_PIN_RS;
- if (lcd_rw_pin == PIN_NOT_SET)
- lcd_rw_pin = DEFAULT_LCD_PIN_RW;
+ if (lcd.pins.e == PIN_NOT_SET)
+ lcd.pins.e = DEFAULT_LCD_PIN_E;
+ if (lcd.pins.rs == PIN_NOT_SET)
+ lcd.pins.rs = DEFAULT_LCD_PIN_RS;
+ if (lcd.pins.rw == PIN_NOT_SET)
+ lcd.pins.rw = DEFAULT_LCD_PIN_RW;
} else {
lcd_write_cmd = lcd_write_cmd_tilcd;
lcd_write_data = lcd_write_data_tilcd;
lcd_clear_fast = lcd_clear_fast_tilcd;
}

- if (lcd_bl_pin == PIN_NOT_SET)
- lcd_bl_pin = DEFAULT_LCD_PIN_BL;
-
- if (lcd_e_pin == PIN_NOT_SET)
- lcd_e_pin = PIN_NONE;
- if (lcd_rs_pin == PIN_NOT_SET)
- lcd_rs_pin = PIN_NONE;
- if (lcd_rw_pin == PIN_NOT_SET)
- lcd_rw_pin = PIN_NONE;
- if (lcd_bl_pin == PIN_NOT_SET)
- lcd_bl_pin = PIN_NONE;
- if (lcd_cl_pin == PIN_NOT_SET)
- lcd_cl_pin = PIN_NONE;
- if (lcd_da_pin == PIN_NOT_SET)
- lcd_da_pin = PIN_NONE;
-
- if (lcd_charset == NOT_SET)
- lcd_charset = DEFAULT_LCD_CHARSET;
-
- if (lcd_charset == LCD_CHARSET_KS0074)
+ if (lcd.pins.bl == PIN_NOT_SET)
+ lcd.pins.bl = DEFAULT_LCD_PIN_BL;
+
+ if (lcd.pins.e == PIN_NOT_SET)
+ lcd.pins.e = PIN_NONE;
+ if (lcd.pins.rs == PIN_NOT_SET)
+ lcd.pins.rs = PIN_NONE;
+ if (lcd.pins.rw == PIN_NOT_SET)
+ lcd.pins.rw = PIN_NONE;
+ if (lcd.pins.bl == PIN_NOT_SET)
+ lcd.pins.bl = PIN_NONE;
+ if (lcd.pins.cl == PIN_NOT_SET)
+ lcd.pins.cl = PIN_NONE;
+ if (lcd.pins.da == PIN_NOT_SET)
+ lcd.pins.da = PIN_NONE;
+
+ if (lcd.charset == NOT_SET)
+ lcd.charset = DEFAULT_LCD_CHARSET;
+
+ if (lcd.charset == LCD_CHARSET_KS0074)
lcd_char_conv = lcd_char_conv_ks0074;
else
lcd_char_conv = NULL;

- if (lcd_bl_pin != PIN_NONE)
+ if (lcd.pins.bl != PIN_NONE)
init_scan_timer();

- pin_to_bits(lcd_e_pin, lcd_bits[LCD_PORT_D][LCD_BIT_E],
+ pin_to_bits(lcd.pins.e, lcd_bits[LCD_PORT_D][LCD_BIT_E],
lcd_bits[LCD_PORT_C][LCD_BIT_E]);
- pin_to_bits(lcd_rs_pin, lcd_bits[LCD_PORT_D][LCD_BIT_RS],
+ pin_to_bits(lcd.pins.rs, lcd_bits[LCD_PORT_D][LCD_BIT_RS],
lcd_bits[LCD_PORT_C][LCD_BIT_RS]);
- pin_to_bits(lcd_rw_pin, lcd_bits[LCD_PORT_D][LCD_BIT_RW],
+ pin_to_bits(lcd.pins.rw, lcd_bits[LCD_PORT_D][LCD_BIT_RW],
lcd_bits[LCD_PORT_C][LCD_BIT_RW]);
- pin_to_bits(lcd_bl_pin, lcd_bits[LCD_PORT_D][LCD_BIT_BL],
+ pin_to_bits(lcd.pins.bl, lcd_bits[LCD_PORT_D][LCD_BIT_BL],
lcd_bits[LCD_PORT_C][LCD_BIT_BL]);
- pin_to_bits(lcd_cl_pin, lcd_bits[LCD_PORT_D][LCD_BIT_CL],
+ pin_to_bits(lcd.pins.cl, lcd_bits[LCD_PORT_D][LCD_BIT_CL],
lcd_bits[LCD_PORT_C][LCD_BIT_CL]);
- pin_to_bits(lcd_da_pin, lcd_bits[LCD_PORT_D][LCD_BIT_DA],
+ pin_to_bits(lcd.pins.da, lcd_bits[LCD_PORT_D][LCD_BIT_DA],
lcd_bits[LCD_PORT_C][LCD_BIT_DA]);

/* before this line, we must NOT send anything to the display.
@@ -2279,6 +2284,23 @@ static int __init panel_init_module(void)
}

/*
+ * Init lcd struct with load-time values to preserve exact current
+ * functionality (at least for now).
+ */
+ lcd.height = lcd_height;
+ lcd.width = lcd_width;
+ lcd.bwidth = lcd_bwidth;
+ lcd.hwidth = lcd_hwidth;
+ lcd.charset = lcd_charset;
+ lcd.proto = lcd_proto;
+ lcd.pins.e = lcd_e_pin;
+ lcd.pins.rs = lcd_rs_pin;
+ lcd.pins.rw = lcd_rw_pin;
+ lcd.pins.cl = lcd_cl_pin;
+ lcd.pins.da = lcd_da_pin;
+ lcd.pins.bl = lcd_bl_pin;
+
+ /*
* Overwrite selection with module param values (both keypad and lcd),
* where the deprecated params have lower prio.
*/
--
2.1.3

2014-11-27 21:37:39

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 9/9] staging: panel: Move LCD-related state into struct lcd

Move more or less all LCD-related state into struct lcd
in order to get better cohesion; use bool instead of int
where it makes sense.

Signed-off-by: Mariusz Gorski <[email protected]>
Acked-by: Willy Tarreau <[email protected]>
---
drivers/staging/panel/panel.c | 255 ++++++++++++++++++++++--------------------
1 file changed, 134 insertions(+), 121 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 19f6767..98325b7 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -225,12 +225,20 @@ static wait_queue_head_t keypad_read_wait;
/* lcd-specific variables */
static struct {
bool enabled;
+ bool initialized;
+ bool must_clear;
+
+ /* TODO: use bool here? */
+ char left_shift;
+
int height;
int width;
int bwidth;
int hwidth;
int charset;
int proto;
+ int light_tempo;
+
/* TODO: use union here? */
struct {
int e;
@@ -240,22 +248,26 @@ static struct {
int da;
int bl;
} pins;
+
+ /* contains the LCD config state */
+ unsigned long int flags;
+
+ /* Contains the LCD X and Y offset */
+ struct {
+ unsigned long int x;
+ unsigned long int y;
+ } addr;
+
+ /* Current escape sequence and it's length or -1 if outside */
+ struct {
+ char buf[LCD_ESCAPE_LEN + 1];
+ int len;
+ } esc_seq;
} lcd;

/* Needed only for init */
static int selected_lcd_type = NOT_SET;

-/* contains the LCD config state */
-static unsigned long int lcd_flags;
-/* contains the LCD X offset */
-static unsigned long int lcd_addr_x;
-/* contains the LCD Y offset */
-static unsigned long int lcd_addr_y;
-/* current escape sequence, 0 terminated */
-static char lcd_escape[LCD_ESCAPE_LEN + 1];
-/* not in escape state. >=0 = escape cmd len */
-static int lcd_escape_len = -1;
-
/*
* Bit masks to convert LCD signals to parallel port outputs.
* _d_ are values for data port, _c_ are for control port.
@@ -438,13 +450,8 @@ static atomic_t keypad_available = ATOMIC_INIT(1);

static struct pardevice *pprt;

-static int lcd_initialized;
static int keypad_initialized;

-static int light_tempo;
-
-static char lcd_must_clear;
-static char lcd_left_shift;
static char init_in_progress;

static void (*lcd_write_cmd)(int);
@@ -880,23 +887,23 @@ static void lcd_write_data_tilcd(int data)
static void lcd_gotoxy(void)
{
lcd_write_cmd(0x80 /* set DDRAM address */
- | (lcd_addr_y ? lcd.hwidth : 0)
+ | (lcd.addr.y ? lcd.hwidth : 0)
/* we force the cursor to stay at the end of the
line if it wants to go farther */
- | ((lcd_addr_x < lcd.bwidth) ? lcd_addr_x &
+ | ((lcd.addr.x < lcd.bwidth) ? lcd.addr.x &
(lcd.hwidth - 1) : lcd.bwidth - 1));
}

static void lcd_print(char c)
{
- if (lcd_addr_x < lcd.bwidth) {
+ if (lcd.addr.x < lcd.bwidth) {
if (lcd_char_conv != NULL)
c = lcd_char_conv[(unsigned char)c];
lcd_write_data(c);
- lcd_addr_x++;
+ lcd.addr.x++;
}
/* prevents the cursor from wrapping onto the next line */
- if (lcd_addr_x == lcd.bwidth)
+ if (lcd.addr.x == lcd.bwidth)
lcd_gotoxy();
}

@@ -905,8 +912,8 @@ static void lcd_clear_fast_s(void)
{
int pos;

- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();

spin_lock_irq(&pprt_lock);
@@ -918,8 +925,8 @@ static void lcd_clear_fast_s(void)
}
spin_unlock_irq(&pprt_lock);

- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
}

@@ -928,8 +935,8 @@ static void lcd_clear_fast_p8(void)
{
int pos;

- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();

spin_lock_irq(&pprt_lock);
@@ -956,8 +963,8 @@ static void lcd_clear_fast_p8(void)
}
spin_unlock_irq(&pprt_lock);

- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
}

@@ -966,8 +973,8 @@ static void lcd_clear_fast_tilcd(void)
{
int pos;

- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();

spin_lock_irq(&pprt_lock);
@@ -979,8 +986,8 @@ static void lcd_clear_fast_tilcd(void)

spin_unlock_irq(&pprt_lock);

- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
}

@@ -988,15 +995,15 @@ static void lcd_clear_fast_tilcd(void)
static void lcd_clear_display(void)
{
lcd_write_cmd(0x01); /* clear display */
- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
/* we must wait a few milliseconds (15) */
long_sleep(15);
}

static void lcd_init_display(void)
{
- lcd_flags = ((lcd.height > 1) ? LCD_FLAG_N : 0)
+ lcd.flags = ((lcd.height > 1) ? LCD_FLAG_N : 0)
| LCD_FLAG_D | LCD_FLAG_C | LCD_FLAG_B;

long_sleep(20); /* wait 20 ms after power-up for the paranoid */
@@ -1009,8 +1016,8 @@ static void lcd_init_display(void)
long_sleep(10);

lcd_write_cmd(0x30 /* set font height and lines number */
- | ((lcd_flags & LCD_FLAG_F) ? 4 : 0)
- | ((lcd_flags & LCD_FLAG_N) ? 8 : 0)
+ | ((lcd.flags & LCD_FLAG_F) ? 4 : 0)
+ | ((lcd.flags & LCD_FLAG_N) ? 8 : 0)
);
long_sleep(10);

@@ -1018,12 +1025,12 @@ static void lcd_init_display(void)
long_sleep(10);

lcd_write_cmd(0x08 /* set display mode */
- | ((lcd_flags & LCD_FLAG_D) ? 4 : 0)
- | ((lcd_flags & LCD_FLAG_C) ? 2 : 0)
- | ((lcd_flags & LCD_FLAG_B) ? 1 : 0)
+ | ((lcd.flags & LCD_FLAG_D) ? 4 : 0)
+ | ((lcd.flags & LCD_FLAG_C) ? 2 : 0)
+ | ((lcd.flags & LCD_FLAG_B) ? 1 : 0)
);

- lcd_backlight((lcd_flags & LCD_FLAG_L) ? 1 : 0);
+ lcd_backlight((lcd.flags & LCD_FLAG_L) ? 1 : 0);

long_sleep(10);

@@ -1046,100 +1053,101 @@ static inline int handle_lcd_special_code(void)

int processed = 0;

- char *esc = lcd_escape + 2;
- int oldflags = lcd_flags;
+ char *esc = lcd.esc_seq.buf + 2;
+ int oldflags = lcd.flags;

/* check for display mode flags */
switch (*esc) {
case 'D': /* Display ON */
- lcd_flags |= LCD_FLAG_D;
+ lcd.flags |= LCD_FLAG_D;
processed = 1;
break;
case 'd': /* Display OFF */
- lcd_flags &= ~LCD_FLAG_D;
+ lcd.flags &= ~LCD_FLAG_D;
processed = 1;
break;
case 'C': /* Cursor ON */
- lcd_flags |= LCD_FLAG_C;
+ lcd.flags |= LCD_FLAG_C;
processed = 1;
break;
case 'c': /* Cursor OFF */
- lcd_flags &= ~LCD_FLAG_C;
+ lcd.flags &= ~LCD_FLAG_C;
processed = 1;
break;
case 'B': /* Blink ON */
- lcd_flags |= LCD_FLAG_B;
+ lcd.flags |= LCD_FLAG_B;
processed = 1;
break;
case 'b': /* Blink OFF */
- lcd_flags &= ~LCD_FLAG_B;
+ lcd.flags &= ~LCD_FLAG_B;
processed = 1;
break;
case '+': /* Back light ON */
- lcd_flags |= LCD_FLAG_L;
+ lcd.flags |= LCD_FLAG_L;
processed = 1;
break;
case '-': /* Back light OFF */
- lcd_flags &= ~LCD_FLAG_L;
+ lcd.flags &= ~LCD_FLAG_L;
processed = 1;
break;
case '*':
/* flash back light using the keypad timer */
if (scan_timer.function != NULL) {
- if (light_tempo == 0 && ((lcd_flags & LCD_FLAG_L) == 0))
+ if (lcd.light_tempo == 0
+ && ((lcd.flags & LCD_FLAG_L) == 0))
lcd_backlight(1);
- light_tempo = FLASH_LIGHT_TEMPO;
+ lcd.light_tempo = FLASH_LIGHT_TEMPO;
}
processed = 1;
break;
case 'f': /* Small Font */
- lcd_flags &= ~LCD_FLAG_F;
+ lcd.flags &= ~LCD_FLAG_F;
processed = 1;
break;
case 'F': /* Large Font */
- lcd_flags |= LCD_FLAG_F;
+ lcd.flags |= LCD_FLAG_F;
processed = 1;
break;
case 'n': /* One Line */
- lcd_flags &= ~LCD_FLAG_N;
+ lcd.flags &= ~LCD_FLAG_N;
processed = 1;
break;
case 'N': /* Two Lines */
- lcd_flags |= LCD_FLAG_N;
+ lcd.flags |= LCD_FLAG_N;
break;
case 'l': /* Shift Cursor Left */
- if (lcd_addr_x > 0) {
+ if (lcd.addr.x > 0) {
/* back one char if not at end of line */
- if (lcd_addr_x < lcd.bwidth)
+ if (lcd.addr.x < lcd.bwidth)
lcd_write_cmd(0x10);
- lcd_addr_x--;
+ lcd.addr.x--;
}
processed = 1;
break;
case 'r': /* shift cursor right */
- if (lcd_addr_x < lcd.width) {
+ if (lcd.addr.x < lcd.width) {
/* allow the cursor to pass the end of the line */
- if (lcd_addr_x <
+ if (lcd.addr.x <
(lcd.bwidth - 1))
lcd_write_cmd(0x14);
- lcd_addr_x++;
+ lcd.addr.x++;
}
processed = 1;
break;
case 'L': /* shift display left */
- lcd_left_shift++;
+ lcd.left_shift++;
lcd_write_cmd(0x18);
processed = 1;
break;
case 'R': /* shift display right */
- lcd_left_shift--;
+ lcd.left_shift--;
lcd_write_cmd(0x1C);
processed = 1;
break;
case 'k': { /* kill end of line */
int x;

- for (x = lcd_addr_x; x < lcd.bwidth; x++)
+ for (x = lcd.addr.x; x < lcd.bwidth; x++)
lcd_write_data(' ');

/* restore cursor position */
@@ -1149,7 +1157,7 @@ static inline int handle_lcd_special_code(void)
}
case 'I': /* reinitialize display */
lcd_init_display();
- lcd_left_shift = 0;
+ lcd.left_shift = 0;
processed = 1;
break;
case 'G': {
@@ -1220,11 +1228,11 @@ static inline int handle_lcd_special_code(void)
while (*esc) {
if (*esc == 'x') {
esc++;
- if (kstrtoul(esc, 10, &lcd_addr_x) < 0)
+ if (kstrtoul(esc, 10, &lcd.addr.x) < 0)
break;
} else if (*esc == 'y') {
esc++;
- if (kstrtoul(esc, 10, &lcd_addr_y) < 0)
+ if (kstrtoul(esc, 10, &lcd.addr.y) < 0)
break;
} else {
break;
@@ -1237,25 +1245,25 @@ static inline int handle_lcd_special_code(void)
}

/* Check whether one flag was changed */
- if (oldflags != lcd_flags) {
+ if (oldflags != lcd.flags) {
/* check whether one of B,C,D flags were changed */
- if ((oldflags ^ lcd_flags) &
+ if ((oldflags ^ lcd.flags) &
(LCD_FLAG_B | LCD_FLAG_C | LCD_FLAG_D))
/* set display mode */
lcd_write_cmd(0x08
- | ((lcd_flags & LCD_FLAG_D) ? 4 : 0)
- | ((lcd_flags & LCD_FLAG_C) ? 2 : 0)
- | ((lcd_flags & LCD_FLAG_B) ? 1 : 0));
+ | ((lcd.flags & LCD_FLAG_D) ? 4 : 0)
+ | ((lcd.flags & LCD_FLAG_C) ? 2 : 0)
+ | ((lcd.flags & LCD_FLAG_B) ? 1 : 0));
/* check whether one of F,N flags was changed */
- else if ((oldflags ^ lcd_flags) & (LCD_FLAG_F | LCD_FLAG_N))
+ else if ((oldflags ^ lcd.flags) & (LCD_FLAG_F | LCD_FLAG_N))
lcd_write_cmd(0x30
- | ((lcd_flags & LCD_FLAG_F) ? 4 : 0)
- | ((lcd_flags & LCD_FLAG_N) ? 8 : 0));
+ | ((lcd.flags & LCD_FLAG_F) ? 4 : 0)
+ | ((lcd.flags & LCD_FLAG_N) ? 8 : 0));
/* check whether L flag was changed */
- else if ((oldflags ^ lcd_flags) & (LCD_FLAG_L)) {
- if (lcd_flags & (LCD_FLAG_L))
+ else if ((oldflags ^ lcd.flags) & (LCD_FLAG_L)) {
+ if (lcd.flags & (LCD_FLAG_L))
lcd_backlight(1);
- else if (light_tempo == 0)
+ else if (lcd.light_tempo == 0)
/* switch off the light only when the tempo
lighting is gone */
lcd_backlight(0);
@@ -1268,29 +1276,29 @@ static inline int handle_lcd_special_code(void)
static void lcd_write_char(char c)
{
/* first, we'll test if we're in escape mode */
- if ((c != '\n') && lcd_escape_len >= 0) {
+ if ((c != '\n') && lcd.esc_seq.len >= 0) {
/* yes, let's add this char to the buffer */
- lcd_escape[lcd_escape_len++] = c;
- lcd_escape[lcd_escape_len] = 0;
+ lcd.esc_seq.buf[lcd.esc_seq.len++] = c;
+ lcd.esc_seq.buf[lcd.esc_seq.len] = 0;
} else {
/* aborts any previous escape sequence */
- lcd_escape_len = -1;
+ lcd.esc_seq.len = -1;

switch (c) {
case LCD_ESCAPE_CHAR:
/* start of an escape sequence */
- lcd_escape_len = 0;
- lcd_escape[lcd_escape_len] = 0;
+ lcd.esc_seq.len = 0;
+ lcd.esc_seq.buf[lcd.esc_seq.len] = 0;
break;
case '\b':
/* go back one char and clear it */
- if (lcd_addr_x > 0) {
+ if (lcd.addr.x > 0) {
/* check if we're not at the
end of the line */
- if (lcd_addr_x < lcd.bwidth)
+ if (lcd.addr.x < lcd.bwidth)
/* back one char */
lcd_write_cmd(0x10);
- lcd_addr_x--;
+ lcd.addr.x--;
}
/* replace with a space */
lcd_write_data(' ');
@@ -1304,15 +1312,15 @@ static void lcd_write_char(char c)
case '\n':
/* flush the remainder of the current line and
go to the beginning of the next line */
- for (; lcd_addr_x < lcd.bwidth; lcd_addr_x++)
+ for (; lcd.addr.x < lcd.bwidth; lcd.addr.x++)
lcd_write_data(' ');
- lcd_addr_x = 0;
- lcd_addr_y = (lcd_addr_y + 1) % lcd.height;
+ lcd.addr.x = 0;
+ lcd.addr.y = (lcd.addr.y + 1) % lcd.height;
lcd_gotoxy();
break;
case '\r':
/* go to the beginning of the same line */
- lcd_addr_x = 0;
+ lcd.addr.x = 0;
lcd_gotoxy();
break;
case '\t':
@@ -1328,32 +1336,32 @@ static void lcd_write_char(char c)

/* now we'll see if we're in an escape mode and if the current
escape sequence can be understood. */
- if (lcd_escape_len >= 2) {
+ if (lcd.esc_seq.len >= 2) {
int processed = 0;

- if (!strcmp(lcd_escape, "[2J")) {
+ if (!strcmp(lcd.esc_seq.buf, "[2J")) {
/* clear the display */
lcd_clear_fast();
processed = 1;
- } else if (!strcmp(lcd_escape, "[H")) {
+ } else if (!strcmp(lcd.esc_seq.buf, "[H")) {
/* cursor to home */
- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
processed = 1;
}
/* codes starting with ^[[L */
- else if ((lcd_escape_len >= 3) &&
- (lcd_escape[0] == '[') &&
- (lcd_escape[1] == 'L')) {
+ else if ((lcd.esc_seq.len >= 3) &&
+ (lcd.esc_seq.buf[0] == '[') &&
+ (lcd.esc_seq.buf[1] == 'L')) {
processed = handle_lcd_special_code();
}

/* LCD special escape codes */
/* flush the escape sequence if it's been processed
or if it is getting too long. */
- if (processed || (lcd_escape_len >= LCD_ESCAPE_LEN))
- lcd_escape_len = -1;
+ if (processed || (lcd.esc_seq.len >= LCD_ESCAPE_LEN))
+ lcd.esc_seq.len = -1;
} /* escape codes */
}

@@ -1386,9 +1394,9 @@ static int lcd_open(struct inode *inode, struct file *file)
if (file->f_mode & FMODE_READ) /* device is write-only */
return -EPERM;

- if (lcd_must_clear) {
+ if (lcd.must_clear) {
lcd_clear_display();
- lcd_must_clear = 0;
+ lcd.must_clear = false;
}
return nonseekable_open(inode, file);
}
@@ -1418,7 +1426,7 @@ static void panel_lcd_print(const char *s)
const char *tmp = s;
int count = strlen(s);

- if (lcd.enabled && lcd_initialized) {
+ if (lcd.enabled && lcd.initialized) {
for (; count-- > 0; tmp++) {
if (!in_interrupt() && (((count + 1) & 0x1f) == 0))
/* let's be a little nice with other processes
@@ -1599,7 +1607,7 @@ static void lcd_init(void)
/* before this line, we must NOT send anything to the display.
* Since lcd_init_display() needs to write data, we have to
* enable mark the LCD initialized just before. */
- lcd_initialized = 1;
+ lcd.initialized = true;
lcd_init_display();

/* display a short message */
@@ -1611,10 +1619,10 @@ static void lcd_init(void)
panel_lcd_print("\x1b[Lc\x1b[Lb\x1b[L*Linux-" UTS_RELEASE "\nPanel-"
PANEL_VERSION);
#endif
- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
/* clear the display on the next device opening */
- lcd_must_clear = 1;
+ lcd.must_clear = true;
lcd_gotoxy();
}

@@ -1950,14 +1958,16 @@ static void panel_scan_timer(void)
panel_process_inputs();
}

- if (lcd.enabled && lcd_initialized) {
+ if (lcd.enabled && lcd.initialized) {
if (keypressed) {
- if (light_tempo == 0 && ((lcd_flags & LCD_FLAG_L) == 0))
+ if (lcd.light_tempo == 0
+ && ((lcd.flags & LCD_FLAG_L) == 0))
lcd_backlight(1);
- light_tempo = FLASH_LIGHT_TEMPO;
- } else if (light_tempo > 0) {
- light_tempo--;
- if (light_tempo == 0 && ((lcd_flags & LCD_FLAG_L) == 0))
+ lcd.light_tempo = FLASH_LIGHT_TEMPO;
+ } else if (lcd.light_tempo > 0) {
+ lcd.light_tempo--;
+ if (lcd.light_tempo == 0
+ && ((lcd.flags & LCD_FLAG_L) == 0))
lcd_backlight(0);
}
}
@@ -2129,7 +2139,7 @@ static void keypad_init(void)
static int panel_notify_sys(struct notifier_block *this, unsigned long code,
void *unused)
{
- if (lcd.enabled && lcd_initialized) {
+ if (lcd.enabled && lcd.initialized) {
switch (code) {
case SYS_DOWN:
panel_lcd_print
@@ -2222,9 +2232,9 @@ static void panel_detach(struct parport *port)
keypad_initialized = 0;
}

- if (lcd.enabled && lcd_initialized) {
+ if (lcd.enabled && lcd.initialized) {
misc_deregister(&lcd_dev);
- lcd_initialized = 0;
+ lcd.initialized = false;
}

parport_release(pprt);
@@ -2300,6 +2310,9 @@ static int __init panel_init_module(void)
lcd.pins.da = lcd_da_pin;
lcd.pins.bl = lcd_bl_pin;

+ /* Leave it for now, just in case */
+ lcd.esc_seq.len = -1;
+
/*
* Overwrite selection with module param values (both keypad and lcd),
* where the deprecated params have lower prio.
@@ -2385,7 +2398,7 @@ static void __exit panel_cleanup_module(void)
panel_lcd_print("\x0cLCD driver " PANEL_VERSION
"\nunloaded.\x1b[Lc\x1b[Lb\x1b[L-");
misc_deregister(&lcd_dev);
- lcd_initialized = 0;
+ lcd.initialized = false;
}

/* TODO: free all input signals */
--
2.1.3

2014-11-27 21:37:33

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 5/9] staging: panel: Start making module params read-only

Start decoupling module params from the actual device state,
both for lcd and keypad, by keeping the params read-only
and moving the device state to related structs.

Signed-off-by: Mariusz Gorski <[email protected]>
Acked-by: Willy Tarreau <[email protected]>
---
drivers/staging/panel/panel.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 5288990..7e5bb53 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -212,6 +212,10 @@ static pmask_t phys_prev;
static char inputs_stable;

/* these variables are specific to the keypad */
+static struct {
+ bool enabled;
+} keypad;
+
static char keypad_buffer[KEYPAD_BUFFER];
static int keypad_buflen;
static int keypad_start;
@@ -219,6 +223,9 @@ static char keypressed;
static wait_queue_head_t keypad_read_wait;

/* lcd-specific variables */
+static struct {
+ bool enabled;
+} lcd;

/* contains the LCD config state */
static unsigned long int lcd_flags;
@@ -1393,7 +1400,7 @@ static void panel_lcd_print(const char *s)
const char *tmp = s;
int count = strlen(s);

- if (lcd_enabled && lcd_initialized) {
+ if (lcd.enabled && lcd_initialized) {
for (; count-- > 0; tmp++) {
if (!in_interrupt() && (((count + 1) & 0x1f) == 0))
/* let's be a little nice with other processes
@@ -1923,7 +1930,7 @@ static void panel_process_inputs(void)

static void panel_scan_timer(void)
{
- if (keypad_enabled && keypad_initialized) {
+ if (keypad.enabled && keypad_initialized) {
if (spin_trylock_irq(&pprt_lock)) {
phys_scan_contacts();

@@ -1935,7 +1942,7 @@ static void panel_scan_timer(void)
panel_process_inputs();
}

- if (lcd_enabled && lcd_initialized) {
+ if (lcd.enabled && lcd_initialized) {
if (keypressed) {
if (light_tempo == 0 && ((lcd_flags & LCD_FLAG_L) == 0))
lcd_backlight(1);
@@ -2114,7 +2121,7 @@ static void keypad_init(void)
static int panel_notify_sys(struct notifier_block *this, unsigned long code,
void *unused)
{
- if (lcd_enabled && lcd_initialized) {
+ if (lcd.enabled && lcd_initialized) {
switch (code) {
case SYS_DOWN:
panel_lcd_print
@@ -2170,13 +2177,13 @@ static void panel_attach(struct parport *port)
/* must init LCD first, just in case an IRQ from the keypad is
* generated at keypad init
*/
- if (lcd_enabled) {
+ if (lcd.enabled) {
lcd_init();
if (misc_register(&lcd_dev))
goto err_unreg_device;
}

- if (keypad_enabled) {
+ if (keypad.enabled) {
keypad_init();
if (misc_register(&keypad_dev))
goto err_lcd_unreg;
@@ -2184,7 +2191,7 @@ static void panel_attach(struct parport *port)
return;

err_lcd_unreg:
- if (lcd_enabled)
+ if (lcd.enabled)
misc_deregister(&lcd_dev);
err_unreg_device:
parport_unregister_device(pprt);
@@ -2202,12 +2209,12 @@ static void panel_detach(struct parport *port)
return;
}

- if (keypad_enabled && keypad_initialized) {
+ if (keypad.enabled && keypad_initialized) {
misc_deregister(&keypad_dev);
keypad_initialized = 0;
}

- if (lcd_enabled && lcd_initialized) {
+ if (lcd.enabled && lcd_initialized) {
misc_deregister(&lcd_dev);
lcd_initialized = 0;
}
@@ -2283,8 +2290,8 @@ static int __init panel_init_module(void)
break;
}

- lcd_enabled = (lcd_type > 0);
- keypad_enabled = (keypad_type > 0);
+ lcd.enabled = (lcd_type > 0);
+ keypad.enabled = (keypad_type > 0);

switch (keypad_type) {
case KEYPAD_TYPE_OLD:
@@ -2309,7 +2316,7 @@ static int __init panel_init_module(void)
return -EIO;
}

- if (!lcd_enabled && !keypad_enabled) {
+ if (!lcd.enabled && !keypad.enabled) {
/* no device enabled, let's release the parport */
if (pprt) {
parport_release(pprt);
@@ -2344,12 +2351,12 @@ static void __exit panel_cleanup_module(void)
del_timer_sync(&scan_timer);

if (pprt != NULL) {
- if (keypad_enabled) {
+ if (keypad.enabled) {
misc_deregister(&keypad_dev);
keypad_initialized = 0;
}

- if (lcd_enabled) {
+ if (lcd.enabled) {
panel_lcd_print("\x0cLCD driver " PANEL_VERSION
"\nunloaded.\x1b[Lc\x1b[Lb\x1b[L-");
misc_deregister(&lcd_dev);
--
2.1.3

2014-11-27 21:38:22

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 8/9] staging: panel: Remove more magic number comparison

Use a defined value instead of magic number comparison
for checking whether a module param value has been set.

Signed-off-by: Mariusz Gorski <[email protected]>
Acked-by: Willy Tarreau <[email protected]>
---
drivers/staging/panel/panel.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 0bdb447..19f6767 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -1494,17 +1494,17 @@ static void lcd_init(void)
}

/* Overwrite with module params set on loading */
- if (lcd_height > -1)
+ if (lcd_height != NOT_SET)
lcd.height = lcd_height;
- if (lcd_width > -1)
+ if (lcd_width != NOT_SET)
lcd.width = lcd_width;
- if (lcd_bwidth > -1)
+ if (lcd_bwidth != NOT_SET)
lcd.bwidth = lcd_bwidth;
- if (lcd_hwidth > -1)
+ if (lcd_hwidth != NOT_SET)
lcd.hwidth = lcd_hwidth;
- if (lcd_charset > -1)
+ if (lcd_charset != NOT_SET)
lcd.charset = lcd_charset;
- if (lcd_proto > -1)
+ if (lcd_proto != NOT_SET)
lcd.proto = lcd_proto;
if (lcd_e_pin != PIN_NOT_SET)
lcd.pins.e = lcd_e_pin;
@@ -2304,16 +2304,16 @@ static int __init panel_init_module(void)
* Overwrite selection with module param values (both keypad and lcd),
* where the deprecated params have lower prio.
*/
- if (keypad_enabled > -1)
+ if (keypad_enabled != NOT_SET)
selected_keypad_type = keypad_enabled;
- if (keypad_type > -1)
+ if (keypad_type != NOT_SET)
selected_keypad_type = keypad_type;

keypad.enabled = (selected_keypad_type > 0);

- if (lcd_enabled > -1)
+ if (lcd_enabled != NOT_SET)
selected_lcd_type = lcd_enabled;
- if (lcd_type > -1)
+ if (lcd_type != NOT_SET)
selected_lcd_type = lcd_type;

lcd.enabled = (selected_lcd_type > 0);
--
2.1.3

2014-11-27 21:38:45

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 6/9] staging: panel: Make two more module params read-only

Make keypad_type and lcd_type module params read-only.
This step also starts making it more clear what is
the precedence of device params coming from different
sources (device profile, runtime module param values etc).

Signed-off-by: Mariusz Gorski <[email protected]>
Acked-by: Willy Tarreau <[email protected]>
---
drivers/staging/panel/panel.c | 71 ++++++++++++++++++++++---------------------
1 file changed, 37 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 7e5bb53..5868a28 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -227,6 +227,9 @@ static struct {
bool enabled;
} lcd;

+/* Needed only for init */
+static int selected_lcd_type = NOT_SET;
+
/* contains the LCD config state */
static unsigned long int lcd_flags;
/* contains the LCD X offset */
@@ -1415,7 +1418,7 @@ static void panel_lcd_print(const char *s)
/* initialize the LCD driver */
static void lcd_init(void)
{
- switch (lcd_type) {
+ switch (selected_lcd_type) {
case LCD_TYPE_OLD:
/* parallel mode, 8 bits */
if (lcd_proto == NOT_SET)
@@ -2233,28 +2236,21 @@ static struct parport_driver panel_driver = {
/* init function */
static int __init panel_init_module(void)
{
- /* for backwards compatibility */
- if (keypad_type == NOT_SET)
- keypad_type = keypad_enabled;
-
- if (lcd_type == NOT_SET)
- lcd_type = lcd_enabled;
+ int selected_keypad_type = NOT_SET;

/* take care of an eventual profile */
switch (profile) {
case PANEL_PROFILE_CUSTOM:
/* custom profile */
- if (keypad_type == NOT_SET)
- keypad_type = DEFAULT_KEYPAD_TYPE;
- if (lcd_type == NOT_SET)
- lcd_type = DEFAULT_LCD_TYPE;
+ selected_keypad_type = DEFAULT_KEYPAD_TYPE;
+ selected_lcd_type = DEFAULT_LCD_TYPE;
break;
case PANEL_PROFILE_OLD:
/* 8 bits, 2*16, old keypad */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_OLD;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_OLD;
+ selected_keypad_type = KEYPAD_TYPE_OLD;
+ selected_lcd_type = LCD_TYPE_OLD;
+
+ /* TODO: This two are a little hacky, sort it out later */
if (lcd_width == NOT_SET)
lcd_width = 16;
if (lcd_hwidth == NOT_SET)
@@ -2262,38 +2258,45 @@ static int __init panel_init_module(void)
break;
case PANEL_PROFILE_NEW:
/* serial, 2*16, new keypad */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_NEW;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_KS0074;
+ selected_keypad_type = KEYPAD_TYPE_NEW;
+ selected_lcd_type = LCD_TYPE_KS0074;
break;
case PANEL_PROFILE_HANTRONIX:
/* 8 bits, 2*16 hantronix-like, no keypad */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_NONE;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_HANTRONIX;
+ selected_keypad_type = KEYPAD_TYPE_NONE;
+ selected_lcd_type = LCD_TYPE_HANTRONIX;
break;
case PANEL_PROFILE_NEXCOM:
/* generic 8 bits, 2*16, nexcom keypad, eg. Nexcom. */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_NEXCOM;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_NEXCOM;
+ selected_keypad_type = KEYPAD_TYPE_NEXCOM;
+ selected_lcd_type = LCD_TYPE_NEXCOM;
break;
case PANEL_PROFILE_LARGE:
/* 8 bits, 2*40, old keypad */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_OLD;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_OLD;
+ selected_keypad_type = KEYPAD_TYPE_OLD;
+ selected_lcd_type = LCD_TYPE_OLD;
break;
}

- lcd.enabled = (lcd_type > 0);
- keypad.enabled = (keypad_type > 0);
+ /*
+ * Overwrite selection with module param values (both keypad and lcd),
+ * where the deprecated params have lower prio.
+ */
+ if (keypad_enabled > -1)
+ selected_keypad_type = keypad_enabled;
+ if (keypad_type > -1)
+ selected_keypad_type = keypad_type;
+
+ keypad.enabled = (selected_keypad_type > 0);
+
+ if (lcd_enabled > -1)
+ selected_lcd_type = lcd_enabled;
+ if (lcd_type > -1)
+ selected_lcd_type = lcd_type;
+
+ lcd.enabled = (selected_lcd_type > 0);

- switch (keypad_type) {
+ switch (selected_keypad_type) {
case KEYPAD_TYPE_OLD:
keypad_profile = old_keypad_profile;
break;
--
2.1.3

2014-11-27 21:39:05

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 4/9] staging: panel: Use defined value or checking module params state

Avoid magic number and use a comparison with a defined value instead
that checks whether module param has been set by the user to some
value at loading time.

Signed-off-by: Mariusz Gorski <[email protected]>
Acked-by: Willy Tarreau <[email protected]>
---
drivers/staging/panel/panel.c | 86 +++++++++++++++++++++----------------------
1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 1b4a211..5288990 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -1411,29 +1411,29 @@ static void lcd_init(void)
switch (lcd_type) {
case LCD_TYPE_OLD:
/* parallel mode, 8 bits */
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_PARALLEL;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_NORMAL;
if (lcd_e_pin == PIN_NOT_SET)
lcd_e_pin = PIN_STROBE;
if (lcd_rs_pin == PIN_NOT_SET)
lcd_rs_pin = PIN_AUTOLF;

- if (lcd_width < 0)
+ if (lcd_width == NOT_SET)
lcd_width = 40;
- if (lcd_bwidth < 0)
+ if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
- if (lcd_hwidth < 0)
+ if (lcd_hwidth == NOT_SET)
lcd_hwidth = 64;
- if (lcd_height < 0)
+ if (lcd_height == NOT_SET)
lcd_height = 2;
break;
case LCD_TYPE_KS0074:
/* serial mode, ks0074 */
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_SERIAL;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_KS0074;
if (lcd_bl_pin == PIN_NOT_SET)
lcd_bl_pin = PIN_AUTOLF;
@@ -1442,20 +1442,20 @@ static void lcd_init(void)
if (lcd_da_pin == PIN_NOT_SET)
lcd_da_pin = PIN_D0;

- if (lcd_width < 0)
+ if (lcd_width == NOT_SET)
lcd_width = 16;
- if (lcd_bwidth < 0)
+ if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
- if (lcd_hwidth < 0)
+ if (lcd_hwidth == NOT_SET)
lcd_hwidth = 16;
- if (lcd_height < 0)
+ if (lcd_height == NOT_SET)
lcd_height = 2;
break;
case LCD_TYPE_NEXCOM:
/* parallel mode, 8 bits, generic */
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_PARALLEL;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_NORMAL;
if (lcd_e_pin == PIN_NOT_SET)
lcd_e_pin = PIN_AUTOLF;
@@ -1464,42 +1464,42 @@ static void lcd_init(void)
if (lcd_rw_pin == PIN_NOT_SET)
lcd_rw_pin = PIN_INITP;

- if (lcd_width < 0)
+ if (lcd_width == NOT_SET)
lcd_width = 16;
- if (lcd_bwidth < 0)
+ if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
- if (lcd_hwidth < 0)
+ if (lcd_hwidth == NOT_SET)
lcd_hwidth = 64;
- if (lcd_height < 0)
+ if (lcd_height == NOT_SET)
lcd_height = 2;
break;
case LCD_TYPE_CUSTOM:
/* customer-defined */
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = DEFAULT_LCD_PROTO;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = DEFAULT_LCD_CHARSET;
/* default geometry will be set later */
break;
case LCD_TYPE_HANTRONIX:
/* parallel mode, 8 bits, hantronix-like */
default:
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_PARALLEL;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_NORMAL;
if (lcd_e_pin == PIN_NOT_SET)
lcd_e_pin = PIN_STROBE;
if (lcd_rs_pin == PIN_NOT_SET)
lcd_rs_pin = PIN_SELECP;

- if (lcd_width < 0)
+ if (lcd_width == NOT_SET)
lcd_width = 16;
- if (lcd_bwidth < 0)
+ if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
- if (lcd_hwidth < 0)
+ if (lcd_hwidth == NOT_SET)
lcd_hwidth = 64;
- if (lcd_height < 0)
+ if (lcd_height == NOT_SET)
lcd_height = 2;
break;
}
@@ -1557,7 +1557,7 @@ static void lcd_init(void)
if (lcd_da_pin == PIN_NOT_SET)
lcd_da_pin = PIN_NONE;

- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = DEFAULT_LCD_CHARSET;

if (lcd_charset == LCD_CHARSET_KS0074)
@@ -2227,58 +2227,58 @@ static struct parport_driver panel_driver = {
static int __init panel_init_module(void)
{
/* for backwards compatibility */
- if (keypad_type < 0)
+ if (keypad_type == NOT_SET)
keypad_type = keypad_enabled;

- if (lcd_type < 0)
+ if (lcd_type == NOT_SET)
lcd_type = lcd_enabled;

/* take care of an eventual profile */
switch (profile) {
case PANEL_PROFILE_CUSTOM:
/* custom profile */
- if (keypad_type < 0)
+ if (keypad_type == NOT_SET)
keypad_type = DEFAULT_KEYPAD_TYPE;
- if (lcd_type < 0)
+ if (lcd_type == NOT_SET)
lcd_type = DEFAULT_LCD_TYPE;
break;
case PANEL_PROFILE_OLD:
/* 8 bits, 2*16, old keypad */
- if (keypad_type < 0)
+ if (keypad_type == NOT_SET)
keypad_type = KEYPAD_TYPE_OLD;
- if (lcd_type < 0)
+ if (lcd_type == NOT_SET)
lcd_type = LCD_TYPE_OLD;
- if (lcd_width < 0)
+ if (lcd_width == NOT_SET)
lcd_width = 16;
- if (lcd_hwidth < 0)
+ if (lcd_hwidth == NOT_SET)
lcd_hwidth = 16;
break;
case PANEL_PROFILE_NEW:
/* serial, 2*16, new keypad */
- if (keypad_type < 0)
+ if (keypad_type == NOT_SET)
keypad_type = KEYPAD_TYPE_NEW;
- if (lcd_type < 0)
+ if (lcd_type == NOT_SET)
lcd_type = LCD_TYPE_KS0074;
break;
case PANEL_PROFILE_HANTRONIX:
/* 8 bits, 2*16 hantronix-like, no keypad */
- if (keypad_type < 0)
+ if (keypad_type == NOT_SET)
keypad_type = KEYPAD_TYPE_NONE;
- if (lcd_type < 0)
+ if (lcd_type == NOT_SET)
lcd_type = LCD_TYPE_HANTRONIX;
break;
case PANEL_PROFILE_NEXCOM:
/* generic 8 bits, 2*16, nexcom keypad, eg. Nexcom. */
- if (keypad_type < 0)
+ if (keypad_type == NOT_SET)
keypad_type = KEYPAD_TYPE_NEXCOM;
- if (lcd_type < 0)
+ if (lcd_type == NOT_SET)
lcd_type = LCD_TYPE_NEXCOM;
break;
case PANEL_PROFILE_LARGE:
/* 8 bits, 2*40, old keypad */
- if (keypad_type < 0)
+ if (keypad_type == NOT_SET)
keypad_type = KEYPAD_TYPE_OLD;
- if (lcd_type < 0)
+ if (lcd_type == NOT_SET)
lcd_type = LCD_TYPE_OLD;
break;
}
--
2.1.3

2014-11-27 21:39:34

by Mariusz Gorski

[permalink] [raw]
Subject: [PATCH v3 2/9] staging: panel: Call init function directly

Remove useless function and let the kernel call the actual
init function directly.

Signed-off-by: Mariusz Gorski <[email protected]>
Acked-by: Willy Tarreau <[email protected]>
---
drivers/staging/panel/panel.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 3dd318a..0d3f09e 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -2222,7 +2222,7 @@ static struct parport_driver panel_driver = {
};

/* init function */
-static int panel_init(void)
+static int __init panel_init_module(void)
{
/* for backwards compatibility */
if (keypad_type < 0)
@@ -2334,11 +2334,6 @@ static int panel_init(void)
return 0;
}

-static int __init panel_init_module(void)
-{
- return panel_init();
-}
-
static void __exit panel_cleanup_module(void)
{
unregister_reboot_notifier(&panel_notifier);
--
2.1.3