linux-kernel - Re: [PATCH v7 03/10] iio: adc: add helpers for parsing ADC nodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z9LdwUVBOEx-Tbvr@smile.fi.intel.com>
Date: Thu, 13 Mar 2025 15:29:37 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
	Jonathan Cameron <jic23@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
	Chen-Yu Tsai <wens@...e.org>,
	Jernej Skrabec <jernej.skrabec@...il.com>,
	Samuel Holland <samuel@...lland.org>,
	Hugo Villeneuve <hvilleneuve@...onoff.com>,
	Nuno Sa <nuno.sa@...log.com>, David Lechner <dlechner@...libre.com>,
	Javier Carrasco <javier.carrasco.cruz@...il.com>,
	Guillaume Stols <gstols@...libre.com>,
	Dumitru Ceclan <mitrutzceclan@...il.com>,
	Trevor Gamblin <tgamblin@...libre.com>,
	Matteo Martelli <matteomartelli3@...il.com>,
	Alisa-Dariana Roman <alisadariana@...il.com>,
	Ramona Alexandra Nechita <ramona.nechita@...log.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
	linux-renesas-soc@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH v7 03/10] iio: adc: add helpers for parsing ADC nodes

On Thu, Mar 13, 2025 at 03:17:27PM +0200, Matti Vaittinen wrote:
> On 13/03/2025 14:31, Andy Shevchenko wrote:
> > On Thu, Mar 13, 2025 at 09:18:18AM +0200, Matti Vaittinen wrote:

...

> > > +	num_chan = iio_adc_device_num_channels(dev);
> > > +	if (num_chan < 1)
> > > +		return num_chan;
> > 
> > This is really interesting code. So, if the above returns negative error code,
> > we return it, if it returns 0, we return success (but 0 channels)?
> 
> Yes. I don't think it's that interesting though. Checking the devicetree
> succeeded while no channels were found. I think returning 0 is very much
> aligned with this.

Right, but as I suggested, let's follow already established APIs that return
-ENOENT and never 0 in similar cases.

> > Shouldn't we do *cs = NULL; at the case of 0 channels if it's a success?
> 
> I suppose you're right.
> 
> But, as you pointed out in review of the 05/10:
> > Usually in other similar APIs we return -ENOENT. And user won't need
> > to have an additional check in case of 0 being considered as an error
> > case too.
> 
> I don't know whether to agree with you here. For majority of the ADC
> drivers, having no channels in devicetree is indeed just another error,
> which I think is not in any ways special.

So...? (I see below your answer :-)

> However, for 33,3333% of the users added in this patch, the "no channels
> found" is not really an error condition ;) The BD79124 could have all
> channels used for GPO - although this would probably be very very unusual.
> (Why buying an ADC chip if you need just a GPO?). Still, this wouldn't be an
> error. (And I need to handle this better in BD79124 probe - so thanks).

ENOENT check is again established for optional/not_found cases.

> > (Under success I assume that returned values are okay to go with, and cs in
> > your case will be left uninitialised or contain something we don't control.
> 
> I see your point although I wouldn't be concerned with cs not being NULL for
> as long as number of channels is zero.
> 
> Anyway, I think it makes sense to simplify ~67% of callers by returning
> -ENODEV if there is no channels. The remaining ~33% can then check for the
> -ENODEV and handle it separately from other returned errors. So, thanks.

Not at all!

-- 
With Best Regards,
Andy Shevchenko