greybus: connection: move legacy-protocol handling to legacy driver

Move legacy protocol and connection handling to the legacy driver.

Rename the former global functions using a common legacy_ prefix.

Note that all legacy protocols suffer from a connection initialisation
race in that the protocol-specific initialisation, which includes
allocation of protocol-specific state containers, can not happen until
*after* the connection has been enabled. This is a major flaw in the
original design that we can now finally address by converting the legacy
protocol drivers into proper bundle (class) drivers.

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

drivers/staging/greybus/connection.c

@@ -12,10 +12,6 @@
 #include "greybus.h"
 
 
-static int gb_connection_bind_protocol(struct gb_connection *connection);
-static void gb_connection_unbind_protocol(struct gb_connection *connection);
-
-
 static DEFINE_SPINLOCK(gb_connections_lock);
 
 /* This is only used at initialization time; no locking is required. */
@@ -340,24 +336,6 @@ gb_connection_control_disconnected(struct gb_connection *connection)
 	}
 }
 
-/*
- * Request protocol version supported by the module.
- */
-static int gb_connection_protocol_get_version(struct gb_connection *connection)
-{
-	int ret;
-
-	ret = gb_protocol_get_version(connection);
-	if (ret) {
-		dev_err(&connection->hd->dev,
-			"%s: failed to get protocol version: %d\n",
-			connection->name, ret);
-		return ret;
-	}
-
-	return 0;
-}
-
 /*
  * Cancel all active operations on a connection.
  *
@@ -526,63 +504,6 @@ out_unlock:
 }
 EXPORT_SYMBOL_GPL(gb_connection_disable);
 
-static int gb_legacy_request_handler(struct gb_operation *operation)
-{
-	struct gb_protocol *protocol = operation->connection->protocol;
-
-	return protocol->request_recv(operation->type, operation);
-}
-
-int gb_connection_legacy_init(struct gb_connection *connection)
-{
-	gb_request_handler_t handler;
-	int ret;
-
-	ret = gb_connection_bind_protocol(connection);
-	if (ret)
-		return ret;
-
-	if (connection->protocol->request_recv)
-		handler = gb_legacy_request_handler;
-	else
-		handler = NULL;
-
-	ret = gb_connection_enable(connection, handler);
-	if (ret)
-		goto err_unbind_protocol;
-
-	ret = gb_connection_protocol_get_version(connection);
-	if (ret)
-		goto err_disable;
-
-	ret = connection->protocol->connection_init(connection);
-	if (ret)
-		goto err_disable;
-
-	return 0;
-
-err_disable:
-	gb_connection_disable(connection);
-err_unbind_protocol:
-	gb_connection_unbind_protocol(connection);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(gb_connection_legacy_init);
-
-void gb_connection_legacy_exit(struct gb_connection *connection)
-{
-	if (connection->state == GB_CONNECTION_STATE_DISABLED)
-		return;
-
-	gb_connection_disable(connection);
-
-	connection->protocol->connection_exit(connection);
-
-	gb_connection_unbind_protocol(connection);
-}
-EXPORT_SYMBOL_GPL(gb_connection_legacy_exit);
-
 /*
  * Tear down a previously set up connection.
  */
@@ -639,31 +560,3 @@ void gb_connection_latency_tag_disable(struct gb_connection *connection)
 	}
 }
 EXPORT_SYMBOL_GPL(gb_connection_latency_tag_disable);
-
-static int gb_connection_bind_protocol(struct gb_connection *connection)
-{
-	struct gb_protocol *protocol;
-
-	protocol = gb_protocol_get(connection->protocol_id,
-				   connection->major,
-				   connection->minor);
-	if (!protocol) {
-		dev_err(&connection->hd->dev,
-				"protocol 0x%02x version %u.%u not found\n",
-				connection->protocol_id,
-				connection->major, connection->minor);
-		return -EPROTONOSUPPORT;
-	}
-	connection->protocol = protocol;
-
-	return 0;
-}
-
-static void gb_connection_unbind_protocol(struct gb_connection *connection)
-{
-	struct gb_protocol *protocol = connection->protocol;
-
-	gb_protocol_put(protocol);
-
-	connection->protocol = NULL;
-}

drivers/staging/greybus/connection.h

@@ -78,9 +78,6 @@ static inline int gb_connection_enable_tx(struct gb_connection *connection)
 void gb_connection_disable_rx(struct gb_connection *connection);
 void gb_connection_disable(struct gb_connection *connection);
 
-int gb_connection_legacy_init(struct gb_connection *connection);
-void gb_connection_legacy_exit(struct gb_connection *connection);
-
 void greybus_data_rcvd(struct gb_host_device *hd, u16 cport_id,
 			u8 *data, size_t length);
 

drivers/staging/greybus/legacy.c

@@ -9,6 +9,106 @@
 
 #include "greybus.h"
 #include "legacy.h"
+#include "protocol.h"
+
+
+static int legacy_connection_get_version(struct gb_connection *connection)
+{
+	int ret;
+
+	ret = gb_protocol_get_version(connection);
+	if (ret) {
+		dev_err(&connection->hd->dev,
+			"%s: failed to get protocol version: %d\n",
+			connection->name, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int legacy_connection_bind_protocol(struct gb_connection *connection)
+{
+	struct gb_protocol *protocol;
+
+	protocol = gb_protocol_get(connection->protocol_id,
+				   connection->major,
+				   connection->minor);
+	if (!protocol) {
+		dev_err(&connection->hd->dev,
+				"protocol 0x%02x version %u.%u not found\n",
+				connection->protocol_id,
+				connection->major, connection->minor);
+		return -EPROTONOSUPPORT;
+	}
+	connection->protocol = protocol;
+
+	return 0;
+}
+
+static void legacy_connection_unbind_protocol(struct gb_connection *connection)
+{
+	struct gb_protocol *protocol = connection->protocol;
+
+	gb_protocol_put(protocol);
+
+	connection->protocol = NULL;
+}
+
+static int legacy_request_handler(struct gb_operation *operation)
+{
+	struct gb_protocol *protocol = operation->connection->protocol;
+
+	return protocol->request_recv(operation->type, operation);
+}
+
+static int legacy_connection_init(struct gb_connection *connection)
+{
+	gb_request_handler_t handler;
+	int ret;
+
+	ret = legacy_connection_bind_protocol(connection);
+	if (ret)
+		return ret;
+
+	if (connection->protocol->request_recv)
+		handler = legacy_request_handler;
+	else
+		handler = NULL;
+
+	ret = gb_connection_enable(connection, handler);
+	if (ret)
+		goto err_unbind_protocol;
+
+	ret = legacy_connection_get_version(connection);
+	if (ret)
+		goto err_disable;
+
+	ret = connection->protocol->connection_init(connection);
+	if (ret)
+		goto err_disable;
+
+	return 0;
+
+err_disable:
+	gb_connection_disable(connection);
+err_unbind_protocol:
+	legacy_connection_unbind_protocol(connection);
+
+	return ret;
+}
+
+static void legacy_connection_exit(struct gb_connection *connection)
+{
+	if (connection->state == GB_CONNECTION_STATE_DISABLED)
+		return;
+
+	gb_connection_disable(connection);
+
+	connection->protocol->connection_exit(connection);
+
+	legacy_connection_unbind_protocol(connection);
+}
 
 static int legacy_probe(struct gb_bundle *bundle,
 			const struct greybus_bundle_id *id)
@@ -23,7 +123,7 @@ static int legacy_probe(struct gb_bundle *bundle,
 		dev_dbg(&bundle->dev, "enabling connection %s\n",
 				connection->name);
 
-		ret = gb_connection_legacy_init(connection);
+		ret = legacy_connection_init(connection);
 		if (ret)
 			goto err_connections_disable;
 	}
@@ -33,7 +133,7 @@ static int legacy_probe(struct gb_bundle *bundle,
 err_connections_disable:
 	list_for_each_entry_reverse(connection, &bundle->connections,
 							bundle_links) {
-		gb_connection_legacy_exit(connection);
+		legacy_connection_exit(connection);
 	}
 
 	return ret;
@@ -48,7 +148,7 @@ static void legacy_disconnect(struct gb_bundle *bundle)
 
 	list_for_each_entry_reverse(connection, &bundle->connections,
 							bundle_links) {
-		gb_connection_legacy_exit(connection);
+		legacy_connection_exit(connection);
 	}
 }
 

drivers/staging/greybus/protocol.c

@@ -141,6 +141,7 @@ struct gb_protocol *gb_protocol_get(u8 id, u8 major, u8 minor)
 
 	return protocol;
 }
+EXPORT_SYMBOL_GPL(gb_protocol_get);
 
 int gb_protocol_get_version(struct gb_connection *connection)
 {
@@ -197,3 +198,4 @@ void gb_protocol_put(struct gb_protocol *protocol)
 out:
 	spin_unlock_irq(&gb_protocols_lock);
 }
+EXPORT_SYMBOL_GPL(gb_protocol_put);