From 83a98864ff62b23dfa93baeaaf340741e263c02b Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 2 May 2016 12:05:55 +0200 Subject: [PATCH 01/30] pwm: Add missing newline checkpatch requires that declarations be separated from code by a blank line. Add one for readability and to silence the warning. Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 7831bc6b51dd..bc0c20ac90a1 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -75,6 +75,7 @@ static void free_pwms(struct pwm_chip *chip) for (i = 0; i < chip->npwm; i++) { struct pwm_device *pwm = &chip->pwms[i]; + radix_tree_delete(&pwm_tree, pwm->pwm); } From 2907f8abb7ec3aec85ceaaf03dfbc16cca0018dc Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 2 May 2016 12:07:34 +0200 Subject: [PATCH 02/30] pwm: Use kcalloc() instead of kzalloc() kcalloc() should be preferred for allocations of arrays over kzalloc() with multiplication. Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index bc0c20ac90a1..2a1a2d566e39 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -255,7 +255,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, if (ret < 0) goto out; - chip->pwms = kzalloc(chip->npwm * sizeof(*pwm), GFP_KERNEL); + chip->pwms = kcalloc(chip->npwm, sizeof(*pwm), GFP_KERNEL); if (!chip->pwms) { ret = -ENOMEM; goto out; From fbd45a12988e75a48d392feb8b0e5feb5d612513 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Tue, 17 May 2016 14:27:25 +0200 Subject: [PATCH 03/30] pwm: Fix pwm_apply_args() call sites pwm_apply_args() is supposed to initialize a PWM device according to the arguments provided by the DT or the PWM lookup, but this function was called inside pwm_device_request(), which in turn was called before the core had a chance to initialize the pwm->args fields. Fix that by calling pwm_apply_args directly in pwm_get() and of_pwm_get() after initializing pwm->args field. This commit also fixes an invalid pointer dereference introduced by commit e39c0df1be5a ("pwm: Introduce the pwm_args concept"). Signed-off-by: Boris Brezillon Fixes: e39c0df1be5a ("pwm: Introduce the pwm_args concept") Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 680fbc795a0a..22cf3959041c 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -128,13 +128,6 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label) set_bit(PWMF_REQUESTED, &pwm->flags); pwm->label = label; - /* - * FIXME: This should be removed once all PWM users properly make use - * of struct pwm_args to initialize the PWM device. As long as this is - * here, the PWM state and hardware state can get out of sync. - */ - pwm_apply_args(pwm); - return 0; } @@ -627,6 +620,13 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id) pwm->label = con_id; + /* + * FIXME: This should be removed once all PWM users properly make use + * of struct pwm_args to initialize the PWM device. As long as this is + * here, the PWM state and hardware state can get out of sync. + */ + pwm_apply_args(pwm); + put: of_node_put(args.np); @@ -754,13 +754,20 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id) if (!chip) goto out; - pwm->args.period = chosen->period; - pwm->args.polarity = chosen->polarity; - pwm = pwm_request_from_chip(chip, chosen->index, con_id ?: dev_id); if (IS_ERR(pwm)) goto out; + pwm->args.period = chosen->period; + pwm->args.polarity = chosen->polarity; + + /* + * FIXME: This should be removed once all PWM users properly make use + * of struct pwm_args to initialize the PWM device. As long as this is + * here, the PWM state and hardware state can get out of sync. + */ + pwm_apply_args(pwm); + out: mutex_unlock(&pwm_lookup_lock); return pwm; From 3587c260906259632df07652743269627f408d85 Mon Sep 17 00:00:00 2001 From: Boris BREZILLON Date: Wed, 30 Mar 2016 22:03:24 +0200 Subject: [PATCH 04/30] pwm: rcar: Make use of pwm_is_enabled() Commit 5c31252c4a86 ("pwm: Add the pwm_is_enabled() helper") introduced a new function to test whether a PWM device is enabled or not without manipulating PWM internal fields. Hiding this is necessary if we want to smoothly move to the atomic PWM config approach without impacting PWM drivers. Fix this driver to use pwm_is_enabled() instead of directly accessing the ->flags field. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/pwm/pwm-rcar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c index 7b8ac0678137..1c85ecc9e7ac 100644 --- a/drivers/pwm/pwm-rcar.c +++ b/drivers/pwm/pwm-rcar.c @@ -157,7 +157,7 @@ static int rcar_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, return div; /* Let the core driver set pwm->period if disabled and duty_ns == 0 */ - if (!test_bit(PWMF_ENABLED, &pwm->flags) && !duty_ns) + if (!pwm_is_enabled(pwm) && !duty_ns) return 0; rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR); From 7f044b09b68d36811518c55f736a20648e8ed6e2 Mon Sep 17 00:00:00 2001 From: Boris BREZILLON Date: Wed, 30 Mar 2016 22:03:25 +0200 Subject: [PATCH 05/30] backlight: pwm_bl: Remove useless call to pwm_set_period() The PWM period will be set when calling pwm_config. Remove this useless call to pwm_set_period(), which might mess up the internal PWM state. Signed-off-by: Boris Brezillon Acked-by: Lee Jones Signed-off-by: Thierry Reding --- drivers/video/backlight/pwm_bl.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index 64f9e1b8655f..a33a290fc956 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -313,10 +313,8 @@ static int pwm_backlight_probe(struct platform_device *pdev) * via the PWM lookup table. */ pb->period = pwm_get_period(pb->pwm); - if (!pb->period && (data->pwm_period_ns > 0)) { + if (!pb->period && (data->pwm_period_ns > 0)) pb->period = data->pwm_period_ns; - pwm_set_period(pb->pwm, data->pwm_period_ns); - } pb->lth_brightness = data->lth_brightness * (pb->period / pb->scale); From 4ff66efd59102280a71d432c9f0e89cdaf66b849 Mon Sep 17 00:00:00 2001 From: Boris BREZILLON Date: Wed, 30 Mar 2016 22:03:26 +0200 Subject: [PATCH 06/30] backlight: lm3630a_bl: Stop messing with the pwm->period field pwm->period field is not supposed to be changed by PWM users. The only ones authorized to change it are the PWM core and PWM drivers. Signed-off-by: Boris Brezillon Acked-by: Lee Jones Signed-off-by: Thierry Reding --- drivers/video/backlight/lm3630a_bl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 35fe4825a454..3d16bd6e6d3f 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -162,7 +162,7 @@ static int lm3630a_intr_config(struct lm3630a_chip *pchip) static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max) { - unsigned int period = pwm_get_period(pchip->pwmd); + unsigned int period = pchip->pdata->pwm_period; unsigned int duty = br * period / br_max; pwm_config(pchip->pwmd, duty, period); @@ -425,7 +425,6 @@ static int lm3630a_probe(struct i2c_client *client, return PTR_ERR(pchip->pwmd); } } - pchip->pwmd->period = pdata->pwm_period; /* interrupt enable : irq 0 is not allowed */ pchip->irq = client->irq; From 459a25afe97cb3d7f978b90c881f4d7aac8fb755 Mon Sep 17 00:00:00 2001 From: Boris BREZILLON Date: Wed, 30 Mar 2016 22:03:27 +0200 Subject: [PATCH 07/30] pwm: Get rid of pwm->lock PWM devices are not protected against concurrent accesses. The lock in struct pwm_device might let PWM users think it is, but it's actually only protecting the enabled state. Removing this lock should be fine as long as all PWM users are aware that accesses to the PWM device have to be serialized, which seems to be the case for all of them except the sysfs interface. Patch the sysfs code by adding a lock to the pwm_export struct and making sure it's taken for all relevant accesses to the exported PWM device. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 19 ++++--------------- drivers/pwm/sysfs.c | 24 ++++++++++++++++++++---- include/linux/pwm.h | 2 -- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 22cf3959041c..cb762cf51332 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -269,7 +269,6 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, pwm->pwm = chip->base + i; pwm->hwpwm = i; pwm->polarity = polarity; - mutex_init(&pwm->lock); radix_tree_insert(&pwm_tree, pwm->pwm, pwm); } @@ -474,22 +473,16 @@ int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity) if (!pwm->chip->ops->set_polarity) return -ENOSYS; - mutex_lock(&pwm->lock); - - if (pwm_is_enabled(pwm)) { - err = -EBUSY; - goto unlock; - } + if (pwm_is_enabled(pwm)) + return -EBUSY; err = pwm->chip->ops->set_polarity(pwm->chip, pwm, polarity); if (err) - goto unlock; + return err; pwm->polarity = polarity; -unlock: - mutex_unlock(&pwm->lock); - return err; + return 0; } EXPORT_SYMBOL_GPL(pwm_set_polarity); @@ -506,16 +499,12 @@ int pwm_enable(struct pwm_device *pwm) if (!pwm) return -EINVAL; - mutex_lock(&pwm->lock); - if (!test_and_set_bit(PWMF_ENABLED, &pwm->flags)) { err = pwm->chip->ops->enable(pwm->chip, pwm); if (err) clear_bit(PWMF_ENABLED, &pwm->flags); } - mutex_unlock(&pwm->lock); - return err; } EXPORT_SYMBOL_GPL(pwm_enable); diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c index 9c90886f4123..187ca0875cf6 100644 --- a/drivers/pwm/sysfs.c +++ b/drivers/pwm/sysfs.c @@ -26,6 +26,7 @@ struct pwm_export { struct device child; struct pwm_device *pwm; + struct mutex lock; }; static struct pwm_export *child_to_pwm_export(struct device *child) @@ -53,7 +54,8 @@ static ssize_t period_store(struct device *child, struct device_attribute *attr, const char *buf, size_t size) { - struct pwm_device *pwm = child_to_pwm_device(child); + struct pwm_export *export = child_to_pwm_export(child); + struct pwm_device *pwm = export->pwm; unsigned int val; int ret; @@ -61,7 +63,9 @@ static ssize_t period_store(struct device *child, if (ret) return ret; + mutex_lock(&export->lock); ret = pwm_config(pwm, pwm_get_duty_cycle(pwm), val); + mutex_unlock(&export->lock); return ret ? : size; } @@ -79,7 +83,8 @@ static ssize_t duty_cycle_store(struct device *child, struct device_attribute *attr, const char *buf, size_t size) { - struct pwm_device *pwm = child_to_pwm_device(child); + struct pwm_export *export = child_to_pwm_export(child); + struct pwm_device *pwm = export->pwm; unsigned int val; int ret; @@ -87,7 +92,9 @@ static ssize_t duty_cycle_store(struct device *child, if (ret) return ret; + mutex_lock(&export->lock); ret = pwm_config(pwm, val, pwm_get_period(pwm)); + mutex_unlock(&export->lock); return ret ? : size; } @@ -105,13 +112,16 @@ static ssize_t enable_store(struct device *child, struct device_attribute *attr, const char *buf, size_t size) { - struct pwm_device *pwm = child_to_pwm_device(child); + struct pwm_export *export = child_to_pwm_export(child); + struct pwm_device *pwm = export->pwm; int val, ret; ret = kstrtoint(buf, 0, &val); if (ret) return ret; + mutex_lock(&export->lock); + switch (val) { case 0: pwm_disable(pwm); @@ -124,6 +134,8 @@ static ssize_t enable_store(struct device *child, break; } + mutex_unlock(&export->lock); + return ret ? : size; } @@ -151,7 +163,8 @@ static ssize_t polarity_store(struct device *child, struct device_attribute *attr, const char *buf, size_t size) { - struct pwm_device *pwm = child_to_pwm_device(child); + struct pwm_export *export = child_to_pwm_export(child); + struct pwm_device *pwm = export->pwm; enum pwm_polarity polarity; int ret; @@ -162,7 +175,9 @@ static ssize_t polarity_store(struct device *child, else return -EINVAL; + mutex_lock(&export->lock); ret = pwm_set_polarity(pwm, polarity); + mutex_unlock(&export->lock); return ret ? : size; } @@ -203,6 +218,7 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm) } export->pwm = pwm; + mutex_init(&export->lock); export->child.release = pwm_export_release; export->child.parent = parent; diff --git a/include/linux/pwm.h b/include/linux/pwm.h index b78d27c42629..d2e7430ccedb 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -106,7 +106,6 @@ enum { * @pwm: global index of the PWM device * @chip: PWM chip providing this PWM device * @chip_data: chip-private data associated with the PWM device - * @lock: used to serialize accesses to the PWM device where necessary * @period: period of the PWM signal (in nanoseconds) * @duty_cycle: duty cycle of the PWM signal (in nanoseconds) * @polarity: polarity of the PWM signal @@ -119,7 +118,6 @@ struct pwm_device { unsigned int pwm; struct pwm_chip *chip; void *chip_data; - struct mutex lock; unsigned int period; unsigned int duty_cycle; From 4b58896f72176b781816a9e14dcea5f755b19b5c Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:22 +0200 Subject: [PATCH 08/30] pwm: Use pwm_get/set_xxx() helpers where appropriate Use pwm_get/set_xxx() helpers instead of directly accessing the pwm->xxx field. Doing that will ease adaptation of the PWM framework to support atomic update. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/pwm/pwm-crc.c | 2 +- drivers/pwm/pwm-lpc18xx-sct.c | 2 +- drivers/pwm/pwm-omap-dmtimer.c | 2 +- drivers/pwm/pwm-sun4i.c | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c index 7101c7020bf4..bd0ebd04856a 100644 --- a/drivers/pwm/pwm-crc.c +++ b/drivers/pwm/pwm-crc.c @@ -75,7 +75,7 @@ static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm, return -EINVAL; } - if (pwm->period != period_ns) { + if (pwm_get_period(pwm) != period_ns) { int clk_div; /* changing the clk divisor, need to disable fisrt */ diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c index 9861fed4e67d..19dc64cab2f0 100644 --- a/drivers/pwm/pwm-lpc18xx-sct.c +++ b/drivers/pwm/pwm-lpc18xx-sct.c @@ -249,7 +249,7 @@ static int lpc18xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) LPC18XX_PWM_EVSTATEMSK(lpc18xx_data->duty_event), LPC18XX_PWM_EVSTATEMSK_ALL); - if (pwm->polarity == PWM_POLARITY_NORMAL) { + if (pwm_get_polarity(pwm) == PWM_POLARITY_NORMAL) { set_event = lpc18xx_pwm->period_event; clear_event = lpc18xx_data->duty_event; res_action = LPC18XX_PWM_RES_SET; diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index b7e6ecba7d5c..3e95090cd7cf 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -192,7 +192,7 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip, load_value, load_value, match_value, match_value); omap->pdata->set_pwm(omap->dm_timer, - pwm->polarity == PWM_POLARITY_INVERSED, + pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED, true, PWM_OMAP_DMTIMER_TRIGGER_OVERFLOW_AND_COMPARE); diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 67af9f62361f..03a99a53c39e 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -354,7 +354,8 @@ static int sun4i_pwm_probe(struct platform_device *pdev) val = sun4i_pwm_readl(pwm, PWM_CTRL_REG); for (i = 0; i < pwm->chip.npwm; i++) if (!(val & BIT_CH(PWM_ACT_STATE, i))) - pwm->chip.pwms[i].polarity = PWM_POLARITY_INVERSED; + pwm_set_polarity(&pwm->chip.pwms[i], + PWM_POLARITY_INVERSED); clk_disable_unprepare(pwm->clk); return 0; From dd0b38b7ca0d8c8aadcf8a17d7c90d36ab8ab6e4 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:23 +0200 Subject: [PATCH 09/30] clk: pwm: Use pwm_get_args() where appropriate The PWM framework has clarified the concept of reference PWM config (the platform dependent config retrieved from the DT or the PWM lookup table) and real PWM state. Use pwm_get_args() when the PWM user wants to retrieve this reference config and not the current state. This is part of the rework allowing the PWM framework to support hardware readout and expose real PWM state even when the PWM has just been requested (before the user calls pwm_config/enable/disable()). Signed-off-by: Boris Brezillon Acked-by: Stephen Boyd Signed-off-by: Thierry Reding --- drivers/clk/clk-pwm.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c index 883045814dac..1630a1f085f7 100644 --- a/drivers/clk/clk-pwm.c +++ b/drivers/clk/clk-pwm.c @@ -59,6 +59,7 @@ static int clk_pwm_probe(struct platform_device *pdev) struct clk_init_data init; struct clk_pwm *clk_pwm; struct pwm_device *pwm; + struct pwm_args pargs; const char *clk_name; struct clk *clk; int ret; @@ -71,22 +72,28 @@ static int clk_pwm_probe(struct platform_device *pdev) if (IS_ERR(pwm)) return PTR_ERR(pwm); - if (!pwm->period) { + pwm_get_args(pwm, &pargs); + if (!pargs.period) { dev_err(&pdev->dev, "invalid PWM period\n"); return -EINVAL; } if (of_property_read_u32(node, "clock-frequency", &clk_pwm->fixed_rate)) - clk_pwm->fixed_rate = NSEC_PER_SEC / pwm->period; + clk_pwm->fixed_rate = NSEC_PER_SEC / pargs.period; - if (pwm->period != NSEC_PER_SEC / clk_pwm->fixed_rate && - pwm->period != DIV_ROUND_UP(NSEC_PER_SEC, clk_pwm->fixed_rate)) { + if (pargs.period != NSEC_PER_SEC / clk_pwm->fixed_rate && + pargs.period != DIV_ROUND_UP(NSEC_PER_SEC, clk_pwm->fixed_rate)) { dev_err(&pdev->dev, "clock-frequency does not match PWM period\n"); return -EINVAL; } - ret = pwm_config(pwm, (pwm->period + 1) >> 1, pwm->period); + /* + * FIXME: pwm_apply_args() should be removed when switching to the + * atomic PWM API. + */ + pwm_apply_args(pwm); + ret = pwm_config(pwm, (pargs.period + 1) >> 1, pargs.period); if (ret < 0) return ret; From 2289711c9d4d588954ff86a06685f1579bf6c446 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:24 +0200 Subject: [PATCH 10/30] hwmon: pwm-fan: Use pwm_get_args() where appropriate The PWM framework has clarified the concept of reference PWM config (the platform dependent config retrieved from the DT or the PWM lookup table) and real PWM state. Use pwm_get_args() when the PWM user wants to retrieve this reference config and not the current state. This is part of the rework allowing the PWM framework to support hardware readout and expose real PWM state even when the PWM has just been requested (before the user calls pwm_config/enable/disable()). Signed-off-by: Boris Brezillon Acked-by: Kamil Debski Signed-off-by: Thierry Reding --- drivers/hwmon/pwm-fan.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index 3e23003f78b0..f9af3935b427 100644 --- a/drivers/hwmon/pwm-fan.c +++ b/drivers/hwmon/pwm-fan.c @@ -40,15 +40,18 @@ struct pwm_fan_ctx { static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm) { + struct pwm_args pargs; unsigned long duty; int ret = 0; + pwm_get_args(ctx->pwm, &pargs); + mutex_lock(&ctx->lock); if (ctx->pwm_value == pwm) goto exit_set_pwm_err; - duty = DIV_ROUND_UP(pwm * (ctx->pwm->period - 1), MAX_PWM); - ret = pwm_config(ctx->pwm, duty, ctx->pwm->period); + duty = DIV_ROUND_UP(pwm * (pargs.period - 1), MAX_PWM); + ret = pwm_config(ctx->pwm, duty, pargs.period); if (ret) goto exit_set_pwm_err; @@ -215,6 +218,7 @@ static int pwm_fan_probe(struct platform_device *pdev) { struct thermal_cooling_device *cdev; struct pwm_fan_ctx *ctx; + struct pwm_args pargs; struct device *hwmon; int duty_cycle; int ret; @@ -233,11 +237,19 @@ static int pwm_fan_probe(struct platform_device *pdev) platform_set_drvdata(pdev, ctx); + /* + * FIXME: pwm_apply_args() should be removed when switching to the + * atomic PWM API. + */ + pwm_apply_args(ctx->pwm); + /* Set duty cycle to maximum allowed */ - duty_cycle = ctx->pwm->period - 1; + pwm_get_args(ctx->pwm, &pargs); + + duty_cycle = pargs.period - 1; ctx->pwm_value = MAX_PWM; - ret = pwm_config(ctx->pwm, duty_cycle, ctx->pwm->period); + ret = pwm_config(ctx->pwm, duty_cycle, pargs.period); if (ret) { dev_err(&pdev->dev, "Failed to configure PWM\n"); return ret; @@ -303,14 +315,16 @@ static int pwm_fan_suspend(struct device *dev) static int pwm_fan_resume(struct device *dev) { struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); + struct pwm_args pargs; unsigned long duty; int ret; if (ctx->pwm_value == 0) return 0; - duty = DIV_ROUND_UP(ctx->pwm_value * (ctx->pwm->period - 1), MAX_PWM); - ret = pwm_config(ctx->pwm, duty, ctx->pwm->period); + pwm_get_args(ctx->pwm, &pargs); + duty = DIV_ROUND_UP(ctx->pwm_value * (pargs.period - 1), MAX_PWM); + ret = pwm_config(ctx->pwm, duty, pargs.period); if (ret) return ret; return pwm_enable(ctx->pwm); From ad4e9fec143c453cae145d7d5ccac902c37fb912 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:25 +0200 Subject: [PATCH 11/30] input: misc: max77693: Use pwm_get_args() where appropriate The PWM framework has clarified the concept of reference PWM config (the platform dependent config retrieved from the DT or the PWM lookup table) and real PWM state. Use pwm_get_args() when the PWM user wants to retrieve this reference config and not the current state. This is part of the rework allowing the PWM framework to support hardware readout and expose real PWM state even when the PWM has just been requested (before the user calls pwm_config/enable/disable()). Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/input/misc/max77693-haptic.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/input/misc/max77693-haptic.c b/drivers/input/misc/max77693-haptic.c index 6d96bff32a0e..29ddeb7be84b 100644 --- a/drivers/input/misc/max77693-haptic.c +++ b/drivers/input/misc/max77693-haptic.c @@ -70,10 +70,13 @@ struct max77693_haptic { static int max77693_haptic_set_duty_cycle(struct max77693_haptic *haptic) { - int delta = (haptic->pwm_dev->period + haptic->pwm_duty) / 2; + struct pwm_args pargs; + int delta; int error; - error = pwm_config(haptic->pwm_dev, delta, haptic->pwm_dev->period); + pwm_get_args(haptic->pwm_dev, &pargs); + delta = (pargs.period + haptic->pwm_duty) / 2; + error = pwm_config(haptic->pwm_dev, delta, pargs.period); if (error) { dev_err(haptic->dev, "failed to configure pwm: %d\n", error); return error; @@ -234,6 +237,7 @@ static int max77693_haptic_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect) { struct max77693_haptic *haptic = input_get_drvdata(dev); + struct pwm_args pargs; u64 period_mag_multi; haptic->magnitude = effect->u.rumble.strong_magnitude; @@ -245,7 +249,8 @@ static int max77693_haptic_play_effect(struct input_dev *dev, void *data, * The formula to convert magnitude to pwm_duty as follows: * - pwm_duty = (magnitude * pwm_period) / MAX_MAGNITUDE(0xFFFF) */ - period_mag_multi = (u64)haptic->pwm_dev->period * haptic->magnitude; + pwm_get_args(haptic->pwm_dev, &pargs); + period_mag_multi = (u64)pargs.period * haptic->magnitude; haptic->pwm_duty = (unsigned int)(period_mag_multi >> MAX_MAGNITUDE_SHIFT); @@ -329,6 +334,12 @@ static int max77693_haptic_probe(struct platform_device *pdev) return PTR_ERR(haptic->pwm_dev); } + /* + * FIXME: pwm_apply_args() should be removed when switching to the + * atomic PWM API. + */ + pwm_apply_args(haptic->pwm_dev); + haptic->motor_reg = devm_regulator_get(&pdev->dev, "haptic"); if (IS_ERR(haptic->motor_reg)) { dev_err(&pdev->dev, "failed to get regulator\n"); From 1b50673dfa6486a69895bd45bb5eeaaede0a2b93 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:26 +0200 Subject: [PATCH 12/30] leds: pwm: Use pwm_get_args() where appropriate The PWM framework has clarified the concept of reference PWM config (the platform dependent config retrieved from the DT or the PWM lookup table) and real PWM state. Use pwm_get_args() when the PWM user wants to retrieve this reference config and not the current state. This is part of the rework allowing the PWM framework to support hardware readout and expose real PWM state even when the PWM has just been requested (before the user calls pwm_config/enable/disable()). Signed-off-by: Boris Brezillon Acked-by: Jacek Anaszewski Signed-off-by: Thierry Reding --- drivers/leds/leds-pwm.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c index 4783bacb2e9d..a9145aa7f36a 100644 --- a/drivers/leds/leds-pwm.c +++ b/drivers/leds/leds-pwm.c @@ -91,6 +91,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, struct led_pwm *led, struct device_node *child) { struct led_pwm_data *led_data = &priv->leds[priv->num_leds]; + struct pwm_args pargs; int ret; led_data->active_low = led->active_low; @@ -117,7 +118,15 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, else led_data->cdev.brightness_set_blocking = led_pwm_set_blocking; - led_data->period = pwm_get_period(led_data->pwm); + /* + * FIXME: pwm_apply_args() should be removed when switching to the + * atomic PWM API. + */ + pwm_apply_args(led_data->pwm); + + pwm_get_args(led_data->pwm, &pargs); + + led_data->period = pargs.period; if (!led_data->period && (led->pwm_period_ns > 0)) led_data->period = led->pwm_period_ns; From 6bc24526f8010f208a243cc73df4c6cc48fa18a4 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:27 +0200 Subject: [PATCH 13/30] regulator: pwm: Use pwm_get_args() where appropriate The PWM framework has clarified the concept of reference PWM config (the platform dependent config retrieved from the DT or the PWM lookup table) and real PWM state. Use pwm_get_args() when the PWM user wants to retrieve this reference config and not the current state. This is part of the rework allowing the PWM framework to support hardware readout and expose real PWM state even when the PWM has just been requested (before the user calls pwm_config/enable/disable()). Signed-off-by: Boris Brezillon Acked-by: Mark Brown Signed-off-by: Thierry Reding --- drivers/regulator/pwm-regulator.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c index 4689d62f4841..ffdb895ace0a 100644 --- a/drivers/regulator/pwm-regulator.c +++ b/drivers/regulator/pwm-regulator.c @@ -59,16 +59,16 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev, unsigned selector) { struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev); - unsigned int pwm_reg_period; + struct pwm_args pargs; int dutycycle; int ret; - pwm_reg_period = pwm_get_period(drvdata->pwm); + pwm_get_args(drvdata->pwm, &pargs); - dutycycle = (pwm_reg_period * + dutycycle = (pargs.period * drvdata->duty_cycle_table[selector].dutycycle) / 100; - ret = pwm_config(drvdata->pwm, dutycycle, pwm_reg_period); + ret = pwm_config(drvdata->pwm, dutycycle, pargs.period); if (ret) { dev_err(&rdev->dev, "Failed to configure PWM\n"); return ret; @@ -138,13 +138,15 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev, { struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev); unsigned int ramp_delay = rdev->constraints->ramp_delay; - unsigned int period = pwm_get_period(drvdata->pwm); + struct pwm_args pargs; int duty_cycle; int ret; + pwm_get_args(drvdata->pwm, &pargs); duty_cycle = pwm_voltage_to_duty_cycle_percentage(rdev, min_uV); - ret = pwm_config(drvdata->pwm, (period / 100) * duty_cycle, period); + ret = pwm_config(drvdata->pwm, (pargs.period / 100) * duty_cycle, + pargs.period); if (ret) { dev_err(&rdev->dev, "Failed to configure PWM\n"); return ret; @@ -281,6 +283,12 @@ static int pwm_regulator_probe(struct platform_device *pdev) return PTR_ERR(drvdata->pwm); } + /* + * FIXME: pwm_apply_args() should be removed when switching to the + * atomic PWM API. + */ + pwm_apply_args(drvdata->pwm); + regulator = devm_regulator_register(&pdev->dev, &drvdata->desc, &config); if (IS_ERR(regulator)) { From 717c18f0e4653bbb74a164e88e5b7ccc5db7684b Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:28 +0200 Subject: [PATCH 14/30] fbdev: ssd1307fb: Use pwm_get_args() where appropriate The PWM framework has clarified the concept of reference PWM config (the platform dependent config retrieved from the DT or the PWM lookup table) and real PWM state. Use pwm_get_args() when the PWM user wants to retrieve this reference config and not the current state. This is part of the rework allowing the PWM framework to support hardware readout and expose real PWM state even when the PWM has just been requested (before the user calls pwm_config/enable/disable()). Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/video/fbdev/ssd1307fb.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c index fa3480815cdb..652f68880d60 100644 --- a/drivers/video/fbdev/ssd1307fb.c +++ b/drivers/video/fbdev/ssd1307fb.c @@ -286,6 +286,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par) { int ret; u32 precharge, dclk, com_invdir, compins; + struct pwm_args pargs; if (par->device_info->need_pwm) { par->pwm = pwm_get(&par->client->dev, NULL); @@ -294,7 +295,15 @@ static int ssd1307fb_init(struct ssd1307fb_par *par) return PTR_ERR(par->pwm); } - par->pwm_period = pwm_get_period(par->pwm); + /* + * FIXME: pwm_apply_args() should be removed when switching to + * the atomic PWM API. + */ + pwm_apply_args(par->pwm); + + pwm_get_args(par->pwm, &pargs); + + par->pwm_period = pargs.period; /* Enable the PWM */ pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period); pwm_enable(par->pwm); From 6cb9644db7364ff5d2980ccd365b8cb684145327 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:29 +0200 Subject: [PATCH 15/30] backlight: pwm_bl: Use pwm_get_args() where appropriate The PWM framework has clarified the concept of reference PWM config (the platform dependent config retrieved from the DT or the PWM lookup table) and real PWM state. Use pwm_get_args() when the PWM user wants to retrieve this reference config and not the current state. This is part of the rework allowing the PWM framework to support hardware readout and expose real PWM state even when the PWM has just been requested (before the user calls pwm_config/enable/disable()). Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/video/backlight/pwm_bl.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index a33a290fc956..b2b366bb0f97 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -201,6 +201,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) struct device_node *node = pdev->dev.of_node; struct pwm_bl_data *pb; int initial_blank = FB_BLANK_UNBLANK; + struct pwm_args pargs; int ret; if (!data) { @@ -306,13 +307,20 @@ static int pwm_backlight_probe(struct platform_device *pdev) dev_dbg(&pdev->dev, "got pwm for backlight\n"); + /* + * FIXME: pwm_apply_args() should be removed when switching to + * the atomic PWM API. + */ + pwm_apply_args(pb->pwm); + /* * The DT case will set the pwm_period_ns field to 0 and store the * period, parsed from the DT, in the PWM device. For the non-DT case, * set the period from platform data if it has not already been set * via the PWM lookup table. */ - pb->period = pwm_get_period(pb->pwm); + pwm_get_args(pb->pwm, &pargs); + pb->period = pargs.period; if (!pb->period && (data->pwm_period_ns > 0)) pb->period = data->pwm_period_ns; From 058be4d6c67d9fc043623cb26e4abe5608755d65 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:30 +0200 Subject: [PATCH 16/30] backlight: lp8788: Explicitly apply PWM config extracted from pwm_args Call pwm_apply_args() just after requesting the PWM device so that the polarity and period are initialized according to the information provided in pwm_args. This is an intermediate state, and pwm_apply_args() should be dropped as soon as the atomic PWM infrastructure is in place and the driver makes use of it. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/video/backlight/lp8788_bl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/video/backlight/lp8788_bl.c b/drivers/video/backlight/lp8788_bl.c index 5d583d7a517b..cf869ec90cce 100644 --- a/drivers/video/backlight/lp8788_bl.c +++ b/drivers/video/backlight/lp8788_bl.c @@ -145,6 +145,12 @@ static void lp8788_pwm_ctrl(struct lp8788_bl *bl, int br, int max_br) } bl->pwm = pwm; + + /* + * FIXME: pwm_apply_args() should be removed when switching to + * the atomic PWM API. + */ + pwm_apply_args(pwm); } pwm_config(bl->pwm, duty, period); From 6fbab835a973cee2e6a7742bbc990055b24de5cd Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:31 +0200 Subject: [PATCH 17/30] backlight: lp855x: Explicitly apply PWM config extracted from pwm_args Call pwm_apply_args() just after requesting the PWM device so that the polarity and period are initialized according to the information provided in pwm_args. This is an intermediate state, and pwm_apply_args() should be dropped as soon as the atomic PWM infrastructure is in place and the driver makes use of it. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/video/backlight/lp855x_bl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c index daca9e6a2bb3..e5b14f52628f 100644 --- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -246,6 +246,12 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) return; lp->pwm = pwm; + + /* + * FIXME: pwm_apply_args() should be removed when switching to + * the atomic PWM API. + */ + pwm_apply_args(pwm); } pwm_config(lp->pwm, duty, period); From 7ff666bc4b821650a03cadeb4108c77328c98c0a Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:32 +0200 Subject: [PATCH 18/30] backlight: lm3630a: explicitly apply PWM config extracted from pwm_args Call pwm_apply_args() just after requesting the PWM device so that the polarity and period are initialized according to the information provided in pwm_args. This is an intermediate state, and pwm_apply_args() should be dropped as soon as the atomic PWM infrastructure is in place and the driver makes use of it. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/video/backlight/lm3630a_bl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 3d16bd6e6d3f..60d6c2ac87aa 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -424,6 +424,12 @@ static int lm3630a_probe(struct i2c_client *client, dev_err(&client->dev, "fail : get pwm device\n"); return PTR_ERR(pchip->pwmd); } + + /* + * FIXME: pwm_apply_args() should be removed when switching to + * the atomic PWM API. + */ + pwm_apply_args(pchip->pwmd); } /* interrupt enable : irq 0 is not allowed */ From de93170b85d7ee2a6a6a881c019160a65adeb636 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:33 +0200 Subject: [PATCH 19/30] input: misc: max8997: Explicitly apply PWM config extracted from pwm_args Call pwm_apply_args() just after requesting the PWM device so that the polarity and period are initialized according to the information provided in pwm_args. This is an intermediate state, and pwm_apply_args() should be dropped as soon as the atomic PWM infrastructure is in place and the driver makes use of it. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/input/misc/max8997_haptic.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/input/misc/max8997_haptic.c b/drivers/input/misc/max8997_haptic.c index a806ba3818f7..bf17f654ed88 100644 --- a/drivers/input/misc/max8997_haptic.c +++ b/drivers/input/misc/max8997_haptic.c @@ -304,6 +304,12 @@ static int max8997_haptic_probe(struct platform_device *pdev) error); goto err_free_mem; } + + /* + * FIXME: pwm_apply_args() should be removed when switching to + * the atomic PWM API. + */ + pwm_apply_args(chip->pwm); break; default: From cfae56f18c7cbdb0542025b0d3f000d9ba0152d0 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:34 +0200 Subject: [PATCH 20/30] input: misc: pwm-beeper: Explicitly apply PWM config extracted from pwm_args Call pwm_apply_args() just after requesting the PWM device so that the polarity and period are initialized according to the information provided in pwm_args. This is an intermediate state, and pwm_apply_args() should be dropped as soon as the atomic PWM infrastructure is in place and the driver makes use of it. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/input/misc/pwm-beeper.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c index f2261ab54701..8d7133268745 100644 --- a/drivers/input/misc/pwm-beeper.c +++ b/drivers/input/misc/pwm-beeper.c @@ -87,6 +87,12 @@ static int pwm_beeper_probe(struct platform_device *pdev) goto err_free; } + /* + * FIXME: pwm_apply_args() should be removed when switching to + * the atomic PWM API. + */ + pwm_apply_args(beeper->pwm); + beeper->input = input_allocate_device(); if (!beeper->input) { dev_err(&pdev->dev, "Failed to allocate input device\n"); From 2347aa7c34be284b266ab6b5f2bdf5daf14f582e Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:35 +0200 Subject: [PATCH 21/30] drm: i915: Explicitly apply PWM config extracted from pwm_args Call pwm_apply_args() just after requesting the PWM device so that the polarity and period are initialized according to the information provided in pwm_args. This is an intermediate state, and pwm_apply_args() should be dropped as soon as the atomic PWM infrastructure is in place and the driver makes use of it. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/gpu/drm/i915/intel_panel.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 21ee6477bf98..dc3a2e4f74c2 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1640,6 +1640,12 @@ static int pwm_setup_backlight(struct intel_connector *connector, return -ENODEV; } + /* + * FIXME: pwm_apply_args() should be removed when switching to + * the atomic PWM API. + */ + pwm_apply_args(panel->backlight.pwm); + retval = pwm_config(panel->backlight.pwm, CRC_PMIC_PWM_PERIOD_NS, CRC_PMIC_PWM_PERIOD_NS); if (retval < 0) { From 102232c54601071304cb6946f9758e3cd74d3e1e Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:36 +0200 Subject: [PATCH 22/30] ARM: Explicitly apply PWM config extracted from pwm_args Call pwm_apply_args() just after requesting the PWM device so that the polarity and period are initialized according to the information provided in pwm_args. This is an intermediate state, and pwm_apply_args() should be dropped as soon as the atomic PWM infrastructure is in place and the driver makes use of it. Signed-off-by: Boris Brezillon Acked-by: Krzysztof Kozlowski Signed-off-by: Thierry Reding --- arch/arm/mach-s3c24xx/mach-rx1950.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/mach-s3c24xx/mach-rx1950.c b/arch/arm/mach-s3c24xx/mach-rx1950.c index 774c982a7b7e..25a139bb9826 100644 --- a/arch/arm/mach-s3c24xx/mach-rx1950.c +++ b/arch/arm/mach-s3c24xx/mach-rx1950.c @@ -496,6 +496,12 @@ static int rx1950_backlight_init(struct device *dev) return PTR_ERR(lcd_pwm); } + /* + * FIXME: pwm_apply_args() should be removed when switching to + * the atomic PWM API. + */ + pwm_apply_args(lcd_pwm); + rx1950_lcd_power(1); rx1950_bl_power(1); From a8c3862551e063344f80c3e05d595f9d8836f355 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:37 +0200 Subject: [PATCH 23/30] pwm: Keep PWM state in sync with hardware state Before the introduction of pwm_args, the core was resetting the PWM period and polarity states to the reference values (those provided through the DT, a PWM lookup table or hardcoded in the driver). Now that all PWM users are correctly using pwm_args to configure their PWM device, we can safely remove the pwm_apply_args() call in pwm_get() and of_pwm_get(). We can also get rid of the pwm_set_period() call in pwm_apply_args(), because PWM users are now directly using pargs->period instead of pwm_get_period(). By doing that we avoid messing with the current PWM period. The only remaining bit in pwm_apply_args() is the initial polarity setting, and it should go away when all PWM users have been patched to use the atomic API (with this API the polarity will be set along with other PWM arguments when configuring the PWM). Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 14 -------------- include/linux/pwm.h | 1 - 2 files changed, 15 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index cb762cf51332..64330595e17b 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -609,13 +609,6 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id) pwm->label = con_id; - /* - * FIXME: This should be removed once all PWM users properly make use - * of struct pwm_args to initialize the PWM device. As long as this is - * here, the PWM state and hardware state can get out of sync. - */ - pwm_apply_args(pwm); - put: of_node_put(args.np); @@ -750,13 +743,6 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id) pwm->args.period = chosen->period; pwm->args.polarity = chosen->polarity; - /* - * FIXME: This should be removed once all PWM users properly make use - * of struct pwm_args to initialize the PWM device. As long as this is - * here, the PWM state and hardware state can get out of sync. - */ - pwm_apply_args(pwm); - out: mutex_unlock(&pwm_lookup_lock); return pwm; diff --git a/include/linux/pwm.h b/include/linux/pwm.h index d2e7430ccedb..7caf549f720e 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -171,7 +171,6 @@ static inline void pwm_get_args(const struct pwm_device *pwm, static inline void pwm_apply_args(struct pwm_device *pwm) { - pwm_set_period(pwm, pwm->args.period); pwm_set_polarity(pwm, pwm->args.polarity); } From 43a276b003ed2e03de9d94b02a1ba49c1849b931 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:38 +0200 Subject: [PATCH 24/30] pwm: Introduce the pwm_state concept The PWM state, represented by its period, duty_cycle and polarity is currently directly stored in the PWM device. Declare a pwm_state structure embedding those field so that we can later use this struct to atomically update all the PWM parameters at once. All pwm_get_xxx() helpers are now implemented as wrappers around pwm_get_state(). Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 8 +++---- include/linux/pwm.h | 54 +++++++++++++++++++++++++++++++++++---------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 64330595e17b..f3f91e716a42 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, pwm->chip = chip; pwm->pwm = chip->base + i; pwm->hwpwm = i; - pwm->polarity = polarity; + pwm->state.polarity = polarity; radix_tree_insert(&pwm_tree, pwm->pwm, pwm); } @@ -446,8 +446,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) if (err) return err; - pwm->duty_cycle = duty_ns; - pwm->period = period_ns; + pwm->state.duty_cycle = duty_ns; + pwm->state.period = period_ns; return 0; } @@ -480,7 +480,7 @@ int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity) if (err) return err; - pwm->polarity = polarity; + pwm->state.polarity = polarity; return 0; } diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 7caf549f720e..51d4005418f9 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -98,6 +98,18 @@ enum { PWMF_EXPORTED = 1 << 2, }; +/* + * struct pwm_state - state of a PWM channel + * @period: PWM period (in nanoseconds) + * @duty_cycle: PWM duty cycle (in nanoseconds) + * @polarity: PWM polarity + */ +struct pwm_state { + unsigned int period; + unsigned int duty_cycle; + enum pwm_polarity polarity; +}; + /** * struct pwm_device - PWM channel object * @label: name of the PWM device @@ -106,10 +118,8 @@ enum { * @pwm: global index of the PWM device * @chip: PWM chip providing this PWM device * @chip_data: chip-private data associated with the PWM device - * @period: period of the PWM signal (in nanoseconds) - * @duty_cycle: duty cycle of the PWM signal (in nanoseconds) - * @polarity: polarity of the PWM signal * @args: PWM arguments + * @state: curent PWM channel state */ struct pwm_device { const char *label; @@ -119,13 +129,21 @@ struct pwm_device { struct pwm_chip *chip; void *chip_data; - unsigned int period; - unsigned int duty_cycle; - enum pwm_polarity polarity; - struct pwm_args args; + struct pwm_state state; }; +/** + * pwm_get_state() - retrieve the current PWM state + * @pwm: PWM device + * @state: state to fill with the current PWM state + */ +static inline void pwm_get_state(const struct pwm_device *pwm, + struct pwm_state *state) +{ + *state = pwm->state; +} + static inline bool pwm_is_enabled(const struct pwm_device *pwm) { return test_bit(PWMF_ENABLED, &pwm->flags); @@ -134,23 +152,31 @@ static inline bool pwm_is_enabled(const struct pwm_device *pwm) static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period) { if (pwm) - pwm->period = period; + pwm->state.period = period; } static inline unsigned int pwm_get_period(const struct pwm_device *pwm) { - return pwm ? pwm->period : 0; + struct pwm_state state; + + pwm_get_state(pwm, &state); + + return state.period; } static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty) { if (pwm) - pwm->duty_cycle = duty; + pwm->state.duty_cycle = duty; } static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm) { - return pwm ? pwm->duty_cycle : 0; + struct pwm_state state; + + pwm_get_state(pwm, &state); + + return state.duty_cycle; } /* @@ -160,7 +186,11 @@ int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity); static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm) { - return pwm ? pwm->polarity : PWM_POLARITY_NORMAL; + struct pwm_state state; + + pwm_get_state(pwm, &state); + + return state.polarity; } static inline void pwm_get_args(const struct pwm_device *pwm, From 09a7e4a3d9fcb95ade2cb02167e85fbeb8315ce0 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:39 +0200 Subject: [PATCH 25/30] pwm: Move the enabled/disabled info into pwm_state Prepare the transition to PWM atomic update by moving the enabled and disabled state into the pwm_state struct. This way we can easily update the whole PWM state by copying the new state in the ->state field. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 13 +++++++++---- include/linux/pwm.h | 11 ++++++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index f3f91e716a42..c240b5437145 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -499,10 +499,10 @@ int pwm_enable(struct pwm_device *pwm) if (!pwm) return -EINVAL; - if (!test_and_set_bit(PWMF_ENABLED, &pwm->flags)) { + if (!pwm_is_enabled(pwm)) { err = pwm->chip->ops->enable(pwm->chip, pwm); - if (err) - clear_bit(PWMF_ENABLED, &pwm->flags); + if (!err) + pwm->state.enabled = true; } return err; @@ -515,8 +515,13 @@ EXPORT_SYMBOL_GPL(pwm_enable); */ void pwm_disable(struct pwm_device *pwm) { - if (pwm && test_and_clear_bit(PWMF_ENABLED, &pwm->flags)) + if (!pwm) + return; + + if (pwm_is_enabled(pwm)) { pwm->chip->ops->disable(pwm->chip, pwm); + pwm->state.enabled = false; + } } EXPORT_SYMBOL_GPL(pwm_disable); diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 51d4005418f9..150563a806d6 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -94,8 +94,7 @@ struct pwm_args { enum { PWMF_REQUESTED = 1 << 0, - PWMF_ENABLED = 1 << 1, - PWMF_EXPORTED = 1 << 2, + PWMF_EXPORTED = 1 << 1, }; /* @@ -103,11 +102,13 @@ enum { * @period: PWM period (in nanoseconds) * @duty_cycle: PWM duty cycle (in nanoseconds) * @polarity: PWM polarity + * @enabled: PWM enabled status */ struct pwm_state { unsigned int period; unsigned int duty_cycle; enum pwm_polarity polarity; + bool enabled; }; /** @@ -146,7 +147,11 @@ static inline void pwm_get_state(const struct pwm_device *pwm, static inline bool pwm_is_enabled(const struct pwm_device *pwm) { - return test_bit(PWMF_ENABLED, &pwm->flags); + struct pwm_state state; + + pwm_get_state(pwm, &state); + + return state.enabled; } static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period) From 15fa8a43c147213a9563903c87b29671035eb6e8 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:40 +0200 Subject: [PATCH 26/30] pwm: Add hardware readout infrastructure Add a ->get_state() function to the pwm_ops struct to let PWM drivers initialize the PWM state attached to a PWM device. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 3 +++ include/linux/pwm.h | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index c240b5437145..a909c64ee863 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -270,6 +270,9 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, pwm->hwpwm = i; pwm->state.polarity = polarity; + if (chip->ops->get_state) + chip->ops->get_state(chip, pwm, &pwm->state); + radix_tree_insert(&pwm_tree, pwm->pwm, pwm); } diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 150563a806d6..33f8decd9f38 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -206,6 +206,29 @@ static inline void pwm_get_args(const struct pwm_device *pwm, static inline void pwm_apply_args(struct pwm_device *pwm) { + /* + * PWM users calling pwm_apply_args() expect to have a fresh config + * where the polarity and period are set according to pwm_args info. + * The problem is, polarity can only be changed when the PWM is + * disabled. + * + * PWM drivers supporting hardware readout may declare the PWM device + * as enabled, and prevent polarity setting, which changes from the + * existing behavior, where all PWM devices are declared as disabled + * at startup (even if they are actually enabled), thus authorizing + * polarity setting. + * + * Instead of setting ->enabled to false, we call pwm_disable() + * before pwm_set_polarity() to ensure that everything is configured + * as expected, and the PWM is really disabled when the user request + * it. + * + * Note that PWM users requiring a smooth handover between the + * bootloader and the kernel (like critical regulators controlled by + * PWM devices) will have to switch to the atomic API and avoid calling + * pwm_apply_args(). + */ + pwm_disable(pwm); pwm_set_polarity(pwm, pwm->args.polarity); } @@ -217,6 +240,9 @@ static inline void pwm_apply_args(struct pwm_device *pwm) * @set_polarity: configure the polarity of this PWM * @enable: enable PWM output toggling * @disable: disable PWM output toggling + * @get_state: get the current PWM state. This function is only + * called once per PWM device when the PWM chip is + * registered. * @dbg_show: optional routine to show contents in debugfs * @owner: helps prevent removal of modules exporting active PWMs */ @@ -229,6 +255,8 @@ struct pwm_ops { enum pwm_polarity polarity); int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm); void (*disable)(struct pwm_chip *chip, struct pwm_device *pwm); + void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state); #ifdef CONFIG_DEBUG_FS void (*dbg_show)(struct pwm_chip *chip, struct seq_file *s); #endif From 5ec803edcb703fe379836f13560b79dfac79b01d Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:41 +0200 Subject: [PATCH 27/30] pwm: Add core infrastructure to allow atomic updates Add an ->apply() method to the pwm_ops struct to allow PWM drivers to implement atomic updates. This method is preferred over the ->enable(), ->disable() and ->config() methods if available. Add the pwm_apply_state() function to the PWM user API. Note that the pwm_apply_state() does not guarantee the atomicity of the update operation, it all depends on the availability and implementation of the ->apply() method. pwm_enable/disable/set_polarity/config() are now implemented as wrappers around the pwm_apply_state() function. pwm_adjust_config() is allowing smooth handover between the bootloader and the kernel. This function tries to adapt the current PWM state to the PWM arguments coming from a PWM lookup table or a DT definition without changing the duty_cycle/period proportion. Signed-off-by: Boris Brezillon [thierry.reding@gmail.com: fix a couple of typos] Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 209 +++++++++++++++++++++------------- include/linux/pwm.h | 269 ++++++++++++++++++++++++++++++-------------- 2 files changed, 314 insertions(+), 164 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index a909c64ee863..729d457861fd 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -226,6 +226,19 @@ void *pwm_get_chip_data(struct pwm_device *pwm) } EXPORT_SYMBOL_GPL(pwm_get_chip_data); +static bool pwm_ops_check(const struct pwm_ops *ops) +{ + /* driver supports legacy, non-atomic operation */ + if (ops->config && ops->enable && ops->disable) + return true; + + /* driver supports atomic operation */ + if (ops->apply) + return true; + + return false; +} + /** * pwmchip_add_with_polarity() - register a new PWM chip * @chip: the PWM chip to add @@ -244,8 +257,10 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, unsigned int i; int ret; - if (!chip || !chip->dev || !chip->ops || !chip->ops->config || - !chip->ops->enable || !chip->ops->disable || !chip->npwm) + if (!chip || !chip->dev || !chip->ops || !chip->npwm) + return -EINVAL; + + if (!pwm_ops_check(chip->ops)) return -EINVAL; mutex_lock(&pwm_lock); @@ -431,102 +446,138 @@ void pwm_free(struct pwm_device *pwm) EXPORT_SYMBOL_GPL(pwm_free); /** - * pwm_config() - change a PWM device configuration + * pwm_apply_state() - atomically apply a new state to a PWM device * @pwm: PWM device - * @duty_ns: "on" time (in nanoseconds) - * @period_ns: duration (in nanoseconds) of one cycle - * - * Returns: 0 on success or a negative error code on failure. + * @state: new state to apply. This can be adjusted by the PWM driver + * if the requested config is not achievable, for example, + * ->duty_cycle and ->period might be approximated. */ -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) +int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state) { int err; - if (!pwm || duty_ns < 0 || period_ns <= 0 || duty_ns > period_ns) - return -EINVAL; - - err = pwm->chip->ops->config(pwm->chip, pwm, duty_ns, period_ns); - if (err) - return err; - - pwm->state.duty_cycle = duty_ns; - pwm->state.period = period_ns; - - return 0; -} -EXPORT_SYMBOL_GPL(pwm_config); - -/** - * pwm_set_polarity() - configure the polarity of a PWM signal - * @pwm: PWM device - * @polarity: new polarity of the PWM signal - * - * Note that the polarity cannot be configured while the PWM device is - * enabled. - * - * Returns: 0 on success or a negative error code on failure. - */ -int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity) -{ - int err; - - if (!pwm || !pwm->chip->ops) - return -EINVAL; - - if (!pwm->chip->ops->set_polarity) - return -ENOSYS; - - if (pwm_is_enabled(pwm)) - return -EBUSY; - - err = pwm->chip->ops->set_polarity(pwm->chip, pwm, polarity); - if (err) - return err; - - pwm->state.polarity = polarity; - - return 0; -} -EXPORT_SYMBOL_GPL(pwm_set_polarity); - -/** - * pwm_enable() - start a PWM output toggling - * @pwm: PWM device - * - * Returns: 0 on success or a negative error code on failure. - */ -int pwm_enable(struct pwm_device *pwm) -{ - int err = 0; - if (!pwm) return -EINVAL; - if (!pwm_is_enabled(pwm)) { - err = pwm->chip->ops->enable(pwm->chip, pwm); - if (!err) - pwm->state.enabled = true; + if (!memcmp(state, &pwm->state, sizeof(*state))) + return 0; + + if (pwm->chip->ops->apply) { + err = pwm->chip->ops->apply(pwm->chip, pwm, state); + if (err) + return err; + + pwm->state = *state; + } else { + /* + * FIXME: restore the initial state in case of error. + */ + if (state->polarity != pwm->state.polarity) { + if (!pwm->chip->ops->set_polarity) + return -ENOTSUPP; + + /* + * Changing the polarity of a running PWM is + * only allowed when the PWM driver implements + * ->apply(). + */ + if (pwm->state.enabled) { + pwm->chip->ops->disable(pwm->chip, pwm); + pwm->state.enabled = false; + } + + err = pwm->chip->ops->set_polarity(pwm->chip, pwm, + state->polarity); + if (err) + return err; + + pwm->state.polarity = state->polarity; + } + + if (state->period != pwm->state.period || + state->duty_cycle != pwm->state.duty_cycle) { + err = pwm->chip->ops->config(pwm->chip, pwm, + state->duty_cycle, + state->period); + if (err) + return err; + + pwm->state.duty_cycle = state->duty_cycle; + pwm->state.period = state->period; + } + + if (state->enabled != pwm->state.enabled) { + if (state->enabled) { + err = pwm->chip->ops->enable(pwm->chip, pwm); + if (err) + return err; + } else { + pwm->chip->ops->disable(pwm->chip, pwm); + } + + pwm->state.enabled = state->enabled; + } } - return err; + return 0; } -EXPORT_SYMBOL_GPL(pwm_enable); +EXPORT_SYMBOL_GPL(pwm_apply_state); /** - * pwm_disable() - stop a PWM output toggling + * pwm_adjust_config() - adjust the current PWM config to the PWM arguments * @pwm: PWM device + * + * This function will adjust the PWM config to the PWM arguments provided + * by the DT or PWM lookup table. This is particularly useful to adapt + * the bootloader config to the Linux one. */ -void pwm_disable(struct pwm_device *pwm) +int pwm_adjust_config(struct pwm_device *pwm) { - if (!pwm) - return; + struct pwm_state state; + struct pwm_args pargs; - if (pwm_is_enabled(pwm)) { - pwm->chip->ops->disable(pwm->chip, pwm); - pwm->state.enabled = false; + pwm_get_args(pwm, &pargs); + pwm_get_state(pwm, &state); + + /* + * If the current period is zero it means that either the PWM driver + * does not support initial state retrieval or the PWM has not yet + * been configured. + * + * In either case, we setup the new period and polarity, and assign a + * duty cycle of 0. + */ + if (!state.period) { + state.duty_cycle = 0; + state.period = pargs.period; + state.polarity = pargs.polarity; + + return pwm_apply_state(pwm, &state); } + + /* + * Adjust the PWM duty cycle/period based on the period value provided + * in PWM args. + */ + if (pargs.period != state.period) { + u64 dutycycle = (u64)state.duty_cycle * pargs.period; + + do_div(dutycycle, state.period); + state.duty_cycle = dutycycle; + state.period = pargs.period; + } + + /* + * If the polarity changed, we should also change the duty cycle. + */ + if (pargs.polarity != state.polarity) { + state.polarity = pargs.polarity; + state.duty_cycle = state.period - state.duty_cycle; + } + + return pwm_apply_state(pwm, &state); } -EXPORT_SYMBOL_GPL(pwm_disable); +EXPORT_SYMBOL_GPL(pwm_adjust_config); static struct pwm_chip *of_node_to_pwmchip(struct device_node *np) { diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 33f8decd9f38..17018f3c066e 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -5,59 +5,7 @@ #include #include -struct pwm_device; struct seq_file; - -#if IS_ENABLED(CONFIG_PWM) -/* - * pwm_request - request a PWM device - */ -struct pwm_device *pwm_request(int pwm_id, const char *label); - -/* - * pwm_free - free a PWM device - */ -void pwm_free(struct pwm_device *pwm); - -/* - * pwm_config - change a PWM device configuration - */ -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); - -/* - * pwm_enable - start a PWM output toggling - */ -int pwm_enable(struct pwm_device *pwm); - -/* - * pwm_disable - stop a PWM output toggling - */ -void pwm_disable(struct pwm_device *pwm); -#else -static inline struct pwm_device *pwm_request(int pwm_id, const char *label) -{ - return ERR_PTR(-ENODEV); -} - -static inline void pwm_free(struct pwm_device *pwm) -{ -} - -static inline int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) -{ - return -EINVAL; -} - -static inline int pwm_enable(struct pwm_device *pwm) -{ - return -EINVAL; -} - -static inline void pwm_disable(struct pwm_device *pwm) -{ -} -#endif - struct pwm_chip; /** @@ -184,11 +132,6 @@ static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm) return state.duty_cycle; } -/* - * pwm_set_polarity - configure the polarity of a PWM signal - */ -int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity); - static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm) { struct pwm_state state; @@ -204,34 +147,6 @@ static inline void pwm_get_args(const struct pwm_device *pwm, *args = pwm->args; } -static inline void pwm_apply_args(struct pwm_device *pwm) -{ - /* - * PWM users calling pwm_apply_args() expect to have a fresh config - * where the polarity and period are set according to pwm_args info. - * The problem is, polarity can only be changed when the PWM is - * disabled. - * - * PWM drivers supporting hardware readout may declare the PWM device - * as enabled, and prevent polarity setting, which changes from the - * existing behavior, where all PWM devices are declared as disabled - * at startup (even if they are actually enabled), thus authorizing - * polarity setting. - * - * Instead of setting ->enabled to false, we call pwm_disable() - * before pwm_set_polarity() to ensure that everything is configured - * as expected, and the PWM is really disabled when the user request - * it. - * - * Note that PWM users requiring a smooth handover between the - * bootloader and the kernel (like critical regulators controlled by - * PWM devices) will have to switch to the atomic API and avoid calling - * pwm_apply_args(). - */ - pwm_disable(pwm); - pwm_set_polarity(pwm, pwm->args.polarity); -} - /** * struct pwm_ops - PWM controller operations * @request: optional hook for requesting a PWM @@ -240,6 +155,10 @@ static inline void pwm_apply_args(struct pwm_device *pwm) * @set_polarity: configure the polarity of this PWM * @enable: enable PWM output toggling * @disable: disable PWM output toggling + * @apply: atomically apply a new PWM config. The state argument + * should be adjusted with the real hardware config (if the + * approximate the period or duty_cycle value, state should + * reflect it) * @get_state: get the current PWM state. This function is only * called once per PWM device when the PWM chip is * registered. @@ -255,6 +174,8 @@ struct pwm_ops { enum pwm_polarity polarity); int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm); void (*disable)(struct pwm_chip *chip, struct pwm_device *pwm); + int (*apply)(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state); void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state *state); #ifdef CONFIG_DEBUG_FS @@ -292,6 +213,115 @@ struct pwm_chip { }; #if IS_ENABLED(CONFIG_PWM) +/* PWM user APIs */ +struct pwm_device *pwm_request(int pwm_id, const char *label); +void pwm_free(struct pwm_device *pwm); +int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state); +int pwm_adjust_config(struct pwm_device *pwm); + +/** + * pwm_config() - change a PWM device configuration + * @pwm: PWM device + * @duty_ns: "on" time (in nanoseconds) + * @period_ns: duration (in nanoseconds) of one cycle + * + * Returns: 0 on success or a negative error code on failure. + */ +static inline int pwm_config(struct pwm_device *pwm, int duty_ns, + int period_ns) +{ + struct pwm_state state; + + if (!pwm) + return -EINVAL; + + pwm_get_state(pwm, &state); + if (state.duty_cycle == duty_ns && state.period == period_ns) + return 0; + + state.duty_cycle = duty_ns; + state.period = period_ns; + return pwm_apply_state(pwm, &state); +} + +/** + * pwm_set_polarity() - configure the polarity of a PWM signal + * @pwm: PWM device + * @polarity: new polarity of the PWM signal + * + * Note that the polarity cannot be configured while the PWM device is + * enabled. + * + * Returns: 0 on success or a negative error code on failure. + */ +static inline int pwm_set_polarity(struct pwm_device *pwm, + enum pwm_polarity polarity) +{ + struct pwm_state state; + + if (!pwm) + return -EINVAL; + + pwm_get_state(pwm, &state); + if (state.polarity == polarity) + return 0; + + /* + * Changing the polarity of a running PWM without adjusting the + * dutycycle/period value is a bit risky (can introduce glitches). + * Return -EBUSY in this case. + * Note that this is allowed when using pwm_apply_state() because + * the user specifies all the parameters. + */ + if (state.enabled) + return -EBUSY; + + state.polarity = polarity; + return pwm_apply_state(pwm, &state); +} + +/** + * pwm_enable() - start a PWM output toggling + * @pwm: PWM device + * + * Returns: 0 on success or a negative error code on failure. + */ +static inline int pwm_enable(struct pwm_device *pwm) +{ + struct pwm_state state; + + if (!pwm) + return -EINVAL; + + pwm_get_state(pwm, &state); + if (state.enabled) + return 0; + + state.enabled = true; + return pwm_apply_state(pwm, &state); +} + +/** + * pwm_disable() - stop a PWM output toggling + * @pwm: PWM device + */ +static inline void pwm_disable(struct pwm_device *pwm) +{ + struct pwm_state state; + + if (!pwm) + return; + + pwm_get_state(pwm, &state); + if (!state.enabled) + return; + + state.enabled = false; + pwm_apply_state(pwm, &state); +} + + +/* PWM provider APIs */ int pwm_set_chip_data(struct pwm_device *pwm, void *data); void *pwm_get_chip_data(struct pwm_device *pwm); @@ -317,6 +347,47 @@ void devm_pwm_put(struct device *dev, struct pwm_device *pwm); bool pwm_can_sleep(struct pwm_device *pwm); #else +static inline struct pwm_device *pwm_request(int pwm_id, const char *label) +{ + return ERR_PTR(-ENODEV); +} + +static inline void pwm_free(struct pwm_device *pwm) +{ +} + +static inline int pwm_apply_state(struct pwm_device *pwm, + const struct pwm_state *state) +{ + return -ENOTSUPP; +} + +static inline int pwm_adjust_config(struct pwm_device *pwm) +{ + return -ENOTSUPP; +} + +static inline int pwm_config(struct pwm_device *pwm, int duty_ns, + int period_ns) +{ + return -EINVAL; +} + +static inline int pwm_set_polarity(struct pwm_device *pwm, + enum pwm_polarity polarity) +{ + return -ENOTSUPP; +} + +static inline int pwm_enable(struct pwm_device *pwm) +{ + return -EINVAL; +} + +static inline void pwm_disable(struct pwm_device *pwm) +{ +} + static inline int pwm_set_chip_data(struct pwm_device *pwm, void *data) { return -EINVAL; @@ -388,6 +459,34 @@ static inline bool pwm_can_sleep(struct pwm_device *pwm) } #endif +static inline void pwm_apply_args(struct pwm_device *pwm) +{ + /* + * PWM users calling pwm_apply_args() expect to have a fresh config + * where the polarity and period are set according to pwm_args info. + * The problem is, polarity can only be changed when the PWM is + * disabled. + * + * PWM drivers supporting hardware readout may declare the PWM device + * as enabled, and prevent polarity setting, which changes from the + * existing behavior, where all PWM devices are declared as disabled + * at startup (even if they are actually enabled), thus authorizing + * polarity setting. + * + * Instead of setting ->enabled to false, we call pwm_disable() + * before pwm_set_polarity() to ensure that everything is configured + * as expected, and the PWM is really disabled when the user request + * it. + * + * Note that PWM users requiring a smooth handover between the + * bootloader and the kernel (like critical regulators controlled by + * PWM devices) will have to switch to the atomic API and avoid calling + * pwm_apply_args(). + */ + pwm_disable(pwm); + pwm_set_polarity(pwm, pwm->args.polarity); +} + struct pwm_lookup { struct list_head list; const char *provider; From a07136fdcf12281781142caf1f78c6696721accd Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:42 +0200 Subject: [PATCH 28/30] pwm: Update documentation Update the PWM subsystem documentation to reflect the atomic PWM changes. Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- Documentation/pwm.txt | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt index ca895fd211e4..789b27c6ec99 100644 --- a/Documentation/pwm.txt +++ b/Documentation/pwm.txt @@ -42,9 +42,26 @@ variants of these functions, devm_pwm_get() and devm_pwm_put(), also exist. After being requested, a PWM has to be configured using: -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); +int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state); -To start/stop toggling the PWM output use pwm_enable()/pwm_disable(). +This API controls both the PWM period/duty_cycle config and the +enable/disable state. + +The pwm_config(), pwm_enable() and pwm_disable() functions are just wrappers +around pwm_apply_state() and should not be used if the user wants to change +several parameter at once. For example, if you see pwm_config() and +pwm_{enable,disable}() calls in the same function, this probably means you +should switch to pwm_apply_state(). + +The PWM user API also allows one to query the PWM state with pwm_get_state(). + +In addition to the PWM state, the PWM API also exposes PWM arguments, which +are the reference PWM config one should use on this PWM. +PWM arguments are usually platform-specific and allows the PWM user to only +care about dutycycle relatively to the full period (like, duty = 50% of the +period). struct pwm_args contains 2 fields (period and polarity) and should +be used to set the initial PWM config (usually done in the probe function +of the PWM user). PWM arguments are retrieved with pwm_get_args(). Using PWMs with the sysfs interface ----------------------------------- @@ -105,6 +122,15 @@ goes low for the remainder of the period. Conversely, a signal with inversed polarity starts low for the duration of the duty cycle and goes high for the remainder of the period. +Drivers are encouraged to implement ->apply() instead of the legacy +->enable(), ->disable() and ->config() methods. Doing that should provide +atomicity in the PWM config workflow, which is required when the PWM controls +a critical device (like a regulator). + +The implementation of ->get_state() (a method used to retrieve initial PWM +state) is also encouraged for the same reason: letting the PWM user know +about the current PWM state would allow him to avoid glitches. + Locking ------- From 39100ceea79ff2efeb2fb094baf120c73d5ccf47 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 14 Apr 2016 21:17:43 +0200 Subject: [PATCH 29/30] pwm: Switch to the atomic API Replace legacy pwm_get/set_xxx() and pwm_config/enable/disable() calls by pwm_get/apply_state(). Signed-off-by: Boris Brezillon Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 5 ++++- drivers/pwm/sysfs.c | 48 ++++++++++++++++++++++++++++++++++----------- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 729d457861fd..b0b87b3b52a6 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -948,13 +948,16 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s) for (i = 0; i < chip->npwm; i++) { struct pwm_device *pwm = &chip->pwms[i]; + struct pwm_state state; + + pwm_get_state(pwm, &state); seq_printf(s, " pwm-%-3d (%-20.20s):", i, pwm->label); if (test_bit(PWMF_REQUESTED, &pwm->flags)) seq_puts(s, " requested"); - if (pwm_is_enabled(pwm)) + if (state.enabled) seq_puts(s, " enabled"); seq_puts(s, "\n"); diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c index 187ca0875cf6..d98599249a05 100644 --- a/drivers/pwm/sysfs.c +++ b/drivers/pwm/sysfs.c @@ -46,8 +46,11 @@ static ssize_t period_show(struct device *child, char *buf) { const struct pwm_device *pwm = child_to_pwm_device(child); + struct pwm_state state; - return sprintf(buf, "%u\n", pwm_get_period(pwm)); + pwm_get_state(pwm, &state); + + return sprintf(buf, "%u\n", state.period); } static ssize_t period_store(struct device *child, @@ -56,6 +59,7 @@ 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; int ret; @@ -64,7 +68,9 @@ static ssize_t period_store(struct device *child, return ret; mutex_lock(&export->lock); - ret = pwm_config(pwm, pwm_get_duty_cycle(pwm), val); + pwm_get_state(pwm, &state); + state.period = val; + ret = pwm_apply_state(pwm, &state); mutex_unlock(&export->lock); return ret ? : size; @@ -75,8 +81,11 @@ static ssize_t duty_cycle_show(struct device *child, char *buf) { const struct pwm_device *pwm = child_to_pwm_device(child); + struct pwm_state state; - return sprintf(buf, "%u\n", pwm_get_duty_cycle(pwm)); + pwm_get_state(pwm, &state); + + return sprintf(buf, "%u\n", state.duty_cycle); } static ssize_t duty_cycle_store(struct device *child, @@ -85,6 +94,7 @@ static ssize_t duty_cycle_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; int ret; @@ -93,7 +103,9 @@ static ssize_t duty_cycle_store(struct device *child, return ret; mutex_lock(&export->lock); - ret = pwm_config(pwm, val, pwm_get_period(pwm)); + pwm_get_state(pwm, &state); + state.duty_cycle = val; + ret = pwm_apply_state(pwm, &state); mutex_unlock(&export->lock); return ret ? : size; @@ -104,8 +116,11 @@ static ssize_t enable_show(struct device *child, char *buf) { const struct pwm_device *pwm = child_to_pwm_device(child); + struct pwm_state state; - return sprintf(buf, "%d\n", pwm_is_enabled(pwm)); + pwm_get_state(pwm, &state); + + return sprintf(buf, "%d\n", state.enabled); } static ssize_t enable_store(struct device *child, @@ -114,6 +129,7 @@ static ssize_t enable_store(struct device *child, { struct pwm_export *export = child_to_pwm_export(child); struct pwm_device *pwm = export->pwm; + struct pwm_state state; int val, ret; ret = kstrtoint(buf, 0, &val); @@ -122,20 +138,24 @@ static ssize_t enable_store(struct device *child, mutex_lock(&export->lock); + pwm_get_state(pwm, &state); + switch (val) { case 0: - pwm_disable(pwm); + state.enabled = false; break; case 1: - ret = pwm_enable(pwm); + state.enabled = true; break; default: ret = -EINVAL; - break; + goto unlock; } - mutex_unlock(&export->lock); + pwm_apply_state(pwm, &state); +unlock: + mutex_unlock(&export->lock); return ret ? : size; } @@ -145,8 +165,11 @@ static ssize_t polarity_show(struct device *child, { const struct pwm_device *pwm = child_to_pwm_device(child); const char *polarity = "unknown"; + struct pwm_state state; - switch (pwm_get_polarity(pwm)) { + pwm_get_state(pwm, &state); + + switch (state.polarity) { case PWM_POLARITY_NORMAL: polarity = "normal"; break; @@ -166,6 +189,7 @@ static ssize_t polarity_store(struct device *child, struct pwm_export *export = child_to_pwm_export(child); struct pwm_device *pwm = export->pwm; enum pwm_polarity polarity; + struct pwm_state state; int ret; if (sysfs_streq(buf, "normal")) @@ -176,7 +200,9 @@ static ssize_t polarity_store(struct device *child, return -EINVAL; mutex_lock(&export->lock); - ret = pwm_set_polarity(pwm, polarity); + pwm_get_state(pwm, &state); + state.polarity = polarity; + ret = pwm_apply_state(pwm, &state); mutex_unlock(&export->lock); return ret ? : size; From 23e3523f5d3a980edf7f189743cf4bb9490400a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Heiko=20St=C3=BCbner?= Date: Thu, 14 Apr 2016 21:17:44 +0200 Subject: [PATCH 30/30] pwm: Add information about polarity, duty cycle and period to debugfs The PWM states make it possible to also output the polarity, duty cycle and period information in the debugfs summary output. This simplifies gathering information about PWMs without needing to walk through the sysfs attributes of every PWM. Signed-off-by: Heiko Stuebner Signed-off-by: Boris Brezillon [thierry.reding@gmail.com: use more spaces in debugfs output] Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index b0b87b3b52a6..c2e1a4bb23ac 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -960,6 +960,11 @@ 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, " polarity: %s", + state.polarity ? "inverse" : "normal"); + seq_puts(s, "\n"); } }