Discussion:
[PATCH v9 03/11] ir-rx51: Use 64-bit division macro
(too old to reply)
Guru Das Srinagesh
2020-03-17 20:05:18 UTC
Permalink
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using DIV_ROUND_CLOSEST_ULL to
handle a 64-bit dividend.

Cc: Mauro Carvalho Chehab <***@kernel.org>
Cc: Richard Fontana <***@redhat.com>
Cc: Thomas Gleixner <***@linutronix.de>
Cc: Kate Stewart <***@linuxfoundation.org>
Cc: Allison Randal <***@lohutok.net>
Cc: linux-***@vger.kernel.org

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
Acked-by: Sean Young <***@mess.org>
---
drivers/media/rc/ir-rx51.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 8574eda..9a5dfd7 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -241,7 +241,8 @@ static int ir_rx51_probe(struct platform_device *dev)
}

/* Use default, in case userspace does not set the carrier */
- ir_rx51.freq = DIV_ROUND_CLOSEST(pwm_get_period(pwm), NSEC_PER_SEC);
+ ir_rx51.freq = DIV_ROUND_CLOSEST_ULL(pwm_get_period(pwm),
+ NSEC_PER_SEC);
pwm_put(pwm);

hrtimer_init(&ir_rx51.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Guru Das Srinagesh
2020-03-17 20:05:19 UTC
Permalink
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by using DIV64_U64_ROUND_CLOSEST to
handle a 64-bit divisor.

Cc: Alexander Shiyan <***@mail.ru>
Cc: Arnd Bergmann <***@arndb.de>

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
---
drivers/pwm/pwm-clps711x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
index 924d39a..ba9500a 100644
--- a/drivers/pwm/pwm-clps711x.c
+++ b/drivers/pwm/pwm-clps711x.c
@@ -43,7 +43,7 @@ static void clps711x_pwm_update_val(struct clps711x_chip *priv, u32 n, u32 v)
static unsigned int clps711x_get_duty(struct pwm_device *pwm, unsigned int v)
{
/* Duty cycle 0..15 max */
- return DIV_ROUND_CLOSEST(v * 0xf, pwm->args.period);
+ return DIV64_U64_ROUND_CLOSEST(v * 0xf, pwm->args.period);
}

static int clps711x_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Arnd Bergmann
2020-03-17 22:22:06 UTC
Permalink
Post by Guru Das Srinagesh
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by using DIV64_U64_ROUND_CLOSEST to
handle a 64-bit divisor.
---
drivers/pwm/pwm-clps711x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
index 924d39a..ba9500a 100644
--- a/drivers/pwm/pwm-clps711x.c
+++ b/drivers/pwm/pwm-clps711x.c
@@ -43,7 +43,7 @@ static void clps711x_pwm_update_val(struct clps711x_chip *priv, u32 n, u32 v)
static unsigned int clps711x_get_duty(struct pwm_device *pwm, unsigned int v)
{
/* Duty cycle 0..15 max */
- return DIV_ROUND_CLOSEST(v * 0xf, pwm->args.period);
+ return DIV64_U64_ROUND_CLOSEST(v * 0xf, pwm->args.period);
}
Is it actually going to exceed U32_MAX? If not, a type cast may be
more appropriate here than the expensive 64-bit division.

Arnd
Guru Das Srinagesh
2020-03-17 23:30:05 UTC
Permalink
Post by Arnd Bergmann
Post by Guru Das Srinagesh
diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
index 924d39a..ba9500a 100644
--- a/drivers/pwm/pwm-clps711x.c
+++ b/drivers/pwm/pwm-clps711x.c
@@ -43,7 +43,7 @@ static void clps711x_pwm_update_val(struct clps711x_chip *priv, u32 n, u32 v)
static unsigned int clps711x_get_duty(struct pwm_device *pwm, unsigned int v)
{
/* Duty cycle 0..15 max */
- return DIV_ROUND_CLOSEST(v * 0xf, pwm->args.period);
+ return DIV64_U64_ROUND_CLOSEST(v * 0xf, pwm->args.period);
}
Is it actually going to exceed U32_MAX? If not, a type cast may be
more appropriate here than the expensive 64-bit division.
With the final change in this patch series, the framework will support
periods that exceed U32_MAX. My concern is that using a typecast would
mean that in those cases, this driver will not support > U32_MAX values.
Using DIV64_U64_ROUND_CLOSEST makes the driver future proof and able to
handle > U32_MAX values correctly. What do you think?

Thank you.

Guru Das.

Guru Das Srinagesh
2020-03-17 20:05:24 UTC
Permalink
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using div_u64 to handle a 64-bit
dividend instead of a straight division operation.

Cc: Maxime Ripard <***@kernel.org>
Cc: Chen-Yu Tsai <***@csie.org>
Cc: Philipp Zabel <***@pengutronix.de>

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
Acked-by: Chen-Yu Tsai <***@csie.org>
---
drivers/pwm/pwm-sun4i.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 3e3efa6..772fdf4 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -286,7 +286,7 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
val = (duty & PWM_DTY_MASK) | PWM_PRD(period);
sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm));
sun4i_pwm->next_period[pwm->hwpwm] = jiffies +
- usecs_to_jiffies(cstate.period / 1000 + 1);
+ usecs_to_jiffies(div_u64(cstate.period, 1000) + 1);
sun4i_pwm->needs_delay[pwm->hwpwm] = true;

if (state->polarity != PWM_POLARITY_NORMAL)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Guru Das Srinagesh
2020-03-17 20:05:21 UTC
Permalink
Since the PWM framework is switching struct pwm_state.period's
datatype to u64, prepare for this transition by using
DIV_ROUND_UP_ULL to handle a 64-bit dividend, and div64_u64 to handle a
64-bit divisor.

Cc: Shawn Guo <***@kernel.org>
Cc: Sascha Hauer <***@pengutronix.de>
Cc: Pengutronix Kernel Team <***@pengutronix.de>
Cc: Fabio Estevam <***@gmail.com>
Cc: NXP Linux Team <linux-***@nxp.com>
Cc: Anson Huang <***@nxp.com>
Cc: Michal Vokáč <***@ysoft.com>
Cc: Adam Ford <***@gmail.com>
Cc: Mukesh Ojha <***@codeaurora.org>
Cc: Dan Carpenter <***@oracle.com>

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
---
drivers/pwm/pwm-imx27.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index 35a7ac42..b7d38d0 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -208,7 +208,7 @@ static void pwm_imx27_wait_fifo_slot(struct pwm_chip *chip,
sr = readl(imx->mmio_base + MX3_PWMSR);
fifoav = FIELD_GET(MX3_PWMSR_FIFOAV, sr);
if (fifoav == MX3_PWMSR_FIFOAV_4WORDS) {
- period_ms = DIV_ROUND_UP(pwm_get_period(pwm),
+ period_ms = DIV_ROUND_UP_ULL(pwm_get_period(pwm),
NSEC_PER_MSEC);
msleep(period_ms);

@@ -240,8 +240,7 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,

period_cycles /= prescale;
c = (unsigned long long)period_cycles * state->duty_cycle;
- do_div(c, state->period);
- duty_cycles = c;
+ duty_cycles = div64_u64(c, state->period);

/*
* according to imx pwm RM, the real period value should be PERIOD
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Guru Das Srinagesh
2020-03-17 20:05:20 UTC
Permalink
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using DIV64_U64_ROUND_CLOSEST to
handle a 64-bit divisor.

Cc: Shawn Guo <***@kernel.org>
Cc: Sascha Hauer <***@pengutronix.de>
Cc: Pengutronix Kernel Team <***@pengutronix.de>
Cc: Fabio Estevam <***@gmail.com>
Cc: NXP Linux Team <linux-***@nxp.com>
Cc: Anson Huang <***@nxp.com>

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
---
drivers/pwm/pwm-imx-tpm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
index 9145f61..53bf364 100644
--- a/drivers/pwm/pwm-imx-tpm.c
+++ b/drivers/pwm/pwm-imx-tpm.c
@@ -126,7 +126,7 @@ static int pwm_imx_tpm_round_state(struct pwm_chip *chip,
real_state->duty_cycle = state->duty_cycle;

tmp = (u64)p->mod * real_state->duty_cycle;
- p->val = DIV_ROUND_CLOSEST_ULL(tmp, real_state->period);
+ p->val = DIV64_U64_ROUND_CLOSEST(tmp, real_state->period);

real_state->polarity = state->polarity;
real_state->enabled = state->enabled;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Guru Das Srinagesh
2020-03-17 20:05:23 UTC
Permalink
Since the PWM framework is switching struct pwm_args.period's
datatype to u64, prepare for this transition by using the right
specifier for printing a 64-bit value.

Cc: Fabrice Gasnier <***@st.com>
Cc: Maxime Coquelin <***@gmail.com>
Cc: Alexandre Torgue <***@st.com>
Cc: Gerald Baeza <***@st.com>
Cc: Benjamin Gaignard <***@linaro.org>
Cc: Axel Lin <***@ingics.com>

Reported-by: kbuild test robot <***@intel.com>
Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
---
drivers/pwm/pwm-stm32-lp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c
index 67fca62..134c146 100644
--- a/drivers/pwm/pwm-stm32-lp.c
+++ b/drivers/pwm/pwm-stm32-lp.c
@@ -61,7 +61,7 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
do_div(div, NSEC_PER_SEC);
if (!div) {
/* Clock is too slow to achieve requested period. */
- dev_dbg(priv->chip.dev, "Can't reach %u ns\n", state->period);
+ dev_dbg(priv->chip.dev, "Can't reach %llu ns\n", state->period);
return -EINVAL;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Guru Das Srinagesh
2020-03-17 20:05:25 UTC
Permalink
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using div_u64 to handle a 64-bit
dividend instead of a straight division operation.

Cc: Lee Jones <***@linaro.org>
Cc: Daniel Thompson <***@linaro.org>
Cc: Jingoo Han <***@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <***@samsung.com>
Cc: linux-***@vger.kernel.org
Cc: dri-***@lists.freedesktop.org
Cc: linux-***@vger.kernel.org

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
Reviewed-by: Daniel Thompson <***@linaro.org>
---
drivers/video/backlight/pwm_bl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index efb4efc..3e5dbcf 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -625,7 +625,8 @@ static int pwm_backlight_probe(struct platform_device *pdev)
pb->scale = data->max_brightness;
}

- pb->lth_brightness = data->lth_brightness * (state.period / pb->scale);
+ pb->lth_brightness = data->lth_brightness * (div_u64(state.period,
+ pb->scale));

props.type = BACKLIGHT_RAW;
props.max_brightness = data->max_brightness;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Guru Das Srinagesh
2020-03-17 20:05:22 UTC
Permalink
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by using DIV64_U64_ROUND_CLOSEST to
handle a 64-bit divisor.

Cc: Palmer Dabbelt <***@dabbelt.com>
Cc: Paul Walmsley <***@sifive.com>
Cc: linux-***@lists.infradead.org
Cc: Yash Shah <***@sifive.com>
Cc: Atish Patra <***@wdc.com>
Cc: Ding Xiang <***@cmss.chinamobile.com>
Cc: Wesley W. Terpstra <***@sifive.com>

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
---
drivers/pwm/pwm-sifive.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index cc63f9b..62de0bb 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -181,7 +181,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
* consecutively
*/
num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH);
- frac = DIV_ROUND_CLOSEST_ULL(num, state->period);
+ frac = DIV64_U64_ROUND_CLOSEST(num, state->period);
/* The hardware cannot generate a 100% duty cycle */
frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Guru Das Srinagesh
2020-03-17 20:05:26 UTC
Permalink
Because period and duty cycle are defined as ints with units of
nanoseconds, the maximum time duration that can be set is limited to
~2.147 seconds. Change their definitions to u64 in the structs of the
PWM framework so that higher durations may be set.

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
---
drivers/pwm/core.c | 4 ++--
drivers/pwm/sysfs.c | 8 ++++----
include/linux/pwm.h | 12 ++++++------
3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 5a7f659..81aa3c2 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -1163,8 +1163,8 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
if (state.enabled)
seq_puts(s, " enabled");

- seq_printf(s, " period: %u ns", state.period);
- seq_printf(s, " duty: %u ns", state.duty_cycle);
+ seq_printf(s, " period: %llu ns", state.period);
+ seq_printf(s, " duty: %llu ns", state.duty_cycle);
seq_printf(s, " polarity: %s",
state.polarity ? "inverse" : "normal");

diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 2389b86..449dbc0 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -42,7 +42,7 @@ static ssize_t period_show(struct device *child,

pwm_get_state(pwm, &state);

- return sprintf(buf, "%u\n", state.period);
+ return sprintf(buf, "%llu\n", state.period);
}

static ssize_t period_store(struct device *child,
@@ -52,10 +52,10 @@ static ssize_t period_store(struct device *child,
struct pwm_export *export = child_to_pwm_export(child);
struct pwm_device *pwm = export->pwm;
struct pwm_state state;
- unsigned int val;
+ u64 val;
int ret;

- ret = kstrtouint(buf, 0, &val);
+ ret = kstrtou64(buf, 0, &val);
if (ret)
return ret;

@@ -77,7 +77,7 @@ static ssize_t duty_cycle_show(struct device *child,

pwm_get_state(pwm, &state);

- return sprintf(buf, "%u\n", state.duty_cycle);
+ return sprintf(buf, "%llu\n", state.duty_cycle);
}

static ssize_t duty_cycle_store(struct device *child,
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 0ef808d..b53f13d 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -39,7 +39,7 @@ enum pwm_polarity {
* current PWM hardware state.
*/
struct pwm_args {
- unsigned int period;
+ u64 period;
enum pwm_polarity polarity;
};

@@ -56,8 +56,8 @@ enum {
* @enabled: PWM enabled status
*/
struct pwm_state {
- unsigned int period;
- unsigned int duty_cycle;
+ u64 period;
+ u64 duty_cycle;
enum pwm_polarity polarity;
bool enabled;
};
@@ -105,13 +105,13 @@ static inline bool pwm_is_enabled(const struct pwm_device *pwm)
return state.enabled;
}

-static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
+static inline void pwm_set_period(struct pwm_device *pwm, u64 period)
{
if (pwm)
pwm->state.period = period;
}

-static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
+static inline u64 pwm_get_period(const struct pwm_device *pwm)
{
struct pwm_state state;

@@ -126,7 +126,7 @@ static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
pwm->state.duty_cycle = duty;
}

-static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
+static inline u64 pwm_get_duty_cycle(const struct pwm_device *pwm)
{
struct pwm_state state;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Guru Das Srinagesh
2020-03-17 20:05:17 UTC
Permalink
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by using DIV_ROUND_UP_ULL to handle
a 64-bit dividend.

Cc: Kamil Debski <***@wypas.org>
Cc: Bartlomiej Zolnierkiewicz <***@samsung.com>
Cc: Jean Delvare <***@suse.com>
Cc: Guenter Roeck <***@roeck-us.net>
Cc: Liam Girdwood <***@gmail.com>
Cc: Mark Brown <***@kernel.org>
Cc: linux-***@vger.kernel.org

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
Acked-by: Guenter Roeck <***@roeck-us.net>
---
drivers/hwmon/pwm-fan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 42ffd2e..283423a 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -437,7 +437,7 @@ static int pwm_fan_resume(struct device *dev)
return 0;

pwm_get_args(ctx->pwm, &pargs);
- duty = DIV_ROUND_UP(ctx->pwm_value * (pargs.period - 1), MAX_PWM);
+ duty = DIV_ROUND_UP_ULL(ctx->pwm_value * (pargs.period - 1), MAX_PWM);
ret = pwm_config(ctx->pwm, duty, pargs.period);
if (ret)
return ret;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Guru Das Srinagesh
2020-03-17 20:05:16 UTC
Permalink
Since the PWM framework is switching struct pwm_state.duty_cycle's
datatype to u64, prepare for this transition by using DIV_ROUND_UP_ULL
to handle a 64-bit dividend.

Cc: Jani Nikula <***@linux.intel.com>
Cc: Joonas Lahtinen <***@linux.intel.com>
Cc: David Airlie <***@linux.ie>
Cc: Daniel Vetter <***@ffwll.ch>
Cc: Chris Wilson <***@chris-wilson.co.uk>
Cc: "Ville Syrjälä" <***@linux.intel.com>
Cc: intel-***@lists.freedesktop.org
Cc: dri-***@lists.freedesktop.org
Cc: Rodrigo Vivi <***@intel.com>
Cc: Maarten Lankhorst <***@linux.intel.com>

Signed-off-by: Guru Das Srinagesh <***@codeaurora.org>
---
drivers/gpu/drm/i915/display/intel_panel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index bc14e9c..843cac1 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -1868,7 +1868,7 @@ static int pwm_setup_backlight(struct intel_connector *connector,

panel->backlight.min = 0; /* 0% */
panel->backlight.max = 100; /* 100% */
- panel->backlight.level = DIV_ROUND_UP(
+ panel->backlight.level = DIV_ROUND_UP_ULL(
pwm_get_duty_cycle(panel->backlight.pwm) * 100,
CRC_PMIC_PWM_PERIOD_NS);
panel->backlight.enabled = panel->backlight.level != 0;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Loading...