diff mbox series

[v3,03/12] platform/x86: asus-wmi: switch to use <linux/units.h> helpers

Message ID 1576075099-3441-4-git-send-email-akinobu.mita@gmail.com (mailing list archive)
State Not Applicable
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 
The asus-wmi driver doesn't implement the thermal device functionality
directly, so including <linux/thermal.h> just for DECI_KELVIN_TO_CELSIUS()
is a bit odd.

This switches the asus-wmi driver to use deci_kelvin_to_millicelsius() 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>
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
* v3
- rename new header name from <linux/temperature.h> to <linux/units.h>

 drivers/platform/x86/asus-wmi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Andy Shevchenko Dec. 11, 2019, 3:31 p.m. UTC | #1 
On Wed, Dec 11, 2019 at 4:39 PM Akinobu Mita <akinobu.mita@gmail.com> wrote:
>
> The asus-wmi driver doesn't implement the thermal device functionality
> directly, so including <linux/thermal.h> just for DECI_KELVIN_TO_CELSIUS()
> is a bit odd.
>
> This switches the asus-wmi driver to use deci_kelvin_to_millicelsius() in
> <linux/units.h>.

> @@ -33,7 +33,7 @@
>  #include <linux/seq_file.h>
>  #include <linux/platform_data/x86/asus-wmi.h>
>  #include <linux/platform_device.h>
> -#include <linux/thermal.h>
> +#include <linux/units.h>
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>

Similar comment about ordering. Can we move it to the end after dmi.h?

> -       return sprintf(buf, "%d\n", value);

> +       return sprintf(buf, "%ld\n",

%d -> %ld must be explained in the commit message (e.g. "due to
function returned type).

> +                      deci_kelvin_to_millicelsius(value & 0xFFFF));

I prefer to have this in one line.
Akinobu Mita Dec. 13, 2019, 4:06 a.m. UTC | #2 
2019年12月12日(木) 0:32 Andy Shevchenko <andy.shevchenko@gmail.com>:

>
> On Wed, Dec 11, 2019 at 4:39 PM Akinobu Mita <akinobu.mita@gmail.com> wrote:
> >
> > The asus-wmi driver doesn't implement the thermal device functionality
> > directly, so including <linux/thermal.h> just for DECI_KELVIN_TO_CELSIUS()
> > is a bit odd.
> >
> > This switches the asus-wmi driver to use deci_kelvin_to_millicelsius() in
> > <linux/units.h>.
>
> > @@ -33,7 +33,7 @@
> >  #include <linux/seq_file.h>
> >  #include <linux/platform_data/x86/asus-wmi.h>
> >  #include <linux/platform_device.h>
> > -#include <linux/thermal.h>
> > +#include <linux/units.h>
> >  #include <linux/acpi.h>
> >  #include <linux/dmi.h>
>
> Similar comment about ordering. Can we move it to the end after dmi.h?

OK.

> > -       return sprintf(buf, "%d\n", value);
>
> > +       return sprintf(buf, "%ld\n",
>
> %d -> %ld must be explained in the commit message (e.g. "due to
> function returned type).

OK.

> > +                      deci_kelvin_to_millicelsius(value & 0xFFFF));
>
> I prefer to have this in one line.

It causes line over 80 characters.
We had the same conversation in v1 :)
Andy Shevchenko Dec. 13, 2019, 9:05 a.m. UTC | #3 
On Fri, Dec 13, 2019 at 6:06 AM Akinobu Mita <akinobu.mita@gmail.com> wrote:
> 2019年12月12日(木) 0:32 Andy Shevchenko <andy.shevchenko@gmail.com>:
> > On Wed, Dec 11, 2019 at 4:39 PM Akinobu Mita <akinobu.mita@gmail.com> wrote:

> > > +       return sprintf(buf, "%ld\n",
> >
> > %d -> %ld must be explained in the commit message (e.g. "due to
> > function returned type).
>
> OK.
>
> > > +                      deci_kelvin_to_millicelsius(value & 0xFFFF));
> >
> > I prefer to have this in one line.
>
> It causes line over 80 characters.
> We had the same conversation in v1 :)

And I really don't care about this.
diff mbox series

Patch

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 821b08e..ed74c0d 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -33,7 +33,7 @@ 
 #include <linux/seq_file.h>
 #include <linux/platform_data/x86/asus-wmi.h>
 #include <linux/platform_device.h>
-#include <linux/thermal.h>
+#include <linux/units.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
 
@@ -1512,9 +1512,8 @@  static ssize_t asus_hwmon_temp1(struct device *dev,
 	if (err < 0)
 		return err;
 
-	value = DECI_KELVIN_TO_CELSIUS((value & 0xFFFF)) * 1000;
-
-	return sprintf(buf, "%d\n", value);
+	return sprintf(buf, "%ld\n",
+		       deci_kelvin_to_millicelsius(value & 0xFFFF));
 }
 
 /* Fan1 */