[v2,0/3] iio: adc: ad4695: add new regmap callbacks, timing improvements

Message

Trevor Gamblin Nov. 13, 2024, 8:52 p.m. UTC

The AD4695 driver currently operates all SPI reads/writes at the speed
appropriate for register access, rather than the max rate for the bus.
Data reads should ideally operate at the latter speed, but making this
change universally makes it possible for data to be corrupted during use
and for unexpected behavior to occur on driver subsequent driver
binds/unbinds. To solve this, introduce custom regmap bus callbacks for
the driver that explicitly set a lower speed only for these operations.

The first patch in this series is a fix introduced after discovering the
corresponding issue during testing of the callbacks. This is a timing
fix that ensures the AD4695 datasheet's timing specs are met, as before
the busy signal would sometimes fail to toggle again following the end
of the conversion sequence. Adding an extra delay in the form of a blank
transfer before every CS deassert in ad4695_buffer_preenable() allows
this requirement to be met. The patch also makes similar changes in
ad4695_read_one_sample() (while also tidying that function somewhat) to
make sure that single reads are still functional with the regmap change.

The second patch is an improvement that increases the robustness of the
exit message in ad4695_exit_conversion_mode(), this time by adding a
delay before the actual exit command. This helps avoid the possibility
that the exit message will be read as data, causing corruption on some
buffered reads.

For additional context, see:
https://lore.kernel.org/linux-iio/[e-mail]/

Suggested-by: David Lechner <[e-mail]>
Signed-off-by: Trevor Gamblin <[e-mail]>
---
Changes in v2:
- Link to v1: https://lore.kernel.org/r/[e-mail]

  [PATCH 1/3]
  - Fix bugs with transfer timings and sequencing in
    ad4695_buffer_preenable() and ad4695_read_one_sample() by adjusting
    xfer structs and simplifying
  - Add clearer comments explaining why the temperature channel is
    handled as-is with the advanced sequencer
  - Add David's Co-Developed-by tag

  [PATCH 2/3]
  - No change

  [PATCH 3/3]
  - Fix bug where ad4695_regmap_config was passed twice to
    devm_regmap_init() in ad4695_probe(), instead of passing
    ad4695_regmap_config once and ad4695_regmap16_config the second time

---
Trevor Gamblin (3):
      iio: adc: ad4695: fix buffered read, single sample timings
      iio: adc: ad4695: make ad4695_exit_conversion_mode() more robust
      iio: adc: ad4695: add custom regmap bus callbacks

 drivers/iio/adc/Kconfig  |   2 +-
 drivers/iio/adc/ad4695.c | 208 +++++++++++++++++++++++++++++++++++++----------
 2 files changed, 165 insertions(+), 45 deletions(-)
---
base-commit: 2d5404caa8c7bb5c4e0435f94b28834ae5456623
change-id: 20241111-tgamblin-ad4695_improvements-7a32a6268c26

Best regards,

Comments

David Lechner Nov. 13, 2024, 8:58 p.m. UTC | #1

On 11/13/24 2:52 PM, Trevor Gamblin wrote:
> The AD4695 driver currently operates all SPI reads/writes at the speed
> appropriate for register access, rather than the max rate for the bus.
> Data reads should ideally operate at the latter speed, but making this
> change universally makes it possible for data to be corrupted during use
> and for unexpected behavior to occur on driver subsequent driver
> binds/unbinds. To solve this, introduce custom regmap bus callbacks for
> the driver that explicitly set a lower speed only for these operations.
> 
> The first patch in this series is a fix introduced after discovering the
> corresponding issue during testing of the callbacks. This is a timing
> fix that ensures the AD4695 datasheet's timing specs are met, as before
> the busy signal would sometimes fail to toggle again following the end
> of the conversion sequence. Adding an extra delay in the form of a blank
> transfer before every CS deassert in ad4695_buffer_preenable() allows
> this requirement to be met. The patch also makes similar changes in
> ad4695_read_one_sample() (while also tidying that function somewhat) to
> make sure that single reads are still functional with the regmap change.
> 
> The second patch is an improvement that increases the robustness of the
> exit message in ad4695_exit_conversion_mode(), this time by adding a
> delay before the actual exit command. This helps avoid the possibility
> that the exit message will be read as data, causing corruption on some
> buffered reads.
> 
> For additional context, see:
> https://lore.kernel.org/linux-iio/[e-mail]/
> 
> Suggested-by: David Lechner <[e-mail]>
> Signed-off-by: Trevor Gamblin <[e-mail]>
> ---
Reviewed-by: David Lechner <[e-mail]>
Tested-by: David Lechner <[e-mail]>