| Message ID | d3ee92a533cd1207cf5c5cc4d7bdbb5c6c267f68.1749063024.git.Jonathan.Santos@analog.com |
|---|---|
| State | New |
| Headers | show |
| Series | iio: adc: ad7768-1: Add features, improvements, and fixes | expand |
On 6/4/25 2:35 PM, Jonathan Santos wrote: > The SYNC_IN pulse width must be at least 1.5 x Tmclk, corresponding to > ~2.5 µs at the lowest supported MCLK frequency. Add a 3 µs delay to > ensure reliable synchronization timing even for the worst-case scenario. > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > --- Reviewed-by: David Lechner <dlechner@baylibre.com> > v10 Changes: > * New patch. > --- > drivers/iio/adc/ad7768-1.c | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c > index 51134023534a..8b414a102864 100644 > --- a/drivers/iio/adc/ad7768-1.c > +++ b/drivers/iio/adc/ad7768-1.c > @@ -252,6 +252,24 @@ static const struct regmap_config ad7768_regmap24_config = { > .max_register = AD7768_REG24_COEFF_DATA, > }; > > +static int ad7768_send_sync_pulse(struct ad7768_state *st) > +{ > + /* > + * The datasheet specifies a minimum SYNC_IN pulse width of 1.5 × Tmclk, > + * where Tmclk is the MCLK period. The supported MCLK frequencies range > + * from 0.6 MHz to 17 MHz, which corresponds to a minimum SYNC_IN pulse > + * width of approximately 2.5 µs in the worst-case scenario (0.6 MHz). > + * > + * Add a delay to ensure the pulse width is always sufficient to > + * trigger synchronization. > + */ > + gpiod_set_value_cansleep(st->gpio_sync_in, 1); > + fsleep(3); > + gpiod_set_value_cansleep(st->gpio_sync_in, 0); > + > + return 0; There is no other return, so this could be a void function. In this case, it is fine because a later patch adds another return. But in the future, be sure to mention that in the commit message (or below the ---) so that reviewers will know why without having to look ahead. > +} > +
On Wed, 4 Jun 2025 15:14:59 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 6/4/25 2:35 PM, Jonathan Santos wrote: > > The SYNC_IN pulse width must be at least 1.5 x Tmclk, corresponding to > > ~2.5 µs at the lowest supported MCLK frequency. Add a 3 µs delay to > > ensure reliable synchronization timing even for the worst-case scenario. > > > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > > --- > > Reviewed-by: David Lechner <dlechner@baylibre.com> This stands fine on it's own so applied even if I don't yet pick up the rest of the series. Applied to the testing branch of iio.git which I'll rebase on rc1 sometime early next week. Thanks, Jonathan > > > v10 Changes: > > * New patch. > > --- > > drivers/iio/adc/ad7768-1.c | 23 +++++++++++++++++++---- > > 1 file changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c > > index 51134023534a..8b414a102864 100644 > > --- a/drivers/iio/adc/ad7768-1.c > > +++ b/drivers/iio/adc/ad7768-1.c > > @@ -252,6 +252,24 @@ static const struct regmap_config ad7768_regmap24_config = { > > .max_register = AD7768_REG24_COEFF_DATA, > > }; > > > > +static int ad7768_send_sync_pulse(struct ad7768_state *st) > > +{ > > + /* > > + * The datasheet specifies a minimum SYNC_IN pulse width of 1.5 × Tmclk, > > + * where Tmclk is the MCLK period. The supported MCLK frequencies range > > + * from 0.6 MHz to 17 MHz, which corresponds to a minimum SYNC_IN pulse > > + * width of approximately 2.5 µs in the worst-case scenario (0.6 MHz). > > + * > > + * Add a delay to ensure the pulse width is always sufficient to > > + * trigger synchronization. > > + */ > > + gpiod_set_value_cansleep(st->gpio_sync_in, 1); > > + fsleep(3); > > + gpiod_set_value_cansleep(st->gpio_sync_in, 0); > > + > > + return 0; > > There is no other return, so this could be a void function. In this case, it is > fine because a later patch adds another return. But in the future, be sure to > mention that in the commit message (or below the ---) so that reviewers will > know why without having to look ahead. > > > +} > > +
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index 51134023534a..8b414a102864 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -252,6 +252,24 @@ static const struct regmap_config ad7768_regmap24_config = { .max_register = AD7768_REG24_COEFF_DATA, }; +static int ad7768_send_sync_pulse(struct ad7768_state *st) +{ + /* + * The datasheet specifies a minimum SYNC_IN pulse width of 1.5 × Tmclk, + * where Tmclk is the MCLK period. The supported MCLK frequencies range + * from 0.6 MHz to 17 MHz, which corresponds to a minimum SYNC_IN pulse + * width of approximately 2.5 µs in the worst-case scenario (0.6 MHz). + * + * Add a delay to ensure the pulse width is always sufficient to + * trigger synchronization. + */ + gpiod_set_value_cansleep(st->gpio_sync_in, 1); + fsleep(3); + gpiod_set_value_cansleep(st->gpio_sync_in, 0); + + return 0; +} + static int ad7768_set_mode(struct ad7768_state *st, enum ad7768_conv_mode mode) { @@ -339,10 +357,7 @@ static int ad7768_set_dig_fil(struct ad7768_state *st, return ret; /* A sync-in pulse is required every time the filter dec rate changes */ - gpiod_set_value(st->gpio_sync_in, 1); - gpiod_set_value(st->gpio_sync_in, 0); - - return 0; + return ad7768_send_sync_pulse(st); } static int ad7768_set_freq(struct ad7768_state *st,
The SYNC_IN pulse width must be at least 1.5 x Tmclk, corresponding to ~2.5 µs at the lowest supported MCLK frequency. Add a 3 µs delay to ensure reliable synchronization timing even for the worst-case scenario. Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> --- v10 Changes: * New patch. --- drivers/iio/adc/ad7768-1.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)