On Mon, 23 Apr 2018 00:03:03 +0200 David Veenstra <davidjulianveenstra@xxxxxxxxx> 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@xxxxxxxxx> 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); _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel