From bea307c16a3a297f87c7ab9a54de686da2afbad5 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Wed, 2 Mar 2016 23:33:34 +0100 Subject: [PATCH 1/8] pwm: img: Test clock rate to avoid division by 0 The clk API may return 0 on clk_get_rate(), so we should check the result before using it as a divisor. Signed-off-by: Wolfram Sang Signed-off-by: Thierry Reding --- drivers/pwm/pwm-img.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/pwm/pwm-img.c b/drivers/pwm/pwm-img.c index 8a029f9bc18c..2fb30deee345 100644 --- a/drivers/pwm/pwm-img.c +++ b/drivers/pwm/pwm-img.c @@ -237,6 +237,11 @@ static int img_pwm_probe(struct platform_device *pdev) } clk_rate = clk_get_rate(pwm->pwm_clk); + if (!clk_rate) { + dev_err(&pdev->dev, "pwm clock has no frequency\n"); + ret = -EINVAL; + goto disable_pwmclk; + } /* The maximum input clock divider is 512 */ val = (u64)NSEC_PER_SEC * 512 * pwm->data->max_timebase; From 0e47b5981a30e856c9c4aba785890528486d1594 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Wed, 2 Mar 2016 23:57:09 +0100 Subject: [PATCH 2/8] pwm: lpc18xx-sct: Test clock rate to avoid division by 0 The clk API may return 0 on clk_get_rate(), so we should check the result before using it as a divisor. Signed-off-by: Wolfram Sang Acked-by: Joachim Eastwood Signed-off-by: Thierry Reding --- drivers/pwm/pwm-lpc18xx-sct.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c index 9163085101bc..9861fed4e67d 100644 --- a/drivers/pwm/pwm-lpc18xx-sct.c +++ b/drivers/pwm/pwm-lpc18xx-sct.c @@ -360,6 +360,11 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev) } lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk); + if (!lpc18xx_pwm->clk_rate) { + dev_err(&pdev->dev, "pwm clock has no frequency\n"); + ret = -EINVAL; + goto disable_pwmclk; + } mutex_init(&lpc18xx_pwm->res_lock); mutex_init(&lpc18xx_pwm->period_lock); From 03d99531aed486f9e7020da3810102df1b91cd34 Mon Sep 17 00:00:00 2001 From: Simon Horman Date: Thu, 25 Feb 2016 10:03:23 +0900 Subject: [PATCH 3/8] pwm: rcar: Depend on ARCH_RENESAS instead of ARCH_SHMOBILE This is part of an ongoing process to migrate from ARCH_SHMOBILE to ARCH_RENESAS the motivation for which being that RENESAS seems to be a more appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs. Signed-off-by: Simon Horman Acked-by: Geert Uytterhoeven Signed-off-by: Thierry Reding --- drivers/pwm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 8cf0dae78555..c182efc62c7b 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -316,7 +316,7 @@ config PWM_RCAR config PWM_RENESAS_TPU tristate "Renesas TPU PWM support" - depends on ARCH_SHMOBILE || COMPILE_TEST + depends on ARCH_RENESAS || COMPILE_TEST depends on HAS_IOMEM help This driver exposes the Timer Pulse Unit (TPU) PWM controller found From c5857e3f94ab2719dfac649a146cb5dd6f21fcf3 Mon Sep 17 00:00:00 2001 From: Vladimir Zapolskiy Date: Sun, 6 Mar 2016 03:21:46 +0200 Subject: [PATCH 4/8] pwm: brcmstb: Fix check of devm_ioremap_resource() return code The change fixes potential oops while accessing iomem on invalid address if devm_ioremap_resource() fails due to some reason. The devm_ioremap_resource() function returns ERR_PTR() and never returns NULL, which makes useless a following check for NULL. Signed-off-by: Vladimir Zapolskiy Fixes: 3a9f5957020f ("pwm: Add Broadcom BCM7038 PWM controller support") Acked-by: Florian Fainelli Signed-off-by: Thierry Reding --- drivers/pwm/pwm-brcmstb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-brcmstb.c b/drivers/pwm/pwm-brcmstb.c index 423ce087cd9c..5d5adee16886 100644 --- a/drivers/pwm/pwm-brcmstb.c +++ b/drivers/pwm/pwm-brcmstb.c @@ -274,8 +274,8 @@ static int brcmstb_pwm_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); p->base = devm_ioremap_resource(&pdev->dev, res); - if (!p->base) { - ret = -ENOMEM; + if (IS_ERR(p->base)) { + ret = PTR_ERR(p->base); goto out_clk; } From f8caa792261c0edded20eba2b8fcc899a1b91819 Mon Sep 17 00:00:00 2001 From: David Rivshin Date: Fri, 29 Jan 2016 23:26:51 -0500 Subject: [PATCH 5/8] pwm: omap-dmtimer: Fix inaccurate period and duty cycle calculations Fix the calculation of load_value and match_value. Currently they are slightly too low, which produces a noticeably wrong PWM rate with sufficiently short periods (i.e. when 1/period approaches clk_rate/2). Example: clk_rate=32768Hz, period=122070ns, duty_cycle=61035ns (8192Hz/50% PWM) Correct values: load = 0xfffffffc, match = 0xfffffffd Current values: load = 0xfffffffa, match = 0xfffffffc effective PWM: period=183105ns, duty_cycle=91553ns (5461Hz/50% PWM) Fixes: 6604c6556db9 ("pwm: Add PWM driver for OMAP using dual-mode timers") Signed-off-by: David Rivshin Acked-by: Neil Armstrong Tested-by: Adam Ford Signed-off-by: Thierry Reding --- drivers/pwm/pwm-omap-dmtimer.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index 826634ec0d5c..0083e75d950f 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -31,6 +31,7 @@ #include #define DM_TIMER_LOAD_MIN 0xfffffffe +#define DM_TIMER_MAX 0xffffffff struct pwm_omap_dmtimer_chip { struct pwm_chip chip; @@ -46,13 +47,13 @@ to_pwm_omap_dmtimer_chip(struct pwm_chip *chip) return container_of(chip, struct pwm_omap_dmtimer_chip, chip); } -static int pwm_omap_dmtimer_calc_value(unsigned long clk_rate, int ns) +static u32 pwm_omap_dmtimer_get_clock_cycles(unsigned long clk_rate, int ns) { u64 c = (u64)clk_rate * ns; do_div(c, NSEC_PER_SEC); - return DM_TIMER_LOAD_MIN - c; + return c; } static void pwm_omap_dmtimer_start(struct pwm_omap_dmtimer_chip *omap) @@ -99,7 +100,8 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, int duty_ns, int period_ns) { struct pwm_omap_dmtimer_chip *omap = to_pwm_omap_dmtimer_chip(chip); - int load_value, match_value; + u32 period_cycles, duty_cycles; + u32 load_value, match_value; struct clk *fclk; unsigned long clk_rate; bool timer_active; @@ -133,11 +135,22 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, /* * Calculate the appropriate load and match values based on the * specified period and duty cycle. The load value determines the - * cycle time and the match value determines the duty cycle. + * period time and the match value determines the duty time. + * + * The period lasts for (DM_TIMER_MAX-load_value+1) clock cycles. + * Similarly, the active time lasts (match_value-load_value+1) cycles. + * The non-active time is the remainder: (DM_TIMER_MAX-match_value) + * clock cycles. + * + * References: + * OMAP4430/60/70 TRM sections 22.2.4.10 and 22.2.4.11 + * AM335x Sitara TRM sections 20.1.3.5 and 20.1.3.6 */ - load_value = pwm_omap_dmtimer_calc_value(clk_rate, period_ns); - match_value = pwm_omap_dmtimer_calc_value(clk_rate, - period_ns - duty_ns); + period_cycles = pwm_omap_dmtimer_get_clock_cycles(clk_rate, period_ns); + duty_cycles = pwm_omap_dmtimer_get_clock_cycles(clk_rate, duty_ns); + + load_value = (DM_TIMER_MAX - period_cycles) + 1; + match_value = load_value + duty_cycles - 1; /* * We MUST stop the associated dual-mode timer before attempting to From cd378881426379a62a7fe67f34b8cbe738302022 Mon Sep 17 00:00:00 2001 From: David Rivshin Date: Fri, 29 Jan 2016 23:26:52 -0500 Subject: [PATCH 6/8] pwm: omap-dmtimer: Add sanity checking for load and match values Add sanity checking to ensure that we do not program load or match values that are out of range if a user requests period or duty_cycle values which are not achievable. The match value cannot be less than the load value (but can be equal), and neither can be 0xffffffff. This means that there must be at least one fclk cycle between load and match, and another between match and overflow. Fixes: 6604c6556db9 ("pwm: Add PWM driver for OMAP using dual-mode timers") Signed-off-by: David Rivshin Acked-by: Neil Armstrong [thierry.reding@gmail.com: minor coding style cleanups] Signed-off-by: Thierry Reding --- drivers/pwm/pwm-omap-dmtimer.c | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index 0083e75d950f..54641e45f45b 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -119,15 +119,13 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, fclk = omap->pdata->get_fclk(omap->dm_timer); if (!fclk) { dev_err(chip->dev, "invalid pmtimer fclk\n"); - mutex_unlock(&omap->mutex); - return -EINVAL; + goto err_einval; } clk_rate = clk_get_rate(fclk); if (!clk_rate) { dev_err(chip->dev, "invalid pmtimer fclk rate\n"); - mutex_unlock(&omap->mutex); - return -EINVAL; + goto err_einval; } dev_dbg(chip->dev, "clk rate: %luHz\n", clk_rate); @@ -142,6 +140,8 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, * The non-active time is the remainder: (DM_TIMER_MAX-match_value) * clock cycles. * + * NOTE: It is required that: load_value <= match_value < DM_TIMER_MAX + * * References: * OMAP4430/60/70 TRM sections 22.2.4.10 and 22.2.4.11 * AM335x Sitara TRM sections 20.1.3.5 and 20.1.3.6 @@ -149,6 +149,27 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, period_cycles = pwm_omap_dmtimer_get_clock_cycles(clk_rate, period_ns); duty_cycles = pwm_omap_dmtimer_get_clock_cycles(clk_rate, duty_ns); + if (period_cycles < 2) { + dev_info(chip->dev, + "period %d ns too short for clock rate %lu Hz\n", + period_ns, clk_rate); + goto err_einval; + } + + if (duty_cycles < 1) { + dev_dbg(chip->dev, + "duty cycle %d ns is too short for clock rate %lu Hz\n", + duty_ns, clk_rate); + dev_dbg(chip->dev, "using minimum of 1 clock cycle\n"); + duty_cycles = 1; + } else if (duty_cycles >= period_cycles) { + dev_dbg(chip->dev, + "duty cycle %d ns is too long for period %d ns at clock rate %lu Hz\n", + duty_ns, period_ns, clk_rate); + dev_dbg(chip->dev, "using maximum of 1 clock cycle less than period\n"); + duty_cycles = period_cycles - 1; + } + load_value = (DM_TIMER_MAX - period_cycles) + 1; match_value = load_value + duty_cycles - 1; @@ -179,6 +200,11 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, mutex_unlock(&omap->mutex); return 0; + +err_einval: + mutex_unlock(&omap->mutex); + + return -EINVAL; } static int pwm_omap_dmtimer_set_polarity(struct pwm_chip *chip, From 7b0883f33809ff0aeca9848193c31629a752bb77 Mon Sep 17 00:00:00 2001 From: David Rivshin Date: Fri, 29 Jan 2016 23:26:53 -0500 Subject: [PATCH 7/8] pwm: omap-dmtimer: Round load and match values rather than truncate When converting period and duty_cycle from nanoseconds to fclk cycles, the error introduced by the integer division can be appreciable, especially in the case of slow fclk or short period. Use DIV_ROUND_CLOSEST_ULL() so that the error is kept to +/- 0.5 clock cycles. Fixes: 6604c6556db9 ("pwm: Add PWM driver for OMAP using dual-mode timers") Signed-off-by: David Rivshin Acked-by: Neil Armstrong Signed-off-by: Thierry Reding --- drivers/pwm/pwm-omap-dmtimer.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index 54641e45f45b..e0679eb399f6 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -49,11 +49,7 @@ to_pwm_omap_dmtimer_chip(struct pwm_chip *chip) static u32 pwm_omap_dmtimer_get_clock_cycles(unsigned long clk_rate, int ns) { - u64 c = (u64)clk_rate * ns; - - do_div(c, NSEC_PER_SEC); - - return c; + return DIV_ROUND_CLOSEST_ULL((u64)clk_rate * ns, NSEC_PER_SEC); } static void pwm_omap_dmtimer_start(struct pwm_omap_dmtimer_chip *omap) From 922201d129c8f9d0c3207dca90ea6ffd8e2242f0 Mon Sep 17 00:00:00 2001 From: David Rivshin Date: Fri, 29 Jan 2016 23:26:54 -0500 Subject: [PATCH 8/8] pwm: omap-dmtimer: Add debug message for effective period and duty cycle After going through the math and constraints checking to compute load and match values, it is helpful to know what the resultant period and duty cycle are. Signed-off-by: David Rivshin Acked-by: Neil Armstrong Signed-off-by: Thierry Reding --- drivers/pwm/pwm-omap-dmtimer.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index e0679eb399f6..b7e6ecba7d5c 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -102,7 +102,8 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, unsigned long clk_rate; bool timer_active; - dev_dbg(chip->dev, "duty cycle: %d, period %d\n", duty_ns, period_ns); + dev_dbg(chip->dev, "requested duty cycle: %d ns, period: %d ns\n", + duty_ns, period_ns); mutex_lock(&omap->mutex); if (duty_ns == pwm_get_duty_cycle(pwm) && @@ -166,6 +167,12 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, duty_cycles = period_cycles - 1; } + dev_dbg(chip->dev, "effective duty cycle: %lld ns, period: %lld ns\n", + DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * duty_cycles, + clk_rate), + DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * period_cycles, + clk_rate)); + load_value = (DM_TIMER_MAX - period_cycles) + 1; match_value = load_value + duty_cycles - 1;