Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1265840

Re: [PATCH 1/2] clk: bcm2835: Support for clock parent selection

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

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.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH 1/2] clk: bcm2835: Support for clock parent selection Remi Pommarel <repk@triplefau.lt> - 2015-11-05 00:10 +0100
  Re: [PATCH 1/2] clk: bcm2835: Support for clock parent selection Eric Anholt <eric@anholt.net> - 2015-11-05 03:10 +0100
    Re: [PATCH 1/2] clk: bcm2835: Support for clock parent selection Remi Pommarel <repk@triplefau.lt> - 2015-11-05 20:00 +0100
      Re: [PATCH 1/2] clk: bcm2835: Support for clock parent selection Eric Anholt <eric@anholt.net> - 2015-11-09 17:40 +0100

csiph-web