2016-04-06 10:42:11

by Laxman Dewangan

[permalink] [raw]
Subject: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}

Some of kernel driver uses the IIO framework to get the sensor
value via ADC or IIO HW driver. The client driver get iio channel
by iio_channel_get() and release it by calling iio_channel_release().

Add resource managed version (devm_*) of these APIs so that if client
calls the devm_iio_channel_get() then it need not to release it explicitly,
it can be done by managed device framework when driver get un-binded.

This reduces the code in error path and also need of .remove callback in
some cases.

Signed-off-by: Laxman Dewangan <[email protected]>
---
drivers/iio/inkern.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/iio/consumer.h | 27 +++++++++++++++++++++++++
2 files changed, 75 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 734a004..18e623f 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -356,6 +356,54 @@ void iio_channel_release(struct iio_channel *channel)
}
EXPORT_SYMBOL_GPL(iio_channel_release);

+static void devm_iio_channel_free(struct device *dev, void *res)
+{
+ struct iio_channel *channel = *(struct iio_channel **)res;
+
+ iio_channel_release(channel);
+}
+
+static int devm_iio_channel_match(struct device *dev, void *res, void *data)
+{
+ struct iio_channel **r = res;
+
+ if (!r || !*r) {
+ WARN_ON(!r || !*r);
+ return 0;
+ }
+
+ return *r == data;
+}
+
+struct iio_channel *devm_iio_channel_get(struct device *dev,
+ const char *channel_name)
+{
+ struct iio_channel **ptr, *channel;
+
+ ptr = devres_alloc(devm_iio_channel_free, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ channel = iio_channel_get(dev, channel_name);
+ if (IS_ERR(channel)) {
+ devres_free(ptr);
+ return channel;
+ }
+
+ *ptr = channel;
+ devres_add(dev, ptr);
+
+ return channel;
+}
+EXPORT_SYMBOL_GPL(devm_iio_channel_get);
+
+void devm_iio_channel_release(struct device *dev, struct iio_channel *channel)
+{
+ WARN_ON(devres_release(dev, devm_iio_channel_free,
+ devm_iio_channel_match, channel));
+}
+EXPORT_SYMBOL_GPL(devm_iio_channel_release);
+
struct iio_channel *iio_channel_get_all(struct device *dev)
{
const char *name;
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index fad5867..e1e033d 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -49,6 +49,33 @@ struct iio_channel *iio_channel_get(struct device *dev,
void iio_channel_release(struct iio_channel *chan);

/**
+ * devm_iio_channel_get() - Resource managed version of iio_channel_get().
+ * @dev: Pointer to consumer device. Device name must match
+ * the name of the device as provided in the iio_map
+ * with which the desired provider to consumer mapping
+ * was registered.
+ * @consumer_channel: Unique name to identify the channel on the consumer
+ * side. This typically describes the channels use within
+ * the consumer. E.g. 'battery_voltage'
+ *
+ * Returns a pointer to negative errno if it is not able to get the iio channel
+ * otherwise returns valid pointer for iio channel.
+ *
+ * The allocated iio channel is automatically released when the device is
+ * unbound.
+ */
+struct iio_channel *devm_iio_channel_get(struct device *dev,
+ const char *consumer_channel);
+/**
+ * devm_iio_channel_release() - Resource managed version of
+ * iio_channel_release().
+ * @dev: Pointer to consumer device for which resource
+ * is allocared.
+ * @chan: The channel to be released.
+ */
+void devm_iio_channel_release(struct device *dev, struct iio_channel *chan);
+
+/**
* iio_channel_get_all() - get all channels associated with a client
* @dev: Pointer to consumer device.
*
--
2.1.4


2016-04-06 10:42:17

by Laxman Dewangan

[permalink] [raw]
Subject: [PATCH 2/3] iio: core: Add devm_ APIs for iio_channel_{get,release}_all

Some of kernel driver uses the IIO framework to get the sensor
value via ADC or IIO HW driver. The client driver get iio channel
by iio_channel_get_all() and release it by calling
iio_channel_release_all().

Add resource managed version (devm_*) of these APIs so that if client
calls the devm_iio_channel_get_all() then it need not to release it
explicitly, it can be done by managed device framework when driver
get un-binded.

This reduces the code in error path and also need of .remove callback in
some cases.

Signed-off-by: Laxman Dewangan <[email protected]>
---
drivers/iio/inkern.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/iio/consumer.h | 26 ++++++++++++++++++++++++++
2 files changed, 62 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 18e623f..8c1abfe 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -489,6 +489,42 @@ void iio_channel_release_all(struct iio_channel *channels)
}
EXPORT_SYMBOL_GPL(iio_channel_release_all);

+static void devm_iio_channel_free_all(struct device *dev, void *res)
+{
+ struct iio_channel *channels = *(struct iio_channel **)res;
+
+ iio_channel_release_all(channels);
+}
+
+struct iio_channel *devm_iio_channel_get_all(struct device *dev)
+{
+ struct iio_channel **ptr, *channels;
+
+ ptr = devres_alloc(devm_iio_channel_free_all, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ channels = iio_channel_get_all(dev);
+ if (IS_ERR(channels)) {
+ devres_free(ptr);
+ return channels;
+ }
+
+ *ptr = channels;
+ devres_add(dev, ptr);
+
+ return channels;
+}
+EXPORT_SYMBOL_GPL(devm_iio_channel_get_all);
+
+void devm_iio_channel_release_all(struct device *dev,
+ struct iio_channel *channels)
+{
+ WARN_ON(devres_release(dev, devm_iio_channel_free_all,
+ devm_iio_channel_match, channels));
+}
+EXPORT_SYMBOL_GPL(devm_iio_channel_release_all);
+
static int iio_channel_read(struct iio_channel *chan, int *val, int *val2,
enum iio_chan_info_enum info)
{
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index e1e033d..3d672f7 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -92,6 +92,32 @@ struct iio_channel *iio_channel_get_all(struct device *dev);
*/
void iio_channel_release_all(struct iio_channel *chan);

+/**
+ * devm_iio_channel_get_all() - Resource managed version of
+ * iio_channel_get_all().
+ * @dev: Pointer to consumer device.
+ *
+ * Returns a pointer to negative errno if it is not able to get the iio channel
+ * otherwise returns an array of iio_channel structures terminated with one with
+ * null iio_dev pointer.
+ *
+ * This function is used by fairly generic consumers to get all the
+ * channels registered as having this consumer.
+ *
+ * The allocated iio channels are automatically released when the device is
+ * unbounded.
+ */
+struct iio_channel *devm_iio_channel_get_all(struct device *dev);
+
+/**
+ * devm_iio_channel_release_all() - Resource managed version of
+ * iio_channel_release_all().
+ * @dev: Pointer to consumer device for which resource
+ * is allocared.
+ * @chan: Array channel to be released.
+ */
+void devm_iio_channel_release_all(struct device *dev, struct iio_channel *chan);
+
struct iio_cb_buffer;
/**
* iio_channel_get_all_cb() - register callback for triggered capture
--
2.1.4

2016-04-06 10:42:16

by Laxman Dewangan

[permalink] [raw]
Subject: [PATCH 3/3] iio: Add resource managed APIs devm_iio_channel_{get,release) in devres

Add following APIs in the list of managed resources of IIO:
devm_iio_channel_get()
devm_iio_channel_get_all()
devm_iio_channel_release()
devm_iio_channel_release_all()

Signed-off-by: Laxman Dewangan <[email protected]>
---
Documentation/driver-model/devres.txt | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index 73b98df..bdc2dfb 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -267,6 +267,10 @@ IIO
devm_iio_kfifo_free()
devm_iio_trigger_alloc()
devm_iio_trigger_free()
+ devm_iio_channel_get()
+ devm_iio_channel_release()
+ devm_iio_channel_get_all()
+ devm_iio_channel_release_all()

IO region
devm_release_mem_region()
--
2.1.4

2016-04-06 13:49:28

by Daniel Baluta

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}

On Wed, Apr 6, 2016 at 1:31 PM, Laxman Dewangan <[email protected]> wrote:
> Some of kernel driver uses the IIO framework to get the sensor
> value via ADC or IIO HW driver. The client driver get iio channel
> by iio_channel_get() and release it by calling iio_channel_release().
>
> Add resource managed version (devm_*) of these APIs so that if client
> calls the devm_iio_channel_get() then it need not to release it explicitly,
> it can be done by managed device framework when driver get un-binded.
>
> This reduces the code in error path and also need of .remove callback in
> some cases.
>

Please provide at least one example of code that uses this API.

2016-04-06 15:09:57

by Laxman Dewangan

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}

Hi Daniel,


On Wednesday 06 April 2016 07:19 PM, Daniel Baluta wrote:
> On Wed, Apr 6, 2016 at 1:31 PM, Laxman Dewangan <[email protected]> wrote:
>> Some of kernel driver uses the IIO framework to get the sensor
>> value via ADC or IIO HW driver. The client driver get iio channel
>> by iio_channel_get() and release it by calling iio_channel_release().
>>
>> Add resource managed version (devm_*) of these APIs so that if client
>> calls the devm_iio_channel_get() then it need not to release it explicitly,
>> it can be done by managed device framework when driver get un-binded.
>>
>> This reduces the code in error path and also need of .remove callback in
>> some cases.
>>
> Please provide at least one example of code that uses this API.

Most of client for this APIs are in other subsystem.
When I was working on the patch
[PATCH 2/2] thermal: generic-adc: Add ADC based thermal sensor driver

if I have devm_iio_channel_get() then I can get .remove callback at all.

I did not use this new APIs in my patch because they are in different
subsystem.



2016-04-10 14:05:44

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}

On 06/04/16 15:58, Laxman Dewangan wrote:
> Hi Daniel,
>
>
> On Wednesday 06 April 2016 07:19 PM, Daniel Baluta wrote:
>> On Wed, Apr 6, 2016 at 1:31 PM, Laxman Dewangan <[email protected]> wrote:
>>> Some of kernel driver uses the IIO framework to get the sensor
>>> value via ADC or IIO HW driver. The client driver get iio channel
>>> by iio_channel_get() and release it by calling iio_channel_release().
>>>
>>> Add resource managed version (devm_*) of these APIs so that if client
>>> calls the devm_iio_channel_get() then it need not to release it explicitly,
>>> it can be done by managed device framework when driver get un-binded.
>>>
>>> This reduces the code in error path and also need of .remove callback in
>>> some cases.
>>>
>> Please provide at least one example of code that uses this API.
>
> Most of client for this APIs are in other subsystem.
> When I was working on the patch
> [PATCH 2/2] thermal: generic-adc: Add ADC based thermal sensor driver
>
> if I have devm_iio_channel_get() then I can get .remove callback at all.
>
> I did not use this new APIs in my patch because they are in different subsystem.
It's actually worse than that having taken a quick look at the generic-adc thermal patch
you reference above.
(perhaps worth cc'ing linux-iio for next version of that).

Without this devm function set you have a race in remove in which I think you can
get attempts to access the channels after they have been released...

> +
> + gti->tz_dev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0,
> + gti, &gadc_thermal_ops);
> + if (IS_ERR(gti->tz_dev)) {
> + ret = PTR_ERR(gti->tz_dev);
> + dev_err(&pdev->dev, "Thermal zone sensor register failed: %d\n",
> + ret);
> + goto sensor_fail;
> + }
This will get cleaned up in remove 'after' the iio_channels are released.
Hence you have a race in which they can probably be accessed after release
(unless some magic is going on that I've missed).
> +
> + return 0;
> +
> +sensor_fail:
> + iio_channel_release(gti->channel);
> + return ret;
> +}
> +
> +static int gadc_thermal_remove(struct platform_device *pdev)
> +{
> + struct gadc_thermal_info *gti = platform_get_drvdata(pdev);
> +
> + iio_channel_release(gti->channel);
> +
> + return 0;
> +}
> +


>
>
>

2016-04-10 14:08:51

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}

On 06/04/16 11:31, Laxman Dewangan wrote:
> Some of kernel driver uses the IIO framework to get the sensor
> value via ADC or IIO HW driver. The client driver get iio channel
> by iio_channel_get() and release it by calling iio_channel_release().
>
> Add resource managed version (devm_*) of these APIs so that if client
> calls the devm_iio_channel_get() then it need not to release it explicitly,
> it can be done by managed device framework when driver get un-binded.
>
> This reduces the code in error path and also need of .remove callback in
> some cases.
>
> Signed-off-by: Laxman Dewangan <[email protected]>

I'm fine with this - but would like it to sit for a few more days to see
if it gets any other feedback.
> ---
> drivers/iio/inkern.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/iio/consumer.h | 27 +++++++++++++++++++++++++
> 2 files changed, 75 insertions(+)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 734a004..18e623f 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -356,6 +356,54 @@ void iio_channel_release(struct iio_channel *channel)
> }
> EXPORT_SYMBOL_GPL(iio_channel_release);
>
> +static void devm_iio_channel_free(struct device *dev, void *res)
> +{
> + struct iio_channel *channel = *(struct iio_channel **)res;
> +
> + iio_channel_release(channel);
> +}
> +
> +static int devm_iio_channel_match(struct device *dev, void *res, void *data)
> +{
> + struct iio_channel **r = res;
> +
> + if (!r || !*r) {
> + WARN_ON(!r || !*r);
> + return 0;
> + }
> +
> + return *r == data;
> +}
> +
> +struct iio_channel *devm_iio_channel_get(struct device *dev,
> + const char *channel_name)
> +{
> + struct iio_channel **ptr, *channel;
> +
> + ptr = devres_alloc(devm_iio_channel_free, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + channel = iio_channel_get(dev, channel_name);
> + if (IS_ERR(channel)) {
> + devres_free(ptr);
> + return channel;
> + }
> +
> + *ptr = channel;
> + devres_add(dev, ptr);
> +
> + return channel;
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_channel_get);
> +
> +void devm_iio_channel_release(struct device *dev, struct iio_channel *channel)
> +{
> + WARN_ON(devres_release(dev, devm_iio_channel_free,
> + devm_iio_channel_match, channel));
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_channel_release);
> +
> struct iio_channel *iio_channel_get_all(struct device *dev)
> {
> const char *name;
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index fad5867..e1e033d 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -49,6 +49,33 @@ struct iio_channel *iio_channel_get(struct device *dev,
> void iio_channel_release(struct iio_channel *chan);
>
> /**
> + * devm_iio_channel_get() - Resource managed version of iio_channel_get().
> + * @dev: Pointer to consumer device. Device name must match
> + * the name of the device as provided in the iio_map
> + * with which the desired provider to consumer mapping
> + * was registered.
> + * @consumer_channel: Unique name to identify the channel on the consumer
> + * side. This typically describes the channels use within
> + * the consumer. E.g. 'battery_voltage'
> + *
> + * Returns a pointer to negative errno if it is not able to get the iio channel
> + * otherwise returns valid pointer for iio channel.
> + *
> + * The allocated iio channel is automatically released when the device is
> + * unbound.
> + */
> +struct iio_channel *devm_iio_channel_get(struct device *dev,
> + const char *consumer_channel);
> +/**
> + * devm_iio_channel_release() - Resource managed version of
> + * iio_channel_release().
> + * @dev: Pointer to consumer device for which resource
> + * is allocared.
> + * @chan: The channel to be released.
> + */
> +void devm_iio_channel_release(struct device *dev, struct iio_channel *chan);
> +
> +/**
> * iio_channel_get_all() - get all channels associated with a client
> * @dev: Pointer to consumer device.
> *
>

2016-04-10 17:46:20

by Laxman Dewangan

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}


On Sunday 10 April 2016 07:35 PM, Jonathan Cameron wrote:
> On 06/04/16 15:58, Laxman Dewangan wrote:
>> Hi Daniel,
>>
>>
>> On Wednesday 06 April 2016 07:19 PM, Daniel Baluta wrote:
>>> On Wed, Apr 6, 2016 at 1:31 PM, Laxman Dewangan <[email protected]> wrote:
>>>> Some of kernel driver uses the IIO framework to get the sensor
>>>> value via ADC or IIO HW driver. The client driver get iio channel
>>>> by iio_channel_get() and release it by calling iio_channel_release().
>>>>
>>>> Add resource managed version (devm_*) of these APIs so that if client
>>>> calls the devm_iio_channel_get() then it need not to release it explicitly,
>>>> it can be done by managed device framework when driver get un-binded.
>>>>
>>>> This reduces the code in error path and also need of .remove callback in
>>>> some cases.
>>>>
>>> Please provide at least one example of code that uses this API.
>> Most of client for this APIs are in other subsystem.
>> When I was working on the patch
>> [PATCH 2/2] thermal: generic-adc: Add ADC based thermal sensor driver
>>
>> if I have devm_iio_channel_get() then I can get .remove callback at all.
>>
>> I did not use this new APIs in my patch because they are in different subsystem.
> It's actually worse than that having taken a quick look at the generic-adc thermal patch
> you reference above.
> (perhaps worth cc'ing linux-iio for next version of that).
Sure. I will CC.

>
> Without this devm function set you have a race in remove in which I think you can
> get attempts to access the channels after they have been released...
Yaah, possibly race for very small time possible.

The limitation of devm_ api usage is that, we can keep using this till
we have devm_ api continuous and if some resource are not there for
devm_ then we can not use further.
Possibly, I need to wait for the devm_iio_channel_get() to merge and
available for all subsystem to use (next release) and then only I can
use devm_thermal_zone_of_sensor_register().


2016-04-16 13:25:17

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}

On 10/04/16 18:35, Laxman Dewangan wrote:
>
> On Sunday 10 April 2016 07:35 PM, Jonathan Cameron wrote:
>> On 06/04/16 15:58, Laxman Dewangan wrote:
>>> Hi Daniel,
>>>
>>>
>>> On Wednesday 06 April 2016 07:19 PM, Daniel Baluta wrote:
>>>> On Wed, Apr 6, 2016 at 1:31 PM, Laxman Dewangan <[email protected]> wrote:
>>>>> Some of kernel driver uses the IIO framework to get the sensor
>>>>> value via ADC or IIO HW driver. The client driver get iio channel
>>>>> by iio_channel_get() and release it by calling iio_channel_release().
>>>>>
>>>>> Add resource managed version (devm_*) of these APIs so that if client
>>>>> calls the devm_iio_channel_get() then it need not to release it explicitly,
>>>>> it can be done by managed device framework when driver get un-binded.
>>>>>
>>>>> This reduces the code in error path and also need of .remove callback in
>>>>> some cases.
>>>>>
>>>> Please provide at least one example of code that uses this API.
>>> Most of client for this APIs are in other subsystem.
>>> When I was working on the patch
>>> [PATCH 2/2] thermal: generic-adc: Add ADC based thermal sensor driver
>>>
>>> if I have devm_iio_channel_get() then I can get .remove callback at all.
>>>
>>> I did not use this new APIs in my patch because they are in different subsystem.
>> It's actually worse than that having taken a quick look at the generic-adc thermal patch
>> you reference above.
>> (perhaps worth cc'ing linux-iio for next version of that).
> Sure. I will CC.
>
>>
>> Without this devm function set you have a race in remove in which I think you can
>> get attempts to access the channels after they have been released...
> Yaah, possibly race for very small time possible.
>
> The limitation of devm_ api usage is that, we can keep using this
> till we have devm_ api continuous and if some resource are not there
> for devm_ then we can not use further. Possibly, I need to wait for
> the devm_iio_channel_get() to merge and available for all subsystem
> to use (next release) and then only I can use
> devm_thermal_zone_of_sensor_register().
>
The alternative would be to merge this devm_ support as a prerequisite for your thermal
patches and have it go through that tree. As it's self contained I have
no particular problem with that if you'd prefer to do it that way.

Otherwise, you will need to do as you say above (not use
devm_thermal_zone_of_sensor_register) to make sure it isn't broken in the meantime.

Jonathan

2016-04-16 15:54:18

by Laxman Dewangan

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}


On Saturday 16 April 2016 06:55 PM, Jonathan Cameron wrote:
> On 10/04/16 18:35, Laxman Dewangan wrote:
>>
>> Yaah, possibly race for very small time possible.
>>
>> The limitation of devm_ api usage is that, we can keep using this
>> till we have devm_ api continuous and if some resource are not there
>> for devm_ then we can not use further. Possibly, I need to wait for
>> the devm_iio_channel_get() to merge and available for all subsystem
>> to use (next release) and then only I can use
>> devm_thermal_zone_of_sensor_register().
>>
> The alternative would be to merge this devm_ support as a prerequisite for your thermal
> patches and have it go through that tree. As it's self contained I have
> no particular problem with that if you'd prefer to do it that way.
>
> Otherwise, you will need to do as you say above (not use
> devm_thermal_zone_of_sensor_register) to make sure it isn't broken in the meantime.
>
>

I have recycled patch of thermal driver to not use devm_ for sensor
registration.

I will post another patch in future once these changes will be available
for all, most probably next release.

I think there is no further comment now on the series so you can
consider for next step.

Thanks,
Laxman

2016-04-17 12:00:23

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}

On 06/04/16 11:31, Laxman Dewangan wrote:
> Some of kernel driver uses the IIO framework to get the sensor
> value via ADC or IIO HW driver. The client driver get iio channel
> by iio_channel_get() and release it by calling iio_channel_release().
>
> Add resource managed version (devm_*) of these APIs so that if client
> calls the devm_iio_channel_get() then it need not to release it explicitly,
> it can be done by managed device framework when driver get un-binded.
>
> This reduces the code in error path and also need of .remove callback in
> some cases.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
Applied to the togreg branch of iio.git.
I guess the thermal driver use case will hit a cycle or so behind this.

Thanks,

Jonathan
> ---
> drivers/iio/inkern.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/iio/consumer.h | 27 +++++++++++++++++++++++++
> 2 files changed, 75 insertions(+)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 734a004..18e623f 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -356,6 +356,54 @@ void iio_channel_release(struct iio_channel *channel)
> }
> EXPORT_SYMBOL_GPL(iio_channel_release);
>
> +static void devm_iio_channel_free(struct device *dev, void *res)
> +{
> + struct iio_channel *channel = *(struct iio_channel **)res;
> +
> + iio_channel_release(channel);
> +}
> +
> +static int devm_iio_channel_match(struct device *dev, void *res, void *data)
> +{
> + struct iio_channel **r = res;
> +
> + if (!r || !*r) {
> + WARN_ON(!r || !*r);
> + return 0;
> + }
> +
> + return *r == data;
> +}
> +
> +struct iio_channel *devm_iio_channel_get(struct device *dev,
> + const char *channel_name)
> +{
> + struct iio_channel **ptr, *channel;
> +
> + ptr = devres_alloc(devm_iio_channel_free, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + channel = iio_channel_get(dev, channel_name);
> + if (IS_ERR(channel)) {
> + devres_free(ptr);
> + return channel;
> + }
> +
> + *ptr = channel;
> + devres_add(dev, ptr);
> +
> + return channel;
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_channel_get);
> +
> +void devm_iio_channel_release(struct device *dev, struct iio_channel *channel)
> +{
> + WARN_ON(devres_release(dev, devm_iio_channel_free,
> + devm_iio_channel_match, channel));
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_channel_release);
> +
> struct iio_channel *iio_channel_get_all(struct device *dev)
> {
> const char *name;
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index fad5867..e1e033d 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -49,6 +49,33 @@ struct iio_channel *iio_channel_get(struct device *dev,
> void iio_channel_release(struct iio_channel *chan);
>
> /**
> + * devm_iio_channel_get() - Resource managed version of iio_channel_get().
> + * @dev: Pointer to consumer device. Device name must match
> + * the name of the device as provided in the iio_map
> + * with which the desired provider to consumer mapping
> + * was registered.
> + * @consumer_channel: Unique name to identify the channel on the consumer
> + * side. This typically describes the channels use within
> + * the consumer. E.g. 'battery_voltage'
> + *
> + * Returns a pointer to negative errno if it is not able to get the iio channel
> + * otherwise returns valid pointer for iio channel.
> + *
> + * The allocated iio channel is automatically released when the device is
> + * unbound.
> + */
> +struct iio_channel *devm_iio_channel_get(struct device *dev,
> + const char *consumer_channel);
> +/**
> + * devm_iio_channel_release() - Resource managed version of
> + * iio_channel_release().
> + * @dev: Pointer to consumer device for which resource
> + * is allocared.
> + * @chan: The channel to be released.
> + */
> +void devm_iio_channel_release(struct device *dev, struct iio_channel *chan);
> +
> +/**
> * iio_channel_get_all() - get all channels associated with a client
> * @dev: Pointer to consumer device.
> *
>

2016-04-17 12:06:10

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 2/3] iio: core: Add devm_ APIs for iio_channel_{get,release}_all

On 06/04/16 11:31, Laxman Dewangan wrote:
> Some of kernel driver uses the IIO framework to get the sensor
> value via ADC or IIO HW driver. The client driver get iio channel
> by iio_channel_get_all() and release it by calling
> iio_channel_release_all().
>
> Add resource managed version (devm_*) of these APIs so that if client
> calls the devm_iio_channel_get_all() then it need not to release it
> explicitly, it can be done by managed device framework when driver
> get un-binded.
>
> This reduces the code in error path and also need of .remove callback in
> some cases.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
Applied to the togreg branch of iio.git. Thanks,

Jonathan
> ---
> drivers/iio/inkern.c | 36 ++++++++++++++++++++++++++++++++++++
> include/linux/iio/consumer.h | 26 ++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 18e623f..8c1abfe 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -489,6 +489,42 @@ void iio_channel_release_all(struct iio_channel *channels)
> }
> EXPORT_SYMBOL_GPL(iio_channel_release_all);
>
> +static void devm_iio_channel_free_all(struct device *dev, void *res)
> +{
> + struct iio_channel *channels = *(struct iio_channel **)res;
> +
> + iio_channel_release_all(channels);
> +}
> +
> +struct iio_channel *devm_iio_channel_get_all(struct device *dev)
> +{
> + struct iio_channel **ptr, *channels;
> +
> + ptr = devres_alloc(devm_iio_channel_free_all, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + channels = iio_channel_get_all(dev);
> + if (IS_ERR(channels)) {
> + devres_free(ptr);
> + return channels;
> + }
> +
> + *ptr = channels;
> + devres_add(dev, ptr);
> +
> + return channels;
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_channel_get_all);
> +
> +void devm_iio_channel_release_all(struct device *dev,
> + struct iio_channel *channels)
> +{
> + WARN_ON(devres_release(dev, devm_iio_channel_free_all,
> + devm_iio_channel_match, channels));
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_channel_release_all);
> +
> static int iio_channel_read(struct iio_channel *chan, int *val, int *val2,
> enum iio_chan_info_enum info)
> {
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index e1e033d..3d672f7 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -92,6 +92,32 @@ struct iio_channel *iio_channel_get_all(struct device *dev);
> */
> void iio_channel_release_all(struct iio_channel *chan);
>
> +/**
> + * devm_iio_channel_get_all() - Resource managed version of
> + * iio_channel_get_all().
> + * @dev: Pointer to consumer device.
> + *
> + * Returns a pointer to negative errno if it is not able to get the iio channel
> + * otherwise returns an array of iio_channel structures terminated with one with
> + * null iio_dev pointer.
> + *
> + * This function is used by fairly generic consumers to get all the
> + * channels registered as having this consumer.
> + *
> + * The allocated iio channels are automatically released when the device is
> + * unbounded.
> + */
> +struct iio_channel *devm_iio_channel_get_all(struct device *dev);
> +
> +/**
> + * devm_iio_channel_release_all() - Resource managed version of
> + * iio_channel_release_all().
> + * @dev: Pointer to consumer device for which resource
> + * is allocared.
> + * @chan: Array channel to be released.
> + */
> +void devm_iio_channel_release_all(struct device *dev, struct iio_channel *chan);
> +
> struct iio_cb_buffer;
> /**
> * iio_channel_get_all_cb() - register callback for triggered capture
>

2016-04-17 17:00:29

by Laxman Dewangan

[permalink] [raw]
Subject: Re: [PATCH 1/3] iio: core: Add devm_ APIs for iio_channel_{get,release}


On Sunday 17 April 2016 05:30 PM, Jonathan Cameron wrote:
> On 06/04/16 11:31, Laxman Dewangan wrote:
>> Some of kernel driver uses the IIO framework to get the sensor
>> value via ADC or IIO HW driver. The client driver get iio channel
>> by iio_channel_get() and release it by calling iio_channel_release().
>>
>> Add resource managed version (devm_*) of these APIs so that if client
>> calls the devm_iio_channel_get() then it need not to release it explicitly,
>> it can be done by managed device framework when driver get un-binded.
>>
>> This reduces the code in error path and also need of .remove callback in
>> some cases.
>>
>> Signed-off-by: Laxman Dewangan <[email protected]>
> Applied to the togreg branch of iio.git.
> I guess the thermal driver use case will hit a cycle or so behind this.
>

Thanks for applying this.

Yes, thermal driver will use this in future release. This is to avoid
any syncup between subsystem.