[PATCH 0/3] slimbus: use 'time_left' instead of 'timeout' with wait_for_*() functions

[PATCH 0/3] slimbus: use 'time_left' instead of 'timeout' with wait_for_*() functions

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_*() functions causing patterns like:

        timeout = wait_for_completion_timeout(...)
        if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
obvious and self explaining.

This is part of a tree-wide series. The rest of the patches can be found here
(some parts may still be WIP):

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/time_left

Because these patches are generated, I audit them before sending. This is why I
will send series step by step. Build bot is happy with these patches, though.
No functional changes intended.

Wolfram Sang (3):
  slimbus: messaging: use 'time_left' variable with
    wait_for_completion_timeout()
  slimbus: qcom-ctrl: use 'time_left' variable with
    wait_for_completion_timeout()
  slimbus: qcom-ngd-ctrl: use 'time_left' variable with
    wait_for_completion_timeout()

 drivers/slimbus/messaging.c     |  9 +++++----
 drivers/slimbus/qcom-ctrl.c     |  7 ++++---
 drivers/slimbus/qcom-ngd-ctrl.c | 29 ++++++++++++++++-------------
 3 files changed, 25 insertions(+), 20 deletions(-)

-- 
2.43.0

[PATCH 2/3] slimbus: qcom-ctrl: use 'time_left' variable with wait_for_completion_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:

	timeout = wait_for_completion_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Fix to the proper variable type 'unsigned long' while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/slimbus/qcom-ctrl.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/slimbus/qcom-ctrl.c b/drivers/slimbus/qcom-ctrl.c
index 400b7b385a44..c613c7517f99 100644
--- a/drivers/slimbus/qcom-ctrl.c
+++ b/drivers/slimbus/qcom-ctrl.c
@@ -330,7 +330,8 @@ static int qcom_xfer_msg(struct slim_controller *sctrl,
 	void *pbuf = slim_alloc_txbuf(ctrl, txn, &done);
 	unsigned long ms = txn->rl + HZ;
 	u8 *puc;
-	int ret = 0, timeout, retries = QCOM_BUF_ALLOC_RETRIES;
+	int ret = 0, retries = QCOM_BUF_ALLOC_RETRIES;
+	unsigned long time_left;
 	u8 la = txn->la;
 	u32 *head;
 	/* HW expects length field to be excluded */
@@ -374,9 +375,9 @@ static int qcom_xfer_msg(struct slim_controller *sctrl,
 		memcpy(puc, txn->msg->wbuf, txn->msg->num_bytes);
 
 	qcom_slim_queue_tx(ctrl, head, txn->rl, MGR_TX_MSG);
-	timeout = wait_for_completion_timeout(&done, msecs_to_jiffies(ms));
+	time_left = wait_for_completion_timeout(&done, msecs_to_jiffies(ms));
 
-	if (!timeout) {
+	if (!time_left) {
 		dev_err(ctrl->dev, "TX timed out:MC:0x%x,mt:0x%x", txn->mc,
 					txn->mt);
 		ret = -ETIMEDOUT;
-- 
2.43.0

[PATCH 3/3] slimbus: qcom-ngd-ctrl: use 'time_left' variable with wait_for_completion_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:

	timeout = wait_for_completion_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Fix to the proper variable type 'unsigned long' while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/slimbus/qcom-ngd-ctrl.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index efeba8275a66..21b4008de4f3 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -789,7 +789,8 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
 	struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(sctrl->dev);
 	DECLARE_COMPLETION_ONSTACK(tx_sent);
 	DECLARE_COMPLETION_ONSTACK(done);
-	int ret, timeout, i;
+	int ret, i;
+	unsigned long time_left;
 	u8 wbuf[SLIM_MSGQ_BUF_LEN];
 	u8 rbuf[SLIM_MSGQ_BUF_LEN];
 	u32 *pbuf;
@@ -891,8 +892,8 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
 		return ret;
 	}
 
-	timeout = wait_for_completion_timeout(&tx_sent, HZ);
-	if (!timeout) {
+	time_left = wait_for_completion_timeout(&tx_sent, HZ);
+	if (!time_left) {
 		dev_err(sctrl->dev, "TX timed out:MC:0x%x,mt:0x%x", txn->mc,
 					txn->mt);
 		mutex_unlock(&ctrl->tx_lock);
@@ -900,8 +901,8 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
 	}
 
 	if (usr_msg) {
-		timeout = wait_for_completion_timeout(&done, HZ);
-		if (!timeout) {
+		time_left = wait_for_completion_timeout(&done, HZ);
+		if (!time_left) {
 			dev_err(sctrl->dev, "TX timed out:MC:0x%x,mt:0x%x",
 				txn->mc, txn->mt);
 			mutex_unlock(&ctrl->tx_lock);
@@ -917,7 +918,8 @@ static int qcom_slim_ngd_xfer_msg_sync(struct slim_controller *ctrl,
 				       struct slim_msg_txn *txn)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
-	int ret, timeout;
+	int ret;
+	unsigned long time_left;
 
 	ret = pm_runtime_get_sync(ctrl->dev);
 	if (ret < 0)
@@ -929,8 +931,8 @@ static int qcom_slim_ngd_xfer_msg_sync(struct slim_controller *ctrl,
 	if (ret)
 		goto pm_put;
 
-	timeout = wait_for_completion_timeout(&done, HZ);
-	if (!timeout) {
+	time_left = wait_for_completion_timeout(&done, HZ);
+	if (!time_left) {
 		dev_err(ctrl->dev, "TX timed out:MC:0x%x,mt:0x%x", txn->mc,
 				txn->mt);
 		ret = -ETIMEDOUT;
@@ -1169,11 +1171,12 @@ static int qcom_slim_ngd_power_up(struct qcom_slim_ngd_ctrl *ctrl)
 	enum qcom_slim_ngd_state cur_state = ctrl->state;
 	struct qcom_slim_ngd *ngd = ctrl->ngd;
 	u32 laddr, rx_msgq;
-	int timeout, ret = 0;
+	int ret = 0;
+	unsigned long time_left;
 
 	if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN) {
-		timeout = wait_for_completion_timeout(&ctrl->qmi.qmi_comp, HZ);
-		if (!timeout)
+		time_left = wait_for_completion_timeout(&ctrl->qmi.qmi_comp, HZ);
+		if (!time_left)
 			return -EREMOTEIO;
 	}
 
@@ -1218,8 +1221,8 @@ static int qcom_slim_ngd_power_up(struct qcom_slim_ngd_ctrl *ctrl)
 				ngd->base + NGD_RX_MSGQ_CFG);
 	qcom_slim_ngd_setup(ctrl);
 
-	timeout = wait_for_completion_timeout(&ctrl->reconf, HZ);
-	if (!timeout) {
+	time_left = wait_for_completion_timeout(&ctrl->reconf, HZ);
+	if (!time_left) {
 		dev_err(ctrl->dev, "capability exchange timed-out\n");
 		return -ETIMEDOUT;
 	}
-- 
2.43.0

Re: [PATCH 0/3] slimbus: use 'time_left' instead of 'timeout' with wait_for_*() functions

[Bjorn Andersson]

On Tue, Apr 30, 2024 at 02:00:58PM +0200, Wolfram Sang wrote:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_for_*() functions causing patterns like:
> 
>         timeout = wait_for_completion_timeout(...)
>         if (!timeout) return -ETIMEDOUT;
> 
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> obvious and self explaining.
> 
> This is part of a tree-wide series. The rest of the patches can be found here
> (some parts may still be WIP):
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/time_left
> 
> Because these patches are generated, I audit them before sending. This is why I
> will send series step by step. Build bot is happy with these patches, though.
> No functional changes intended.
> 

Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>

Regards,
Bjorn

> Wolfram Sang (3):
>   slimbus: messaging: use 'time_left' variable with
>     wait_for_completion_timeout()
>   slimbus: qcom-ctrl: use 'time_left' variable with
>     wait_for_completion_timeout()
>   slimbus: qcom-ngd-ctrl: use 'time_left' variable with
>     wait_for_completion_timeout()
> 
>  drivers/slimbus/messaging.c     |  9 +++++----
>  drivers/slimbus/qcom-ctrl.c     |  7 ++++---
>  drivers/slimbus/qcom-ngd-ctrl.c | 29 ++++++++++++++++-------------
>  3 files changed, 25 insertions(+), 20 deletions(-)
> 
> -- 
> 2.43.0
> 

Re: [PATCH 0/3] slimbus: use 'time_left' instead of 'timeout' with wait_for_*() functions

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On Tue, Apr 30, 2024 at 06:30:00AM -0700, Bjorn Andersson wrote:
> On Tue, Apr 30, 2024 at 02:00:58PM +0200, Wolfram Sang wrote:
> > There is a confusing pattern in the kernel to use a variable named 'timeout' to
> > store the result of wait_for_*() functions causing patterns like:
> > 
> >         timeout = wait_for_completion_timeout(...)
> >         if (!timeout) return -ETIMEDOUT;
> > 
> > with all kinds of permutations. Use 'time_left' as a variable to make the code
> > obvious and self explaining.
> > 
> > This is part of a tree-wide series. The rest of the patches can be found here
> > (some parts may still be WIP):
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/time_left
> > 
> > Because these patches are generated, I audit them before sending. This is why I
> > will send series step by step. Build bot is happy with these patches, though.
> > No functional changes intended.
> > 
> 
> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>

Thanks, Bjorn. Is Srinivas still pick slimbus patches?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/3] slimbus: use 'time_left' instead of 'timeout' with wait_for_*() functions

[Srinivas Kandagatla]

On Tue, 30 Apr 2024 14:00:58 +0200, Wolfram Sang wrote:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_for_*() functions causing patterns like:
> 
>         timeout = wait_for_completion_timeout(...)
>         if (!timeout) return -ETIMEDOUT;
> 
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> obvious and self explaining.
> 
> [...]

Applied, thanks!

[1/3] slimbus: messaging: use 'time_left' variable with wait_for_completion_timeout()
      commit: 0eb9dda9d1db40acbabe923fe22c002e13890d39
[2/3] slimbus: qcom-ctrl: use 'time_left' variable with wait_for_completion_timeout()
      commit: 7d317b95d0334371481ec00ca31f5bf76bae8a82
[3/3] slimbus: qcom-ngd-ctrl: use 'time_left' variable with wait_for_completion_timeout()
      commit: 9f5fd5e2aebf06c37355cacc7f4b4410bc0ea233

Best regards,
-- 
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>