From 164e9f434ae2072cb6a1c0781d4c9af762f63226 Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Mon, 21 Apr 2025 17:55:17 +0800 Subject: [PATCH 1/5] riscv: dts: sifive: unleashed/unmatched: Remove PWM controlled LED's active-low properties This removes the active-low properties of the PWM-controlled LEDs in the HiFive Unmatched device tree. The reference is hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1]. Link: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf [0] Link: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf [1] Acked-by: Conor Dooley Reviewed-by: Conor Dooley Signed-off-by: Vincent Chen Signed-off-by: Nylon Chen Signed-off-by: Linux RISC-V bot --- arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts | 12 ++++-------- arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 12 ++++-------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts index 900a50526d7719..06731b8c7bc3bb 100644 --- a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts +++ b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts @@ -49,32 +49,28 @@ compatible = "pwm-leds"; led-d1 { - pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>; - active-low; + pwms = <&pwm0 0 7812500 0>; color = ; max-brightness = <255>; label = "d1"; }; led-d2 { - pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>; - active-low; + pwms = <&pwm0 1 7812500 0>; color = ; max-brightness = <255>; label = "d2"; }; led-d3 { - pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>; - active-low; + pwms = <&pwm0 2 7812500 0>; color = ; max-brightness = <255>; label = "d3"; }; led-d4 { - pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>; - active-low; + pwms = <&pwm0 3 7812500 0>; color = ; max-brightness = <255>; label = "d4"; diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts index 72b87b08ab444e..03ce2cee4e976f 100644 --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts @@ -51,8 +51,7 @@ compatible = "pwm-leds"; led-d12 { - pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>; - active-low; + pwms = <&pwm0 0 7812500 0>; color = ; max-brightness = <255>; label = "d12"; @@ -68,20 +67,17 @@ label = "d2"; led-red { - pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>; - active-low; + pwms = <&pwm0 2 7812500 0>; color = ; }; led-green { - pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>; - active-low; + pwms = <&pwm0 1 7812500 0>; color = ; }; led-blue { - pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>; - active-low; + pwms = <&pwm0 3 7812500 0>; color = ; }; }; From 2c1582b1775319aa889282948866962046383dbb Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Mon, 21 Apr 2025 17:55:18 +0800 Subject: [PATCH 2/5] pwm: sifive: change the PWM algorithm The `frac` variable represents the pulse inactive time, and the result of this algorithm is the pulse active time. Therefore, we must reverse the result. The reference is SiFive FU740-C000 Manual[0] Link: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf [0] Co-developed-by: Zong Li Signed-off-by: Zong Li Co-developed-by: Vincent Chen Signed-off-by: Vincent Chen Signed-off-by: Nylon Chen Signed-off-by: Linux RISC-V bot --- drivers/pwm/pwm-sifive.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c index d5b647e6be7812..bb9146267bc53c 100644 --- a/drivers/pwm/pwm-sifive.c +++ b/drivers/pwm/pwm-sifive.c @@ -110,9 +110,10 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state *state) { struct pwm_sifive_ddata *ddata = pwm_sifive_chip_to_ddata(chip); - u32 duty, val; + u32 duty, val, inactive; - duty = readl(ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm)); + inactive = readl(ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm)); + duty = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - inactive; state->enabled = duty > 0; @@ -123,7 +124,7 @@ static int 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_NORMAL; return 0; } @@ -137,9 +138,9 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, unsigned long long num; bool enabled; int ret = 0; - u32 frac; + u32 frac, inactive; - if (state->polarity != PWM_POLARITY_INVERSED) + if (state->polarity != PWM_POLARITY_NORMAL) return -EINVAL; cur_state = pwm->state; @@ -157,8 +158,9 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, */ num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH); frac = DIV64_U64_ROUND_CLOSEST(num, state->period); - /* The hardware cannot generate a 100% duty cycle */ + /* The hardware cannot generate a 0% duty cycle */ frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); + inactive = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; mutex_lock(&ddata->lock); if (state->period != ddata->approx_period) { @@ -190,7 +192,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, } } - writel(frac, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm)); + writel(inactive, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm)); if (!state->enabled) clk_disable(ddata->clk); From bb0ef27425acac4d12a3611ff576e09d02e634d8 Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Mon, 21 Apr 2025 17:55:19 +0800 Subject: [PATCH 3/5] pwm: sifive: Fix the error in the idempotent test within the pwm_apply_state_debug function Round the result to the nearest whole number. This ensures that real_period is always a reasonable integer that is not lower than the actual value. e.g. $ echo 110 > /sys/devices/platform/led-controller-1/leds/d12/brightness $ .apply is not idempotent (ena=1 pol=0 1739692/4032985) -> (ena=1 pol=0 1739630/4032985) Co-developed-by: Zong Li Signed-off-by: Zong Li Signed-off-by: Nylon Chen Signed-off-by: Linux RISC-V bot --- drivers/pwm/pwm-sifive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c index bb9146267bc53c..6259f8500f7114 100644 --- a/drivers/pwm/pwm-sifive.c +++ b/drivers/pwm/pwm-sifive.c @@ -101,7 +101,7 @@ static void pwm_sifive_update_clock(struct pwm_sifive_ddata *ddata, /* As scale <= 15 the shift operation cannot overflow. */ num = (unsigned long long)NSEC_PER_SEC << (PWM_SIFIVE_CMPWIDTH + scale); - ddata->real_period = div64_ul(num, rate); + ddata->real_period = DIV_ROUND_UP_ULL(num, rate); dev_dbg(ddata->parent, "New real_period = %u ns\n", ddata->real_period); } From 2bdfb8ebea66f65c854f06ed1f5464c1bac87873 Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Mon, 21 Apr 2025 17:55:20 +0800 Subject: [PATCH 4/5] pwm: sifive: Fix rounding issues in apply and get_state functions Fix PWM apply and get_state rounding to ensure consistency between setting and reading values This fixes the reported errors: pwm-sifive 10021000.pwm: .apply is supposed to round down duty_cycle (requested: 360/504000, applied: 361/504124) pwm-sifive 10021000.pwm: .apply is supposed to round down period (requested: 504000, applied: 504124) Co-developed-by: Zong Li Signed-off-by: Zong Li Signed-off-by: Nylon Chen Signed-off-by: Linux RISC-V bot --- drivers/pwm/pwm-sifive.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c index 6259f8500f7114..739214949bfddd 100644 --- a/drivers/pwm/pwm-sifive.c +++ b/drivers/pwm/pwm-sifive.c @@ -122,8 +122,8 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, state->enabled = false; state->period = ddata->real_period; - state->duty_cycle = - (u64)duty * ddata->real_period >> PWM_SIFIVE_CMPWIDTH; + state->duty_cycle = DIV_ROUND_UP_ULL((u64)duty * ddata->real_period, + (1U << PWM_SIFIVE_CMPWIDTH)); state->polarity = PWM_POLARITY_NORMAL; return 0; @@ -157,7 +157,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, * consecutively */ num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH); - frac = DIV64_U64_ROUND_CLOSEST(num, state->period); + frac = num / state->period; /* The hardware cannot generate a 0% duty cycle */ frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); inactive = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; From 8d8538f59a7a1d9bd01f40c23804354ccc24e169 Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Mon, 21 Apr 2025 17:55:21 +0800 Subject: [PATCH 5/5] pwm: sifive: clarify inverted compare logic in comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FU740‑C000 manual says “pwms ≥ pwmcmpX -> HIGH”, but in Figure 29 pwmcmpXcenter is forced to 0 via an XOR, so hardware actually outputs HIGH when pwms < pwmcmpX. Thus pwmcmp holds the off‑period count, and the driver must invert it to expose a normal active‑high interface. Co-developed-by: Zong Li Signed-off-by: Zong Li Signed-off-by: Nylon Chen Signed-off-by: Linux RISC-V bot --- drivers/pwm/pwm-sifive.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c index 739214949bfddd..d6341d1a0107c2 100644 --- a/drivers/pwm/pwm-sifive.c +++ b/drivers/pwm/pwm-sifive.c @@ -4,11 +4,28 @@ * For SiFive's PWM IP block documentation please refer Chapter 14 of * Reference Manual : https://static.dev.sifive.com/FU540-C000-v1.0.pdf * + * PWM output inversion: According to the SiFive Reference manual + * the output of each comparator is high whenever the value of pwms is + * greater than or equal to the corresponding pwmcmpX[Reference Manual]. + * + * Figure 29 in the same manual shows that the pwmcmpXcenter bit is + * hard‑tied to 0 (XNOR), which effectively inverts the comparison so that + * the output goes HIGH when `pwms < pwmcmpX`. + * + * In other words, each pwmcmp register actually defines the **inactive** + * (low) period of the pulse, not the active time exactly opposite to what + * the documentation text implies. + * + * To compensate, this driver always **inverts** the duty value when reading + * or writing pwmcmp registers , so that users interact with a conventional + * **active‑high** PWM interface. + * + * * Limitations: * - When changing both duty cycle and period, we cannot prevent in * software that the output might produce a period with mixed * settings (new period length and old duty cycle). - * - The hardware cannot generate a 100% duty cycle. + * - The hardware cannot generate a 0% duty cycle. * - The hardware generates only inverted output. */ #include @@ -113,6 +130,8 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, u32 duty, val, inactive; inactive = readl(ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm)); + /* PWM hardware uses 'inactive' counts in pwmcmp, so invert to get actual duty. + * Here, 'inactive' is the low time and we compute duty as max_count - inactive. */ duty = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - inactive; state->enabled = duty > 0; @@ -160,6 +179,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, frac = num / state->period; /* The hardware cannot generate a 0% duty cycle */ frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); + /* pwmcmp register must be loaded with the inactive - time count (invert the duty) */ inactive = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; mutex_lock(&ddata->lock);