This series adds support for setting the polarity via DT to the
pwm-mxs driver.
The DT binding is updated, but I'm not touching the existing .dts or
.dtsi files - it seems that the same was done for bcm2835 in commits
46421d9d8e802e570dfa4d793a4938d2642ec7a7 and
8a88b2a2017d1e7e80db53080baff591fd454722, while
arch/arm/boot/dts/bcm283x.dtsi still has #pwm-cells = <2>.
v2:
- Rebase to v5.4-rc1
- Address comments from Uwe.
- Add Rob's ack to patch 4.
- New patches 5 and 6. The last one is independent of the others, but
I stumbled on this when rebasing and found the signature had
changed.
Rasmus Villemoes (6):
pwm: mxs: implement ->apply
pwm: mxs: remove legacy methods
pwm: mxs: add support for inverse polarity
dt-bindings: pwm: mxs-pwm: Increase #pwm-cells
pwm: mxs: avoid a division in mxs_pwm_apply()
pwm: update comment on struct pwm_ops::apply
.../devicetree/bindings/pwm/mxs-pwm.txt | 4 +-
drivers/pwm/pwm-mxs.c | 101 +++++++++---------
include/linux/pwm.h | 5 +-
3 files changed, 53 insertions(+), 57 deletions(-)
--
2.20.1
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 <[email protected]>
---
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);
--
2.20.1
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 <[email protected]>
---
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)
--
2.20.1
Commit 71523d1812ac (pwm: Ensure pwm_apply_state() doesn't modify the
state argument) updated the kernel-doc for pwm_apply_state(), but not
for the ->apply callback in the pwm_ops struct.
Signed-off-by: Rasmus Villemoes <[email protected]>
---
include/linux/pwm.h | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index b2c9c460947d..0ef808d925bb 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -243,10 +243,7 @@ pwm_set_relative_duty_cycle(struct pwm_state *state, unsigned int duty_cycle,
* @request: optional hook for requesting a PWM
* @free: optional hook for freeing a PWM
* @capture: capture and report PWM signal
- * @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)
+ * @apply: atomically apply a new PWM config
* @get_state: get the current PWM state. This function is only
* called once per PWM device when the PWM chip is
* registered.
--
2.20.1
In preparation for supporting setting the polarity, switch the driver
to support the ->apply method.
Signed-off-by: Rasmus Villemoes <[email protected]>
---
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,
--
2.20.1
Since we now have ->apply, these are no longer relevant.
Signed-off-by: Rasmus Villemoes <[email protected]>
---
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,
};
--
2.20.1
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 <[email protected]>
Signed-off-by: Rasmus Villemoes <[email protected]>
---
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 96cdde5f6208..1697dcd3b07c 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.txt in this directory for a description of
+- #pwm-cells: should be 3. See pwm.txt 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>;
};
--
2.20.1
On Fri, Oct 04, 2019 at 03:32:06PM +0200, Rasmus Villemoes wrote:
> 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 <[email protected]>
> ---
> 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
One small nitpick: I would like to see this name have a mxs_pwm_ prefix.
But even without this change:
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Fri, Oct 04, 2019 at 03:32:07PM +0200, Rasmus Villemoes wrote:
> Commit 71523d1812ac (pwm: Ensure pwm_apply_state() doesn't modify the
> state argument) updated the kernel-doc for pwm_apply_state(), but not
> for the ->apply callback in the pwm_ops struct.
>
> Signed-off-by: Rasmus Villemoes <[email protected]>
> ---
> include/linux/pwm.h | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index b2c9c460947d..0ef808d925bb 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -243,10 +243,7 @@ pwm_set_relative_duty_cycle(struct pwm_state *state, unsigned int duty_cycle,
> * @request: optional hook for requesting a PWM
> * @free: optional hook for freeing a PWM
> * @capture: capture and report PWM signal
> - * @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)
> + * @apply: atomically apply a new PWM config
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Hello,
On Fri, Oct 04, 2019 at 03:32:02PM +0200, Rasmus Villemoes wrote:
> In preparation for supporting setting the polarity, switch the driver
> to support the ->apply method.
>
> Signed-off-by: Rasmus Villemoes <[email protected]>
> ---
> 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);
@Thierry: I wonder if it would be beneficial to stop the calculation of
register contents if !state->enabled here. The only drawback (I'm aware)
is that pwm_get_state won't return the previously set .period and
.duty_cycle. (I also wonder if we should return (e.g.) .duty = 0,
.period = 1 in pwm_get_state() if the PWM is off.)
For the patch (which is orthogonal regarding the above question):
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Fri, Oct 04, 2019 at 03:32:04PM +0200, Rasmus Villemoes wrote:
> If I'm reading of_pwm_xlate_with_flags() right, existing device trees
> that set #pwm-cells = 2 will continue to work.
Yes, that's what I expect, too.
> Signed-off-by: Rasmus Villemoes <[email protected]>
> ---
> 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);
Is the avoidance of glitches still true when period changes? I assume
that yes, but I wonder if you tested that.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Fri, Oct 04, 2019 at 03:32:03PM +0200, Rasmus Villemoes wrote:
> Since we now have ->apply, these are no longer relevant.
>
> Signed-off-by: Rasmus Villemoes <[email protected]>
nice and easy,
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Fri, Oct 04, 2019 at 03:32:05PM +0200, Rasmus Villemoes wrote:
> 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 <[email protected]>
Acked-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Fri, Oct 4, 2019 at 6:33 AM Rasmus Villemoes
<[email protected]> wrote:
>
> Commit 71523d1812ac (pwm: Ensure pwm_apply_state() doesn't modify the
> state argument) updated the kernel-doc for pwm_apply_state(), but not
> for the ->apply callback in the pwm_ops struct.
>
> Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
> ---
> include/linux/pwm.h | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index b2c9c460947d..0ef808d925bb 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -243,10 +243,7 @@ pwm_set_relative_duty_cycle(struct pwm_state *state, unsigned int duty_cycle,
> * @request: optional hook for requesting a PWM
> * @free: optional hook for freeing a PWM
> * @capture: capture and report PWM signal
> - * @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)
> + * @apply: atomically apply a new PWM config
> * @get_state: get the current PWM state. This function is only
> * called once per PWM device when the PWM chip is
> * registered.
> --
> 2.20.1
>
On Fri, Oct 04, 2019 at 03:32:01PM +0200, Rasmus Villemoes wrote:
> This series adds support for setting the polarity via DT to the
> pwm-mxs driver.
>
> The DT binding is updated, but I'm not touching the existing .dts or
> .dtsi files - it seems that the same was done for bcm2835 in commits
> 46421d9d8e802e570dfa4d793a4938d2642ec7a7 and
> 8a88b2a2017d1e7e80db53080baff591fd454722, while
> arch/arm/boot/dts/bcm283x.dtsi still has #pwm-cells = <2>.
>
> v2:
> - Rebase to v5.4-rc1
> - Address comments from Uwe.
> - Add Rob's ack to patch 4.
> - New patches 5 and 6. The last one is independent of the others, but
> I stumbled on this when rebasing and found the signature had
> changed.
>
> Rasmus Villemoes (6):
> pwm: mxs: implement ->apply
> pwm: mxs: remove legacy methods
> pwm: mxs: add support for inverse polarity
> dt-bindings: pwm: mxs-pwm: Increase #pwm-cells
> pwm: mxs: avoid a division in mxs_pwm_apply()
> pwm: update comment on struct pwm_ops::apply
>
> .../devicetree/bindings/pwm/mxs-pwm.txt | 4 +-
> drivers/pwm/pwm-mxs.c | 101 +++++++++---------
> include/linux/pwm.h | 5 +-
> 3 files changed, 53 insertions(+), 57 deletions(-)
Applied, thanks.
Thierry