Messages in this thread |  | | Date | Thu, 10 Jan 2019 11:04:56 -0300 | | From | Paul Cercueil <> | | Subject | Re: [PATCH v9 14/27] pwm: jz4740: Improve algorithm of clock calculation |
| |
Adding Stephen to the discussion. Adding Stephen to the discussion.
On Sat, Jan 5, 2019 at 6:27 PM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > Hello Paul, > > On Sat, Jan 05, 2019 at 06:05:38PM -0300, Paul Cercueil wrote: >> On Sat, Jan 5, 2019 at 4:57 PM, Uwe Kleine-König >> <u.kleine-koenig@pengutronix.de> wrote: >> > You are assuming stuff here about the parent clk which isn't >> guaranteed >> > (AFAICT) by the clk framework: If you call clk_round_rate(clk, >> rate - 1) >> > this might well return rate even if the clock could run slower >> than >> > rate. >> >> It may not be guaranteed by the clock framework itself, but it is >> guaranteed >> to behave like that on this family of SoCs. > > You shouldn't rely on that. Experience shows that people will start > copying code to machines where this is not guaranteed. Even if they > don't copy and only learn from reading this is bad. Also how do you > guarantee that this won't change in the future making the pwm code > break > without noticing? > > If you use an API better don't assume more things given than are > guaranteed by the API. > > Having said that I would consider it sensible to introduce something > like clk_roundup_rate() and clk_rounddown_rate() which would allow > calculations like that.
@Stephen: Some context: my algorithm makes use of clk_round_rate(clk, rate - 1) to get the next (smaller) clock rate that a clock support.
Is it something safe to assume? If not is there a better way?
>> > Wouldn't it make sense to start iterating with rate = 0xffff * >> 1e9 / >> > period? Otherwise you get bad configurations if rate is >> considerable >> > slower than necessary. >> >> The algorithm will start with 'rate' being the parent clock's rate, >> which >> will always be the highest rate that the child clock will support. > > Ah right, I missed that bit.
Thanks, -Paul
|  |