Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Thu, 30 Nov 2023 15:33:02 +0200
From:   Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To:     Petre Rodan <petre.rodan@subdimension.ro>
Cc:     linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
        Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Angel Iglesias <ang.iglesiasg@gmail.com>,
        Matti Vaittinen <mazziesaccount@gmail.com>,
        Andreas Klinger <ak@it-klinger.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Subject: Re: [PATCH v6 2/2] iio: pressure: driver for Honeywell HSC/SSC series
Message-ID: <ZWiPDlNJCbUAtIy8@smile.fi.intel.com>
References: <20231129170425.3562-1-petre.rodan@subdimension.ro>
 <20231129170425.3562-2-petre.rodan@subdimension.ro>
 <ZWdzz7VzCW5ctend@smile.fi.intel.com>
 <ZWeNNMfqKquDYI9X@sunspire>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ZWeNNMfqKquDYI9X@sunspire>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Wed, Nov 29, 2023 at 09:12:52PM +0200, Petre Rodan wrote:
> On Wed, Nov 29, 2023 at 07:24:31PM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 29, 2023 at 07:04:12PM +0200, Petre Rodan wrote:

...

> > > v6: modifications based on Andy's review
> > >     - use str_has_prefix(), match_string() instead of strncmp()
> > 
> > And why not using the respective property API for that case where
> > match_string() is used?
> 
> I'm lost again.
> 
> 437:  ret = device_property_read_string(dev, "honeywell,pressure-triplet",
> 					&triplet);
> [..]
> 455:	ret = match_string(hsc_triplet_variants, HSC_VARIANTS_MAX,
> 						triplet);
> 		if (ret < 0)
> 			return dev_err_probe(dev, -EINVAL,
> 				"honeywell,pressure-triplet is invalid\n");
> 
> 		hsc->pmin = hsc_range_config[ret].pmin;
> 		hsc->pmax = hsc_range_config[ret].pmax;
> 
> triplet is got via device_property_read_string(), is there some other property
> function I should be using?

I think I mentioned that API, but for your convenience
device_property_match_property_string().

...

> > > +	tmp = div_s64(((s64)(hsc->pmax - hsc->pmin)) * MICRO,
> > > +		      hsc->outmax - hsc->outmin);
> > > +	hsc->p_scale = div_s64_rem(tmp, NANO, &hsc->p_scale_dec);
> > > +	tmp = div_s64(((s64)hsc->pmin * (s64)(hsc->outmax - hsc->outmin)) *
> > > +		      MICRO, hsc->pmax - hsc->pmin);
> > 
> > Why not put MICRO on the previous line?
> 
> oh well, from the review I understood you were asking for the replacement of
> NANO with MICRO on the previous instruction and it did not make much sense (
> units are in pascal and we need a kilopascal output to userland)
> 
> now I understood it's an indentation request. however moving MICRO will cross
> the 80 column rule. but if there will be yet another modification request
> I'll move it.

I understand that it breaks the 80 character rule, but my point is to have
consistency between two divisions (see quoted context) along with the logical
split — line split on argument list split.

-- 
With Best Regards,
Andy Shevchenko