linux-kernel - Re: [PATCH 2/8] iio: add enable and disable services to iio backend framework

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]

Date: Wed, 19 Jun 2024 07:21:06 +0200
From: Nuno Sá <noname.nuno@...il.com>
To: Olivier Moysan <olivier.moysan@...s.st.com>, Nuno Sa
 <nuno.sa@...log.com>,  Jonathan Cameron <jic23@...nel.org>, Lars-Peter
 Clausen <lars@...afoo.de>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/8] iio: add enable and disable services to iio backend
 framework

On Tue, 2024-06-18 at 18:08 +0200, Olivier Moysan wrote:
> Add iio_backend_disable() and iio_backend_enable() APIs to allow
> IIO backend consumer to request backend disabling and enabling.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@...s.st.com>
> ---

Hi Olivier,

small notes from me...

>  drivers/iio/industrialio-backend.c | 26 ++++++++++++++++++++++++++
>  include/linux/iio/backend.h        |  2 ++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> backend.c
> index b950e30018ca..d3db048c086b 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -166,6 +166,32 @@ int devm_iio_backend_enable(struct device *dev, struct
> iio_backend *back)
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_iio_backend_enable, IIO_BACKEND);
>  
> +/**
> + * iio_backend_enable - Backend enable
> + * @dev: Consumer device for the backend
> + * @back: Backend device
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_enable(struct device *dev, struct iio_backend *back)
> +{
> +	return iio_backend_op_call(back, enable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_enable, IIO_BACKEND);

We do already have devm_iio_backend_enable(). From a correctness stand point and even
scalability, that API should now call your new iio_backend_enable() instead of
directly call iio_backend_op_call(). I guess that change could be in this patch.

> +
> +/**
> + * iio_backend_disable - Backend disable
> + * @dev: Consumer device for the backend
> + * @back: Backend device
> + *
> + */
> +void iio_backend_disable(struct device *dev, struct iio_backend *back)
> +{
> +	iio_backend_void_op_call(back, disable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_disable, IIO_BACKEND);
> +

We also have __iio_backend_disable() which is static since all users were using
devm_iio_backend_enable(). I understand that's not suitable for you but I would
instead rename the existing function to iio_backend_disable() and export it.

With the above changes:

Reviewed-by: Nuno Sa <nuno.sa@...log.com>

- Nuno Sá