greybus: count rather than list protocol users

We don't really need a list of protocol users, we can just keep
track of how many there are.  Get rid of the list and use a count
instead.

Also, have gb_protocol_get() return the protocol rather than assigning
a passed-in connection pointer's protocol.  Make a comparable change
to the gb_protocol_put() interface.

Get rid of gb_protocol_find() (the version that locks), because it
is no longer needed.

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

diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c
index 703c286..e3000f7 100644 (file)
--- a/drivers/staging/greybus/connection.c
+++ b/drivers/staging/greybus/connection.c
@@ -164,9 +164,9 @@ struct gb_connection *gb_connection_create(struct gb_interface *interface,
        if (!connection)
                return NULL;
 
-       INIT_LIST_HEAD(&connection->protocol_links);
        /* XXX Will have to establish connections to get version */
-       if (!gb_protocol_get(connection, protocol_id, major, minor)) {
+       connection->protocol = gb_protocol_get(protocol_id, major, minor);
+       if (!connection->protocol) {
                pr_err("protocol 0x%02hhx not found\n", protocol_id);
                kfree(connection);
                return NULL;
@@ -175,7 +175,7 @@ struct gb_connection *gb_connection_create(struct gb_interface *interface,
        hd = interface->gmod->hd;
        connection->hd = hd;
        if (!gb_connection_hd_cport_id_alloc(connection)) {
-               gb_protocol_put(connection);
+               gb_protocol_put(connection->protocol);
                kfree(connection);
                return NULL;
        }
@@ -198,7 +198,7 @@ struct gb_connection *gb_connection_create(struct gb_interface *interface,
                pr_err("failed to add connection device for cport 0x%04hx\n",
                        cport_id);
                gb_connection_hd_cport_id_free(connection);
-               gb_protocol_put(connection);
+               gb_protocol_put(connection->protocol);
                put_device(&connection->dev);
                return NULL;
        }
@@ -239,8 +239,7 @@ 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);
+       gb_protocol_put(connection->protocol);
 
        device_del(&connection->dev);
 }

diff --git a/drivers/staging/greybus/connection.h b/drivers/staging/greybus/connection.h
index 8056993..ea54334 100644 (file)
--- a/drivers/staging/greybus/connection.h
+++ b/drivers/staging/greybus/connection.h
@@ -41,7 +41,6 @@ struct gb_connection {
        struct list_head                interface_links;
 
        struct gb_protocol              *protocol;
-       struct list_head                protocol_links;
 
        enum gb_connection_state        state;
 

diff --git a/drivers/staging/greybus/protocol.c b/drivers/staging/greybus/protocol.c
index 704b180..347d52c 100644 (file)
--- a/drivers/staging/greybus/protocol.c
+++ b/drivers/staging/greybus/protocol.c
@@ -24,18 +24,6 @@ static struct gb_protocol *_gb_protocol_find(u8 id, u8 major, u8 minor)
        return NULL;
 }
 
-/* This is basically for debug */
-static struct gb_protocol *gb_protocol_find(u8 id, u8 major, u8 minor)
-{
-       struct gb_protocol *protocol;
-
-       spin_lock_irq(&gb_protocols_lock);
-       protocol = _gb_protocol_find(id, major, minor);
-       spin_unlock_irq(&gb_protocols_lock);
-
-       return protocol;
-}
-
 /* Returns true if protocol was succesfully registered, false otherwise */
 bool gb_protocol_register(u8 id, u8 major, u8 minor)
 {
@@ -49,7 +37,6 @@ bool gb_protocol_register(u8 id, u8 major, u8 minor)
        protocol->id = id;
        protocol->major = major;
        protocol->minor = minor;
-       INIT_LIST_HEAD(&protocol->connections);
 
        spin_lock_irq(&gb_protocols_lock);
        existing = _gb_protocol_find(id, major, minor);
@@ -68,64 +55,64 @@ bool gb_protocol_register(u8 id, u8 major, u8 minor)
 /* Returns true if successful, false otherwise */
 bool gb_protocol_deregister(struct gb_protocol *protocol)
 {
+       u8 protocol_count;
+
        spin_lock_irq(&gb_protocols_lock);
-       if (list_empty(&protocol->connections))
-               list_del(&protocol->links);
-       else
-               protocol = NULL;        /* Protocol is still in use */
+       protocol = _gb_protocol_find(protocol->id, protocol->major,
+                                               protocol->minor);
+       if (protocol) {
+               protocol_count = protocol->count;
+               if (!protocol_count)
+                       list_del(&protocol->links);
+       }
        spin_unlock_irq(&gb_protocols_lock);
        kfree(protocol);
 
-       return protocol != NULL;
+       return protocol && !protocol_count;
 }
 
-/* Returns true if successful, false otherwise */
-bool
-gb_protocol_get(struct gb_connection *connection, u8 id, u8 major, u8 minor)
+/* Returns the requested protocol if available, or a null pointer */
+struct gb_protocol *gb_protocol_get(u8 id, u8 major, u8 minor)
 {
        struct gb_protocol *protocol;
-
-       /* Sanity */
-       if (!list_empty(&connection->protocol_links) ||
-                       !connection->protocol->id) {
-               gb_connection_err(connection,
-                       "connection already has protocol");
-               return false;
-       }
+       u8 protocol_count;
 
        spin_lock_irq(&gb_protocols_lock);
        protocol = _gb_protocol_find(id, major, minor);
-       if (protocol)
-               list_add(&connection->protocol_links, &protocol->connections);
+       if (protocol) {
+               protocol_count = protocol->count;
+               if (protocol_count != U8_MAX)
+                       protocol->count++;
+       }
        spin_unlock_irq(&gb_protocols_lock);
-       connection->protocol = protocol;
 
-       return protocol != NULL;
+       if (protocol)
+               WARN_ON(protocol_count == U8_MAX);
+       else
+               pr_err("protocol id %hhu version %hhu.%hhu not found\n",
+                       id, major, minor);
+
+       return protocol;
 }
 
-void gb_protocol_put(struct gb_connection *connection)
+void gb_protocol_put(struct gb_protocol *protocol)
 {
-       struct gb_protocol *protocol = connection->protocol;
        u8 major = protocol->major;
        u8 minor = protocol->minor;
-
-       /* Sanity checks */
-       if (list_empty(&connection->protocol_links)) {
-               gb_connection_err(connection,
-                       "connection protocol not recorded");
-               return;
-       }
-       if (!protocol) {
-               gb_connection_err(connection, "connection has no protocol");
-               return;
-       }
-       if (gb_protocol_find(protocol->id, major, minor) != protocol)  {
-               gb_connection_err(connection, "connection protocol not found");
-               return;
-       }
+       u8 protocol_count;
 
        spin_lock_irq(&gb_protocols_lock);
-       list_del(&connection->protocol_links);
-       connection->protocol = NULL;
+       protocol = _gb_protocol_find(protocol->id, protocol->major,
+                                               protocol->minor);
+       if (protocol) {
+               protocol_count = protocol->count;
+               if (protocol_count)
+                       protocol->count--;
+       }
        spin_unlock_irq(&gb_protocols_lock);
+       if (protocol)
+               WARN_ON(!protocol_count);
+       else
+               pr_err("protocol id %hhu version %hhu.%hhu not found\n",
+                       protocol->id, major, minor);
 }

diff --git a/drivers/staging/greybus/protocol.h b/drivers/staging/greybus/protocol.h
index d53f67d..aa7b554 100644 (file)
--- a/drivers/staging/greybus/protocol.h
+++ b/drivers/staging/greybus/protocol.h
@@ -20,16 +20,15 @@ struct gb_protocol {
        u8                      id;
        u8                      major;
        u8                      minor;
+       u8                      count;
 
-       struct list_head        connections;    /* protocol users */
        struct list_head        links;          /* global list */
 };
 
 bool gb_protocol_register(u8 id, u8 major, u8 minor);
 bool gb_protocol_deregister(struct gb_protocol *protocol);
 
-bool gb_protocol_get(struct gb_connection *connection, u8 id,
-                               u8 major, u8 minor);
-void gb_protocol_put(struct gb_connection *connection);
+struct gb_protocol *gb_protocol_get(u8 id, u8 major, u8 minor);
+void gb_protocol_put(struct gb_protocol *protocol);
 
 #endif /* __PROTOCOL_H */