From b548d1d580e6711282ed125b0d3aa8b3381792f0 Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Tue, 22 Apr 2025 16:53:08 +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 8e73b4664f019e7b0c1cff365c9a9230ba331c54 Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Tue, 22 Apr 2025 16:53:09 +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 19fcfce472dfbe2127dfd228e9dc225460ca62c3 Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Tue, 22 Apr 2025 16:53:10 +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 d3c37a7caabf1180ae2f23329a8aefc32376cda7 Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Tue, 22 Apr 2025 16:53:11 +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 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c index 6259f8500f7114..ac874c6dc89058 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,8 @@ 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; + do_div(frac, 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 270110f866ab646da4a2a607d3f63373aa27af3f Mon Sep 17 00:00:00 2001 From: Nylon Chen Date: Tue, 22 Apr 2025 16:53:12 +0800 Subject: [PATCH 5/5] pwm: sifive: clarify inverted compare logic in comments The reference 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 | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c index ac874c6dc89058..4f337a236a5f77 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,10 @@ 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; @@ -161,6 +182,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, do_div(frac, 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(invert the duty) */ inactive = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; mutex_lock(&ddata->lock);