Re: [PATCH] iio: core: Propagate error codes from OF layer to client drivers

[Jonathan Cameron]

On 25/08/14 13:57, Ivan T. Ivanov wrote:
> Do not overwrite error codes returned from of_iio_channel_get().
> Error codes are used to distinguish between "io-channel-names"
> not present in DT bindings, property is optional, and IIO channel
> provider driver still not being loaded, defer probe.
> 
> Signed-off-by: Ivan T. Ivanov <iivanov@xxxxxxxxxx>
Cc'd Guenter who often takes an interest in this code (and wrote it ;)

Mostly seems logical to me, though I don't like the change of
priority in the last bit.  I've also just taken a fix for this
code so there may be some fuzz from that once it's propogated
through to mainline and back to the togreg tree of iio.git

Thanks,

Jonathan
> ---
>  drivers/iio/inkern.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index c749700..66a6cde 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -162,7 +162,7 @@ err_free_channel:
>  static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
>  						      const char *name)
>  {
> -	struct iio_channel *chan = NULL;
> +	struct iio_channel *chan = ERR_PTR(-ENODEV);
>  
>  	/* Walk up the tree of devices looking for a matching iio channel */
>  	while (np) {
> @@ -183,7 +183,7 @@ static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
>  		else if (name && index >= 0) {
>  			pr_err("ERROR: could not get IIO channel %s:%s(%i)\n",
>  				np->full_name, name ? name : "", index);
> -			return NULL;
> +			break;
>  		}
>  
>  		/*
> @@ -193,7 +193,7 @@ static struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
>  		 */
>  		np = np->parent;
>  		if (np && !of_get_property(np, "io-channel-ranges", NULL))
> -			return NULL;
> +			break;
>  	}
>  
>  	return chan;
> @@ -243,12 +243,12 @@ error_free_chans:
>  static inline struct iio_channel *
>  of_iio_channel_get_by_name(struct device_node *np, const char *name)
>  {
> -	return NULL;
> +	return ERR_PTR(-ENODEV);
>  }
>  
>  static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  {
> -	return NULL;
> +	return ERR_PTR(-ENODEV);
>  }
>  
>  #endif /* CONFIG_OF */
> @@ -312,14 +312,14 @@ struct iio_channel *iio_channel_get(struct device *dev,
>  	const char *name = dev ? dev_name(dev) : NULL;
>  	struct iio_channel *channel;
>  
> -	if (dev) {
> -		channel = of_iio_channel_get_by_name(dev->of_node,
> -						     channel_name);
> -		if (channel != NULL)
> -			return channel;
> -	}
> +	channel = iio_channel_get_sys(name, channel_name);
> +	if (!IS_ERR(channel))
> +		return channel;
> +
> +	if (!dev)
> +		return channel;
>  
> -	return iio_channel_get_sys(name, channel_name);
> +	return of_iio_channel_get_by_name(dev->of_node, channel_name);
>  }
Why reorder the logic?   This makes this patch less obviously
correct for limited obvious gain?

Previously the priority was clearly given to device tree bindings
wherease now it is given to board file provided map elements.  It
would be interesting to see boards with both provided, but it is
possible.
>  EXPORT_SYMBOL_GPL(iio_channel_get);
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/