linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Cercueil <paul@crapouillou.net>
To: "Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: od@zcrc.me, linux-pwm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Paul Cercueil <paul@crapouillou.net>
Subject: [PATCH v3 3/4] pwm: jz4740: Make PWM start with the active part
Date: Wed, 27 May 2020 13:52:24 +0200	[thread overview]
Message-ID: <20200527115225.10069-3-paul@crapouillou.net> (raw)
In-Reply-To: <20200527115225.10069-1-paul@crapouillou.net>

The PWM in Ingenic SoCs starts in inactive state until the internal
timer reaches the duty value, then becomes active until the timer
reaches the period value. In theory, we should then use (period - duty)
as the real duty value, as a high duty value would otherwise result in
the PWM pin being inactive most of the time.

This is the reason why the duty value was inverted in the driver until
now, but it still had the problem that it would not start with the
active part.

To address this remaining issue, the common trick is to invert the
duty, and invert the polarity when the PWM is enabled.

Since the duty was already inverted, and we invert it again, we now
program the hardware for the requested duty, and simply invert the
polarity when the PWM is enabled.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v2: Add documentation about why we invert the polarity, and
    	improve commit message
    v3: No change

 drivers/pwm/pwm-jz4740.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index 4fe9d99ac9a9..fe06ca8ce30f 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -6,7 +6,6 @@
  * Limitations:
  * - The .apply callback doesn't complete the currently running period before
  *   reconfiguring the hardware.
- * - Each period starts with the inactive part.
  */
 
 #include <linux/clk.h>
@@ -163,7 +162,7 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	/* Calculate duty value */
 	tmp = (unsigned long long)rate * state->duty_cycle;
 	do_div(tmp, NSEC_PER_SEC);
-	duty = period - tmp;
+	duty = tmp;
 
 	if (duty >= period)
 		duty = period - 1;
@@ -189,18 +188,26 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
 			   TCU_TCSR_PWM_SD, TCU_TCSR_PWM_SD);
 
-	/* Set polarity */
-	switch (state->polarity) {
-	case PWM_POLARITY_NORMAL:
+	/*
+	 * Set polarity.
+	 *
+	 * The PWM starts in inactive state until the internal timer reaches the
+	 * duty value, then becomes active until the timer reaches the period
+	 * value. In theory, we should then use (period - duty) as the real duty
+	 * value, as a high duty value would otherwise result in the PWM pin
+	 * being inactive most of the time.
+	 *
+	 * Here, we don't do that, and instead invert the polarity of the PWM
+	 * when it is active. This trick makes the PWM start with its active
+	 * state instead of its inactive state.
+	 */
+	if ((state->polarity == PWM_POLARITY_NORMAL) ^ state->enabled)
 		regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
 				   TCU_TCSR_PWM_INITL_HIGH, 0);
-		break;
-	case PWM_POLARITY_INVERSED:
+	else
 		regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
 				   TCU_TCSR_PWM_INITL_HIGH,
 				   TCU_TCSR_PWM_INITL_HIGH);
-		break;
-	}
 
 	if (state->enabled)
 		jz4740_pwm_enable(chip, pwm);
-- 
2.26.2


  parent reply	other threads:[~2020-05-27 11:53 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-27 11:52 [PATCH v3 1/4] pwm: jz4740: Drop dependency on MACH_INGENIC Paul Cercueil
2020-05-27 11:52 ` [PATCH v3 2/4] pwm: jz4740: Enhance precision in calculation of duty cycle Paul Cercueil
2020-05-27 11:52 ` Paul Cercueil [this message]
2020-05-27 11:52 ` [PATCH v3 4/4] pwm: jz4740: Add support for the JZ4725B Paul Cercueil
2020-06-02 12:41 ` [PATCH v3 1/4] pwm: jz4740: Drop dependency on MACH_INGENIC Thierry Reding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200527115225.10069-3-paul@crapouillou.net \
    --to=paul@crapouillou.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=od@zcrc.me \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).