[PATCH 0/3] greybus: svc: fix hello processing

[PATCH 0/3] greybus: svc: fix hello processing

[Johan Hovold]

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

[PATCH 1/3] greybus: svc: fix an error handling bug in gb_svc_hello()

[Johan Hovold]

From: Dan Carpenter <dan.carpenter@oracle.com>

Cleanup if gb_svc_queue_deferred_request() fails.

Fixes: ee2f2074fdb2 ("greybus: svc: reconfig APBridgeA-Switch link to handle required load")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/r/20220202072016.GA6748@kili
Cc: stable@vger.kernel.org      # 4.9
[johan: fix commit summary prefix and rename label ]
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/greybus/svc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/greybus/svc.c b/drivers/greybus/svc.c
index ce7740ef449b..51d0875a3480 100644
--- a/drivers/greybus/svc.c
+++ b/drivers/greybus/svc.c
@@ -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

[PATCH 2/3] greybus: svc: clean up hello error path

[Johan Hovold]

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.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/greybus/svc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/greybus/svc.c b/drivers/greybus/svc.c
index 51d0875a3480..4f93d6b2f4ed 100644
--- a/drivers/greybus/svc.c
+++ b/drivers/greybus/svc.c
@@ -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

[PATCH 3/3] greybus: svc: clean up link configuration hack at hello

[Johan Hovold]

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.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/greybus/svc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/greybus/svc.c b/drivers/greybus/svc.c
index 4f93d6b2f4ed..56d2b44d6fef 100644
--- a/drivers/greybus/svc.c
+++ b/drivers/greybus/svc.c
@@ -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

Re: [PATCH 0/3] greybus: svc: fix hello processing

[Dan Carpenter]

Thanks, Johan!

regards,
dan carpenter