mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 00/12] PWM: add support for ->apply, polarity and STM32
@ 2020-03-30 14:57 Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 01/12] led: pwm: always initialize PWM LEDs as inactive Ahmad Fatoum
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox

Hi,

- Port Linux apply API to make porting future PWM drivers easier
- Remove duplicated PWM state, so device variables can be used to query
  and influence actual configuration
- Add support for PWM polarity
- Add support for STM32

Cheers,
Ahmad Fatoum (12):
  led: pwm: always initialize PWM LEDs as inactive
  PWM: core: remove FLAG_ENABLED
  PWM: core: remove ineffectual pwm_{set,get}_duty_cycle
  PWM: core: group PWM state into new struct pwm_state
  PWM: core: remove old PWM API in favor of Linux ->apply
  PWM: core: retire pwm_set_period
  PWM: core: apply initial state in of_pwm_request
  video: backlight-pwm: use new pwm_apply_state API
  led: pwm: use new pwm_apply_state API
  PWM: core: add apply API support for polarity
  of: introduce of_property_count_elems_of_size
  PWM: add support for STM32

 arch/arm/dts/stm32mp151.dtsi     |  12 +
 drivers/led/led-pwm.c            |  19 +-
 drivers/mfd/Kconfig              |   7 +
 drivers/mfd/Makefile             |   1 +
 drivers/mfd/stm32-timers.c       |  74 ++++++
 drivers/of/base.c                |  32 +++
 drivers/pwm/Kconfig              |   6 +
 drivers/pwm/Makefile             |   1 +
 drivers/pwm/core.c               | 178 ++++++++++----
 drivers/pwm/pwm-imx.c            |  32 +--
 drivers/pwm/pwm-mxs.c            |  37 ++-
 drivers/pwm/pwm-stm32.c          | 400 +++++++++++++++++++++++++++++++
 drivers/pwm/pxa_pwm.c            |  93 +++----
 drivers/video/backlight-pwm.c    |  13 +-
 include/linux/mfd/stm32-timers.h |  97 ++++++++
 include/of.h                     |   8 +
 include/pwm.h                    |  62 +++--
 17 files changed, 917 insertions(+), 155 deletions(-)
 create mode 100644 drivers/mfd/stm32-timers.c
 create mode 100644 drivers/pwm/pwm-stm32.c
 create mode 100644 include/linux/mfd/stm32-timers.h

-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/12] led: pwm: always initialize PWM LEDs as inactive
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 02/12] PWM: core: remove FLAG_ENABLED Ahmad Fatoum
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox

While the active-low property is respected when setting brightness,
it's ignored when initializing the LED as off in the probe function
and thus LEDs are active on startup. Fix this.

Fixes: 769eb5e7bb ("led: pwm: support active-low property")
Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
 drivers/led/led-pwm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/led/led-pwm.c b/drivers/led/led-pwm.c
index 8a358dde88b3..4935572ec2d2 100644
--- a/drivers/led/led-pwm.c
+++ b/drivers/led/led-pwm.c
@@ -73,7 +73,7 @@ static int led_pwm_of_probe(struct device_d *dev)
 
 		pwmled->led.set = led_pwm_set;
 
-		pwm_config(pwmled->pwm, 0, pwmled->period);
+		led_pwm_set(&pwmled->led, 0);
 		pwm_enable(pwmled->pwm);
 
 		ret = led_register(&pwmled->led);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 02/12] PWM: core: remove FLAG_ENABLED
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 01/12] led: pwm: always initialize PWM LEDs as inactive Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 03/12] PWM: core: remove ineffectual pwm_{set,get}_duty_cycle Ahmad Fatoum
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

In preparation for moving to a struct pwm_state like Linux does, turn
the flag into a variable.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pwm/core.c | 12 ++++++++----
 include/pwm.h      |  2 ++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index c8016999f0d2..cae23c98ee2d 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -24,7 +24,6 @@ struct pwm_device {
 	struct			pwm_chip *chip;
 	unsigned long		flags;
 #define FLAG_REQUESTED	0
-#define FLAG_ENABLED	1
 	struct list_head	node;
 	struct device_d		*hwdev;
 	struct device_d		dev;
@@ -283,8 +282,10 @@ int pwm_enable(struct pwm_device *pwm)
 {
 	pwm->p_enable = 1;
 
-	if (!test_and_set_bit(FLAG_ENABLED, &pwm->flags))
+	if (!pwm->chip->p_enable) {
+		pwm->chip->p_enable = 1;
 		return pwm->chip->ops->enable(pwm->chip);
+	}
 
 	return 0;
 }
@@ -297,7 +298,10 @@ void pwm_disable(struct pwm_device *pwm)
 {
 	pwm->p_enable = 0;
 
-	if (test_and_clear_bit(FLAG_ENABLED, &pwm->flags))
-		pwm->chip->ops->disable(pwm->chip);
+	if (!pwm->chip->p_enable)
+		return;
+
+	pwm->chip->p_enable = 0;
+	pwm->chip->ops->disable(pwm->chip);
 }
 EXPORT_SYMBOL_GPL(pwm_disable);
diff --git a/include/pwm.h b/include/pwm.h
index ca01f5b53d1c..98af1299748a 100644
--- a/include/pwm.h
+++ b/include/pwm.h
@@ -63,6 +63,7 @@ struct pwm_ops {
  * @ops: The callbacks for this PWM
  * @duty_ns: The duty cycle of the PWM, in nano-seconds
  * @period_ns: The period of the PWM, in nano-seconds
+ * @p_enable: whether the PWM is enabled
  */
 struct pwm_chip {
 	int			id;
@@ -70,6 +71,7 @@ struct pwm_chip {
 	const struct pwm_ops	*ops;
 	int			duty_ns;
 	int			period_ns;
+	int			p_enable;
 };
 
 int pwmchip_add(struct pwm_chip *chip, struct device_d *dev);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 03/12] PWM: core: remove ineffectual pwm_{set,get}_duty_cycle
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 01/12] led: pwm: always initialize PWM LEDs as inactive Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 02/12] PWM: core: remove FLAG_ENABLED Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 04/12] PWM: core: group PWM state into new struct pwm_state Ahmad Fatoum
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

The setter sets a value unused anywhere, but in the getter.
As the functions are unused, just drop them.
There's pwm_config that can be used for this instead.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pwm/core.c | 10 ----------
 include/pwm.h      |  2 --
 2 files changed, 12 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index cae23c98ee2d..eaf7de18295e 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -265,16 +265,6 @@ unsigned int pwm_get_period(struct pwm_device *pwm)
 	return pwm->period_ns;
 }
 
-void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty_ns)
-{
-	pwm->duty_ns = duty_ns;
-}
-
-unsigned int pwm_get_duty_cycle(struct pwm_device *pwm)
-{
-	return pwm->duty_ns;
-}
-
 /*
  * pwm_enable - start a PWM output toggling
  */
diff --git a/include/pwm.h b/include/pwm.h
index 98af1299748a..9897e86545c9 100644
--- a/include/pwm.h
+++ b/include/pwm.h
@@ -34,8 +34,6 @@ void pwm_disable(struct pwm_device *pwm);
 
 void pwm_set_period(struct pwm_device *pwm, unsigned int period);
 unsigned int pwm_get_period(struct pwm_device *pwm);
-void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty);
-unsigned int pwm_get_duty_cycle(struct pwm_device *pwm);
 
 struct pwm_chip;
 
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 04/12] PWM: core: group PWM state into new struct pwm_state
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2020-03-30 14:57 ` [PATCH 03/12] PWM: core: remove ineffectual pwm_{set,get}_duty_cycle Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 05/12] PWM: core: remove old PWM API in favor of Linux ->apply Ahmad Fatoum
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

As a prerequisite for moving to the new apply API, we need to group all
state into one struct that we can apply at once. Prepare for this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pwm/core.c | 34 ++++++++++++++++------------------
 include/pwm.h      | 21 +++++++++++++++------
 2 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index eaf7de18295e..8d3bfa3bb259 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -28,9 +28,7 @@ struct pwm_device {
 	struct device_d		*hwdev;
 	struct device_d		dev;
 
-	unsigned int		duty_ns;
-	unsigned int		period_ns;
-	unsigned int		p_enable;
+	struct pwm_state	params;
 };
 
 static LIST_HEAD(pwm_list);
@@ -51,7 +49,7 @@ static int set_duty_period_ns(struct param_d *p, void *priv)
 {
 	struct pwm_device *pwm = priv;
 
-	pwm_config(pwm, pwm->chip->duty_ns, pwm->chip->period_ns);
+	pwm_config(pwm, pwm->params.duty_ns, pwm->params.period_ns);
 
 	return 0;
 }
@@ -60,7 +58,7 @@ static int set_enable(struct param_d *p, void *priv)
 {
 	struct pwm_device *pwm = priv;
 
-	if (pwm->p_enable)
+	if (pwm->params.p_enable)
 		pwm_enable(pwm);
 	else
 		pwm_disable(pwm);
@@ -99,17 +97,17 @@ int pwmchip_add(struct pwm_chip *chip, struct device_d *dev)
 	list_add_tail(&pwm->node, &pwm_list);
 
 	p = dev_add_param_uint32(&pwm->dev, "duty_ns", set_duty_period_ns,
-			NULL, &pwm->chip->duty_ns, "%u", pwm);
+			NULL, &pwm->params.duty_ns, "%u", pwm);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
 	p = dev_add_param_uint32(&pwm->dev, "period_ns", set_duty_period_ns,
-			NULL, &pwm->chip->period_ns, "%u", pwm);
+			NULL, &pwm->params.period_ns, "%u", pwm);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
 	p = dev_add_param_bool(&pwm->dev, "enable", set_enable,
-			NULL, &pwm->p_enable, pwm);
+			NULL, &pwm->params.p_enable, pwm);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
@@ -242,8 +240,8 @@ EXPORT_SYMBOL_GPL(pwm_free);
  */
 int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 {
-	pwm->chip->duty_ns = duty_ns;
-	pwm->chip->period_ns = period_ns;
+	pwm->chip->state.duty_ns = duty_ns;
+	pwm->chip->state.period_ns = period_ns;
 
 	if (period_ns == 0)
 		return -EINVAL;
@@ -257,12 +255,12 @@ EXPORT_SYMBOL_GPL(pwm_config);
 
 void pwm_set_period(struct pwm_device *pwm, unsigned int period_ns)
 {
-	pwm->period_ns = period_ns;
+	pwm->chip->state.period_ns = period_ns;
 }
 
 unsigned int pwm_get_period(struct pwm_device *pwm)
 {
-	return pwm->period_ns;
+	return pwm->chip->state.period_ns;
 }
 
 /*
@@ -270,10 +268,10 @@ unsigned int pwm_get_period(struct pwm_device *pwm)
  */
 int pwm_enable(struct pwm_device *pwm)
 {
-	pwm->p_enable = 1;
+	pwm->params.p_enable = 1;
 
-	if (!pwm->chip->p_enable) {
-		pwm->chip->p_enable = 1;
+	if (!pwm->chip->state.p_enable) {
+		pwm->chip->state.p_enable = 1;
 		return pwm->chip->ops->enable(pwm->chip);
 	}
 
@@ -286,12 +284,12 @@ EXPORT_SYMBOL_GPL(pwm_enable);
  */
 void pwm_disable(struct pwm_device *pwm)
 {
-	pwm->p_enable = 0;
+	pwm->params.p_enable = 0;
 
-	if (!pwm->chip->p_enable)
+	if (!pwm->chip->state.p_enable)
 		return;
 
-	pwm->chip->p_enable = 0;
+	pwm->chip->state.p_enable = 0;
 	pwm->chip->ops->disable(pwm->chip);
 }
 EXPORT_SYMBOL_GPL(pwm_disable);
diff --git a/include/pwm.h b/include/pwm.h
index 9897e86545c9..c0ce414cb536 100644
--- a/include/pwm.h
+++ b/include/pwm.h
@@ -37,6 +37,18 @@ unsigned int pwm_get_period(struct pwm_device *pwm);
 
 struct pwm_chip;
 
+/*
+ * struct pwm_state - state of a PWM channel
+ * @period_ns: PWM period (in nanoseconds)
+ * @duty_ns: PWM duty cycle (in nanoseconds)
+ * @p_enable: PWM enabled status
+ */
+struct pwm_state {
+	unsigned int period_ns;
+	unsigned int duty_ns;
+	unsigned int p_enable;
+};
+
 /**
  * struct pwm_ops - PWM operations
  * @request: optional hook for requesting a PWM
@@ -59,17 +71,14 @@ struct pwm_ops {
  * @id: The id of this pwm
  * @devname: unique identifier for this pwm
  * @ops: The callbacks for this PWM
- * @duty_ns: The duty cycle of the PWM, in nano-seconds
- * @period_ns: The period of the PWM, in nano-seconds
- * @p_enable: whether the PWM is enabled
+ * @state: current state of the PWM
  */
 struct pwm_chip {
 	int			id;
 	const char		*devname;
 	const struct pwm_ops	*ops;
-	int			duty_ns;
-	int			period_ns;
-	int			p_enable;
+
+	struct pwm_state	state;
 };
 
 int pwmchip_add(struct pwm_chip *chip, struct device_d *dev);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 05/12] PWM: core: remove old PWM API in favor of Linux ->apply
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2020-03-30 14:57 ` [PATCH 04/12] PWM: core: group PWM state into new struct pwm_state Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 06/12] PWM: core: retire pwm_set_period Ahmad Fatoum
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox

Linux has the new atomic PWM API in addition to the old one for backward
compatibility. We only have three PWM drivers in here, so port them over
to the new ->apply API.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pwm/core.c    | 66 +++++++++++++++++++++---------
 drivers/pwm/pwm-imx.c | 32 ++++++---------
 drivers/pwm/pwm-mxs.c | 37 +++++++----------
 drivers/pwm/pxa_pwm.c | 93 +++++++++++++++++++++++--------------------
 include/pwm.h         | 48 ++++++++++++++--------
 5 files changed, 156 insertions(+), 120 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 8d3bfa3bb259..89b9756d0a2d 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -49,9 +49,7 @@ static int set_duty_period_ns(struct param_d *p, void *priv)
 {
 	struct pwm_device *pwm = priv;
 
-	pwm_config(pwm, pwm->params.duty_ns, pwm->params.period_ns);
-
-	return 0;
+	return pwm_apply_state(pwm, &pwm->params);
 }
 
 static int set_enable(struct param_d *p, void *priv)
@@ -235,21 +233,50 @@ void pwm_free(struct pwm_device *pwm)
 }
 EXPORT_SYMBOL_GPL(pwm_free);
 
+void pwm_get_state(const struct pwm_device *pwm,
+		   struct pwm_state *state)
+{
+	*state = pwm->chip->state;
+}
+EXPORT_SYMBOL_GPL(pwm_get_state);
+
+int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
+{
+	struct pwm_chip *chip = pwm->chip;
+	int ret = -EINVAL;
+
+	if (state->period_ns == 0)
+		goto err;
+
+	if (state->duty_ns > state->period_ns)
+		goto err;
+
+	ret = chip->ops->apply(chip, state);
+err:
+	if (ret == 0)
+		chip->state = *state;
+
+	pwm->params = chip->state;
+	return ret;
+}
+
 /*
  * pwm_config - change a PWM device configuration
  */
 int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 {
-	pwm->chip->state.duty_ns = duty_ns;
-	pwm->chip->state.period_ns = period_ns;
+	struct pwm_state state;
 
-	if (period_ns == 0)
+	if (duty_ns < 0 || period_ns < 0)
 		return -EINVAL;
 
-	if (duty_ns > period_ns)
-		return -EINVAL;
+	pwm_get_state(pwm, &state);
+	if (state.duty_ns == duty_ns && state.period_ns == period_ns)
+		return 0;
 
-	return pwm->chip->ops->config(pwm->chip, duty_ns, period_ns);
+	state.duty_ns = duty_ns;
+	state.period_ns = period_ns;
+	return pwm_apply_state(pwm, &state);
 }
 EXPORT_SYMBOL_GPL(pwm_config);
 
@@ -268,14 +295,14 @@ unsigned int pwm_get_period(struct pwm_device *pwm)
  */
 int pwm_enable(struct pwm_device *pwm)
 {
-	pwm->params.p_enable = 1;
+	struct pwm_state state;
 
-	if (!pwm->chip->state.p_enable) {
-		pwm->chip->state.p_enable = 1;
-		return pwm->chip->ops->enable(pwm->chip);
-	}
+	pwm_get_state(pwm, &state);
+	if (state.p_enable)
+		return 0;
 
-	return 0;
+	state.p_enable = true;
+	return pwm_apply_state(pwm, &state);
 }
 EXPORT_SYMBOL_GPL(pwm_enable);
 
@@ -284,12 +311,13 @@ EXPORT_SYMBOL_GPL(pwm_enable);
  */
 void pwm_disable(struct pwm_device *pwm)
 {
-	pwm->params.p_enable = 0;
+	struct pwm_state state;
 
-	if (!pwm->chip->state.p_enable)
+	pwm_get_state(pwm, &state);
+	if (!state.p_enable)
 		return;
 
-	pwm->chip->state.p_enable = 0;
-	pwm->chip->ops->disable(pwm->chip);
+	state.p_enable = false;
+	pwm_apply_state(pwm, &state);
 }
 EXPORT_SYMBOL_GPL(pwm_disable);
diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index b620e502f262..8407b2f5e1bd 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -155,37 +155,31 @@ static void imx_pwm_set_enable_v2(struct pwm_chip *chip, bool enable)
 	writel(val, imx->mmio_base + MX3_PWMCR);
 }
 
-static int imx_pwm_config(struct pwm_chip *chip,
-		int duty_ns, int period_ns)
+static int imx_pwm_apply(struct pwm_chip *chip, const struct pwm_state *state)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
+	bool enabled;
 	int ret;
 
-	ret = imx->config(chip, duty_ns, period_ns);
+	enabled = chip->state.p_enable;
 
-	return ret;
-}
+	if (enabled && !state->p_enable) {
+		imx->set_enable(chip, false);
+		return 0;
+	}
 
-static int imx_pwm_enable(struct pwm_chip *chip)
-{
-	struct imx_chip *imx = to_imx_chip(chip);
+	ret = imx->config(chip, state->duty_ns, state->period_ns);
+	if (ret)
+		return ret;
 
-	imx->set_enable(chip, true);
+	if (!enabled && state->p_enable)
+		imx->set_enable(chip, true);
 
 	return 0;
 }
 
-static void imx_pwm_disable(struct pwm_chip *chip)
-{
-	struct imx_chip *imx = to_imx_chip(chip);
-
-	imx->set_enable(chip, false);
-}
-
 static struct pwm_ops imx_pwm_ops = {
-	.enable = imx_pwm_enable,
-	.disable = imx_pwm_disable,
-	.config = imx_pwm_config,
+	.apply = imx_pwm_apply,
 };
 
 struct imx_pwm_data {
diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c
index e72f1dbcb0df..a06040ac3268 100644
--- a/drivers/pwm/pwm-mxs.c
+++ b/drivers/pwm/pwm-mxs.c
@@ -52,18 +52,26 @@ struct mxs_pwm {
 
 #define to_mxs_pwm_chip(_chip) container_of(_chip, struct mxs_pwm_chip, chip)
 
-static int mxs_pwm_config(struct pwm_chip *chip, int duty_ns, int period_ns)
+static int mxs_pwm_apply(struct pwm_chip *chip, const struct pwm_state *state)
 {
 	struct mxs_pwm_chip *mxs = to_mxs_pwm_chip(chip);
 	int div = 0;
 	unsigned int period_cycles, duty_cycles;
 	unsigned long rate;
 	unsigned long long c;
+	bool enabled;
+
+	enabled = chip->state.p_enable;
+
+	if (enabled && !state->p_enable) {
+		writel(1 << mxs->chip.id, mxs->mxs->base + PWM_CTRL + CLR);
+		return 0;
+	}
 
 	rate = clk_get_rate(mxs->mxs->clk);
 	while (1) {
 		c = rate / cdiv[div];
-		c = c * period_ns;
+		c = c * state->period_ns;
 		do_div(c, 1000000000);
 		if (c < PERIOD_PERIOD_MAX)
 			break;
@@ -73,8 +81,8 @@ static int mxs_pwm_config(struct pwm_chip *chip, int duty_ns, int period_ns)
 	}
 
 	period_cycles = c;
-	c *= duty_ns;
-	do_div(c, period_ns);
+	c *= state->duty_ns;
+	do_div(c, state->period_ns);
 	duty_cycles = c;
 
 	writel(duty_cycles << 16,
@@ -83,29 +91,14 @@ static int mxs_pwm_config(struct pwm_chip *chip, int duty_ns, int period_ns)
 	       PERIOD_INACTIVE_LOW | PERIOD_CDIV(div),
 			mxs->mxs->base + PWM_PERIOD0 + mxs->chip.id * 0x20);
 
-	return 0;
-}
-
-static int mxs_pwm_enable(struct pwm_chip *chip)
-{
-	struct mxs_pwm_chip *mxs = to_mxs_pwm_chip(chip);
-
-	writel(1 << mxs->chip.id, mxs->mxs->base + PWM_CTRL + SET);
+	if (!enabled && state->p_enable)
+		writel(1 << mxs->chip.id, mxs->mxs->base + PWM_CTRL + SET);
 
 	return 0;
 }
 
-static void mxs_pwm_disable(struct pwm_chip *chip)
-{
-	struct mxs_pwm_chip *mxs = to_mxs_pwm_chip(chip);
-
-	writel(1 << mxs->chip.id, mxs->mxs->base + PWM_CTRL + CLR);
-}
-
 static struct pwm_ops mxs_pwm_ops = {
-	.config = mxs_pwm_config,
-	.enable = mxs_pwm_enable,
-	.disable = mxs_pwm_disable,
+	.apply = mxs_pwm_apply,
 };
 
 static int mxs_pwm_probe(struct device_d *dev)
diff --git a/drivers/pwm/pxa_pwm.c b/drivers/pwm/pxa_pwm.c
index 4575817e946c..78d1489d570f 100644
--- a/drivers/pwm/pxa_pwm.c
+++ b/drivers/pwm/pxa_pwm.c
@@ -41,19 +41,60 @@ static struct pxa_pwm_chip *to_pxa_pwm_chip(struct pwm_chip *chip)
 	return container_of(chip, struct pxa_pwm_chip, chip);
 }
 
+static int pxa_pwm_enable(struct pxa_pwm_chip *pxa_pwm)
+{
+	switch (pxa_pwm->id) {
+	case 0:
+	case 2:
+		CKEN |= CKEN_PWM0;
+		break;
+	case 1:
+	case 3:
+		CKEN |= CKEN_PWM1;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static void pxa_pwm_disable(struct pxa_pwm_chip *pxa_pwm)
+{
+	switch (pxa_pwm->id) {
+	case 0:
+	case 2:
+		CKEN &= ~CKEN_PWM0;
+		break;
+	case 1:
+	case 3:
+		CKEN &= ~CKEN_PWM1;
+		break;
+	default:
+		break;
+	}
+}
+
 /*
  * period_ns    = 10^9 * (PRESCALE + 1) * (PV + 1) / PWM_CLK_RATE
  * duty_ns      = 10^9 * (PRESCALE + 1) * DC / PWM_CLK_RATE
  * PWM_CLK_RATE = 13 MHz
  */
-static int pxa_pwm_config(struct pwm_chip *chip, int duty_ns, int period_ns)
+static int pxa_pwm_apply(struct pwm_chip *chip, const struct pwm_state *state)
 {
 	unsigned long long c;
 	unsigned long period_cycles, prescale, pv, dc;
 	struct pxa_pwm_chip *pxa_pwm = to_pxa_pwm_chip(chip);
+	bool enabled;
+
+	enabled = chip->state.p_enable;
+
+	if (enabled && !state->p_enable) {
+		pxa_pwm_disable(pxa_pwm);
+		return 0;
+	}
 
 	c = pxa_get_pwmclk();
-	c = c * period_ns;
+	c = c * state->period_ns;
 	do_div(c, 1000000000);
 	period_cycles = c;
 
@@ -65,10 +106,10 @@ static int pxa_pwm_config(struct pwm_chip *chip, int duty_ns, int period_ns)
 	if (prescale > 63)
 		return -EINVAL;
 
-	if (duty_ns == period_ns)
+	if (state->duty_ns == state->period_ns)
 		dc = PWMDCR_FD;
 	else
-		dc = (pv + 1) * duty_ns / period_ns;
+		dc = (pv + 1) * state->duty_ns / state->period_ns;
 
 	/* NOTE: the clock to PWM has to be enabled first
 	 * before writing to the registers
@@ -77,50 +118,16 @@ static int pxa_pwm_config(struct pwm_chip *chip, int duty_ns, int period_ns)
 	writel(dc, pxa_pwm->iobase + PWMDCR);
 	writel(pv, pxa_pwm->iobase + PWMPCR);
 
-	return 0;
-}
-
-static int pxa_pwm_enable(struct pwm_chip *chip)
-{
-	struct pxa_pwm_chip *pxa_pwm = to_pxa_pwm_chip(chip);
-
-	switch (pxa_pwm->id) {
-	case 0:
-	case 2:
-		CKEN |= CKEN_PWM0;
-		break;
-	case 1:
-	case 3:
-		CKEN |= CKEN_PWM1;
-		break;
-	default:
-		return -EINVAL;
+	if (!enabled && state->p_enable) {
+		pxa_pwm_enable(pxa_pwm);
+		return 0;
 	}
-	return 0;
-}
 
-static void pxa_pwm_disable(struct pwm_chip *chip)
-{
-	struct pxa_pwm_chip *pxa_pwm = to_pxa_pwm_chip(chip);
-
-	switch (pxa_pwm->id) {
-	case 0:
-	case 2:
-		CKEN &= ~CKEN_PWM0;
-		break;
-	case 1:
-	case 3:
-		CKEN &= ~CKEN_PWM1;
-		break;
-	default:
-		break;
-	}
+	return 0;
 }
 
 static struct pwm_ops pxa_pwm_ops = {
-	.config = pxa_pwm_config,
-	.enable = pxa_pwm_enable,
-	.disable = pxa_pwm_disable,
+	.apply = pxa_pwm_apply,
 };
 
 static int pxa_pwm_probe(struct device_d *dev)
diff --git a/include/pwm.h b/include/pwm.h
index c0ce414cb536..7431ecfb42bf 100644
--- a/include/pwm.h
+++ b/include/pwm.h
@@ -5,6 +5,18 @@
 struct pwm_device;
 struct device_d;
 
+/*
+ * struct pwm_state - state of a PWM channel
+ * @period_ns: PWM period (in nanoseconds)
+ * @duty_ns: PWM duty cycle (in nanoseconds)
+ * @p_enable: PWM enabled status
+ */
+struct pwm_state {
+	unsigned int period_ns;
+	unsigned int duty_ns;
+	unsigned int p_enable;
+};
+
 /*
  * pwm_request - request a PWM device
  */
@@ -17,6 +29,11 @@ struct pwm_device *of_pwm_request(struct device_node *np, const char *con_id);
  */
 void pwm_free(struct pwm_device *pwm);
 
+/*
+ * pwm_config - change a PWM device configuration
+ */
+int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state);
+
 /*
  * pwm_config - change a PWM device configuration
  */
@@ -37,33 +54,30 @@ unsigned int pwm_get_period(struct pwm_device *pwm);
 
 struct pwm_chip;
 
-/*
- * struct pwm_state - state of a PWM channel
- * @period_ns: PWM period (in nanoseconds)
- * @duty_ns: PWM duty cycle (in nanoseconds)
- * @p_enable: PWM enabled status
+/**
+ * pwm_get_state() - retrieve the current PWM state
+ * @pwm: PWM device
+ * @state: state to fill with the current PWM state
  */
-struct pwm_state {
-	unsigned int period_ns;
-	unsigned int duty_ns;
-	unsigned int p_enable;
-};
+void pwm_get_state(const struct pwm_device *pwm, struct pwm_state *state);
+
+/**
+ * pwm_apply_state() - apply the passed PWM state
+ * @pwm: PWM device
+ * @state: state to apply to pwm device
+ */
+int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state);
 
 /**
  * struct pwm_ops - PWM operations
  * @request: optional hook for requesting a PWM
  * @free: optional hook for freeing a PWM
- * @config: configure duty cycles and period length for this PWM
- * @enable: enable PWM output toggling
- * @disable: disable PWM output toggling
+ * @apply: apply specified pwm state
  */
 struct pwm_ops {
 	int (*request)(struct pwm_chip *chip);
 	void (*free)(struct pwm_chip *chip);
-	int (*config)(struct pwm_chip *chip, int duty_ns,
-						int period_ns);
-	int (*enable)(struct pwm_chip *chip);
-	void (*disable)(struct pwm_chip *chip);
+	int (*apply)(struct pwm_chip *chip, const struct pwm_state *state);
 };
 
 /**
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 06/12] PWM: core: retire pwm_set_period
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2020-03-30 14:57 ` [PATCH 05/12] PWM: core: remove old PWM API in favor of Linux ->apply Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 07/12] PWM: core: apply initial state in of_pwm_request Ahmad Fatoum
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

Client code can use pwm_apply_state or pwm_config to set period as well
as duty cycle. Having a pwm_set_period thus doesn't add any value.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pwm/core.c | 7 +------
 include/pwm.h      | 1 -
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 89b9756d0a2d..b42b3092a3f8 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -213,7 +213,7 @@ struct pwm_device *of_pwm_request(struct device_node *np, const char *con_id)
 	}
 
 	if (args.args_count > 1)
-		pwm_set_period(pwm, args.args[1]);
+		pwm->chip->state.period_ns = args.args[1];
 
 	ret = __pwm_request(pwm);
 	if (ret)
@@ -280,11 +280,6 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 }
 EXPORT_SYMBOL_GPL(pwm_config);
 
-void pwm_set_period(struct pwm_device *pwm, unsigned int period_ns)
-{
-	pwm->chip->state.period_ns = period_ns;
-}
-
 unsigned int pwm_get_period(struct pwm_device *pwm)
 {
 	return pwm->chip->state.period_ns;
diff --git a/include/pwm.h b/include/pwm.h
index 7431ecfb42bf..36b1eb8131fb 100644
--- a/include/pwm.h
+++ b/include/pwm.h
@@ -49,7 +49,6 @@ int pwm_enable(struct pwm_device *pwm);
  */
 void pwm_disable(struct pwm_device *pwm);
 
-void pwm_set_period(struct pwm_device *pwm, unsigned int period);
 unsigned int pwm_get_period(struct pwm_device *pwm);
 
 struct pwm_chip;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 07/12] PWM: core: apply initial state in of_pwm_request
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2020-03-30 14:57 ` [PATCH 06/12] PWM: core: retire pwm_set_period Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 08/12] video: backlight-pwm: use new pwm_apply_state API Ahmad Fatoum
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox

This functions prepares a state that can later be tweaked and applied
to the PWM device with pwm_apply_state(). This is a convenient function
that first retrieves the current PWM state and the replaces the period
with the reference values defined in pwm->args.
Once the function returns, you can adjust the ->enabled and ->duty_cycle
fields according to your needs before calling pwm_apply_state().

Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
 drivers/pwm/core.c | 70 +++++++++++++++++++++++++++++++++++++++++++---
 include/pwm.h      |  6 ++++
 2 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index b42b3092a3f8..9206fc0b9eda 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -20,6 +20,23 @@
 #include <linux/list.h>
 #include <linux/err.h>
 
+/**
+ * struct pwm_args - board-dependent PWM arguments
+ * @period_ns: reference period
+ *
+ * This structure describes board-dependent arguments attached to a PWM
+ * device. These arguments are usually retrieved from the PWM lookup table or
+ * device tree.
+ *
+ * Do not confuse this with the PWM state: PWM arguments represent the initial
+ * configuration that users want to use on this PWM device rather than the
+ * current PWM hardware state.
+ */
+
+struct pwm_args {
+	unsigned int period_ns;
+};
+
 struct pwm_device {
 	struct			pwm_chip *chip;
 	unsigned long		flags;
@@ -29,6 +46,7 @@ struct pwm_device {
 	struct device_d		dev;
 
 	struct pwm_state	params;
+	struct pwm_args		args;
 };
 
 static LIST_HEAD(pwm_list);
@@ -194,6 +212,7 @@ struct pwm_device *of_pwm_request(struct device_node *np, const char *con_id)
 	struct of_phandle_args args;
 	int index = 0;
 	struct pwm_device *pwm;
+	struct pwm_state state;
 	int ret;
 
 	if (con_id)
@@ -213,11 +232,17 @@ struct pwm_device *of_pwm_request(struct device_node *np, const char *con_id)
 	}
 
 	if (args.args_count > 1)
-		pwm->chip->state.period_ns = args.args[1];
+		pwm->args.period_ns = args.args[1];
 
 	ret = __pwm_request(pwm);
 	if (ret)
-		return ERR_PTR(-ret);
+		return ERR_PTR(ret);
+
+	pwm_init_state(pwm, &state);
+
+	ret = pwm_apply_state(pwm, &state);
+	if (ret)
+		return ERR_PTR(ret);
 
 	return pwm;
 }
@@ -233,13 +258,50 @@ void pwm_free(struct pwm_device *pwm)
 }
 EXPORT_SYMBOL_GPL(pwm_free);
 
-void pwm_get_state(const struct pwm_device *pwm,
-		   struct pwm_state *state)
+void pwm_get_state(const struct pwm_device *pwm, struct pwm_state *state)
 {
 	*state = pwm->chip->state;
 }
 EXPORT_SYMBOL_GPL(pwm_get_state);
 
+static void pwm_get_args(const struct pwm_device *pwm, struct pwm_args *args)
+{
+	*args = pwm->args;
+}
+
+/**
+ * pwm_init_state() - prepare a new state to be applied with pwm_apply_state()
+ * @pwm: PWM device
+ * @state: state to fill with the prepared PWM state
+ *
+ * This functions prepares a state that can later be tweaked and applied
+ * to the PWM device with pwm_apply_state(). This is a convenient function
+ * that first retrieves the current PWM state and the replaces the period
+ * with the reference values defined in pwm->args.
+ * Once the function returns, you can adjust the ->enabled and ->duty_cycle
+ * fields according to your needs before calling pwm_apply_state().
+ *
+ * ->duty_cycle is initially set to zero to avoid cases where the current
+ * ->duty_cycle value exceed the pwm_args->period one, which would trigger
+ * an error if the user calls pwm_apply_state() without adjusting ->duty_cycle
+ * first.
+ */
+void pwm_init_state(const struct pwm_device *pwm,
+		    struct pwm_state *state)
+{
+	struct pwm_args args;
+
+	/* First get the current state. */
+	pwm_get_state(pwm, state);
+
+	/* Then fill it with the reference config */
+	pwm_get_args(pwm, &args);
+
+	state->period_ns = args.period_ns;
+	state->duty_ns = 0;
+}
+EXPORT_SYMBOL_GPL(pwm_init_state);
+
 int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
 {
 	struct pwm_chip *chip = pwm->chip;
diff --git a/include/pwm.h b/include/pwm.h
index 36b1eb8131fb..67ea0f9bcb05 100644
--- a/include/pwm.h
+++ b/include/pwm.h
@@ -29,6 +29,12 @@ struct pwm_device *of_pwm_request(struct device_node *np, const char *con_id);
  */
 void pwm_free(struct pwm_device *pwm);
 
+/*
+ * pwm_init_state - prepare a new state from device tree args
+ */
+void pwm_init_state(const struct pwm_device *pwm,
+		    struct pwm_state *state);
+
 /*
  * pwm_config - change a PWM device configuration
  */
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 08/12] video: backlight-pwm: use new pwm_apply_state API
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
                   ` (6 preceding siblings ...)
  2020-03-30 14:57 ` [PATCH 07/12] PWM: core: apply initial state in of_pwm_request Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-31  6:10   ` Sascha Hauer
  2020-03-30 14:57 ` [PATCH 09/12] led: pwm: " Ahmad Fatoum
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

Use pwm_apply_state we can avoid having to store PWM state in the
instance structure and in future we have an easy way to support new
parameters like inverted duty cycle.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/video/backlight-pwm.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/video/backlight-pwm.c b/drivers/video/backlight-pwm.c
index 9111a42d7544..8b6494dac929 100644
--- a/drivers/video/backlight-pwm.c
+++ b/drivers/video/backlight-pwm.c
@@ -33,7 +33,6 @@ struct pwm_backlight {
 	struct backlight_device backlight;
 	struct pwm_device *pwm;
 	struct regulator *power;
-	uint32_t period;
 	unsigned int *levels;
 	int enable_gpio;
 	int enable_active_high;
@@ -91,13 +90,16 @@ static int backlight_pwm_disable(struct pwm_backlight *pwm_backlight)
 static int compute_duty_cycle(struct pwm_backlight *pwm_backlight, int brightness)
 {
 	int duty_cycle;
+	struct pwm_state state;
+
+	pwm_get_state(pwm_backlight->pwm, &state);
 
 	if (pwm_backlight->levels)
 		duty_cycle = pwm_backlight->levels[brightness];
 	else
 		duty_cycle = brightness;
 
-	return duty_cycle * pwm_backlight->period / pwm_backlight->scale;
+	return duty_cycle * state.period_ns / pwm_backlight->scale;
 }
 
 static int backlight_pwm_set(struct backlight_device *backlight,
@@ -105,9 +107,11 @@ static int backlight_pwm_set(struct backlight_device *backlight,
 {
 	struct pwm_backlight *pwm_backlight = container_of(backlight,
 			struct pwm_backlight, backlight);
+	struct pwm_state state;
 
-	pwm_config(pwm_backlight->pwm, compute_duty_cycle(pwm_backlight, brightness),
-		   pwm_backlight->period);
+	pwm_get_state(pwm_backlight->pwm, &state);
+	state.duty_ns = compute_duty_cycle(pwm_backlight, brightness);
+	pwm_apply_state(pwm_backlight->pwm, &state);
 
 	if (brightness)
 		return backlight_pwm_enable(pwm_backlight);
@@ -192,7 +196,6 @@ static int backlight_pwm_of_probe(struct device_d *dev)
 
 	pwm_backlight = xzalloc(sizeof(*pwm_backlight));
 	pwm_backlight->pwm = pwm;
-	pwm_backlight->period = pwm_get_period(pwm);
 
 	ret = pwm_backlight_parse_dt(dev, pwm_backlight);
 	if (ret)
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 09/12] led: pwm: use new pwm_apply_state API
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
                   ` (7 preceding siblings ...)
  2020-03-30 14:57 ` [PATCH 08/12] video: backlight-pwm: use new pwm_apply_state API Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 10/12] PWM: core: add apply API support for polarity Ahmad Fatoum
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

To support PWM_POLARITY_INVERTED for PWM LEDs, we need to to use the
apply API.  Do so.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/led/led-pwm.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/led/led-pwm.c b/drivers/led/led-pwm.c
index 4935572ec2d2..19d9d4d48a3d 100644
--- a/drivers/led/led-pwm.c
+++ b/drivers/led/led-pwm.c
@@ -29,22 +29,27 @@ struct pwmled {
 	bool active_low;
 	struct led led;
 	struct pwm_device *pwm;
-	uint32_t period;
 };
 
 static void led_pwm_set(struct led *led, unsigned int brightness)
 {
 	struct pwmled *pwmled = container_of(led, struct pwmled, led);
-	unsigned long long duty =  pwmled->period;
+	unsigned long long duty;
+	struct pwm_state state;
 	unsigned int max = pwmled->led.max_value;
 
-        duty *= brightness;
+	pwm_get_state(pwmled->pwm, &state);
+
+	duty = state.period_ns * brightness;
         do_div(duty, max);
 
 	if (pwmled->active_low)
-		duty = pwmled->period - duty;
+		duty = state.period_ns - duty;
+
+	state.p_enable = true;
+	state.duty_ns = duty;
 
-	pwm_config(pwmled->pwm, duty, pwmled->period);
+	pwm_apply_state(pwmled->pwm, &state);
 }
 
 static int led_pwm_of_probe(struct device_d *dev)
@@ -68,13 +73,11 @@ static int led_pwm_of_probe(struct device_d *dev)
 		if (ret)
 			return ret;
 
-		pwmled->period = pwm_get_period(pwmled->pwm);
 		pwmled->active_low = of_property_read_bool(child, "active-low");
 
 		pwmled->led.set = led_pwm_set;
 
 		led_pwm_set(&pwmled->led, 0);
-		pwm_enable(pwmled->pwm);
 
 		ret = led_register(&pwmled->led);
 		if (ret)
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 10/12] PWM: core: add apply API support for polarity
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
                   ` (8 preceding siblings ...)
  2020-03-30 14:57 ` [PATCH 09/12] led: pwm: " Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 11/12] of: introduce of_property_count_elems_of_size Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 12/12] PWM: add support for STM32 Ahmad Fatoum
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

Some PWM chips support outputting an inverted PWM signal.
Add API support for this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pwm/core.c | 21 +++++++++++++++++----
 include/pwm.h      |  6 ++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 9206fc0b9eda..05dad93e5ce3 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -23,6 +23,7 @@
 /**
  * struct pwm_args - board-dependent PWM arguments
  * @period_ns: reference period
+ * @polarity: reference polarity
  *
  * This structure describes board-dependent arguments attached to a PWM
  * device. These arguments are usually retrieved from the PWM lookup table or
@@ -35,6 +36,7 @@
 
 struct pwm_args {
 	unsigned int period_ns;
+	unsigned int polarity;
 };
 
 struct pwm_device {
@@ -63,7 +65,7 @@ static struct pwm_device *_find_pwm(const char *devname)
 	return NULL;
 }
 
-static int set_duty_period_ns(struct param_d *p, void *priv)
+static int apply_params(struct param_d *p, void *priv)
 {
 	struct pwm_device *pwm = priv;
 
@@ -112,12 +114,12 @@ int pwmchip_add(struct pwm_chip *chip, struct device_d *dev)
 
 	list_add_tail(&pwm->node, &pwm_list);
 
-	p = dev_add_param_uint32(&pwm->dev, "duty_ns", set_duty_period_ns,
+	p = dev_add_param_uint32(&pwm->dev, "duty_ns", apply_params,
 			NULL, &pwm->params.duty_ns, "%u", pwm);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
-	p = dev_add_param_uint32(&pwm->dev, "period_ns", set_duty_period_ns,
+	p = dev_add_param_uint32(&pwm->dev, "period_ns", apply_params,
 			NULL, &pwm->params.period_ns, "%u", pwm);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
@@ -127,6 +129,11 @@ int pwmchip_add(struct pwm_chip *chip, struct device_d *dev)
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
+	p = dev_add_param_bool(&pwm->dev, "inverted", apply_params,
+			       NULL, &pwm->params.polarity, pwm);
+	if (IS_ERR(p))
+		return PTR_ERR(p);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pwmchip_add);
@@ -234,6 +241,11 @@ struct pwm_device *of_pwm_request(struct device_node *np, const char *con_id)
 	if (args.args_count > 1)
 		pwm->args.period_ns = args.args[1];
 
+	pwm->args.polarity = PWM_POLARITY_NORMAL;
+
+	if (args.args_count > 2 && args.args[2] & PWM_POLARITY_INVERTED)
+		pwm->args.polarity = PWM_POLARITY_INVERTED;
+
 	ret = __pwm_request(pwm);
 	if (ret)
 		return ERR_PTR(ret);
@@ -277,7 +289,7 @@ static void pwm_get_args(const struct pwm_device *pwm, struct pwm_args *args)
  * This functions prepares a state that can later be tweaked and applied
  * to the PWM device with pwm_apply_state(). This is a convenient function
  * that first retrieves the current PWM state and the replaces the period
- * with the reference values defined in pwm->args.
+ * and polarity fields with the reference values defined in pwm->args.
  * Once the function returns, you can adjust the ->enabled and ->duty_cycle
  * fields according to your needs before calling pwm_apply_state().
  *
@@ -298,6 +310,7 @@ void pwm_init_state(const struct pwm_device *pwm,
 	pwm_get_args(pwm, &args);
 
 	state->period_ns = args.period_ns;
+	state->polarity = args.polarity;
 	state->duty_ns = 0;
 }
 EXPORT_SYMBOL_GPL(pwm_init_state);
diff --git a/include/pwm.h b/include/pwm.h
index 67ea0f9bcb05..b67ab13d2e2d 100644
--- a/include/pwm.h
+++ b/include/pwm.h
@@ -2,18 +2,24 @@
 #ifndef __PWM_H
 #define __PWM_H
 
+#include <dt-bindings/pwm/pwm.h>
+
 struct pwm_device;
 struct device_d;
 
+#define	PWM_POLARITY_NORMAL	0
+
 /*
  * struct pwm_state - state of a PWM channel
  * @period_ns: PWM period (in nanoseconds)
  * @duty_ns: PWM duty cycle (in nanoseconds)
+ * @polarity: PWM polarity
  * @p_enable: PWM enabled status
  */
 struct pwm_state {
 	unsigned int period_ns;
 	unsigned int duty_ns;
+	unsigned int polarity;
 	unsigned int p_enable;
 };
 
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 11/12] of: introduce of_property_count_elems_of_size
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
                   ` (9 preceding siblings ...)
  2020-03-30 14:57 ` [PATCH 10/12] PWM: core: add apply API support for polarity Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-30 14:57 ` [PATCH 12/12] PWM: add support for STM32 Ahmad Fatoum
  11 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

Import the Linux helper, so code using it may be more easily ported.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/base.c | 32 ++++++++++++++++++++++++++++++++
 include/of.h      |  8 ++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 6d13b66a0e3e..346d72e1e09e 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -738,6 +738,38 @@ int of_property_read_u32_index(const struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(of_property_read_u32_index);
 
+/**
+ * of_property_count_elems_of_size - Count the number of elements in a property
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @elem_size:	size of the individual element
+ *
+ * Search for a property in a device node and count the number of elements of
+ * size elem_size in it. Returns number of elements on sucess, -EINVAL if the
+ * property does not exist or its length does not match a multiple of elem_size
+ * and -ENODATA if the property does not have a value.
+ */
+int of_property_count_elems_of_size(const struct device_node *np,
+				const char *propname, int elem_size)
+{
+	struct property *prop = of_find_property(np, propname, NULL);
+
+	if (!prop)
+		return -EINVAL;
+	if (!prop->value)
+		return -ENODATA;
+
+	if (prop->length % elem_size != 0) {
+		pr_err("size of %s in node %pOF is not a multiple of %d\n",
+		       propname, np, elem_size);
+		return -EINVAL;
+	}
+
+	return prop->length / elem_size;
+}
+EXPORT_SYMBOL_GPL(of_property_count_elems_of_size);
+
 /**
  * of_property_read_u8_array - Find and read an array of u8 from a property.
  *
diff --git a/include/of.h b/include/of.h
index 8d6c018b73ec..eea5abae459b 100644
--- a/include/of.h
+++ b/include/of.h
@@ -186,6 +186,8 @@ extern struct device_node *of_find_node_by_reproducible_name(struct device_node
 extern int of_property_read_u32_index(const struct device_node *np,
 				       const char *propname,
 				       u32 index, u32 *out_value);
+extern int of_property_count_elems_of_size(const struct device_node *np,
+				const char *propname, int elem_size);
 extern int of_property_read_u8_array(const struct device_node *np,
 			const char *propname, u8 *out_values, size_t sz);
 extern int of_property_read_u16_array(const struct device_node *np,
@@ -425,6 +427,12 @@ static inline int of_property_read_u32_index(const struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_property_count_elems_of_size(const struct device_node *np,
+			const char *propname, int elem_size)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u8_array(const struct device_node *np,
 				const char *propname, u8 *out_values, size_t sz)
 {
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 12/12] PWM: add support for STM32
  2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
                   ` (10 preceding siblings ...)
  2020-03-30 14:57 ` [PATCH 11/12] of: introduce of_property_count_elems_of_size Ahmad Fatoum
@ 2020-03-30 14:57 ` Ahmad Fatoum
  2020-03-31  6:41   ` Sascha Hauer
  11 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-30 14:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

This driver adds support for PWM driver on STM32 platform
based on the Linux v5.4 support.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/dts/stm32mp151.dtsi     |  12 +
 drivers/mfd/Kconfig              |   7 +
 drivers/mfd/Makefile             |   1 +
 drivers/mfd/stm32-timers.c       |  74 ++++++
 drivers/pwm/Kconfig              |   6 +
 drivers/pwm/Makefile             |   1 +
 drivers/pwm/pwm-stm32.c          | 400 +++++++++++++++++++++++++++++++
 include/linux/mfd/stm32-timers.h |  97 ++++++++
 8 files changed, 598 insertions(+)
 create mode 100644 drivers/mfd/stm32-timers.c
 create mode 100644 drivers/pwm/pwm-stm32.c
 create mode 100644 include/linux/mfd/stm32-timers.h

diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
index 2a70a747e76e..a0870c446f45 100644
--- a/arch/arm/dts/stm32mp151.dtsi
+++ b/arch/arm/dts/stm32mp151.dtsi
@@ -21,6 +21,18 @@
 		mmc0 = &sdmmc1;
 		mmc1 = &sdmmc2;
 		mmc2 = &sdmmc3;
+		pwm1 = &{/soc/timer@44000000/pwm};
+		pwm2 = &{/soc/timer@40000000/pwm};
+		pwm3 = &{/soc/timer@40001000/pwm};
+		pwm4 = &{/soc/timer@40002000/pwm};
+		pwm5 = &{/soc/timer@40003000/pwm};
+		pwm8 = &{/soc/timer@44001000/pwm};
+		pwm12 = &{/soc/timer@40006000/pwm};
+		pwm13 = &{/soc/timer@40007000/pwm};
+		pwm14 = &{/soc/timer@40008000/pwm};
+		pwm15 = &{/soc/timer@44006000/pwm};
+		pwm16 = &{/soc/timer@44007000/pwm};
+		pwm17 = &{/soc/timer@44008000/pwm};
 	};
 
 	psci {
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index f4cc71ef0ec8..ddf117712e77 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -82,4 +82,11 @@ config SMSC_SUPERIO
        help
          Select this to probe for IO-port connected SMSC Super I/O chips.
 
+config MFD_STM32_TIMERS
+	bool "STM32 Timers"
+	depends on ARCH_STM32MP
+	help
+	  Select this to get regmap support for the timer blocks on STM32
+	  MCUs and MPUs.
+
 endmenu
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 0c24493e3d86..a3b296a803f8 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_MFD_STPMIC1)	+= stpmic1.o
 obj-$(CONFIG_MFD_SUPERIO)	+= superio.o
 obj-$(CONFIG_FINTEK_SUPERIO)	+= fintek-superio.o
 obj-$(CONFIG_SMSC_SUPERIO)	+= smsc-superio.o
+obj-$(CONFIG_MFD_STM32_TIMERS)	+= stm32-timers.o
diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
new file mode 100644
index 000000000000..c53a25687e14
--- /dev/null
+++ b/drivers/mfd/stm32-timers.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2016
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
+ */
+
+#include <common.h>
+#include <linux/clk.h>
+#include <driver.h>
+#include <init.h>
+#include <io.h>
+#include <linux/bitfield.h>
+#include <linux/mfd/stm32-timers.h>
+#include <of.h>
+#include <linux/reset.h>
+
+#define STM32_TIMERS_MAX_REGISTERS	0x3fc
+
+static const struct regmap_config stm32_timers_regmap_cfg = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = sizeof(u32),
+	.max_register = STM32_TIMERS_MAX_REGISTERS,
+};
+
+static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
+{
+	/*
+	 * Only the available bits will be written so when readback
+	 * we get the maximum value of auto reload register
+	 */
+	regmap_write(ddata->regmap, TIM_ARR, ~0L);
+	regmap_read(ddata->regmap, TIM_ARR, &ddata->max_arr);
+	regmap_write(ddata->regmap, TIM_ARR, 0x0);
+}
+
+static int stm32_timers_probe(struct device_d *dev)
+{
+	struct stm32_timers *ddata;
+	struct resource *res;
+
+	ddata = xzalloc(sizeof(*ddata));
+
+	res = dev_request_mem_resource(dev, 0);
+	if (IS_ERR(res))
+		return PTR_ERR(res);
+
+	ddata->regmap = regmap_init_mmio_clk(dev, "int", IOMEM(res->start),
+					     &stm32_timers_regmap_cfg);
+	if (IS_ERR(ddata->regmap))
+		return PTR_ERR(ddata->regmap);
+
+	ddata->clk = clk_get(dev, NULL);
+	if (IS_ERR(ddata->clk))
+		return PTR_ERR(ddata->clk);
+
+	stm32_timers_get_arr_size(ddata);
+
+	dev->priv = ddata;
+
+	return of_platform_populate(dev->device_node, NULL, dev);
+}
+
+static const struct of_device_id stm32_timers_of_match[] = {
+	{ .compatible = "st,stm32-timers", },
+	{ /* sentinel */ },
+};
+
+static struct driver_d stm32_timers_driver = {
+	.name = "stm32-timers",
+	.probe = stm32_timers_probe,
+	.of_compatible = stm32_timers_of_match,
+};
+coredevice_platform_driver(stm32_timers_driver);
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 97c3deff107b..9268aac9122a 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -28,4 +28,10 @@ config PWM_MXS
 	help
 	  This enables PWM support for Freescale i.MX23/i.MX28 SoCs
 
+config PWM_STM32
+	bool "STM32 PWM Support"
+	depends on ARCH_STM32MP
+	help
+	  This enables PWM support for STM32 MCUs and MPUs.
+
 endif
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 46865a24eeb4..c0a27becefcd 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_PWM)		+= core.o
 obj-$(CONFIG_PWM_PXA)		+= pxa_pwm.o
 obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o
 obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
+obj-$(CONFIG_PWM_STM32)		+= pwm-stm32.o
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
new file mode 100644
index 000000000000..061644e4d883
--- /dev/null
+++ b/drivers/pwm/pwm-stm32.c
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016 STMicroelectronics 2016
+ * Copyright (C) 2020 Pengutronix
+ *
+ * Author: Gerald Baeza <gerald.baeza@st.com>
+ *	   Ahmad Fatoum <a.fatoum@pengutronix.de>
+ */
+
+#include <common.h>
+#include <linux/clk.h>
+#include <driver.h>
+#include <init.h>
+#include <io.h>
+#include <linux/bitfield.h>
+#include <linux/mfd/stm32-timers.h>
+#include <linux/math64.h>
+#include <of.h>
+#include <pwm.h>
+
+#define CCMR_CHANNEL_SHIFT 8
+#define CCMR_CHANNEL_MASK  0xFF
+#define MAX_BREAKINPUT 2
+
+struct stm32_pwm {
+	struct clk *clk;
+	struct regmap *regmap;
+	u32 max_arr;
+	bool have_complementary_output;
+	struct pwm_chip pwms[4];
+};
+
+struct stm32_breakinput {
+	u32 index;
+	u32 level;
+	u32 filter;
+};
+
+#define for_each_stm32_pwm(i, chip, pwm) \
+	for (chip[i = 0] = pwm->pwms[0]; i < 4 && chip->ops; chip = chip[++i])
+
+static inline struct stm32_pwm *to_stm32_pwm_dev(struct pwm_chip *chip)
+{
+	struct pwm_chip (*pwms)[4] = (void *)&chip[-chip->id];
+	return container_of(pwms, struct stm32_pwm, pwms);
+}
+
+static u32 active_channels(struct stm32_pwm *dev)
+{
+	u32 ccer;
+
+	regmap_read(dev->regmap, TIM_CCER, &ccer);
+
+	return ccer & TIM_CCER_CCXE;
+}
+
+static int write_ccrx(struct stm32_pwm *dev, unsigned ch, u32 value)
+{
+	switch (ch) {
+	case 0:
+		return regmap_write(dev->regmap, TIM_CCR1, value);
+	case 1:
+		return regmap_write(dev->regmap, TIM_CCR2, value);
+	case 2:
+		return regmap_write(dev->regmap, TIM_CCR3, value);
+	case 3:
+		return regmap_write(dev->regmap, TIM_CCR4, value);
+	}
+	return -EINVAL;
+}
+
+#define TIM_CCER_CC12P (TIM_CCER_CC1P | TIM_CCER_CC2P)
+#define TIM_CCER_CC12E (TIM_CCER_CC1E | TIM_CCER_CC2E)
+#define TIM_CCER_CC34P (TIM_CCER_CC3P | TIM_CCER_CC4P)
+#define TIM_CCER_CC34E (TIM_CCER_CC3E | TIM_CCER_CC4E)
+
+static int stm32_pwm_set_polarity(struct stm32_pwm *priv, unsigned ch,
+				  unsigned polarity)
+{
+	u32 mask;
+
+	mask = TIM_CCER_CC1P << (ch * 4);
+	if (priv->have_complementary_output)
+		mask |= TIM_CCER_CC1NP << (ch * 4);
+
+	regmap_update_bits(priv->regmap, TIM_CCER, mask,
+			   polarity == PWM_POLARITY_NORMAL ? 0 : mask);
+
+	return 0;
+}
+
+static int stm32_pwm_config(struct stm32_pwm *priv, unsigned ch,
+			    int duty_ns, int period_ns)
+{
+	unsigned long long prd, div, dty;
+	unsigned int prescaler = 0;
+	u32 ccmr, mask, shift;
+
+	/* Period and prescaler values depends on clock rate */
+	div = (unsigned long long)clk_get_rate(priv->clk) * period_ns;
+
+	do_div(div, NSEC_PER_SEC);
+	prd = div;
+
+	while (div > priv->max_arr) {
+		prescaler++;
+		div = prd;
+		do_div(div, prescaler + 1);
+	}
+
+	prd = div;
+
+	if (prescaler > MAX_TIM_PSC)
+		return -EINVAL;
+
+	/*
+	 * All channels share the same prescaler and counter so when two
+	 * channels are active at the same time we can't change them
+	 */
+	if (active_channels(priv) & ~(1 << ch * 4)) {
+		u32 psc, arr;
+
+		regmap_read(priv->regmap, TIM_PSC, &psc);
+		regmap_read(priv->regmap, TIM_ARR, &arr);
+
+		if ((psc != prescaler) || (arr != prd - 1))
+			return -EBUSY;
+	}
+
+	regmap_write(priv->regmap, TIM_PSC, prescaler);
+	regmap_write(priv->regmap, TIM_ARR, prd - 1);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
+
+	/* Calculate the duty cycles */
+	dty = prd * duty_ns;
+	do_div(dty, period_ns);
+
+	write_ccrx(priv, ch, dty);
+
+	/* Configure output mode */
+	shift = (ch & 0x1) * CCMR_CHANNEL_SHIFT;
+	ccmr = (TIM_CCMR_PE | TIM_CCMR_M1) << shift;
+	mask = CCMR_CHANNEL_MASK << shift;
+
+	if (ch < 2)
+		regmap_update_bits(priv->regmap, TIM_CCMR1, mask, ccmr);
+	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);
+
+	return 0;
+}
+
+static int stm32_pwm_enable(struct stm32_pwm *priv, unsigned ch)
+{
+	u32 mask;
+	int ret;
+
+	ret = clk_enable(priv->clk);
+	if (ret)
+		return ret;
+
+	/* Enable channel */
+	mask = TIM_CCER_CC1E << (ch * 4);
+	if (priv->have_complementary_output)
+		mask |= TIM_CCER_CC1NE << (ch * 4);
+
+	regmap_update_bits(priv->regmap, TIM_CCER, mask, mask);
+
+	/* Make sure that registers are updated */
+	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+
+	/* Enable controller */
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
+
+	return 0;
+}
+
+static void stm32_pwm_disable(struct stm32_pwm *priv, unsigned ch)
+{
+	u32 mask;
+
+	/* Disable channel */
+	mask = TIM_CCER_CC1E << (ch * 4);
+	if (priv->have_complementary_output)
+		mask |= TIM_CCER_CC1NE << (ch * 4);
+
+	regmap_update_bits(priv->regmap, TIM_CCER, mask, 0);
+
+	/* When all channels are disabled, we can disable the controller */
+	if (!active_channels(priv))
+		regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+
+	clk_disable(priv->clk);
+}
+
+static int stm32_pwm_apply(struct pwm_chip *chip, const struct pwm_state *state)
+{
+	bool enabled;
+	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
+	int ret;
+
+	enabled = chip->state.p_enable;
+
+	if (enabled && !state->p_enable) {
+		stm32_pwm_disable(priv, chip->id);
+		return 0;
+	}
+
+	if (state->polarity != chip->state.polarity)
+		stm32_pwm_set_polarity(priv, chip->id, state->polarity);
+
+	ret = stm32_pwm_config(priv, chip->id,
+			       state->duty_ns, state->period_ns);
+	if (ret)
+		return ret;
+
+	if (!enabled && state->p_enable)
+		ret = stm32_pwm_enable(priv, chip->id);
+
+	return ret;
+}
+
+static const struct pwm_ops stm32pwm_ops = {
+	.apply = stm32_pwm_apply,
+};
+
+static int stm32_pwm_set_breakinput(struct stm32_pwm *priv,
+				    int index, int level, int filter)
+{
+	u32 bke = (index == 0) ? TIM_BDTR_BKE : TIM_BDTR_BK2E;
+	int shift = (index == 0) ? TIM_BDTR_BKF_SHIFT : TIM_BDTR_BK2F_SHIFT;
+	u32 mask = (index == 0) ? TIM_BDTR_BKE | TIM_BDTR_BKP | TIM_BDTR_BKF
+				: TIM_BDTR_BK2E | TIM_BDTR_BK2P | TIM_BDTR_BK2F;
+	u32 bdtr = bke;
+
+	/*
+	 * The both bits could be set since only one will be wrote
+	 * due to mask value.
+	 */
+	if (level)
+		bdtr |= TIM_BDTR_BKP | TIM_BDTR_BK2P;
+
+	bdtr |= (filter & TIM_BDTR_BKF_MASK) << shift;
+
+	regmap_update_bits(priv->regmap, TIM_BDTR, mask, bdtr);
+
+	regmap_read(priv->regmap, TIM_BDTR, &bdtr);
+
+	return (bdtr & bke) ? 0 : -EINVAL;
+}
+
+static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv,
+				       struct device_node *np)
+{
+	struct stm32_breakinput breakinput[MAX_BREAKINPUT];
+	int nb, ret, i, array_size;
+
+	nb = of_property_count_elems_of_size(np, "st,breakinput",
+					     sizeof(struct stm32_breakinput));
+
+	/*
+	 * Because "st,breakinput" parameter is optional do not make probe
+	 * failed if it doesn't exist.
+	 */
+	if (nb <= 0)
+		return 0;
+
+	if (nb > MAX_BREAKINPUT)
+		return -EINVAL;
+
+	array_size = nb * sizeof(struct stm32_breakinput) / sizeof(u32);
+	ret = of_property_read_u32_array(np, "st,breakinput",
+					 (u32 *)breakinput, array_size);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < nb && !ret; i++) {
+		ret = stm32_pwm_set_breakinput(priv,
+					       breakinput[i].index,
+					       breakinput[i].level,
+					       breakinput[i].filter);
+	}
+
+	return ret;
+}
+
+static void stm32_pwm_detect_complementary(struct stm32_pwm *priv)
+{
+	u32 ccer;
+
+	/*
+	 * If complementary bit doesn't exist writing 1 will have no
+	 * effect so we can detect it.
+	 */
+	regmap_update_bits(priv->regmap,
+			   TIM_CCER, TIM_CCER_CC1NE, TIM_CCER_CC1NE);
+	regmap_read(priv->regmap, TIM_CCER, &ccer);
+	regmap_update_bits(priv->regmap, TIM_CCER, TIM_CCER_CC1NE, 0);
+
+	priv->have_complementary_output = (ccer != 0);
+}
+
+static int stm32_pwm_detect_channels(struct stm32_pwm *priv)
+{
+	u32 ccer;
+	int npwm = 0;
+
+	/*
+	 * If channels enable bits don't exist writing 1 will have no
+	 * effect so we can detect and count them.
+	 */
+	regmap_update_bits(priv->regmap,
+			   TIM_CCER, TIM_CCER_CCXE, TIM_CCER_CCXE);
+	regmap_read(priv->regmap, TIM_CCER, &ccer);
+	regmap_update_bits(priv->regmap, TIM_CCER, TIM_CCER_CCXE, 0);
+
+	if (ccer & TIM_CCER_CC1E)
+		npwm++;
+
+	if (ccer & TIM_CCER_CC2E)
+		npwm++;
+
+	if (ccer & TIM_CCER_CC3E)
+		npwm++;
+
+	if (ccer & TIM_CCER_CC4E)
+		npwm++;
+
+	return npwm;
+}
+
+static int id = -1;
+
+static int stm32_pwm_probe(struct device_d *dev)
+{
+	struct device_node *np = dev->device_node;
+	struct stm32_timers *ddata = dev->parent->priv;
+	struct stm32_pwm *priv;
+	const char *alias;
+	int ret, i;
+	int npwms;
+
+	priv = xzalloc(sizeof(*priv));
+	dev->priv = priv;
+
+	priv->regmap = ddata->regmap;
+	priv->clk = ddata->clk;
+	priv->max_arr = ddata->max_arr;
+
+	if (!priv->regmap || !priv->clk)
+		return -EINVAL;
+
+	ret = stm32_pwm_apply_breakinputs(priv, np);
+	if (ret)
+		return ret;
+
+	stm32_pwm_detect_complementary(priv);
+
+	npwms = stm32_pwm_detect_channels(priv);
+
+	alias = of_alias_get(dev->device_node);
+	if (!alias)
+		id++;
+
+	for (i = 0; i < npwms; i++) {
+		struct pwm_chip *chip = &priv->pwms[i];
+
+		if (alias)
+			chip->devname = basprintf("%sch%u", alias, i + 1);
+		else
+			chip->devname = basprintf("pwm%uch%u", id, i + 1);
+
+		chip->ops = &stm32pwm_ops;
+		chip->id = i;
+
+		ret = pwmchip_add(chip, dev);
+		if (ret < 0) {
+			dev_err(dev, "failed to add pwm chip %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id stm32_pwm_of_match[] = {
+	{ .compatible = "st,stm32-pwm",	},
+	{ /* sentinel */ },
+};
+
+static struct driver_d stm32_pwm_driver = {
+	.name = "stm32-pwm",
+	.probe	= stm32_pwm_probe,
+	.of_compatible = stm32_pwm_of_match,
+};
+coredevice_platform_driver(stm32_pwm_driver);
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
new file mode 100644
index 000000000000..28fad44598f9
--- /dev/null
+++ b/include/linux/mfd/stm32-timers.h
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) STMicroelectronics 2016
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
+ */
+
+#ifndef _LINUX_STM32_GPTIMER_H_
+#define _LINUX_STM32_GPTIMER_H_
+
+#include <clock.h>
+#include <regmap.h>
+
+#define TIM_CR1		0x00	/* Control Register 1      */
+#define TIM_CR2		0x04	/* Control Register 2      */
+#define TIM_SMCR	0x08	/* Slave mode control reg  */
+#define TIM_DIER	0x0C	/* DMA/interrupt register  */
+#define TIM_SR		0x10	/* Status register	   */
+#define TIM_EGR		0x14	/* Event Generation Reg    */
+#define TIM_CCMR1	0x18	/* Capt/Comp 1 Mode Reg    */
+#define TIM_CCMR2	0x1C	/* Capt/Comp 2 Mode Reg    */
+#define TIM_CCER	0x20	/* Capt/Comp Enable Reg    */
+#define TIM_CNT		0x24	/* Counter		   */
+#define TIM_PSC		0x28	/* Prescaler               */
+#define TIM_ARR		0x2c	/* Auto-Reload Register    */
+#define TIM_CCR1	0x34	/* Capt/Comp Register 1    */
+#define TIM_CCR2	0x38	/* Capt/Comp Register 2    */
+#define TIM_CCR3	0x3C	/* Capt/Comp Register 3    */
+#define TIM_CCR4	0x40	/* Capt/Comp Register 4    */
+#define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
+#define TIM_DCR		0x48	/* DMA control register    */
+#define TIM_DMAR	0x4C	/* DMA register for transfer */
+
+#define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
+#define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */
+#define TIM_CR1_ARPE	BIT(7)	/* Auto-reload Preload Ena */
+#define TIM_CR2_MMS	(BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
+#define TIM_CR2_MMS2	GENMASK(23, 20) /* Master mode selection 2 */
+#define TIM_SMCR_SMS	(BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
+#define TIM_SMCR_TS	(BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
+#define TIM_DIER_UIE	BIT(0)	/* Update interrupt	   */
+#define TIM_DIER_UDE	BIT(8)  /* Update DMA request Enable */
+#define TIM_DIER_CC1DE	BIT(9)  /* CC1 DMA request Enable  */
+#define TIM_DIER_CC2DE	BIT(10) /* CC2 DMA request Enable  */
+#define TIM_DIER_CC3DE	BIT(11) /* CC3 DMA request Enable  */
+#define TIM_DIER_CC4DE	BIT(12) /* CC4 DMA request Enable  */
+#define TIM_DIER_COMDE	BIT(13) /* COM DMA request Enable  */
+#define TIM_DIER_TDE	BIT(14) /* Trigger DMA request Enable */
+#define TIM_SR_UIF	BIT(0)	/* Update interrupt flag   */
+#define TIM_EGR_UG	BIT(0)	/* Update Generation       */
+#define TIM_CCMR_PE	BIT(3)	/* Channel Preload Enable  */
+#define TIM_CCMR_M1	(BIT(6) | BIT(5))  /* Channel PWM Mode 1 */
+#define TIM_CCMR_CC1S		(BIT(0) | BIT(1)) /* Capture/compare 1 sel */
+#define TIM_CCMR_IC1PSC		GENMASK(3, 2)	/* Input capture 1 prescaler */
+#define TIM_CCMR_CC2S		(BIT(8) | BIT(9)) /* Capture/compare 2 sel */
+#define TIM_CCMR_IC2PSC		GENMASK(11, 10)	/* Input capture 2 prescaler */
+#define TIM_CCMR_CC1S_TI1	BIT(0)	/* IC1/IC3 selects TI1/TI3 */
+#define TIM_CCMR_CC1S_TI2	BIT(1)	/* IC1/IC3 selects TI2/TI4 */
+#define TIM_CCMR_CC2S_TI2	BIT(8)	/* IC2/IC4 selects TI2/TI4 */
+#define TIM_CCMR_CC2S_TI1	BIT(9)	/* IC2/IC4 selects TI1/TI3 */
+#define TIM_CCER_CC1E	BIT(0)	/* Capt/Comp 1  out Ena    */
+#define TIM_CCER_CC1P	BIT(1)	/* Capt/Comp 1  Polarity   */
+#define TIM_CCER_CC1NE	BIT(2)	/* Capt/Comp 1N out Ena    */
+#define TIM_CCER_CC1NP	BIT(3)	/* Capt/Comp 1N Polarity   */
+#define TIM_CCER_CC2E	BIT(4)	/* Capt/Comp 2  out Ena    */
+#define TIM_CCER_CC2P	BIT(5)	/* Capt/Comp 2  Polarity   */
+#define TIM_CCER_CC3E	BIT(8)	/* Capt/Comp 3  out Ena    */
+#define TIM_CCER_CC3P	BIT(9)	/* Capt/Comp 3  Polarity   */
+#define TIM_CCER_CC4E	BIT(12)	/* Capt/Comp 4  out Ena    */
+#define TIM_CCER_CC4P	BIT(13)	/* Capt/Comp 4  Polarity   */
+#define TIM_CCER_CCXE	(BIT(0) | BIT(4) | BIT(8) | BIT(12))
+#define TIM_BDTR_BKE	BIT(12) /* Break input enable	   */
+#define TIM_BDTR_BKP	BIT(13) /* Break input polarity	   */
+#define TIM_BDTR_AOE	BIT(14)	/* Automatic Output Enable */
+#define TIM_BDTR_MOE	BIT(15)	/* Main Output Enable      */
+#define TIM_BDTR_BKF	(BIT(16) | BIT(17) | BIT(18) | BIT(19))
+#define TIM_BDTR_BK2F	(BIT(20) | BIT(21) | BIT(22) | BIT(23))
+#define TIM_BDTR_BK2E	BIT(24) /* Break 2 input enable	   */
+#define TIM_BDTR_BK2P	BIT(25) /* Break 2 input polarity  */
+#define TIM_DCR_DBA	GENMASK(4, 0)	/* DMA base addr */
+#define TIM_DCR_DBL	GENMASK(12, 8)	/* DMA burst len */
+
+#define MAX_TIM_PSC		0xFFFF
+#define MAX_TIM_ICPSC		0x3
+#define TIM_CR2_MMS_SHIFT	4
+#define TIM_CR2_MMS2_SHIFT	20
+#define TIM_SMCR_TS_SHIFT	4
+#define TIM_BDTR_BKF_MASK	0xF
+#define TIM_BDTR_BKF_SHIFT	16
+#define TIM_BDTR_BK2F_SHIFT	20
+
+struct stm32_timers {
+	struct clk *clk;
+	struct regmap *regmap;
+	u32 max_arr;
+};
+
+#endif
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 08/12] video: backlight-pwm: use new pwm_apply_state API
  2020-03-30 14:57 ` [PATCH 08/12] video: backlight-pwm: use new pwm_apply_state API Ahmad Fatoum
@ 2020-03-31  6:10   ` Sascha Hauer
  2020-03-31  6:54     ` Ahmad Fatoum
  0 siblings, 1 reply; 19+ messages in thread
From: Sascha Hauer @ 2020-03-31  6:10 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, Ahmad Fatoum

On Mon, Mar 30, 2020 at 04:57:13PM +0200, Ahmad Fatoum wrote:
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> Use pwm_apply_state we can avoid having to store PWM state in the
> instance structure and in future we have an easy way to support new
> parameters like inverted duty cycle.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/video/backlight-pwm.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/video/backlight-pwm.c b/drivers/video/backlight-pwm.c
> index 9111a42d7544..8b6494dac929 100644
> --- a/drivers/video/backlight-pwm.c
> +++ b/drivers/video/backlight-pwm.c
> @@ -33,7 +33,6 @@ struct pwm_backlight {
>  	struct backlight_device backlight;
>  	struct pwm_device *pwm;
>  	struct regulator *power;
> -	uint32_t period;
>  	unsigned int *levels;
>  	int enable_gpio;
>  	int enable_active_high;
> @@ -91,13 +90,16 @@ static int backlight_pwm_disable(struct pwm_backlight *pwm_backlight)
>  static int compute_duty_cycle(struct pwm_backlight *pwm_backlight, int brightness)
>  {
>  	int duty_cycle;
> +	struct pwm_state state;
> +
> +	pwm_get_state(pwm_backlight->pwm, &state);
>  
>  	if (pwm_backlight->levels)
>  		duty_cycle = pwm_backlight->levels[brightness];
>  	else
>  		duty_cycle = brightness;
>  
> -	return duty_cycle * pwm_backlight->period / pwm_backlight->scale;
> +	return duty_cycle * state.period_ns / pwm_backlight->scale;
>  }
>  
>  static int backlight_pwm_set(struct backlight_device *backlight,
> @@ -105,9 +107,11 @@ static int backlight_pwm_set(struct backlight_device *backlight,
>  {
>  	struct pwm_backlight *pwm_backlight = container_of(backlight,
>  			struct pwm_backlight, backlight);
> +	struct pwm_state state;
>  
> -	pwm_config(pwm_backlight->pwm, compute_duty_cycle(pwm_backlight, brightness),
> -		   pwm_backlight->period);
> +	pwm_get_state(pwm_backlight->pwm, &state);

You read the current pwm state here...

> +	state.duty_ns = compute_duty_cycle(pwm_backlight, brightness);

and once again in compute_duty_cycle(). I think it would be nicer to
reorganize this a bit, maybe pass the state to compute_duty_cycle.

> +	pwm_apply_state(pwm_backlight->pwm, &state);
>  
>  	if (brightness)
>  		return backlight_pwm_enable(pwm_backlight);

I would assume that if you switch to the pwm_apply_state API then you do
it entirely. backlight_pwm_enable() still uses the old API to enable the
PWM.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 12/12] PWM: add support for STM32
  2020-03-30 14:57 ` [PATCH 12/12] PWM: add support for STM32 Ahmad Fatoum
@ 2020-03-31  6:41   ` Sascha Hauer
  2020-03-31  6:49     ` Ahmad Fatoum
  0 siblings, 1 reply; 19+ messages in thread
From: Sascha Hauer @ 2020-03-31  6:41 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, Ahmad Fatoum

On Mon, Mar 30, 2020 at 04:57:17PM +0200, Ahmad Fatoum wrote:
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> This driver adds support for PWM driver on STM32 platform
> based on the Linux v5.4 support.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm/dts/stm32mp151.dtsi     |  12 +
>  drivers/mfd/Kconfig              |   7 +
>  drivers/mfd/Makefile             |   1 +
>  drivers/mfd/stm32-timers.c       |  74 ++++++
>  drivers/pwm/Kconfig              |   6 +
>  drivers/pwm/Makefile             |   1 +
>  drivers/pwm/pwm-stm32.c          | 400 +++++++++++++++++++++++++++++++
>  include/linux/mfd/stm32-timers.h |  97 ++++++++
>  8 files changed, 598 insertions(+)
>  create mode 100644 drivers/mfd/stm32-timers.c
>  create mode 100644 drivers/pwm/pwm-stm32.c
>  create mode 100644 include/linux/mfd/stm32-timers.h
> 
> diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
> index 2a70a747e76e..a0870c446f45 100644
> --- a/arch/arm/dts/stm32mp151.dtsi
> +++ b/arch/arm/dts/stm32mp151.dtsi
> @@ -21,6 +21,18 @@
>  		mmc0 = &sdmmc1;
>  		mmc1 = &sdmmc2;
>  		mmc2 = &sdmmc3;
> +		pwm1 = &{/soc/timer@44000000/pwm};
> +		pwm2 = &{/soc/timer@40000000/pwm};
> +		pwm3 = &{/soc/timer@40001000/pwm};
> +		pwm4 = &{/soc/timer@40002000/pwm};
> +		pwm5 = &{/soc/timer@40003000/pwm};
> +		pwm8 = &{/soc/timer@44001000/pwm};
> +		pwm12 = &{/soc/timer@40006000/pwm};
> +		pwm13 = &{/soc/timer@40007000/pwm};
> +		pwm14 = &{/soc/timer@40008000/pwm};
> +		pwm15 = &{/soc/timer@44006000/pwm};
> +		pwm16 = &{/soc/timer@44007000/pwm};
> +		pwm17 = &{/soc/timer@44008000/pwm};

The other aliases start counting at 0. Why not the PWMs?

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 12/12] PWM: add support for STM32
  2020-03-31  6:41   ` Sascha Hauer
@ 2020-03-31  6:49     ` Ahmad Fatoum
  2020-03-31  7:49       ` Sascha Hauer
  0 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-31  6:49 UTC (permalink / raw)
  To: Sascha Hauer, Ahmad Fatoum; +Cc: barebox

Hello,

On 3/31/20 8:41 AM, Sascha Hauer wrote:
>> +		pwm1 = &{/soc/timer@44000000/pwm};
>> +		pwm2 = &{/soc/timer@40000000/pwm};
>> +		pwm3 = &{/soc/timer@40001000/pwm};
>> +		pwm4 = &{/soc/timer@40002000/pwm};
>> +		pwm5 = &{/soc/timer@40003000/pwm};
>> +		pwm8 = &{/soc/timer@44001000/pwm};
>> +		pwm12 = &{/soc/timer@40006000/pwm};
>> +		pwm13 = &{/soc/timer@40007000/pwm};
>> +		pwm14 = &{/soc/timer@40008000/pwm};
>> +		pwm15 = &{/soc/timer@44006000/pwm};
>> +		pwm16 = &{/soc/timer@44007000/pwm};
>> +		pwm17 = &{/soc/timer@44008000/pwm};
> 
> The other aliases start counting at 0. Why not the PWMs?

I named them to match the SoC data sheet,

The final device names are e.g. pwm12ch2, corresponding to TIM12_CH2
in the SoC datasheet. It's IMO the best user experience, because otherwise
you'll always have to look at the aliases in barebox to determine,
which PWM corresponds to what's printed in the schematics.

Cheers
Amad
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 08/12] video: backlight-pwm: use new pwm_apply_state API
  2020-03-31  6:10   ` Sascha Hauer
@ 2020-03-31  6:54     ` Ahmad Fatoum
  2020-03-31  7:49       ` Sascha Hauer
  0 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2020-03-31  6:54 UTC (permalink / raw)
  To: Sascha Hauer, Ahmad Fatoum; +Cc: barebox

Hi,

On 3/31/20 8:10 AM, Sascha Hauer wrote:
> On Mon, Mar 30, 2020 at 04:57:13PM +0200, Ahmad Fatoum wrote:
>> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>
>> Use pwm_apply_state we can avoid having to store PWM state in the
>> instance structure and in future we have an easy way to support new
>> parameters like inverted duty cycle.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/video/backlight-pwm.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/video/backlight-pwm.c b/drivers/video/backlight-pwm.c
>> index 9111a42d7544..8b6494dac929 100644
>> --- a/drivers/video/backlight-pwm.c
>> +++ b/drivers/video/backlight-pwm.c
>> @@ -33,7 +33,6 @@ struct pwm_backlight {
>>  	struct backlight_device backlight;
>>  	struct pwm_device *pwm;
>>  	struct regulator *power;
>> -	uint32_t period;
>>  	unsigned int *levels;
>>  	int enable_gpio;
>>  	int enable_active_high;
>> @@ -91,13 +90,16 @@ static int backlight_pwm_disable(struct pwm_backlight *pwm_backlight)
>>  static int compute_duty_cycle(struct pwm_backlight *pwm_backlight, int brightness)
>>  {
>>  	int duty_cycle;
>> +	struct pwm_state state;
>> +
>> +	pwm_get_state(pwm_backlight->pwm, &state);
>>  
>>  	if (pwm_backlight->levels)
>>  		duty_cycle = pwm_backlight->levels[brightness];
>>  	else
>>  		duty_cycle = brightness;
>>  
>> -	return duty_cycle * pwm_backlight->period / pwm_backlight->scale;
>> +	return duty_cycle * state.period_ns / pwm_backlight->scale;
>>  }
>>  
>>  static int backlight_pwm_set(struct backlight_device *backlight,
>> @@ -105,9 +107,11 @@ static int backlight_pwm_set(struct backlight_device *backlight,
>>  {
>>  	struct pwm_backlight *pwm_backlight = container_of(backlight,
>>  			struct pwm_backlight, backlight);
>> +	struct pwm_state state;
>>  
>> -	pwm_config(pwm_backlight->pwm, compute_duty_cycle(pwm_backlight, brightness),
>> -		   pwm_backlight->period);
>> +	pwm_get_state(pwm_backlight->pwm, &state);
> 
> You read the current pwm state here...
> 
>> +	state.duty_ns = compute_duty_cycle(pwm_backlight, brightness);
> 
> and once again in compute_duty_cycle(). I think it would be nicer to
> reorganize this a bit, maybe pass the state to compute_duty_cycle.
> 
>> +	pwm_apply_state(pwm_backlight->pwm, &state);
>>  
>>  	if (brightness)
>>  		return backlight_pwm_enable(pwm_backlight);
> 
> I would assume that if you switch to the pwm_apply_state API then you do
> it entirely. backlight_pwm_enable() still uses the old API to enable the
> PWM.

Ineed. Rest of patches should apply and build cleanly without this and the last one.
If they're ok, can you apply them and I respin only those two?

Cheers
Ahmad

> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 12/12] PWM: add support for STM32
  2020-03-31  6:49     ` Ahmad Fatoum
@ 2020-03-31  7:49       ` Sascha Hauer
  0 siblings, 0 replies; 19+ messages in thread
From: Sascha Hauer @ 2020-03-31  7:49 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, Ahmad Fatoum

On Tue, Mar 31, 2020 at 08:49:33AM +0200, Ahmad Fatoum wrote:
> Hello,
> 
> On 3/31/20 8:41 AM, Sascha Hauer wrote:
> >> +		pwm1 = &{/soc/timer@44000000/pwm};
> >> +		pwm2 = &{/soc/timer@40000000/pwm};
> >> +		pwm3 = &{/soc/timer@40001000/pwm};
> >> +		pwm4 = &{/soc/timer@40002000/pwm};
> >> +		pwm5 = &{/soc/timer@40003000/pwm};
> >> +		pwm8 = &{/soc/timer@44001000/pwm};
> >> +		pwm12 = &{/soc/timer@40006000/pwm};
> >> +		pwm13 = &{/soc/timer@40007000/pwm};
> >> +		pwm14 = &{/soc/timer@40008000/pwm};
> >> +		pwm15 = &{/soc/timer@44006000/pwm};
> >> +		pwm16 = &{/soc/timer@44007000/pwm};
> >> +		pwm17 = &{/soc/timer@44008000/pwm};
> > 
> > The other aliases start counting at 0. Why not the PWMs?
> 
> I named them to match the SoC data sheet,
> 
> The final device names are e.g. pwm12ch2, corresponding to TIM12_CH2
> in the SoC datasheet. It's IMO the best user experience, because otherwise
> you'll always have to look at the aliases in barebox to determine,
> which PWM corresponds to what's printed in the schematics.

ok.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 08/12] video: backlight-pwm: use new pwm_apply_state API
  2020-03-31  6:54     ` Ahmad Fatoum
@ 2020-03-31  7:49       ` Sascha Hauer
  0 siblings, 0 replies; 19+ messages in thread
From: Sascha Hauer @ 2020-03-31  7:49 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, Ahmad Fatoum

On Tue, Mar 31, 2020 at 08:54:00AM +0200, Ahmad Fatoum wrote:
> Hi,
> 
> On 3/31/20 8:10 AM, Sascha Hauer wrote:
> > On Mon, Mar 30, 2020 at 04:57:13PM +0200, Ahmad Fatoum wrote:
> >> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >>
> >> Use pwm_apply_state we can avoid having to store PWM state in the
> >> instance structure and in future we have an easy way to support new
> >> parameters like inverted duty cycle.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  drivers/video/backlight-pwm.c | 13 ++++++++-----
> >>  1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/video/backlight-pwm.c b/drivers/video/backlight-pwm.c
> >> index 9111a42d7544..8b6494dac929 100644
> >> --- a/drivers/video/backlight-pwm.c
> >> +++ b/drivers/video/backlight-pwm.c
> >> @@ -33,7 +33,6 @@ struct pwm_backlight {
> >>  	struct backlight_device backlight;
> >>  	struct pwm_device *pwm;
> >>  	struct regulator *power;
> >> -	uint32_t period;
> >>  	unsigned int *levels;
> >>  	int enable_gpio;
> >>  	int enable_active_high;
> >> @@ -91,13 +90,16 @@ static int backlight_pwm_disable(struct pwm_backlight *pwm_backlight)
> >>  static int compute_duty_cycle(struct pwm_backlight *pwm_backlight, int brightness)
> >>  {
> >>  	int duty_cycle;
> >> +	struct pwm_state state;
> >> +
> >> +	pwm_get_state(pwm_backlight->pwm, &state);
> >>  
> >>  	if (pwm_backlight->levels)
> >>  		duty_cycle = pwm_backlight->levels[brightness];
> >>  	else
> >>  		duty_cycle = brightness;
> >>  
> >> -	return duty_cycle * pwm_backlight->period / pwm_backlight->scale;
> >> +	return duty_cycle * state.period_ns / pwm_backlight->scale;
> >>  }
> >>  
> >>  static int backlight_pwm_set(struct backlight_device *backlight,
> >> @@ -105,9 +107,11 @@ static int backlight_pwm_set(struct backlight_device *backlight,
> >>  {
> >>  	struct pwm_backlight *pwm_backlight = container_of(backlight,
> >>  			struct pwm_backlight, backlight);
> >> +	struct pwm_state state;
> >>  
> >> -	pwm_config(pwm_backlight->pwm, compute_duty_cycle(pwm_backlight, brightness),
> >> -		   pwm_backlight->period);
> >> +	pwm_get_state(pwm_backlight->pwm, &state);
> > 
> > You read the current pwm state here...
> > 
> >> +	state.duty_ns = compute_duty_cycle(pwm_backlight, brightness);
> > 
> > and once again in compute_duty_cycle(). I think it would be nicer to
> > reorganize this a bit, maybe pass the state to compute_duty_cycle.
> > 
> >> +	pwm_apply_state(pwm_backlight->pwm, &state);
> >>  
> >>  	if (brightness)
> >>  		return backlight_pwm_enable(pwm_backlight);
> > 
> > I would assume that if you switch to the pwm_apply_state API then you do
> > it entirely. backlight_pwm_enable() still uses the old API to enable the
> > PWM.
> 
> Ineed. Rest of patches should apply and build cleanly without this and the last one.
> If they're ok, can you apply them and I respin only those two?

Just did that.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-03-31  7:49 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30 14:57 [PATCH 00/12] PWM: add support for ->apply, polarity and STM32 Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 01/12] led: pwm: always initialize PWM LEDs as inactive Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 02/12] PWM: core: remove FLAG_ENABLED Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 03/12] PWM: core: remove ineffectual pwm_{set,get}_duty_cycle Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 04/12] PWM: core: group PWM state into new struct pwm_state Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 05/12] PWM: core: remove old PWM API in favor of Linux ->apply Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 06/12] PWM: core: retire pwm_set_period Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 07/12] PWM: core: apply initial state in of_pwm_request Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 08/12] video: backlight-pwm: use new pwm_apply_state API Ahmad Fatoum
2020-03-31  6:10   ` Sascha Hauer
2020-03-31  6:54     ` Ahmad Fatoum
2020-03-31  7:49       ` Sascha Hauer
2020-03-30 14:57 ` [PATCH 09/12] led: pwm: " Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 10/12] PWM: core: add apply API support for polarity Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 11/12] of: introduce of_property_count_elems_of_size Ahmad Fatoum
2020-03-30 14:57 ` [PATCH 12/12] PWM: add support for STM32 Ahmad Fatoum
2020-03-31  6:41   ` Sascha Hauer
2020-03-31  6:49     ` Ahmad Fatoum
2020-03-31  7:49       ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox