Re: [PATCH] iio: cros_ec_accel_legacy: Always release lock when returning from _read()

[Benson Leung <bleung@google.com>]

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

Hi Matthias,

On Mon, Jul 15, 2019 at 12:10:17PM -0700, Matthias Kaehlcke wrote:
> Before doing any actual work cros_ec_accel_legacy_read() acquires
> a mutex, which is released at the end of the function. However for
> 'calibbias' channels the function returns directly, without releasing
> the lock. The next attempt to acquire the lock blocks forever. Instead
> of an explicit return statement use the common return path, which
> releases the lock.
> 
> Fixes: 11b86c7004ef1 ("platform/chrome: Add cros_ec_accel_legacy driver")
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/iio/accel/cros_ec_accel_legacy.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> index 46bb2e421bb9..27ca4a64dddf 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -206,7 +206,8 @@ static int cros_ec_accel_legacy_read(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		/* Calibration not supported. */
>  		*val = 0;
> -		return IIO_VAL_INT;
> +		ret = IIO_VAL_INT;
> +		break;

The value of ret is not used below this. It seems to be only used in
case IIO_CHAN_INFO_RAW. In fact, with your change,
there's no return value at all and we just reach the end of
cros_ec_accel_legacy_read.

>  	default:
>  		return -EINVAL;
>  	}
> -- 
> 2.22.0.510.g264f2c817a-goog
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

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