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

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

[Ivan T. Ivanov]

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@mm-sol.com>
---
 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);
 }
 EXPORT_SYMBOL_GPL(iio_channel_get);
 
-- 
1.9.1

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@mm-sol.com>
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);
>  
> 

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

[Guenter Roeck]

On Mon, Aug 25, 2014 at 05:10:31PM +0100, Jonathan Cameron wrote:
> 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@mm-sol.com>
> 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.

I am not entirely sure I understand what problem this patch is supposed
to fix on top of the patch you just applied, and I am also a bit concerned
about reversing the logic. Also, iio_channel_get_sys can return -ENOMEM
and -EINVAL besides -ENODEV, all of which is now being ignored unless dev is
set, and then it is returned unconditionally. So instead of ignoring error
codes from of_iio_channel_get_by_name, the code now ignores error codes
from iio_channel_get_sys under some circumstances (which, coincidentially,
does not return -EPROBE_DEFER), and in other circumstances may return an error
even if devicetree data exists. Why and how is that better than before ?
Seems to me it just introduces a whole number of new failure conditions.

Guenter

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

[Ivan T. Ivanov]

Hi,

On Mon, 2014-08-25 at 09:54 -0700, Guenter Roeck wrote:
> On Mon, Aug 25, 2014 at 05:10:31PM +0100, Jonathan Cameron wrote:
> > 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@mm-sol.com>
> > 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

It is propagated, but exactly this patch [1] is causing the troubles :-)

> > 
> > 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?

I would like to avoid white listing EPROBE_DEFER error code on return of
of_get() function.

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

I see.

> 
> I am not entirely sure I understand what problem this patch is supposed
> to fix on top of the patch you just applied, 

Patch [1], Guenter please fix you date, introduce regression. It
breaks deferred probe mechanism.

> and I am also a bit concerned
> about reversing the logic. Also, iio_channel_get_sys can return -ENOMEM
> and -EINVAL besides -ENODEV, all of which is now being ignored 

Yes, and thats why we continue with trying to find channel using OF.

> unless dev is set, and then it is returned unconditionally. 

If dev is not valid there is no point to go and ask OF layer for
channel, so jut go out with error code from get_sys()

> So instead of ignoring error
> codes from of_iio_channel_get_by_name, the code now ignores error codes
> from iio_channel_get_sys under some circumstances (which, coincidentially,
> does not return -EPROBE_DEFER), and in other circumstances may return an error
> even if devicetree data exists. Why and how is that better than before ?
> Seems to me it just introduces a whole number of new failure conditions.

We should not mask error codes from OF layer. EPROBE_DEFER is one of
them. So instead of checking for them explicitly on return from of_iio_get(),
I decided that will be future proof if I just reverse the order of functions. 

Another, less intrusive, solution will be if we revert last patch and explicitly
check for EPROBE_DEFER on of_ by_name() return. How this sounds?

Regards,
Ivan


[1] commit a2c12493ed7e63a18cef33a71686d12ffcd6600e
Author: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Date:   Thu Nov 6 12:11:00 2014 +0000

    iio: of_iio_channel_get_by_name() returns non-null pointers for error legs
 

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

[Guenter Roeck]

On 08/26/2014 12:51 AM, Ivan T. Ivanov wrote:
> Hi,
>
[ ... ]
>
> Another, less intrusive, solution will be if we revert last patch and explicitly
> check for EPROBE_DEFER on of_ by_name() return. How this sounds?
>
How is that different to the just accepted patch ?

Guenter

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

[Ivan T. Ivanov]

On Tue, 2014-08-26 at 06:25 -0700, Guenter Roeck wrote:
> On 08/26/2014 12:51 AM, Ivan T. Ivanov wrote:
> > Hi,
> >
> [ ... ]
> >
> > Another, less intrusive, solution will be if we revert last patch and explicitly
> > check for EPROBE_DEFER on of_ by_name() return. How this sounds?
> >
> How is that different to the just accepted patch ?

You mean this one[1]. I have prepared fix last Friday and forget to
check again before posting it, sorry. Please ignore my patch.

Thanks, 
Ivan

[1] iio:inkern: fix overwritten -EPROBE_DEFER in of_iio_channel_get_by_name

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

[Opensource [Adam Thomson]]

T24gQXVndXN0IDI2LCAyMDE0IDE0OjQ4LCBJdmFuIFQuIEl2YW5vdiB3cm90ZToNCg0KPiBPbiBU
dWUsIDIwMTQtMDgtMjYgYXQgMDY6MjUgLTA3MDAsIEd1ZW50ZXIgUm9lY2sgd3JvdGU6DQo+ID4g
T24gMDgvMjYvMjAxNCAxMjo1MSBBTSwgSXZhbiBULiBJdmFub3Ygd3JvdGU6DQo+ID4gPiBIaSwN
Cj4gPiA+DQo+ID4gWyAuLi4gXQ0KPiA+ID4NCj4gPiA+IEFub3RoZXIsIGxlc3MgaW50cnVzaXZl
LCBzb2x1dGlvbiB3aWxsIGJlIGlmIHdlIHJldmVydCBsYXN0IHBhdGNoIGFuZCBleHBsaWNpdGx5
DQo+ID4gPiBjaGVjayBmb3IgRVBST0JFX0RFRkVSIG9uIG9mXyBieV9uYW1lKCkgcmV0dXJuLiBI
b3cgdGhpcyBzb3VuZHM/DQo+ID4gPg0KPiA+IEhvdyBpcyB0aGF0IGRpZmZlcmVudCB0byB0aGUg
anVzdCBhY2NlcHRlZCBwYXRjaCA/DQo+IA0KPiBZb3UgbWVhbiB0aGlzIG9uZVsxXS4gSSBoYXZl
IHByZXBhcmVkIGZpeCBsYXN0IEZyaWRheSBhbmQgZm9yZ2V0IHRvDQo+IGNoZWNrIGFnYWluIGJl
Zm9yZSBwb3N0aW5nIGl0LCBzb3JyeS4gUGxlYXNlIGlnbm9yZSBteSBwYXRjaC4NCj4gDQo+IFRo
YW5rcywNCj4gSXZhbg0KPiANCj4gWzFdIGlpbzppbmtlcm46IGZpeCBvdmVyd3JpdHRlbiAtRVBS
T0JFX0RFRkVSIGluIG9mX2lpb19jaGFubmVsX2dldF9ieV9uYW1lDQo+IA0KDQpBcG9sb2dpZXMg
b24gbXkgcGFydCBmb3IgZml4aW5nIG9uZSBwcm9ibGVtIGFuZCBpbnRyb2R1Y2luZyBhbm90aGVy
LiBEaWRuJ3Qgc2VlDQp0aGF0IGluIG15IHRlc3RpbmcsIGFuZCBtaXNzZWQgdGhhdCBwb3RlbnRp
YWwgcmV0dXJuIHZhbHVlIHdoZW4gZXhhbWluaW5nIHRoZQ0KY29kZS4gQXQgdGhlIHRpbWUgSXQg
bG9va2VkIGxpa2UgdGhlIHBhcmVudCBmdW5jdGlvbiB3b3VsZCBvbmx5IGV4cGVjdCBOVUxMDQpw
b2ludGVycyBmb3IgZmFpbHVyZSwgZXNwZWNpYWxseSBnaXZlbiB0aGUgbm9uIENPTkZJR19PRiBk
ZWZpbml0aW9ucyBvZiB0aGUNCmZ1bmN0aW9ucy4gU2hvdWxkJ3ZlIGRlbHZlZCBmdXJ0aGVyLiA6
KA0K

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

[Adam Thomson]

T24gQXVndXN0IDI2LCAyMDE0IDE1OjM2LCBBZGFtIFRob21zb24gd3JvdGU6DQoNCj4gT24gQXVn
dXN0IDI2LCAyMDE0IDE0OjQ4LCBJdmFuIFQuIEl2YW5vdiB3cm90ZToNCj4NCj4gPiBPbiBUdWUs
IDIwMTQtMDgtMjYgYXQgMDY6MjUgLTA3MDAsIEd1ZW50ZXIgUm9lY2sgd3JvdGU6DQo+ID4gPiBP
biAwOC8yNi8yMDE0IDEyOjUxIEFNLCBJdmFuIFQuIEl2YW5vdiB3cm90ZToNCj4gPiA+ID4gSGks
DQo+ID4gPiA+DQo+ID4gPiBbIC4uLiBdDQo+ID4gPiA+DQo+ID4gPiA+IEFub3RoZXIsIGxlc3Mg
aW50cnVzaXZlLCBzb2x1dGlvbiB3aWxsIGJlIGlmIHdlIHJldmVydCBsYXN0IHBhdGNoIGFuZCBl
eHBsaWNpdGx5DQo+ID4gPiA+IGNoZWNrIGZvciBFUFJPQkVfREVGRVIgb24gb2ZfIGJ5X25hbWUo
KSByZXR1cm4uIEhvdyB0aGlzIHNvdW5kcz8NCj4gPiA+ID4NCj4gPiA+IEhvdyBpcyB0aGF0IGRp
ZmZlcmVudCB0byB0aGUganVzdCBhY2NlcHRlZCBwYXRjaCA/DQo+ID4NCj4gPiBZb3UgbWVhbiB0
aGlzIG9uZVsxXS4gSSBoYXZlIHByZXBhcmVkIGZpeCBsYXN0IEZyaWRheSBhbmQgZm9yZ2V0IHRv
DQo+ID4gY2hlY2sgYWdhaW4gYmVmb3JlIHBvc3RpbmcgaXQsIHNvcnJ5LiBQbGVhc2UgaWdub3Jl
IG15IHBhdGNoLg0KPiA+DQo+ID4gVGhhbmtzLA0KPiA+IEl2YW4NCj4gPg0KPiA+IFsxXSBpaW86
aW5rZXJuOiBmaXggb3ZlcndyaXR0ZW4gLUVQUk9CRV9ERUZFUiBpbiBvZl9paW9fY2hhbm5lbF9n
ZXRfYnlfbmFtZQ0KPiA+DQo+DQo+IEFwb2xvZ2llcyBvbiBteSBwYXJ0IGZvciBmaXhpbmcgb25l
IHByb2JsZW0gYW5kIGludHJvZHVjaW5nIGFub3RoZXIuIERpZG4ndCBzZWUNCj4gdGhhdCBpbiBt
eSB0ZXN0aW5nLCBhbmQgbWlzc2VkIHRoYXQgcG90ZW50aWFsIHJldHVybiB2YWx1ZSB3aGVuIGV4
YW1pbmluZyB0aGUNCj4gY29kZS4gQXQgdGhlIHRpbWUgSXQgbG9va2VkIGxpa2UgdGhlIHBhcmVu
dCBmdW5jdGlvbiB3b3VsZCBvbmx5IGV4cGVjdCBOVUxMDQo+IHBvaW50ZXJzIGZvciBmYWlsdXJl
LCBlc3BlY2lhbGx5IGdpdmVuIHRoZSBub24gQ09ORklHX09GIGRlZmluaXRpb25zIG9mIHRoZQ0K
PiBmdW5jdGlvbnMuIFNob3VsZCd2ZSBkZWx2ZWQgZnVydGhlci4gOigNCg0KT24gQXVndXN0IDI2
LCAyMDE0IDE1OjM2LCBBZGFtIFRob21zb24gd3JvdGU6DQoNCj4gT24gQXVndXN0IDI2LCAyMDE0
IDE0OjQ4LCBJdmFuIFQuIEl2YW5vdiB3cm90ZToNCj4NCj4gPiBPbiBUdWUsIDIwMTQtMDgtMjYg
YXQgMDY6MjUgLTA3MDAsIEd1ZW50ZXIgUm9lY2sgd3JvdGU6DQo+ID4gPiBPbiAwOC8yNi8yMDE0
IDEyOjUxIEFNLCBJdmFuIFQuIEl2YW5vdiB3cm90ZToNCj4gPiA+ID4gSGksDQo+ID4gPiA+DQo+
ID4gPiBbIC4uLiBdDQo+ID4gPiA+DQo+ID4gPiA+IEFub3RoZXIsIGxlc3MgaW50cnVzaXZlLCBz
b2x1dGlvbiB3aWxsIGJlIGlmIHdlIHJldmVydCBsYXN0IHBhdGNoIGFuZCBleHBsaWNpdGx5DQo+
ID4gPiA+IGNoZWNrIGZvciBFUFJPQkVfREVGRVIgb24gb2ZfIGJ5X25hbWUoKSByZXR1cm4uIEhv
dyB0aGlzIHNvdW5kcz8NCj4gPiA+ID4NCj4gPiA+IEhvdyBpcyB0aGF0IGRpZmZlcmVudCB0byB0
aGUganVzdCBhY2NlcHRlZCBwYXRjaCA/DQo+ID4NCj4gPiBZb3UgbWVhbiB0aGlzIG9uZVsxXS4g
SSBoYXZlIHByZXBhcmVkIGZpeCBsYXN0IEZyaWRheSBhbmQgZm9yZ2V0IHRvDQo+ID4gY2hlY2sg
YWdhaW4gYmVmb3JlIHBvc3RpbmcgaXQsIHNvcnJ5LiBQbGVhc2UgaWdub3JlIG15IHBhdGNoLg0K
PiA+DQo+ID4gVGhhbmtzLA0KPiA+IEl2YW4NCj4gPg0KPiA+IFsxXSBpaW86aW5rZXJuOiBmaXgg
b3ZlcndyaXR0ZW4gLUVQUk9CRV9ERUZFUiBpbiBvZl9paW9fY2hhbm5lbF9nZXRfYnlfbmFtZQ0K
PiA+DQo+DQo+IEFwb2xvZ2llcyBvbiBteSBwYXJ0IGZvciBmaXhpbmcgb25lIHByb2JsZW0gYW5k
IGludHJvZHVjaW5nIGFub3RoZXIuIERpZG4ndCBzZWUNCj4gdGhhdCBpbiBteSB0ZXN0aW5nLCBh
bmQgbWlzc2VkIHRoYXQgcG90ZW50aWFsIHJldHVybiB2YWx1ZSB3aGVuIGV4YW1pbmluZyB0aGUN
Cj4gY29kZS4gQXQgdGhlIHRpbWUgSXQgbG9va2VkIGxpa2UgdGhlIHBhcmVudCBmdW5jdGlvbiB3
b3VsZCBvbmx5IGV4cGVjdCBOVUxMDQo+IHBvaW50ZXJzIGZvciBmYWlsdXJlLCBlc3BlY2lhbGx5
IGdpdmVuIHRoZSBub24gQ09ORklHX09GIGRlZmluaXRpb25zIG9mIHRoZQ0KPiBmdW5jdGlvbnMu
IFNob3VsZCd2ZSBkZWx2ZWQgZnVydGhlci4gOigNCg0KSW4gdGVzdGluZyBwYXRjaCBjb2RlIGZv
ciBvbmUgb2Ygb3VyIGRldmljZXMsIEkndmUgdmVyaWZpZWQgd2hhdCBHdWVudGVyDQptZW50aW9u
ZWQgcHJldmlvdXNseSBpbiB0aGlzIHRocmVhZCwgaW4gdGhhdCBpaW9fY2hhbm5lbF9nZXRfc3lz
KCkgZG9lc24ndA0KcmV0dXJuIC1FUFJPQkVfREVGRVIuIFRoaXMgbWVhbnMgdGhhdCBkcml2ZXJz
IHVzaW5nIHRoZSBkZWZhdWx0IG1hcCBhcHByb2FjaCBvZg0KYWNjZXNzaW5nIElJTyBjaGFubmVs
cyB3aWxsIGZhaWwgdG8gaW5zdGFudGlhdGUgYXMgdGhleSBkbyBub3Qga25vdyB0aGV5DQpuZWVk
IHRvIGRlZmVyIHRoZWlyIHByb2JlIChoYXZlIHNlZW4gdGhpcyB3aXRoIG15IGRyaXZlciB0b28p
Lg0KDQpIYXZlbid0IGxvb2tlZCBpbiBkZXRhaWwgeWV0IGFzIHRvIGhvdywgYnV0IEkgZ3Vlc3Mg
dGhpcyBpcyBzb21ldGhpbmcgdGhhdCB3aWxsDQphbHNvIG5lZWQgdG8gYmUgYWRkcmVzc2VkLg0K
TGVnYWwgRGlzY2xhaW1lcjogVGhpcyBlLW1haWwgY29tbXVuaWNhdGlvbiAoYW5kIGFueSBhdHRh
Y2htZW50L3MpIGlzIGNvbmZpZGVudGlhbCBhbmQgY29udGFpbnMgcHJvcHJpZXRhcnkgaW5mb3Jt
YXRpb24sIHNvbWUgb3IgYWxsIG9mIHdoaWNoIG1heSBiZSBsZWdhbGx5IHByaXZpbGVnZWQuIEl0
IGlzIGludGVuZGVkIHNvbGVseSBmb3IgdGhlIHVzZSBvZiB0aGUgaW5kaXZpZHVhbCBvciBlbnRp
dHkgdG8gd2hpY2ggaXQgaXMgYWRkcmVzc2VkLiBBY2Nlc3MgdG8gdGhpcyBlbWFpbCBieSBhbnlv
bmUgZWxzZSBpcyB1bmF1dGhvcml6ZWQuIElmIHlvdSBhcmUgbm90IHRoZSBpbnRlbmRlZCByZWNp
cGllbnQsIGFueSBkaXNjbG9zdXJlLCBjb3B5aW5nLCBkaXN0cmlidXRpb24gb3IgYW55IGFjdGlv
biB0YWtlbiBvciBvbWl0dGVkIHRvIGJlIHRha2VuIGluIHJlbGlhbmNlIG9uIGl0LCBpcyBwcm9o
aWJpdGVkIGFuZCBtYXkgYmUgdW5sYXdmdWwuDQoNClBsZWFzZSBjb25zaWRlciB0aGUgZW52aXJv
bm1lbnQgYmVmb3JlIHByaW50aW5nIHRoaXMgZS1tYWlsDQoNCg0K

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

[Jonathan Cameron]

On 27/08/14 16:59, Adam Thomson wrote:
> On August 26, 2014 15:36, Adam Thomson wrote:
> 
>> On August 26, 2014 14:48, Ivan T. Ivanov wrote:
>>
>>> On Tue, 2014-08-26 at 06:25 -0700, Guenter Roeck wrote:
>>>> On 08/26/2014 12:51 AM, Ivan T. Ivanov wrote:
>>>>> Hi,
>>>>>
>>>> [ ... ]
>>>>>
>>>>> Another, less intrusive, solution will be if we revert last patch and explicitly
>>>>> check for EPROBE_DEFER on of_ by_name() return. How this sounds?
>>>>>
>>>> How is that different to the just accepted patch ?
>>>
>>> You mean this one[1]. I have prepared fix last Friday and forget to
>>> check again before posting it, sorry. Please ignore my patch.
>>>
>>> Thanks,
>>> Ivan
>>>
>>> [1] iio:inkern: fix overwritten -EPROBE_DEFER in of_iio_channel_get_by_name
>>>
>>
>> Apologies on my part for fixing one problem and introducing another. Didn't see
>> that in my testing, and missed that potential return value when examining the
>> code. At the time It looked like the parent function would only expect NULL
>> pointers for failure, especially given the non CONFIG_OF definitions of the
>> functions. Should've delved further. :(
> 
> On August 26, 2014 15:36, Adam Thomson wrote:
> 
>> On August 26, 2014 14:48, Ivan T. Ivanov wrote:
>>
>>> On Tue, 2014-08-26 at 06:25 -0700, Guenter Roeck wrote:
>>>> On 08/26/2014 12:51 AM, Ivan T. Ivanov wrote:
>>>>> Hi,
>>>>>
>>>> [ ... ]
>>>>>
>>>>> Another, less intrusive, solution will be if we revert last patch and explicitly
>>>>> check for EPROBE_DEFER on of_ by_name() return. How this sounds?
>>>>>
>>>> How is that different to the just accepted patch ?
>>>
>>> You mean this one[1]. I have prepared fix last Friday and forget to
>>> check again before posting it, sorry. Please ignore my patch.
>>>
>>> Thanks,
>>> Ivan
>>>
>>> [1] iio:inkern: fix overwritten -EPROBE_DEFER in of_iio_channel_get_by_name
>>>
>>
>> Apologies on my part for fixing one problem and introducing another. Didn't see
>> that in my testing, and missed that potential return value when examining the
>> code. At the time It looked like the parent function would only expect NULL
>> pointers for failure, especially given the non CONFIG_OF definitions of the
>> functions. Should've delved further. :(
> 
> In testing patch code for one of our devices, I've verified what Guenter
> mentioned previously in this thread, in that iio_channel_get_sys() doesn't
> return -EPROBE_DEFER. This means that drivers using the default map approach of
> accessing IIO channels will fail to instantiate as they do not know they
> need to defer their probe (have seen this with my driver too).
> 
> Haven't looked in detail yet as to how, but I guess this is something that will
> also need to be addressed.
Yes, that stuff (I think) predates deferred probing and no one has gotten around
to bringing it up to date as yet.

All contributions welcome!

J

> Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.
> 
> Please consider the environment before printing this e-mail
> 
>