Hi Uwe, On Thu, 7 Nov 2019 at 07:51, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > Hello Clément, > > On Wed, Nov 06, 2019 at 10:24:39PM +0100, Clément Péron wrote: > > On Tue, 5 Nov 2019 at 15:57, Uwe Kleine-König > > <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > > On Tue, Nov 05, 2019 at 02:14:53PM +0100, Clément Péron wrote: > > > > + bypass = state->enabled && > > > > + (state->period * clk_rate >= NSEC_PER_SEC) && > > > > > > This is too coarse. With state->period = 1000000 this is fulfilled > > > (unless the multiplication overflows). > > > > Sorry, misunderstood the previous mail > > > > What about something like this ? > > ((state->period - 1) * clk_rate <= NSEC_PER_SEC) && > > ((state->period + 1) * clk_rate >= NSEC_PER_SEC) && > > ((state->duty_cycle - 1) * 2 <= state->period) && > > ((state->duty_cycle + 1) * 2 >= state->period); > > > > We are sure that the user is looking for a PWM around the OSC with a > > 50% duty cycle ? > > This again is too strict. The general policy to fulfill a request is: > > 1) provide the longest possible period not bigger than requested > 2) provide the longest possible duty cycle not bigger than requested > 3) if possible complete the currently running period before switching > and don't return to the user before the new setting is active. > Document the behaviour prominently because the code (usually) > doesn't allow to understand the hardware's features here. > 4) A disabled PWM should output the inactive level Thanks for the explanation > > And then there is a corner case: If the user requests .duty_cycle = 0, > .enabled = 1 it is ok to provide .enabled = 0 iff otherwise 0% isn't > possible. > > So the right check for bypass is: > > state->period * clk_rate >= NSEC_PER_SEC && > state->period * clk_rate < whatevercanbereachedwithoutbypass && > state->duty_cycle * clk_rate * 2 >= NSEC_PER_SEC The shortest PWM ratio which is not a constant output is 12MHz. (Prescal 1, 2 entire cycle and 1 active cycle) So something like this : state->period * clk_rate >= NSEC_PER_SEC && state->period * clk_rate < 2 * NSEC_PER_SEC && state->duty_cycle * clk_rate * 2 >= NSEC_PER_SEC I will send a v4, Thanks for the help Regards, Clément > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | http://www.pengutronix.de/ |