2019年12月7日(土) 1:49 Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>: > > On 28/11/2019 15:54, Akinobu Mita wrote: > > There are several helper macros to convert kelvin to/from Celsius in > > <linux/thermal.h> for thermal drivers. These are useful for any other > > drivers or subsystems, but it's odd to include <linux/thermal.h> just for > > the helpers. > > > > This adds a new <linux/temperature.h> that provides the equivalent inline > > functions for any drivers or subsystems. It is intended to replace the > > helpers in <linux/thermal.h>. > > > > Cc: Sujith Thomas <sujith.thomas@xxxxxxxxx> > > Cc: Darren Hart <dvhart@xxxxxxxxxxxxx> > > Cc: Andy Shevchenko <andy@xxxxxxxxxxxxx> > > Cc: Zhang Rui <rui.zhang@xxxxxxxxx> > > Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> > > Cc: Amit Kucheria <amit.kucheria@xxxxxxxxxxxxx> > > Cc: Jean Delvare <jdelvare@xxxxxxxx> > > Cc: Guenter Roeck <linux@xxxxxxxxxxxx> > > Cc: Keith Busch <kbusch@xxxxxxxxxx> > > Cc: Jens Axboe <axboe@xxxxxx> > > Cc: Christoph Hellwig <hch@xxxxxx> > > Cc: Sagi Grimberg <sagi@xxxxxxxxxxx> > > Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx> > > --- > > * v2 > > - add deci_kelvin_to_millicelsius_with_offset() in linux/temperature.h > > - stop including linux/temperature.h from linux/thermal.h > > > > include/linux/temperature.h | 51 +++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > create mode 100644 include/linux/temperature.h > > > > diff --git a/include/linux/temperature.h b/include/linux/temperature.h > > new file mode 100644 > > index 0000000..679e70a > > --- /dev/null > > +++ b/include/linux/temperature.h > > @@ -0,0 +1,51 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _LINUX_TEMPERATURE_H > > +#define _LINUX_TEMPERATURE_H > > It sounds strange to create a temperature file just for a few conversion > functions. Why not create an units.h file, so some more conversions > could be added later (not necessarily related to temperature) ? Fine with me. > > +#include <linux/kernel.h> > > + > > +#define ABSOLUTE_ZERO_MILLICELSIUS -273150 > > + > > +static inline long kelvin_to_millicelsius(long t) > > Why 'long' ? There's no specific reason, but the existing DECI_KELVIN_TO_CELSIUS() in linux/thermal.h converts to 'long'. Do you prefer changing 'long' to 'int'? > > +{ > > + return t * 1000 + ABSOLUTE_ZERO_MILLICELSIUS; > > +} > > + > > +static inline long millicelsius_to_kelvin(long t) > > +{ > > + return DIV_ROUND_CLOSEST(t - ABSOLUTE_ZERO_MILLICELSIUS, 1000); > > Please don't duplicate these operations, just do the conversion to the > right unit and then call a single function. > > Replace the constant by macros like what we find in time64.h. > > eg. > #define MILLICELSIUS_PER_CELCIUS 1000 How about this? #define ABSOLUTE_ZERO_MILLICELSIUS -273150 static inline long milli_kelvin_to_millicelsius(long t) { return t + ABSOLUTE_ZERO_MILLICELSIUS; } static inline long millicelsius_to_milli_kelvin(long t) { return t - ABSOLUTE_ZERO_MILLICELSIUS; } #define MILLIDEGREE_PER_DEGREE 1000 #define MILLIDEGREE_PER_DECIDEGREE 100 static inline long kelvin_to_millicelsius(long t) { return milli_kelvin_to_millicelsius(t * MILLIDEGREE_PER_DEGREE); } static inline long millicelsius_to_kelvin(long t) { t = millicelsius_to_milli_kelvin(t); return DIV_ROUND_CLOSEST(t, MILLIDEGREE_PER_DEGREE); } static inline long deci_kelvin_to_celsius(long t) { t = milli_kelvin_to_millicelsius(t * MILLIDEGREE_PER_DECIDEGREE); return DIV_ROUND_CLOSEST(t, MILLIDEGREE_PER_DEGREE); } static inline long celsius_to_deci_kelvin(long t) { t = millicelsius_to_milli_kelvin(t * MILLIDEGREE_PER_DEGREE); return DIV_ROUND_CLOSEST(t, MILLIDEGREE_PER_DECIDEGREE); } /** * deci_kelvin_to_millicelsius_with_offset - convert Kelvin to Celsius * @t: temperature value in decidegrees Kelvin * @offset: difference between Kelvin and Celsius in millidegrees * * Return: temperature value in millidegrees Celsius */ static inline long deci_kelvin_to_millicelsius_with_offset(long t, long offset) { return t * MILLIDEGREE_PER_DECIDEGREE - offset; } static inline long deci_kelvin_to_millicelsius(long t) { return milli_kelvin_to_millicelsius(t * MILLIDEGREE_PER_DECIDEGREE); } static inline long millicelsius_to_deci_kelvin(long t) { t = millicelsius_to_milli_kelvin(t); return DIV_ROUND_CLOSEST(t, MILLIDEGREE_PER_DECIDEGREE); } > So you can really do the cleanup in all the drivers, like removing: OK. I'll try as much as possible.