[PATCH v7 00/12] iio: adc: Add support for AD4170 series of ADCs

Marcelo Schmitt posted 12 patches 3 months, 1 week ago
There is a newer version of this series
Documentation/ABI/testing/sysfs-bus-iio       |    1 +
.../bindings/iio/adc/adi,ad4170-4.yaml        |  555 +++
MAINTAINERS                                   |    8 +
drivers/iio/adc/Kconfig                       |   16 +
drivers/iio/adc/Makefile                      |    1 +
drivers/iio/adc/ad4170-4.c                    | 3020 +++++++++++++++++
6 files changed, 3601 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4170-4.yaml
create mode 100644 drivers/iio/adc/ad4170-4.c
[PATCH v7 00/12] iio: adc: Add support for AD4170 series of ADCs
Posted by Marcelo Schmitt 3 months, 1 week ago
Hello,

AD4170 support v7 comes after testing the driver with even more variations of
channel setups. Signal offset and amplification can be tricky to grasp at times.

Thank you to all reviewers of previous versions. This intends to comply with all
comments and suggestions to v6.

Same amount of patches than v6.

Patch 1 adds device tree documentation for the parts.
Patch 2 adds basic device support.
Patch 3 adds support for calibration scale.
Patch 4 adds support for calibration bias.
Patch 5 adds sinc5+avg to filter_type_available IIO ABI documentation.
Patch 6 adds support for sample frequency along with filter type configuration.
Patch 7 adds support for buffered ADC reading.
Patch 8 adds clock provider support
Patch 9 adds GPIO controller support.
Patch 10 adds internal temperature sensor support.
Patch 11 adds support for external RTD and bridge circuit sensors.
Patch 12 adds timestamp channel

Change log v6 -> v7

[Generic changes]
- Renamed dt-doc and driver from ad4170 to ad4170-4. This will hopefully be more 
  future proof since there is precedent of different devices with names ending
  with and without -N (e.g. AD7091R and AD7091R-5, AD7768 and AD7768-1).

[Device tree changes]
- Dropped leftover/unneeded list of valid reference-buffer values.
- Set vbias at AIN8 in example. The vbias is expected to be set on the IN- pin.

[Basic driver patch]
- Refactored ad4170_parse_adc_channel_type() to use fwnode_property_present()
  so to return errors early if required properties are not present.
- Use spi_write_then_read() to skip dealing with DMA safe buffers in slow data paths.
- Minor tweaks to comments such as 'handle options ...' -> 'handle PGA options ...'.

[sinc5+avg ABI patch]
- Fixed IIO ABI documentation by specifying the correct filter enabled with sinc5+avg.

[Digital filter and sample frequency config patch]
- Now reading SPS table within filter type switch to ensure in bounds array access.
- Squeezed info_mask_separate additions to reduce change diff.

[Buffer support patch]
- Dropped extra assignment of st->trig->dev.parent.

[GPIO controller patch]
- Used local device pointer (replaced &st->spi->dev with dev) in ad4170_parse_firmware().

[External sensor patch]
- adi,vbias was previously set per channel in ad4170 dt-binding but it is set
  per device in adi,ad4130.yaml. Fixed ad4170 parsing of adi,vbias. The
  dt-binding had been updated to the established use of adi,vbias in v4.

Link to v6: https://lore.kernel.org/linux-iio/cover.1750258776.git.marcelo.schmitt@analog.com/
Link to v5: https://lore.kernel.org/linux-iio/cover.1749582679.git.marcelo.schmitt@analog.com/ 
Link to v4: https://lore.kernel.org/linux-iio/cover.1748829860.git.marcelo.schmitt@analog.com/
Link to v3: https://lore.kernel.org/linux-iio/cover.1747083143.git.marcelo.schmitt@analog.com/
Link to v2: https://lore.kernel.org/linux-iio/cover.1745841276.git.marcelo.schmitt@analog.com/
Link to v1: https://lore.kernel.org/linux-iio/cover.1744200264.git.marcelo.schmitt@analog.com/


Ana-Maria Cusco (1):
  iio: adc: Add basic support for AD4170-4

Marcelo Schmitt (11):
  dt-bindings: iio: adc: Add AD4170
  iio: adc: ad4170-4: Add support for calibration gain
  iio: adc: ad4170-4: Add support for calibration bias
  Documentation: ABI: IIO: Add sinc5+avg to the filter_type_available
    list
  iio: adc: ad4170-4: Add digital filter and sample frequency config
    support
  iio: adc: ad4170-4: Add support for buffered data capture
  iio: adc: ad4170-4: Add clock provider support
  iio: adc: ad4170-4: Add GPIO controller support
  iio: adc: ad4170-4: Add support for internal temperature sensor
  iio: adc: ad4170-4: Add support for weigh scale and RTD sensors
  iio: adc: ad4170-4: Add timestamp channel

 Documentation/ABI/testing/sysfs-bus-iio       |    1 +
 .../bindings/iio/adc/adi,ad4170-4.yaml        |  555 +++
 MAINTAINERS                                   |    8 +
 drivers/iio/adc/Kconfig                       |   16 +
 drivers/iio/adc/Makefile                      |    1 +
 drivers/iio/adc/ad4170-4.c                    | 3020 +++++++++++++++++
 6 files changed, 3601 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4170-4.yaml
 create mode 100644 drivers/iio/adc/ad4170-4.c


base-commit: 42498420746a4db923f03d048a0ebc9bd2371f56
-- 
2.47.2
Re: [PATCH v7 00/12] iio: adc: Add support for AD4170 series of ADCs
Posted by Andy Shevchenko 3 months, 1 week ago
On Mon, Jun 30, 2025 at 10:57:32AM -0300, Marcelo Schmitt wrote:
> Hello,
> 
> AD4170 support v7 comes after testing the driver with even more variations of
> channel setups. Signal offset and amplification can be tricky to grasp at times.
> 
> Thank you to all reviewers of previous versions. This intends to comply with all
> comments and suggestions to v6.
> 
> Same amount of patches than v6.

...

>  6 files changed, 3601 insertions(+)

This is weird. At least patches 11 & 12 have '-' lines...

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v7 00/12] iio: adc: Add support for AD4170 series of ADCs
Posted by Marcelo Schmitt 3 months, 1 week ago
On 07/02, Andy Shevchenko wrote:
> On Mon, Jun 30, 2025 at 10:57:32AM -0300, Marcelo Schmitt wrote:
> > Hello,
> > 
> > AD4170 support v7 comes after testing the driver with even more variations of
> > channel setups. Signal offset and amplification can be tricky to grasp at times.
> > 
> > Thank you to all reviewers of previous versions. This intends to comply with all
> > comments and suggestions to v6.
> > 
> > Same amount of patches than v6.
> 
> ...
> 
> >  6 files changed, 3601 insertions(+)
> 
> This is weird. At least patches 11 & 12 have '-' lines...
> 
Yeah, sorry about that. These ADCs are fancy such that the base driver is about
1500 LoCs due to channel setup handling and support for multiple combinations of
voltage references and channel setups.

About the '-' lines, I will rework ad4170_parse_channel_node() on earlier
patches to avoid 3 line removals in patch 11. Patch 12 is only makes sense
after patch 7 and I think it would lead to '-' lines if coming before patch 10
since both increment the number of IIO channels. Anyway, I'll see how to further
reduce the number of lines being removed.


Best regards,
Marcelo

> -- 
> With Best Regards,
> Andy Shevchenko
> 
>
Re: [PATCH v7 00/12] iio: adc: Add support for AD4170 series of ADCs
Posted by Andy Shevchenko 3 months, 1 week ago
On Wed, Jul 02, 2025 at 09:00:42AM -0300, Marcelo Schmitt wrote:
> On 07/02, Andy Shevchenko wrote:
> > On Mon, Jun 30, 2025 at 10:57:32AM -0300, Marcelo Schmitt wrote:

...

> > >  6 files changed, 3601 insertions(+)
> > 
> > This is weird. At least patches 11 & 12 have '-' lines...
> > 
> Yeah, sorry about that. These ADCs are fancy such that the base driver is about
> 1500 LoCs due to channel setup handling and support for multiple combinations of
> voltage references and channel setups.
> 
> About the '-' lines, I will rework ad4170_parse_channel_node() on earlier
> patches to avoid 3 line removals in patch 11. Patch 12 is only makes sense
> after patch 7 and I think it would lead to '-' lines if coming before patch 10
> since both increment the number of IIO channels. Anyway, I'll see how to further
> reduce the number of lines being removed.

My point is that the above statistics is mangled and I don't know how I can
trust the contents of this series if it already lied about that.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v7 00/12] iio: adc: Add support for AD4170 series of ADCs
Posted by Marcelo Schmitt 3 months, 1 week ago
On 07/02, Andy Shevchenko wrote:
> On Wed, Jul 02, 2025 at 09:00:42AM -0300, Marcelo Schmitt wrote:
> > On 07/02, Andy Shevchenko wrote:
> > > On Mon, Jun 30, 2025 at 10:57:32AM -0300, Marcelo Schmitt wrote:
> 
> ...
> 
> > > >  6 files changed, 3601 insertions(+)
> > > 
> > > This is weird. At least patches 11 & 12 have '-' lines...
> > > 
> > Yeah, sorry about that. These ADCs are fancy such that the base driver is about
> > 1500 LoCs due to channel setup handling and support for multiple combinations of
> > voltage references and channel setups.
> > 
> > About the '-' lines, I will rework ad4170_parse_channel_node() on earlier
> > patches to avoid 3 line removals in patch 11. Patch 12 is only makes sense
> > after patch 7 and I think it would lead to '-' lines if coming before patch 10
> > since both increment the number of IIO channels. Anyway, I'll see how to further
> > reduce the number of lines being removed.
> 
> My point is that the above statistics is mangled and I don't know how I can
> trust the contents of this series if it already lied about that.

Looks like git format-patch summarizes the changes from all patches when
printing the statistics to the cover letter. Also, git format-patch doc [1]
says the 'changes' dirstat option (default behavior) doesn't count
rearranged lines as much as other changes. There are cover letters of other
patch sets where the number of '-' lines don't match the sum of lines
removed by each patch. [2] and [3] are examples of that.

[1]: https://git-scm.com/docs/git-format-patch
[2]: https://lore.kernel.org/linux-iio/20250630-losd-3-inv-icm42600-add-wom-support-v6-0-5bb0c84800d9@tdk.com/
[3]: https://lore.kernel.org/linux-iio/20250627-iio-adc-ad7173-add-spi-offload-support-v2-0-f49c55599113@baylibre.com/

This set doesn't remove stuff that existed prior to it so I think it makes
sense the cover letter to show that lines are only being added.

I'll send v8 with the change I mentioned earlier. Unless patches 11 and 12
already look good the way they are in v7.

Best regards,
Marcelo
Re: [PATCH v7 00/12] iio: adc: Add support for AD4170 series of ADCs
Posted by Andy Shevchenko 3 months ago
On Wed, Jul 02, 2025 at 03:59:12PM -0300, Marcelo Schmitt wrote:
> On 07/02, Andy Shevchenko wrote:
> > On Wed, Jul 02, 2025 at 09:00:42AM -0300, Marcelo Schmitt wrote:
> > > On 07/02, Andy Shevchenko wrote:
> > > > On Mon, Jun 30, 2025 at 10:57:32AM -0300, Marcelo Schmitt wrote:

...

> > > > >  6 files changed, 3601 insertions(+)
> > > > 
> > > > This is weird. At least patches 11 & 12 have '-' lines...
> > > > 
> > > Yeah, sorry about that. These ADCs are fancy such that the base driver is about
> > > 1500 LoCs due to channel setup handling and support for multiple combinations of
> > > voltage references and channel setups.
> > > 
> > > About the '-' lines, I will rework ad4170_parse_channel_node() on earlier
> > > patches to avoid 3 line removals in patch 11. Patch 12 is only makes sense
> > > after patch 7 and I think it would lead to '-' lines if coming before patch 10
> > > since both increment the number of IIO channels. Anyway, I'll see how to further
> > > reduce the number of lines being removed.
> > 
> > My point is that the above statistics is mangled and I don't know how I can
> > trust the contents of this series if it already lied about that.
> 
> Looks like git format-patch summarizes the changes from all patches when
> printing the statistics to the cover letter. Also, git format-patch doc [1]
> says the 'changes' dirstat option (default behavior) doesn't count
> rearranged lines as much as other changes.

TIL. Thanks for pointing that out.

> There are cover letters of other
> patch sets where the number of '-' lines don't match the sum of lines
> removed by each patch. [2] and [3] are examples of that.

That's different I believe due to the diff algorithm in use
(btw, do you use histogramm?).

> [1]: https://git-scm.com/docs/git-format-patch
> [2]: https://lore.kernel.org/linux-iio/20250630-losd-3-inv-icm42600-add-wom-support-v6-0-5bb0c84800d9@tdk.com/
> [3]: https://lore.kernel.org/linux-iio/20250627-iio-adc-ad7173-add-spi-offload-support-v2-0-f49c55599113@baylibre.com/
> 
> This set doesn't remove stuff that existed prior to it so I think it makes
> sense the cover letter to show that lines are only being added.
> 
> I'll send v8 with the change I mentioned earlier. Unless patches 11 and 12
> already look good the way they are in v7.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v7 00/12] iio: adc: Add support for AD4170 series of ADCs
Posted by Marcelo Schmitt 3 months ago
On 07/03, Andy Shevchenko wrote:
> On Wed, Jul 02, 2025 at 03:59:12PM -0300, Marcelo Schmitt wrote:
> > On 07/02, Andy Shevchenko wrote:
> > > On Wed, Jul 02, 2025 at 09:00:42AM -0300, Marcelo Schmitt wrote:
> > > > On 07/02, Andy Shevchenko wrote:
> > > > > On Mon, Jun 30, 2025 at 10:57:32AM -0300, Marcelo Schmitt wrote:
> 
> ...
> 
> > > > > >  6 files changed, 3601 insertions(+)
> > > > > 
> > > > > This is weird. At least patches 11 & 12 have '-' lines...
> > > > > 
> > > > Yeah, sorry about that. These ADCs are fancy such that the base driver is about
> > > > 1500 LoCs due to channel setup handling and support for multiple combinations of
> > > > voltage references and channel setups.
> > > > 
> > > > About the '-' lines, I will rework ad4170_parse_channel_node() on earlier
> > > > patches to avoid 3 line removals in patch 11. Patch 12 is only makes sense
> > > > after patch 7 and I think it would lead to '-' lines if coming before patch 10
> > > > since both increment the number of IIO channels. Anyway, I'll see how to further
> > > > reduce the number of lines being removed.
> > > 
> > > My point is that the above statistics is mangled and I don't know how I can
> > > trust the contents of this series if it already lied about that.
> > 
> > Looks like git format-patch summarizes the changes from all patches when
> > printing the statistics to the cover letter. Also, git format-patch doc [1]
> > says the 'changes' dirstat option (default behavior) doesn't count
> > rearranged lines as much as other changes.
> 
> TIL. Thanks for pointing that out.
> 
> > There are cover letters of other
> > patch sets where the number of '-' lines don't match the sum of lines
> > removed by each patch. [2] and [3] are examples of that.
> 
> That's different I believe due to the diff algorithm in use
> (btw, do you use histogramm?).

Nope, I was using the default diff algorithm to generate the patches.
I tried the --histogram option today but didn't notice any difference. Maybe
default and histogram output the same result for the ad4170 set.
I'll send v8 using histogram alg. Nevertheless, is there any preferred alg for
generating patches (so I can use what's best on next patch sets)? 

Thanks,
Marcelo
Re: [PATCH v7 00/12] iio: adc: Add support for AD4170 series of ADCs
Posted by Jonathan Cameron 3 months ago
On Thu, 3 Jul 2025 17:24:07 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 07/03, Andy Shevchenko wrote:
> > On Wed, Jul 02, 2025 at 03:59:12PM -0300, Marcelo Schmitt wrote:  
> > > On 07/02, Andy Shevchenko wrote:  
> > > > On Wed, Jul 02, 2025 at 09:00:42AM -0300, Marcelo Schmitt wrote:  
> > > > > On 07/02, Andy Shevchenko wrote:  
> > > > > > On Mon, Jun 30, 2025 at 10:57:32AM -0300, Marcelo Schmitt wrote:  
> > 
> > ...
> >   
> > > > > > >  6 files changed, 3601 insertions(+)  
> > > > > > 
> > > > > > This is weird. At least patches 11 & 12 have '-' lines...
> > > > > >   
> > > > > Yeah, sorry about that. These ADCs are fancy such that the base driver is about
> > > > > 1500 LoCs due to channel setup handling and support for multiple combinations of
> > > > > voltage references and channel setups.
> > > > > 
> > > > > About the '-' lines, I will rework ad4170_parse_channel_node() on earlier
> > > > > patches to avoid 3 line removals in patch 11. Patch 12 is only makes sense
> > > > > after patch 7 and I think it would lead to '-' lines if coming before patch 10
> > > > > since both increment the number of IIO channels. Anyway, I'll see how to further
> > > > > reduce the number of lines being removed.  
> > > > 
> > > > My point is that the above statistics is mangled and I don't know how I can
> > > > trust the contents of this series if it already lied about that.  
> > > 
> > > Looks like git format-patch summarizes the changes from all patches when
> > > printing the statistics to the cover letter. Also, git format-patch doc [1]
> > > says the 'changes' dirstat option (default behavior) doesn't count
> > > rearranged lines as much as other changes.  
> > 
> > TIL. Thanks for pointing that out.
> >   
> > > There are cover letters of other
> > > patch sets where the number of '-' lines don't match the sum of lines
> > > removed by each patch. [2] and [3] are examples of that.  
> > 
> > That's different I believe due to the diff algorithm in use
> > (btw, do you use histogramm?).  
> 
> Nope, I was using the default diff algorithm to generate the patches.
> I tried the --histogram option today but didn't notice any difference. Maybe
> default and histogram output the same result for the ad4170 set.
> I'll send v8 using histogram alg. Nevertheless, is there any preferred alg for
> generating patches (so I can use what's best on next patch sets)? 

Given the size and complexity of this driver, I've applied it to the testing
branch of iio.git to get some extra build coverage.  Last minute reviews
still welcome but I may ask for patches on top depending on what they affect.

Hence applied to the togreg branch of iio.git and pushed out as testing for
0-day to take a poke at them.

Thanks,

Jonathan

> 
> Thanks,
> Marcelo
>