[PATCH v9 01/11] drm/i915: Use 64-bit division macro

[PATCH v9 01/11] drm/i915: Use 64-bit division macro

[Guru Das Srinagesh]

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 <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Signed-off-by: Guru Das Srinagesh <gurus@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

[PATCH v9 02/11] hwmon: pwm-fan: Use 64-bit division macro

[Guru Das Srinagesh]

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 <kamil@wypas.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-hwmon@vger.kernel.org

Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Guenter Roeck <linux@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

[PATCH v9 03/11] ir-rx51: Use 64-bit division macro

[Guru Das Srinagesh]

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 <mchehab@kernel.org>
Cc: Richard Fontana <rfontana@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Allison Randal <allison@lohutok.net>
Cc: linux-media@vger.kernel.org

Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Sean Young <sean@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

[PATCH v9 04/11] pwm: clps711x: Use 64-bit division macro

[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.

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

Signed-off-by: Guru Das Srinagesh <gurus@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

[PATCH v9 05/11] pwm: pwm-imx-tpm: Use 64-bit division macro

[Guru Das Srinagesh]

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 <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Anson Huang <Anson.Huang@nxp.com>

Signed-off-by: Guru Das Srinagesh <gurus@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

[PATCH v9 06/11] pwm: imx27: Use 64-bit division macro and function

[Guru Das Srinagesh]

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 <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Michal Vokáč <michal.vokac@ysoft.com>
Cc: Adam Ford <aford173@gmail.com>
Cc: Mukesh Ojha <mojha@codeaurora.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>

Signed-off-by: Guru Das Srinagesh <gurus@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

[PATCH v9 07/11] pwm: sifive: Use 64-bit division macro

[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.

Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: linux-riscv@lists.infradead.org
Cc: Yash Shah <yash.shah@sifive.com>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Ding Xiang <dingxiang@cmss.chinamobile.com>
Cc: Wesley W. Terpstra <wesley@sifive.com>

Signed-off-by: Guru Das Srinagesh <gurus@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

[PATCH v9 08/11] pwm: stm32-lp: Use %llu format specifier for period

[Guru Das Srinagesh]

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 <fabrice.gasnier@st.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Gerald Baeza <gerald.baeza@st.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Axel Lin <axel.lin@ingics.com>

Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Guru Das Srinagesh <gurus@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

[PATCH v9 09/11] pwm: sun4i: Use 64-bit division function

[Guru Das Srinagesh]

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 <mripard@kernel.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Philipp Zabel <p.zabel@pengutronix.de>

Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Chen-Yu Tsai <wens@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

[PATCH v9 10/11] backlight: pwm_bl: Use 64-bit division function

[Guru Das Srinagesh]

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 <lee.jones@linaro.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: linux-pwm@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org

Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Reviewed-by: Daniel Thompson <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

[PATCH v9 11/11] pwm: core: Convert period and duty cycle to u64

[Guru Das Srinagesh]

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 <gurus@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

Re: [PATCH v9 04/11] pwm: clps711x: Use 64-bit division macro

[Arnd Bergmann]

On Tue, Mar 17, 2020 at 9:05 PM Guru Das Srinagesh <gurus@codeaurora.org> wrote:
>
> 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 <shc_work@mail.ru>
> Cc: Arnd Bergmann <arnd@arndb.de>
>
> Signed-off-by: Guru Das Srinagesh <gurus@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);
>  }

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

Re: [PATCH v9 04/11] pwm: clps711x: Use 64-bit division macro

[Guru Das Srinagesh]

On Tue, Mar 17, 2020 at 11:22:06PM +0100, Arnd Bergmann wrote:
> > 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.

Re: [PATCH v9 04/11] pwm: clps711x: Use 64-bit division macro

[Arnd Bergmann]

On Wed, Mar 18, 2020 at 12:30 AM Guru Das Srinagesh
<gurus@codeaurora.org> wrote:
>
> On Tue, Mar 17, 2020 at 11:22:06PM +0100, Arnd Bergmann wrote:
> > > 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?

Ah, so if the period can actually be larger than U32_MAX, you need to
handle that case. However, I see that the divident in this code (v * 0xf)
is still a 32-bit number, so a correct and efficient implementation could be

   if (pwm->args.period > (UINT_MAX / 0xf))
          return 0;
   return DIV_ROUND_CLOSEST(v * 0xf, (u32)pwm->args.period);

        Arnd

Re: [PATCH v9 04/11] pwm: clps711x: Use 64-bit division macro

[Guru Das Srinagesh]

On Wed, Mar 18, 2020 at 10:49:34AM +0100, Arnd Bergmann wrote:
> On Wed, Mar 18, 2020 at 12:30 AM Guru Das Srinagesh
> <gurus@codeaurora.org> wrote:
> >
> > On Tue, Mar 17, 2020 at 11:22:06PM +0100, Arnd Bergmann wrote:
> > > > 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?
> 
> Ah, so if the period can actually be larger than U32_MAX, you need to
> handle that case. However, I see that the divident in this code (v * 0xf)
> is still a 32-bit number, so a correct and efficient implementation could be
> 
>    if (pwm->args.period > (UINT_MAX / 0xf))

Shouldn't the if condition be the following? Or am I missing
something here?

     if (pwm->args.period > (UINT_MAX / (v * 0xf)))
     					^^^^^^^^^

>           return 0;
>    return DIV_ROUND_CLOSEST(v * 0xf, (u32)pwm->args.period);

Thank you.

Guru Das.

Re: [PATCH v9 01/11] drm/i915: Use 64-bit division macro

[Jani Nikula]

On Tue, 17 Mar 2020, Guru Das Srinagesh <gurus@codeaurora.org> wrote:
> 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 <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Also ack for merging this via whichever tree you prefer; please let me
know if you want me to take this via drm-intel.

> ---
>  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;

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH v9 04/11] pwm: clps711x: Use 64-bit division macro

[Arnd Bergmann]

On Wed, Mar 18, 2020 at 6:00 PM Guru Das Srinagesh <gurus@codeaurora.org> wrote:
>
> On Wed, Mar 18, 2020 at 10:49:34AM +0100, Arnd Bergmann wrote:
> > On Wed, Mar 18, 2020 at 12:30 AM Guru Das Srinagesh
> > <gurus@codeaurora.org> wrote:
> > >
> > > On Tue, Mar 17, 2020 at 11:22:06PM +0100, Arnd Bergmann wrote:
> > > > > 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?
> >
> > Ah, so if the period can actually be larger than U32_MAX, you need to
> > handle that case. However, I see that the divident in this code (v * 0xf)
> > is still a 32-bit number, so a correct and efficient implementation could be
> >
> >    if (pwm->args.period > (UINT_MAX / 0xf))
>
> Shouldn't the if condition be the following? Or am I missing
> something here?
>
>      if (pwm->args.period > (UINT_MAX / (v * 0xf)))
>                                         ^^^^^^^^^

That would require performing a division, which is an external function
call on ARMv4. My version just checks for an upper bound and completely
avoids the division. You could also just check for ">UINT_MAX" if you
find that clearer.

      Arnd

Re: [PATCH v9 01/11] drm/i915: Use 64-bit division macro

[Guru Das Srinagesh]

On Wed, Mar 18, 2020 at 09:08:45PM +0200, Jani Nikula wrote:
> On Tue, 17 Mar 2020, Guru Das Srinagesh <gurus@codeaurora.org> wrote:
> > 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 <jani.nikula@linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >
> > Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Thanks!

> Also ack for merging this via whichever tree you prefer; please let me
> know if you want me to take this via drm-intel.

I'm not sure if this patch can be taken via drm-intel in isolation as it
is part of a series... Will let you know if I need to make such a
request.

Thank you.

Guru Das.

Re: [PATCH v9 04/11] pwm: clps711x: Use 64-bit division macro

[Guru Das Srinagesh]

On Wed, Mar 18, 2020 at 08:38:29PM +0100, Arnd Bergmann wrote:
> On Wed, Mar 18, 2020 at 6:00 PM Guru Das Srinagesh <gurus@codeaurora.org> wrote:
> >
> > On Wed, Mar 18, 2020 at 10:49:34AM +0100, Arnd Bergmann wrote:
> > > On Wed, Mar 18, 2020 at 12:30 AM Guru Das Srinagesh
> > > <gurus@codeaurora.org> wrote:
> > > >
> > > > On Tue, Mar 17, 2020 at 11:22:06PM +0100, Arnd Bergmann wrote:
> > > > > > 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?
> > >
> > > Ah, so if the period can actually be larger than U32_MAX, you need to
> > > handle that case. However, I see that the divident in this code (v * 0xf)
> > > is still a 32-bit number, so a correct and efficient implementation could be
> > >
> > >    if (pwm->args.period > (UINT_MAX / 0xf))
> >
> > Shouldn't the if condition be the following? Or am I missing
> > something here?
> >
> >      if (pwm->args.period > (UINT_MAX / (v * 0xf)))
> >                                         ^^^^^^^^^
> 
> That would require performing a division, which is an external function
> call on ARMv4. My version just checks for an upper bound and completely
> avoids the division. You could also just check for ">UINT_MAX" if you
> find that clearer.

Thanks, have checked for UINT_MAX in v10 of my patchset that I just
uploaded. Could you please review it?

Thank you.

Guru Das.