[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20231123153005.GD1354538@google.com>
Date: Thu, 23 Nov 2023 15:30:05 +0000
From: Lee Jones <lee@...nel.org>
To: Maciej Strozek <mstrozek@...nsource.cirrus.com>
Cc: patches@...nsource.cirrus.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mfd: wm831x: Remove redundant forever while loop
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@...nsource.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
>
--
Lee Jones [李琼斯]
Powered by blists - more mailing lists