[PATCH 3/16] i8042: Make use of new serio start and stop methods

Dmitry Torokhov
2004-12-29T07:38:48Z

===================================================================

ChangeSet@1.1957.1.30, 2004-11-12 01:26:42-05:00, dtor_core@ameritech.net
Input: i8042 - make use of new serio start() and stop() callbacks
to ensure that i8042 interrupt handler that is shared among
several ports does not reference deleted ports. Also rename
i8042_valies structure to i8042_port, consolidate handling
of KBD, AUX and MUX ports, rearrange interrupt handler code.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

i8042.c | 319 ++++++++++++++++++++++++++++++++--------------------------------
i8042.h | 6 -
2 files changed, 165 insertions(+), 160 deletions(-)

===================================================================

diff -Nru a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
--- a/drivers/input/serio/i8042.c 2004-12-29 01:46:50 -05:00
+++ b/drivers/input/serio/i8042.c 2004-12-29 01:46:50 -05:00
@@ -19,6 +19,7 @@
#include <linux/init.h>
#include <linux/serio.h>
#include <linux/err.h>
+#include <linux/rcupdate.h>

#include <asm/io.h>

@@ -82,7 +83,8 @@

spinlock_t i8042_lock = SPIN_LOCK_UNLOCKED;

-struct i8042_values {
+struct i8042_port {
+ struct serio *serio;
int irq;
unsigned char disable;
unsigned char irqen;
@@ -91,25 +93,25 @@
char name[8];
};

-static struct i8042_values i8042_kbd_values = {
- .disable = I8042_CTR_KBDDIS,
- .irqen = I8042_CTR_KBDINT,
- .mux = -1,
- .name = "KBD",
-};
-
-static struct i8042_values i8042_aux_values = {
- .disable = I8042_CTR_AUXDIS,
- .irqen = I8042_CTR_AUXINT,
- .mux = -1,
- .name = "AUX",
+#define I8042_KBD_PORT_NO 0
+#define I8042_AUX_PORT_NO 1
+#define I8042_MUX_PORT_NO 2
+#define I8042_NUM_PORTS (I8042_NUM_MUX_PORTS + 2)
+static struct i8042_port i8042_ports[I8042_NUM_PORTS] = {
+ {
+ .disable = I8042_CTR_KBDDIS,
+ .irqen = I8042_CTR_KBDINT,
+ .mux = -1,
+ .name = "KBD",
+ },
+ {
+ .disable = I8042_CTR_AUXDIS,
+ .irqen = I8042_CTR_AUXINT,
+ .mux = -1,
+ .name = "AUX",
+ }
};

-static struct i8042_values i8042_mux_values[I8042_NUM_MUX_PORTS];
-
-static struct serio *i8042_kbd_port;
-static struct serio *i8042_aux_port;
-static struct serio *i8042_mux_port[I8042_NUM_MUX_PORTS];
static unsigned char i8042_initial_ctr;
static unsigned char i8042_ctr;
static unsigned char i8042_mux_open;
@@ -117,6 +119,7 @@
static struct timer_list i8042_timer;
static struct platform_device *i8042_platform_device;

+
/*
* Shared IRQ's require a device pointer, but this driver doesn't support
* multiple devices
@@ -250,19 +253,19 @@
* i8042_aux_write() sends a byte out through the aux interface.
*/

-static int i8042_aux_write(struct serio *port, unsigned char c)
+static int i8042_aux_write(struct serio *serio, unsigned char c)
{
- struct i8042_values *values = port->port_data;
+ struct i8042_port *port = serio->port_data;
int retval;

/*
* Send the byte out.
*/

- if (values->mux == -1)
+ if (port->mux == -1)
retval = i8042_command(&c, I8042_CMD_AUX_SEND);
else
- retval = i8042_command(&c, I8042_CMD_MUX_SEND + values->mux);
+ retval = i8042_command(&c, I8042_CMD_MUX_SEND + port->mux);

/*
* Make sure the interrupt happens and the character is received even
@@ -278,9 +281,10 @@
* i8042_activate_port() enables port on a chip.
*/

-static int i8042_activate_port(struct serio *port)
+static int i8042_activate_port(struct i8042_port *port)
{
- struct i8042_values *values = port->port_data;
+ if (!port->serio)
+ return -1;

i8042_flush();

@@ -288,12 +292,12 @@
* Enable port again here because it is disabled if we are
* resuming (normally it is enabled already).
*/
- i8042_ctr &= ~values->disable;
+ i8042_ctr &= ~port->disable;

- i8042_ctr |= values->irqen;
+ i8042_ctr |= port->irqen;

if (i8042_command(&i8042_ctr, I8042_CMD_CTL_WCTR)) {
- i8042_ctr &= ~values->irqen;
+ i8042_ctr &= ~port->irqen;
return -1;
}

@@ -306,22 +310,22 @@
* It allocates the interrupt and calls i8042_enable_port.
*/

-static int i8042_open(struct serio *port)
+static int i8042_open(struct serio *serio)
{
- struct i8042_values *values = port->port_data;
+ struct i8042_port *port = serio->port_data;

- if (values->mux != -1)
+ if (port->mux != -1)
if (i8042_mux_open++)
return 0;

- if (request_irq(values->irq, i8042_interrupt,
+ if (request_irq(port->irq, i8042_interrupt,
SA_SHIRQ, "i8042", i8042_request_irq_cookie)) {
- printk(KERN_ERR "i8042.c: Can't get irq %d for %s, unregistering the port.\n", values->irq, values->name);
+ printk(KERN_ERR "i8042.c: Can't get irq %d for %s, unregistering the port.\n", port->irq, port->name);
goto irq_fail;
}

if (i8042_activate_port(port)) {
- printk(KERN_ERR "i8042.c: Can't activate %s, unregistering the port\n", values->name);
+ printk(KERN_ERR "i8042.c: Can't activate %s, unregistering the port\n", port->name);
goto activate_fail;
}

@@ -330,11 +334,10 @@
return 0;

activate_fail:
- free_irq(values->irq, i8042_request_irq_cookie);
+ free_irq(port->irq, i8042_request_irq_cookie);

irq_fail:
- values->exists = 0;
- serio_unregister_port_delayed(port);
+ serio_unregister_port_delayed(serio);

return -1;
}
@@ -345,27 +348,58 @@
* the BIOS could have used the AUX interrupt for PCI.
*/

-static void i8042_close(struct serio *port)
+static void i8042_close(struct serio *serio)
{
- struct i8042_values *values = port->port_data;
+ struct i8042_port *port = serio->port_data;

- if (values->mux != -1)
+ if (port->mux != -1)
if (--i8042_mux_open)
return;

- i8042_ctr &= ~values->irqen;
+ i8042_ctr &= ~port->irqen;

if (i8042_command(&i8042_ctr, I8042_CMD_CTL_WCTR)) {
- printk(KERN_ERR "i8042.c: Can't write CTR while closing %s.\n", values->name);
- return;
+ printk(KERN_WARNING "i8042.c: Can't write CTR while closing %s.\n", port->name);
+/*
+ * We still want to continue and free IRQ so if more data keeps coming in
+ * kernel will just ignore the irq.
+ */
}

- free_irq(values->irq, i8042_request_irq_cookie);
+ free_irq(port->irq, i8042_request_irq_cookie);

i8042_flush();
}

/*
+ * i8042_start() is called by serio core when port is about to finish
+ * registering. It will mark port as existing so i8042_interrupt can
+ * start sending data through it.
+ */
+static int i8042_start(struct serio *serio)
+{
+ struct i8042_port *port = serio->port_data;
+
+ port->exists = 1;
+ mb();
+ return 0;
+}
+
+/*
+ * i8042_stop() marks serio port as non-existing so i8042_interrupt
+ * will not try to send data to the port that is about to go away.
+ * The function is called by serio core as part of unregister procedure.
+ */
+static void i8042_stop(struct serio *serio)
+{
+ struct i8042_port *port = serio->port_data;
+
+ port->exists = 0;
+ synchronize_kernel();
+ port->serio = NULL;
+}
+
+/*
* i8042_interrupt() is the most important function in this driver -
* it handles the interrupts from the i8042, and sends incoming bytes
* to the upper layers.
@@ -373,25 +407,25 @@

static irqreturn_t i8042_interrupt(int irq, void *dev_id, struct pt_regs *regs)
{
+ struct i8042_port *port;
unsigned long flags;
- unsigned char str, data = 0;
+ unsigned char str, data;
unsigned int dfl;
- unsigned int aux_idx;
+ unsigned int port_no;
int ret;

mod_timer(&i8042_timer, jiffies + I8042_POLL_PERIOD);

spin_lock_irqsave(&i8042_lock, flags);
str = i8042_read_status();
- if (str & I8042_STR_OBF)
- data = i8042_read_data();
- spin_unlock_irqrestore(&i8042_lock, flags);
-
- if (~str & I8042_STR_OBF) {
+ if (unlikely(~str & I8042_STR_OBF)) {
+ spin_unlock_irqrestore(&i8042_lock, flags);
if (irq) dbg("Interrupt %d, without any data", irq);
ret = 0;
goto out;
}
+ data = i8042_read_data();
+ spin_unlock_irqrestore(&i8042_lock, flags);

if (i8042_mux_present && (str & I8042_STR_AUXDATA)) {
static unsigned long last_transmit;
@@ -423,39 +457,28 @@
}
}

- aux_idx = (str >> 6) & 3;
-
- dbg("%02x <- i8042 (interrupt, aux%d, %d%s%s)",
- data, aux_idx, irq,
- dfl & SERIO_PARITY ? ", bad parity" : "",
- dfl & SERIO_TIMEOUT ? ", timeout" : "");
-
- if (likely(i8042_mux_values[aux_idx].exists))
- serio_interrupt(i8042_mux_port[aux_idx], data, dfl, regs);
-
+ port_no = I8042_MUX_PORT_NO + ((str >> 6) & 3);
last_str = str;
last_transmit = jiffies;
- goto irq_ret;
+ } else {
+
+ dfl = ((str & I8042_STR_PARITY) ? SERIO_PARITY : 0) |
+ ((str & I8042_STR_TIMEOUT) ? SERIO_TIMEOUT : 0);
+
+ port_no = (str & I8042_STR_AUXDATA) ?
+ I8042_AUX_PORT_NO : I8042_KBD_PORT_NO;
}

- dfl = ((str & I8042_STR_PARITY) ? SERIO_PARITY : 0) |
- ((str & I8042_STR_TIMEOUT) ? SERIO_TIMEOUT : 0);
+ port = &i8042_ports[port_no];

dbg("%02x <- i8042 (interrupt, %s, %d%s%s)",
- data, (str & I8042_STR_AUXDATA) ? "aux" : "kbd", irq,
- dfl & SERIO_PARITY ? ", bad parity" : "",
- dfl & SERIO_TIMEOUT ? ", timeout" : "");
+ data, port->name, irq,
+ dfl & SERIO_PARITY ? ", bad parity" : "",
+ dfl & SERIO_TIMEOUT ? ", timeout" : "");

+ if (likely(port->exists))
+ serio_interrupt(port->serio, data, dfl, regs);

- if (str & I8042_STR_AUXDATA) {
- if (likely(i8042_aux_values.exists))
- serio_interrupt(i8042_aux_port, data, dfl, regs);
- } else {
- if (likely(i8042_kbd_values.exists))
- serio_interrupt(i8042_kbd_port, data, dfl, regs);
- }
-
-irq_ret:
ret = 1;
out:
return IRQ_RETVAL(ret);
@@ -467,7 +490,7 @@
* Legacy) mode.
*/

-static int i8042_enable_mux_mode(struct i8042_values *values, unsigned char *mux_version)
+static int i8042_enable_mux_mode(unsigned char *mux_version)
{

unsigned char param;
@@ -505,7 +528,7 @@
* the controller has been switched into Multiplexed mode
*/

-static int i8042_enable_mux_ports(struct i8042_values *values)
+static int i8042_enable_mux_ports(void)
{
unsigned char param;
int i;
@@ -540,11 +563,11 @@
* LCS/Telegraphics.
*/

-static int __init i8042_check_mux(struct i8042_values *values)
+static int __init i8042_check_mux(void)
{
unsigned char mux_version;

- if (i8042_enable_mux_mode(values, &mux_version))
+ if (i8042_enable_mux_mode(&mux_version))
return -1;

/* Workaround for broken chips which seem to support MUX, but in reality don't. */
@@ -555,7 +578,7 @@
printk(KERN_INFO "i8042.c: Detected active multiplexing controller, rev %d.%d.\n",
(mux_version >> 4) & 0xf, mux_version & 0xf);

- if (i8042_enable_mux_ports(values))
+ if (i8042_enable_mux_ports())
return -1;

i8042_mux_present = 1;
@@ -568,7 +591,7 @@
* the presence of an AUX interface.
*/

-static int __init i8042_check_aux(struct i8042_values *values)
+static int __init i8042_check_aux(void)
{
unsigned char param;
static int i8042_check_aux_cookie;
@@ -578,10 +601,10 @@
* in trying to detect AUX presence.
*/

- if (request_irq(values->irq, i8042_interrupt, SA_SHIRQ,
- "i8042", &i8042_check_aux_cookie))
+ if (request_irq(i8042_ports[I8042_AUX_PORT_NO].irq, i8042_interrupt,
+ SA_SHIRQ, "i8042", &i8042_check_aux_cookie))
return -1;
- free_irq(values->irq, &i8042_check_aux_cookie);
+ free_irq(i8042_ports[I8042_AUX_PORT_NO].irq, &i8042_check_aux_cookie);

/*
* Get rid of bytes in the queue.
@@ -646,27 +669,25 @@
* registers it, and reports to the user.
*/

-static int __init i8042_port_register(struct serio *port)
+static int __init i8042_port_register(struct i8042_port *port)
{
- struct i8042_values *values = port->port_data;
-
- values->exists = 1;
-
- i8042_ctr &= ~values->disable;
+ i8042_ctr &= ~port->disable;

if (i8042_command(&i8042_ctr, I8042_CMD_CTL_WCTR)) {
printk(KERN_WARNING "i8042.c: Can't write CTR while registering.\n");
- values->exists = 0;
+ kfree(port->serio);
+ port->serio = NULL;
+ i8042_ctr |= port->disable;
return -1;
}

printk(KERN_INFO "serio: i8042 %s port at %#lx,%#lx irq %d\n",
- values->name,
+ port->name,
(unsigned long) I8042_DATA_REG,
(unsigned long) I8042_COMMAND_REG,
- values->irq);
+ port->irq);

- serio_register_port(port);
+ serio_register_port(port->serio);

return 0;
}
@@ -811,15 +832,9 @@
* Reset anything that is connected to the ports.
*/

- if (i8042_kbd_values.exists)
- serio_cleanup(i8042_kbd_port);
-
- if (i8042_aux_values.exists)
- serio_cleanup(i8042_aux_port);
-
- for (i = 0; i < I8042_NUM_MUX_PORTS; i++)
- if (i8042_mux_values[i].exists)
- serio_cleanup(i8042_mux_port[i]);
+ for (i = 0; i < I8042_NUM_PORTS; i++)
+ if (i8042_ports[i].exists)
+ serio_cleanup(i8042_ports[i].serio);

i8042_controller_reset();
}
@@ -897,8 +912,7 @@
}

if (i8042_mux_present)
- if (i8042_enable_mux_mode(&i8042_aux_values, NULL) ||
- i8042_enable_mux_ports(&i8042_aux_values)) {
+ if (i8042_enable_mux_mode(NULL) || i8042_enable_mux_ports()) {
printk(KERN_WARNING "i8042: failed to resume active multiplexor, mouse won't work.\n");
}

@@ -906,15 +920,9 @@
* Reconnect anything that was connected to the ports.
*/

- if (i8042_kbd_values.exists && i8042_activate_port(i8042_kbd_port) == 0)
- serio_reconnect(i8042_kbd_port);
-
- if (i8042_aux_values.exists && i8042_activate_port(i8042_aux_port) == 0)
- serio_reconnect(i8042_aux_port);
-
- for (i = 0; i < I8042_NUM_MUX_PORTS; i++)
- if (i8042_mux_values[i].exists && i8042_activate_port(i8042_mux_port[i]) == 0)
- serio_reconnect(i8042_mux_port[i]);
+ for (i = 0; i < I8042_NUM_PORTS; i++)
+ if (i8042_activate_port(&i8042_ports[i]) == 0)
+ serio_reconnect(i8042_ports[i].serio);
/*
* Restart timer (for polling "stuck" data)
*/
@@ -944,9 +952,10 @@
.shutdown = i8042_shutdown,
};

-static struct serio * __init i8042_allocate_kbd_port(void)
+static void __init i8042_create_kbd_port(void)
{
struct serio *serio;
+ struct i8042_port *port = &i8042_ports[I8042_KBD_PORT_NO];

serio = kmalloc(sizeof(struct serio), GFP_KERNEL);
if (serio) {
@@ -955,18 +964,22 @@
serio->write = i8042_dumbkbd ? NULL : i8042_kbd_write,
serio->open = i8042_open,
serio->close = i8042_close,
- serio->port_data = &i8042_kbd_values,
+ serio->start = i8042_start,
+ serio->stop = i8042_stop,
+ serio->port_data = port,
serio->dev.parent = &i8042_platform_device->dev;
strlcpy(serio->name, "i8042 Kbd Port", sizeof(serio->name));
strlcpy(serio->phys, I8042_KBD_PHYS_DESC, sizeof(serio->phys));
- }

- return serio;
+ port->serio = serio;
+ i8042_port_register(port);
+ }
}

-static struct serio * __init i8042_allocate_aux_port(void)
+static void __init i8042_create_aux_port(void)
{
struct serio *serio;
+ struct i8042_port *port = &i8042_ports[I8042_AUX_PORT_NO];

serio = kmalloc(sizeof(struct serio), GFP_KERNEL);
if (serio) {
@@ -975,38 +988,44 @@
serio->write = i8042_aux_write;
serio->open = i8042_open;
serio->close = i8042_close;
- serio->port_data = &i8042_aux_values,
+ serio->start = i8042_start,
+ serio->stop = i8042_stop,
+ serio->port_data = port,
serio->dev.parent = &i8042_platform_device->dev;
strlcpy(serio->name, "i8042 Aux Port", sizeof(serio->name));
strlcpy(serio->phys, I8042_AUX_PHYS_DESC, sizeof(serio->phys));
- }

- return serio;
+ port->serio = serio;
+ i8042_port_register(port);
+ }
}

-static struct serio * __init i8042_allocate_mux_port(int index)
+static void __init i8042_create_mux_port(int index)
{
struct serio *serio;
- struct i8042_values *values = &i8042_mux_values[index];
+ struct i8042_port *port = &i8042_ports[I8042_MUX_PORT_NO + index];

serio = kmalloc(sizeof(struct serio), GFP_KERNEL);
if (serio) {
- *values = i8042_aux_values;
- snprintf(values->name, sizeof(values->name), "AUX%d", index);
- values->mux = index;
-
memset(serio, 0, sizeof(struct serio));
serio->type = SERIO_8042;
serio->write = i8042_aux_write;
serio->open = i8042_open;
serio->close = i8042_close;
- serio->port_data = values;
+ serio->start = i8042_start,
+ serio->stop = i8042_stop,
+ serio->port_data = port;
serio->dev.parent = &i8042_platform_device->dev;
snprintf(serio->name, sizeof(serio->name), "i8042 Aux-%d Port", index);
snprintf(serio->phys, sizeof(serio->phys), I8042_MUX_PHYS_DESC, index + 1);
- }

- return serio;
+ *port = i8042_ports[I8042_AUX_PORT_NO];
+ port->exists = 0;
+ snprintf(port->name, sizeof(port->name), "AUX%d", index);
+ port->mux = index;
+ port->serio = serio;
+ i8042_port_register(port);
+ }
}

int __init i8042_init(void)
@@ -1022,8 +1041,8 @@
if (i8042_platform_init())
return -EBUSY;

- i8042_aux_values.irq = I8042_AUX_IRQ;
- i8042_kbd_values.irq = I8042_KBD_IRQ;
+ i8042_ports[I8042_AUX_PORT_NO].irq = I8042_AUX_IRQ;
+ i8042_ports[I8042_KBD_PORT_NO].irq = I8042_KBD_IRQ;

if (i8042_controller_init())
return -ENODEV;
@@ -1038,23 +1057,15 @@
return PTR_ERR(i8042_platform_device);
}

- if (!i8042_noaux && !i8042_check_aux(&i8042_aux_values)) {
- if (!i8042_nomux && !i8042_check_mux(&i8042_aux_values))
- for (i = 0; i < I8042_NUM_MUX_PORTS; i++) {
- i8042_mux_port[i] = i8042_allocate_mux_port(i);
- if (i8042_mux_port[i])
- i8042_port_register(i8042_mux_port[i]);
- }
- else {
- i8042_aux_port = i8042_allocate_aux_port();
- if (i8042_aux_port)
- i8042_port_register(i8042_aux_port);
- }
+ if (!i8042_noaux && !i8042_check_aux()) {
+ if (!i8042_nomux && !i8042_check_mux())
+ for (i = 0; i < I8042_NUM_MUX_PORTS; i++)
+ i8042_create_mux_port(i);
+ else
+ i8042_create_aux_port();
}

- i8042_kbd_port = i8042_allocate_kbd_port();
- if (i8042_kbd_port)
- i8042_port_register(i8042_kbd_port);
+ i8042_create_kbd_port();

mod_timer(&i8042_timer, jiffies + I8042_POLL_PERIOD);

@@ -1067,15 +1078,9 @@

i8042_controller_cleanup();

- if (i8042_kbd_values.exists)
- serio_unregister_port(i8042_kbd_port);
-
- if (i8042_aux_values.exists)
- serio_unregister_port(i8042_aux_port);
-
- for (i = 0; i < I8042_NUM_MUX_PORTS; i++)
- if (i8042_mux_values[i].exists)
- serio_unregister_port(i8042_mux_port[i]);
+ for (i = 0; i < I8042_NUM_PORTS; i++)
+ if (i8042_ports[i].exists)
+ serio_unregister_port(i8042_ports[i].serio);

del_timer_sync(&i8042_timer);

diff -Nru a/drivers/input/serio/i8042.h b/drivers/input/serio/i8042.h
--- a/drivers/input/serio/i8042.h 2004-12-29 01:46:50 -05:00
+++ b/drivers/input/serio/i8042.h 2004-12-29 01:46:50 -05:00
@@ -117,13 +117,13 @@
*/

#ifdef DEBUG
-static unsigned long i8042_start;
-#define dbg_init() do { i8042_start = jiffies; } while (0)
+static unsigned long i8042_start_time;
+#define dbg_init() do { i8042_start_time = jiffies; } while (0)
#define dbg(format, arg...) \
do { \
if (i8042_debug) \
printk(KERN_DEBUG __FILE__ ": " format " [%d]\n" , \
- ## arg, (int) (jiffies - i8042_start)); \
+ ## arg, (int) (jiffies - i8042_start_time)); \
} while (0)
#else
#define dbg_init() do { } while (0)