Messages in this thread | | | From | Eric Anholt <> | Subject | Re: [PATCH 1/2] clk: bcm2835: Support for clock parent selection | Date | Mon, 09 Nov 2015 08:38:50 -0800 |
| |
Remi Pommarel <repk@triplefau.lt> writes:
> Hi, > > On Wed, Nov 04, 2015 at 06:03:31PM -0800, Eric Anholt wrote: > > [...] > >> >> It looks like you've dropped the use of the divisor off of the PLL >> channel when setting a rate. That seems bad for all the other clocks in >> the system, and a feature we couldn't lose. > > Sorry, but I'm not sure to understand your point here. Are you afraid > that clocks such as PWM, H264, etc, have lost the ability to divide the > rate from the PLL or oscillator clock they cosume as source ? > > If so, I think it's ok. If I'm not wrong here, clk_set_rate() first > calls clk->determinate_rate() then calls clk->set_rate(). This patch > makes bcm2835_clock_determine_source() to only select the parent to use > and does not set the clock's rate itself. The clock's rate is set later > on when bcm2835_clock_set_parent() is called. > > bcm2835_clock_set_parent() still divides the parent rate so we are not > loosing this feature here.
I see. You're leaving req->rate as-is, so that it gets passed back in on the set_rate() call. Since you've chosen only a parent with a rate greater than ours, we know we'll be able to divide into it.
This has the downside that anything using the min/max rate clamping doesn't get to know before setting that we might be out of bounds.
> It would probably be better to have bcm2835_clock_determine_source() > selects the parent by choosing the one that provides the rate which, > after being divided, generates the highest but lower rate out of the > PWM clock itself. > > Moreover, if you agree with the above modification I see no reason to > not call it "bcm2835_clock_determine_rate"
This sounds like what the function should be doing. [unhandled content-type:application/pgp-signature] | |