linux-kernel - Re: [PATCH v2 01/13] iio: backend: add support for filter config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c352c000a9d2c855dc4e5b01e16682a239e8cae7.camel@gmail.com>
Date: Fri, 11 Apr 2025 16:37:28 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>, jic23@...nel.org, 
	robh@...nel.org, conor+dt@...nel.org, linux-iio@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 01/13] iio: backend: add support for filter config

Hi Antoniu,

I do not have time today for going through all the series but I'll already leave
my comment on this on..

On Fri, 2025-04-11 at 15:36 +0300, Antoniu Miclaus wrote:
> Add backend support for digital filter enable/disable.
> 
> This setting can be adjusted within the IP cores interfacing devices.
> 
> The IP core can be configured based on the state of the actual
> digital filter configuration of the part.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> ---
> changes in v2:
>  - improve commit description
>  drivers/iio/industrialio-backend.c | 26 ++++++++++++++++++++++++++
>  include/linux/iio/backend.h        |  6 ++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> backend.c
> index d4ad36f54090..ffafe7c73508 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -778,6 +778,32 @@ static int __devm_iio_backend_get(struct device *dev,
> struct iio_backend *back)
>  	return 0;
>  }
>  
> +/**
> + * iio_backend_filter_enable - Enable filter
> + * @back: Backend device
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_filter_enable(struct iio_backend *back)
> +{
> +	return iio_backend_op_call(back, filter_enable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_filter_enable, "IIO_BACKEND");
> +
> +/**
> + * iio_backend_filter_disable - Disable filter
> + * @back: Backend device
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_filter_disable(struct iio_backend *back)
> +{
> +	return iio_backend_op_call(back, filter_disable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_filter_disable, "IIO_BACKEND");

This seems to resemble the filter_type IIO attr so I would likely be more
explicit in the API naming. Like 'iio_backend_filter_type_set()'. And that also
takes me into the more important point. I would consider having this API taking
an unsigned int filter_type (or an enum with the same possibilities as defined
in the ABI) argument rather than an enable vs disable thing. Like this, we're
just thinking about this particular usecase but it can very well happen that in
the future some backends might need to know the specific filter being
configured. Sure we could change things later on but doing it now is pretty
straight so why not :)?

- Nuno Sá