greybus: svc: fix hello processing

Dan found and fixed a bug in the SVC HELLO request error handling which was introduced when adding a temporary hack to reconfigure the link at HELLO.

The implementation of the hack abuses the deferred-request processing mechanism to send the link-configuration request, which makes the HELLO processing a bit hard to follow.

The last two patches attempt to remedy this.

This could go into either 5.17-rc or -next.

Johan

Dan Carpenter (1):
greybus: svc: fix an error handling bug in gb_svc_hello()

Johan Hovold (2):
greybus: svc: clean up hello error path
greybus: svc: clean up link configuration hack at hello

 drivers/greybus/svc.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

2.34.1

From: Dan Carpenter <[email protected]>

Cleanup if gb_svc_queue_deferred_request() fails.

---
 drivers/greybus/svc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

index ce7740ef449b..51d0875a3480 100644
@@ -866,8 +866,14 @@ static int gb_svc_hello(struct gb_operation *op)
 
 	gb_svc_debugfs_init(svc);
 
-	return gb_svc_queue_deferred_request(op);
+	ret = gb_svc_queue_deferred_request(op);
+	if (ret)
+		goto err_remove_debugfs;
+
+	return 0;
 
+err_remove_debugfs:
+	gb_svc_debugfs_exit(svc);
 err_unregister_device:
 	gb_svc_watchdog_destroy(svc);
 	device_del(&svc->dev);

2.34.1

While currently safe, it is unnecessary (and confusing) to try to destroy the watchdog when watchdog creation fails.

Change the corresponding error path to only deregister the svc.

---
 drivers/greybus/svc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 51d0875a3480..4f93d6b2f4ed 100644
@@ -861,7 +861,7 @@ static int gb_svc_hello(struct gb_operation *op)
 	ret = gb_svc_watchdog_create(svc);
 	if (ret) {
 		dev_err(&svc->dev, "failed to create watchdog: %d\n", ret);
-		goto err_unregister_device;
+		goto err_deregister_svc;
 	}
 
 	gb_svc_debugfs_init(svc);

@@ -874,9 +874,10 @@ static int gb_svc_hello(struct gb_operation *op)
 
 err_remove_debugfs:
 	gb_svc_debugfs_exit(svc);
-err_unregister_device:
 	gb_svc_watchdog_destroy(svc);
+err_deregister_svc:
 	device_del(&svc->dev);
+
 	return ret;
 }
 

2.34.1

Commit ee2f2074fdb2 ("greybus: svc: reconfig APBridgeA-Switch link to handle required load") added a temporary hack which reconfigures the link at HELLO by abusing the deferred request processing mechanism.

Restructure the HELLO request processing so that the link-configuration work is queued before creating the debugfs files and add a comment explaining why it's there.

---
 drivers/greybus/svc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

index 4f93d6b2f4ed..56d2b44d6fef 100644
@@ -864,16 +864,19 @@ static int gb_svc_hello(struct gb_operation *op)
 		goto err_deregister_svc;
 	}
 
-	gb_svc_debugfs_init(svc);
-
+	/*
+	 * FIXME: This is a temporary hack to reconfigure the link at HELLO
+	 * (which abuses the deferred request processing mechanism).
+	 */
 	ret = gb_svc_queue_deferred_request(op);
 	if (ret)
-		goto err_remove_debugfs;
+		goto err_destroy_watchdog;
+
+	gb_svc_debugfs_init(svc);
 
 	return 0;
 
-err_remove_debugfs:
-	gb_svc_debugfs_exit(svc);
+err_destroy_watchdog:
 	gb_svc_watchdog_destroy(svc);
 err_deregister_svc:
 	device_del(&svc->dev);

2.34.1

Thanks, Johan!

regards,
dan carpenter