diff mbox series

mfd: wm831x: Remove redundant forever while loop

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

Commit Message

Maciej Strozek Nov. 17, 2023, 3:40 p.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>
---
 drivers/mfd/wm831x-auxadc.c | 46 +++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

Comments

Charles Keepax Nov. 17, 2023, 3:52 p.m. UTC | #1 
On Fri, Nov 17, 2023 at 03:40:37PM +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
kernel test robot Nov. 18, 2023, 4:44 a.m. UTC | #2 
Hi Maciej,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on lee-leds/for-leds-next lee-mfd/for-mfd-fixes linus/master v6.7-rc1 next-20231117]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Maciej-Strozek/mfd-wm831x-Remove-redundant-forever-while-loop/20231117-234810
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:    https://lore.kernel.org/r/20231117154037.67676-1-mstrozek%40opensource.cirrus.com
patch subject: [PATCH] mfd: wm831x: Remove redundant forever while loop
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231118/202311181257.FLaZt746-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231118/202311181257.FLaZt746-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311181257.FLaZt746-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/mfd/wm831x-auxadc.c: In function 'wm831x_auxadc_read_polled':
>> drivers/mfd/wm831x-auxadc.c:155:23: warning: unused variable 'timeout' [-Wunused-variable]
     155 |         int ret, src, timeout;
         |                       ^~~~~~~


vim +/timeout +155 drivers/mfd/wm831x-auxadc.c

78bb3688ea1830 Mark Brown     2011-06-07  151  
78bb3688ea1830 Mark Brown     2011-06-07  152  static int wm831x_auxadc_read_polled(struct wm831x *wm831x,
78bb3688ea1830 Mark Brown     2011-06-07  153  				     enum wm831x_auxadc input)
78bb3688ea1830 Mark Brown     2011-06-07  154  {
78bb3688ea1830 Mark Brown     2011-06-07 @155  	int ret, src, timeout;
e69b6de181167a Mark Brown     2011-06-02  156  
e69b6de181167a Mark Brown     2011-06-02  157  	mutex_lock(&wm831x->auxadc_lock);
e69b6de181167a Mark Brown     2011-06-02  158  
e69b6de181167a Mark Brown     2011-06-02  159  	ret = wm831x_set_bits(wm831x, WM831X_AUXADC_CONTROL,
e69b6de181167a Mark Brown     2011-06-02  160  			      WM831X_AUX_ENA, WM831X_AUX_ENA);
e69b6de181167a Mark Brown     2011-06-02  161  	if (ret < 0) {
e69b6de181167a Mark Brown     2011-06-02  162  		dev_err(wm831x->dev, "Failed to enable AUXADC: %d\n", ret);
e69b6de181167a Mark Brown     2011-06-02  163  		goto out;
e69b6de181167a Mark Brown     2011-06-02  164  	}
e69b6de181167a Mark Brown     2011-06-02  165  
e69b6de181167a Mark Brown     2011-06-02  166  	/* We force a single source at present */
e69b6de181167a Mark Brown     2011-06-02  167  	src = input;
e69b6de181167a Mark Brown     2011-06-02  168  	ret = wm831x_reg_write(wm831x, WM831X_AUXADC_SOURCE,
e69b6de181167a Mark Brown     2011-06-02  169  			       1 << src);
e69b6de181167a Mark Brown     2011-06-02  170  	if (ret < 0) {
e69b6de181167a Mark Brown     2011-06-02  171  		dev_err(wm831x->dev, "Failed to set AUXADC source: %d\n", ret);
e69b6de181167a Mark Brown     2011-06-02  172  		goto out;
e69b6de181167a Mark Brown     2011-06-02  173  	}
e69b6de181167a Mark Brown     2011-06-02  174  
e69b6de181167a Mark Brown     2011-06-02  175  	ret = wm831x_set_bits(wm831x, WM831X_AUXADC_CONTROL,
e69b6de181167a Mark Brown     2011-06-02  176  			      WM831X_AUX_CVT_ENA, WM831X_AUX_CVT_ENA);
e69b6de181167a Mark Brown     2011-06-02  177  	if (ret < 0) {
e69b6de181167a Mark Brown     2011-06-02  178  		dev_err(wm831x->dev, "Failed to start AUXADC: %d\n", ret);
e69b6de181167a Mark Brown     2011-06-02  179  		goto disable;
e69b6de181167a Mark Brown     2011-06-02  180  	}
e69b6de181167a Mark Brown     2011-06-02  181  
5e253590978d7f Maciej Strozek 2023-11-17  182  	/* If we're not using interrupts then read the
5e253590978d7f Maciej Strozek 2023-11-17  183  	 * interrupt status register
5e253590978d7f Maciej Strozek 2023-11-17  184  	 */
5e253590978d7f Maciej Strozek 2023-11-17  185  	msleep(20);
e69b6de181167a Mark Brown     2011-06-02  186  	ret = wm831x_reg_read(wm831x,
e69b6de181167a Mark Brown     2011-06-02  187  				WM831X_INTERRUPT_STATUS_1);
e69b6de181167a Mark Brown     2011-06-02  188  	if (ret < 0) {
e69b6de181167a Mark Brown     2011-06-02  189  		dev_err(wm831x->dev,
e69b6de181167a Mark Brown     2011-06-02  190  			"ISR 1 read failed: %d\n", ret);
e69b6de181167a Mark Brown     2011-06-02  191  		goto disable;
e69b6de181167a Mark Brown     2011-06-02  192  	}
e69b6de181167a Mark Brown     2011-06-02  193  
e69b6de181167a Mark Brown     2011-06-02  194  	/* Did it complete? */
e69b6de181167a Mark Brown     2011-06-02  195  	if (ret & WM831X_AUXADC_DATA_EINT) {
e69b6de181167a Mark Brown     2011-06-02  196  		wm831x_reg_write(wm831x,
e69b6de181167a Mark Brown     2011-06-02  197  					WM831X_INTERRUPT_STATUS_1,
e69b6de181167a Mark Brown     2011-06-02  198  					WM831X_AUXADC_DATA_EINT);
e69b6de181167a Mark Brown     2011-06-02  199  	} else {
e69b6de181167a Mark Brown     2011-06-02  200  		dev_err(wm831x->dev,
e69b6de181167a Mark Brown     2011-06-02  201  			"AUXADC conversion timeout\n");
e69b6de181167a Mark Brown     2011-06-02  202  		ret = -EBUSY;
e69b6de181167a Mark Brown     2011-06-02  203  		goto disable;
e69b6de181167a Mark Brown     2011-06-02  204  	}
e69b6de181167a Mark Brown     2011-06-02  205  
e69b6de181167a Mark Brown     2011-06-02  206  	ret = wm831x_reg_read(wm831x, WM831X_AUXADC_DATA);
e69b6de181167a Mark Brown     2011-06-02  207  	if (ret < 0) {
e69b6de181167a Mark Brown     2011-06-02  208  		dev_err(wm831x->dev,
e69b6de181167a Mark Brown     2011-06-02  209  			"Failed to read AUXADC data: %d\n", ret);
e69b6de181167a Mark Brown     2011-06-02  210  		goto disable;
e69b6de181167a Mark Brown     2011-06-02  211  	}
e69b6de181167a Mark Brown     2011-06-02  212  
78bb3688ea1830 Mark Brown     2011-06-07  213  	src = ((ret & WM831X_AUX_DATA_SRC_MASK)
e69b6de181167a Mark Brown     2011-06-02  214  	       >> WM831X_AUX_DATA_SRC_SHIFT) - 1;
e69b6de181167a Mark Brown     2011-06-02  215  
e69b6de181167a Mark Brown     2011-06-02  216  	if (src == 14)
e69b6de181167a Mark Brown     2011-06-02  217  		src = WM831X_AUX_CAL;
e69b6de181167a Mark Brown     2011-06-02  218  
e69b6de181167a Mark Brown     2011-06-02  219  	if (src != input) {
e69b6de181167a Mark Brown     2011-06-02  220  		dev_err(wm831x->dev, "Data from source %d not %d\n",
e69b6de181167a Mark Brown     2011-06-02  221  			src, input);
e69b6de181167a Mark Brown     2011-06-02  222  		ret = -EINVAL;
e69b6de181167a Mark Brown     2011-06-02  223  	} else {
78bb3688ea1830 Mark Brown     2011-06-07  224  		ret &= WM831X_AUX_DATA_MASK;
e69b6de181167a Mark Brown     2011-06-02  225  	}
e69b6de181167a Mark Brown     2011-06-02  226  
e69b6de181167a Mark Brown     2011-06-02  227  disable:
e69b6de181167a Mark Brown     2011-06-02  228  	wm831x_set_bits(wm831x, WM831X_AUXADC_CONTROL, WM831X_AUX_ENA, 0);
e69b6de181167a Mark Brown     2011-06-02  229  out:
e69b6de181167a Mark Brown     2011-06-02  230  	mutex_unlock(&wm831x->auxadc_lock);
e69b6de181167a Mark Brown     2011-06-02  231  	return ret;
e69b6de181167a Mark Brown     2011-06-02  232  }
e69b6de181167a Mark Brown     2011-06-02  233
diff mbox series

Patch

diff --git a/drivers/mfd/wm831x-auxadc.c b/drivers/mfd/wm831x-auxadc.c
index 65b98f3fbd92..49f0ffd68d47 100644
--- a/drivers/mfd/wm831x-auxadc.c
+++ b/drivers/mfd/wm831x-auxadc.c
@@ -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);