Hello William, On Wed, Nov 01, 2023 at 10:22:44AM +0800, William Qiu wrote: > > > On 2023/10/20 19:25, Uwe Kleine-König wrote: > >> + void __iomem *base = pwm->data->get_ch_base ? > >> + pwm->data->get_ch_base(pwm->regs, dev->hwpwm) : pwm->regs; > >> + u32 period_data, duty_data, ctrl_data; > >> + > >> + period_data = readl(REG_OCPWM_LRC(base)); > >> + duty_data = readl(REG_OCPWM_HRC(base)); > >> + ctrl_data = readl(REG_OCPWM_CTRL(base)); > >> + > >> + state->period = DIV_ROUND_CLOSEST_ULL((u64)period_data * NSEC_PER_SEC, pwm->clk_rate); > >> + state->duty_cycle = DIV_ROUND_CLOSEST_ULL((u64)duty_data * NSEC_PER_SEC, pwm->clk_rate); > > > > Please test your driver with PWM_DEBUG enabled. The rounding is wrong > > here. > > The conclusion after checking is: when the period or duty_cycle value set > by the user is not divisible (1000000000/49.5M), there will be an error. > This error is due to hardware accuracy. So why is rounding is wrong? > rockchip also has a similar implementation drivers/pwm/ pwm-rockchip.c I fail to follow. Where is an error? The general policy (for new drivers at least) is to implement the biggest period possible not bigger than the requested period. That means that .apply must round down and to make .apply ∘ .get_state idempotent .get_state must round up to match. Assuming a clkrate of 49500000 Hz the actual period for REG_OCPWM_LRC = 400 is 8080.808ns and for REG_OCPWM_LRC = 401 is 8101.010. So with REG_OCPWM_LRC = 401 .get_state should report state.period = 8102 [ns] because if you call .apply with .period = 8101 [ns] you're supposed to use REG_OCPWM_LRC = 400. Rounding using DIV_ROUND_CLOSEST doesn't give consistent behaviour in some cases. Consider a PWM that can implement the following periods (and none in between): 20.1 ns 20.4 ns 21.7 ns With round-to-nearest a request to configure 21 ns will yield 20.4 ns. If you call .get_state there the driver will return 20 ns. However configuring 20 ns results in a period of 20.1 ns. With rounding as requested above you get a consistent behaviour. After .apply_state(period=21) .get_state() returns period=21. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
