From: Dragos Bogdan <[email protected]>
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 <[email protected]>
Signed-off-by: Alexandru Ardelean <[email protected]>
---
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;
--
2.17.1
On Wed, 29 Apr 2020 10:21:29 +0300
Alexandru Ardelean <[email protected]> wrote:
> From: Dragos Bogdan <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Alexandru Ardelean <[email protected]>
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 <[email protected]> wrote:
>
> > From: Dragos Bogdan <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Alexandru Ardelean <[email protected]>
>
> 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;
> >