[PATCH] pwm: sun4i: Round delay time up to a nearest jiffy

[Roman Beranek]

Hello Uwe,

On Fri Apr 30, 2021 at 8:41 AM CEST, Uwe Kleine-König wrote:
> On Fri, Apr 30, 2021 at 04:19:32AM +0200, Roman Beránek wrote:
> > On Thu, Apr 29, 2021 at 2:04 PM Uwe Kleine-König wrote:
> > > On Wed, Apr 28, 2021 at 02:14:31PM +0200, Roman Beránek wrote:
> > > > Correct, the output may stay in an active state. I only discovered this
> > > > bug as I investigated a report of unreliable screen timeout. The period
> > > > we use the PWM with is 50 us.
> > >
> > > What I don't like here is that the delay is quite coarse and might still
> > > be too short. (Maybe I miss something, but consider the current period
> > > is quite long, then you configure a short one and then disable. In this
> > > case the inital long setting might still be active.)
> > 
> > The delay is calculated from the original period (cstate.period),
> > not the one that was just written into PWM_CHx_PRD 2 lines above.
>
> Yes, but that's not good enough. Consider the PWM is running with a
> period of 4s and the period just started. Then you call
>
> pwm_apply_state(mypwm, &(struct pwm_state){
> .period = 20,
> .enabled = 1,
> ...
> })
>
> This doesn't result into the waiting code being run, because
> .enabled = 1. Then immidiately after that call:
>
> pwm_apply_state(mypwm, &(struct pwm_state){
> .period = 20,
> .enabled = 0,
> ...
> })
>
> which triggers the waiting but only based on a period of 20 ns while the
> 4s period is still active.

OK, now I see what you mean. To remedy this, the delay shall occur
regardless of state->enabled.

In my view, this lies beneath the scope of the patch though and would be
better served as a follow-up.

Cheers,
Roman