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 <[email protected]> ---- 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 <[email protected]> + * Partially rewritten 2012, Zoltan Kelemen <[email protected]> * * 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); }
_______________________________________________ devel mailing list [email protected] http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
