From: Ivaylo Dimitrov <[email protected]>
Correctly initialize device channels, otherwise writing the calibration
values to sysfs nodes does not work
Signed-off-by: Ivaylo Dimitrov <[email protected]>
---
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
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)
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
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
>
From: Ivaylo Dimitrov <[email protected]>
Use the correct channel2 member instead of channel when dealing with sysfs
reads/writes
Signed-off-by: Ivaylo Dimitrov <[email protected]>
---
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
> Use the correct channel2 member instead of channel when dealing with sysfs
> reads/writes
> Signed-off-by: Ivaylo Dimitrov <[email protected]>
as a bonus, m is renamed to mask
Acked-by: Peter Meerwald <[email protected]>
> ---
> 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)
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 <[email protected]>
>
> 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 <[email protected]>
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);
>>
>
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 <[email protected]>
>>
>> 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 <[email protected]>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html