Received-SPF: pass (google.com: domain of linux-kernel+bounces-86770-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 29 Feb 2024 15:42:53 +0200
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Matti Vaittinen <mazziesaccount@gmail.com>
Cc: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>, Marek Vasut <marex@denx.de>,
	Anshul Dalal <anshulusr@gmail.com>,
	Javier Carrasco <javier.carrasco.cruz@gmail.com>,
	Matt Ranostay <matt@ranostay.sg>,
	Stefan Windfeldt-Prytz <stefan.windfeldt-prytz@axis.com>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 5/5] iio: light: Add support for APDS9306 Light Sensor
Message-ID: <ZeCJ3T8HVaQZC1Ps@smile.fi.intel.com>
References: <20240228122408.18619-1-subhajit.ghosh@tweaklogic.com>
 <20240228122408.18619-6-subhajit.ghosh@tweaklogic.com>
 <a828e77c-b3d4-49bb-b0bb-b9fd6cb7d114@gmail.com>
 <Zd9tApJClX7Frq20@smile.fi.intel.com>
 <45386f39-a034-4d70-a6d4-8804c27aadce@tweaklogic.com>
 <21ecfb62-30b7-4073-bad6-46a9e08e08b0@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <21ecfb62-30b7-4073-bad6-46a9e08e08b0@gmail.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Thu, Feb 29, 2024 at 02:58:52PM +0200, Matti Vaittinen wrote:
> On 2/29/24 14:34, Subhajit Ghosh wrote:
> > On 29/2/24 03:57, Andy Shevchenko wrote:
> > > On Wed, Feb 28, 2024 at 03:08:56PM +0200, Matti Vaittinen wrote:
> > > > On 2/28/24 14:24, Subhajit Ghosh wrote:

..

> > > > > +??? if (gain_new < 0) {
> > > > > +??????? dev_err_ratelimited(dev, "Unsupported gain with time\n");
> > > > > +??????? return gain_new;
> > > > > +??? }
> > > 
> > > What is the difference between negative response from the function
> > > itself and
> > > similar in gain_new?
> > > 
> > -ve response form the function is an error condition.
> > -ve value in gain_new means - no valid gains could be computed.
> > In case of error conditions from the function, the gain_new is also set
> > to -1.
> > My use case is valid hardware gain so I went for checking only gain_new.
> > Matti will be the best person to answer on this.
> 
> I now rely on the kerneldoc for the
> iio_gts_find_new_gain_by_old_gain_time() as it seems reasonable to me:
> 
> * Return: 0 if an exactly matching supported new gain was found. When a
> * non-zero value is returned, the @new_gain will be set to a negative or
> * positive value. The negative value means that no gain could be computed.
> * Positive value will be the "best possible new gain there could be". There
> * can be two reasons why finding the "best possible" new gain is not deemed
> * successful. 1) This new value cannot be supported by the hardware. 2) The
> new
> * gain required to maintain the scale would not be an integer. In this case,
> * the "best possible" new gain will be a floored optimal gain, which may or
> * may not be supported by the hardware.

> Eg, if ret is zero, there is no need to check validity of the gain_new but
> it is guaranteed to be one of the supported gains.

Right, but this kernel doc despite being so verbose does not fully answer my
question. What is the semantic of that "negative value"? I would expect to have
the error code there (maybe different to what the function returns), but at
least be able to return it to the upper layers if needed.

Hence 2 ARs I see:
1) clarify the kernel documentation there;
2) update the semantic of the gain_new to simplify caller's code.

-- 
With Best Regards,
Andy Shevchenko