| Message ID | 8e171d819a0916d7a4870bf70cfeada8ba9ebdc6.1524432236.git.davidjulianveenstra@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 23 Apr 2018 00:03:03 +0200 David Veenstra <davidjulianveenstra@gmail.com> 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 <davidjulianveenstra@gmail.com> Looks good to me. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > Changes in v3: > - change type of rx to __be16. > - remove unneeded variable vel. > - remove unneeded type cast to s16 (patch line 79). > > drivers/staging/iio/resolver/ad2s1200.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c > index 17d65cb437fd..998f458dc1bd 100644 > --- a/drivers/staging/iio/resolver/ad2s1200.c > +++ b/drivers/staging/iio/resolver/ad2s1200.c > @@ -47,7 +47,7 @@ struct ad2s1200_state { > struct spi_device *sdev; > int sample; > int rdvel; > - u8 rx[2] ____cacheline_aligned; > + __be16 rx ____cacheline_aligned; > }; > > static int ad2s1200_read_raw(struct iio_dev *indio_dev, > @@ -58,7 +58,6 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev, > { > struct ad2s1200_state *st = iio_priv(indio_dev); > int ret = 0; > - s16 vel; > > mutex_lock(&st->lock); > gpio_set_value(st->sample, 0); > @@ -68,7 +67,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev, > gpio_set_value(st->sample, 1); > gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL)); > > - ret = spi_read(st->sdev, st->rx, 2); > + ret = spi_read(st->sdev, &st->rx, 2); > if (ret < 0) { > mutex_unlock(&st->lock); > return ret; > @@ -76,12 +75,10 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev, > > switch (chan->type) { > case IIO_ANGL: > - *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4); > + *val = be16_to_cpup(&st->rx) >> 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(be16_to_cpup(&st->rx) >> 4, 11); > break; > default: > mutex_unlock(&st->lock); -- 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
diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c index 17d65cb437fd..998f458dc1bd 100644 --- a/drivers/staging/iio/resolver/ad2s1200.c +++ b/drivers/staging/iio/resolver/ad2s1200.c @@ -47,7 +47,7 @@ struct ad2s1200_state { struct spi_device *sdev; int sample; int rdvel; - u8 rx[2] ____cacheline_aligned; + __be16 rx ____cacheline_aligned; }; static int ad2s1200_read_raw(struct iio_dev *indio_dev, @@ -58,7 +58,6 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev, { struct ad2s1200_state *st = iio_priv(indio_dev); int ret = 0; - s16 vel; mutex_lock(&st->lock); gpio_set_value(st->sample, 0); @@ -68,7 +67,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev, gpio_set_value(st->sample, 1); gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL)); - ret = spi_read(st->sdev, st->rx, 2); + ret = spi_read(st->sdev, &st->rx, 2); if (ret < 0) { mutex_unlock(&st->lock); return ret; @@ -76,12 +75,10 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev, switch (chan->type) { case IIO_ANGL: - *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4); + *val = be16_to_cpup(&st->rx) >> 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(be16_to_cpup(&st->rx) >> 4, 11); break; default: mutex_unlock(&st->lock);
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 <davidjulianveenstra@gmail.com> --- Changes in v3: - change type of rx to __be16. - remove unneeded variable vel. - remove unneeded type cast to s16 (patch line 79). drivers/staging/iio/resolver/ad2s1200.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)