diff mbox series

[v3,02/12] ACPI: thermal: switch to use <linux/units.h> helpers

Message ID 1576075099-3441-3-git-send-email-akinobu.mita@gmail.com (mailing list archive)
State Not Applicable
Delegated to: Johannes Berg
Headers show
Series add header file for kelvin to/from Celsius conversion helpers | expand

Commit Message

Akinobu Mita Dec. 11, 2019, 2:38 p.m. UTC 
This switches the ACPI thermal zone driver to use celsius_to_deci_kelvin(),
deci_kelvin_to_celsius(), and deci_kelvin_to_millicelsius_with_offset() in
<linux/units.h> instead of helpers in <linux/thermal.h>.

This is preparation for centralizing the kelvin to/from Celsius conversion
helpers in <linux/units.h>.

Cc: Sujith Thomas <sujith.thomas@intel.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Amit Kucheria <amit.kucheria@verdurent.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Keith Busch <kbusch@kernel.org>
Cc: Jens Axboe <axboe@fb.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
* v3
- rename new header name from <linux/temperature.h> to <linux/units.h>

 drivers/acpi/thermal.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

Comments

Andy Shevchenko Dec. 11, 2019, 3:29 p.m. UTC | #1 
On Wed, Dec 11, 2019 at 4:39 PM Akinobu Mita <akinobu.mita@gmail.com> wrote:
>
> This switches the ACPI thermal zone driver to use celsius_to_deci_kelvin(),
> deci_kelvin_to_celsius(), and deci_kelvin_to_millicelsius_with_offset() in
> <linux/units.h> instead of helpers in <linux/thermal.h>.
>
> This is preparation for centralizing the kelvin to/from Celsius conversion
> helpers in <linux/units.h>.

>  #include <linux/reboot.h>
>  #include <linux/device.h>
>  #include <linux/thermal.h>

> +#include <linux/units.h>

Can we try to keep *some* order, i.e. put this after acpi.h below?

>  #include <linux/acpi.h>
>  #include <linux/workqueue.h>
>  #include <linux/uaccess.h>

>                         } else if (crt > 0) {
> -                               unsigned long crt_k = CELSIUS_TO_DECI_KELVIN(crt);
> +                               unsigned long crt_k =
> +                                       celsius_to_deci_kelvin(crt);

It used to be one line, why do two?

>         pr_info(PREFIX "%s [%s] (%ld C)\n", acpi_device_name(device),
> -               acpi_device_bid(device), DECI_KELVIN_TO_CELSIUS(tz->temperature));
> +               acpi_device_bid(device),
> +               deci_kelvin_to_celsius(tz->temperature));

Ditto.
Akinobu Mita Dec. 13, 2019, 4:03 a.m. UTC | #2 
2019年12月12日(木) 0:29 Andy Shevchenko <andy.shevchenko@gmail.com>:
>
> On Wed, Dec 11, 2019 at 4:39 PM Akinobu Mita <akinobu.mita@gmail.com> wrote:
> >
> > This switches the ACPI thermal zone driver to use celsius_to_deci_kelvin(),
> > deci_kelvin_to_celsius(), and deci_kelvin_to_millicelsius_with_offset() in
> > <linux/units.h> instead of helpers in <linux/thermal.h>.
> >
> > This is preparation for centralizing the kelvin to/from Celsius conversion
> > helpers in <linux/units.h>.
>
> >  #include <linux/reboot.h>
> >  #include <linux/device.h>
> >  #include <linux/thermal.h>
>
> > +#include <linux/units.h>
>
> Can we try to keep *some* order, i.e. put this after acpi.h below?

OK.

> >  #include <linux/acpi.h>
> >  #include <linux/workqueue.h>
> >  #include <linux/uaccess.h>
>
> >                         } else if (crt > 0) {
> > -                               unsigned long crt_k = CELSIUS_TO_DECI_KELVIN(crt);
> > +                               unsigned long crt_k =
> > +                                       celsius_to_deci_kelvin(crt);
>
> It used to be one line, why do two?

Because this line over 80 characters.  This patch doesn't make this line
longer, but checkpatch.pl complains about it.

> >         pr_info(PREFIX "%s [%s] (%ld C)\n", acpi_device_name(device),
> > -               acpi_device_bid(device), DECI_KELVIN_TO_CELSIUS(tz->temperature));
> > +               acpi_device_bid(device),
> > +               deci_kelvin_to_celsius(tz->temperature));
>
> Ditto.

Same as above, checkpatch.pl complains about line over 80 characters.
Andy Shevchenko Dec. 13, 2019, 9:08 a.m. UTC | #3 
On Fri, Dec 13, 2019 at 6:03 AM Akinobu Mita <akinobu.mita@gmail.com> wrote:
> 2019年12月12日(木) 0:29 Andy Shevchenko <andy.shevchenko@gmail.com>:
> > On Wed, Dec 11, 2019 at 4:39 PM Akinobu Mita <akinobu.mita@gmail.com> wrote:

> > >                         } else if (crt > 0) {
> > > -                               unsigned long crt_k = CELSIUS_TO_DECI_KELVIN(crt);
> > > +                               unsigned long crt_k =
> > > +                                       celsius_to_deci_kelvin(crt);
> >
> > It used to be one line, why do two?
>
> Because this line over 80 characters.  This patch doesn't make this line
> longer, but checkpatch.pl complains about it.

I'm not a maintainer here, but as it was before why not to leave as is
(in terms of line length)?
Supposedly maintainers are okay with it.

> > >         pr_info(PREFIX "%s [%s] (%ld C)\n", acpi_device_name(device),
> > > -               acpi_device_bid(device), DECI_KELVIN_TO_CELSIUS(tz->temperature));
> > > +               acpi_device_bid(device),
> > > +               deci_kelvin_to_celsius(tz->temperature));
> >
> > Ditto.
>
> Same as above, checkpatch.pl complains about line over 80 characters.

Same as above.

P.S. checkpatch is recommendation, and not something one must follow.
diff mbox series

Patch

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index d831a61..8a436f5 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -24,6 +24,7 @@ 
 #include <linux/reboot.h>
 #include <linux/device.h>
 #include <linux/thermal.h>
+#include <linux/units.h>
 #include <linux/acpi.h>
 #include <linux/workqueue.h>
 #include <linux/uaccess.h>
@@ -172,7 +173,7 @@  struct acpi_thermal {
 	struct acpi_handle_list devices;
 	struct thermal_zone_device *thermal_zone;
 	int tz_enabled;
-	int kelvin_offset;
+	int kelvin_offset;	/* in millidegrees */
 	struct work_struct thermal_check_work;
 };
 
@@ -297,7 +298,9 @@  static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 			if (crt == -1) {
 				tz->trips.critical.flags.valid = 0;
 			} else if (crt > 0) {
-				unsigned long crt_k = CELSIUS_TO_DECI_KELVIN(crt);
+				unsigned long crt_k =
+					celsius_to_deci_kelvin(crt);
+
 				/*
 				 * Allow override critical threshold
 				 */
@@ -333,7 +336,7 @@  static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 		if (psv == -1) {
 			status = AE_SUPPORT;
 		} else if (psv > 0) {
-			tmp = CELSIUS_TO_DECI_KELVIN(psv);
+			tmp = celsius_to_deci_kelvin(psv);
 			status = AE_OK;
 		} else {
 			status = acpi_evaluate_integer(tz->device->handle,
@@ -413,7 +416,7 @@  static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 					break;
 				if (i == 1)
 					tz->trips.active[0].temperature =
-						CELSIUS_TO_DECI_KELVIN(act);
+						celsius_to_deci_kelvin(act);
 				else
 					/*
 					 * Don't allow override higher than
@@ -421,9 +424,9 @@  static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 					 */
 					tz->trips.active[i - 1].temperature =
 						(tz->trips.active[i - 2].temperature <
-						CELSIUS_TO_DECI_KELVIN(act) ?
+						celsius_to_deci_kelvin(act) ?
 						tz->trips.active[i - 2].temperature :
-						CELSIUS_TO_DECI_KELVIN(act));
+						celsius_to_deci_kelvin(act));
 				break;
 			} else {
 				tz->trips.active[i].temperature = tmp;
@@ -519,7 +522,7 @@  static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
 	if (result)
 		return result;
 
-	*temp = DECI_KELVIN_TO_MILLICELSIUS_WITH_OFFSET(tz->temperature,
+	*temp = deci_kelvin_to_millicelsius_with_offset(tz->temperature,
 							tz->kelvin_offset);
 	return 0;
 }
@@ -624,7 +627,7 @@  static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 
 	if (tz->trips.critical.flags.valid) {
 		if (!trip) {
-			*temp = DECI_KELVIN_TO_MILLICELSIUS_WITH_OFFSET(
+			*temp = deci_kelvin_to_millicelsius_with_offset(
 				tz->trips.critical.temperature,
 				tz->kelvin_offset);
 			return 0;
@@ -634,7 +637,7 @@  static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 
 	if (tz->trips.hot.flags.valid) {
 		if (!trip) {
-			*temp = DECI_KELVIN_TO_MILLICELSIUS_WITH_OFFSET(
+			*temp = deci_kelvin_to_millicelsius_with_offset(
 				tz->trips.hot.temperature,
 				tz->kelvin_offset);
 			return 0;
@@ -644,7 +647,7 @@  static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 
 	if (tz->trips.passive.flags.valid) {
 		if (!trip) {
-			*temp = DECI_KELVIN_TO_MILLICELSIUS_WITH_OFFSET(
+			*temp = deci_kelvin_to_millicelsius_with_offset(
 				tz->trips.passive.temperature,
 				tz->kelvin_offset);
 			return 0;
@@ -655,7 +658,7 @@  static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
 		tz->trips.active[i].flags.valid; i++) {
 		if (!trip) {
-			*temp = DECI_KELVIN_TO_MILLICELSIUS_WITH_OFFSET(
+			*temp = deci_kelvin_to_millicelsius_with_offset(
 				tz->trips.active[i].temperature,
 				tz->kelvin_offset);
 			return 0;
@@ -672,7 +675,7 @@  static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
 	struct acpi_thermal *tz = thermal->devdata;
 
 	if (tz->trips.critical.flags.valid) {
-		*temperature = DECI_KELVIN_TO_MILLICELSIUS_WITH_OFFSET(
+		*temperature = deci_kelvin_to_millicelsius_with_offset(
 				tz->trips.critical.temperature,
 				tz->kelvin_offset);
 		return 0;
@@ -692,7 +695,7 @@  static int thermal_get_trend(struct thermal_zone_device *thermal,
 
 	if (type == THERMAL_TRIP_ACTIVE) {
 		int trip_temp;
-		int temp = DECI_KELVIN_TO_MILLICELSIUS_WITH_OFFSET(
+		int temp = deci_kelvin_to_millicelsius_with_offset(
 					tz->temperature, tz->kelvin_offset);
 		if (thermal_get_trip_temp(thermal, trip, &trip_temp))
 			return -EINVAL;
@@ -1043,9 +1046,9 @@  static void acpi_thermal_guess_offset(struct acpi_thermal *tz)
 {
 	if (tz->trips.critical.flags.valid &&
 	    (tz->trips.critical.temperature % 5) == 1)
-		tz->kelvin_offset = 2731;
+		tz->kelvin_offset = 273100;
 	else
-		tz->kelvin_offset = 2732;
+		tz->kelvin_offset = 273200;
 }
 
 static void acpi_thermal_check_fn(struct work_struct *work)
@@ -1087,7 +1090,8 @@  static int acpi_thermal_add(struct acpi_device *device)
 	INIT_WORK(&tz->thermal_check_work, acpi_thermal_check_fn);
 
 	pr_info(PREFIX "%s [%s] (%ld C)\n", acpi_device_name(device),
-		acpi_device_bid(device), DECI_KELVIN_TO_CELSIUS(tz->temperature));
+		acpi_device_bid(device),
+		deci_kelvin_to_celsius(tz->temperature));
 	goto end;
 
 free_memory: