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>

diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c
index 3339ef9..7a967be 100644 (file)
--- a/drivers/staging/greybus/connection.c
+++ b/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;
-}

diff --git a/drivers/staging/greybus/connection.h b/drivers/staging/greybus/connection.h
index 8813f54..33deeb6 100644 (file)
--- a/drivers/staging/greybus/connection.h
+++ b/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);
 

diff --git a/drivers/staging/greybus/legacy.c b/drivers/staging/greybus/legacy.c
index e8d9cb9..fd847f4 100644 (file)
--- a/drivers/staging/greybus/legacy.c
+++ b/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);
        }
 }
 

diff --git a/drivers/staging/greybus/protocol.c b/drivers/staging/greybus/protocol.c
index d69f648..057ab60 100644 (file)
--- a/drivers/staging/greybus/protocol.c
+++ b/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);