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 <[email protected]>
---
drivers/mfd/wm831x-auxadc.c | 46 +++++++++++++++++--------------------
1 file changed, 21 insertions(+), 25 deletions(-)
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);
--
2.34.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 <[email protected]>
> ---
Acked-by: Charles Keepax <[email protected]>
Thanks,
Charles
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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki