diff mbox series

[v2] mfd: wm831x: Remove redundant forever while loop

Message ID 20231120102259.74210-1-mstrozek@opensource.cirrus.com
State New
Headers
Series [v2] mfd: wm831x: Remove redundant forever while loop |

Commit Message

Maciej Strozek Nov. 20, 2023, 10:22 a.m. UTC 
  Current code excutes only once despite the while loop, so remove the
loop. Also msleep(1) will likely result in a larger sleep, so increase
its value for clarity while keeping the same behaviour.

Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
---
V1 -> V2: removed unused variable 'timeout'

 drivers/mfd/wm831x-auxadc.c | 48 +++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

--
2.34.1

Comments

Charles Keepax Nov. 20, 2023, 10:39 a.m. UTC | #1 
On Mon, Nov 20, 2023 at 10:22:59AM +0000, Maciej Strozek wrote:
> Current code excutes only once despite the while loop, so remove the
> loop. Also msleep(1) will likely result in a larger sleep, so increase
> its value for clarity while keeping the same behaviour.
> 
> Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles
Lee Jones Nov. 23, 2023, 3:30 p.m. UTC | #2 
On Mon, 20 Nov 2023, Maciej Strozek wrote:

> Current code excutes only once despite the while loop, so remove the
> loop. Also msleep(1) will likely result in a larger sleep, so increase
> its value for clarity while keeping the same behaviour.
> 
> Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
> ---
> V1 -> V2: removed unused variable 'timeout'
> 
>  drivers/mfd/wm831x-auxadc.c | 48 +++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/mfd/wm831x-auxadc.c b/drivers/mfd/wm831x-auxadc.c
> index 65b98f3fbd92..77f8e51a5753 100644
> --- a/drivers/mfd/wm831x-auxadc.c
> +++ b/drivers/mfd/wm831x-auxadc.c
> @@ -152,7 +152,7 @@ static irqreturn_t wm831x_auxadc_irq(int irq, void *irq_data)
>  static int wm831x_auxadc_read_polled(struct wm831x *wm831x,
>  				     enum wm831x_auxadc input)
>  {
> -	int ret, src, timeout;
> +	int ret, src;
> 
>  	mutex_lock(&wm831x->auxadc_lock);
> 
> @@ -179,32 +179,28 @@ static int wm831x_auxadc_read_polled(struct wm831x *wm831x,
>  		goto disable;
>  	}
> 
> -	/* If we're not using interrupts then poll the
> -	 * interrupt status register */
> -	timeout = 5;
> -	while (timeout) {
> -		msleep(1);
> -
> -		ret = wm831x_reg_read(wm831x,
> -				      WM831X_INTERRUPT_STATUS_1);
> -		if (ret < 0) {
> -			dev_err(wm831x->dev,
> -				"ISR 1 read failed: %d\n", ret);
> -			goto disable;
> -		}
> +	/* If we're not using interrupts then read the
> +	 * interrupt status register
> +	 */

Can someone submit a commit to fix this comment.

> +	msleep(20);

New line here please.

> +	ret = wm831x_reg_read(wm831x,
> +				WM831X_INTERRUPT_STATUS_1);

Odd line-up.

> +	if (ret < 0) {
> +		dev_err(wm831x->dev,
> +			"ISR 1 read failed: %d\n", ret);
> +		goto disable;
> +	}
> 
> -		/* Did it complete? */
> -		if (ret & WM831X_AUXADC_DATA_EINT) {
> -			wm831x_reg_write(wm831x,
> -					 WM831X_INTERRUPT_STATUS_1,
> -					 WM831X_AUXADC_DATA_EINT);
> -			break;
> -		} else {
> -			dev_err(wm831x->dev,
> -				"AUXADC conversion timeout\n");
> -			ret = -EBUSY;
> -			goto disable;
> -		}
> +	/* Did it complete? */
> +	if (ret & WM831X_AUXADC_DATA_EINT) {
> +		wm831x_reg_write(wm831x,
> +					WM831X_INTERRUPT_STATUS_1,
> +					WM831X_AUXADC_DATA_EINT);

Very odd line-up.

> +	} else {
> +		dev_err(wm831x->dev,
> +			"AUXADC conversion timeout\n");
> +		ret = -EBUSY;
> +		goto disable;
>  	}
> 
>  	ret = wm831x_reg_read(wm831x, WM831X_AUXADC_DATA);
> --
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/mfd/wm831x-auxadc.c b/drivers/mfd/wm831x-auxadc.c
index 65b98f3fbd92..77f8e51a5753 100644
--- a/drivers/mfd/wm831x-auxadc.c
+++ b/drivers/mfd/wm831x-auxadc.c
@@ -152,7 +152,7 @@  static irqreturn_t wm831x_auxadc_irq(int irq, void *irq_data)
 static int wm831x_auxadc_read_polled(struct wm831x *wm831x,
 				     enum wm831x_auxadc input)
 {
-	int ret, src, timeout;
+	int ret, src;

 	mutex_lock(&wm831x->auxadc_lock);

@@ -179,32 +179,28 @@  static int wm831x_auxadc_read_polled(struct wm831x *wm831x,
 		goto disable;
 	}

-	/* If we're not using interrupts then poll the
-	 * interrupt status register */
-	timeout = 5;
-	while (timeout) {
-		msleep(1);
-
-		ret = wm831x_reg_read(wm831x,
-				      WM831X_INTERRUPT_STATUS_1);
-		if (ret < 0) {
-			dev_err(wm831x->dev,
-				"ISR 1 read failed: %d\n", ret);
-			goto disable;
-		}
+	/* If we're not using interrupts then read the
+	 * interrupt status register
+	 */
+	msleep(20);
+	ret = wm831x_reg_read(wm831x,
+				WM831X_INTERRUPT_STATUS_1);
+	if (ret < 0) {
+		dev_err(wm831x->dev,
+			"ISR 1 read failed: %d\n", ret);
+		goto disable;
+	}

-		/* Did it complete? */
-		if (ret & WM831X_AUXADC_DATA_EINT) {
-			wm831x_reg_write(wm831x,
-					 WM831X_INTERRUPT_STATUS_1,
-					 WM831X_AUXADC_DATA_EINT);
-			break;
-		} else {
-			dev_err(wm831x->dev,
-				"AUXADC conversion timeout\n");
-			ret = -EBUSY;
-			goto disable;
-		}
+	/* Did it complete? */
+	if (ret & WM831X_AUXADC_DATA_EINT) {
+		wm831x_reg_write(wm831x,
+					WM831X_INTERRUPT_STATUS_1,
+					WM831X_AUXADC_DATA_EINT);
+	} else {
+		dev_err(wm831x->dev,
+			"AUXADC conversion timeout\n");
+		ret = -EBUSY;
+		goto disable;
 	}

 	ret = wm831x_reg_read(wm831x, WM831X_AUXADC_DATA);