Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1682786 > unrolled thread

[PATCH v2 9/9] staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change()

Started byBrian Masney <masneyb@onstation.org>
First post2017-07-07 01:00 +0200
Last post2017-07-09 19:50 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 9/9] staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change() Brian Masney <masneyb@onstation.org> - 2017-07-07 01:00 +0200
    Re: [PATCH v2 9/9] staging: iio: tsl2x7x: check return value from  tsl2x7x_invoke_change() Jonathan Cameron <jic23@kernel.org> - 2017-07-09 19:50 +0200

#1682786 — [PATCH v2 9/9] staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change()

FromBrian Masney <masneyb@onstation.org>
Date2017-07-07 01:00 +0200
Subject[PATCH v2 9/9] staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change()
Message-ID<u0nUB-3uc-5@gated-at.bofh.it>
The return value from tsl2x7x_invoke_change() was not checked in most
places in the driver. This patch adds the proper error checks. The
return values inside tsl2x7x_invoke_change() are now checked by
this patch as well.

Previously, if there was an error turning the chip back on, then the
driver would attempt to turn the chip off and incorrectly return
success. The code to power off the chip is removed by this patch
since we should fail fast.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 55 +++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 033468d..c00278d 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -771,22 +771,24 @@ int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
 {
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	int device_status = chip->tsl2x7x_chip_status;
+	int ret;
 
 	mutex_lock(&chip->als_mutex);
 	mutex_lock(&chip->prox_mutex);
 
-	if (device_status == TSL2X7X_CHIP_WORKING)
-		tsl2x7x_chip_off(indio_dev);
-
-	tsl2x7x_chip_on(indio_dev);
+	if (device_status == TSL2X7X_CHIP_WORKING) {
+		ret = tsl2x7x_chip_off(indio_dev);
+		if (ret < 0)
+			goto unlock;
+	}
 
-	if (device_status != TSL2X7X_CHIP_WORKING)
-		tsl2x7x_chip_off(indio_dev);
+	ret = tsl2x7x_chip_on(indio_dev);
 
+unlock:
 	mutex_unlock(&chip->prox_mutex);
 	mutex_unlock(&chip->als_mutex);
 
-	return 0;
+	return ret;
 }
 
 static
@@ -925,6 +927,7 @@ static ssize_t in_illuminance0_target_input_store(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	unsigned long value;
+	int ret;
 
 	if (kstrtoul(buf, 0, &value))
 		return -EINVAL;
@@ -932,7 +935,9 @@ static ssize_t in_illuminance0_target_input_store(struct device *dev,
 	if (value)
 		chip->tsl2x7x_settings.als_cal_target = value;
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return len;
 }
@@ -981,7 +986,9 @@ static ssize_t in_intensity0_thresh_period_store(struct device *dev,
 	dev_info(&chip->client->dev, "%s: als persistence = %d",
 		 __func__, filter_delay);
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return IIO_VAL_INT_PLUS_MICRO;
 }
@@ -1029,7 +1036,10 @@ static ssize_t in_proximity0_thresh_period_store(struct device *dev,
 	dev_info(&chip->client->dev, "%s: prox persistence = %d",
 		 __func__, filter_delay);
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
+
 
 	return IIO_VAL_INT_PLUS_MICRO;
 }
@@ -1040,6 +1050,7 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	bool value;
+	int ret;
 
 	if (strtobool(buf, &value))
 		return -EINVAL;
@@ -1047,7 +1058,9 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
 	if (value)
 		tsl2x7x_als_calibrate(indio_dev);
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return len;
 }
@@ -1087,7 +1100,7 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	int value[ARRAY_SIZE(chip->tsl2x7x_device_lux) * 3 + 1];
-	int n;
+	int n, ret;
 
 	get_options(buf, ARRAY_SIZE(value), value);
 
@@ -1115,7 +1128,9 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
 	memset(chip->tsl2x7x_device_lux, 0, sizeof(chip->tsl2x7x_device_lux));
 	memcpy(chip->tsl2x7x_device_lux, &value[1], (value[0] * 4));
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return len;
 }
@@ -1126,6 +1141,7 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	bool value;
+	int ret;
 
 	if (strtobool(buf, &value))
 		return -EINVAL;
@@ -1133,7 +1149,9 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
 	if (value)
 		tsl2x7x_prox_cal(indio_dev);
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return len;
 }
@@ -1161,6 +1179,7 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
 					  int val)
 {
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
+	int ret;
 
 	if (chan->type == IIO_INTENSITY) {
 		if (val)
@@ -1174,7 +1193,9 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
 			chip->tsl2x7x_settings.interrupts_en &= 0x10;
 	}
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return 0;
 }
@@ -1422,9 +1443,7 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 		return -EINVAL;
 	}
 
-	tsl2x7x_invoke_change(indio_dev);
-
-	return 0;
+	return tsl2x7x_invoke_change(indio_dev);
 }
 
 static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
-- 
2.9.4

[toc] | [next] | [standalone]

#1683784 — Re: [PATCH v2 9/9] staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change()

FromJonathan Cameron <jic23@kernel.org>
Date2017-07-09 19:50 +0200
SubjectRe: [PATCH v2 9/9] staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change()
Message-ID<u1ovg-2iE-17@gated-at.bofh.it>
In reply to#1682786
On Thu,  6 Jul 2017 18:56:26 -0400
Brian Masney <masneyb@onstation.org> wrote:

> The return value from tsl2x7x_invoke_change() was not checked in most
> places in the driver. This patch adds the proper error checks. The
> return values inside tsl2x7x_invoke_change() are now checked by
> this patch as well.
> 
> Previously, if there was an error turning the chip back on, then the
> driver would attempt to turn the chip off and incorrectly return
> success. The code to power off the chip is removed by this patch
> since we should fail fast.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
This went on cleanly even without patch 8 so:

Applied to the togreg branch of iio.git and pushed out as testing.

Another good set Brian, looking forward to the next one!

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 55 +++++++++++++++++++++++++------------
>  1 file changed, 37 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 033468d..c00278d 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -771,22 +771,24 @@ int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
>  {
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	int device_status = chip->tsl2x7x_chip_status;
> +	int ret;
>  
>  	mutex_lock(&chip->als_mutex);
>  	mutex_lock(&chip->prox_mutex);
>  
> -	if (device_status == TSL2X7X_CHIP_WORKING)
> -		tsl2x7x_chip_off(indio_dev);
> -
> -	tsl2x7x_chip_on(indio_dev);
> +	if (device_status == TSL2X7X_CHIP_WORKING) {
> +		ret = tsl2x7x_chip_off(indio_dev);
> +		if (ret < 0)
> +			goto unlock;
> +	}
>  
> -	if (device_status != TSL2X7X_CHIP_WORKING)
> -		tsl2x7x_chip_off(indio_dev);
> +	ret = tsl2x7x_chip_on(indio_dev);
>  
> +unlock:
>  	mutex_unlock(&chip->prox_mutex);
>  	mutex_unlock(&chip->als_mutex);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static
> @@ -925,6 +927,7 @@ static ssize_t in_illuminance0_target_input_store(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	unsigned long value;
> +	int ret;
>  
>  	if (kstrtoul(buf, 0, &value))
>  		return -EINVAL;
> @@ -932,7 +935,9 @@ static ssize_t in_illuminance0_target_input_store(struct device *dev,
>  	if (value)
>  		chip->tsl2x7x_settings.als_cal_target = value;
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return len;
>  }
> @@ -981,7 +986,9 @@ static ssize_t in_intensity0_thresh_period_store(struct device *dev,
>  	dev_info(&chip->client->dev, "%s: als persistence = %d",
>  		 __func__, filter_delay);
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return IIO_VAL_INT_PLUS_MICRO;
>  }
> @@ -1029,7 +1036,10 @@ static ssize_t in_proximity0_thresh_period_store(struct device *dev,
>  	dev_info(&chip->client->dev, "%s: prox persistence = %d",
>  		 __func__, filter_delay);
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
> +
>  
>  	return IIO_VAL_INT_PLUS_MICRO;
>  }
> @@ -1040,6 +1050,7 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	bool value;
> +	int ret;
>  
>  	if (strtobool(buf, &value))
>  		return -EINVAL;
> @@ -1047,7 +1058,9 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
>  	if (value)
>  		tsl2x7x_als_calibrate(indio_dev);
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return len;
>  }
> @@ -1087,7 +1100,7 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	int value[ARRAY_SIZE(chip->tsl2x7x_device_lux) * 3 + 1];
> -	int n;
> +	int n, ret;
>  
>  	get_options(buf, ARRAY_SIZE(value), value);
>  
> @@ -1115,7 +1128,9 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
>  	memset(chip->tsl2x7x_device_lux, 0, sizeof(chip->tsl2x7x_device_lux));
>  	memcpy(chip->tsl2x7x_device_lux, &value[1], (value[0] * 4));
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return len;
>  }
> @@ -1126,6 +1141,7 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	bool value;
> +	int ret;
>  
>  	if (strtobool(buf, &value))
>  		return -EINVAL;
> @@ -1133,7 +1149,9 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
>  	if (value)
>  		tsl2x7x_prox_cal(indio_dev);
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return len;
>  }
> @@ -1161,6 +1179,7 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
>  					  int val)
>  {
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> +	int ret;
>  
>  	if (chan->type == IIO_INTENSITY) {
>  		if (val)
> @@ -1174,7 +1193,9 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
>  			chip->tsl2x7x_settings.interrupts_en &= 0x10;
>  	}
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return 0;
>  }
> @@ -1422,9 +1443,7 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  		return -EINVAL;
>  	}
>  
> -	tsl2x7x_invoke_change(indio_dev);
> -
> -	return 0;
> +	return tsl2x7x_invoke_change(indio_dev);
>  }
>  
>  static DEVICE_ATTR_RO(in_proximity0_calibscale_available);

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web