[PATCH v6 0/6] pwm: tiehrpwm: add .get_state and tidy register handling

[PATCH v6 0/6] pwm: tiehrpwm: add .get_state and tidy register handling

[Rafael V. Volkmer]

On Fri, 12 Sep 2025 11:57:44 +0200, Uwe Kleine-König wrote:
> FTR: I applied my series now and it should appear in next on Monday. So
> going forward should have become a tad easier for you.

Hi Uwe,

This v6 is rebased on top of your recent tiehrpwm series in pwm/for-next,
so all of your fixes (including the prescaler corner case and AQCTL cleanups)
are kept as-is and this series only builds on that behaviour.

The only places where there is some unavoidable "policy" are .get_state()
and the probe-time handling of channels that firmware might have left
running. Both follow the same semantics that the driver already used:
a channel is considered enabled if AQCTL is non-zero and not software-
forced in AQCSFRC, and disabled otherwise. At probe we take a best-effort
snapshot of AQCTL(A/B) and AQCSFRC before touching clocks or runtime PM;
if the block is power-gated or the clock is off these registers read as
0 and we deliberately interpret that as "not configured / disabled" to
avoid inventing state that the hardware doesnt expose.

For channels that look active at probe, we only bring tbclk and runtime PM
usage into line with what is already programmed in hardware; we dont
introduce any additional permanent bias beyond reflecting that observed
state.

Best regards,
Rafael V. Volkmer

Rafael .V Volkmer (6):
  pwm: tiehrpwm: use GENMASK() and FIELD_PREP() for register fields
  pwm: tiehrpwm: use GENMASK() and FIELD_PREP() for prescalers
  pwm: tiehrpwm: refactor AQCTL polarity macros
  pwm: tiehrpwm: implement .get_state callback
  pwm: tiehrpwm: handle already-running channels at probe
  pwm: tiehrpwm: tidy period conflict check

 drivers/pwm/pwm-tiehrpwm.c | 332 ++++++++++++++++++++++++++++---------
 1 file changed, 258 insertions(+), 74 deletions(-)

-- 
2.43.0

[PATCH v6 1/6] pwm: tiehrpwm: use GENMASK() and FIELD_PREP() for register fields

[Rafael V. Volkmer]

Convert register field definitions in AQCTL, AQCSFRC and TBCTL to use
GENMASK() and FIELD_PREP(). This clarifies the bit layout, reduces
hand-rolled shift logic and aligns the driver with common kernel patterns.

No functional change.

Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com>
---
 drivers/pwm/pwm-tiehrpwm.c | 91 ++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 42 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 7a86cb090f76..d2151065083b 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -13,6 +13,7 @@
 #include <linux/clk.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
+#include <linux/bitfield.h>
 
 /* EHRPWM registers and bits definitions */
 
@@ -21,22 +22,24 @@
 #define TBPRD			0x0A
 
 #define TBCTL_PRDLD_MASK	BIT(3)
-#define TBCTL_PRDLD_SHDW	0
-#define TBCTL_PRDLD_IMDT	BIT(3)
-#define TBCTL_CLKDIV_MASK	(BIT(12) | BIT(11) | BIT(10) | BIT(9) | \
-				BIT(8) | BIT(7))
-#define TBCTL_CTRMODE_MASK	(BIT(1) | BIT(0))
-#define TBCTL_CTRMODE_UP	0
-#define TBCTL_CTRMODE_DOWN	BIT(0)
-#define TBCTL_CTRMODE_UPDOWN	BIT(1)
-#define TBCTL_CTRMODE_FREEZE	(BIT(1) | BIT(0))
+#define TBCTL_PRDLD_SHDW	FIELD_PREP(TBCTL_PRDLD_MASK, 0)
+#define TBCTL_PRDLD_IMDT	FIELD_PREP(TBCTL_PRDLD_MASK, 1)
+
+#define TBCTL_CLKDIV_MASK	GENMASK(12, 7)
+
+#define TBCTL_CTRMODE_MASK	GENMASK(1, 0)
+#define TBCTL_CTRMODE_UP	FIELD_PREP(TBCTL_CTRMODE_MASK, 0)
+#define TBCTL_CTRMODE_DOWN	FIELD_PREP(TBCTL_CTRMODE_MASK, 1)
+#define TBCTL_CTRMODE_UPDOWN	FIELD_PREP(TBCTL_CTRMODE_MASK, 2)
+#define TBCTL_CTRMODE_FREEZE	FIELD_PREP(TBCTL_CTRMODE_MASK, 3)
 
 #define TBCTL_HSPCLKDIV_SHIFT	7
 #define TBCTL_CLKDIV_SHIFT	10
 
 #define CLKDIV_MAX		7
 #define HSPCLKDIV_MAX		7
-#define PERIOD_MAX		0x10000
+#define TBPRD_BITS		16
+#define PERIOD_MAX		BIT(TBPRD_BITS)
 
 /* compare module registers */
 #define CMPA			0x12
@@ -48,44 +51,48 @@
 #define AQSFRC			0x1A
 #define AQCSFRC			0x1C
 
-#define AQCTL_CBU_MASK		(BIT(9) | BIT(8))
-#define AQCTL_CBU_FRCLOW	BIT(8)
-#define AQCTL_CBU_FRCHIGH	BIT(9)
-#define AQCTL_CBU_FRCTOGGLE	(BIT(9) | BIT(8))
-#define AQCTL_CAU_MASK		(BIT(5) | BIT(4))
-#define AQCTL_CAU_FRCLOW	BIT(4)
-#define AQCTL_CAU_FRCHIGH	BIT(5)
-#define AQCTL_CAU_FRCTOGGLE	(BIT(5) | BIT(4))
-#define AQCTL_PRD_MASK		(BIT(3) | BIT(2))
-#define AQCTL_PRD_FRCLOW	BIT(2)
-#define AQCTL_PRD_FRCHIGH	BIT(3)
-#define AQCTL_PRD_FRCTOGGLE	(BIT(3) | BIT(2))
-#define AQCTL_ZRO_MASK		(BIT(1) | BIT(0))
-#define AQCTL_ZRO_FRCLOW	BIT(0)
-#define AQCTL_ZRO_FRCHIGH	BIT(1)
-#define AQCTL_ZRO_FRCTOGGLE	(BIT(1) | BIT(0))
+#define AQCTL_CBU_MASK		GENMASK(9, 8)
+#define AQCTL_CBU_FRCLOW	FIELD_PREP(AQCTL_CBU_MASK, 1)
+#define AQCTL_CBU_FRCHIGH	FIELD_PREP(AQCTL_CBU_MASK, 2)
+#define AQCTL_CBU_FRCTOGGLE	FIELD_PREP(AQCTL_CBU_MASK, 3)
+
+#define AQCTL_CAU_MASK		GENMASK(5, 4)
+#define AQCTL_CAU_FRCLOW	FIELD_PREP(AQCTL_CAU_MASK, 1)
+#define AQCTL_CAU_FRCHIGH	FIELD_PREP(AQCTL_CAU_MASK, 2)
+#define AQCTL_CAU_FRCTOGGLE	FIELD_PREP(AQCTL_CAU_MASK, 3)
+
+#define AQCTL_PRD_MASK		GENMASK(3, 2)
+#define AQCTL_PRD_FRCLOW	FIELD_PREP(AQCTL_PRD_MASK, 1)
+#define AQCTL_PRD_FRCHIGH	FIELD_PREP(AQCTL_PRD_MASK, 2)
+#define AQCTL_PRD_FRCTOGGLE	FIELD_PREP(AQCTL_PRD_MASK, 3)
+
+#define AQCTL_ZRO_MASK		GENMASK(1, 0)
+#define AQCTL_ZRO_FRCLOW	FIELD_PREP(AQCTL_ZRO_MASK, 1)
+#define AQCTL_ZRO_FRCHIGH	FIELD_PREP(AQCTL_ZRO_MASK, 2)
+#define AQCTL_ZRO_FRCTOGGLE	FIELD_PREP(AQCTL_ZRO_MASK, 3)
 
 #define AQCTL_CHANA_POLNORMAL	(AQCTL_CAU_FRCLOW | AQCTL_ZRO_FRCHIGH)
 #define AQCTL_CHANA_POLINVERSED	(AQCTL_CAU_FRCHIGH | AQCTL_ZRO_FRCLOW)
 #define AQCTL_CHANB_POLNORMAL	(AQCTL_CBU_FRCLOW | AQCTL_ZRO_FRCHIGH)
 #define AQCTL_CHANB_POLINVERSED	(AQCTL_CBU_FRCHIGH | AQCTL_ZRO_FRCLOW)
 
-#define AQSFRC_RLDCSF_MASK	(BIT(7) | BIT(6))
-#define AQSFRC_RLDCSF_ZRO	0
-#define AQSFRC_RLDCSF_PRD	BIT(6)
-#define AQSFRC_RLDCSF_ZROPRD	BIT(7)
-#define AQSFRC_RLDCSF_IMDT	(BIT(7) | BIT(6))
-
-#define AQCSFRC_CSFB_MASK	(BIT(3) | BIT(2))
-#define AQCSFRC_CSFB_FRCDIS	0
-#define AQCSFRC_CSFB_FRCLOW	BIT(2)
-#define AQCSFRC_CSFB_FRCHIGH	BIT(3)
-#define AQCSFRC_CSFB_DISSWFRC	(BIT(3) | BIT(2))
-#define AQCSFRC_CSFA_MASK	(BIT(1) | BIT(0))
-#define AQCSFRC_CSFA_FRCDIS	0
-#define AQCSFRC_CSFA_FRCLOW	BIT(0)
-#define AQCSFRC_CSFA_FRCHIGH	BIT(1)
-#define AQCSFRC_CSFA_DISSWFRC	(BIT(1) | BIT(0))
+#define AQSFRC_RLDCSF_MASK	GENMASK(7, 6)
+#define AQSFRC_RLDCSF_ZRO	FIELD_PREP(AQSFRC_RLDCSF_MASK, 0)
+#define AQSFRC_RLDCSF_PRD	FIELD_PREP(AQSFRC_RLDCSF_MASK, 1)
+#define AQSFRC_RLDCSF_ZROPRD	FIELD_PREP(AQSFRC_RLDCSF_MASK, 2)
+#define AQSFRC_RLDCSF_IMDT	FIELD_PREP(AQSFRC_RLDCSF_MASK, 3)
+
+#define AQCSFRC_CSFB_MASK	GENMASK(3, 2)
+#define AQCSFRC_CSFB_FRCDIS	FIELD_PREP(AQCSFRC_CSFB_MASK, 0)
+#define AQCSFRC_CSFB_FRCLOW	FIELD_PREP(AQCSFRC_CSFB_MASK, 1)
+#define AQCSFRC_CSFB_FRCHIGH	FIELD_PREP(AQCSFRC_CSFB_MASK, 2)
+#define AQCSFRC_CSFB_DISSWFRC	FIELD_PREP(AQCSFRC_CSFB_MASK, 3)
+
+#define AQCSFRC_CSFA_MASK	GENMASK(1, 0)
+#define AQCSFRC_CSFA_FRCDIS	FIELD_PREP(AQCSFRC_CSFA_MASK, 0)
+#define AQCSFRC_CSFA_FRCLOW	FIELD_PREP(AQCSFRC_CSFA_MASK, 1)
+#define AQCSFRC_CSFA_FRCHIGH	FIELD_PREP(AQCSFRC_CSFA_MASK, 2)
+#define AQCSFRC_CSFA_DISSWFRC	FIELD_PREP(AQCSFRC_CSFA_MASK, 3)
 
 #define NUM_PWM_CHANNEL		2	/* EHRPWM channels */
 
-- 
2.43.0

[PATCH v6 2/6] pwm: tiehrpwm: use GENMASK() and FIELD_PREP() for prescalers

[Rafael V. Volkmer]

Replace manual handling of CLKDIV and HSPCLKDIV with GENMASK() and
FIELD_PREP(). Introduce TBCTL_PRESCALE_MASK so both fields can be
updated in a single ehrpwm_modify() call, and drop the now-unused
TBCTL_*_SHIFT macros.

This improves readability and reduces the chance of off-by-shift errors.

No functional change.

Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com>
---
 drivers/pwm/pwm-tiehrpwm.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index d2151065083b..1ad8577139be 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -25,7 +25,9 @@
 #define TBCTL_PRDLD_SHDW	FIELD_PREP(TBCTL_PRDLD_MASK, 0)
 #define TBCTL_PRDLD_IMDT	FIELD_PREP(TBCTL_PRDLD_MASK, 1)
 
-#define TBCTL_CLKDIV_MASK	GENMASK(12, 7)
+#define TBCTL_CLKDIV_MASK	GENMASK(12, 10)
+#define TBCTL_HSPCLKDIV_MASK	GENMASK(9, 7)
+#define TBCTL_PRESCALE_MASK	(TBCTL_CLKDIV_MASK | TBCTL_HSPCLKDIV_MASK)
 
 #define TBCTL_CTRMODE_MASK	GENMASK(1, 0)
 #define TBCTL_CTRMODE_UP	FIELD_PREP(TBCTL_CTRMODE_MASK, 0)
@@ -33,9 +35,6 @@
 #define TBCTL_CTRMODE_UPDOWN	FIELD_PREP(TBCTL_CTRMODE_MASK, 2)
 #define TBCTL_CTRMODE_FREEZE	FIELD_PREP(TBCTL_CTRMODE_MASK, 3)
 
-#define TBCTL_HSPCLKDIV_SHIFT	7
-#define TBCTL_CLKDIV_SHIFT	10
-
 #define CLKDIV_MAX		7
 #define HSPCLKDIV_MAX		7
 #define TBPRD_BITS		16
@@ -169,8 +168,8 @@ static int set_prescale_div(unsigned long rqst_prescaler, u16 *prescale_div,
 			*prescale_div = (1 << clkdiv) *
 					(hspclkdiv ? (hspclkdiv * 2) : 1);
 			if (*prescale_div >= rqst_prescaler) {
-				*tb_clk_div = (clkdiv << TBCTL_CLKDIV_SHIFT) |
-					(hspclkdiv << TBCTL_HSPCLKDIV_SHIFT);
+				*tb_clk_div = FIELD_PREP(TBCTL_CLKDIV_MASK, clkdiv) |
+					FIELD_PREP(TBCTL_HSPCLKDIV_MASK, hspclkdiv);
 				return 0;
 			}
 		}
@@ -247,7 +246,7 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	pm_runtime_get_sync(pwmchip_parent(chip));
 
 	/* Update clock prescaler values */
-	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
+	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_PRESCALE_MASK, tb_divval);
 
 	if (pwm->hwpwm == 1) {
 		/* Channel 1 configured with compare B register */
-- 
2.43.0

[PATCH v6 3/6] pwm: tiehrpwm: refactor AQCTL polarity macros

[Rafael V. Volkmer]

Refactor the AQCTL polarity macros so that the selected channel and
count direction are explicit in the name (CHA/CHB and UP/DN).  Keep the
existing up-count behaviour and introduce matching down-count variants
for the CAD/CBD events.

Centralize the action values (FRCLOW/FRCHIGH/FRCTOGGLE) as generic
AQCTL_* enums and use FIELD_PREP() with the corresponding *_MASK for all
events (CAU/CBU/CAD/CBD/ZRO).  This reduces duplication and makes the
AQCTL programming more self-documenting.

No functional change intended.

Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com>
---
 drivers/pwm/pwm-tiehrpwm.c | 59 +++++++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 1ad8577139be..9f1be35912d3 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -50,30 +50,49 @@
 #define AQSFRC			0x1A
 #define AQCSFRC			0x1C
 
+#define AQCTL_FRCLOW		1
+#define AQCTL_FRCHIGH		2
+#define AQCTL_FRCTOGGLE		3
+
+#define AQCTL_CBD_MASK		GENMASK(11, 10)
+#define AQCTL_CBD_FRCLOW	FIELD_PREP(AQCTL_CBD_MASK, AQCTL_FRCLOW)
+#define AQCTL_CBD_FRCHIGH	FIELD_PREP(AQCTL_CBD_MASK, AQCTL_FRCHIGH)
+#define AQCTL_CBD_FRCTOGGLE	FIELD_PREP(AQCTL_CBD_MASK, AQCTL_FRCTOGGLE)
+
 #define AQCTL_CBU_MASK		GENMASK(9, 8)
-#define AQCTL_CBU_FRCLOW	FIELD_PREP(AQCTL_CBU_MASK, 1)
-#define AQCTL_CBU_FRCHIGH	FIELD_PREP(AQCTL_CBU_MASK, 2)
-#define AQCTL_CBU_FRCTOGGLE	FIELD_PREP(AQCTL_CBU_MASK, 3)
+#define AQCTL_CBU_FRCLOW	FIELD_PREP(AQCTL_CBU_MASK, AQCTL_FRCLOW)
+#define AQCTL_CBU_FRCHIGH	FIELD_PREP(AQCTL_CBU_MASK, AQCTL_FRCHIGH)
+#define AQCTL_CBU_FRCTOGGLE	FIELD_PREP(AQCTL_CBU_MASK, AQCTL_FRCTOGGLE)
+
+#define AQCTL_CAD_MASK		GENMASK(7, 6)
+#define AQCTL_CAD_FRCLOW	FIELD_PREP(AQCTL_CAD_MASK, AQCTL_FRCLOW)
+#define AQCTL_CAD_FRCHIGH	FIELD_PREP(AQCTL_CAD_MASK, AQCTL_FRCHIGH)
+#define AQCTL_CAD_FRCTOGGLE	FIELD_PREP(AQCTL_CAD_MASK, AQCTL_FRCTOGGLE)
 
 #define AQCTL_CAU_MASK		GENMASK(5, 4)
-#define AQCTL_CAU_FRCLOW	FIELD_PREP(AQCTL_CAU_MASK, 1)
-#define AQCTL_CAU_FRCHIGH	FIELD_PREP(AQCTL_CAU_MASK, 2)
-#define AQCTL_CAU_FRCTOGGLE	FIELD_PREP(AQCTL_CAU_MASK, 3)
+#define AQCTL_CAU_FRCLOW	FIELD_PREP(AQCTL_CAU_MASK, AQCTL_FRCLOW)
+#define AQCTL_CAU_FRCHIGH	FIELD_PREP(AQCTL_CAU_MASK, AQCTL_FRCHIGH)
+#define AQCTL_CAU_FRCTOGGLE	FIELD_PREP(AQCTL_CAU_MASK, AQCTL_FRCTOGGLE)
 
 #define AQCTL_PRD_MASK		GENMASK(3, 2)
-#define AQCTL_PRD_FRCLOW	FIELD_PREP(AQCTL_PRD_MASK, 1)
-#define AQCTL_PRD_FRCHIGH	FIELD_PREP(AQCTL_PRD_MASK, 2)
-#define AQCTL_PRD_FRCTOGGLE	FIELD_PREP(AQCTL_PRD_MASK, 3)
+#define AQCTL_PRD_FRCLOW	FIELD_PREP(AQCTL_PRD_MASK, AQCTL_FRCLOW)
+#define AQCTL_PRD_FRCHIGH	FIELD_PREP(AQCTL_PRD_MASK, AQCTL_FRCHIGH)
+#define AQCTL_PRD_FRCTOGGLE	FIELD_PREP(AQCTL_PRD_MASK, AQCTL_FRCTOGGLE)
 
 #define AQCTL_ZRO_MASK		GENMASK(1, 0)
-#define AQCTL_ZRO_FRCLOW	FIELD_PREP(AQCTL_ZRO_MASK, 1)
-#define AQCTL_ZRO_FRCHIGH	FIELD_PREP(AQCTL_ZRO_MASK, 2)
-#define AQCTL_ZRO_FRCTOGGLE	FIELD_PREP(AQCTL_ZRO_MASK, 3)
+#define AQCTL_ZRO_FRCLOW	FIELD_PREP(AQCTL_ZRO_MASK, AQCTL_FRCLOW)
+#define AQCTL_ZRO_FRCHIGH	FIELD_PREP(AQCTL_ZRO_MASK, AQCTL_FRCHIGH)
+#define AQCTL_ZRO_FRCTOGGLE	FIELD_PREP(AQCTL_ZRO_MASK, AQCTL_FRCTOGGLE)
+
+#define AQCTL_CHA_UP_POLNORMAL	(AQCTL_CAU_FRCLOW | AQCTL_ZRO_FRCHIGH)
+#define AQCTL_CHA_UP_POLINVERSE	(AQCTL_CAU_FRCHIGH | AQCTL_ZRO_FRCLOW)
+#define AQCTL_CHB_UP_POLNORMAL	(AQCTL_CBU_FRCLOW | AQCTL_ZRO_FRCHIGH)
+#define AQCTL_CHB_UP_POLINVERSE	(AQCTL_CBU_FRCHIGH | AQCTL_ZRO_FRCLOW)
 
-#define AQCTL_CHANA_POLNORMAL	(AQCTL_CAU_FRCLOW | AQCTL_ZRO_FRCHIGH)
-#define AQCTL_CHANA_POLINVERSED	(AQCTL_CAU_FRCHIGH | AQCTL_ZRO_FRCLOW)
-#define AQCTL_CHANB_POLNORMAL	(AQCTL_CBU_FRCLOW | AQCTL_ZRO_FRCHIGH)
-#define AQCTL_CHANB_POLINVERSED	(AQCTL_CBU_FRCHIGH | AQCTL_ZRO_FRCLOW)
+#define AQCTL_CHA_DN_POLNORMAL     (AQCTL_CAD_FRCLOW | AQCTL_ZRO_FRCHIGH)
+#define AQCTL_CHA_DN_POLINVERSE    (AQCTL_CAD_FRCHIGH | AQCTL_ZRO_FRCLOW)
+#define AQCTL_CHB_DN_POLNORMAL     (AQCTL_CBD_FRCLOW | AQCTL_ZRO_FRCHIGH)
+#define AQCTL_CHB_DN_POLINVERSE    (AQCTL_CBD_FRCHIGH | AQCTL_ZRO_FRCLOW)
 
 #define AQSFRC_RLDCSF_MASK	GENMASK(7, 6)
 #define AQSFRC_RLDCSF_ZRO	FIELD_PREP(AQSFRC_RLDCSF_MASK, 0)
@@ -256,9 +275,9 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		aqctl_mask = AQCTL_CBU_MASK;
 
 		if (polarity == PWM_POLARITY_INVERSED)
-			aqctl_val = AQCTL_CHANB_POLINVERSED;
+			aqctl_val = AQCTL_CHB_UP_POLINVERSE;
 		else
-			aqctl_val = AQCTL_CHANB_POLNORMAL;
+			aqctl_val = AQCTL_CHB_UP_POLNORMAL;
 
 		/* if duty_cycle is big, don't toggle on CBU */
 		if (duty_cycles > period_cycles)
@@ -272,9 +291,9 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		aqctl_mask = AQCTL_CAU_MASK;
 
 		if (polarity == PWM_POLARITY_INVERSED)
-			aqctl_val = AQCTL_CHANA_POLINVERSED;
+			aqctl_val = AQCTL_CHA_UP_POLINVERSE;
 		else
-			aqctl_val = AQCTL_CHANA_POLNORMAL;
+			aqctl_val = AQCTL_CHA_UP_POLNORMAL;
 
 		/* if duty_cycle is big, don't toggle on CAU */
 		if (duty_cycles > period_cycles)
-- 
2.43.0

[PATCH v6 4/6] pwm: tiehrpwm: implement .get_state callback

[Rafael V. Volkmer]

Implement ehrpwm_get_state() so that consumers can query the current
hardware configuration via pwm_get_state().

The callback reconstructs period and duty_cycle in nanoseconds from
TBPRD, CMPA/B and the TBCTL prescaler fields. It also inspects AQCSFRC
to treat software-forced outputs as disabled, and decodes the CAx/CDx
action-qualifier bits in AQCTL(A/B) to report the current polarity.

To make polarity decoding reliable, the configuration path now programs
both up- and down-count compare actions (CAx/CDx) consistently for each
polarity.

This lets consumers query the effective hardware state instead of just
the last requested settings.

Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com>
---
 drivers/pwm/pwm-tiehrpwm.c | 132 +++++++++++++++++++++++++++++++++----
 1 file changed, 121 insertions(+), 11 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 9f1be35912d3..cc525626b2f9 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -114,6 +114,9 @@
 
 #define NUM_PWM_CHANNEL		2	/* EHRPWM channels */
 
+#define PWM_CHA			0
+#define PWM_CHB			1
+
 struct ehrpwm_context {
 	u16 tbctl;
 	u16 tbprd;
@@ -267,37 +270,37 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	/* Update clock prescaler values */
 	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_PRESCALE_MASK, tb_divval);
 
-	if (pwm->hwpwm == 1) {
+	if (pwm->hwpwm == PWM_CHB) {
 		/* Channel 1 configured with compare B register */
 		cmp_reg = CMPB;
 
 		aqctl_reg = AQCTLB;
-		aqctl_mask = AQCTL_CBU_MASK;
+		aqctl_mask = AQCTL_CBU_MASK | AQCTL_CBD_MASK;
 
 		if (polarity == PWM_POLARITY_INVERSED)
-			aqctl_val = AQCTL_CHB_UP_POLINVERSE;
+			aqctl_val = AQCTL_CHB_UP_POLINVERSE | AQCTL_CHB_DN_POLINVERSE;
 		else
-			aqctl_val = AQCTL_CHB_UP_POLNORMAL;
+			aqctl_val = AQCTL_CHB_UP_POLNORMAL | AQCTL_CHB_DN_POLNORMAL;
 
-		/* if duty_cycle is big, don't toggle on CBU */
+		/* if duty_cycle is big, don't toggle on compare events */
 		if (duty_cycles > period_cycles)
-			aqctl_val &= ~AQCTL_CBU_MASK;
+			aqctl_val &= ~(AQCTL_CBU_MASK | AQCTL_CBD_MASK);
 
 	} else {
 		/* Channel 0 configured with compare A register */
 		cmp_reg = CMPA;
 
 		aqctl_reg = AQCTLA;
-		aqctl_mask = AQCTL_CAU_MASK;
+		aqctl_mask = AQCTL_CAU_MASK | AQCTL_CAD_MASK;
 
 		if (polarity == PWM_POLARITY_INVERSED)
-			aqctl_val = AQCTL_CHA_UP_POLINVERSE;
+			aqctl_val = AQCTL_CHA_UP_POLINVERSE | AQCTL_CHA_DN_POLINVERSE;
 		else
-			aqctl_val = AQCTL_CHA_UP_POLNORMAL;
+			aqctl_val = AQCTL_CHA_UP_POLNORMAL | AQCTL_CHA_DN_POLNORMAL;
 
-		/* if duty_cycle is big, don't toggle on CAU */
+		/* if duty_cycle is big, don't toggle on compare events */
 		if (duty_cycles > period_cycles)
-			aqctl_val &= ~AQCTL_CAU_MASK;
+			aqctl_val &= ~(AQCTL_CAU_MASK | AQCTL_CAD_MASK);
 	}
 
 	aqctl_mask |= AQCTL_PRD_MASK | AQCTL_ZRO_MASK;
@@ -426,9 +429,116 @@ static int ehrpwm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	return err;
 }
 
+static int ehrpwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+			    struct pwm_state *state)
+{
+	int ret = 0;
+
+	struct ehrpwm_pwm_chip *pc = NULL;
+
+	/* Registers */
+	u16 aqcsfrc_reg, aqctl_reg, tbprd_reg, tbctl_reg;
+
+	/* Bits */
+	u8 csf_bits, clkdiv_bits, hspclkdiv_bits;
+
+	/* Values */
+	u64 period_ticks, duty_ticks, ps_div;
+
+	/* Actions */
+	u8 up_action, down_action;
+
+	pc = to_ehrpwm_pwm_chip(chip);
+
+	/*
+	 * The 'hwpwm' field identifies which hardware output channel (e.g.,
+	 * 0 for channel A and 1 for channel B) of the eHRPWM module is in use.
+	 */
+	if (pwm->hwpwm == PWM_CHA) {
+		aqcsfrc_reg = readw(pc->mmio_base + AQCSFRC);
+		csf_bits = FIELD_GET(AQCSFRC_CSFA_MASK, aqcsfrc_reg);
+		aqctl_reg = readw(pc->mmio_base + AQCTLA);
+	} else {
+		aqcsfrc_reg = readw(pc->mmio_base + AQCSFRC);
+		csf_bits = FIELD_GET(AQCSFRC_CSFB_MASK, aqcsfrc_reg);
+		aqctl_reg = readw(pc->mmio_base + AQCTLB);
+	}
+
+	if (csf_bits || !aqctl_reg) {
+		state->enabled = false;
+		return 0;
+	}
+
+	state->enabled = true;
+
+	tbprd_reg = readw(pc->mmio_base + TBPRD);
+	period_ticks = (u64)tbprd_reg + 1u;
+
+	tbctl_reg = readw(pc->mmio_base + TBCTL);
+	hspclkdiv_bits = FIELD_GET(TBCTL_HSPCLKDIV_MASK, tbctl_reg);
+	clkdiv_bits = FIELD_GET(TBCTL_CLKDIV_MASK, tbctl_reg);
+
+	ps_div = (1 << clkdiv_bits) * (hspclkdiv_bits ? (hspclkdiv_bits * 2) : 1);
+
+	/*
+	 * period (in ns) = (period_ticks * preescaler * 1e9) / clk_rate
+	 * Using DIV_ROUND_UP_ULL to avoid floating-point operations.
+	 */
+	state->period = DIV_ROUND_UP_ULL(period_ticks * ps_div * NSEC_PER_SEC, pc->clk_rate);
+
+	if (pwm->hwpwm == PWM_CHA)
+		duty_ticks = readw(pc->mmio_base + CMPA);
+	else
+		duty_ticks = readw(pc->mmio_base + CMPB);
+
+	/*
+	 * duty_cycle (in ns) = (duty_ticks * preescaler * 1e9) / clk_rate
+	 * Using DIV_ROUND_UP_ULL to avoid floating-point operations.
+	 */
+	state->duty_cycle = DIV_ROUND_UP_ULL(duty_ticks * ps_div * NSEC_PER_SEC, pc->clk_rate);
+
+	/*
+	 * The 'hwpwm' field identifies which hardware output channel (e.g.,
+	 * 0 for channel A and 1 for channel B) of the eHRPWM module is in use.
+	 */
+	if (pwm->hwpwm == PWM_CHA) {
+		aqctl_reg = readw(pc->mmio_base + AQCTLA);
+		up_action = FIELD_GET(AQCTL_CAU_MASK, aqctl_reg);
+		down_action = FIELD_GET(AQCTL_CAD_MASK, aqctl_reg);
+	} else {
+		aqctl_reg = readw(pc->mmio_base + AQCTLB);
+		up_action = FIELD_GET(AQCTL_CBU_MASK, aqctl_reg);
+		down_action = FIELD_GET(AQCTL_CBD_MASK, aqctl_reg);
+	}
+
+	/*
+	 * Evaluate the actions to determine the PWM polarity:
+	 *  - In this driver, NORMAL polarity is programmed as:
+	 *      ZRO = FRCHIGH and CAx/CDx = FRCLOW.
+	 *  - INVERSED polarity is programmed as:
+	 *      ZRO = FRCLOW and CAx/CDx = FRCHIGH.
+	 *
+	 * So if both up/down compare actions are FRCLOW we treat it as
+	 * NORMAL, and if both are FRCHIGH we treat it as INVERSED.
+	 */
+	if (up_action == AQCTL_FRCLOW && down_action == AQCTL_FRCLOW) {
+		state->polarity = PWM_POLARITY_NORMAL;
+	} else if (up_action == AQCTL_FRCHIGH && down_action == AQCTL_FRCHIGH) {
+		state->polarity = PWM_POLARITY_INVERSED;
+	} else {
+		state->polarity = PWM_POLARITY_NORMAL;
+		dev_dbg(pwmchip_parent(chip),
+			"ehrpwm: unknown polarity bits (0x%x/0x%x), defaulting to NORMAL\n",
+			up_action, down_action);
+	}
+
+	return ret;
+}
+
 static const struct pwm_ops ehrpwm_pwm_ops = {
 	.free = ehrpwm_pwm_free,
 	.apply = ehrpwm_pwm_apply,
+	.get_state = ehrpwm_get_state,
 };
 
 static const struct of_device_id ehrpwm_of_match[] = {
-- 
2.43.0

[PATCH v6 5/6] pwm: tiehrpwm: handle already-running channels at probe

[Rafael V. Volkmer]

Detect active A/B outputs from AQCTL(A/B) when they are not
software-forced in AQCSFRC, take one pm_runtime_get_sync() per active
channel, and enable tbclk only if at least one channel is active. This
keeps runtime PM usage counts and tbclk state consistent with the
actual hardware configuration.

Before, an eHRPWM instance left running by firmware or the bootloader
could have no runtime PM reference and no tbclk enable, leaving the
driver out of sync with hardware. After this change, PM and clock
state match the active channels observed at probe.

Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com>
---
 drivers/pwm/pwm-tiehrpwm.c | 64 +++++++++++++++++++++++++++++++++-----
 1 file changed, 57 insertions(+), 7 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index cc525626b2f9..1a9be36fc7b2 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -554,13 +554,35 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
 	struct ehrpwm_pwm_chip *pc;
 	struct pwm_chip *chip;
 	struct clk *clk;
-	int ret;
+	int ret, ch_idx, ch_disable;
+	u16 aqcsfrc_reg, aqctla_reg, aqctlb_reg;
+	bool enabled_ch[NUM_PWM_CHANNEL] = { false, false };
 
 	chip = devm_pwmchip_alloc(&pdev->dev, NUM_PWM_CHANNEL, sizeof(*pc));
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
 	pc = to_ehrpwm_pwm_chip(chip);
 
+	pc->mmio_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pc->mmio_base))
+		return PTR_ERR(pc->mmio_base);
+
+	/*
+	 * Best-effort snapshot of AQCSFRC/AQCTLx before touching clocks or
+	 * runtime PM. If the eHRPWM block is power-gated or its clock is
+	 * disabled these registers are expected to read as 0, which we
+	 * interpret as "channel not configured / disabled".
+	 */
+	aqcsfrc_reg = readw(pc->mmio_base + AQCSFRC);
+	aqctla_reg = readw(pc->mmio_base + AQCTLA);
+	aqctlb_reg = readw(pc->mmio_base + AQCTLB);
+
+	if (aqctla_reg != 0 && !FIELD_GET(AQCSFRC_CSFA_MASK, aqcsfrc_reg))
+		enabled_ch[PWM_CHA] = true;
+
+	if (aqctlb_reg != 0 && !FIELD_GET(AQCSFRC_CSFB_MASK, aqcsfrc_reg))
+		enabled_ch[PWM_CHB] = true;
+
 	clk = devm_clk_get(&pdev->dev, "fck");
 	if (IS_ERR(clk)) {
 		if (of_device_is_compatible(np, "ti,am33xx-ecap")) {
@@ -580,10 +602,6 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
 
 	chip->ops = &ehrpwm_pwm_ops;
 
-	pc->mmio_base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(pc->mmio_base))
-		return PTR_ERR(pc->mmio_base);
-
 	/* Acquire tbclk for Time Base EHRPWM submodule */
 	pc->tbclk = devm_clk_get(&pdev->dev, "tbclk");
 	if (IS_ERR(pc->tbclk))
@@ -595,17 +613,49 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	if (enabled_ch[PWM_CHA] || enabled_ch[PWM_CHB]) {
+		ret = clk_enable(pc->tbclk);
+		if (ret) {
+			dev_err_probe(&pdev->dev, ret, "clk_enable(tbclk) failed\n");
+			goto err_clk_unprepare;
+		}
+	}
+
 	ret = pwmchip_add(chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
-		goto err_clk_unprepare;
+		goto err_disable_tbclk;
 	}
 
 	platform_set_drvdata(pdev, chip);
 	pm_runtime_enable(&pdev->dev);
 
-	return 0;
+	/*
+	 * Treat channels that were configured and not software-forced at probe
+	 * time as "pre-enabled": take a runtime PM reference so the eHRPWM block
+	 * stays powered while such channels exist. Consumers still get/put PM
+	 * on top of this bias via pwm_enable()/pwm_disable().
+	 */
+	for (ch_idx = 0; ch_idx < NUM_PWM_CHANNEL; ch_idx++) {
+		if (enabled_ch[ch_idx]) {
+			ret = pm_runtime_get_sync(&pdev->dev);
+			if (ret < 0) {
+				for (ch_disable = 0; ch_disable <= ch_idx; ch_disable++) {
+					if (enabled_ch[ch_disable])
+						pm_runtime_put_noidle(&pdev->dev);
+				}
+
+				pwmchip_remove(chip);
+				pm_runtime_disable(&pdev->dev);
+				goto err_disable_tbclk;
+			}
+		}
+	}
 
+	return 0;
+err_disable_tbclk:
+	if (enabled_ch[PWM_CHA] || enabled_ch[PWM_CHB])
+		clk_disable(pc->tbclk);
 err_clk_unprepare:
 	clk_unprepare(pc->tbclk);
 
-- 
2.43.0

[PATCH v6 6/6] pwm: tiehrpwm: tidy period conflict check

[Rafael V. Volkmer]

Tidy ehrpwm_pwm_config(): drop redundant parentheses and keep the
period conflict condition on a single line to follow kernel style.

This change addresses a style warning reported by checkpatch.pl.
No functional change intended.

Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com>
---
 drivers/pwm/pwm-tiehrpwm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 4def04fc6b1c..xxxxxxxxxxxx 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -233,8 +233,7 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * same period register for multiple channels.
 	 */
 	for (i = 0; i < NUM_PWM_CHANNEL; i++) {
-		if (pc->period_cycles[i] &&
-				(pc->period_cycles[i] != period_cycles)) {
+		if (pc->period_cycles[i] && pc->period_cycles[i] != period_cycles) {
 			/*
 			 * Allow channel to reconfigure period if no other
 			 * channels being configured.
-- 
2.43.0

Re: [PATCH v6 1/6] pwm: tiehrpwm: use GENMASK() and FIELD_PREP() for register fields

[Uwe Kleine-König]

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

On Sun, Nov 23, 2025 at 08:25:21PM -0300, Rafael V. Volkmer wrote:
> Convert register field definitions in AQCTL, AQCSFRC and TBCTL to use
> GENMASK() and FIELD_PREP(). This clarifies the bit layout, reduces
> hand-rolled shift logic and aligns the driver with common kernel patterns.
> 
> No functional change.
> 
> Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com>
> ---
>  drivers/pwm/pwm-tiehrpwm.c | 91 ++++++++++++++++++++------------------
>  1 file changed, 49 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 7a86cb090f76..d2151065083b 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -13,6 +13,7 @@
>  #include <linux/clk.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/of.h>
> +#include <linux/bitfield.h>
>  
>  /* EHRPWM registers and bits definitions */
>  
> @@ -21,22 +22,24 @@
>  #define TBPRD			0x0A
>  
>  #define TBCTL_PRDLD_MASK	BIT(3)

I'd prefer s/TBCTL_PRDLD_MASK/TBCTL_PRDLD/ here. This way _MASK isn't
special compared to _SHOW (and others).

As you touch the register defines it would also be great to give them a
common prefix (e.g. TIEHRPWM_).

> -#define TBCTL_PRDLD_SHDW	0
> -#define TBCTL_PRDLD_IMDT	BIT(3)
> -#define TBCTL_CLKDIV_MASK	(BIT(12) | BIT(11) | BIT(10) | BIT(9) | \
> -				BIT(8) | BIT(7))
> -#define TBCTL_CTRMODE_MASK	(BIT(1) | BIT(0))
> -#define TBCTL_CTRMODE_UP	0
> -#define TBCTL_CTRMODE_DOWN	BIT(0)
> -#define TBCTL_CTRMODE_UPDOWN	BIT(1)
> -#define TBCTL_CTRMODE_FREEZE	(BIT(1) | BIT(0))
> +#define TBCTL_PRDLD_SHDW	FIELD_PREP(TBCTL_PRDLD_MASK, 0)
> +#define TBCTL_PRDLD_IMDT	FIELD_PREP(TBCTL_PRDLD_MASK, 1)
> +
> +#define TBCTL_CLKDIV_MASK	GENMASK(12, 7)
> +
> +#define TBCTL_CTRMODE_MASK	GENMASK(1, 0)
> +#define TBCTL_CTRMODE_UP	FIELD_PREP(TBCTL_CTRMODE_MASK, 0)
> +#define TBCTL_CTRMODE_DOWN	FIELD_PREP(TBCTL_CTRMODE_MASK, 1)
> +#define TBCTL_CTRMODE_UPDOWN	FIELD_PREP(TBCTL_CTRMODE_MASK, 2)
> +#define TBCTL_CTRMODE_FREEZE	FIELD_PREP(TBCTL_CTRMODE_MASK, 3)
>  
>  #define TBCTL_HSPCLKDIV_SHIFT	7
>  #define TBCTL_CLKDIV_SHIFT	10
>  
>  #define CLKDIV_MAX		7
>  #define HSPCLKDIV_MAX		7
> -#define PERIOD_MAX		0x10000
> +#define TBPRD_BITS		16
> +#define PERIOD_MAX		BIT(TBPRD_BITS)

If you do something like:

	#define TIEHRPWM_TBPRD		0xa
	#define TIEHRPWM_TBPRD_TBPRD		GENMASK(15, 0)

(which I admit looks a bit redundant, but still represents the
register layout properly) you can use

	FIELD_MAX(TIEHRPWM_TBPRD_TBPRD)

to define the maximal period.

Best regards
Uwe

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

Re: [PATCH v6 4/6] pwm: tiehrpwm: implement .get_state callback

[Uwe Kleine-König]

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

Hello Rafael,

On Sun, Nov 23, 2025 at 08:31:51PM -0300, Rafael V. Volkmer wrote:
> Implement ehrpwm_get_state() so that consumers can query the current
> hardware configuration via pwm_get_state().

Note this is wrong as pwm_get_state() only yields the last request but
not the actual hardware setting.

> The callback reconstructs period and duty_cycle in nanoseconds from
> TBPRD, CMPA/B and the TBCTL prescaler fields. It also inspects AQCSFRC
> to treat software-forced outputs as disabled, and decodes the CAx/CDx
> action-qualifier bits in AQCTL(A/B) to report the current polarity.
> 
> To make polarity decoding reliable, the configuration path now programs
> both up- and down-count compare actions (CAx/CDx) consistently for each
> polarity.
> 
> This lets consumers query the effective hardware state instead of just
> the last requested settings.
> 
> Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com>
> ---
>  drivers/pwm/pwm-tiehrpwm.c | 132 +++++++++++++++++++++++++++++++++----
>  1 file changed, 121 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 9f1be35912d3..cc525626b2f9 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -114,6 +114,9 @@
>  
>  #define NUM_PWM_CHANNEL		2	/* EHRPWM channels */
>  
> +#define PWM_CHA			0
> +#define PWM_CHB			1

Please don't introduce more defines with a PWM_ prefix that look more
generic than they are.

>  struct ehrpwm_context {
>  	u16 tbctl;
>  	u16 tbprd;
> @@ -267,37 +270,37 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  	/* Update clock prescaler values */
>  	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_PRESCALE_MASK, tb_divval);
>  
> -	if (pwm->hwpwm == 1) {
> +	if (pwm->hwpwm == PWM_CHB) {
>  		/* Channel 1 configured with compare B register */
>  		cmp_reg = CMPB;
>  
>  		aqctl_reg = AQCTLB;
> -		aqctl_mask = AQCTL_CBU_MASK;
> +		aqctl_mask = AQCTL_CBU_MASK | AQCTL_CBD_MASK;
>  
>  		if (polarity == PWM_POLARITY_INVERSED)
> -			aqctl_val = AQCTL_CHB_UP_POLINVERSE;
> +			aqctl_val = AQCTL_CHB_UP_POLINVERSE | AQCTL_CHB_DN_POLINVERSE;
>  		else
> -			aqctl_val = AQCTL_CHB_UP_POLNORMAL;
> +			aqctl_val = AQCTL_CHB_UP_POLNORMAL | AQCTL_CHB_DN_POLNORMAL;
>  
> -		/* if duty_cycle is big, don't toggle on CBU */
> +		/* if duty_cycle is big, don't toggle on compare events */
>  		if (duty_cycles > period_cycles)
> -			aqctl_val &= ~AQCTL_CBU_MASK;
> +			aqctl_val &= ~(AQCTL_CBU_MASK | AQCTL_CBD_MASK);
>  
>  	} else {
>  		/* Channel 0 configured with compare A register */
>  		cmp_reg = CMPA;
>  
>  		aqctl_reg = AQCTLA;
> -		aqctl_mask = AQCTL_CAU_MASK;
> +		aqctl_mask = AQCTL_CAU_MASK | AQCTL_CAD_MASK;
>  
>  		if (polarity == PWM_POLARITY_INVERSED)
> -			aqctl_val = AQCTL_CHA_UP_POLINVERSE;
> +			aqctl_val = AQCTL_CHA_UP_POLINVERSE | AQCTL_CHA_DN_POLINVERSE;
>  		else
> -			aqctl_val = AQCTL_CHA_UP_POLNORMAL;
> +			aqctl_val = AQCTL_CHA_UP_POLNORMAL | AQCTL_CHA_DN_POLNORMAL;
>  
> -		/* if duty_cycle is big, don't toggle on CAU */
> +		/* if duty_cycle is big, don't toggle on compare events */
>  		if (duty_cycles > period_cycles)
> -			aqctl_val &= ~AQCTL_CAU_MASK;
> +			aqctl_val &= ~(AQCTL_CAU_MASK | AQCTL_CAD_MASK);
>  	}
>  
>  	aqctl_mask |= AQCTL_PRD_MASK | AQCTL_ZRO_MASK;

Can you please split out these changes? These if branches are nearly
identical, I wonder if there is a way to make them smaller without
making it considerably less understandable.

> @@ -426,9 +429,116 @@ static int ehrpwm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	return err;
>  }
>  
> +static int ehrpwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> +			    struct pwm_state *state)
> +{
> +	int ret = 0;
> +
> +	struct ehrpwm_pwm_chip *pc = NULL;
> +
> +	/* Registers */
> +	u16 aqcsfrc_reg, aqctl_reg, tbprd_reg, tbctl_reg;
> +
> +	/* Bits */
> +	u8 csf_bits, clkdiv_bits, hspclkdiv_bits;
> +
> +	/* Values */
> +	u64 period_ticks, duty_ticks, ps_div;
> +
> +	/* Actions */
> +	u8 up_action, down_action;
> +
> +	pc = to_ehrpwm_pwm_chip(chip);
> +
> +	/*
> +	 * The 'hwpwm' field identifies which hardware output channel (e.g.,
> +	 * 0 for channel A and 1 for channel B) of the eHRPWM module is in use.
> +	 */
> +	if (pwm->hwpwm == PWM_CHA) {
> +		aqcsfrc_reg = readw(pc->mmio_base + AQCSFRC);
> +		csf_bits = FIELD_GET(AQCSFRC_CSFA_MASK, aqcsfrc_reg);
> +		aqctl_reg = readw(pc->mmio_base + AQCTLA);
> +	} else {
> +		aqcsfrc_reg = readw(pc->mmio_base + AQCSFRC);
> +		csf_bits = FIELD_GET(AQCSFRC_CSFB_MASK, aqcsfrc_reg);
> +		aqctl_reg = readw(pc->mmio_base + AQCTLB);
> +	}
> +
> +	if (csf_bits || !aqctl_reg) {
> +		state->enabled = false;
> +		return 0;
> +	}
> +
> +	state->enabled = true;
> +
> +	tbprd_reg = readw(pc->mmio_base + TBPRD);
> +	period_ticks = (u64)tbprd_reg + 1u;
> +
> +	tbctl_reg = readw(pc->mmio_base + TBCTL);
> +	hspclkdiv_bits = FIELD_GET(TBCTL_HSPCLKDIV_MASK, tbctl_reg);
> +	clkdiv_bits = FIELD_GET(TBCTL_CLKDIV_MASK, tbctl_reg);
> +
> +	ps_div = (1 << clkdiv_bits) * (hspclkdiv_bits ? (hspclkdiv_bits * 2) : 1);
> +
> +	/*
> +	 * period (in ns) = (period_ticks * preescaler * 1e9) / clk_rate
> +	 * Using DIV_ROUND_UP_ULL to avoid floating-point operations.
> +	 */
> +	state->period = DIV_ROUND_UP_ULL(period_ticks * ps_div * NSEC_PER_SEC, pc->clk_rate);
> +
> +	if (pwm->hwpwm == PWM_CHA)
> +		duty_ticks = readw(pc->mmio_base + CMPA);
> +	else
> +		duty_ticks = readw(pc->mmio_base + CMPB);
> +
> +	/*
> +	 * duty_cycle (in ns) = (duty_ticks * preescaler * 1e9) / clk_rate
> +	 * Using DIV_ROUND_UP_ULL to avoid floating-point operations.
> +	 */
> +	state->duty_cycle = DIV_ROUND_UP_ULL(duty_ticks * ps_div * NSEC_PER_SEC, pc->clk_rate);
> +
> +	/*
> +	 * The 'hwpwm' field identifies which hardware output channel (e.g.,
> +	 * 0 for channel A and 1 for channel B) of the eHRPWM module is in use.
> +	 */
> +	if (pwm->hwpwm == PWM_CHA) {
> +		aqctl_reg = readw(pc->mmio_base + AQCTLA);
> +		up_action = FIELD_GET(AQCTL_CAU_MASK, aqctl_reg);
> +		down_action = FIELD_GET(AQCTL_CAD_MASK, aqctl_reg);
> +	} else {
> +		aqctl_reg = readw(pc->mmio_base + AQCTLB);
> +		up_action = FIELD_GET(AQCTL_CBU_MASK, aqctl_reg);
> +		down_action = FIELD_GET(AQCTL_CBD_MASK, aqctl_reg);
> +	}
> +
> +	/*
> +	 * Evaluate the actions to determine the PWM polarity:
> +	 *  - In this driver, NORMAL polarity is programmed as:
> +	 *      ZRO = FRCHIGH and CAx/CDx = FRCLOW.
> +	 *  - INVERSED polarity is programmed as:
> +	 *      ZRO = FRCLOW and CAx/CDx = FRCHIGH.
> +	 *
> +	 * So if both up/down compare actions are FRCLOW we treat it as
> +	 * NORMAL, and if both are FRCHIGH we treat it as INVERSED.
> +	 */
> +	if (up_action == AQCTL_FRCLOW && down_action == AQCTL_FRCLOW) {
> +		state->polarity = PWM_POLARITY_NORMAL;
> +	} else if (up_action == AQCTL_FRCHIGH && down_action == AQCTL_FRCHIGH) {
> +		state->polarity = PWM_POLARITY_INVERSED;
> +	} else {
> +		state->polarity = PWM_POLARITY_NORMAL;
> +		dev_dbg(pwmchip_parent(chip),
> +			"ehrpwm: unknown polarity bits (0x%x/0x%x), defaulting to NORMAL\n",
> +			up_action, down_action);
> +	}
> +
> +	return ret;
> +}
> +
>  static const struct pwm_ops ehrpwm_pwm_ops = {
>  	.free = ehrpwm_pwm_free,
>  	.apply = ehrpwm_pwm_apply,
> +	.get_state = ehrpwm_get_state,
>  };

Without having looked in detail: 

A superior improvement over implementing .get_state() is implementing the
new callbacks round_waveform_tohw, round_waveform_fromhw, read_waveform
and write_waveform. And bonus points if you also implement offset
support.

(A "no" here is completely fine, just let me know if you like the idea
and want to work on it. Otherwise I'll look in more detail here.)

At one point in time I had that on my todo list but dropped the ball
after the fixes that went into 6.18-rc1.

>  static const struct of_device_id ehrpwm_of_match[] = {

Best regards
Uwe

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

Re: [PATCH v6 4/6] pwm: tiehrpwm: implement .get_state callback

[Rafael V. Volkmer]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]

Hi Uwe,

On Mon, 24 Nov 2025 17:51:47 +0100, Uwe Kleine-König wrote:
> Note this is wrong as pwm_get_state() only yields the last request but
> not the actual hardware setting.

You’re right about the misleading wording in the commit log:
pwm_get_state() returns the last requested state, not a live snapshot of
the hardware. In the next revision I’ll rephrase the description so it’s
clear that ehrpwm_get_state() is only used to initialize pwm->state from
the current eHRPWM registers when the PWM is first requested.

I’ll also address the other points you mentioned in this series.

> A superior improvement over implementing .get_state() is implementing the
> new callbacks round_waveform_tohw, round_waveform_fromhw, read_waveform
> and write_waveform. And bonus points if you also implement offset
> support.

I like this suggestion and would be interested in doing it for tiehrpwm,
but for now I’d prefer to keep this series limited to the smaller
cleanups, the get_state() implementation and the probe changes, so the
scope stays manageable. My current plan would be to first send a new
revision with those pieces, and then follow up with a separate series
that converts tiehrpwm to round_waveform_tohw / round_waveform_fromhw
plus read_waveform / write_waveform and adds offset support.

From my reading of the PWM core, it seems acceptable for a driver to
provide both the legacy apply/get_state callbacks and the waveform
callbacks, with the logic factored so that they share
the same hardware decoding. If you’d strongly prefer that tiehrpwm is
converted directly to waveform without an intermediate get_state()
series, I can rework the patches in that direction instead.

Best regards,
Rafael V. Volkmer

Re: [PATCH v6 4/6] pwm: tiehrpwm: implement .get_state callback

[Uwe Kleine-König]

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

Hello Rafael,

On Tue, Nov 25, 2025 at 12:48:30AM -0300, Rafael V. Volkmer wrote:
> From my reading of the PWM core, it seems acceptable for a driver to
> provide both the legacy apply/get_state callbacks and the waveform
> callbacks, with the logic factored so that they share
> the same hardware decoding.

No, that makes no sense. The waveform callbacks can do (nearly)
everything that the legacy calls can, so the idea to keep both types
strikes me.

> If you’d strongly prefer that tiehrpwm is
> converted directly to waveform without an intermediate get_state()
> series, I can rework the patches in that direction instead.

I don't prefer it strongly, I just wonder how sensible this is regarding
your (and a bit my) work force.

Best regards
Uwe

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

Re: [PATCH v6 4/6] pwm: tiehrpwm: implement .get_state callback

[Rafael V. Volkmer]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

Hi Uwe,

ok, I’ll do that then and rework the series to adapt tiehrpwm
directly to the waveform callbacks, dropping the legacy
apply/get_state ones. I’ll try to get to this and have a new
version ready sometime next week.

Before I respin the whole series, would you mind having a quick
look at patch 5/6 (the one handling already-running channels at probe)?
If that direction looks fine to you, I can fold any comments into it
and then send everything again as a single updated series.

Best regards,
Rafael V. Volkmer

Re: [PATCH v6 4/6] pwm: tiehrpwm: implement .get_state callback

[Uwe Kleine-König]

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

Hello Rafael,

On Tue, Nov 25, 2025 at 05:39:19PM -0300, Rafael V. Volkmer wrote:
> ok, I’ll do that then and rework the series to adapt tiehrpwm
> directly to the waveform callbacks, dropping the legacy
> apply/get_state ones. I’ll try to get to this and have a new
> version ready sometime next week.
> 
> Before I respin the whole series, would you mind having a quick
> look at patch 5/6 (the one handling already-running channels at probe)?
> If that direction looks fine to you, I can fold any comments into it
> and then send everything again as a single updated series.

Conceptually patch #5 is right. After a very quick glance I wonder if

+       if (enabled_ch[PWM_CHA] || enabled_ch[PWM_CHB]) {
+               ret = clk_enable(pc->tbclk);

is wrong and you need to clk_enable() twice if both enabled_ch[PWM_CHA]
and enabled_ch[PWM_CHB] are true.

Can you please use `git send-email --no-chain-reply-to` for future
submissions? While this sometimes results in having the patches not in
their apply order, it reduces the nesting which helps me and also it the
more common setting.

Best regards
Uwe

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