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

2016-04-17 Thread Laxman Dewangan 


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 

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.


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-17 Thread Jonathan Cameron 
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 
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.
>   *
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-16 Thread Laxman Dewangan 


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

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-10 Thread Laxman Dewangan 


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  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().



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-10 Thread Jonathan Cameron 
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 

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.
>   *
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-10 Thread Jonathan Cameron 
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  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(>dev, 0,
> + gti, _thermal_ops);
> + if (IS_ERR(gti->tz_dev)) {
> + ret = PTR_ERR(gti->tz_dev);
> + dev_err(>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;
> +}
> +


> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-06 Thread Laxman Dewangan 

Hi Daniel,


On Wednesday 06 April 2016 07:19 PM, Daniel Baluta wrote:

On Wed, Apr 6, 2016 at 1:31 PM, 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.


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.




--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html