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

Groups > linux.kernel > #1630144 > unrolled thread

[PATCH v3 0/2] staging: iio: isl29028: move out of staging

Started byBrian Masney <masneyb@onstation.org>
First post2017-04-25 03:50 +0200
Last post2017-04-26 08:10 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/2] staging: iio: isl29028: move out of staging Brian Masney <masneyb@onstation.org> - 2017-04-25 03:50 +0200
    [PATCH v3 1/2] staging: iio: isl29028: correct proximity sleep times Brian Masney <masneyb@onstation.org> - 2017-04-25 03:50 +0200
      Re: [PATCH v3 1/2] staging: iio: isl29028: correct proximity sleep  times Jonathan Cameron <jic23@kernel.org> - 2017-04-26 07:40 +0200
    Re: [PATCH v3 0/2] staging: iio: isl29028: move out of staging Jonathan Cameron <jic23@kernel.org> - 2017-04-26 07:30 +0200
    Re: [PATCH v3 2/2] staging: iio: isl29028: move out of staging Joe Perches <joe@perches.com> - 2017-04-26 07:50 +0200
      Re: [PATCH v3 2/2] staging: iio: isl29028: move out of staging Jonathan Cameron <jic23@kernel.org> - 2017-04-26 08:10 +0200

#1630144 — [PATCH v3 0/2] staging: iio: isl29028: move out of staging

FromBrian Masney <masneyb@onstation.org>
Date2017-04-25 03:50 +0200
Subject[PATCH v3 0/2] staging: iio: isl29028: move out of staging
Message-ID<tzXM5-6p1-5@gated-at.bofh.it>
Minor cleanup to the proximity sampling to move this driver out of
staging.

Datasheet:
http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29028.pdf

Changes since v2:
- Reject any value that is not in the
  in_proximity_sampling_frequency_available sysfs attribute.
- Changed the sampling frequency 83.3 to 80. The data sheet lists 12.5
  ms, but the original code listed 12 ms, which was the cause of the
  discrepancy.
Changes since v1:
- in_proximity_sampling_frequency_available sysfs attribute now shows
  decimals.

Jonathan: Sorry it took me a little over two months to get back to you
on this driver. I got side tracked with other projects.

Brian Masney (2):
  staging: iio: isl29028: correct proximity sleep times
  staging: iio: isl29028: move out of staging

 drivers/iio/light/Kconfig            |  10 +
 drivers/iio/light/Makefile           |   1 +
 drivers/iio/light/isl29028.c         | 723 +++++++++++++++++++++++++++++++++++
 drivers/staging/iio/light/Kconfig    |  10 -
 drivers/staging/iio/light/Makefile   |   1 -
 drivers/staging/iio/light/isl29028.c | 693 ---------------------------------
 6 files changed, 734 insertions(+), 704 deletions(-)
 create mode 100644 drivers/iio/light/isl29028.c
 delete mode 100644 drivers/staging/iio/light/isl29028.c

-- 
2.9.3

[toc] | [next] | [standalone]

#1630146 — [PATCH v3 1/2] staging: iio: isl29028: correct proximity sleep times

FromBrian Masney <masneyb@onstation.org>
Date2017-04-25 03:50 +0200
Subject[PATCH v3 1/2] staging: iio: isl29028: correct proximity sleep times
Message-ID<tzXM5-6p1-9@gated-at.bofh.it>
In reply to#1630144
The sysfs attribute in_proximity_sampling_frequency_available currently
shows the values 1 3 5 10 13 20 83 100. These values are supposed to
correspond to the sleep values 800 400 200 100 75 50 12.5 0 (all in ms).
When passing in a sampling frequency of 3, it actually uses a sleep
time of 200ms instead of the expected 400ms value. This patch changes
the value shown by this sysfs attribute to use fixed-point numbers so
that the correct sampling frequency is shown to the user. This patch
also changes the code that updates the proximity sampling frequency to
only allow values that are shown in the _available sysfs attribute.

The original code showed the value 83 that corresponds to the sleep
time 12 ms. The data sheet actually lists 12.5 ms as the sleep time,
so the proximity frequency was updated to 80.

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

diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
index 5375e7a..aeb5082 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -64,8 +64,25 @@
 
 #define ISL29028_POWER_OFF_DELAY_MS		2000
 
-static const unsigned int isl29028_prox_sleep_time[] = {800, 400, 200, 100, 75,
-							50, 12, 0};
+struct isl29028_prox_data {
+	int sampling_int;
+	int sampling_fract;
+	int sleep_time;
+};
+
+static const struct isl29028_prox_data isl29028_prox_data[] = {
+	{   1, 250000, 800 },
+	{   2, 500000, 400 },
+	{   5,      0, 200 },
+	{  10,      0, 100 },
+	{  13, 300000,  75 },
+	{  20,      0,  50 },
+	{  80,      0,  13 }, /*
+			       * Note: Data sheet lists 12.5 ms sleep time.
+			       * Round up a half millisecond for msleep().
+			       */
+	{ 100,  0,   0 }
+};
 
 enum isl29028_als_ir_mode {
 	ISL29028_MODE_NONE = 0,
@@ -76,32 +93,37 @@ enum isl29028_als_ir_mode {
 struct isl29028_chip {
 	struct mutex			lock;
 	struct regmap			*regmap;
-	unsigned int			prox_sampling;
+	int				prox_sampling_int;
+	int				prox_sampling_frac;
 	bool				enable_prox;
 	int				lux_scale;
 	enum isl29028_als_ir_mode	als_ir_mode;
 };
 
-static int isl29028_find_prox_sleep_time_index(int sampling)
+static int isl29028_find_prox_sleep_index(int sampling_int, int sampling_fract)
 {
-	unsigned int period = DIV_ROUND_UP(1000, sampling);
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(isl29028_prox_sleep_time); ++i) {
-		if (period >= isl29028_prox_sleep_time[i])
-			break;
+	for (i = 0; i < ARRAY_SIZE(isl29028_prox_data); ++i) {
+		if (isl29028_prox_data[i].sampling_int == sampling_int &&
+		    isl29028_prox_data[i].sampling_fract == sampling_fract)
+			return i;
 	}
 
-	return i;
+	return -EINVAL;
 }
 
 static int isl29028_set_proxim_sampling(struct isl29028_chip *chip,
-					unsigned int sampling)
+					int sampling_int, int sampling_fract)
 {
 	struct device *dev = regmap_get_device(chip->regmap);
 	int sleep_index, ret;
 
-	sleep_index = isl29028_find_prox_sleep_time_index(sampling);
+	sleep_index = isl29028_find_prox_sleep_index(sampling_int,
+						     sampling_fract);
+	if (sleep_index < 0)
+		return sleep_index;
+
 	ret = regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
 				 ISL29028_CONF_PROX_SLP_MASK,
 				 sleep_index << ISL29028_CONF_PROX_SLP_SH);
@@ -112,16 +134,18 @@ static int isl29028_set_proxim_sampling(struct isl29028_chip *chip,
 		return ret;
 	}
 
-	chip->prox_sampling = sampling;
+	chip->prox_sampling_int = sampling_int;
+	chip->prox_sampling_frac = sampling_fract;
 
 	return ret;
 }
 
 static int isl29028_enable_proximity(struct isl29028_chip *chip)
 {
-	int sleep_index, ret;
+	int prox_index, ret;
 
-	ret = isl29028_set_proxim_sampling(chip, chip->prox_sampling);
+	ret = isl29028_set_proxim_sampling(chip, chip->prox_sampling_int,
+					   chip->prox_sampling_frac);
 	if (ret < 0)
 		return ret;
 
@@ -132,8 +156,12 @@ static int isl29028_enable_proximity(struct isl29028_chip *chip)
 		return ret;
 
 	/* Wait for conversion to be complete for first sample */
-	sleep_index = isl29028_find_prox_sleep_time_index(chip->prox_sampling);
-	msleep(isl29028_prox_sleep_time[sleep_index]);
+	prox_index = isl29028_find_prox_sleep_index(chip->prox_sampling_int,
+						    chip->prox_sampling_frac);
+	if (prox_index < 0)
+		return prox_index;
+
+	msleep(isl29028_prox_data[prox_index].sleep_time);
 
 	return 0;
 }
@@ -361,7 +389,7 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 			break;
 		}
 
-		ret = isl29028_set_proxim_sampling(chip, val);
+		ret = isl29028_set_proxim_sampling(chip, val, val2);
 		break;
 	case IIO_LIGHT:
 		if (mask != IIO_CHAN_INFO_SCALE) {
@@ -439,7 +467,8 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 		if (chan->type != IIO_PROXIMITY)
 			break;
 
-		*val = chip->prox_sampling;
+		*val = chip->prox_sampling_int;
+		*val2 = chip->prox_sampling_frac;
 		ret = IIO_VAL_INT;
 		break;
 	case IIO_CHAN_INFO_SCALE:
@@ -472,7 +501,7 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 }
 
 static IIO_CONST_ATTR(in_proximity_sampling_frequency_available,
-				"1 3 5 10 13 20 83 100");
+				"1.25 2.5 5 10 13.3 20 80 100");
 static IIO_CONST_ATTR(in_illuminance_scale_available, "125 2000");
 
 #define ISL29028_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
@@ -571,7 +600,8 @@ static int isl29028_probe(struct i2c_client *client,
 	}
 
 	chip->enable_prox  = false;
-	chip->prox_sampling = 20;
+	chip->prox_sampling_int = 20;
+	chip->prox_sampling_frac = 0;
 	chip->lux_scale = 2000;
 
 	ret = regmap_write(chip->regmap, ISL29028_REG_TEST1_MODE, 0x0);
-- 
2.9.3

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

#1631156 — Re: [PATCH v3 1/2] staging: iio: isl29028: correct proximity sleep times

FromJonathan Cameron <jic23@kernel.org>
Date2017-04-26 07:40 +0200
SubjectRe: [PATCH v3 1/2] staging: iio: isl29028: correct proximity sleep times
Message-ID<tAnQe-6oE-9@gated-at.bofh.it>
In reply to#1630146
On 25/04/17 02:34, Brian Masney wrote:
> The sysfs attribute in_proximity_sampling_frequency_available currently
> shows the values 1 3 5 10 13 20 83 100. These values are supposed to
> correspond to the sleep values 800 400 200 100 75 50 12.5 0 (all in ms).
> When passing in a sampling frequency of 3, it actually uses a sleep
> time of 200ms instead of the expected 400ms value. This patch changes
> the value shown by this sysfs attribute to use fixed-point numbers so
> that the correct sampling frequency is shown to the user. This patch
> also changes the code that updates the proximity sampling frequency to
> only allow values that are shown in the _available sysfs attribute.
> 
> The original code showed the value 83 that corresponds to the sleep
> time 12 ms. The data sheet actually lists 12.5 ms as the sleep time,
> so the proximity frequency was updated to 80.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/isl29028.c | 70 +++++++++++++++++++++++++-----------
>  1 file changed, 50 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/isl29028.c b/drivers/staging/iio/light/isl29028.c
> index 5375e7a..aeb5082 100644
> --- a/drivers/staging/iio/light/isl29028.c
> +++ b/drivers/staging/iio/light/isl29028.c
> @@ -64,8 +64,25 @@
>  
>  #define ISL29028_POWER_OFF_DELAY_MS		2000
>  
> -static const unsigned int isl29028_prox_sleep_time[] = {800, 400, 200, 100, 75,
> -							50, 12, 0};
> +struct isl29028_prox_data {
> +	int sampling_int;
> +	int sampling_fract;
> +	int sleep_time;
> +};
> +
> +static const struct isl29028_prox_data isl29028_prox_data[] = {
> +	{   1, 250000, 800 },
> +	{   2, 500000, 400 },
> +	{   5,      0, 200 },
> +	{  10,      0, 100 },
> +	{  13, 300000,  75 },
> +	{  20,      0,  50 },
> +	{  80,      0,  13 }, /*
> +			       * Note: Data sheet lists 12.5 ms sleep time.
> +			       * Round up a half millisecond for msleep().
> +			       */
> +	{ 100,  0,   0 }
> +};
>  
>  enum isl29028_als_ir_mode {
>  	ISL29028_MODE_NONE = 0,
> @@ -76,32 +93,37 @@ enum isl29028_als_ir_mode {
>  struct isl29028_chip {
>  	struct mutex			lock;
>  	struct regmap			*regmap;
> -	unsigned int			prox_sampling;
> +	int				prox_sampling_int;
> +	int				prox_sampling_frac;
>  	bool				enable_prox;
>  	int				lux_scale;
>  	enum isl29028_als_ir_mode	als_ir_mode;
>  };
>  
> -static int isl29028_find_prox_sleep_time_index(int sampling)
> +static int isl29028_find_prox_sleep_index(int sampling_int, int sampling_fract)
>  {
> -	unsigned int period = DIV_ROUND_UP(1000, sampling);
>  	int i;
>  
> -	for (i = 0; i < ARRAY_SIZE(isl29028_prox_sleep_time); ++i) {
> -		if (period >= isl29028_prox_sleep_time[i])
> -			break;
> +	for (i = 0; i < ARRAY_SIZE(isl29028_prox_data); ++i) {
> +		if (isl29028_prox_data[i].sampling_int == sampling_int &&
> +		    isl29028_prox_data[i].sampling_fract == sampling_fract)
> +			return i;
>  	}
>  
> -	return i;
> +	return -EINVAL;
>  }
>  
>  static int isl29028_set_proxim_sampling(struct isl29028_chip *chip,
> -					unsigned int sampling)
> +					int sampling_int, int sampling_fract)
>  {
>  	struct device *dev = regmap_get_device(chip->regmap);
>  	int sleep_index, ret;
>  
> -	sleep_index = isl29028_find_prox_sleep_time_index(sampling);
> +	sleep_index = isl29028_find_prox_sleep_index(sampling_int,
> +						     sampling_fract);
> +	if (sleep_index < 0)
> +		return sleep_index;
> +
>  	ret = regmap_update_bits(chip->regmap, ISL29028_REG_CONFIGURE,
>  				 ISL29028_CONF_PROX_SLP_MASK,
>  				 sleep_index << ISL29028_CONF_PROX_SLP_SH);
> @@ -112,16 +134,18 @@ static int isl29028_set_proxim_sampling(struct isl29028_chip *chip,
>  		return ret;
>  	}
>  
> -	chip->prox_sampling = sampling;
> +	chip->prox_sampling_int = sampling_int;
> +	chip->prox_sampling_frac = sampling_fract;
>  
>  	return ret;
>  }
>  
>  static int isl29028_enable_proximity(struct isl29028_chip *chip)
>  {
> -	int sleep_index, ret;
> +	int prox_index, ret;
>  
> -	ret = isl29028_set_proxim_sampling(chip, chip->prox_sampling);
> +	ret = isl29028_set_proxim_sampling(chip, chip->prox_sampling_int,
> +					   chip->prox_sampling_frac);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -132,8 +156,12 @@ static int isl29028_enable_proximity(struct isl29028_chip *chip)
>  		return ret;
>  
>  	/* Wait for conversion to be complete for first sample */
> -	sleep_index = isl29028_find_prox_sleep_time_index(chip->prox_sampling);
> -	msleep(isl29028_prox_sleep_time[sleep_index]);
> +	prox_index = isl29028_find_prox_sleep_index(chip->prox_sampling_int,
> +						    chip->prox_sampling_frac);
> +	if (prox_index < 0)
> +		return prox_index;
> +
> +	msleep(isl29028_prox_data[prox_index].sleep_time);
>  
>  	return 0;
>  }
> @@ -361,7 +389,7 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
>  			break;
>  		}
>  
> -		ret = isl29028_set_proxim_sampling(chip, val);
> +		ret = isl29028_set_proxim_sampling(chip, val, val2);
>  		break;
>  	case IIO_LIGHT:
>  		if (mask != IIO_CHAN_INFO_SCALE) {
> @@ -439,7 +467,8 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
>  		if (chan->type != IIO_PROXIMITY)
>  			break;
>  
> -		*val = chip->prox_sampling;
> +		*val = chip->prox_sampling_int;
> +		*val2 = chip->prox_sampling_frac;
>  		ret = IIO_VAL_INT;
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
> @@ -472,7 +501,7 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
>  }
>  
>  static IIO_CONST_ATTR(in_proximity_sampling_frequency_available,
> -				"1 3 5 10 13 20 83 100");
> +				"1.25 2.5 5 10 13.3 20 80 100");
>  static IIO_CONST_ATTR(in_illuminance_scale_available, "125 2000");
>  
>  #define ISL29028_CONST_ATTR(name) (&iio_const_attr_##name.dev_attr.attr)
> @@ -571,7 +600,8 @@ static int isl29028_probe(struct i2c_client *client,
>  	}
>  
>  	chip->enable_prox  = false;
> -	chip->prox_sampling = 20;
> +	chip->prox_sampling_int = 20;
> +	chip->prox_sampling_frac = 0;
>  	chip->lux_scale = 2000;
>  
>  	ret = regmap_write(chip->regmap, ISL29028_REG_TEST1_MODE, 0x0);
>

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

#1631152

FromJonathan Cameron <jic23@kernel.org>
Date2017-04-26 07:30 +0200
Message-ID<tAnGx-6lD-3@gated-at.bofh.it>
In reply to#1630144
On 25/04/17 02:34, Brian Masney wrote:
> Minor cleanup to the proximity sampling to move this driver out of
> staging.
> 
> Datasheet:
> http://www.intersil.com/content/dam/Intersil/documents/isl2/isl29028.pdf
> 
> Changes since v2:
> - Reject any value that is not in the
>   in_proximity_sampling_frequency_available sysfs attribute.
> - Changed the sampling frequency 83.3 to 80. The data sheet lists 12.5
>   ms, but the original code listed 12 ms, which was the cause of the
>   discrepancy.
> Changes since v1:
> - in_proximity_sampling_frequency_available sysfs attribute now shows
>   decimals.
> 
> Jonathan: Sorry it took me a little over two months to get back to you
> on this driver. I got side tracked with other projects.
*laughs* I have series that I haven't gotten back to reviews on in
years!  What's a few months ;)

Thanks though as this work is good stuff!

Jonathan
> 
> Brian Masney (2):
>   staging: iio: isl29028: correct proximity sleep times
>   staging: iio: isl29028: move out of staging
> 
>  drivers/iio/light/Kconfig            |  10 +
>  drivers/iio/light/Makefile           |   1 +
>  drivers/iio/light/isl29028.c         | 723 +++++++++++++++++++++++++++++++++++
>  drivers/staging/iio/light/Kconfig    |  10 -
>  drivers/staging/iio/light/Makefile   |   1 -
>  drivers/staging/iio/light/isl29028.c | 693 ---------------------------------
>  6 files changed, 734 insertions(+), 704 deletions(-)
>  create mode 100644 drivers/iio/light/isl29028.c
>  delete mode 100644 drivers/staging/iio/light/isl29028.c
>

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

#1631159 — Re: [PATCH v3 2/2] staging: iio: isl29028: move out of staging

FromJoe Perches <joe@perches.com>
Date2017-04-26 07:50 +0200
SubjectRe: [PATCH v3 2/2] staging: iio: isl29028: move out of staging
Message-ID<tAnZU-6rV-7@gated-at.bofh.it>
In reply to#1630144
On Wed, 2017-04-26 at 06:30 +0100, Jonathan Cameron wrote:
> On 25/04/17 02:34, Brian Masney wrote:
> > Move ISL29028 ALS / Proximity Sensor out of staging and into mainline.
> > 
> > Signed-off-by: Brian Masney <masneyb@onstation.org>

Hey Brian.

Next time it's better to use git format-patch -M
to reduce the patch size and show renames.

> Applied to the togreg branch of iio.git.
> 
> Thanks,
> 
> Jonathan
> > ---
> >  drivers/iio/light/Kconfig            |  10 +
> >  drivers/iio/light/Makefile           |   1 +
> >  drivers/iio/light/isl29028.c         | 723 +++++++++++++++++++++++++++++++++++
> >  drivers/staging/iio/light/Kconfig    |  10 -
> >  drivers/staging/iio/light/Makefile   |   1 -
> >  drivers/staging/iio/light/isl29028.c | 723 -----------------------------------
> >  6 files changed, 734 insertions(+), 734 deletions(-)
> >  create mode 100644 drivers/iio/light/isl29028.c
> >  delete mode 100644 drivers/staging/iio/light/isl29028.c

[quoted the entire patch]

And Jonathan, please trim your replies a bit more.

Thanks.

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

#1631167 — Re: [PATCH v3 2/2] staging: iio: isl29028: move out of staging

FromJonathan Cameron <jic23@kernel.org>
Date2017-04-26 08:10 +0200
SubjectRe: [PATCH v3 2/2] staging: iio: isl29028: move out of staging
Message-ID<tAojf-6NY-9@gated-at.bofh.it>
In reply to#1631159
On 26/04/17 06:49, Joe Perches wrote:
> On Wed, 2017-04-26 at 06:30 +0100, Jonathan Cameron wrote:
>> On 25/04/17 02:34, Brian Masney wrote:
>>> Move ISL29028 ALS / Proximity Sensor out of staging and into mainline.
>>>
>>> Signed-off-by: Brian Masney <masneyb@onstation.org>
> 
> Hey Brian.
> 
> Next time it's better to use git format-patch -M
> to reduce the patch size and show renames.
My fault :)  For patches doing a move of an IIO driver
out of staging I've asked that people don't
use -M to encourage people to do a 'big picture' review
before we move them out of staging. 

This particular one went pretty smoothly, but in some other
staging move patches it's been a different story!

As Brian has done quite a few of these he is getting pretty
good at it ;)

Jonathan
> 
>> Applied to the togreg branch of iio.git
>>
>> Thanks,
>>
>> Jonathan
>>> ---
>>>  drivers/iio/light/Kconfig            |  10 +
>>>  drivers/iio/light/Makefile           |   1 +
>>>  drivers/iio/light/isl29028.c         | 723 +++++++++++++++++++++++++++++++++++
>>>  drivers/staging/iio/light/Kconfig    |  10 -
>>>  drivers/staging/iio/light/Makefile   |   1 -
>>>  drivers/staging/iio/light/isl29028.c | 723 -----------------------------------
>>>  6 files changed, 734 insertions(+), 734 deletions(-)
>>>  create mode 100644 drivers/iio/light/isl29028.c
>>>  delete mode 100644 drivers/staging/iio/light/isl29028.c
> 
> [quoted the entire patch]
> 
> And Jonathan, please trim your replies a bit more.
> 
> Thanks.
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web