Received-SPF: pass (google.com: domain of linux-kernel+bounces-103437-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 14 Mar 2024 14:51:09 +0000
From: Jonathan Cameron <jic23@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Vasileios Amoiridis <vassilisamir@gmail.com>, lars@metafoo.de,
 ang.iglesiasg@gmail.com, mazziesaccount@gmail.com, ak@it-klinger.de,
 petre.rodan@subdimension.ro, linus.walleij@linaro.org,
 phil@raspberrypi.com, 579lpy@gmail.com, linux-iio@vger.kernel.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/6] iio: pressure: Simplify and make more clear
 temperature readings
Message-ID: <20240314145109.232f6d05@jic23-huawei>
In-Reply-To: <ZfH4znQ2FK6j5LWK@smile.fi.intel.com>
References: <20240313174007.1934983-1-vassilisamir@gmail.com>
	<20240313174007.1934983-5-vassilisamir@gmail.com>
	<ZfH4znQ2FK6j5LWK@smile.fi.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Wed, 13 Mar 2024 21:04:46 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, Mar 13, 2024 at 06:40:05PM +0100, Vasileios Amoiridis wrote:
> > The read_press/read_humid functions need the updated t_fine value  
> 
> read_press()/read_humid()
> 
> 
> > in order to calculate the current pressure/humidity. Temperature
> > reads should be removed from the read_press/read_humid functions  
> 
> read_press()/read_humid()
> 
> > and should be placed in the oneshot captures before the pressure
> > and humidity reads. This makes the code more intuitive.  
> 
> ...
> 
> > +			if (strcmp(indio_dev->name, "bmp580"))
> > +				data->chip_info->read_temp(data);
> > +  
> 
> > +			if (strcmp(indio_dev->name, "bmp580"))
> > +				data->chip_info->read_temp(data);  
> 
> Yeah, not a fan of the strcmp().
> 
Yes - that's a non starter.  Add a flag to say it is necessary to
chip_info that doesn't rely on name matching.  If you do it the way
you have here, you have to add another strcmp for each new device
supported that needs this code to run.  Add a flag and you just
set that in the chip_info structure.  Much more flexible and extensible.
Usual description of this is "when you can do things with data rather than
code, do it with data".

Jonathan