Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
On 12/06/13 02:01, Doug Anderson wrote:
> Hi,
>
> Mike pointed me at this series since I'm running into parenting
> problems at the moment as well...
>
> On Mon, May 20, 2013 at 9:44 PM, Saravana Kannan
> wrote:
>> While writing a similar code for our internal tree, I quickly came to the
>> realization that, "all parents are equal, but some are more equal than
>> others". The simplest example is a clock tree like this:
>>
>> Source -> Divider -> Mux
>> Source -> Mux
>>
>> A rate of Y can be achieved for Mux by either running Source at Y and
>> picking that input or running Source at Y * 2 and Divider set to div-2 and
>> picking the Divider input.
>>
>> The solution seems to be a priority list of parents. I'm sure there would be
>> other reason (jitter, clock quality, etc) for a mux to pick one parent vs.
>> another when both of them can provide the required rate.
>>
>> I think this loop should loop over parents based on their priority order.
>> So, parents should become a struct of { clk, index } and have the parents
>> listed in the order of priority. Well, at least in that long run that would
>> be better to avoid messing up parent/index values. But for now, you could
>> just have a priority array of index or parents.
>>
>> It might not fit 100% of the cases where two parents can provide the same
>> rate, but it should fit most use cases.
>
> I'm slightly worried about similar problems, but I don't have a really
> great solution.
>
> In my case I'm working on exynos5 hardware which has a bunch of PLLs
> and a crazy number of muxing options. Many of the muxing options are
> not really meant to be used but seem to have been added to the SoC as
> a "backup plan" of some sort. :-/ Most of the PLLs are intended to
> be used for one purpose and one purpose only though muxing options
> allow them to be used all over the place.
>
> For instance if I look at my current bootup of exynos5250-snow, I see:
>
> fout_apll/clk_rate:17
> fout_bpll/clk_rate:8
> fout_cpll/clk_rate:33300
> fout_epll/clk_rate:45158401
> fout_gpll/clk_rate:53300
> fout_mpll/clk_rate:16
>
> * APLL is intended to be the parent of the 2 ARM cores and changes due
> to cpu load
> * EPLL is intended to be the parent for audio and changes dpending on
> audio playback rates.
> * GPLL is intended to be the parent of the GPU and changes due to gpu load
> * VPLL is intended to be the parent for video related things and could
> change depending on the LCD.
> * MPLL doesn't change a lot and is intended to be the parent for most things.
> * In some systems BPLL can be used for memory or GPU
>
> My main concern here is the CCF will end up deciding at some point
> that it should reparent some clock onto a PLL that is going to change
> a whole lot. Maybe the user will plug in an SD card that requests a
> frequency of 52MHz and at the moment we'll be running EPLL at 104 MHz
> so it will be a perfect match! ...but then the user wants to play
> audio at a different rate. The audio code assumes that it can mess
> with its clock and we've got code setup to call CLK_SET_RATE_PARENT
> all the way up to EPLL. That will really mess with the SD card.
> Really we'd rather just have the SD card clock always parented on the
> stable MPLL and it's OK if 52MHz gets rounded down to 50MHz.
One possible approach to handling this is by making drivers register
clock rate change notifications, which can reject a clock rate change,
however I believe at the moment this would simply make the clock rate
change fail rather than finding an alternative change which isn't
rejected, so it may have limited use at the moment.
> Of course, on another board maybe they don't have an audio codec and
> aren't using epll for audio and have realized that EPLL would be a
> perfect way to get their SD card to run 4% faster. It ought to work.
>
>
> I guess to summarize the above:
>
> * It seems like much of the muxing on exynos5250 is just too
> complicated to leave it to an simple automated algorithm.
> * It seems like we can't make muxing decisions on the SoC level.
> * Your automatic muxing patches don't hurt me and could be useful for
> _some_ of the muxing options, just not the top PLL ones.
>
> ...but the only place that leaves me for my muxing needs is the device
> tree. ...and as Mike pointed out on IRC the device tree should
> describe hardware, not policy. Ick.
Yes, we have a similar situation with the TZ1090, except it may be worse
as some of the clocks/devices may be driven by different non-Linux cores
or hardware threads (e.g. the WiFi/DAB firmware drives the I2C/SPI bus
the tuner chip is connected to, and related clocks), in which case Linux
literally has no knowledge of whether the hardware is already in use
aside from some form of application-specific configuration (currently
munged into device tree due to the lack of an alternative).
Cheers
James
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
On 21/05/13 05:44, Saravana Kannan wrote:
> On 05/20/2013 06:28 AM, James Hogan wrote:
>> Implement clk-mux remuxing if the CLK_SET_RATE_NO_REPARENT flag isn't
>> set. This implements determine_rate for clk-mux to propagate to each
>> parent and to choose the best one (like clk-divider this chooses the
>> parent which provides the fastest rate <= the requested rate).
>>
>> The determine_rate op is implemented as a core helper function so that
>> it can be easily used by more complex clocks which incorporate muxes.
>>
>> Signed-off-by: James Hogan
>> Cc: Mike Turquette
>> Cc: linux-arm-ker...@lists.infradead.org
>> ---
>> Changes in v4:
>>
>> * never pass NULL to determine_rate's best_parent_clk parameter.
>>
>> Changes in v3:
>>
>> * rename/invert CLK_SET_RATE_REMUX to CLK_SET_RATE_NO_REPARENT and move
>>to patch 3.
>>
>> drivers/clk/clk-mux.c| 1 +
>> drivers/clk/clk.c| 49
>>
>> include/linux/clk-provider.h | 3 +++
>> 3 files changed, 53 insertions(+)
>>
>> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
>> index 25b1734..cecfa01 100644
>> --- a/drivers/clk/clk-mux.c
>> +++ b/drivers/clk/clk-mux.c
>> @@ -100,6 +100,7 @@ static int clk_mux_set_parent(struct clk_hw *hw,
>> u8 index)
>> const struct clk_ops clk_mux_ops = {
>> .get_parent = clk_mux_get_parent,
>> .set_parent = clk_mux_set_parent,
>> +.determine_rate = __clk_mux_determine_rate,
>> };
>> EXPORT_SYMBOL_GPL(clk_mux_ops);
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 3ce4810..85b661d 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -692,6 +692,55 @@ struct clk *__clk_lookup(const char *name)
>> return NULL;
>> }
>>
>> +/*
>> + * Helper for finding best parent to provide a given frequency. This
>> can be used
>> + * directly as a determine_rate callback (e.g. for a mux), or from a
>> more
>> + * complex clock that may combine a mux with other operations.
>> + */
>> +long __clk_mux_determine_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long *best_parent_rate,
>> + struct clk **best_parent_p)
>> +{
>> +struct clk *clk = hw->clk, *parent, *best_parent = NULL;
>> +int i, num_parents;
>> +unsigned long parent_rate, best = 0;
>> +
>> +/* if NO_REPARENT flag set, pass through to current parent */
>> +if (clk->flags & CLK_SET_RATE_NO_REPARENT) {
>> +parent = clk->parent;
>> +if (clk->flags & CLK_SET_RATE_PARENT)
>> +best = __clk_round_rate(parent, rate);
>> +else if (parent)
>> +best = __clk_get_rate(parent);
>> +else
>> +best = __clk_get_rate(clk);
>> +goto out;
>> +}
>> +
>> +/* find the parent that can provide the fastest rate <= rate */
>> +num_parents = clk->num_parents;
>> +for (i = 0; i < num_parents; i++) {
>
> While writing a similar code for our internal tree, I quickly came to
> the realization that, "all parents are equal, but some are more equal
> than others". The simplest example is a clock tree like this:
>
> Source -> Divider -> Mux
> Source -> Mux
>
> A rate of Y can be achieved for Mux by either running Source at Y and
> picking that input or running Source at Y * 2 and Divider set to div-2
> and picking the Divider input.
>
> The solution seems to be a priority list of parents. I'm sure there
> would be other reason (jitter, clock quality, etc) for a mux to pick one
> parent vs. another when both of them can provide the required rate.
>
> I think this loop should loop over parents based on their priority
> order. So, parents should become a struct of { clk, index } and have the
> parents listed in the order of priority. Well, at least in that long run
> that would be better to avoid messing up parent/index values. But for
> now, you could just have a priority array of index or parents.
>
> It might not fit 100% of the cases where two parents can provide the
> same rate, but it should fit most use cases.
Sorry for the delay replying.
What you say sounds reasonable. As Stephen Boyd suggested though, I'd
like to omit this from this patchset as its something that can be
tackled later.
Cheers
James
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
On 21/05/13 05:44, Saravana Kannan wrote:
On 05/20/2013 06:28 AM, James Hogan wrote:
Implement clk-mux remuxing if the CLK_SET_RATE_NO_REPARENT flag isn't
set. This implements determine_rate for clk-mux to propagate to each
parent and to choose the best one (like clk-divider this chooses the
parent which provides the fastest rate = the requested rate).
The determine_rate op is implemented as a core helper function so that
it can be easily used by more complex clocks which incorporate muxes.
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Mike Turquette mturque...@linaro.org
Cc: linux-arm-ker...@lists.infradead.org
---
Changes in v4:
* never pass NULL to determine_rate's best_parent_clk parameter.
Changes in v3:
* rename/invert CLK_SET_RATE_REMUX to CLK_SET_RATE_NO_REPARENT and move
to patch 3.
drivers/clk/clk-mux.c| 1 +
drivers/clk/clk.c| 49
include/linux/clk-provider.h | 3 +++
3 files changed, 53 insertions(+)
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 25b1734..cecfa01 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -100,6 +100,7 @@ static int clk_mux_set_parent(struct clk_hw *hw,
u8 index)
const struct clk_ops clk_mux_ops = {
.get_parent = clk_mux_get_parent,
.set_parent = clk_mux_set_parent,
+.determine_rate = __clk_mux_determine_rate,
};
EXPORT_SYMBOL_GPL(clk_mux_ops);
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 3ce4810..85b661d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -692,6 +692,55 @@ struct clk *__clk_lookup(const char *name)
return NULL;
}
+/*
+ * Helper for finding best parent to provide a given frequency. This
can be used
+ * directly as a determine_rate callback (e.g. for a mux), or from a
more
+ * complex clock that may combine a mux with other operations.
+ */
+long __clk_mux_determine_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *best_parent_rate,
+ struct clk **best_parent_p)
+{
+struct clk *clk = hw-clk, *parent, *best_parent = NULL;
+int i, num_parents;
+unsigned long parent_rate, best = 0;
+
+/* if NO_REPARENT flag set, pass through to current parent */
+if (clk-flags CLK_SET_RATE_NO_REPARENT) {
+parent = clk-parent;
+if (clk-flags CLK_SET_RATE_PARENT)
+best = __clk_round_rate(parent, rate);
+else if (parent)
+best = __clk_get_rate(parent);
+else
+best = __clk_get_rate(clk);
+goto out;
+}
+
+/* find the parent that can provide the fastest rate = rate */
+num_parents = clk-num_parents;
+for (i = 0; i num_parents; i++) {
While writing a similar code for our internal tree, I quickly came to
the realization that, all parents are equal, but some are more equal
than others. The simplest example is a clock tree like this:
Source - Divider - Mux
Source - Mux
A rate of Y can be achieved for Mux by either running Source at Y and
picking that input or running Source at Y * 2 and Divider set to div-2
and picking the Divider input.
The solution seems to be a priority list of parents. I'm sure there
would be other reason (jitter, clock quality, etc) for a mux to pick one
parent vs. another when both of them can provide the required rate.
I think this loop should loop over parents based on their priority
order. So, parents should become a struct of { clk, index } and have the
parents listed in the order of priority. Well, at least in that long run
that would be better to avoid messing up parent/index values. But for
now, you could just have a priority array of index or parents.
It might not fit 100% of the cases where two parents can provide the
same rate, but it should fit most use cases.
Sorry for the delay replying.
What you say sounds reasonable. As Stephen Boyd suggested though, I'd
like to omit this from this patchset as its something that can be
tackled later.
Cheers
James
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
On 12/06/13 02:01, Doug Anderson wrote:
Hi,
Mike pointed me at this series since I'm running into parenting
problems at the moment as well...
On Mon, May 20, 2013 at 9:44 PM, Saravana Kannan skan...@codeaurora.org
wrote:
While writing a similar code for our internal tree, I quickly came to the
realization that, all parents are equal, but some are more equal than
others. The simplest example is a clock tree like this:
Source - Divider - Mux
Source - Mux
A rate of Y can be achieved for Mux by either running Source at Y and
picking that input or running Source at Y * 2 and Divider set to div-2 and
picking the Divider input.
The solution seems to be a priority list of parents. I'm sure there would be
other reason (jitter, clock quality, etc) for a mux to pick one parent vs.
another when both of them can provide the required rate.
I think this loop should loop over parents based on their priority order.
So, parents should become a struct of { clk, index } and have the parents
listed in the order of priority. Well, at least in that long run that would
be better to avoid messing up parent/index values. But for now, you could
just have a priority array of index or parents.
It might not fit 100% of the cases where two parents can provide the same
rate, but it should fit most use cases.
I'm slightly worried about similar problems, but I don't have a really
great solution.
In my case I'm working on exynos5 hardware which has a bunch of PLLs
and a crazy number of muxing options. Many of the muxing options are
not really meant to be used but seem to have been added to the SoC as
a backup plan of some sort. :-/ Most of the PLLs are intended to
be used for one purpose and one purpose only though muxing options
allow them to be used all over the place.
For instance if I look at my current bootup of exynos5250-snow, I see:
fout_apll/clk_rate:17
fout_bpll/clk_rate:8
fout_cpll/clk_rate:33300
fout_epll/clk_rate:45158401
fout_gpll/clk_rate:53300
fout_mpll/clk_rate:16
* APLL is intended to be the parent of the 2 ARM cores and changes due
to cpu load
* EPLL is intended to be the parent for audio and changes dpending on
audio playback rates.
* GPLL is intended to be the parent of the GPU and changes due to gpu load
* VPLL is intended to be the parent for video related things and could
change depending on the LCD.
* MPLL doesn't change a lot and is intended to be the parent for most things.
* In some systems BPLL can be used for memory or GPU
My main concern here is the CCF will end up deciding at some point
that it should reparent some clock onto a PLL that is going to change
a whole lot. Maybe the user will plug in an SD card that requests a
frequency of 52MHz and at the moment we'll be running EPLL at 104 MHz
so it will be a perfect match! ...but then the user wants to play
audio at a different rate. The audio code assumes that it can mess
with its clock and we've got code setup to call CLK_SET_RATE_PARENT
all the way up to EPLL. That will really mess with the SD card.
Really we'd rather just have the SD card clock always parented on the
stable MPLL and it's OK if 52MHz gets rounded down to 50MHz.
One possible approach to handling this is by making drivers register
clock rate change notifications, which can reject a clock rate change,
however I believe at the moment this would simply make the clock rate
change fail rather than finding an alternative change which isn't
rejected, so it may have limited use at the moment.
Of course, on another board maybe they don't have an audio codec and
aren't using epll for audio and have realized that EPLL would be a
perfect way to get their SD card to run 4% faster. It ought to work.
I guess to summarize the above:
* It seems like much of the muxing on exynos5250 is just too
complicated to leave it to an simple automated algorithm.
* It seems like we can't make muxing decisions on the SoC level.
* Your automatic muxing patches don't hurt me and could be useful for
_some_ of the muxing options, just not the top PLL ones.
...but the only place that leaves me for my muxing needs is the device
tree. ...and as Mike pointed out on IRC the device tree should
describe hardware, not policy. Ick.
Yes, we have a similar situation with the TZ1090, except it may be worse
as some of the clocks/devices may be driven by different non-Linux cores
or hardware threads (e.g. the WiFi/DAB firmware drives the I2C/SPI bus
the tuner chip is connected to, and related clocks), in which case Linux
literally has no knowledge of whether the hardware is already in use
aside from some form of application-specific configuration (currently
munged into device tree due to the lack of an alternative).
Cheers
James
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
On Wed, Jun 12, 2013 at 10:55 AM, Doug Anderson wrote:
> Mike,
>
> On Wed, Jun 12, 2013 at 10:45 AM, Mike Turquette
> wrote:
>
>>> * It seems like we can't make muxing decisions on the SoC level.
>>> * Your automatic muxing patches don't hurt me and could be useful for
>>> _some_ of the muxing options, just not the top PLL ones.
>>
>> For the time being you won't be affected by this until you start using
>> .determine_rate. Even then we have the flag which disables this
>> behavior.
>
> Yup, exactly! :) So I have no objections to the auto remuxing, it
> just doesn't solve all of my problems.
>
>
>>> ...but the only place that leaves me for my muxing needs is the device
>>> tree. ...and as Mike pointed out on IRC the device tree should
>>> describe hardware, not policy. Ick.
>>
>> This sounds like another vote for configtree ;-)
>
> Yes. It sounds like for now we're just going to carry some patches to
> setup our clocks, but a configtree seems like it would solve this type
> of problem.
>
> One question to raise: if we're going to need to come up with a
> solution for defining parents for things like PLLs, does it decrease
> the need for the auto-remuxing patches? AKA: if we use some type of
> mechanism like configtree to specify muxing, would that be enough? I
> don't know the answer, but just thought I'd raise the question...
It's a good question. I can think of some cases where auto-muxing
would still be helpful. There are drivers out there that don't need to
know the details of the clock tree hierarchy and automagic muxing will
help remove those details from some of the drivers.
However I think that boot-time configuration of muxes that don't
switch (configtree), combined with custom mux clock code that
implements recommendations from HW engineers and the data sheet will
probably be more common than lots and lots of generic re-muxing. I
think that implementing those recommendations on which parent to use
is the problem Saravana was trying to solve with the priority list of
parents
How to encode that information in DT is a bit of a problem. I guess
one approach could be to create a mux table in DT and instead of
ordering items in that list based on bitfield index (which feel
natural to most programmers), instead let the order imply priority.
This is a tricky hack since we're *technically* only describing the
hardware with the mux table, but the priority encoding is silently
encoded. E.g:
clock: clock@4a008100 {
#clock-cells = <0>;
compatible = "mux-clock";
clocks = <_foo>, <_bar>, <_baz>;
reg = <0x4a008100 0x4>;
mask = <0x3>;
shift = <0>;
table = <_baz 0x2>, <_foo 0x0>, <_bar 0x1>;
};
Thoughts?
Regards,
Mike
>
> -Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
Mike, On Wed, Jun 12, 2013 at 10:45 AM, Mike Turquette wrote: >> * It seems like we can't make muxing decisions on the SoC level. >> * Your automatic muxing patches don't hurt me and could be useful for >> _some_ of the muxing options, just not the top PLL ones. > > For the time being you won't be affected by this until you start using > .determine_rate. Even then we have the flag which disables this > behavior. Yup, exactly! :) So I have no objections to the auto remuxing, it just doesn't solve all of my problems. >> ...but the only place that leaves me for my muxing needs is the device >> tree. ...and as Mike pointed out on IRC the device tree should >> describe hardware, not policy. Ick. > > This sounds like another vote for configtree ;-) Yes. It sounds like for now we're just going to carry some patches to setup our clocks, but a configtree seems like it would solve this type of problem. One question to raise: if we're going to need to come up with a solution for defining parents for things like PLLs, does it decrease the need for the auto-remuxing patches? AKA: if we use some type of mechanism like configtree to specify muxing, would that be enough? I don't know the answer, but just thought I'd raise the question... -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
Quoting Doug Anderson (2013-06-11 18:01:01)
> Hi,
>
> Mike pointed me at this series since I'm running into parenting
> problems at the moment as well...
>
> On Mon, May 20, 2013 at 9:44 PM, Saravana Kannan
> wrote:
> > While writing a similar code for our internal tree, I quickly came to the
> > realization that, "all parents are equal, but some are more equal than
> > others". The simplest example is a clock tree like this:
> >
> > Source -> Divider -> Mux
> > Source -> Mux
> >
> > A rate of Y can be achieved for Mux by either running Source at Y and
> > picking that input or running Source at Y * 2 and Divider set to div-2 and
> > picking the Divider input.
> >
> > The solution seems to be a priority list of parents. I'm sure there would be
> > other reason (jitter, clock quality, etc) for a mux to pick one parent vs.
> > another when both of them can provide the required rate.
> >
> > I think this loop should loop over parents based on their priority order.
> > So, parents should become a struct of { clk, index } and have the parents
> > listed in the order of priority. Well, at least in that long run that would
> > be better to avoid messing up parent/index values. But for now, you could
> > just have a priority array of index or parents.
> >
> > It might not fit 100% of the cases where two parents can provide the same
> > rate, but it should fit most use cases.
>
> I'm slightly worried about similar problems, but I don't have a really
> great solution.
>
> In my case I'm working on exynos5 hardware which has a bunch of PLLs
> and a crazy number of muxing options. Many of the muxing options are
> not really meant to be used but seem to have been added to the SoC as
> a "backup plan" of some sort. :-/ Most of the PLLs are intended to
> be used for one purpose and one purpose only though muxing options
> allow them to be used all over the place.
>
> For instance if I look at my current bootup of exynos5250-snow, I see:
>
> fout_apll/clk_rate:17
> fout_bpll/clk_rate:8
> fout_cpll/clk_rate:33300
> fout_epll/clk_rate:45158401
> fout_gpll/clk_rate:53300
> fout_mpll/clk_rate:16
>
> * APLL is intended to be the parent of the 2 ARM cores and changes due
> to cpu load
> * EPLL is intended to be the parent for audio and changes dpending on
> audio playback rates.
> * GPLL is intended to be the parent of the GPU and changes due to gpu load
> * VPLL is intended to be the parent for video related things and could
> change depending on the LCD.
> * MPLL doesn't change a lot and is intended to be the parent for most things.
> * In some systems BPLL can be used for memory or GPU
>
> My main concern here is the CCF will end up deciding at some point
> that it should reparent some clock onto a PLL that is going to change
> a whole lot. Maybe the user will plug in an SD card that requests a
> frequency of 52MHz and at the moment we'll be running EPLL at 104 MHz
> so it will be a perfect match! ...but then the user wants to play
> audio at a different rate. The audio code assumes that it can mess
> with its clock and we've got code setup to call CLK_SET_RATE_PARENT
> all the way up to EPLL. That will really mess with the SD card.
> Really we'd rather just have the SD card clock always parented on the
> stable MPLL and it's OK if 52MHz gets rounded down to 50MHz.
>
> Of course, on another board maybe they don't have an audio codec and
> aren't using epll for audio and have realized that EPLL would be a
> perfect way to get their SD card to run 4% faster. It ought to work.
>
>
> I guess to summarize the above:
>
> * It seems like much of the muxing on exynos5250 is just too
> complicated to leave it to an simple automated algorithm.
I'd wager this will be true at the PLL-level for most complex SoCs.
Having a general re-muxing algorithm makes a lot more sense for simpler
cases like a fast 100MHz parent and a slow 32KHz parent, and the
requested rate makes it painfully obvious which one to pick. This is
less true for PLLs that can provide a very wide range of frequencies and
have lots of children.
This is analogous to lots of the other basic clock types where we try to
have common code that benefits a wide range of users, but if you need
something custom then you implement a new clock type/driver. If you
have a limited set of combinations that you want to support out of many
possible combinations (e.g. your "backup muxing options") then it might
be better for you to skip the remuxing algorithm and instead call
clk_set_parent explicitly from your clock driver, perhaps in your
clock's .set_rate and/or .set_parent callbacks.
> * It seems like we can't make muxing decisions on the SoC level.
> * Your automatic muxing patches don't hurt me and could be useful for
> _some_ of the muxing options, just not the top PLL ones.
For the time being you won't be affected by this until you start using
.determine_rate. Even then we have the flag which disables this
behavior.
>
>
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
Quoting Doug Anderson (2013-06-11 18:01:01)
Hi,
Mike pointed me at this series since I'm running into parenting
problems at the moment as well...
On Mon, May 20, 2013 at 9:44 PM, Saravana Kannan skan...@codeaurora.org
wrote:
While writing a similar code for our internal tree, I quickly came to the
realization that, all parents are equal, but some are more equal than
others. The simplest example is a clock tree like this:
Source - Divider - Mux
Source - Mux
A rate of Y can be achieved for Mux by either running Source at Y and
picking that input or running Source at Y * 2 and Divider set to div-2 and
picking the Divider input.
The solution seems to be a priority list of parents. I'm sure there would be
other reason (jitter, clock quality, etc) for a mux to pick one parent vs.
another when both of them can provide the required rate.
I think this loop should loop over parents based on their priority order.
So, parents should become a struct of { clk, index } and have the parents
listed in the order of priority. Well, at least in that long run that would
be better to avoid messing up parent/index values. But for now, you could
just have a priority array of index or parents.
It might not fit 100% of the cases where two parents can provide the same
rate, but it should fit most use cases.
I'm slightly worried about similar problems, but I don't have a really
great solution.
In my case I'm working on exynos5 hardware which has a bunch of PLLs
and a crazy number of muxing options. Many of the muxing options are
not really meant to be used but seem to have been added to the SoC as
a backup plan of some sort. :-/ Most of the PLLs are intended to
be used for one purpose and one purpose only though muxing options
allow them to be used all over the place.
For instance if I look at my current bootup of exynos5250-snow, I see:
fout_apll/clk_rate:17
fout_bpll/clk_rate:8
fout_cpll/clk_rate:33300
fout_epll/clk_rate:45158401
fout_gpll/clk_rate:53300
fout_mpll/clk_rate:16
* APLL is intended to be the parent of the 2 ARM cores and changes due
to cpu load
* EPLL is intended to be the parent for audio and changes dpending on
audio playback rates.
* GPLL is intended to be the parent of the GPU and changes due to gpu load
* VPLL is intended to be the parent for video related things and could
change depending on the LCD.
* MPLL doesn't change a lot and is intended to be the parent for most things.
* In some systems BPLL can be used for memory or GPU
My main concern here is the CCF will end up deciding at some point
that it should reparent some clock onto a PLL that is going to change
a whole lot. Maybe the user will plug in an SD card that requests a
frequency of 52MHz and at the moment we'll be running EPLL at 104 MHz
so it will be a perfect match! ...but then the user wants to play
audio at a different rate. The audio code assumes that it can mess
with its clock and we've got code setup to call CLK_SET_RATE_PARENT
all the way up to EPLL. That will really mess with the SD card.
Really we'd rather just have the SD card clock always parented on the
stable MPLL and it's OK if 52MHz gets rounded down to 50MHz.
Of course, on another board maybe they don't have an audio codec and
aren't using epll for audio and have realized that EPLL would be a
perfect way to get their SD card to run 4% faster. It ought to work.
I guess to summarize the above:
* It seems like much of the muxing on exynos5250 is just too
complicated to leave it to an simple automated algorithm.
I'd wager this will be true at the PLL-level for most complex SoCs.
Having a general re-muxing algorithm makes a lot more sense for simpler
cases like a fast 100MHz parent and a slow 32KHz parent, and the
requested rate makes it painfully obvious which one to pick. This is
less true for PLLs that can provide a very wide range of frequencies and
have lots of children.
This is analogous to lots of the other basic clock types where we try to
have common code that benefits a wide range of users, but if you need
something custom then you implement a new clock type/driver. If you
have a limited set of combinations that you want to support out of many
possible combinations (e.g. your backup muxing options) then it might
be better for you to skip the remuxing algorithm and instead call
clk_set_parent explicitly from your clock driver, perhaps in your
clock's .set_rate and/or .set_parent callbacks.
* It seems like we can't make muxing decisions on the SoC level.
* Your automatic muxing patches don't hurt me and could be useful for
_some_ of the muxing options, just not the top PLL ones.
For the time being you won't be affected by this until you start using
.determine_rate. Even then we have the flag which disables this
behavior.
...but the only place that leaves me for my muxing needs is the device
tree. ...and as Mike
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
Mike, On Wed, Jun 12, 2013 at 10:45 AM, Mike Turquette mturque...@linaro.org wrote: * It seems like we can't make muxing decisions on the SoC level. * Your automatic muxing patches don't hurt me and could be useful for _some_ of the muxing options, just not the top PLL ones. For the time being you won't be affected by this until you start using .determine_rate. Even then we have the flag which disables this behavior. Yup, exactly! :) So I have no objections to the auto remuxing, it just doesn't solve all of my problems. ...but the only place that leaves me for my muxing needs is the device tree. ...and as Mike pointed out on IRC the device tree should describe hardware, not policy. Ick. This sounds like another vote for configtree ;-) Yes. It sounds like for now we're just going to carry some patches to setup our clocks, but a configtree seems like it would solve this type of problem. One question to raise: if we're going to need to come up with a solution for defining parents for things like PLLs, does it decrease the need for the auto-remuxing patches? AKA: if we use some type of mechanism like configtree to specify muxing, would that be enough? I don't know the answer, but just thought I'd raise the question... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
On Wed, Jun 12, 2013 at 10:55 AM, Doug Anderson diand...@chromium.org wrote:
Mike,
On Wed, Jun 12, 2013 at 10:45 AM, Mike Turquette mturque...@linaro.org
wrote:
* It seems like we can't make muxing decisions on the SoC level.
* Your automatic muxing patches don't hurt me and could be useful for
_some_ of the muxing options, just not the top PLL ones.
For the time being you won't be affected by this until you start using
.determine_rate. Even then we have the flag which disables this
behavior.
Yup, exactly! :) So I have no objections to the auto remuxing, it
just doesn't solve all of my problems.
...but the only place that leaves me for my muxing needs is the device
tree. ...and as Mike pointed out on IRC the device tree should
describe hardware, not policy. Ick.
This sounds like another vote for configtree ;-)
Yes. It sounds like for now we're just going to carry some patches to
setup our clocks, but a configtree seems like it would solve this type
of problem.
One question to raise: if we're going to need to come up with a
solution for defining parents for things like PLLs, does it decrease
the need for the auto-remuxing patches? AKA: if we use some type of
mechanism like configtree to specify muxing, would that be enough? I
don't know the answer, but just thought I'd raise the question...
It's a good question. I can think of some cases where auto-muxing
would still be helpful. There are drivers out there that don't need to
know the details of the clock tree hierarchy and automagic muxing will
help remove those details from some of the drivers.
However I think that boot-time configuration of muxes that don't
switch (configtree), combined with custom mux clock code that
implements recommendations from HW engineers and the data sheet will
probably be more common than lots and lots of generic re-muxing. I
think that implementing those recommendations on which parent to use
is the problem Saravana was trying to solve with the priority list of
parents
How to encode that information in DT is a bit of a problem. I guess
one approach could be to create a mux table in DT and instead of
ordering items in that list based on bitfield index (which feel
natural to most programmers), instead let the order imply priority.
This is a tricky hack since we're *technically* only describing the
hardware with the mux table, but the priority encoding is silently
encoded. E.g:
clock: clock@4a008100 {
#clock-cells = 0;
compatible = mux-clock;
clocks = clock_foo, clock_bar, clock_baz;
reg = 0x4a008100 0x4;
mask = 0x3;
shift = 0;
table = clock_baz 0x2, clock_foo 0x0, clock_bar 0x1;
};
Thoughts?
Regards,
Mike
-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
Hi,
Mike pointed me at this series since I'm running into parenting
problems at the moment as well...
On Mon, May 20, 2013 at 9:44 PM, Saravana Kannan wrote:
> While writing a similar code for our internal tree, I quickly came to the
> realization that, "all parents are equal, but some are more equal than
> others". The simplest example is a clock tree like this:
>
> Source -> Divider -> Mux
> Source -> Mux
>
> A rate of Y can be achieved for Mux by either running Source at Y and
> picking that input or running Source at Y * 2 and Divider set to div-2 and
> picking the Divider input.
>
> The solution seems to be a priority list of parents. I'm sure there would be
> other reason (jitter, clock quality, etc) for a mux to pick one parent vs.
> another when both of them can provide the required rate.
>
> I think this loop should loop over parents based on their priority order.
> So, parents should become a struct of { clk, index } and have the parents
> listed in the order of priority. Well, at least in that long run that would
> be better to avoid messing up parent/index values. But for now, you could
> just have a priority array of index or parents.
>
> It might not fit 100% of the cases where two parents can provide the same
> rate, but it should fit most use cases.
I'm slightly worried about similar problems, but I don't have a really
great solution.
In my case I'm working on exynos5 hardware which has a bunch of PLLs
and a crazy number of muxing options. Many of the muxing options are
not really meant to be used but seem to have been added to the SoC as
a "backup plan" of some sort. :-/ Most of the PLLs are intended to
be used for one purpose and one purpose only though muxing options
allow them to be used all over the place.
For instance if I look at my current bootup of exynos5250-snow, I see:
fout_apll/clk_rate:17
fout_bpll/clk_rate:8
fout_cpll/clk_rate:33300
fout_epll/clk_rate:45158401
fout_gpll/clk_rate:53300
fout_mpll/clk_rate:16
* APLL is intended to be the parent of the 2 ARM cores and changes due
to cpu load
* EPLL is intended to be the parent for audio and changes dpending on
audio playback rates.
* GPLL is intended to be the parent of the GPU and changes due to gpu load
* VPLL is intended to be the parent for video related things and could
change depending on the LCD.
* MPLL doesn't change a lot and is intended to be the parent for most things.
* In some systems BPLL can be used for memory or GPU
My main concern here is the CCF will end up deciding at some point
that it should reparent some clock onto a PLL that is going to change
a whole lot. Maybe the user will plug in an SD card that requests a
frequency of 52MHz and at the moment we'll be running EPLL at 104 MHz
so it will be a perfect match! ...but then the user wants to play
audio at a different rate. The audio code assumes that it can mess
with its clock and we've got code setup to call CLK_SET_RATE_PARENT
all the way up to EPLL. That will really mess with the SD card.
Really we'd rather just have the SD card clock always parented on the
stable MPLL and it's OK if 52MHz gets rounded down to 50MHz.
Of course, on another board maybe they don't have an audio codec and
aren't using epll for audio and have realized that EPLL would be a
perfect way to get their SD card to run 4% faster. It ought to work.
I guess to summarize the above:
* It seems like much of the muxing on exynos5250 is just too
complicated to leave it to an simple automated algorithm.
* It seems like we can't make muxing decisions on the SoC level.
* Your automatic muxing patches don't hurt me and could be useful for
_some_ of the muxing options, just not the top PLL ones.
...but the only place that leaves me for my muxing needs is the device
tree. ...and as Mike pointed out on IRC the device tree should
describe hardware, not policy. Ick.
-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
Hi,
Mike pointed me at this series since I'm running into parenting
problems at the moment as well...
On Mon, May 20, 2013 at 9:44 PM, Saravana Kannan skan...@codeaurora.org wrote:
While writing a similar code for our internal tree, I quickly came to the
realization that, all parents are equal, but some are more equal than
others. The simplest example is a clock tree like this:
Source - Divider - Mux
Source - Mux
A rate of Y can be achieved for Mux by either running Source at Y and
picking that input or running Source at Y * 2 and Divider set to div-2 and
picking the Divider input.
The solution seems to be a priority list of parents. I'm sure there would be
other reason (jitter, clock quality, etc) for a mux to pick one parent vs.
another when both of them can provide the required rate.
I think this loop should loop over parents based on their priority order.
So, parents should become a struct of { clk, index } and have the parents
listed in the order of priority. Well, at least in that long run that would
be better to avoid messing up parent/index values. But for now, you could
just have a priority array of index or parents.
It might not fit 100% of the cases where two parents can provide the same
rate, but it should fit most use cases.
I'm slightly worried about similar problems, but I don't have a really
great solution.
In my case I'm working on exynos5 hardware which has a bunch of PLLs
and a crazy number of muxing options. Many of the muxing options are
not really meant to be used but seem to have been added to the SoC as
a backup plan of some sort. :-/ Most of the PLLs are intended to
be used for one purpose and one purpose only though muxing options
allow them to be used all over the place.
For instance if I look at my current bootup of exynos5250-snow, I see:
fout_apll/clk_rate:17
fout_bpll/clk_rate:8
fout_cpll/clk_rate:33300
fout_epll/clk_rate:45158401
fout_gpll/clk_rate:53300
fout_mpll/clk_rate:16
* APLL is intended to be the parent of the 2 ARM cores and changes due
to cpu load
* EPLL is intended to be the parent for audio and changes dpending on
audio playback rates.
* GPLL is intended to be the parent of the GPU and changes due to gpu load
* VPLL is intended to be the parent for video related things and could
change depending on the LCD.
* MPLL doesn't change a lot and is intended to be the parent for most things.
* In some systems BPLL can be used for memory or GPU
My main concern here is the CCF will end up deciding at some point
that it should reparent some clock onto a PLL that is going to change
a whole lot. Maybe the user will plug in an SD card that requests a
frequency of 52MHz and at the moment we'll be running EPLL at 104 MHz
so it will be a perfect match! ...but then the user wants to play
audio at a different rate. The audio code assumes that it can mess
with its clock and we've got code setup to call CLK_SET_RATE_PARENT
all the way up to EPLL. That will really mess with the SD card.
Really we'd rather just have the SD card clock always parented on the
stable MPLL and it's OK if 52MHz gets rounded down to 50MHz.
Of course, on another board maybe they don't have an audio codec and
aren't using epll for audio and have realized that EPLL would be a
perfect way to get their SD card to run 4% faster. It ought to work.
I guess to summarize the above:
* It seems like much of the muxing on exynos5250 is just too
complicated to leave it to an simple automated algorithm.
* It seems like we can't make muxing decisions on the SoC level.
* Your automatic muxing patches don't hurt me and could be useful for
_some_ of the muxing options, just not the top PLL ones.
...but the only place that leaves me for my muxing needs is the device
tree. ...and as Mike pointed out on IRC the device tree should
describe hardware, not policy. Ick.
-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
On 05/20/2013 06:28 AM, James Hogan wrote:
Implement clk-mux remuxing if the CLK_SET_RATE_NO_REPARENT flag isn't
set. This implements determine_rate for clk-mux to propagate to each
parent and to choose the best one (like clk-divider this chooses the
parent which provides the fastest rate <= the requested rate).
The determine_rate op is implemented as a core helper function so that
it can be easily used by more complex clocks which incorporate muxes.
Signed-off-by: James Hogan
Cc: Mike Turquette
Cc: linux-arm-ker...@lists.infradead.org
---
Changes in v4:
* never pass NULL to determine_rate's best_parent_clk parameter.
Changes in v3:
* rename/invert CLK_SET_RATE_REMUX to CLK_SET_RATE_NO_REPARENT and move
to patch 3.
drivers/clk/clk-mux.c| 1 +
drivers/clk/clk.c| 49
include/linux/clk-provider.h | 3 +++
3 files changed, 53 insertions(+)
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 25b1734..cecfa01 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -100,6 +100,7 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
const struct clk_ops clk_mux_ops = {
.get_parent = clk_mux_get_parent,
.set_parent = clk_mux_set_parent,
+ .determine_rate = __clk_mux_determine_rate,
};
EXPORT_SYMBOL_GPL(clk_mux_ops);
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 3ce4810..85b661d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -692,6 +692,55 @@ struct clk *__clk_lookup(const char *name)
return NULL;
}
+/*
+ * Helper for finding best parent to provide a given frequency. This can be
used
+ * directly as a determine_rate callback (e.g. for a mux), or from a more
+ * complex clock that may combine a mux with other operations.
+ */
+long __clk_mux_determine_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *best_parent_rate,
+ struct clk **best_parent_p)
+{
+ struct clk *clk = hw->clk, *parent, *best_parent = NULL;
+ int i, num_parents;
+ unsigned long parent_rate, best = 0;
+
+ /* if NO_REPARENT flag set, pass through to current parent */
+ if (clk->flags & CLK_SET_RATE_NO_REPARENT) {
+ parent = clk->parent;
+ if (clk->flags & CLK_SET_RATE_PARENT)
+ best = __clk_round_rate(parent, rate);
+ else if (parent)
+ best = __clk_get_rate(parent);
+ else
+ best = __clk_get_rate(clk);
+ goto out;
+ }
+
+ /* find the parent that can provide the fastest rate <= rate */
+ num_parents = clk->num_parents;
+ for (i = 0; i < num_parents; i++) {
While writing a similar code for our internal tree, I quickly came to
the realization that, "all parents are equal, but some are more equal
than others". The simplest example is a clock tree like this:
Source -> Divider -> Mux
Source -> Mux
A rate of Y can be achieved for Mux by either running Source at Y and
picking that input or running Source at Y * 2 and Divider set to div-2
and picking the Divider input.
The solution seems to be a priority list of parents. I'm sure there
would be other reason (jitter, clock quality, etc) for a mux to pick one
parent vs. another when both of them can provide the required rate.
I think this loop should loop over parents based on their priority
order. So, parents should become a struct of { clk, index } and have the
parents listed in the order of priority. Well, at least in that long run
that would be better to avoid messing up parent/index values. But for
now, you could just have a priority array of index or parents.
It might not fit 100% of the cases where two parents can provide the
same rate, but it should fit most use cases.
+ parent = clk_get_parent_by_index(clk, i);
+ if (!parent)
+ continue;
+ if (clk->flags & CLK_SET_RATE_PARENT)
+ parent_rate = __clk_round_rate(parent, rate);
+ else
+ parent_rate = __clk_get_rate(parent);
+ if (parent_rate <= rate && parent_rate > best) {
+ best_parent = parent;
+ best = parent_rate;
+ }
+ }
+
+out:
+ if (best_parent)
+ *best_parent_p = best_parent;
+ *best_parent_rate = best;
+
+ return best;
+}
+
Thanks,
Saravana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 5/5] clk: clk-mux: implement remuxing on set_rate
On 05/20/2013 06:28 AM, James Hogan wrote:
Implement clk-mux remuxing if the CLK_SET_RATE_NO_REPARENT flag isn't
set. This implements determine_rate for clk-mux to propagate to each
parent and to choose the best one (like clk-divider this chooses the
parent which provides the fastest rate = the requested rate).
The determine_rate op is implemented as a core helper function so that
it can be easily used by more complex clocks which incorporate muxes.
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Mike Turquette mturque...@linaro.org
Cc: linux-arm-ker...@lists.infradead.org
---
Changes in v4:
* never pass NULL to determine_rate's best_parent_clk parameter.
Changes in v3:
* rename/invert CLK_SET_RATE_REMUX to CLK_SET_RATE_NO_REPARENT and move
to patch 3.
drivers/clk/clk-mux.c| 1 +
drivers/clk/clk.c| 49
include/linux/clk-provider.h | 3 +++
3 files changed, 53 insertions(+)
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 25b1734..cecfa01 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -100,6 +100,7 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
const struct clk_ops clk_mux_ops = {
.get_parent = clk_mux_get_parent,
.set_parent = clk_mux_set_parent,
+ .determine_rate = __clk_mux_determine_rate,
};
EXPORT_SYMBOL_GPL(clk_mux_ops);
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 3ce4810..85b661d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -692,6 +692,55 @@ struct clk *__clk_lookup(const char *name)
return NULL;
}
+/*
+ * Helper for finding best parent to provide a given frequency. This can be
used
+ * directly as a determine_rate callback (e.g. for a mux), or from a more
+ * complex clock that may combine a mux with other operations.
+ */
+long __clk_mux_determine_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *best_parent_rate,
+ struct clk **best_parent_p)
+{
+ struct clk *clk = hw-clk, *parent, *best_parent = NULL;
+ int i, num_parents;
+ unsigned long parent_rate, best = 0;
+
+ /* if NO_REPARENT flag set, pass through to current parent */
+ if (clk-flags CLK_SET_RATE_NO_REPARENT) {
+ parent = clk-parent;
+ if (clk-flags CLK_SET_RATE_PARENT)
+ best = __clk_round_rate(parent, rate);
+ else if (parent)
+ best = __clk_get_rate(parent);
+ else
+ best = __clk_get_rate(clk);
+ goto out;
+ }
+
+ /* find the parent that can provide the fastest rate = rate */
+ num_parents = clk-num_parents;
+ for (i = 0; i num_parents; i++) {
While writing a similar code for our internal tree, I quickly came to
the realization that, all parents are equal, but some are more equal
than others. The simplest example is a clock tree like this:
Source - Divider - Mux
Source - Mux
A rate of Y can be achieved for Mux by either running Source at Y and
picking that input or running Source at Y * 2 and Divider set to div-2
and picking the Divider input.
The solution seems to be a priority list of parents. I'm sure there
would be other reason (jitter, clock quality, etc) for a mux to pick one
parent vs. another when both of them can provide the required rate.
I think this loop should loop over parents based on their priority
order. So, parents should become a struct of { clk, index } and have the
parents listed in the order of priority. Well, at least in that long run
that would be better to avoid messing up parent/index values. But for
now, you could just have a priority array of index or parents.
It might not fit 100% of the cases where two parents can provide the
same rate, but it should fit most use cases.
+ parent = clk_get_parent_by_index(clk, i);
+ if (!parent)
+ continue;
+ if (clk-flags CLK_SET_RATE_PARENT)
+ parent_rate = __clk_round_rate(parent, rate);
+ else
+ parent_rate = __clk_get_rate(parent);
+ if (parent_rate = rate parent_rate best) {
+ best_parent = parent;
+ best = parent_rate;
+ }
+ }
+
+out:
+ if (best_parent)
+ *best_parent_p = best_parent;
+ *best_parent_rate = best;
+
+ return best;
+}
+
Thanks,
Saravana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
