greybus: connection: move legacy-protocol handling to legacy driver
authorJohan Hovold <johan@hovoldconsulting.com>
Tue, 19 Jan 2016 11:51:16 +0000 (12:51 +0100)
committerGreg Kroah-Hartman <gregkh@google.com>
Tue, 19 Jan 2016 20:17:13 +0000 (12:17 -0800)
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
drivers/staging/greybus/connection.h
drivers/staging/greybus/legacy.c
drivers/staging/greybus/protocol.c

index 3339ef956788469bd7982cd7018cd0fb7bbaefbd..7a967befc89734b40e17e427994d0fa9f58af3a3 100644 (file)
 #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;
-}
index 8813f54eab9209001c93d5f5013586e9c551f276..33deeb64dd29e002e4804965f74e46e10e2b468d 100644 (file)
@@ -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);
 
index e8d9cb93b831dacfa15f1a76fc7a93f66df9e425..fd847f42376fe9f13f6c98e85ff6f27a56c1f847 100644 (file)
@@ -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);
        }
 }
 
index d69f64801b4f476fdb5b20a72109596049a85dac..057ab6057ee6248e4c04f933defd8889bf35ede0 100644 (file)
@@ -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);