Messages in this thread |  | | Date | Fri, 22 Feb 2019 22:17:58 -0300 | | From | Paul Cercueil <> | | Subject | Re: [PATCH v9 14/27] pwm: jz4740: Improve algorithm of clock calculation |
| |
Hi,
Le jeu. 10 janv. 2019 à 11:04, Paul Cercueil <paul@crapouillou.net> a écrit : > 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?
Bump.
What should I do here?
>>> > 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
|  |