2023-06-27 18:33:35

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v11 6/9] soc: qcom: cpr: Use u64 for frequency

32 bits is not enough for over-2.changeGHz frequencies. Move all variables
that operate on Hz to u64 to avoid overflows.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/soc/qcom/cpr-common.c | 13 +++++++------
drivers/soc/qcom/cpr-common.h | 9 ++++-----
drivers/soc/qcom/cpr.c | 6 +++---
3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/soc/qcom/cpr-common.c b/drivers/soc/qcom/cpr-common.c
index 44c681bbbf13..3e3a4a61cfde 100644
--- a/drivers/soc/qcom/cpr-common.c
+++ b/drivers/soc/qcom/cpr-common.c
@@ -218,7 +218,7 @@ unsigned int cpr_get_fuse_corner(struct dev_pm_opp *opp)
}
EXPORT_SYMBOL_GPL(cpr_get_fuse_corner);

-unsigned long cpr_get_opp_hz_for_req(struct dev_pm_opp *ref,
+u64 cpr_get_opp_hz_for_req(struct dev_pm_opp *ref,
struct device *cpu_dev)
{
u64 rate = 0;
@@ -250,7 +250,7 @@ unsigned long cpr_get_opp_hz_for_req(struct dev_pm_opp *ref,
out_ref:
of_node_put(desc_np);

- return (unsigned long) rate;
+ return rate;
}
EXPORT_SYMBOL_GPL(cpr_get_opp_hz_for_req);

@@ -260,7 +260,7 @@ int cpr_calculate_scaling(struct device *dev,
const struct corner *corner)
{
u32 quot_diff = 0;
- unsigned long freq_diff;
+ u64 freq_diff;
int scaling;
const struct fuse_corner *fuse, *prev_fuse;
int ret;
@@ -280,8 +280,9 @@ int cpr_calculate_scaling(struct device *dev,
}

freq_diff = fuse->max_freq - prev_fuse->max_freq;
- freq_diff /= 1000000; /* Convert to MHz */
- scaling = 1000 * quot_diff / freq_diff;
+ freq_diff = div_u64(freq_diff, 1000000); /* Convert to MHz */
+ scaling = 1000 * quot_diff;
+ do_div(scaling, freq_diff);
return min(scaling, fdata->max_quot_scale);
}
EXPORT_SYMBOL_GPL(cpr_calculate_scaling);
@@ -289,7 +290,7 @@ EXPORT_SYMBOL_GPL(cpr_calculate_scaling);
int cpr_interpolate(const struct corner *corner, int step_volt,
const struct fuse_corner_data *fdata)
{
- unsigned long f_high, f_low, f_diff;
+ u64 f_high, f_low, f_diff;
int uV_high, uV_low, uV;
u64 temp, temp_limit;
const struct fuse_corner *fuse, *prev_fuse;
diff --git a/drivers/soc/qcom/cpr-common.h b/drivers/soc/qcom/cpr-common.h
index 0aa227617d2f..1b2fa344eb09 100644
--- a/drivers/soc/qcom/cpr-common.h
+++ b/drivers/soc/qcom/cpr-common.h
@@ -42,7 +42,7 @@ struct fuse_corner {
int step_quot;
const struct reg_sequence *accs;
int num_accs;
- unsigned long max_freq;
+ u64 max_freq;
u8 ring_osc_idx;
};

@@ -54,13 +54,13 @@ struct corner {
int quot_adjust;
u32 save_ctl;
u32 save_irq;
- unsigned long freq;
+ u64 freq;
struct fuse_corner *fuse_corner;
};

struct corner_data {
unsigned int fuse_corner;
- unsigned long freq;
+ u64 freq;
};

struct acc_desc {
@@ -92,8 +92,7 @@ int cpr_populate_fuse_common(struct device *dev,
int cpr_find_initial_corner(struct device *dev, struct clk *cpu_clk,
struct corner *corners, int num_corners);
u32 cpr_get_fuse_corner(struct dev_pm_opp *opp);
-unsigned long cpr_get_opp_hz_for_req(struct dev_pm_opp *ref,
- struct device *cpu_dev);
+u64 cpr_get_opp_hz_for_req(struct dev_pm_opp *ref, struct device *cpu_dev);
int cpr_calculate_scaling(struct device *dev,
const char *quot_offset,
const struct fuse_corner_data *fdata,
diff --git a/drivers/soc/qcom/cpr.c b/drivers/soc/qcom/cpr.c
index cc1e4c462d1f..33066b8b2811 100644
--- a/drivers/soc/qcom/cpr.c
+++ b/drivers/soc/qcom/cpr.c
@@ -827,8 +827,8 @@ static int cpr_corner_init(struct cpr_drv *drv)
struct corner_data *cdata;
const struct fuse_corner_data *fdata;
bool apply_scaling;
- unsigned long freq_diff, freq_diff_mhz;
- unsigned long freq;
+ unsigned long freq_diff_mhz;
+ u64 freq, freq_diff;
int step_volt = regulator_get_linear_step(drv->vdd_apc);
struct dev_pm_opp *opp;

@@ -941,7 +941,7 @@ static int cpr_corner_init(struct cpr_drv *drv)

if (apply_scaling) {
freq_diff = fuse->max_freq - corner->freq;
- freq_diff_mhz = freq_diff / 1000000;
+ freq_diff_mhz = (u32)div_u64(freq_diff, 1000000);
corner->quot_adjust = scaling * freq_diff_mhz / 1000;

corner->uV = cpr_interpolate(corner, step_volt, fdata);

--
2.41.0



2023-06-27 20:53:01

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v11 6/9] soc: qcom: cpr: Use u64 for frequency

On 6/27/2023 12:30 PM, Konrad Dybcio wrote:
> 32 bits is not enough for over-2.changeGHz frequencies. Move all variables
> that operate on Hz to u64 to avoid overflows.
>
> Signed-off-by: Konrad Dybcio <[email protected]>


I get the following warning when building this -

CC drivers/soc/qcom/cpr-common.o
In file included from ./include/linux/device.h:15:0,
from ./include/linux/platform_device.h:13,
from ./include/linux/of_device.h:5,
from drivers/soc/qcom/cpr.c:18:
drivers/soc/qcom/cpr.c: In function ‘cpr_corner_init’:
drivers/soc/qcom/cpr.c:870:21: warning: format ‘%lu’ expects argument of
type ‘long unsigned int’, but argument 4 has type ‘u64 {aka long long
unsigned int}’ [-Wformat=]
dev_dbg(drv->dev, "freq: %lu level: %u fuse level: %u\n",
^
./include/linux/dev_printk.h:129:27: note: in definition of macro
‘dev_printk’
_dev_printk(level, dev, fmt, ##__VA_ARGS__); \
^~~
./include/linux/dev_printk.h:163:31: note: in expansion of macro ‘dev_fmt’
dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
^~~~~~~
drivers/soc/qcom/cpr.c:870:3: note: in expansion of macro ‘dev_dbg’
dev_dbg(drv->dev, "freq: %lu level: %u fuse level: %u\n",
^~~~~~~

2023-06-27 21:07:48

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH v11 6/9] soc: qcom: cpr: Use u64 for frequency

On 27.06.2023 22:15, Jeffrey Hugo wrote:
> On 6/27/2023 12:30 PM, Konrad Dybcio wrote:
>> 32 bits is not enough for over-2.changeGHz frequencies. Move all variables
>> that operate on Hz to u64 to avoid overflows.
>>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>
>
> I get the following warning when building this -
Ugh. Either didn't notice or clang wasn't unhappy with it.

Thanks for noticing.

Konrad
>
>   CC      drivers/soc/qcom/cpr-common.o
> In file included from ./include/linux/device.h:15:0,
>                  from ./include/linux/platform_device.h:13,
>                  from ./include/linux/of_device.h:5,
>                  from drivers/soc/qcom/cpr.c:18:
> drivers/soc/qcom/cpr.c: In function ‘cpr_corner_init’:
> drivers/soc/qcom/cpr.c:870:21: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘u64 {aka long long unsigned int}’ [-Wformat=]
>    dev_dbg(drv->dev, "freq: %lu level: %u fuse level: %u\n",
>                      ^
> ./include/linux/dev_printk.h:129:27: note: in definition of macro ‘dev_printk’
>    _dev_printk(level, dev, fmt, ##__VA_ARGS__);  \
>                            ^~~
> ./include/linux/dev_printk.h:163:31: note: in expansion of macro ‘dev_fmt’
>    dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
>                                ^~~~~~~
> drivers/soc/qcom/cpr.c:870:3: note: in expansion of macro ‘dev_dbg’
>    dev_dbg(drv->dev, "freq: %lu level: %u fuse level: %u\n",
>    ^~~~~~~