greybus: interface: Fetch interface id instead of module id during setup

There can be more than one interface on a module and we need to know the
interface for which the event has occurred.

But at the same time we may not need the module id at all. During initial phase
when AP is probed, the AP will receive the unique Endo id which shall be enough
to draw relationships between interface and module ids.

Code for that isn't available today and so lets create another routine to get
module id (which needs to be fixed separately), which will simply return
interface id passed to it.

Now that we have interface id, update rest of the code to use it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

diff --git a/drivers/staging/greybus/ap.c b/drivers/staging/greybus/ap.c
index ea197ac57b524001be1de03c816dd5b6e744d934..132ecb4a7974d9dc17d41cb7e53313be62c59a26 100644 (file)
--- a/drivers/staging/greybus/ap.c
+++ b/drivers/staging/greybus/ap.c
@@ -140,10 +140,10 @@ static void svc_management(struct svc_function_unipro_management *management,
                hd->device_id = management->ap_id.device_id;
                break;
        case SVC_MANAGEMENT_LINK_UP:
-               intf = gb_interface_find(hd, management->link_up.module_id);
+               intf = gb_interface_find(hd, management->link_up.interface_id);
                if (!intf) {
-                       dev_err(hd->parent, "Module ID %d not found\n",
-                               management->link_up.module_id);
+                       dev_err(hd->parent, "Interface ID %d not found\n",
+                               management->link_up.interface_id);
                        return;
                }
                ret = gb_bundle_init(intf,
@@ -151,9 +151,8 @@ static void svc_management(struct svc_function_unipro_management *management,
                                management->link_up.device_id);
                if (ret) {
                        dev_err(hd->parent,
-                               "error %d initializing interface %hhu bundle %hhu\n",
-                               ret, management->link_up.module_id,
-                               management->link_up.interface_id);
+                               "error %d initializing bundles for interface %hhu\n",
+                               ret, management->link_up.interface_id);
                        return;
                }
                break;
@@ -165,11 +164,11 @@ static void svc_management(struct svc_function_unipro_management *management,
 static void svc_hotplug(struct svc_function_hotplug *hotplug,
                        int payload_length, struct greybus_host_device *hd)
 {
-       u8 module_id = hotplug->module_id;
+       u8 interface_id = hotplug->interface_id;
 
        switch (hotplug->hotplug_event) {
        case SVC_HOTPLUG_EVENT:
-               /* Add a new module to the system */
+               /* Add a new interface to the system */
                if (payload_length < 0x03) {
                        /* Hotplug message is at least 3 bytes big */
                        dev_err(hd->parent,
@@ -177,13 +176,13 @@ static void svc_hotplug(struct svc_function_hotplug *hotplug,
                                payload_length);
                        return;
                }
-               dev_dbg(hd->parent, "module id %d added\n", module_id);
-               gb_add_interface(hd, module_id, hotplug->data,
+               dev_dbg(hd->parent, "interface id %d added\n", interface_id);
+               gb_add_interface(hd, interface_id, hotplug->data,
                                 payload_length - 0x02);
                break;
 
        case SVC_HOTUNPLUG_EVENT:
-               /* Remove a module from the system */
+               /* Remove a interface from the system */
                if (payload_length != 0x02) {
                        /* Hotunplug message is only 2 bytes big */
                        dev_err(hd->parent,
@@ -191,8 +190,8 @@ static void svc_hotplug(struct svc_function_hotplug *hotplug,
                                payload_length);
                        return;
                }
-               dev_dbg(hd->parent, "module id %d removed\n", module_id);
-               gb_remove_interface(hd, module_id);
+               dev_dbg(hd->parent, "interface id %d removed\n", interface_id);
+               gb_remove_interface(hd, interface_id);
                break;
 
        default:
@@ -206,7 +205,7 @@ static void svc_hotplug(struct svc_function_hotplug *hotplug,
 static void svc_power(struct svc_function_power *power,
                      int payload_length, struct greybus_host_device *hd)
 {
-       u8 module_id = power->module_id;
+       u8 interface_id = power->interface_id;
 
        /*
         * The AP is only allowed to get a Battery Status message, not a Battery
@@ -230,8 +229,8 @@ static void svc_power(struct svc_function_power *power,
                return;
        }
 
-       dev_dbg(hd->parent, "power status for module id %d is %d\n",
-               module_id, power->status.status);
+       dev_dbg(hd->parent, "power status for interface id %d is %d\n",
+               interface_id, power->status.status);
 
        // FIXME - do something with the power information, like update our
        // battery information...

diff --git a/drivers/staging/greybus/interface.c b/drivers/staging/greybus/interface.c
index 122281f2cd2b0739ad3303926d0ee0902095088f..5c64bc77348764dde8252fd904c33bb5716e97d9 100644 (file)
--- a/drivers/staging/greybus/interface.c
+++ b/drivers/staging/greybus/interface.c
@@ -77,12 +77,12 @@ gb_interface_match_id(struct gb_interface *intf,
 // FIXME, odds are you don't want to call this function, rework the caller to
 // not need it please.
 struct gb_interface *gb_interface_find(struct greybus_host_device *hd,
-                                      u8 module_id)
+                                      u8 interface_id)
 {
        struct gb_interface *intf;
 
        list_for_each_entry(intf, &hd->interfaces, links)
-               if (intf->module->module_id == module_id)
+               if (intf->interface_id == interface_id)
                        return intf;
 
        return NULL;
@@ -105,28 +105,28 @@ struct device_type greybus_interface_type = {
  * phone.  An interface is the physical connection on that module.  A
  * module may have more than one interface.
  *
- * Create a gb_interface structure to represent a discovered module.
- * The position within the Endo is encoded in the "module_id" argument.
+ * Create a gb_interface structure to represent a discovered interface.
+ * The position of interface within the Endo is encoded in "interface_id"
+ * argument.
+ *
  * Returns a pointer to the new interfce or a null pointer if a
  * failure occurs due to memory exhaustion.
  */
 static struct gb_interface *gb_interface_create(struct greybus_host_device *hd,
-                                               u8 module_id)
+                                               u8 interface_id)
 {
        struct gb_module *module;
        struct gb_interface *intf;
        int retval;
-       u8 interface_id = module_id;
 
-       // FIXME we need an interface id here to check for this properly!
        intf = gb_interface_find(hd, interface_id);
        if (intf) {
-               dev_err(hd->parent, "Duplicate module id %d will not be created\n",
-                       module_id);
+               dev_err(hd->parent, "Duplicate interface with interface-id: %d will not be created\n",
+                       interface_id);
                return NULL;
        }
 
-       module = gb_module_find_or_create(hd, module_id);
+       module = gb_module_find_or_create(hd, get_module_id(interface_id));
        if (!module)
                return NULL;
 
@@ -136,6 +136,7 @@ static struct gb_interface *gb_interface_create(struct greybus_host_device *hd,
 
        intf->hd = hd;          /* XXX refcount? */
        intf->module = module;
+       intf->interface_id = interface_id;
        INIT_LIST_HEAD(&intf->bundles);
        INIT_LIST_HEAD(&intf->manifest_descs);
 
@@ -149,8 +150,8 @@ static struct gb_interface *gb_interface_create(struct greybus_host_device *hd,
 
        retval = device_add(&intf->dev);
        if (retval) {
-               pr_err("failed to add module device for id 0x%02hhx\n",
-                       module_id);
+               pr_err("failed to add interface device for id 0x%02hhx\n",
+                      interface_id);
                goto free_intf;
        }
 
@@ -199,12 +200,12 @@ static void gb_interface_destroy(struct gb_interface *intf)
  * Pass in a buffer that _should_ contain a Greybus module manifest
  * and register a greybus device structure with the kernel core.
  */
-void gb_add_interface(struct greybus_host_device *hd, u8 module_id,
-                     u8 *data, int size)
+void gb_add_interface(struct greybus_host_device *hd, u8 interface_id, u8 *data,
+                     int size)
 {
        struct gb_interface *intf;
 
-       intf = gb_interface_create(hd, module_id);
+       intf = gb_interface_create(hd, interface_id);
        if (!intf) {
                dev_err(hd->parent, "failed to create interface\n");
                return;
@@ -234,14 +235,15 @@ err_parse:
        gb_interface_destroy(intf);
 }
 
-void gb_remove_interface(struct greybus_host_device *hd, u8 module_id)
+void gb_remove_interface(struct greybus_host_device *hd, u8 interface_id)
 {
-       struct gb_interface *intf = gb_interface_find(hd, module_id);
+       struct gb_interface *intf = gb_interface_find(hd, interface_id);
 
        if (intf)
                gb_interface_destroy(intf);
        else
-               dev_err(hd->parent, "interface id %d not found\n", module_id);
+               dev_err(hd->parent, "interface id %d not found\n",
+                       interface_id);
 }
 
 void gb_remove_interfaces(struct greybus_host_device *hd)

diff --git a/drivers/staging/greybus/interface.h b/drivers/staging/greybus/interface.h
index f444e311bfac31c951a9d3b8717988a612f22168..45ce99609656e7c57da31cebf7792580fdbdfef8 100644 (file)
--- a/drivers/staging/greybus/interface.h
+++ b/drivers/staging/greybus/interface.h
@@ -49,11 +49,11 @@ const struct greybus_interface_id *
                              const struct greybus_interface_id *id);
 
 struct gb_interface *gb_interface_find(struct greybus_host_device *hd,
-                                      u8 module_id);
+                                      u8 interface_id);
 
-void gb_add_interface(struct greybus_host_device *hd, u8 module_id,
-                     u8 *data, int size);
-void gb_remove_interface(struct greybus_host_device *hd, u8 module_id);
+void gb_add_interface(struct greybus_host_device *hd, u8 interface_id, u8 *data,
+                     int size);
+void gb_remove_interface(struct greybus_host_device *hd, u8 interface_id);
 void gb_remove_interfaces(struct greybus_host_device *hd);
 
 

diff --git a/drivers/staging/greybus/module.c b/drivers/staging/greybus/module.c
index 538182b60dd921433dccfd8281f7d77c4763f28f..e8c1c07cff60f3a0bc59db132e4f3739631897a3 100644 (file)
--- a/drivers/staging/greybus/module.c
+++ b/drivers/staging/greybus/module.c
@@ -58,6 +58,17 @@ struct device_type greybus_module_type = {
        .release =      greybus_module_release,
 };
 
+u8 get_module_id(u8 interface_id)
+{
+       /*
+        * FIXME:
+        *
+        * We should be able to find it from Endo ID passed during greybus
+        * control operation, while setting up AP.
+        */
+       return interface_id;
+}
+
 static int module_find(struct device *dev, void *data)
 {
        struct gb_module *module;

diff --git a/drivers/staging/greybus/module.h b/drivers/staging/greybus/module.h
index 4f02e46301e1bc31d4a6427ab520052e10c631f3..f3e3bdd6a67182a8f749e7b5e84eeea8b182e0e2 100644 (file)
--- a/drivers/staging/greybus/module.h
+++ b/drivers/staging/greybus/module.h
@@ -24,5 +24,6 @@ struct gb_module *gb_module_find_or_create(struct greybus_host_device *hd,
                                           u8 module_id);
 void gb_module_remove(struct gb_module *module);
 
+u8 get_module_id(u8 interface_id);
 
 #endif /* __MODULE_H */

diff --git a/drivers/staging/greybus/svc_msg.h b/drivers/staging/greybus/svc_msg.h
index 471baa59078eba088b3aec6f7dd3aab55e639df6..cb7bb19975de8df4ed146d8d32486dd15977d1f2 100644 (file)
--- a/drivers/staging/greybus/svc_msg.h
+++ b/drivers/staging/greybus/svc_msg.h
@@ -52,13 +52,12 @@ struct svc_function_unipro_set_route {
 };
 
 struct svc_function_unipro_link_up {
-       __u8    module_id;
-       __u8    interface_id;
+       __u8    interface_id;   /* Interface id within the Endo */
        __u8    device_id;
 };
 
 struct svc_function_ap_id {
-       __u8    module_id;
+       __u8    interface_id;
        __u8    device_id;
 };
 
@@ -82,13 +81,9 @@ enum svc_function_hotplug_event {
        SVC_HOTUNPLUG_EVENT     = 0x01,
 };
 
-/* XXX
- * Does a hotplug come from module insertion, or from detection of each
- * interface (UniPro device) in a module?  Assume the former for now.
- */
 struct svc_function_hotplug {
        __u8    hotplug_event;  /* enum svc_function_hotplug_event */
-       __u8    module_id;
+       __u8    interface_id;   /* Interface id within the Endo */
        __u8    data[0];
 };
 
@@ -121,7 +116,7 @@ struct svc_function_power_battery_status_request {
  */
 struct svc_function_power {
        __u8    power_type;     /* enum svc_function_power_type */
-       __u8    module_id;
+       __u8    interface_id;
        union {
                struct svc_function_power_battery_status                status;
                struct svc_function_power_battery_status_request        request;