1. Patchew
  2. linux
  3. [PATCH 0/8] iio: adc: dfsdm: add scaling support
  4. View patch

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

Olivier Moysan posted 8 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH 2/8] iio: add enable and disable services to iio backend framework
Posted by Olivier Moysan 1 year, 6 months ago 
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@foss.st.com>
---
 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);
+
+/**
+ * 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);
+
 /**
  * iio_backend_data_format_set - Configure the channel data format
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index cff486699054..81277e5b6160 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -120,6 +120,8 @@ struct iio_backend_ops {
 int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
 int iio_backend_chan_disable(struct iio_backend *back, unsigned int chan);
 int devm_iio_backend_enable(struct device *dev, struct iio_backend *back);
+int iio_backend_enable(struct device *dev, struct iio_backend *back);
+void iio_backend_disable(struct device *dev, struct iio_backend *back);
 int iio_backend_data_format_set(struct iio_backend *back, unsigned int chan,
 				const struct iio_backend_data_fmt *data);
 int iio_backend_data_source_set(struct iio_backend *back, unsigned int chan,
-- 
2.25.1
Re: [PATCH 2/8] iio: add enable and disable services to iio backend framework
Posted by Nuno Sá 1 year, 6 months ago 
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@foss.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@analog.com>

- Nuno Sá
Re: [PATCH 2/8] iio: add enable and disable services to iio backend framework
Posted by Olivier MOYSAN 1 year, 6 months ago 
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@foss.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.
>

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@analog.com>
> 
> - Nuno Sá
>
Re: [PATCH 2/8] iio: add enable and disable services to iio backend framework
Posted by Nuno Sá 1 year, 6 months ago 
On Wed, 2024-06-19 at 17:59 +0200, Olivier MOYSAN wrote:
> 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@foss.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.
> > 
> 
> 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.
> 

Oh, yes, you're right. I would prefer your later option. Call
__iio_backend_disable() from __iio_backend_disable() with a proper typed
parameter. I also just realized your 'struct device *dev' parameter. I think it
can be removed for these APIs. The only reason for it is for
devm_add_action_or_reset() which we don't need-

- Nuno Sá
> >
Re: [PATCH 2/8] iio: add enable and disable services to iio backend framework
Posted by Jonathan Cameron 1 year, 5 months ago 
On Thu, 20 Jun 2024 12:07:47 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Wed, 2024-06-19 at 17:59 +0200, Olivier MOYSAN wrote:
> > 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@foss.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.
> > >   
> > 
> > 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.
> >   
> 
> Oh, yes, you're right. I would prefer your later option. Call
> __iio_backend_disable() from __iio_backend_disable() with a proper typed
?  That looks like an infinite loop :)  
> parameter. I also just realized your 'struct device *dev' parameter. I think it
> can be removed for these APIs. The only reason for it is for
> devm_add_action_or_reset() which we don't need-
> 
> - Nuno Sá
> > >   
>
Re: [PATCH 2/8] iio: add enable and disable services to iio backend framework
Posted by Nuno Sá 1 year, 5 months ago 
On Sun, 2024-06-23 at 14:56 +0100, Jonathan Cameron wrote:
> On Thu, 20 Jun 2024 12:07:47 +0200
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Wed, 2024-06-19 at 17:59 +0200, Olivier MOYSAN wrote:
> > > 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@foss.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.
> > > >   
> > > 
> > > 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.
> > >   
> > 
> > Oh, yes, you're right. I would prefer your later option. Call
> > __iio_backend_disable() from __iio_backend_disable() with a proper typed
> ?  That looks like an infinite loop :)  

Ahaha, yes it looks. But hopefully you got what I really meant :)

- Nuno Sá
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.