All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: "Nuno Sá" <nuno.sa@analog.com>
Cc: <linux-imx@nxp.com>, <linux-renesas-soc@vger.kernel.org>,
	<linux-mips@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<chrome-platform@lists.linux.dev>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	<linux-mediatek@lists.infradead.org>,
	<linux-stm32@st-md-mailman.stormreply.com>,
	<linux-arm-msm@vger.kernel.org>, <linux-iio@vger.kernel.org>,
	<openbmc@lists.ozlabs.org>, Cai Huoqing <cai.huoqing@linux.dev>,
	Benjamin Fair <benjaminfair@google.com>,
	Jishnu Prakash <quic_jprakash@quicinc.com>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Amit Kucheria <amitk@kernel.org>, Andy Gross <agross@kernel.org>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	Haibo Chen <haibo.chen@nxp.com>,
	Benson Leung <bleung@chromium.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	Christophe Branchereau <cbranchereau@gmail.com>,
	Patrick Venture <venture@google.com>,
	Arnd Bergmann <arnd@arndb.de>, Nancy Yuen <yuenn@google.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	"Gwendal Grignou" <gwendal@chromium.org>,
	Saravanan Sekar <sravanhome@gmail.com>,
	"Tali Perry" <tali.perry1@gmail.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Paul Cercueil <paul@crapouillou.net>,
	Thara Gopinath <thara.gopinath@linaro.org>,
	Avi Fishman <avifishman70@gmail.com>,
	"Lorenzo Bianconi" <lorenzo@kernel.org>,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Fabrice Gasnier <fabrice.gasnier@foss.st.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Tomer Maimon <tmaimon77@gmail.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>,
	Zhang Rui <rui.zhang@intel.com>, Shawn Guo <shawnguo@kernel.org>,
	"Guenter Roeck" <groeck@chromium.org>,
	Fabio Estevam <festevam@gmail.com>,
	"Olivier Moysan" <olivier.moysan@foss.st.com>,
	Eugen Hristev <eugen.hristev@microchip.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Subject: Re: [PATCH 22/34] iio: inkern: only return error codes in iio_channel_get_*() APIs
Date: Sat, 11 Jun 2022 16:17:01 +0100	[thread overview]
Message-ID: <20220611161701.46a68837@jic23-huawei> (raw)
In-Reply-To: <20220610084545.547700-23-nuno.sa@analog.com>

On Fri, 10 Jun 2022 10:45:33 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> APIs like of_iio_channel_get_by_name() and of_iio_channel_get_all() were
> returning a mix of NULL and error pointers being NULL the way to

pointers with NULL being the way to...

> "notify" that we should do a "system" lookup for channels. This make
> it very confusing and prone to errors as commit dbbccf7c20bf
> ("iio: inkern: fix return value in devm_of_iio_channel_get_by_name()")
> proves. On top of this, patterns like 'if (channel != NULL) return channel'
> were being used where channel could actually be an error code which
> makes the code hard to read.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/inkern.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 87fd2a0d44f2..31d9c122199a 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -214,7 +214,7 @@ static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
>  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;
>  
>  	/* Walk up the tree of devices looking for a matching iio channel */
>  	while (np) {
> @@ -231,11 +231,11 @@ struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
>  							 name);
>  		chan = of_iio_channel_get(np, index);
>  		if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
> -			break;

This original behaviour is 'interesting'. If we get a error like -ENOMEM
we should return it rather than carry on.  Do we have enough knowledge
of possible errors here to be more explicit on when we keep looking up
the tree?  I think we can get -ENOENT from of_parse_phandle_with_args()

That raises an interesting question on whether -ENODEV is the right response
for the previously NULL case or is -ENOENT more consistent with other
of_ functions?  No device could be thought of as being the case that needs
to defer (in hope it turns up later) whereas no entry means it will never
succeed.

> -		else if (name && index >= 0) {
> +			return chan;
> +		if (name && index >= 0) {
>  			pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
>  				np, name ? name : "", index);
> -			return NULL;
> +			return chan;
>  		}
>  
>  		/*
> @@ -245,10 +245,10 @@ 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;
> +			return chan;
>  	}
>  
> -	return chan;
> +	return ERR_PTR(-ENODEV);
>  }
>  EXPORT_SYMBOL_GPL(of_iio_channel_get_by_name);
>  
> @@ -267,8 +267,8 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  			break;
>  	} while (++nummaps);
>  
> -	if (nummaps == 0)	/* no error, return NULL to search map table */
> -		return NULL;
> +	if (nummaps == 0)	/* return -ENODEV to search map table */
> +		return ERR_PTR(-ENODEV);
>  
>  	/* NULL terminated array to save passing size */
>  	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
> @@ -295,7 +295,7 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  
>  static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  {
> -	return NULL;
> +	return ERR_PTR(-ENODEV);
>  }
>  
>  #endif /* CONFIG_OF */
> @@ -362,7 +362,7 @@ struct iio_channel *iio_channel_get(struct device *dev,
>  	if (dev) {
>  		channel = of_iio_channel_get_by_name(dev->of_node,
>  						     channel_name);
> -		if (channel != NULL)
> +		if (!IS_ERR(channel) || PTR_ERR(channel) == -EPROBE_DEFER)
>  			return channel;
>  	}
>  
> @@ -412,8 +412,6 @@ struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
>  	channel = of_iio_channel_get_by_name(np, channel_name);
>  	if (IS_ERR(channel))
>  		return channel;
> -	if (!channel)
> -		return ERR_PTR(-ENODEV);
>  
>  	ret = devm_add_action_or_reset(dev, devm_iio_channel_free, channel);
>  	if (ret)
> @@ -436,7 +434,7 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
>  		return ERR_PTR(-EINVAL);
>  
>  	chans = of_iio_channel_get_all(dev);
> -	if (chans)
> +	if (!IS_ERR(chans) || PTR_ERR(chans) == -EPROBE_DEFER)

Hmm. We only want to carry on if the error is -ENODEV.  Anything else
should be reported up the stack.

That might be the only error left, but I think we should be explicit.

>  		return chans;
>  
>  	name = dev_name(dev);


WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <jic23@kernel.org>
To: "Nuno Sá" <nuno.sa@analog.com>
Cc: <linux-imx@nxp.com>, <linux-renesas-soc@vger.kernel.org>,
	<linux-mips@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<chrome-platform@lists.linux.dev>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	<linux-mediatek@lists.infradead.org>,
	<linux-stm32@st-md-mailman.stormreply.com>,
	<linux-arm-msm@vger.kernel.org>, <linux-iio@vger.kernel.org>,
	<openbmc@lists.ozlabs.org>, Cai Huoqing <cai.huoqing@linux.dev>,
	Benjamin Fair <benjaminfair@google.com>,
	Jishnu Prakash <quic_jprakash@quicinc.com>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Amit Kucheria <amitk@kernel.org>, Andy Gross <agross@kernel.org>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	Haibo Chen <haibo.chen@nxp.com>,
	Benson Leung <bleung@chromium.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	Christophe Branchereau <cbranchereau@gmail.com>,
	Patrick Venture <venture@google.com>,
	Arnd Bergmann <arnd@arndb.de>, Nancy Yuen <yuenn@google.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	"Gwendal Grignou" <gwendal@chromium.org>,
	Saravanan Sekar <sravanhome@gmail.com>,
	"Tali Perry" <tali.perry1@gmail.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Paul Cercueil <paul@crapouillou.net>,
	Thara Gopinath <thara.gopinath@linaro.org>,
	Avi Fishman <avifishman70@gmail.com>,
	"Lorenzo Bianconi" <lorenzo@kernel.org>,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Fabrice Gasnier <fabrice.gasnier@foss.st.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Tomer Maimon <tmaimon77@gmail.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>,
	Zhang Rui <rui.zhang@intel.com>, Shawn Guo <shawnguo@kernel.org>,
	"Guenter Roeck" <groeck@chromium.org>,
	Fabio Estevam <festevam@gmail.com>,
	"Olivier Moysan" <olivier.moysan@foss.st.com>,
	Eugen Hristev <eugen.hristev@microchip.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Subject: Re: [PATCH 22/34] iio: inkern: only return error codes in iio_channel_get_*() APIs
Date: Sat, 11 Jun 2022 16:17:01 +0100	[thread overview]
Message-ID: <20220611161701.46a68837@jic23-huawei> (raw)
In-Reply-To: <20220610084545.547700-23-nuno.sa@analog.com>

On Fri, 10 Jun 2022 10:45:33 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> APIs like of_iio_channel_get_by_name() and of_iio_channel_get_all() were
> returning a mix of NULL and error pointers being NULL the way to

pointers with NULL being the way to...

> "notify" that we should do a "system" lookup for channels. This make
> it very confusing and prone to errors as commit dbbccf7c20bf
> ("iio: inkern: fix return value in devm_of_iio_channel_get_by_name()")
> proves. On top of this, patterns like 'if (channel != NULL) return channel'
> were being used where channel could actually be an error code which
> makes the code hard to read.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/inkern.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 87fd2a0d44f2..31d9c122199a 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -214,7 +214,7 @@ static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
>  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;
>  
>  	/* Walk up the tree of devices looking for a matching iio channel */
>  	while (np) {
> @@ -231,11 +231,11 @@ struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
>  							 name);
>  		chan = of_iio_channel_get(np, index);
>  		if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
> -			break;

This original behaviour is 'interesting'. If we get a error like -ENOMEM
we should return it rather than carry on.  Do we have enough knowledge
of possible errors here to be more explicit on when we keep looking up
the tree?  I think we can get -ENOENT from of_parse_phandle_with_args()

That raises an interesting question on whether -ENODEV is the right response
for the previously NULL case or is -ENOENT more consistent with other
of_ functions?  No device could be thought of as being the case that needs
to defer (in hope it turns up later) whereas no entry means it will never
succeed.

> -		else if (name && index >= 0) {
> +			return chan;
> +		if (name && index >= 0) {
>  			pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
>  				np, name ? name : "", index);
> -			return NULL;
> +			return chan;
>  		}
>  
>  		/*
> @@ -245,10 +245,10 @@ 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;
> +			return chan;
>  	}
>  
> -	return chan;
> +	return ERR_PTR(-ENODEV);
>  }
>  EXPORT_SYMBOL_GPL(of_iio_channel_get_by_name);
>  
> @@ -267,8 +267,8 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  			break;
>  	} while (++nummaps);
>  
> -	if (nummaps == 0)	/* no error, return NULL to search map table */
> -		return NULL;
> +	if (nummaps == 0)	/* return -ENODEV to search map table */
> +		return ERR_PTR(-ENODEV);
>  
>  	/* NULL terminated array to save passing size */
>  	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
> @@ -295,7 +295,7 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  
>  static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  {
> -	return NULL;
> +	return ERR_PTR(-ENODEV);
>  }
>  
>  #endif /* CONFIG_OF */
> @@ -362,7 +362,7 @@ struct iio_channel *iio_channel_get(struct device *dev,
>  	if (dev) {
>  		channel = of_iio_channel_get_by_name(dev->of_node,
>  						     channel_name);
> -		if (channel != NULL)
> +		if (!IS_ERR(channel) || PTR_ERR(channel) == -EPROBE_DEFER)
>  			return channel;
>  	}
>  
> @@ -412,8 +412,6 @@ struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
>  	channel = of_iio_channel_get_by_name(np, channel_name);
>  	if (IS_ERR(channel))
>  		return channel;
> -	if (!channel)
> -		return ERR_PTR(-ENODEV);
>  
>  	ret = devm_add_action_or_reset(dev, devm_iio_channel_free, channel);
>  	if (ret)
> @@ -436,7 +434,7 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
>  		return ERR_PTR(-EINVAL);
>  
>  	chans = of_iio_channel_get_all(dev);
> -	if (chans)
> +	if (!IS_ERR(chans) || PTR_ERR(chans) == -EPROBE_DEFER)

Hmm. We only want to carry on if the error is -ENODEV.  Anything else
should be reported up the stack.

That might be the only error left, but I think we should be explicit.

>  		return chans;
>  
>  	name = dev_name(dev);


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <jic23@kernel.org>
To: "Nuno Sá" <nuno.sa@analog.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Tomer Maimon <tmaimon77@gmail.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	linux-iio@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>,
	Amit Kucheria <amitk@kernel.org>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Tali Perry <tali.perry1@gmail.com>,
	Paul Cercueil <paul@crapouillou.net>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Guenter Roeck <groeck@chromium.org>,
	Fabio Estevam <festevam@gmail.com>,
	linux-stm32@st-md-mailman.stormreply.com,
	chrome-platform@lists.linux.dev,
	Lars-Peter Clausen <lars@metafoo.de>,
	Benjamin Fair <benjaminfair@google.com>,
	openbmc@lists.ozlabs.org,
	Jishnu Prakash <quic_jprakash@quicinc.com>,
	Haibo Chen <haibo.chen@nxp.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Andy Gross <agross@kernel.org>,
	linux-imx@nxp.com, Olivier Moysan <olivier.moysan@foss.st.com>,
	Zhang Rui <rui.zhang@intel.com>,
	Christophe Branchereau <cbranchereau@gmail.com>,
	 Saravanan Sekar <sravanhome@gmail.com>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	linux-arm-msm@vger.kernel.org,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Fabrice Gasnier <fabrice.gasnier@foss.st.com>,
	linux-mediatek@lists.infradead.org,
	Eugen Hristev <eugen.hristev@microchip.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Gwendal Grignou <gwendal@chromium.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Benson Leung <bleung@chromium.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	Lorenzo Bianconi <lorenzo@kernel.org>,
	Avi Fishman <avifishman70@gmail.com>,
	Patrick Venture <venture@google.com>,
	linux-mips@vger.kernel.org,
	Thara Gopinath <thara.gopinath@linaro.org>,
	linux-renesas-soc@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Cai Huoqing <cai.huoqing@linux.dev>,
	Shawn Guo <shawnguo@kernel.org>,
	Claudiu Beznea <claudiu.beznea@microchip.com>
Subject: Re: [PATCH 22/34] iio: inkern: only return error codes in iio_channel_get_*() APIs
Date: Sat, 11 Jun 2022 16:17:01 +0100	[thread overview]
Message-ID: <20220611161701.46a68837@jic23-huawei> (raw)
In-Reply-To: <20220610084545.547700-23-nuno.sa@analog.com>

On Fri, 10 Jun 2022 10:45:33 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> APIs like of_iio_channel_get_by_name() and of_iio_channel_get_all() were
> returning a mix of NULL and error pointers being NULL the way to

pointers with NULL being the way to...

> "notify" that we should do a "system" lookup for channels. This make
> it very confusing and prone to errors as commit dbbccf7c20bf
> ("iio: inkern: fix return value in devm_of_iio_channel_get_by_name()")
> proves. On top of this, patterns like 'if (channel != NULL) return channel'
> were being used where channel could actually be an error code which
> makes the code hard to read.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/inkern.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 87fd2a0d44f2..31d9c122199a 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -214,7 +214,7 @@ static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
>  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;
>  
>  	/* Walk up the tree of devices looking for a matching iio channel */
>  	while (np) {
> @@ -231,11 +231,11 @@ struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
>  							 name);
>  		chan = of_iio_channel_get(np, index);
>  		if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
> -			break;

This original behaviour is 'interesting'. If we get a error like -ENOMEM
we should return it rather than carry on.  Do we have enough knowledge
of possible errors here to be more explicit on when we keep looking up
the tree?  I think we can get -ENOENT from of_parse_phandle_with_args()

That raises an interesting question on whether -ENODEV is the right response
for the previously NULL case or is -ENOENT more consistent with other
of_ functions?  No device could be thought of as being the case that needs
to defer (in hope it turns up later) whereas no entry means it will never
succeed.

> -		else if (name && index >= 0) {
> +			return chan;
> +		if (name && index >= 0) {
>  			pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
>  				np, name ? name : "", index);
> -			return NULL;
> +			return chan;
>  		}
>  
>  		/*
> @@ -245,10 +245,10 @@ 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;
> +			return chan;
>  	}
>  
> -	return chan;
> +	return ERR_PTR(-ENODEV);
>  }
>  EXPORT_SYMBOL_GPL(of_iio_channel_get_by_name);
>  
> @@ -267,8 +267,8 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  			break;
>  	} while (++nummaps);
>  
> -	if (nummaps == 0)	/* no error, return NULL to search map table */
> -		return NULL;
> +	if (nummaps == 0)	/* return -ENODEV to search map table */
> +		return ERR_PTR(-ENODEV);
>  
>  	/* NULL terminated array to save passing size */
>  	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
> @@ -295,7 +295,7 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  
>  static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  {
> -	return NULL;
> +	return ERR_PTR(-ENODEV);
>  }
>  
>  #endif /* CONFIG_OF */
> @@ -362,7 +362,7 @@ struct iio_channel *iio_channel_get(struct device *dev,
>  	if (dev) {
>  		channel = of_iio_channel_get_by_name(dev->of_node,
>  						     channel_name);
> -		if (channel != NULL)
> +		if (!IS_ERR(channel) || PTR_ERR(channel) == -EPROBE_DEFER)
>  			return channel;
>  	}
>  
> @@ -412,8 +412,6 @@ struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
>  	channel = of_iio_channel_get_by_name(np, channel_name);
>  	if (IS_ERR(channel))
>  		return channel;
> -	if (!channel)
> -		return ERR_PTR(-ENODEV);
>  
>  	ret = devm_add_action_or_reset(dev, devm_iio_channel_free, channel);
>  	if (ret)
> @@ -436,7 +434,7 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
>  		return ERR_PTR(-EINVAL);
>  
>  	chans = of_iio_channel_get_all(dev);
> -	if (chans)
> +	if (!IS_ERR(chans) || PTR_ERR(chans) == -EPROBE_DEFER)

Hmm. We only want to carry on if the error is -ENODEV.  Anything else
should be reported up the stack.

That might be the only error left, but I think we should be explicit.

>  		return chans;
>  
>  	name = dev_name(dev);


  parent reply	other threads:[~2022-06-11 15:08 UTC|newest]

Thread overview: 246+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-10  8:45 [PATCH 00/34] make iio inkern interface firmware agnostic Nuno Sá
2022-06-10  8:45 ` Nuno Sá
2022-06-10  8:45 ` Nuno Sá
2022-06-10  8:45 ` Nuno Sá
2022-06-10  8:45 ` [PATCH 01/34] iio: adc: ad7606: explicitly add proper header files Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 13:59   ` Jonathan Cameron
2022-06-11 13:59     ` Jonathan Cameron
2022-06-11 13:59     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 02/34] iio: adc: ad7606_par: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:00   ` Jonathan Cameron
2022-06-11 14:00     ` Jonathan Cameron
2022-06-11 14:00     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 03/34] iio: adc: berlin2-adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:02   ` Jonathan Cameron
2022-06-11 14:02     ` Jonathan Cameron
2022-06-11 14:02     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 04/34] iio: adc: imx7d_adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:04   ` Jonathan Cameron
2022-06-11 14:04     ` Jonathan Cameron
2022-06-11 14:04     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 05/34] iio: adc: imx8qxp-adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:05   ` Jonathan Cameron
2022-06-11 14:05     ` Jonathan Cameron
2022-06-11 14:05     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 06/34] iio: adc: ingenic-adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10 14:45   ` Andy Shevchenko
2022-06-10 14:45     ` Andy Shevchenko
2022-06-10 19:49     ` Nuno Sá
2022-06-10 19:49       ` Nuno Sá
2022-06-10 19:49       ` Nuno Sá
2022-06-11 14:07       ` Jonathan Cameron
2022-06-11 14:07         ` Jonathan Cameron
2022-06-11 14:07         ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 07/34] iio: adc: mp2629_adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:08   ` Jonathan Cameron
2022-06-11 14:08     ` Jonathan Cameron
2022-06-11 14:08     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 08/34] iio: adc: mt6360-adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:09   ` Jonathan Cameron
2022-06-11 14:09     ` Jonathan Cameron
2022-06-11 14:09     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 09/34] iio: adc: npcm_adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:12   ` Jonathan Cameron
2022-06-11 14:12     ` Jonathan Cameron
2022-06-11 14:12     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 10/34] iio: adc: rzg2l_adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:13   ` Jonathan Cameron
2022-06-11 14:13     ` Jonathan Cameron
2022-06-11 14:13     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 11/34] iio: common: cros_ec_lid_angle: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:14   ` Jonathan Cameron
2022-06-11 14:14     ` Jonathan Cameron
2022-06-11 14:14     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 12/34] iio: common: cros_ec_sensors: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:16   ` Jonathan Cameron
2022-06-11 14:16     ` Jonathan Cameron
2022-06-11 14:16     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 13/34] iio: dac: stm32-dac: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:17   ` Jonathan Cameron
2022-06-11 14:17     ` Jonathan Cameron
2022-06-11 14:17     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 14/34] iio: dac: vf610_dac: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:19   ` Jonathan Cameron
2022-06-11 14:19     ` Jonathan Cameron
2022-06-11 14:19     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 15/34] iio: humidity: hts221_buffer: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10 14:47   ` Andy Shevchenko
2022-06-10 14:47     ` Andy Shevchenko
2022-06-11 14:22     ` Jonathan Cameron
2022-06-11 14:22       ` Jonathan Cameron
2022-06-11 14:22       ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 16/34] iio: light: cros_ec_light_prox: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:23   ` Jonathan Cameron
2022-06-11 14:23     ` Jonathan Cameron
2022-06-11 14:23     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 17/34] iio: pressure: cros_ec_baro: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45 ` [PATCH 18/34] iio: trigger: stm32-lptimer-trigger: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45 ` [PATCH 19/34] iio: core: drop of.h from iio.h Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 14:30   ` Jonathan Cameron
2022-06-11 14:30     ` Jonathan Cameron
2022-06-11 14:30     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 20/34] iio: inkern: only relase the device node when done with it Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10 14:56   ` Andy Shevchenko
2022-06-10 14:56     ` Andy Shevchenko
2022-06-10 20:08     ` Nuno Sá
2022-06-10 20:08       ` Nuno Sá
2022-06-10 20:08       ` Nuno Sá
2022-06-11 14:59       ` Jonathan Cameron
2022-06-11 14:59         ` Jonathan Cameron
2022-06-11 14:59         ` Jonathan Cameron
2022-06-13  7:20         ` Nuno Sá
2022-06-13  7:20           ` Nuno Sá
2022-06-13  7:20           ` Nuno Sá
2022-06-18 14:03           ` Jonathan Cameron
2022-06-18 14:03             ` Jonathan Cameron
2022-06-18 14:13           ` Jonathan Cameron
2022-06-18 14:13             ` Jonathan Cameron
2022-06-18 17:30   ` Jonathan Cameron
2022-06-18 17:30     ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 21/34] iio: inkern: fix return value in devm_of_iio_channel_get_by_name() Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10 14:56   ` Andy Shevchenko
2022-06-10 14:56     ` Andy Shevchenko
2022-06-10  8:45 ` [PATCH 22/34] iio: inkern: only return error codes in iio_channel_get_*() APIs Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10 15:05   ` Andy Shevchenko
2022-06-10 15:05     ` Andy Shevchenko
2022-06-10 19:48     ` Nuno Sá
2022-06-10 19:48       ` Nuno Sá
2022-06-10 19:48       ` Nuno Sá
2022-06-11 15:17   ` Jonathan Cameron [this message]
2022-06-11 15:17     ` Jonathan Cameron
2022-06-11 15:17     ` Jonathan Cameron
2022-06-13  7:06     ` Nuno Sá
2022-06-13  7:06       ` Nuno Sá
2022-06-13  7:06       ` Nuno Sá
2022-06-18 14:06       ` Jonathan Cameron
2022-06-18 14:06         ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 23/34] iio: inkern: split of_iio_channel_get_by_name() Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10 15:07   ` Andy Shevchenko
2022-06-10 15:07     ` Andy Shevchenko
2022-06-10  8:45 ` [PATCH 24/34] iio: inkern: move to fwnode properties Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10 15:19   ` Andy Shevchenko
2022-06-10 15:19     ` Andy Shevchenko
2022-06-10 20:01     ` Nuno Sá
2022-06-10 20:01       ` Nuno Sá
2022-06-10 20:01       ` Nuno Sá
2022-06-11 15:30       ` Jonathan Cameron
2022-06-11 15:30         ` Jonathan Cameron
2022-06-11 15:30         ` Jonathan Cameron
2022-06-11 15:32         ` Jonathan Cameron
2022-06-11 15:32           ` Jonathan Cameron
2022-06-13  7:13           ` Nuno Sá
2022-06-18 14:09             ` Jonathan Cameron
2022-06-10  8:45 ` [PATCH 25/34] thermal: qcom: qcom-spmi-adc-tm5: convert to IIO fwnode API Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10 15:20   ` Andy Shevchenko
2022-06-10 15:20     ` Andy Shevchenko
2022-06-10 19:42     ` Nuno Sá
2022-06-10 19:42       ` Nuno Sá
2022-06-10 19:42       ` Nuno Sá
2022-06-10  8:45 ` [PATCH 26/34] iio: adc: ingenic-adc: convert to IIO fwnode interface Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45 ` [PATCH 27/34] iio: adc: ab8500-gpadc: convert to device properties Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-15 14:26   ` Linus Walleij
2022-06-15 14:26     ` Linus Walleij
2022-06-10  8:45 ` [PATCH 28/34] iio: adc: at91-sama5d2_adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45 ` [PATCH 29/34] iio: adc: qcom-pm8xxx-xoadc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-15 14:27   ` Linus Walleij
2022-06-15 14:27     ` Linus Walleij
2022-06-10  8:45 ` [PATCH 30/34] iio: adc: qcom-spmi-vadc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-16 13:15   ` Linus Walleij
2022-06-16 13:15     ` Linus Walleij
2022-06-10  8:45 ` [PATCH 31/34] iio: adc: qcom-spmi-adc5: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-16 13:16   ` Linus Walleij
2022-06-16 13:16     ` Linus Walleij
2022-06-10  8:45 ` [PATCH 32/34] iio: adc: stm32-adc: " Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-11 15:47   ` Jonathan Cameron
2022-06-11 15:47     ` Jonathan Cameron
2022-06-11 15:47     ` Jonathan Cameron
2022-06-17 15:58     ` Fabrice Gasnier
2022-06-17 15:58       ` Fabrice Gasnier
2022-06-10  8:45 ` [PATCH 33/34] iio: inkern: remove OF dependencies Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45 ` [PATCH 34/34] iio: inkern: fix coding style warnings Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10  8:45   ` Nuno Sá
2022-06-10 15:53   ` Joe Simmons-Talbott
2022-06-10 15:53     ` Joe Simmons-Talbott
2022-06-10 15:53     ` Joe Simmons-Talbott
2022-06-10 19:51     ` Nuno Sá
2022-06-10 19:51       ` Nuno Sá
2022-06-10 19:51       ` Nuno Sá
2022-06-12 17:39       ` Geert Uytterhoeven
2022-06-12 17:39         ` Geert Uytterhoeven
2022-06-12 17:39         ` Geert Uytterhoeven
2022-06-13  7:23         ` Nuno Sá
2022-06-13  7:23           ` Nuno Sá
2022-06-13  7:23           ` Nuno Sá
2022-06-10 14:48 ` [PATCH 00/34] make iio inkern interface firmware agnostic Andy Shevchenko
2022-06-10 14:48   ` Andy Shevchenko
2022-06-10 15:28   ` Andy Shevchenko
2022-06-10 15:28     ` Andy Shevchenko
2022-06-11 15:50     ` Jonathan Cameron
2022-06-11 15:50       ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220611161701.46a68837@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Michael.Hennerich@analog.com \
    --cc=agross@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=amitk@kernel.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=arnd@arndb.de \
    --cc=avifishman70@gmail.com \
    --cc=benjaminfair@google.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=bleung@chromium.org \
    --cc=cai.huoqing@linux.dev \
    --cc=cbranchereau@gmail.com \
    --cc=chrome-platform@lists.linux.dev \
    --cc=claudiu.beznea@microchip.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=eugen.hristev@microchip.com \
    --cc=fabrice.gasnier@foss.st.com \
    --cc=festevam@gmail.com \
    --cc=groeck@chromium.org \
    --cc=gwendal@chromium.org \
    --cc=haibo.chen@nxp.com \
    --cc=kernel@pengutronix.de \
    --cc=lars@metafoo.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=lorenzo@kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=nicolas.ferre@microchip.com \
    --cc=nuno.sa@analog.com \
    --cc=olivier.moysan@foss.st.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=paul@crapouillou.net \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=quic_jprakash@quicinc.com \
    --cc=rafael@kernel.org \
    --cc=rui.zhang@intel.com \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=sravanhome@gmail.com \
    --cc=tali.perry1@gmail.com \
    --cc=thara.gopinath@linaro.org \
    --cc=tmaimon77@gmail.com \
    --cc=venture@google.com \
    --cc=yuenn@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.