| Message ID | 20200429072129.81504-1-alexandru.ardelean@analog.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: iio: ad2s1210: Fix SPI reading | expand |
On Wed, 29 Apr 2020 10:21:29 +0300 Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > From: Dragos Bogdan <dragos.bogdan@analog.com> > > If the serial interface is used, the 8-bit address should be latched using > the rising edge of the WR/FSYNC signal. > > This basically means that a CS change is required between the first byte > sent, and the second one. > This change splits the single-transfer transfer of 2 bytes into 2 transfers > with a single byte, and CS change in-between. > > Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> Fixes tag would have been nice. I've had a go by picking a patch where I refactored this code, but I think the issue probably predates that one. Its in 2011 so I doubt anyone will try going past that with backports ;) Applied to the fixes-togreg branch of iio.git and marked for stable. I'm guessing this means you have hardware and hope to get this one out of staging shortly? *crosses fingers* :) Jonathan > --- > drivers/staging/iio/resolver/ad2s1210.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c > index 4b25a3a314ed..ed404355ea4c 100644 > --- a/drivers/staging/iio/resolver/ad2s1210.c > +++ b/drivers/staging/iio/resolver/ad2s1210.c > @@ -130,17 +130,24 @@ static int ad2s1210_config_write(struct ad2s1210_state *st, u8 data) > static int ad2s1210_config_read(struct ad2s1210_state *st, > unsigned char address) > { > - struct spi_transfer xfer = { > - .len = 2, > - .rx_buf = st->rx, > - .tx_buf = st->tx, > + struct spi_transfer xfers[] = { > + { > + .len = 1, > + .rx_buf = &st->rx[0], > + .tx_buf = &st->tx[0], > + .cs_change = 1, > + }, { > + .len = 1, > + .rx_buf = &st->rx[1], > + .tx_buf = &st->tx[1], > + }, > }; > int ret = 0; > > ad2s1210_set_mode(MOD_CONFIG, st); > st->tx[0] = address | AD2S1210_MSB_IS_HIGH; > st->tx[1] = AD2S1210_REG_FAULT; > - ret = spi_sync_transfer(st->sdev, &xfer, 1); > + ret = spi_sync_transfer(st->sdev, xfers, 2); > if (ret < 0) > return ret; >
On Sun, 2020-05-03 at 12:37 +0100, Jonathan Cameron wrote: > [External] > > On Wed, 29 Apr 2020 10:21:29 +0300 > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > > From: Dragos Bogdan <dragos.bogdan@analog.com> > > > > If the serial interface is used, the 8-bit address should be latched using > > the rising edge of the WR/FSYNC signal. > > > > This basically means that a CS change is required between the first byte > > sent, and the second one. > > This change splits the single-transfer transfer of 2 bytes into 2 transfers > > with a single byte, and CS change in-between. > > > > Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > Fixes tag would have been nice. I've had a go by picking a patch where I > refactored this code, but I think the issue probably predates that one. > Its in 2011 so I doubt anyone will try going past that with backports ;) > Apologies again for not considering Fixes tag. At this point, I feel bad for repeating the apology, as it may sound like hollow words. But, I guess this could have skipped going through the fixes route. The patch has been living in our tree for a while. > Applied to the fixes-togreg branch of iio.git and marked for stable. > > I'm guessing this means you have hardware and hope to get this one out > of staging shortly? *crosses fingers* :) Sorry, but not at this point in time. I just fished this from our tree. I also handle our kernel upgrades [on our side], and whenever I do an update, some upstreamed patches disappear from our tree, and some stand-out and I wonder how come this wasn't sent upstream. This is one of them. I don't know if I'll be able to find someone [in the near future] to allocate this to [for moving out-of-staging]. Right now, the priority [on our side] is the high-speed converters. > > Jonathan > > > --- > > drivers/staging/iio/resolver/ad2s1210.c | 17 ++++++++++++----- > > 1 file changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/staging/iio/resolver/ad2s1210.c > > b/drivers/staging/iio/resolver/ad2s1210.c > > index 4b25a3a314ed..ed404355ea4c 100644 > > --- a/drivers/staging/iio/resolver/ad2s1210.c > > +++ b/drivers/staging/iio/resolver/ad2s1210.c > > @@ -130,17 +130,24 @@ static int ad2s1210_config_write(struct ad2s1210_state > > *st, u8 data) > > static int ad2s1210_config_read(struct ad2s1210_state *st, > > unsigned char address) > > { > > - struct spi_transfer xfer = { > > - .len = 2, > > - .rx_buf = st->rx, > > - .tx_buf = st->tx, > > + struct spi_transfer xfers[] = { > > + { > > + .len = 1, > > + .rx_buf = &st->rx[0], > > + .tx_buf = &st->tx[0], > > + .cs_change = 1, > > + }, { > > + .len = 1, > > + .rx_buf = &st->rx[1], > > + .tx_buf = &st->tx[1], > > + }, > > }; > > int ret = 0; > > > > ad2s1210_set_mode(MOD_CONFIG, st); > > st->tx[0] = address | AD2S1210_MSB_IS_HIGH; > > st->tx[1] = AD2S1210_REG_FAULT; > > - ret = spi_sync_transfer(st->sdev, &xfer, 1); > > + ret = spi_sync_transfer(st->sdev, xfers, 2); > > if (ret < 0) > > return ret; > >
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c index 4b25a3a314ed..ed404355ea4c 100644 --- a/drivers/staging/iio/resolver/ad2s1210.c +++ b/drivers/staging/iio/resolver/ad2s1210.c @@ -130,17 +130,24 @@ static int ad2s1210_config_write(struct ad2s1210_state *st, u8 data) static int ad2s1210_config_read(struct ad2s1210_state *st, unsigned char address) { - struct spi_transfer xfer = { - .len = 2, - .rx_buf = st->rx, - .tx_buf = st->tx, + struct spi_transfer xfers[] = { + { + .len = 1, + .rx_buf = &st->rx[0], + .tx_buf = &st->tx[0], + .cs_change = 1, + }, { + .len = 1, + .rx_buf = &st->rx[1], + .tx_buf = &st->tx[1], + }, }; int ret = 0; ad2s1210_set_mode(MOD_CONFIG, st); st->tx[0] = address | AD2S1210_MSB_IS_HIGH; st->tx[1] = AD2S1210_REG_FAULT; - ret = spi_sync_transfer(st->sdev, &xfer, 1); + ret = spi_sync_transfer(st->sdev, xfers, 2); if (ret < 0) return ret;