mbox series

[RFC,0/4] pwm: sunxi: Add support Allwinner D1 PWM

Message ID 20240518-pwm_d1-v1-0-311fc5fe2248@jookia.org
Headers show
Series pwm: sunxi: Add support Allwinner D1 PWM | expand

Message

John Watts May 18, 2024, 3:54 a.m. UTC
This patch series adds support for the Allwinner D1, T113 and R329 PWM.

This code isn't based on any kernel code but instead written from scratch with
the goal of handling the PWM pairs deterministically.

I've tested this on T113 hardware and it works very well.

Signed-off-by: John Watts <contact@jookia.org>
---
John Watts (4):
      clk: sunxi: Add PWM bus clock and reset
      pinctrl: sunxi: Add PWM7 pinctrl for the D1
      pwm: sunxi: Add support Allwinner D1 PWM
      [FOR TESTING ONLY] sunxi-d1s-t113: Add D1 and T113 PWM node

 arch/riscv/dts/sunxi-d1s-t113.dtsi                 |  12 +
 drivers/clk/sunxi/clk_d1.c                         |   4 +
 drivers/pinctrl/sunxi/pinctrl-sunxi.c              |   1 +
 drivers/pwm/Kconfig                                |   6 +
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/sunxi_pwm_d1.c                         | 542 +++++++++++++++++++++
 .../src/riscv/allwinner/sunxi-d1s-t113.dtsi        |  12 +
 7 files changed, 578 insertions(+)
---
base-commit: 312efdf9c9297382b4e1d04c50376573764b5c00
change-id: 20240518-pwm_d1-b43d263ea143

Best regards,

Comments

John Watts May 20, 2024, 6:10 a.m. UTC | #1
After a long time reading the datasheet I found out my approach was
actually wrong and led to an off by on error.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/pwm/sunxi_pwm_d1.c | 57 +++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/pwm/sunxi_pwm_d1.c b/drivers/pwm/sunxi_pwm_d1.c
index 6c57bc6e85..f396afff3e 100644
--- a/drivers/pwm/sunxi_pwm_d1.c
+++ b/drivers/pwm/sunxi_pwm_d1.c
@@ -1,34 +1,41 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright 2022 Jookia <contact@jookia.org>
 /*
- * The Allwinner D1's PWM channels are 16-bit counters with up to
- * 65537 cycles (yes, you read that correctly).
+ * The Allwinner D1's PWM channels are paired 16-bit counters.
  *
  * Each channel must be programmed using three variables:
  * - The entire cycle count (used for the period)
- * - The active cycle count (the count of inactive cycles)
- * - The polarity (specifies if the signal is active high or low)
- * The cycle counts are at minimum 1 and at maximum 65536.
+ * - The active cycle count (used for the duty cycle)
+ * - The active state polarity (specifies if the signal goes high or low)
  *
- * The controller will output the number of entire cycles plus one
- * extra, with any cycles after the active cycle output as active.
+ * All counts are zero based, but the datasheet spends a lot of time
+ * adding 1 to the entire cycle count. There's no hidden extra cycle,
+ * it's just trying to make it human understandable. After all, you can
+ * have 0 active counts for a 100% duty cycle, but 0 entire cycles
+ * doesn't really mean sense or work in time calculations.
  *
- * Consider a PWM period of 128 nanoseconds and a cycle period of 32.
- * Setting the entire cycle count to 3 and active cycle count to 4
- * gives an output like so:
+ * The counter works like this (quoting the datasheet):
+ * - PCNTR = (PCNTR == PWM_ENTIRE_CYCLE) ? 0 : PCNTR + 1
+ * - PCNTR > (PWM_ENTIRE_CYCLE - PWM_ACT_CYCLE) = Output active state
+ * - PCNTR <= (PWM_ENTIRE_CYCLE - PWM_ACT_CYCLE) = Output inactive state
  *
- * - Cycle 1 runs 0 to 32 nanoseconds, inactive
- * - Cycle 2 runs 32 to 64 nanoseconds, inactive
- * - Cycle 3 runs 64 to 96 nanoseconds, inactive
- * - Cycle 4 runs 96 to 128 nanoseconds, inactive
- * - Cycle 5 is skipped but would run 128 to 160 nanoseconds, active
+ * Here's a 2-bit table of cycle counts versus active cycle counts:
+ *   Active:  |       0 |        1  |        2  |        3 |
+ *   Count 0  | Active  | Inactive  | Inactive  | Inactive |
+ *   Count 1  | Active  | Active    | Inactive  | Inactive |
+ *   Count 2  | Active  | Active    | Active    | Inactive |
+ *   Count 3  | Active  | Active    | Active    | Active   |
  *
- * If we set the entire count to 4, cycle 5 would run and we wouldn't be
- * able to specify it as inactive as the active count only goes up to 4.
+ * An entire count of 2 and active count of 3 would always be inactive.
  *
- * In practice this means we want to set the entire cycle to be one less
- * then the actual number of cycles we want, so we can set the number of
- * active cycles to be up to maximum for a fully inactive signal.
+ * The main takeaways here for us are:
+ * - The counter wraps around when it hits the entire cycle count
+ * - The output is active after the counter equals the active cycle count
+ * - An active count of 0 means the period is a 100% active cycle
+ * - An active count larger than the entire cycle count is a 0% active cycle
+ *
+ * This driver deals with the last problem by limiting the entire cycles
+ * to 65534 so we can always specify 65535 for a 0% active cycle.
  *
  * The PWM channels are paired and clocked together, resulting in a
  * cycle time found using the following formula:
@@ -123,7 +130,7 @@ int find_channel_dividers(uint period_ns,
 			  uint parent_hz,
 			  struct pwm_timings *out)
 {
-	uint ideal_cycle_ns = div_ns(period_ns, 65536);
+	uint ideal_cycle_ns = div_ns(period_ns, 65535);
 	int common_div = out->common_div;
 	int prescaler = 1;
 	uint cycle_ns = 0;
@@ -160,10 +167,10 @@ int find_channel_timings(const struct pwm_channel *in,
 	if (find_channel_dividers(in->period_ns, parent_hz, &new))
 		return -1;
 
-	new.entire_cycles = (in->period_ns / new.cycle_ns) - 1;
+	new.entire_cycles = (in->period_ns / new.cycle_ns);
 	new.active_cycles = (in->duty_ns / new.cycle_ns);
-	new.period_ns = (new.entire_cycles + 1) * new.cycle_ns;
-	new.duty_ns = new.active_cycles * new.cycle_ns;
+	new.period_ns = (new.entire_cycles * new.cycle_ns);
+	new.duty_ns = (new.active_cycles * new.cycle_ns);
 	new.polarity = in->polarity;
 
 	if (error_too_large(new.period_ns, in->period_ns))
@@ -311,7 +318,7 @@ void enable_channel(void *base, int channel, struct pwm_timings *timings)
 {
 	u32 pwm_active = (timings->polarity) ? 0 : PCR_PWM_ACTIVE;
 	u32 prescale = (timings->prescale_k - 1);
-	u32 entire_cycles = timings->entire_cycles;
+	u32 entire_cycles = (timings->entire_cycles - 1);
 	u32 active_cycles = timings->active_cycles;
 
 	u32 PCR_clear = (PCR_PRESCAL_K_MASK | PCR_PWM_ACTIVE);
John Watts May 20, 2024, 6:14 a.m. UTC | #2
After a long time reading the datasheet I found out my approach was
actually wrong and led to an off by on error.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/pwm/sunxi_pwm_d1.c | 57 +++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/pwm/sunxi_pwm_d1.c b/drivers/pwm/sunxi_pwm_d1.c
index 6c57bc6e85..f396afff3e 100644
--- a/drivers/pwm/sunxi_pwm_d1.c
+++ b/drivers/pwm/sunxi_pwm_d1.c
@@ -1,34 +1,41 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright 2022 Jookia <contact@jookia.org>
 /*
- * The Allwinner D1's PWM channels are 16-bit counters with up to
- * 65537 cycles (yes, you read that correctly).
+ * The Allwinner D1's PWM channels are paired 16-bit counters.
  *
  * Each channel must be programmed using three variables:
  * - The entire cycle count (used for the period)
- * - The active cycle count (the count of inactive cycles)
- * - The polarity (specifies if the signal is active high or low)
- * The cycle counts are at minimum 1 and at maximum 65536.
+ * - The active cycle count (used for the duty cycle)
+ * - The active state polarity (specifies if the signal goes high or low)
  *
- * The controller will output the number of entire cycles plus one
- * extra, with any cycles after the active cycle output as active.
+ * All counts are zero based, but the datasheet spends a lot of time
+ * adding 1 to the entire cycle count. There's no hidden extra cycle,
+ * it's just trying to make it human understandable. After all, you can
+ * have 0 active counts for a 100% duty cycle, but 0 entire cycles
+ * doesn't really mean sense or work in time calculations.
  *
- * Consider a PWM period of 128 nanoseconds and a cycle period of 32.
- * Setting the entire cycle count to 3 and active cycle count to 4
- * gives an output like so:
+ * The counter works like this (quoting the datasheet):
+ * - PCNTR = (PCNTR == PWM_ENTIRE_CYCLE) ? 0 : PCNTR + 1
+ * - PCNTR > (PWM_ENTIRE_CYCLE - PWM_ACT_CYCLE) = Output active state
+ * - PCNTR <= (PWM_ENTIRE_CYCLE - PWM_ACT_CYCLE) = Output inactive state
  *
- * - Cycle 1 runs 0 to 32 nanoseconds, inactive
- * - Cycle 2 runs 32 to 64 nanoseconds, inactive
- * - Cycle 3 runs 64 to 96 nanoseconds, inactive
- * - Cycle 4 runs 96 to 128 nanoseconds, inactive
- * - Cycle 5 is skipped but would run 128 to 160 nanoseconds, active
+ * Here's a 2-bit table of cycle counts versus active cycle counts:
+ *   Active:  |       0 |        1  |        2  |        3 |
+ *   Count 0  | Active  | Inactive  | Inactive  | Inactive |
+ *   Count 1  | Active  | Active    | Inactive  | Inactive |
+ *   Count 2  | Active  | Active    | Active    | Inactive |
+ *   Count 3  | Active  | Active    | Active    | Active   |
  *
- * If we set the entire count to 4, cycle 5 would run and we wouldn't be
- * able to specify it as inactive as the active count only goes up to 4.
+ * An entire count of 2 and active count of 3 would always be inactive.
  *
- * In practice this means we want to set the entire cycle to be one less
- * then the actual number of cycles we want, so we can set the number of
- * active cycles to be up to maximum for a fully inactive signal.
+ * The main takeaways here for us are:
+ * - The counter wraps around when it hits the entire cycle count
+ * - The output is active after the counter equals the active cycle count
+ * - An active count of 0 means the period is a 100% active cycle
+ * - An active count larger than the entire cycle count is a 0% active cycle
+ *
+ * This driver deals with the last problem by limiting the entire cycles
+ * to 65534 so we can always specify 65535 for a 0% active cycle.
  *
  * The PWM channels are paired and clocked together, resulting in a
  * cycle time found using the following formula:
@@ -123,7 +130,7 @@ int find_channel_dividers(uint period_ns,
 			  uint parent_hz,
 			  struct pwm_timings *out)
 {
-	uint ideal_cycle_ns = div_ns(period_ns, 65536);
+	uint ideal_cycle_ns = div_ns(period_ns, 65535);
 	int common_div = out->common_div;
 	int prescaler = 1;
 	uint cycle_ns = 0;
@@ -160,10 +167,10 @@ int find_channel_timings(const struct pwm_channel *in,
 	if (find_channel_dividers(in->period_ns, parent_hz, &new))
 		return -1;
 
-	new.entire_cycles = (in->period_ns / new.cycle_ns) - 1;
+	new.entire_cycles = (in->period_ns / new.cycle_ns);
 	new.active_cycles = (in->duty_ns / new.cycle_ns);
-	new.period_ns = (new.entire_cycles + 1) * new.cycle_ns;
-	new.duty_ns = new.active_cycles * new.cycle_ns;
+	new.period_ns = (new.entire_cycles * new.cycle_ns);
+	new.duty_ns = (new.active_cycles * new.cycle_ns);
 	new.polarity = in->polarity;
 
 	if (error_too_large(new.period_ns, in->period_ns))
@@ -311,7 +318,7 @@ void enable_channel(void *base, int channel, struct pwm_timings *timings)
 {
 	u32 pwm_active = (timings->polarity) ? 0 : PCR_PWM_ACTIVE;
 	u32 prescale = (timings->prescale_k - 1);
-	u32 entire_cycles = timings->entire_cycles;
+	u32 entire_cycles = (timings->entire_cycles - 1);
 	u32 active_cycles = timings->active_cycles;
 
 	u32 PCR_clear = (PCR_PRESCAL_K_MASK | PCR_PWM_ACTIVE);