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-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html