| Message ID | 20250509095234.643890-1-nylon.chen@sifive.com |
|---|---|
| Headers | show |
| Series | Change PWM-controlled LED pin active mode and algorithm | expand |
Hello, patches 1 and 2 ideally go in together, right? @Conor, does your Ack mean that you're OK with me taking the dts change via my pwm tree? On Fri, May 09, 2025 at 05:52:29PM +0800, Nylon Chen wrote: > Nylon Chen (5): > riscv: dts: sifive: unleashed/unmatched: Remove PWM controlled LED's > active-low properties > pwm: sifive: change the PWM algorithm > pwm: sifive: Fix the error in the idempotent test within the > pwm_apply_state_debug function > pwm: sifive: Fix rounding issues in apply and get_state functions > pwm: sifive: clarify inverted compare logic in comments The objective of patches #3 and #4 is the same, right? Both prevent that .apply ∘ .get_state is idempotent, right? And I'd also squash patches #2 and #5 to have the comments updated together with the respective code changes. I think the algorithm implemented in this driver is more complicated than it should be, but this is not this series's fault, so I tend to apply it with the above mentioned squashes. Best regards Uwe
On Wed, May 28, 2025 at 12:21:45PM +0200, Uwe Kleine-König wrote: > Hello, > > patches 1 and 2 ideally go in together, right? @Conor, does your Ack > mean that you're OK with me taking the dts change via my pwm tree? That's exactly what an ack from me means, yes.
According to the circuit diagram of User LEDs - RGB described in the manual hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1]. The behavior of PWM is acitve-high. According to the descriptionof PWM for pwmcmp in SiFive FU740-C000 Manual[2]. The pwm algorithm is (PW) pulse active time = (D) duty * (T) period. The `frac` variable is pulse "inactive" time so we need to invert it. So this patchset removes active-low in DTS and adds reverse logic to the driver. Links: - [0]: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf - [1]: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf - [2]: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf Updated patches: 1 New patches: 0 Unchanged patches: 4 Changed in v14: - Change `frac` from `u32` to `u64` and cast the constant in `min()` to `u64` so that `do_div(frac, state->period)` no longer triggers type-mismatch Changed in v13: - Fix syntax error: Added missing closing parenthesis in do_div() function call. Changed in v12: - Replace division with do_div() to fix __udivdi3 modpost error. Changed in v11: - Fix rounding consistency in apply() and get_state() - Add code comments to help clarify Reference Manual errors. Changed in v10: - Add 'inactive' variable in apply() to match pwm_sifive_get_state() style - Update comment about hardware limitation - it cannot generate 0% duty cycle rather than 100% duty cycle Changed in v9: - Fix commit message to adhere to 75 columns rule. - Update commit message's subject. - Add a variable for inactive logic. Changed in v8: - Fix Signed-off-by and Co-developed-by typo. Changed in v7: - Remove active-low strings from hifive-unleashed-a00.dts file. Changed in v6: - Separate the idempotent test bug fixes into a new patch. - Move the reversing the duty before the line checking state->enabled. - Fix the algorithm and change it to take the minimum value first and then reverse it. Changed in v5: - Add the updates to the PWM algorithm based on version 2 back in. - Replace div64_ul with DIV_ROUND_UP_ULL to correct the error in the period value of the idempotent test in pwm_apply_state_debug. Changed in v4: - Remove previous updates to the PWM algorithm. Changed in v3: - Convert the reference link to standard link. - Move the inverted function before taking the minimum value. - Change polarity check condition(high and low). - Pick the biggest period length possible that is not bigger than the requested period. Changed in v2: - Convert the reference link to standard link. - Fix typo: s/sifive unmatched:/sifive: unmatched:/. - Remove active-low from hifive-unleashed-a00.dts. - Include this reference link in the dts and pwm commit messages. Nylon Chen (5): riscv: dts: sifive: unleashed/unmatched: Remove PWM controlled LED's active-low properties pwm: sifive: change the PWM algorithm pwm: sifive: Fix the error in the idempotent test within the pwm_apply_state_debug function pwm: sifive: Fix rounding issues in apply and get_state functions pwm: sifive: clarify inverted compare logic in comments .../boot/dts/sifive/hifive-unleashed-a00.dts | 12 ++--- .../boot/dts/sifive/hifive-unmatched-a00.dts | 12 ++--- drivers/pwm/pwm-sifive.c | 52 ++++++++++++++----- 3 files changed, 47 insertions(+), 29 deletions(-)