On Thu, 2019-09-12 at 16:43 +0200, Andrea Merello wrote: > [External] > > The driver calls udelay(2) after each SPI xfer. However, according to > the specifications, the SPI timing should be as follows: > > 1- The end of SPI xfer (CNV/CS rising edge) causes the device to initiate > the conversion phase, which takes up to 2.2uS. Yes, but there does not seem to be a minimum time for conversion. ( 2.2 uS is the max value ) This can be confusing a bit (I know). If you do see issues with 2 uS, we can probably try 3 uS (+1 uS which is roughly half the max value). Though, we are already gaining min 200 nS from the fact that the acquisition time is 1.8 uS and the delay is 2 uS. But if there aren't any visible issues, I would leave 2 uS. Increasing this delay can annoy people that would like to have some speed when reading samples. I know 1-2 uS isn't much of a performance killer, but if there aren't reasons to change it, I wouldn't. > > 2- At the end of the conversion phase, the device starts the acquisition > phase for the next conversion automatically (regardless to the state of > CNV pin); the conversion phase should last at least 1.8 uS > > The whole cycle timing is thus 4uS long. The SPI data is read during the > acquisition phase (RAC mode, no need to worry about "Tdata"). > > In order to be compliant wrt these timing specifications we should wait > 4uS after each SPI xfer (that is conservative, because there is also the > SPI xfer duration itself - which at the maximum supported clock should be > about 320nS). > > This patch enlarges the delay up to 4uS and it also removes the explicit > calls to udelay(), relying on spi_transfer->delay_usecs. > I like the switch from explicit udelay() to spi_transfer->delay_usecs. The code looks cleaner. > Signed-off-by: Andrea Merello <andrea.merello@xxxxxxxxx> > --- > drivers/iio/adc/ad7949.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/drivers/iio/adc/ad7949.c b/drivers/iio/adc/ad7949.c > index 5c2b3446fa4a..25d1e1b24257 100644 > --- a/drivers/iio/adc/ad7949.c > +++ b/drivers/iio/adc/ad7949.c > @@ -69,6 +69,7 @@ static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val, > .tx_buf = &ad7949_adc->buffer, > .len = 2, > .bits_per_word = bits_per_word, > + .delay_usecs = 4, > }, > }; > > @@ -77,11 +78,6 @@ static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val, > spi_message_init_with_transfers(&msg, tx, 1); > ret = spi_sync(ad7949_adc->spi, &msg); > > - /* > - * This delay is to avoid a new request before the required time to > - * send a new command to the device > - */ > - udelay(2); > return ret; > } > > @@ -97,6 +93,7 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, > .rx_buf = &ad7949_adc->buffer, > .len = 2, > .bits_per_word = bits_per_word, > + .delay_usecs = 4, > }, > }; > > @@ -112,12 +109,6 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, > if (ret) > return ret; > > - /* > - * This delay is to avoid a new request before the required time to > - * send a new command to the device > - */ > - udelay(2); > - > ad7949_adc->current_channel = channel; > > *val = ad7949_adc->buffer & mask;
