greybus: hd: fix svc-connection handling

Create the svc connection when registering the host-device and
remove the current svc connection hacks that "upgraded" the svc
connection once the endo id and ap interface id was known.

Note that the old implementation was partly based on a misunderstanding
as it was the remote interface id, rather than the local AP interface id,
that used to define a connection (but we also needed the endo_id).

The remote interface is no longer needed as static connections, such as
the svc connection, are now simply defined by the host-device and host
cport id.

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

6 files changed, 17 insertions, 98 deletions

diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c
index 0ac3a8e..7beab74 100644
--- a/drivers/staging/greybus/connection.c
+++ b/drivers/staging/greybus/connection.c
@@ -77,24 +77,6 @@ static void gb_connection_kref_release(struct kref *kref)
 	mutex_unlock(&connection_mutex);
 }
 
-int svc_update_connection(struct gb_interface *intf,
-			  struct gb_connection *connection)
-{
-	struct gb_bundle *bundle;
-
-	bundle = gb_bundle_create(intf, GB_SVC_BUNDLE_ID, GREYBUS_CLASS_SVC);
-	if (!bundle)
-		return -EINVAL;
-
-	connection->bundle = bundle;
-
-	spin_lock_irq(&gb_connections_lock);
-	list_add(&connection->bundle_links, &bundle->connections);
-	spin_unlock_irq(&gb_connections_lock);
-
-	return 0;
-}
-
 static void gb_connection_init_name(struct gb_connection *connection)
 {
 	u16 hd_cport_id = connection->hd_cport_id;
diff --git a/drivers/staging/greybus/connection.h b/drivers/staging/greybus/connection.h
index 028f278..8006262 100644
--- a/drivers/staging/greybus/connection.h
+++ b/drivers/staging/greybus/connection.h
@@ -51,9 +51,6 @@ struct gb_connection {
 	void				*private;
 };
 
-int svc_update_connection(struct gb_interface *intf,
-			  struct gb_connection *connection);
-
 struct gb_connection *gb_connection_create_static(struct gb_host_device *hd,
 				u16 hd_cport_id, u8 protocol_id);
 struct gb_connection *gb_connection_create_dynamic(struct gb_interface *intf,
diff --git a/drivers/staging/greybus/hd.c b/drivers/staging/greybus/hd.c
index 88d2f01..2ee5d4f 100644
--- a/drivers/staging/greybus/hd.c
+++ b/drivers/staging/greybus/hd.c
@@ -98,6 +98,18 @@ struct gb_host_device *gb_hd_create(struct gb_hd_driver *driver,
 }
 EXPORT_SYMBOL_GPL(gb_hd_create);
 
+static int gb_hd_create_svc_connection(struct gb_host_device *hd)
+{
+	hd->svc_connection = gb_connection_create_static(hd, GB_SVC_CPORT_ID,
+							GREYBUS_PROTOCOL_SVC);
+	if (!hd->svc_connection) {
+		dev_err(&hd->dev, "failed to create svc connection\n");
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
 int gb_hd_add(struct gb_host_device *hd)
 {
 	int ret;
@@ -106,19 +118,10 @@ int gb_hd_add(struct gb_host_device *hd)
 	if (ret)
 		return ret;
 
-	/*
-	 * Initialize AP's SVC protocol connection:
-	 *
-	 * This is required as part of early initialization of the host device
-	 * as we need this connection in order to start any kind of message
-	 * exchange between the AP and the SVC. SVC will start with a
-	 * 'get-version' request followed by a 'svc-hello' message and at that
-	 * time we will create a fully initialized svc-connection, as we need
-	 * endo-id and AP's interface id for that.
-	 */
-	if (!gb_ap_svc_connection_create(hd)) {
+	ret = gb_hd_create_svc_connection(hd);
+	if (ret) {
 		device_del(&hd->dev);
-		return -ENOMEM;
+		return ret;
 	}
 
 	return 0;
@@ -135,9 +138,7 @@ void gb_hd_del(struct gb_host_device *hd)
 	gb_interfaces_remove(hd);
 	gb_endo_remove(hd->endo);
 
-	/* Is the SVC still using the partially uninitialized connection ? */
-	if (hd->initial_svc_connection)
-		gb_connection_destroy(hd->initial_svc_connection);
+	gb_connection_destroy(hd->svc_connection);
 
 	device_del(&hd->dev);
 }
diff --git a/drivers/staging/greybus/hd.h b/drivers/staging/greybus/hd.h
index 6bc9ce3..72716e0 100644
--- a/drivers/staging/greybus/hd.h
+++ b/drivers/staging/greybus/hd.h
@@ -41,8 +41,8 @@ struct gb_host_device {
 	size_t buffer_size_max;
 
 	struct gb_endo *endo;
-	struct gb_connection *initial_svc_connection;
 	struct gb_svc *svc;
+	struct gb_connection *svc_connection;
 
 	/* Private data for the host driver */
 	unsigned long hd_priv[0] __aligned(sizeof(s64));
diff --git a/drivers/staging/greybus/svc.c b/drivers/staging/greybus/svc.c
index 3beb3a2..11fa8c9 100644
--- a/drivers/staging/greybus/svc.c
+++ b/drivers/staging/greybus/svc.c
@@ -49,51 +49,6 @@ static struct attribute *svc_attrs[] = {
 };
 ATTRIBUTE_GROUPS(svc);
 
-/*
- * AP's SVC cport is required early to get messages from the SVC. This happens
- * even before the Endo is created and hence any modules or interfaces.
- *
- * This is a temporary connection, used only at initial bootup.
- */
-struct gb_connection *
-gb_ap_svc_connection_create(struct gb_host_device *hd)
-{
-	struct gb_connection *connection;
-
-	connection = gb_connection_create_static(hd, GB_SVC_CPORT_ID,
-							GREYBUS_PROTOCOL_SVC);
-
-	return connection;
-}
-
-/*
- * We know endo-type and AP's interface id now, lets create a proper svc
- * connection (and its interface/bundle) now and get rid of the initial
- * 'partially' initialized one svc connection.
- */
-static struct gb_interface *
-gb_ap_interface_create(struct gb_host_device *hd,
-		       struct gb_connection *connection, u8 interface_id)
-{
-	struct gb_interface *intf;
-	struct device *dev = &hd->endo->dev;
-
-	intf = gb_interface_create(hd, interface_id);
-	if (!intf) {
-		dev_err(dev, "%s: Failed to create interface with id %hhu\n",
-			__func__, interface_id);
-		return NULL;
-	}
-
-	intf->device_id = GB_DEVICE_ID_AP;
-	svc_update_connection(intf, connection);
-
-	/* Its no longer a partially initialized connection */
-	hd->initial_svc_connection = NULL;
-
-	return intf;
-}
-
 static int gb_svc_intf_device_id(struct gb_svc *svc, u8 intf_id, u8 device_id)
 {
 	struct gb_svc_intf_device_id_request request;
@@ -360,7 +315,6 @@ static int gb_svc_hello(struct gb_operation *op)
 	struct gb_svc *svc = connection->private;
 	struct gb_host_device *hd = connection->hd;
 	struct gb_svc_hello_request *hello_request;
-	struct gb_interface *intf;
 	int ret;
 
 	/*
@@ -389,16 +343,6 @@ static int gb_svc_hello(struct gb_operation *op)
 	if (ret)
 		return ret;
 
-	/*
-	 * Endo and its modules are ready now, fix AP's partially initialized
-	 * svc protocol and its connection.
-	 */
-	intf = gb_ap_interface_create(hd, connection, svc->ap_intf_id);
-	if (!intf) {
-		gb_endo_remove(hd->endo);
-		return ret;
-	}
-
 	return 0;
 }
 
@@ -720,9 +664,6 @@ static int gb_svc_connection_init(struct gb_connection *connection)
 
 	hd->svc = svc;
 
-	WARN_ON(connection->hd->initial_svc_connection);
-	connection->hd->initial_svc_connection = connection;
-
 	return 0;
 }
 
diff --git a/drivers/staging/greybus/svc.h b/drivers/staging/greybus/svc.h
index e05785f..99be041 100644
--- a/drivers/staging/greybus/svc.h
+++ b/drivers/staging/greybus/svc.h
@@ -41,6 +41,4 @@ int gb_svc_dme_peer_set(struct gb_svc *svc, u8 intf_id, u16 attr, u16 selector,
 int gb_svc_protocol_init(void);
 void gb_svc_protocol_exit(void);
 
-struct gb_connection *gb_ap_svc_connection_create(struct gb_host_device *hd);
-
 #endif /* __SVC_H */