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

Groups > linux.kernel > #1503762 > unrolled thread

[PATCH 1/7] iio: light: tsl2583: return proper error code

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/7] iio: light: tsl2583: return proper error code Brian Masney <masneyb@onstation.org> - 2016-10-19 16:50 +0200
    [PATCH 3/7] iio: light: tsl2583: use DEVICE_ATTR_{RO, RW, WO} macros Brian Masney <masneyb@onstation.org> - 2016-10-19 16:50 +0200
      Re: [PATCH 3/7] iio: light: tsl2583: use DEVICE_ATTR_{RO, RW, WO}  macros Peter Meerwald-Stadler <pmeerw@pmeerw.net> - 2016-10-19 18:00 +0200
      Re: [PATCH 3/7] iio: light: tsl2583: use DEVICE_ATTR_{RO, RW, WO} macros Jonathan Cameron <jic23@jic23.retrosnub.co.uk> - 2016-10-19 18:40 +0200
    [PATCH 2/7] iio: light: tsl2583: change functions to only have a single exit point Brian Masney <masneyb@onstation.org> - 2016-10-19 17:00 +0200
      Re: [PATCH 2/7] iio: light: tsl2583: change functions to only have a  single exit point Dan Carpenter <dan.carpenter@oracle.com> - 2016-10-19 17:00 +0200
    [PATCH 4/7] iio: light: tsl2583: return proper error code in sysfs store functions Brian Masney <masneyb@onstation.org> - 2016-10-19 17:00 +0200
    [PATCH 5/7] iio: light: tsl2583: check return values from taos_chip_{on,off} Brian Masney <masneyb@onstation.org> - 2016-10-19 17:00 +0200
    Re: [PATCH 1/7] iio: light: tsl2583: return proper error code Jonathan Cameron <jic23@kernel.org> - 2016-10-22 19:20 +0200

#1503762 — [PATCH 1/7] iio: light: tsl2583: return proper error code

taos_gain_store() and taos_als_calibrate() both have a code path where
-1 was returned. This patch changes the code so that a proper error code
is returned to make the code consistent with the error paths that are
present within those same functions.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 08f1583..1e42a19 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -359,7 +359,7 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
 			!= (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON)) {
 		dev_err(&chip->client->dev,
 			"taos_als_calibrate failed: device not powered on with ADC enabled\n");
-		return -1;
+		return -ENODATA;
 	}
 
 	ret = i2c_smbus_write_byte(chip->client,
@@ -569,7 +569,7 @@ static ssize_t taos_gain_store(struct device *dev,
 		break;
 	default:
 		dev_err(dev, "Invalid Gain Index (must be 1,8,16,111)\n");
-		return -1;
+		return -EINVAL;
 	}
 
 	return len;
-- 
2.7.4

[toc] | [next] | [standalone]

#1503768 — [PATCH 3/7] iio: light: tsl2583: use DEVICE_ATTR_{RO, RW, WO} macros

Use the DEVICE_ATTR_RO, IIO_DEVICE_ATTR_RW, and IIO_DEVICE_ATTR_WO
macros to create the device attributes.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2583.c | 117 ++++++++++++++++++------------------
 1 file changed, 57 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 8448a87..bbb8fc3 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -501,8 +501,8 @@ static int taos_chip_off(struct iio_dev *indio_dev)
 
 /* Sysfs Interface Functions */
 
-static ssize_t taos_power_state_show(struct device *dev,
-				     struct device_attribute *attr, char *buf)
+static ssize_t power_state_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -510,9 +510,9 @@ static ssize_t taos_power_state_show(struct device *dev,
 	return sprintf(buf, "%d\n", chip->taos_chip_status);
 }
 
-static ssize_t taos_power_state_store(struct device *dev,
-				      struct device_attribute *attr,
-				      const char *buf, size_t len)
+static ssize_t power_state_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	int value;
@@ -528,8 +528,9 @@ static ssize_t taos_power_state_store(struct device *dev,
 	return len;
 }
 
-static ssize_t taos_gain_show(struct device *dev,
-			      struct device_attribute *attr, char *buf)
+static ssize_t illuminance0_calibscale_show(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -553,9 +554,9 @@ static ssize_t taos_gain_show(struct device *dev,
 	return sprintf(buf, "%s\n", gain);
 }
 
-static ssize_t taos_gain_store(struct device *dev,
-			       struct device_attribute *attr,
-			       const char *buf, size_t len)
+static ssize_t illuminance0_calibscale_store(struct device *dev,
+					     struct device_attribute *attr,
+					     const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -587,15 +588,16 @@ gain_store_done:
 	return ret;
 }
 
-static ssize_t taos_gain_available_show(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
+static ssize_t illuminance0_calibscale_available_show(struct device *dev,
+						  struct device_attribute *attr,
+						  char *buf)
 {
 	return sprintf(buf, "%s\n", "1 8 16 111");
 }
 
-static ssize_t taos_als_time_show(struct device *dev,
-				  struct device_attribute *attr, char *buf)
+static ssize_t illuminance0_integration_time_show(struct device *dev,
+						  struct device_attribute *attr,
+						  char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -603,9 +605,9 @@ static ssize_t taos_als_time_show(struct device *dev,
 	return sprintf(buf, "%d\n", chip->taos_settings.als_time);
 }
 
-static ssize_t taos_als_time_store(struct device *dev,
-				   struct device_attribute *attr,
-				   const char *buf, size_t len)
+static ssize_t illuminance0_integration_time_store(struct device *dev,
+						  struct device_attribute *attr,
+						  const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -627,16 +629,17 @@ als_time_store_done:
 	return ret;
 }
 
-static ssize_t taos_als_time_available_show(struct device *dev,
-					    struct device_attribute *attr,
-					    char *buf)
+static ssize_t illuminance0_integration_time_available_show(struct device *dev,
+						  struct device_attribute *attr,
+						  char *buf)
 {
 	return sprintf(buf, "%s\n",
 		"50 100 150 200 250 300 350 400 450 500 550 600 650");
 }
 
-static ssize_t taos_als_trim_show(struct device *dev,
-				  struct device_attribute *attr, char *buf)
+static ssize_t illuminance0_calibbias_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -644,9 +647,9 @@ static ssize_t taos_als_trim_show(struct device *dev,
 	return sprintf(buf, "%d\n", chip->taos_settings.als_gain_trim);
 }
 
-static ssize_t taos_als_trim_store(struct device *dev,
-				   struct device_attribute *attr,
-				   const char *buf, size_t len)
+static ssize_t illuminance0_calibbias_store(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -661,9 +664,9 @@ static ssize_t taos_als_trim_store(struct device *dev,
 	return len;
 }
 
-static ssize_t taos_als_cal_target_show(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
+static ssize_t illuminance0_input_target_show(struct device *dev,
+					      struct device_attribute *attr,
+					      char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -671,9 +674,9 @@ static ssize_t taos_als_cal_target_show(struct device *dev,
 	return sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
 }
 
-static ssize_t taos_als_cal_target_store(struct device *dev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t len)
+static ssize_t illuminance0_input_target_store(struct device *dev,
+					       struct device_attribute *attr,
+					       const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -688,8 +691,9 @@ static ssize_t taos_als_cal_target_store(struct device *dev,
 	return len;
 }
 
-static ssize_t taos_lux_show(struct device *dev, struct device_attribute *attr,
-			     char *buf)
+static ssize_t illuminance0_input_show(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
 {
 	int ret;
 
@@ -700,9 +704,9 @@ static ssize_t taos_lux_show(struct device *dev, struct device_attribute *attr,
 	return sprintf(buf, "%d\n", ret);
 }
 
-static ssize_t taos_do_calibrate(struct device *dev,
-				 struct device_attribute *attr,
-				 const char *buf, size_t len)
+static ssize_t illuminance0_calibrate_store(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	int value;
@@ -716,8 +720,9 @@ static ssize_t taos_do_calibrate(struct device *dev,
 	return len;
 }
 
-static ssize_t taos_luxtable_show(struct device *dev,
-				  struct device_attribute *attr, char *buf)
+static ssize_t illuminance0_lux_table_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
 {
 	int i;
 	int offset = 0;
@@ -741,9 +746,9 @@ static ssize_t taos_luxtable_show(struct device *dev,
 	return offset;
 }
 
-static ssize_t taos_luxtable_store(struct device *dev,
-				   struct device_attribute *attr,
-				   const char *buf, size_t len)
+static ssize_t illuminance0_lux_table_store(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -781,29 +786,21 @@ luxable_store_done:
 	return ret;
 }
 
-static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
-		taos_power_state_show, taos_power_state_store);
+static DEVICE_ATTR_RW(power_state);
 
-static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
-		taos_gain_show, taos_gain_store);
-static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
-		taos_gain_available_show, NULL);
+static DEVICE_ATTR_RW(illuminance0_calibscale);
+static DEVICE_ATTR_RO(illuminance0_calibscale_available);
 
-static DEVICE_ATTR(illuminance0_integration_time, S_IRUGO | S_IWUSR,
-		taos_als_time_show, taos_als_time_store);
-static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
-		taos_als_time_available_show, NULL);
+static DEVICE_ATTR_RW(illuminance0_integration_time);
+static DEVICE_ATTR_RO(illuminance0_integration_time_available);
 
-static DEVICE_ATTR(illuminance0_calibbias, S_IRUGO | S_IWUSR,
-		taos_als_trim_show, taos_als_trim_store);
+static DEVICE_ATTR_RW(illuminance0_calibbias);
 
-static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
-		taos_als_cal_target_show, taos_als_cal_target_store);
+static DEVICE_ATTR_RW(illuminance0_input_target);
 
-static DEVICE_ATTR(illuminance0_input, S_IRUGO, taos_lux_show, NULL);
-static DEVICE_ATTR(illuminance0_calibrate, S_IWUSR, NULL, taos_do_calibrate);
-static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
-		taos_luxtable_show, taos_luxtable_store);
+static DEVICE_ATTR_RO(illuminance0_input);
+static DEVICE_ATTR_WO(illuminance0_calibrate);
+static DEVICE_ATTR_RW(illuminance0_lux_table);
 
 static struct attribute *sysfs_attrs_ctrl[] = {
 	&dev_attr_power_state.attr,
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1504001 — Re: [PATCH 3/7] iio: light: tsl2583: use DEVICE_ATTR_{RO, RW, WO} macros

> Use the DEVICE_ATTR_RO, IIO_DEVICE_ATTR_RW, and IIO_DEVICE_ATTR_WO
> macros to create the device attributes.

great that you work on cleaning this up!

the patch does a bit more than is claimed, it also renames stuff; 
eventually, a proper prefix ('tsl2583_') should be used...


maybe you can unify the two files in this patch series?
./staging/iio/Documentation/sysfs-bus-iio-light-tsl2583
./staging/iio/Documentation/light/sysfs-bus-iio-light-tsl2583

Suggested-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
:-)


> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  drivers/staging/iio/light/tsl2583.c | 117 ++++++++++++++++++------------------
>  1 file changed, 57 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index 8448a87..bbb8fc3 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -501,8 +501,8 @@ static int taos_chip_off(struct iio_dev *indio_dev)
>  
>  /* Sysfs Interface Functions */
>  
> -static ssize_t taos_power_state_show(struct device *dev,
> -				     struct device_attribute *attr, char *buf)
> +static ssize_t power_state_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -510,9 +510,9 @@ static ssize_t taos_power_state_show(struct device *dev,
>  	return sprintf(buf, "%d\n", chip->taos_chip_status);
>  }
>  
> -static ssize_t taos_power_state_store(struct device *dev,
> -				      struct device_attribute *attr,
> -				      const char *buf, size_t len)
> +static ssize_t power_state_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	int value;
> @@ -528,8 +528,9 @@ static ssize_t taos_power_state_store(struct device *dev,
>  	return len;
>  }
>  
> -static ssize_t taos_gain_show(struct device *dev,
> -			      struct device_attribute *attr, char *buf)
> +static ssize_t illuminance0_calibscale_show(struct device *dev,
> +					    struct device_attribute *attr,
> +					    char *buf)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -553,9 +554,9 @@ static ssize_t taos_gain_show(struct device *dev,
>  	return sprintf(buf, "%s\n", gain);
>  }
>  
> -static ssize_t taos_gain_store(struct device *dev,
> -			       struct device_attribute *attr,
> -			       const char *buf, size_t len)
> +static ssize_t illuminance0_calibscale_store(struct device *dev,
> +					     struct device_attribute *attr,
> +					     const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -587,15 +588,16 @@ gain_store_done:
>  	return ret;
>  }
>  
> -static ssize_t taos_gain_available_show(struct device *dev,
> -					struct device_attribute *attr,
> -					char *buf)
> +static ssize_t illuminance0_calibscale_available_show(struct device *dev,
> +						  struct device_attribute *attr,
> +						  char *buf)
>  {
>  	return sprintf(buf, "%s\n", "1 8 16 111");
>  }
>  
> -static ssize_t taos_als_time_show(struct device *dev,
> -				  struct device_attribute *attr, char *buf)
> +static ssize_t illuminance0_integration_time_show(struct device *dev,
> +						  struct device_attribute *attr,
> +						  char *buf)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -603,9 +605,9 @@ static ssize_t taos_als_time_show(struct device *dev,
>  	return sprintf(buf, "%d\n", chip->taos_settings.als_time);
>  }
>  
> -static ssize_t taos_als_time_store(struct device *dev,
> -				   struct device_attribute *attr,
> -				   const char *buf, size_t len)
> +static ssize_t illuminance0_integration_time_store(struct device *dev,
> +						  struct device_attribute *attr,
> +						  const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -627,16 +629,17 @@ als_time_store_done:
>  	return ret;
>  }
>  
> -static ssize_t taos_als_time_available_show(struct device *dev,
> -					    struct device_attribute *attr,
> -					    char *buf)
> +static ssize_t illuminance0_integration_time_available_show(struct device *dev,
> +						  struct device_attribute *attr,
> +						  char *buf)
>  {
>  	return sprintf(buf, "%s\n",
>  		"50 100 150 200 250 300 350 400 450 500 550 600 650");
>  }
>  
> -static ssize_t taos_als_trim_show(struct device *dev,
> -				  struct device_attribute *attr, char *buf)
> +static ssize_t illuminance0_calibbias_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -644,9 +647,9 @@ static ssize_t taos_als_trim_show(struct device *dev,
>  	return sprintf(buf, "%d\n", chip->taos_settings.als_gain_trim);
>  }
>  
> -static ssize_t taos_als_trim_store(struct device *dev,
> -				   struct device_attribute *attr,
> -				   const char *buf, size_t len)
> +static ssize_t illuminance0_calibbias_store(struct device *dev,
> +					    struct device_attribute *attr,
> +					    const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -661,9 +664,9 @@ static ssize_t taos_als_trim_store(struct device *dev,
>  	return len;
>  }
>  
> -static ssize_t taos_als_cal_target_show(struct device *dev,
> -					struct device_attribute *attr,
> -					char *buf)
> +static ssize_t illuminance0_input_target_show(struct device *dev,
> +					      struct device_attribute *attr,
> +					      char *buf)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -671,9 +674,9 @@ static ssize_t taos_als_cal_target_show(struct device *dev,
>  	return sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
>  }
>  
> -static ssize_t taos_als_cal_target_store(struct device *dev,
> -					 struct device_attribute *attr,
> -					 const char *buf, size_t len)
> +static ssize_t illuminance0_input_target_store(struct device *dev,
> +					       struct device_attribute *attr,
> +					       const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -688,8 +691,9 @@ static ssize_t taos_als_cal_target_store(struct device *dev,
>  	return len;
>  }
>  
> -static ssize_t taos_lux_show(struct device *dev, struct device_attribute *attr,
> -			     char *buf)
> +static ssize_t illuminance0_input_show(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buf)
>  {
>  	int ret;
>  
> @@ -700,9 +704,9 @@ static ssize_t taos_lux_show(struct device *dev, struct device_attribute *attr,
>  	return sprintf(buf, "%d\n", ret);
>  }
>  
> -static ssize_t taos_do_calibrate(struct device *dev,
> -				 struct device_attribute *attr,
> -				 const char *buf, size_t len)
> +static ssize_t illuminance0_calibrate_store(struct device *dev,
> +					    struct device_attribute *attr,
> +					    const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	int value;
> @@ -716,8 +720,9 @@ static ssize_t taos_do_calibrate(struct device *dev,
>  	return len;
>  }
>  
> -static ssize_t taos_luxtable_show(struct device *dev,
> -				  struct device_attribute *attr, char *buf)
> +static ssize_t illuminance0_lux_table_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
>  {
>  	int i;
>  	int offset = 0;
> @@ -741,9 +746,9 @@ static ssize_t taos_luxtable_show(struct device *dev,
>  	return offset;
>  }
>  
> -static ssize_t taos_luxtable_store(struct device *dev,
> -				   struct device_attribute *attr,
> -				   const char *buf, size_t len)
> +static ssize_t illuminance0_lux_table_store(struct device *dev,
> +					    struct device_attribute *attr,
> +					    const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -781,29 +786,21 @@ luxable_store_done:
>  	return ret;
>  }
>  
> -static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> -		taos_power_state_show, taos_power_state_store);
> +static DEVICE_ATTR_RW(power_state);
>  
> -static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
> -		taos_gain_show, taos_gain_store);
> -static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
> -		taos_gain_available_show, NULL);
> +static DEVICE_ATTR_RW(illuminance0_calibscale);
> +static DEVICE_ATTR_RO(illuminance0_calibscale_available);
>  
> -static DEVICE_ATTR(illuminance0_integration_time, S_IRUGO | S_IWUSR,
> -		taos_als_time_show, taos_als_time_store);
> -static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
> -		taos_als_time_available_show, NULL);
> +static DEVICE_ATTR_RW(illuminance0_integration_time);
> +static DEVICE_ATTR_RO(illuminance0_integration_time_available);
>  
> -static DEVICE_ATTR(illuminance0_calibbias, S_IRUGO | S_IWUSR,
> -		taos_als_trim_show, taos_als_trim_store);
> +static DEVICE_ATTR_RW(illuminance0_calibbias);
>  
> -static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
> -		taos_als_cal_target_show, taos_als_cal_target_store);
> +static DEVICE_ATTR_RW(illuminance0_input_target);
>  
> -static DEVICE_ATTR(illuminance0_input, S_IRUGO, taos_lux_show, NULL);
> -static DEVICE_ATTR(illuminance0_calibrate, S_IWUSR, NULL, taos_do_calibrate);
> -static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
> -		taos_luxtable_show, taos_luxtable_store);
> +static DEVICE_ATTR_RO(illuminance0_input);
> +static DEVICE_ATTR_WO(illuminance0_calibrate);
> +static DEVICE_ATTR_RW(illuminance0_lux_table);
>  
>  static struct attribute *sysfs_attrs_ctrl[] = {
>  	&dev_attr_power_state.attr,
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

[toc] | [prev] | [next] | [standalone]

#1504128 — Re: [PATCH 3/7] iio: light: tsl2583: use DEVICE_ATTR_{RO, RW, WO} macros

On 19 October 2016 11:32:06 BST, Brian Masney <masneyb@onstation.org> wrote:
>Use the DEVICE_ATTR_RO, IIO_DEVICE_ATTR_RW, and IIO_DEVICE_ATTR_WO
>macros to create the device attributes.
>
>Signed-off-by: Brian Masney <masneyb@onstation.org>

Hi Brian, 

One very quick comment.  Driver should be using an iio_chan_spec array and relevant
 info_mask elements plus read_raw etc.

That will get rid of about half of these so I would do that before cleaning up the remainder.

J
>---
>drivers/staging/iio/light/tsl2583.c | 117
>++++++++++++++++++------------------
> 1 file changed, 57 insertions(+), 60 deletions(-)
>
>diff --git a/drivers/staging/iio/light/tsl2583.c
>b/drivers/staging/iio/light/tsl2583.c
>index 8448a87..bbb8fc3 100644
>--- a/drivers/staging/iio/light/tsl2583.c
>+++ b/drivers/staging/iio/light/tsl2583.c
>@@ -501,8 +501,8 @@ static int taos_chip_off(struct iio_dev *indio_dev)
> 
> /* Sysfs Interface Functions */
> 
>-static ssize_t taos_power_state_show(struct device *dev,
>-				     struct device_attribute *attr, char *buf)
>+static ssize_t power_state_show(struct device *dev,
>+				struct device_attribute *attr, char *buf)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -510,9 +510,9 @@ static ssize_t taos_power_state_show(struct device
>*dev,
> 	return sprintf(buf, "%d\n", chip->taos_chip_status);
> }
> 
>-static ssize_t taos_power_state_store(struct device *dev,
>-				      struct device_attribute *attr,
>-				      const char *buf, size_t len)
>+static ssize_t power_state_store(struct device *dev,
>+				 struct device_attribute *attr,
>+				 const char *buf, size_t len)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	int value;
>@@ -528,8 +528,9 @@ static ssize_t taos_power_state_store(struct device
>*dev,
> 	return len;
> }
> 
>-static ssize_t taos_gain_show(struct device *dev,
>-			      struct device_attribute *attr, char *buf)
>+static ssize_t illuminance0_calibscale_show(struct device *dev,
>+					    struct device_attribute *attr,
>+					    char *buf)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -553,9 +554,9 @@ static ssize_t taos_gain_show(struct device *dev,
> 	return sprintf(buf, "%s\n", gain);
> }
> 
>-static ssize_t taos_gain_store(struct device *dev,
>-			       struct device_attribute *attr,
>-			       const char *buf, size_t len)
>+static ssize_t illuminance0_calibscale_store(struct device *dev,
>+					     struct device_attribute *attr,
>+					     const char *buf, size_t len)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -587,15 +588,16 @@ gain_store_done:
> 	return ret;
> }
> 
>-static ssize_t taos_gain_available_show(struct device *dev,
>-					struct device_attribute *attr,
>-					char *buf)
>+static ssize_t illuminance0_calibscale_available_show(struct device
>*dev,
>+						  struct device_attribute *attr,
>+						  char *buf)
> {
> 	return sprintf(buf, "%s\n", "1 8 16 111");
> }
> 
>-static ssize_t taos_als_time_show(struct device *dev,
>-				  struct device_attribute *attr, char *buf)
>+static ssize_t illuminance0_integration_time_show(struct device *dev,
>+						  struct device_attribute *attr,
>+						  char *buf)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -603,9 +605,9 @@ static ssize_t taos_als_time_show(struct device
>*dev,
> 	return sprintf(buf, "%d\n", chip->taos_settings.als_time);
> }
> 
>-static ssize_t taos_als_time_store(struct device *dev,
>-				   struct device_attribute *attr,
>-				   const char *buf, size_t len)
>+static ssize_t illuminance0_integration_time_store(struct device *dev,
>+						  struct device_attribute *attr,
>+						  const char *buf, size_t len)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -627,16 +629,17 @@ als_time_store_done:
> 	return ret;
> }
> 
>-static ssize_t taos_als_time_available_show(struct device *dev,
>-					    struct device_attribute *attr,
>-					    char *buf)
>+static ssize_t illuminance0_integration_time_available_show(struct
>device *dev,
>+						  struct device_attribute *attr,
>+						  char *buf)
> {
> 	return sprintf(buf, "%s\n",
> 		"50 100 150 200 250 300 350 400 450 500 550 600 650");
> }
> 
>-static ssize_t taos_als_trim_show(struct device *dev,
>-				  struct device_attribute *attr, char *buf)
>+static ssize_t illuminance0_calibbias_show(struct device *dev,
>+					   struct device_attribute *attr,
>+					   char *buf)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -644,9 +647,9 @@ static ssize_t taos_als_trim_show(struct device
>*dev,
> 	return sprintf(buf, "%d\n", chip->taos_settings.als_gain_trim);
> }
> 
>-static ssize_t taos_als_trim_store(struct device *dev,
>-				   struct device_attribute *attr,
>-				   const char *buf, size_t len)
>+static ssize_t illuminance0_calibbias_store(struct device *dev,
>+					    struct device_attribute *attr,
>+					    const char *buf, size_t len)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -661,9 +664,9 @@ static ssize_t taos_als_trim_store(struct device
>*dev,
> 	return len;
> }
> 
>-static ssize_t taos_als_cal_target_show(struct device *dev,
>-					struct device_attribute *attr,
>-					char *buf)
>+static ssize_t illuminance0_input_target_show(struct device *dev,
>+					      struct device_attribute *attr,
>+					      char *buf)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -671,9 +674,9 @@ static ssize_t taos_als_cal_target_show(struct
>device *dev,
> 	return sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
> }
> 
>-static ssize_t taos_als_cal_target_store(struct device *dev,
>-					 struct device_attribute *attr,
>-					 const char *buf, size_t len)
>+static ssize_t illuminance0_input_target_store(struct device *dev,
>+					       struct device_attribute *attr,
>+					       const char *buf, size_t len)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -688,8 +691,9 @@ static ssize_t taos_als_cal_target_store(struct
>device *dev,
> 	return len;
> }
> 
>-static ssize_t taos_lux_show(struct device *dev, struct
>device_attribute *attr,
>-			     char *buf)
>+static ssize_t illuminance0_input_show(struct device *dev,
>+				       struct device_attribute *attr,
>+				       char *buf)
> {
> 	int ret;
> 
>@@ -700,9 +704,9 @@ static ssize_t taos_lux_show(struct device *dev,
>struct device_attribute *attr,
> 	return sprintf(buf, "%d\n", ret);
> }
> 
>-static ssize_t taos_do_calibrate(struct device *dev,
>-				 struct device_attribute *attr,
>-				 const char *buf, size_t len)
>+static ssize_t illuminance0_calibrate_store(struct device *dev,
>+					    struct device_attribute *attr,
>+					    const char *buf, size_t len)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	int value;
>@@ -716,8 +720,9 @@ static ssize_t taos_do_calibrate(struct device
>*dev,
> 	return len;
> }
> 
>-static ssize_t taos_luxtable_show(struct device *dev,
>-				  struct device_attribute *attr, char *buf)
>+static ssize_t illuminance0_lux_table_show(struct device *dev,
>+					   struct device_attribute *attr,
>+					   char *buf)
> {
> 	int i;
> 	int offset = 0;
>@@ -741,9 +746,9 @@ static ssize_t taos_luxtable_show(struct device
>*dev,
> 	return offset;
> }
> 
>-static ssize_t taos_luxtable_store(struct device *dev,
>-				   struct device_attribute *attr,
>-				   const char *buf, size_t len)
>+static ssize_t illuminance0_lux_table_store(struct device *dev,
>+					    struct device_attribute *attr,
>+					    const char *buf, size_t len)
> {
> 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> 	struct tsl2583_chip *chip = iio_priv(indio_dev);
>@@ -781,29 +786,21 @@ luxable_store_done:
> 	return ret;
> }
> 
>-static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
>-		taos_power_state_show, taos_power_state_store);
>+static DEVICE_ATTR_RW(power_state);
> 
>-static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
>-		taos_gain_show, taos_gain_store);
>-static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
>-		taos_gain_available_show, NULL);
>+static DEVICE_ATTR_RW(illuminance0_calibscale);
>+static DEVICE_ATTR_RO(illuminance0_calibscale_available);
> 
>-static DEVICE_ATTR(illuminance0_integration_time, S_IRUGO | S_IWUSR,
>-		taos_als_time_show, taos_als_time_store);
>-static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
>-		taos_als_time_available_show, NULL);
>+static DEVICE_ATTR_RW(illuminance0_integration_time);
>+static DEVICE_ATTR_RO(illuminance0_integration_time_available);
> 
>-static DEVICE_ATTR(illuminance0_calibbias, S_IRUGO | S_IWUSR,
>-		taos_als_trim_show, taos_als_trim_store);
>+static DEVICE_ATTR_RW(illuminance0_calibbias);
> 
>-static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
>-		taos_als_cal_target_show, taos_als_cal_target_store);
>+static DEVICE_ATTR_RW(illuminance0_input_target);
> 
>-static DEVICE_ATTR(illuminance0_input, S_IRUGO, taos_lux_show, NULL);
>-static DEVICE_ATTR(illuminance0_calibrate, S_IWUSR, NULL,
>taos_do_calibrate);
>-static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
>-		taos_luxtable_show, taos_luxtable_store);
>+static DEVICE_ATTR_RO(illuminance0_input);
>+static DEVICE_ATTR_WO(illuminance0_calibrate);
>+static DEVICE_ATTR_RW(illuminance0_lux_table);
> 
> static struct attribute *sysfs_attrs_ctrl[] = {
> 	&dev_attr_power_state.attr,

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]

#1503771 — [PATCH 2/7] iio: light: tsl2583: change functions to only have a single exit point

Change the following functions to only have a single exit point:
taos_i2c_read(), taos_als_calibrate(), taos_chip_on(),
taos_gain_store(), taos_gain_available_show(), taos_luxtable_store()
and taos_probe().

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 1e42a19..8448a87 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -159,7 +159,7 @@ static void taos_defaults(struct tsl2583_chip *chip)
 static int
 taos_i2c_read(struct i2c_client *client, u8 reg, u8 *val, unsigned int len)
 {
-	int i, ret;
+	int i, ret = 0;
 
 	for (i = 0; i < len; i++) {
 		/* select register to write */
@@ -168,14 +168,16 @@ taos_i2c_read(struct i2c_client *client, u8 reg, u8 *val, unsigned int len)
 			dev_err(&client->dev,
 				"taos_i2c_read failed to write register %x\n",
 				reg);
-			return ret;
+			goto read_done;
 		}
 		/* read the data */
 		*val = i2c_smbus_read_byte(client);
 		val++;
 		reg++;
 	}
-	return 0;
+
+read_done:
+	return ret;
 }
 
 /*
@@ -351,7 +353,7 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
 		dev_err(&chip->client->dev,
 			"taos_als_calibrate failed to reach the CNTRL register, ret=%d\n",
 			ret);
-		return ret;
+		goto calibrate_done;
 	}
 
 	reg_val = i2c_smbus_read_byte(chip->client);
@@ -359,7 +361,8 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
 			!= (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON)) {
 		dev_err(&chip->client->dev,
 			"taos_als_calibrate failed: device not powered on with ADC enabled\n");
-		return -ENODATA;
+		ret = -ENODATA;
+		goto calibrate_done;
 	}
 
 	ret = i2c_smbus_write_byte(chip->client,
@@ -368,19 +371,21 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
 		dev_err(&chip->client->dev,
 			"taos_als_calibrate failed to reach the STATUS register, ret=%d\n",
 			ret);
-		return ret;
+		goto calibrate_done;
 	}
 	reg_val = i2c_smbus_read_byte(chip->client);
 
 	if ((reg_val & TSL258X_STA_ADC_VALID) != TSL258X_STA_ADC_VALID) {
 		dev_err(&chip->client->dev,
 			"taos_als_calibrate failed: STATUS - ADC not valid.\n");
-		return -ENODATA;
+		ret = -ENODATA;
+		goto calibrate_done;
 	}
 	lux_val = taos_get_lux(indio_dev);
 	if (lux_val < 0) {
 		dev_err(&chip->client->dev, "taos_als_calibrate failed to get lux\n");
-		return lux_val;
+		ret = lux_val;
+		goto calibrate_done;
 	}
 	gain_trim_val = (unsigned int)(((chip->taos_settings.als_cal_target)
 			* chip->taos_settings.als_gain_trim) / lux_val);
@@ -389,11 +394,15 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
 		dev_err(&chip->client->dev,
 			"taos_als_calibrate failed: trim_val of %d is out of range\n",
 			gain_trim_val);
-		return -ENODATA;
+		ret = -ENODATA;
+		goto calibrate_done;
 	}
 	chip->taos_settings.als_gain_trim = (int)gain_trim_val;
+	ret = gain_trim_val;
+
+calibrate_done:
+	return ret;
 
-	return (int)gain_trim_val;
 }
 
 /*
@@ -403,7 +412,7 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
 static int taos_chip_on(struct iio_dev *indio_dev)
 {
 	int i;
-	int ret;
+	int ret = -EINVAL;
 	u8 *uP;
 	u8 utmp;
 	int als_count;
@@ -414,7 +423,7 @@ static int taos_chip_on(struct iio_dev *indio_dev)
 	if (chip->taos_chip_status == TSL258X_CHIP_WORKING) {
 		/* if forcing a register update - turn off, then on */
 		dev_info(&chip->client->dev, "device is already enabled\n");
-		return -EINVAL;
+		goto chip_on_done;
 	}
 
 	/* determine als integration register */
@@ -442,7 +451,7 @@ static int taos_chip_on(struct iio_dev *indio_dev)
 					TSL258X_CMD_REG | TSL258X_CNTRL, utmp);
 	if (ret < 0) {
 		dev_err(&chip->client->dev, "taos_chip_on failed on CNTRL reg.\n");
-		return ret;
+		goto chip_on_done;
 	}
 
 	/*
@@ -456,7 +465,7 @@ static int taos_chip_on(struct iio_dev *indio_dev)
 		if (ret < 0) {
 			dev_err(&chip->client->dev,
 				"taos_chip_on failed on reg %d.\n", i);
-			return ret;
+			goto chip_on_done;
 		}
 	}
 
@@ -471,10 +480,11 @@ static int taos_chip_on(struct iio_dev *indio_dev)
 					utmp);
 	if (ret < 0) {
 		dev_err(&chip->client->dev, "taos_chip_on failed on 2nd CTRL reg.\n");
-		return ret;
+		goto chip_on_done;
 	}
 	chip->taos_chip_status = TSL258X_CHIP_WORKING;
 
+chip_on_done:
 	return ret;
 }
 
@@ -549,10 +559,10 @@ static ssize_t taos_gain_store(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
-	int value;
+	int value, ret = -EINVAL;
 
 	if (kstrtoint(buf, 0, &value))
-		return -EINVAL;
+		goto gain_store_done;
 
 	switch (value) {
 	case 1:
@@ -569,10 +579,12 @@ static ssize_t taos_gain_store(struct device *dev,
 		break;
 	default:
 		dev_err(dev, "Invalid Gain Index (must be 1,8,16,111)\n");
-		return -EINVAL;
+		goto gain_store_done;
 	}
+	ret = len;
 
-	return len;
+gain_store_done:
+	return ret;
 }
 
 static ssize_t taos_gain_available_show(struct device *dev,
@@ -597,20 +609,22 @@ static ssize_t taos_als_time_store(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
-	int value;
+	int value, ret = -EINVAL;
 
 	if (kstrtoint(buf, 0, &value))
-		return -EINVAL;
+		goto als_time_store_done;
 
 	if ((value < 50) || (value > 650))
-		return -EINVAL;
+		goto als_time_store_done;
 
 	if (value % 50)
-		return -EINVAL;
+		goto als_time_store_done;
 
 	chip->taos_settings.als_time = value;
+	ret = len;
 
-	return len;
+als_time_store_done:
+	return ret;
 }
 
 static ssize_t taos_als_time_available_show(struct device *dev,
@@ -734,7 +748,7 @@ static ssize_t taos_luxtable_store(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
 	int value[ARRAY_SIZE(taos_device_lux) * 3 + 1];
-	int n;
+	int n, ret = -EINVAL;
 
 	get_options(buf, ARRAY_SIZE(value), value);
 
@@ -746,11 +760,11 @@ static ssize_t taos_luxtable_store(struct device *dev,
 	n = value[0];
 	if ((n % 3) || n < 6 || n > ((ARRAY_SIZE(taos_device_lux) - 1) * 3)) {
 		dev_info(dev, "LUX TABLE INPUT ERROR 1 Value[0]=%d\n", n);
-		return -EINVAL;
+		goto luxable_store_done;
 	}
 	if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0) {
 		dev_info(dev, "LUX TABLE INPUT ERROR 2 Value[0]=%d\n", n);
-		return -EINVAL;
+		goto luxable_store_done;
 	}
 
 	if (chip->taos_chip_status == TSL258X_CHIP_WORKING)
@@ -761,8 +775,10 @@ static ssize_t taos_luxtable_store(struct device *dev,
 	memcpy(taos_device_lux, &value[1], (value[0] * 4));
 
 	taos_chip_on(indio_dev);
+	ret = len;
 
-	return len;
+luxable_store_done:
+	return ret;
 }
 
 static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
@@ -833,12 +849,15 @@ static int taos_probe(struct i2c_client *clientp,
 	if (!i2c_check_functionality(clientp->adapter,
 				     I2C_FUNC_SMBUS_BYTE_DATA)) {
 		dev_err(&clientp->dev, "taos_probe() - i2c smbus byte data func unsupported\n");
-		return -EOPNOTSUPP;
+		ret = -EOPNOTSUPP;
+		goto probe_done;
 	}
 
 	indio_dev = devm_iio_device_alloc(&clientp->dev, sizeof(*chip));
-	if (!indio_dev)
-		return -ENOMEM;
+	if (!indio_dev) {
+		ret = -ENOMEM;
+		goto probe_done;
+	}
 	chip = iio_priv(indio_dev);
 	chip->client = clientp;
 	i2c_set_clientdata(clientp, indio_dev);
@@ -854,14 +873,14 @@ static int taos_probe(struct i2c_client *clientp,
 			dev_err(&clientp->dev,
 				"i2c_smbus_write_byte to cmd reg failed in taos_probe(), err = %d\n",
 				ret);
-			return ret;
+			goto probe_done;
 		}
 		ret = i2c_smbus_read_byte(clientp);
 		if (ret < 0) {
 			dev_err(&clientp->dev,
 				"i2c_smbus_read_byte from reg failed in taos_probe(), err = %d\n",
 				ret);
-			return ret;
+			goto probe_done;
 		}
 		buf[i] = ret;
 	}
@@ -869,7 +888,8 @@ static int taos_probe(struct i2c_client *clientp,
 	if (!taos_tsl258x_device(buf)) {
 		dev_info(&clientp->dev,
 			 "i2c device found but does not match expected id in taos_probe()\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto probe_done;
 	}
 
 	ret = i2c_smbus_write_byte(clientp, (TSL258X_CMD_REG | TSL258X_CNTRL));
@@ -877,7 +897,7 @@ static int taos_probe(struct i2c_client *clientp,
 		dev_err(&clientp->dev,
 			"i2c_smbus_write_byte() to cmd reg failed in taos_probe(), err = %d\n",
 			ret);
-		return ret;
+		goto probe_done;
 	}
 
 	indio_dev->info = &tsl2583_info;
@@ -887,7 +907,7 @@ static int taos_probe(struct i2c_client *clientp,
 	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 	if (ret) {
 		dev_err(&clientp->dev, "iio registration failed\n");
-		return ret;
+		goto probe_done;
 	}
 
 	/* Load up the V2 defaults (these are hard coded defaults for now) */
@@ -897,7 +917,9 @@ static int taos_probe(struct i2c_client *clientp,
 	taos_chip_on(indio_dev);
 
 	dev_info(&clientp->dev, "Light sensor found.\n");
-	return 0;
+
+probe_done:
+	return ret;
 }
 
 #ifdef CONFIG_PM_SLEEP
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1503792 — Re: [PATCH 2/7] iio: light: tsl2583: change functions to only have a single exit point

On Wed, Oct 19, 2016 at 06:32:05AM -0400, Brian Masney wrote:
> Change the following functions to only have a single exit point:
> taos_i2c_read(), taos_als_calibrate(), taos_chip_on(),
> taos_gain_store(), taos_gain_available_show(), taos_luxtable_store()
> and taos_probe().
> 

What's the point of this?  This style of code just makes things more
complicated and leads to "forgot the error code" bugs.  People think
that it future proofs the code in case we add locking but I have looked
into this and it has minimal if any impact at preventing locking bugs.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1503796 — [PATCH 4/7] iio: light: tsl2583: return proper error code in sysfs store functions

illuminance0_calibbias_store(), illuminance0_input_target_store(), and
illuminance0_calibrate_store() did not return an error code when
an invalid value was passed in. The input was checked to see if the
input was valid, however the caller would not be notified that an
invalid value was passed in. This patch changes these three functions to
return -EINVAL when invalid input is passed in.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2583.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index bbb8fc3..a60433e 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -657,9 +657,10 @@ static ssize_t illuminance0_calibbias_store(struct device *dev,
 
 	if (kstrtoint(buf, 0, &value))
 		return -EINVAL;
+	else if (!value)
+		return -EINVAL;
 
-	if (value)
-		chip->taos_settings.als_gain_trim = value;
+	chip->taos_settings.als_gain_trim = value;
 
 	return len;
 }
@@ -684,9 +685,10 @@ static ssize_t illuminance0_input_target_store(struct device *dev,
 
 	if (kstrtoint(buf, 0, &value))
 		return -EINVAL;
+	else if (!value)
+		return -EINVAL;
 
-	if (value)
-		chip->taos_settings.als_cal_target = value;
+	chip->taos_settings.als_cal_target = value;
 
 	return len;
 }
@@ -713,9 +715,10 @@ static ssize_t illuminance0_calibrate_store(struct device *dev,
 
 	if (kstrtoint(buf, 0, &value))
 		return -EINVAL;
+	else if (value != 1)
+		return -EINVAL;
 
-	if (value == 1)
-		taos_als_calibrate(indio_dev);
+	taos_als_calibrate(indio_dev);
 
 	return len;
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1503802 — [PATCH 5/7] iio: light: tsl2583: check return values from taos_chip_{on,off}

The return values from taos_chip_on() and taos_chip_off() was not
checked in several places. This patch adds proper error checking to
these function calls.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index a60433e..60f0ce9 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -515,15 +515,18 @@ static ssize_t power_state_store(struct device *dev,
 				 const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	int value;
+	int value, ret;
 
 	if (kstrtoint(buf, 0, &value))
 		return -EINVAL;
 
 	if (!value)
-		taos_chip_off(indio_dev);
+		ret = taos_chip_off(indio_dev);
 	else
-		taos_chip_on(indio_dev);
+		ret = taos_chip_on(indio_dev);
+
+	if (ret < 0)
+		return ret;
 
 	return len;
 }
@@ -775,14 +778,20 @@ static ssize_t illuminance0_lux_table_store(struct device *dev,
 		goto luxable_store_done;
 	}
 
-	if (chip->taos_chip_status == TSL258X_CHIP_WORKING)
-		taos_chip_off(indio_dev);
+	if (chip->taos_chip_status == TSL258X_CHIP_WORKING) {
+		ret = taos_chip_off(indio_dev);
+		if (ret < 0)
+			return ret;
+	}
 
 	/* Zero out the table */
 	memset(taos_device_lux, 0, sizeof(taos_device_lux));
 	memcpy(taos_device_lux, &value[1], (value[0] * 4));
 
-	taos_chip_on(indio_dev);
+	ret = taos_chip_on(indio_dev);
+	if (ret < 0)
+		return ret;
+
 	ret = len;
 
 luxable_store_done:
@@ -914,7 +923,9 @@ static int taos_probe(struct i2c_client *clientp,
 	taos_defaults(chip);
 
 	/* Make sure the chip is on */
-	taos_chip_on(indio_dev);
+	ret = taos_chip_on(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	dev_info(&clientp->dev, "Light sensor found.\n");
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1506563

On 19/10/16 11:32, Brian Masney wrote:
> taos_gain_store() and taos_als_calibrate() both have a code path where
> -1 was returned. This patch changes the code so that a proper error code
> is returned to make the code consistent with the error paths that are
> present within those same functions.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  drivers/staging/iio/light/tsl2583.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index 08f1583..1e42a19 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -359,7 +359,7 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
>  			!= (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON)) {
>  		dev_err(&chip->client->dev,
>  			"taos_als_calibrate failed: device not powered on with ADC enabled\n");
> -		return -1;
> +		return -ENODATA;
Hmm. Not sure about this as the error value.  Perhaps a simple -EINVAL?

Also, this is still eating the possible error returned directly by
i2c_smbus_read_byte - would you mind fixing that whilst we are here?
>  	}
>  
>  	ret = i2c_smbus_write_byte(chip->client,
> @@ -569,7 +569,7 @@ static ssize_t taos_gain_store(struct device *dev,
>  		break;
>  	default:
>  		dev_err(dev, "Invalid Gain Index (must be 1,8,16,111)\n");
> -		return -1;
> +		return -EINVAL;
>  	}
>  
>  	return len;
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web