From 684309e5043efaaa94d9b4e65f53f7fdde1f6675 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Sat, 28 Jan 2017 17:10:39 +0200 Subject: pwm: lpss: Avoid potential overflow of base_unit The resolution of base_unit is derived from base_unit_bits and thus must be equal to (2^base_unit_bits - 1). Otherwise frequency and therefore base_unit might potentially overflow. Prevent the above by substracting 1 in all cases where base_unit_bits or derivative is used. Reviewed-by: Mika Westerberg Signed-off-by: Andy Shevchenko Signed-off-by: Thierry Reding --- drivers/pwm/pwm-lpss.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/pwm/pwm-lpss.c') diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c index 72c0bce5a75c..8642feeb8abd 100644 --- a/drivers/pwm/pwm-lpss.c +++ b/drivers/pwm/pwm-lpss.c @@ -102,7 +102,7 @@ static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm, * The equation is: * base_unit = round(base_unit_range * freq / c) */ - base_unit_range = BIT(lpwm->info->base_unit_bits); + base_unit_range = BIT(lpwm->info->base_unit_bits) - 1; freq *= base_unit_range; base_unit = DIV_ROUND_CLOSEST_ULL(freq, c); @@ -117,8 +117,8 @@ static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm, ctrl = pwm_lpss_read(pwm); ctrl &= ~PWM_ON_TIME_DIV_MASK; - ctrl &= ~((base_unit_range - 1) << PWM_BASE_UNIT_SHIFT); - base_unit &= (base_unit_range - 1); + ctrl &= ~(base_unit_range << PWM_BASE_UNIT_SHIFT); + base_unit &= base_unit_range; ctrl |= (u32) base_unit << PWM_BASE_UNIT_SHIFT; ctrl |= on_time_div; pwm_lpss_write(pwm, ctrl); -- cgit From b5c050c719922901891bdd8580d224e2a0035db5 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Sat, 28 Jan 2017 17:10:40 +0200 Subject: pwm: lpss: Allow duty cycle to be 0 A duty cycle is represented by values [0..] which reflects [0%..100%]. 0% of the duty cycle means always off (logical "0") on output. Allow this in the driver. Reviewed-by: Mika Westerberg Signed-off-by: Andy Shevchenko Signed-off-by: Thierry Reding --- drivers/pwm/pwm-lpss.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/pwm/pwm-lpss.c') diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c index 8642feeb8abd..ffa01ab907a6 100644 --- a/drivers/pwm/pwm-lpss.c +++ b/drivers/pwm/pwm-lpss.c @@ -107,8 +107,6 @@ static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm, base_unit = DIV_ROUND_CLOSEST_ULL(freq, c); - if (duty_ns <= 0) - duty_ns = 1; on_time_div = 255ULL * duty_ns; do_div(on_time_div, period_ns); on_time_div = 255ULL - on_time_div; -- cgit From b14e8ceff03404cce4d9b85204246d3ed1259ec7 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Sat, 28 Jan 2017 17:10:41 +0200 Subject: pwm: lpss: Switch to new atomic API Instead of doing things separately, which is not so reliable on some platforms, switch the driver to use new atomic API, i.e. ->apply() callback. The change has been tested on Intel platforms such as Broxton, BayTrail, and Merrifield. Reviewed-by: Mika Westerberg Signed-off-by: Andy Shevchenko Signed-off-by: Thierry Reding --- drivers/pwm/pwm-lpss.c | 64 +++++++++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 35 deletions(-) (limited to 'drivers/pwm/pwm-lpss.c') diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c index ffa01ab907a6..09869f91d2d0 100644 --- a/drivers/pwm/pwm-lpss.c +++ b/drivers/pwm/pwm-lpss.c @@ -82,15 +82,20 @@ static inline void pwm_lpss_write(const struct pwm_device *pwm, u32 value) static void pwm_lpss_update(struct pwm_device *pwm) { + /* + * Set a limit for busyloop since not all implementations correctly + * clear PWM_SW_UPDATE bit (at least it's not visible on OS side). + */ + unsigned int count = 10; + pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_SW_UPDATE); - /* Give it some time to propagate */ - usleep_range(10, 50); + while (pwm_lpss_read(pwm) & PWM_SW_UPDATE && --count) + usleep_range(10, 20); } -static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm, - int duty_ns, int period_ns) +static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm, + int duty_ns, int period_ns) { - struct pwm_lpss_chip *lpwm = to_lpwm(chip); unsigned long long on_time_div; unsigned long c = lpwm->info->clk_rate, base_unit_range; unsigned long long base_unit, freq = NSEC_PER_SEC; @@ -111,8 +116,6 @@ static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm, do_div(on_time_div, period_ns); on_time_div = 255ULL - on_time_div; - pm_runtime_get_sync(chip->dev); - ctrl = pwm_lpss_read(pwm); ctrl &= ~PWM_ON_TIME_DIV_MASK; ctrl &= ~(base_unit_range << PWM_BASE_UNIT_SHIFT); @@ -120,42 +123,33 @@ static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm, ctrl |= (u32) base_unit << PWM_BASE_UNIT_SHIFT; ctrl |= on_time_div; pwm_lpss_write(pwm, ctrl); - - /* - * If the PWM is already enabled we need to notify the hardware - * about the change by setting PWM_SW_UPDATE. - */ - if (pwm_is_enabled(pwm)) - pwm_lpss_update(pwm); - - pm_runtime_put(chip->dev); - - return 0; } -static int pwm_lpss_enable(struct pwm_chip *chip, struct pwm_device *pwm) +static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) { - pm_runtime_get_sync(chip->dev); + struct pwm_lpss_chip *lpwm = to_lpwm(chip); - /* - * Hardware must first see PWM_SW_UPDATE before the PWM can be - * enabled. - */ - pwm_lpss_update(pwm); - pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_ENABLE); - return 0; -} + if (state->enabled) { + if (!pwm_is_enabled(pwm)) { + pm_runtime_get_sync(chip->dev); + pwm_lpss_prepare(lpwm, pwm, state->duty_cycle, state->period); + pwm_lpss_update(pwm); + pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_ENABLE); + } else { + pwm_lpss_prepare(lpwm, pwm, state->duty_cycle, state->period); + pwm_lpss_update(pwm); + } + } else if (pwm_is_enabled(pwm)) { + pwm_lpss_write(pwm, pwm_lpss_read(pwm) & ~PWM_ENABLE); + pm_runtime_put(chip->dev); + } -static void pwm_lpss_disable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - pwm_lpss_write(pwm, pwm_lpss_read(pwm) & ~PWM_ENABLE); - pm_runtime_put(chip->dev); + return 0; } static const struct pwm_ops pwm_lpss_ops = { - .config = pwm_lpss_config, - .enable = pwm_lpss_enable, - .disable = pwm_lpss_disable, + .apply = pwm_lpss_apply, .owner = THIS_MODULE, }; -- cgit From 10d56a4cb1c6c894c60acbaec0f8aa44aba833b0 Mon Sep 17 00:00:00 2001 From: Ilkka Koskinen Date: Sat, 28 Jan 2017 17:10:42 +0200 Subject: pwm: lpss: Avoid reconfiguring while UPDATE bit is still enabled PWM Configuration register has SW_UPDATE bit that is set when a new configuration is written to the register. The bit is automatically cleared at the start of the next output cycle by the IP block. If one writes a new configuration to the register while it still has the bit enabled, PWM may freeze. That is, while one can still write to the register, it won't have an effect. Thus, we try to sleep long enough that the bit gets cleared and make sure the bit is not enabled while we update the configuration. Reviewed-by: Mika Westerberg Tested-by: Richard Griffiths Signed-off-by: Ilkka Koskinen Signed-off-by: Andy Shevchenko Signed-off-by: Thierry Reding --- drivers/pwm/pwm-lpss.c | 52 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 9 deletions(-) (limited to 'drivers/pwm/pwm-lpss.c') diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c index 09869f91d2d0..46670276690d 100644 --- a/drivers/pwm/pwm-lpss.c +++ b/drivers/pwm/pwm-lpss.c @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -80,17 +81,37 @@ static inline void pwm_lpss_write(const struct pwm_device *pwm, u32 value) writel(value, lpwm->regs + pwm->hwpwm * PWM_SIZE + PWM); } -static void pwm_lpss_update(struct pwm_device *pwm) +static int pwm_lpss_update(struct pwm_device *pwm) { + struct pwm_lpss_chip *lpwm = to_lpwm(pwm->chip); + const void __iomem *addr = lpwm->regs + pwm->hwpwm * PWM_SIZE + PWM; + const unsigned int ms = 500 * USEC_PER_MSEC; + u32 val; + int err; + + pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_SW_UPDATE); + /* - * Set a limit for busyloop since not all implementations correctly - * clear PWM_SW_UPDATE bit (at least it's not visible on OS side). + * PWM Configuration register has SW_UPDATE bit that is set when a new + * configuration is written to the register. The bit is automatically + * cleared at the start of the next output cycle by the IP block. + * + * If one writes a new configuration to the register while it still has + * the bit enabled, PWM may freeze. That is, while one can still write + * to the register, it won't have an effect. Thus, we try to sleep long + * enough that the bit gets cleared and make sure the bit is not + * enabled while we update the configuration. */ - unsigned int count = 10; + err = readl_poll_timeout(addr, val, !(val & PWM_SW_UPDATE), 40, ms); + if (err) + dev_err(pwm->chip->dev, "PWM_SW_UPDATE was not cleared\n"); - pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_SW_UPDATE); - while (pwm_lpss_read(pwm) & PWM_SW_UPDATE && --count) - usleep_range(10, 20); + return err; +} + +static inline int pwm_lpss_is_updating(struct pwm_device *pwm) +{ + return (pwm_lpss_read(pwm) & PWM_SW_UPDATE) ? -EBUSY : 0; } static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm, @@ -129,16 +150,29 @@ static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state *state) { struct pwm_lpss_chip *lpwm = to_lpwm(chip); + int ret; if (state->enabled) { if (!pwm_is_enabled(pwm)) { pm_runtime_get_sync(chip->dev); + ret = pwm_lpss_is_updating(pwm); + if (ret) { + pm_runtime_put(chip->dev); + return ret; + } pwm_lpss_prepare(lpwm, pwm, state->duty_cycle, state->period); - pwm_lpss_update(pwm); + ret = pwm_lpss_update(pwm); + if (ret) { + pm_runtime_put(chip->dev); + return ret; + } pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_ENABLE); } else { + ret = pwm_lpss_is_updating(pwm); + if (ret) + return ret; pwm_lpss_prepare(lpwm, pwm, state->duty_cycle, state->period); - pwm_lpss_update(pwm); + return pwm_lpss_update(pwm); } } else if (pwm_is_enabled(pwm)) { pwm_lpss_write(pwm, pwm_lpss_read(pwm) & ~PWM_ENABLE); -- cgit From 9900073cf5587662df9b7ef59f649ff100229d85 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Sat, 28 Jan 2017 17:10:43 +0200 Subject: pwm: lpss: Do not export board infos for different PWM types The PWM LPSS probe drivers just pass a pointer to the exported board info structures to pwm_lpss_probe() based on device PCI or ACPI ID. In order to remove the knowledge of specific devices from library part of the driver and reduce noise in exported namespace just duplicate the board info structures and stop exporting them. Signed-off-by: Andy Shevchenko Signed-off-by: Thierry Reding --- drivers/pwm/pwm-lpss-pci.c | 21 +++++++++++++++++++++ drivers/pwm/pwm-lpss-platform.c | 21 +++++++++++++++++++++ drivers/pwm/pwm-lpss.c | 24 ------------------------ drivers/pwm/pwm-lpss.h | 4 ---- 4 files changed, 42 insertions(+), 28 deletions(-) (limited to 'drivers/pwm/pwm-lpss.c') diff --git a/drivers/pwm/pwm-lpss-pci.c b/drivers/pwm/pwm-lpss-pci.c index 3622f093490e..2e7a9a48060d 100644 --- a/drivers/pwm/pwm-lpss-pci.c +++ b/drivers/pwm/pwm-lpss-pci.c @@ -17,6 +17,27 @@ #include "pwm-lpss.h" +/* BayTrail */ +static const struct pwm_lpss_boardinfo pwm_lpss_byt_info = { + .clk_rate = 25000000, + .npwm = 1, + .base_unit_bits = 16, +}; + +/* Braswell */ +static const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { + .clk_rate = 19200000, + .npwm = 1, + .base_unit_bits = 16, +}; + +/* Broxton */ +static const struct pwm_lpss_boardinfo pwm_lpss_bxt_info = { + .clk_rate = 19200000, + .npwm = 4, + .base_unit_bits = 22, +}; + static int pwm_lpss_probe_pci(struct pci_dev *pdev, const struct pci_device_id *id) { diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c index 54433fc6d1a4..b22b6fdadb9a 100644 --- a/drivers/pwm/pwm-lpss-platform.c +++ b/drivers/pwm/pwm-lpss-platform.c @@ -18,6 +18,27 @@ #include "pwm-lpss.h" +/* BayTrail */ +static const struct pwm_lpss_boardinfo pwm_lpss_byt_info = { + .clk_rate = 25000000, + .npwm = 1, + .base_unit_bits = 16, +}; + +/* Braswell */ +static const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { + .clk_rate = 19200000, + .npwm = 1, + .base_unit_bits = 16, +}; + +/* Broxton */ +static const struct pwm_lpss_boardinfo pwm_lpss_bxt_info = { + .clk_rate = 19200000, + .npwm = 4, + .base_unit_bits = 22, +}; + static int pwm_lpss_probe_platform(struct platform_device *pdev) { const struct pwm_lpss_boardinfo *info; diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c index 46670276690d..689d2c1cbead 100644 --- a/drivers/pwm/pwm-lpss.c +++ b/drivers/pwm/pwm-lpss.c @@ -38,30 +38,6 @@ struct pwm_lpss_chip { const struct pwm_lpss_boardinfo *info; }; -/* BayTrail */ -const struct pwm_lpss_boardinfo pwm_lpss_byt_info = { - .clk_rate = 25000000, - .npwm = 1, - .base_unit_bits = 16, -}; -EXPORT_SYMBOL_GPL(pwm_lpss_byt_info); - -/* Braswell */ -const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { - .clk_rate = 19200000, - .npwm = 1, - .base_unit_bits = 16, -}; -EXPORT_SYMBOL_GPL(pwm_lpss_bsw_info); - -/* Broxton */ -const struct pwm_lpss_boardinfo pwm_lpss_bxt_info = { - .clk_rate = 19200000, - .npwm = 4, - .base_unit_bits = 22, -}; -EXPORT_SYMBOL_GPL(pwm_lpss_bxt_info); - static inline struct pwm_lpss_chip *to_lpwm(struct pwm_chip *chip) { return container_of(chip, struct pwm_lpss_chip, chip); diff --git a/drivers/pwm/pwm-lpss.h b/drivers/pwm/pwm-lpss.h index 04766e0d41aa..c94cd7c2695d 100644 --- a/drivers/pwm/pwm-lpss.h +++ b/drivers/pwm/pwm-lpss.h @@ -24,10 +24,6 @@ struct pwm_lpss_boardinfo { unsigned long base_unit_bits; }; -extern const struct pwm_lpss_boardinfo pwm_lpss_byt_info; -extern const struct pwm_lpss_boardinfo pwm_lpss_bsw_info; -extern const struct pwm_lpss_boardinfo pwm_lpss_bxt_info; - struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev, struct resource *r, const struct pwm_lpss_boardinfo *info); int pwm_lpss_remove(struct pwm_lpss_chip *lpwm); -- cgit