From 4ccb6b7abb8ee4ff6fc28468ffe893caa730ea13 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Tue, 28 Oct 2014 19:36:00 -0500 Subject: [PATCH] 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 Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/Makefile | 1 + drivers/staging/greybus/connection.c | 17 +++-- drivers/staging/greybus/connection.h | 4 +- drivers/staging/greybus/greybus.h | 1 + drivers/staging/greybus/operation.c | 2 +- drivers/staging/greybus/protocol.c | 122 +++++++++++++++++++++++++++++++++++ drivers/staging/greybus/protocol.h | 26 ++++++++ 7 files changed, 167 insertions(+), 6 deletions(-) create mode 100644 drivers/staging/greybus/protocol.c create mode 100644 drivers/staging/greybus/protocol.h diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile index 42a3944..39874de 100644 --- 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 6d5085d..dac47b3 100644 --- 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 830abe7..8056993c 100644 --- 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 3a8d8f1..f6c90e0 100644 --- 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 0388242..cc278bc 100644 --- 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 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 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 */ -- 2.7.4