From 697e55d35dcb441cc5bd800efae0f98ec8d63fd9 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 20 Oct 2014 23:01:04 -0500 Subject: [PATCH] greybus: improve module cleanup code When a module gets destroyed all of its state and the state of its interfaces and connections (etc.) need to be torn down. This is not now being done properly. Add this teardown code. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/battery-gb.c | 8 ++++++++ drivers/staging/greybus/connection.c | 24 ++++++++++++++++++++++++ drivers/staging/greybus/connection.h | 1 + drivers/staging/greybus/core.c | 13 ++++++------- drivers/staging/greybus/greybus.h | 5 +++++ drivers/staging/greybus/interface.c | 14 ++++++++++++++ drivers/staging/greybus/interface.h | 1 + drivers/staging/greybus/module.c | 23 +++++++++++++++++++---- 8 files changed, 78 insertions(+), 11 deletions(-) diff --git a/drivers/staging/greybus/battery-gb.c b/drivers/staging/greybus/battery-gb.c index 5d0db61..a3b2bee 100644 --- a/drivers/staging/greybus/battery-gb.c +++ b/drivers/staging/greybus/battery-gb.c @@ -145,6 +145,14 @@ int gb_battery_device_init(struct gb_connection *connection) return 0; } +void gb_battery_device_exit(struct gb_connection *connection) +{ + struct gb_battery *gb = connection->private; + + power_supply_unregister(&gb->bat); + kfree(gb); +} + void gb_battery_disconnect(struct gb_module *gmod) { #if 0 diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c index 09fe25d..e2340d8 100644 --- a/drivers/staging/greybus/connection.c +++ b/drivers/staging/greybus/connection.c @@ -220,3 +220,27 @@ int gb_connection_init(struct gb_connection *connection) } return -ENXIO; } + +void gb_connection_exit(struct gb_connection *connection) +{ + switch (connection->protocol) { + case GREYBUS_PROTOCOL_I2C: + gb_i2c_device_exit(connection); + break; + case GREYBUS_PROTOCOL_GPIO: + gb_gpio_controller_exit(connection); + break; + case GREYBUS_PROTOCOL_BATTERY: + gb_battery_device_exit(connection); + break; + case GREYBUS_PROTOCOL_CONTROL: + case GREYBUS_PROTOCOL_AP: + case GREYBUS_PROTOCOL_UART: + case GREYBUS_PROTOCOL_HID: + case GREYBUS_PROTOCOL_VENDOR: + default: + gb_connection_err(connection, "unimplemented protocol %u", + (u32)connection->protocol); + break; + } +} diff --git a/drivers/staging/greybus/connection.h b/drivers/staging/greybus/connection.h index bb22c52..685c1ff 100644 --- a/drivers/staging/greybus/connection.h +++ b/drivers/staging/greybus/connection.h @@ -35,6 +35,7 @@ struct gb_connection *gb_connection_create(struct gb_interface *interface, void gb_connection_destroy(struct gb_connection *connection); int gb_connection_init(struct gb_connection *connection); +void gb_connection_exit(struct gb_connection *connection); struct gb_connection *gb_hd_connection_find(struct greybus_host_device *hd, u16 cport_id); diff --git a/drivers/staging/greybus/core.c b/drivers/staging/greybus/core.c index b59dee1..bc27ad6 100644 --- a/drivers/staging/greybus/core.c +++ b/drivers/staging/greybus/core.c @@ -119,10 +119,9 @@ static void greybus_module_release(struct device *dev) { struct gb_module *gmod = to_gb_module(dev); - kfree(gmod); + gb_module_destroy(gmod); } - static struct device_type greybus_module_type = { .name = "greybus_module", .release = greybus_module_release, @@ -157,7 +156,7 @@ void gb_add_module(struct greybus_host_device *hd, u8 module_id, */ if (!gb_manifest_parse(gmod, data, size)) { dev_err(hd->parent, "manifest error\n"); - goto error; + goto err_module; } /* @@ -180,14 +179,14 @@ void gb_add_module(struct greybus_host_device *hd, u8 module_id, retval = device_add(&gmod->dev); if (retval) - goto error; + goto err_device; gb_module_interfaces_init(gmod); - return; -error: - gb_module_destroy(gmod); + return; +err_device: put_device(&gmod->dev); +err_module: greybus_module_release(&gmod->dev); } diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h index c09572c..bbd90b4 100644 --- a/drivers/staging/greybus/greybus.h +++ b/drivers/staging/greybus/greybus.h @@ -265,8 +265,13 @@ void gb_deregister_cport_complete(u16 cport_id); extern const struct attribute_group *greybus_module_groups[]; int gb_i2c_device_init(struct gb_connection *connection); +void gb_i2c_device_exit(struct gb_connection *connection); + int gb_battery_device_init(struct gb_connection *connection); +void gb_battery_device_exit(struct gb_connection *connection); + int gb_gpio_controller_init(struct gb_connection *connection); +void gb_gpio_controller_exit(struct gb_connection *connection); int gb_tty_init(void); void gb_tty_exit(void); diff --git a/drivers/staging/greybus/interface.c b/drivers/staging/greybus/interface.c index 0c2fdd3..645f05b 100644 --- a/drivers/staging/greybus/interface.c +++ b/drivers/staging/greybus/interface.c @@ -54,6 +54,8 @@ void gb_interface_destroy(struct gb_interface *interface) list_del(&interface->links); spin_unlock_irq(&gb_interfaces_lock); + gb_interface_connections_exit(interface); + /* kref_put(gmod); */ kfree(interface); } @@ -72,3 +74,15 @@ int gb_interface_connections_init(struct gb_interface *interface) return ret; } + +void gb_interface_connections_exit(struct gb_interface *interface) +{ + struct gb_connection *connection; + struct gb_connection *next; + + list_for_each_entry_safe(connection, next, &interface->connections, + interface_links) { + gb_connection_exit(connection); + gb_connection_destroy(connection); + } +} diff --git a/drivers/staging/greybus/interface.h b/drivers/staging/greybus/interface.h index 1019a98..f0c9e1d 100644 --- a/drivers/staging/greybus/interface.h +++ b/drivers/staging/greybus/interface.h @@ -23,5 +23,6 @@ struct gb_interface *gb_interface_create(struct gb_module *gmod, u8 module_id); void gb_interface_destroy(struct gb_interface *interface); int gb_interface_connections_init(struct gb_interface *interface); +void gb_interface_connections_exit(struct gb_interface *interface); #endif /* __INTERFACE_H */ diff --git a/drivers/staging/greybus/module.c b/drivers/staging/greybus/module.c index 699cd00..2883947 100644 --- a/drivers/staging/greybus/module.c +++ b/drivers/staging/greybus/module.c @@ -44,6 +44,15 @@ const struct greybus_module_id *gb_module_match_id(struct gb_module *gmod, return NULL; } +static void gb_module_interfaces_exit(struct gb_module *gmod) +{ + struct gb_interface *interface; + struct gb_interface *next; + + list_for_each_entry_safe(interface, next, &gmod->interfaces, links) + gb_interface_destroy(interface); +} + /* * A Greybus module represents a user-replacable component on an Ara * phone. @@ -62,7 +71,7 @@ struct gb_module *gb_module_create(struct greybus_host_device *hd, u8 module_id) return NULL; gmod->hd = hd; /* XXX refcount? */ - gmod->module_id = module_id; + gmod->module_id = module_id; /* XXX check for dups */ INIT_LIST_HEAD(&gmod->interfaces); spin_lock_irq(&gb_modules_lock); @@ -80,15 +89,21 @@ void gb_module_destroy(struct gb_module *gmod) if (WARN_ON(!gmod)) return; - kfree(gmod->product_string); - kfree(gmod->vendor_string); - spin_lock_irq(&gb_modules_lock); list_del(&gmod->links); spin_unlock_irq(&gb_modules_lock); + gb_module_interfaces_exit(gmod); + /* XXX Do something with gmod->gb_tty */ + + put_device(&gmod->dev); + /* kfree(gmod->dev->name); */ + + kfree(gmod->product_string); + kfree(gmod->vendor_string); /* kref_put(module->hd); */ + kfree(gmod); } -- 2.7.4