From 0e44765743c06664773475cd07684a70a29a6816 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Wed, 5 Nov 2014 16:12:51 -0600 Subject: [PATCH] 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 Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/connection.c | 11 ++-- drivers/staging/greybus/connection.h | 1 - drivers/staging/greybus/protocol.c | 91 ++++++++++++---------------- drivers/staging/greybus/protocol.h | 7 +-- 4 files changed, 47 insertions(+), 63 deletions(-) diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c index 703c286f5631..e3000f7eb799 100644 --- 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 8056993c00ac..ea54334238bd 100644 --- 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 704b180f0ffb..347d52c3445c 100644 --- 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 d53f67def779..aa7b5548e8a8 100644 --- 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 */ -- 2.39.5