Re: [PATCH v3 07/10] iio: provide of_iio_channel_get_by_name() and devm_ version it

Jonathan Cameron
2020-09-09T18:10:05Z

On Wed, 9 Sep 2020 17:42:45 +0300
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:

> There might be cases when the IIO channel is attached to the device
> subnode instead of being attached to the main device node. Allow drivers
> to query IIO channels by using device tree nodes.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/iio/inkern.c | 33 +++++++++++++++++++++++++--------
> include/linux/iio/consumer.h | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index ede99e0d5371..3083f886d3da 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -180,8 +180,8 @@ static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
> return ERR_PTR(err);
> }
>
> -static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
> - const char *name)
> +struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
> + const char *name)

Doesn't this need and EXPORT_SYMBOL_GPL?

Otherwise I'm fine with the patch.

Jonathan

> {
> struct iio_channel *chan = NULL;
>
> @@ -261,12 +261,6 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
>
> #else /* CONFIG_OF */
>
> -static inline struct iio_channel *
> -of_iio_channel_get_by_name(struct device_node *np, const char *name)
> -{
> - return NULL;
> -}
> -
> static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
> {
> return NULL;
> @@ -382,6 +376,29 @@ struct iio_channel *devm_iio_channel_get(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(devm_iio_channel_get);
>
> +struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
> + struct device_node *np,
> + 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 = of_iio_channel_get_by_name(np, channel_name);
> + if (IS_ERR(channel)) {
> + devres_free(ptr);
> + return channel;
> + }
> +
> + *ptr = channel;
> + devres_add(dev, ptr);
> +
> + return channel;
> +}
> +EXPORT_SYMBOL_GPL(devm_of_iio_channel_get_by_name);
> +
> 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 c4118dcb8e05..0a90ba8fa1bb 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -13,6 +13,7 @@
> struct iio_dev;
> struct iio_chan_spec;
> struct device;
> +struct device_node;
>
> /**
> * struct iio_channel - everything needed for a consumer to use a channel
> @@ -97,6 +98,41 @@ void iio_channel_release_all(struct iio_channel *chan);
> */
> struct iio_channel *devm_iio_channel_get_all(struct device *dev);
>
> +/**
> + * of_iio_channel_get_by_name() - get description of all that is needed to access channel.
> + * @np: Pointer to consumer device tree node
> + * @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'
> + */
> +#ifdef CONFIG_OF
> +struct iio_channel *of_iio_channel_get_by_name(struct device_node *np, const char *name);
> +#else
> +static inline struct iio_channel *
> +of_iio_channel_get_by_name(struct device_node *np, const char *name)
> +{
> + return NULL;
> +}
> +#endif
> +
> +/**
> + * devm_of_iio_channel_get_by_name() - Resource managed version of of_iio_channel_get_by_name().
> + * @dev: Pointer to consumer device.
> + * @np: Pointer to consumer device tree node
> + * @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_of_iio_channel_get_by_name(struct device *dev,
> + struct device_node *np,
> + const char *consumer_channel);
> +
> struct iio_cb_buffer;
> /**
> * iio_channel_get_all_cb() - register callback for triggered capture