The macro for the unit conversion for frequency is duplicated in
different places.
Provide this macro in the 'units' header, so it can be reused.
Signed-off-by: Daniel Lezcano <[email protected]>
---
include/linux/units.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/linux/units.h b/include/linux/units.h
index dcc30a53fa93..218ec0d314b6 100644
--- a/include/linux/units.h
+++ b/include/linux/units.h
@@ -4,6 +4,10 @@
#include <linux/math.h>
+#define HZ_PER_KHZ 1000L
+#define KHZ_PER_MHZ 1000L
+#define HZ_PER_MHZ 1000000L
+
#define MILLIWATT_PER_WATT 1000L
#define MICROWATT_PER_MILLIWATT 1000L
#define MICROWATT_PER_WATT 1000000L
--
2.17.1
The HZ_PER_KHZ macro definition is duplicated in different subsystems.
The macro now exists in include/linux/units.h, make use of it and
remove all the duplicated ones.
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/devfreq/devfreq.c | 2 +-
drivers/iio/light/as73211.c | 3 +--
drivers/thermal/devfreq_cooling.c | 2 +-
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 6aa10de792b3..4c636c336ace 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -26,6 +26,7 @@
#include <linux/hrtimer.h>
#include <linux/of.h>
#include <linux/pm_qos.h>
+#include <linux/units.h>
#include "governor.h"
#define CREATE_TRACE_POINTS
@@ -33,7 +34,6 @@
#define IS_SUPPORTED_FLAG(f, name) ((f & DEVFREQ_GOV_FLAG_##name) ? true : false)
#define IS_SUPPORTED_ATTR(f, name) ((f & DEVFREQ_GOV_ATTR_##name) ? true : false)
-#define HZ_PER_KHZ 1000
static struct class *devfreq_class;
static struct dentry *devfreq_debugfs;
diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
index 7b32dfaee9b3..3ba2378df3dd 100644
--- a/drivers/iio/light/as73211.c
+++ b/drivers/iio/light/as73211.c
@@ -24,8 +24,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/pm.h>
-
-#define HZ_PER_KHZ 1000
+#include <linux/units.h>
#define AS73211_DRV_NAME "as73211"
diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
index fed3121ff2a1..fa5b8b0c7604 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -19,10 +19,10 @@
#include <linux/pm_opp.h>
#include <linux/pm_qos.h>
#include <linux/thermal.h>
+#include <linux/units.h>
#include <trace/events/thermal.h>
-#define HZ_PER_KHZ 1000
#define SCALE_ERROR_MITIGATION 100
static DEFINE_IDA(devfreq_ida);
--
2.17.1
On 2/24/21 5:30 AM, Daniel Lezcano wrote:
> The HZ_PER_KHZ macro definition is duplicated in different subsystems.
>
> The macro now exists in include/linux/units.h, make use of it and
> remove all the duplicated ones.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
> drivers/devfreq/devfreq.c | 2 +-
> drivers/iio/light/as73211.c | 3 +--
> drivers/thermal/devfreq_cooling.c | 2 +-
> 3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 6aa10de792b3..4c636c336ace 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -26,6 +26,7 @@
> #include <linux/hrtimer.h>
> #include <linux/of.h>
> #include <linux/pm_qos.h>
> +#include <linux/units.h>
> #include "governor.h"
>
> #define CREATE_TRACE_POINTS
> @@ -33,7 +34,6 @@
>
> #define IS_SUPPORTED_FLAG(f, name) ((f & DEVFREQ_GOV_FLAG_##name) ? true : false)
> #define IS_SUPPORTED_ATTR(f, name) ((f & DEVFREQ_GOV_ATTR_##name) ? true : false)
> -#define HZ_PER_KHZ 1000
>
> static struct class *devfreq_class;
> static struct dentry *devfreq_debugfs;
> diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
> index 7b32dfaee9b3..3ba2378df3dd 100644
> --- a/drivers/iio/light/as73211.c
> +++ b/drivers/iio/light/as73211.c
> @@ -24,8 +24,7 @@
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/pm.h>
> -
> -#define HZ_PER_KHZ 1000
> +#include <linux/units.h>
>
> #define AS73211_DRV_NAME "as73211"
>
> diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
> index fed3121ff2a1..fa5b8b0c7604 100644
> --- a/drivers/thermal/devfreq_cooling.c
> +++ b/drivers/thermal/devfreq_cooling.c
> @@ -19,10 +19,10 @@
> #include <linux/pm_opp.h>
> #include <linux/pm_qos.h>
> #include <linux/thermal.h>
> +#include <linux/units.h>
>
> #include <trace/events/thermal.h>
>
> -#define HZ_PER_KHZ 1000
> #define SCALE_ERROR_MITIGATION 100
>
> static DEFINE_IDA(devfreq_ida);
>
For devfreq part,
Acked-by: Chanwoo Choi <[email protected]>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
On Wed, 24 Feb 2021 10:39:36 +0200 Andy Shevchenko <[email protected]> wrote:
> On Wednesday, February 24, 2021, Andy Shevchenko <[email protected]>
> wrote:
>
> >
> >
> > On Tuesday, February 23, 2021, Daniel Lezcano <[email protected]>
> > wrote:
> >
> >> The macro for the unit conversion for frequency is duplicated in
> >> different places.
> >>
> >> Provide this macro in the 'units' header, so it can be reused.
> >>
> >>
> >
> > Thanks! That was the idea behind my reviews to add those definitions
> > explicitly in the users. I just want to be sure you covered them all. Also
> > there are few non-standard names for above in some drivers (they can be
> > fixed on per driver basis in separate patches though).
> >
> >
>
> Seems you introduced a common macro and forget about dropping it elsewhere.
>
> https://elixir.bootlin.com/linux/latest/A/ident/HZ_PER_MHZ
Yes. And HZ_PER_KHZ.
Also, why make them signed types? Negative Hz is physically
nonsensical. If that upsets some code somewhere because it was dealing
with signed types then, well, that code needed fixing anyway.
Ditto MILLIWATT_PER_WATT and friends, sigh.
Hi Andrew,
On 04/03/2021 01:31, Andrew Morton wrote:
> On Wed, 24 Feb 2021 10:39:36 +0200 Andy Shevchenko <[email protected]> wrote:
>
>> On Wednesday, February 24, 2021, Andy Shevchenko <[email protected]>
>> wrote:
>>
>>>
>>>
>>> On Tuesday, February 23, 2021, Daniel Lezcano <[email protected]>
>>> wrote:
>>>
>>>> The macro for the unit conversion for frequency is duplicated in
>>>> different places.
>>>>
>>>> Provide this macro in the 'units' header, so it can be reused.
>>>>
>>>>
>>>
>>> Thanks! That was the idea behind my reviews to add those definitions
>>> explicitly in the users. I just want to be sure you covered them all. Also
>>> there are few non-standard names for above in some drivers (they can be
>>> fixed on per driver basis in separate patches though).
>>>
>>>
>>
>> Seems you introduced a common macro and forget about dropping it elsewhere.
>>
>> https://elixir.bootlin.com/linux/latest/A/ident/HZ_PER_MHZ
>
> Yes. And HZ_PER_KHZ.
Thanks for the review, it is fixed it in the v2.
> Also, why make them signed types? Negative Hz is physically
> nonsensical. If that upsets some code somewhere because it was dealing
> with signed types then, well, that code needed fixing anyway.
>
> Ditto MILLIWATT_PER_WATT and friends, sigh.
At the first glance converting to unsigned long should not hurt the
users of this macro.
The current series introduces the macro and its usage but by converting
the existing type.
Is it ok if I send a separate series to change the units from L to UL?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Thu, Mar 4, 2021 at 2:31 AM Andrew Morton <[email protected]> wrote:
> On Wed, 24 Feb 2021 10:39:36 +0200 Andy Shevchenko <[email protected]> wrote:
...
> Also, why make them signed types? Negative Hz is physically
> nonsensical. If that upsets some code somewhere because it was dealing
> with signed types then, well, that code needed fixing anyway.
>
> Ditto MILLIWATT_PER_WATT and friends, sigh.
And all seconds also. I guess it's historically like this and in any
case I don't expect we will have multipliers that don't fit long long.
There might be some subtle integral promotion rules that screw the
flow up, but I have not much experience with it to be honest.
--
With Best Regards,
Andy Shevchenko
On Thu, 4 Mar 2021 13:41:27 +0100 Daniel Lezcano <[email protected]> wrote:
> > Also, why make them signed types? Negative Hz is physically
> > nonsensical. If that upsets some code somewhere because it was dealing
> > with signed types then, well, that code needed fixing anyway.
> >
> > Ditto MILLIWATT_PER_WATT and friends, sigh.
>
> At the first glance converting to unsigned long should not hurt the
> users of this macro.
>
> The current series introduces the macro and its usage but by converting
> the existing type.
>
> Is it ok if I send a separate series to change the units from L to UL?
That's the way to do it...