lkml.org 
[lkml]   [2012]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH 1/3] staging:panel: Rewrite for fixing synchronization issues
Partial rewrite of panel driver to make it more robust. The old code contains
problems potentially leading to kernel oops, deadlocks and memory leaks.

The patch contains the following changes:
* Handle asynchronous attach/detach to/from parport.
* Proper synchronization of multiple opens to same device.
* Proper disabling of softirqs when locking pprt from user context.
* Replaced interruptible_sleep_on in keypad_read to avoid race conditions.
* Changed to del_timer_sync to make sure timer function finishes before
unloading module.
* Added owner to file_operations to avoid module being unloaded with dangling
opens.
* Check status of user-mode memory write in keypad_read.
* Check for and handle errors in keypad_init.
* Properly free memory for logical_inputs to avoid memory leak on unload.

Signed-off-by: Zoltan Kelemen <zoltan@digisec.se>
----
diff -ru linux-next/drivers/staging/panel/panel.c panel-patch1/drivers/staging/panel/panel.c
--- linux-next/drivers/staging/panel/panel.c 2012-06-27 05:03:21.000000000 +0200
+++ panel-patch1/drivers/staging/panel/panel.c 2012-06-28 11:49:21.034467337 +0200
@@ -1,6 +1,7 @@
/*
* Front panel driver for Linux
* Copyright (C) 2000-2008, Willy Tarreau <w@1wt.eu>
+ * Partially rewritten 2012, Zoltan Kelemen <zoltan@digisec.se>
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -21,14 +22,9 @@
* Several profiles are provided for commonly found LCD+keypad modules on the
* market, such as those found in Nexcom's appliances.
*
- * FIXME:
- * - the initialization/deinitialization process is very dirty and should
- * be rewritten. It may even be buggy.
- *
* TODO:
* - document 24 keys keyboard (3 rows of 8 cols, 32 diodes + 2 inputs)
* - make the LCD a part of a virtual screen of Vx*Vy
- * - make the inputs list smp-safe
* - change the keyboard to a double mapping : signals -> key_id -> values
* so that applications can change values without knowing signals
*
@@ -62,7 +58,7 @@
#define LCD_MINOR 156
#define KEYPAD_MINOR 185

-#define PANEL_VERSION "0.9.5"
+#define PANEL_VERSION "0.9.6"

#define LCD_MAXBYTES 256 /* max burst write */

@@ -70,6 +66,7 @@

/* poll the keyboard this every second */
#define INPUT_POLL_TIME (HZ/50)
+
/* a key starts to repeat after this times INPUT_POLL_TIME */
#define KEYPAD_REP_START (10)
/* a key repeats this times INPUT_POLL_TIME */
@@ -129,7 +126,7 @@
#define LCD_FLAG_L 0x0080 /* backlight enabled */

#define LCD_ESCAPE_LEN 24 /* max chars for LCD escape command */
-#define LCD_ESCAPE_CHAR 27 /* use char 27 for escape command */
+#define LCD_ESCAPE_CHAR 27 /* use char 27 for escape command */

/* macros to simplify use of the parallel port */
#define r_ctr(x) (parport_read_control((x)->port))
@@ -138,12 +135,6 @@
#define w_ctr(x, y) do { parport_write_control((x)->port, (y)); } while (0)
#define w_dtr(x, y) do { parport_write_data((x)->port, (y)); } while (0)

-/* this defines which bits are to be used and which ones to be ignored */
-/* logical or of the output bits involved in the scan matrix */
-static __u8 scan_mask_o;
-/* logical or of the input bits involved in the scan matrix */
-static __u8 scan_mask_i;
-
typedef __u64 pmask_t;

enum input_type {
@@ -183,8 +174,6 @@
} u;
};

-LIST_HEAD(logical_inputs); /* list of all defined logical inputs */
-
/* physical contacts history
* Physical contacts are a 45 bits string of 9 groups of 5 bits each.
* The 8 lower groups correspond to output bits 0 to 7, and the 9th group
@@ -196,6 +185,9 @@
* <-----unused------><gnd><d07><d06><d05><d04><d03><d02><d01><d00>
*/

+/*
+ * Group of keypad variables accessed only from timer function.
+ */
/* what has just been read from the I/O ports */
static pmask_t phys_read;
/* previous phys_read */
@@ -207,25 +199,67 @@
/* 0 means that at least one logical signal needs be computed */
static char inputs_stable;

-/* these variables are specific to the keypad */
+/*
+ * Group of keypad variables shared between timer/process context code,
+ * protected by a spinlock. Keypresses are stored at the write end of the
+ * circular buffer, when the keypad device is open.
+ */
+static DEFINE_SPINLOCK(keypad_lock);
+static int keypad_open_cnt;
static char keypad_buffer[KEYPAD_BUFFER];
-static int keypad_buflen;
static int keypad_start;
-static char keypressed;
-static wait_queue_head_t keypad_read_wait;
+static int keypad_buflen;
+
+/* Wait queue shared by process/timer context used for blocked reads */
+static DECLARE_WAIT_QUEUE_HEAD(keypad_read_wait);
+
+/*
+ * Keypad variables initialized once when attaching to parallel port, there-
+ * after used as readonly or updated only from timer function.
+ */
+static int keypad_initialized; /* Nonzero when vars below initialized */
+static LIST_HEAD(logical_inputs); /* List of all defined logical inputs */
+static __u8 scan_mask_o; /* Output bits involved in scan matrix */
+static __u8 scan_mask_i; /* Input bits involved in scan matrix */
+static int keypressed; /* Key pressed (light backlight) */
+
+/* LCD vars used from timer */
+static int light_tempo; /* Time left before backlight off */
+static int backlight_on; /* Nonzero when backlight is on */
+static struct timer_list scan_timer; /* Timer used for keypad scanning */
+static int timer_running; /* Nonzero when timer enabled */

-/* lcd-specific variables */
+/*
+ * Group of parallel-port-related variables. They are protected in process
+ * context with a mutex.
+ *
+ * The system basically has two states: port available (pprt != NULL) and port
+ * unavailable (pprt == NULL). When the port is available all globals are
+ * initialized and valid and the timer function may be running. When port is
+ * unavailable the timer is not running and most globals are undefined.
+ */
+static DEFINE_MUTEX(port_mutex); /* Protects variables below */
+static struct pardevice *pprt; /* Non-NULL when port is available */
+static unsigned long int lcd_flags; /* Contains the LCD config state */
+static unsigned long int lcd_addr_x; /* Contains the LCD X offset */
+static unsigned long int lcd_addr_y; /* Contains the LCD Y offset */
+static char lcd_must_clear; /* Clear display on next device open */
+static char lcd_escape[LCD_ESCAPE_LEN+1]; /* Current escape sequence */
+static int lcd_escape_len; /* >=0 = escape cmd len, -1 no escape */
+
+/*
+ * Protects port accesses between timer/process context code and the variables
+ * below.
+ */
+static DEFINE_SPINLOCK(pprt_lock);
+static int backlight_flash; /* Turn on backlight for short time */
+static int force_backlight_on; /* Nonzero to lock backlight on */

-/* 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;
+static unsigned long lcd_is_open; /* Nonzero if LCD device opened */
+
+static void (*lcd_write_cmd) (int);
+static void (*lcd_write_data) (int);
+static void (*lcd_clear_fast) (void);

/*
* Bit masks to convert LCD signals to parallel port outputs.
@@ -401,27 +435,6 @@

#endif /* DEFAULT_PROFILE == 0 */

-/* global variables */
-static int keypad_open_cnt; /* #times opened */
-static int lcd_open_cnt; /* #times opened */
-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);
-static void (*lcd_write_data) (int);
-static void (*lcd_clear_fast) (void);
-
-static DEFINE_SPINLOCK(pprt_lock);
-static struct timer_list scan_timer;
-
MODULE_DESCRIPTION("Generic parallel port LCD/Keypad driver");

static int parport = -1;
@@ -609,7 +622,7 @@
unsigned char da; /* serial LCD data */
} bits;

-static void init_scan_timer(void);
+static void start_scan_timer(void);

/* sets data port bits according to current signals values */
static int set_data_bits(void)
@@ -667,7 +680,7 @@
* out(dport, in(dport) & d_val[2] | d_val[signal_state])
* out(cport, in(cport) & c_val[2] | c_val[signal_state])
*/
-void pin_to_bits(int pin, unsigned char *d_val, unsigned char *c_val)
+static void pin_to_bits(int pin, unsigned char *d_val, unsigned char *c_val)
{
int d_bit, c_bit, inv;

@@ -748,45 +761,74 @@
}
}

-/* turn the backlight on or off */
-static void lcd_backlight(int on)
+
+/*
+ * Lock/unlock parallel port for use between timer/process context. Used by
+ * most low-level functions accessing the port. Assumed that caller is
+ * process context since we disable softirqs. The timer function locks the port
+ * itself and only calls unlocked functions.
+ */
+static void lock_pprt(void)
+{
+ spin_lock_bh(&pprt_lock);
+}
+
+static void unlock_pprt(void)
+{
+ spin_unlock_bh(&pprt_lock);
+}
+
+
+/* turn the backlight on or off. assumes caller grabbed the spin lock */
+static void lcd_backlight_unlocked(int on)
{
if (lcd_bl_pin == PIN_NONE)
return;

- /* The backlight is activated by setting the AUTOFEED line to +5V */
- spin_lock(&pprt_lock);
+ /* The backlight is activated by setting the backlight line */
bits.bl = on;
+ backlight_on = on;
panel_set_bits();
- spin_unlock(&pprt_lock);
+}
+
+/*
+ * Turn backlight on from process context. Remember backlight setting, if
+ * enabled here it is not turned off when expired in the timer.
+ */
+static void lcd_backlight(int on)
+{
+ lock_pprt();
+ lcd_backlight_unlocked(on);
+ force_backlight_on = on;
+ unlock_pprt();
}

/* send a command to the LCD panel in serial mode */
static void lcd_write_cmd_s(int cmd)
{
- spin_lock(&pprt_lock);
+ lock_pprt();
lcd_send_serial(0x1F); /* R/W=W, RS=0 */
lcd_send_serial(cmd & 0x0F);
lcd_send_serial((cmd >> 4) & 0x0F);
udelay(40); /* the shortest command takes at least 40 us */
- spin_unlock(&pprt_lock);
+ unlock_pprt();
}

/* send data to the LCD panel in serial mode */
static void lcd_write_data_s(int data)
{
- spin_lock(&pprt_lock);
+ lock_pprt();
lcd_send_serial(0x5F); /* R/W=W, RS=1 */
lcd_send_serial(data & 0x0F);
lcd_send_serial((data >> 4) & 0x0F);
udelay(40); /* the shortest data takes at least 40 us */
- spin_unlock(&pprt_lock);
+ unlock_pprt();
}

/* send a command to the LCD panel in 8 bits parallel mode */
static void lcd_write_cmd_p8(int cmd)
{
- spin_lock(&pprt_lock);
+ lock_pprt();
/* present the data to the data port */
w_dtr(pprt, cmd);
udelay(20); /* maintain the data during 20 us before the strobe */
@@ -802,13 +844,13 @@
set_ctrl_bits();

udelay(120); /* the shortest command takes at least 120 us */
- spin_unlock(&pprt_lock);
+ unlock_pprt();
}

/* send data to the LCD panel in 8 bits parallel mode */
static void lcd_write_data_p8(int data)
{
- spin_lock(&pprt_lock);
+ lock_pprt();
/* present the data to the data port */
w_dtr(pprt, data);
udelay(20); /* maintain the data during 20 us before the strobe */
@@ -824,27 +866,27 @@
set_ctrl_bits();

udelay(45); /* the shortest data takes at least 45 us */
- spin_unlock(&pprt_lock);
+ unlock_pprt();
}

/* send a command to the TI LCD panel */
static void lcd_write_cmd_tilcd(int cmd)
{
- spin_lock(&pprt_lock);
+ lock_pprt();
/* present the data to the control port */
w_ctr(pprt, cmd);
udelay(60);
- spin_unlock(&pprt_lock);
+ unlock_pprt();
}

/* send data to the TI LCD panel */
static void lcd_write_data_tilcd(int data)
{
- spin_lock(&pprt_lock);
+ lock_pprt();
/* present the data to the data port */
w_dtr(pprt, data);
udelay(60);
- spin_unlock(&pprt_lock);
+ unlock_pprt();
}

static void lcd_gotoxy(void)
@@ -877,14 +919,14 @@
lcd_addr_x = lcd_addr_y = 0;
lcd_gotoxy();

- spin_lock(&pprt_lock);
+ lock_pprt();
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);
udelay(40); /* the shortest data takes at least 40 us */
}
- spin_unlock(&pprt_lock);
+ unlock_pprt();

lcd_addr_x = lcd_addr_y = 0;
lcd_gotoxy();
@@ -897,7 +939,7 @@
lcd_addr_x = lcd_addr_y = 0;
lcd_gotoxy();

- spin_lock(&pprt_lock);
+ lock_pprt();
for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
/* present the data to the data port */
w_dtr(pprt, ' ');
@@ -919,7 +961,7 @@
/* the shortest data takes at least 45 us */
udelay(45);
}
- spin_unlock(&pprt_lock);
+ unlock_pprt();

lcd_addr_x = lcd_addr_y = 0;
lcd_gotoxy();
@@ -932,14 +974,14 @@
lcd_addr_x = lcd_addr_y = 0;
lcd_gotoxy();

- spin_lock(&pprt_lock);
+ lock_pprt();
for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
/* present the data to the data port */
w_dtr(pprt, ' ');
udelay(60);
}

- spin_unlock(&pprt_lock);
+ unlock_pprt();

lcd_addr_x = lcd_addr_y = 0;
lcd_gotoxy();
@@ -995,12 +1037,13 @@
}

/*
- * These are the file operation function for user access to /dev/lcd
+ * These are the file operation functions for user access to /dev/lcd
* This function can also be called from inside the kernel, by
* setting file and ppos to NULL.
*
*/

+/* Called with port mutex held and port being available */
static inline int handle_lcd_special_code(void)
{
/* LCD special codes */
@@ -1044,13 +1087,10 @@
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))
- lcd_backlight(1);
- light_tempo = FLASH_LIGHT_TEMPO;
- }
+ case '*': /* Flash back light using the keypad timer */
+ lock_pprt();
+ backlight_flash = 1;
+ unlock_pprt();
processed = 1;
break;
case 'f': /* Small Font */
@@ -1088,12 +1128,10 @@
processed = 1;
break;
case 'L': /* shift display left */
- lcd_left_shift++;
lcd_write_cmd(0x18);
processed = 1;
break;
case 'R': /* shift display right */
- lcd_left_shift--;
lcd_write_cmd(0x1C);
processed = 1;
break;
@@ -1109,7 +1147,6 @@
}
case 'I': /* reinitialize display */
lcd_init_display();
- lcd_left_shift = 0;
processed = 1;
break;
case 'G': {
@@ -1211,36 +1248,27 @@
| ((lcd_flags & LCD_FLAG_F) ? 4 : 0)
| ((lcd_flags & LCD_FLAG_N) ? 8 : 0));
/* check wether L flag was changed */
- else if ((oldflags ^ lcd_flags) & (LCD_FLAG_L)) {
- if (lcd_flags & (LCD_FLAG_L))
- lcd_backlight(1);
- else if (light_tempo == 0)
- /* switch off the light only when the tempo
- lighting is gone */
- lcd_backlight(0);
- }
+ else if ((oldflags ^ lcd_flags) & (LCD_FLAG_L))
+ lcd_backlight((lcd_flags & LCD_FLAG_L) ? 1 : 0);
}

return processed;
}

-static ssize_t lcd_write(struct file *file,
- const char *buf, size_t count, loff_t *ppos)
+
+static ssize_t lcd_write_unlocked(struct file *file,
+ const char *buf, size_t count, loff_t *ppos)
{
const char *tmp = buf;
char c;

for (; count-- > 0; (ppos ? (*ppos)++ : 0), ++tmp) {
- if (!in_interrupt() && (((count + 1) & 0x1f) == 0))
- /* let's be a little nice with other processes
- that need some CPU */
- schedule();
-
if (ppos == NULL && file == NULL)
/* let's not use get_user() from the kernel ! */
c = *tmp;
- else if (get_user(c, tmp))
+ else if (get_user(c, tmp)) {
return -EFAULT;
+ }

/* first, we'll test if we're in escape mode */
if ((c != '\n') && lcd_escape_len >= 0) {
@@ -1334,29 +1362,67 @@
return tmp - buf;
}

-static int lcd_open(struct inode *inode, struct file *file)
+
+static ssize_t lcd_write(struct file *file,
+ const char *buf, size_t count, loff_t *ppos)
{
- if (lcd_open_cnt)
- return -EBUSY; /* open only once at a time */
+ ssize_t ret;
+
+ /* Protect access to the port and check that it's still available */
+ mutex_lock(&port_mutex);

- if (file->f_mode & FMODE_READ) /* device is write-only */
+ if (pprt == NULL) {
+ mutex_unlock(&port_mutex);
+ return -ENODEV;
+ }
+
+ ret = lcd_write_unlocked(file, buf, count, ppos);
+
+ mutex_unlock(&port_mutex);
+
+ return ret;
+}
+
+
+static int lcd_open(struct inode *inode, struct file *file)
+{
+ /* Device is write-only */
+ if (file->f_mode & FMODE_READ)
return -EPERM;

+ /* Only one open allowed at a time */
+ if (test_and_set_bit(0, &lcd_is_open))
+ return -EBUSY;
+
+ mutex_lock(&port_mutex);
+
+ /* Check for port still being available */
+ if (!pprt) {
+ mutex_unlock(&port_mutex);
+ return -ENODEV;
+ }
+
+ lcd_escape_len = -1;
+
+ /* Clear display if needed */
if (lcd_must_clear) {
lcd_clear_display();
lcd_must_clear = 0;
}
- lcd_open_cnt++;
+
+ mutex_unlock(&port_mutex);
+
return nonseekable_open(inode, file);
}

static int lcd_release(struct inode *inode, struct file *file)
{
- lcd_open_cnt--;
+ clear_bit(0, &lcd_is_open);
return 0;
}

static const struct file_operations lcd_fops = {
+ .owner = THIS_MODULE,
.write = lcd_write,
.open = lcd_open,
.release = lcd_release,
@@ -1369,15 +1435,29 @@
&lcd_fops
};

+/*
+ * Print LCD messages, assumes caller has taken port mutex and checked that
+ * the port is available.
+ */
+static void panel_lcd_print_unlocked(char *s)
+{
+ lcd_write_unlocked(NULL, s, strlen(s), NULL);
+}
+
+
/* public function usable from the kernel for any purpose */
-void panel_lcd_print(char *s)
+static void panel_lcd_print(char *s)
{
- if (lcd_enabled && lcd_initialized)
- lcd_write(NULL, s, strlen(s), NULL);
+ mutex_lock(&port_mutex);
+
+ if (pprt)
+ panel_lcd_print_unlocked(s);
+
+ mutex_unlock(&port_mutex);
}

-/* initialize the LCD driver */
-void lcd_init(void)
+/* Initialize LCD driver module globals */
+static void lcd_init_globals(void)
{
switch (lcd_type) {
case LCD_TYPE_OLD:
@@ -1536,9 +1616,6 @@
else
lcd_char_conv = NULL;

- if (lcd_bl_pin != PIN_NONE)
- init_scan_timer();
-
pin_to_bits(lcd_e_pin, 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],
@@ -1551,21 +1628,24 @@
lcd_bits[LCD_PORT_C][LCD_BIT_CL]);
pin_to_bits(lcd_da_pin, 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.
- * Since lcd_init_display() needs to write data, we have to
- * enable mark the LCD initialized just before. */
- lcd_initialized = 1;
+/* Do LCD panel initialization. Caller must hold port mutex */
+static void lcd_init(void)
+{
lcd_init_display();

+ lcd_escape_len = -1;
+
/* display a short message */
#ifdef CONFIG_PANEL_CHANGE_MESSAGE
#ifdef CONFIG_PANEL_BOOT_MESSAGE
- panel_lcd_print("\x1b[Lc\x1b[Lb\x1b[L*" CONFIG_PANEL_BOOT_MESSAGE);
+ panel_lcd_print_unlocked("\x1b[Lc\x1b[Lb\x1b[L*"
+ CONFIG_PANEL_BOOT_MESSAGE);
#endif
#else
- panel_lcd_print("\x1b[Lc\x1b[Lb\x1b[L*Linux-" UTS_RELEASE "\nPanel-"
- PANEL_VERSION);
+ panel_lcd_print_unlocked("\x1b[Lc\x1b[Lb\x1b[L*Linux-" UTS_RELEASE
+ "\nPanel-" PANEL_VERSION);
#endif
lcd_addr_x = lcd_addr_y = 0;
/* clear the display on the next device opening */
@@ -1573,6 +1653,14 @@
lcd_gotoxy();
}

+
+/* Do LCD panel cleanup. Caller must hold port mutex */
+static void lcd_cleanup(void)
+{
+ panel_lcd_print_unlocked("\x0cLCD driver " PANEL_VERSION
+ "\nstopped.\x1b[Lc\x1b[Lb\x1b[L-");
+}
+
/*
* These are the file operation function for user access to /dev/keypad
*/
@@ -1580,53 +1668,88 @@
static ssize_t keypad_read(struct file *file,
char *buf, size_t count, loff_t *ppos)
{
+ static char key_buf[KEYPAD_BUFFER];
+ int i;

- unsigned i = *ppos;
- char *tmp = buf;
+ if (count == 0)
+ return 0;
+
+ /*
+ * In case data is available in the global buffer, copy it to our local
+ * buffer. This is needed since we can't copy to the user buffer while
+ * holding the spinlock. Our local buffer can be static since there can
+ * only be one opener of the device.
+ *
+ * In case there is no data available, we put the process on a wait
+ * queue. It will be woken up when data arrives.
+ */
+ spin_lock_bh(&keypad_lock);
+
+ while (keypad_buflen == 0) {
+ spin_unlock_bh(&keypad_lock);

- if (keypad_buflen == 0) {
if (file->f_flags & O_NONBLOCK)
return -EAGAIN;

- interruptible_sleep_on(&keypad_read_wait);
- if (signal_pending(current))
- return -EINTR;
+ if (wait_event_interruptible(keypad_read_wait,
+ (keypad_buflen != 0)))
+ return -ERESTARTSYS;
+
+ spin_lock_bh(&keypad_lock);
}

- for (; count-- > 0 && (keypad_buflen > 0);
- ++i, ++tmp, --keypad_buflen) {
- put_user(keypad_buffer[keypad_start], tmp);
+ for (i = 0; (size_t) i < count && i < keypad_buflen; i++) {
+ key_buf[i] = keypad_buffer[keypad_start];
keypad_start = (keypad_start + 1) % KEYPAD_BUFFER;
}
- *ppos = i;
+ keypad_buflen -= i;

- return tmp - buf;
+ spin_unlock_bh(&keypad_lock);
+
+ if (copy_to_user(buf, key_buf, i))
+ return -EFAULT;
+
+ (*ppos) += (loff_t) i;
+
+ return (ssize_t) i;
}

static int keypad_open(struct inode *inode, struct file *file)
{
-
- if (keypad_open_cnt)
- return -EBUSY; /* open only once at a time */
+ int r;

if (file->f_mode & FMODE_WRITE) /* device is read-only */
return -EPERM;

- keypad_buflen = 0; /* flush the buffer on opening */
- keypad_open_cnt++;
- return 0;
+ spin_lock_bh(&keypad_lock);
+
+ if (keypad_open_cnt)
+ r = -EBUSY; /* open only once at a time */
+ else {
+ keypad_open_cnt++;
+ keypad_buflen = 0; /* flush the buffer on opening */
+ keypad_start = 0;
+ r = 0;
+ }
+
+ spin_unlock_bh(&keypad_lock);
+
+ return r;
}

static int keypad_release(struct inode *inode, struct file *file)
{
+ spin_lock_bh(&keypad_lock);
keypad_open_cnt--;
+ spin_unlock_bh(&keypad_lock);
return 0;
}

static const struct file_operations keypad_fops = {
- .read = keypad_read, /* read */
- .open = keypad_open, /* open */
- .release = keypad_release, /* close */
+ .owner = THIS_MODULE,
+ .read = keypad_read,
+ .open = keypad_open,
+ .release = keypad_release,
.llseek = default_llseek,
};

@@ -1638,17 +1761,23 @@

static void keypad_send_key(char *string, int max_len)
{
- if (init_in_progress)
- return;
+ int wake = 0;

/* send the key to the device only if a process is attached to it. */
+ spin_lock(&keypad_lock);
+
if (keypad_open_cnt > 0) {
while (max_len-- && keypad_buflen < KEYPAD_BUFFER && *string) {
keypad_buffer[(keypad_start + keypad_buflen++) %
KEYPAD_BUFFER] = *string++;
}
- wake_up_interruptible(&keypad_read_wait);
+ wake = 1;
}
+
+ spin_unlock(&keypad_lock);
+
+ if (wake)
+ wake_up_interruptible(&keypad_read_wait);
}

/* this function scans all the bits involving at least one logical signal,
@@ -1660,6 +1789,8 @@
* reads will be kept as they previously were in their logical form (phys_prev).
* A signal which has just switched will have a 1 in
* (phys_read ^ phys_read_prev).
+ *
+ * Assumes caller holds port spinlock.
*/
static void phys_scan_contacts(void)
{
@@ -1838,9 +1969,8 @@
struct logical_input *input;

#if 0
- printk(KERN_DEBUG
- "entering panel_process_inputs with pp=%016Lx & pc=%016Lx\n",
- phys_prev, phys_curr);
+ pr_debug("entering panel_process_inputs with pp=%016Lx & pc=%016Lx\n",
+ phys_prev, phys_curr);
#endif

keypressed = 0;
@@ -1889,45 +2019,64 @@

static void panel_scan_timer(void)
{
- if (keypad_enabled && keypad_initialized) {
- if (spin_trylock(&pprt_lock)) {
- phys_scan_contacts();
-
- /* no need for the parport anymore */
- spin_unlock(&pprt_lock);
- }
+ /* If keypad available scan keys and send to waiting process */
+ if (keypad_initialized) {
+ spin_lock(&pprt_lock);
+ phys_scan_contacts();
+ spin_unlock(&pprt_lock);

if (!inputs_stable || phys_curr != phys_prev)
panel_process_inputs();
}

- if (lcd_enabled && lcd_initialized) {
- if (keypressed) {
- if (light_tempo == 0 && ((lcd_flags & LCD_FLAG_L) == 0))
- lcd_backlight(1);
+ /* Turn backlight on/off if available */
+ if (lcd_enabled && lcd_bl_pin != PIN_NONE) {
+ spin_lock(&pprt_lock);
+
+ if (backlight_flash || keypressed) {
+ backlight_flash = 0;
+ if (light_tempo == 0 && !backlight_on)
+ lcd_backlight_unlocked(1);
light_tempo = FLASH_LIGHT_TEMPO;
} else if (light_tempo > 0) {
light_tempo--;
- if (light_tempo == 0 && ((lcd_flags & LCD_FLAG_L) == 0))
- lcd_backlight(0);
+ if (light_tempo == 0 && backlight_on &&
+ !force_backlight_on) {
+ lcd_backlight_unlocked(0);
+ }
}
+
+ spin_unlock(&pprt_lock);
}

mod_timer(&scan_timer, jiffies + INPUT_POLL_TIME);
}

-static void init_scan_timer(void)
+/* Start scan timer, assuming timer is not running */
+static void start_scan_timer(void)
{
- if (scan_timer.function != NULL)
- return; /* already started */
+ light_tempo = 0;

init_timer(&scan_timer);
scan_timer.expires = jiffies + INPUT_POLL_TIME;
scan_timer.data = 0;
- scan_timer.function = (void *)&panel_scan_timer;
+ scan_timer.function = (void *) &panel_scan_timer;
add_timer(&scan_timer);
+ timer_running = 1;
+}
+
+/*
+ * Stop scan timer and wait for it to finish. Assumes that timer is running.
+ * Note that we don't bother waking up any processes in the wait queue, since
+ * we shouldn't have come here with dangling opens to the keypad.
+ */
+static void stop_scan_timer(void)
+{
+ del_timer_sync(&scan_timer);
+ timer_running = 0;
}

+
/* converts a name of the form "({BbAaPpSsEe}{01234567-})*" to a series of bits.
* if <omask> or <imask> are non-null, they will be or'ed with the bits
* corresponding to out and in bits respectively.
@@ -1979,22 +2128,22 @@

/* tries to bind a key to the signal name <name>. The key will send the
* strings <press>, <repeat>, <release> for these respective events.
- * Returns the pointer to the new key if ok, NULL if the key could not be bound.
+ * Returns 0 on success or negative error code. Updates the logical_inputs,
+ * scan_mask_i and scan_mask_o globals.
*/
-static struct logical_input *panel_bind_key(char *name, char *press,
- char *repeat, char *release)
+static int panel_bind_key(char *name, char *press, char *repeat, char *release)
{
struct logical_input *key;

key = kzalloc(sizeof(struct logical_input), GFP_KERNEL);
if (!key) {
- printk(KERN_ERR "panel: not enough memory\n");
- return NULL;
+ pr_err("panel: not enough memory\n");
+ return -ENOMEM;
}
if (!input_name2mask(name, &key->mask, &key->value, &scan_mask_i,
&scan_mask_o)) {
kfree(key);
- return NULL;
+ return -EINVAL;
}

key->type = INPUT_TYPE_KBD;
@@ -2003,15 +2152,15 @@
key->fall_time = 1;

#if 0
- printk(KERN_DEBUG "bind: <%s> : m=%016Lx v=%016Lx\n", name, key->mask,
- key->value);
+ pr_debug("bind: <%s> : m=%016Lx v=%016Lx\n", name, key->mask,
+ key->value);
#endif
strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str));
strncpy(key->u.kbd.repeat_str, repeat, sizeof(key->u.kbd.repeat_str));
strncpy(key->u.kbd.release_str, release,
sizeof(key->u.kbd.release_str));
list_add(&key->list, &logical_inputs);
- return key;
+ return 0;
}

#if 0
@@ -2031,7 +2180,7 @@

callback = kmalloc(sizeof(struct logical_input), GFP_KERNEL);
if (!callback) {
- printk(KERN_ERR "panel: not enough memory\n");
+ pr_err("panel: not enough memory\n");
return NULL;
}
memset(callback, 0, sizeof(struct logical_input));
@@ -2052,23 +2201,46 @@
}
#endif

-static void keypad_init(void)
+/* Free memory for keypad variables. Assumes that timer function doesn't run */
+static void keypad_cleanup(void)
+{
+ struct logical_input *p, *next;
+
+ list_for_each_entry_safe(p, next, &logical_inputs, list) {
+ list_del(&p->list);
+ kfree(p);
+ }
+
+ keypad_initialized = 0;
+}
+
+
+/*
+ * Allocate and initialize keypad variables. Assumes that timer function
+ * doesn't run. Returns 0 on success or negative error code.
+ */
+static int keypad_init(void)
{
int keynum;
- init_waitqueue_head(&keypad_read_wait);
- keypad_buflen = 0; /* flushes any eventual noisy keystroke */
+ int r;

- /* Let's create all known keys */
+ scan_mask_o = scan_mask_i = 0;

+ /* Let's create all known keys */
for (keynum = 0; keypad_profile[keynum][0][0]; keynum++) {
- panel_bind_key(keypad_profile[keynum][0],
- keypad_profile[keynum][1],
- keypad_profile[keynum][2],
- keypad_profile[keynum][3]);
+ r = panel_bind_key(keypad_profile[keynum][0],
+ keypad_profile[keynum][1],
+ keypad_profile[keynum][2],
+ keypad_profile[keynum][3]);
+ if (r < 0) {
+ keypad_cleanup();
+ return r;
+ }
+
}

- init_scan_timer();
keypad_initialized = 1;
+ return 0;
}

/**************************************************/
@@ -2078,7 +2250,7 @@
static int panel_notify_sys(struct notifier_block *this, unsigned long code,
void *unused)
{
- if (lcd_enabled && lcd_initialized) {
+ if (lcd_enabled) {
switch (code) {
case SYS_DOWN:
panel_lcd_print
@@ -2089,7 +2261,8 @@
("\x0cSystem Halted.\x1b[Lc\x1b[Lb\x1b[L+");
break;
case SYS_POWER_OFF:
- panel_lcd_print("\x0cPower off.\x1b[Lc\x1b[Lb\x1b[L+");
+ panel_lcd_print
+ ("\x0cPower off.\x1b[Lc\x1b[Lb\x1b[L+");
break;
default:
break;
@@ -2106,85 +2279,141 @@

static void panel_attach(struct parport *port)
{
+ struct pardevice *portdev;
+ int r;
+
+ /*
+ * If this is the port we wanted, attempt to claim the port for
+ * ourselves.
+ */
if (port->number != parport)
return;

- if (pprt) {
- printk(KERN_ERR
- "panel_attach(): port->number=%d parport=%d, "
- "already registered !\n",
- port->number, parport);
+ portdev = parport_register_device(port, "panel", NULL, NULL, NULL, 0,
+ NULL);
+ if (portdev == NULL) {
+ pr_err("Panel: Failed to register with parport%d.\n", parport);
return;
}

- pprt = parport_register_device(port, "panel", NULL, NULL, /* pf, kf */
- NULL,
- /*PARPORT_DEV_EXCL */
- 0, (void *)&pprt);
- if (pprt == NULL) {
- pr_err("panel_attach(): port->number=%d parport=%d, "
- "parport_register_device() failed\n",
- port->number, parport);
+ if (parport_claim(portdev)) {
+ pr_err("Panel: Failed to claim parport%d.\n", parport);
+ parport_unregister_device(portdev);
return;
}

- if (parport_claim(pprt)) {
- printk(KERN_ERR
- "Panel: could not claim access to parport%d. "
- "Aborting.\n", parport);
- goto err_unreg_device;
+ /*
+ * Port successfully owned, update global state and initialize LCD if
+ * enabled.
+ */
+ mutex_lock(&port_mutex);
+
+ if (pprt) {
+ mutex_unlock(&port_mutex);
+ pr_err("Panel: Parport%d already registered.\n", parport);
+ parport_release(portdev);
+ parport_unregister_device(portdev);
+ return;
}

- /* must init LCD first, just in case an IRQ from the keypad is
- * generated at keypad init
- */
- if (lcd_enabled) {
+ pprt = portdev;
+
+ if (lcd_enabled)
lcd_init();
- if (misc_register(&lcd_dev))
- goto err_unreg_device;
- }

+ mutex_unlock(&port_mutex);
+
+ pr_info("Panel: registered on parport%d (io=0x%lx).\n", parport,
+ port->base);
+
+ /*
+ * Port and LCD initialized. After this point we keep ownership of the
+ * port in all cases. It will only be released in detach.
+ */
+
+ /*
+ * Initialize keypad and timer. Start timer only if keypad is enabled
+ * or an LCD with backlight support is enabled.
+ *
+ * We can do this without any locking since they only need to be
+ * synchronized with detach, and parport makes sure that detach is not
+ * called while we are in attach.
+ */
if (keypad_enabled) {
- keypad_init();
- if (misc_register(&keypad_dev))
- goto err_lcd_unreg;
+ r = keypad_init();
+ if (r < 0)
+ pr_err("Panel: error %d initializing keypad.\n", r);
}
- return;

-err_lcd_unreg:
- if (lcd_enabled)
- misc_deregister(&lcd_dev);
-err_unreg_device:
- parport_unregister_device(pprt);
- pprt = NULL;
+ if (keypad_initialized || (lcd_enabled && (lcd_bl_pin != PIN_NONE)))
+ start_scan_timer();
+
+ /* Finally attempt to register misc devices for LCD/keypad */
+ if (lcd_enabled) {
+ r = misc_register(&lcd_dev);
+ if (r < 0)
+ pr_err("Panel: error %d registering LCD dev.\n", r);
+ }
+
+ if (keypad_initialized) {
+ r = misc_register(&keypad_dev);
+ if (r < 0)
+ pr_err("Panel: error %d registering keypad dev.\n", r);
+ }
}

static void panel_detach(struct parport *port)
{
+ struct pardevice *portdev;
+
+ /*
+ * Quick check for our port. Note that it is safe to access pprt
+ * without a lock in this case since it can not change under us
+ * (parport synchronizes multiple calls to attach/detach).
+ */
if (port->number != parport)
return;

- if (!pprt) {
- printk(KERN_ERR
- "panel_detach(): port->number=%d parport=%d, "
- "nothing to unregister.\n",
- port->number, parport);
+ if (!pprt || pprt->port != port) {
+ pr_err("Panel: Nothing to unregister for parport%d.\n",
+ parport);
return;
}

- if (keypad_enabled && keypad_initialized) {
+ /* Detach is for our port. Stop timer and free keypad variables. */
+ if (timer_running)
+ stop_scan_timer();
+
+ if (keypad_initialized)
+ keypad_cleanup();
+
+ /* Unregister misc devices if registered */
+ if (!list_empty(&keypad_dev.list)) {
misc_deregister(&keypad_dev);
- keypad_initialized = 0;
+ INIT_LIST_HEAD(&keypad_dev.list);
}

- if (lcd_enabled && lcd_initialized) {
+ if (!list_empty(&lcd_dev.list)) {
misc_deregister(&lcd_dev);
- lcd_initialized = 0;
+ INIT_LIST_HEAD(&lcd_dev.list);
}

- parport_release(pprt);
- parport_unregister_device(pprt);
+ /* Finally release the port */
+ mutex_lock(&port_mutex);
+
+ if (lcd_enabled)
+ lcd_cleanup();
+
+ portdev = pprt;
pprt = NULL;
+
+ mutex_unlock(&port_mutex);
+
+ parport_release(portdev);
+ parport_unregister_device(portdev);
+
+ pr_info("Panel: deregistered from parport%d (io=0x%lx).\n", parport,
+ port->base);
}

static struct parport_driver panel_driver = {
@@ -2193,8 +2422,12 @@
.detach = panel_detach,
};

-/* init function */
-int panel_init(void)
+/*
+ * Initialize global variables from configuration. These are initialized when
+ * the module is loaded and remain constant until panel_cleanup is called when
+ * the module is unloaded.
+ */
+static void panel_init_globals(void)
{
/* for backwards compatibility */
if (keypad_type < 0)
@@ -2269,78 +2502,62 @@
case KEYPAD_TYPE_NEXCOM:
keypad_profile = nexcom_keypad_profile;
break;
- default:
- keypad_profile = NULL;
- break;
}

- /* tells various subsystems about the fact that we are initializing */
- init_in_progress = 1;
+ lcd_init_globals();
+}

- if (parport_register_driver(&panel_driver)) {
- printk(KERN_ERR
- "Panel: could not register with parport. Aborting.\n");
- return -EIO;
- }
+static int __init panel_init_module(void)
+{
+ int r;
+
+ panel_init_globals();

+ /* Used to check with list_empty if devices were registered */
+ INIT_LIST_HEAD(&lcd_dev.list);
+ INIT_LIST_HEAD(&keypad_dev.list);
+
+ /* We have nothing to do if neither lcd nor keypad enabled */
if (!lcd_enabled && !keypad_enabled) {
- /* no device enabled, let's release the parport */
- if (pprt) {
- parport_release(pprt);
- parport_unregister_device(pprt);
- pprt = NULL;
- }
- parport_unregister_driver(&panel_driver);
- printk(KERN_ERR "Panel driver version " PANEL_VERSION
- " disabled.\n");
+ pr_err("Panel driver version " PANEL_VERSION " disabled.\n");
return -ENODEV;
}

+ /*
+ * Register with parport. From this point the code must be prepared to
+ * handle attach/detach. parport notifies us immediately about existing
+ * ports.
+ */
+ r = parport_register_driver(&panel_driver);
+ if (r) {
+ pr_err("Panel: could not register with parport. Aborting.\n");
+ return r;
+ }
+
register_reboot_notifier(&panel_notifier);

- if (pprt)
- printk(KERN_INFO "Panel driver version " PANEL_VERSION
- " registered on parport%d (io=0x%lx).\n", parport,
- pprt->port->base);
- else
- printk(KERN_INFO "Panel driver version " PANEL_VERSION
- " not yet registered\n");
- /* tells various subsystems about the fact that initialization
- is finished */
- init_in_progress = 0;
- return 0;
-}
+ /*
+ * We might already have found our port (the typical case). Inform the
+ * user if the port was not yet found. Note that it is safe to access
+ * the port variable without the lock in this special case.
+ */
+ pr_info("Panel driver version " PANEL_VERSION " loaded.\n");
+ if (!pprt)
+ pr_info("Waiting for parallel port to become available.\n");

-static int __init panel_init_module(void)
-{
- return panel_init();
+ return 0;
}

static void __exit panel_cleanup_module(void)
{
unregister_reboot_notifier(&panel_notifier);

- if (scan_timer.function != NULL)
- del_timer(&scan_timer);
-
- if (pprt != NULL) {
- if (keypad_enabled) {
- misc_deregister(&keypad_dev);
- keypad_initialized = 0;
- }
-
- if (lcd_enabled) {
- panel_lcd_print("\x0cLCD driver " PANEL_VERSION
- "\nunloaded.\x1b[Lc\x1b[Lb\x1b[L-");
- misc_deregister(&lcd_dev);
- lcd_initialized = 0;
- }
-
- /* TODO: free all input signals */
- parport_release(pprt);
- parport_unregister_device(pprt);
- pprt = NULL;
- }
+ /*
+ * The unregister function makes sure that detach is called for all
+ * ports, and that detach has finished running before returning here.
+ * Therefore there is not much for us to do here since detach handles
+ * all cleanup for the port.
+ */
parport_unregister_driver(&panel_driver);
}
\
 
 \ /
  Last update: 2012-06-28 23:41    [W:0.076 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site