[PATCH] iio: tsl2563: Initialize channels

[Ivaylo Dimitrov]

From: Ivaylo Dimitrov <freema...@abv.bg>

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

Signed-off-by: Ivaylo Dimitrov <ivo.g.di...@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[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.

Peter Meerwald
+43-664-2444418 (mobile)

[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

[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

[Ivaylo Dimitrov]

From: Ivaylo Dimitrov <freema...@abv.bg>

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

Signed-off-by: Ivaylo Dimitrov <ivo.g.di...@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

[Peter Meerwald]

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

> Signed-off-by: Ivaylo Dimitrov <ivo.g.di...@gmail.com>

as a bonus, m is renamed to mask

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

Peter Meerwald
+43-664-2444418 (mobile)

[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.di...@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 <pme...@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!

> To unsubscribe from this list: send the line "unsubscribe linux-iio" in

[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.di...@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 <pme...@pmeerw.net>
Applied to the fixes-togreg branch of iio.git - initially pushed out as testing
for the autobuilders to thoroughly hammer.
>