From cfc4c189bc70b1acc17e6f1abf1dc1c0ae890bd8 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 21 Oct 2019 12:51:56 +0200 Subject: [PATCH 01/37] pwm: Read initial hardware state at request time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drivers that support reading the hardware state (using ->get_state()) may want to rely on per-PWM data to do so. Defer reading the hardware state for the first time until the PWM has been requested and after drivers have had a chance to allocate per-PWM data. Conceptually this is also a more natural place to read the hardware state because the PWM core doesn't need to know the hardware state of a PWM unless there is a user for it. This also ensures that the state is read everytime a user requests a PWM. If the PWM changes between users for some reason, the PWM core will reload the state from hardware and keep its copy of the state up-to-date. Tested-by: Enric Balletbo i Serra Tested-by: Michal Vokáč Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index f877e77d9184..e067873c6cc5 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -114,6 +114,9 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label) } } + if (pwm->chip->ops->get_state) + pwm->chip->ops->get_state(pwm->chip, pwm, &pwm->state); + set_bit(PWMF_REQUESTED, &pwm->flags); pwm->label = label; @@ -283,9 +286,6 @@ 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); } From 1db37f9561b2b3f57d84b6253a9cd97f6289f8e1 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 17 Oct 2019 13:21:15 +0200 Subject: [PATCH 02/37] pwm: cros-ec: Cache duty cycle value The ChromeOS embedded controller doesn't differentiate between disabled and duty cycle being 0. In order not to potentially confuse consumers, cache the duty cycle and return the cached value instead of the real value when the PWM is disabled. Tested-by: Enric Balletbo i Serra Signed-off-by: Thierry Reding --- drivers/pwm/pwm-cros-ec.c | 58 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c index 89497448d217..09c08dee099e 100644 --- a/drivers/pwm/pwm-cros-ec.c +++ b/drivers/pwm/pwm-cros-ec.c @@ -25,11 +25,39 @@ struct cros_ec_pwm_device { struct pwm_chip chip; }; +/** + * struct cros_ec_pwm - per-PWM driver data + * @duty_cycle: cached duty cycle + */ +struct cros_ec_pwm { + u16 duty_cycle; +}; + static inline struct cros_ec_pwm_device *pwm_to_cros_ec_pwm(struct pwm_chip *c) { return container_of(c, struct cros_ec_pwm_device, chip); } +static int cros_ec_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm) +{ + struct cros_ec_pwm *channel; + + channel = kzalloc(sizeof(*channel), GFP_KERNEL); + if (!channel) + return -ENOMEM; + + pwm_set_chip_data(pwm, channel); + + return 0; +} + +static void cros_ec_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) +{ + struct cros_ec_pwm *channel = pwm_get_chip_data(pwm); + + kfree(channel); +} + static int cros_ec_pwm_set_duty(struct cros_ec_device *ec, u8 index, u16 duty) { struct { @@ -96,7 +124,9 @@ static int cros_ec_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, const struct pwm_state *state) { struct cros_ec_pwm_device *ec_pwm = pwm_to_cros_ec_pwm(chip); - int duty_cycle; + struct cros_ec_pwm *channel = pwm_get_chip_data(pwm); + u16 duty_cycle; + int ret; /* The EC won't let us change the period */ if (state->period != EC_PWM_MAX_DUTY) @@ -108,13 +138,20 @@ static int cros_ec_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, */ duty_cycle = state->enabled ? state->duty_cycle : 0; - return cros_ec_pwm_set_duty(ec_pwm->ec, pwm->hwpwm, duty_cycle); + ret = cros_ec_pwm_set_duty(ec_pwm->ec, pwm->hwpwm, duty_cycle); + if (ret < 0) + return ret; + + channel->duty_cycle = state->duty_cycle; + + return 0; } static void cros_ec_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state *state) { struct cros_ec_pwm_device *ec_pwm = pwm_to_cros_ec_pwm(chip); + struct cros_ec_pwm *channel = pwm_get_chip_data(pwm); int ret; ret = cros_ec_pwm_get_duty(ec_pwm->ec, pwm->hwpwm); @@ -126,8 +163,19 @@ static void cros_ec_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, state->enabled = (ret > 0); state->period = EC_PWM_MAX_DUTY; - /* Note that "disabled" and "duty cycle == 0" are treated the same */ - state->duty_cycle = ret; + /* + * Note that "disabled" and "duty cycle == 0" are treated the same. If + * the cached duty cycle is not zero, used the cached duty cycle. This + * ensures that the configured duty cycle is kept across a disable and + * enable operation and avoids potentially confusing consumers. + * + * For the case of the initial hardware readout, channel->duty_cycle + * will be 0 and the actual duty cycle read from the EC is used. + */ + if (ret == 0 && channel->duty_cycle > 0) + state->duty_cycle = channel->duty_cycle; + else + state->duty_cycle = ret; } static struct pwm_device * @@ -149,6 +197,8 @@ cros_ec_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) } static const struct pwm_ops cros_ec_pwm_ops = { + .request = cros_ec_pwm_request, + .free = cros_ec_pwm_free, .get_state = cros_ec_pwm_get_state, .apply = cros_ec_pwm_apply, .owner = THIS_MODULE, From a3597d6c89d70ff0bcb1dd74dc0a88442fe79da6 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 17 Oct 2019 12:56:00 +0200 Subject: [PATCH 03/37] pwm: imx27: Cache duty cycle register value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hardware register containing the duty cycle value cannot be accessed when the PWM is disabled. This causes the ->get_state() callback to read back a duty cycle value of 0, which can confuse consumer drivers. Tested-by: Michal Vokáč Tested-by: Adam Ford Signed-off-by: Thierry Reding --- drivers/pwm/pwm-imx27.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c index ae11d8577f18..4113d5cd4c62 100644 --- a/drivers/pwm/pwm-imx27.c +++ b/drivers/pwm/pwm-imx27.c @@ -85,6 +85,13 @@ struct pwm_imx27_chip { struct clk *clk_per; void __iomem *mmio_base; struct pwm_chip chip; + + /* + * The driver cannot read the current duty cycle from the hardware if + * the hardware is disabled. Cache the last programmed duty cycle + * value to return in that case. + */ + unsigned int duty_cycle; }; #define to_pwm_imx27_chip(chip) container_of(chip, struct pwm_imx27_chip, chip) @@ -155,14 +162,17 @@ static void pwm_imx27_get_state(struct pwm_chip *chip, tmp = NSEC_PER_SEC * (u64)(period + 2); state->period = DIV_ROUND_CLOSEST_ULL(tmp, pwm_clk); - /* PWMSAR can be read only if PWM is enabled */ - if (state->enabled) { + /* + * PWMSAR can be read only if PWM is enabled. If the PWM is disabled, + * use the cached value. + */ + if (state->enabled) val = readl(imx->mmio_base + MX3_PWMSAR); - tmp = NSEC_PER_SEC * (u64)(val); - state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, pwm_clk); - } else { - state->duty_cycle = 0; - } + else + val = imx->duty_cycle; + + tmp = NSEC_PER_SEC * (u64)(val); + state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, pwm_clk); if (!state->enabled) pwm_imx27_clk_disable_unprepare(chip); @@ -261,6 +271,13 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, writel(duty_cycles, imx->mmio_base + MX3_PWMSAR); writel(period_cycles, imx->mmio_base + MX3_PWMPR); + /* + * Store the duty cycle for future reference in cases where + * the MX3_PWMSAR register can't be read (i.e. when the PWM + * is disabled). + */ + imx->duty_cycle = duty_cycles; + cr = MX3_PWMCR_PRESCALER_SET(prescale) | MX3_PWMCR_STOPEN | MX3_PWMCR_DOZEN | MX3_PWMCR_WAITEN | FIELD_PREP(MX3_PWMCR_CLKSRC, MX3_PWMCR_CLKSRC_IPG_HIGH) | From bd88d319abe9a4bb6cc63b23cc760bec46e81fe6 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 17 Oct 2019 17:11:41 +0200 Subject: [PATCH 04/37] pwm: imx27: Unconditionally write state to hardware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The i.MX driver currently uses a shortcut and doesn't write all of the state through to the hardware when the PWM is disabled. This causes an inconsistent state to be read back by consumers with the result of them malfunctioning. Fix this by always writing the full state through to the hardware registers so that the correct state can always be read back. Tested-by: Michal Vokáč Tested-by: Adam Ford Signed-off-by: Thierry Reding --- drivers/pwm/pwm-imx27.c | 110 ++++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 56 deletions(-) diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c index 4113d5cd4c62..59d8b1289808 100644 --- a/drivers/pwm/pwm-imx27.c +++ b/drivers/pwm/pwm-imx27.c @@ -230,70 +230,68 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, pwm_get_state(pwm, &cstate); - if (state->enabled) { - c = clk_get_rate(imx->clk_per); - c *= state->period; + c = clk_get_rate(imx->clk_per); + c *= state->period; - do_div(c, 1000000000); - period_cycles = c; + do_div(c, 1000000000); + period_cycles = c; - prescale = period_cycles / 0x10000 + 1; + prescale = period_cycles / 0x10000 + 1; - period_cycles /= prescale; - c = (unsigned long long)period_cycles * state->duty_cycle; - do_div(c, state->period); - duty_cycles = c; + period_cycles /= prescale; + c = (unsigned long long)period_cycles * state->duty_cycle; + do_div(c, state->period); + duty_cycles = c; - /* - * according to imx pwm RM, the real period value should be - * PERIOD value in PWMPR plus 2. - */ - if (period_cycles > 2) - period_cycles -= 2; - else - period_cycles = 0; + /* + * according to imx pwm RM, the real period value should be PERIOD + * value in PWMPR plus 2. + */ + if (period_cycles > 2) + period_cycles -= 2; + else + period_cycles = 0; - /* - * Wait for a free FIFO slot if the PWM is already enabled, and - * flush the FIFO if the PWM was disabled and is about to be - * enabled. - */ - if (cstate.enabled) { - pwm_imx27_wait_fifo_slot(chip, pwm); - } else { - ret = pwm_imx27_clk_prepare_enable(chip); - if (ret) - return ret; + /* + * Wait for a free FIFO slot if the PWM is already enabled, and flush + * the FIFO if the PWM was disabled and is about to be enabled. + */ + if (cstate.enabled) { + pwm_imx27_wait_fifo_slot(chip, pwm); + } else { + ret = pwm_imx27_clk_prepare_enable(chip); + if (ret) + return ret; - pwm_imx27_sw_reset(chip); - } - - writel(duty_cycles, imx->mmio_base + MX3_PWMSAR); - writel(period_cycles, imx->mmio_base + MX3_PWMPR); - - /* - * Store the duty cycle for future reference in cases where - * the MX3_PWMSAR register can't be read (i.e. when the PWM - * is disabled). - */ - imx->duty_cycle = duty_cycles; - - cr = MX3_PWMCR_PRESCALER_SET(prescale) | - MX3_PWMCR_STOPEN | MX3_PWMCR_DOZEN | MX3_PWMCR_WAITEN | - FIELD_PREP(MX3_PWMCR_CLKSRC, MX3_PWMCR_CLKSRC_IPG_HIGH) | - MX3_PWMCR_DBGEN | MX3_PWMCR_EN; - - if (state->polarity == PWM_POLARITY_INVERSED) - cr |= FIELD_PREP(MX3_PWMCR_POUTC, - MX3_PWMCR_POUTC_INVERTED); - - writel(cr, imx->mmio_base + MX3_PWMCR); - } else if (cstate.enabled) { - writel(0, imx->mmio_base + MX3_PWMCR); - - pwm_imx27_clk_disable_unprepare(chip); + pwm_imx27_sw_reset(chip); } + writel(duty_cycles, imx->mmio_base + MX3_PWMSAR); + writel(period_cycles, imx->mmio_base + MX3_PWMPR); + + /* + * Store the duty cycle for future reference in cases where the + * MX3_PWMSAR register can't be read (i.e. when the PWM is disabled). + */ + imx->duty_cycle = duty_cycles; + + cr = MX3_PWMCR_PRESCALER_SET(prescale) | + MX3_PWMCR_STOPEN | MX3_PWMCR_DOZEN | MX3_PWMCR_WAITEN | + FIELD_PREP(MX3_PWMCR_CLKSRC, MX3_PWMCR_CLKSRC_IPG_HIGH) | + MX3_PWMCR_DBGEN; + + if (state->polarity == PWM_POLARITY_INVERSED) + cr |= FIELD_PREP(MX3_PWMCR_POUTC, + MX3_PWMCR_POUTC_INVERTED); + + if (state->enabled) + cr |= MX3_PWMCR_EN; + + writel(cr, imx->mmio_base + MX3_PWMCR); + + if (!state->enabled && cstate.enabled) + pwm_imx27_clk_disable_unprepare(chip); + return 0; } From a7fe985633f927401037d2905df2c9701dff09a1 Mon Sep 17 00:00:00 2001 From: Jernej Skrabec Date: Sun, 24 Nov 2019 18:29:03 +0100 Subject: [PATCH 05/37] pwm: sun4i: Add an optional probe for reset line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H6 PWM core needs deasserted reset line in order to work. Add an optional probe for it. Signed-off-by: Jernej Skrabec Reviewed-by: Uwe Kleine-König Signed-off-by: Clément Péron Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 581d23287333..487899d4cc3f 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -78,6 +79,7 @@ struct sun4i_pwm_data { struct sun4i_pwm_chip { struct pwm_chip chip; struct clk *clk; + struct reset_control *rst; void __iomem *base; spinlock_t ctrl_lock; const struct sun4i_pwm_data *data; @@ -364,6 +366,22 @@ static int sun4i_pwm_probe(struct platform_device *pdev) if (IS_ERR(pwm->clk)) return PTR_ERR(pwm->clk); + pwm->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL); + if (IS_ERR(pwm->rst)) { + if (PTR_ERR(pwm->rst) != -EPROBE_DEFER) + dev_err(&pdev->dev, "get reset failed %pe\n", + pwm->rst); + return PTR_ERR(pwm->rst); + } + + /* Deassert reset */ + ret = reset_control_deassert(pwm->rst); + if (ret) { + dev_err(&pdev->dev, "cannot deassert reset control: %pe\n", + ERR_PTR(ret)); + return ret; + } + pwm->chip.dev = &pdev->dev; pwm->chip.ops = &sun4i_pwm_ops; pwm->chip.base = -1; @@ -376,19 +394,31 @@ static int sun4i_pwm_probe(struct platform_device *pdev) ret = pwmchip_add(&pwm->chip); if (ret < 0) { dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret); - return ret; + goto err_pwm_add; } platform_set_drvdata(pdev, pwm); return 0; + +err_pwm_add: + reset_control_assert(pwm->rst); + + return ret; } static int sun4i_pwm_remove(struct platform_device *pdev) { struct sun4i_pwm_chip *pwm = platform_get_drvdata(pdev); + int ret; - return pwmchip_remove(&pwm->chip); + ret = pwmchip_remove(&pwm->chip); + if (ret) + return ret; + + reset_control_assert(pwm->rst); + + return 0; } static struct platform_driver sun4i_pwm_driver = { From b8d74644f34a82a6dcd5f45d5bd57e64f1db0d4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20P=C3=A9ron?= Date: Sun, 24 Nov 2019 18:29:04 +0100 Subject: [PATCH 06/37] pwm: sun4i: Prefer "mod" clock to unnamed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New device tree bindings called the source clock of the module "mod" when several clocks are defined. Try to get a clock called "mod" if nothing is found try to get an unnamed clock. Reviewed-by: Uwe Kleine-König Signed-off-by: Clément Péron Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 487899d4cc3f..80026167044b 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -362,9 +362,34 @@ static int sun4i_pwm_probe(struct platform_device *pdev) if (IS_ERR(pwm->base)) return PTR_ERR(pwm->base); - pwm->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(pwm->clk)) + /* + * All hardware variants need a source clock that is divided and + * then feeds the counter that defines the output wave form. In the + * device tree this clock is either unnamed or called "mod". + * Some variants (e.g. H6) need another clock to access the + * hardware registers; this is called "bus". + * So we request "mod" first (and ignore the corner case that a + * parent provides a "mod" clock while the right one would be the + * unnamed one of the PWM device) and if this is not found we fall + * back to the first clock of the PWM. + */ + pwm->clk = devm_clk_get_optional(&pdev->dev, "mod"); + if (IS_ERR(pwm->clk)) { + if (PTR_ERR(pwm->rst) != -EPROBE_DEFER) + dev_err(&pdev->dev, "get mod clock failed %pe\n", + pwm->clk); return PTR_ERR(pwm->clk); + } + + if (!pwm->clk) { + pwm->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(pwm->clk)) { + if (PTR_ERR(pwm->rst) != -EPROBE_DEFER) + dev_err(&pdev->dev, "get unnamed clock failed %pe\n", + pwm->clk); + return PTR_ERR(pwm->clk); + } + } pwm->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL); if (IS_ERR(pwm->rst)) { From 5b090b430d750961305030232314b6acdb0102aa Mon Sep 17 00:00:00 2001 From: Jernej Skrabec Date: Sun, 24 Nov 2019 18:29:05 +0100 Subject: [PATCH 07/37] pwm: sun4i: Add an optional probe for bus clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H6 PWM core needs bus clock to be enabled in order to work. Add an optional probe for it. Signed-off-by: Jernej Skrabec Signed-off-by: Clément Péron Reviewed-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 80026167044b..a6727dd89e28 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -78,6 +78,7 @@ struct sun4i_pwm_data { struct sun4i_pwm_chip { struct pwm_chip chip; + struct clk *bus_clk; struct clk *clk; struct reset_control *rst; void __iomem *base; @@ -391,6 +392,14 @@ static int sun4i_pwm_probe(struct platform_device *pdev) } } + pwm->bus_clk = devm_clk_get_optional(&pdev->dev, "bus"); + if (IS_ERR(pwm->bus_clk)) { + if (PTR_ERR(pwm->rst) != -EPROBE_DEFER) + dev_err(&pdev->dev, "get bus clock failed %pe\n", + pwm->bus_clk); + return PTR_ERR(pwm->bus_clk); + } + pwm->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL); if (IS_ERR(pwm->rst)) { if (PTR_ERR(pwm->rst) != -EPROBE_DEFER) @@ -407,6 +416,17 @@ static int sun4i_pwm_probe(struct platform_device *pdev) return ret; } + /* + * We're keeping the bus clock on for the sake of simplicity. + * Actually it only needs to be on for hardware register accesses. + */ + ret = clk_prepare_enable(pwm->bus_clk); + if (ret) { + dev_err(&pdev->dev, "cannot prepare and enable bus_clk %pe\n", + ERR_PTR(ret)); + goto err_bus; + } + pwm->chip.dev = &pdev->dev; pwm->chip.ops = &sun4i_pwm_ops; pwm->chip.base = -1; @@ -427,6 +447,8 @@ static int sun4i_pwm_probe(struct platform_device *pdev) return 0; err_pwm_add: + clk_disable_unprepare(pwm->bus_clk); +err_bus: reset_control_assert(pwm->rst); return ret; @@ -441,6 +463,7 @@ static int sun4i_pwm_remove(struct platform_device *pdev) if (ret) return ret; + clk_disable_unprepare(pwm->bus_clk); reset_control_assert(pwm->rst); return 0; From fa4d81784681a26bcf7d2a43c6ac5cf991ef28f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20P=C3=A9ron?= Date: Sun, 24 Nov 2019 18:29:06 +0100 Subject: [PATCH 08/37] pwm: sun4i: Always calculate params when applying new parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bypass mode will require to be re-calculated when the pwm state is changed. Remove the condition so pwm_sun4i_calculate is always called. Reviewed-by: Uwe Kleine-König Signed-off-by: Clément Péron Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 56 ++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index a6727dd89e28..e369b5a398f4 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -202,9 +202,9 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, { struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip); struct pwm_state cstate; - u32 ctrl; + u32 ctrl, duty, period, val; int ret; - unsigned int delay_us; + unsigned int delay_us, prescaler; unsigned long now; pwm_get_state(pwm, &cstate); @@ -220,43 +220,37 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, spin_lock(&sun4i_pwm->ctrl_lock); ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG); - if ((cstate.period != state->period) || - (cstate.duty_cycle != state->duty_cycle)) { - u32 period, duty, val; - unsigned int prescaler; - - ret = sun4i_pwm_calculate(sun4i_pwm, state, - &duty, &period, &prescaler); - if (ret) { - dev_err(chip->dev, "period exceeds the maximum value\n"); - spin_unlock(&sun4i_pwm->ctrl_lock); - if (!cstate.enabled) - clk_disable_unprepare(sun4i_pwm->clk); - return ret; - } - - if (PWM_REG_PRESCAL(ctrl, pwm->hwpwm) != prescaler) { - /* Prescaler changed, the clock has to be gated */ - ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm); - sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG); - - ctrl &= ~BIT_CH(PWM_PRESCAL_MASK, pwm->hwpwm); - ctrl |= BIT_CH(prescaler, pwm->hwpwm); - } - - val = (duty & PWM_DTY_MASK) | PWM_PRD(period); - sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm)); - sun4i_pwm->next_period[pwm->hwpwm] = jiffies + - usecs_to_jiffies(cstate.period / 1000 + 1); - sun4i_pwm->needs_delay[pwm->hwpwm] = true; + ret = sun4i_pwm_calculate(sun4i_pwm, state, &duty, &period, &prescaler); + if (ret) { + dev_err(chip->dev, "period exceeds the maximum value\n"); + spin_unlock(&sun4i_pwm->ctrl_lock); + if (!cstate.enabled) + clk_disable_unprepare(sun4i_pwm->clk); + return ret; } + if (PWM_REG_PRESCAL(ctrl, pwm->hwpwm) != prescaler) { + /* Prescaler changed, the clock has to be gated */ + ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm); + sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG); + + ctrl &= ~BIT_CH(PWM_PRESCAL_MASK, pwm->hwpwm); + ctrl |= BIT_CH(prescaler, pwm->hwpwm); + } + + val = (duty & PWM_DTY_MASK) | PWM_PRD(period); + sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm)); + sun4i_pwm->next_period[pwm->hwpwm] = jiffies + + usecs_to_jiffies(cstate.period / 1000 + 1); + sun4i_pwm->needs_delay[pwm->hwpwm] = true; + if (state->polarity != PWM_POLARITY_NORMAL) ctrl &= ~BIT_CH(PWM_ACT_STATE, pwm->hwpwm); else ctrl |= BIT_CH(PWM_ACT_STATE, pwm->hwpwm); ctrl |= BIT_CH(PWM_CLK_GATING, pwm->hwpwm); + if (state->enabled) { ctrl |= BIT_CH(PWM_EN, pwm->hwpwm); } else if (!sun4i_pwm->needs_delay[pwm->hwpwm]) { From 9f28e95b5286fce63a3d0d90dc7ca43eca8dda58 Mon Sep 17 00:00:00 2001 From: Jernej Skrabec Date: Sun, 24 Nov 2019 18:29:07 +0100 Subject: [PATCH 09/37] pwm: sun4i: Add support to output source clock directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PWM core has an option to bypass whole logic and output unchanged source clock as PWM output. This is achieved by enabling bypass bit. Note that when bypass is enabled, no other setting has any meaning, not even enable bit. This mode of operation is needed to achieve high enough frequency to serve as clock source for AC200 chip which is integrated into same package as H6 SoC. Signed-off-by: Jernej Skrabec Signed-off-by: Clément Péron Reviewed-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 48 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index e369b5a398f4..63aa9da92c22 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -3,6 +3,10 @@ * Driver for Allwinner sun4i Pulse Width Modulation Controller * * Copyright (C) 2014 Alexandre Belloni + * + * Limitations: + * - When outputing the source clock directly, the PWM logic will be bypassed + * and the currently running period is not guaranteed to be completed */ #include @@ -73,6 +77,7 @@ static const u32 prescaler_table[] = { struct sun4i_pwm_data { bool has_prescaler_bypass; + bool has_direct_mod_clk_output; unsigned int npwm; }; @@ -118,6 +123,20 @@ static void sun4i_pwm_get_state(struct pwm_chip *chip, val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG); + /* + * PWM chapter in H6 manual has a diagram which explains that if bypass + * bit is set, no other setting has any meaning. Even more, experiment + * proved that also enable bit is ignored in this case. + */ + if ((val & BIT_CH(PWM_BYPASS, pwm->hwpwm)) && + sun4i_pwm->data->has_direct_mod_clk_output) { + state->period = DIV_ROUND_UP_ULL(NSEC_PER_SEC, clk_rate); + state->duty_cycle = DIV_ROUND_UP_ULL(state->period, 2); + state->polarity = PWM_POLARITY_NORMAL; + state->enabled = true; + return; + } + if ((PWM_REG_PRESCAL(val, pwm->hwpwm) == PWM_PRESCAL_MASK) && sun4i_pwm->data->has_prescaler_bypass) prescaler = 1; @@ -149,13 +168,24 @@ static void sun4i_pwm_get_state(struct pwm_chip *chip, static int sun4i_pwm_calculate(struct sun4i_pwm_chip *sun4i_pwm, const struct pwm_state *state, - u32 *dty, u32 *prd, unsigned int *prsclr) + u32 *dty, u32 *prd, unsigned int *prsclr, + bool *bypass) { u64 clk_rate, div = 0; unsigned int pval, prescaler = 0; clk_rate = clk_get_rate(sun4i_pwm->clk); + *bypass = sun4i_pwm->data->has_direct_mod_clk_output && + state->enabled && + (state->period * clk_rate >= NSEC_PER_SEC) && + (state->period * clk_rate < 2 * NSEC_PER_SEC) && + (state->duty_cycle * clk_rate * 2 >= NSEC_PER_SEC); + + /* Skip calculation of other parameters if we bypass them */ + if (*bypass) + return 0; + if (sun4i_pwm->data->has_prescaler_bypass) { /* First, test without any prescaler when available */ prescaler = PWM_PRESCAL_MASK; @@ -206,6 +236,7 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, int ret; unsigned int delay_us, prescaler; unsigned long now; + bool bypass; pwm_get_state(pwm, &cstate); @@ -220,7 +251,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, spin_lock(&sun4i_pwm->ctrl_lock); ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG); - ret = sun4i_pwm_calculate(sun4i_pwm, state, &duty, &period, &prescaler); + ret = sun4i_pwm_calculate(sun4i_pwm, state, &duty, &period, &prescaler, + &bypass); if (ret) { dev_err(chip->dev, "period exceeds the maximum value\n"); spin_unlock(&sun4i_pwm->ctrl_lock); @@ -229,6 +261,18 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, return ret; } + if (sun4i_pwm->data->has_direct_mod_clk_output) { + if (bypass) { + ctrl |= BIT_CH(PWM_BYPASS, pwm->hwpwm); + /* We can skip other parameter */ + sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG); + spin_unlock(&sun4i_pwm->ctrl_lock); + return 0; + } + + ctrl &= ~BIT_CH(PWM_BYPASS, pwm->hwpwm); + } + if (PWM_REG_PRESCAL(ctrl, pwm->hwpwm) != prescaler) { /* Prescaler changed, the clock has to be gated */ ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm); From fdd2c12e3761f0418596cd0e0156719a255d23c8 Mon Sep 17 00:00:00 2001 From: Jernej Skrabec Date: Sun, 24 Nov 2019 18:29:08 +0100 Subject: [PATCH 10/37] pwm: sun4i: Add support for H6 PWM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that sun4i PWM driver supports deasserting reset line and enabling bus clock, support for H6 PWM can be added. Note that while H6 PWM has two channels, only first one is wired to output pin. Second channel is used as a clock source to companion AC200 chip which is bundled into same package. Signed-off-by: Jernej Skrabec Acked-by: Uwe Kleine-König Signed-off-by: Clément Péron Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 63aa9da92c22..1afd41ebd3fd 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -360,6 +360,12 @@ static const struct sun4i_pwm_data sun4i_pwm_single_bypass = { .npwm = 1, }; +static const struct sun4i_pwm_data sun50i_h6_pwm_data = { + .has_prescaler_bypass = true, + .has_direct_mod_clk_output = true, + .npwm = 2, +}; + static const struct of_device_id sun4i_pwm_dt_ids[] = { { .compatible = "allwinner,sun4i-a10-pwm", @@ -376,6 +382,9 @@ static const struct of_device_id sun4i_pwm_dt_ids[] = { }, { .compatible = "allwinner,sun8i-h3-pwm", .data = &sun4i_pwm_single_bypass, + }, { + .compatible = "allwinner,sun50i-h6-pwm", + .data = &sun50i_h6_pwm_data, }, { /* sentinel */ }, From bf29c2ff82fd37f4a5e0c6983016c15430ac6e35 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 4 Oct 2019 15:32:02 +0200 Subject: [PATCH 11/37] pwm: mxs: Implement ->apply() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In preparation for supporting setting the polarity, switch the driver to support the ->apply() method. Signed-off-by: Rasmus Villemoes Reviewed-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-mxs.c | 70 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c index b14376b47ac8..10efd3de0bb3 100644 --- a/drivers/pwm/pwm-mxs.c +++ b/drivers/pwm/pwm-mxs.c @@ -26,6 +26,7 @@ #define PERIOD_PERIOD_MAX 0x10000 #define PERIOD_ACTIVE_HIGH (3 << 16) #define PERIOD_INACTIVE_LOW (2 << 18) +#define PERIOD_POLARITY_NORMAL (PERIOD_ACTIVE_HIGH | PERIOD_INACTIVE_LOW) #define PERIOD_CDIV(div) (((div) & 0x7) << 20) #define PERIOD_CDIV_MAX 8 @@ -41,6 +42,74 @@ struct mxs_pwm_chip { #define to_mxs_pwm_chip(_chip) container_of(_chip, struct mxs_pwm_chip, chip) +static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct mxs_pwm_chip *mxs = to_mxs_pwm_chip(chip); + int ret, div = 0; + unsigned int period_cycles, duty_cycles; + unsigned long rate; + unsigned long long c; + + if (state->polarity != PWM_POLARITY_NORMAL) + return -ENOTSUPP; + + /* + * If the PWM channel is disabled, make sure to turn on the + * clock before calling clk_get_rate() and writing to the + * registers. Otherwise, just keep it enabled. + */ + if (!pwm_is_enabled(pwm)) { + ret = clk_prepare_enable(mxs->clk); + if (ret) + return ret; + } + + if (!state->enabled && pwm_is_enabled(pwm)) + writel(1 << pwm->hwpwm, mxs->base + PWM_CTRL + CLR); + + rate = clk_get_rate(mxs->clk); + while (1) { + c = rate / cdiv[div]; + c = c * state->period; + do_div(c, 1000000000); + if (c < PERIOD_PERIOD_MAX) + break; + div++; + if (div >= PERIOD_CDIV_MAX) + return -EINVAL; + } + + period_cycles = c; + c *= state->duty_cycle; + do_div(c, state->period); + duty_cycles = c; + + /* + * The data sheet the says registers must be written to in + * this order (ACTIVEn, then PERIODn). Also, the new settings + * only take effect at the beginning of a new period, avoiding + * glitches. + */ + writel(duty_cycles << 16, + mxs->base + PWM_ACTIVE0 + pwm->hwpwm * 0x20); + writel(PERIOD_PERIOD(period_cycles) | PERIOD_POLARITY_NORMAL | PERIOD_CDIV(div), + mxs->base + PWM_PERIOD0 + pwm->hwpwm * 0x20); + + if (state->enabled) { + if (!pwm_is_enabled(pwm)) { + /* + * The clock was enabled above. Just enable + * the channel in the control register. + */ + writel(1 << pwm->hwpwm, mxs->base + PWM_CTRL + SET); + } + } else { + clk_disable_unprepare(mxs->clk); + } + return 0; +} + static int mxs_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, int duty_ns, int period_ns) { @@ -116,6 +185,7 @@ static void mxs_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) } static const struct pwm_ops mxs_pwm_ops = { + .apply = mxs_pwm_apply, .config = mxs_pwm_config, .enable = mxs_pwm_enable, .disable = mxs_pwm_disable, From ebbfb1592c8dedb22278211b89ea9b705faad58e Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 4 Oct 2019 15:32:03 +0200 Subject: [PATCH 12/37] pwm: mxs: Remove legacy methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we now have ->apply(), these are no longer relevant. Signed-off-by: Rasmus Villemoes Reviewed-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-mxs.c | 77 ------------------------------------------- 1 file changed, 77 deletions(-) diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c index 10efd3de0bb3..5a6835e18fc6 100644 --- a/drivers/pwm/pwm-mxs.c +++ b/drivers/pwm/pwm-mxs.c @@ -110,85 +110,8 @@ static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, return 0; } -static int mxs_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, - int duty_ns, int period_ns) -{ - struct mxs_pwm_chip *mxs = to_mxs_pwm_chip(chip); - int ret, div = 0; - unsigned int period_cycles, duty_cycles; - unsigned long rate; - unsigned long long c; - - rate = clk_get_rate(mxs->clk); - while (1) { - c = rate / cdiv[div]; - c = c * period_ns; - do_div(c, 1000000000); - if (c < PERIOD_PERIOD_MAX) - break; - div++; - if (div >= PERIOD_CDIV_MAX) - return -EINVAL; - } - - period_cycles = c; - c *= duty_ns; - do_div(c, period_ns); - duty_cycles = c; - - /* - * If the PWM channel is disabled, make sure to turn on the clock - * before writing the register. Otherwise, keep it enabled. - */ - if (!pwm_is_enabled(pwm)) { - ret = clk_prepare_enable(mxs->clk); - if (ret) - return ret; - } - - writel(duty_cycles << 16, - mxs->base + PWM_ACTIVE0 + pwm->hwpwm * 0x20); - writel(PERIOD_PERIOD(period_cycles) | PERIOD_ACTIVE_HIGH | - PERIOD_INACTIVE_LOW | PERIOD_CDIV(div), - mxs->base + PWM_PERIOD0 + pwm->hwpwm * 0x20); - - /* - * If the PWM is not enabled, turn the clock off again to save power. - */ - if (!pwm_is_enabled(pwm)) - clk_disable_unprepare(mxs->clk); - - return 0; -} - -static int mxs_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct mxs_pwm_chip *mxs = to_mxs_pwm_chip(chip); - int ret; - - ret = clk_prepare_enable(mxs->clk); - if (ret) - return ret; - - writel(1 << pwm->hwpwm, mxs->base + PWM_CTRL + SET); - - return 0; -} - -static void mxs_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct mxs_pwm_chip *mxs = to_mxs_pwm_chip(chip); - - writel(1 << pwm->hwpwm, mxs->base + PWM_CTRL + CLR); - - clk_disable_unprepare(mxs->clk); -} - static const struct pwm_ops mxs_pwm_ops = { .apply = mxs_pwm_apply, - .config = mxs_pwm_config, - .enable = mxs_pwm_enable, - .disable = mxs_pwm_disable, .owner = THIS_MODULE, }; From 2cf0f6fece5b97e67c8d5e84fda3790fe213454a Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 4 Oct 2019 15:32:04 +0200 Subject: [PATCH 13/37] pwm: mxs: Add support for inverse polarity If I'm reading of_pwm_xlate_with_flags() right, existing device trees that set #pwm-cells = 2 will continue to work. Signed-off-by: Rasmus Villemoes Signed-off-by: Thierry Reding --- drivers/pwm/pwm-mxs.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c index 5a6835e18fc6..57562221c439 100644 --- a/drivers/pwm/pwm-mxs.c +++ b/drivers/pwm/pwm-mxs.c @@ -25,8 +25,11 @@ #define PERIOD_PERIOD(p) ((p) & 0xffff) #define PERIOD_PERIOD_MAX 0x10000 #define PERIOD_ACTIVE_HIGH (3 << 16) +#define PERIOD_ACTIVE_LOW (2 << 16) +#define PERIOD_INACTIVE_HIGH (3 << 18) #define PERIOD_INACTIVE_LOW (2 << 18) #define PERIOD_POLARITY_NORMAL (PERIOD_ACTIVE_HIGH | PERIOD_INACTIVE_LOW) +#define PERIOD_POLARITY_INVERSE (PERIOD_ACTIVE_LOW | PERIOD_INACTIVE_HIGH) #define PERIOD_CDIV(div) (((div) & 0x7) << 20) #define PERIOD_CDIV_MAX 8 @@ -50,9 +53,7 @@ static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, unsigned int period_cycles, duty_cycles; unsigned long rate; unsigned long long c; - - if (state->polarity != PWM_POLARITY_NORMAL) - return -ENOTSUPP; + unsigned int pol_bits; /* * If the PWM channel is disabled, make sure to turn on the @@ -91,9 +92,12 @@ static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, * only take effect at the beginning of a new period, avoiding * glitches. */ + + pol_bits = state->polarity == PWM_POLARITY_NORMAL ? + PERIOD_POLARITY_NORMAL : PERIOD_POLARITY_INVERSE; writel(duty_cycles << 16, mxs->base + PWM_ACTIVE0 + pwm->hwpwm * 0x20); - writel(PERIOD_PERIOD(period_cycles) | PERIOD_POLARITY_NORMAL | PERIOD_CDIV(div), + writel(PERIOD_PERIOD(period_cycles) | pol_bits | PERIOD_CDIV(div), mxs->base + PWM_PERIOD0 + pwm->hwpwm * 0x20); if (state->enabled) { @@ -135,6 +139,8 @@ static int mxs_pwm_probe(struct platform_device *pdev) mxs->chip.dev = &pdev->dev; mxs->chip.ops = &mxs_pwm_ops; + mxs->chip.of_xlate = of_pwm_xlate_with_flags; + mxs->chip.of_pwm_n_cells = 3; mxs->chip.base = -1; ret = of_property_read_u32(np, "fsl,pwm-number", &mxs->chip.npwm); From 366486e0afaa115ec456db124fc07d824a1c1fb1 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 4 Oct 2019 15:32:05 +0200 Subject: [PATCH 14/37] dt-bindings: pwm: mxs-pwm: Increase #pwm-cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need to increase the pwm-cells for the optional flags parameter, in order to implement support for polarity setting via DT. Acked-by: Rob Herring Signed-off-by: Rasmus Villemoes Acked-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/pwm/mxs-pwm.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/pwm/mxs-pwm.txt b/Documentation/devicetree/bindings/pwm/mxs-pwm.txt index 1b06f86a7091..a1b8a482f873 100644 --- a/Documentation/devicetree/bindings/pwm/mxs-pwm.txt +++ b/Documentation/devicetree/bindings/pwm/mxs-pwm.txt @@ -3,7 +3,7 @@ Freescale MXS PWM controller Required properties: - compatible: should be "fsl,imx23-pwm" - reg: physical base address and length of the controller's registers -- #pwm-cells: should be 2. See pwm.yaml in this directory for a description of +- #pwm-cells: should be 3. See pwm.yaml in this directory for a description of the cells format. - fsl,pwm-number: the number of PWM devices @@ -12,6 +12,6 @@ Example: pwm: pwm@80064000 { compatible = "fsl,imx28-pwm", "fsl,imx23-pwm"; reg = <0x80064000 0x2000>; - #pwm-cells = <2>; + #pwm-cells = <3>; fsl,pwm-number = <8>; }; From 3c64ed74d619c25c65abb1fcafcb1aff835867d5 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 4 Oct 2019 15:32:06 +0200 Subject: [PATCH 15/37] pwm: mxs: Avoid a division in mxs_pwm_apply() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the divisor is not a compile-time constant (unless gcc somehow decided to unroll the loop PERIOD_CDIV_MAX times), this does a somewhat expensive 32/32 division. Replace that with a right shift. We still have a 64/32 division just below, but at least in that case the divisor is compile-time constant. Signed-off-by: Rasmus Villemoes Reviewed-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-mxs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c index 57562221c439..f2e57fcf8f8b 100644 --- a/drivers/pwm/pwm-mxs.c +++ b/drivers/pwm/pwm-mxs.c @@ -33,8 +33,8 @@ #define PERIOD_CDIV(div) (((div) & 0x7) << 20) #define PERIOD_CDIV_MAX 8 -static const unsigned int cdiv[PERIOD_CDIV_MAX] = { - 1, 2, 4, 8, 16, 64, 256, 1024 +static const u8 cdiv_shift[PERIOD_CDIV_MAX] = { + 0, 1, 2, 3, 4, 6, 8, 10 }; struct mxs_pwm_chip { @@ -71,7 +71,7 @@ static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, rate = clk_get_rate(mxs->clk); while (1) { - c = rate / cdiv[div]; + c = rate >> cdiv_shift[div]; c = c * state->period; do_div(c, 1000000000); if (c < PERIOD_PERIOD_MAX) From 3c269ba6d8af044ecdb69c610b24697a43ef71c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sat, 24 Aug 2019 02:10:36 +0200 Subject: [PATCH 16/37] pwm: atmel: Add a hint where to find hardware documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most Microchip (formerly Atmel) chips have publicly available manuals. A comprehensive list is already contained in the documentation folder. Reference this list in the header of the driver to allow reviewers to find the relevant manuals. Signed-off-by: Uwe Kleine-König Acked-by: Nicolas Ferre Signed-off-by: Thierry Reding --- drivers/pwm/pwm-atmel.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c index 9ba733467e26..cd6be47b5160 100644 --- a/drivers/pwm/pwm-atmel.c +++ b/drivers/pwm/pwm-atmel.c @@ -4,6 +4,9 @@ * * Copyright (C) 2013 Atmel Corporation * Bo Shen + * + * Links to reference manuals for the supported PWM chips can be found in + * Documentation/arm/microchip.rst. */ #include From ff55e7a314143e717f4d96271b541c29111502df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sat, 24 Aug 2019 02:10:37 +0200 Subject: [PATCH 17/37] pwm: atmel: Use a constant for maximum prescale value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The maximal prescale value is 10 for all supported variants. So drop the member in the variant description and introduce a global constant instead. This reduces the size of the variant descriptions and the .apply() callback can be compiled a bit more effectively. Acked-by: Claudiu Beznea Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-atmel.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c index cd6be47b5160..199255b26aaa 100644 --- a/drivers/pwm/pwm-atmel.c +++ b/drivers/pwm/pwm-atmel.c @@ -50,6 +50,8 @@ #define PWMV2_CPRD 0x0C #define PWMV2_CPRDUPD 0x10 +#define PWM_MAX_PRES 10 + struct atmel_pwm_registers { u8 period; u8 period_upd; @@ -59,7 +61,6 @@ struct atmel_pwm_registers { struct atmel_pwm_config { u32 max_period; - u32 max_pres; }; struct atmel_pwm_data { @@ -126,7 +127,7 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip, for (*pres = 0; cycles > atmel_pwm->data->cfg.max_period; cycles >>= 1) (*pres)++; - if (*pres > atmel_pwm->data->cfg.max_pres) { + if (*pres > PWM_MAX_PRES) { dev_err(chip->dev, "pres exceeds the maximum value\n"); return -EINVAL; } @@ -289,7 +290,6 @@ static const struct atmel_pwm_data atmel_sam9rl_pwm_data = { .cfg = { /* 16 bits to keep period and duty. */ .max_period = 0xffff, - .max_pres = 10, }, }; @@ -303,7 +303,6 @@ static const struct atmel_pwm_data atmel_sama5_pwm_data = { .cfg = { /* 16 bits to keep period and duty. */ .max_period = 0xffff, - .max_pres = 10, }, }; @@ -317,7 +316,6 @@ static const struct atmel_pwm_data mchp_sam9x60_pwm_data = { .cfg = { /* 32 bits to keep period and duty. */ .max_period = 0xffffffff, - .max_pres = 10, }, }; From 2101c878f767fc6341ce0057e3b53d6129e76d99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sat, 24 Aug 2019 02:10:38 +0200 Subject: [PATCH 18/37] pwm: atmel: Replace loop in prescale calculation by ad-hoc calculation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The calculated values are the same with the modified algorithm. The only difference is that the calculation is a bit more efficient. Acked-by: Claudiu Beznea Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-atmel.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c index 199255b26aaa..c9fdf1671bb3 100644 --- a/drivers/pwm/pwm-atmel.c +++ b/drivers/pwm/pwm-atmel.c @@ -60,7 +60,7 @@ struct atmel_pwm_registers { }; struct atmel_pwm_config { - u32 max_period; + u32 period_bits; }; struct atmel_pwm_data { @@ -119,17 +119,27 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip, { struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip); unsigned long long cycles = state->period; + int shift; /* Calculate the period cycles and prescale value */ cycles *= clk_get_rate(atmel_pwm->clk); do_div(cycles, NSEC_PER_SEC); - for (*pres = 0; cycles > atmel_pwm->data->cfg.max_period; cycles >>= 1) - (*pres)++; + /* + * The register for the period length is cfg.period_bits bits wide. + * So for each bit the number of clock cycles is wider divide the input + * clock frequency by two using pres and shift cprd accordingly. + */ + shift = fls(cycles) - atmel_pwm->data->cfg.period_bits; - if (*pres > PWM_MAX_PRES) { + if (shift > PWM_MAX_PRES) { dev_err(chip->dev, "pres exceeds the maximum value\n"); return -EINVAL; + } else if (shift > 0) { + *pres = shift; + cycles >>= *pres; + } else { + *pres = 0; } *cprd = cycles; @@ -289,7 +299,7 @@ static const struct atmel_pwm_data atmel_sam9rl_pwm_data = { }, .cfg = { /* 16 bits to keep period and duty. */ - .max_period = 0xffff, + .period_bits = 16, }, }; @@ -302,7 +312,7 @@ static const struct atmel_pwm_data atmel_sama5_pwm_data = { }, .cfg = { /* 16 bits to keep period and duty. */ - .max_period = 0xffff, + .period_bits = 16, }, }; @@ -315,7 +325,7 @@ static const struct atmel_pwm_data mchp_sam9x60_pwm_data = { }, .cfg = { /* 32 bits to keep period and duty. */ - .max_period = 0xffffffff, + .period_bits = 32, }, }; From 998d189a817b8aae5b9e416c40dcb35b1fd79d3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sat, 24 Aug 2019 02:10:39 +0200 Subject: [PATCH 19/37] pwm: atmel: Document known weaknesses of both hardware and software MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This documents the my findings while reading through the driver and the reference manual. Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-atmel.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c index c9fdf1671bb3..3ff3c171437e 100644 --- a/drivers/pwm/pwm-atmel.c +++ b/drivers/pwm/pwm-atmel.c @@ -7,6 +7,16 @@ * * Links to reference manuals for the supported PWM chips can be found in * Documentation/arm/microchip.rst. + * + * Limitations: + * - Periods start with the inactive level. + * - Hardware has to be stopped in general to update settings. + * + * Software bugs/possible improvements: + * - When atmel_pwm_apply() is called with state->enabled=false a change in + * state->polarity isn't honored. + * - Instead of sleeping to wait for a completed period, the interrupt + * functionality could be used. */ #include From 02afb811e0cf96958ac69c4b3372088180c829a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sat, 24 Aug 2019 02:10:40 +0200 Subject: [PATCH 20/37] pwm: atmel: Use register accessors for channels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes it a bit easier when instrumenting register access to only have to add code in one place. Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-atmel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c index 3ff3c171437e..b8b47c315ac0 100644 --- a/drivers/pwm/pwm-atmel.c +++ b/drivers/pwm/pwm-atmel.c @@ -111,7 +111,7 @@ static inline u32 atmel_pwm_ch_readl(struct atmel_pwm_chip *chip, { unsigned long base = PWM_CH_REG_OFFSET + ch * PWM_CH_REG_SIZE; - return readl_relaxed(chip->base + base + offset); + return atmel_pwm_readl(chip, base + offset); } static inline void atmel_pwm_ch_writel(struct atmel_pwm_chip *chip, @@ -120,7 +120,7 @@ static inline void atmel_pwm_ch_writel(struct atmel_pwm_chip *chip, { unsigned long base = PWM_CH_REG_OFFSET + ch * PWM_CH_REG_SIZE; - writel_relaxed(val, chip->base + base + offset); + atmel_pwm_writel(chip, base + offset, val); } static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip, From 651b510a74d4e473281efc4eaf2a83963988de48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sat, 24 Aug 2019 02:10:41 +0200 Subject: [PATCH 21/37] pwm: atmel: Implement .get_state() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function reads back the configured parameters from the hardware. As .apply() rounds down (mostly) I'm rounding up in .get_state() to achieve that applying a state just read from hardware is a no-op. Signed-off-by: Uwe Kleine-König Acked-by: Claudiu Beznea Signed-off-by: Thierry Reding --- drivers/pwm/pwm-atmel.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c index b8b47c315ac0..6161e7e3e9ac 100644 --- a/drivers/pwm/pwm-atmel.c +++ b/drivers/pwm/pwm-atmel.c @@ -295,8 +295,48 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, return 0; } +static void atmel_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) +{ + struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip); + u32 sr, cmr; + + sr = atmel_pwm_readl(atmel_pwm, PWM_SR); + cmr = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR); + + if (sr & (1 << pwm->hwpwm)) { + unsigned long rate = clk_get_rate(atmel_pwm->clk); + u32 cdty, cprd, pres; + u64 tmp; + + pres = cmr & PWM_CMR_CPRE_MSK; + + cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, + atmel_pwm->data->regs.period); + tmp = (u64)cprd * NSEC_PER_SEC; + tmp <<= pres; + state->period = DIV64_U64_ROUND_UP(tmp, rate); + + cdty = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, + atmel_pwm->data->regs.duty); + tmp = (u64)cdty * NSEC_PER_SEC; + tmp <<= pres; + state->duty_cycle = DIV64_U64_ROUND_UP(tmp, rate); + + state->enabled = true; + } else { + state->enabled = false; + } + + if (cmr & PWM_CMR_CPOL) + state->polarity = PWM_POLARITY_INVERSED; + else + state->polarity = PWM_POLARITY_NORMAL; +} + static const struct pwm_ops atmel_pwm_ops = { .apply = atmel_pwm_apply, + .get_state = atmel_pwm_get_state, .owner = THIS_MODULE, }; From f24e564129f3b7508eff363353099657ff774d2b Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Mon, 30 Dec 2019 18:21:12 +0100 Subject: [PATCH 22/37] pwm: Fix minor Kconfig whitespace issues Remove double whitespace after "config" keyword. Signed-off-by: Krzysztof Kozlowski Signed-off-by: Thierry Reding --- drivers/pwm/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index bd21655c37a6..c865d688f6b4 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -490,7 +490,7 @@ config PWM_TEGRA To compile this driver as a module, choose M here: the module will be called pwm-tegra. -config PWM_TIECAP +config PWM_TIECAP tristate "ECAP PWM support" depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_KEYSTONE || ARCH_K3 help @@ -499,7 +499,7 @@ config PWM_TIECAP To compile this driver as a module, choose M here: the module will be called pwm-tiecap. -config PWM_TIEHRPWM +config PWM_TIEHRPWM tristate "EHRPWM PWM support" depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_K3 help From bca900829d6035a782d8209c834400ed44539c1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Thu, 24 Oct 2019 09:14:09 +0200 Subject: [PATCH 23/37] pwm: rcar: Drop useless call to pwm_get_state() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pwm_get_state has no side effects and the resulting pwm_state is unused. So drop the call to pwm_get_state() and the local variable from rcar_pwm_apply(). The call was introduced in commit 7f68ce8287d3 ("pwm: rcar: Add support "atomic" API") and already then was useless. Signed-off-by: Uwe Kleine-König Reviewed-by: Yoshihiro Shimoda Signed-off-by: Thierry Reding --- drivers/pwm/pwm-rcar.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c index 852eb2347954..6fac8eb98d54 100644 --- a/drivers/pwm/pwm-rcar.c +++ b/drivers/pwm/pwm-rcar.c @@ -161,11 +161,9 @@ static int rcar_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, const struct pwm_state *state) { struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip); - struct pwm_state cur_state; int div, ret; /* This HW/driver only supports normal polarity */ - pwm_get_state(pwm, &cur_state); if (state->polarity != PWM_POLARITY_NORMAL) return -ENOTSUPP; From af4fab8bedcfe87f297c65ded128e7ee036488c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Thu, 24 Oct 2019 09:14:10 +0200 Subject: [PATCH 24/37] pwm: rcar: Document inability to set duty_cycle = 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When .apply() is called with state->duty_cycle = 0 the duty_ns parameter to rcar_pwm_set_counter() is 0 which results in ph being 0 and rcar_pwm_set_counter() returning -EINVAL. Signed-off-by: Uwe Kleine-König Reviewed-by: Yoshihiro Shimoda Signed-off-by: Thierry Reding --- drivers/pwm/pwm-rcar.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c index 6fac8eb98d54..2685577b6dd4 100644 --- a/drivers/pwm/pwm-rcar.c +++ b/drivers/pwm/pwm-rcar.c @@ -3,6 +3,9 @@ * R-Car PWM Timer driver * * Copyright (C) 2015 Renesas Electronics Corporation + * + * Limitations: + * - The hardware cannot generate a 0% duty cycle. */ #include From 1188829abc2abbef0da7a082fdb6a315a050a0a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Thu, 24 Oct 2019 10:08:29 +0200 Subject: [PATCH 25/37] pwm: Implement tracing for .get_state() and .apply_state() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows to log all calls to the driver's lowlevel functions which simplifies debugging in some cases. Signed-off-by: Uwe Kleine-König Reviewed-by: Steven Rostedt (VMware) Signed-off-by: Thierry Reding --- drivers/pwm/core.c | 9 +++++- include/trace/events/pwm.h | 58 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 include/trace/events/pwm.h diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index e067873c6cc5..5a7f6598c05f 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -20,6 +20,9 @@ #include +#define CREATE_TRACE_POINTS +#include + #define MAX_PWMS 1024 static DEFINE_MUTEX(pwm_lookup_lock); @@ -114,8 +117,10 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label) } } - if (pwm->chip->ops->get_state) + if (pwm->chip->ops->get_state) { pwm->chip->ops->get_state(pwm->chip, pwm, &pwm->state); + trace_pwm_get(pwm, &pwm->state); + } set_bit(PWMF_REQUESTED, &pwm->flags); pwm->label = label; @@ -472,6 +477,8 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state) if (err) return err; + trace_pwm_apply(pwm, state); + pwm->state = *state; } else { /* diff --git a/include/trace/events/pwm.h b/include/trace/events/pwm.h new file mode 100644 index 000000000000..cf243de41cc8 --- /dev/null +++ b/include/trace/events/pwm.h @@ -0,0 +1,58 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#undef TRACE_SYSTEM +#define TRACE_SYSTEM pwm + +#if !defined(_TRACE_PWM_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_PWM_H + +#include +#include + +DECLARE_EVENT_CLASS(pwm, + + TP_PROTO(struct pwm_device *pwm, const struct pwm_state *state), + + TP_ARGS(pwm, state), + + TP_STRUCT__entry( + __field(struct pwm_device *, pwm) + __field(u64, period) + __field(u64, duty_cycle) + __field(enum pwm_polarity, polarity) + __field(bool, enabled) + ), + + TP_fast_assign( + __entry->pwm = pwm; + __entry->period = state->period; + __entry->duty_cycle = state->duty_cycle; + __entry->polarity = state->polarity; + __entry->enabled = state->enabled; + ), + + TP_printk("%p: period=%llu duty_cycle=%llu polarity=%d enabled=%d", + __entry->pwm, __entry->period, __entry->duty_cycle, + __entry->polarity, __entry->enabled) + +); + +DEFINE_EVENT(pwm, pwm_apply, + + TP_PROTO(struct pwm_device *pwm, const struct pwm_state *state), + + TP_ARGS(pwm, state) + +); + +DEFINE_EVENT(pwm, pwm_get, + + TP_PROTO(struct pwm_device *pwm, const struct pwm_state *state), + + TP_ARGS(pwm, state) + +); + +#endif /* _TRACE_PWM_H */ + +/* This part must be outside protection */ +#include From 43efdc8f0e6d7088ec61bd55a73bf853f002d043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Mon, 11 Nov 2019 10:03:54 +0100 Subject: [PATCH 26/37] pwm: omap-dmtimer: Remove PWM chip in .remove before making it unfunctional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the old code (e.g.) mutex_destroy() was called before pwmchip_remove(). Between these two calls it is possible that a PWM callback is used which tries to grab the mutex. Fixes: 6604c6556db9 ("pwm: Add PWM driver for OMAP using dual-mode timers") Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-omap-dmtimer.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index 00772fc53490..bdf94c78655f 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -351,6 +351,11 @@ put: static int pwm_omap_dmtimer_remove(struct platform_device *pdev) { struct pwm_omap_dmtimer_chip *omap = platform_get_drvdata(pdev); + int ret; + + ret = pwmchip_remove(&omap->chip); + if (ret) + return ret; if (pm_runtime_active(&omap->dm_timer_pdev->dev)) omap->pdata->stop(omap->dm_timer); @@ -359,7 +364,7 @@ static int pwm_omap_dmtimer_remove(struct platform_device *pdev) mutex_destroy(&omap->mutex); - return pwmchip_remove(&omap->chip); + return 0; } static const struct of_device_id pwm_omap_dmtimer_of_match[] = { From c4cf7aa57eb83b108d2d9c6c37c143388fee2a4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Mon, 11 Nov 2019 10:03:55 +0100 Subject: [PATCH 27/37] pwm: omap-dmtimer: Simplify error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of doing error handling in the middle of ->probe(), move error handling and freeing the reference to timer to the end. This fixes a resource leak as dm_timer wasn't freed when allocating *omap failed. Implementation note: The put: label was never reached without a goto and ret being unequal to 0, so the removed return statement is fine. Fixes: 6604c6556db9 ("pwm: Add PWM driver for OMAP using dual-mode timers") Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-omap-dmtimer.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index bdf94c78655f..e36fcad668a6 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -298,15 +298,10 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) goto put; } -put: - of_node_put(timer); - if (ret < 0) - return ret; - omap = devm_kzalloc(&pdev->dev, sizeof(*omap), GFP_KERNEL); if (!omap) { - pdata->free(dm_timer); - return -ENOMEM; + ret = -ENOMEM; + goto err_alloc_omap; } omap->pdata = pdata; @@ -339,13 +334,28 @@ put: ret = pwmchip_add(&omap->chip); if (ret < 0) { dev_err(&pdev->dev, "failed to register PWM\n"); - omap->pdata->free(omap->dm_timer); - return ret; + goto err_pwmchip_add; } + of_node_put(timer); + platform_set_drvdata(pdev, omap); return 0; + +err_pwmchip_add: + + /* + * *omap is allocated using devm_kzalloc, + * so no free necessary here + */ +err_alloc_omap: + + pdata->free(dm_timer); +put: + of_node_put(timer); + + return ret; } static int pwm_omap_dmtimer_remove(struct platform_device *pdev) From c7cb3a1dd53f63c64fb2b567d0be130b92a44d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Mon, 11 Nov 2019 10:03:56 +0100 Subject: [PATCH 28/37] pwm: omap-dmtimer: put_device() after of_find_device_by_node() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was found by coccicheck: drivers/pwm/pwm-omap-dmtimer.c:304:2-8: ERROR: missing put_device; call of_find_device_by_node on line 255, but without a corresponding object release within this function. Reported-by: Markus Elfring Fixes: 6604c6556db9 ("pwm: Add PWM driver for OMAP using dual-mode timers") Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-omap-dmtimer.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index e36fcad668a6..88a3c5690fea 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -256,7 +256,7 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) if (!timer_pdev) { dev_err(&pdev->dev, "Unable to find Timer pdev\n"); ret = -ENODEV; - goto put; + goto err_find_timer_pdev; } timer_pdata = dev_get_platdata(&timer_pdev->dev); @@ -264,7 +264,7 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) dev_dbg(&pdev->dev, "dmtimer pdata structure NULL, deferring probe\n"); ret = -EPROBE_DEFER; - goto put; + goto err_platdata; } pdata = timer_pdata->timer_ops; @@ -283,19 +283,19 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev) !pdata->write_counter) { dev_err(&pdev->dev, "Incomplete dmtimer pdata structure\n"); ret = -EINVAL; - goto put; + goto err_platdata; } if (!of_get_property(timer, "ti,timer-pwm", NULL)) { dev_err(&pdev->dev, "Missing ti,timer-pwm capability\n"); ret = -ENODEV; - goto put; + goto err_timer_property; } dm_timer = pdata->request_by_node(timer); if (!dm_timer) { ret = -EPROBE_DEFER; - goto put; + goto err_request_timer; } omap = devm_kzalloc(&pdev->dev, sizeof(*omap), GFP_KERNEL); @@ -352,7 +352,14 @@ err_pwmchip_add: err_alloc_omap: pdata->free(dm_timer); -put: +err_request_timer: + +err_timer_property: +err_platdata: + + put_device(&timer_pdev->dev); +err_find_timer_pdev: + of_node_put(timer); return ret; @@ -372,6 +379,8 @@ static int pwm_omap_dmtimer_remove(struct platform_device *pdev) omap->pdata->free(omap->dm_timer); + put_device(&omap->dm_timer_pdev->dev); + mutex_destroy(&omap->mutex); return 0; From 9f2919e9b40ec480609ee1f45567531de06ab3fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Mon, 11 Nov 2019 10:03:57 +0100 Subject: [PATCH 29/37] pwm: omap-dmtimer: Allow compiling with COMPILE_TEST MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dependency on OMAP_DM_TIMER is only a runtime dependency. Also OMAP_DM_TIMER cannot be enabled without ARCH_OMAP being enabled. Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index c865d688f6b4..5d8dcccd26ca 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -328,7 +328,8 @@ config PWM_MXS config PWM_OMAP_DMTIMER tristate "OMAP Dual-Mode Timer PWM support" - depends on OF && ARCH_OMAP && OMAP_DM_TIMER + depends on OF + depends on OMAP_DM_TIMER || COMPILE_TEST help Generic PWM framework driver for OMAP Dual-Mode Timer PWM output From 3e954d9626895d374704bb49a8fb538a974ebcf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20P=C3=A9ron?= Date: Mon, 13 Jan 2020 10:23:13 +0100 Subject: [PATCH 30/37] pwm: sun4i: Move pwm_calculate() out of spin_lock() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pwm_calculate() calls clk_get_rate() while holding a spin_lock(). This create an issue as clk_get_rate() may sleep. Move pwm_calculate() out of this spin_lock(). Fixes: c32c5c50d4fe ("pwm: sun4i: Switch to atomic PWM") Reported-by: Alexander Finger Sugested-by: Vasily Khoruzhick Tested-by: Alexander Finger Reviewed-by: Uwe Kleine-König Signed-off-by: Clément Péron Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 1afd41ebd3fd..6b230029dc49 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -248,19 +248,18 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, } } - spin_lock(&sun4i_pwm->ctrl_lock); - ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG); - ret = sun4i_pwm_calculate(sun4i_pwm, state, &duty, &period, &prescaler, &bypass); if (ret) { dev_err(chip->dev, "period exceeds the maximum value\n"); - spin_unlock(&sun4i_pwm->ctrl_lock); if (!cstate.enabled) clk_disable_unprepare(sun4i_pwm->clk); return ret; } + spin_lock(&sun4i_pwm->ctrl_lock); + ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG); + if (sun4i_pwm->data->has_direct_mod_clk_output) { if (bypass) { ctrl |= BIT_CH(PWM_BYPASS, pwm->hwpwm); From cba8d3bfdc967561b1c953f67d2053d526ae1f88 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 9 Jan 2020 01:27:35 -0600 Subject: [PATCH 31/37] pwm: sun4i: Fix inconsistent IS_ERR and PTR_ERR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix inconsistent IS_ERR and PTR_ERR in sun4i_pwm_probe(). The proper pointers to be passed as arguments are pwm->clk and pwm->bus_clk. This bug was detected with the help of Coccinelle. Fixes: b8d74644f34a ("pwm: sun4i: Prefer "mod" clock to unnamed") Fixes: 5b090b430d75 ("pwm: sun4i: Add an optional probe for bus clock") Signed-off-by: Gustavo A. R. Silva Reviewed-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 6b230029dc49..a573e9d147c2 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -422,7 +422,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev) */ pwm->clk = devm_clk_get_optional(&pdev->dev, "mod"); if (IS_ERR(pwm->clk)) { - if (PTR_ERR(pwm->rst) != -EPROBE_DEFER) + if (PTR_ERR(pwm->clk) != -EPROBE_DEFER) dev_err(&pdev->dev, "get mod clock failed %pe\n", pwm->clk); return PTR_ERR(pwm->clk); @@ -431,7 +431,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev) if (!pwm->clk) { pwm->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(pwm->clk)) { - if (PTR_ERR(pwm->rst) != -EPROBE_DEFER) + if (PTR_ERR(pwm->clk) != -EPROBE_DEFER) dev_err(&pdev->dev, "get unnamed clock failed %pe\n", pwm->clk); return PTR_ERR(pwm->clk); @@ -440,7 +440,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev) pwm->bus_clk = devm_clk_get_optional(&pdev->dev, "bus"); if (IS_ERR(pwm->bus_clk)) { - if (PTR_ERR(pwm->rst) != -EPROBE_DEFER) + if (PTR_ERR(pwm->bus_clk) != -EPROBE_DEFER) dev_err(&pdev->dev, "get bus clock failed %pe\n", pwm->bus_clk); return PTR_ERR(pwm->bus_clk); From a368c34340c2543592f7bcd8941b797091a9e74a Mon Sep 17 00:00:00 2001 From: Anson Huang Date: Mon, 30 Dec 2019 11:02:40 +0800 Subject: [PATCH 32/37] pwm: imx27: Eliminate error message for defer probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For defer probe error, no need to output error message which will cause confusion. Signed-off-by: Anson Huang Acked-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-imx27.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c index 59d8b1289808..35a7ac42269c 100644 --- a/drivers/pwm/pwm-imx27.c +++ b/drivers/pwm/pwm-imx27.c @@ -319,9 +319,13 @@ static int pwm_imx27_probe(struct platform_device *pdev) imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); if (IS_ERR(imx->clk_ipg)) { - dev_err(&pdev->dev, "getting ipg clock failed with %ld\n", - PTR_ERR(imx->clk_ipg)); - return PTR_ERR(imx->clk_ipg); + int ret = PTR_ERR(imx->clk_ipg); + + if (ret != -EPROBE_DEFER) + dev_err(&pdev->dev, + "getting ipg clock failed with %d\n", + ret); + return ret; } imx->clk_per = devm_clk_get(&pdev->dev, "per"); From fdf47ff69d61a4e77f5536ee44bd869334053cb6 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Mon, 9 Dec 2019 15:25:03 -0800 Subject: [PATCH 33/37] pwm: bcm2835: Allow building for ARCH_BRCMSTB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BCM7211 is supported using ARCH_BRCMSTB and uses this PWM controller driver, make it possible to build it. Signed-off-by: Florian Fainelli Acked-by: Uwe Kleine-König 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 5d8dcccd26ca..30190beeb6e9 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -100,7 +100,7 @@ config PWM_BCM_KONA config PWM_BCM2835 tristate "BCM2835 PWM support" - depends on ARCH_BCM2835 + depends on ARCH_BCM2835 || ARCH_BRCMSTB help PWM framework driver for BCM2835 controller (Raspberry Pi) From f6003f948226c87b6ce0e5130a43f7b705fe0fd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Tue, 10 Dec 2019 11:24:44 +0100 Subject: [PATCH 34/37] pwm: sun4i: Narrow scope of local variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The variable pval is only used in a single block in the function sun4i_pwm_calculate(). So declare it in a more local scope to simplify the function for humans and compilers. While at it also simplify assignment to pval. While the diffstat for this patch is negative for this patch I still thing the advantage of having a narrower scope is beneficial. In my compiler / .config setup (gcc 8.2.1, arm/imx_v6_v7_defconfig + COMPILE_TEST + PWM_SUN4I) this change doesn't result in any binary changes. Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index a573e9d147c2..0decc7cde133 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -172,7 +172,7 @@ static int sun4i_pwm_calculate(struct sun4i_pwm_chip *sun4i_pwm, bool *bypass) { u64 clk_rate, div = 0; - unsigned int pval, prescaler = 0; + unsigned int prescaler = 0; clk_rate = clk_get_rate(sun4i_pwm->clk); @@ -203,9 +203,11 @@ static int sun4i_pwm_calculate(struct sun4i_pwm_chip *sun4i_pwm, if (prescaler == 0) { /* Go up from the first divider */ for (prescaler = 0; prescaler < PWM_PRESCAL_MASK; prescaler++) { - if (!prescaler_table[prescaler]) + unsigned int pval = prescaler_table[prescaler]; + + if (!pval) continue; - pval = prescaler_table[prescaler]; + div = clk_rate; do_div(div, pval); div = div * state->period; From 0c73201c5c094256236b129799ab6761b491d8cd Mon Sep 17 00:00:00 2001 From: Fabrice Gasnier Date: Thu, 21 Nov 2019 11:58:00 +0100 Subject: [PATCH 35/37] pwm: stm32: Remove automatic output enable Don't use AOE (automatic output enable) by default. In case of break events, PWM is automatically re-enabled on next PWM cycle otherwise. Signed-off-by: Fabrice Gasnier Signed-off-by: Thierry Reding --- drivers/pwm/pwm-stm32.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index 7ff48c14fae8..d3be944f2ae9 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -377,9 +377,7 @@ static int stm32_pwm_config(struct stm32_pwm *priv, int ch, else regmap_update_bits(priv->regmap, TIM_CCMR2, mask, ccmr); - regmap_update_bits(priv->regmap, TIM_BDTR, - TIM_BDTR_MOE | TIM_BDTR_AOE, - TIM_BDTR_MOE | TIM_BDTR_AOE); + regmap_update_bits(priv->regmap, TIM_BDTR, TIM_BDTR_MOE, TIM_BDTR_MOE); return 0; } From 413c2a110d649a3a313ee91920e3d9373b9f2045 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 20 Jan 2020 15:22:37 +0100 Subject: [PATCH 36/37] pwm: sun4i: Initialize variables before use GCC can't always determine that the duty, period and prescaler values are initialized when returning from sun4i_pwm_calculate(), so help out a little by initializing them to 0. Signed-off-by: Thierry Reding --- drivers/pwm/pwm-sun4i.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 0decc7cde133..3e3efa6c768f 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -234,9 +234,9 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, { struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip); struct pwm_state cstate; - u32 ctrl, duty, period, val; + u32 ctrl, duty = 0, period = 0, val; int ret; - unsigned int delay_us, prescaler; + unsigned int delay_us, prescaler = 0; unsigned long now; bool bypass; From 9871abffc81048e20f02e15d6aa4558a44ad53ea Mon Sep 17 00:00:00 2001 From: yu kuai Date: Mon, 20 Jan 2020 19:51:43 +0800 Subject: [PATCH 37/37] pwm: Remove set but not set variable 'pwm' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes gcc '-Wunused-but-set-variable' warning: drivers/pwm/pwm-pca9685.c: In function ‘pca9685_pwm_gpio_free’: drivers/pwm/pwm-pca9685.c:162:21: warning: variable ‘pwm’ set but not used [-Wunused-but-set-variable] It is never used, and so can be removed. In that case, hold and release the lock 'pca->lock' can be removed since nothing will be done between them. Fixes: e926b12c611c ("pwm: Clear chip_data in pwm_put()") Signed-off-by: yu kuai Acked-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-pca9685.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c index 168684b02ebc..b07bdca3d510 100644 --- a/drivers/pwm/pwm-pca9685.c +++ b/drivers/pwm/pwm-pca9685.c @@ -159,13 +159,9 @@ static void pca9685_pwm_gpio_set(struct gpio_chip *gpio, unsigned int offset, static void pca9685_pwm_gpio_free(struct gpio_chip *gpio, unsigned int offset) { struct pca9685 *pca = gpiochip_get_data(gpio); - struct pwm_device *pwm; pca9685_pwm_gpio_set(gpio, offset, 0); pm_runtime_put(pca->chip.dev); - mutex_lock(&pca->lock); - pwm = &pca->chip.pwms[offset]; - mutex_unlock(&pca->lock); } static int pca9685_pwm_gpio_get_direction(struct gpio_chip *chip,