Re: [PATCH v1 2/2] iio: temperature: add support for MCP998X

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Mon, 28 Apr 2025 12:57:05 +0000
<Victor.Duicu@microchip.com> wrote:

> On Fri, 2025-04-18 at 19:07 +0100, Jonathan Cameron wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > On Tue, 15 Apr 2025 16:26:23 +0300
> > <victor.duicu@microchip.com> wrote:
> >   
> > > From: Victor Duicu <victor.duicu@microchip.com>
> > > 
> > > This is the driver for Microchip MCP998X/33 and MCP998XD/33D
> > > Multichannel Automotive Monitor Family.
> > > 
> > > Signed-off-by: Victor Duicu <victor.duicu@microchip.com>  
> >   
> 
> Hi Jonathan,
> 
> > Hi Victor,
> > 
> > Various comments inline,
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> > > ---
> > >  .../testing/sysfs-bus-iio-temperature-mcp9982 |  17 +
> > >  MAINTAINERS                                   |   7 +
> > >  drivers/iio/temperature/Kconfig               |  10 +
> > >  drivers/iio/temperature/Makefile              |   1 +
> > >  drivers/iio/temperature/mcp9982.c             | 794
> > > ++++++++++++++++++
> > >  5 files changed, 829 insertions(+)
> > >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-
> > > temperature-mcp9982
> > >  create mode 100644 drivers/iio/temperature/mcp9982.c
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-temperature-
> > > mcp9982 b/Documentation/ABI/testing/sysfs-bus-iio-temperature-
> > > mcp9982
> > > new file mode 100644
> > > index 000000000000..de3360fb05be
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-temperature-mcp9982
> > > @@ -0,0 +1,17 @@
> > > +What:               
> > > /sys/bus/iio/devices/iio:deviceX/running_average_window  
> > 
> > As later in review, I think we can control this via the low pass
> > filter 3dB point
> > and use standard ABI.
> > 
> >   
> > >   
> ...
> 
> > Hmm.  A running average is a low pass filter.  Can we control this
> > instead via
> > standard ABI and the 3dB point?  Take a look at the filter ABI in
> > Documentation/ABI/testing/sysfs-bus-iio
> > 
> > Custom ABI is rarely used in real cases because the tools tend not to
> > know about it
> > so we avoid it if we possibly can.
> >   
> > > +
> > >   
> 
> 
> The moving average filter is used to smooth the temperature spikes.
> The user should be able to set the size of the window to
> a few values: 1(disable the filter), 4 and 8.
> The user does not have access to the frequency properties.

Assuming the device is self clocking, then we know the frequency and
hence can consider an averaging filter as a type of low pass filter (which is
what it effectively is) and control via the 3dB point.

Whether it is documented that way is just a question of how they
decided to describe it in the datasheet.

A moving average is also known as a box car filter. 
https://www.wavewalkerdsp.com/2022/08/03/bandwidth-of-a-moving-average-filter/

Approximately (pi / N)*sampling frequency (I think anyway, I only took a quick
look).

Doing that allows you to map it to standard ABI.

> 
> Best Regards,
> Victor
> >   
>