Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx> 於 2025年1月21日 週二 下午3:47寫道： > > Hello, > > On Sun, Jan 19, 2025 at 03:03:16PM +0800, Nylon Chen wrote: > > I ran some basic tests by changing the period and duty cycle in both > > decreasing and increasing sequences (see the script below). > > What is clk_get_rate(ddata->clk) for you? 130 MHz > > > # Backward testing for period (decreasing) > > echo "Testing period backward..." > > > > seq 15000 -1 5000 | while read p; do > > > > echo $p > /sys/class/pwm/pwmchip1/pwm0/period > > > > echo "Testing period: $p" > > > > done > > > > > > # Forward testing for period (increasing) > > echo "Testing period forward..." > > > > seq 5000 1 15000 | while read p; do > > > > echo $p > /sys/class/pwm/pwmchip1/pwm0/period > > > > echo "Testing period: $p" > > > > done > > > > > > # Backward testing for duty cycle (decreasing) > > echo "Testing duty cycle backward..." > > > > for duty in $(seq 10000 -1 0); do > > > > echo $duty > /sys/class/pwm/pwmchip1/pwm0/duty_cycle > > > > echo "Testing duty cycle: $duty" > > > > done > > > > > > # Forward testing for duty cycle (increasing) > > > > echo "Testing duty cycle forward..." > > > > for duty in $(seq 0 1 10000); do > > > > echo $duty > /sys/class/pwm/pwmchip1/pwm0/duty_cycle > > > > echo "Testing duty cycle: $duty" > > > > done > > > > > > > > In these particular tests, I didn’t see any functional difference or > > unexpected behavior whether I used DIV64_U64_ROUND_CLOSEST() or > > DIV64_U64_ROUND_UP. > > Of course, there’s a chance my tests haven’t covered every scenario, > > so there could still be edge cases I missed. > > Just to be sure: You have PWM_DEBUG enabled? Yes, to verify my patch, I always enable it by default. > > > From what I understand, your main concern is to ensure we never end up > > with a duty cycle that’s smaller than what the user requested, which a > > round-up approach would help guarantee. If you still recommend making > > that change to achieve the desired behavior, I’m happy to update the > > code accordingly(CLOSEST->UP). > > No, .apply should round down and so to ensure that > > pwm_get_state(mypwm, &state); > pwm_apply(mypwm, &state); > > doesn't modify the hardware setting, .get_state has to round up. I have some questions that I'd like to further confirm, to ensure I understand correctly and can adjust the patch accordingly your earlier suggestions were as follows: "Round-closest is usually wrong in an .apply() callback. I didn't do the detailed math, but I think you need to round up here." The more recent suggestions are as follows: "No, .apply should round down and so to ensure that....." I speculate that the latter suggestion is to ensure idempotency between .apply and .get_state, avoiding unnecessary hardware setting modifications during repeated reading and applying processes. However, the earlier suggestions seem to conflict with this. If needed, I can provide more test results or make further adjustments. Thank you again for your patient guidance. > > Best regards > Uwe
