[PATCH 15/16] serio bus implementation cleanup

Dmitry Torokhov
2004-12-29T08:07:11Z

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

ChangeSet@1.1975, 2004-12-28 01:24:56-05:00, dtor_core@ameritech.net
Input: make serio implementation more in line with standard
driver model implementations. serio_register_port is
always asynchronous to allow freely registering child
ports. When deregistering serio core still takes care
of destroying children ports first.

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

drivers/input/mouse/psmouse-base.c | 11 -
drivers/input/mouse/synaptics.c | 10 -
drivers/input/serio/serio.c | 363 ++++++++++++++++++++-----------------
include/linux/serio.h | 17 +
4 files changed, 219 insertions(+), 182 deletions(-)

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

diff -Nru a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
--- a/drivers/input/mouse/psmouse-base.c 2004-12-29 01:53:31 -05:00
+++ b/drivers/input/mouse/psmouse-base.c 2004-12-29 01:53:31 -05:00
@@ -692,6 +692,7 @@
memset(psmouse, 0, sizeof(struct psmouse));

ps2_init(&psmouse->ps2dev, serio);
+ sprintf(psmouse->phys, "%s/input0", serio->phys);
psmouse->dev.evbit[0] = BIT(EV_KEY) | BIT(EV_REL);
psmouse->dev.keybit[LONG(BTN_MOUSE)] = BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
psmouse->dev.relbit[0] = BIT(REL_X) | BIT(REL_Y);
@@ -727,8 +728,6 @@

sprintf(psmouse->devname, "%s %s %s",
psmouse_protocols[psmouse->type], psmouse->vendor, psmouse->name);
- sprintf(psmouse->phys, "%s/input0",
- serio->phys);

psmouse->dev.name = psmouse->devname;
psmouse->dev.phys = psmouse->phys;
@@ -751,14 +750,6 @@
device_create_file(&serio->dev, &psmouse_attr_rate);
device_create_file(&serio->dev, &psmouse_attr_resolution);
device_create_file(&serio->dev, &psmouse_attr_resetafter);
-
- if (serio->child) {
- /*
- * Nothing to be done here, serio core will detect that
- * the driver set serio->child and will register it for us.
- */
- printk(KERN_INFO "serio: %s port at %s\n", serio->child->name, psmouse->phys);
- }

psmouse_activate(psmouse);

diff -Nru a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
--- a/drivers/input/mouse/synaptics.c 2004-12-29 01:53:31 -05:00
+++ b/drivers/input/mouse/synaptics.c 2004-12-29 01:53:31 -05:00
@@ -296,7 +296,8 @@

psmouse->pt_activate = synaptics_pt_activate;

- psmouse->ps2dev.serio->child = serio;
+ printk(KERN_INFO "serio: %s port at %s\n", serio->name, psmouse->phys);
+ serio_register_port(serio);
}

/*****************************************************************************
@@ -552,6 +553,7 @@
{
synaptics_reset(psmouse);
kfree(psmouse->private);
+ psmouse->private = NULL;
}

static int synaptics_reconnect(struct psmouse *psmouse)
@@ -640,9 +642,6 @@

priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS;

- if (SYN_CAP_PASS_THROUGH(priv->capabilities))
- synaptics_pt_create(psmouse);
-
print_ident(priv);
set_input_params(&psmouse->dev, priv);

@@ -651,6 +650,9 @@
psmouse->disconnect = synaptics_disconnect;
psmouse->reconnect = synaptics_reconnect;
psmouse->pktsize = 6;
+
+ if (SYN_CAP_PASS_THROUGH(priv->capabilities))
+ synaptics_pt_create(psmouse);

#if defined(__i386__)
/*
diff -Nru a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
--- a/drivers/input/serio/serio.c 2004-12-29 01:53:31 -05:00
+++ b/drivers/input/serio/serio.c 2004-12-29 01:53:31 -05:00
@@ -42,10 +42,9 @@
MODULE_LICENSE("GPL");

EXPORT_SYMBOL(serio_interrupt);
-EXPORT_SYMBOL(serio_register_port);
-EXPORT_SYMBOL(serio_register_port_delayed);
+EXPORT_SYMBOL(__serio_register_port);
EXPORT_SYMBOL(serio_unregister_port);
-EXPORT_SYMBOL(serio_unregister_port_delayed);
+EXPORT_SYMBOL(__serio_unregister_port_delayed);
EXPORT_SYMBOL(serio_register_driver);
EXPORT_SYMBOL(serio_unregister_driver);
EXPORT_SYMBOL(serio_open);
@@ -53,19 +52,20 @@
EXPORT_SYMBOL(serio_rescan);
EXPORT_SYMBOL(serio_reconnect);

-static DECLARE_MUTEX(serio_sem); /* protects serio_list and serio_diriver_list */
+/*
+ * serio_sem protects entire serio subsystem and is taken every time
+ * serio port or driver registrered or unregistered.
+ */
+static DECLARE_MUTEX(serio_sem);
+
static LIST_HEAD(serio_list);
-static LIST_HEAD(serio_driver_list);
-static unsigned int serio_no;

struct bus_type serio_bus = {
.name = "serio",
};

-static void serio_find_driver(struct serio *serio);
-static void serio_create_port(struct serio *serio);
+static void serio_add_port(struct serio *serio);
static void serio_destroy_port(struct serio *serio);
-static void serio_connect_port(struct serio *serio, struct serio_driver *drv);
static void serio_reconnect_port(struct serio *serio);
static void serio_disconnect_port(struct serio *serio);

@@ -82,37 +82,42 @@
return 0;
}

-static int serio_bind_driver(struct serio *serio, struct serio_driver *drv)
+/*
+ * Basic serio -> driver core mappings
+ */
+
+static void serio_bind_driver(struct serio *serio, struct serio_driver *drv)
{
- get_driver(&drv->driver);
+ down_write(&serio_bus.subsys.rwsem);

if (serio_match_port(drv->ids, serio)) {
+ serio->dev.driver = &drv->driver;
drv->connect(serio, drv);
-
- if (serio->drv) {
- down_write(&serio_bus.subsys.rwsem);
- serio->dev.driver = &drv->driver;
- device_bind_driver(&serio->dev);
- up_write(&serio_bus.subsys.rwsem);
- return 1;
+ if (!serio->drv) {
+ serio->dev.driver = NULL;
+ goto out;
}
+ device_bind_driver(&serio->dev);
}
+out:
+ up_write(&serio_bus.subsys.rwsem);
+}

- put_driver(&drv->driver);
- return 0;
+static void serio_release_driver(struct serio *serio)
+{
+ down_write(&serio_bus.subsys.rwsem);
+ device_release_driver(&serio->dev);
+ up_write(&serio_bus.subsys.rwsem);
}

-/* serio_find_driver() must be called with serio_sem down. */
static void serio_find_driver(struct serio *serio)
{
- struct serio_driver *drv;
-
- list_for_each_entry(drv, &serio_driver_list, node)
- if (!drv->manual_bind)
- if (serio_bind_driver(serio, drv))
- break;
+ down_write(&serio_bus.subsys.rwsem);
+ device_attach(&serio->dev);
+ up_write(&serio_bus.subsys.rwsem);
}

+
/*
* Serio event processing.
*/
@@ -120,6 +125,7 @@
struct serio_event {
int type;
struct serio *serio;
+ struct module *owner;
struct list_head node;
};

@@ -136,7 +142,8 @@
static DECLARE_COMPLETION(serio_exited);
static int serio_pid;

-static void serio_queue_event(struct serio *serio, enum serio_event_type event_type)
+static void serio_queue_event(struct serio *serio, struct module *owner,
+ enum serio_event_type event_type)
{
unsigned long flags;
struct serio_event *event;
@@ -159,17 +166,30 @@
}

if ((event = kmalloc(sizeof(struct serio_event), GFP_ATOMIC))) {
+ if (!try_module_get(owner)) {
+ printk(KERN_WARNING "serio: Can't get module reference, dropping event %d\n", event_type);
+ goto out;
+ }
+
event->type = event_type;
event->serio = serio;
+ event->owner = owner;

list_add_tail(&event->node, &serio_event_list);
wake_up(&serio_wait);
+ } else {
+ printk(KERN_ERR "serio: Not enough memory to queue event %d\n", event_type);
}
-
out:
spin_unlock_irqrestore(&serio_event_lock, flags);
}

+static void serio_free_event(struct serio_event *event)
+{
+ module_put(event->owner);
+ kfree(event);
+}
+
static void serio_remove_duplicate_events(struct serio_event *event)
{
struct list_head *node, *next;
@@ -179,7 +199,7 @@
spin_lock_irqsave(&serio_event_lock, flags);

list_for_each_safe(node, next, &serio_event_list) {
- e = container_of(node, struct serio_event, node);
+ e = list_entry(node, struct serio_event, node);
if (event->serio == e->serio) {
/*
* If this event is of different type we should not
@@ -187,9 +207,10 @@
* that were sent back-to-back.
*/
if (event->type != e->type)
- break; /* Stop, when need to preserve event flow */
+ break;
+
list_del_init(node);
- kfree(e);
+ serio_free_event(e);
}
}

@@ -211,7 +232,7 @@
}

node = serio_event_list.next;
- event = container_of(node, struct serio_event, node);
+ event = list_entry(node, struct serio_event, node);
list_del_init(node);

spin_unlock_irqrestore(&serio_event_lock, flags);
@@ -223,39 +244,44 @@
{
struct serio_event *event;

- while ((event = serio_get_event())) {
+ down(&serio_sem);

- down(&serio_sem);
+ while ((event = serio_get_event())) {

switch (event->type) {
- case SERIO_REGISTER_PORT :
- serio_create_port(event->serio);
- serio_connect_port(event->serio, NULL);
+ case SERIO_REGISTER_PORT:
+ serio_add_port(event->serio);
+ serio_find_driver(event->serio);
break;

- case SERIO_UNREGISTER_PORT :
+ case SERIO_UNREGISTER_PORT:
serio_disconnect_port(event->serio);
serio_destroy_port(event->serio);
break;

- case SERIO_RECONNECT :
+ case SERIO_RECONNECT:
serio_reconnect_port(event->serio);
break;

- case SERIO_RESCAN :
+ case SERIO_RESCAN:
serio_disconnect_port(event->serio);
- serio_connect_port(event->serio, NULL);
+ serio_find_driver(event->serio);
break;
+
default:
break;
}

- up(&serio_sem);
serio_remove_duplicate_events(event);
- kfree(event);
+ serio_free_event(event);
}
+
+ up(&serio_sem);
}

+/*
+ * Remove all events that have been submitted for a given serio port.
+ */
static void serio_remove_pending_events(struct serio *serio)
{
struct list_head *node, *next;
@@ -265,16 +291,42 @@
spin_lock_irqsave(&serio_event_lock, flags);

list_for_each_safe(node, next, &serio_event_list) {
- event = container_of(node, struct serio_event, node);
+ event = list_entry(node, struct serio_event, node);
if (event->serio == serio) {
list_del_init(node);
- kfree(event);
+ serio_free_event(event);
}
}

spin_unlock_irqrestore(&serio_event_lock, flags);
}

+/*
+ * Destroy child serio port (if any) that has not been fully registered yet.
+ *
+ * Note that we rely on the fact that port can have only one child and therefore
+ * only one child registration request can be pending. Additionally, children
+ * are registered by driver's connect() handler so there can't be a grandchild
+ * pending registration together with a child.
+ */
+static struct serio *serio_get_pending_child(struct serio *parent)
+{
+ struct serio_event *event;
+ struct serio *serio = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&serio_event_lock, flags);
+
+ list_for_each_entry(event, &serio_event_list, node) {
+ if (event->type == SERIO_REGISTER_PORT && event->serio->parent == parent) {
+ serio = event->serio;
+ break;
+ }
+ }
+
+ spin_unlock_irqrestore(&serio_event_lock, flags);
+ return serio;
+}

static int serio_thread(void *nothing)
{
@@ -323,10 +375,10 @@
serio_reconnect_port(serio);
} else if (!strncmp(buf, "rescan", count)) {
serio_disconnect_port(serio);
- serio_connect_port(serio, NULL);
+ serio_find_driver(serio);
} else if ((drv = driver_find(buf, &serio_bus)) != NULL) {
serio_disconnect_port(serio);
- serio_connect_port(serio, to_serio_driver(drv));
+ serio_bind_driver(serio, to_serio_driver(drv));
put_driver(drv);
} else {
retval = -EINVAL;
@@ -376,22 +428,43 @@
module_put(THIS_MODULE);
}

-static void serio_create_port(struct serio *serio)
+/*
+ * Prepare serio port for registration.
+ */
+static void serio_init_port(struct serio *serio)
{
- try_module_get(THIS_MODULE);
+ static atomic_t serio_no = ATOMIC_INIT(0);
+
+ __module_get(THIS_MODULE);

spin_lock_init(&serio->lock);
init_MUTEX(&serio->drv_sem);
- list_add_tail(&serio->node, &serio_list);
- snprintf(serio->dev.bus_id, sizeof(serio->dev.bus_id), "serio%d", serio_no++);
+ device_initialize(&serio->dev);
+ snprintf(serio->dev.bus_id, sizeof(serio->dev.bus_id),
+ "serio%d", atomic_inc_return(&serio_no) - 1);
serio->dev.bus = &serio_bus;
serio->dev.release = serio_release_port;
if (serio->parent)
serio->dev.parent = &serio->parent->dev;
- device_initialize(&serio->dev);
+}
+
+/*
+ * Complete serio port registration.
+ * Driver core will attempt to find appropriate driver for the port.
+ */
+static void serio_add_port(struct serio *serio)
+{
+ if (serio->parent) {
+ serio_pause_rx(serio->parent);
+ serio->parent->child = serio;
+ serio_continue_rx(serio->parent);
+ }
+
+ list_add_tail(&serio->node, &serio_list);
if (serio->start)
serio->start(serio);
device_add(&serio->dev);
+ serio->registered = 1;
}

/*
@@ -400,77 +473,43 @@
*/
static void serio_destroy_port(struct serio *serio)
{
- struct serio_driver *drv = serio->drv;
- unsigned long flags;
+ struct serio *child;

- serio_remove_pending_events(serio);
- list_del_init(&serio->node);
-
- if (drv) {
- drv->disconnect(serio);
- down_write(&serio_bus.subsys.rwsem);
- device_release_driver(&serio->dev);
- up_write(&serio_bus.subsys.rwsem);
- put_driver(&drv->driver);
+ child = serio_get_pending_child(serio);
+ if (child) {
+ serio_remove_pending_events(child);
+ put_device(&child->dev);
}

if (serio->stop)
serio->stop(serio);

if (serio->parent) {
- spin_lock_irqsave(&serio->parent->lock, flags);
+ serio_pause_rx(serio->parent);
serio->parent->child = NULL;
- spin_unlock_irqrestore(&serio->parent->lock, flags);
+ serio->parent = NULL;
+ serio_continue_rx(serio->parent);
}

- device_unregister(&serio->dev);
-}
-
-/*
- * serio_connect_port() tries to bind the port and possible all its
- * children to appropriate drivers. If driver passed in the function will not
- * try otehr drivers when binding parent port.
- */
-static void serio_connect_port(struct serio *serio, struct serio_driver *drv)
-{
- WARN_ON(serio->drv);
- WARN_ON(serio->child);
-
- if (drv)
- serio_bind_driver(serio, drv);
- else if (!serio->manual_bind)
- serio_find_driver(serio);
-
- /* Ok, now bind children, if any */
- while (serio->child) {
- serio = serio->child;
-
- WARN_ON(serio->drv);
- WARN_ON(serio->child);
-
- serio_create_port(serio);
-
- if (!serio->manual_bind) {
- /*
- * With children we just _prefer_ passed in driver,
- * but we will try other options in case preferred
- * is not the one
- */
- if (!drv || !serio_bind_driver(serio, drv))
- serio_find_driver(serio);
- }
+ if (serio->registered) {
+ device_del(&serio->dev);
+ list_del_init(&serio->node);
+ serio->registered = 0;
}
+
+ serio_remove_pending_events(serio);
+ put_device(&serio->dev);
}

/*
- *
+ * Reconnect serio port and all its children (re-initialize attached devices)
*/
static void serio_reconnect_port(struct serio *serio)
{
do {
if (!serio->drv || !serio->drv->reconnect || serio->drv->reconnect(serio)) {
serio_disconnect_port(serio);
- serio_connect_port(serio, NULL);
+ serio_find_driver(serio);
/* Ok, old children are now gone, we are done */
break;
}
@@ -484,8 +523,7 @@
*/
static void serio_disconnect_port(struct serio *serio)
{
- struct serio_driver *drv = serio->drv;
- struct serio *s;
+ struct serio *s, *parent;

if (serio->child) {
/*
@@ -493,56 +531,46 @@
* first, staring with the leaf one, since we don't want
* to do recursion
*/
+ for (s = serio; s->child; s = s->child)
+ /* empty */;
+
do {
- s = serio->child;
- } while (s->child);
+ parent = s->parent;

- while (s != serio) {
- s = s->parent;
- serio_destroy_port(s->child);
- }
+ serio_release_driver(s);
+ serio_destroy_port(s);
+ } while ((s = parent) != serio);
}

/*
* Ok, no children left, now disconnect this port
*/
- if (drv) {
- drv->disconnect(serio);
- down_write(&serio_bus.subsys.rwsem);
- device_release_driver(&serio->dev);
- up_write(&serio_bus.subsys.rwsem);
- put_driver(&drv->driver);
- }
+ serio_release_driver(serio);
}

void serio_rescan(struct serio *serio)
{
- serio_queue_event(serio, SERIO_RESCAN);
+ serio_queue_event(serio, NULL, SERIO_RESCAN);
}

void serio_reconnect(struct serio *serio)
{
- serio_queue_event(serio, SERIO_RECONNECT);
-}
-
-void serio_register_port(struct serio *serio)
-{
- down(&serio_sem);
- serio_create_port(serio);
- serio_connect_port(serio, NULL);
- up(&serio_sem);
+ serio_queue_event(serio, NULL, SERIO_RECONNECT);
}

/*
* Submits register request to kseriod for subsequent execution.
- * Can be used when it is not obvious whether the serio_sem is
- * taken or not and when delayed execution is feasible.
+ * Note that port registration is always asynchronous.
*/
-void serio_register_port_delayed(struct serio *serio)
+void __serio_register_port(struct serio *serio, struct module *owner)
{
- serio_queue_event(serio, SERIO_REGISTER_PORT);
+ serio_init_port(serio);
+ serio_queue_event(serio, owner, SERIO_REGISTER_PORT);
}

+/*
+ * Synchronously unregisters serio port.
+ */
void serio_unregister_port(struct serio *serio)
{
down(&serio_sem);
@@ -552,13 +580,13 @@
}

/*
- * Submits unregister request to kseriod for subsequent execution.
+ * Submits register request to kseriod for subsequent execution.
* Can be used when it is not obvious whether the serio_sem is
* taken or not and when delayed execution is feasible.
*/
-void serio_unregister_port_delayed(struct serio *serio)
+void __serio_unregister_port_delayed(struct serio *serio, struct module *owner)
{
- serio_queue_event(serio, SERIO_UNREGISTER_PORT);
+ serio_queue_event(serio, owner, SERIO_UNREGISTER_PORT);
}

@@ -603,34 +631,33 @@
__ATTR_NULL
};

-void serio_register_driver(struct serio_driver *drv)
+static int serio_driver_probe(struct device *dev)
{
- struct serio *serio;
+ struct serio *serio = to_serio_port(dev);
+ struct serio_driver *drv = to_serio_driver(dev->driver);

- down(&serio_sem);
+ drv->connect(serio, drv);
+ return serio->drv ? 0 : -ENODEV;
+}

- list_add_tail(&drv->node, &serio_driver_list);
+static int serio_driver_remove(struct device *dev)
+{
+ struct serio *serio = to_serio_port(dev);
+ struct serio_driver *drv = to_serio_driver(dev->driver);

- drv->driver.bus = &serio_bus;
- driver_register(&drv->driver);
+ drv->disconnect(serio);
+ return 0;
+}

- if (drv->manual_bind)
- goto out;
+void serio_register_driver(struct serio_driver *drv)
+{
+ down(&serio_sem);

-start_over:
- list_for_each_entry(serio, &serio_list, node) {
- if (!serio->drv) {
- serio_connect_port(serio, drv);
- /*
- * if new child appeared then the list is changed,
- * we need to start over
- */
- if (serio->child)
- goto start_over;
- }
- }
+ drv->driver.bus = &serio_bus;
+ drv->driver.probe = serio_driver_probe;
+ drv->driver.remove = serio_driver_remove;
+ driver_register(&drv->driver);

-out:
up(&serio_sem);
}

@@ -639,21 +666,19 @@
struct serio *serio;

down(&serio_sem);
-
- list_del_init(&drv->node);
+ drv->manual_bind = 1; /* so serio_find_driver ignores it */

start_over:
list_for_each_entry(serio, &serio_list, node) {
if (serio->drv == drv) {
serio_disconnect_port(serio);
- serio_connect_port(serio, NULL);
+ serio_find_driver(serio);
/* we could've deleted some ports, restart */
goto start_over;
}
}

driver_unregister(&drv->driver);
-
up(&serio_sem);
}

@@ -666,6 +691,17 @@
up(&serio->drv_sem);
}

+static int serio_bus_match(struct device *dev, struct device_driver *drv)
+{
+ struct serio *serio = to_serio_port(dev);
+ struct serio_driver *serio_drv = to_serio_driver(drv);
+
+ if (serio->manual_bind || serio_drv->manual_bind)
+ return 0;
+
+ return serio_match_port(serio_drv->ids, serio);
+}
+
/* called from serio_driver->connect/disconnect methods under serio_sem */
int serio_open(struct serio *serio, struct serio_driver *drv)
{
@@ -697,7 +733,7 @@

if (likely(serio->drv)) {
ret = serio->drv->interrupt(serio, data, dfl, regs);
- } else if (!dfl) {
+ } else if (!dfl && serio->registered) {
serio_rescan(serio);
ret = IRQ_HANDLED;
}
@@ -710,12 +746,13 @@
static int __init serio_init(void)
{
if (!(serio_pid = kernel_thread(serio_thread, NULL, CLONE_KERNEL))) {
- printk(KERN_WARNING "serio: Failed to start kseriod\n");
+ printk(KERN_ERR "serio: Failed to start kseriod\n");
return -1;
}

serio_bus.dev_attrs = serio_device_attrs;
serio_bus.drv_attrs = serio_driver_attrs;
+ serio_bus.match = serio_bus_match;
bus_register(&serio_bus);

return 0;
diff -Nru a/include/linux/serio.h b/include/linux/serio.h
--- a/include/linux/serio.h 2004-12-29 01:53:31 -05:00
+++ b/include/linux/serio.h 2004-12-29 01:53:31 -05:00
@@ -51,6 +51,7 @@
struct semaphore drv_sem; /* protects serio->drv so attributes can pin driver */

struct device dev;
+ unsigned int registered; /* port has been fully registered with driver core */

struct list_head node;
};
@@ -72,8 +73,6 @@
void (*cleanup)(struct serio *);

struct device_driver driver;
-
- struct list_head node;
};
#define to_serio_driver(d) container_of(d, struct serio_driver, driver)

@@ -83,10 +82,18 @@
void serio_reconnect(struct serio *serio);
irqreturn_t serio_interrupt(struct serio *serio, unsigned char data, unsigned int flags, struct pt_regs *regs);

-void serio_register_port(struct serio *serio);
-void serio_register_port_delayed(struct serio *serio);
+void __serio_register_port(struct serio *serio, struct module *owner);
+static inline void serio_register_port(struct serio *serio)
+{
+ __serio_register_port(serio, THIS_MODULE);
+}
+
void serio_unregister_port(struct serio *serio);
-void serio_unregister_port_delayed(struct serio *serio);
+void __serio_unregister_port_delayed(struct serio *serio, struct module *owner);
+static inline void serio_unregister_port_delayed(struct serio *serio)
+{
+ __serio_unregister_port_delayed(serio, THIS_MODULE);
+}

void serio_register_driver(struct serio_driver *drv);
void serio_unregister_driver(struct serio_driver *drv);