greybus: introduce protocol abstraction

Define a protocol structure that will allow protocols to be
registered dynamically.  For now we just introduce a bookkeeping
data structure.  Upcoming patches will move protocol-related methods
into the protocol structure, and will start registering protocol
handlers dynamically.

A list of connections using a given protocol is maintained so we can
tell when a protocol is no longer in use.  This may not be necessary
(we could use a kref instead) but it may turn out to be a good way
to clean things up.

The interface is gb_protocol_get() and gb_protocol_put() for a
connection, allowing the protocol to be looked up and the connection
structure to be inserted into its list.

Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile
index 42a3944f4eeb2253a30096da603637550a2e3336..39874deacab08eae66514354ee20c54ee6f7d914 100644 (file)
--- a/drivers/staging/greybus/Makefile
+++ b/drivers/staging/greybus/Makefile
@@ -7,6 +7,7 @@ greybus-y :=    core.o          \
                module.o        \
                interface.o     \
                connection.o    \
+               protocol.o      \
                operation.o     \
                i2c-gb.o        \
                gpio-gb.o       \

diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c
index 6d5085d2b6aaec4efb8c99f8a26df18e2772c09c..dac47b33855f257395b4a1a6e8272380ac883b94 100644 (file)
--- a/drivers/staging/greybus/connection.c
+++ b/drivers/staging/greybus/connection.c
@@ -116,7 +116,7 @@ protocol_id_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
        struct gb_connection *connection = to_gb_connection(dev);
 
-       return sprintf(buf, "%d", connection->protocol_id);
+       return sprintf(buf, "%d", connection->protocol->id);
 }
 static DEVICE_ATTR_RO(protocol_id);
 
@@ -162,17 +162,23 @@ struct gb_connection *gb_connection_create(struct gb_interface *interface,
        if (!connection)
                return NULL;
 
+       INIT_LIST_HEAD(&connection->protocol_links);
+       if (!gb_protocol_get(connection, protocol_id)) {
+               kfree(connection);
+               return NULL;
+       }
+
        hd = interface->gmod->hd;
        connection->hd = hd;                    /* XXX refcount? */
        if (!gb_connection_hd_cport_id_alloc(connection)) {
                /* kref_put(connection->hd); */
+               gb_protocol_put(connection);
                kfree(connection);
                return NULL;
        }
 
        connection->interface = interface;
        connection->interface_cport_id = cport_id;
-       connection->protocol_id = protocol_id;
        connection->state = GB_CONNECTION_STATE_DISABLED;
 
        connection->dev.parent = &interface->dev;
@@ -188,6 +194,7 @@ struct gb_connection *gb_connection_create(struct gb_interface *interface,
        if (retval) {
                gb_connection_hd_cport_id_free(connection);
                /* kref_put(connection->hd); */
+               gb_protocol_put(connection);
                kfree(connection);
                return NULL;
        }
@@ -228,6 +235,8 @@ void gb_connection_destroy(struct gb_connection *connection)
        spin_unlock_irq(&gb_connections_lock);
 
        gb_connection_hd_cport_id_free(connection);
+       /* kref_put(connection->hd); */
+       gb_protocol_put(connection);
 
        device_del(&connection->dev);
 }
@@ -267,7 +276,7 @@ int gb_connection_init(struct gb_connection *connection)
 
        /* Need to enable the connection to initialize it */
        connection->state = GB_CONNECTION_STATE_ENABLED;
-       switch (connection->protocol_id) {
+       switch (connection->protocol->id) {
        case GREYBUS_PROTOCOL_I2C:
                connection->handler = &gb_i2c_connection_handler;
                break;
@@ -287,7 +296,7 @@ int gb_connection_init(struct gb_connection *connection)
        case GREYBUS_PROTOCOL_VENDOR:
        default:
                gb_connection_err(connection, "unimplemented protocol %hhu",
-                       connection->protocol_id);
+                       connection->protocol->id);
                ret = -ENXIO;
                break;
        }

diff --git a/drivers/staging/greybus/connection.h b/drivers/staging/greybus/connection.h
index 830abe7060ba8a29d5e064e672aae57e6625a235..8056993c00acf1465dc7978b379ef8d890367b67 100644 (file)
--- a/drivers/staging/greybus/connection.h
+++ b/drivers/staging/greybus/connection.h
@@ -39,7 +39,9 @@ struct gb_connection {
 
        struct rb_node                  hd_node;
        struct list_head                interface_links;
-       u8                              protocol_id;
+
+       struct gb_protocol              *protocol;
+       struct list_head                protocol_links;
 
        enum gb_connection_state        state;
 

diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h
index 3a8d8f1c077ca7eda6abf9303aa5099482a53104..f6c90e0c0f22a968ff3239c6dedcd9b6ace1ca2f 100644 (file)
--- a/drivers/staging/greybus/greybus.h
+++ b/drivers/staging/greybus/greybus.h
@@ -26,6 +26,7 @@
 #include "module.h"
 #include "interface.h"
 #include "connection.h"
+#include "protocol.h"
 #include "operation.h"
 
 

diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c
index 0388242d9b79585eed8a439d3679ecd7cff81be7..cc278bcb499d88fdc3e33652f6648319fcce9f76 100644 (file)
--- a/drivers/staging/greybus/operation.c
+++ b/drivers/staging/greybus/operation.c
@@ -200,7 +200,7 @@ static gb_operation_recv_handler gb_operation_recv_handlers[] = {
 
 static void gb_operation_request_handle(struct gb_operation *operation)
 {
-       u8 protocol_id = operation->connection->protocol_id;
+       u8 protocol_id = operation->connection->protocol->id;
 
        /* Subtract one from array size to stay within u8 range */
        if (protocol_id <= (u8)(ARRAY_SIZE(gb_operation_recv_handlers) - 1)) {

diff --git a/drivers/staging/greybus/protocol.c b/drivers/staging/greybus/protocol.c
new file mode 100644 (file)
index 0000000..52944ec
--- /dev/null
+++ b/drivers/staging/greybus/protocol.c
@@ -0,0 +1,122 @@
+/*
+ * Greybus protocol handling
+ *
+ * Copyright 2014 Google Inc.
+ *
+ * Released under the GPLv2 only.
+ */
+
+#include "greybus.h"
+
+/* Global list of registered protocols */
+static DEFINE_SPINLOCK(gb_protocols_lock);
+static LIST_HEAD(gb_protocols);
+
+/* Caller must hold gb_protocols_lock */
+struct gb_protocol *_gb_protocol_find(u8 id)
+{
+       struct gb_protocol *protocol;
+
+       list_for_each_entry(protocol, &gb_protocols, links)
+               if (protocol->id == id)
+                       return protocol;
+       return NULL;
+}
+
+/* This is basically for debug */
+static struct gb_protocol *gb_protocol_find(u8 id)
+{
+       struct gb_protocol *protocol;
+
+       spin_lock_irq(&gb_protocols_lock);
+       protocol = _gb_protocol_find(id);
+       spin_unlock_irq(&gb_protocols_lock);
+
+       return protocol;
+}
+
+/* Returns true if protocol was succesfully registered, false otherwise */
+bool gb_protocol_register(u8 id)
+{
+       struct gb_protocol *protocol;
+       struct gb_protocol *existing;
+
+       /* Initialize it speculatively */
+       protocol = kzalloc(sizeof(*protocol), GFP_KERNEL);
+       if (!protocol)
+               return false;
+       protocol->id = id;
+       INIT_LIST_HEAD(&protocol->connections);
+
+       spin_lock_irq(&gb_protocols_lock);
+       existing = _gb_protocol_find(id);
+       if (!existing)
+               list_add(&protocol->links, &gb_protocols);
+       spin_unlock_irq(&gb_protocols_lock);
+
+       if (existing) {
+               kfree(protocol);
+               protocol = NULL;
+       }
+
+       return protocol != NULL;
+}
+
+/* Returns true if successful, false otherwise */
+bool gb_protocol_deregister(struct gb_protocol *protocol)
+{
+       spin_lock_irq(&gb_protocols_lock);
+       if (list_empty(&protocol->connections))
+               list_del(&protocol->links);
+       else
+               protocol = NULL;        /* Protocol is still in use */
+       spin_unlock_irq(&gb_protocols_lock);
+       kfree(protocol);
+
+       return protocol != NULL;
+}
+
+/* Returns true if successful, false otherwise */
+bool gb_protocol_get(struct gb_connection *connection, u8 id)
+{
+       struct gb_protocol *protocol;
+
+       /* Sanity */
+       if (!list_empty(&connection->protocol_links) ||
+                       !connection->protocol->id) {
+               gb_connection_err(connection,
+                       "connection already has protocol");
+               return false;
+       }
+
+       spin_lock_irq(&gb_protocols_lock);
+       protocol = _gb_protocol_find(id);
+       if (protocol)
+               list_add(&connection->protocol_links, &protocol->connections);
+       spin_unlock_irq(&gb_protocols_lock);
+       connection->protocol = protocol;
+
+       return protocol != NULL;
+}
+
+void gb_protocol_put(struct gb_connection *connection)
+{
+       struct gb_protocol *protocol = connection->protocol;
+
+       /* Sanity checks */
+       if (list_empty(&connection->protocol_links)) {
+               gb_connection_err(connection,
+                       "connection protocol not recorded");
+               return;
+       }
+       if (!protocol || gb_protocol_find(protocol->id) != protocol)  {
+               gb_connection_err(connection,
+                       "connection has undefined protocol");
+               return;
+       }
+
+       spin_lock_irq(&gb_protocols_lock);
+       list_del(&connection->protocol_links);
+       connection->protocol = NULL;
+       spin_unlock_irq(&gb_protocols_lock);
+}

diff --git a/drivers/staging/greybus/protocol.h b/drivers/staging/greybus/protocol.h
new file mode 100644 (file)
index 0000000..d244e9d
--- /dev/null
+++ b/drivers/staging/greybus/protocol.h
@@ -0,0 +1,26 @@
+/*
+ * Greybus protocol handling
+ *
+ * Copyright 2014 Google Inc.
+ *
+ * Released under the GPLv2 only.
+ */
+
+#ifndef __PROTOCOL_H
+#define __PROTOCOL_H
+
+#include "greybus.h"
+
+struct gb_protocol {
+       u8                              id;
+       struct list_head                connections;    /* protocol users */
+       struct list_head                links;          /* global list */
+};
+
+bool gb_protocol_register(u8 id);
+bool gb_protocol_deregister(struct gb_protocol *protocol);
+
+bool gb_protocol_get(struct gb_connection *connection, u8 id);
+void gb_protocol_put(struct gb_connection *connection);
+
+#endif /* __PROTOCOL_H */