[PATCH] iio: accel: kxsd9: fix the usage of spi_w8r8()

[PATCH] iio: accel: kxsd9: fix the usage of spi_w8r8()

[Linus Walleij]

These two spi_w8r8() calls return a value with is used by the code
following the error check. The dubious use was caused by a cleanup
patch.

Fixes: d34dbee8ac8e ("staging:iio:accel:kxsd9 cleanup and conversion to iio_chan_spec.")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Jonathan: I suspect the driver was never tested after the cleanup?
I'm trying to use the driver to bolt on I2C support, but I'm a bit
suspicious that it's not really working for SPI even.
---
 drivers/iio/accel/kxsd9.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 923f56598d4b..3a9f106787d2 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -81,7 +81,7 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 
 	mutex_lock(&st->buf_lock);
 	ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
-	if (ret)
+	if (ret < 0)
 		goto error_ret;
 	st->tx[0] = KXSD9_WRITE(KXSD9_REG_CTRL_C);
 	st->tx[1] = (ret & ~KXSD9_FS_MASK) | i;
@@ -163,7 +163,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 		break;
 	case IIO_CHAN_INFO_SCALE:
 		ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
-		if (ret)
+		if (ret < 0)
 			goto error_ret;
 		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
 		ret = IIO_VAL_INT_PLUS_MICRO;
-- 
2.4.11

Re: [PATCH] iio: accel: kxsd9: fix the usage of spi_w8r8()

[Jonathan Cameron]

On 17 June 2016 14:22:24 BST, Linus Walleij <linus.walleij@linaro.org> wrote:
>These two spi_w8r8() calls return a value with is used by the code
>following the error check. The dubious use was caused by a cleanup
>patch.
>
>Fixes: d34dbee8ac8e ("staging:iio:accel:kxsd9 cleanup and conversion to
>iio_chan_spec.")
>Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>---
>Jonathan: I suspect the driver was never tested after the cleanup?
>I'm trying to use the driver to bolt on I2C support, but I'm a bit
>suspicious that it's not really working for SPI even.

Sadly it is more than possible. My test bodge rig for this one was down for a long time. (Hides guilty expression)
Most of it is back running now as I was looking at the sca3000 driver recently
 which used the same mashup (with another adapter cable).

Pretty simple device iirc so shouldn't be too hard to fix. Original driver got written
 in about a day long ago...

Where did you find one of these. I thought they were long obsoleted?

J
>---
> drivers/iio/accel/kxsd9.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
>index 923f56598d4b..3a9f106787d2 100644
>--- a/drivers/iio/accel/kxsd9.c
>+++ b/drivers/iio/accel/kxsd9.c
>@@ -81,7 +81,7 @@ static int kxsd9_write_scale(struct iio_dev
>*indio_dev, int micro)
> 
> 	mutex_lock(&st->buf_lock);
> 	ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
>-	if (ret)
>+	if (ret < 0)
> 		goto error_ret;
> 	st->tx[0] = KXSD9_WRITE(KXSD9_REG_CTRL_C);
> 	st->tx[1] = (ret & ~KXSD9_FS_MASK) | i;
>@@ -163,7 +163,7 @@ static int kxsd9_read_raw(struct iio_dev
>*indio_dev,
> 		break;
> 	case IIO_CHAN_INFO_SCALE:
> 		ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
>-		if (ret)
>+		if (ret < 0)
> 			goto error_ret;
> 		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
> 		ret = IIO_VAL_INT_PLUS_MICRO;

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH] iio: accel: kxsd9: fix the usage of spi_w8r8()

[Linus Walleij]

On Fri, Jun 17, 2016 at 6:22 PM, Jonathan Cameron
<jic23@jic23.retrosnub.co.uk> wrote:

> Where did you find one of these. I thought they were long obsoleted?

The original Qualcomm APQ8060 Dragonboard. I'm mainlining it as a test
target right now, mostly for GPIO work:
http://dflund.se/~triad/krad/dragonboard/dragonboard.jpg

It has the following on I2C on a special sensor board (totally undocumented
and no real mention in even the source code in the BSP for the board for
some of them, I guess they never got around to enable some of them):

- Kiorin KXSD9 accelerometer - this culprit

- Bosch BM085 pressure sensor/altimeter - works flawlessly with upstream
driver and a few patches (just sent).

- Capella CM3605 proximity and ambient light sensor (same as used on the
original Nexus One, yay) this require you to turn on a special IR LED for
the proximity sensor to work (!)

- AK8975 Compass/Magnetometer - just works nicely with the upstream
driver and a minor patch for an extra regulator

- Invensense MPU3050 Gyroscope
drivers/input/misc/mpu3050.c
Thinking about just moving this driver across if it's working as a starting
point, like I did with the BH1780 driver. Is this one of those who would need
a IIO <-> input bridge if we migrate it?

Yours,
Linus Walleij

Re: [PATCH] iio: accel: kxsd9: fix the usage of spi_w8r8()

[Jonathan Cameron]

On 17/06/16 17:22, Jonathan Cameron wrote:
> 
> 
> On 17 June 2016 14:22:24 BST, Linus Walleij <linus.walleij@linaro.org> wrote:
>> These two spi_w8r8() calls return a value with is used by the code
>> following the error check. The dubious use was caused by a cleanup
>> patch.
>>
>> Fixes: d34dbee8ac8e ("staging:iio:accel:kxsd9 cleanup and conversion to
>> iio_chan_spec.")
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
This patch is 'obviously' correct so applied and marked for stable.

Thanks,

Jonathan
>> ---
>> Jonathan: I suspect the driver was never tested after the cleanup?
>> I'm trying to use the driver to bolt on I2C support, but I'm a bit
>> suspicious that it's not really working for SPI even.
> 
> Sadly it is more than possible. My test bodge rig for this one was down for a long time. (Hides guilty expression)
> Most of it is back running now as I was looking at the sca3000 driver recently
>  which used the same mashup (with another adapter cable).
> 
> Pretty simple device iirc so shouldn't be too hard to fix. Original driver got written
>  in about a day long ago...
> 
> Where did you find one of these. I thought they were long obsoleted?
> 
> J
>> ---
>> drivers/iio/accel/kxsd9.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
>> index 923f56598d4b..3a9f106787d2 100644
>> --- a/drivers/iio/accel/kxsd9.c
>> +++ b/drivers/iio/accel/kxsd9.c
>> @@ -81,7 +81,7 @@ static int kxsd9_write_scale(struct iio_dev
>> *indio_dev, int micro)
>>
>> 	mutex_lock(&st->buf_lock);
>> 	ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
>> -	if (ret)
>> +	if (ret < 0)
>> 		goto error_ret;
>> 	st->tx[0] = KXSD9_WRITE(KXSD9_REG_CTRL_C);
>> 	st->tx[1] = (ret & ~KXSD9_FS_MASK) | i;
>> @@ -163,7 +163,7 @@ static int kxsd9_read_raw(struct iio_dev
>> *indio_dev,
>> 		break;
>> 	case IIO_CHAN_INFO_SCALE:
>> 		ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
>> -		if (ret)
>> +		if (ret < 0)
>> 			goto error_ret;
>> 		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
>> 		ret = IIO_VAL_INT_PLUS_MICRO;
>