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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Nuno,

On 6/19/24 07:21, Nuno Sá wrote:
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@xxxxxxxxxxx>
---

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.


Sure. I have updated the 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.


Just renaming is not sufficient. The reason is that devm_add_action_or_reset() require an action with action(void *) prototype. So the prototype of iio_backend_disable() has to be changed to void iio_backend_disable(void *back). I placed the same arguments in enable and disable for symmetry, but *dev is not required for time being in disable API. So it can make sense to change iio_backend_disable() prototype.
alternatively, we can call __iio_backend_disable() through this API.
Please, let me know is you have a preference.

Thanks
Olivier

With the above changes:

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

- Nuno Sá





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux