mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/9] staging: panel: Refactor panel initialization
@ 2014-11-27 21:36 Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 1/9] staging: panel: Set default parport module param value Mariusz Gorski
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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 <w@1wt.eu>

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/9] staging: panel: Set default parport module param value
  2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
@ 2014-11-27 21:36 ` Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 2/9] staging: panel: Call init function directly Mariusz Gorski
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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

Signed-off-by: Mariusz Gorski <marius.gorski@gmail.com>
Acked-by: Willy Tarreau <w@1wt.eu>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 2/9] staging: panel: Call init function directly
  2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 1/9] staging: panel: Set default parport module param value Mariusz Gorski
@ 2014-11-27 21:36 ` Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 3/9] staging: panel: Remove magic numbers Mariusz Gorski
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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

Signed-off-by: Mariusz Gorski <marius.gorski@gmail.com>
Acked-by: Willy Tarreau <w@1wt.eu>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 3/9] staging: panel: Remove magic numbers
  2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 1/9] staging: panel: Set default parport module param value Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 2/9] staging: panel: Call init function directly Mariusz Gorski
@ 2014-11-27 21:36 ` Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 4/9] staging: panel: Use defined value or checking module params state Mariusz Gorski
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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 <marius.gorski@gmail.com>
Acked-by: Willy Tarreau <w@1wt.eu>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 4/9] staging: panel: Use defined value or checking module params state
  2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
                   ` (2 preceding siblings ...)
  2014-11-27 21:36 ` [PATCH v3 3/9] staging: panel: Remove magic numbers Mariusz Gorski
@ 2014-11-27 21:36 ` Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 5/9] staging: panel: Start making module params read-only Mariusz Gorski
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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 <marius.gorski@gmail.com>
Acked-by: Willy Tarreau <w@1wt.eu>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 5/9] staging: panel: Start making module params read-only
  2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
                   ` (3 preceding siblings ...)
  2014-11-27 21:36 ` [PATCH v3 4/9] staging: panel: Use defined value or checking module params state Mariusz Gorski
@ 2014-11-27 21:36 ` Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 6/9] staging: panel: Make two more " Mariusz Gorski
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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 <marius.gorski@gmail.com>
Acked-by: Willy Tarreau <w@1wt.eu>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 6/9] staging: panel: Make two more module params read-only
  2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
                   ` (4 preceding siblings ...)
  2014-11-27 21:36 ` [PATCH v3 5/9] staging: panel: Start making module params read-only Mariusz Gorski
@ 2014-11-27 21:36 ` Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 7/9] staging: panel: Refactor LCD init code Mariusz Gorski
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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 <marius.gorski@gmail.com>
Acked-by: Willy Tarreau <w@1wt.eu>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 7/9] staging: panel: Refactor LCD init code
  2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
                   ` (5 preceding siblings ...)
  2014-11-27 21:36 ` [PATCH v3 6/9] staging: panel: Make two more " Mariusz Gorski
@ 2014-11-27 21:36 ` Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 8/9] staging: panel: Remove more magic number comparison Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 9/9] staging: panel: Move LCD-related state into struct lcd Mariusz Gorski
  8 siblings, 0 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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 <marius.gorski@gmail.com>
Acked-by: Willy Tarreau <w@1wt.eu>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 8/9] staging: panel: Remove more magic number comparison
  2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
                   ` (6 preceding siblings ...)
  2014-11-27 21:36 ` [PATCH v3 7/9] staging: panel: Refactor LCD init code Mariusz Gorski
@ 2014-11-27 21:36 ` Mariusz Gorski
  2014-11-27 21:36 ` [PATCH v3 9/9] staging: panel: Move LCD-related state into struct lcd Mariusz Gorski
  8 siblings, 0 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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

Signed-off-by: Mariusz Gorski <marius.gorski@gmail.com>
Acked-by: Willy Tarreau <w@1wt.eu>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 9/9] staging: panel: Move LCD-related state into struct lcd
  2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
                   ` (7 preceding siblings ...)
  2014-11-27 21:36 ` [PATCH v3 8/9] staging: panel: Remove more magic number comparison Mariusz Gorski
@ 2014-11-27 21:36 ` Mariusz Gorski
  8 siblings, 0 replies; 10+ messages in thread
From: Mariusz Gorski @ 2014-11-27 21:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Willy Tarreau; +Cc: devel, linux-kernel

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 <marius.gorski@gmail.com>
Acked-by: Willy Tarreau <w@1wt.eu>
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-11-27 21:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-27 21:36 [PATCH v3 0/9] staging: panel: Refactor panel initialization Mariusz Gorski
2014-11-27 21:36 ` [PATCH v3 1/9] staging: panel: Set default parport module param value Mariusz Gorski
2014-11-27 21:36 ` [PATCH v3 2/9] staging: panel: Call init function directly Mariusz Gorski
2014-11-27 21:36 ` [PATCH v3 3/9] staging: panel: Remove magic numbers Mariusz Gorski
2014-11-27 21:36 ` [PATCH v3 4/9] staging: panel: Use defined value or checking module params state Mariusz Gorski
2014-11-27 21:36 ` [PATCH v3 5/9] staging: panel: Start making module params read-only Mariusz Gorski
2014-11-27 21:36 ` [PATCH v3 6/9] staging: panel: Make two more " Mariusz Gorski
2014-11-27 21:36 ` [PATCH v3 7/9] staging: panel: Refactor LCD init code Mariusz Gorski
2014-11-27 21:36 ` [PATCH v3 8/9] staging: panel: Remove more magic number comparison Mariusz Gorski
2014-11-27 21:36 ` [PATCH v3 9/9] staging: panel: Move LCD-related state into struct lcd Mariusz Gorski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®