Re: [PATCH v2 00/10] iio: adc: ti-ads112c14: add filter support

[Jonathan Cameron]

On Fri, 04 Sep 2026 17:09:45 -0500
"David Lechner (TI)" <dlechner@xxxxxxxxxxxx> wrote:

> I was hoping to avoid this, but until [0] lands, sashiko doesn't know
> how to handle patch series with dependencies. So I have combined my
> three outstanding ti-ads112c14 series into a single one. It's really
> too much all at once, but this seems the best way to speed up the review
> process at the moment.
> 
> [0]: https://github.com/sashiko-dev/sashiko/pull/389

Nice. I hope that lands.  Not being in a sensible position to cherry-pick
off the start of a series is leading to a lot more patches outstanding
on list than I like.  I was drawing up a list of asks but seems folk
are already on top of them!

As a temporary solution this is fine.

Given my goldfish brain I can't remember if I reviewed much of this
recently so I'll just start at the top again!

Thanks

Jonathan


> 
> So we now have:
> 
> * patches 1-3: "iio: adc: ti-ads112c14: continuous mode support", last
>   posted as v5 [1]
> * patches 4-5: "iio: adc: ti-ads112c14: add burnout current support",
>   last posted as v2 [2]
> * patches 6-10: "iio: adc: ti-ads112c14: add filter support", last
>   posted as v1 [3]
> 
> Apart from the changes listed below, the patches are the same as in
> those postings.
> 
> [1]: https://patch.msgid.link/20260831-iio-adc-ti-ads112c14-continuous-mode-v5-0-76f80a04b94f@xxxxxxxxxxxx
> [2]: https://patch.msgid.link/20260827-iio-adc-ti-ads112c14-burnout-v2-0-00a1fab9e2d1@xxxxxxxxxxxx
> [3]: https://patch.msgid.link/20260807-iio-adc-ti-ads112c14-filter-support-v1-0-4d3ba00caf18@xxxxxxxxxxxx
> 
> The rest of this cover letter describes the filter support portion.
> 
> TI ADS112C14 has several features related to filtering that are all
> interconnected. And to make things more interesting, not of the register
> fields map directly to IIO attributes. So this is one of those cases
> where we need to bend the rules a bit and just document it (we've
> already discussed this a bit in the previous series for this driver and
> came to this conclusion).
> 
> Here is the high-level overview:
> 
> We are adding sampling_frequency, oversampling_ratio, filter_type, and
> a (new to IIO) settlingtime attribute.
> 
> Since register fields have different meanings depending on filter type
> we have a quirky rule that if the filter type is sinc4 or sinc4+sinc1,
> then you need to set the oversampling ratio first in order to see the
> expected available values for the sampling frequency. For sinc4+sinc1+pf1
> it is the other way around, you have to set sampling frequency first
> in order to see the expected available oversampling ratios.
> 
> In other drivers, we've opted to store the requested values for dependant
> attributes like this and pick the closest available one when actually
> starting sampling. I opted not to do that here as there is not much
> overlap between settings. And as we will see below, there are other
> reasons for being picky about sampling frequency.
> 
> We also discussed in another series about a proposed settlingtime
> attribute. The conclusion was that it should be the total settling time
> delay (in seconds) before a chip takes the first sample (after any
> settings have changed). In this chip there is a DELAY field in a
> register that programs some extra delay in addition to an always present
> fixed delay. So the way the attribute will work for this chip is that
> the settlingtime_available attribute will list the range including a
> minimum value. This happens to be the always present fixed delay. So the
> difference between that and the current value of the settlingtime
> attribute will be programed as the DELAY value. It also seems that the
> fixed latency period includes the conversion time. We've just glossed
> over that for now and not subtracted that from the settlingtime
>  attribute.
> 
> Now, here is where things really get interesting/complicated. There are
> even more settings that affect the settling time. We defined the
> settling time as just a delay before the first sample. However, there
> are a couple of things that trigger the "first" sample. On this chip,
> the first sample only counts in continuous sampling mode. So only works
> as described when using the DRDY trigger in this driver. When using
> a generic trigger, e.g. a hrtimer trigger, single-shot sampling mode
> is used, so every sample is a "first" sample and has the settling delay
> added. This is mostly a non-issue other than it could throw people off
> that they cannot set the hrtimer frequency close to the sampling
> frequency attribute and actually get that sampling rate.
> 
> And there a few other idiosyncrasies we haven't accounted for. To keep
> things simple, we've implemented settlingtime as tDELAY + tLATENCY
> (datasheet values). But this actually include the conversion time as
> well. Also, tLATENCY is longer if you are coming out of standby mode
> (this doesn't matter at this point since we didn't implement power
> management, but we wouldn't want to change it and break userspace
> later).
> 
> Then there is also input chopping where the positive and negative input
> channels are swapped in the mux in the ADC on each sample. In these
> cases, every conversion requires the settling time because the mux is
> switched after every conversion. And the actual first sample has
> additional delay (presumably does two conversions). So the first sample
> takes tGC_LATENCY = 2 × (tDELAY + tLATENCY) – 12 tMOD and every sample
> after that takes tGC_DATA = tDELAY + tLATENCY – 12 tMOD. For this one,
> I have valued simplicity over accuracy in the implementation, so it is
> the same where settlingtime = tDELAY + tLATENCY and not worried about
> the 12 tMOD difference. I think it makes sense to keep settlingtime
> as a single tDELAY + tLATENCY in this case since the when the mux
> changes after each sample, the next sample is now the first sample after
> settings have changed.
> 
> This sort of breaks the definition of sampling frequency though since
> in IIO, the 1 / sampling frequency is the time between each sample
> being sent over the bus. I don't really want to change how sampling
> frequency is implemented here though because the current values match
> the datasheet which can be used to infer information like where the
> notches in the filter are. The actual observed sampling rate will be
> 1 / tGC_DATA.
> 
> Having written all of this out now though, I'm tempted to go back and
> change the settlingtime attribute implementation to be more accurate.
> In any case, tDELAY will always be easy to infer because it is the
> current value minus the minimum value (from the _available attribute).
> Then when using input chopping one could get a reasonably accurate
> sample period by taking 1 / sampling_frequency + settlingtime.
> 
> We will follow this up later with a documentation patch that explains
> all of this too.
> 
> Signed-off-by: David Lechner (TI) <dlechner@xxxxxxxxxxxx>
> ---
> Changes in v2:
> - Combined the continuous mode support and burnout current support
>   series into this series (see above).
> - Rebased on the current iio/testing.
> - Bumped KernelVersion to 7.4 in the settlingtime ABI documentation so
>   that it matches the rest of the new ABI in this series.
> - Patches 1-3 (continuous mode): no changes since v5 [1].
>    - FWIW, I didn't think sashiko's comments on the IRQ were realistic.
>      This could never be used with a level interrupt. And we haven't
>      typically tried to handle spurious interrupts in the past either.
>      Everything it suggested could only happen with broken hardware or
>      excessive noise (which I suppose counts as broken hardware).
> - Patches 4-5 (burnout), changes since v2 [2]:
>   - Pass an integer rather than a boolean to FIELD_PREP() for the global
>     chop enable bit.
>   - Drop the blank line after looking up the measurement.
>   - Return the conversion error in preference to the error from turning
>     the burnout current back off.
>   - Use a local variable for the "burn-out-current-nanoamp" property
>     name.
>   - Compare against ADS112C14_DEVICE_CFG_BOCS_DISABLED explicitly
>     instead of testing for non-zero.
>   - Fix KernelVersion in the ABI docs.
> - Patches 6-10 (filter support), changes since v1 [3]:
>   - Move the driver code that was accidentally squashed into the
>     "iio: ABI: add sinc4+sinc1+pf1 filter_type" patch back to the
>     "iio: adc: ti-ads112c14: add filter support" patch where it belongs.
>   - Fixed typos in the external clock patch commit message.
>   - Added a note to the settlingtime ABI documentation that the settling
>     time can apply to more than just the first conversion.
> - Link to v1: https://patch.msgid.link/20260807-iio-adc-ti-ads112c14-filter-support-v1-0-4d3ba00caf18@xxxxxxxxxxxx
> 
> ---
> David Lechner (TI) (10):
>       iio: adc: ti-ads112c14: add DRDY interrupt support
>       iio: adc: ti-ads112c14: create data read helper functions
>       iio: adc: ti-ads112c14: add continuous mode support
>       iio: adc: ti-ads112c14: add burnout current support
>       iio: ABI: add sysfs attribute for _burnoutraw
>       iio: adc: ti-ads112c14: support external clock
>       iio: adc: ti-ads112c14: add filter support
>       iio: ABI: add sinc4+sinc1+pf1 filter_type
>       iio: adc: ti-ads112c14: add settlingtime attribute
>       iio: ABI: add settlingtime attributes
> 
>  Documentation/ABI/testing/sysfs-bus-iio     |   27 +
>  Documentation/ABI/testing/sysfs-bus-iio-adc |    9 +
>  drivers/iio/adc/ti-ads112c14.c              | 1286 +++++++++++++++++++++++++--
>  3 files changed, 1263 insertions(+), 59 deletions(-)
> ---
> base-commit: e7c1d459e542bc4a9c57f558e8ca1b14df7a7eef
> change-id: 20260807-iio-adc-ti-ads112c14-filter-support-8a56850f590f
> 
> Best regards,
> --  
> David Lechner (TI) <dlechner@xxxxxxxxxxxx>
>