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 <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

diff --git a/drivers/staging/greybus/Documentation/sysfs-bus-greybus b/drivers/staging/greybus/Documentation/sysfs-bus-greybus
index 41ffc40..a18ee7e 100644 (file)
--- 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 <greg@kroah.com>
+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 d51c563..2553312 100644 (file)
--- 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 9185c7f..61399e7 100644 (file)
--- 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 5c498e0..5077253 100644 (file)
--- 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 9401695..96b3027 100644 (file)
--- 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)