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>

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...

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)

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);
 
 

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;

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 */

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;