linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
       [not found] <20200317123231.2843297-1-oleksandr.suvorov@toradex.com>
@ 2020-03-17 12:32 ` Oleksandr Suvorov
  2020-03-17 13:34   ` Paul Barker
                     ` (2 more replies)
  2020-03-17 12:32 ` [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag Oleksandr Suvorov
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-17 12:32 UTC (permalink / raw)
  To: devicetree, linux-pwm
  Cc: Paul Barker, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker,
	Oleksandr Suvorov, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, Thierry Reding, Tony Prisk, bcm-kernel-feedback-list,
	linux-amlogic, linux-arm-kernel, linux-kernel, linux-riscv,
	linux-rockchip

The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
Rename it to PWM_POLARITY_INVERTED.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 drivers/pwm/core.c             |  4 ++--
 drivers/pwm/pwm-atmel-hlcdc.c  |  2 +-
 drivers/pwm/pwm-atmel-tcb.c    | 16 ++++++++--------
 drivers/pwm/pwm-atmel.c        |  2 +-
 drivers/pwm/pwm-bcm-iproc.c    |  2 +-
 drivers/pwm/pwm-bcm-kona.c     |  2 +-
 drivers/pwm/pwm-ep93xx.c       |  2 +-
 drivers/pwm/pwm-fsl-ftm.c      |  2 +-
 drivers/pwm/pwm-hibvt.c        |  2 +-
 drivers/pwm/pwm-imx-tpm.c      |  2 +-
 drivers/pwm/pwm-imx27.c        |  4 ++--
 drivers/pwm/pwm-jz4740.c       |  2 +-
 drivers/pwm/pwm-meson.c        |  6 +++---
 drivers/pwm/pwm-omap-dmtimer.c |  4 ++--
 drivers/pwm/pwm-renesas-tpu.c  |  6 +++---
 drivers/pwm/pwm-rockchip.c     |  4 ++--
 drivers/pwm/pwm-sifive.c       |  4 ++--
 drivers/pwm/pwm-sun4i.c        |  2 +-
 drivers/pwm/pwm-tiecap.c       |  2 +-
 drivers/pwm/pwm-tiehrpwm.c     |  4 ++--
 drivers/pwm/pwm-vt8500.c       |  2 +-
 drivers/pwm/pwm-zx.c           |  4 ++--
 drivers/pwm/sysfs.c            |  4 ++--
 include/linux/pwm.h            |  4 ++--
 24 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 5a7f6598c05f..08afbb5b98aa 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -152,7 +152,7 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
 	pwm->args.polarity = PWM_POLARITY_NORMAL;
 
 	if (args->args_count > 2 && args->args[2] & PWM_POLARITY_INVERTED)
-		pwm->args.polarity = PWM_POLARITY_INVERSED;
+		pwm->args.polarity = PWM_POLARITY_INVERTED;
 
 	return pwm;
 }
@@ -819,7 +819,7 @@ static struct pwm_device *acpi_pwm_get(struct fwnode_handle *fwnode)
 	pwm->args.polarity = PWM_POLARITY_NORMAL;
 
 	if (args.nargs > 2 && args.args[2] & PWM_POLARITY_INVERTED)
-		pwm->args.polarity = PWM_POLARITY_INVERSED;
+		pwm->args.polarity = PWM_POLARITY_INVERTED;
 #endif
 
 	return pwm;
diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
index dcbc0489dfd4..b53a188479b0 100644
--- a/drivers/pwm/pwm-atmel-hlcdc.c
+++ b/drivers/pwm/pwm-atmel-hlcdc.c
@@ -270,7 +270,7 @@ static int atmel_hlcdc_pwm_probe(struct platform_device *pdev)
 	chip->chip.of_xlate = of_pwm_xlate_with_flags;
 	chip->chip.of_pwm_n_cells = 3;
 
-	ret = pwmchip_add_with_polarity(&chip->chip, PWM_POLARITY_INVERSED);
+	ret = pwmchip_add_with_polarity(&chip->chip, PWM_POLARITY_INVERTED);
 	if (ret) {
 		clk_disable_unprepare(hlcdc->periph_clk);
 		return ret;
diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index 85c53701958c..98526a286347 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -152,7 +152,7 @@ static void atmel_tcb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	/*
 	 * If duty is 0 the timer will be stopped and we have to
 	 * configure the output correctly on software trigger:
-	 *  - set output to high if PWM_POLARITY_INVERSED
+	 *  - set output to high if PWM_POLARITY_INVERTED
 	 *  - set output to low if PWM_POLARITY_NORMAL
 	 *
 	 * This is why we're reverting polarity in this case.
@@ -166,13 +166,13 @@ static void atmel_tcb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	/* flush old setting and set the new one */
 	if (index == 0) {
 		cmr &= ~ATMEL_TC_ACMR_MASK;
-		if (polarity == PWM_POLARITY_INVERSED)
+		if (polarity == PWM_POLARITY_INVERTED)
 			cmr |= ATMEL_TC_ASWTRG_CLEAR;
 		else
 			cmr |= ATMEL_TC_ASWTRG_SET;
 	} else {
 		cmr &= ~ATMEL_TC_BCMR_MASK;
-		if (polarity == PWM_POLARITY_INVERSED)
+		if (polarity == PWM_POLARITY_INVERTED)
 			cmr |= ATMEL_TC_BSWTRG_CLEAR;
 		else
 			cmr |= ATMEL_TC_BSWTRG_SET;
@@ -211,7 +211,7 @@ static int atmel_tcb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	/*
 	 * If duty is 0 the timer will be stopped and we have to
 	 * configure the output correctly on software trigger:
-	 *  - set output to high if PWM_POLARITY_INVERSED
+	 *  - set output to high if PWM_POLARITY_INVERTED
 	 *  - set output to low if PWM_POLARITY_NORMAL
 	 *
 	 * This is why we're reverting polarity in this case.
@@ -229,13 +229,13 @@ static int atmel_tcb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 		cmr &= ~ATMEL_TC_ACMR_MASK;
 
 		/* Set CMR flags according to given polarity */
-		if (polarity == PWM_POLARITY_INVERSED)
+		if (polarity == PWM_POLARITY_INVERTED)
 			cmr |= ATMEL_TC_ASWTRG_CLEAR;
 		else
 			cmr |= ATMEL_TC_ASWTRG_SET;
 	} else {
 		cmr &= ~ATMEL_TC_BCMR_MASK;
-		if (polarity == PWM_POLARITY_INVERSED)
+		if (polarity == PWM_POLARITY_INVERTED)
 			cmr |= ATMEL_TC_BSWTRG_CLEAR;
 		else
 			cmr |= ATMEL_TC_BSWTRG_SET;
@@ -249,12 +249,12 @@ static int atmel_tcb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	 */
 	if (tcbpwm->duty != tcbpwm->period && tcbpwm->duty > 0) {
 		if (index == 0) {
-			if (polarity == PWM_POLARITY_INVERSED)
+			if (polarity == PWM_POLARITY_INVERTED)
 				cmr |= ATMEL_TC_ACPA_SET | ATMEL_TC_ACPC_CLEAR;
 			else
 				cmr |= ATMEL_TC_ACPA_CLEAR | ATMEL_TC_ACPC_SET;
 		} else {
-			if (polarity == PWM_POLARITY_INVERSED)
+			if (polarity == PWM_POLARITY_INVERTED)
 				cmr |= ATMEL_TC_BCPB_SET | ATMEL_TC_BCPC_CLEAR;
 			else
 				cmr |= ATMEL_TC_BCPB_CLEAR | ATMEL_TC_BCPC_SET;
diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 6161e7e3e9ac..2d42f97e4b81 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -329,7 +329,7 @@ static void atmel_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	}
 
 	if (cmr & PWM_CMR_CPOL)
-		state->polarity = PWM_POLARITY_INVERSED;
+		state->polarity = PWM_POLARITY_INVERTED;
 	else
 		state->polarity = PWM_POLARITY_NORMAL;
 }
diff --git a/drivers/pwm/pwm-bcm-iproc.c b/drivers/pwm/pwm-bcm-iproc.c
index 1f829edd8ee7..574bec61e0ac 100644
--- a/drivers/pwm/pwm-bcm-iproc.c
+++ b/drivers/pwm/pwm-bcm-iproc.c
@@ -97,7 +97,7 @@ static void iproc_pwmc_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (value & BIT(IPROC_PWM_CTRL_POLARITY_SHIFT(pwm->hwpwm)))
 		state->polarity = PWM_POLARITY_NORMAL;
 	else
-		state->polarity = PWM_POLARITY_INVERSED;
+		state->polarity = PWM_POLARITY_INVERTED;
 
 	value = readl(ip->base + IPROC_PWM_PRESCALE_OFFSET);
 	prescale = value >> IPROC_PWM_PRESCALE_SHIFT(pwm->hwpwm);
diff --git a/drivers/pwm/pwm-bcm-kona.c b/drivers/pwm/pwm-bcm-kona.c
index 81da91df2529..02da511814f1 100644
--- a/drivers/pwm/pwm-bcm-kona.c
+++ b/drivers/pwm/pwm-bcm-kona.c
@@ -303,7 +303,7 @@ static int kona_pwmc_probe(struct platform_device *pdev)
 
 	clk_disable_unprepare(kp->clk);
 
-	ret = pwmchip_add_with_polarity(&kp->chip, PWM_POLARITY_INVERSED);
+	ret = pwmchip_add_with_polarity(&kp->chip, PWM_POLARITY_INVERTED);
 	if (ret < 0)
 		dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);
 
diff --git a/drivers/pwm/pwm-ep93xx.c b/drivers/pwm/pwm-ep93xx.c
index 4bab73073ad7..02345b6f9fe8 100644
--- a/drivers/pwm/pwm-ep93xx.c
+++ b/drivers/pwm/pwm-ep93xx.c
@@ -124,7 +124,7 @@ static int ep93xx_pwm_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (ret)
 		return ret;
 
-	if (polarity == PWM_POLARITY_INVERSED)
+	if (polarity == PWM_POLARITY_INVERTED)
 		writew(0x1, ep93xx_pwm->base + EP93XX_PWMx_INVERT);
 	else
 		writew(0x0, ep93xx_pwm->base + EP93XX_PWMx_INVERT);
diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
index 59272a920479..75dc30978c19 100644
--- a/drivers/pwm/pwm-fsl-ftm.c
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -287,7 +287,7 @@ static int fsl_pwm_apply_config(struct fsl_pwm_chip *fpc,
 	regmap_write(fpc->regmap, FTM_CV(pwm->hwpwm), duty);
 
 	reg_polarity = 0;
-	if (newstate->polarity == PWM_POLARITY_INVERSED)
+	if (newstate->polarity == PWM_POLARITY_INVERTED)
 		reg_polarity = BIT(pwm->hwpwm);
 
 	regmap_update_bits(fpc->regmap, FTM_POL, BIT(pwm->hwpwm), reg_polarity);
diff --git a/drivers/pwm/pwm-hibvt.c b/drivers/pwm/pwm-hibvt.c
index ad205fdad372..c57a94e7da0f 100644
--- a/drivers/pwm/pwm-hibvt.c
+++ b/drivers/pwm/pwm-hibvt.c
@@ -120,7 +120,7 @@ static void hibvt_pwm_set_polarity(struct pwm_chip *chip,
 {
 	struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
 
-	if (polarity == PWM_POLARITY_INVERSED)
+	if (polarity == PWM_POLARITY_INVERTED)
 		hibvt_pwm_set_bits(hi_pwm_chip->base, PWM_CTRL_ADDR(pwm->hwpwm),
 				PWM_POLARITY_MASK, (0x1 << PWM_POLARITY_SHIFT));
 	else
diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
index 9145f6160649..461ab2c08616 100644
--- a/drivers/pwm/pwm-imx-tpm.c
+++ b/drivers/pwm/pwm-imx-tpm.c
@@ -156,7 +156,7 @@ static void pwm_imx_tpm_get_state(struct pwm_chip *chip,
 	/* get polarity */
 	val = readl(tpm->base + PWM_IMX_TPM_CnSC(pwm->hwpwm));
 	if ((val & PWM_IMX_TPM_CnSC_ELS) == PWM_IMX_TPM_CnSC_ELS_INVERSED)
-		state->polarity = PWM_POLARITY_INVERSED;
+		state->polarity = PWM_POLARITY_INVERTED;
 	else
 		/*
 		 * Assume reserved values (2b00 and 2b11) to yield
diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index 35a7ac42269c..33d344445254 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -146,7 +146,7 @@ static void pwm_imx27_get_state(struct pwm_chip *chip,
 		state->polarity = PWM_POLARITY_NORMAL;
 		break;
 	case MX3_PWMCR_POUTC_INVERTED:
-		state->polarity = PWM_POLARITY_INVERSED;
+		state->polarity = PWM_POLARITY_INVERTED;
 		break;
 	default:
 		dev_warn(chip->dev, "can't set polarity, output disconnected");
@@ -280,7 +280,7 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	     FIELD_PREP(MX3_PWMCR_CLKSRC, MX3_PWMCR_CLKSRC_IPG_HIGH) |
 	     MX3_PWMCR_DBGEN;
 
-	if (state->polarity == PWM_POLARITY_INVERSED)
+	if (state->polarity == PWM_POLARITY_INVERTED)
 		cr |= FIELD_PREP(MX3_PWMCR_POUTC,
 				MX3_PWMCR_POUTC_INVERTED);
 
diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index 9d78cc21cb12..67075d18561f 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -130,7 +130,7 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	case PWM_POLARITY_NORMAL:
 		ctrl &= ~JZ_TIMER_CTRL_PWM_ACTIVE_LOW;
 		break;
-	case PWM_POLARITY_INVERSED:
+	case PWM_POLARITY_INVERTED:
 		ctrl |= JZ_TIMER_CTRL_PWM_ACTIVE_LOW;
 		break;
 	}
diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 6245bbdb6e6c..2d368bfc680d 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -8,7 +8,7 @@
  * N cycles for the first half period.
  * The hardware has no "polarity" setting. This driver reverses the period
  * cycles (the low length is inverted with the high length) for
- * PWM_POLARITY_INVERSED. This means that .get_state cannot read the polarity
+ * PWM_POLARITY_INVERTED. This means that .get_state cannot read the polarity
  * from the hardware.
  * Setting the duty cycle will disable and re-enable the PWM output.
  * Disabling the PWM stops the output immediately (without waiting for the
@@ -168,7 +168,7 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
 	duty = state->duty_cycle;
 	period = state->period;
 
-	if (state->polarity == PWM_POLARITY_INVERSED)
+	if (state->polarity == PWM_POLARITY_INVERTED)
 		duty = period - duty;
 
 	fin_freq = clk_get_rate(channel->clk);
@@ -275,7 +275,7 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		return -EINVAL;
 
 	if (!state->enabled) {
-		if (state->polarity == PWM_POLARITY_INVERSED) {
+		if (state->polarity == PWM_POLARITY_INVERTED) {
 			/*
 			 * This IP block revision doesn't have an "always high"
 			 * setting which we can use for "inverted disabled".
diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
index 88a3c5690fea..082ccec93133 100644
--- a/drivers/pwm/pwm-omap-dmtimer.c
+++ b/drivers/pwm/pwm-omap-dmtimer.c
@@ -190,7 +190,7 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip,
 		load_value, load_value,	match_value, match_value);
 
 	omap->pdata->set_pwm(omap->dm_timer,
-			      pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED,
+			      pwm_get_polarity(pwm) == PWM_POLARITY_INVERTED,
 			      true,
 			      PWM_OMAP_DMTIMER_TRIGGER_OVERFLOW_AND_COMPARE);
 
@@ -220,7 +220,7 @@ static int pwm_omap_dmtimer_set_polarity(struct pwm_chip *chip,
 	 */
 	mutex_lock(&omap->mutex);
 	omap->pdata->set_pwm(omap->dm_timer,
-			      polarity == PWM_POLARITY_INVERSED,
+			      polarity == PWM_POLARITY_INVERTED,
 			      true,
 			      PWM_OMAP_DMTIMER_TRIGGER_OVERFLOW_AND_COMPARE);
 	mutex_unlock(&omap->mutex);
diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
index 4a855a21b782..32beeb93ade1 100644
--- a/drivers/pwm/pwm-renesas-tpu.c
+++ b/drivers/pwm/pwm-renesas-tpu.c
@@ -108,17 +108,17 @@ static void tpu_pwm_set_pin(struct tpu_pwm_device *pwm,
 	switch (state) {
 	case TPU_PIN_INACTIVE:
 		tpu_pwm_write(pwm, TPU_TIORn,
-			      pwm->polarity == PWM_POLARITY_INVERSED ?
+			      pwm->polarity == PWM_POLARITY_INVERTED ?
 			      TPU_TIOR_IOA_1 : TPU_TIOR_IOA_0);
 		break;
 	case TPU_PIN_PWM:
 		tpu_pwm_write(pwm, TPU_TIORn,
-			      pwm->polarity == PWM_POLARITY_INVERSED ?
+			      pwm->polarity == PWM_POLARITY_INVERTED ?
 			      TPU_TIOR_IOA_0_SET : TPU_TIOR_IOA_1_CLR);
 		break;
 	case TPU_PIN_ACTIVE:
 		tpu_pwm_write(pwm, TPU_TIORn,
-			      pwm->polarity == PWM_POLARITY_INVERSED ?
+			      pwm->polarity == PWM_POLARITY_INVERTED ?
 			      TPU_TIOR_IOA_0 : TPU_TIOR_IOA_1);
 		break;
 	}
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 73352e6fbccb..c6158d559790 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -91,7 +91,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
 				 true : false;
 
 	if (pc->data->supports_polarity && !(val & PWM_DUTY_POSITIVE))
-		state->polarity = PWM_POLARITY_INVERSED;
+		state->polarity = PWM_POLARITY_INVERTED;
 	else
 		state->polarity = PWM_POLARITY_NORMAL;
 
@@ -135,7 +135,7 @@ static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	if (pc->data->supports_polarity) {
 		ctrl &= ~PWM_POLARITY_MASK;
-		if (state->polarity == PWM_POLARITY_INVERSED)
+		if (state->polarity == PWM_POLARITY_INVERTED)
 			ctrl |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
 		else
 			ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index cc63f9baa481..409123405a11 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -124,7 +124,7 @@ static void pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	state->period = ddata->real_period;
 	state->duty_cycle =
 		(u64)duty * ddata->real_period >> PWM_SIFIVE_CMPWIDTH;
-	state->polarity = PWM_POLARITY_INVERSED;
+	state->polarity = PWM_POLARITY_INVERTED;
 }
 
 static int pwm_sifive_enable(struct pwm_chip *chip, bool enable)
@@ -157,7 +157,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	int ret = 0;
 	u32 frac;
 
-	if (state->polarity != PWM_POLARITY_INVERSED)
+	if (state->polarity != PWM_POLARITY_INVERTED)
 		return -EINVAL;
 
 	ret = clk_enable(ddata->clk);
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 3e3efa6c768f..7ddcdefd2a97 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -149,7 +149,7 @@ static void sun4i_pwm_get_state(struct pwm_chip *chip,
 	if (val & BIT_CH(PWM_ACT_STATE, pwm->hwpwm))
 		state->polarity = PWM_POLARITY_NORMAL;
 	else
-		state->polarity = PWM_POLARITY_INVERSED;
+		state->polarity = PWM_POLARITY_INVERTED;
 
 	if ((val & BIT_CH(PWM_CLK_GATING | PWM_EN, pwm->hwpwm)) ==
 	    BIT_CH(PWM_CLK_GATING | PWM_EN, pwm->hwpwm))
diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index ab38c8203b79..b96b388f0969 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -118,7 +118,7 @@ static int ecap_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	value = readw(pc->mmio_base + ECCTL2);
 
-	if (polarity == PWM_POLARITY_INVERSED)
+	if (polarity == PWM_POLARITY_INVERTED)
 		/* Duty cycle defines LOW period of PWM */
 		value |= ECCTL2_APWM_POL_LOW;
 	else
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 7b4c770ce9d6..71c337443dd5 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -193,7 +193,7 @@ static void configure_polarity(struct ehrpwm_pwm_chip *pc, int chan)
 		aqctl_reg = AQCTLB;
 		aqctl_mask = AQCTL_CBU_MASK;
 
-		if (pc->polarity[chan] == PWM_POLARITY_INVERSED)
+		if (pc->polarity[chan] == PWM_POLARITY_INVERTED)
 			aqctl_val = AQCTL_CHANB_POLINVERSED;
 		else
 			aqctl_val = AQCTL_CHANB_POLNORMAL;
@@ -201,7 +201,7 @@ static void configure_polarity(struct ehrpwm_pwm_chip *pc, int chan)
 		aqctl_reg = AQCTLA;
 		aqctl_mask = AQCTL_CAU_MASK;
 
-		if (pc->polarity[chan] == PWM_POLARITY_INVERSED)
+		if (pc->polarity[chan] == PWM_POLARITY_INVERTED)
 			aqctl_val = AQCTL_CHANA_POLINVERSED;
 		else
 			aqctl_val = AQCTL_CHANA_POLNORMAL;
diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c
index 11d45e56a923..fc434965c5ed 100644
--- a/drivers/pwm/pwm-vt8500.c
+++ b/drivers/pwm/pwm-vt8500.c
@@ -165,7 +165,7 @@ static int vt8500_pwm_set_polarity(struct pwm_chip *chip,
 
 	val = readl(vt8500->base + REG_CTRL(pwm->hwpwm));
 
-	if (polarity == PWM_POLARITY_INVERSED)
+	if (polarity == PWM_POLARITY_INVERTED)
 		val |= CTRL_INVERT;
 	else
 		val &= ~CTRL_INVERT;
diff --git a/drivers/pwm/pwm-zx.c b/drivers/pwm/pwm-zx.c
index e2c21cc34a96..dc7d20e52c52 100644
--- a/drivers/pwm/pwm-zx.c
+++ b/drivers/pwm/pwm-zx.c
@@ -75,7 +75,7 @@ static void zx_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (value & ZX_PWM_POLAR)
 		state->polarity = PWM_POLARITY_NORMAL;
 	else
-		state->polarity = PWM_POLARITY_INVERSED;
+		state->polarity = PWM_POLARITY_INVERTED;
 
 	if (value & ZX_PWM_EN)
 		state->enabled = true;
@@ -158,7 +158,7 @@ static int zx_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	if (state->polarity != cstate.polarity)
 		zx_pwm_set_mask(zpc, pwm->hwpwm, ZX_PWM_MODE, ZX_PWM_POLAR,
-				(state->polarity == PWM_POLARITY_INVERSED) ?
+				(state->polarity == PWM_POLARITY_INVERTED) ?
 				 0 : ZX_PWM_POLAR);
 
 	if (state->period != cstate.period ||
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 2389b8669846..769ac09c56c2 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -166,7 +166,7 @@ static ssize_t polarity_show(struct device *child,
 		polarity = "normal";
 		break;
 
-	case PWM_POLARITY_INVERSED:
+	case PWM_POLARITY_INVERTED:
 		polarity = "inversed";
 		break;
 	}
@@ -187,7 +187,7 @@ static ssize_t polarity_store(struct device *child,
 	if (sysfs_streq(buf, "normal"))
 		polarity = PWM_POLARITY_NORMAL;
 	else if (sysfs_streq(buf, "inversed"))
-		polarity = PWM_POLARITY_INVERSED;
+		polarity = PWM_POLARITY_INVERTED;
 	else
 		return -EINVAL;
 
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 0ef808d925bb..38b7ed8ef913 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -16,13 +16,13 @@ struct pwm_chip;
  * @PWM_POLARITY_NORMAL: a high signal for the duration of the duty-
  * cycle, followed by a low signal for the remainder of the pulse
  * period
- * @PWM_POLARITY_INVERSED: a low signal for the duration of the duty-
+ * @PWM_POLARITY_INVERTED: a low signal for the duration of the duty-
  * cycle, followed by a high signal for the remainder of the pulse
  * period
  */
 enum pwm_polarity {
 	PWM_POLARITY_NORMAL,
-	PWM_POLARITY_INVERSED,
+	PWM_POLARITY_INVERTED,
 };
 
 /**
-- 
2.24.1


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

* [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag
       [not found] <20200317123231.2843297-1-oleksandr.suvorov@toradex.com>
  2020-03-17 12:32 ` [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum Oleksandr Suvorov
@ 2020-03-17 12:32 ` Oleksandr Suvorov
  2020-03-17 17:43   ` Thierry Reding
  2020-03-17 22:58   ` Laurent Pinchart
  2020-03-17 12:32 ` [RFC PATCH 3/7] dt-bindings: pwm: add normal " Oleksandr Suvorov
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-17 12:32 UTC (permalink / raw)
  To: devicetree, linux-pwm
  Cc: Paul Barker, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker,
	Oleksandr Suvorov, Rob Herring, Thierry Reding, linux-kernel

Add the description of PWM_POLARITY_NORMAL flag.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 Documentation/devicetree/bindings/pwm/pwm.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
index 084886bd721e..440c6b9a6a4e 100644
--- a/Documentation/devicetree/bindings/pwm/pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm.txt
@@ -46,6 +46,7 @@ period in nanoseconds.
 Optionally, the pwm-specifier can encode a number of flags (defined in
 <dt-bindings/pwm/pwm.h>) in a third cell:
 - PWM_POLARITY_INVERTED: invert the PWM signal polarity
+- PWM_POLARITY_NORMAL: don't invert the PWM signal polarity
 
 Example with optional PWM specifier for inverse polarity
 
-- 
2.24.1


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

* [RFC PATCH 3/7] dt-bindings: pwm: add normal PWM polarity flag
       [not found] <20200317123231.2843297-1-oleksandr.suvorov@toradex.com>
  2020-03-17 12:32 ` [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum Oleksandr Suvorov
  2020-03-17 12:32 ` [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag Oleksandr Suvorov
@ 2020-03-17 12:32 ` Oleksandr Suvorov
  2020-03-17 13:36   ` Paul Barker
                     ` (2 more replies)
  2020-03-17 12:32 ` [RFC PATCH 4/7] dt-bindings: pwm: add description of PWM polarity Oleksandr Suvorov
                   ` (3 subsequent siblings)
  6 siblings, 3 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-17 12:32 UTC (permalink / raw)
  To: devicetree, linux-pwm
  Cc: Paul Barker, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker,
	Oleksandr Suvorov, Rob Herring, linux-kernel

PWM can have a normal polarity and a reverted one. The reverted polarity
value is defined.
Define the PWM_POLARITY_NORMAL to be used further.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 include/dt-bindings/pwm/pwm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
index ab9a077e3c7d..6b58caa6385e 100644
--- a/include/dt-bindings/pwm/pwm.h
+++ b/include/dt-bindings/pwm/pwm.h
@@ -10,6 +10,7 @@
 #ifndef _DT_BINDINGS_PWM_PWM_H
 #define _DT_BINDINGS_PWM_PWM_H
 
+#define PWM_POLARITY_NORMAL			0
 #define PWM_POLARITY_INVERTED			(1 << 0)
 
 #endif
-- 
2.24.1


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

* [RFC PATCH 4/7] dt-bindings: pwm: add description of PWM polarity
       [not found] <20200317123231.2843297-1-oleksandr.suvorov@toradex.com>
                   ` (2 preceding siblings ...)
  2020-03-17 12:32 ` [RFC PATCH 3/7] dt-bindings: pwm: add normal " Oleksandr Suvorov
@ 2020-03-17 12:32 ` Oleksandr Suvorov
  2020-03-17 23:01   ` Laurent Pinchart
  2020-03-17 12:32 ` [RFC PATCH 5/7] pwm: replace polarity enum with macros Oleksandr Suvorov
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-17 12:32 UTC (permalink / raw)
  To: devicetree, linux-pwm
  Cc: Paul Barker, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker,
	Oleksandr Suvorov, Rob Herring, linux-kernel

Move the description of the PWM signal polarity from
<linux/pwm.h>, prepare for removing the polarity
definition from <linux/pwm.h>.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 include/dt-bindings/pwm/pwm.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
index 6b58caa6385e..c07da2088a61 100644
--- a/include/dt-bindings/pwm/pwm.h
+++ b/include/dt-bindings/pwm/pwm.h
@@ -10,7 +10,16 @@
 #ifndef _DT_BINDINGS_PWM_PWM_H
 #define _DT_BINDINGS_PWM_PWM_H
 
+/**
+ * a high signal for the duration of the duty-cycle, followed by a low signal
+ * for the remainder of the pulse period.
+ */
 #define PWM_POLARITY_NORMAL			0
+
+/**
+ * a low signal for the duration of the duty-cycle, followed by a high signal
+ * for the remainder of the pulse period.
+ */
 #define PWM_POLARITY_INVERTED			(1 << 0)
 
 #endif
-- 
2.24.1


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

* [RFC PATCH 5/7] pwm: replace polarity enum with macros
       [not found] <20200317123231.2843297-1-oleksandr.suvorov@toradex.com>
                   ` (3 preceding siblings ...)
  2020-03-17 12:32 ` [RFC PATCH 4/7] dt-bindings: pwm: add description of PWM polarity Oleksandr Suvorov
@ 2020-03-17 12:32 ` Oleksandr Suvorov
  2020-03-17 12:32 ` [RFC PATCH 6/7] arm64: dts: pwm: replace polarity constant with macro Oleksandr Suvorov
  2020-03-17 12:32 ` [RFC PATCH 7/7] arm: " Oleksandr Suvorov
  6 siblings, 0 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-17 12:32 UTC (permalink / raw)
  To: devicetree, linux-pwm
  Cc: Paul Barker, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker,
	Oleksandr Suvorov, Alex Elder, Alexandre Belloni,
	Alexandre Torgue, Bartlomiej Zolnierkiewicz, Daniel Thompson,
	Fabrice Gasnier, Florian Fainelli, Greg Kroah-Hartman,
	Ingo Molnar, Jingoo Han, Johan Hovold, Lee Jones,
	Ludovic Desroches, Maxime Coquelin, Milo Kim, Nicolas Ferre,
	Nicolas Saenz Julienne, Ray Jui, Scott Branden, Steven Rostedt,
	Thierry Reding, Tony Prisk, Vladimir Zapolskiy,
	bcm-kernel-feedback-list, devel, dri-devel, greybus-dev,
	linux-arm-kernel, linux-fbdev, linux-kernel, linux-rpi-kernel,
	linux-stm32

To avoid duplication of pwm polarity definitions,
remove "enum pwm_polarity" and use macros instead.

Prepare to use both polarity flags in DTs.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 drivers/pwm/core.c                  |  2 +-
 drivers/pwm/pwm-atmel-tcb.c         |  8 ++++----
 drivers/pwm/pwm-bcm-kona.c          |  2 +-
 drivers/pwm/pwm-bcm2835.c           |  2 +-
 drivers/pwm/pwm-berlin.c            |  2 +-
 drivers/pwm/pwm-ep93xx.c            |  2 +-
 drivers/pwm/pwm-hibvt.c             |  2 +-
 drivers/pwm/pwm-lpc18xx-sct.c       |  2 +-
 drivers/pwm/pwm-omap-dmtimer.c      |  2 +-
 drivers/pwm/pwm-renesas-tpu.c       |  4 ++--
 drivers/pwm/pwm-samsung.c           |  2 +-
 drivers/pwm/pwm-stm32.c             |  2 +-
 drivers/pwm/pwm-tiecap.c            |  2 +-
 drivers/pwm/pwm-tiehrpwm.c          |  4 ++--
 drivers/pwm/pwm-vt8500.c            |  2 +-
 drivers/pwm/sysfs.c                 |  2 +-
 drivers/staging/greybus/pwm.c       |  2 +-
 drivers/video/backlight/lp8788_bl.c |  2 +-
 include/linux/mfd/lp8788.h          |  2 +-
 include/linux/pwm.h                 | 29 ++++++++---------------------
 include/trace/events/pwm.h          |  2 +-
 21 files changed, 33 insertions(+), 46 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 08afbb5b98aa..2cb9db8d545b 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -257,7 +257,7 @@ static bool pwm_ops_check(const struct pwm_ops *ops)
  * Returns: 0 on success or a negative error code on failure.
  */
 int pwmchip_add_with_polarity(struct pwm_chip *chip,
-			      enum pwm_polarity polarity)
+			      unsigned int polarity)
 {
 	struct pwm_device *pwm;
 	unsigned int i;
diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index 98526a286347..9e8a0b4b1751 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -31,7 +31,7 @@
 				 ATMEL_TC_BEEVT | ATMEL_TC_BSWTRG)
 
 struct atmel_tcb_pwm_device {
-	enum pwm_polarity polarity;	/* PWM polarity */
+	unsigned int polarity;		/* PWM polarity */
 	unsigned div;			/* PWM clock divider */
 	unsigned duty;			/* PWM duty expressed in clk cycles */
 	unsigned period;		/* PWM period expressed in clk cycles */
@@ -60,7 +60,7 @@ static inline struct atmel_tcb_pwm_chip *to_tcb_chip(struct pwm_chip *chip)
 
 static int atmel_tcb_pwm_set_polarity(struct pwm_chip *chip,
 				      struct pwm_device *pwm,
-				      enum pwm_polarity polarity)
+				      unsigned int polarity)
 {
 	struct atmel_tcb_pwm_device *tcbpwm = pwm_get_chip_data(pwm);
 
@@ -147,7 +147,7 @@ static void atmel_tcb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	unsigned group = pwm->hwpwm / 2;
 	unsigned index = pwm->hwpwm % 2;
 	unsigned cmr;
-	enum pwm_polarity polarity = tcbpwm->polarity;
+	unsigned int polarity = tcbpwm->polarity;
 
 	/*
 	 * If duty is 0 the timer will be stopped and we have to
@@ -206,7 +206,7 @@ static int atmel_tcb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	unsigned group = pwm->hwpwm / 2;
 	unsigned index = pwm->hwpwm % 2;
 	u32 cmr;
-	enum pwm_polarity polarity = tcbpwm->polarity;
+	unsigned int polarity = tcbpwm->polarity;
 
 	/*
 	 * If duty is 0 the timer will be stopped and we have to
diff --git a/drivers/pwm/pwm-bcm-kona.c b/drivers/pwm/pwm-bcm-kona.c
index 02da511814f1..83eab0cc51ce 100644
--- a/drivers/pwm/pwm-bcm-kona.c
+++ b/drivers/pwm/pwm-bcm-kona.c
@@ -174,7 +174,7 @@ static int kona_pwmc_config(struct pwm_chip *chip, struct pwm_device *pwm,
 }
 
 static int kona_pwmc_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
-				  enum pwm_polarity polarity)
+				  unsigned int polarity)
 {
 	struct kona_pwmc *kp = to_kona_pwmc(chip);
 	unsigned int chan = pwm->hwpwm;
diff --git a/drivers/pwm/pwm-bcm2835.c b/drivers/pwm/pwm-bcm2835.c
index 91e24f01b54e..2110aef85f19 100644
--- a/drivers/pwm/pwm-bcm2835.c
+++ b/drivers/pwm/pwm-bcm2835.c
@@ -107,7 +107,7 @@ static void bcm2835_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 }
 
 static int bcm2835_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
-				enum pwm_polarity polarity)
+				unsigned int polarity)
 {
 	struct bcm2835_pwm *pc = to_bcm2835_pwm(chip);
 	u32 value;
diff --git a/drivers/pwm/pwm-berlin.c b/drivers/pwm/pwm-berlin.c
index b91c477cc84b..1a080bf33047 100644
--- a/drivers/pwm/pwm-berlin.c
+++ b/drivers/pwm/pwm-berlin.c
@@ -127,7 +127,7 @@ static int berlin_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm_dev,
 
 static int berlin_pwm_set_polarity(struct pwm_chip *chip,
 				   struct pwm_device *pwm_dev,
-				   enum pwm_polarity polarity)
+				   unsigned int polarity)
 {
 	struct berlin_pwm_chip *pwm = to_berlin_pwm_chip(chip);
 	u32 value;
diff --git a/drivers/pwm/pwm-ep93xx.c b/drivers/pwm/pwm-ep93xx.c
index 02345b6f9fe8..71897ad5f6a0 100644
--- a/drivers/pwm/pwm-ep93xx.c
+++ b/drivers/pwm/pwm-ep93xx.c
@@ -111,7 +111,7 @@ static int ep93xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 }
 
 static int ep93xx_pwm_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
-			       enum pwm_polarity polarity)
+			       unsigned int polarity)
 {
 	struct ep93xx_pwm *ep93xx_pwm = to_ep93xx_pwm(chip);
 	int ret;
diff --git a/drivers/pwm/pwm-hibvt.c b/drivers/pwm/pwm-hibvt.c
index c57a94e7da0f..7e39abce0c14 100644
--- a/drivers/pwm/pwm-hibvt.c
+++ b/drivers/pwm/pwm-hibvt.c
@@ -116,7 +116,7 @@ static void hibvt_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static void hibvt_pwm_set_polarity(struct pwm_chip *chip,
 					struct pwm_device *pwm,
-					enum pwm_polarity polarity)
+					unsigned int polarity)
 {
 	struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
 
diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index 5ff11145c1a3..3ebb7cca0204 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -225,7 +225,7 @@ static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static int lpc18xx_pwm_set_polarity(struct pwm_chip *chip,
 				    struct pwm_device *pwm,
-				    enum pwm_polarity polarity)
+				    unsigned int polarity)
 {
 	return 0;
 }
diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
index 082ccec93133..ebbd1fe57d57 100644
--- a/drivers/pwm/pwm-omap-dmtimer.c
+++ b/drivers/pwm/pwm-omap-dmtimer.c
@@ -210,7 +210,7 @@ static int pwm_omap_dmtimer_config(struct pwm_chip *chip,
 
 static int pwm_omap_dmtimer_set_polarity(struct pwm_chip *chip,
 					 struct pwm_device *pwm,
-					 enum pwm_polarity polarity)
+					 unsigned int polarity)
 {
 	struct pwm_omap_dmtimer_chip *omap = to_pwm_omap_dmtimer_chip(chip);
 
diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
index 32beeb93ade1..3c594cef2d5a 100644
--- a/drivers/pwm/pwm-renesas-tpu.c
+++ b/drivers/pwm/pwm-renesas-tpu.c
@@ -72,7 +72,7 @@ struct tpu_pwm_device {
 	struct tpu_device *tpu;
 	unsigned int channel;		/* Channel number in the TPU */
 
-	enum pwm_polarity polarity;
+	unsigned int polarity;
 	unsigned int prescaler;
 	u16 period;
 	u16 duty;
@@ -325,7 +325,7 @@ static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *_pwm,
 }
 
 static int tpu_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *_pwm,
-				enum pwm_polarity polarity)
+				unsigned int polarity)
 {
 	struct tpu_pwm_device *pwm = pwm_get_chip_data(_pwm);
 
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 87a886f7dc2f..7bf4f76e25bc 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -413,7 +413,7 @@ static void pwm_samsung_set_invert(struct samsung_pwm_chip *chip,
 
 static int pwm_samsung_set_polarity(struct pwm_chip *chip,
 				    struct pwm_device *pwm,
-				    enum pwm_polarity polarity)
+				    unsigned int polarity)
 {
 	struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip);
 	bool invert = (polarity == PWM_POLARITY_NORMAL);
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index d3be944f2ae9..a83ea66f36c9 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -383,7 +383,7 @@ static int stm32_pwm_config(struct stm32_pwm *priv, int ch,
 }
 
 static int stm32_pwm_set_polarity(struct stm32_pwm *priv, int ch,
-				  enum pwm_polarity polarity)
+				  unsigned int polarity)
 {
 	u32 mask;
 
diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index b96b388f0969..744144f83355 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -109,7 +109,7 @@ static int ecap_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 }
 
 static int ecap_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
-				 enum pwm_polarity polarity)
+				 unsigned int polarity)
 {
 	struct ecap_pwm_chip *pc = to_ecap_pwm_chip(chip);
 	u16 value;
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 71c337443dd5..cde0231e835a 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -109,7 +109,7 @@ struct ehrpwm_pwm_chip {
 	unsigned long clk_rate;
 	void __iomem *mmio_base;
 	unsigned long period_cycles[NUM_PWM_CHANNEL];
-	enum pwm_polarity polarity[NUM_PWM_CHANNEL];
+	unsigned int polarity[NUM_PWM_CHANNEL];
 	struct clk *tbclk;
 	struct ehrpwm_context ctx;
 };
@@ -306,7 +306,7 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static int ehrpwm_pwm_set_polarity(struct pwm_chip *chip,
 				   struct pwm_device *pwm,
-				   enum pwm_polarity polarity)
+				   unsigned int polarity)
 {
 	struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
 
diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c
index fc434965c5ed..076c9f207d64 100644
--- a/drivers/pwm/pwm-vt8500.c
+++ b/drivers/pwm/pwm-vt8500.c
@@ -158,7 +158,7 @@ static void vt8500_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 
 static int vt8500_pwm_set_polarity(struct pwm_chip *chip,
 				   struct pwm_device *pwm,
-				   enum pwm_polarity polarity)
+				   unsigned int polarity)
 {
 	struct vt8500_chip *vt8500 = to_vt8500_chip(chip);
 	u32 val;
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 769ac09c56c2..7cf787ac5e23 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -180,7 +180,7 @@ static ssize_t polarity_store(struct device *child,
 {
 	struct pwm_export *export = child_to_pwm_export(child);
 	struct pwm_device *pwm = export->pwm;
-	enum pwm_polarity polarity;
+	unsigned int polarity;
 	struct pwm_state state;
 	int ret;
 
diff --git a/drivers/staging/greybus/pwm.c b/drivers/staging/greybus/pwm.c
index 891a6a672378..338c76c4be9b 100644
--- a/drivers/staging/greybus/pwm.c
+++ b/drivers/staging/greybus/pwm.c
@@ -213,7 +213,7 @@ static int gb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 };
 
 static int gb_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
-			       enum pwm_polarity polarity)
+			       unsigned int polarity)
 {
 	struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip);
 
diff --git a/drivers/video/backlight/lp8788_bl.c b/drivers/video/backlight/lp8788_bl.c
index ba42f3fe0c73..a589d5e51865 100644
--- a/drivers/video/backlight/lp8788_bl.c
+++ b/drivers/video/backlight/lp8788_bl.c
@@ -37,7 +37,7 @@ struct lp8788_bl_config {
 	enum lp8788_bl_full_scale_current full_scale;
 	enum lp8788_bl_ramp_step rise_time;
 	enum lp8788_bl_ramp_step fall_time;
-	enum pwm_polarity pwm_pol;
+	unsigned int pwm_pol;
 };
 
 struct lp8788_bl {
diff --git a/include/linux/mfd/lp8788.h b/include/linux/mfd/lp8788.h
index 3d5c480d58ea..e0321aedf4c0 100644
--- a/include/linux/mfd/lp8788.h
+++ b/include/linux/mfd/lp8788.h
@@ -227,7 +227,7 @@ struct lp8788_backlight_platform_data {
 	enum lp8788_bl_full_scale_current full_scale;
 	enum lp8788_bl_ramp_step rise_time;
 	enum lp8788_bl_ramp_step fall_time;
-	enum pwm_polarity pwm_pol;
+	unsigned int pwm_pol;
 	unsigned int period_ns;
 };
 
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 38b7ed8ef913..c7b35f0602fa 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -5,26 +5,13 @@
 #include <linux/err.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <dt-bindings/pwm/pwm.h>
 
 struct pwm_capture;
 struct seq_file;
 
 struct pwm_chip;
 
-/**
- * enum pwm_polarity - polarity of a PWM signal
- * @PWM_POLARITY_NORMAL: a high signal for the duration of the duty-
- * cycle, followed by a low signal for the remainder of the pulse
- * period
- * @PWM_POLARITY_INVERTED: a low signal for the duration of the duty-
- * cycle, followed by a high signal for the remainder of the pulse
- * period
- */
-enum pwm_polarity {
-	PWM_POLARITY_NORMAL,
-	PWM_POLARITY_INVERTED,
-};
-
 /**
  * struct pwm_args - board-dependent PWM arguments
  * @period: reference period
@@ -40,7 +27,7 @@ enum pwm_polarity {
  */
 struct pwm_args {
 	unsigned int period;
-	enum pwm_polarity polarity;
+	unsigned int polarity;
 };
 
 enum {
@@ -58,7 +45,7 @@ enum {
 struct pwm_state {
 	unsigned int period;
 	unsigned int duty_cycle;
-	enum pwm_polarity polarity;
+	unsigned int polarity;
 	bool enabled;
 };
 
@@ -135,7 +122,7 @@ static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
 	return state.duty_cycle;
 }
 
-static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
+static inline unsigned int pwm_get_polarity(const struct pwm_device *pwm)
 {
 	struct pwm_state state;
 
@@ -268,7 +255,7 @@ struct pwm_ops {
 	int (*config)(struct pwm_chip *chip, struct pwm_device *pwm,
 		      int duty_ns, int period_ns);
 	int (*set_polarity)(struct pwm_chip *chip, struct pwm_device *pwm,
-			    enum pwm_polarity polarity);
+			    unsigned int polarity);
 	int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm);
 	void (*disable)(struct pwm_chip *chip, struct pwm_device *pwm);
 };
@@ -391,7 +378,7 @@ int pwm_set_chip_data(struct pwm_device *pwm, void *data);
 void *pwm_get_chip_data(struct pwm_device *pwm);
 
 int pwmchip_add_with_polarity(struct pwm_chip *chip,
-			      enum pwm_polarity polarity);
+			      unsigned int polarity);
 int pwmchip_add(struct pwm_chip *chip);
 int pwmchip_remove(struct pwm_chip *chip);
 struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
@@ -471,7 +458,7 @@ static inline int pwmchip_add(struct pwm_chip *chip)
 	return -EINVAL;
 }
 
-static inline int pwmchip_add_inversed(struct pwm_chip *chip)
+static inline int pwmchip_add_inverted(struct pwm_chip *chip)
 {
 	return -EINVAL;
 }
@@ -569,7 +556,7 @@ struct pwm_lookup {
 	const char *dev_id;
 	const char *con_id;
 	unsigned int period;
-	enum pwm_polarity polarity;
+	unsigned int polarity;
 	const char *module; /* optional, may be NULL */
 };
 
diff --git a/include/trace/events/pwm.h b/include/trace/events/pwm.h
index cf243de41cc8..e3f40ba2ab9c 100644
--- a/include/trace/events/pwm.h
+++ b/include/trace/events/pwm.h
@@ -18,7 +18,7 @@ DECLARE_EVENT_CLASS(pwm,
 		__field(struct pwm_device *, pwm)
 		__field(u64, period)
 		__field(u64, duty_cycle)
-		__field(enum pwm_polarity, polarity)
+		__field(unsigned int, polarity)
 		__field(bool, enabled)
 	),
 
-- 
2.24.1


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

* [RFC PATCH 6/7] arm64: dts: pwm: replace polarity constant with macro
       [not found] <20200317123231.2843297-1-oleksandr.suvorov@toradex.com>
                   ` (4 preceding siblings ...)
  2020-03-17 12:32 ` [RFC PATCH 5/7] pwm: replace polarity enum with macros Oleksandr Suvorov
@ 2020-03-17 12:32 ` Oleksandr Suvorov
  2020-03-20 10:03   ` Krzysztof Kozlowski
  2020-03-17 12:32 ` [RFC PATCH 7/7] arm: " Oleksandr Suvorov
  6 siblings, 1 reply; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-17 12:32 UTC (permalink / raw)
  To: devicetree, linux-pwm
  Cc: Paul Barker, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker,
	Oleksandr Suvorov, Andrius Štikonas, Andy Yan, Chen-Yu Tsai,
	Christoph Muellner, Daniel Lezcano, Enric Balletbo i Serra,
	Heiko Stuebner, Hugh Cole-Baker, Jagan Teki, Johan Jonker,
	Katsuhiro Suzuki, Kever Yang, Kevin Hilman, Krzysztof Kozlowski,
	Kukjin Kim, Marc Zyngier, Markus Reichl, Maxime Ripard,
	Miquel Raynal, Nick Xie, Philipp Tomsich, Rob Herring,
	Robin Murphy, Soeren Moch, linux-amlogic, linux-arm-kernel,
	linux-kernel, linux-rockchip, linux-samsung-soc

There is the PWM_POLARITY_NORMAL defined and describled in
<dt-bindings/pwm/pwm.h> and used by pwm drivers.

This patch converts all '0' constant in pwms parameters into
PWM_POLARITY_NORMAL.

Replace with sed regexp:
's/(pwms = <&[a-zA-Z_0-9]+ [0-9]+ [0-9]+) 0>/\1 PWM_POLARITY_NORMAL>/'

Then:
- include pwm.h in some dts/dtsi to fix building errors about undefined
  symbols.
- fix the patman warnings about the code format;

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts      | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts       | 2 +-
 arch/arm64/boot/dts/amlogic/meson-axg-s400.dts             | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi                 | 1 +
 arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi          | 1 +
 arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts          | 5 +++--
 arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts            | 2 +-
 arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts         | 5 +++--
 arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi    | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts       | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-g12b-ugoos-am6.dts       | 7 ++++---
 arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi        | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi                  | 1 +
 arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts       | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts     | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi           | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi       | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi          | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts       | 3 ++-
 .../boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts    | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts | 2 +-
 .../arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi      | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts      | 5 +++--
 arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts         | 3 ++-
 arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi         | 4 +++-
 arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts     | 2 +-
 arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts           | 7 ++++---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi      | 3 ++-
 arch/arm64/boot/dts/rockchip/px30-evb.dts                  | 2 +-
 arch/arm64/boot/dts/rockchip/px30.dtsi                     | 1 +
 arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts             | 2 +-
 arch/arm64/boot/dts/rockchip/rk3308.dtsi                   | 1 +
 arch/arm64/boot/dts/rockchip/rk3399-evb.dts                | 4 ++--
 arch/arm64/boot/dts/rockchip/rk3399-firefly.dts            | 2 +-
 arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi    | 2 +-
 arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi       | 4 ++--
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi               | 6 +++---
 arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi       | 2 +-
 arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts          | 2 +-
 arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi            | 2 +-
 arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi         | 2 +-
 arch/arm64/boot/dts/rockchip/rk3399-sapphire-excavator.dts | 2 +-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi                   | 1 +
 44 files changed, 77 insertions(+), 51 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index 64b1c54f87c0..adbcf6abf338 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -23,7 +23,7 @@ aliases {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm 0 50000 0>;
+		pwms = <&pwm 0 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 5 10 15 20 30 40 55 70 85 100>;
 		default-brightness-level = <2>;
 		enable-gpios = <&pio 3 23 GPIO_ACTIVE_HIGH>; /* PD23 */
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index f5df5f705b72..083f3dc9deaa 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -21,7 +21,7 @@ aliases {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm 0 50000 0>;
+		pwms = <&pwm 0 50000 PWM_POLARITY_NORMAL>;
 		power-supply = <&reg_dcdc1>;
 		brightness-levels = <0 5 7 10 14 20 28 40 56 80 112>;
 		default-brightness-level = <5>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
index cb1360ae1211..97b31a914def 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
@@ -356,7 +356,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ab 0 30518 0>; /* PWM_A at 32.768KHz */
+		/* PWM_A at 32.768KHz */
+		pwms = <&pwm_ab 0 30518 PWM_POLARITY_NORMAL>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index aace3d32a3df..e536436f6306 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -10,6 +10,7 @@
 #include <dt-bindings/gpio/meson-axg-gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/pwm/pwm.h>
 #include <dt-bindings/reset/amlogic,meson-axg-audio-arb.h>
 #include <dt-bindings/reset/amlogic,meson-axg-reset.h>
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 87b9a47a51b9..c1f8232fdde1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/clock/g12a-aoclkc.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/pwm/pwm.h>
 #include <dt-bindings/reset/amlogic,meson-g12a-reset.h>
 #include <dt-bindings/thermal/thermal.h>
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
index 168f460e11fa..7408c5b58105 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
@@ -141,7 +141,7 @@ vddcpu: regulator-vddcpu {
 
 		vin-supply = <&dc_in>;
 
-		pwms = <&pwm_AO_cd 1 1250 0>;
+		pwms = <&pwm_AO_cd 1 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
@@ -176,7 +176,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sound {
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
index 2a324f0136e3..7b1e17267ae3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
@@ -141,7 +141,7 @@ vddcpu: regulator-vddcpu {
 
 		vin-supply = <&main_12v>;
 
-		pwms = <&pwm_AO_cd 1 1250 0>;
+		pwms = <&pwm_AO_cd 1 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
index 4f2596d82989..8cc90ae9b326 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
@@ -141,7 +141,7 @@ vddcpu: regulator-vddcpu {
 
 		vin-supply = <&dc_in>;
 
-		pwms = <&pwm_AO_cd 1 1250 0>;
+		pwms = <&pwm_AO_cd 1 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
@@ -232,7 +232,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi
index b1fab5749ca8..9d1411724dbb 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi
@@ -20,7 +20,7 @@ vddcpu_a: regulator-vddcpu-a {
 
 		vin-supply = <&dc_in>;
 
-		pwms = <&pwm_ab 0 1250 0>;
+		pwms = <&pwm_ab 0 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
@@ -39,7 +39,7 @@ vddcpu_b: regulator-vddcpu-b {
 
 		vin-supply = <&vsys_3v3>;
 
-		pwms = <&pwm_AO_cd 1 1250 0>;
+		pwms = <&pwm_AO_cd 1 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
index 8830d3844885..4c3252b0cc67 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
@@ -127,7 +127,7 @@ vddcpu_a: regulator-vddcpu-a {
 
 		vin-supply = <&main_12v>;
 
-		pwms = <&pwm_ab 0 1250 0>;
+		pwms = <&pwm_ab 0 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
@@ -146,7 +146,7 @@ vddcpu_b: regulator-vddcpu-b {
 
 		vin-supply = <&main_12v>;
 
-		pwms = <&pwm_AO_cd 1 1250 0>;
+		pwms = <&pwm_AO_cd 1 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-ugoos-am6.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-ugoos-am6.dts
index ccd0bced01e8..15759f3df472 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-ugoos-am6.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-ugoos-am6.dts
@@ -109,7 +109,7 @@ vddcpu_a: regulator-vddcpu-a {
 
 		vin-supply = <&main_12v>;
 
-		pwms = <&pwm_ab 0 1250 0>;
+		pwms = <&pwm_ab 0 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
@@ -128,7 +128,7 @@ vddcpu_b: regulator-vddcpu-b {
 
 		vin-supply = <&main_12v>;
 
-		pwms = <&pwm_AO_cd 1 1250 0>;
+		pwms = <&pwm_AO_cd 1 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
@@ -282,7 +282,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
index 12d5e333e5f2..4e1a429b3dff 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
@@ -72,7 +72,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 40db06e28b66..d584c99e60c6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -12,6 +12,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	interrupt-parent = <&gic>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
index d6ca684e0e61..6dc2e6aafe97 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
@@ -79,7 +79,8 @@ wifi_32k: wifi-32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
index 65ec7dea828c..77d1d729e857 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
@@ -109,7 +109,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index e803a466fe4e..41406fba88bc 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -83,7 +83,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index 45cb83625951..59e436c41da9 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -77,7 +77,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	hdmi-connector {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
index dee51cf95223..2d0c4badfe3b 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
@@ -82,7 +82,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts
index a1119cfb0280..90cfdb96cba9 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts
@@ -93,7 +93,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts
index c8d74e61dec1..69b9f2a31ae3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts
@@ -72,7 +72,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
index 440bc23c7342..95b654846db8 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
@@ -47,7 +47,7 @@ pwmleds {
 
 		power {
 			label = "vim:red:power";
-			pwms = <&pwm_AO_ab 1 7812500 0>;
+			pwms = <&pwm_AO_ab 1 7812500 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 		};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts
index 62dd87821ce5..28c814460d72 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts
@@ -72,7 +72,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi
index 6ac678f88bd8..b7923f2278b7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi
@@ -76,7 +76,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
index d5dc12878dfe..674e5e765a32 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
@@ -87,7 +87,7 @@ pwmleds {
 
 		power {
 			label = "vim:red:power";
-			pwms = <&pwm_AO_ab 1 7812500 0>;
+			pwms = <&pwm_AO_ab 1 7812500 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 		};
@@ -187,7 +187,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts
index 420a88e9a195..87475bd61cdf 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts
@@ -81,7 +81,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
index 90815fa25ec6..a5babf55a937 100644
--- a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi
@@ -7,6 +7,7 @@
 
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/meson-g12a-gpio.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "Khadas VIM3";
@@ -167,7 +168,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
index dbbf29a0dbf6..23e3bc0cc765 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
@@ -25,7 +25,7 @@ vddcpu: regulator-vddcpu {
 
 		vin-supply = <&vsys_3v3>;
 
-		pwms = <&pwm_AO_cd 1 1250 0>;
+		pwms = <&pwm_AO_cd 1 1250 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
index cb1b48f5b8b1..e55631b815c3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
@@ -116,7 +116,7 @@ pwmleds {
 
 		power {
 			label = "sei610:red:power";
-			pwms = <&pwm_AO_ab 0 30518 0>;
+			pwms = <&pwm_AO_ab 0 30518 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 			active-low;
@@ -187,7 +187,7 @@ vddcpu: regulator-vddcpu {
 
 		vin-supply = <&dc_in>;
 
-		pwms = <&pwm_AO_cd 1 1500 0>;
+		pwms = <&pwm_AO_cd 1 1500 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
@@ -336,7 +336,8 @@ wifi32k: wifi32k {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
+		/* PWM_E at 32.768KHz */
+		pwms = <&pwm_ef 0 30518 PWM_POLARITY_NORMAL>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index 250fc01de78d..f70703df97e3 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -14,6 +14,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/pwm/pwm.h>
 #include <dt-bindings/sound/samsung-i2s.h>
 
 / {
@@ -913,7 +914,7 @@ charger_reg: CHARGER {
 		haptic: max77843-haptic {
 			compatible = "maxim,max77843-haptic";
 			haptic-supply = <&ldo38_reg>;
-			pwms = <&pwm 0 33670 0>;
+			pwms = <&pwm 0 33670 PWM_POLARITY_NORMAL>;
 			pwm-names = "haptic";
 		};
 	};
diff --git a/arch/arm64/boot/dts/rockchip/px30-evb.dts b/arch/arm64/boot/dts/rockchip/px30-evb.dts
index 0a680257d9c2..f9359ece5b28 100644
--- a/arch/arm64/boot/dts/rockchip/px30-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/px30-evb.dts
@@ -57,7 +57,7 @@ vol-up-key {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 25000 0>;
+		pwms = <&pwm1 0 25000 PWM_POLARITY_NORMAL>;
 		power-supply = <&vcc3v3_lcd>;
 	};
 
diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index 60de8e9c421b..97bf03d536a3 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/rockchip.h>
 #include <dt-bindings/power/px30-power.h>
+#include <dt-bindings/pwm/pwm.h>
 #include <dt-bindings/soc/rockchip,boot-mode.h>
 #include <dt-bindings/thermal/thermal.h>
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts b/arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts
index aa256350b18f..502d2d9fa8dc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts
@@ -22,7 +22,7 @@ ir_rx {
 
 	ir_tx {
 		compatible = "pwm-ir-tx";
-		pwms = <&pwm5 0 25000 0>;
+		pwms = <&pwm5 0 25000 PWM_POLARITY_NORMAL>;
 	};
 
 	leds {
diff --git a/arch/arm64/boot/dts/rockchip/rk3308.dtsi b/arch/arm64/boot/dts/rockchip/rk3308.dtsi
index ac43bc3f7031..f3d0295a8046 100644
--- a/arch/arm64/boot/dts/rockchip/rk3308.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3308.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/pwm/pwm.h>
 #include <dt-bindings/soc/rockchip,boot-mode.h>
 #include <dt-bindings/thermal/thermal.h>
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
index 694b0d08d644..6fd15469b6f0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
@@ -47,7 +47,7 @@ backlight: backlight {
 			240 241 242 243 244 245 246 247
 			248 249 250 251 252 253 254 255>;
 		default-brightness-level = <200>;
-		pwms = <&pwm0 0 25000 0>;
+		pwms = <&pwm0 0 25000 PWM_POLARITY_NORMAL>;
 	};
 
 	edp_panel: edp-panel {
@@ -72,7 +72,7 @@ clkin_gmac: external-gmac-clock {
 
 	vdd_center: vdd-center {
 		compatible = "pwm-regulator";
-		pwms = <&pwm3 0 25000 0>;
+		pwms = <&pwm3 0 25000 PWM_POLARITY_NORMAL>;
 		regulator-name = "vdd_center";
 		regulator-min-microvolt = <800000>;
 		regulator-max-microvolt = <1400000>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
index d63faf38cc81..c233f8a2f444 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
@@ -20,7 +20,7 @@ chosen {
 	backlight: backlight {
 		compatible = "pwm-backlight";
 		enable-gpios = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>;
-		pwms = <&pwm0 0 25000 0>;
+		pwms = <&pwm0 0 25000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <
 			  0   1   2   3   4   5   6   7
 			  8   9  10  11  12  13  14  15
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
index 1384dabbdf40..ae44d55e38d1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
@@ -50,7 +50,7 @@ ppvar_centerlogic_pwm: ppvar-centerlogic-pwm {
 		compatible = "pwm-regulator";
 		regulator-name = "ppvar_centerlogic_pwm";
 
-		pwms = <&pwm3 0 3337 0>;
+		pwms = <&pwm3 0 3337 PWM_POLARITY_NORMAL>;
 		pwm-supply = <&ppvar_sys>;
 		pwm-dutycycle-range = <100 0>;
 		pwm-dutycycle-unit = <100>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
index 4373ed732af7..e7c2d31bece2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
@@ -164,7 +164,7 @@ backlight: backlight {
 		enable-gpios = <&gpio4 21 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&bl_en>;
-		pwms = <&pwm1 0 1000000 0>;
+		pwms = <&pwm1 0 1000000 PWM_POLARITY_NORMAL>;
 		pwm-delay-us = <10000>;
 	};
 
@@ -217,7 +217,7 @@ &ppvar_bigcpu {
 
 &ppvar_bigcpu_pwm {
 	/* On scarlet ppvar big cpu use pwm3 */
-	pwms = <&pwm3 0 3337 0>;
+	pwms = <&pwm3 0 3337 PWM_POLARITY_NORMAL>;
 	regulator-min-microvolt = <800074>;
 	regulator-max-microvolt = <1299226>;
 };
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 2f3997740068..f5ebafb64820 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -101,7 +101,7 @@ ppvar_bigcpu_pwm: ppvar-bigcpu-pwm {
 		compatible = "pwm-regulator";
 		regulator-name = "ppvar_bigcpu_pwm";
 
-		pwms = <&pwm1 0 3337 0>;
+		pwms = <&pwm1 0 3337 PWM_POLARITY_NORMAL>;
 		pwm-supply = <&ppvar_sys>;
 		pwm-dutycycle-range = <100 0>;
 		pwm-dutycycle-unit = <100>;
@@ -130,7 +130,7 @@ ppvar_litcpu_pwm: ppvar-litcpu-pwm {
 		compatible = "pwm-regulator";
 		regulator-name = "ppvar_litcpu_pwm";
 
-		pwms = <&pwm2 0 3337 0>;
+		pwms = <&pwm2 0 3337 PWM_POLARITY_NORMAL>;
 		pwm-supply = <&ppvar_sys>;
 		pwm-dutycycle-range = <100 0>;
 		pwm-dutycycle-unit = <100>;
@@ -159,7 +159,7 @@ ppvar_gpu_pwm: ppvar-gpu-pwm {
 		compatible = "pwm-regulator";
 		regulator-name = "ppvar_gpu_pwm";
 
-		pwms = <&pwm0 0 3337 0>;
+		pwms = <&pwm0 0 3337 PWM_POLARITY_NORMAL>;
 		pwm-supply = <&ppvar_sys>;
 		pwm-dutycycle-range = <100 0>;
 		pwm-dutycycle-unit = <100>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
index e87a04477440..979fd0832efd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
@@ -161,7 +161,7 @@ fan: pwm-fan {
 		cooling-levels = <0 150 200 255>;
 		#cooling-cells = <2>;
 		fan-supply = <&vsys_5v0>;
-		pwms = <&pwm0 0 40000 0>;
+		pwms = <&pwm0 0 40000 PWM_POLARITY_NORMAL>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts
index e0d75617bb7e..c63a115930aa 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts
@@ -62,7 +62,7 @@ fan: pwm-fan {
 		cooling-levels = <0 12 18 255>;
 		#cooling-cells = <2>;
 		fan-supply = <&vcc12v0_sys>;
-		pwms = <&pwm1 0 50000 0>;
+		pwms = <&pwm1 0 50000 PWM_POLARITY_NORMAL>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
index 9f225e9c3d54..3656a8d71b4a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
@@ -19,7 +19,7 @@ chosen {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm0 0 25000 0>;
+		pwms = <&pwm0 0 25000 PWM_POLARITY_NORMAL>;
 	};
 
 	clkin_gmac: external-gmac-clock {
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 9bca25801260..2712270e93ca 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -58,7 +58,7 @@ fan: pwm-fan {
 		compatible = "pwm-fan";
 		#cooling-cells = <2>;
 		fan-supply = <&vcc12v_dcin>;
-		pwms = <&pwm1 0 50000 0>;
+		pwms = <&pwm1 0 50000 PWM_POLARITY_NORMAL>;
 	};
 
 	sdio_pwrseq: sdio-pwrseq {
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-sapphire-excavator.dts b/arch/arm64/boot/dts/rockchip/rk3399-sapphire-excavator.dts
index b4d8f60b7e44..0b658374fb9a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-sapphire-excavator.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-sapphire-excavator.dts
@@ -79,7 +79,7 @@ backlight: backlight {
 			248 249 250 251 252 253 254 255>;
 		default-brightness-level = <200>;
 		enable-gpios = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>;
-		pwms = <&pwm0 0 25000 0>;
+		pwms = <&pwm0 0 25000 PWM_POLARITY_NORMAL>;
 		status = "okay";
 	};
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 28c7ee540439..8e7dfa8449c6 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/rockchip.h>
 #include <dt-bindings/power/rk3399-power.h>
+#include <dt-bindings/pwm/pwm.h>
 #include <dt-bindings/thermal/thermal.h>
 
 / {
-- 
2.24.1


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

* [RFC PATCH 7/7] arm: dts: pwm: replace polarity constant with macro
       [not found] <20200317123231.2843297-1-oleksandr.suvorov@toradex.com>
                   ` (5 preceding siblings ...)
  2020-03-17 12:32 ` [RFC PATCH 6/7] arm64: dts: pwm: replace polarity constant with macro Oleksandr Suvorov
@ 2020-03-17 12:32 ` Oleksandr Suvorov
  2020-03-20 10:02   ` Krzysztof Kozlowski
  6 siblings, 1 reply; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-17 12:32 UTC (permalink / raw)
  To: devicetree, linux-pwm
  Cc: Paul Barker, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker,
	Oleksandr Suvorov, Alexandre Belloni, Bartosz Golaszewski,
	Benoît Cousson, Chen-Yu Tsai, David Lechner, Fabio Estevam,
	Geert Uytterhoeven, Heiko Stuebner, Jisheng Zhang, Kevin Hilman,
	Krzysztof Kozlowski, Kukjin Kim, Ludovic Desroches, Magnus Damm,
	Maxime Ripard, NXP Linux Team, Nicolas Ferre,
	Pengutronix Kernel Team, Peter Rosin, Rob Herring, Sascha Hauer,
	Sebastian Hesselbarth, Sekhar Nori, Shawn Guo, Stefan Agner,
	Tony Lindgren, linux-amlogic, linux-arm-kernel, linux-kernel,
	linux-omap, linux-renesas-soc, linux-rockchip, linux-samsung-soc

There is the PWM_POLARITY_NORMAL defined and describled in
<dt-bindings/pwm/pwm.h> and used by pwm drivers.

This patch converts all '0' constant in pwms parameters into
PWM_POLARITY_NORMAL.

Replace with sed regexp:
's/(pwms = <&[a-zA-Z_0-9]+ [0-9]+ [0-9]+) 0>/\1 PWM_POLARITY_NORMAL>/'

Then:
- included pwm.h in some dts/dtsi to fix building errors about undefined
  symbols.
- fixed the patman warnings about the code format;

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 arch/arm/boot/dts/am335x-cm-t335.dts               | 2 +-
 arch/arm/boot/dts/am335x-evm.dts                   | 2 +-
 arch/arm/boot/dts/am3517-evm.dts                   | 2 +-
 arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi      | 2 +-
 arch/arm/boot/dts/at91-kizbox2-common.dtsi         | 6 +++---
 arch/arm/boot/dts/at91-kizbox3_common.dtsi         | 8 ++++----
 arch/arm/boot/dts/at91-kizboxmini-common.dtsi      | 6 +++---
 arch/arm/boot/dts/at91-nattis-2-natte-2.dts        | 2 +-
 arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts        | 2 +-
 arch/arm/boot/dts/at91sam9n12ek.dts                | 2 +-
 arch/arm/boot/dts/at91sam9x5dm.dtsi                | 2 +-
 arch/arm/boot/dts/berlin2cd-google-chromecast.dts  | 4 ++--
 arch/arm/boot/dts/da850-evm.dts                    | 2 +-
 arch/arm/boot/dts/da850-lego-ev3.dts               | 4 ++--
 arch/arm/boot/dts/exynos4412-midas.dtsi            | 2 +-
 arch/arm/boot/dts/exynos4412-odroidu3.dts          | 2 +-
 arch/arm/boot/dts/exynos5250-snow-common.dtsi      | 2 +-
 arch/arm/boot/dts/exynos5410-odroidxu.dts          | 2 +-
 arch/arm/boot/dts/exynos5420-peach-pit.dts         | 2 +-
 arch/arm/boot/dts/exynos5422-odroidhc1.dts         | 2 +-
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 2 +-
 arch/arm/boot/dts/exynos5422-odroidxu4.dts         | 2 +-
 arch/arm/boot/dts/exynos54xx-odroidxu-leds.dtsi    | 4 ++--
 arch/arm/boot/dts/exynos5800-peach-pi.dts          | 2 +-
 arch/arm/boot/dts/imx53-tx53-x13x.dts              | 5 +++--
 arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts          | 2 +-
 arch/arm/boot/dts/imx6q-display5.dtsi              | 2 +-
 arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts       | 2 +-
 arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts       | 2 +-
 arch/arm/boot/dts/imx6qdl-tx6-lvds.dtsi            | 4 ++--
 arch/arm/boot/dts/imx7-colibri.dtsi                | 4 +++-
 arch/arm/boot/dts/imx7d-nitrogen7.dts              | 3 ++-
 arch/arm/boot/dts/imx7d-pico.dtsi                  | 3 ++-
 arch/arm/boot/dts/imx7d-sdb.dts                    | 3 ++-
 arch/arm/boot/dts/imx7ulp-evk.dts                  | 3 ++-
 arch/arm/boot/dts/iwg20d-q7-common.dtsi            | 2 +-
 arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi   | 2 +-
 arch/arm/boot/dts/meson8b-ec100.dts                | 4 ++--
 arch/arm/boot/dts/meson8b-mxq.dts                  | 4 ++--
 arch/arm/boot/dts/meson8b-odroidc1.dts             | 4 ++--
 arch/arm/boot/dts/motorola-mapphone-common.dtsi    | 3 ++-
 arch/arm/boot/dts/omap3-gta04.dtsi                 | 2 +-
 arch/arm/boot/dts/omap3-n900.dts                   | 2 +-
 arch/arm/boot/dts/rk3288-veyron-edp.dtsi           | 2 +-
 arch/arm/boot/dts/rk3288-veyron.dtsi               | 2 +-
 arch/arm/boot/dts/rv1108-evb.dts                   | 2 +-
 arch/arm/boot/dts/s3c6410-mini6410.dts             | 2 +-
 arch/arm/boot/dts/s5pv210-aries.dtsi               | 2 +-
 arch/arm/boot/dts/s5pv210-smdkv210.dts             | 2 +-
 arch/arm/boot/dts/sun5i-gr8-evb.dts                | 2 +-
 arch/arm/boot/dts/vf-colibri.dtsi                  | 4 +++-
 51 files changed, 76 insertions(+), 66 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-cm-t335.dts b/arch/arm/boot/dts/am335x-cm-t335.dts
index c6fe9db660e2..cb1ec60fb455 100644
--- a/arch/arm/boot/dts/am335x-cm-t335.dts
+++ b/arch/arm/boot/dts/am335x-cm-t335.dts
@@ -48,7 +48,7 @@ vwlan_fixed: fixedregulator2 {
 
 	backlight {
 		compatible = "pwm-backlight";
-		pwms = <&ecap0 0 50000 0>;
+		pwms = <&ecap0 0 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 51 53 56 62 75 101 152 255>;
 		default-brightness-level = <8>;
 	};
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 68252dab32c3..dd8eba3d85c7 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -115,7 +115,7 @@ switch10 {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&ecap0 0 50000 0>;
+		pwms = <&ecap0 0 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 51 53 56 62 75 101 152 255>;
 		default-brightness-level = <8>;
 	};
diff --git a/arch/arm/boot/dts/am3517-evm.dts b/arch/arm/boot/dts/am3517-evm.dts
index a1fd3e63e86e..206e00db67f7 100644
--- a/arch/arm/boot/dts/am3517-evm.dts
+++ b/arch/arm/boot/dts/am3517-evm.dts
@@ -144,7 +144,7 @@ bl: backlight {
 		pinctrl-names = "default";
 		power-supply = <&vdd_io_reg>;
 		pinctrl-0 = <&backlight_pins>;
-		pwms = <&pwm11 0 5000000 0>;
+		pwms = <&pwm11 0 5000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
 		default-brightness-level = <7>;
 		enable-gpios = <&gpio6 22 GPIO_ACTIVE_HIGH>; /* gpio_182 */
diff --git a/arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi b/arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi
index bea920b192b6..839b0d1e8a17 100644
--- a/arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi
+++ b/arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi
@@ -10,7 +10,7 @@
 / {
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&hlcdc_pwm 0 50000 0>;
+		pwms = <&hlcdc_pwm 0 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
 		status = "okay";
diff --git a/arch/arm/boot/dts/at91-kizbox2-common.dtsi b/arch/arm/boot/dts/at91-kizbox2-common.dtsi
index af38253a6e7a..cea5226c5118 100644
--- a/arch/arm/boot/dts/at91-kizbox2-common.dtsi
+++ b/arch/arm/boot/dts/at91-kizbox2-common.dtsi
@@ -63,21 +63,21 @@ pwm_leds {
 
 		blue {
 			label = "pwm:blue:user";
-			pwms = <&pwm0 2 10000000 0>;
+			pwms = <&pwm0 2 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "none";
 		};
 
 		green {
 			label = "pwm:green:user";
-			pwms = <&pwm0 1 10000000 0>;
+			pwms = <&pwm0 1 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 		};
 
 		red {
 			label = "pwm:red:user";
-			pwms = <&pwm0 0 10000000 0>;
+			pwms = <&pwm0 0 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 		};
diff --git a/arch/arm/boot/dts/at91-kizbox3_common.dtsi b/arch/arm/boot/dts/at91-kizbox3_common.dtsi
index 299e74d23184..f1a8448ba6e1 100644
--- a/arch/arm/boot/dts/at91-kizbox3_common.dtsi
+++ b/arch/arm/boot/dts/at91-kizbox3_common.dtsi
@@ -73,7 +73,7 @@ &pinctrl_pwm0_pwm_h2
 
 		red {
 			label = "pwm:red:user";
-			pwms = <&pwm0 0 10000000 0>;
+			pwms = <&pwm0 0 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 			status = "disabled";
@@ -81,7 +81,7 @@ red {
 
 		green {
 			label = "pwm:green:user";
-			pwms = <&pwm0 1 10000000 0>;
+			pwms = <&pwm0 1 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 			status = "disabled";
@@ -89,14 +89,14 @@ green {
 
 		blue {
 			label = "pwm:blue:user";
-			pwms = <&pwm0 2 10000000 0>;
+			pwms = <&pwm0 2 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			status = "disabled";
 		};
 
 		white {
 			label = "pwm:white:user";
-			pwms = <&pwm0 3 10000000 0>;
+			pwms = <&pwm0 3 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			status = "disabled";
 		};
diff --git a/arch/arm/boot/dts/at91-kizboxmini-common.dtsi b/arch/arm/boot/dts/at91-kizboxmini-common.dtsi
index fddf267b2d17..60dad4ff6f27 100644
--- a/arch/arm/boot/dts/at91-kizboxmini-common.dtsi
+++ b/arch/arm/boot/dts/at91-kizboxmini-common.dtsi
@@ -59,7 +59,7 @@ leds: pwm_leds {
 
 		led_blue: pwm_blue {
 			label = "pwm:blue:user";
-			pwms = <&pwm0 2 10000000 0>;
+			pwms = <&pwm0 2 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "none";
 			status = "disabled";
@@ -67,14 +67,14 @@ led_blue: pwm_blue {
 
 		led_green: pwm_green {
 			label = "pwm:green:user";
-			pwms = <&pwm0 0 10000000 0>;
+			pwms = <&pwm0 0 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 		};
 
 		led_red: pwm_red {
 			label = "pwm:red:user";
-			pwms = <&pwm0 1 10000000 0>;
+			pwms = <&pwm0 1 10000000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 		};
diff --git a/arch/arm/boot/dts/at91-nattis-2-natte-2.dts b/arch/arm/boot/dts/at91-nattis-2-natte-2.dts
index 4f123477e631..d9813299fd47 100644
--- a/arch/arm/boot/dts/at91-nattis-2-natte-2.dts
+++ b/arch/arm/boot/dts/at91-nattis-2-natte-2.dts
@@ -42,7 +42,7 @@ bl_reg: backlight-regulator {
 
 	panel_bl: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&hlcdc_pwm 0 100000 0>;
+		pwms = <&hlcdc_pwm 0 100000 PWM_POLARITY_NORMAL>;
 
 		brightness-levels = < 0  1  2  3  4  5  6  7  8  9
 				     10 11 12 13 14 15 16 17 18 19
diff --git a/arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts b/arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts
index 4d7cee569ff2..0ab4533ca789 100644
--- a/arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts
+++ b/arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts
@@ -105,7 +105,7 @@ pinctrl_usba_vbus: usba_vbus {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&hlcdc_pwm 0 50000 0>;
+		pwms = <&hlcdc_pwm 0 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
 		status = "okay";
diff --git a/arch/arm/boot/dts/at91sam9n12ek.dts b/arch/arm/boot/dts/at91sam9n12ek.dts
index d36e162a8817..958c87abd865 100644
--- a/arch/arm/boot/dts/at91sam9n12ek.dts
+++ b/arch/arm/boot/dts/at91sam9n12ek.dts
@@ -189,7 +189,7 @@ usb0: ohci@500000 {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&hlcdc_pwm 0 50000 0>;
+		pwms = <&hlcdc_pwm 0 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
 		power-supply = <&bl_reg>;
diff --git a/arch/arm/boot/dts/at91sam9x5dm.dtsi b/arch/arm/boot/dts/at91sam9x5dm.dtsi
index a9278038af3b..a04ca0b08494 100644
--- a/arch/arm/boot/dts/at91sam9x5dm.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5dm.dtsi
@@ -11,7 +11,7 @@
 / {
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&hlcdc_pwm 0 50000 0>;
+		pwms = <&hlcdc_pwm 0 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
 		power-supply = <&bl_reg>;
diff --git a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
index 56fa951bc86f..12c19f45efad 100644
--- a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
+++ b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
@@ -41,14 +41,14 @@ leds {
 
 		white {
 			label = "white";
-			pwms = <&pwm 0 600000 0>;
+			pwms = <&pwm 0 600000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 			linux,default-trigger = "default-on";
 		};
 
 		red {
 			label = "red";
-			pwms = <&pwm 1 600000 0>;
+			pwms = <&pwm 1 600000 PWM_POLARITY_NORMAL>;
 			max-brightness = <255>;
 		};
 	};
diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index f2e7609e5346..d62503cb9a88 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -34,7 +34,7 @@ backlight: backlight-pwm {
 		 * schematic needs to be 1015171 (15 March 2010), Rev A
 		 * or newer.
 		 */
-		pwms = <&ecap2 0 50000 0>;
+		pwms = <&ecap2 0 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 10 20 30 40 50 60 70 80 90 99>;
 		default-brightness-level = <7>;
 	};
diff --git a/arch/arm/boot/dts/da850-lego-ev3.dts b/arch/arm/boot/dts/da850-lego-ev3.dts
index afd04a423856..4dc42901c6f4 100644
--- a/arch/arm/boot/dts/da850-lego-ev3.dts
+++ b/arch/arm/boot/dts/da850-lego-ev3.dts
@@ -118,7 +118,7 @@ sound {
 		compatible = "pwm-beeper";
 		pinctrl-names = "default";
 		pinctrl-0 = <&ehrpwm0b_pins>;
-		pwms = <&ehrpwm0 1 1000000 0>;
+		pwms = <&ehrpwm0 1 1000000 PWM_POLARITY_NORMAL>;
 		amp-supply = <&amp>;
 	};
 
@@ -185,7 +185,7 @@ bt_slow_clk: bt-clock {
 		compatible = "pwm-clock";
 		#clock-cells = <0>;
 		clock-frequency = <32768>;
-		pwms = <&ecap2 0 30518 0>;
+		pwms = <&ecap2 0 30518 PWM_POLARITY_NORMAL>;
 	};
 
 	/* ARM local RAM */
diff --git a/arch/arm/boot/dts/exynos4412-midas.dtsi b/arch/arm/boot/dts/exynos4412-midas.dtsi
index 3023bc3b68ce..ee64081abce9 100644
--- a/arch/arm/boot/dts/exynos4412-midas.dtsi
+++ b/arch/arm/boot/dts/exynos4412-midas.dtsi
@@ -171,7 +171,7 @@ charger_reg: CHARGER {
 			max77693_haptic {
 				compatible = "maxim,max77693-haptic";
 				haptic-supply = <&ldo26_reg>;
-				pwms = <&pwm 0 38022 0>;
+				pwms = <&pwm 0 38022 PWM_POLARITY_NORMAL>;
 			};
 
 			charger {
diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index 8ff243ba4542..d97b290565e2 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -33,7 +33,7 @@ led1 {
 
 	fan0: pwm-fan {
 		compatible = "pwm-fan";
-		pwms = <&pwm 0 10000 0>;
+		pwms = <&pwm 0 10000 PWM_POLARITY_NORMAL>;
 		#cooling-cells = <2>;
 		cooling-levels = <0 102 170 230>;
 	};
diff --git a/arch/arm/boot/dts/exynos5250-snow-common.dtsi b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
index c952a615148e..8b94b9d88f1e 100644
--- a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
@@ -196,7 +196,7 @@ xxti {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm 0 1000000 0>;
+		pwms = <&pwm 0 1000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 100 500 1000 1500 2000 2500 2800>;
 		default-brightness-level = <7>;
 		enable-gpios = <&gpx3 0 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm/boot/dts/exynos5410-odroidxu.dts b/arch/arm/boot/dts/exynos5410-odroidxu.dts
index 4f9297ae0763..5edd2b94135a 100644
--- a/arch/arm/boot/dts/exynos5410-odroidxu.dts
+++ b/arch/arm/boot/dts/exynos5410-odroidxu.dts
@@ -37,7 +37,7 @@ emmc_pwrseq: pwrseq {
 
 	fan0: pwm-fan {
 		compatible = "pwm-fan";
-		pwms = <&pwm 0 20972 0>;
+		pwms = <&pwm 0 20972 PWM_POLARITY_NORMAL>;
 		#cooling-cells = <2>;
 		cooling-levels = <0 130 170 230>;
 	};
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 2bcbdf8a39bf..1a89f3783b42 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -34,7 +34,7 @@ aliases {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm 0 1000000 0>;
+		pwms = <&pwm 0 1000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 100 500 1000 1500 2000 2500 2800>;
 		default-brightness-level = <7>;
 		power-supply = <&tps65090_fet1>;
diff --git a/arch/arm/boot/dts/exynos5422-odroidhc1.dts b/arch/arm/boot/dts/exynos5422-odroidhc1.dts
index 812659260278..c7d81c4d3465 100644
--- a/arch/arm/boot/dts/exynos5422-odroidhc1.dts
+++ b/arch/arm/boot/dts/exynos5422-odroidhc1.dts
@@ -20,7 +20,7 @@ pwmleds {
 
 		blueled {
 			label = "blue:heartbeat";
-			pwms = <&pwm 2 2000000 0>;
+			pwms = <&pwm 2 2000000 PWM_POLARITY_NORMAL>;
 			pwm-names = "pwm2";
 			max_brightness = <255>;
 			linux,default-trigger = "heartbeat";
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
index 5da2d81e3be2..ecebb6867329 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
@@ -43,7 +43,7 @@ emmc_pwrseq: pwrseq {
 
 	fan0: pwm-fan {
 		compatible = "pwm-fan";
-		pwms = <&pwm 0 20972 0>;
+		pwms = <&pwm 0 20972 PWM_POLARITY_NORMAL>;
 		#cooling-cells = <2>;
 		cooling-levels = <0 130 170 230>;
 	};
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu4.dts b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
index 892d389d6d09..ef22f2b37dea 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu4.dts
+++ b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
@@ -22,7 +22,7 @@ pwmleds {
 
 		blueled {
 			label = "blue:heartbeat";
-			pwms = <&pwm 2 2000000 0>;
+			pwms = <&pwm 2 2000000 PWM_POLARITY_NORMAL>;
 			pwm-names = "pwm2";
 			max_brightness = <255>;
 			linux,default-trigger = "heartbeat";
diff --git a/arch/arm/boot/dts/exynos54xx-odroidxu-leds.dtsi b/arch/arm/boot/dts/exynos54xx-odroidxu-leds.dtsi
index 56acd832f0b3..90d80aa8b160 100644
--- a/arch/arm/boot/dts/exynos54xx-odroidxu-leds.dtsi
+++ b/arch/arm/boot/dts/exynos54xx-odroidxu-leds.dtsi
@@ -16,7 +16,7 @@ pwmleds {
 
 		greenled {
 			label = "green:mmc0";
-			pwms = <&pwm 1 2000000 0>;
+			pwms = <&pwm 1 2000000 PWM_POLARITY_NORMAL>;
 			pwm-names = "pwm1";
 			/*
 			 * Green LED is much brighter than the others
@@ -28,7 +28,7 @@ greenled {
 
 		blueled {
 			label = "blue:heartbeat";
-			pwms = <&pwm 2 2000000 0>;
+			pwms = <&pwm 2 2000000 PWM_POLARITY_NORMAL>;
 			pwm-names = "pwm2";
 			max_brightness = <255>;
 			linux,default-trigger = "heartbeat";
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 60ab0effe474..ef289c040a78 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -32,7 +32,7 @@ aliases {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm 0 1000000 0>;
+		pwms = <&pwm 0 1000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 100 500 1000 1500 2000 2500 2800>;
 		default-brightness-level = <7>;
 		enable-gpios = <&gpx2 2 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm/boot/dts/imx53-tx53-x13x.dts b/arch/arm/boot/dts/imx53-tx53-x13x.dts
index 6cdf2082c742..57d5d7fe3881 100644
--- a/arch/arm/boot/dts/imx53-tx53-x13x.dts
+++ b/arch/arm/boot/dts/imx53-tx53-x13x.dts
@@ -48,6 +48,7 @@
 /dts-v1/;
 #include "imx53-tx53.dtsi"
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "Ka-Ro electronics TX53 module (LVDS)";
@@ -61,7 +62,7 @@ aliases {
 
 	backlight0: backlight0 {
 		compatible = "pwm-backlight";
-		pwms = <&pwm2 0 500000 0>;
+		pwms = <&pwm2 0 500000 PWM_POLARITY_NORMAL>;
 		power-supply = <&reg_3v3>;
 		brightness-levels = <
 			  0  1  2  3  4  5  6  7  8  9
@@ -81,7 +82,7 @@ backlight0: backlight0 {
 
 	backlight1: backlight1 {
 		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 500000 0>;
+		pwms = <&pwm1 0 500000 PWM_POLARITY_NORMAL>;
 		power-supply = <&reg_3v3>;
 		brightness-levels = <
 			  0  1  2  3  4  5  6  7  8  9
diff --git a/arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts b/arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts
index 51a9bb9d6bc2..12c43823b1ed 100644
--- a/arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts
+++ b/arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts
@@ -50,7 +50,7 @@ / {
 };
 
 &backlight {
-	pwms = <&pwm2 0 500000 0>;
+	pwms = <&pwm2 0 500000 PWM_POLARITY_NORMAL>;
 	/delete-property/ turn-on-delay-ms;
 };
 
diff --git a/arch/arm/boot/dts/imx6q-display5.dtsi b/arch/arm/boot/dts/imx6q-display5.dtsi
index 83524bb99eb3..b37382b0a4f1 100644
--- a/arch/arm/boot/dts/imx6q-display5.dtsi
+++ b/arch/arm/boot/dts/imx6q-display5.dtsi
@@ -56,7 +56,7 @@ backlight_lvds: backlight {
 		compatible = "pwm-backlight";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_backlight>;
-		pwms = <&pwm2 0 5000000 0>;
+		pwms = <&pwm2 0 5000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <  0   1   2   3   4   5   6   7   8   9
 				      10  11  12  13  14  15  16  17  18  19
 				      20  21  22  23  24  25  26  27  28  29
diff --git a/arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts b/arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts
index ac3050a835e5..507133858719 100644
--- a/arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts
+++ b/arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts
@@ -50,7 +50,7 @@ / {
 };
 
 &backlight {
-	pwms = <&pwm2 0 500000 0>;
+	pwms = <&pwm2 0 500000 PWM_POLARITY_NORMAL>;
 	/delete-property/ turn-on-delay-ms;
 };
 
diff --git a/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts b/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts
index a773f252816c..6656da97c7d2 100644
--- a/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts
+++ b/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts
@@ -50,7 +50,7 @@ / {
 };
 
 &backlight {
-	pwms = <&pwm2 0 500000 0>;
+	pwms = <&pwm2 0 500000 PWM_POLARITY_NORMAL>;
 	/delete-property/ turn-on-delay-ms;
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-tx6-lvds.dtsi b/arch/arm/boot/dts/imx6qdl-tx6-lvds.dtsi
index 2ca2eb37e14f..04387d0fe8f3 100644
--- a/arch/arm/boot/dts/imx6qdl-tx6-lvds.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-tx6-lvds.dtsi
@@ -48,7 +48,7 @@ aliases {
 
 	backlight0: backlight0 {
 		compatible = "pwm-backlight";
-		pwms = <&pwm2 0 500000 0>;
+		pwms = <&pwm2 0 500000 PWM_POLARITY_NORMAL>;
 		power-supply = <&reg_lcd0_pwr>;
 		brightness-levels = < 0  1  2  3  4  5  6  7  8  9
 				     10 11 12 13 14 15 16 17 18 19
@@ -66,7 +66,7 @@ backlight0: backlight0 {
 
 	backlight1: backlight1 {
 		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 500000 0>;
+		pwms = <&pwm1 0 500000 PWM_POLARITY_NORMAL>;
 		power-supply = <&reg_lcd1_pwr>;
 		brightness-levels = < 0  1  2  3  4  5  6  7  8  9
 				     10 11 12 13 14 15 16 17 18 19
diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
index dbb98b03f9ad..4b0d8f35e119 100644
--- a/arch/arm/boot/dts/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri.dtsi
@@ -4,12 +4,14 @@
  *
  */
 
+#include <dt-bindings/pwm/pwm.h>
+
 / {
 	bl: backlight {
 		compatible = "pwm-backlight";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_gpio_bl_on>;
-		pwms = <&pwm1 0 5000000 0>;
+		pwms = <&pwm1 0 5000000 PWM_POLARITY_NORMAL>;
 		enable-gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
 	};
 
diff --git a/arch/arm/boot/dts/imx7d-nitrogen7.dts b/arch/arm/boot/dts/imx7d-nitrogen7.dts
index 6b4acea1ef79..b3e32b3a7a35 100644
--- a/arch/arm/boot/dts/imx7d-nitrogen7.dts
+++ b/arch/arm/boot/dts/imx7d-nitrogen7.dts
@@ -6,6 +6,7 @@
 /dts-v1/;
 
 #include "imx7d.dtsi"
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "Boundary Devices i.MX7 Nitrogen7 Board";
@@ -26,7 +27,7 @@ backlight-j9 {
 
 	backlight_lcd: backlight-j20 {
 		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 5000000 0>;
+		pwms = <&pwm1 0 5000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
 		status = "okay";
diff --git a/arch/arm/boot/dts/imx7d-pico.dtsi b/arch/arm/boot/dts/imx7d-pico.dtsi
index e57da0d32b98..08f0f46f1e60 100644
--- a/arch/arm/boot/dts/imx7d-pico.dtsi
+++ b/arch/arm/boot/dts/imx7d-pico.dtsi
@@ -5,11 +5,12 @@
 /dts-v1/;
 
 #include "imx7d.dtsi"
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm4 0 50000 0>;
+		pwms = <&pwm4 0 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 36 72 108 144 180 216 255>;
 		default-brightness-level = <6>;
 	};
diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
index 869efbc4af42..e924ee8eddcb 100644
--- a/arch/arm/boot/dts/imx7d-sdb.dts
+++ b/arch/arm/boot/dts/imx7d-sdb.dts
@@ -5,6 +5,7 @@
 /dts-v1/;
 
 #include "imx7d.dtsi"
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "Freescale i.MX7 SabreSD Board";
@@ -129,7 +130,7 @@ reg_fec2_3v3: regulator-fec2-3v3 {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 5000000 0>;
+		pwms = <&pwm1 0 5000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
 		status = "okay";
diff --git a/arch/arm/boot/dts/imx7ulp-evk.dts b/arch/arm/boot/dts/imx7ulp-evk.dts
index eff51e113db4..818bc0fac3d4 100644
--- a/arch/arm/boot/dts/imx7ulp-evk.dts
+++ b/arch/arm/boot/dts/imx7ulp-evk.dts
@@ -8,6 +8,7 @@
 /dts-v1/;
 
 #include "imx7ulp.dtsi"
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "NXP i.MX7ULP EVK";
@@ -24,7 +25,7 @@ memory@60000000 {
 
 	backlight {
 		compatible = "pwm-backlight";
-		pwms = <&tpm4 1 50000 0>;
+		pwms = <&tpm4 1 50000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 20 25 30 35 40 100>;
 		default-brightness-level = <6>;
 		status = "okay";
diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
index ebbe1518ef8a..e9ea04540c16 100644
--- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
+++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
@@ -49,7 +49,7 @@ audio_clock: audio_clock {
 	lcd_backlight: backlight {
 		compatible = "pwm-backlight";
 
-		pwms = <&pwm3 0 5000000 0>;
+		pwms = <&pwm3 0 5000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <7>;
 		enable-gpios = <&gpio5 14 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi b/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
index f7b82ced4080..4bf696c3c242 100644
--- a/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
+++ b/arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi
@@ -149,7 +149,7 @@ bl: backlight {
 		compatible = "pwm-backlight";
 		pinctrl-names = "default";
 		pinctrl-0 = <&backlight_pins>;
-		pwms = <&pwm10 0 5000000 0>;
+		pwms = <&pwm10 0 5000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
 		default-brightness-level = <7>;
 		enable-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>; /* gpio_154 */
diff --git a/arch/arm/boot/dts/meson8b-ec100.dts b/arch/arm/boot/dts/meson8b-ec100.dts
index 163a200d5a7b..0380206eb13f 100644
--- a/arch/arm/boot/dts/meson8b-ec100.dts
+++ b/arch/arm/boot/dts/meson8b-ec100.dts
@@ -150,7 +150,7 @@ vcck: regulator-vcck {
 
 		vin-supply = <&vcc_5v>;
 
-		pwms = <&pwm_cd 0 1148 0>;
+		pwms = <&pwm_cd 0 1148 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
@@ -234,7 +234,7 @@ vddee: regulator-vddee {
 
 		vin-supply = <&vcc_5v>;
 
-		pwms = <&pwm_cd 1 1148 0>;
+		pwms = <&pwm_cd 1 1148 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
diff --git a/arch/arm/boot/dts/meson8b-mxq.dts b/arch/arm/boot/dts/meson8b-mxq.dts
index 33037ef62d0a..910942d4785a 100644
--- a/arch/arm/boot/dts/meson8b-mxq.dts
+++ b/arch/arm/boot/dts/meson8b-mxq.dts
@@ -39,7 +39,7 @@ vcck: regulator-vcck {
 		regulator-min-microvolt = <860000>;
 		regulator-max-microvolt = <1140000>;
 
-		pwms = <&pwm_cd 0 1148 0>;
+		pwms = <&pwm_cd 0 1148 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
@@ -86,7 +86,7 @@ vddee: regulator-vddee {
 
 		vin-supply = <&vcc_5v>;
 
-		pwms = <&pwm_cd 1 1148 0>;
+		pwms = <&pwm_cd 1 1148 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <100 0>;
 
 		regulator-boot-on;
diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts
index a2a47804fc4a..cb7411fc6d84 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -132,7 +132,7 @@ vcck: regulator-vcck {
 
 		vin-supply = <&p5v0>;
 
-		pwms = <&pwm_cd 0 12218 0>;
+		pwms = <&pwm_cd 0 12218 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <91 0>;
 
 		regulator-boot-on;
@@ -164,7 +164,7 @@ vddee: regulator-vddee {
 
 		vin-supply = <&p5v0>;
 
-		pwms = <&pwm_cd 1 12218 0>;
+		pwms = <&pwm_cd 1 12218 PWM_POLARITY_NORMAL>;
 		pwm-dutycycle-range = <91 0>;
 
 		regulator-boot-on;
diff --git a/arch/arm/boot/dts/motorola-mapphone-common.dtsi b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
index b6e82b165f5c..87b78dbe4b42 100644
--- a/arch/arm/boot/dts/motorola-mapphone-common.dtsi
+++ b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
@@ -178,7 +178,8 @@ pwm9: dmtimer-pwm-9 {
 
 	vibrator {
 		compatible = "pwm-vibrator";
-		pwms = <&pwm9 0 10000000 0>, <&pwm8 0 10000000 0>;
+		pwms = <&pwm9 0 10000000 PWM_POLARITY_NORMAL>,
+		       <&pwm8 0 10000000 PWM_POLARITY_NORMAL>;
 		pwm-names = "enable", "direction";
 		direction-duty-cycle-ns = <10000000>;
 	};
diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
index 409a758c99f1..a30278267ac2 100644
--- a/arch/arm/boot/dts/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/omap3-gta04.dtsi
@@ -138,7 +138,7 @@ lcd_in: endpoint {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm11 0 12000000 0>;
+		pwms = <&pwm11 0 12000000 PWM_POLARITY_NORMAL>;
 		pwm-names = "backlight";
 		brightness-levels = <0 11 20 30 40 50 60 70 80 90 100>;
 		default-brightness-level = <9>;	/* => 90 */
diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 4089d97405c9..99e5f0e5a271 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -152,7 +152,7 @@ pwm9: dmtimer-pwm {
 
 	ir: n900-ir {
 		compatible = "nokia,n900-ir";
-		pwms = <&pwm9 0 26316 0>; /* 38000 Hz */
+		pwms = <&pwm9 0 26316 PWM_POLARITY_NORMAL>; /* 38000 Hz */
 	};
 
 	rom_rng: rng {
diff --git a/arch/arm/boot/dts/rk3288-veyron-edp.dtsi b/arch/arm/boot/dts/rk3288-veyron-edp.dtsi
index 32c0f10765dd..56404139ac28 100644
--- a/arch/arm/boot/dts/rk3288-veyron-edp.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-edp.dtsi
@@ -47,7 +47,7 @@ backlight: backlight {
 		enable-gpios = <&gpio7 RK_PA2 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&bl_en>;
-		pwms = <&pwm0 0 1000000 0>;
+		pwms = <&pwm0 0 1000000 PWM_POLARITY_NORMAL>;
 		post-pwm-on-delay-ms = <10>;
 		pwm-off-delay-ms = <10>;
 		power-supply = <&backlight_regulator>;
diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 54a6838d73f5..58dd4fbd057c 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -101,7 +101,7 @@ vdd_logic: vdd-logic {
 		compatible = "pwm-regulator";
 		regulator-name = "vdd_logic";
 
-		pwms = <&pwm1 0 1994 0>;
+		pwms = <&pwm1 0 1994 PWM_POLARITY_NORMAL>;
 		pwm-supply = <&vcc33_sys>;
 
 		pwm-dutycycle-range = <0x7b 0>;
diff --git a/arch/arm/boot/dts/rv1108-evb.dts b/arch/arm/boot/dts/rv1108-evb.dts
index 30f3d0470ad9..dc7efab6af93 100644
--- a/arch/arm/boot/dts/rv1108-evb.dts
+++ b/arch/arm/boot/dts/rv1108-evb.dts
@@ -53,7 +53,7 @@ backlight: backlight {
 			240 241 242 243 244 245 246 247
 			248 249 250 251 252 253 254 255>;
 		default-brightness-level = <200>;
-		pwms = <&pwm0 0 25000 0>;
+		pwms = <&pwm0 0 25000 PWM_POLARITY_NORMAL>;
 	};
 
 	vcc_sys: vsys-regulator {
diff --git a/arch/arm/boot/dts/s3c6410-mini6410.dts b/arch/arm/boot/dts/s3c6410-mini6410.dts
index 1aeac33b0d34..30814ae5c09d 100644
--- a/arch/arm/boot/dts/s3c6410-mini6410.dts
+++ b/arch/arm/boot/dts/s3c6410-mini6410.dts
@@ -159,7 +159,7 @@ led-4 {
 
 	buzzer {
 		compatible = "pwm-beeper";
-		pwms = <&pwm 0 1000000 0>;
+		pwms = <&pwm 0 1000000 PWM_POLARITY_NORMAL>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pwm0_out>;
 	};
diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index 8ff70b856334..68c909588fa8 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -324,7 +324,7 @@ fuelgauge@36 {
 
 	vibrator: pwm-vibrator {
 		compatible = "pwm-vibrator";
-		pwms = <&pwm 1 44642 0>;
+		pwms = <&pwm 1 44642 PWM_POLARITY_NORMAL>;
 		pwm-names = "enable";
 		vcc-supply = <&vibrator_pwr>;
 		pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/s5pv210-smdkv210.dts b/arch/arm/boot/dts/s5pv210-smdkv210.dts
index 84b38f185199..440c6811ff58 100644
--- a/arch/arm/boot/dts/s5pv210-smdkv210.dts
+++ b/arch/arm/boot/dts/s5pv210-smdkv210.dts
@@ -42,7 +42,7 @@ ethernet@18000000 {
 
 	backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm 3 5000000 0>;
+		pwms = <&pwm 3 5000000 PWM_POLARITY_NORMAL>;
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
 		pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/sun5i-gr8-evb.dts b/arch/arm/boot/dts/sun5i-gr8-evb.dts
index 4c20d731a9c6..0e4e2cee3a64 100644
--- a/arch/arm/boot/dts/sun5i-gr8-evb.dts
+++ b/arch/arm/boot/dts/sun5i-gr8-evb.dts
@@ -69,7 +69,7 @@ chosen {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm 0 10000 0>;
+		pwms = <&pwm 0 10000 PWM_POLARITY_NORMAL>;
 		enable-gpios = <&axp_gpio 1 GPIO_ACTIVE_HIGH>;
 
 		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
diff --git a/arch/arm/boot/dts/vf-colibri.dtsi b/arch/arm/boot/dts/vf-colibri.dtsi
index fba37b8756f7..ff7c703cde4f 100644
--- a/arch/arm/boot/dts/vf-colibri.dtsi
+++ b/arch/arm/boot/dts/vf-colibri.dtsi
@@ -39,6 +39,8 @@
  *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
+#include <dt-bindings/pwm/pwm.h>
+
 / {
 	aliases {
 		ethernet0 = &fec1;
@@ -49,7 +51,7 @@ bl: backlight {
 		compatible = "pwm-backlight";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_gpio_bl_on>;
-		pwms = <&pwm0 0 5000000 0>;
+		pwms = <&pwm0 0 5000000 PWM_POLARITY_NORMAL>;
 		enable-gpios = <&gpio1 13 GPIO_ACTIVE_HIGH>;
 		status = "disabled";
 	};
-- 
2.24.1


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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-17 12:32 ` [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum Oleksandr Suvorov
@ 2020-03-17 13:34   ` Paul Barker
  2020-03-17 21:32     ` Uwe Kleine-König
  2020-03-17 16:26   ` Claudiu.Beznea
  2020-03-17 17:40   ` Thierry Reding
  2 siblings, 1 reply; 41+ messages in thread
From: Paul Barker @ 2020-03-17 13:34 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker,
	Alexandre Belloni, Chen-Yu Tsai, Claudiu Beznea, Fabio Estevam,
	Florian Fainelli, Heiko Stuebner, Kevin Hilman,
	Ludovic Desroches, Maxime Ripard, NXP Linux Team, Nicolas Ferre,
	Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, Thierry Reding, Tony Prisk, bcm-kernel-feedback-list,
	linux-amlogic, linux-arm-kernel, linux-kernel, linux-riscv,
	linux-rockchip

On Tue, 17 Mar 2020 14:32:25 +0200
Oleksandr Suvorov <oleksandr.suvorov@toradex.com> wrote:

> The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> Rename it to PWM_POLARITY_INVERTED.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>

Looks good. PWM_POLARITY_INVERSED confused me when I was working in this area
recently. After applying to master there's no more instances of
PWM_POLARITY_INVERSED in the source tree.

Reviewed-by: Paul Barker <pbarker@konsulko.com>

-- 
Paul Barker
Konsulko Group

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

* Re: [RFC PATCH 3/7] dt-bindings: pwm: add normal PWM polarity flag
  2020-03-17 12:32 ` [RFC PATCH 3/7] dt-bindings: pwm: add normal " Oleksandr Suvorov
@ 2020-03-17 13:36   ` Paul Barker
  2020-03-17 14:06     ` Oleksandr Suvorov
  2020-03-17 21:36   ` Uwe Kleine-König
  2020-03-17 22:56   ` Laurent Pinchart
  2 siblings, 1 reply; 41+ messages in thread
From: Paul Barker @ 2020-03-17 13:36 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	linux-kernel

On Tue, 17 Mar 2020 14:32:27 +0200
Oleksandr Suvorov <oleksandr.suvorov@toradex.com> wrote:

> PWM can have a normal polarity and a reverted one. The reverted polarity
> value is defined.
> Define the PWM_POLARITY_NORMAL to be used further.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>

s/reverted/inverted/

-- 
Paul Barker
Konsulko Group

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

* Re: [RFC PATCH 3/7] dt-bindings: pwm: add normal PWM polarity flag
  2020-03-17 13:36   ` Paul Barker
@ 2020-03-17 14:06     ` Oleksandr Suvorov
  0 siblings, 0 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-17 14:06 UTC (permalink / raw)
  To: Paul Barker
  Cc: devicetree, linux-pwm, Uwe Kleine-König, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	linux-kernel

On Tue, Mar 17, 2020 at 3:37 PM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Tue, 17 Mar 2020 14:32:27 +0200
> Oleksandr Suvorov <oleksandr.suvorov@toradex.com> wrote:
>
> > PWM can have a normal polarity and a reverted one. The reverted polarity
> > value is defined.
> > Define the PWM_POLARITY_NORMAL to be used further.
> >
> > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
>
> s/reverted/inverted/

Thank you, Paul, for so fast review! I'll fix it in the next version
of the patchset.

>
> --
> Paul Barker
> Konsulko Group

-- 
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-17 12:32 ` [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum Oleksandr Suvorov
  2020-03-17 13:34   ` Paul Barker
@ 2020-03-17 16:26   ` Claudiu.Beznea
  2020-03-17 16:39     ` Oleksandr Suvorov
  2020-03-17 17:40   ` Thierry Reding
  2 siblings, 1 reply; 41+ messages in thread
From: Claudiu.Beznea @ 2020-03-17 16:26 UTC (permalink / raw)
  To: oleksandr.suvorov, devicetree, linux-pwm
  Cc: pbarker, u.kleine-koenig, laurent.pinchart, marcel.ziswiler,
	igor.opaniuk, philippe.schenker, alexandre.belloni, wens,
	festevam, f.fainelli, heiko, khilman, Ludovic.Desroches, mripard,
	linux-imx, Nicolas.Ferre, palmer, paul, paul.walmsley, kernel,
	rjui, s.hauer, sbranden, shawnguo, thierry.reding, linux,
	bcm-kernel-feedback-list, linux-amlogic, linux-arm-kernel,
	linux-kernel, linux-riscv, linux-rockchip



On 17.03.2020 14:32, Oleksandr Suvorov wrote:
> @@ -187,7 +187,7 @@ static ssize_t polarity_store(struct device *child,
>         if (sysfs_streq(buf, "normal"))
>                 polarity = PWM_POLARITY_NORMAL;
>         else if (sysfs_streq(buf, "inversed"))

You may also consider this string     ^

> -               polarity = PWM_POLARITY_INVERSED;
> +               polarity = PWM_POLARITY_INVERTED;

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-17 16:26   ` Claudiu.Beznea
@ 2020-03-17 16:39     ` Oleksandr Suvorov
  0 siblings, 0 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-17 16:39 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, alexandre.belloni, wens, Fabio Estevam,
	f.fainelli, heiko, khilman, Ludovic.Desroches, mripard,
	NXP Linux Team, Nicolas.Ferre, palmer, paul, paul.walmsley,
	Pengutronix Kernel Team, rjui, Sascha Hauer, sbranden, Shawn Guo,
	Thierry Reding, linux, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

On Tue, Mar 17, 2020 at 6:27 PM <Claudiu.Beznea@microchip.com> wrote:
>
>
>
> On 17.03.2020 14:32, Oleksandr Suvorov wrote:
> > @@ -187,7 +187,7 @@ static ssize_t polarity_store(struct device *child,
> >         if (sysfs_streq(buf, "normal"))
> >                 polarity = PWM_POLARITY_NORMAL;
> >         else if (sysfs_streq(buf, "inversed"))
>
> You may also consider this string     ^

Thanks for the feedback, Claudiu.

I thought about it and decided not to change the ABI, as this change
can impact lots of user-land applications.
As a minimum, I can push this change as a separate patch to be able to
revert the change of ABI only.
What do you think?

> > -               polarity = PWM_POLARITY_INVERSED;
> > +               polarity = PWM_POLARITY_INVERTED;



-- 
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-17 12:32 ` [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum Oleksandr Suvorov
  2020-03-17 13:34   ` Paul Barker
  2020-03-17 16:26   ` Claudiu.Beznea
@ 2020-03-17 17:40   ` Thierry Reding
  2020-03-17 21:00     ` Uwe Kleine-König
  2020-03-18 11:47     ` Oleksandr Suvorov
  2 siblings, 2 replies; 41+ messages in thread
From: Thierry Reding @ 2020-03-17 17:40 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, Tony Prisk, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

On Tue, Mar 17, 2020 at 02:32:25PM +0200, Oleksandr Suvorov wrote:
> The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> Rename it to PWM_POLARITY_INVERTED.

It isn't misspelled. "inversed" is a synonym for "inverted". Both
spellings are correct.

And as you noted in the cover letter, there's a conflict between the
macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
in the wrong order you'll get a compile error.

The enum was named this way on purpose to make it separate from the
definition for the DT bindings. Note that DT bindings are an ABI and can
never change, whereas the enum pwm_polarity is part of a Linux internal
API and doesn't have the same restrictions as an ABI.

As far as I'm concerned this is completely unnecessary churn that's
potentially going to come back and bite us, so I see no reason to accept
this.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag
  2020-03-17 12:32 ` [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag Oleksandr Suvorov
@ 2020-03-17 17:43   ` Thierry Reding
  2020-03-17 21:30     ` Uwe Kleine-König
  2020-03-17 22:58   ` Laurent Pinchart
  1 sibling, 1 reply; 41+ messages in thread
From: Thierry Reding @ 2020-03-17 17:43 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Rob Herring, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

On Tue, Mar 17, 2020 at 02:32:26PM +0200, Oleksandr Suvorov wrote:
> Add the description of PWM_POLARITY_NORMAL flag.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> ---
> 
>  Documentation/devicetree/bindings/pwm/pwm.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
> index 084886bd721e..440c6b9a6a4e 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
> @@ -46,6 +46,7 @@ period in nanoseconds.
>  Optionally, the pwm-specifier can encode a number of flags (defined in
>  <dt-bindings/pwm/pwm.h>) in a third cell:
>  - PWM_POLARITY_INVERTED: invert the PWM signal polarity
> +- PWM_POLARITY_NORMAL: don't invert the PWM signal polarity

This doesn't make sense. PWM_POLARITY_NORMAL is not part of the DT ABI.
The third cell of the specifier is a bitmask of flags.

PWM_POLARITY_NORMAL is an enumeration value that evaluates to 0, so it
makes absolutely no sense as a flag. PWM signals are considered to be
"normal" by default, so no flag is necessary to specify that.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-17 17:40   ` Thierry Reding
@ 2020-03-17 21:00     ` Uwe Kleine-König
  2020-03-18 22:59       ` Thierry Reding
  2020-03-18 11:47     ` Oleksandr Suvorov
  1 sibling, 1 reply; 41+ messages in thread
From: Uwe Kleine-König @ 2020-03-17 21:00 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, Tony Prisk, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

Hello,

On Tue, Mar 17, 2020 at 06:40:43PM +0100, Thierry Reding wrote:
> On Tue, Mar 17, 2020 at 02:32:25PM +0200, Oleksandr Suvorov wrote:
> > The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> > Rename it to PWM_POLARITY_INVERTED.
> 
> It isn't misspelled. "inversed" is a synonym for "inverted". Both
> spellings are correct.

Some time ago I stumbled about "inversed", too. My spell checker doesn't
know it and I checked some dictionaries and none of them knew that word:

https://www.lexico.com/search?utf8=%E2%9C%93&filter=dictionary&dictionary=en&query=inversed
https://de.pons.com/%C3%BCbersetzung/englisch-deutsch/inversed
https://dictionary.cambridge.org/spellcheck/english-german/?q=inversed

https://en.wiktionary.org/wiki/inverse#Verb mentions "inverse" as a verb
having "inversed" as past participle.

Having said this I think (independent of the question if "inversed"
exists) using two similar terms for the same thing just results in
confusion. I hit that in the past already and I like it being addressed.

> And as you noted in the cover letter, there's a conflict between the
> macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
> in the wrong order you'll get a compile error.

There are also other symbols that exist twice (GPIO_ACTIVE_HIGH was the
first to come to my mind). I'm not aware of any problems related to
these. What am I missing?
 
> The enum was named this way on purpose to make it separate from the
> definition for the DT bindings.

Then please let's make it different by picking a different prefix or
something like that.

> Note that DT bindings are an ABI and can
> never change, whereas the enum pwm_polarity is part of a Linux internal
> API and doesn't have the same restrictions as an ABI.

I thought only binary device trees (dtb) are supposed to be ABI.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

* Re: [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag
  2020-03-17 17:43   ` Thierry Reding
@ 2020-03-17 21:30     ` Uwe Kleine-König
  2020-03-18 23:05       ` Thierry Reding
  2020-03-18 23:19       ` Thierry Reding
  0 siblings, 2 replies; 41+ messages in thread
From: Uwe Kleine-König @ 2020-03-17 21:30 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Rob Herring, linux-kernel

Hello Thierry,

On Tue, Mar 17, 2020 at 06:43:44PM +0100, Thierry Reding wrote:
> On Tue, Mar 17, 2020 at 02:32:26PM +0200, Oleksandr Suvorov wrote:
> > Add the description of PWM_POLARITY_NORMAL flag.
> > 
> > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > ---
> > 
> >  Documentation/devicetree/bindings/pwm/pwm.txt | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
> > index 084886bd721e..440c6b9a6a4e 100644
> > --- a/Documentation/devicetree/bindings/pwm/pwm.txt
> > +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
> > @@ -46,6 +46,7 @@ period in nanoseconds.
> >  Optionally, the pwm-specifier can encode a number of flags (defined in
> >  <dt-bindings/pwm/pwm.h>) in a third cell:
> >  - PWM_POLARITY_INVERTED: invert the PWM signal polarity
> > +- PWM_POLARITY_NORMAL: don't invert the PWM signal polarity
> 
> This doesn't make sense. PWM_POLARITY_NORMAL is not part of the DT ABI.

"is not part of the DT ABI" is hardly a good reason. If it's sensible to
be used, it is sensible to define it.

(And as far as I understand it, the term PWM_POLARITY_INVERTED isn't
part of the DT ABI either. Only the value 1 has a meaning (for some PWM
controlers).)

> The third cell of the specifier is a bitmask of flags.
> 
> PWM_POLARITY_NORMAL is an enumeration value that evaluates to 0, so it
> makes absolutely no sense as a flag.

Using 0 or PWM_POLARITY_NORMAL doesn't have an effect on the compiled
device tree, that's true. But having the term PWM_POLARITY_NORMAL (in
contrast to a plain 0) in a dts file is useful in my eyes for human
readers.

> PWM signals are considered to be "normal" by default, so no flag is
> necessary to specify that.

GPIOs are considered to be active high by default, still there is
GPIO_ACTIVE_HIGH (which also evaluates to 0). Also there is
IRQ_TYPE_NONE.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-17 13:34   ` Paul Barker
@ 2020-03-17 21:32     ` Uwe Kleine-König
  0 siblings, 0 replies; 41+ messages in thread
From: Uwe Kleine-König @ 2020-03-17 21:32 UTC (permalink / raw)
  To: Paul Barker
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker,
	Alexandre Belloni, Chen-Yu Tsai, Claudiu Beznea, Fabio Estevam,
	Florian Fainelli, Heiko Stuebner, Kevin Hilman,
	Ludovic Desroches, Maxime Ripard, NXP Linux Team, Nicolas Ferre,
	Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, Thierry Reding, Tony Prisk, bcm-kernel-feedback-list,
	linux-amlogic, linux-arm-kernel, linux-kernel, linux-riscv,
	linux-rockchip

On Tue, Mar 17, 2020 at 01:34:50PM +0000, Paul Barker wrote:
> On Tue, 17 Mar 2020 14:32:25 +0200
> Oleksandr Suvorov <oleksandr.suvorov@toradex.com> wrote:
> 
> > The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> > Rename it to PWM_POLARITY_INVERTED.
> > 
> > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> 
> Looks good. PWM_POLARITY_INVERSED confused me when I was working in this area
> recently.

Same for me.

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

* Re: [RFC PATCH 3/7] dt-bindings: pwm: add normal PWM polarity flag
  2020-03-17 12:32 ` [RFC PATCH 3/7] dt-bindings: pwm: add normal " Oleksandr Suvorov
  2020-03-17 13:36   ` Paul Barker
@ 2020-03-17 21:36   ` Uwe Kleine-König
  2020-03-17 22:56   ` Laurent Pinchart
  2 siblings, 0 replies; 41+ messages in thread
From: Uwe Kleine-König @ 2020-03-17 21:36 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Paul Barker, Laurent Pinchart,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	linux-kernel

On Tue, Mar 17, 2020 at 02:32:27PM +0200, Oleksandr Suvorov wrote:
> PWM can have a normal polarity and a reverted one. The reverted polarity
> value is defined.
> Define the PWM_POLARITY_NORMAL to be used further.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> ---
> 
>  include/dt-bindings/pwm/pwm.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
> index ab9a077e3c7d..6b58caa6385e 100644
> --- a/include/dt-bindings/pwm/pwm.h
> +++ b/include/dt-bindings/pwm/pwm.h
> @@ -10,6 +10,7 @@
>  #ifndef _DT_BINDINGS_PWM_PWM_H
>  #define _DT_BINDINGS_PWM_PWM_H
>  
> +#define PWM_POLARITY_NORMAL			0
>  #define PWM_POLARITY_INVERTED			(1 << 0)

Maybe define PWM_POLARITY_NORMAL as (0 << 0) to make it more obvious
that it is the inverse of PWM_POLARITY_INVERTED?

But even when kept as is I like hafing PWM_POLARITY_NORMAL in the
binding definitions.

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

* Re: [RFC PATCH 3/7] dt-bindings: pwm: add normal PWM polarity flag
  2020-03-17 12:32 ` [RFC PATCH 3/7] dt-bindings: pwm: add normal " Oleksandr Suvorov
  2020-03-17 13:36   ` Paul Barker
  2020-03-17 21:36   ` Uwe Kleine-König
@ 2020-03-17 22:56   ` Laurent Pinchart
  2020-03-18  9:20     ` Uwe Kleine-König
  2 siblings, 1 reply; 41+ messages in thread
From: Laurent Pinchart @ 2020-03-17 22:56 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	linux-kernel

Hi Oleksandr,

Thank you for the patch.

On Tue, Mar 17, 2020 at 02:32:27PM +0200, Oleksandr Suvorov wrote:
> PWM can have a normal polarity and a reverted one. The reverted polarity
> value is defined.

As mentioned by Paul, I'd use "inverted" instead of "reverted". Your
patch series is trying to standardized on "inverted", let's not add
another term :-)

I would squash this patch with 2/7, apart from that it looks fine.
However, I also agree with Thierry that the PWM cell that contains this
value is a bitmask, so once we get more flags it may get a bit awkward.
Will we have one macro for each flag that will evaluate to 0 to report
that the flag isn't set ? Or should we define a single PWM_FLAG_NONE (or
similarly named) macro ? In retrospect, maybe PWM_POLARITY_INVERTED
should have been named PWM_FLAG_POLARITY_INVERTED.

> Define the PWM_POLARITY_NORMAL to be used further.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> ---
> 
>  include/dt-bindings/pwm/pwm.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
> index ab9a077e3c7d..6b58caa6385e 100644
> --- a/include/dt-bindings/pwm/pwm.h
> +++ b/include/dt-bindings/pwm/pwm.h
> @@ -10,6 +10,7 @@
>  #ifndef _DT_BINDINGS_PWM_PWM_H
>  #define _DT_BINDINGS_PWM_PWM_H
>  
> +#define PWM_POLARITY_NORMAL			0
>  #define PWM_POLARITY_INVERTED			(1 << 0)
>  
>  #endif

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag
  2020-03-17 12:32 ` [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag Oleksandr Suvorov
  2020-03-17 17:43   ` Thierry Reding
@ 2020-03-17 22:58   ` Laurent Pinchart
  1 sibling, 0 replies; 41+ messages in thread
From: Laurent Pinchart @ 2020-03-17 22:58 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	Thierry Reding, linux-kernel

Hi Oleksandr,

Thank you for the patch.

On Tue, Mar 17, 2020 at 02:32:26PM +0200, Oleksandr Suvorov wrote:
> Add the description of PWM_POLARITY_NORMAL flag.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> ---
> 
>  Documentation/devicetree/bindings/pwm/pwm.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
> index 084886bd721e..440c6b9a6a4e 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
> @@ -46,6 +46,7 @@ period in nanoseconds.
>  Optionally, the pwm-specifier can encode a number of flags (defined in
>  <dt-bindings/pwm/pwm.h>) in a third cell:
>  - PWM_POLARITY_INVERTED: invert the PWM signal polarity
> +- PWM_POLARITY_NORMAL: don't invert the PWM signal polarity

With the previous line mentioning that the pwm-specifier can encode a
number of *flags*, this becomes confusing: reading the documentation
only and not pwm.h, one could wonder what happens if none or both of
PWM_POLARITY_INVERTED and PWM_POLARITY_NORMAL are specified :-(

>  
>  Example with optional PWM specifier for inverse polarity
>  

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC PATCH 4/7] dt-bindings: pwm: add description of PWM polarity
  2020-03-17 12:32 ` [RFC PATCH 4/7] dt-bindings: pwm: add description of PWM polarity Oleksandr Suvorov
@ 2020-03-17 23:01   ` Laurent Pinchart
  2020-03-18 11:37     ` Oleksandr Suvorov
  0 siblings, 1 reply; 41+ messages in thread
From: Laurent Pinchart @ 2020-03-17 23:01 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	linux-kernel

Hi Oleksandr,

Thank you for the patch.

On Tue, Mar 17, 2020 at 02:32:28PM +0200, Oleksandr Suvorov wrote:
> Move the description of the PWM signal polarity from
> <linux/pwm.h>, prepare for removing the polarity
> definition from <linux/pwm.h>.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> ---
> 
>  include/dt-bindings/pwm/pwm.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
> index 6b58caa6385e..c07da2088a61 100644
> --- a/include/dt-bindings/pwm/pwm.h
> +++ b/include/dt-bindings/pwm/pwm.h
> @@ -10,7 +10,16 @@
>  #ifndef _DT_BINDINGS_PWM_PWM_H
>  #define _DT_BINDINGS_PWM_PWM_H
>  
> +/**
> + * a high signal for the duration of the duty-cycle, followed by a low signal
> + * for the remainder of the pulse period.
> + */

Last time I checked, kernedoc didn't support documenting macros (enums
are supported).

>  #define PWM_POLARITY_NORMAL			0
> +
> +/**
> + * a low signal for the duration of the duty-cycle, followed by a high signal
> + * for the remainder of the pulse period.
> + */
>  #define PWM_POLARITY_INVERTED			(1 << 0)
>  
>  #endif

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC PATCH 3/7] dt-bindings: pwm: add normal PWM polarity flag
  2020-03-17 22:56   ` Laurent Pinchart
@ 2020-03-18  9:20     ` Uwe Kleine-König
  0 siblings, 0 replies; 41+ messages in thread
From: Uwe Kleine-König @ 2020-03-18  9:20 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	linux-kernel

Hello Laurent,

On Wed, Mar 18, 2020 at 12:56:56AM +0200, Laurent Pinchart wrote:
> On Tue, Mar 17, 2020 at 02:32:27PM +0200, Oleksandr Suvorov wrote:
> > PWM can have a normal polarity and a reverted one. The reverted polarity
> > value is defined.
> 
> I would squash this patch with 2/7, apart from that it looks fine.
> However, I also agree with Thierry that the PWM cell that contains this
> value is a bitmask, so once we get more flags it may get a bit awkward.

For me the usefulness of PWM_POLARITY_NORMAL increases with more bits
used. That's because if there are 5 things that can be set there and the
patch author mentions only the two that are non-zero, I as a reviewer
don't know if the author actually know and thought about the other
three. If however they spell out PWM_POLARITY_NORMAL it's quite sure
they want normal polarity.

> Will we have one macro for each flag that will evaluate to 0 to report
> that the flag isn't set ?

Yes. Given the above mentioned advantage this is cheap enough in my
eyes.

> Or should we define a single PWM_FLAG_NONE (or
> similarly named) macro ?

I like one macro for each bit field better for the above mentioned
reason.

> In retrospect, maybe PWM_POLARITY_INVERTED
> should have been named PWM_FLAG_POLARITY_INVERTED.

Seems to be subjective. I don't see much added semantic that justifies
the longer name.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

* Re: [RFC PATCH 4/7] dt-bindings: pwm: add description of PWM polarity
  2020-03-17 23:01   ` Laurent Pinchart
@ 2020-03-18 11:37     ` Oleksandr Suvorov
  2020-03-18 12:29       ` Laurent Pinchart
  0 siblings, 1 reply; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-18 11:37 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	linux-kernel

On Wed, Mar 18, 2020 at 1:02 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Oleksandr,
>
> Thank you for the patch.
>
> On Tue, Mar 17, 2020 at 02:32:28PM +0200, Oleksandr Suvorov wrote:
> > Move the description of the PWM signal polarity from
> > <linux/pwm.h>, prepare for removing the polarity
> > definition from <linux/pwm.h>.
> >
> > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > ---
> >
> >  include/dt-bindings/pwm/pwm.h | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
> > index 6b58caa6385e..c07da2088a61 100644
> > --- a/include/dt-bindings/pwm/pwm.h
> > +++ b/include/dt-bindings/pwm/pwm.h
> > @@ -10,7 +10,16 @@
> >  #ifndef _DT_BINDINGS_PWM_PWM_H
> >  #define _DT_BINDINGS_PWM_PWM_H
> >
> > +/**
> > + * a high signal for the duration of the duty-cycle, followed by a low signal
> > + * for the remainder of the pulse period.
> > + */
>
> Last time I checked, kernedoc didn't support documenting macros (enums
> are supported).

That's why I dropped the kerneldoc tags leaving the descriptions only.

> >  #define PWM_POLARITY_NORMAL                  0
> > +
> > +/**
> > + * a low signal for the duration of the duty-cycle, followed by a high signal
> > + * for the remainder of the pulse period.
> > + */
> >  #define PWM_POLARITY_INVERTED                        (1 << 0)
> >
> >  #endif
>
> --
> Regards,
>
> Laurent Pinchart

-- 
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-17 17:40   ` Thierry Reding
  2020-03-17 21:00     ` Uwe Kleine-König
@ 2020-03-18 11:47     ` Oleksandr Suvorov
  1 sibling, 0 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-18 11:47 UTC (permalink / raw)
  To: Thierry Reding
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, Tony Prisk, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

On Tue, Mar 17, 2020 at 7:41 PM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Tue, Mar 17, 2020 at 02:32:25PM +0200, Oleksandr Suvorov wrote:
> > The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> > Rename it to PWM_POLARITY_INVERTED.
>
> It isn't misspelled. "inversed" is a synonym for "inverted". Both
> spellings are correct.

> And as you noted in the cover letter, there's a conflict between the
> macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
> in the wrong order you'll get a compile error.

This patch is a part of the patchset, which in result removes enum at all,
so there will be no definition conflict.

> The enum was named this way on purpose to make it separate from the
> definition for the DT bindings. Note that DT bindings are an ABI and can
> never change, whereas the enum pwm_polarity is part of a Linux internal
> API and doesn't have the same restrictions as an ABI.

AFAIU, DTS files are not a part of ABI.

I understand that enums are better than macros for some reasons.
However, I think it is dangerous to use duplicate definitions in
different places when values of these definitions use in the same
code.
So, given that the enum cannot be used in DT, I left only macros.

You personally wrote that the enum pwm_polarity can change, so the
desynchronization I quite possible.

> As far as I'm concerned this is completely unnecessary churn that's
> potentially going to come back and bite us, so I see no reason to accept
> this.


>
> Thierry
--
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

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

* Re: [RFC PATCH 4/7] dt-bindings: pwm: add description of PWM polarity
  2020-03-18 11:37     ` Oleksandr Suvorov
@ 2020-03-18 12:29       ` Laurent Pinchart
  2020-03-18 12:36         ` Oleksandr Suvorov
  0 siblings, 1 reply; 41+ messages in thread
From: Laurent Pinchart @ 2020-03-18 12:29 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	linux-kernel

Hi Oleksandr,

On Wed, Mar 18, 2020 at 01:37:00PM +0200, Oleksandr Suvorov wrote:
> On Wed, Mar 18, 2020 at 1:02 AM Laurent Pinchart wrote:
> > On Tue, Mar 17, 2020 at 02:32:28PM +0200, Oleksandr Suvorov wrote:
> > > Move the description of the PWM signal polarity from
> > > <linux/pwm.h>, prepare for removing the polarity
> > > definition from <linux/pwm.h>.
> > >
> > > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > > ---
> > >
> > >  include/dt-bindings/pwm/pwm.h | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
> > > index 6b58caa6385e..c07da2088a61 100644
> > > --- a/include/dt-bindings/pwm/pwm.h
> > > +++ b/include/dt-bindings/pwm/pwm.h
> > > @@ -10,7 +10,16 @@
> > >  #ifndef _DT_BINDINGS_PWM_PWM_H
> > >  #define _DT_BINDINGS_PWM_PWM_H
> > >
> > > +/**
> > > + * a high signal for the duration of the duty-cycle, followed by a low signal
> > > + * for the remainder of the pulse period.
> > > + */
> >
> > Last time I checked, kernedoc didn't support documenting macros (enums
> > are supported).
> 
> That's why I dropped the kerneldoc tags leaving the descriptions only.

But you forgot to replace /** with /* :-) Sorry for not being clear
about what I meant.

> > >  #define PWM_POLARITY_NORMAL                  0
> > > +
> > > +/**
> > > + * a low signal for the duration of the duty-cycle, followed by a high signal
> > > + * for the remainder of the pulse period.
> > > + */
> > >  #define PWM_POLARITY_INVERTED                        (1 << 0)
> > >
> > >  #endif

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC PATCH 4/7] dt-bindings: pwm: add description of PWM polarity
  2020-03-18 12:29       ` Laurent Pinchart
@ 2020-03-18 12:36         ` Oleksandr Suvorov
  0 siblings, 0 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-18 12:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Marcel Ziswiler, Igor Opaniuk, Philippe Schenker, Rob Herring,
	linux-kernel

On Wed, Mar 18, 2020 at 2:30 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Oleksandr,
>
> On Wed, Mar 18, 2020 at 01:37:00PM +0200, Oleksandr Suvorov wrote:
> > On Wed, Mar 18, 2020 at 1:02 AM Laurent Pinchart wrote:
> > > On Tue, Mar 17, 2020 at 02:32:28PM +0200, Oleksandr Suvorov wrote:
> > > > Move the description of the PWM signal polarity from
> > > > <linux/pwm.h>, prepare for removing the polarity
> > > > definition from <linux/pwm.h>.
> > > >
> > > > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > > > ---
> > > >
> > > >  include/dt-bindings/pwm/pwm.h | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
> > > > index 6b58caa6385e..c07da2088a61 100644
> > > > --- a/include/dt-bindings/pwm/pwm.h
> > > > +++ b/include/dt-bindings/pwm/pwm.h
> > > > @@ -10,7 +10,16 @@
> > > >  #ifndef _DT_BINDINGS_PWM_PWM_H
> > > >  #define _DT_BINDINGS_PWM_PWM_H
> > > >
> > > > +/**
> > > > + * a high signal for the duration of the duty-cycle, followed by a low signal
> > > > + * for the remainder of the pulse period.
> > > > + */
> > >
> > > Last time I checked, kernedoc didn't support documenting macros (enums
> > > are supported).
> >
> > That's why I dropped the kerneldoc tags leaving the descriptions only.
>
> But you forgot to replace /** with /* :-) Sorry for not being clear
> about what I meant.

Ah, yes, thanks! I'll fix it in the next version :)

>
> > > >  #define PWM_POLARITY_NORMAL                  0
> > > > +
> > > > +/**
> > > > + * a low signal for the duration of the duty-cycle, followed by a high signal
> > > > + * for the remainder of the pulse period.
> > > > + */
> > > >  #define PWM_POLARITY_INVERTED                        (1 << 0)
> > > >
> > > >  #endif
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-17 21:00     ` Uwe Kleine-König
@ 2020-03-18 22:59       ` Thierry Reding
  2020-03-19  6:50         ` Uwe Kleine-König
  2020-03-19 11:40         ` Oleksandr Suvorov
  0 siblings, 2 replies; 41+ messages in thread
From: Thierry Reding @ 2020-03-18 22:59 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, Tony Prisk, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

[-- Attachment #1: Type: text/plain, Size: 3499 bytes --]

On Tue, Mar 17, 2020 at 10:00:42PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On Tue, Mar 17, 2020 at 06:40:43PM +0100, Thierry Reding wrote:
> > On Tue, Mar 17, 2020 at 02:32:25PM +0200, Oleksandr Suvorov wrote:
> > > The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> > > Rename it to PWM_POLARITY_INVERTED.
> > 
> > It isn't misspelled. "inversed" is a synonym for "inverted". Both
> > spellings are correct.
> 
> Some time ago I stumbled about "inversed", too. My spell checker doesn't
> know it and I checked some dictionaries and none of them knew that word:
> 
> https://www.lexico.com/search?utf8=%E2%9C%93&filter=dictionary&dictionary=en&query=inversed
> https://de.pons.com/%C3%BCbersetzung/englisch-deutsch/inversed
> https://dictionary.cambridge.org/spellcheck/english-german/?q=inversed
> 
> https://en.wiktionary.org/wiki/inverse#Verb mentions "inverse" as a verb
> having "inversed" as past participle.

Here are the first three results from a Google query:

	https://www.yourdictionary.com/inversed
	https://www.dictionary.com/browse/inversed
	https://en.wiktionary.org/wiki/inversed

> Having said this I think (independent of the question if "inversed"
> exists) using two similar terms for the same thing just results in
> confusion. I hit that in the past already and I like it being addressed.

I don't know. It's pretty common to use different words for the same
thing. They're called synonyms.

> > And as you noted in the cover letter, there's a conflict between the
> > macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
> > in the wrong order you'll get a compile error.
> 
> There are also other symbols that exist twice (GPIO_ACTIVE_HIGH was the
> first to come to my mind). I'm not aware of any problems related to
> these. What am I missing?

There's currently no problem, obviously. But if for some reason the
include files end up being included in a different order (i.e. the
dt-bindings header is included before linux/pwm.h) then the macro will
be evaluated and result in something like:

	enum pwm_polarity {
		PWM_POLARITY_NORMAL,
		1,
	};

and that's not valid C, so will cause a build error.

> > The enum was named this way on purpose to make it separate from the
> > definition for the DT bindings.
> 
> Then please let's make it different by picking a different prefix or
> something like that.

Again, seems to me like unnecessary churn. Feel free to propose
something, but I recall being in the same position at the time and this
was the best I could come up with.

> > Note that DT bindings are an ABI and can
> > never change, whereas the enum pwm_polarity is part of a Linux internal
> > API and doesn't have the same restrictions as an ABI.
> 
> I thought only binary device trees (dtb) are supposed to be ABI.

Yes, the DTB is the ABI. dt-bindings/pwm/pwm.h is used to generate DTBs,
which basically makes it ABI as well. Yes, the symbol name may not be
part of the ABI, but changing the symbol becomes very inconvenient
because everyone that depends on it would have to change. Why bother?

My point is that enum pwm_polarity is an API in the kernel and hence its
easy to change or extend. But since that is not the same for the DTB, we
need to be careful what from the internal kernel API leaks into the DTB.
That's why they are different symbols, so that it is clear that what's
in dt-bindings/pwm/pwm.h is the ABI.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag
  2020-03-17 21:30     ` Uwe Kleine-König
@ 2020-03-18 23:05       ` Thierry Reding
  2020-03-19  7:05         ` Uwe Kleine-König
  2020-03-18 23:19       ` Thierry Reding
  1 sibling, 1 reply; 41+ messages in thread
From: Thierry Reding @ 2020-03-18 23:05 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Rob Herring, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

On Tue, Mar 17, 2020 at 10:30:56PM +0100, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Tue, Mar 17, 2020 at 06:43:44PM +0100, Thierry Reding wrote:
> > On Tue, Mar 17, 2020 at 02:32:26PM +0200, Oleksandr Suvorov wrote:
> > > Add the description of PWM_POLARITY_NORMAL flag.
> > > 
> > > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > > ---
> > > 
> > >  Documentation/devicetree/bindings/pwm/pwm.txt | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > index 084886bd721e..440c6b9a6a4e 100644
> > > --- a/Documentation/devicetree/bindings/pwm/pwm.txt
> > > +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > @@ -46,6 +46,7 @@ period in nanoseconds.
> > >  Optionally, the pwm-specifier can encode a number of flags (defined in
> > >  <dt-bindings/pwm/pwm.h>) in a third cell:
> > >  - PWM_POLARITY_INVERTED: invert the PWM signal polarity
> > > +- PWM_POLARITY_NORMAL: don't invert the PWM signal polarity
> > 
> > This doesn't make sense. PWM_POLARITY_NORMAL is not part of the DT ABI.
> 
> "is not part of the DT ABI" is hardly a good reason. If it's sensible to
> be used, it is sensible to define it.

That's exactly it. It's not sensible at all to use it. If you define it
here it means people are allowed to do stuff like this:

	pwms = <&pwm 1234 PWM_POLARITY_INVERTED | PWM_POLARITY_NORMAL>;

which doesn't make sense. What's more, it impossible for the code to
even notice that you're being silly because | PWM_POLARITY_NORMAL is
just | 0 and that's just a nop.

> (And as far as I understand it, the term PWM_POLARITY_INVERTED isn't
> part of the DT ABI either. Only the value 1 has a meaning (for some PWM
> controlers).)
> 
> > The third cell of the specifier is a bitmask of flags.
> > 
> > PWM_POLARITY_NORMAL is an enumeration value that evaluates to 0, so it
> > makes absolutely no sense as a flag.
> 
> Using 0 or PWM_POLARITY_NORMAL doesn't have an effect on the compiled
> device tree, that's true. But having the term PWM_POLARITY_NORMAL (in
> contrast to a plain 0) in a dts file is useful in my eyes for human
> readers.
> 
> > PWM signals are considered to be "normal" by default, so no flag is
> > necessary to specify that.
> 
> GPIOs are considered to be active high by default, still there is
> GPIO_ACTIVE_HIGH (which also evaluates to 0). Also there is
> IRQ_TYPE_NONE.

I'm aware of those. That doesn't mean that everything needs to have
symbolic names.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag
  2020-03-17 21:30     ` Uwe Kleine-König
  2020-03-18 23:05       ` Thierry Reding
@ 2020-03-18 23:19       ` Thierry Reding
  1 sibling, 0 replies; 41+ messages in thread
From: Thierry Reding @ 2020-03-18 23:19 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Rob Herring, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3179 bytes --]

On Tue, Mar 17, 2020 at 10:30:56PM +0100, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Tue, Mar 17, 2020 at 06:43:44PM +0100, Thierry Reding wrote:
> > On Tue, Mar 17, 2020 at 02:32:26PM +0200, Oleksandr Suvorov wrote:
> > > Add the description of PWM_POLARITY_NORMAL flag.
> > > 
> > > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > > ---
> > > 
> > >  Documentation/devicetree/bindings/pwm/pwm.txt | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > index 084886bd721e..440c6b9a6a4e 100644
> > > --- a/Documentation/devicetree/bindings/pwm/pwm.txt
> > > +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > @@ -46,6 +46,7 @@ period in nanoseconds.
> > >  Optionally, the pwm-specifier can encode a number of flags (defined in
> > >  <dt-bindings/pwm/pwm.h>) in a third cell:
> > >  - PWM_POLARITY_INVERTED: invert the PWM signal polarity
> > > +- PWM_POLARITY_NORMAL: don't invert the PWM signal polarity
> > 
> > This doesn't make sense. PWM_POLARITY_NORMAL is not part of the DT ABI.
> 
> "is not part of the DT ABI" is hardly a good reason. If it's sensible to
> be used, it is sensible to define it.

That's exactly it. It's not sensible at all to use it. If you define it
here it means people are allowed to do stuff like this:

	pwms = <&pwm 1234 PWM_POLARITY_INVERTED | PWM_POLARITY_NORMAL>;

which doesn't make sense. What's more, it impossible for the code to
even notice that you're being silly because | PWM_POLARITY_NORMAL is
just | 0 and that's just a nop.

Thierry

> > The third cell of the specifier is a bitmask of flags.
> > 
> > PWM_POLARITY_NORMAL is an enumeration value that evaluates to 0, so it
> > makes absolutely no sense as a flag.
> 
> Using 0 or PWM_POLARITY_NORMAL doesn't have an effect on the compiled
> device tree, that's true. But having the term PWM_POLARITY_NORMAL (in
> contrast to a plain 0) in a dts file is useful in my eyes for human
> readers.

Yes, I suppose that's true.

> > PWM signals are considered to be "normal" by default, so no flag is
> > necessary to specify that.
> 
> GPIOs are considered to be active high by default, still there is
> GPIO_ACTIVE_HIGH (which also evaluates to 0). Also there is
> IRQ_TYPE_NONE.

I'm aware of these. They carry the same risks as I mentioned above,
though. You can easily make mistakes that no software will be able to
detect. If you make a GPIO GPIO_ACTIVE_HIGH | GPIO_ACTIVE_LOW, it
becomes really confusing as to what that means. It really means that the
GPIO will be active-low because GPIO_ACTIVE_HIGH doesn't do anything.
But just reading that may make you think that perhaps HIGH is better
(because, well, it's high, or perhaps because it is listed first).
Having both may also be interpreted as "don't care".

In my opinion things become much easier when you don't have any of the
alternatives. If you don't specify PWM_POLARITY_INVERTED, well, it's
just not going to be inverted, it's going to be normal. No need to be
extra verbose about that.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-18 22:59       ` Thierry Reding
@ 2020-03-19  6:50         ` Uwe Kleine-König
  2020-03-19 16:37           ` Thierry Reding
  2020-03-19 11:40         ` Oleksandr Suvorov
  1 sibling, 1 reply; 41+ messages in thread
From: Uwe Kleine-König @ 2020-03-19  6:50 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip


[Dropped Tony Prisk from recipients as the address bounces]

Hello,

On Wed, Mar 18, 2020 at 11:59:53PM +0100, Thierry Reding wrote:
> On Tue, Mar 17, 2020 at 10:00:42PM +0100, Uwe Kleine-König wrote:
> > Hello,
> > 
> > On Tue, Mar 17, 2020 at 06:40:43PM +0100, Thierry Reding wrote:
> > > On Tue, Mar 17, 2020 at 02:32:25PM +0200, Oleksandr Suvorov wrote:
> > > > The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> > > > Rename it to PWM_POLARITY_INVERTED.
> > > 
> > > It isn't misspelled. "inversed" is a synonym for "inverted". Both
> > > spellings are correct.
> > 
> > Some time ago I stumbled about "inversed", too. My spell checker doesn't
> > know it and I checked some dictionaries and none of them knew that word:
> > 
> > https://www.lexico.com/search?utf8=%E2%9C%93&filter=dictionary&dictionary=en&query=inversed
> > https://de.pons.com/%C3%BCbersetzung/englisch-deutsch/inversed
> > https://dictionary.cambridge.org/spellcheck/english-german/?q=inversed
> > 
> > https://en.wiktionary.org/wiki/inverse#Verb mentions "inverse" as a verb
> > having "inversed" as past participle.
> 
> Here are the first three results from a Google query:
> 
> 	https://www.yourdictionary.com/inversed

There is something fishy. In the Verb section it says indeed, that it is
the past participle and simple past of inverse. The entry for inverse
however only has sections that identify this word as adjective or noun;
not a verb.

> 	https://www.dictionary.com/browse/inversed

Not sure I'd count this as hint that inversed exists. The entry shown to
me under this URL is about "inverse" and it has

	verb (used with object), in·versed, in·vers·ing.
		? to invert.

Does this mean: "Did you mean invert instead?"

> 	https://en.wiktionary.org/wiki/inversed

Yeah, that's the one I found, too.

I still have the impression that "inversed" is in use because people
don't know better and understand the intended meaning. And this results
in leaking of this word into the references.

> > Having said this I think (independent of the question if "inversed"
> > exists) using two similar terms for the same thing just results in
> > confusion. I hit that in the past already and I like it being addressed.
> 
> I don't know. It's pretty common to use different words for the same
> thing. They're called synonyms.

In literature yes, I agree. In a novel it is annoying to repeat the same
words over and over again and some variation is good. In programming
however the goal is a different one. There the goal should be to be
precise and consistent.

> > > And as you noted in the cover letter, there's a conflict between the
> > > macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
> > > in the wrong order you'll get a compile error.
> > 
> > There are also other symbols that exist twice (GPIO_ACTIVE_HIGH was the
> > first to come to my mind). I'm not aware of any problems related to
> > these. What am I missing?
> 
> There's currently no problem, obviously. But if for some reason the
> include files end up being included in a different order (i.e. the
> dt-bindings header is included before linux/pwm.h) then the macro will
> be evaluated and result in something like:
> 
> 	enum pwm_polarity {
> 		PWM_POLARITY_NORMAL,
> 		1,
> 	};
> 
> and that's not valid C, so will cause a build error.

I admit I didn't look closely here and I assume you are right. If I
understand Oleksandr right this is only an intermediate step and when
the series is applied completely this issue is gone. Still it might be
worth to improve the series here.

My original question was about similar problems with GPIO_ACTIVE_HIGH.
Are you aware of problems there?

> > > Note that DT bindings are an ABI and can
> > > never change, whereas the enum pwm_polarity is part of a Linux internal
> > > API and doesn't have the same restrictions as an ABI.
> > 
> > I thought only binary device trees (dtb) are supposed to be ABI.
> 
> Yes, the DTB is the ABI. dt-bindings/pwm/pwm.h is used to generate DTBs,
> which basically makes it ABI as well.

We disagree here. With this argument you could fix quite some things as
ABI.

> Yes, the symbol name may not be part of the ABI, but changing the
> symbol becomes very inconvenient because everyone that depends on it
> would have to change.

Oleksandr adapted all in-tree users, so it only affects out-of-tree
users. In my book this is fine.

> Why bother?

To make the API more precise and consistent. That's a good goal in my
eyes.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

* Re: [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag
  2020-03-18 23:05       ` Thierry Reding
@ 2020-03-19  7:05         ` Uwe Kleine-König
  2020-03-19 17:04           ` Thierry Reding
  0 siblings, 1 reply; 41+ messages in thread
From: Uwe Kleine-König @ 2020-03-19  7:05 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Rob Herring, linux-kernel

On Thu, Mar 19, 2020 at 12:05:39AM +0100, Thierry Reding wrote:
> On Tue, Mar 17, 2020 at 10:30:56PM +0100, Uwe Kleine-König wrote:
> > Hello Thierry,
> > 
> > On Tue, Mar 17, 2020 at 06:43:44PM +0100, Thierry Reding wrote:
> > > On Tue, Mar 17, 2020 at 02:32:26PM +0200, Oleksandr Suvorov wrote:
> > > > Add the description of PWM_POLARITY_NORMAL flag.
> > > > 
> > > > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > > > ---
> > > > 
> > > >  Documentation/devicetree/bindings/pwm/pwm.txt | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > > index 084886bd721e..440c6b9a6a4e 100644
> > > > --- a/Documentation/devicetree/bindings/pwm/pwm.txt
> > > > +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > > @@ -46,6 +46,7 @@ period in nanoseconds.
> > > >  Optionally, the pwm-specifier can encode a number of flags (defined in
> > > >  <dt-bindings/pwm/pwm.h>) in a third cell:
> > > >  - PWM_POLARITY_INVERTED: invert the PWM signal polarity
> > > > +- PWM_POLARITY_NORMAL: don't invert the PWM signal polarity
> > > 
> > > This doesn't make sense. PWM_POLARITY_NORMAL is not part of the DT ABI.
> > 
> > "is not part of the DT ABI" is hardly a good reason. If it's sensible to
> > be used, it is sensible to define it.
> 
> That's exactly it. It's not sensible at all to use it.

If you think the argument is "It is not sensible to be used." then please
say so and don't add "PWM_POLARITY_NORMAL is not part of the DT ABI."
which seems to be irrelevant now.

> If you define it here it means people are allowed to do stuff like
> this:
> 
> 	pwms = <&pwm 1234 PWM_POLARITY_INVERTED | PWM_POLARITY_NORMAL>;
> 
> which doesn't make sense.

I would hope that a human reader would catch this.

> What's more, it impossible for the code to even notice that you're
> being silly because | PWM_POLARITY_NORMAL is just | 0 and that's just
> a nop.

I think this argument is a bad one. Whenever you introduce a
function or symbol you can use it in a wrong way. With this argument you
can also say GPIO_ACTIVE_LOW doesn't make sense because

	pwms = <&pwm 1234 GPIO_ACTIVE_LOW>;

is silly.

Keep well and fit,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-18 22:59       ` Thierry Reding
  2020-03-19  6:50         ` Uwe Kleine-König
@ 2020-03-19 11:40         ` Oleksandr Suvorov
  2020-03-19 12:10           ` Uwe Kleine-König
  2020-03-19 16:44           ` Thierry Reding
  1 sibling, 2 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-19 11:40 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Uwe Kleine-König, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, Tony Prisk, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

On Thu, Mar 19, 2020 at 1:00 AM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Tue, Mar 17, 2020 at 10:00:42PM +0100, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Tue, Mar 17, 2020 at 06:40:43PM +0100, Thierry Reding wrote:
> > > On Tue, Mar 17, 2020 at 02:32:25PM +0200, Oleksandr Suvorov wrote:
> > > > The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> > > > Rename it to PWM_POLARITY_INVERTED.
> > >
> > > It isn't misspelled. "inversed" is a synonym for "inverted". Both
> > > spellings are correct.
> >
> > Some time ago I stumbled about "inversed", too. My spell checker doesn't
> > know it and I checked some dictionaries and none of them knew that word:
> >
> > https://www.lexico.com/search?utf8=%E2%9C%93&filter=dictionary&dictionary=en&query=inversed
> > https://de.pons.com/%C3%BCbersetzung/englisch-deutsch/inversed
> > https://dictionary.cambridge.org/spellcheck/english-german/?q=inversed
> >
> > https://en.wiktionary.org/wiki/inverse#Verb mentions "inverse" as a verb
> > having "inversed" as past participle.
>
> Here are the first three results from a Google query:
>
>         https://www.yourdictionary.com/inversed
>         https://www.dictionary.com/browse/inversed
>         https://en.wiktionary.org/wiki/inversed
>
> > Having said this I think (independent of the question if "inversed"
> > exists) using two similar terms for the same thing just results in
> > confusion. I hit that in the past already and I like it being addressed.
>
> I don't know. It's pretty common to use different words for the same
> thing. They're called synonyms.
>
> > > And as you noted in the cover letter, there's a conflict between the
> > > macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
> > > in the wrong order you'll get a compile error.
> >
> > There are also other symbols that exist twice (GPIO_ACTIVE_HIGH was the
> > first to come to my mind). I'm not aware of any problems related to
> > these. What am I missing?
>
> There's currently no problem, obviously. But if for some reason the
> include files end up being included in a different order (i.e. the
> dt-bindings header is included before linux/pwm.h) then the macro will
> be evaluated and result in something like:
>
>         enum pwm_polarity {
>                 PWM_POLARITY_NORMAL,
>                 1,
>         };
>
> and that's not valid C, so will cause a build error.
>
> > > The enum was named this way on purpose to make it separate from the
> > > definition for the DT bindings.
> >
> > Then please let's make it different by picking a different prefix or
> > something like that.
>
> Again, seems to me like unnecessary churn. Feel free to propose
> something, but I recall being in the same position at the time and this
> was the best I could come up with.
>
> > > Note that DT bindings are an ABI and can
> > > never change, whereas the enum pwm_polarity is part of a Linux internal
> > > API and doesn't have the same restrictions as an ABI.
> >
> > I thought only binary device trees (dtb) are supposed to be ABI.
>
> Yes, the DTB is the ABI. dt-bindings/pwm/pwm.h is used to generate DTBs,
> which basically makes it ABI as well. Yes, the symbol name may not be
> part of the ABI, but changing the symbol becomes very inconvenient
> because everyone that depends on it would have to change. Why bother?
>
> My point is that enum pwm_polarity is an API in the kernel and hence its
> easy to change or extend. But since that is not the same for the DTB, we
> need to be careful what from the internal kernel API leaks into the DTB.
> That's why they are different symbols, so that it is clear that what's
> in dt-bindings/pwm/pwm.h is the ABI.

Thierry, I see the PWM core converts the bit field "third cell" into
the polarity variable.
Now I probably understand your sight and agree that we shouldn't give
the same names to bits in bitfield (dts) and values of a variable.

But there are lots of useless "0" values of third cell of "pwms"
option in dts files.

I see 2 ways now:
- just remove all "0" "third cell" from "pwms" options in dts files. I
see this "0" confuses some people.
- convert pwm_state.polarity into pwm_state.flags and use bitfield
directly from dtb.
  It simplifies the parsing logic and makes adding new flags easier.

What do think?

>
> Thierry

-- 
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-19 11:40         ` Oleksandr Suvorov
@ 2020-03-19 12:10           ` Uwe Kleine-König
  2020-03-19 12:57             ` Oleksandr Suvorov
  2020-03-19 16:44           ` Thierry Reding
  1 sibling, 1 reply; 41+ messages in thread
From: Uwe Kleine-König @ 2020-03-19 12:10 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: Thierry Reding, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

Hello,

[dropping Tony Prisk <linux@prisktech.co.nz> from recipients]

On Thu, Mar 19, 2020 at 01:40:28PM +0200, Oleksandr Suvorov wrote:
> Thierry, I see the PWM core converts the bit field "third cell" into
> the polarity variable.
> Now I probably understand your sight and agree that we shouldn't give
> the same names to bits in bitfield (dts) and values of a variable.
> 
> But there are lots of useless "0" values of third cell of "pwms"
> option in dts files.
> 
> I see 2 ways now:
> - just remove all "0" "third cell" from "pwms" options in dts files. I
> see this "0" confuses some people.

Then you have to overwrite pwm-cells of the provider. If there are two
PWMs used from the same provider and only one is inverted this won't
work. (Not entirely sure I understood your suggestion.) So I don't like
this suggestion.

And also in my eyes this isn't clearer, just more complicated to use.

> - convert pwm_state.polarity into pwm_state.flags and use bitfield
>   directly from dtb.
>   It simplifies the parsing logic and makes adding new flags easier.

*shrug*, I don't care much.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-19 12:10           ` Uwe Kleine-König
@ 2020-03-19 12:57             ` Oleksandr Suvorov
  0 siblings, 0 replies; 41+ messages in thread
From: Oleksandr Suvorov @ 2020-03-19 12:57 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Thierry Reding, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

On Thu, Mar 19, 2020 at 2:11 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Hello,
>
> [dropping Tony Prisk <linux@prisktech.co.nz> from recipients]
>
> On Thu, Mar 19, 2020 at 01:40:28PM +0200, Oleksandr Suvorov wrote:
> > Thierry, I see the PWM core converts the bit field "third cell" into
> > the polarity variable.
> > Now I probably understand your sight and agree that we shouldn't give
> > the same names to bits in bitfield (dts) and values of a variable.
> >
> > But there are lots of useless "0" values of third cell of "pwms"
> > option in dts files.
> >
> > I see 2 ways now:
> > - just remove all "0" "third cell" from "pwms" options in dts files. I
> > see this "0" confuses some people.
>
> Then you have to overwrite pwm-cells of the provider. If there are two
> PWMs used from the same provider and only one is inverted this won't
> work. (Not entirely sure I understood your suggestion.) So I don't like
> this suggestion.

Good point, agree. But we still have the unnamed "0".

What about renaming the dt-bindings macro PWM_POLARITY_INVERTED
and add the new one for the normal polarity?
Like PWM_FLAG_POLARITY_NORMAL / PWM_FLAG_POLARITY_INVERTED or
DT_PWM_POLARITY_NORMAL / DT_PWM_POLARITY_INVERTED?

Using different prefix will prevent interfering names of enum and
macros in the future
and will allow using the named nop-flag PWM_FLAG_POLARITY_NORMAL in dts.

> And also in my eyes this isn't clearer, just more complicated to use.
>
> > - convert pwm_state.polarity into pwm_state.flags and use bitfield
> >   directly from dtb.
> >   It simplifies the parsing logic and makes adding new flags easier.
>
> *shrug*, I don't care much.
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |



-- 
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-19  6:50         ` Uwe Kleine-König
@ 2020-03-19 16:37           ` Thierry Reding
  2020-03-19 17:30             ` Uwe Kleine-König
  0 siblings, 1 reply; 41+ messages in thread
From: Thierry Reding @ 2020-03-19 16:37 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

[-- Attachment #1: Type: text/plain, Size: 6885 bytes --]

On Thu, Mar 19, 2020 at 07:50:39AM +0100, Uwe Kleine-König wrote:
> 
> [Dropped Tony Prisk from recipients as the address bounces]
> 
> Hello,
> 
> On Wed, Mar 18, 2020 at 11:59:53PM +0100, Thierry Reding wrote:
> > On Tue, Mar 17, 2020 at 10:00:42PM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > On Tue, Mar 17, 2020 at 06:40:43PM +0100, Thierry Reding wrote:
> > > > On Tue, Mar 17, 2020 at 02:32:25PM +0200, Oleksandr Suvorov wrote:
> > > > > The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> > > > > Rename it to PWM_POLARITY_INVERTED.
> > > > 
> > > > It isn't misspelled. "inversed" is a synonym for "inverted". Both
> > > > spellings are correct.
> > > 
> > > Some time ago I stumbled about "inversed", too. My spell checker doesn't
> > > know it and I checked some dictionaries and none of them knew that word:
> > > 
> > > https://www.lexico.com/search?utf8=%E2%9C%93&filter=dictionary&dictionary=en&query=inversed
> > > https://de.pons.com/%C3%BCbersetzung/englisch-deutsch/inversed
> > > https://dictionary.cambridge.org/spellcheck/english-german/?q=inversed
> > > 
> > > https://en.wiktionary.org/wiki/inverse#Verb mentions "inverse" as a verb
> > > having "inversed" as past participle.
> > 
> > Here are the first three results from a Google query:
> > 
> > 	https://www.yourdictionary.com/inversed
> 
> There is something fishy. In the Verb section it says indeed, that it is
> the past participle and simple past of inverse. The entry for inverse
> however only has sections that identify this word as adjective or noun;
> not a verb.
> 
> > 	https://www.dictionary.com/browse/inversed
> 
> Not sure I'd count this as hint that inversed exists. The entry shown to
> me under this URL is about "inverse" and it has
> 
> 	verb (used with object), in·versed, in·vers·ing.
> 		? to invert.
> 
> Does this mean: "Did you mean invert instead?"
> 
> > 	https://en.wiktionary.org/wiki/inversed
> 
> Yeah, that's the one I found, too.
> 
> I still have the impression that "inversed" is in use because people
> don't know better and understand the intended meaning. And this results
> in leaking of this word into the references.
> 
> > > Having said this I think (independent of the question if "inversed"
> > > exists) using two similar terms for the same thing just results in
> > > confusion. I hit that in the past already and I like it being addressed.
> > 
> > I don't know. It's pretty common to use different words for the same
> > thing. They're called synonyms.
> 
> In literature yes, I agree. In a novel it is annoying to repeat the same
> words over and over again and some variation is good. In programming
> however the goal is a different one. There the goal should be to be
> precise and consistent.

We also need to make sure that things don't break. It's a very bad idea
to have a macro with the same name as an enum value for reasons I stated
before. I think that's the most important thing here.

Also, if inversed is a synonym of inverted, we don't loose any precision
at all. All you have to remember is that you're dealing with a device
tree constant in one case and an API enumeration in the other.

So I think the current form is actually more precise, though I guess it
could be confusing if you don't care about the difference.

> > > > And as you noted in the cover letter, there's a conflict between the
> > > > macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
> > > > in the wrong order you'll get a compile error.
> > > 
> > > There are also other symbols that exist twice (GPIO_ACTIVE_HIGH was the
> > > first to come to my mind). I'm not aware of any problems related to
> > > these. What am I missing?
> > 
> > There's currently no problem, obviously. But if for some reason the
> > include files end up being included in a different order (i.e. the
> > dt-bindings header is included before linux/pwm.h) then the macro will
> > be evaluated and result in something like:
> > 
> > 	enum pwm_polarity {
> > 		PWM_POLARITY_NORMAL,
> > 		1,
> > 	};
> > 
> > and that's not valid C, so will cause a build error.
> 
> I admit I didn't look closely here and I assume you are right. If I
> understand Oleksandr right this is only an intermediate step and when
> the series is applied completely this issue is gone. Still it might be
> worth to improve the series here.

	$ gcc -o /dev/null -x c - <<- EOF
	>     #define PWM_POLARITY_INVERTED (1 << 0)
	>
	>     enum pwm_polarity {
	>         PWM_POLARITY_NORMAL,
	>         PWM_POLARITY_INVERTED,
	>     };
	> EOF
	<stdin>:1:35: error: expected identifier before ‘(’ token
	<stdin>:5:9: note: in expansion of macro ‘PWM_POLARITY_INVERTED’

Q.E.D.

> My original question was about similar problems with GPIO_ACTIVE_HIGH.
> Are you aware of problems there?

The problem exists there equally. We're probably not running into it
because drivers don't end up including dt-bindings/gpio/gpio.h and
include/linux/gpio/machine.h at the same time. Or they end up always
including them in the right order.

For PWM the situation is slightly more complicated because we only have
one header for the kernel API, so the likelihood of including it along
with the dt-bindings header is increased compared to GPIO.

> > > > Note that DT bindings are an ABI and can
> > > > never change, whereas the enum pwm_polarity is part of a Linux internal
> > > > API and doesn't have the same restrictions as an ABI.
> > > 
> > > I thought only binary device trees (dtb) are supposed to be ABI.
> > 
> > Yes, the DTB is the ABI. dt-bindings/pwm/pwm.h is used to generate DTBs,
> > which basically makes it ABI as well.
> 
> We disagree here. With this argument you could fix quite some things as
> ABI.

I don't understand what you're trying to say.

> > Yes, the symbol name may not be part of the ABI, but changing the
> > symbol becomes very inconvenient because everyone that depends on it
> > would have to change.
> 
> Oleksandr adapted all in-tree users, so it only affects out-of-tree
> users. In my book this is fine.

There used to be a time when it was assumed that eventually device tree
sources would live outside of the kernel tree. Given that they are a HW
description, they really ought not to be relying on the Linux kernel
tree as a way of keeping them consistent. That's really only out of
convenience.

> > Why bother?
> 
> To make the API more precise and consistent. That's a good goal in my
> eyes.

PWM_POLARITY_INVERTED is not part of the API. It doesn't exist for
anything other than to make the device tree more readable.

I now regret that we ever introduced this in the first place. Perhaps it
would've been better to just deal with raw numbers instead.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-19 11:40         ` Oleksandr Suvorov
  2020-03-19 12:10           ` Uwe Kleine-König
@ 2020-03-19 16:44           ` Thierry Reding
  1 sibling, 0 replies; 41+ messages in thread
From: Thierry Reding @ 2020-03-19 16:44 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: Uwe Kleine-König, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, Tony Prisk, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

[-- Attachment #1: Type: text/plain, Size: 5770 bytes --]

On Thu, Mar 19, 2020 at 01:40:28PM +0200, Oleksandr Suvorov wrote:
> On Thu, Mar 19, 2020 at 1:00 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > On Tue, Mar 17, 2020 at 10:00:42PM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > On Tue, Mar 17, 2020 at 06:40:43PM +0100, Thierry Reding wrote:
> > > > On Tue, Mar 17, 2020 at 02:32:25PM +0200, Oleksandr Suvorov wrote:
> > > > > The polarity enum definition PWM_POLARITY_INVERSED is misspelled.
> > > > > Rename it to PWM_POLARITY_INVERTED.
> > > >
> > > > It isn't misspelled. "inversed" is a synonym for "inverted". Both
> > > > spellings are correct.
> > >
> > > Some time ago I stumbled about "inversed", too. My spell checker doesn't
> > > know it and I checked some dictionaries and none of them knew that word:
> > >
> > > https://www.lexico.com/search?utf8=%E2%9C%93&filter=dictionary&dictionary=en&query=inversed
> > > https://de.pons.com/%C3%BCbersetzung/englisch-deutsch/inversed
> > > https://dictionary.cambridge.org/spellcheck/english-german/?q=inversed
> > >
> > > https://en.wiktionary.org/wiki/inverse#Verb mentions "inverse" as a verb
> > > having "inversed" as past participle.
> >
> > Here are the first three results from a Google query:
> >
> >         https://www.yourdictionary.com/inversed
> >         https://www.dictionary.com/browse/inversed
> >         https://en.wiktionary.org/wiki/inversed
> >
> > > Having said this I think (independent of the question if "inversed"
> > > exists) using two similar terms for the same thing just results in
> > > confusion. I hit that in the past already and I like it being addressed.
> >
> > I don't know. It's pretty common to use different words for the same
> > thing. They're called synonyms.
> >
> > > > And as you noted in the cover letter, there's a conflict between the
> > > > macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
> > > > in the wrong order you'll get a compile error.
> > >
> > > There are also other symbols that exist twice (GPIO_ACTIVE_HIGH was the
> > > first to come to my mind). I'm not aware of any problems related to
> > > these. What am I missing?
> >
> > There's currently no problem, obviously. But if for some reason the
> > include files end up being included in a different order (i.e. the
> > dt-bindings header is included before linux/pwm.h) then the macro will
> > be evaluated and result in something like:
> >
> >         enum pwm_polarity {
> >                 PWM_POLARITY_NORMAL,
> >                 1,
> >         };
> >
> > and that's not valid C, so will cause a build error.
> >
> > > > The enum was named this way on purpose to make it separate from the
> > > > definition for the DT bindings.
> > >
> > > Then please let's make it different by picking a different prefix or
> > > something like that.
> >
> > Again, seems to me like unnecessary churn. Feel free to propose
> > something, but I recall being in the same position at the time and this
> > was the best I could come up with.
> >
> > > > Note that DT bindings are an ABI and can
> > > > never change, whereas the enum pwm_polarity is part of a Linux internal
> > > > API and doesn't have the same restrictions as an ABI.
> > >
> > > I thought only binary device trees (dtb) are supposed to be ABI.
> >
> > Yes, the DTB is the ABI. dt-bindings/pwm/pwm.h is used to generate DTBs,
> > which basically makes it ABI as well. Yes, the symbol name may not be
> > part of the ABI, but changing the symbol becomes very inconvenient
> > because everyone that depends on it would have to change. Why bother?
> >
> > My point is that enum pwm_polarity is an API in the kernel and hence its
> > easy to change or extend. But since that is not the same for the DTB, we
> > need to be careful what from the internal kernel API leaks into the DTB.
> > That's why they are different symbols, so that it is clear that what's
> > in dt-bindings/pwm/pwm.h is the ABI.
> 
> Thierry, I see the PWM core converts the bit field "third cell" into
> the polarity variable.

Yes. And if there were other fields in that third cell, there'd be other
variables that would be derived from those.

> Now I probably understand your sight and agree that we shouldn't give
> the same names to bits in bitfield (dts) and values of a variable.
> 
> But there are lots of useless "0" values of third cell of "pwms"
> option in dts files.

These aren't useless 0 values. 0 doesn't mean "don't care", it's a very
specific value. In this case it implies that the polarity is "normal".
That's very useful if you have a PWM controller that can set the
polarity.

> I see 2 ways now:
> - just remove all "0" "third cell" from "pwms" options in dts files. I
> see this "0" confuses some people.

Some drivers already do this by supporting 2-cell and 3-cell specifiers.
Not everyone does this and they shouldn't need to. The bindings are very
clear about what to do with that third cell and how to parse it. It's
just a matter of reading and understanding the bindings. It's not
exactly rocket science, so why do we have to jump through hoops to try
and simplify it?

> - convert pwm_state.polarity into pwm_state.flags and use bitfield
> directly from dtb.
>   It simplifies the parsing logic and makes adding new flags easier.

And then you have to go and and parsing code everywhere to deal with
these new flags. I prefer to have this parsing code in the core where
it's written once and used everywhere, instead of letting drivers deal
with flags that they might interpret wrongly.

Also remember that the third cell may be extended in the future and not
all data in in may end up being a simple flag.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag
  2020-03-19  7:05         ` Uwe Kleine-König
@ 2020-03-19 17:04           ` Thierry Reding
  2020-03-30 21:00             ` Rob Herring
  0 siblings, 1 reply; 41+ messages in thread
From: Thierry Reding @ 2020-03-19 17:04 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Rob Herring, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4026 bytes --]

On Thu, Mar 19, 2020 at 08:05:10AM +0100, Uwe Kleine-König wrote:
> On Thu, Mar 19, 2020 at 12:05:39AM +0100, Thierry Reding wrote:
> > On Tue, Mar 17, 2020 at 10:30:56PM +0100, Uwe Kleine-König wrote:
> > > Hello Thierry,
> > > 
> > > On Tue, Mar 17, 2020 at 06:43:44PM +0100, Thierry Reding wrote:
> > > > On Tue, Mar 17, 2020 at 02:32:26PM +0200, Oleksandr Suvorov wrote:
> > > > > Add the description of PWM_POLARITY_NORMAL flag.
> > > > > 
> > > > > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > > > > ---
> > > > > 
> > > > >  Documentation/devicetree/bindings/pwm/pwm.txt | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > > > index 084886bd721e..440c6b9a6a4e 100644
> > > > > --- a/Documentation/devicetree/bindings/pwm/pwm.txt
> > > > > +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > > > @@ -46,6 +46,7 @@ period in nanoseconds.
> > > > >  Optionally, the pwm-specifier can encode a number of flags (defined in
> > > > >  <dt-bindings/pwm/pwm.h>) in a third cell:
> > > > >  - PWM_POLARITY_INVERTED: invert the PWM signal polarity
> > > > > +- PWM_POLARITY_NORMAL: don't invert the PWM signal polarity
> > > > 
> > > > This doesn't make sense. PWM_POLARITY_NORMAL is not part of the DT ABI.
> > > 
> > > "is not part of the DT ABI" is hardly a good reason. If it's sensible to
> > > be used, it is sensible to define it.
> > 
> > That's exactly it. It's not sensible at all to use it.
> 
> If you think the argument is "It is not sensible to be used." then please
> say so and don't add "PWM_POLARITY_NORMAL is not part of the DT ABI."
> which seems to be irrelevant now.

I did say that, didn't I? I said that it doesn't make sense because it
isn't part of the ABI. And since this is the document that defines the
DT ABI, why should we add something that isn't part of the ABI?

Now, if you want to make this part of the ABI, then that should be done
as part of this patch so that the patch is actually consistent.

> > If you define it here it means people are allowed to do stuff like
> > this:
> > 
> > 	pwms = <&pwm 1234 PWM_POLARITY_INVERTED | PWM_POLARITY_NORMAL>;
> > 
> > which doesn't make sense.
> 
> I would hope that a human reader would catch this.

Maybe. At the same time we're now moving towards automatic checking of
device trees against a binding. These tools will only ever see the
binary representation, so won't be able to spot this nonsense. The whole
purpose of having these tools is so that we don't have to do the tedious
work of manually inspecting all device tree files. It's also not
guaranteed that we'll even be seeing all of the device tree files ever
written against these bindings.

> 
> > What's more, it impossible for the code to even notice that you're
> > being silly because | PWM_POLARITY_NORMAL is just | 0 and that's just
> > a nop.
> 
> I think this argument is a bad one. Whenever you introduce a
> function or symbol you can use it in a wrong way. With this argument you
> can also say GPIO_ACTIVE_LOW doesn't make sense because
> 
> 	pwms = <&pwm 1234 GPIO_ACTIVE_LOW>;
> 
> is silly.

Yes, it's also obviously silly to try and eat a hammer. I was assuming
people have at least /some/ sense and try not to use GPIO related flags
with PWM specifiers. And because I think that people aren't totally
stupid, I think they'll be capable of understanding that by default a
PWM will be "normal" and only if they want to deviate from "normal" do
they need to do something special, like specify PWM_POLARITY_INVERTED.

I'm growing tired of this discussion, to be honest. So if you really
absolutely must have PWM_POLARITY_NORMAL so that you can read DT files
without having to think, then fine, I'll take a patch that adds that.
But please let's not confuse the definitions for DT with the polarity
enum in the API.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum
  2020-03-19 16:37           ` Thierry Reding
@ 2020-03-19 17:30             ` Uwe Kleine-König
  0 siblings, 0 replies; 41+ messages in thread
From: Uwe Kleine-König @ 2020-03-19 17:30 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Oleksandr Suvorov, devicetree, linux-pwm, Paul Barker,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Chen-Yu Tsai,
	Claudiu Beznea, Fabio Estevam, Florian Fainelli, Heiko Stuebner,
	Kevin Hilman, Ludovic Desroches, Maxime Ripard, NXP Linux Team,
	Nicolas Ferre, Palmer Dabbelt, Paul Cercueil, Paul Walmsley,
	Pengutronix Kernel Team, Ray Jui, Sascha Hauer, Scott Branden,
	Shawn Guo, bcm-kernel-feedback-list, linux-amlogic,
	linux-arm-kernel, linux-kernel, linux-riscv, linux-rockchip

Hello,

On Thu, Mar 19, 2020 at 05:37:00PM +0100, Thierry Reding wrote:
> On Thu, Mar 19, 2020 at 07:50:39AM +0100, Uwe Kleine-König wrote:
> > On Wed, Mar 18, 2020 at 11:59:53PM +0100, Thierry Reding wrote:
> > > On Tue, Mar 17, 2020 at 10:00:42PM +0100, Uwe Kleine-König wrote:
> > > > Having said this I think (independent of the question if "inversed"
> > > > exists) using two similar terms for the same thing just results in
> > > > confusion. I hit that in the past already and I like it being addressed.
> > > 
> > > I don't know. It's pretty common to use different words for the same
> > > thing. They're called synonyms.
> > 
> > In literature yes, I agree. In a novel it is annoying to repeat the same
> > words over and over again and some variation is good. In programming
> > however the goal is a different one. There the goal should be to be
> > precise and consistent.
> 
> We also need to make sure that things don't break.

And I'm entirely on your side here.

> It's a very bad idea to have a macro with the same name as an enum
> value for reasons I stated before. I think that's the most important
> thing here.

You might have missed it, but that's OK for me, too. And note that after
applying the whole series the enum is gone and so the problem. (First
hunk of include/linux/pwm.h in patch 5.)

> Also, if inversed is a synonym of inverted, we don't loose any precision
> at all.

grep doesn't know about synonyms, so if I grep for stuff about inverted
PWMs in the kernel I completely miss one half as it's called inversed
there. (Yeah sure, I can also grep for "inversed|inverted", but therefor
I have to know first that both are used interchangable here.)

That's a bit like a schematic that has "RESET#" in one place and
"nRESET" in an other. If you stumble about that you wonder if they are
two different names for the same signal or if they are actually two
different ones.

Have you ever read a specification that described some property, gave it
a name and then later used a synonym to describe it? In my eyes that's a
bad idea.

> All you have to remember is that you're dealing with a device
> tree constant in one case and an API enumeration in the other.

Everything you need to remember (or learn) about a subsystem makes it
harder work with it.
 
> So I think the current form is actually more precise, though I guess it
> could be confusing if you don't care about the difference.

If there is a technical need to have different names that's one thing.
But using synonyms to differentiate them is not optimal. Then please
let's have names where looking at the identifier makes it obvious which
is for the device trees and which for the API enum.

> > > > > And as you noted in the cover letter, there's a conflict between the
> > > > > macro defined in dt-bindings/pwm/pwm.txt. If they end up being included
> > > > > in the wrong order you'll get a compile error.
> > > > 
> > > > There are also other symbols that exist twice (GPIO_ACTIVE_HIGH was the
> > > > first to come to my mind). I'm not aware of any problems related to
> > > > these. What am I missing?
> > > 
> > > There's currently no problem, obviously. But if for some reason the
> > > include files end up being included in a different order (i.e. the
> > > dt-bindings header is included before linux/pwm.h) then the macro will
> > > be evaluated and result in something like:
> > > 
> > > 	enum pwm_polarity {
> > > 		PWM_POLARITY_NORMAL,
> > > 		1,
> > > 	};
> > > 
> > > and that's not valid C, so will cause a build error.
> > 
> > I admit I didn't look closely here and I assume you are right. If I
> > understand Oleksandr right this is only an intermediate step and when
> > the series is applied completely this issue is gone. Still it might be
> > worth to improve the series here.
> 
> 	$ gcc -o /dev/null -x c - <<- EOF
> 	>     #define PWM_POLARITY_INVERTED (1 << 0)
> 	>
> 	>     enum pwm_polarity {
> 	>         PWM_POLARITY_NORMAL,
> 	>         PWM_POLARITY_INVERTED,
> 	>     };
> 	> EOF
> 	<stdin>:1:35: error: expected identifier before ‘(’ token
> 	<stdin>:5:9: note: in expansion of macro ‘PWM_POLARITY_INVERTED’
> 
> Q.E.D.

I don't understand why you proved something here. I didn't doubt this.

> > My original question was about similar problems with GPIO_ACTIVE_HIGH.
> > Are you aware of problems there?
> 
> The problem exists there equally. We're probably not running into it
> because drivers don't end up including dt-bindings/gpio/gpio.h and
> include/linux/gpio/machine.h at the same time. Or they end up always
> including them in the right order.

Oh, that's worse than I expected. There are two .c files that include
dt-bindings/gpio/gpio.h:

	drivers/rtc/rtc-omap.c
	drivers/tty/serial/omap-serial.c

So the definition isn't even used in the gpio core to parse dt-stuff.
(And both files don't use any definition of that file :-|)

> For PWM the situation is slightly more complicated because we only have
> one header for the kernel API, so the likelihood of including it along
> with the dt-bindings header is increased compared to GPIO.

If a consumer or provider includes the dt-bindings file there is
something fishy. (Still catching this with a compiler message better
than "expected identifier before ‘(’ token" would be good.)
 
> > > > > Note that DT bindings are an ABI and can
> > > > > never change, whereas the enum pwm_polarity is part of a Linux internal
> > > > > API and doesn't have the same restrictions as an ABI.
> > > > 
> > > > I thought only binary device trees (dtb) are supposed to be ABI.
> > > 
> > > Yes, the DTB is the ABI. dt-bindings/pwm/pwm.h is used to generate DTBs,
> > > which basically makes it ABI as well.
> > 
> > We disagree here. With this argument you could fix quite some things as
> > ABI.
> 
> I don't understand what you're trying to say.

I don't want to follow your argument that dt-bindings/pwm/pwm.h is ABI
as well. device tree binaries follow an ABI (similar to machine code),
but the compiler and the source code (including headers) are not.

> > > Yes, the symbol name may not be part of the ABI, but changing the
> > > symbol becomes very inconvenient because everyone that depends on it
> > > would have to change.
> > 
> > Oleksandr adapted all in-tree users, so it only affects out-of-tree
> > users. In my book this is fine.
> 
> There used to be a time when it was assumed that eventually device tree
> sources would live outside of the kernel tree. Given that they are a HW
> description, they really ought not to be relying on the Linux kernel
> tree as a way of keeping them consistent. That's really only out of
> convenience.

The other way round however is fine, isn't it? So use the dt definition
in the kernel should be ok.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

* Re: [RFC PATCH 7/7] arm: dts: pwm: replace polarity constant with macro
  2020-03-17 12:32 ` [RFC PATCH 7/7] arm: " Oleksandr Suvorov
@ 2020-03-20 10:02   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2020-03-20 10:02 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Alexandre Belloni, Bartosz Golaszewski,
	Benoît Cousson, Chen-Yu Tsai, David Lechner, Fabio Estevam,
	Geert Uytterhoeven, Heiko Stuebner, Jisheng Zhang, Kevin Hilman,
	Kukjin Kim, Ludovic Desroches, Magnus Damm, Maxime Ripard,
	NXP Linux Team, Nicolas Ferre, Pengutronix Kernel Team,
	Peter Rosin, Rob Herring, Sascha Hauer, Sebastian Hesselbarth,
	Sekhar Nori, Shawn Guo, Stefan Agner, Tony Lindgren,
	linux-amlogic, linux-arm-kernel, linux-kernel, linux-omap,
	linux-renesas-soc, linux-rockchip, linux-samsung-soc

On Tue, Mar 17, 2020 at 02:32:31PM +0200, Oleksandr Suvorov wrote:
> There is the PWM_POLARITY_NORMAL defined and describled in
> <dt-bindings/pwm/pwm.h> and used by pwm drivers.
> 
> This patch converts all '0' constant in pwms parameters into
> PWM_POLARITY_NORMAL.
> 
> Replace with sed regexp:
> 's/(pwms = <&[a-zA-Z_0-9]+ [0-9]+ [0-9]+) 0>/\1 PWM_POLARITY_NORMAL>/'
> 
> Then:
> - included pwm.h in some dts/dtsi to fix building errors about undefined
>   symbols.
> - fixed the patman warnings about the code format;
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> ---
> 
>  arch/arm/boot/dts/am335x-cm-t335.dts               | 2 +-
>  arch/arm/boot/dts/am335x-evm.dts                   | 2 +-
>  arch/arm/boot/dts/am3517-evm.dts                   | 2 +-
>  arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi      | 2 +-
>  arch/arm/boot/dts/at91-kizbox2-common.dtsi         | 6 +++---
>  arch/arm/boot/dts/at91-kizbox3_common.dtsi         | 8 ++++----
>  arch/arm/boot/dts/at91-kizboxmini-common.dtsi      | 6 +++---
>  arch/arm/boot/dts/at91-nattis-2-natte-2.dts        | 2 +-
>  arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts        | 2 +-
>  arch/arm/boot/dts/at91sam9n12ek.dts                | 2 +-
>  arch/arm/boot/dts/at91sam9x5dm.dtsi                | 2 +-
>  arch/arm/boot/dts/berlin2cd-google-chromecast.dts  | 4 ++--
>  arch/arm/boot/dts/da850-evm.dts                    | 2 +-
>  arch/arm/boot/dts/da850-lego-ev3.dts               | 4 ++--
>  arch/arm/boot/dts/exynos4412-midas.dtsi            | 2 +-
>  arch/arm/boot/dts/exynos4412-odroidu3.dts          | 2 +-
>  arch/arm/boot/dts/exynos5250-snow-common.dtsi      | 2 +-
>  arch/arm/boot/dts/exynos5410-odroidxu.dts          | 2 +-
>  arch/arm/boot/dts/exynos5420-peach-pit.dts         | 2 +-
>  arch/arm/boot/dts/exynos5422-odroidhc1.dts         | 2 +-
>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 2 +-
>  arch/arm/boot/dts/exynos5422-odroidxu4.dts         | 2 +-
>  arch/arm/boot/dts/exynos54xx-odroidxu-leds.dtsi    | 4 ++--
>  arch/arm/boot/dts/exynos5800-peach-pi.dts          | 2 +-
>  arch/arm/boot/dts/imx53-tx53-x13x.dts              | 5 +++--
>  arch/arm/boot/dts/imx6dl-tx6dl-comtft.dts          | 2 +-
>  arch/arm/boot/dts/imx6q-display5.dtsi              | 2 +-
>  arch/arm/boot/dts/imx6q-tx6q-1010-comtft.dts       | 2 +-
>  arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts       | 2 +-
>  arch/arm/boot/dts/imx6qdl-tx6-lvds.dtsi            | 4 ++--
>  arch/arm/boot/dts/imx7-colibri.dtsi                | 4 +++-
>  arch/arm/boot/dts/imx7d-nitrogen7.dts              | 3 ++-
>  arch/arm/boot/dts/imx7d-pico.dtsi                  | 3 ++-
>  arch/arm/boot/dts/imx7d-sdb.dts                    | 3 ++-
>  arch/arm/boot/dts/imx7ulp-evk.dts                  | 3 ++-
>  arch/arm/boot/dts/iwg20d-q7-common.dtsi            | 2 +-
>  arch/arm/boot/dts/logicpd-torpedo-baseboard.dtsi   | 2 +-
>  arch/arm/boot/dts/meson8b-ec100.dts                | 4 ++--
>  arch/arm/boot/dts/meson8b-mxq.dts                  | 4 ++--
>  arch/arm/boot/dts/meson8b-odroidc1.dts             | 4 ++--
>  arch/arm/boot/dts/motorola-mapphone-common.dtsi    | 3 ++-
>  arch/arm/boot/dts/omap3-gta04.dtsi                 | 2 +-
>  arch/arm/boot/dts/omap3-n900.dts                   | 2 +-
>  arch/arm/boot/dts/rk3288-veyron-edp.dtsi           | 2 +-
>  arch/arm/boot/dts/rk3288-veyron.dtsi               | 2 +-
>  arch/arm/boot/dts/rv1108-evb.dts                   | 2 +-
>  arch/arm/boot/dts/s3c6410-mini6410.dts             | 2 +-
>  arch/arm/boot/dts/s5pv210-aries.dtsi               | 2 +-
>  arch/arm/boot/dts/s5pv210-smdkv210.dts             | 2 +-

For Exynos/S3C/S5P:
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

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

* Re: [RFC PATCH 6/7] arm64: dts: pwm: replace polarity constant with macro
  2020-03-17 12:32 ` [RFC PATCH 6/7] arm64: dts: pwm: replace polarity constant with macro Oleksandr Suvorov
@ 2020-03-20 10:03   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2020-03-20 10:03 UTC (permalink / raw)
  To: Oleksandr Suvorov
  Cc: devicetree, linux-pwm, Paul Barker, Uwe Kleine-König,
	Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, Andrius Štikonas, Andy Yan, Chen-Yu Tsai,
	Christoph Muellner, Daniel Lezcano, Enric Balletbo i Serra,
	Heiko Stuebner, Hugh Cole-Baker, Jagan Teki, Johan Jonker,
	Katsuhiro Suzuki, Kever Yang, Kevin Hilman, Kukjin Kim,
	Marc Zyngier, Markus Reichl, Maxime Ripard, Miquel Raynal,
	Nick Xie, Philipp Tomsich, Rob Herring, Robin Murphy,
	Soeren Moch, linux-amlogic, linux-arm-kernel, linux-kernel,
	linux-rockchip, linux-samsung-soc

On Tue, Mar 17, 2020 at 02:32:30PM +0200, Oleksandr Suvorov wrote:
> There is the PWM_POLARITY_NORMAL defined and describled in
> <dt-bindings/pwm/pwm.h> and used by pwm drivers.
> 
> This patch converts all '0' constant in pwms parameters into
> PWM_POLARITY_NORMAL.
> 
> Replace with sed regexp:
> 's/(pwms = <&[a-zA-Z_0-9]+ [0-9]+ [0-9]+) 0>/\1 PWM_POLARITY_NORMAL>/'
> 
> Then:
> - include pwm.h in some dts/dtsi to fix building errors about undefined
>   symbols.
> - fix the patman warnings about the code format;
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> ---
> 
>  arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts      | 2 +-
>  arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts       | 2 +-
>  arch/arm64/boot/dts/amlogic/meson-axg-s400.dts             | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi                 | 1 +
>  arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi          | 1 +
>  arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts          | 5 +++--
>  arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts            | 2 +-
>  arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts         | 5 +++--
>  arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi    | 4 ++--
>  arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts       | 4 ++--
>  arch/arm64/boot/dts/amlogic/meson-g12b-ugoos-am6.dts       | 7 ++++---
>  arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi        | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi                  | 1 +
>  arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts       | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts     | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi           | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi       | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi          | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts       | 3 ++-
>  .../boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts    | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts | 2 +-
>  .../arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi      | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts      | 5 +++--
>  arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts         | 3 ++-
>  arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi         | 4 +++-
>  arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts     | 2 +-
>  arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts           | 7 ++++---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi      | 3 ++-

For Exynos:
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

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

* Re: [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag
  2020-03-19 17:04           ` Thierry Reding
@ 2020-03-30 21:00             ` Rob Herring
  0 siblings, 0 replies; 41+ messages in thread
From: Rob Herring @ 2020-03-30 21:00 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Uwe Kleine-König, Oleksandr Suvorov, devicetree, linux-pwm,
	Paul Barker, Laurent Pinchart, Marcel Ziswiler, Igor Opaniuk,
	Philippe Schenker, linux-kernel

On Thu, Mar 19, 2020 at 06:04:55PM +0100, Thierry Reding wrote:
> On Thu, Mar 19, 2020 at 08:05:10AM +0100, Uwe Kleine-König wrote:
> > On Thu, Mar 19, 2020 at 12:05:39AM +0100, Thierry Reding wrote:
> > > On Tue, Mar 17, 2020 at 10:30:56PM +0100, Uwe Kleine-König wrote:
> > > > Hello Thierry,
> > > > 
> > > > On Tue, Mar 17, 2020 at 06:43:44PM +0100, Thierry Reding wrote:
> > > > > On Tue, Mar 17, 2020 at 02:32:26PM +0200, Oleksandr Suvorov wrote:
> > > > > > Add the description of PWM_POLARITY_NORMAL flag.
> > > > > > 
> > > > > > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > > > > > ---
> > > > > > 
> > > > > >  Documentation/devicetree/bindings/pwm/pwm.txt | 1 +
> > > > > >  1 file changed, 1 insertion(+)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > > > > index 084886bd721e..440c6b9a6a4e 100644
> > > > > > --- a/Documentation/devicetree/bindings/pwm/pwm.txt
> > > > > > +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
> > > > > > @@ -46,6 +46,7 @@ period in nanoseconds.
> > > > > >  Optionally, the pwm-specifier can encode a number of flags (defined in
> > > > > >  <dt-bindings/pwm/pwm.h>) in a third cell:
> > > > > >  - PWM_POLARITY_INVERTED: invert the PWM signal polarity
> > > > > > +- PWM_POLARITY_NORMAL: don't invert the PWM signal polarity
> > > > > 
> > > > > This doesn't make sense. PWM_POLARITY_NORMAL is not part of the DT ABI.
> > > > 
> > > > "is not part of the DT ABI" is hardly a good reason. If it's sensible to
> > > > be used, it is sensible to define it.
> > > 
> > > That's exactly it. It's not sensible at all to use it.
> > 
> > If you think the argument is "It is not sensible to be used." then please
> > say so and don't add "PWM_POLARITY_NORMAL is not part of the DT ABI."
> > which seems to be irrelevant now.
> 
> I did say that, didn't I? I said that it doesn't make sense because it
> isn't part of the ABI. And since this is the document that defines the
> DT ABI, why should we add something that isn't part of the ABI?
> 
> Now, if you want to make this part of the ABI, then that should be done
> as part of this patch so that the patch is actually consistent.
> 
> > > If you define it here it means people are allowed to do stuff like
> > > this:
> > > 
> > > 	pwms = <&pwm 1234 PWM_POLARITY_INVERTED | PWM_POLARITY_NORMAL>;
> > > 
> > > which doesn't make sense.
> > 
> > I would hope that a human reader would catch this.
> 
> Maybe. At the same time we're now moving towards automatic checking of
> device trees against a binding. These tools will only ever see the
> binary representation, so won't be able to spot this nonsense. The whole
> purpose of having these tools is so that we don't have to do the tedious
> work of manually inspecting all device tree files. It's also not
> guaranteed that we'll even be seeing all of the device tree files ever
> written against these bindings.
> 
> > 
> > > What's more, it impossible for the code to even notice that you're
> > > being silly because | PWM_POLARITY_NORMAL is just | 0 and that's just
> > > a nop.
> > 
> > I think this argument is a bad one. Whenever you introduce a
> > function or symbol you can use it in a wrong way. With this argument you
> > can also say GPIO_ACTIVE_LOW doesn't make sense because
> > 
> > 	pwms = <&pwm 1234 GPIO_ACTIVE_LOW>;
> > 
> > is silly.
> 
> Yes, it's also obviously silly to try and eat a hammer. I was assuming
> people have at least /some/ sense and try not to use GPIO related flags
> with PWM specifiers. And because I think that people aren't totally
> stupid, I think they'll be capable of understanding that by default a
> PWM will be "normal" and only if they want to deviate from "normal" do
> they need to do something special, like specify PWM_POLARITY_INVERTED.
> 
> I'm growing tired of this discussion, to be honest. So if you really
> absolutely must have PWM_POLARITY_NORMAL so that you can read DT files
> without having to think, then fine, I'll take a patch that adds that.
> But please let's not confuse the definitions for DT with the polarity
> enum in the API.

I agree with Thierry here. I'd give my reasons, but I've got 200 other 
patches to review.

Rob

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

end of thread, other threads:[~2020-03-30 21:00 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200317123231.2843297-1-oleksandr.suvorov@toradex.com>
2020-03-17 12:32 ` [RFC PATCH 1/7] pwm: rename the PWM_POLARITY_INVERSED enum Oleksandr Suvorov
2020-03-17 13:34   ` Paul Barker
2020-03-17 21:32     ` Uwe Kleine-König
2020-03-17 16:26   ` Claudiu.Beznea
2020-03-17 16:39     ` Oleksandr Suvorov
2020-03-17 17:40   ` Thierry Reding
2020-03-17 21:00     ` Uwe Kleine-König
2020-03-18 22:59       ` Thierry Reding
2020-03-19  6:50         ` Uwe Kleine-König
2020-03-19 16:37           ` Thierry Reding
2020-03-19 17:30             ` Uwe Kleine-König
2020-03-19 11:40         ` Oleksandr Suvorov
2020-03-19 12:10           ` Uwe Kleine-König
2020-03-19 12:57             ` Oleksandr Suvorov
2020-03-19 16:44           ` Thierry Reding
2020-03-18 11:47     ` Oleksandr Suvorov
2020-03-17 12:32 ` [RFC PATCH 2/7] dt-bindings: pwm: document the PWM polarity flag Oleksandr Suvorov
2020-03-17 17:43   ` Thierry Reding
2020-03-17 21:30     ` Uwe Kleine-König
2020-03-18 23:05       ` Thierry Reding
2020-03-19  7:05         ` Uwe Kleine-König
2020-03-19 17:04           ` Thierry Reding
2020-03-30 21:00             ` Rob Herring
2020-03-18 23:19       ` Thierry Reding
2020-03-17 22:58   ` Laurent Pinchart
2020-03-17 12:32 ` [RFC PATCH 3/7] dt-bindings: pwm: add normal " Oleksandr Suvorov
2020-03-17 13:36   ` Paul Barker
2020-03-17 14:06     ` Oleksandr Suvorov
2020-03-17 21:36   ` Uwe Kleine-König
2020-03-17 22:56   ` Laurent Pinchart
2020-03-18  9:20     ` Uwe Kleine-König
2020-03-17 12:32 ` [RFC PATCH 4/7] dt-bindings: pwm: add description of PWM polarity Oleksandr Suvorov
2020-03-17 23:01   ` Laurent Pinchart
2020-03-18 11:37     ` Oleksandr Suvorov
2020-03-18 12:29       ` Laurent Pinchart
2020-03-18 12:36         ` Oleksandr Suvorov
2020-03-17 12:32 ` [RFC PATCH 5/7] pwm: replace polarity enum with macros Oleksandr Suvorov
2020-03-17 12:32 ` [RFC PATCH 6/7] arm64: dts: pwm: replace polarity constant with macro Oleksandr Suvorov
2020-03-20 10:03   ` Krzysztof Kozlowski
2020-03-17 12:32 ` [RFC PATCH 7/7] arm: " Oleksandr Suvorov
2020-03-20 10:02   ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).