Hello Thierry, On Fri, May 03, 2019 at 12:59:15PM +0200, Thierry Reding wrote: > On Thu, May 02, 2019 at 10:42:43AM +0200, Uwe Kleine-König wrote: > > On Thu, May 02, 2019 at 10:09:51AM +0200, Boris Brezillon wrote: > > > On Thu, 2 May 2019 09:33:26 +0200 > > > Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > > > On Thu, May 02, 2019 at 09:16:38AM +0200, Boris Brezillon wrote: > > > > > I do understand that some users might want to check how the HW will > > > > > adjust the period/duty before applying the new setup, and in that > > > > > regard, having pwm_round_rate() is a good thing. But in any case, it's > > > > > hard for the user to decide how to adjust things to get what it wants > > > > > (should he increase/decrease the period/duty?). Whenever I wrote pwm_round_rate I actually meant pwm_round_state. > > > > It depends on the use case. For one of them I suggested an algorithm. > > > > > > Yes, I was just trying to say that passing a PWM state to > > > pwm_round_state() is not enough, we need extra info if we want to make > > > it useful, like the rounding policy, the accepted deviation on period, > > > duty or the duty/period ratio, .... > > > > Ack. My suggestion is that round_rate should do: > > > > if polarity is unsupported: > > polarity = !polarity > > duty_cycle = period - duty_cycle > > This should really be up to the consumer. The PWM framework or driver > doesn't know whether or not the consumer cares about the polarity or > whether it only cares about the power output. You don't understand my idea. If the hardware cannot implement the requested state the round_state function has to return a best approximation according to some rules. After that the consumer can decide if they want that or not. As the hardware isn't touched nothing bad happens. (Well, apart from that I still cannot imagine a use case where with the current possibilities of the PWM API a consumer can really care about the polarity.) > > period = biggest supportable period <= requested period, 0 if no > > such period exists. > > > > duty_cycle = biggest supportable duty cycle <= requested > > duty_cycle, 0 if no such value exists > > This doesn't really work. We need some way to detect "value does not > exist" that is different from value == 0, because value == 0 is a > legitimate use-case. Same as above. If I asked for duty_cycle = 5 ns and get back duty_cycle = 0 ns this means the driver cannot implement a duty_cycle in the interval (0, 5] ns, nothing more. > > This would allow to let the consumer (or framework helper function) > > decide which deviation is ok. > > So what's the consumer supposed to do if it gets back these values? If the value is then known to be the best (or good enough if the consumer doesn't really care), it is used. Otherwise a different state is rounded or the consumer gives up if it becomes clear that the hardware cannot satisfy their needs. > How does it know how to scale them if the deviation is not okay? It depends on what the consumer considers optimal. I already gave one example code how I think pwm_round_state should be used. Another is: If the consumer wants a period as near as possible to a given target period the algorithm with the round_state function as suggested would look as follows (duty_cycle ignored here for the ease of discussion): mystate.period = target_period ... pwm_round_state(pwm, &mystate) if (mystate.period == target_period) return mystate.period mystate.period = 2 * target_period - mystate.period pwm_round_state(pwm, &mystate) if (mystate.period < target_period) return mystate.period; do { best_bigger = mystate.period mystate.period -= 1 pwm_round_state(pwm, &mystate) } while (mystate.period > target_period) return best_bigger > What in case the hardware can do achieve a good period that is > slightly bigger than the requested period and which would give a very > good result? The above algorithm answers this question. Theoretically the requirements for pwm_round_state could be changed such that it is supposed to yield the nearest hit instead of the next smaller one, but this only shifts the complexity to a different area. (I think my suggestion is the better one though because the math for the lowlevel drivers and also the helper functions is easier then. Also the search terminates earlier if the consumer wants something bigger than the driver can implement. Moreover "biggest value not bigger than requested" is (IMHO) easier to understand for the affected coders than "nearest value" because there are more ugly special cases. (Consider I ask for (duty_cycle, period) = (10, 20) and the driver can implement (9, 18) and (12, 21). Which of these is nearer?)) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | _______________________________________________ Linux-rockchip mailing list Linux-rockchip@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/linux-rockchip