greybus: connection: set request handlers at creation

Set the connection request handler at creation rather than when enabling
the connection.

This is possible now that connections are created by the drivers that
use them rather than by core at manifest parsing time.
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>

drivers/staging/greybus/connection.c

@@ -118,6 +118,7 @@ static void gb_connection_init_name(struct gb_connection *connection)
  * @intf:		remote interface, or NULL for static connections
  * @bundle:		remote-interface bundle (may be NULL)
  * @cport_id:		remote-interface cport id, or 0 for static connections
+ * @handler:		request handler (may be NULL)
  *
  * Create a Greybus connection, representing the bidirectional link
  * between a CPort on a (local) Greybus host device and a CPort on
@@ -135,7 +136,8 @@ static void gb_connection_init_name(struct gb_connection *connection)
 static struct gb_connection *
 _gb_connection_create(struct gb_host_device *hd, int hd_cport_id,
 				struct gb_interface *intf,
-				struct gb_bundle *bundle, int cport_id)
+				struct gb_bundle *bundle, int cport_id,
+				gb_request_handler_t handler)
 {
 	struct gb_connection *connection;
 	struct ida *id_map = &hd->cport_id_map;
@@ -176,8 +178,8 @@ _gb_connection_create(struct gb_host_device *hd, int hd_cport_id,
 	connection->intf_cport_id = cport_id;
 	connection->hd = hd;
 	connection->intf = intf;
-
 	connection->bundle = bundle;
+	connection->handler = handler;
 	connection->state = GB_CONNECTION_STATE_DISABLED;
 
 	atomic_set(&connection->op_cycle, 0);
@@ -221,23 +223,26 @@ _gb_connection_create(struct gb_host_device *hd, int hd_cport_id,
 }
 
 struct gb_connection *
-gb_connection_create_static(struct gb_host_device *hd, u16 hd_cport_id)
+gb_connection_create_static(struct gb_host_device *hd, u16 hd_cport_id,
+					gb_request_handler_t handler)
 {
-	return _gb_connection_create(hd, hd_cport_id, NULL, NULL, 0);
+	return _gb_connection_create(hd, hd_cport_id, NULL, NULL, 0, handler);
 }
 
 struct gb_connection *
 gb_connection_create_control(struct gb_interface *intf)
 {
-	return _gb_connection_create(intf->hd, -1, intf, NULL, 0);
+	return _gb_connection_create(intf->hd, -1, intf, NULL, 0, NULL);
 }
 
 struct gb_connection *
-gb_connection_create(struct gb_bundle *bundle, u16 cport_id)
+gb_connection_create(struct gb_bundle *bundle, u16 cport_id,
+					gb_request_handler_t handler)
 {
 	struct gb_interface *intf = bundle->intf;
 
-	return _gb_connection_create(intf->hd, -1, intf, bundle, cport_id);
+	return _gb_connection_create(intf->hd, -1, intf, bundle, cport_id,
+					handler);
 }
 EXPORT_SYMBOL_GPL(gb_connection_create);
 
@@ -424,39 +429,42 @@ gb_connection_flush_incoming_operations(struct gb_connection *connection,
 	}
 }
 
-int gb_connection_enable(struct gb_connection *connection,
-				gb_request_handler_t handler)
+/*
+ * _gb_connection_enable() - enable a connection
+ * @connection:		connection to enable
+ * @rx:			whether to enable incoming requests
+ *
+ * Connection-enable helper for DISABLED->ENABLED, DISABLED->ENABLED_TX, and
+ * ENABLED_TX->ENABLED state transitions.
+ *
+ * Locking: Caller holds connection->mutex.
+ */
+static int _gb_connection_enable(struct gb_connection *connection, bool rx)
 {
 	int ret;
 
-	mutex_lock(&connection->mutex);
-
-	if (connection->state == GB_CONNECTION_STATE_ENABLED)
-		goto out_unlock;
-
+	/* Handle ENABLED_TX -> ENABLED transitions. */
 	if (connection->state == GB_CONNECTION_STATE_ENABLED_TX) {
-		if (!handler)
-			goto out_unlock;
+		if (!(connection->handler && rx))
+			return 0;
 
 		spin_lock_irq(&connection->lock);
-		connection->handler = handler;
 		connection->state = GB_CONNECTION_STATE_ENABLED;
 		spin_unlock_irq(&connection->lock);
 
-		goto out_unlock;
+		return 0;
 	}
 
 	ret = gb_connection_hd_cport_enable(connection);
 	if (ret)
-		goto err_unlock;
+		return ret;
 
 	ret = gb_connection_svc_connection_create(connection);
 	if (ret)
 		goto err_hd_cport_disable;
 
 	spin_lock_irq(&connection->lock);
-	connection->handler = handler;
-	if (handler)
+	if (connection->handler && rx)
 		connection->state = GB_CONNECTION_STATE_ENABLED;
 	else
 		connection->state = GB_CONNECTION_STATE_ENABLED_TX;
@@ -466,28 +474,60 @@ int gb_connection_enable(struct gb_connection *connection,
 	if (ret)
 		goto err_svc_destroy;
 
-out_unlock:
-	mutex_unlock(&connection->mutex);
-
 	return 0;
 
 err_svc_destroy:
 	spin_lock_irq(&connection->lock);
 	connection->state = GB_CONNECTION_STATE_DISABLED;
 	gb_connection_cancel_operations(connection, -ESHUTDOWN);
-	connection->handler = NULL;
 	spin_unlock_irq(&connection->lock);
 
 	gb_connection_svc_connection_destroy(connection);
 err_hd_cport_disable:
 	gb_connection_hd_cport_disable(connection);
-err_unlock:
+
+	return ret;
+}
+
+int gb_connection_enable(struct gb_connection *connection)
+{
+	int ret = 0;
+
+	mutex_lock(&connection->mutex);
+
+	if (connection->state == GB_CONNECTION_STATE_ENABLED)
+		goto out_unlock;
+
+	ret = _gb_connection_enable(connection, true);
+out_unlock:
 	mutex_unlock(&connection->mutex);
 
 	return ret;
 }
 EXPORT_SYMBOL_GPL(gb_connection_enable);
 
+int gb_connection_enable_tx(struct gb_connection *connection)
+{
+	int ret = 0;
+
+	mutex_lock(&connection->mutex);
+
+	if (connection->state == GB_CONNECTION_STATE_ENABLED) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	if (connection->state == GB_CONNECTION_STATE_ENABLED_TX)
+		goto out_unlock;
+
+	ret = _gb_connection_enable(connection, false);
+out_unlock:
+	mutex_unlock(&connection->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(gb_connection_enable_tx);
+
 void gb_connection_disable_rx(struct gb_connection *connection)
 {
 	mutex_lock(&connection->mutex);
@@ -499,7 +539,6 @@ void gb_connection_disable_rx(struct gb_connection *connection)
 	}
 	connection->state = GB_CONNECTION_STATE_ENABLED_TX;
 	gb_connection_flush_incoming_operations(connection, -ESHUTDOWN);
-	connection->handler = NULL;
 	spin_unlock_irq(&connection->lock);
 
 out_unlock:
@@ -518,7 +557,6 @@ void gb_connection_disable(struct gb_connection *connection)
 	spin_lock_irq(&connection->lock);
 	connection->state = GB_CONNECTION_STATE_DISABLED;
 	gb_connection_cancel_operations(connection, -ESHUTDOWN);
-	connection->handler = NULL;
 	spin_unlock_irq(&connection->lock);
 
 	gb_connection_svc_connection_destroy(connection);

drivers/staging/greybus/connection.h

@@ -55,10 +55,10 @@ struct gb_connection {
 };
 
 struct gb_connection *gb_connection_create_static(struct gb_host_device *hd,
-				u16 hd_cport_id);
+				u16 hd_cport_id, gb_request_handler_t handler);
 struct gb_connection *gb_connection_create_control(struct gb_interface *intf);
 struct gb_connection *gb_connection_create(struct gb_bundle *bundle,
-				u16 cport_id);
+				u16 cport_id, gb_request_handler_t handler);
 void gb_connection_destroy(struct gb_connection *connection);
 
 static inline bool gb_connection_is_static(struct gb_connection *connection)
@@ -66,12 +66,8 @@ static inline bool gb_connection_is_static(struct gb_connection *connection)
 	return !connection->intf;
 }
 
-int gb_connection_enable(struct gb_connection *connection,
-					gb_request_handler_t handler);
-static inline int gb_connection_enable_tx(struct gb_connection *connection)
-{
-	return gb_connection_enable(connection, NULL);
-}
+int gb_connection_enable(struct gb_connection *connection);
+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);
 

drivers/staging/greybus/legacy.c

@@ -49,18 +49,12 @@ static int legacy_request_handler(struct gb_operation *operation)
 static int legacy_connection_init(struct legacy_connection *lc)
 {
 	struct gb_connection *connection = lc->connection;
-	gb_request_handler_t handler;
 	int ret;
 
 	dev_dbg(&connection->bundle->dev, "%s - %s\n", __func__,
 			connection->name);
 
-	if (connection->protocol->request_recv)
-		handler = legacy_request_handler;
-	else
-		handler = NULL;
-
-	ret = gb_connection_enable(connection, handler);
+	ret = gb_connection_enable(connection);
 	if (ret)
 		return ret;
 
@@ -102,6 +96,7 @@ static int legacy_connection_create(struct legacy_connection *lc,
 {
 	struct gb_connection *connection;
 	struct gb_protocol *protocol;
+	gb_request_handler_t handler;
 	u8 major, minor;
 	int ret;
 
@@ -122,7 +117,13 @@ static int legacy_connection_create(struct legacy_connection *lc,
 		return -EPROTONOSUPPORT;
 	}
 
-	connection = gb_connection_create(bundle, le16_to_cpu(desc->id));
+	if (protocol->request_recv)
+		handler = legacy_request_handler;
+	else
+		handler = NULL;
+
+	connection = gb_connection_create(bundle, le16_to_cpu(desc->id),
+						handler);
 	if (IS_ERR(connection)) {
 		ret = PTR_ERR(connection);
 		goto err_protocol_put;

drivers/staging/greybus/svc.c

@@ -895,7 +895,8 @@ struct gb_svc *gb_svc_create(struct gb_host_device *hd)
 		goto err_put_device;
 	}
 
-	svc->connection = gb_connection_create_static(hd, GB_SVC_CPORT_ID);
+	svc->connection = gb_connection_create_static(hd, GB_SVC_CPORT_ID,
+						gb_svc_request_handler);
 	if (IS_ERR(svc->connection)) {
 		dev_err(&svc->dev, "failed to create connection: %ld\n",
 				PTR_ERR(svc->connection));
@@ -922,7 +923,7 @@ int gb_svc_add(struct gb_svc *svc)
 	 * is added from the connection request handler when enough
 	 * information has been received.
 	 */
-	ret = gb_connection_enable(svc->connection, gb_svc_request_handler);
+	ret = gb_connection_enable(svc->connection);
 	if (ret)
 		return ret;