diff mbox series

[v5,1/3] dt-bindings:iio:adc: add generic settling-time-us and oversampling-ratio channel properties

Message ID 20210329073131.1759-2-o.rempel@pengutronix.de
State New
Headers show
Series [v5,1/3] dt-bindings:iio:adc: add generic settling-time-us and oversampling-ratio channel properties | expand

Commit Message

Oleksij Rempel March 29, 2021, 7:31 a.m. UTC
Settling time and over sampling is a typical challenge for different IIO ADC
devices. So, introduce channel specific settling-time-us and oversampling-ratio
properties to cover this use case.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 Documentation/devicetree/bindings/iio/adc/adc.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Jonathan Cameron March 29, 2021, 10:25 a.m. UTC | #1
On Mon, 29 Mar 2021 09:31:29 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Settling time and over sampling is a typical challenge for different IIO ADC
> devices. So, introduce channel specific settling-time-us and oversampling-ratio
> properties to cover this use case.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/iio/adc/adc.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> index 912a7635edc4..d5bc86d2a2af 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> @@ -39,4 +39,12 @@ properties:
>        The first value specifies the positive input pin, the second
>        specifies the negative input pin.
>  
> +  settling-time-us:
> +    description:
> +      Time between enabling the channel and firs stable readings.

first

> +
> +  oversampling-ratio:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Number of data samples which are averaged for each read.

I think I've asked about this in previous reviews, but I want a clear statement
of why you think this property is a feature of the 'board' (and hence should be
in device tree) rather than setting sensible defaults and leaving any control
to userspace?

Jonathan

> +
>  additionalProperties: true
Oleksij Rempel March 29, 2021, 1:06 p.m. UTC | #2
On Mon, Mar 29, 2021 at 11:25:32AM +0100, Jonathan Cameron wrote:
> On Mon, 29 Mar 2021 09:31:29 +0200
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > Settling time and over sampling is a typical challenge for different IIO ADC
> > devices. So, introduce channel specific settling-time-us and oversampling-ratio
> > properties to cover this use case.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/iio/adc/adc.yaml | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> > index 912a7635edc4..d5bc86d2a2af 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> > @@ -39,4 +39,12 @@ properties:
> >        The first value specifies the positive input pin, the second
> >        specifies the negative input pin.
> >  
> > +  settling-time-us:
> > +    description:
> > +      Time between enabling the channel and firs stable readings.
> 
> first

ack

> > +
> > +  oversampling-ratio:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Number of data samples which are averaged for each read.
> 
> I think I've asked about this in previous reviews, but I want a clear statement
> of why you think this property is a feature of the 'board' (and hence should be
> in device tree) rather than setting sensible defaults and leaving any control
> to userspace?

yes, my reply was:
> Oversampling is used as replacement of or addition to the low-pass filter. The
> filter can be implemented on board, but it will change settling time
> characteristic. Since low-pass filter is board specific characteristic, this
> property belongs in device tree as well.

I could imagine that this values can be overwritten from user space for
diagnostic, but we need some working default values. 

Should I integrate this comment in to the yaml?

Regards,
Oleksij
Jonathan Cameron March 29, 2021, 3:21 p.m. UTC | #3
On Mon, 29 Mar 2021 15:06:20 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> On Mon, Mar 29, 2021 at 11:25:32AM +0100, Jonathan Cameron wrote:
> > On Mon, 29 Mar 2021 09:31:29 +0200
> > Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> >   
> > > Settling time and over sampling is a typical challenge for different IIO ADC
> > > devices. So, introduce channel specific settling-time-us and oversampling-ratio
> > > properties to cover this use case.
> > > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > >  Documentation/devicetree/bindings/iio/adc/adc.yaml | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> > > index 912a7635edc4..d5bc86d2a2af 100644
> > > --- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> > > @@ -39,4 +39,12 @@ properties:
> > >        The first value specifies the positive input pin, the second
> > >        specifies the negative input pin.
> > >  
> > > +  settling-time-us:
> > > +    description:
> > > +      Time between enabling the channel and firs stable readings.  
> > 
> > first  
> 
> ack
> 
> > > +
> > > +  oversampling-ratio:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: Number of data samples which are averaged for each read.  
> > 
> > I think I've asked about this in previous reviews, but I want a clear statement
> > of why you think this property is a feature of the 'board' (and hence should be
> > in device tree) rather than setting sensible defaults and leaving any control
> > to userspace?  
> 
> yes, my reply was:

Ah. I missed it somewhere along the way, thanks for repeating here.

> > Oversampling is used as replacement of or addition to the low-pass filter. The
> > filter can be implemented on board, but it will change settling time
> > characteristic. Since low-pass filter is board specific characteristic, this
> > property belongs in device tree as well.  
> 
> I could imagine that this values can be overwritten from user space for
> diagnostic, but we need some working default values. 

Hmm. So low pass filters are interesting whether they are actually a characteristic
of the board (obviously they are if they are resistors/ caps etc on the board),
or of the application. Some applications want noisy messy data,
others not so much. What filter you need to achieve a specific noise level on
a given board is indeed a characteristic of the board.  However, what that noise
level is (which actually drives the decision) is not a board characteristic.
If we have a configurable filter, then that can be argued to be a policy decision
and hence userspace, not DT.

> 
> Should I integrate this comment in to the yaml?

Definitely.  Whilst I'm not that keen on this one, you have made a reasonable argument
that it is 'sort of' a board characteristic, so I can live with that as long as
it is there.   Perhaps the slightly amended version of the above.

"Oversampling is used as replacement of or addition to the low-pass filter.
In some cases, the desired filtering characteristics are a function the
device design and can interact with other characteristics such as
settling time."

Jonathan


> 
> Regards,
> Oleksij
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
index 912a7635edc4..d5bc86d2a2af 100644
--- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
@@ -39,4 +39,12 @@  properties:
       The first value specifies the positive input pin, the second
       specifies the negative input pin.
 
+  settling-time-us:
+    description:
+      Time between enabling the channel and firs stable readings.
+
+  oversampling-ratio:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Number of data samples which are averaged for each read.
+
 additionalProperties: true