From 36602a2981c85de7b0b8600eb12cbad3d80eacd9 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sat, 23 Apr 2016 18:47:25 +0200 Subject: [PATCH] greybus: module: implement controlled module removal Implement controlled module removal through a new module attribute "eject". When a non-zero argument is written to the attribute, all interfaces of the module are disabled (e.g. bundles are deregistered) and deactivated (e.g. powered off) before instructing the SVC to physically eject the module. Note that the module device is not deregistered until the SVC has reported the physical removal of all of its interfaces. A new interface mutex is added to enforce interface state-change serialisation. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- .../greybus/Documentation/sysfs-bus-greybus | 8 ++++ drivers/staging/greybus/interface.c | 22 ++++++++- drivers/staging/greybus/interface.h | 3 ++ drivers/staging/greybus/module.c | 48 ++++++++++++++++++- drivers/staging/greybus/svc.c | 4 ++ 5 files changed, 83 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/Documentation/sysfs-bus-greybus b/drivers/staging/greybus/Documentation/sysfs-bus-greybus index 41ffc40b8efb..a18ee7eed75a 100644 --- a/drivers/staging/greybus/Documentation/sysfs-bus-greybus +++ b/drivers/staging/greybus/Documentation/sysfs-bus-greybus @@ -14,6 +14,14 @@ Description: A Module M on the bus N, where M is the 1-byte interface ID of the module's primary interface. +What: /sys/bus/greybus/device/N-M/eject +Date: March 2016 +KernelVersion: 4.XX +Contact: Greg Kroah-Hartman +Description: + Writing a non-zero argument to this attibute disables the + module's interfaces before physically ejecting it. + What: /sys/bus/greybus/device/N-M/module_id Date: March 2016 KernelVersion: 4.XX diff --git a/drivers/staging/greybus/interface.c b/drivers/staging/greybus/interface.c index d51c5635f22b..2553312dc332 100644 --- a/drivers/staging/greybus/interface.c +++ b/drivers/staging/greybus/interface.c @@ -372,6 +372,7 @@ struct gb_interface *gb_interface_create(struct gb_module *module, intf->interface_id = interface_id; INIT_LIST_HEAD(&intf->bundles); INIT_LIST_HEAD(&intf->manifest_descs); + mutex_init(&intf->mutex); /* Invalid device id to start with */ intf->device_id = GB_INTERFACE_DEVICE_ID_BAD; @@ -388,10 +389,18 @@ struct gb_interface *gb_interface_create(struct gb_module *module, return intf; } +/* + * Activate an interface. + * + * Locking: Caller holds the interface mutex. + */ int gb_interface_activate(struct gb_interface *intf) { int ret; + if (intf->ejected) + return -ENODEV; + ret = gb_interface_read_dme(intf); if (ret) return ret; @@ -403,6 +412,11 @@ int gb_interface_activate(struct gb_interface *intf) return 0; } +/* + * Deactivate an interface. + * + * Locking: Caller holds the interface mutex. + */ void gb_interface_deactivate(struct gb_interface *intf) { gb_interface_route_destroy(intf); @@ -412,6 +426,8 @@ void gb_interface_deactivate(struct gb_interface *intf) * Enable an interface by enabling its control connection, fetching the * manifest and other information over it, and finally registering its child * devices. + * + * Locking: Caller holds the interface mutex. */ int gb_interface_enable(struct gb_interface *intf) { @@ -516,7 +532,11 @@ err_put_control: return ret; } -/* Disable an interface and destroy its bundles. */ +/* + * Disable an interface and destroy its bundles. + * + * Locking: Caller holds the interface mutex. + */ void gb_interface_disable(struct gb_interface *intf) { struct gb_bundle *bundle; diff --git a/drivers/staging/greybus/interface.h b/drivers/staging/greybus/interface.h index 9185c7f1bd32..61399e7ea102 100644 --- a/drivers/staging/greybus/interface.h +++ b/drivers/staging/greybus/interface.h @@ -39,7 +39,10 @@ struct gb_interface { unsigned long quirks; + struct mutex mutex; + bool disconnected; + bool ejected; bool enabled; }; #define to_gb_interface(d) container_of(d, struct gb_interface, dev) diff --git a/drivers/staging/greybus/module.c b/drivers/staging/greybus/module.c index 5c498e0a4eaf..5077253037c8 100644 --- a/drivers/staging/greybus/module.c +++ b/drivers/staging/greybus/module.c @@ -10,6 +10,43 @@ #include "greybus.h" +static ssize_t eject_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct gb_module *module = to_gb_module(dev); + struct gb_interface *intf; + size_t i; + long val; + int ret; + + ret = kstrtol(buf, 0, &val); + if (ret) + return ret; + + if (!val) + return len; + + for (i = 0; i < module->num_interfaces; ++i) { + intf = module->interfaces[i]; + + mutex_lock(&intf->mutex); + /* Set flag to prevent concurrent activation. */ + intf->ejected = true; + gb_interface_disable(intf); + gb_interface_deactivate(intf); + mutex_unlock(&intf->mutex); + } + + /* Tell the SVC to eject the primary interface. */ + ret = gb_svc_intf_eject(module->hd->svc, module->module_id); + if (ret) + return ret; + + return len; +} +static DEVICE_ATTR_WO(eject); + static ssize_t module_id_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -29,6 +66,7 @@ static ssize_t num_interfaces_show(struct device *dev, static DEVICE_ATTR_RO(num_interfaces); static struct attribute *module_attrs[] = { + &dev_attr_eject.attr, &dev_attr_module_id.attr, &dev_attr_num_interfaces.attr, NULL, @@ -101,12 +139,14 @@ static void gb_module_register_interface(struct gb_interface *intf) u8 intf_id = intf->interface_id; int ret; + mutex_lock(&intf->mutex); + ret = gb_interface_activate(intf); if (ret) { dev_err(&module->dev, "failed to activate interface %u: %d\n", intf_id, ret); gb_interface_add(intf); - return; + goto err_unlock; } ret = gb_interface_add(intf); @@ -120,10 +160,14 @@ static void gb_module_register_interface(struct gb_interface *intf) goto err_interface_deactivate; } + mutex_unlock(&intf->mutex); + return; err_interface_deactivate: gb_interface_deactivate(intf); +err_unlock: + mutex_unlock(&intf->mutex); } static void gb_module_deregister_interface(struct gb_interface *intf) @@ -132,8 +176,10 @@ static void gb_module_deregister_interface(struct gb_interface *intf) if (intf->module->disconnected) intf->disconnected = true; + mutex_lock(&intf->mutex); gb_interface_disable(intf); gb_interface_deactivate(intf); + mutex_unlock(&intf->mutex); gb_interface_del(intf); } diff --git a/drivers/staging/greybus/svc.c b/drivers/staging/greybus/svc.c index 94016954a6ab..96b3027db528 100644 --- a/drivers/staging/greybus/svc.c +++ b/drivers/staging/greybus/svc.c @@ -682,6 +682,8 @@ static void gb_svc_intf_reenable(struct gb_svc *svc, struct gb_interface *intf) { int ret; + mutex_lock(&intf->mutex); + /* Mark as disconnected to prevent I/O during disable. */ intf->disconnected = true; gb_interface_disable(intf); @@ -694,6 +696,8 @@ static void gb_svc_intf_reenable(struct gb_svc *svc, struct gb_interface *intf) gb_interface_deactivate(intf); } + + mutex_unlock(&intf->mutex); } static void gb_svc_process_intf_hotplug(struct gb_operation *operation) -- 2.20.1