Re: [PATCH v3 05/15] iio: inkern: move to fwnode properties

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

On Fri, 15 Jul 2022 14:28:53 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> This moves the IIO in kernel interface to use fwnode properties and thus
> be firmware agnostic.
> 
> Note that the interface is still not firmware agnostic. At this point we
> have both OF and fwnode interfaces so that we don't break any user. On
> top of this we also want to have a per driver conversion and that is the
> main reason we have both of_xlate() and fwnode_xlate() support.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Nice. One trivial follow through of wrong comment syntax. I'll fix up
if I pick the series up before you do a respin for other reasons.
> ---
>  drivers/iio/inkern.c         | 159 ++++++++++++++++++-----------------
>  include/linux/iio/consumer.h |  36 ++++----
>  include/linux/iio/iio.h      |   5 ++
>  3 files changed, 108 insertions(+), 92 deletions(-)
> 
> -struct iio_channel *__of_iio_channel_get_by_name(struct device_node *np,
> -						 const char *name)
> +struct iio_channel *
> +__fwnode_iio_channel_get_by_name(struct fwnode_handle *fwnode, const char *name)
>  {
>  	struct iio_channel *chan;
>  	int index = 0;
> @@ -220,19 +236,20 @@ struct iio_channel *__of_iio_channel_get_by_name(struct device_node *np,
>  	/*
>  	 * For named iio channels, first look up the name in the
>  	 * "io-channel-names" property.  If it cannot be found, the
> -	 * index will be an error code, and of_iio_channel_get()
> +	 * index will be an error code, and fwnode_iio_channel_get()
>  	 * will fail.
>  	 */
>  	if (name)
> -		index = of_property_match_string(np, "io-channel-names", name);
> +		index = fwnode_property_match_string(fwnode, "io-channel-names",
> +						     name);
>  
> -	chan = of_iio_channel_get(np, index);
> +	chan = fwnode_iio_channel_get(fwnode, index);
>  	if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
>  		return chan;
>  	if (name) {
>  		if (index >= 0) {
> -			pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
> -			       np, name, index);
> +			pr_err("ERROR: could not get IIO channel %pfw:%s(%i)\n",
> +			       fwnode, name, index);
>  			/*
>  			 * In this case, we found 'name' in 'io-channel-names'
>  			 * but somehow we still fail so that we should not proceed
> @@ -242,16 +259,16 @@ struct iio_channel *__of_iio_channel_get_by_name(struct device_node *np,
>  			 */
>  			return ERR_PTR(-EINVAL);
>  		}
> -		/* If index < 0, then of_parse_phandle_with_args() fails
> -		 * with -EINVAL which is expected. We should not proceed
> -		 * if we get any other error.

Wrong comment syntax. I guess I can fix this one as well if nothing else comes up.
Or I might be lazy and only fix this one as it replaces the previous wrong one.

> +		/* If index < 0, then fwnode_property_get_reference_args() fails
> +		 * with -EINVAL or -ENOENT (ACPI case) which is expected. We
> +		 * should not proceed if we get any other error.
>  		 */
> -		if (PTR_ERR(chan) != -EINVAL)
> +		if (PTR_ERR(chan) != -EINVAL && PTR_ERR(chan) != -ENOENT)
>  			return chan;
>  	} else if (PTR_ERR(chan) != -ENOENT) {
>  		/*
>  		 * if !name, then we should only proceed the lookup if
> -		 * of_parse_phandle_with_args() returns -ENOENT.
> +		 * fwnode_property_get_reference_args() returns -ENOENT.
>  		 */
>  		return chan;
>  	}
> @@ -260,13 +277,14 @@ struct iio_channel *__of_iio_channel_get_by_name(struct device_node *np,
>  	return ERR_PTR(-ENODEV);
>  }