greybus: interface: move route creation to interface activation

Creating and destroying a route to an interface is arguably an interface
operation and belongs with the interface code.

Add new interface_activate and interface_deactivate helpers that will be
used to activate and deactivate an interface in the new interface boot
sequence.

Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

diff --git a/drivers/staging/greybus/interface.c b/drivers/staging/greybus/interface.c
index a13b221..48f64fb 100644 (file)
--- a/drivers/staging/greybus/interface.c
+++ b/drivers/staging/greybus/interface.c
@@ -10,6 +10,71 @@
 #include "greybus.h"
 
 
+static int gb_interface_route_create(struct gb_interface *intf)
+{
+       struct gb_svc *svc = intf->hd->svc;
+       u8 intf_id = intf->interface_id;
+       u8 device_id;
+       int ret;
+
+       /*
+        * Create a device id for the interface:
+        * - device id 0 (GB_DEVICE_ID_SVC) belongs to the SVC
+        * - device id 1 (GB_DEVICE_ID_AP) belongs to the AP
+        *
+        * XXX Do we need to allocate device ID for SVC or the AP here? And what
+        * XXX about an AP with multiple interface blocks?
+        */
+       ret = ida_simple_get(&svc->device_id_map,
+                            GB_DEVICE_ID_MODULES_START, 0, GFP_KERNEL);
+       if (ret < 0) {
+               dev_err(&intf->dev, "failed to allocate device id: %d\n", ret);
+               return ret;
+       }
+       device_id = ret;
+
+       ret = gb_svc_intf_device_id(svc, intf_id, device_id);
+       if (ret) {
+               dev_err(&intf->dev, "failed to set device id %u: %d\n",
+                               device_id, ret);
+               goto err_ida_remove;
+       }
+
+       /* Create a two-way route between the AP and the new interface. */
+       ret = gb_svc_route_create(svc, svc->ap_intf_id, GB_DEVICE_ID_AP,
+                                 intf_id, device_id);
+       if (ret) {
+               dev_err(&intf->dev, "failed to create route: %d\n", ret);
+               goto err_svc_id_free;
+       }
+
+       intf->device_id = device_id;
+
+       return 0;
+
+err_svc_id_free:
+       /*
+        * XXX Should we tell SVC that this id doesn't belong to interface
+        * XXX anymore.
+        */
+err_ida_remove:
+       ida_simple_remove(&svc->device_id_map, device_id);
+
+       return ret;
+}
+
+static void gb_interface_route_destroy(struct gb_interface *intf)
+{
+       struct gb_svc *svc = intf->hd->svc;
+
+       if (intf->device_id == GB_DEVICE_ID_BAD)
+               return;
+
+       gb_svc_route_destroy(svc, svc->ap_intf_id, intf->interface_id);
+       ida_simple_remove(&svc->device_id_map, intf->device_id);
+       intf->device_id = GB_DEVICE_ID_BAD;
+}
+
 /*
  * T_TstSrcIncrement is written by the module on ES2 as a stand-in for the
  * init-status attribute ES3_INIT_STATUS. The AP needs to read and clear it
@@ -213,6 +278,22 @@ struct gb_interface *gb_interface_create(struct gb_host_device *hd,
        return intf;
 }
 
+int gb_interface_activate(struct gb_interface *intf)
+{
+       int ret;
+
+       ret = gb_interface_route_create(intf);
+       if (ret)
+               return ret;
+
+       return 0;
+}
+
+void gb_interface_deactivate(struct gb_interface *intf)
+{
+       gb_interface_route_destroy(intf);
+}
+
 /*
  * Enable an interface by enabling its control connection and fetching the
  * manifest and other information over it.

diff --git a/drivers/staging/greybus/interface.h b/drivers/staging/greybus/interface.h
index 96caabc..5dfaea5 100644 (file)
--- a/drivers/staging/greybus/interface.h
+++ b/drivers/staging/greybus/interface.h
@@ -50,6 +50,8 @@ struct gb_interface *gb_interface_find(struct gb_host_device *hd,
 
 struct gb_interface *gb_interface_create(struct gb_host_device *hd,
                                         u8 interface_id);
+int gb_interface_activate(struct gb_interface *intf);
+void gb_interface_deactivate(struct gb_interface *intf);
 int gb_interface_enable(struct gb_interface *intf);
 void gb_interface_disable(struct gb_interface *intf);
 int gb_interface_add(struct gb_interface *intf);

diff --git a/drivers/staging/greybus/svc.c b/drivers/staging/greybus/svc.c
index ca2f34e..5c517d7 100644 (file)
--- a/drivers/staging/greybus/svc.c
+++ b/drivers/staging/greybus/svc.c
@@ -108,7 +108,7 @@ static struct attribute *svc_attrs[] = {
 };
 ATTRIBUTE_GROUPS(svc);
 
-static int gb_svc_intf_device_id(struct gb_svc *svc, u8 intf_id, u8 device_id)
+int gb_svc_intf_device_id(struct gb_svc *svc, u8 intf_id, u8 device_id)
 {
        struct gb_svc_intf_device_id_request request;
 
@@ -251,7 +251,7 @@ void gb_svc_connection_destroy(struct gb_svc *svc, u8 intf1_id, u16 cport1_id,
 EXPORT_SYMBOL_GPL(gb_svc_connection_destroy);
 
 /* Creates bi-directional routes between the devices */
-static int gb_svc_route_create(struct gb_svc *svc, u8 intf1_id, u8 dev1_id,
+int gb_svc_route_create(struct gb_svc *svc, u8 intf1_id, u8 dev1_id,
                               u8 intf2_id, u8 dev2_id)
 {
        struct gb_svc_route_create_request request;
@@ -266,7 +266,7 @@ static int gb_svc_route_create(struct gb_svc *svc, u8 intf1_id, u8 dev1_id,
 }
 
 /* Destroys bi-directional routes between the devices */
-static void gb_svc_route_destroy(struct gb_svc *svc, u8 intf1_id, u8 intf2_id)
+void gb_svc_route_destroy(struct gb_svc *svc, u8 intf1_id, u8 intf2_id)
 {
        struct gb_svc_route_destroy_request request;
        int ret;
@@ -397,78 +397,12 @@ static int gb_svc_hello(struct gb_operation *op)
        return 0;
 }
 
-static int gb_svc_interface_route_create(struct gb_svc *svc,
-                                               struct gb_interface *intf)
-{
-       u8 intf_id = intf->interface_id;
-       u8 device_id;
-       int ret;
-
-       /*
-        * Create a device id for the interface:
-        * - device id 0 (GB_DEVICE_ID_SVC) belongs to the SVC
-        * - device id 1 (GB_DEVICE_ID_AP) belongs to the AP
-        *
-        * XXX Do we need to allocate device ID for SVC or the AP here? And what
-        * XXX about an AP with multiple interface blocks?
-        */
-       ret = ida_simple_get(&svc->device_id_map,
-                            GB_DEVICE_ID_MODULES_START, 0, GFP_KERNEL);
-       if (ret < 0) {
-               dev_err(&svc->dev, "failed to allocate device id for interface %u: %d\n",
-                               intf_id, ret);
-               return ret;
-       }
-       device_id = ret;
-
-       ret = gb_svc_intf_device_id(svc, intf_id, device_id);
-       if (ret) {
-               dev_err(&svc->dev, "failed to set device id %u for interface %u: %d\n",
-                               device_id, intf_id, ret);
-               goto err_ida_remove;
-       }
-
-       /* Create a two-way route between the AP and the new interface. */
-       ret = gb_svc_route_create(svc, svc->ap_intf_id, GB_DEVICE_ID_AP,
-                                 intf_id, device_id);
-       if (ret) {
-               dev_err(&svc->dev, "failed to create route to interface %u (device id %u): %d\n",
-                               intf_id, device_id, ret);
-               goto err_svc_id_free;
-       }
-
-       intf->device_id = device_id;
-
-       return 0;
-
-err_svc_id_free:
-       /*
-        * XXX Should we tell SVC that this id doesn't belong to interface
-        * XXX anymore.
-        */
-err_ida_remove:
-       ida_simple_remove(&svc->device_id_map, device_id);
-
-       return ret;
-}
-
-static void gb_svc_interface_route_destroy(struct gb_svc *svc,
-                                               struct gb_interface *intf)
-{
-       if (intf->device_id == GB_DEVICE_ID_BAD)
-               return;
-
-       gb_svc_route_destroy(svc, svc->ap_intf_id, intf->interface_id);
-       ida_simple_remove(&svc->device_id_map, intf->device_id);
-       intf->device_id = GB_DEVICE_ID_BAD;
-}
-
 static void gb_svc_intf_remove(struct gb_svc *svc, struct gb_interface *intf)
 {
        intf->disconnected = true;
 
        gb_interface_disable(intf);
-       gb_svc_interface_route_destroy(svc, intf);
+       gb_interface_deactivate(intf);
        gb_interface_remove(intf);
 }
 
@@ -546,9 +480,12 @@ static void gb_svc_process_intf_hotplug(struct gb_operation *operation)
                intf->product_id = product_id;
        }
 
-       ret = gb_svc_interface_route_create(svc, intf);
-       if (ret)
+       ret = gb_interface_activate(intf);
+       if (ret) {
+               dev_err(&svc->dev, "failed to activate interface %u: %d\n",
+                               intf_id, ret);
                goto out_interface_add;
+       }
 
        ret = gb_interface_enable(intf);
        if (ret) {

diff --git a/drivers/staging/greybus/svc.h b/drivers/staging/greybus/svc.h
index 904b2dd..8950baf 100644 (file)
--- a/drivers/staging/greybus/svc.h
+++ b/drivers/staging/greybus/svc.h
@@ -48,6 +48,10 @@ int gb_svc_add(struct gb_svc *svc);
 void gb_svc_del(struct gb_svc *svc);
 void gb_svc_put(struct gb_svc *svc);
 
+int gb_svc_intf_device_id(struct gb_svc *svc, u8 intf_id, u8 device_id);
+int gb_svc_route_create(struct gb_svc *svc, u8 intf1_id, u8 dev1_id,
+                              u8 intf2_id, u8 dev2_id);
+void gb_svc_route_destroy(struct gb_svc *svc, u8 intf1_id, u8 intf2_id);
 int gb_svc_connection_create(struct gb_svc *svc, u8 intf1_id, u16 cport1_id,
                             u8 intf2_id, u16 cport2_id, u8 cport_flags);
 void gb_svc_connection_destroy(struct gb_svc *svc, u8 intf1_id, u16 cport1_id,