[PATCH] iio: tsl2563: Initialize channels

[PATCH] iio: tsl2563: Initialize channels

[Ivaylo Dimitrov]

From: Ivaylo Dimitrov <freemangordon@abv.bg>

Correctly initialize device channels, otherwise writing the calibration
values to sysfs nodes does not work

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/iio/light/tsl2563.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index 5e5d9de..a663ea2 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -460,7 +460,7 @@ static int tsl2563_write_raw(struct iio_dev *indio_dev,
 {
 	struct tsl2563_chip *chip = iio_priv(indio_dev);
 
-	if (chan->channel == IIO_MOD_LIGHT_BOTH)
+	if (chan->channel == 0)
 		chip->calib0 = calib_from_sysfs(val);
 	else
 		chip->calib1 = calib_from_sysfs(val);
@@ -543,11 +543,12 @@ static const struct iio_event_spec tsl2563_events[] = {
 static const struct iio_chan_spec tsl2563_channels[] = {
 	{
 		.type = IIO_LIGHT,
+		.channel = 0,
 		.indexed = 1,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
-		.channel = 0,
 	}, {
 		.type = IIO_INTENSITY,
+		.channel = 0,
 		.modified = 1,
 		.channel2 = IIO_MOD_LIGHT_BOTH,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
@@ -556,6 +557,7 @@ static const struct iio_chan_spec tsl2563_channels[] = {
 		.num_event_specs = ARRAY_SIZE(tsl2563_events),
 	}, {
 		.type = IIO_INTENSITY,
+		.channel = 1,
 		.modified = 1,
 		.channel2 = IIO_MOD_LIGHT_IR,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-- 
1.5.6.1

Re: [PATCH] iio: tsl2563: Initialize channels

[Peter Meerwald]

Hello,

> Correctly initialize device channels, otherwise writing the calibration
> values to sysfs nodes does not work

I think the fix should rather be

	if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
		chip->calib0 = calib_from_sysfs(val);
	else if (chan->channel2 == IIO_MOD_LIGHT_IR)
		chip->calib1 = calib_from_sysfs(val);
	else
		return -EINVAL;

since only the INTENSITY channels have a CALIBSCALE info

the light channel incorrectly sets .indexed = 1 and .channel = 0 which 
should be unnecessary since there is only one IIO_LIGHT channel

regards, p.

> ---
>  drivers/iio/light/tsl2563.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
> index 5e5d9de..a663ea2 100644
> --- a/drivers/iio/light/tsl2563.c
> +++ b/drivers/iio/light/tsl2563.c
> @@ -460,7 +460,7 @@ static int tsl2563_write_raw(struct iio_dev *indio_dev,
>  {
>  	struct tsl2563_chip *chip = iio_priv(indio_dev);
>  
> -	if (chan->channel == IIO_MOD_LIGHT_BOTH)
> +	if (chan->channel == 0)
>  		chip->calib0 = calib_from_sysfs(val);
>  	else
>  		chip->calib1 = calib_from_sysfs(val);
> @@ -543,11 +543,12 @@ static const struct iio_event_spec tsl2563_events[] = {
>  static const struct iio_chan_spec tsl2563_channels[] = {
>  	{
>  		.type = IIO_LIGHT,
> +		.channel = 0,
>  		.indexed = 1,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> -		.channel = 0,
>  	}, {
>  		.type = IIO_INTENSITY,
> +		.channel = 0,
>  		.modified = 1,
>  		.channel2 = IIO_MOD_LIGHT_BOTH,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> @@ -556,6 +557,7 @@ static const struct iio_chan_spec tsl2563_channels[] = {
>  		.num_event_specs = ARRAY_SIZE(tsl2563_events),
>  	}, {
>  		.type = IIO_INTENSITY,
> +		.channel = 1,
>  		.modified = 1,
>  		.channel2 = IIO_MOD_LIGHT_IR,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH] iio: tsl2563: Initialize channels

[Ivaylo Dimitrov]

On 12.01.2014 22:55, Peter Meerwald wrote:
>
> I think the fix should rather be
>
> 	if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
> 		chip->calib0 = calib_from_sysfs(val);
> 	else if (chan->channel2 == IIO_MOD_LIGHT_IR)
> 		chip->calib1 = calib_from_sysfs(val);
> 	else
> 		return -EINVAL;
>
> since only the INTENSITY channels have a CALIBSCALE info
>
> the light channel incorrectly sets .indexed = 1 and .channel = 0 which
> should be unnecessary since there is only one IIO_LIGHT channel
>
> regards, p.
>

I thought so when I was preparing the patch, but unfortunately couldn't 
find any documentation for iio subsystem structs and defines, besides 
what is in the header files (and it is not very descriptive), so I took 
the easy route.

However, I'll send a new patch with the above (and a little more) changes.

Thanks,
Ivo

Re: [PATCH] iio: tsl2563: Initialize channels

[Jonathan Cameron]

On 12/01/14 21:08, Ivaylo Dimitrov wrote:
> On 12.01.2014 22:55, Peter Meerwald wrote:
>>
>> I think the fix should rather be
>>
>>     if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>>         chip->calib0 = calib_from_sysfs(val);
>>     else if (chan->channel2 == IIO_MOD_LIGHT_IR)
>>         chip->calib1 = calib_from_sysfs(val);
>>     else
>>         return -EINVAL;
>>
>> since only the INTENSITY channels have a CALIBSCALE info
>>
>> the light channel incorrectly sets .indexed = 1 and .channel = 0 which
>> should be unnecessary since there is only one IIO_LIGHT channel
>>
>> regards, p.
>>
>
> I thought so when I was preparing the patch, but unfortunately couldn't find any documentation for iio subsystem structs and defines, besides what is in the header files (and it is not very descriptive), so I took the easy route.
>
> However, I'll send a new patch with the above (and a little more) changes.
Leave the indexed bit alone.  Whilst unnecessary it is fine within the ABI
and changing it would be a userspace ABI change.

Otherwise, as described by Peter will be fine.

Jonathan
>
> Thanks,
> Ivo
>

[PATCH] iio: tsl2563: Use the correct channel2 member

[Ivaylo Dimitrov]

From: Ivaylo Dimitrov <freemangordon@abv.bg>

Use the correct channel2 member instead of channel when dealing with sysfs
reads/writes

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/iio/light/tsl2563.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index 5e5d9de..2be6df3 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -460,10 +460,14 @@ static int tsl2563_write_raw(struct iio_dev *indio_dev,
 {
 	struct tsl2563_chip *chip = iio_priv(indio_dev);
 
-	if (chan->channel == IIO_MOD_LIGHT_BOTH)
+	if (mask != IIO_CHAN_INFO_CALIBSCALE)
+		return -EINVAL;
+	if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
 		chip->calib0 = calib_from_sysfs(val);
-	else
+	else if (chan->channel2 == IIO_MOD_LIGHT_IR)
 		chip->calib1 = calib_from_sysfs(val);
+	else
+		return -EINVAL;
 
 	return 0;
 }
@@ -472,14 +476,14 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val,
 			    int *val2,
-			    long m)
+			    long mask)
 {
 	int ret = -EINVAL;
 	u32 calib0, calib1;
 	struct tsl2563_chip *chip = iio_priv(indio_dev);
 
 	mutex_lock(&chip->lock);
-	switch (m) {
+	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 	case IIO_CHAN_INFO_PROCESSED:
 		switch (chan->type) {
@@ -498,7 +502,7 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
 			ret = tsl2563_get_adc(chip);
 			if (ret)
 				goto error_ret;
-			if (chan->channel == 0)
+			if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
 				*val = chip->data0;
 			else
 				*val = chip->data1;
@@ -510,7 +514,7 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
 		break;
 
 	case IIO_CHAN_INFO_CALIBSCALE:
-		if (chan->channel == 0)
+		if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
 			*val = calib_to_sysfs(chip->calib0);
 		else
 			*val = calib_to_sysfs(chip->calib1);
-- 
1.5.6.1

Re: [PATCH] iio: tsl2563: Use the correct channel2 member

[Peter Meerwald]

> Use the correct channel2 member instead of channel when dealing with sysfs
> reads/writes
 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

as a bonus, m is renamed to mask

Acked-by: Peter Meerwald <pmeerw@pmeerw.net>

> ---
>  drivers/iio/light/tsl2563.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
> index 5e5d9de..2be6df3 100644
> --- a/drivers/iio/light/tsl2563.c
> +++ b/drivers/iio/light/tsl2563.c
> @@ -460,10 +460,14 @@ static int tsl2563_write_raw(struct iio_dev *indio_dev,
>  {
>  	struct tsl2563_chip *chip = iio_priv(indio_dev);
>  
> -	if (chan->channel == IIO_MOD_LIGHT_BOTH)
> +	if (mask != IIO_CHAN_INFO_CALIBSCALE)
> +		return -EINVAL;
> +	if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>  		chip->calib0 = calib_from_sysfs(val);
> -	else
> +	else if (chan->channel2 == IIO_MOD_LIGHT_IR)
>  		chip->calib1 = calib_from_sysfs(val);
> +	else
> +		return -EINVAL;
>  
>  	return 0;
>  }
> @@ -472,14 +476,14 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
>  			    int *val,
>  			    int *val2,
> -			    long m)
> +			    long mask)
>  {
>  	int ret = -EINVAL;
>  	u32 calib0, calib1;
>  	struct tsl2563_chip *chip = iio_priv(indio_dev);
>  
>  	mutex_lock(&chip->lock);
> -	switch (m) {
> +	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
>  	case IIO_CHAN_INFO_PROCESSED:
>  		switch (chan->type) {
> @@ -498,7 +502,7 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
>  			ret = tsl2563_get_adc(chip);
>  			if (ret)
>  				goto error_ret;
> -			if (chan->channel == 0)
> +			if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>  				*val = chip->data0;
>  			else
>  				*val = chip->data1;
> @@ -510,7 +514,7 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
>  		break;
>  
>  	case IIO_CHAN_INFO_CALIBSCALE:
> -		if (chan->channel == 0)
> +		if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>  			*val = calib_to_sysfs(chip->calib0);
>  		else
>  			*val = calib_to_sysfs(chip->calib1);
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH] iio: tsl2563: Use the correct channel2 member

[Jonathan Cameron]

On 13/01/14 21:25, Peter Meerwald wrote:
>
>> Use the correct channel2 member instead of channel when dealing with sysfs
>> reads/writes
>
>> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>
> as a bonus, m is renamed to mask
Which would have been relevant back when it was a mask.  Still it doesn't
make things worse, so never mind ;)  One day someone might be bored enough
to clean up all the places in read_raw etc where the variable is called mask
but is infact a straight number.
>
> Acked-by: Peter Meerwald <pmeerw@pmeerw.net>
Applied to the fixes-togreg branch of iio.git  - initially pushed out as testing
for the autobuilders to thoroughly hammer.
>
>> ---
>>   drivers/iio/light/tsl2563.c |   16 ++++++++++------
>>   1 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
>> index 5e5d9de..2be6df3 100644
>> --- a/drivers/iio/light/tsl2563.c
>> +++ b/drivers/iio/light/tsl2563.c
>> @@ -460,10 +460,14 @@ static int tsl2563_write_raw(struct iio_dev *indio_dev,
>>   {
>>   	struct tsl2563_chip *chip = iio_priv(indio_dev);
>>
>> -	if (chan->channel == IIO_MOD_LIGHT_BOTH)
>> +	if (mask != IIO_CHAN_INFO_CALIBSCALE)
>> +		return -EINVAL;
>> +	if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>>   		chip->calib0 = calib_from_sysfs(val);
>> -	else
>> +	else if (chan->channel2 == IIO_MOD_LIGHT_IR)
>>   		chip->calib1 = calib_from_sysfs(val);
>> +	else
>> +		return -EINVAL;
>>
>>   	return 0;
>>   }
>> @@ -472,14 +476,14 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
>>   			    struct iio_chan_spec const *chan,
>>   			    int *val,
>>   			    int *val2,
>> -			    long m)
>> +			    long mask)
>>   {
>>   	int ret = -EINVAL;
>>   	u32 calib0, calib1;
>>   	struct tsl2563_chip *chip = iio_priv(indio_dev);
>>
>>   	mutex_lock(&chip->lock);
>> -	switch (m) {
>> +	switch (mask) {
>>   	case IIO_CHAN_INFO_RAW:
>>   	case IIO_CHAN_INFO_PROCESSED:
>>   		switch (chan->type) {
>> @@ -498,7 +502,7 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
>>   			ret = tsl2563_get_adc(chip);
>>   			if (ret)
>>   				goto error_ret;
>> -			if (chan->channel == 0)
>> +			if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>>   				*val = chip->data0;
>>   			else
>>   				*val = chip->data1;
>> @@ -510,7 +514,7 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
>>   		break;
>>
>>   	case IIO_CHAN_INFO_CALIBSCALE:
>> -		if (chan->channel == 0)
>> +		if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>>   			*val = calib_to_sysfs(chip->calib0);
>>   		else
>>   			*val = calib_to_sysfs(chip->calib1);
>>
>

Re: [PATCH] iio: tsl2563: Use the correct channel2 member

[Jonathan Cameron]

On 18/01/14 11:37, Jonathan Cameron wrote:
>
>
> On 13/01/14 21:25, Peter Meerwald wrote:
>>
>>> Use the correct channel2 member instead of channel when dealing with sysfs
>>> reads/writes
>>
>>> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>>
>> as a bonus, m is renamed to mask
> Which would have been relevant back when it was a mask.  Still it doesn't
> make things worse, so never mind ;)  One day someone might be bored enough
> to clean up all the places in read_raw etc where the variable is called mask
> but is infact a straight number.
>>
>> Acked-by: Peter Meerwald <pmeerw@pmeerw.net>
> Applied to the fixes-togreg branch of iio.git  - initially pushed out as testing
> for the autobuilders to thoroughly hammer.
I'm dozing this morning.  This of course the fixes-togreg branch which doesn't get
separately pushed out for autobuilding fun and goes straight out as fixes-togreg!


>>
>>> ---
>>>   drivers/iio/light/tsl2563.c |   16 ++++++++++------
>>>   1 files changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
>>> index 5e5d9de..2be6df3 100644
>>> --- a/drivers/iio/light/tsl2563.c
>>> +++ b/drivers/iio/light/tsl2563.c
>>> @@ -460,10 +460,14 @@ static int tsl2563_write_raw(struct iio_dev *indio_dev,
>>>   {
>>>       struct tsl2563_chip *chip = iio_priv(indio_dev);
>>>
>>> -    if (chan->channel == IIO_MOD_LIGHT_BOTH)
>>> +    if (mask != IIO_CHAN_INFO_CALIBSCALE)
>>> +        return -EINVAL;
>>> +    if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>>>           chip->calib0 = calib_from_sysfs(val);
>>> -    else
>>> +    else if (chan->channel2 == IIO_MOD_LIGHT_IR)
>>>           chip->calib1 = calib_from_sysfs(val);
>>> +    else
>>> +        return -EINVAL;
>>>
>>>       return 0;
>>>   }
>>> @@ -472,14 +476,14 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
>>>                   struct iio_chan_spec const *chan,
>>>                   int *val,
>>>                   int *val2,
>>> -                long m)
>>> +                long mask)
>>>   {
>>>       int ret = -EINVAL;
>>>       u32 calib0, calib1;
>>>       struct tsl2563_chip *chip = iio_priv(indio_dev);
>>>
>>>       mutex_lock(&chip->lock);
>>> -    switch (m) {
>>> +    switch (mask) {
>>>       case IIO_CHAN_INFO_RAW:
>>>       case IIO_CHAN_INFO_PROCESSED:
>>>           switch (chan->type) {
>>> @@ -498,7 +502,7 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
>>>               ret = tsl2563_get_adc(chip);
>>>               if (ret)
>>>                   goto error_ret;
>>> -            if (chan->channel == 0)
>>> +            if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>>>                   *val = chip->data0;
>>>               else
>>>                   *val = chip->data1;
>>> @@ -510,7 +514,7 @@ static int tsl2563_read_raw(struct iio_dev *indio_dev,
>>>           break;
>>>
>>>       case IIO_CHAN_INFO_CALIBSCALE:
>>> -        if (chan->channel == 0)
>>> +        if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
>>>               *val = calib_to_sysfs(chip->calib0);
>>>           else
>>>               *val = calib_to_sysfs(chip->calib1);
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html