Re: [PATCH] staging: lustre: Change return type to vm_fault_t

[Matthew Wilcox]

On Sun, Apr 22, 2018 at 03:47:24AM +0530, Souptick Joarder wrote:
> @@ -261,7 +261,7 @@ static inline int to_fault_error(int result)
>   * \retval VM_FAULT_ERROR on general error
>   * \retval NOPAGE_OOM not have memory for allocate new page
>   */
> -static int ll_fault0(struct vm_area_struct *vma, struct vm_fault *vmf)
> +static vm_fault_t ll_fault0(struct vm_area_struct *vma, struct vm_fault *vmf)
>  {
>   struct lu_env  *env;
>   struct cl_io*io;

Did you compile-test this with the sparse changes?  Because I can see
a problem here:

env = cl_env_get();
if (IS_ERR(env))
return PTR_ERR(env);

> @@ -269,7 +269,7 @@ static int ll_fault0(struct vm_area_struct *vma, struct 
> vm_fault *vmf)
>   struct page  *vmpage;
>   unsigned long   ra_flags;
>   int   result = 0;
> - int   fault_ret = 0;
> + vm_fault_t  fault_ret = 0;

What odd indentation ...

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: lustre: Change return type to vm_fault_t

[Souptick Joarder]

Use new return type vm_fault_t for fault handler. For
now, this is just documenting that the function returns
a VM_FAULT value rather than an errno. Once all instances
are converted, vm_fault_t will become a distinct type.

Commit 1c8f422059ae ("mm: change return type to vm_fault_t")

Signed-off-by: Souptick Joarder 
---
 drivers/staging/lustre/lustre/llite/llite_mmap.c | 29 
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_mmap.c 
b/drivers/staging/lustre/lustre/llite/llite_mmap.c
index c0533bd..6aafe7c 100644
--- a/drivers/staging/lustre/lustre/llite/llite_mmap.c
+++ b/drivers/staging/lustre/lustre/llite/llite_mmap.c
@@ -231,7 +231,7 @@ static int ll_page_mkwrite0(struct vm_area_struct *vma, 
struct page *vmpage,
return result;
 }
 
-static inline int to_fault_error(int result)
+static inline vm_fault_t to_fault_error(int result)
 {
switch (result) {
case 0:
@@ -261,7 +261,7 @@ static inline int to_fault_error(int result)
  * \retval VM_FAULT_ERROR on general error
  * \retval NOPAGE_OOM not have memory for allocate new page
  */
-static int ll_fault0(struct vm_area_struct *vma, struct vm_fault *vmf)
+static vm_fault_t ll_fault0(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
struct lu_env  *env;
struct cl_io*io;
@@ -269,7 +269,7 @@ static int ll_fault0(struct vm_area_struct *vma, struct 
vm_fault *vmf)
struct page  *vmpage;
unsigned long   ra_flags;
int   result = 0;
-   int   fault_ret = 0;
+   vm_fault_t  fault_ret = 0;
u16 refcheck;
 
env = cl_env_get();
@@ -278,7 +278,7 @@ static int ll_fault0(struct vm_area_struct *vma, struct 
vm_fault *vmf)
 
io = ll_fault_io_init(env, vma, vmf->pgoff, _flags);
if (IS_ERR(io)) {
-   result = to_fault_error(PTR_ERR(io));
+   fault_ret = to_fault_error(PTR_ERR(io));
goto out;
}
 
@@ -319,7 +319,7 @@ static int ll_fault0(struct vm_area_struct *vma, struct 
vm_fault *vmf)
if (result != 0 && !(fault_ret & VM_FAULT_RETRY))
fault_ret |= to_fault_error(result);
 
-   CDEBUG(D_MMAP, "%s fault %d/%d\n", current->comm, fault_ret, result);
+   CDEBUG(D_MMAP, "%s fault %x/%d\n", current->comm, fault_ret, result);
return fault_ret;
 }
 
@@ -327,7 +327,7 @@ static int ll_fault(struct vm_fault *vmf)
 {
int count = 0;
bool printed = false;
-   int result;
+   vm_fault_t result;
sigset_t set;
 
/* Only SIGKILL and SIGTERM are allowed for fault/nopage/mkwrite
@@ -370,12 +370,13 @@ static int ll_page_mkwrite(struct vm_fault *vmf)
int count = 0;
bool printed = false;
bool retry;
-   int result;
+   int err;
+   vm_fault_t ret;
 
file_update_time(vma->vm_file);
do {
retry = false;
-   result = ll_page_mkwrite0(vma, vmf->page, );
+   err = ll_page_mkwrite0(vma, vmf->page, );
 
if (!printed && ++count > 16) {
const struct dentry *de = vma->vm_file->f_path.dentry;
@@ -387,25 +388,25 @@ static int ll_page_mkwrite(struct vm_fault *vmf)
}
} while (retry);
 
-   switch (result) {
+   switch (err) {
case 0:
LASSERT(PageLocked(vmf->page));
-   result = VM_FAULT_LOCKED;
+   ret = VM_FAULT_LOCKED;
break;
case -ENODATA:
case -EAGAIN:
case -EFAULT:
-   result = VM_FAULT_NOPAGE;
+   ret = VM_FAULT_NOPAGE;
break;
case -ENOMEM:
-   result = VM_FAULT_OOM;
+   ret = VM_FAULT_OOM;
break;
default:
-   result = VM_FAULT_SIGBUS;
+   ret = VM_FAULT_SIGBUS;
break;
}
 
-   return result;
+   return ret;
 }
 
 /**
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] stagging:iio:meter: Add ade7854_write_raw function

[John Syne]

Attached is a spreadsheet which should help explain the calibration process.



ADE7878_calibration.xlsx
Description: MS-Excel 2007 spreadsheet



Here is the calibration guide:
http://www.analog.com/media/en/technical-documentation/application-notes/AN-1076.pdf



Regards,
John





> On Apr 21, 2018, at 10:26 AM, Jonathan Cameron  wrote:
> 
> On Sat, 21 Apr 2018 08:56:19 -0300
> Rodrigo Siqueira  wrote:
> 
>> This patch adds the ade7854_write_raw() function which is responsible
>> for handling the write operation for registers: AIGAIN, BIGAIN, CIGAIN,
>> NIGAIN, AVGAIN, BVGAIN, and CVGAIN. Finally, this patch completely
>> removes the old ABI used for handling the registers mentioned above.
>> 
>> Signed-off-by: Rodrigo Siqueira 
> The baby steps approach here is good, but with all 3 patches as one it
> would have been a lot easier to follow.
> 
> This is almost fine except for the issue I had with how the channels
> were defined in the first place.
> 
> Also a question about scales inline...
> 
>> ---
>> drivers/staging/iio/meter/ade7854.c | 60 -
>> 1 file changed, 25 insertions(+), 35 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/meter/ade7854.c 
>> b/drivers/staging/iio/meter/ade7854.c
>> index 242ecde75900..df19c8b4b5d7 100644
>> --- a/drivers/staging/iio/meter/ade7854.c
>> +++ b/drivers/staging/iio/meter/ade7854.c
>> @@ -228,34 +228,6 @@ static int ade7854_reset(struct device *dev)
>>  return st->write_reg(dev, ADE7854_CONFIG, val, 16);
>> }
>> 
>> -static IIO_DEV_ATTR_AIGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_AIGAIN);
>> -static IIO_DEV_ATTR_BIGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_BIGAIN);
>> -static IIO_DEV_ATTR_CIGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_CIGAIN);
>> -static IIO_DEV_ATTR_NIGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_NIGAIN);
>> -static IIO_DEV_ATTR_AVGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_AVGAIN);
>> -static IIO_DEV_ATTR_BVGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_BVGAIN);
>> -static IIO_DEV_ATTR_CVGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_CVGAIN);
>> static IIO_DEV_ATTR_APPARENT_POWER_A_GAIN(0644,
>>  ade7854_read_24bit,
>>  ade7854_write_24bit,
>> @@ -497,6 +469,30 @@ static int ade7854_read_raw(struct iio_dev *indio_dev,
>>  return -EINVAL;
>> }
>> 
>> +static int ade7854_write_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int val, int val2, long mask)
>> +{
>> +struct ade7854_state *st = iio_priv(indio_dev);
>> +int ret;
>> +
>> +if (mask != IIO_CHAN_INFO_SCALE)
>> +return -EINVAL;
>> +
>> +switch (chan->type) {
>> +case IIO_CURRENT:
>> +case IIO_VOLTAGE:
> Probably need some range checking to make sure we have a sane value.
> 
> Also, I'm curious about units here.
> 
> you are using CHAN_INFO_SCALE which would mean this is linked to
> the _RAW value (the actual reading of the channel) by _RAW*_SCALE = value in
> defined base units for the channel type.  So I'd expect to see some
> code here doing a conversion to the internal format for the device.
> 
> I wouldn't expect to see a value from userspace written unconverted
> into the register.
> 
> (I missed this in the previous patch - sorry).
> 
> Jonathan
> 
>> +ret = st->write_reg(_dev->dev, chan->address, val, 24);
>> +if (ret < 0)
>> +return ret;
>> +return 0;
>> +default:
>> +break;
>> +}
>> +
>> +return -EINVAL;
>> +}
>> +
>> static int ade7854_initial_setup(struct iio_dev *indio_dev)
>> {
>>  int ret;
>> @@ -521,13 +517,6 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("8000");
>> static IIO_CONST_ATTR(name, "ade7854");
>> 
>> static struct attribute *ade7854_attributes[] = {
>> -_dev_attr_aigain.dev_attr.attr,
>> -_dev_attr_bigain.dev_attr.attr,
>> -_dev_attr_cigain.dev_attr.attr,
>> -_dev_attr_nigain.dev_attr.attr,
>> -_dev_attr_avgain.dev_attr.attr,
>> -_dev_attr_bvgain.dev_attr.attr,
>> -_dev_attr_cvgain.dev_attr.attr,
>>  _dev_attr_linecyc.dev_attr.attr,
>>  _dev_attr_sagcyc.dev_attr.attr,
>>  _dev_attr_cfcyc.dev_attr.attr,
>> @@ -599,6 +588,7 @@ static const struct attribute_group 
>> ade7854_attribute_group = {
>> static const struct iio_info ade7854_info = {
>>  .attrs = _attribute_group,
>>  .read_raw = _read_raw,
>> +.write_raw = _write_raw,
>> };
>> 
>> int ade7854_probe(struct iio_dev *indio_dev, struct device *dev)

Re: [PATCH 3/3] stagging:iio:meter: Add ade7854_write_raw function

[John Syne]

> On Apr 21, 2018, at 10:26 AM, Jonathan Cameron  wrote:
> 
> On Sat, 21 Apr 2018 08:56:19 -0300
> Rodrigo Siqueira  wrote:
> 
>> This patch adds the ade7854_write_raw() function which is responsible
>> for handling the write operation for registers: AIGAIN, BIGAIN, CIGAIN,
>> NIGAIN, AVGAIN, BVGAIN, and CVGAIN. Finally, this patch completely
>> removes the old ABI used for handling the registers mentioned above.
>> 
>> Signed-off-by: Rodrigo Siqueira 
> The baby steps approach here is good, but with all 3 patches as one it
> would have been a lot easier to follow.
> 
> This is almost fine except for the issue I had with how the channels
> were defined in the first place.
> 
> Also a question about scales inline...
> 
>> ---
>> drivers/staging/iio/meter/ade7854.c | 60 -
>> 1 file changed, 25 insertions(+), 35 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/meter/ade7854.c 
>> b/drivers/staging/iio/meter/ade7854.c
>> index 242ecde75900..df19c8b4b5d7 100644
>> --- a/drivers/staging/iio/meter/ade7854.c
>> +++ b/drivers/staging/iio/meter/ade7854.c
>> @@ -228,34 +228,6 @@ static int ade7854_reset(struct device *dev)
>>  return st->write_reg(dev, ADE7854_CONFIG, val, 16);
>> }
>> 
>> -static IIO_DEV_ATTR_AIGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_AIGAIN);
>> -static IIO_DEV_ATTR_BIGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_BIGAIN);
>> -static IIO_DEV_ATTR_CIGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_CIGAIN);
>> -static IIO_DEV_ATTR_NIGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_NIGAIN);
>> -static IIO_DEV_ATTR_AVGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_AVGAIN);
>> -static IIO_DEV_ATTR_BVGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_BVGAIN);
>> -static IIO_DEV_ATTR_CVGAIN(0644,
>> -NULL,
>> -ade7854_write_24bit,
>> -ADE7854_CVGAIN);
>> static IIO_DEV_ATTR_APPARENT_POWER_A_GAIN(0644,
>>  ade7854_read_24bit,
>>  ade7854_write_24bit,
>> @@ -497,6 +469,30 @@ static int ade7854_read_raw(struct iio_dev *indio_dev,
>>  return -EINVAL;
>> }
>> 
>> +static int ade7854_write_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int val, int val2, long mask)
>> +{
>> +struct ade7854_state *st = iio_priv(indio_dev);
>> +int ret;
>> +
>> +if (mask != IIO_CHAN_INFO_SCALE)
>> +return -EINVAL;
>> +
>> +switch (chan->type) {
>> +case IIO_CURRENT:
>> +case IIO_VOLTAGE:
> Probably need some range checking to make sure we have a sane value.
> 
> Also, I'm curious about units here.
> 
> you are using CHAN_INFO_SCALE which would mean this is linked to
> the _RAW value (the actual reading of the channel) by _RAW*_SCALE = value in
> defined base units for the channel type.  So I'd expect to see some
> code here doing a conversion to the internal format for the device.
These should be IIO_CHAN_INFO_HARDWAREGAIN as they are used to correct 
any imperfections of the sensors. A user space application calculates the error 
and
based on a formula determines the correct register value. 

Meter values such as Voltage, Current, Power, Energy use a value/LSB method, 
which is determined while applying a known voltage and current. After that, any
parameter is multiplied by the respective value/LSB to get a real world value. 

Regards,
John
> 
> I wouldn't expect to see a value from userspace written unconverted
> into the register.
> 
> (I missed this in the previous patch - sorry).
> 
> Jonathan
> 
>> +ret = st->write_reg(_dev->dev, chan->address, val, 24);
>> +if (ret < 0)
>> +return ret;
>> +return 0;
>> +default:
>> +break;
>> +}
>> +
>> +return -EINVAL;
>> +}
>> +
>> static int ade7854_initial_setup(struct iio_dev *indio_dev)
>> {
>>  int ret;
>> @@ -521,13 +517,6 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("8000");
>> static IIO_CONST_ATTR(name, "ade7854");
>> 
>> static struct attribute *ade7854_attributes[] = {
>> -_dev_attr_aigain.dev_attr.attr,
>> -_dev_attr_bigain.dev_attr.attr,
>> -_dev_attr_cigain.dev_attr.attr,
>> -_dev_attr_nigain.dev_attr.attr,
>> -_dev_attr_avgain.dev_attr.attr,
>> -_dev_attr_bvgain.dev_attr.attr,
>> -_dev_attr_cvgain.dev_attr.attr,
>>  _dev_attr_linecyc.dev_attr.attr,
>>  _dev_attr_sagcyc.dev_attr.attr,
>>  _dev_attr_cfcyc.dev_attr.attr,
>> @@ -599,6 +588,7 @@ static const struct attribute_group 
>> ade7854_attribute_group = {
>> static const struct iio_info ade7854_info = {
>>  .attrs = _attribute_group,
>> 

Re: [PATCH 3/3] stagging:iio:meter: Add ade7854_write_raw function

[Jonathan Cameron]

On Sat, 21 Apr 2018 08:56:19 -0300
Rodrigo Siqueira  wrote:

> This patch adds the ade7854_write_raw() function which is responsible
> for handling the write operation for registers: AIGAIN, BIGAIN, CIGAIN,
> NIGAIN, AVGAIN, BVGAIN, and CVGAIN. Finally, this patch completely
> removes the old ABI used for handling the registers mentioned above.
> 
> Signed-off-by: Rodrigo Siqueira 
The baby steps approach here is good, but with all 3 patches as one it
would have been a lot easier to follow.

This is almost fine except for the issue I had with how the channels
were defined in the first place.

Also a question about scales inline...

> ---
>  drivers/staging/iio/meter/ade7854.c | 60 -
>  1 file changed, 25 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854.c 
> b/drivers/staging/iio/meter/ade7854.c
> index 242ecde75900..df19c8b4b5d7 100644
> --- a/drivers/staging/iio/meter/ade7854.c
> +++ b/drivers/staging/iio/meter/ade7854.c
> @@ -228,34 +228,6 @@ static int ade7854_reset(struct device *dev)
>   return st->write_reg(dev, ADE7854_CONFIG, val, 16);
>  }
>  
> -static IIO_DEV_ATTR_AIGAIN(0644,
> - NULL,
> - ade7854_write_24bit,
> - ADE7854_AIGAIN);
> -static IIO_DEV_ATTR_BIGAIN(0644,
> - NULL,
> - ade7854_write_24bit,
> - ADE7854_BIGAIN);
> -static IIO_DEV_ATTR_CIGAIN(0644,
> - NULL,
> - ade7854_write_24bit,
> - ADE7854_CIGAIN);
> -static IIO_DEV_ATTR_NIGAIN(0644,
> - NULL,
> - ade7854_write_24bit,
> - ADE7854_NIGAIN);
> -static IIO_DEV_ATTR_AVGAIN(0644,
> - NULL,
> - ade7854_write_24bit,
> - ADE7854_AVGAIN);
> -static IIO_DEV_ATTR_BVGAIN(0644,
> - NULL,
> - ade7854_write_24bit,
> - ADE7854_BVGAIN);
> -static IIO_DEV_ATTR_CVGAIN(0644,
> - NULL,
> - ade7854_write_24bit,
> - ADE7854_CVGAIN);
>  static IIO_DEV_ATTR_APPARENT_POWER_A_GAIN(0644,
>   ade7854_read_24bit,
>   ade7854_write_24bit,
> @@ -497,6 +469,30 @@ static int ade7854_read_raw(struct iio_dev *indio_dev,
>   return -EINVAL;
>  }
>  
> +static int ade7854_write_raw(struct iio_dev *indio_dev,
> +  struct iio_chan_spec const *chan,
> +  int val, int val2, long mask)
> +{
> + struct ade7854_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + if (mask != IIO_CHAN_INFO_SCALE)
> + return -EINVAL;
> +
> + switch (chan->type) {
> + case IIO_CURRENT:
> + case IIO_VOLTAGE:
Probably need some range checking to make sure we have a sane value.

Also, I'm curious about units here.

you are using CHAN_INFO_SCALE which would mean this is linked to
the _RAW value (the actual reading of the channel) by _RAW*_SCALE = value in
defined base units for the channel type.  So I'd expect to see some
code here doing a conversion to the internal format for the device.

I wouldn't expect to see a value from userspace written unconverted
into the register.

(I missed this in the previous patch - sorry).

Jonathan

> + ret = st->write_reg(_dev->dev, chan->address, val, 24);
> + if (ret < 0)
> + return ret;
> + return 0;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
>  static int ade7854_initial_setup(struct iio_dev *indio_dev)
>  {
>   int ret;
> @@ -521,13 +517,6 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("8000");
>  static IIO_CONST_ATTR(name, "ade7854");
>  
>  static struct attribute *ade7854_attributes[] = {
> - _dev_attr_aigain.dev_attr.attr,
> - _dev_attr_bigain.dev_attr.attr,
> - _dev_attr_cigain.dev_attr.attr,
> - _dev_attr_nigain.dev_attr.attr,
> - _dev_attr_avgain.dev_attr.attr,
> - _dev_attr_bvgain.dev_attr.attr,
> - _dev_attr_cvgain.dev_attr.attr,
>   _dev_attr_linecyc.dev_attr.attr,
>   _dev_attr_sagcyc.dev_attr.attr,
>   _dev_attr_cfcyc.dev_attr.attr,
> @@ -599,6 +588,7 @@ static const struct attribute_group 
> ade7854_attribute_group = {
>  static const struct iio_info ade7854_info = {
>   .attrs = _attribute_group,
>   .read_raw = _read_raw,
> + .write_raw = _write_raw,
>  };
>  
>  int ade7854_probe(struct iio_dev *indio_dev, struct device *dev)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] stagging:iio:meter: Add ade7854_read_raw function

[Jonathan Cameron]

On Sat, 21 Apr 2018 08:55:52 -0300
Rodrigo Siqueira  wrote:

> This patch adds the ade7854_read_raw() function which is responsible for
> handling the read operation for registers: AIGAIN, BIGAIN, CIGAIN,
> NIGAIN, AVGAIN, BVGAIN, and CVGAIN. For the sake of simplicity, this
> patch only adds basic manipulation for current and voltage channels.
> Finally, this patch disables the old approach for reading data.
> 
> Signed-off-by: Rodrigo Siqueira 

Other than the previous mentioned issue with the way the channels are
defined, this is nearly fine.

See inline.

> ---
>  drivers/staging/iio/meter/ade7854.c | 41 -
>  1 file changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854.c 
> b/drivers/staging/iio/meter/ade7854.c
> index 2fbb2570ba54..242ecde75900 100644
> --- a/drivers/staging/iio/meter/ade7854.c
> +++ b/drivers/staging/iio/meter/ade7854.c
> @@ -229,31 +229,31 @@ static int ade7854_reset(struct device *dev)
>  }
>  
>  static IIO_DEV_ATTR_AIGAIN(0644,
> - ade7854_read_24bit,
> + NULL,
>   ade7854_write_24bit,
>   ADE7854_AIGAIN);
>  static IIO_DEV_ATTR_BIGAIN(0644,
> - ade7854_read_24bit,
> + NULL,
>   ade7854_write_24bit,
>   ADE7854_BIGAIN);
>  static IIO_DEV_ATTR_CIGAIN(0644,
> - ade7854_read_24bit,
> + NULL,
>   ade7854_write_24bit,
>   ADE7854_CIGAIN);
>  static IIO_DEV_ATTR_NIGAIN(0644,
> - ade7854_read_24bit,
> + NULL,
>   ade7854_write_24bit,
>   ADE7854_NIGAIN);
>  static IIO_DEV_ATTR_AVGAIN(0644,
> - ade7854_read_24bit,
> + NULL,
>   ade7854_write_24bit,
>   ADE7854_AVGAIN);
>  static IIO_DEV_ATTR_BVGAIN(0644,
> - ade7854_read_24bit,
> + NULL,
>   ade7854_write_24bit,
>   ADE7854_BVGAIN);
>  static IIO_DEV_ATTR_CVGAIN(0644,
> - ade7854_read_24bit,
> + NULL,
>   ade7854_write_24bit,
>   ADE7854_CVGAIN);
>  static IIO_DEV_ATTR_APPARENT_POWER_A_GAIN(0644,
> @@ -471,6 +471,32 @@ static int ade7854_set_irq(struct device *dev, bool 
> enable)
>   return st->write_reg(dev, ADE7854_MASK0, irqen, 32);
>  }
>  
> +static int ade7854_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val,
> + int *val2,
> + long mask)
> +{
> + struct ade7854_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + if (mask != IIO_CHAN_INFO_SCALE)
> + return -EINVAL;
Given you specified sampling frequency for the channels in the previous
patch this is a little odd!

> +
> + switch (chan->type) {
> + case IIO_CURRENT:
> + case IIO_VOLTAGE:
> + ret = st->read_reg(_dev->dev, chan->address, val, 24);
> + if (ret < 0)
> + return ret;
> + return IIO_VAL_INT;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
>  static int ade7854_initial_setup(struct iio_dev *indio_dev)
>  {
>   int ret;
> @@ -572,6 +598,7 @@ static const struct attribute_group 
> ade7854_attribute_group = {
>  
>  static const struct iio_info ade7854_info = {
>   .attrs = _attribute_group,
> + .read_raw = _read_raw,
>  };
>  
>  int ade7854_probe(struct iio_dev *indio_dev, struct device *dev)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] stagging:iio:meter: Add iio_chan_spec

[Jonathan Cameron]

On Sat, 21 Apr 2018 08:55:08 -0300
Rodrigo Siqueira  wrote:

> This patch adds iio_chan_spec struct. Additionally, the channel adds the
> support for handling AIGAIN, BIGAIN, CIGAIN, NIGAIN, AVGAIN, BVGAIN, and
> CVGAIN.
> 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  drivers/staging/iio/meter/ade7854.c | 42 +
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/staging/iio/meter/ade7854.c 
> b/drivers/staging/iio/meter/ade7854.c
> index 029c3bf42d4d..2fbb2570ba54 100644
> --- a/drivers/staging/iio/meter/ade7854.c
> +++ b/drivers/staging/iio/meter/ade7854.c
> @@ -22,6 +22,48 @@
>  #include "meter.h"
>  #include "ade7854.h"
>  
> +#define PHASEA "phaseA"
> +#define PHASEB "phaseB"
> +#define PHASEC "phaseC"
> +#define NEUTRAL "neutral"
> +
> +#define ADE7854_CHANNEL(_type, _name, _mask, _reg) { \
> + .type = _type,  \
> + .indexed = 1,   \
> + .channel = 0,   \
> + .extend_name = _name,   \
> + .info_mask_separate = _mask,\
> + .address = _reg,\
> + .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
This is odd. It defines the sampling frequency.  whilst providing now
such read_raw functionality.  So you will get a sampling frequency attribute
that doesn't work.
> + .scan_index = -1,   \
There is no buffered support so this isn't needed.

> +}
> +
> +static const struct iio_chan_spec ade7854_channels[] = {
> + /* Current */
> + ADE7854_CHANNEL(IIO_CURRENT, PHASEA,
> + BIT(IIO_CHAN_INFO_SCALE),
Each patch needs to stand on it's own (I might no apply them
all).  This adds the scale attributes, but the support for
them to actually work is in the next patch.

It isn't sensible to break this up into 3 patches and it actually
makes it harder to review (as they only make sense together).

Jonathan

> + ADE7854_AIGAIN),
> + ADE7854_CHANNEL(IIO_CURRENT, PHASEB,
> + BIT(IIO_CHAN_INFO_SCALE),
> + ADE7854_BIGAIN),
> + ADE7854_CHANNEL(IIO_CURRENT, PHASEC,
> + BIT(IIO_CHAN_INFO_SCALE),
> + ADE7854_CIGAIN),
> + ADE7854_CHANNEL(IIO_CURRENT, NEUTRAL,
> + BIT(IIO_CHAN_INFO_SCALE),
> + ADE7854_NIGAIN),
> + /* Voltage */
> + ADE7854_CHANNEL(IIO_VOLTAGE, PHASEA,
> + BIT(IIO_CHAN_INFO_SCALE),
> + ADE7854_AVGAIN),
> + ADE7854_CHANNEL(IIO_VOLTAGE, PHASEB,
> + BIT(IIO_CHAN_INFO_SCALE),
> + ADE7854_BVGAIN),
> + ADE7854_CHANNEL(IIO_VOLTAGE, PHASEC,
> + BIT(IIO_CHAN_INFO_SCALE),
> + ADE7854_CVGAIN),
> +};
> +
>  static ssize_t ade7854_read_8bit(struct device *dev,
>struct device_attribute *attr,
>char *buf)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/3] stagging:iio:meter: Add essential IIO API structures for ADE7854

[Jonathan Cameron]

On Sat, 21 Apr 2018 08:54:45 -0300
Rodrigo Siqueira  wrote:

> This patchset aims to update ADE7854 by adding the required IIO API
> components. The first patch adds the iio_chan_spec for handling seven
> different registers (all of them with a similar behavior). The second
> patch appends the read_raw function defined by the IIO API. Finally, the
> third patch adds the write_raw function and remove the attributes used
> for handling the seven registers. This patchset has the contribution of
> John Syne, which was responsible for mapping the correct ABI name per
> element in the ADE7854; additionally, John provided codes that helped to
> shape these patches.
> 
> Rodrigo Siqueira (3):
>   stagging:iio:meter: Add iio_chan_spec
>   stagging:iio:meter: Add ade7854_read_raw function
>   stagging:iio:meter: Add ade7854_write_raw function
> 
>  drivers/staging/iio/meter/ade7854.c | 129 
>  1 file changed, 94 insertions(+), 35 deletions(-)
> 
Hi Rodrigo,

Please don't do it like this.   The original discussion with John
involved the addition of an extra chunk of core logic so we would have
the ability to specify the channel without using extended_name.

Extended name is not intended to differentiate between channels (it
isn't in general enough to do so - though it works in this little
corner case of sysfs attributes like you have here).

So the plan is to add another field to the iio_chan_spec structure
to allow for the complexity we need.  See the long discussions
over how we represent the myriad of channels in here.

If you need some pointers, feel free to ask but it may take
me a little while to put together a full description of what needs
doing.

I'll put a few more direct comments in the individual patches.

Thanks,

Jonathan

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 13/13] staging: iio: ad2s1200: Move driver out of staging

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:32:01 +0200
David Veenstra  wrote:

> Move the iio driver for the ad2s1200 resolver-to-digital
> converter out of staging, into mainline iio subsystems.
> 
> Signed-off-by: David Veenstra 
I'll look at this once the minor issues with the other patches
are cleared up (so in v3!)

Thanks for your hard work on this. It seems to be heading rapidly
in the right direction.

Jonathan

> ---
> Changes in v2:
>   - Added commit message.
>   - Also move device tree binding documentation out of staging.
>   - Disabled move detection.
> 
>  .../devicetree/bindings/iio/resolver/ad2s1200.txt  |  16 ++
>  .../bindings/staging/iio/resolver/ad2s1200.txt |  16 --
>  drivers/iio/Kconfig|   1 +
>  drivers/iio/Makefile   |   1 +
>  drivers/iio/resolver/Kconfig   |  17 ++
>  drivers/iio/resolver/Makefile  |   5 +
>  drivers/iio/resolver/ad2s1200.c| 250 
> +
>  drivers/staging/iio/resolver/Kconfig   |  12 -
>  drivers/staging/iio/resolver/Makefile  |   1 -
>  drivers/staging/iio/resolver/ad2s1200.c| 250 
> -
>  10 files changed, 290 insertions(+), 279 deletions(-)
>  create mode 100644 
> Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
>  delete mode 100644 
> Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
>  create mode 100644 drivers/iio/resolver/Kconfig
>  create mode 100644 drivers/iio/resolver/Makefile
>  create mode 100644 drivers/iio/resolver/ad2s1200.c
>  delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt 
> b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
> new file mode 100644
> index ..85c009987878
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
> @@ -0,0 +1,16 @@
> +Analog Devices AD2S1200 Resolver-to-Digital Converter
> +
> +Required properties:
> + - compatible : should be "adi,ad2s1200"
> + - reg : the SPI chip select number of the device
> + - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
> + - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
> +
> +Example:
> +
> + resolver {
> + compatible = "adi,ad2s1200";
> + reg = <4>;
> + sample-gpios = < 5 GPIO_ACTIVE_HIGH>;
> + rdvel-gpios = < 6 GPIO_ACTIVE_HIGH>;
> + };
> diff --git 
> a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt 
> b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> deleted file mode 100644
> index 85c009987878..
> --- a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -Analog Devices AD2S1200 Resolver-to-Digital Converter
> -
> -Required properties:
> - - compatible : should be "adi,ad2s1200"
> - - reg : the SPI chip select number of the device
> - - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
> - - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
> -
> -Example:
> -
> - resolver {
> - compatible = "adi,ad2s1200";
> - reg = <4>;
> - sample-gpios = < 5 GPIO_ACTIVE_HIGH>;
> - rdvel-gpios = < 6 GPIO_ACTIVE_HIGH>;
> - };
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index b3c8c6ef0dff..4bec3ccbf4a1 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -92,6 +92,7 @@ source "drivers/iio/potentiometer/Kconfig"
>  source "drivers/iio/potentiostat/Kconfig"
>  source "drivers/iio/pressure/Kconfig"
>  source "drivers/iio/proximity/Kconfig"
> +source "drivers/iio/resolver/Kconfig"
>  source "drivers/iio/temperature/Kconfig"
>  
>  endif # IIO
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index b16b2e9ddc40..1865361b8714 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -35,5 +35,6 @@ obj-y += potentiometer/
>  obj-y += potentiostat/
>  obj-y += pressure/
>  obj-y += proximity/
> +obj-y += resolver/
>  obj-y += temperature/
>  obj-y += trigger/
> diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
> new file mode 100644
> index ..2ced9f22aa70
> --- /dev/null
> +++ b/drivers/iio/resolver/Kconfig
> @@ -0,0 +1,17 @@
> +#
> +# Resolver/Synchro drivers
> +#
> +menu "Resolver to digital converters"
> +
> +config AD2S1200
> + tristate "Analog Devices ad2s1200/ad2s1205 driver"
> + depends on SPI
> + depends on GPIOLIB || COMPILE_TEST
> + help
> +   Say yes here to build support for Analog Devices spi resolver
> +   to digital converters, ad2s1200 and ad2s1205, provides direct access
> +   via sysfs.
> +
> +   To compile this driver as a module, choose M here: the
> +  

Re: [PATCH v2 12/13] staging: iio: ad2s1200: Add scaling factor for angle channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:31:48 +0200
David Veenstra  wrote:

> A fractional scaling factor of approximately 2 * Pi / (2^12 -1) is added,
> to scale the 12-bits angular position to radians.
> 
> Signed-off-by: David Veenstra 
> ---
> Changes in v2:
>   - This patch replaces the patch that changed the
> the channel for angular position to inclination
> channel.
> 
>  drivers/staging/iio/resolver/ad2s1200.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index 6c56257be3b1..4f5dd28b174a 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -71,6 +71,17 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>   switch (m) {
>   case IIO_CHAN_INFO_SCALE:
>   switch (chan->type) {
> + case IIO_ANGL:
> + /*
> +  * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
> +  *
> +  * Since there only fit 3 whole radians in 360 degrees,
> +  * usage of iio_convert_raw_to_processed for this
> +  * channel will be highly inaccurate.
> +  */
> + *val = 103993;
> + *val2 = 16551 * 0xFFF;
> + return IIO_VAL_FRACTIONAL;
Again, use IIO_VAL_INT_PLUS_MICRO and specify it as a decimal.  There is
no need to jump through these hoops when we are unlikely to have any users
that need to keep to integer only maths.

Thanks,

Jonathan

>   case IIO_ANGL_VEL:
>   /*
>* 2 * Pi ~= 2 * 103993 / 33102
> @@ -133,6 +144,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
>   .indexed = 1,
>   .channel = 0,
>   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>   }, {
>   .type = IIO_ANGL_VEL,
>   .indexed = 1,

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 11/13] staging: iio: Documentation: Add missing sysfs docs for angle channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:31:37 +0200
David Veenstra  wrote:

> The iio resolver drivers in staging use angle channels. This patch
> add missing documentation for this type of channel.
> 
> As was discussed in [1], radians is chosen as the unit, to match the
> unit of angular velocity.
> 
> [1] https://marc.info/?l=linux-driver-devel=152190078308330=2
> 
> Signed-off-by: David Veenstra 
> ---
> Change in v2:
>   - Introduces in this version.
> 
>  Documentation/ABI/testing/sysfs-bus-iio | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio 
> b/Documentation/ABI/testing/sysfs-bus-iio
> index 6a5f34b4d5b9..8ad0e55f99ee 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -190,6 +190,15 @@ Description:
>   but should match other such assignments on device).
>   Units after application of scale and offset are m/s^2.
>  
> +What:/sys/bus/iio/devices/iio:deviceX/in_angl_x_raw
> +What:/sys/bus/iio/devices/iio:deviceX/in_angl_y_raw
> +What:/sys/bus/iio/devices/iio:deviceX/in_angl_z_raw
This surprised me.  A resolver is not going to inherently have any
notion of a particular axis.
Would expect.
in_angl_raw

Jonathan

> +KernelVersion:   4.17
> +Contact: linux-...@vger.kernel.org
> +Description:
> + Angle about axis x, y or z (may be arbitrarily assigned). Units
> + after application of scale and offset are radians.
> +
>  What:/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
>  What:/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
>  What:/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
> @@ -297,6 +306,7 @@ What: 
> /sys/bus/iio/devices/iio:deviceX/in_pressure_offset
>  What:
> /sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
>  What:/sys/bus/iio/devices/iio:deviceX/in_magn_offset
>  What:/sys/bus/iio/devices/iio:deviceX/in_rot_offset
> +What:/sys/bus/iio/devices/iio:deviceX/in_angl_offset
>  KernelVersion:   2.6.35
>  Contact: linux-...@vger.kernel.org
>  Description:
> @@ -350,6 +360,7 @@ What: 
> /sys/bus/iio/devices/iio:deviceX/in_humidityrelative_scale
>  What:
> /sys/bus/iio/devices/iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_scale
>  What:/sys/bus/iio/devices/iio:deviceX/in_illuminance_scale
>  What:/sys/bus/iio/devices/iio:deviceX/in_countY_scale
> +What:/sys/bus/iio/devices/iio:deviceX/in_angl_scale
>  KernelVersion:   2.6.35
>  Contact: linux-...@vger.kernel.org
>  Description:

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 10/13] staging: iio: ad2s1200: Add scaling factor for angular velocity channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:31:09 +0200
David Veenstra  wrote:

> The sysfs iio ABI states radians per second is expected as the unit for
> angular velocity, but the 12-bit angular velocity register has rps
> as its unit. So a fractional scaling factor of approximately 2 * Pi is
> added to the angular velocity channel.
> 
> The added comments will also be relevant for the scaling factor of
> the angle channel.
> 
> Signed-off-by: David Veenstra 
Comment inline.  The function you are talking about isn't used
in the majority of likely use cases for this part.  The maths will actually
be done in userspace (which can use floating point).

Thanks,

Jonathan

> ---
> Changes in v2:
>   - Move explanation of Pi approximation to top of switch statement,
> as this will also be relevant to angle channel.
>   - Replaced 33102 / 2 with 16551 on line 84.
> 
>  drivers/staging/iio/resolver/ad2s1200.c | 84 
> +++--
>  1 file changed, 59 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index 29a9bb666e7b..6c56257be3b1 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -60,38 +60,71 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>   int ret = 0;
>   u16 vel;
>  
> - mutex_lock(>lock);
> - gpiod_set_value(st->sample, 0);
> + /*
> +  * Below a fractional approximation of Pi is needed.
> +  * The following approximation will be used: 103993 / 33102.
> +  * This is accurate in 9 decimals places.
> +  *
> +  * This fraction is based on OEIS series of nominator/denominator
> +  * of convergents to Pi (A002485 and A002486).
> +  */
> + switch (m) {
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_ANGL_VEL:
> + /*
> +  * 2 * Pi ~= 2 * 103993 / 33102
> +  *
> +  * iio_convert_raw_to_processed uses integer
> +  * division. This will cause at most 5% error
> +  * (for very small values). But for 99.5% of the values
> +  * it will cause less that 1% error.
This is an interesting comment, but relies on anyone actually
using iio_convert_raw_to_processed with this device.

I would imagine that 'in kernel' users of a resolver (who would use
that function) will be few and far between.  Mostly this will just
get passed to userspace.  That involves this being converted to
a decimal.  I would just specify it as one in the first place.

> +  */
> + *val = 103993;
> + *val2 = 16551;
> + return IIO_VAL_FRACTIONAL;
> + default:
> + return -EINVAL;
> + }
> + break;
> + case IIO_CHAN_INFO_RAW:
> + mutex_lock(>lock);
> + gpiod_set_value(st->sample, 0);
> +
> + /* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> + udelay(1);
> + gpiod_set_value(st->sample, 1);
> + gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +
> + ret = spi_read(st->sdev, st->rx, 2);
> + if (ret < 0) {
> + mutex_unlock(>lock);
> + return ret;
> + }
>  
> - /* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> - udelay(1);
> - gpiod_set_value(st->sample, 1);
> - gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> + vel = be16_to_cpup((__be16 *)st->rx);
> + switch (chan->type) {
> + case IIO_ANGL:
> + *val = vel >> 4;
> + break;
> + case IIO_ANGL_VEL:
> + *val = sign_extend32((s16)vel >> 4, 11);
> + break;
> + default:
> + mutex_unlock(>lock);
> + return -EINVAL;
> + }
>  
> - ret = spi_read(st->sdev, st->rx, 2);
> - if (ret < 0) {
> + /* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> + udelay(1);
>   mutex_unlock(>lock);
> - return ret;
> - }
>  
> - vel = be16_to_cpup((__be16 *)st->rx);
> - switch (chan->type) {
> - case IIO_ANGL:
> - *val = vel >> 4;
> - break;
> - case IIO_ANGL_VEL:
> - *val = sign_extend32((s16)vel >> 4, 11);
> - break;
> + return IIO_VAL_INT;
>   default:
> - mutex_unlock(>lock);
> - return -EINVAL;
> + break;
>   }
>  
> - /* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> - udelay(1);
> - mutex_unlock(>lock);
> -
> - return IIO_VAL_INT;
> + return -EINVAL;
>  }
>  

Re: [PATCH 02/13] staging: iio: tsl2x7x: use GPL-2.0+ SPDX license identifier

[Brian Masney]

On Sat, Apr 21, 2018 at 05:16:38PM +0100, Jonathan Cameron wrote:
> On Fri, 20 Apr 2018 20:41:42 -0400
> Brian Masney  wrote:
> 
> > The summary text for the GPL is not needed since the SPDX identifier
> > is a legally binding shorthand that can be used instead.
> > 
> > Signed-off-by: Brian Masney 
> I sanity checked against other drivers because I wasn't 100% sure
> this wasn't a valid formatting for SPDX.  It doesn't seem to be.
> Normally convention is
> //SPDX... 
> On the first line of the file.
> C style comments also fine, but it needs to be a comment line on it's
> own.  This is all about making it trivial for automated tools to find.

The style that you referenced is the most common style, however I saw
quite a few places in the kernel use the style used by this patch so I
thought that it may be acceptable.

$ grep -r SPDX arch/ drivers/ include/ | grep \* | grep -v ":\/"

I'll resubmit the patch next week with hopefully the last bit of
cleanups to get this driver out of staging.

Brian



> > ---
> >  drivers/staging/iio/light/tsl2x7x.c | 10 +-
> >  drivers/staging/iio/light/tsl2x7x.h | 14 +-
> >  2 files changed, 2 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> > b/drivers/staging/iio/light/tsl2x7x.c
> > index eeccfbb0eb1f..9cdcc8c9e812 100644
> > --- a/drivers/staging/iio/light/tsl2x7x.c
> > +++ b/drivers/staging/iio/light/tsl2x7x.c
> > @@ -5,15 +5,7 @@
> >   * Copyright (c) 2012, TAOS Corporation.
> >   * Copyright (c) 2017-2018 Brian Masney 
> >   *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License as published by
> > - * the Free Software Foundation; either version 2 of the License, or
> > - * (at your option) any later version.
> > - *
> > - * This program is distributed in the hope that it will be useful, but 
> > WITHOUT
> > - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > - * more details.
> > + * SPDX-License-Identifier: GPL-2.0+
> >   */
> >  
> >  #include 
> > diff --git a/drivers/staging/iio/light/tsl2x7x.h 
> > b/drivers/staging/iio/light/tsl2x7x.h
> > index d382cdbb976e..992ee2465609 100644
> > --- a/drivers/staging/iio/light/tsl2x7x.h
> > +++ b/drivers/staging/iio/light/tsl2x7x.h
> > @@ -4,19 +4,7 @@
> >   *
> >   * Copyright (c) 2012, TAOS Corporation.
> >   *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License as published by
> > - * the Free Software Foundation; either version 2 of the License, or
> > - * (at your option) any later version.
> > - *
> > - * This program is distributed in the hope that it will be useful, but 
> > WITHOUT
> > - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > - * more details.
> > - *
> > - * You should have received a copy of the GNU General Public License along
> > - * with this program; if not, write to the Free Software Foundation, Inc.,
> > - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> > + * SPDX-License-Identifier: GPL-2.0+
> >   */
> >  
> >  #ifndef __TSL2X7X_H
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 09/13] staging: iio: ad2s1200: Add documentation for device tree binding

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:54 +0200
David Veenstra  wrote:

> Add documentation for the added device tree bindings.
> 
> Signed-off-by: David Veenstra 
Straight forward, but please introduce it directly in
bindings/iio/resolver rather than moving it.

The binding doesn't require a driver to be outside of staging
so there would be no harm in doing this directly.

Jonathan

> ---
> Changes in v2:
>   - Introduced in this version.
> 
>  .../bindings/staging/iio/resolver/ad2s1200.txt   | 16 
> 
>  1 file changed, 16 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt 
> b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> new file mode 100644
> index ..85c009987878
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> @@ -0,0 +1,16 @@
> +Analog Devices AD2S1200 Resolver-to-Digital Converter
> +
> +Required properties:
> + - compatible : should be "adi,ad2s1200"
> + - reg : the SPI chip select number of the device
> + - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
> + - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
> +
> +Example:
> +
> + resolver {
> + compatible = "adi,ad2s1200";
> + reg = <4>;
> + sample-gpios = < 5 GPIO_ACTIVE_HIGH>;
> + rdvel-gpios = < 6 GPIO_ACTIVE_HIGH>;
> + };

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 08/13] staging: iio: ad2s1200: Replace legacy gpio API with modern API

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:44 +0200
David Veenstra  wrote:

> The legacy, integer based gpio API is replaced with the descriptor
> based API.
> 
> For compatibility, it is first tried to use the platform data to
> request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
> gpio function.
> 
> Signed-off-by: David Veenstra 
I would suggest that we simply force any out of tree users of the
platform data to update the platform data.  Drop the the old
versions entirely..

We can be mean to out of tree board files so let us make our own
lives easy.

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 51 
> -
>  1 file changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index 11ed9c7332e6..29a9bb666e7b 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -44,8 +45,8 @@
>  struct ad2s1200_state {
>   struct mutex lock;
>   struct spi_device *sdev;
> - int sample;
> - int rdvel;
> + struct gpio_desc *sample;
> + struct gpio_desc *rdvel;
>   u8 rx[2] cacheline_aligned;
>  };
>  
> @@ -60,12 +61,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>   u16 vel;
>  
>   mutex_lock(>lock);
> - gpio_set_value(st->sample, 0);
> + gpiod_set_value(st->sample, 0);
>  
>   /* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>   udelay(1);
> - gpio_set_value(st->sample, 1);
> - gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> + gpiod_set_value(st->sample, 1);
> + gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>  
>   ret = spi_read(st->sdev, st->rx, 2);
>   if (ret < 0) {
> @@ -121,13 +122,18 @@ static int ad2s1200_probe(struct spi_device *spi)
>  
>   dev = >dev;
>  
> - for (pn = 0; pn < AD2S1200_PN; pn++) {
> - ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
> - DRV_NAME);
> - if (ret) {
> - dev_err(dev, "request gpio pin %d failed\n",
> - pins[pn]);
> - return ret;
> + if (pins) {
> + for (pn = 0; pn < AD2S1200_PN; pn++) {
> + ret = devm_gpio_request_one(dev, pins[pn],
> + GPIOF_DIR_OUT,
> + DRV_NAME);
> + if (ret) {
> + dev_err(dev,
> + "Failed to claim gpio %d\n: err=%d",
> + pins[pn],
> + ret);
> + return ret;
> + }
>   }
>   }
>  
> @@ -139,8 +145,25 @@ static int ad2s1200_probe(struct spi_device *spi)
>   st = iio_priv(indio_dev);
>   mutex_init(>lock);
>   st->sdev = spi;
> - st->sample = pins[0];
> - st->rdvel = pins[1];
> +
> + if (pins) {
> + st->sample = gpio_to_desc(pins[0]);
> + st->rdvel = gpio_to_desc(pins[1]);
> + } else {
> + st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
> + if (IS_ERR(st->sample)) {
> + dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
> + PTR_ERR(st->sample));
> + return PTR_ERR(st->sample);
> + }
> +
> + st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
> + if (IS_ERR(st->rdvel)) {
> + dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
> + PTR_ERR(st->rdvel));
> + return PTR_ERR(st->rdvel);
> + }
> + }
>  
>   indio_dev->dev.parent = dev;
>   indio_dev->info = _info;

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 07/13] staging: iio: ad2s1200: Improve readability with be16_to_cpup

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:32 +0200
David Veenstra  wrote:

> The manual states that the data is contained in the upper 12 bits
> of the 16 bits read by spi. The code that extracts these 12 bits
> is correct for both be and le machines, but this is not clear
> from a first glance.
> 
> To improve readability the relevant expressions are replaced
> with equivalent expressions that use be16_to_cpup.
> 
> Signed-off-by: David Veenstra 
> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index 0a5fc9917e32..11ed9c7332e6 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -57,7 +57,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  {
>   struct ad2s1200_state *st = iio_priv(indio_dev);
>   int ret = 0;
> - s16 vel;
> + u16 vel;
>  
>   mutex_lock(>lock);
>   gpio_set_value(st->sample, 0);
> @@ -73,14 +73,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>   return ret;
>   }
>  
> + vel = be16_to_cpup((__be16 *)st->rx);
Well it isn't vel (velocity) in one case so now the name is misleading.

Also that type cast suggests a fairly obvious cleanup associated with this one.
Why not make st->rx a __be16 in the first pace avoiding the need to cast at all?

Then you could just have *val = be16_to_cpu(st->rx) >> 4 and similar.

>   switch (chan->type) {
>   case IIO_ANGL:
> - *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> + *val = vel >> 4;
>   break;
>   case IIO_ANGL_VEL:
> - vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> - vel = sign_extend32(vel, 11);
> - *val = vel;
> + *val = sign_extend32((s16)vel >> 4, 11);
If you were to use an intermediate that was s16 then the sign extend would
be automatic.  Perhaps it is clear to do it like this though.. 
Up to you.

>   break;
>   default:
>   mutex_unlock(>lock);

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 06/13] staging: iio: ad2s1200: Introduce variable for repeated value

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:19 +0200
David Veenstra  wrote:

> Add variable to hold >dev in ad2s1200_probe. This value is repeatedly
> used in ad2s1200_probe.
> 
> Signed-off-by: David Veenstra 
No significant gain in readability.   Perhaps even a slight lost I'm
going to say no to this one.

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index f07aab7e7a35..0a5fc9917e32 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -117,19 +117,22 @@ static int ad2s1200_probe(struct spi_device *spi)
>   unsigned short *pins = spi->dev.platform_data;
>   struct ad2s1200_state *st;
>   struct iio_dev *indio_dev;
> + struct device *dev;
>   int pn, ret = 0;
>  
> + dev = >dev;
> +
>   for (pn = 0; pn < AD2S1200_PN; pn++) {
> - ret = devm_gpio_request_one(>dev, pins[pn], GPIOF_DIR_OUT,
> + ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>   DRV_NAME);
>   if (ret) {
> - dev_err(>dev, "request gpio pin %d failed\n",
> + dev_err(dev, "request gpio pin %d failed\n",
>   pins[pn]);
>   return ret;
>   }
>   }
>  
> - indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>   if (!indio_dev)
>   return -ENOMEM;
>  
> @@ -140,14 +143,14 @@ static int ad2s1200_probe(struct spi_device *spi)
>   st->sample = pins[0];
>   st->rdvel = pins[1];
>  
> - indio_dev->dev.parent = >dev;
> + indio_dev->dev.parent = dev;
>   indio_dev->info = _info;
>   indio_dev->modes = INDIO_DIRECT_MODE;
>   indio_dev->channels = ad2s1200_channels;
>   indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
>   indio_dev->name = spi_get_device_id(spi)->name;
>  
> - ret = devm_iio_device_register(>dev, indio_dev);
> + ret = devm_iio_device_register(dev, indio_dev);
>   if (ret)
>   return ret;
>  

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 05/13] staging: iio: ad2s1200: Add kernel docs to driver state

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:03 +0200
David Veenstra  wrote:

> Add missing kernel docs to the ad2s1200 driver state.
> 
> Signed-off-by: David Veenstra 
> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index 357fe3c382b3..f07aab7e7a35 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -33,6 +33,14 @@
>  /* clock period in nano second */
>  #define AD2S1200_TSCLK   (10 / AD2S1200_HZ)
>  
> +/**
> + * struct ad2s1200_state - driver instance specific data
> + * @lock:protect driver state

This doc for locks needs to be more specific.  From a quick
glance I think it does two things.
1) Ensures that we don't have concurrent accesses changing the
gpio control lines.
2) Protects the rx buffer against concurrent accesses.

It doesn't have anything much to do with the rest of this state
structure.

> + * @sdev:spi device
> + * @sample:  GPIO pin SAMPLE
> + * @rdvel:   GPIO pin RDVEL
> + * @rx:  buffer for spi transfers
> + */
>  struct ad2s1200_state {
>   struct mutex lock;
>   struct spi_device *sdev;

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 03/13] staging: iio: ad2s1200: Reverse Christmas tree ordering

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:29:08 +0200
David Veenstra  wrote:

> Reorders the variable declarations to prefer a reverse Christmas tree
> order to improve readability.
> 
> Signed-off-by: David Veenstra 
Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index ffcdf4e8eb92..b6c3a3c8f7fe 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -46,9 +46,9 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>int *val2,
>long m)
>  {
> + struct ad2s1200_state *st = iio_priv(indio_dev);
>   int ret = 0;
>   s16 vel;
> - struct ad2s1200_state *st = iio_priv(indio_dev);
>  
>   mutex_lock(>lock);
>   gpio_set_value(st->sample, 0);
> @@ -101,10 +101,10 @@ static const struct iio_info ad2s1200_info = {
>  
>  static int ad2s1200_probe(struct spi_device *spi)
>  {
> + unsigned short *pins = spi->dev.platform_data;
>   struct ad2s1200_state *st;
>   struct iio_dev *indio_dev;
>   int pn, ret = 0;
> - unsigned short *pins = spi->dev.platform_data;
>  
>   for (pn = 0; pn < AD2S1200_PN; pn++) {
>   ret = devm_gpio_request_one(>dev, pins[pn], GPIOF_DIR_OUT,

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 04/13] staging: iio: ad2s1200: Add blank lines

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:29:52 +0200
David Veenstra  wrote:

> Add blank lines to improve readability.
> 
> Signed-off-by: David Veenstra 
Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index b6c3a3c8f7fe..357fe3c382b3 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -9,6 +9,7 @@
>   * published by the Free Software Foundation.
>   *
>   */
> +
>  #include 
>  #include 
>  #include 
> @@ -52,10 +53,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  
>   mutex_lock(>lock);
>   gpio_set_value(st->sample, 0);
> +
>   /* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>   udelay(1);
>   gpio_set_value(st->sample, 1);
>   gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +
>   ret = spi_read(st->sdev, st->rx, 2);
>   if (ret < 0) {
>   mutex_unlock(>lock);
> @@ -75,9 +78,11 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>   mutex_unlock(>lock);
>   return -EINVAL;
>   }
> +
>   /* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>   udelay(1);
>   mutex_unlock(>lock);
> +
>   return IIO_VAL_INT;
>  }
>  
> @@ -115,9 +120,11 @@ static int ad2s1200_probe(struct spi_device *spi)
>   return ret;
>   }
>   }
> +
>   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
>   if (!indio_dev)
>   return -ENOMEM;
> +
>   spi_set_drvdata(spi, indio_dev);
>   st = iio_priv(indio_dev);
>   mutex_init(>lock);

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 02/13] staging: iio: ad2s1200: Sort includes alphabetically

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:28:52 +0200
David Veenstra  wrote:

> This patches sorts all the includes in alphabetic order.
> 
> Signed-off-by: David Veenstra 
Applied,

Thanks

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index 5d7ed0034422..ffcdf4e8eb92 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -9,15 +9,15 @@
>   * published by the Free Software Foundation.
>   *
>   */
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
> -#include 
> +#include 
> +#include 
> +#include 
> +#include 
>  
>  #include 
>  #include 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 01/13] staging: iio: ad2s1200: Remove unneeded include

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:28:32 +0200
David Veenstra  wrote:

> This patches removes unneeded slab.h header.
> 
> Signed-off-by: David Veenstra 
This one surprised me, but indeed there are no direct
users of any memory allocation in this file.

Applied to the togreg branch of iio.git and pushed out as
testing for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> Changes in v2:
>   - Introduced in this version.
> 
> drivers/staging/iio/resolver/ad2s1200.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> b/drivers/staging/iio/resolver/ad2s1200.c
> index aa62c64e9bc4..5d7ed0034422 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -13,7 +13,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 11/13] staging: iio: tsl2x7x: use device defaults for als_time, prox_time and wait_time

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:51 -0400
Brian Masney  wrote:

> This patch changes the defaults of the als_time, prox_time and
> wait_time to match the defaults according to the TSL2772 datasheet.
> 
> Signed-off-by: Brian Masney 
Applied, thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index a7b4fcba7935..293810ff11b9 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -201,11 +201,11 @@ static const struct tsl2x7x_lux 
> *tsl2x7x_default_lux_table_group[] = {
>  };
>  
>  static const struct tsl2x7x_settings tsl2x7x_default_settings = {
> - .als_time = 219, /* 101 ms */
> + .als_time = 255, /* 2.73 ms */
>   .als_gain = 0,
> - .prox_time = 254, /* 5.4 ms */
> + .prox_time = 255, /* 2.73 ms */
>   .prox_gain = 0,
> - .wait_time = 245,
> + .wait_time = 255,
>   .prox_config = 0,
>   .als_gain_trim = 1000,
>   .als_cal_target = 150,

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 13/13] staging: iio: tsl2x7x: rename prox_config to als_prox_config

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:53 -0400
Brian Masney  wrote:

> The configuration register on the device is represented with the
> prox_config member on the tsl2x7x_settings structure. According to the
> TSL2772 data sheet, this register can hold: 1) the proximity drive
> level, 2) ALS/Proximity long wait, and 3) the ALS gain level. This
> patch renames prox_config to als_prox_config since ALS settings can
> be stored here as well.
> 
> Signed-off-by: Brian Masney 
Applied to the togreg branch of iio.git.

So I think I picked up all but 2 where there were minor suggestions.

Looking forward to the final set!

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 7 ---
>  drivers/staging/iio/light/tsl2x7x.h | 5 +++--
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 05c0f3d5fac0..708b2c6bdf4b 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -56,7 +56,7 @@
>  #define TSL2X7X_PRX_MAXTHRESHLO  0X0A
>  #define TSL2X7X_PRX_MAXTHRESHHI  0X0B
>  #define TSL2X7X_PERSISTENCE  0x0C
> -#define TSL2X7X_PRX_CONFIG   0x0D
> +#define TSL2X7X_ALS_PRX_CONFIG   0x0D
>  #define TSL2X7X_PRX_COUNT0x0E
>  #define TSL2X7X_GAIN 0x0F
>  #define TSL2X7X_NOTUSED  0x10
> @@ -207,7 +207,7 @@ static const struct tsl2x7x_settings 
> tsl2x7x_default_settings = {
>   .prox_time = 255, /* 2.73 ms */
>   .prox_gain = 0,
>   .wait_time = 255,
> - .prox_config = 0,
> + .als_prox_config = 0,
>   .als_gain_trim = 1000,
>   .als_cal_target = 150,
>   .als_persistence = 1,
> @@ -594,7 +594,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>   /* Non calculated parameters */
>   chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prox_time;
>   chip->tsl2x7x_config[TSL2X7X_WAIT_TIME] = chip->settings.wait_time;
> - chip->tsl2x7x_config[TSL2X7X_PRX_CONFIG] = chip->settings.prox_config;
> + chip->tsl2x7x_config[TSL2X7X_ALS_PRX_CONFIG] =
> + chip->settings.als_prox_config;
>  
>   chip->tsl2x7x_config[TSL2X7X_ALS_MINTHRESHLO] =
>   (chip->settings.als_thresh_low) & 0xFF;
> diff --git a/drivers/staging/iio/light/tsl2x7x.h 
> b/drivers/staging/iio/light/tsl2x7x.h
> index 85d8fe7a94c8..6e30e71a2127 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -48,7 +48,8 @@ struct tsl2x7x_lux {
>   *  increments. Total integration time is
>   *  (256 - prx_time) * 2.73.
>   *  @prox_gain: Index into the tsl2x7x_prx_gain array.
> - *  @prox_config:   Prox configuration filters.
> + *  @als_prox_config:   The value of the ALS / Proximity configuration
> + *  register.
>   *  @als_cal_target:Known external ALS reading for calibration.
>   *  @als_persistence:   H/W Filters, Number of 'out of limits' ALS 
> readings.
>   *  @als_interrupt_en:  Enable/Disable ALS interrupts
> @@ -73,7 +74,7 @@ struct tsl2x7x_settings {
>   int wait_time;
>   int prox_time;
>   int prox_gain;
> - int prox_config;
> + int als_prox_config;
>   int als_cal_target;
>   u8 als_persistence;
>   bool als_interrupt_en;

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 12/13] staging: iio: tsl2x7x: various comment cleanups

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:52 -0400
Brian Masney  wrote:

> This patch removes several unnecessary comments, changes some comments
> so that the use as much of the allowable 80 characters as possible, adds
> the proper whitespace, removes some structure members from the kernel
> docs that are no longer present, and improves the existing kernel doc
> information for some existing structure members.
> 
> Signed-off-by: Brian Masney 
Looks sensible

Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 59 
> +
>  drivers/staging/iio/light/tsl2x7x.h | 48 +++---
>  2 files changed, 51 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 293810ff11b9..05c0f3d5fac0 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1,6 +1,6 @@
>  /*
> - * Device driver for monitoring ambient light intensity in (lux)
> - * and proximity detection (prox) within the TAOS TSL2X7X family of devices.
> + * Device driver for monitoring ambient light intensity in (lux) and 
> proximity
> + * detection (prox) within the TAOS TSL2X7X family of devices.
>   *
>   * Copyright (c) 2012, TAOS Corporation.
>   * Copyright (c) 2017-2018 Brian Masney 
> @@ -21,7 +21,7 @@
>  #include 
>  #include "tsl2x7x.h"
>  
> -/* Cal defs*/
> +/* Cal defs */
>  #define PROX_STAT_CAL0
>  #define PROX_STAT_SAMP   1
>  #define MAX_SAMPLES_CAL  200
> @@ -34,10 +34,11 @@
>  /* Lux calculation constants */
>  #define TSL2X7X_LUX_CALC_OVER_FLOW   65535
>  
> -/* TAOS Register definitions - note:
> - * depending on device, some of these register are not used and the
> - * register address is benign.
> +/*
> + * TAOS Register definitions - Note: depending on device, some of these 
> register
> + * are not used and the register address is benign.
>   */
> +
>  /* 2X7X register offsets */
>  #define TSL2X7X_MAX_CONFIG_REG   16
>  
> @@ -342,15 +343,14 @@ static int tsl2x7x_read_autoinc_regs(struct 
> tsl2X7X_chip *chip, int lower_reg,
>   * @indio_dev:   pointer to IIO device
>   *
>   * The raw ch0 and ch1 values of the ambient light sensed in the last
> - * integration cycle are read from the device.
> - * Time scale factor array values are adjusted based on the integration time.
> - * The raw values are multiplied by a scale factor, and device gain is 
> obtained
> - * using gain index. Limit checks are done next, then the ratio of a multiple
> - * of ch1 value, to the ch0 value, is calculated. Array tsl2x7x_device_lux[]
> - * is then scanned to find the first ratio value that is just above the ratio
> - * we just calculated. The ch0 and ch1 multiplier constants in the array are
> - * then used along with the time scale factor array values, to calculate the
> - * lux.
> + * integration cycle are read from the device. Time scale factor array values
> + * are adjusted based on the integration time. The raw values are multiplied
> + * by a scale factor, and device gain is obtained using gain index. Limit
> + * checks are done next, then the ratio of a multiple of ch1 value, to the
> + * ch0 value, is calculated. Array tsl2x7x_device_lux[] is then scanned to
> + * find the first ratio value that is just above the ratio we just 
> calculated.
> + * The ch0 and ch1 multiplier constants in the array are then used along with
> + * the time scale factor array values, to calculate the lux.
>   */
>  static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  {
> @@ -363,7 +363,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>   mutex_lock(>als_mutex);
>  
>   if (chip->tsl2x7x_chip_status != TSL2X7X_CHIP_WORKING) {
> - /* device is not enabled */
>   dev_err(>client->dev, "%s: device is not enabled\n",
>   __func__);
>   ret = -EBUSY;
> @@ -374,7 +373,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>   if (ret < 0)
>   goto out_unlock;
>  
> - /* is data new & valid */
>   if (!(ret & TSL2X7X_STA_ADC_VALID)) {
>   dev_err(>client->dev,
>   "%s: data not valid yet\n", __func__);
> @@ -430,12 +428,12 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>   lux = (lux + (chip->als_time_scale >> 1)) /
>   chip->als_time_scale;
>  
> - /* adjust for active gain scale
> -  * The tsl2x7x_device_lux tables have a factor of 256 built-in.
> -  * User-specified gain provides a multiplier.
> + /*
> +  * adjust for active gain scale. The tsl2x7x_device_lux tables have a
> +  * factor of 256 built-in. User-specified gain provides a multiplier.
>* Apply user-specified gain before shifting right to retain precision.
> -  * Use 64 

Re: [PATCH 10/13] staging: iio: tsl2x7x: rename prx to prox for consistency

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:50 -0400
Brian Masney  wrote:

> The driver mostly uses the 'prox' naming convention for most of the
> proximity settings, however prx_time and tsl2x7x_prx_gain was present.
> This patch renames these to prox_time and tsl2x7x_prox_gain for
> consistency with everything else in the driver.
> 
> The kernel documentation for prx_gain is corrected to prox_gain so that
> it matches what is actually in the structure.
> 
> Signed-off-by: Brian Masney 
Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 12 ++--
>  drivers/staging/iio/light/tsl2x7x.h |  6 +++---
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 87b99deef7a8..a7b4fcba7935 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -203,7 +203,7 @@ static const struct tsl2x7x_lux 
> *tsl2x7x_default_lux_table_group[] = {
>  static const struct tsl2x7x_settings tsl2x7x_default_settings = {
>   .als_time = 219, /* 101 ms */
>   .als_gain = 0,
> - .prx_time = 254, /* 5.4 ms */
> + .prox_time = 254, /* 5.4 ms */
>   .prox_gain = 0,
>   .wait_time = 245,
>   .prox_config = 0,
> @@ -230,7 +230,7 @@ static const s16 tsl2x7x_als_gain[] = {
>   120
>  };
>  
> -static const s16 tsl2x7x_prx_gain[] = {
> +static const s16 tsl2x7x_prox_gain[] = {
>   1,
>   2,
>   4,
> @@ -594,7 +594,7 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>   u8 *dev_reg, reg_val;
>  
>   /* Non calculated parameters */
> - chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prx_time;
> + chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prox_time;
>   chip->tsl2x7x_config[TSL2X7X_WAIT_TIME] = chip->settings.wait_time;
>   chip->tsl2x7x_config[TSL2X7X_PRX_CONFIG] = chip->settings.prox_config;
>  
> @@ -1021,7 +1021,7 @@ static int tsl2x7x_write_event_value(struct iio_dev 
> *indio_dev,
>   if (chan->type == IIO_INTENSITY)
>   time = chip->settings.als_time;
>   else
> - time = chip->settings.prx_time;
> + time = chip->settings.prox_time;
>  
>   y = (TSL2X7X_MAX_TIMER_CNT - time) + 1;
>   z = y * TSL2X7X_MIN_ITIME;
> @@ -1090,7 +1090,7 @@ static int tsl2x7x_read_event_value(struct iio_dev 
> *indio_dev,
>   time = chip->settings.als_time;
>   mult = chip->settings.als_persistence;
>   } else {
> - time = chip->settings.prx_time;
> + time = chip->settings.prox_time;
>   mult = chip->settings.prox_persistence;
>   }
>  
> @@ -1153,7 +1153,7 @@ static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
>   if (chan->type == IIO_LIGHT)
>   *val = tsl2x7x_als_gain[chip->settings.als_gain];
>   else
> - *val = tsl2x7x_prx_gain[chip->settings.prox_gain];
> + *val = tsl2x7x_prox_gain[chip->settings.prox_gain];
>   ret = IIO_VAL_INT;
>   break;
>   case IIO_CHAN_INFO_CALIBBIAS:
> diff --git a/drivers/staging/iio/light/tsl2x7x.h 
> b/drivers/staging/iio/light/tsl2x7x.h
> index 2c96f0b39b1e..408e5a89edb1 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -44,9 +44,9 @@ struct tsl2x7x_lux {
>   *  aperture effects.
>   *  @wait_time: Time between PRX and ALS cycles
>   *  in 2.7 periods
> - *  @prx_time:  5.2ms prox integration time -
> + *  @prox_time: 5.2ms prox integration time -
>   *  decrease in 2.7ms periods
> - *  @prx_gain:  Proximity gain index
> + *  @prox_gain: Proximity gain index
>   *  @prox_config:   Prox configuration filters.
>   *  @als_cal_target:Known external ALS reading for
>   *  calibration.
> @@ -68,7 +68,7 @@ struct tsl2x7x_settings {
>   int als_gain;
>   int als_gain_trim;
>   int wait_time;
> - int prx_time;
> + int prox_time;
>   int prox_gain;
>   int prox_config;
>   int als_cal_target;

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 09/13] staging: iio: tsl2x7x: move power and diode settings into header file

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:49 -0400
Brian Masney  wrote:

> The power and diode defines are needed for the platform data so this
> patch moves the defines out of the .c file and into the header file. A
> comment for the diode is also cleaned up while this code is touched.
> 
> Signed-off-by: Brian Masney 
Makes sense.

Applied,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 12 
>  drivers/staging/iio/light/tsl2x7x.h | 12 
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 15bc0af1bb6c..87b99deef7a8 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -103,18 +103,6 @@
>  #define TSL2X7X_CNTL_PROXPON_ENBL0x0F
>  #define TSL2X7X_CNTL_INTPROXPON_ENBL 0x2F
>  
> -/*Prox diode to use */
> -#define TSL2X7X_DIODE0   0x01
> -#define TSL2X7X_DIODE1   0x02
> -#define TSL2X7X_DIODE_BOTH   0x03
> -
> -/* LED Power */
> -#define TSL2X7X_100_mA   0x00
> -#define TSL2X7X_50_mA0x01
> -#define TSL2X7X_25_mA0x02
> -#define TSL2X7X_13_mA0x03
> -#define TSL2X7X_MAX_TIMER_CNT0xFF
> -
>  #define TSL2X7X_MIN_ITIME3
>  
>  /* TAOS txx2x7x Device family members */
> diff --git a/drivers/staging/iio/light/tsl2x7x.h 
> b/drivers/staging/iio/light/tsl2x7x.h
> index 992ee2465609..2c96f0b39b1e 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -23,6 +23,18 @@ struct tsl2x7x_lux {
>  #define TSL2X7X_DEFAULT_TABLE_BYTES (sizeof(struct tsl2x7x_lux) * \
>TSL2X7X_DEF_LUX_TABLE_SZ)
>  
> +/* Proximity diode to use */
> +#define TSL2X7X_DIODE0  0x01
> +#define TSL2X7X_DIODE1  0x02
> +#define TSL2X7X_DIODE_BOTH  0x03
> +
> +/* LED Power */
> +#define TSL2X7X_100_mA  0x00
> +#define TSL2X7X_50_mA   0x01
> +#define TSL2X7X_25_mA   0x02
> +#define TSL2X7X_13_mA   0x03
> +#define TSL2X7X_MAX_TIMER_CNT   0xFF
> +
>  /**
>   * struct tsl2x7x_default_settings - power on defaults unless
>   *   overridden by platform data.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 08/13] staging: iio: tsl2x7x: add range checking to three sysfs attributes

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:48 -0400
Brian Masney  wrote:

> The sysfs attributes in_illuminance0_target_input,
> in_illuminance0_calibrate, and in_proximity0_calibrate did not have
> proper range checking in place so this patch adds the correct range
> checks.
> 
> Signed-off-by: Brian Masney 

Comment inline.


> ---
>  drivers/staging/iio/light/tsl2x7x.c | 25 +++--
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 56730baea927..15bc0af1bb6c 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -835,9 +835,10 @@ static ssize_t in_illuminance0_target_input_store(struct 
> device *dev,
>   if (kstrtoul(buf, 0, ))
>   return -EINVAL;
>  
> - if (value)
> - chip->settings.als_cal_target = value;
> + if (value < 0 || value > 65535)
> + return -ERANGE;
How about using kstrtou16 which does this check internally...

>  
> + chip->settings.als_cal_target = value;
>   ret = tsl2x7x_invoke_change(indio_dev);
>   if (ret < 0)
>   return ret;
> @@ -853,14 +854,12 @@ static ssize_t in_illuminance0_calibrate_store(struct 
> device *dev,
>   bool value;
>   int ret;
>  
> - if (strtobool(buf, ))
> + if (kstrtobool(buf, ) || !value)
>   return -EINVAL;
>  
> - if (value) {
> - ret = tsl2x7x_als_calibrate(indio_dev);
> - if (ret < 0)
> - return ret;
> - }
> + ret = tsl2x7x_als_calibrate(indio_dev);
> + if (ret < 0)
> + return ret;
>  
>   ret = tsl2x7x_invoke_change(indio_dev);
>   if (ret < 0)
> @@ -946,14 +945,12 @@ static ssize_t in_proximity0_calibrate_store(struct 
> device *dev,
>   bool value;
>   int ret;
>  
> - if (strtobool(buf, ))
> + if (kstrtobool(buf, ) || !value)
>   return -EINVAL;
>  
> - if (value) {
> - ret = tsl2x7x_prox_cal(indio_dev);
> - if (ret < 0)
> - return ret;
> - }
> + ret = tsl2x7x_prox_cal(indio_dev);
> + if (ret < 0)
> + return ret;
>  
>   ret = tsl2x7x_invoke_change(indio_dev);
>   if (ret < 0)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 07/13] staging: iio: tsl2x7x: simplify device id verification

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:47 -0400
Brian Masney  wrote:

> This patch renames tsl2x7x_device_id() to tsl2x7x_device_id_verif(),
> removes the unnecessary pointer on the id parameter, and only calls
> the verification function once.
> 
> Signed-off-by: Brian Masney 
That double call is just weird..

Anyhow, 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/tsl2x7x.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index d202bc7e1f4f..56730baea927 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1277,22 +1277,22 @@ static DEVICE_ATTR_WO(in_proximity0_calibrate);
>  static DEVICE_ATTR_RW(in_illuminance0_lux_table);
>  
>  /* Use the default register values to identify the Taos device */
> -static int tsl2x7x_device_id(int *id, int target)
> +static int tsl2x7x_device_id_verif(int id, int target)
>  {
>   switch (target) {
>   case tsl2571:
>   case tsl2671:
>   case tsl2771:
> - return (*id & 0xf0) == TRITON_ID;
> + return (id & 0xf0) == TRITON_ID;
>   case tmd2671:
>   case tmd2771:
> - return (*id & 0xf0) == HALIBUT_ID;
> + return (id & 0xf0) == HALIBUT_ID;
>   case tsl2572:
>   case tsl2672:
>   case tmd2672:
>   case tsl2772:
>   case tmd2772:
> - return (*id & 0xf0) == SWORDFISH_ID;
> + return (id & 0xf0) == SWORDFISH_ID;
>   }
>  
>   return -EINVAL;
> @@ -1612,8 +1612,7 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>   if (ret < 0)
>   return ret;
>  
> - if ((!tsl2x7x_device_id(, id->driver_data)) ||
> - (tsl2x7x_device_id(, id->driver_data) == -EINVAL)) {
> + if (tsl2x7x_device_id_verif(ret, id->driver_data) <= 0) {
>   dev_info(>client->dev,
>"%s: i2c device found does not match expected id\n",
>   __func__);

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 06/13] staging: iio: tsl2x7x: simplify tsl2x7x_write_interrupt_config return

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:46 -0400
Brian Masney  wrote:

> tsl2x7x_write_interrupt_config() has an unnecessary return value check
> at the end of the function. This patch changes the function to just
> return the value from the call to tsl2x7x_invoke_change().
> 
> Signed-off-by: Brian Masney 
Nice little cleanup.

Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 8d8af0cf9768..d202bc7e1f4f 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -982,18 +982,13 @@ 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)
>   chip->settings.als_interrupt_en = val ? true : false;
>   else
>   chip->settings.prox_interrupt_en = val ? true : false;
>  
> - ret = tsl2x7x_invoke_change(indio_dev);
> - if (ret < 0)
> - return ret;
> -
> - return 0;
> + return tsl2x7x_invoke_change(indio_dev);
>  }
>  
>  static int tsl2x7x_write_event_value(struct iio_dev *indio_dev,

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 05/13] staging: iio: tsl2x7x: remove unnecessary chip status checks in suspend/resume

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:45 -0400
Brian Masney  wrote:

> tsl2x7x_suspend() and tsl2x7x_resume() both check to see what the
> current chip status is. These checks are not necessary so this patch
> removes those checks.
> 
> Signed-off-by: Brian Masney 
This description could have been clearer... The key point is we
can always know what state we are in when we hit these functions anyway
(I think).

Anyhow applied as is 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/tsl2x7x.c | 16 ++--
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index f37fc74b8fbc..8d8af0cf9768 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1682,27 +1682,15 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  static int tsl2x7x_suspend(struct device *dev)
>  {
>   struct iio_dev *indio_dev = dev_get_drvdata(dev);
> - struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> - int ret = 0;
> -
> - if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
> - ret = tsl2x7x_chip_off(indio_dev);
> - chip->tsl2x7x_chip_status = TSL2X7X_CHIP_SUSPENDED;
> - }
>  
> - return ret;
> + return tsl2x7x_chip_off(indio_dev);
>  }
>  
>  static int tsl2x7x_resume(struct device *dev)
>  {
>   struct iio_dev *indio_dev = dev_get_drvdata(dev);
> - struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> - int ret = 0;
>  
> - if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_SUSPENDED)
> - ret = tsl2x7x_chip_on(indio_dev);
> -
> - return ret;
> + return tsl2x7x_chip_on(indio_dev);
>  }
>  
>  static int tsl2x7x_remove(struct i2c_client *client)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 04/13] staging: iio: tsl2x7x: simplify tsl2x7x_clear_interrupts function

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:44 -0400
Brian Masney  wrote:

> tsl2x7x_clear_interrupts() takes a reg argument but there are only
> two callers to this function and both callers pass the same value.
> Since this function was introduced, interrupts are now working
> properly for this driver, and several unnecessary calls to
> tsl2x7x_clear_interrupts() were removed. This patch removes the
> tsl2x7x_clear_interrupts() function and replaces the two callers
> with the i2c_smbus_write_byte() call instead.
> 
> Signed-off-by: Brian Masney 
Applied

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 32 +++-
>  1 file changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 95a00b965c5e..f37fc74b8fbc 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -271,20 +271,6 @@ static const u8 device_channel_config[] = {
>   ALSPRX2
>  };
>  
> -static int tsl2x7x_clear_interrupts(struct tsl2X7X_chip *chip, int reg)
> -{
> - int ret;
> -
> - ret = i2c_smbus_write_byte(chip->client,
> -TSL2X7X_CMD_REG | TSL2X7X_CMD_SPL_FN | reg);
> - if (ret < 0)
> - dev_err(>client->dev,
> - "%s: failed to clear interrupt status %x: %d\n",
> - __func__, reg, ret);
> -
> - return ret;
> -}
> -
>  static int tsl2x7x_read_status(struct tsl2X7X_chip *chip)
>  {
>   int ret;
> @@ -714,9 +700,15 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>   if (ret < 0)
>   return ret;
>  
> - ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
> - if (ret < 0)
> + ret = i2c_smbus_write_byte(chip->client,
> +TSL2X7X_CMD_REG | TSL2X7X_CMD_SPL_FN |
> +TSL2X7X_CMD_PROXALS_INT_CLR);
> + if (ret < 0) {
> + dev_err(>client->dev,
> + "%s: failed to clear interrupt status: %d\n",
> + __func__, ret);
>   return ret;
> + }
>  
>   chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
>  
> @@ -1341,7 +1333,13 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void 
> *private)
>  timestamp);
>   }
>  
> - tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
> + ret = i2c_smbus_write_byte(chip->client,
> +TSL2X7X_CMD_REG | TSL2X7X_CMD_SPL_FN |
> +TSL2X7X_CMD_PROXALS_INT_CLR);
> + if (ret < 0)
> + dev_err(>client->dev,
> + "%s: failed to clear interrupt status: %d\n",
> + __func__, ret);
>  
>   return IRQ_HANDLED;
>  }

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/13] staging: iio: tsl2x7x: don't return error in IRQ handler

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:43 -0400
Brian Masney  wrote:

> tsl2x7x_event_handler() could return an error and this could cause the
> interrupt to remain masked. We shouldn't return an error in the
> interrupt handler so this patch always returns IRQ_HANDLED. An error
> will be logged if one occurs.
> 
> Signed-off-by: Brian Masney 
There is a slight argument that we should report an error from the 
interrupt clear if it generates an error, but I'm not that bothered
as that is unlikely to happen and people rarely see these errors in
their logs anyway.

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/tsl2x7x.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 9cdcc8c9e812..95a00b965c5e 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1320,7 +1320,7 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void 
> *private)
>  
>   ret = tsl2x7x_read_status(chip);
>   if (ret < 0)
> - return ret;
> + return IRQ_HANDLED;
>  
>   /* What type of interrupt do we need to process */
>   if (ret & TSL2X7X_STA_PRX_INTR) {
> @@ -1341,9 +1341,7 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void 
> *private)
>  timestamp);
>   }
>  
> - ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
> - if (ret < 0)
> - return ret;
> + tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
>  
>   return IRQ_HANDLED;
>  }

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/13] staging: iio: tsl2x7x: use GPL-2.0+ SPDX license identifier

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:42 -0400
Brian Masney  wrote:

> The summary text for the GPL is not needed since the SPDX identifier
> is a legally binding shorthand that can be used instead.
> 
> Signed-off-by: Brian Masney 
I sanity checked against other drivers because I wasn't 100% sure
this wasn't a valid formatting for SPDX.  It doesn't seem to be.
Normally convention is
//SPDX... 
On the first line of the file.
C style comments also fine, but it needs to be a comment line on it's
own.  This is all about making it trivial for automated tools to find.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 10 +-
>  drivers/staging/iio/light/tsl2x7x.h | 14 +-
>  2 files changed, 2 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index eeccfbb0eb1f..9cdcc8c9e812 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -5,15 +5,7 @@
>   * Copyright (c) 2012, TAOS Corporation.
>   * Copyright (c) 2017-2018 Brian Masney 
>   *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but 
> WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> - * more details.
> + * SPDX-License-Identifier: GPL-2.0+
>   */
>  
>  #include 
> diff --git a/drivers/staging/iio/light/tsl2x7x.h 
> b/drivers/staging/iio/light/tsl2x7x.h
> index d382cdbb976e..992ee2465609 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -4,19 +4,7 @@
>   *
>   * Copyright (c) 2012, TAOS Corporation.
>   *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but 
> WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> - * more details.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, write to the Free Software Foundation, Inc.,
> - * 51 Franklin Street, Fifth Floor, Boston, MA   02110-1301, USA.
> + * SPDX-License-Identifier: GPL-2.0+
>   */
>  
>  #ifndef __TSL2X7X_H

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/13] staging: iio: tsl2x7x: move integration_time* attributes to IIO_INTENSITY channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 20:41:41 -0400
Brian Masney  wrote:

> The integration_time* attributes are currently associated with the
> IIO_LIGHT channel but should be associated with the IIO_INTENSITY
> channel. Directory listing of the sysfs attributes for a TSL2772
> with this patch applied:
> 
> dev
> events
> in_illuminance0_calibrate
> in_illuminance0_calibscale_available
> in_illuminance0_input
> in_illuminance0_lux_table
> in_illuminance0_target_input
> in_intensity0_calibbias
> in_intensity0_calibscale
> in_intensity0_integration_time
> in_intensity0_integration_time_available
> in_intensity0_raw
> in_intensity1_raw
> in_proximity0_calibrate
> in_proximity0_calibscale
> in_proximity0_calibscale_available
> in_proximity0_raw
> name
> of_node
> power
> subsystem
> uevent
> 
> Signed-off-by: Brian Masney 
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/tsl2x7x.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 9991b0483956..eeccfbb0eb1f 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -827,7 +827,7 @@ in_illuminance0_calibscale_available_show(struct device 
> *dev,
>  
>  static IIO_CONST_ATTR(in_proximity0_calibscale_available, "1 2 4 8");
>  
> -static IIO_CONST_ATTR(in_illuminance0_integration_time_available,
> +static IIO_CONST_ATTR(in_intensity0_integration_time_available,
>   ".00272 - .696");
>  
>  static ssize_t in_illuminance0_target_input_show(struct device *dev,
> @@ -1358,7 +1358,7 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void 
> *private)
>  
>  static struct attribute *tsl2x7x_ALS_device_attrs[] = {
>   _attr_in_illuminance0_calibscale_available.attr,
> - _const_attr_in_illuminance0_integration_time_available
> + _const_attr_in_intensity0_integration_time_available
>   .dev_attr.attr,
>   _attr_in_illuminance0_target_input.attr,
>   _attr_in_illuminance0_calibrate.attr,
> @@ -1373,7 +1373,7 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
>  
>  static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
>   _attr_in_illuminance0_calibscale_available.attr,
> - _const_attr_in_illuminance0_integration_time_available
> + _const_attr_in_intensity0_integration_time_available
>   .dev_attr.attr,
>   _attr_in_illuminance0_target_input.attr,
>   _attr_in_illuminance0_calibrate.attr,
> @@ -1389,7 +1389,7 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
>  
>  static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
>   _attr_in_illuminance0_calibscale_available.attr,
> - _const_attr_in_illuminance0_integration_time_available
> + _const_attr_in_intensity0_integration_time_available
>   .dev_attr.attr,
>   _attr_in_illuminance0_target_input.attr,
>   _attr_in_illuminance0_calibrate.attr,
> @@ -1489,13 +1489,13 @@ static const struct tsl2x7x_chip_info 
> tsl2x7x_chip_info_tbl[] = {
>   .type = IIO_LIGHT,
>   .indexed = 1,
>   .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> -   BIT(IIO_CHAN_INFO_INT_TIME),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>   }, {
>   .type = IIO_INTENSITY,
>   .indexed = 1,
>   .channel = 0,
>   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_INT_TIME) |
>   BIT(IIO_CHAN_INFO_CALIBSCALE) |
>   BIT(IIO_CHAN_INFO_CALIBBIAS),
>   .event_spec = tsl2x7x_events,
> @@ -1529,13 +1529,13 @@ static const struct tsl2x7x_chip_info 
> tsl2x7x_chip_info_tbl[] = {
>   .type = IIO_LIGHT,
>   .indexed = 1,
>   .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> -   BIT(IIO_CHAN_INFO_INT_TIME),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>   }, {
>   .type = IIO_INTENSITY,
>   .indexed = 1,
>   .channel = 0,
>   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_INT_TIME) |
>   BIT(IIO_CHAN_INFO_CALIBSCALE) |
>   BIT(IIO_CHAN_INFO_CALIBBIAS),
>   .event_spec = tsl2x7x_events,
> @@ -1578,13 +1578,13 @@ static const struct tsl2x7x_chip_info 

Re: [PATCH 48/61] staging: iio: adc: simplify getting .drvdata

[Jonathan Cameron]

On Thu, 19 Apr 2018 16:06:18 +0200
Wolfram Sang  wrote:

> We should get drvdata from struct device directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang 
Applied, thanks,

Jonathan

> ---
> 
> Build tested only. buildbot is happy. Please apply individually.
> 
>  drivers/staging/iio/adc/ad7606_par.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606_par.c 
> b/drivers/staging/iio/adc/ad7606_par.c
> index 3eb6f8f312dd..a34c2a1d5373 100644
> --- a/drivers/staging/iio/adc/ad7606_par.c
> +++ b/drivers/staging/iio/adc/ad7606_par.c
> @@ -18,8 +18,7 @@
>  static int ad7606_par16_read_block(struct device *dev,
>  int count, void *buf)
>  {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>   struct ad7606_state *st = iio_priv(indio_dev);
>  
>   insw((unsigned long)st->base_address, buf, count);
> @@ -34,8 +33,7 @@ static const struct ad7606_bus_ops ad7606_par16_bops = {
>  static int ad7606_par8_read_block(struct device *dev,
> int count, void *buf)
>  {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>   struct ad7606_state *st = iio_priv(indio_dev);
>  
>   insb((unsigned long)st->base_address, buf, count * 2);

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] stagging:iio:meter: Add ade7854_write_raw function

[Rodrigo Siqueira]

This patch adds the ade7854_write_raw() function which is responsible
for handling the write operation for registers: AIGAIN, BIGAIN, CIGAIN,
NIGAIN, AVGAIN, BVGAIN, and CVGAIN. Finally, this patch completely
removes the old ABI used for handling the registers mentioned above.

Signed-off-by: Rodrigo Siqueira 
---
 drivers/staging/iio/meter/ade7854.c | 60 -
 1 file changed, 25 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7854.c 
b/drivers/staging/iio/meter/ade7854.c
index 242ecde75900..df19c8b4b5d7 100644
--- a/drivers/staging/iio/meter/ade7854.c
+++ b/drivers/staging/iio/meter/ade7854.c
@@ -228,34 +228,6 @@ static int ade7854_reset(struct device *dev)
return st->write_reg(dev, ADE7854_CONFIG, val, 16);
 }
 
-static IIO_DEV_ATTR_AIGAIN(0644,
-   NULL,
-   ade7854_write_24bit,
-   ADE7854_AIGAIN);
-static IIO_DEV_ATTR_BIGAIN(0644,
-   NULL,
-   ade7854_write_24bit,
-   ADE7854_BIGAIN);
-static IIO_DEV_ATTR_CIGAIN(0644,
-   NULL,
-   ade7854_write_24bit,
-   ADE7854_CIGAIN);
-static IIO_DEV_ATTR_NIGAIN(0644,
-   NULL,
-   ade7854_write_24bit,
-   ADE7854_NIGAIN);
-static IIO_DEV_ATTR_AVGAIN(0644,
-   NULL,
-   ade7854_write_24bit,
-   ADE7854_AVGAIN);
-static IIO_DEV_ATTR_BVGAIN(0644,
-   NULL,
-   ade7854_write_24bit,
-   ADE7854_BVGAIN);
-static IIO_DEV_ATTR_CVGAIN(0644,
-   NULL,
-   ade7854_write_24bit,
-   ADE7854_CVGAIN);
 static IIO_DEV_ATTR_APPARENT_POWER_A_GAIN(0644,
ade7854_read_24bit,
ade7854_write_24bit,
@@ -497,6 +469,30 @@ static int ade7854_read_raw(struct iio_dev *indio_dev,
return -EINVAL;
 }
 
+static int ade7854_write_raw(struct iio_dev *indio_dev,
+struct iio_chan_spec const *chan,
+int val, int val2, long mask)
+{
+   struct ade7854_state *st = iio_priv(indio_dev);
+   int ret;
+
+   if (mask != IIO_CHAN_INFO_SCALE)
+   return -EINVAL;
+
+   switch (chan->type) {
+   case IIO_CURRENT:
+   case IIO_VOLTAGE:
+   ret = st->write_reg(_dev->dev, chan->address, val, 24);
+   if (ret < 0)
+   return ret;
+   return 0;
+   default:
+   break;
+   }
+
+   return -EINVAL;
+}
+
 static int ade7854_initial_setup(struct iio_dev *indio_dev)
 {
int ret;
@@ -521,13 +517,6 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("8000");
 static IIO_CONST_ATTR(name, "ade7854");
 
 static struct attribute *ade7854_attributes[] = {
-   _dev_attr_aigain.dev_attr.attr,
-   _dev_attr_bigain.dev_attr.attr,
-   _dev_attr_cigain.dev_attr.attr,
-   _dev_attr_nigain.dev_attr.attr,
-   _dev_attr_avgain.dev_attr.attr,
-   _dev_attr_bvgain.dev_attr.attr,
-   _dev_attr_cvgain.dev_attr.attr,
_dev_attr_linecyc.dev_attr.attr,
_dev_attr_sagcyc.dev_attr.attr,
_dev_attr_cfcyc.dev_attr.attr,
@@ -599,6 +588,7 @@ static const struct attribute_group ade7854_attribute_group 
= {
 static const struct iio_info ade7854_info = {
.attrs = _attribute_group,
.read_raw = _read_raw,
+   .write_raw = _write_raw,
 };
 
 int ade7854_probe(struct iio_dev *indio_dev, struct device *dev)
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] stagging:iio:meter: Add ade7854_read_raw function

[Rodrigo Siqueira]

This patch adds the ade7854_read_raw() function which is responsible for
handling the read operation for registers: AIGAIN, BIGAIN, CIGAIN,
NIGAIN, AVGAIN, BVGAIN, and CVGAIN. For the sake of simplicity, this
patch only adds basic manipulation for current and voltage channels.
Finally, this patch disables the old approach for reading data.

Signed-off-by: Rodrigo Siqueira 
---
 drivers/staging/iio/meter/ade7854.c | 41 -
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7854.c 
b/drivers/staging/iio/meter/ade7854.c
index 2fbb2570ba54..242ecde75900 100644
--- a/drivers/staging/iio/meter/ade7854.c
+++ b/drivers/staging/iio/meter/ade7854.c
@@ -229,31 +229,31 @@ static int ade7854_reset(struct device *dev)
 }
 
 static IIO_DEV_ATTR_AIGAIN(0644,
-   ade7854_read_24bit,
+   NULL,
ade7854_write_24bit,
ADE7854_AIGAIN);
 static IIO_DEV_ATTR_BIGAIN(0644,
-   ade7854_read_24bit,
+   NULL,
ade7854_write_24bit,
ADE7854_BIGAIN);
 static IIO_DEV_ATTR_CIGAIN(0644,
-   ade7854_read_24bit,
+   NULL,
ade7854_write_24bit,
ADE7854_CIGAIN);
 static IIO_DEV_ATTR_NIGAIN(0644,
-   ade7854_read_24bit,
+   NULL,
ade7854_write_24bit,
ADE7854_NIGAIN);
 static IIO_DEV_ATTR_AVGAIN(0644,
-   ade7854_read_24bit,
+   NULL,
ade7854_write_24bit,
ADE7854_AVGAIN);
 static IIO_DEV_ATTR_BVGAIN(0644,
-   ade7854_read_24bit,
+   NULL,
ade7854_write_24bit,
ADE7854_BVGAIN);
 static IIO_DEV_ATTR_CVGAIN(0644,
-   ade7854_read_24bit,
+   NULL,
ade7854_write_24bit,
ADE7854_CVGAIN);
 static IIO_DEV_ATTR_APPARENT_POWER_A_GAIN(0644,
@@ -471,6 +471,32 @@ static int ade7854_set_irq(struct device *dev, bool enable)
return st->write_reg(dev, ADE7854_MASK0, irqen, 32);
 }
 
+static int ade7854_read_raw(struct iio_dev *indio_dev,
+   struct iio_chan_spec const *chan,
+   int *val,
+   int *val2,
+   long mask)
+{
+   struct ade7854_state *st = iio_priv(indio_dev);
+   int ret;
+
+   if (mask != IIO_CHAN_INFO_SCALE)
+   return -EINVAL;
+
+   switch (chan->type) {
+   case IIO_CURRENT:
+   case IIO_VOLTAGE:
+   ret = st->read_reg(_dev->dev, chan->address, val, 24);
+   if (ret < 0)
+   return ret;
+   return IIO_VAL_INT;
+   default:
+   break;
+   }
+
+   return -EINVAL;
+}
+
 static int ade7854_initial_setup(struct iio_dev *indio_dev)
 {
int ret;
@@ -572,6 +598,7 @@ static const struct attribute_group ade7854_attribute_group 
= {
 
 static const struct iio_info ade7854_info = {
.attrs = _attribute_group,
+   .read_raw = _read_raw,
 };
 
 int ade7854_probe(struct iio_dev *indio_dev, struct device *dev)
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] stagging:iio:meter: Add iio_chan_spec

[Rodrigo Siqueira]

This patch adds iio_chan_spec struct. Additionally, the channel adds the
support for handling AIGAIN, BIGAIN, CIGAIN, NIGAIN, AVGAIN, BVGAIN, and
CVGAIN.

Signed-off-by: Rodrigo Siqueira 
---
 drivers/staging/iio/meter/ade7854.c | 42 +
 1 file changed, 42 insertions(+)

diff --git a/drivers/staging/iio/meter/ade7854.c 
b/drivers/staging/iio/meter/ade7854.c
index 029c3bf42d4d..2fbb2570ba54 100644
--- a/drivers/staging/iio/meter/ade7854.c
+++ b/drivers/staging/iio/meter/ade7854.c
@@ -22,6 +22,48 @@
 #include "meter.h"
 #include "ade7854.h"
 
+#define PHASEA "phaseA"
+#define PHASEB "phaseB"
+#define PHASEC "phaseC"
+#define NEUTRAL "neutral"
+
+#define ADE7854_CHANNEL(_type, _name, _mask, _reg) {   \
+   .type = _type,  \
+   .indexed = 1,   \
+   .channel = 0,   \
+   .extend_name = _name,   \
+   .info_mask_separate = _mask,\
+   .address = _reg,\
+   .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
+   .scan_index = -1,   \
+}
+
+static const struct iio_chan_spec ade7854_channels[] = {
+   /* Current */
+   ADE7854_CHANNEL(IIO_CURRENT, PHASEA,
+   BIT(IIO_CHAN_INFO_SCALE),
+   ADE7854_AIGAIN),
+   ADE7854_CHANNEL(IIO_CURRENT, PHASEB,
+   BIT(IIO_CHAN_INFO_SCALE),
+   ADE7854_BIGAIN),
+   ADE7854_CHANNEL(IIO_CURRENT, PHASEC,
+   BIT(IIO_CHAN_INFO_SCALE),
+   ADE7854_CIGAIN),
+   ADE7854_CHANNEL(IIO_CURRENT, NEUTRAL,
+   BIT(IIO_CHAN_INFO_SCALE),
+   ADE7854_NIGAIN),
+   /* Voltage */
+   ADE7854_CHANNEL(IIO_VOLTAGE, PHASEA,
+   BIT(IIO_CHAN_INFO_SCALE),
+   ADE7854_AVGAIN),
+   ADE7854_CHANNEL(IIO_VOLTAGE, PHASEB,
+   BIT(IIO_CHAN_INFO_SCALE),
+   ADE7854_BVGAIN),
+   ADE7854_CHANNEL(IIO_VOLTAGE, PHASEC,
+   BIT(IIO_CHAN_INFO_SCALE),
+   ADE7854_CVGAIN),
+};
+
 static ssize_t ade7854_read_8bit(struct device *dev,
 struct device_attribute *attr,
 char *buf)
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/3] stagging:iio:meter: Add essential IIO API structures for ADE7854

[Rodrigo Siqueira]

This patchset aims to update ADE7854 by adding the required IIO API
components. The first patch adds the iio_chan_spec for handling seven
different registers (all of them with a similar behavior). The second
patch appends the read_raw function defined by the IIO API. Finally, the
third patch adds the write_raw function and remove the attributes used
for handling the seven registers. This patchset has the contribution of
John Syne, which was responsible for mapping the correct ABI name per
element in the ADE7854; additionally, John provided codes that helped to
shape these patches.

Rodrigo Siqueira (3):
  stagging:iio:meter: Add iio_chan_spec
  stagging:iio:meter: Add ade7854_read_raw function
  stagging:iio:meter: Add ade7854_write_raw function

 drivers/staging/iio/meter/ade7854.c | 129 
 1 file changed, 94 insertions(+), 35 deletions(-)

-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/8] [media] v4l: rcar_fdp1: Change platform dependency to ARCH_RENESAS

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Friday, 20 April 2018 16:28:29 EEST Geert Uytterhoeven wrote:
> The Renesas Fine Display Processor driver is used on Renesas R-Car SoCs
> only.  Since commit 9b5ba0df4ea4f940 ("ARM: shmobile: Introduce
> ARCH_RENESAS") is ARCH_RENESAS a more appropriate platform dependency
> than the legacy ARCH_SHMOBILE, hence use the former.
> 
> This will allow to drop ARCH_SHMOBILE on ARM and ARM64 in the near
> future.
> 
> Signed-off-by: Geert Uytterhoeven 

Acked-by: Laurent Pinchart 

How would you like to get this merged ?

> ---
>  drivers/media/platform/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index f9235e8f8e962d2e..7ad4725f9d1f9627 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -396,7 +396,7 @@ config VIDEO_SH_VEU
>  config VIDEO_RENESAS_FDP1
>   tristate "Renesas Fine Display Processor"
>   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
> - depends on ARCH_SHMOBILE || COMPILE_TEST
> + depends on ARCH_RENESAS || COMPILE_TEST
>   depends on (!ARCH_RENESAS && !VIDEO_RENESAS_FCP) || VIDEO_RENESAS_FCP
>   select VIDEOBUF2_DMA_CONTIG
>   select V4L2_MEM2MEM_DEV

-- 
Regards,

Laurent Pinchart



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel