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
