linux-kernel - Re: [PATCH v2 1/2] iio: accel: fxls8962af: Fix temperature calculation

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: <20250504164654.1ecf7215@jic23-huawei>
Date: Sun, 4 May 2025 16:46:54 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Sean Nyekjaer <sean@...nix.com>, Marcelo Schmitt
 <marcelo.schmitt1@...il.com>, Lars-Peter Clausen <lars@...afoo.de>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2 1/2] iio: accel: fxls8962af: Fix temperature
 calculation

On Fri, 2 May 2025 17:19:44 +0300
Andy Shevchenko <andy.shevchenko@...il.com> wrote:

> On Fri, May 2, 2025 at 9:15 AM Sean Nyekjaer <sean@...nix.com> wrote:
> >
> > According to spec temperature should be returned in milli degrees Celsius.
> > Add in_temp_scale to calculate from Celsius to milli Celsius.  
> 
> ...
> 
> > +/* Raw temp channel scale */
> > +#define FXLS8962AF_TEMP_SCALE                  1000  
> 
> Wouldn't constants from units.h be helpful here?

Whilst you are just continuing local style, I'm not sure
these defines really bring much at all given the TEMP_SCALE
one for instance is only used in one place which is
explicitly getting the temperature scale.  It's not a magic
number that needs a define. It's a number that means it's own
value :)

Using MILLI there would be a nice bit of self documentation
though.
> 
> >  #define FXLS8962AF_AUTO_SUSPEND_DELAY_MS       2000  
> 
> (2 * MSEC_PER_SEC)
> 
> This gives immediately that we want 2 seconds of delay.
> 
True but not part of this patch so that would be a nice
little follow up.  Possibly dropping this define in favour
of using that inline.

Jonathan