Re: [PATCH 0/4] clk: mvebu: fix clk init order
Hi Gregory, El 18/02/14 06:47, Gregory CLEMENT escribió: (snip) (...) what would be an acceptable version would be the something like the patch attached. There will be still an issue if old dtb is used with recent kernel, but at least the user will be warned. The patch you attached is similar in spirit to what I suggested, but with way more warnings sprinkled around. I don't really mind either way, and if you prefer big warnings so be it; it's your driver after all :-) This code only fix the Armada 370 case, a complete solution should modify the dtsi for Armada XP, Armada 375, Armada 38x, Kirkwood and Dove. It should also make the outputname mandatory for gate-clk for consistency. Again, all of them are your calls. Fix the issue as you see fit; as long as it's a technically sound I'm ok with it. But please don't reinvent the wheel on framework code under a principle that has no proven reason to be to cover up for a buggy driver. Cheers, and have a great Wednesday :) Emilio PS: I'd really appreciate it if you could keep me cc'ed on respins of patches I comment on in the future. -- 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 0/4] clk: mvebu: fix clk init order
On Tue, Feb 18, 2014 at 10:47:00AM +0100, Gregory CLEMENT wrote:
> On 17/02/2014 19:19, Ezequiel Garcia wrote:
> > On Mon, Feb 17, 2014 at 04:59:01PM +0100, Gregory CLEMENT wrote:
> > [..]
> >>>
> >>> Right. If you think it adds a regression, then that's a perfectly valid
> >>> reasons
> >>> for nacking.
> >>>
> >>> However, I'd like to double-check we have such a regression. I guess
> >>> you're
> >>> talking about the "tclk" name being hardcoded. IMHO, it's hardcoded in the
> >>> driver in the first place:
> >>>
> >>> void __init mvebu_coreclk_setup(struct device_node *np,
> >>> const struct coreclk_soc_desc *desc)
> >>> {
> >>> const char *tclk_name = "tclk";
> >>> [..]
> >>
> >>
> >> Here it is just about giving a name to a clock. As in the device tree
> >> we only refer to the clock by index, the name don't matter.
> >>
> >
> > Unless I'm really confused about what's the problem here, the *name* is
> > *all* that matters.
> >
> > We're having a clock *registration* order issue (which is different from
> > clock
> > enable). Let me try to explain the issue, in case it's still not clear.
> >
> > I'll stick to the current specific problem but it can apply to any other
> > pair of parent/child.
> >
> > Some of the mvebu clocks are registered as part of the "core" clock
> > group, modeled by the "marvell,armada-370-core-clock" compatible node.
> >
> > Another group of mvebu clocks are registered as part of the "gating"
> > clock group, modeled by the "marvell,armada-370-gating-clock" compatible
> > node.
> >
> > By default, all the gating clocks are child of the first registered core
> > clock. This clock is named "tclk" by default, and this name can be
> > overloaded
> > in the devicetree.
> >
> > So far, so good, right?
> >
> > The issue we're trying to fix, arises from the mvebu_clk_gating_setup()
> > trying to get the name of this "tclk", as a registered clock.
> >
> > In other words, the current code needs the tclk to be registered, for it
> > will ask his name like this:
> >
> > const char *default_parent = NULL;
> >
> > clk = of_clk_get(np, 0);
> > if (!IS_ERR(clk)) {
> > default_parent = __clk_get_name(clk);
> > clk_put(clk);
> > }
> >
> > Once it gets the name, all goes smoothly. Notice how the clock is obtained
> > for the sole purpose of getting the name of it, which shows clearly it's the
> > *name* that matters.
> >
> > The ordering issue happens when the gating clock group is probed, before
> > the core clock group. In that case, it's not possible to get the
> > " 0" (which is wrongly assumed to be registered), and so it's
> > not possible to get the name.
> >
> > So the root of the problem is that snippet above, which adds a
> > completely unneeded registration order requirement. Instead, we should
> > be looking for the names: we can hardcode the name or fetch it from the
> > devicetree (Emilio has posted patches for both).
> >
> > I really don't see why we're not fixing this, instead of adding yet
> > another layer of complexity to the problem.
>
> All this have already discussed in the previous emails. And even if Emilio
> denied introducing a regression, it was what the code did. See my example
> here:
> http://article.gmane.org/gmane.linux.kernel/1649439
>
> Now as you both are really annoying with it, what would be an acceptable
> version would be the something like the patch attached. There will be still
> an issue if old dtb is used with recent kernel, but at least the user will
> be warned.
>
The regression you're talking about is will happen if the user decides to set
an awkward parent as the *default* parent of the gate clock group.
It's just the *default* that's specified in the devicetree, not the actual
parent,
since we've designed this so a particular clock parent can always be specified
from
the array in the driver.
> This code only fix the Armada 370 case, a complete solution should modify
> the dtsi for Armada XP, Armada 375, Armada 38x, Kirkwood and Dove. It should
> also make the outputname mandatory for gate-clk for consistency.
>
I think you're missing the point in discussion. If you propose a patch
for the mvebu clock driver, then I guess you admit the problem is
solvable in the driver.
So, the real questions are:
Do we want to enforce a clock registration order?
Is this a framework defect? Or are our mvebu clocks doing things wrong?
As I tried to explained in detailed above, I think the mvebu clocks are doing
really evil things by needing a registered clock, just so it can retrieve the
default clock *name*. It's not even the clock name, given we allow to
override such default name.
A clock never needed a parent clock to be registered to be able to
register itself, it can just use the parent's clock name.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line
Re: [PATCH 0/4] clk: mvebu: fix clk init order
On Tue, Feb 18, 2014 at 10:47:00AM +0100, Gregory CLEMENT wrote:
On 17/02/2014 19:19, Ezequiel Garcia wrote:
On Mon, Feb 17, 2014 at 04:59:01PM +0100, Gregory CLEMENT wrote:
[..]
Right. If you think it adds a regression, then that's a perfectly valid
reasons
for nacking.
However, I'd like to double-check we have such a regression. I guess
you're
talking about the tclk name being hardcoded. IMHO, it's hardcoded in the
driver in the first place:
void __init mvebu_coreclk_setup(struct device_node *np,
const struct coreclk_soc_desc *desc)
{
const char *tclk_name = tclk;
[..]
Here it is just about giving a name to a clock. As in the device tree
we only refer to the clock by index, the name don't matter.
Unless I'm really confused about what's the problem here, the *name* is
*all* that matters.
We're having a clock *registration* order issue (which is different from
clock
enable). Let me try to explain the issue, in case it's still not clear.
I'll stick to the current specific problem but it can apply to any other
pair of parent/child.
Some of the mvebu clocks are registered as part of the core clock
group, modeled by the marvell,armada-370-core-clock compatible node.
Another group of mvebu clocks are registered as part of the gating
clock group, modeled by the marvell,armada-370-gating-clock compatible
node.
By default, all the gating clocks are child of the first registered core
clock. This clock is named tclk by default, and this name can be
overloaded
in the devicetree.
So far, so good, right?
The issue we're trying to fix, arises from the mvebu_clk_gating_setup()
trying to get the name of this tclk, as a registered clock.
In other words, the current code needs the tclk to be registered, for it
will ask his name like this:
const char *default_parent = NULL;
clk = of_clk_get(np, 0);
if (!IS_ERR(clk)) {
default_parent = __clk_get_name(clk);
clk_put(clk);
}
Once it gets the name, all goes smoothly. Notice how the clock is obtained
for the sole purpose of getting the name of it, which shows clearly it's the
*name* that matters.
The ordering issue happens when the gating clock group is probed, before
the core clock group. In that case, it's not possible to get the
coreclk 0 (which is wrongly assumed to be registered), and so it's
not possible to get the name.
So the root of the problem is that snippet above, which adds a
completely unneeded registration order requirement. Instead, we should
be looking for the names: we can hardcode the name or fetch it from the
devicetree (Emilio has posted patches for both).
I really don't see why we're not fixing this, instead of adding yet
another layer of complexity to the problem.
All this have already discussed in the previous emails. And even if Emilio
denied introducing a regression, it was what the code did. See my example
here:
http://article.gmane.org/gmane.linux.kernel/1649439
Now as you both are really annoying with it, what would be an acceptable
version would be the something like the patch attached. There will be still
an issue if old dtb is used with recent kernel, but at least the user will
be warned.
The regression you're talking about is will happen if the user decides to set
an awkward parent as the *default* parent of the gate clock group.
It's just the *default* that's specified in the devicetree, not the actual
parent,
since we've designed this so a particular clock parent can always be specified
from
the array in the driver.
This code only fix the Armada 370 case, a complete solution should modify
the dtsi for Armada XP, Armada 375, Armada 38x, Kirkwood and Dove. It should
also make the outputname mandatory for gate-clk for consistency.
I think you're missing the point in discussion. If you propose a patch
for the mvebu clock driver, then I guess you admit the problem is
solvable in the driver.
So, the real questions are:
Do we want to enforce a clock registration order?
Is this a framework defect? Or are our mvebu clocks doing things wrong?
As I tried to explained in detailed above, I think the mvebu clocks are doing
really evil things by needing a registered clock, just so it can retrieve the
default clock *name*. It's not even the clock name, given we allow to
override such default name.
A clock never needed a parent clock to be registered to be able to
register itself, it can just use the parent's clock name.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
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 0/4] clk: mvebu: fix clk init order
Hi Gregory, El 18/02/14 06:47, Gregory CLEMENT escribió: (snip) (...) what would be an acceptable version would be the something like the patch attached. There will be still an issue if old dtb is used with recent kernel, but at least the user will be warned. The patch you attached is similar in spirit to what I suggested, but with way more warnings sprinkled around. I don't really mind either way, and if you prefer big warnings so be it; it's your driver after all :-) This code only fix the Armada 370 case, a complete solution should modify the dtsi for Armada XP, Armada 375, Armada 38x, Kirkwood and Dove. It should also make the outputname mandatory for gate-clk for consistency. Again, all of them are your calls. Fix the issue as you see fit; as long as it's a technically sound I'm ok with it. But please don't reinvent the wheel on framework code under a principle that has no proven reason to be to cover up for a buggy driver. Cheers, and have a great Wednesday :) Emilio PS: I'd really appreciate it if you could keep me cc'ed on respins of patches I comment on in the future. -- 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 0/4] clk: mvebu: fix clk init order
On 17/02/2014 19:19, Ezequiel Garcia wrote:
> On Mon, Feb 17, 2014 at 04:59:01PM +0100, Gregory CLEMENT wrote:
> [..]
>>>
>>> Right. If you think it adds a regression, then that's a perfectly valid
>>> reasons
>>> for nacking.
>>>
>>> However, I'd like to double-check we have such a regression. I guess you're
>>> talking about the "tclk" name being hardcoded. IMHO, it's hardcoded in the
>>> driver in the first place:
>>>
>>> void __init mvebu_coreclk_setup(struct device_node *np,
>>> const struct coreclk_soc_desc *desc)
>>> {
>>> const char *tclk_name = "tclk";
>>> [..]
>>
>>
>> Here it is just about giving a name to a clock. As in the device tree
>> we only refer to the clock by index, the name don't matter.
>>
>
> Unless I'm really confused about what's the problem here, the *name* is
> *all* that matters.
>
> We're having a clock *registration* order issue (which is different from clock
> enable). Let me try to explain the issue, in case it's still not clear.
>
> I'll stick to the current specific problem but it can apply to any other
> pair of parent/child.
>
> Some of the mvebu clocks are registered as part of the "core" clock
> group, modeled by the "marvell,armada-370-core-clock" compatible node.
>
> Another group of mvebu clocks are registered as part of the "gating"
> clock group, modeled by the "marvell,armada-370-gating-clock" compatible
> node.
>
> By default, all the gating clocks are child of the first registered core
> clock. This clock is named "tclk" by default, and this name can be overloaded
> in the devicetree.
>
> So far, so good, right?
>
> The issue we're trying to fix, arises from the mvebu_clk_gating_setup()
> trying to get the name of this "tclk", as a registered clock.
>
> In other words, the current code needs the tclk to be registered, for it
> will ask his name like this:
>
> const char *default_parent = NULL;
>
> clk = of_clk_get(np, 0);
> if (!IS_ERR(clk)) {
> default_parent = __clk_get_name(clk);
> clk_put(clk);
> }
>
> Once it gets the name, all goes smoothly. Notice how the clock is obtained
> for the sole purpose of getting the name of it, which shows clearly it's the
> *name* that matters.
>
> The ordering issue happens when the gating clock group is probed, before
> the core clock group. In that case, it's not possible to get the
> " 0" (which is wrongly assumed to be registered), and so it's
> not possible to get the name.
>
> So the root of the problem is that snippet above, which adds a
> completely unneeded registration order requirement. Instead, we should
> be looking for the names: we can hardcode the name or fetch it from the
> devicetree (Emilio has posted patches for both).
>
> I really don't see why we're not fixing this, instead of adding yet
> another layer of complexity to the problem.
All this have already discussed in the previous emails. And even if Emilio
denied introducing a regression, it was what the code did. See my example
here:
http://article.gmane.org/gmane.linux.kernel/1649439
Now as you both are really annoying with it, what would be an acceptable
version would be the something like the patch attached. There will be still
an issue if old dtb is used with recent kernel, but at least the user will
be warned.
This code only fix the Armada 370 case, a complete solution should modify
the dtsi for Armada XP, Armada 375, Armada 38x, Kirkwood and Dove. It should
also make the outputname mandatory for gate-clk for consistency.
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
>From ca82f66b8fbb8247b0ec2c407726105e1e1af419 Mon Sep 17 00:00:00 2001
From: Gregory CLEMENT
Date: Tue, 18 Feb 2014 10:38:08 +0100
Subject: [PATCH] [RFC] clk:mvebu: make clock-output-names mandatory
Signed-off-by: Gregory CLEMENT
---
.../devicetree/bindings/clock/mvebu-core-clock.txt | 7 ++--
arch/arm/boot/dts/armada-370.dtsi | 1 +
drivers/clk/mvebu/common.c | 38 +++---
3 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
index 307a503c5db8..4f2e3953b6a6 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
@@ -40,16 +40,15 @@ Required properties:
"marvell,mv88f6180-core-clock" - for Kirkwood MV88f6180 SoC
- reg : shall be the register address of the Sample-At-Reset (SAR) register
- #clock-cells : from common clock binding; shall be set to 1
-
-Optional properties:
-- clock-output-names : from common clock binding; allows overwrite default clock
- output names ("tclk", "cpuclk", "l2clk", "ddrclk")
+- clock-output-names : from common clock binding; should be the clock
+ output names
Re: [PATCH 0/4] clk: mvebu: fix clk init order
On 17/02/2014 19:19, Ezequiel Garcia wrote:
On Mon, Feb 17, 2014 at 04:59:01PM +0100, Gregory CLEMENT wrote:
[..]
Right. If you think it adds a regression, then that's a perfectly valid
reasons
for nacking.
However, I'd like to double-check we have such a regression. I guess you're
talking about the tclk name being hardcoded. IMHO, it's hardcoded in the
driver in the first place:
void __init mvebu_coreclk_setup(struct device_node *np,
const struct coreclk_soc_desc *desc)
{
const char *tclk_name = tclk;
[..]
Here it is just about giving a name to a clock. As in the device tree
we only refer to the clock by index, the name don't matter.
Unless I'm really confused about what's the problem here, the *name* is
*all* that matters.
We're having a clock *registration* order issue (which is different from clock
enable). Let me try to explain the issue, in case it's still not clear.
I'll stick to the current specific problem but it can apply to any other
pair of parent/child.
Some of the mvebu clocks are registered as part of the core clock
group, modeled by the marvell,armada-370-core-clock compatible node.
Another group of mvebu clocks are registered as part of the gating
clock group, modeled by the marvell,armada-370-gating-clock compatible
node.
By default, all the gating clocks are child of the first registered core
clock. This clock is named tclk by default, and this name can be overloaded
in the devicetree.
So far, so good, right?
The issue we're trying to fix, arises from the mvebu_clk_gating_setup()
trying to get the name of this tclk, as a registered clock.
In other words, the current code needs the tclk to be registered, for it
will ask his name like this:
const char *default_parent = NULL;
clk = of_clk_get(np, 0);
if (!IS_ERR(clk)) {
default_parent = __clk_get_name(clk);
clk_put(clk);
}
Once it gets the name, all goes smoothly. Notice how the clock is obtained
for the sole purpose of getting the name of it, which shows clearly it's the
*name* that matters.
The ordering issue happens when the gating clock group is probed, before
the core clock group. In that case, it's not possible to get the
coreclk 0 (which is wrongly assumed to be registered), and so it's
not possible to get the name.
So the root of the problem is that snippet above, which adds a
completely unneeded registration order requirement. Instead, we should
be looking for the names: we can hardcode the name or fetch it from the
devicetree (Emilio has posted patches for both).
I really don't see why we're not fixing this, instead of adding yet
another layer of complexity to the problem.
All this have already discussed in the previous emails. And even if Emilio
denied introducing a regression, it was what the code did. See my example
here:
http://article.gmane.org/gmane.linux.kernel/1649439
Now as you both are really annoying with it, what would be an acceptable
version would be the something like the patch attached. There will be still
an issue if old dtb is used with recent kernel, but at least the user will
be warned.
This code only fix the Armada 370 case, a complete solution should modify
the dtsi for Armada XP, Armada 375, Armada 38x, Kirkwood and Dove. It should
also make the outputname mandatory for gate-clk for consistency.
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
From ca82f66b8fbb8247b0ec2c407726105e1e1af419 Mon Sep 17 00:00:00 2001
From: Gregory CLEMENT gregory.clem...@free-electrons.com
Date: Tue, 18 Feb 2014 10:38:08 +0100
Subject: [PATCH] [RFC] clk:mvebu: make clock-output-names mandatory
Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com
---
.../devicetree/bindings/clock/mvebu-core-clock.txt | 7 ++--
arch/arm/boot/dts/armada-370.dtsi | 1 +
drivers/clk/mvebu/common.c | 38 +++---
3 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
index 307a503c5db8..4f2e3953b6a6 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
@@ -40,16 +40,15 @@ Required properties:
marvell,mv88f6180-core-clock - for Kirkwood MV88f6180 SoC
- reg : shall be the register address of the Sample-At-Reset (SAR) register
- #clock-cells : from common clock binding; shall be set to 1
-
-Optional properties:
-- clock-output-names : from common clock binding; allows overwrite default clock
- output names (tclk, cpuclk, l2clk, ddrclk)
+- clock-output-names : from common clock binding; should be the clock
+ output names given above
Example:
core_clk: core-clocks@d0214 {
Re: [PATCH 0/4] clk: mvebu: fix clk init order
On Mon, Feb 17, 2014 at 04:59:01PM +0100, Gregory CLEMENT wrote:
[..]
> >
> > Right. If you think it adds a regression, then that's a perfectly valid
> > reasons
> > for nacking.
> >
> > However, I'd like to double-check we have such a regression. I guess you're
> > talking about the "tclk" name being hardcoded. IMHO, it's hardcoded in the
> > driver in the first place:
> >
> > void __init mvebu_coreclk_setup(struct device_node *np,
> > const struct coreclk_soc_desc *desc)
> > {
> > const char *tclk_name = "tclk";
> > [..]
>
>
> Here it is just about giving a name to a clock. As in the device tree
> we only refer to the clock by index, the name don't matter.
>
Unless I'm really confused about what's the problem here, the *name* is
*all* that matters.
We're having a clock *registration* order issue (which is different from clock
enable). Let me try to explain the issue, in case it's still not clear.
I'll stick to the current specific problem but it can apply to any other
pair of parent/child.
Some of the mvebu clocks are registered as part of the "core" clock
group, modeled by the "marvell,armada-370-core-clock" compatible node.
Another group of mvebu clocks are registered as part of the "gating"
clock group, modeled by the "marvell,armada-370-gating-clock" compatible
node.
By default, all the gating clocks are child of the first registered core
clock. This clock is named "tclk" by default, and this name can be overloaded
in the devicetree.
So far, so good, right?
The issue we're trying to fix, arises from the mvebu_clk_gating_setup()
trying to get the name of this "tclk", as a registered clock.
In other words, the current code needs the tclk to be registered, for it
will ask his name like this:
const char *default_parent = NULL;
clk = of_clk_get(np, 0);
if (!IS_ERR(clk)) {
default_parent = __clk_get_name(clk);
clk_put(clk);
}
Once it gets the name, all goes smoothly. Notice how the clock is obtained
for the sole purpose of getting the name of it, which shows clearly it's the
*name* that matters.
The ordering issue happens when the gating clock group is probed, before
the core clock group. In that case, it's not possible to get the
" 0" (which is wrongly assumed to be registered), and so it's
not possible to get the name.
So the root of the problem is that snippet above, which adds a
completely unneeded registration order requirement. Instead, we should
be looking for the names: we can hardcode the name or fetch it from the
devicetree (Emilio has posted patches for both).
I really don't see why we're not fixing this, instead of adding yet
another layer of complexity to the problem.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
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 0/4] clk: mvebu: fix clk init order
On 17/02/2014 16:44, Ezequiel Garcia wrote:
> On Mon, Feb 17, 2014 at 04:28:41PM +0100, Gregory CLEMENT wrote:
>> On 17/02/2014 16:21, Ezequiel Garcia wrote:
>>> On Mon, Feb 17, 2014 at 03:25:22PM +0100, Gregory CLEMENT wrote:
On 17/02/2014 15:13, Ezequiel Garcia wrote:
> On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote:
>> On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote:
>>> This patch set fixes clk init order that went upside-down with
>>> v3.14. I haven't really investigated what caused this, but I assume
>>> it is related with DT node reordering by addresses.
>>>
>>> Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets
>>> registered before core clocks driver. Unfortunately, we cannot
>>> return -EPROBE_DEFER in drivers initialized by clk_of_init. As the
>>> init order for our drivers is always core clocks before clock gating,
>>> we maintain init order ourselves by hooking CLK_OF_DECLARE to one
>>> init function that will register core clocks before clock gating
>>> driver.
>>>
>>> This patch is based on pre-v3.14-rc1 mainline and should go in as
>>> fixes for it. As we now send MVEBU clk pull-requests to Mike directly,
>>> I suggest Jason picks it up as a topic branch.
>>>
>>> The patches have been boot tested on Dove and compile-tested only
>>> for Kirkwood, Armada 370 and XP.
>>>
>>> Sebastian Hesselbarth (4):
>>> clk: mvebu: armada-370: maintain clock init order
>>> clk: mvebu: armada-xp: maintain clock init order
>>> clk: mvebu: dove: maintain clock init order
>>> clk: mvebu: kirkwood: maintain clock init order
>>>
>>> drivers/clk/mvebu/armada-370.c | 21 ++---
>>> drivers/clk/mvebu/armada-xp.c | 20 +---
>>> drivers/clk/mvebu/dove.c | 19 +--
>>> drivers/clk/mvebu/kirkwood.c | 34 --
>>> 4 files changed, 44 insertions(+), 50 deletions(-)
>>
>> Whole series applied to mvebu/clk-fixes.
>>
>
> Are we still in time to consider Emilio's oneline proposal?
> (Emilio: would you mind preparing a suitable patch against dove,
> kirkwood, armada370/xp, so we can see the real thing?).
I am still strongly against this proposal because hard-coded the parent
clock name in the driver seems very wrong and moreover in some
circumstances
(if there is no output-name, which is our default case) this proposal
just ignored the parent clock given by the device tree and this looked
more wrong.
>>>
>>> So you're against the proposal as a permanent fix, *and* against the
>>> proposal as a workaround fix?
>>
>> Yes
>>
>>>
>
> Sebastian fix works perfect, and it easy to understand. However, it has
> quite a large diffstat. When compared to Emilio's oneline proposal, it
> seems to me it would be preferable, unless it's broken.
>
> Workaround or not, the fact is this code will be in v3.14, so maybe we
> can spend some time considering a cleaner option.
>
>>>
>>> Before discussing the solution as compared to your for-v3.15 clock
>>> registration order patch, I wanted to trigger some discussion around
>>> replacing this big and intrusive workaround with Emilio's oneline fix.
>>>
>>> Let's suppose we're considering them as workaround to live just one or
>>> two releases. Wouldn't it be better to take the least instrusive?
>>>
>>
>> The better solution is the one which doesn't add another regression and until
>> today I though we had an agreement to use the patch set from Sebastian. If
>> I remember well Jason had sent a pull request for it.
>>
>>
>
> Right. If you think it adds a regression, then that's a perfectly valid
> reasons
> for nacking.
>
> However, I'd like to double-check we have such a regression. I guess you're
> talking about the "tclk" name being hardcoded. IMHO, it's hardcoded in the
> driver in the first place:
>
> void __init mvebu_coreclk_setup(struct device_node *np,
> const struct coreclk_soc_desc *desc)
> {
> const char *tclk_name = "tclk";
> [..]
Here it is just about giving a name to a clock. As in the device tree
we only refer to the clock by index, the name don't matter.
>
> So it wouldn't be too nasty?
>
> I think you also mentioned the hypothetical case where the name
> is overloaded in the devicetree, so it's not "tclk", no? In that case,
> Emilio's fix would break.
I don't think I mentioned this case. I mentioned that this "fix" will
ignore the device tree.
>
> However, we don't have such situation in our 'canonical' devicetrees,
> so if the devicetree is allowed to be change, it can also be
> changed to add clock-output-name's so the name doesn't have to be
> obtained after the clock is registered.
>
> Which would solve it, no?
I really don't understand why you want introduce
Re: [PATCH 0/4] clk: mvebu: fix clk init order
On Mon, Feb 17, 2014 at 04:28:41PM +0100, Gregory CLEMENT wrote:
> On 17/02/2014 16:21, Ezequiel Garcia wrote:
> > On Mon, Feb 17, 2014 at 03:25:22PM +0100, Gregory CLEMENT wrote:
> >> On 17/02/2014 15:13, Ezequiel Garcia wrote:
> >>> On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote:
> On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote:
> > This patch set fixes clk init order that went upside-down with
> > v3.14. I haven't really investigated what caused this, but I assume
> > it is related with DT node reordering by addresses.
> >
> > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets
> > registered before core clocks driver. Unfortunately, we cannot
> > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the
> > init order for our drivers is always core clocks before clock gating,
> > we maintain init order ourselves by hooking CLK_OF_DECLARE to one
> > init function that will register core clocks before clock gating
> > driver.
> >
> > This patch is based on pre-v3.14-rc1 mainline and should go in as
> > fixes for it. As we now send MVEBU clk pull-requests to Mike directly,
> > I suggest Jason picks it up as a topic branch.
> >
> > The patches have been boot tested on Dove and compile-tested only
> > for Kirkwood, Armada 370 and XP.
> >
> > Sebastian Hesselbarth (4):
> > clk: mvebu: armada-370: maintain clock init order
> > clk: mvebu: armada-xp: maintain clock init order
> > clk: mvebu: dove: maintain clock init order
> > clk: mvebu: kirkwood: maintain clock init order
> >
> > drivers/clk/mvebu/armada-370.c | 21 ++---
> > drivers/clk/mvebu/armada-xp.c | 20 +---
> > drivers/clk/mvebu/dove.c | 19 +--
> > drivers/clk/mvebu/kirkwood.c | 34 --
> > 4 files changed, 44 insertions(+), 50 deletions(-)
>
> Whole series applied to mvebu/clk-fixes.
>
> >>>
> >>> Are we still in time to consider Emilio's oneline proposal?
> >>> (Emilio: would you mind preparing a suitable patch against dove,
> >>> kirkwood, armada370/xp, so we can see the real thing?).
> >>
> >> I am still strongly against this proposal because hard-coded the parent
> >> clock name in the driver seems very wrong and moreover in some
> >> circumstances
> >> (if there is no output-name, which is our default case) this proposal
> >> just ignored the parent clock given by the device tree and this looked
> >> more wrong.
> >>
> >
> > So you're against the proposal as a permanent fix, *and* against the
> > proposal as a workaround fix?
>
> Yes
>
> >
> >>>
> >>> Sebastian fix works perfect, and it easy to understand. However, it has
> >>> quite a large diffstat. When compared to Emilio's oneline proposal, it
> >>> seems to me it would be preferable, unless it's broken.
> >>>
> >>> Workaround or not, the fact is this code will be in v3.14, so maybe we
> >>> can spend some time considering a cleaner option.
> >>>
> >
> > Before discussing the solution as compared to your for-v3.15 clock
> > registration order patch, I wanted to trigger some discussion around
> > replacing this big and intrusive workaround with Emilio's oneline fix.
> >
> > Let's suppose we're considering them as workaround to live just one or
> > two releases. Wouldn't it be better to take the least instrusive?
> >
>
> The better solution is the one which doesn't add another regression and until
> today I though we had an agreement to use the patch set from Sebastian. If
> I remember well Jason had sent a pull request for it.
>
>
Right. If you think it adds a regression, then that's a perfectly valid reasons
for nacking.
However, I'd like to double-check we have such a regression. I guess you're
talking about the "tclk" name being hardcoded. IMHO, it's hardcoded in the
driver in the first place:
void __init mvebu_coreclk_setup(struct device_node *np,
const struct coreclk_soc_desc *desc)
{
const char *tclk_name = "tclk";
[..]
So it wouldn't be too nasty?
I think you also mentioned the hypothetical case where the name
is overloaded in the devicetree, so it's not "tclk", no? In that case,
Emilio's fix would break.
However, we don't have such situation in our 'canonical' devicetrees,
so if the devicetree is allowed to be change, it can also be
changed to add clock-output-name's so the name doesn't have to be
obtained after the clock is registered.
Which would solve it, no?
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
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 0/4] clk: mvebu: fix clk init order
Hi,
El 17/02/14 12:04, Gregory CLEMENT escribió:
(snip)
Please read what I have written: "if there is no output-name, which is our
default case) this proposal just ignored the parent clock given by the device
tree".
Extract of your code from the link you pointed:
const char *default_parent = "tclk";
[...]
of_property_read_string_index(clkspec.np, "clock-output-names",
clkspec.args_count ?
clkspec.args[0] : 0,
_parent);
example of a valid dts:
gateclk: clock-gating-control@18220 {
compatible = "marvell,foo-bar-gating-clock";
reg = <0x18220 0x4>;
clocks = < 1>;
#clock-cells = <1>;
};
So in this fictional but still valid example, the device tree indicates that
the parent
clock of the gating clock is the 2nd clock provided by the coreclk which is
currently
"cpuclk". As no clock-output-names is used, then this will be totally ignore
and instead
of using "cpuclk" as parent "tclk" will be used.
I can see your point now, but as this is completely fictional, I'd say
it's irrelevant. You can just add the names if Marvell ever makes a chip
that sources the gates from the second coreclk. As far as I can see on
the device trees in Linux, all mvebu hardware always sources them from
tclk. Don't try to over-engineer your driver for something that is
unlikely to happen in reality.
If you in the future need to support another legacy platform with a
half-cooked DT not listing the names, you can always list the right
parent on the divisor table (see link for example) and override the default.
http://lxr.free-electrons.com/source/drivers/clk/mvebu/kirkwood.c?v=3.13#L222
I hope this example will show you, what I disagree with this proposal and why it
introduce some regression.
It's not a regression if things don't break :-)
Cheers,
Emilio
--
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 0/4] clk: mvebu: fix clk init order
On 17/02/2014 16:21, Ezequiel Garcia wrote: > On Mon, Feb 17, 2014 at 03:25:22PM +0100, Gregory CLEMENT wrote: >> On 17/02/2014 15:13, Ezequiel Garcia wrote: >>> On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: > This patch set fixes clk init order that went upside-down with > v3.14. I haven't really investigated what caused this, but I assume > it is related with DT node reordering by addresses. > > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > registered before core clocks driver. Unfortunately, we cannot > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > init order for our drivers is always core clocks before clock gating, > we maintain init order ourselves by hooking CLK_OF_DECLARE to one > init function that will register core clocks before clock gating > driver. > > This patch is based on pre-v3.14-rc1 mainline and should go in as > fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > I suggest Jason picks it up as a topic branch. > > The patches have been boot tested on Dove and compile-tested only > for Kirkwood, Armada 370 and XP. > > Sebastian Hesselbarth (4): > clk: mvebu: armada-370: maintain clock init order > clk: mvebu: armada-xp: maintain clock init order > clk: mvebu: dove: maintain clock init order > clk: mvebu: kirkwood: maintain clock init order > > drivers/clk/mvebu/armada-370.c | 21 ++--- > drivers/clk/mvebu/armada-xp.c | 20 +--- > drivers/clk/mvebu/dove.c | 19 +-- > drivers/clk/mvebu/kirkwood.c | 34 -- > 4 files changed, 44 insertions(+), 50 deletions(-) Whole series applied to mvebu/clk-fixes. >>> >>> Are we still in time to consider Emilio's oneline proposal? >>> (Emilio: would you mind preparing a suitable patch against dove, >>> kirkwood, armada370/xp, so we can see the real thing?). >> >> I am still strongly against this proposal because hard-coded the parent >> clock name in the driver seems very wrong and moreover in some circumstances >> (if there is no output-name, which is our default case) this proposal >> just ignored the parent clock given by the device tree and this looked >> more wrong. >> > > So you're against the proposal as a permanent fix, *and* against the > proposal as a workaround fix? Yes > >>> >>> Sebastian fix works perfect, and it easy to understand. However, it has >>> quite a large diffstat. When compared to Emilio's oneline proposal, it >>> seems to me it would be preferable, unless it's broken. >>> >>> Workaround or not, the fact is this code will be in v3.14, so maybe we >>> can spend some time considering a cleaner option. >>> > > Before discussing the solution as compared to your for-v3.15 clock > registration order patch, I wanted to trigger some discussion around > replacing this big and intrusive workaround with Emilio's oneline fix. > > Let's suppose we're considering them as workaround to live just one or > two releases. Wouldn't it be better to take the least instrusive? > The better solution is the one which doesn't add another regression and until today I though we had an agreement to use the patch set from Sebastian. If I remember well Jason had sent a pull request for it. -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On Mon, Feb 17, 2014 at 03:25:22PM +0100, Gregory CLEMENT wrote: > On 17/02/2014 15:13, Ezequiel Garcia wrote: > > On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: > >> On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: > >>> This patch set fixes clk init order that went upside-down with > >>> v3.14. I haven't really investigated what caused this, but I assume > >>> it is related with DT node reordering by addresses. > >>> > >>> Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > >>> registered before core clocks driver. Unfortunately, we cannot > >>> return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > >>> init order for our drivers is always core clocks before clock gating, > >>> we maintain init order ourselves by hooking CLK_OF_DECLARE to one > >>> init function that will register core clocks before clock gating > >>> driver. > >>> > >>> This patch is based on pre-v3.14-rc1 mainline and should go in as > >>> fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > >>> I suggest Jason picks it up as a topic branch. > >>> > >>> The patches have been boot tested on Dove and compile-tested only > >>> for Kirkwood, Armada 370 and XP. > >>> > >>> Sebastian Hesselbarth (4): > >>> clk: mvebu: armada-370: maintain clock init order > >>> clk: mvebu: armada-xp: maintain clock init order > >>> clk: mvebu: dove: maintain clock init order > >>> clk: mvebu: kirkwood: maintain clock init order > >>> > >>> drivers/clk/mvebu/armada-370.c | 21 ++--- > >>> drivers/clk/mvebu/armada-xp.c | 20 +--- > >>> drivers/clk/mvebu/dove.c | 19 +-- > >>> drivers/clk/mvebu/kirkwood.c | 34 -- > >>> 4 files changed, 44 insertions(+), 50 deletions(-) > >> > >> Whole series applied to mvebu/clk-fixes. > >> > > > > Are we still in time to consider Emilio's oneline proposal? > > (Emilio: would you mind preparing a suitable patch against dove, > > kirkwood, armada370/xp, so we can see the real thing?). > > I am still strongly against this proposal because hard-coded the parent > clock name in the driver seems very wrong and moreover in some circumstances > (if there is no output-name, which is our default case) this proposal > just ignored the parent clock given by the device tree and this looked > more wrong. > So you're against the proposal as a permanent fix, *and* against the proposal as a workaround fix? > > > > Sebastian fix works perfect, and it easy to understand. However, it has > > quite a large diffstat. When compared to Emilio's oneline proposal, it > > seems to me it would be preferable, unless it's broken. > > > > Workaround or not, the fact is this code will be in v3.14, so maybe we > > can spend some time considering a cleaner option. > > Before discussing the solution as compared to your for-v3.15 clock registration order patch, I wanted to trigger some discussion around replacing this big and intrusive workaround with Emilio's oneline fix. Let's suppose we're considering them as workaround to live just one or two releases. Wouldn't it be better to take the least instrusive? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On 17/02/2014 15:42, Emilio López wrote:
> Hi Gregory, Ezequiel,
>
> >> Are we still in time to consider Emilio's oneline proposal?
>>> (Emilio: would you mind preparing a suitable patch against dove,
>>> kirkwood, armada370/xp, so we can see the real thing?).
>
> The patch is in a common file, so it does not need patching anything for
> each platform.
>
>> I am still strongly against this proposal because hard-coded the parent
>> clock name in the driver seems very wrong
>
> It is hardcoded already when the parent is registered, so I do not
> understand your concern.
>
> http://lxr.free-electrons.com/source/drivers/clk/mvebu/common.c?v=3.13#L34
>
>> and moreover in some circumstances
>> (if there is no output-name, which is our default case) this proposal
>> just ignored the parent clock given by the device tree and this looked
>> more wrong.
>
> I have sent a second patch addressing this comment, but you do not seem
> to have taken too serious a look at it.
>
> http://www.spinics.net/lists/arm-kernel/msg305922.html
>
Please read what I have written: "if there is no output-name, which is our
default case) this proposal just ignored the parent clock given by the device
tree".
Extract of your code from the link you pointed:
const char *default_parent = "tclk";
[...]
of_property_read_string_index(clkspec.np, "clock-output-names",
clkspec.args_count ?
clkspec.args[0] : 0,
_parent);
example of a valid dts:
gateclk: clock-gating-control@18220 {
compatible = "marvell,foo-bar-gating-clock";
reg = <0x18220 0x4>;
clocks = < 1>;
#clock-cells = <1>;
};
So in this fictional but still valid example, the device tree indicates that
the parent
clock of the gating clock is the 2nd clock provided by the coreclk which is
currently
"cpuclk". As no clock-output-names is used, then this will be totally ignore
and instead
of using "cpuclk" as parent "tclk" will be used.
I hope this example will show you, what I disagree with this proposal and why it
introduce some regression.
Regards,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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 0/4] clk: mvebu: fix clk init order
Hi Gregory, Ezequiel, >> Are we still in time to consider Emilio's oneline proposal? (Emilio: would you mind preparing a suitable patch against dove, kirkwood, armada370/xp, so we can see the real thing?). The patch is in a common file, so it does not need patching anything for each platform. I am still strongly against this proposal because hard-coded the parent clock name in the driver seems very wrong It is hardcoded already when the parent is registered, so I do not understand your concern. http://lxr.free-electrons.com/source/drivers/clk/mvebu/common.c?v=3.13#L34 and moreover in some circumstances (if there is no output-name, which is our default case) this proposal just ignored the parent clock given by the device tree and this looked more wrong. I have sent a second patch addressing this comment, but you do not seem to have taken too serious a look at it. http://www.spinics.net/lists/arm-kernel/msg305922.html Cheers, Emilio -- 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 0/4] clk: mvebu: fix clk init order
On 17/02/2014 15:13, Ezequiel Garcia wrote: > On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: >> On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: >>> This patch set fixes clk init order that went upside-down with >>> v3.14. I haven't really investigated what caused this, but I assume >>> it is related with DT node reordering by addresses. >>> >>> Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets >>> registered before core clocks driver. Unfortunately, we cannot >>> return -EPROBE_DEFER in drivers initialized by clk_of_init. As the >>> init order for our drivers is always core clocks before clock gating, >>> we maintain init order ourselves by hooking CLK_OF_DECLARE to one >>> init function that will register core clocks before clock gating >>> driver. >>> >>> This patch is based on pre-v3.14-rc1 mainline and should go in as >>> fixes for it. As we now send MVEBU clk pull-requests to Mike directly, >>> I suggest Jason picks it up as a topic branch. >>> >>> The patches have been boot tested on Dove and compile-tested only >>> for Kirkwood, Armada 370 and XP. >>> >>> Sebastian Hesselbarth (4): >>> clk: mvebu: armada-370: maintain clock init order >>> clk: mvebu: armada-xp: maintain clock init order >>> clk: mvebu: dove: maintain clock init order >>> clk: mvebu: kirkwood: maintain clock init order >>> >>> drivers/clk/mvebu/armada-370.c | 21 ++--- >>> drivers/clk/mvebu/armada-xp.c | 20 +--- >>> drivers/clk/mvebu/dove.c | 19 +-- >>> drivers/clk/mvebu/kirkwood.c | 34 -- >>> 4 files changed, 44 insertions(+), 50 deletions(-) >> >> Whole series applied to mvebu/clk-fixes. >> > > Are we still in time to consider Emilio's oneline proposal? > (Emilio: would you mind preparing a suitable patch against dove, > kirkwood, armada370/xp, so we can see the real thing?). I am still strongly against this proposal because hard-coded the parent clock name in the driver seems very wrong and moreover in some circumstances (if there is no output-name, which is our default case) this proposal just ignored the parent clock given by the device tree and this looked more wrong. > > Sebastian fix works perfect, and it easy to understand. However, it has > quite a large diffstat. When compared to Emilio's oneline proposal, it > seems to me it would be preferable, unless it's broken. > > Workaround or not, the fact is this code will be in v3.14, so maybe we > can spend some time considering a cleaner option. > > Or is this just rubbish? > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: > On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: > > This patch set fixes clk init order that went upside-down with > > v3.14. I haven't really investigated what caused this, but I assume > > it is related with DT node reordering by addresses. > > > > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > > registered before core clocks driver. Unfortunately, we cannot > > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > > init order for our drivers is always core clocks before clock gating, > > we maintain init order ourselves by hooking CLK_OF_DECLARE to one > > init function that will register core clocks before clock gating > > driver. > > > > This patch is based on pre-v3.14-rc1 mainline and should go in as > > fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > > I suggest Jason picks it up as a topic branch. > > > > The patches have been boot tested on Dove and compile-tested only > > for Kirkwood, Armada 370 and XP. > > > > Sebastian Hesselbarth (4): > > clk: mvebu: armada-370: maintain clock init order > > clk: mvebu: armada-xp: maintain clock init order > > clk: mvebu: dove: maintain clock init order > > clk: mvebu: kirkwood: maintain clock init order > > > > drivers/clk/mvebu/armada-370.c | 21 ++--- > > drivers/clk/mvebu/armada-xp.c | 20 +--- > > drivers/clk/mvebu/dove.c | 19 +-- > > drivers/clk/mvebu/kirkwood.c | 34 -- > > 4 files changed, 44 insertions(+), 50 deletions(-) > > Whole series applied to mvebu/clk-fixes. > Are we still in time to consider Emilio's oneline proposal? (Emilio: would you mind preparing a suitable patch against dove, kirkwood, armada370/xp, so we can see the real thing?). Sebastian fix works perfect, and it easy to understand. However, it has quite a large diffstat. When compared to Emilio's oneline proposal, it seems to me it would be preferable, unless it's broken. Workaround or not, the fact is this code will be in v3.14, so maybe we can spend some time considering a cleaner option. Or is this just rubbish? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) Whole series applied to mvebu/clk-fixes. Are we still in time to consider Emilio's oneline proposal? (Emilio: would you mind preparing a suitable patch against dove, kirkwood, armada370/xp, so we can see the real thing?). Sebastian fix works perfect, and it easy to understand. However, it has quite a large diffstat. When compared to Emilio's oneline proposal, it seems to me it would be preferable, unless it's broken. Workaround or not, the fact is this code will be in v3.14, so maybe we can spend some time considering a cleaner option. Or is this just rubbish? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On 17/02/2014 15:13, Ezequiel Garcia wrote: On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) Whole series applied to mvebu/clk-fixes. Are we still in time to consider Emilio's oneline proposal? (Emilio: would you mind preparing a suitable patch against dove, kirkwood, armada370/xp, so we can see the real thing?). I am still strongly against this proposal because hard-coded the parent clock name in the driver seems very wrong and moreover in some circumstances (if there is no output-name, which is our default case) this proposal just ignored the parent clock given by the device tree and this looked more wrong. Sebastian fix works perfect, and it easy to understand. However, it has quite a large diffstat. When compared to Emilio's oneline proposal, it seems to me it would be preferable, unless it's broken. Workaround or not, the fact is this code will be in v3.14, so maybe we can spend some time considering a cleaner option. Or is this just rubbish? -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
Hi Gregory, Ezequiel, Are we still in time to consider Emilio's oneline proposal? (Emilio: would you mind preparing a suitable patch against dove, kirkwood, armada370/xp, so we can see the real thing?). The patch is in a common file, so it does not need patching anything for each platform. I am still strongly against this proposal because hard-coded the parent clock name in the driver seems very wrong It is hardcoded already when the parent is registered, so I do not understand your concern. http://lxr.free-electrons.com/source/drivers/clk/mvebu/common.c?v=3.13#L34 and moreover in some circumstances (if there is no output-name, which is our default case) this proposal just ignored the parent clock given by the device tree and this looked more wrong. I have sent a second patch addressing this comment, but you do not seem to have taken too serious a look at it. http://www.spinics.net/lists/arm-kernel/msg305922.html Cheers, Emilio -- 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 0/4] clk: mvebu: fix clk init order
On 17/02/2014 15:42, Emilio López wrote:
Hi Gregory, Ezequiel,
Are we still in time to consider Emilio's oneline proposal?
(Emilio: would you mind preparing a suitable patch against dove,
kirkwood, armada370/xp, so we can see the real thing?).
The patch is in a common file, so it does not need patching anything for
each platform.
I am still strongly against this proposal because hard-coded the parent
clock name in the driver seems very wrong
It is hardcoded already when the parent is registered, so I do not
understand your concern.
http://lxr.free-electrons.com/source/drivers/clk/mvebu/common.c?v=3.13#L34
and moreover in some circumstances
(if there is no output-name, which is our default case) this proposal
just ignored the parent clock given by the device tree and this looked
more wrong.
I have sent a second patch addressing this comment, but you do not seem
to have taken too serious a look at it.
http://www.spinics.net/lists/arm-kernel/msg305922.html
Please read what I have written: if there is no output-name, which is our
default case) this proposal just ignored the parent clock given by the device
tree.
Extract of your code from the link you pointed:
const char *default_parent = tclk;
[...]
of_property_read_string_index(clkspec.np, clock-output-names,
clkspec.args_count ?
clkspec.args[0] : 0,
default_parent);
example of a valid dts:
gateclk: clock-gating-control@18220 {
compatible = marvell,foo-bar-gating-clock;
reg = 0x18220 0x4;
clocks = coreclk 1;
#clock-cells = 1;
};
So in this fictional but still valid example, the device tree indicates that
the parent
clock of the gating clock is the 2nd clock provided by the coreclk which is
currently
cpuclk. As no clock-output-names is used, then this will be totally ignore
and instead
of using cpuclk as parent tclk will be used.
I hope this example will show you, what I disagree with this proposal and why it
introduce some regression.
Regards,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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 0/4] clk: mvebu: fix clk init order
On Mon, Feb 17, 2014 at 03:25:22PM +0100, Gregory CLEMENT wrote: On 17/02/2014 15:13, Ezequiel Garcia wrote: On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) Whole series applied to mvebu/clk-fixes. Are we still in time to consider Emilio's oneline proposal? (Emilio: would you mind preparing a suitable patch against dove, kirkwood, armada370/xp, so we can see the real thing?). I am still strongly against this proposal because hard-coded the parent clock name in the driver seems very wrong and moreover in some circumstances (if there is no output-name, which is our default case) this proposal just ignored the parent clock given by the device tree and this looked more wrong. So you're against the proposal as a permanent fix, *and* against the proposal as a workaround fix? Sebastian fix works perfect, and it easy to understand. However, it has quite a large diffstat. When compared to Emilio's oneline proposal, it seems to me it would be preferable, unless it's broken. Workaround or not, the fact is this code will be in v3.14, so maybe we can spend some time considering a cleaner option. Before discussing the solution as compared to your for-v3.15 clock registration order patch, I wanted to trigger some discussion around replacing this big and intrusive workaround with Emilio's oneline fix. Let's suppose we're considering them as workaround to live just one or two releases. Wouldn't it be better to take the least instrusive? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On 17/02/2014 16:21, Ezequiel Garcia wrote: On Mon, Feb 17, 2014 at 03:25:22PM +0100, Gregory CLEMENT wrote: On 17/02/2014 15:13, Ezequiel Garcia wrote: On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) Whole series applied to mvebu/clk-fixes. Are we still in time to consider Emilio's oneline proposal? (Emilio: would you mind preparing a suitable patch against dove, kirkwood, armada370/xp, so we can see the real thing?). I am still strongly against this proposal because hard-coded the parent clock name in the driver seems very wrong and moreover in some circumstances (if there is no output-name, which is our default case) this proposal just ignored the parent clock given by the device tree and this looked more wrong. So you're against the proposal as a permanent fix, *and* against the proposal as a workaround fix? Yes Sebastian fix works perfect, and it easy to understand. However, it has quite a large diffstat. When compared to Emilio's oneline proposal, it seems to me it would be preferable, unless it's broken. Workaround or not, the fact is this code will be in v3.14, so maybe we can spend some time considering a cleaner option. Before discussing the solution as compared to your for-v3.15 clock registration order patch, I wanted to trigger some discussion around replacing this big and intrusive workaround with Emilio's oneline fix. Let's suppose we're considering them as workaround to live just one or two releases. Wouldn't it be better to take the least instrusive? The better solution is the one which doesn't add another regression and until today I though we had an agreement to use the patch set from Sebastian. If I remember well Jason had sent a pull request for it. -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
Hi,
El 17/02/14 12:04, Gregory CLEMENT escribió:
(snip)
Please read what I have written: if there is no output-name, which is our
default case) this proposal just ignored the parent clock given by the device
tree.
Extract of your code from the link you pointed:
const char *default_parent = tclk;
[...]
of_property_read_string_index(clkspec.np, clock-output-names,
clkspec.args_count ?
clkspec.args[0] : 0,
default_parent);
example of a valid dts:
gateclk: clock-gating-control@18220 {
compatible = marvell,foo-bar-gating-clock;
reg = 0x18220 0x4;
clocks = coreclk 1;
#clock-cells = 1;
};
So in this fictional but still valid example, the device tree indicates that
the parent
clock of the gating clock is the 2nd clock provided by the coreclk which is
currently
cpuclk. As no clock-output-names is used, then this will be totally ignore
and instead
of using cpuclk as parent tclk will be used.
I can see your point now, but as this is completely fictional, I'd say
it's irrelevant. You can just add the names if Marvell ever makes a chip
that sources the gates from the second coreclk. As far as I can see on
the device trees in Linux, all mvebu hardware always sources them from
tclk. Don't try to over-engineer your driver for something that is
unlikely to happen in reality.
If you in the future need to support another legacy platform with a
half-cooked DT not listing the names, you can always list the right
parent on the divisor table (see link for example) and override the default.
http://lxr.free-electrons.com/source/drivers/clk/mvebu/kirkwood.c?v=3.13#L222
I hope this example will show you, what I disagree with this proposal and why it
introduce some regression.
It's not a regression if things don't break :-)
Cheers,
Emilio
--
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 0/4] clk: mvebu: fix clk init order
On Mon, Feb 17, 2014 at 04:28:41PM +0100, Gregory CLEMENT wrote:
On 17/02/2014 16:21, Ezequiel Garcia wrote:
On Mon, Feb 17, 2014 at 03:25:22PM +0100, Gregory CLEMENT wrote:
On 17/02/2014 15:13, Ezequiel Garcia wrote:
On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote:
On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote:
This patch set fixes clk init order that went upside-down with
v3.14. I haven't really investigated what caused this, but I assume
it is related with DT node reordering by addresses.
Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets
registered before core clocks driver. Unfortunately, we cannot
return -EPROBE_DEFER in drivers initialized by clk_of_init. As the
init order for our drivers is always core clocks before clock gating,
we maintain init order ourselves by hooking CLK_OF_DECLARE to one
init function that will register core clocks before clock gating
driver.
This patch is based on pre-v3.14-rc1 mainline and should go in as
fixes for it. As we now send MVEBU clk pull-requests to Mike directly,
I suggest Jason picks it up as a topic branch.
The patches have been boot tested on Dove and compile-tested only
for Kirkwood, Armada 370 and XP.
Sebastian Hesselbarth (4):
clk: mvebu: armada-370: maintain clock init order
clk: mvebu: armada-xp: maintain clock init order
clk: mvebu: dove: maintain clock init order
clk: mvebu: kirkwood: maintain clock init order
drivers/clk/mvebu/armada-370.c | 21 ++---
drivers/clk/mvebu/armada-xp.c | 20 +---
drivers/clk/mvebu/dove.c | 19 +--
drivers/clk/mvebu/kirkwood.c | 34 --
4 files changed, 44 insertions(+), 50 deletions(-)
Whole series applied to mvebu/clk-fixes.
Are we still in time to consider Emilio's oneline proposal?
(Emilio: would you mind preparing a suitable patch against dove,
kirkwood, armada370/xp, so we can see the real thing?).
I am still strongly against this proposal because hard-coded the parent
clock name in the driver seems very wrong and moreover in some
circumstances
(if there is no output-name, which is our default case) this proposal
just ignored the parent clock given by the device tree and this looked
more wrong.
So you're against the proposal as a permanent fix, *and* against the
proposal as a workaround fix?
Yes
Sebastian fix works perfect, and it easy to understand. However, it has
quite a large diffstat. When compared to Emilio's oneline proposal, it
seems to me it would be preferable, unless it's broken.
Workaround or not, the fact is this code will be in v3.14, so maybe we
can spend some time considering a cleaner option.
Before discussing the solution as compared to your for-v3.15 clock
registration order patch, I wanted to trigger some discussion around
replacing this big and intrusive workaround with Emilio's oneline fix.
Let's suppose we're considering them as workaround to live just one or
two releases. Wouldn't it be better to take the least instrusive?
The better solution is the one which doesn't add another regression and until
today I though we had an agreement to use the patch set from Sebastian. If
I remember well Jason had sent a pull request for it.
Right. If you think it adds a regression, then that's a perfectly valid reasons
for nacking.
However, I'd like to double-check we have such a regression. I guess you're
talking about the tclk name being hardcoded. IMHO, it's hardcoded in the
driver in the first place:
void __init mvebu_coreclk_setup(struct device_node *np,
const struct coreclk_soc_desc *desc)
{
const char *tclk_name = tclk;
[..]
So it wouldn't be too nasty?
I think you also mentioned the hypothetical case where the name
is overloaded in the devicetree, so it's not tclk, no? In that case,
Emilio's fix would break.
However, we don't have such situation in our 'canonical' devicetrees,
so if the devicetree is allowed to be change, it can also be
changed to add clock-output-name's so the name doesn't have to be
obtained after the clock is registered.
Which would solve it, no?
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
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 0/4] clk: mvebu: fix clk init order
On 17/02/2014 16:44, Ezequiel Garcia wrote:
On Mon, Feb 17, 2014 at 04:28:41PM +0100, Gregory CLEMENT wrote:
On 17/02/2014 16:21, Ezequiel Garcia wrote:
On Mon, Feb 17, 2014 at 03:25:22PM +0100, Gregory CLEMENT wrote:
On 17/02/2014 15:13, Ezequiel Garcia wrote:
On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote:
On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote:
This patch set fixes clk init order that went upside-down with
v3.14. I haven't really investigated what caused this, but I assume
it is related with DT node reordering by addresses.
Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets
registered before core clocks driver. Unfortunately, we cannot
return -EPROBE_DEFER in drivers initialized by clk_of_init. As the
init order for our drivers is always core clocks before clock gating,
we maintain init order ourselves by hooking CLK_OF_DECLARE to one
init function that will register core clocks before clock gating
driver.
This patch is based on pre-v3.14-rc1 mainline and should go in as
fixes for it. As we now send MVEBU clk pull-requests to Mike directly,
I suggest Jason picks it up as a topic branch.
The patches have been boot tested on Dove and compile-tested only
for Kirkwood, Armada 370 and XP.
Sebastian Hesselbarth (4):
clk: mvebu: armada-370: maintain clock init order
clk: mvebu: armada-xp: maintain clock init order
clk: mvebu: dove: maintain clock init order
clk: mvebu: kirkwood: maintain clock init order
drivers/clk/mvebu/armada-370.c | 21 ++---
drivers/clk/mvebu/armada-xp.c | 20 +---
drivers/clk/mvebu/dove.c | 19 +--
drivers/clk/mvebu/kirkwood.c | 34 --
4 files changed, 44 insertions(+), 50 deletions(-)
Whole series applied to mvebu/clk-fixes.
Are we still in time to consider Emilio's oneline proposal?
(Emilio: would you mind preparing a suitable patch against dove,
kirkwood, armada370/xp, so we can see the real thing?).
I am still strongly against this proposal because hard-coded the parent
clock name in the driver seems very wrong and moreover in some
circumstances
(if there is no output-name, which is our default case) this proposal
just ignored the parent clock given by the device tree and this looked
more wrong.
So you're against the proposal as a permanent fix, *and* against the
proposal as a workaround fix?
Yes
Sebastian fix works perfect, and it easy to understand. However, it has
quite a large diffstat. When compared to Emilio's oneline proposal, it
seems to me it would be preferable, unless it's broken.
Workaround or not, the fact is this code will be in v3.14, so maybe we
can spend some time considering a cleaner option.
Before discussing the solution as compared to your for-v3.15 clock
registration order patch, I wanted to trigger some discussion around
replacing this big and intrusive workaround with Emilio's oneline fix.
Let's suppose we're considering them as workaround to live just one or
two releases. Wouldn't it be better to take the least instrusive?
The better solution is the one which doesn't add another regression and until
today I though we had an agreement to use the patch set from Sebastian. If
I remember well Jason had sent a pull request for it.
Right. If you think it adds a regression, then that's a perfectly valid
reasons
for nacking.
However, I'd like to double-check we have such a regression. I guess you're
talking about the tclk name being hardcoded. IMHO, it's hardcoded in the
driver in the first place:
void __init mvebu_coreclk_setup(struct device_node *np,
const struct coreclk_soc_desc *desc)
{
const char *tclk_name = tclk;
[..]
Here it is just about giving a name to a clock. As in the device tree
we only refer to the clock by index, the name don't matter.
So it wouldn't be too nasty?
I think you also mentioned the hypothetical case where the name
is overloaded in the devicetree, so it's not tclk, no? In that case,
Emilio's fix would break.
I don't think I mentioned this case. I mentioned that this fix will
ignore the device tree.
However, we don't have such situation in our 'canonical' devicetrees,
so if the devicetree is allowed to be change, it can also be
changed to add clock-output-name's so the name doesn't have to be
obtained after the clock is registered.
Which would solve it, no?
I really don't understand why you want introduce potential problem, just
to save a few line of code. A code that works perfect, and it easy
to understand as you wrote.
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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 0/4] clk: mvebu: fix clk init order
On Mon, Feb 17, 2014 at 04:59:01PM +0100, Gregory CLEMENT wrote:
[..]
Right. If you think it adds a regression, then that's a perfectly valid
reasons
for nacking.
However, I'd like to double-check we have such a regression. I guess you're
talking about the tclk name being hardcoded. IMHO, it's hardcoded in the
driver in the first place:
void __init mvebu_coreclk_setup(struct device_node *np,
const struct coreclk_soc_desc *desc)
{
const char *tclk_name = tclk;
[..]
Here it is just about giving a name to a clock. As in the device tree
we only refer to the clock by index, the name don't matter.
Unless I'm really confused about what's the problem here, the *name* is
*all* that matters.
We're having a clock *registration* order issue (which is different from clock
enable). Let me try to explain the issue, in case it's still not clear.
I'll stick to the current specific problem but it can apply to any other
pair of parent/child.
Some of the mvebu clocks are registered as part of the core clock
group, modeled by the marvell,armada-370-core-clock compatible node.
Another group of mvebu clocks are registered as part of the gating
clock group, modeled by the marvell,armada-370-gating-clock compatible
node.
By default, all the gating clocks are child of the first registered core
clock. This clock is named tclk by default, and this name can be overloaded
in the devicetree.
So far, so good, right?
The issue we're trying to fix, arises from the mvebu_clk_gating_setup()
trying to get the name of this tclk, as a registered clock.
In other words, the current code needs the tclk to be registered, for it
will ask his name like this:
const char *default_parent = NULL;
clk = of_clk_get(np, 0);
if (!IS_ERR(clk)) {
default_parent = __clk_get_name(clk);
clk_put(clk);
}
Once it gets the name, all goes smoothly. Notice how the clock is obtained
for the sole purpose of getting the name of it, which shows clearly it's the
*name* that matters.
The ordering issue happens when the gating clock group is probed, before
the core clock group. In that case, it's not possible to get the
coreclk 0 (which is wrongly assumed to be registered), and so it's
not possible to get the name.
So the root of the problem is that snippet above, which adds a
completely unneeded registration order requirement. Instead, we should
be looking for the names: we can hardcode the name or fetch it from the
devicetree (Emilio has posted patches for both).
I really don't see why we're not fixing this, instead of adding yet
another layer of complexity to the problem.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
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 0/4] clk: mvebu: fix clk init order
On Thu, Feb 06, 2014 at 02:08:39PM -0300, Ezequiel Garcia wrote: > On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: > > On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: > > > This patch set fixes clk init order that went upside-down with > > > v3.14. I haven't really investigated what caused this, but I assume > > > it is related with DT node reordering by addresses. > > > > > > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > > > registered before core clocks driver. Unfortunately, we cannot > > > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > > > init order for our drivers is always core clocks before clock gating, > > > we maintain init order ourselves by hooking CLK_OF_DECLARE to one > > > init function that will register core clocks before clock gating > > > driver. > > > > > > This patch is based on pre-v3.14-rc1 mainline and should go in as > > > fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > > > I suggest Jason picks it up as a topic branch. > > > > > > The patches have been boot tested on Dove and compile-tested only > > > for Kirkwood, Armada 370 and XP. > > > > > > Sebastian Hesselbarth (4): > > > clk: mvebu: armada-370: maintain clock init order > > > clk: mvebu: armada-xp: maintain clock init order > > > clk: mvebu: dove: maintain clock init order > > > clk: mvebu: kirkwood: maintain clock init order > > > > > > drivers/clk/mvebu/armada-370.c | 21 ++--- > > > drivers/clk/mvebu/armada-xp.c | 20 +--- > > > drivers/clk/mvebu/dove.c | 19 +-- > > > drivers/clk/mvebu/kirkwood.c | 34 -- > > > 4 files changed, 44 insertions(+), 50 deletions(-) > > > > Whole series applied to mvebu/clk-fixes. > > > > FWIW, Tested-by: Ezequiel Garcia > > Observed "division by zero" get fixed by this, on A370 Mirabox, Kirkwood > Topkick and Dove Cubox. Added for patches 1, 3, and 4 (all but armada-xp.c) Thanks for testing. thx, Jason. -- 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 0/4] clk: mvebu: fix clk init order
On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: > On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: > > This patch set fixes clk init order that went upside-down with > > v3.14. I haven't really investigated what caused this, but I assume > > it is related with DT node reordering by addresses. > > > > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > > registered before core clocks driver. Unfortunately, we cannot > > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > > init order for our drivers is always core clocks before clock gating, > > we maintain init order ourselves by hooking CLK_OF_DECLARE to one > > init function that will register core clocks before clock gating > > driver. > > > > This patch is based on pre-v3.14-rc1 mainline and should go in as > > fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > > I suggest Jason picks it up as a topic branch. > > > > The patches have been boot tested on Dove and compile-tested only > > for Kirkwood, Armada 370 and XP. > > > > Sebastian Hesselbarth (4): > > clk: mvebu: armada-370: maintain clock init order > > clk: mvebu: armada-xp: maintain clock init order > > clk: mvebu: dove: maintain clock init order > > clk: mvebu: kirkwood: maintain clock init order > > > > drivers/clk/mvebu/armada-370.c | 21 ++--- > > drivers/clk/mvebu/armada-xp.c | 20 +--- > > drivers/clk/mvebu/dove.c | 19 +-- > > drivers/clk/mvebu/kirkwood.c | 34 -- > > 4 files changed, 44 insertions(+), 50 deletions(-) > > Whole series applied to mvebu/clk-fixes. > FWIW, Tested-by: Ezequiel Garcia Observed "division by zero" get fixed by this, on A370 Mirabox, Kirkwood Topkick and Dove Cubox. -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) Whole series applied to mvebu/clk-fixes. FWIW, Tested-by: Ezequiel Garcia ezequiel.gar...@free-electrons.com Observed division by zero get fixed by this, on A370 Mirabox, Kirkwood Topkick and Dove Cubox. -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On Thu, Feb 06, 2014 at 02:08:39PM -0300, Ezequiel Garcia wrote: On Wed, Feb 05, 2014 at 01:34:57PM -0500, Jason Cooper wrote: On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) Whole series applied to mvebu/clk-fixes. FWIW, Tested-by: Ezequiel Garcia ezequiel.gar...@free-electrons.com Observed division by zero get fixed by this, on A370 Mirabox, Kirkwood Topkick and Dove Cubox. Added for patches 1, 3, and 4 (all but armada-xp.c) Thanks for testing. thx, Jason. -- 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 0/4] clk: mvebu: fix clk init order
On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: > This patch set fixes clk init order that went upside-down with > v3.14. I haven't really investigated what caused this, but I assume > it is related with DT node reordering by addresses. > > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > registered before core clocks driver. Unfortunately, we cannot > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > init order for our drivers is always core clocks before clock gating, > we maintain init order ourselves by hooking CLK_OF_DECLARE to one > init function that will register core clocks before clock gating > driver. > > This patch is based on pre-v3.14-rc1 mainline and should go in as > fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > I suggest Jason picks it up as a topic branch. > > The patches have been boot tested on Dove and compile-tested only > for Kirkwood, Armada 370 and XP. > > Sebastian Hesselbarth (4): > clk: mvebu: armada-370: maintain clock init order > clk: mvebu: armada-xp: maintain clock init order > clk: mvebu: dove: maintain clock init order > clk: mvebu: kirkwood: maintain clock init order > > drivers/clk/mvebu/armada-370.c | 21 ++--- > drivers/clk/mvebu/armada-xp.c | 20 +--- > drivers/clk/mvebu/dove.c | 19 +-- > drivers/clk/mvebu/kirkwood.c | 34 -- > 4 files changed, 44 insertions(+), 50 deletions(-) Whole series applied to mvebu/clk-fixes. thx, Jason. -- 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 0/4] clk: mvebu: fix clk init order
On Wed, Feb 05, 2014 at 06:08:02AM -0800, Mike Turquette wrote: > Quoting Sebastian Hesselbarth (2014-01-25 10:19:06) > > This patch set fixes clk init order that went upside-down with > > v3.14. I haven't really investigated what caused this, but I assume > > it is related with DT node reordering by addresses. > > > > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > > registered before core clocks driver. Unfortunately, we cannot > > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > > init order for our drivers is always core clocks before clock gating, > > we maintain init order ourselves by hooking CLK_OF_DECLARE to one > > init function that will register core clocks before clock gating > > driver. > > > > This patch is based on pre-v3.14-rc1 mainline and should go in as > > fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > > I suggest Jason picks it up as a topic branch. > > Sebastian, > > These patches look OK to me. I'd rather take Gregory Clement's "respect > the clock dependencies in of_clk_init" patch towards 3.15, so this fix > will still be necessary for the current -rc's. > > Jason, will you be sending a PR? Ahh, just saw this now. ignore my previous comments about using Gregory's series as the fix. Sure, I'll put this together for a pr to the clk tree. thx, Jason. > > The patches have been boot tested on Dove and compile-tested only > > for Kirkwood, Armada 370 and XP. > > > > Sebastian Hesselbarth (4): > > clk: mvebu: armada-370: maintain clock init order > > clk: mvebu: armada-xp: maintain clock init order > > clk: mvebu: dove: maintain clock init order > > clk: mvebu: kirkwood: maintain clock init order > > > > drivers/clk/mvebu/armada-370.c | 21 ++--- > > drivers/clk/mvebu/armada-xp.c | 20 +--- > > drivers/clk/mvebu/dove.c | 19 +-- > > drivers/clk/mvebu/kirkwood.c | 34 -- > > 4 files changed, 44 insertions(+), 50 deletions(-) > > > > --- > > Cc: Mike Turquette > > Cc: Jason Cooper > > Cc: Andrew Lunn > > Cc: Gregory Clement > > Cc: Thomas Petazzoni > > Cc: Ezequiel Garcia > > Cc: linux-arm-ker...@lists.infradead.org > > Cc: linux-kernel@vger.kernel.org > > -- > > 1.8.5.2 > > -- 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 0/4] clk: mvebu: fix clk init order
Quoting Sebastian Hesselbarth (2014-01-25 10:19:06) > This patch set fixes clk init order that went upside-down with > v3.14. I haven't really investigated what caused this, but I assume > it is related with DT node reordering by addresses. > > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > registered before core clocks driver. Unfortunately, we cannot > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > init order for our drivers is always core clocks before clock gating, > we maintain init order ourselves by hooking CLK_OF_DECLARE to one > init function that will register core clocks before clock gating > driver. > > This patch is based on pre-v3.14-rc1 mainline and should go in as > fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > I suggest Jason picks it up as a topic branch. Sebastian, These patches look OK to me. I'd rather take Gregory Clement's "respect the clock dependencies in of_clk_init" patch towards 3.15, so this fix will still be necessary for the current -rc's. Jason, will you be sending a PR? Thanks, Mike > > The patches have been boot tested on Dove and compile-tested only > for Kirkwood, Armada 370 and XP. > > Sebastian Hesselbarth (4): > clk: mvebu: armada-370: maintain clock init order > clk: mvebu: armada-xp: maintain clock init order > clk: mvebu: dove: maintain clock init order > clk: mvebu: kirkwood: maintain clock init order > > drivers/clk/mvebu/armada-370.c | 21 ++--- > drivers/clk/mvebu/armada-xp.c | 20 +--- > drivers/clk/mvebu/dove.c | 19 +-- > drivers/clk/mvebu/kirkwood.c | 34 -- > 4 files changed, 44 insertions(+), 50 deletions(-) > > --- > Cc: Mike Turquette > Cc: Jason Cooper > Cc: Andrew Lunn > Cc: Gregory Clement > Cc: Thomas Petazzoni > Cc: Ezequiel Garcia > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > -- > 1.8.5.2 > -- 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 0/4] clk: mvebu: fix clk init order
Quoting Sebastian Hesselbarth (2014-01-25 10:19:06) This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. Sebastian, These patches look OK to me. I'd rather take Gregory Clement's respect the clock dependencies in of_clk_init patch towards 3.15, so this fix will still be necessary for the current -rc's. Jason, will you be sending a PR? Thanks, Mike The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) --- Cc: Mike Turquette mturque...@linaro.org Cc: Jason Cooper ja...@lakedaemon.net Cc: Andrew Lunn and...@lunn.ch Cc: Gregory Clement gregory.clem...@free-electrons.com Cc: Thomas Petazzoni thomas.petazz...@free-electrons.com Cc: Ezequiel Garcia ezequiel.gar...@free-electrons.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org -- 1.8.5.2 -- 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 0/4] clk: mvebu: fix clk init order
On Wed, Feb 05, 2014 at 06:08:02AM -0800, Mike Turquette wrote: Quoting Sebastian Hesselbarth (2014-01-25 10:19:06) This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. Sebastian, These patches look OK to me. I'd rather take Gregory Clement's respect the clock dependencies in of_clk_init patch towards 3.15, so this fix will still be necessary for the current -rc's. Jason, will you be sending a PR? Ahh, just saw this now. ignore my previous comments about using Gregory's series as the fix. Sure, I'll put this together for a pr to the clk tree. thx, Jason. The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) --- Cc: Mike Turquette mturque...@linaro.org Cc: Jason Cooper ja...@lakedaemon.net Cc: Andrew Lunn and...@lunn.ch Cc: Gregory Clement gregory.clem...@free-electrons.com Cc: Thomas Petazzoni thomas.petazz...@free-electrons.com Cc: Ezequiel Garcia ezequiel.gar...@free-electrons.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org -- 1.8.5.2 -- 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 0/4] clk: mvebu: fix clk init order
On Sat, Jan 25, 2014 at 07:19:06PM +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) Whole series applied to mvebu/clk-fixes. thx, Jason. -- 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 0/4] clk: mvebu: fix clk init order
Hello, On Tue, 04 Feb 2014 15:58:44 +0100, Gregory CLEMENT wrote: > > @Jason, Andrew, Gregory, Thomas: > > Now that v3.14 is out, anything against taking this in as fixes for rc1? > > I am not found of this solution I still think it should be done > at framework level. However we still have this very annoying issue, > and this fix is better than nothing. So I am not against taking this > for rc1 with the hope that it will be later revert with a better > solution. Same opinion here. I'm fine with this solution as a temporary measure, but it would be good to solve this problem in a nicer way. Also, making this change doesn't impact the DT in any way, there is no problem in having a temporary not perfect solution, and improve it later on. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On 04/02/2014 00:36, Sebastian Hesselbarth wrote: > On 02/04/2014 12:16 AM, Willy Tarreau wrote: >> On Thu, Jan 30, 2014 at 11:31:32AM +0100, Sebastian Hesselbarth wrote: >>> On 01/30/14 11:24, Gregory CLEMENT wrote: On 25/01/2014 19:19, Sebastian Hesselbarth wrote: > This patch set fixes clk init order that went upside-down with > v3.14. I haven't really investigated what caused this, but I assume > it is related with DT node reordering by addresses. Can you explain what kind of issue do you observe? >>> >>> Sure. When probing CLK_OF_DECLAREed clock drivers, clock-gating driver >>> gets registered before core-clocks. It therefore cannot resolve it's >>> parent clock name for tclk and all clock gates will have no parent >>> clock. >>> >>> Usually, you'll see in some drivers (e.g. v643xx_eth) div_by_zero errors >>> poping up, when they calculate a frequency division factors based on >>> clock gate frequency, which should have been tclk but is 0 now. >> >> Well, to be honnest, I have no idea whether your patch is the right way >> to fix the problem or not, but what I can say is that it fixes such oopses >> that appear in 3.14-rc1 when booting on mirabox : >> >> Division by zero in kernel. > > Willy, > > you have hit exactly the reason for this patch. > > [...] >> By the way, seeing how often a trick related to the DT is nedeed to solve an >> oops or a panic, I'm really scared that this whole DT mess is just becoming >> the exact copy of the ACPI mess (but 15 years later) and we'll experience the >> same horrible things :-( Sometimes I'm wondering whether there are not too >> many structural things put in there... > > To be precise, it is not a DT-related trick at all. You would have the > same issues, if you'd register those "low-level" (i.e. early) drivers > without DT. It is more about missing init ordering, here. > > There could be different ways to work this out, even elevating clock > devices to "normal" probed devices could be possible. I am sure, in the > long run, it will work out, but now this is a fix for v3.14-rc1. > > @Jason, Andrew, Gregory, Thomas: > Now that v3.14 is out, anything against taking this in as fixes for rc1? Hi Sebastian, I am not found of this solution I still think it should be done at framework level. However we still have this very annoying issue, and this fix is better than nothing. So I am not against taking this for rc1 with the hope that it will be later revert with a better solution. Thanks, Gregory > > Sebastian > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On 04/02/2014 00:36, Sebastian Hesselbarth wrote: On 02/04/2014 12:16 AM, Willy Tarreau wrote: On Thu, Jan 30, 2014 at 11:31:32AM +0100, Sebastian Hesselbarth wrote: On 01/30/14 11:24, Gregory CLEMENT wrote: On 25/01/2014 19:19, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Can you explain what kind of issue do you observe? Sure. When probing CLK_OF_DECLAREed clock drivers, clock-gating driver gets registered before core-clocks. It therefore cannot resolve it's parent clock name for tclk and all clock gates will have no parent clock. Usually, you'll see in some drivers (e.g. v643xx_eth) div_by_zero errors poping up, when they calculate a frequency division factors based on clock gate frequency, which should have been tclk but is 0 now. Well, to be honnest, I have no idea whether your patch is the right way to fix the problem or not, but what I can say is that it fixes such oopses that appear in 3.14-rc1 when booting on mirabox : Division by zero in kernel. Willy, you have hit exactly the reason for this patch. [...] By the way, seeing how often a trick related to the DT is nedeed to solve an oops or a panic, I'm really scared that this whole DT mess is just becoming the exact copy of the ACPI mess (but 15 years later) and we'll experience the same horrible things :-( Sometimes I'm wondering whether there are not too many structural things put in there... To be precise, it is not a DT-related trick at all. You would have the same issues, if you'd register those low-level (i.e. early) drivers without DT. It is more about missing init ordering, here. There could be different ways to work this out, even elevating clock devices to normal probed devices could be possible. I am sure, in the long run, it will work out, but now this is a fix for v3.14-rc1. @Jason, Andrew, Gregory, Thomas: Now that v3.14 is out, anything against taking this in as fixes for rc1? Hi Sebastian, I am not found of this solution I still think it should be done at framework level. However we still have this very annoying issue, and this fix is better than nothing. So I am not against taking this for rc1 with the hope that it will be later revert with a better solution. Thanks, Gregory Sebastian -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
Hello, On Tue, 04 Feb 2014 15:58:44 +0100, Gregory CLEMENT wrote: @Jason, Andrew, Gregory, Thomas: Now that v3.14 is out, anything against taking this in as fixes for rc1? I am not found of this solution I still think it should be done at framework level. However we still have this very annoying issue, and this fix is better than nothing. So I am not against taking this for rc1 with the hope that it will be later revert with a better solution. Same opinion here. I'm fine with this solution as a temporary measure, but it would be good to solve this problem in a nicer way. Also, making this change doesn't impact the DT in any way, there is no problem in having a temporary not perfect solution, and improve it later on. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On 02/04/2014 12:16 AM, Willy Tarreau wrote: On Thu, Jan 30, 2014 at 11:31:32AM +0100, Sebastian Hesselbarth wrote: On 01/30/14 11:24, Gregory CLEMENT wrote: On 25/01/2014 19:19, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Can you explain what kind of issue do you observe? Sure. When probing CLK_OF_DECLAREed clock drivers, clock-gating driver gets registered before core-clocks. It therefore cannot resolve it's parent clock name for tclk and all clock gates will have no parent clock. Usually, you'll see in some drivers (e.g. v643xx_eth) div_by_zero errors poping up, when they calculate a frequency division factors based on clock gate frequency, which should have been tclk but is 0 now. Well, to be honnest, I have no idea whether your patch is the right way to fix the problem or not, but what I can say is that it fixes such oopses that appear in 3.14-rc1 when booting on mirabox : Division by zero in kernel. Willy, you have hit exactly the reason for this patch. [...] By the way, seeing how often a trick related to the DT is nedeed to solve an oops or a panic, I'm really scared that this whole DT mess is just becoming the exact copy of the ACPI mess (but 15 years later) and we'll experience the same horrible things :-( Sometimes I'm wondering whether there are not too many structural things put in there... To be precise, it is not a DT-related trick at all. You would have the same issues, if you'd register those "low-level" (i.e. early) drivers without DT. It is more about missing init ordering, here. There could be different ways to work this out, even elevating clock devices to "normal" probed devices could be possible. I am sure, in the long run, it will work out, but now this is a fix for v3.14-rc1. @Jason, Andrew, Gregory, Thomas: Now that v3.14 is out, anything against taking this in as fixes for rc1? Sebastian -- 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 0/4] clk: mvebu: fix clk init order
Hi Sebastian, On Thu, Jan 30, 2014 at 11:31:32AM +0100, Sebastian Hesselbarth wrote: > On 01/30/14 11:24, Gregory CLEMENT wrote: > >On 25/01/2014 19:19, Sebastian Hesselbarth wrote: > >>This patch set fixes clk init order that went upside-down with > >>v3.14. I haven't really investigated what caused this, but I assume > >>it is related with DT node reordering by addresses. > > > >Can you explain what kind of issue do you observe? > > Sure. When probing CLK_OF_DECLAREed clock drivers, clock-gating driver > gets registered before core-clocks. It therefore cannot resolve it's > parent clock name for tclk and all clock gates will have no parent > clock. > > Usually, you'll see in some drivers (e.g. v643xx_eth) div_by_zero errors > poping up, when they calculate a frequency division factors based on > clock gate frequency, which should have been tclk but is 0 now. Well, to be honnest, I have no idea whether your patch is the right way to fix the problem or not, but what I can say is that it fixes such oopses that appear in 3.14-rc1 when booting on mirabox : Division by zero in kernel. CPU: 0 PID: 6 Comm: kworker/u2:0 Not tainted 3.13.0-bck-mbx #6 Workqueue: kmmcd mmc_rescan [] (unwind_backtrace+0x1/0x98) from [] (show_stack+0xb/0xc) [] (show_stack+0xb/0xc) from [] (Ldiv0+0x9/0x12) [] (Ldiv0+0x9/0x12) from [] (mvsd_set_ios+0xcb/0x160) [] (mvsd_set_ios+0xcb/0x160) from [] (__mmc_set_clock+0x2d/0 x40) [] (__mmc_set_clock+0x2d/0x40) from [] (mmc_sdio_init_card+0 x391/0x808) [] (mmc_sdio_init_card+0x391/0x808) from [] (mmc_attach_sdio +0x5b/0x218) [] (mmc_attach_sdio+0x5b/0x218) from [] (mmc_rescan+0x159/0x 1b4) [] (mmc_rescan+0x159/0x1b4) from [] (process_one_work+0xa9/0 x21c) [] (process_one_work+0xa9/0x21c) from [] (worker_thread+0xb5 /0x248) [] (worker_thread+0xb5/0x248) from [] (kthread+0x7b/0x94) +0xa7/0x138) [] (kernel_init_freeable+0xa7/0x138) from [] (kernel_init+0x 7/0xb8) [] (kernel_init+0x7/0xb8) from [] (ret_from_fork+0x11/0x34) mvsdio d00d4000.mvsdio: lacking card detect (fall back to polling) By the way, seeing how often a trick related to the DT is nedeed to solve an oops or a panic, I'm really scared that this whole DT mess is just becoming the exact copy of the ACPI mess (but 15 years later) and we'll experience the same horrible things :-( Sometimes I'm wondering whether there are not too many structural things put in there... Regards, Willy -- 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 0/4] clk: mvebu: fix clk init order
Hi Sebastian, On Thu, Jan 30, 2014 at 11:31:32AM +0100, Sebastian Hesselbarth wrote: On 01/30/14 11:24, Gregory CLEMENT wrote: On 25/01/2014 19:19, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Can you explain what kind of issue do you observe? Sure. When probing CLK_OF_DECLAREed clock drivers, clock-gating driver gets registered before core-clocks. It therefore cannot resolve it's parent clock name for tclk and all clock gates will have no parent clock. Usually, you'll see in some drivers (e.g. v643xx_eth) div_by_zero errors poping up, when they calculate a frequency division factors based on clock gate frequency, which should have been tclk but is 0 now. Well, to be honnest, I have no idea whether your patch is the right way to fix the problem or not, but what I can say is that it fixes such oopses that appear in 3.14-rc1 when booting on mirabox : Division by zero in kernel. CPU: 0 PID: 6 Comm: kworker/u2:0 Not tainted 3.13.0-bck-mbx #6 Workqueue: kmmcd mmc_rescan [c0010865] (unwind_backtrace+0x1/0x98) from [c000e947] (show_stack+0xb/0xc) [c000e947] (show_stack+0xb/0xc) from [c0135913] (Ldiv0+0x9/0x12) [c0135913] (Ldiv0+0x9/0x12) from [c01f708b] (mvsd_set_ios+0xcb/0x160) [c01f708b] (mvsd_set_ios+0xcb/0x160) from [c01ec1fd] (__mmc_set_clock+0x2d/0 x40) [c01ec1fd] (__mmc_set_clock+0x2d/0x40) from [c01f1a59] (mmc_sdio_init_card+0 x391/0x808) [c01f1a59] (mmc_sdio_init_card+0x391/0x808) from [c01f2163] (mmc_attach_sdio +0x5b/0x218) [c01f2163] (mmc_attach_sdio+0x5b/0x218) from [c01ed0d9] (mmc_rescan+0x159/0x 1b4) [c01ed0d9] (mmc_rescan+0x159/0x1b4) from [c0024579] (process_one_work+0xa9/0 x21c) [c0024579] (process_one_work+0xa9/0x21c) from [c002494d] (worker_thread+0xb5 /0x248) [c002494d] (worker_thread+0xb5/0x248) from [c0027f1b] (kthread+0x7b/0x94) +0xa7/0x138) [c04228b3] (kernel_init_freeable+0xa7/0x138) from [c027e6cf] (kernel_init+0x 7/0xb8) [c027e6cf] (kernel_init+0x7/0xb8) from [c000cb9d] (ret_from_fork+0x11/0x34) mvsdio d00d4000.mvsdio: lacking card detect (fall back to polling) By the way, seeing how often a trick related to the DT is nedeed to solve an oops or a panic, I'm really scared that this whole DT mess is just becoming the exact copy of the ACPI mess (but 15 years later) and we'll experience the same horrible things :-( Sometimes I'm wondering whether there are not too many structural things put in there... Regards, Willy -- 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 0/4] clk: mvebu: fix clk init order
On 02/04/2014 12:16 AM, Willy Tarreau wrote: On Thu, Jan 30, 2014 at 11:31:32AM +0100, Sebastian Hesselbarth wrote: On 01/30/14 11:24, Gregory CLEMENT wrote: On 25/01/2014 19:19, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Can you explain what kind of issue do you observe? Sure. When probing CLK_OF_DECLAREed clock drivers, clock-gating driver gets registered before core-clocks. It therefore cannot resolve it's parent clock name for tclk and all clock gates will have no parent clock. Usually, you'll see in some drivers (e.g. v643xx_eth) div_by_zero errors poping up, when they calculate a frequency division factors based on clock gate frequency, which should have been tclk but is 0 now. Well, to be honnest, I have no idea whether your patch is the right way to fix the problem or not, but what I can say is that it fixes such oopses that appear in 3.14-rc1 when booting on mirabox : Division by zero in kernel. Willy, you have hit exactly the reason for this patch. [...] By the way, seeing how often a trick related to the DT is nedeed to solve an oops or a panic, I'm really scared that this whole DT mess is just becoming the exact copy of the ACPI mess (but 15 years later) and we'll experience the same horrible things :-( Sometimes I'm wondering whether there are not too many structural things put in there... To be precise, it is not a DT-related trick at all. You would have the same issues, if you'd register those low-level (i.e. early) drivers without DT. It is more about missing init ordering, here. There could be different ways to work this out, even elevating clock devices to normal probed devices could be possible. I am sure, in the long run, it will work out, but now this is a fix for v3.14-rc1. @Jason, Andrew, Gregory, Thomas: Now that v3.14 is out, anything against taking this in as fixes for rc1? Sebastian -- 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 0/4] clk: mvebu: fix clk init order
On 01/30/14 11:24, Gregory CLEMENT wrote: On 25/01/2014 19:19, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Can you explain what kind of issue do you observe? Sure. When probing CLK_OF_DECLAREed clock drivers, clock-gating driver gets registered before core-clocks. It therefore cannot resolve it's parent clock name for tclk and all clock gates will have no parent clock. Usually, you'll see in some drivers (e.g. v643xx_eth) div_by_zero errors poping up, when they calculate a frequency division factors based on clock gate frequency, which should have been tclk but is 0 now. I have just tested the master branch of Linus and (excepted for SATA but Andrew will send a patch set soon), I didn't experiment any issues on Armada 370 and Armada XP based boards. -- 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 0/4] clk: mvebu: fix clk init order
Hi Sebastian, On 25/01/2014 19:19, Sebastian Hesselbarth wrote: > This patch set fixes clk init order that went upside-down with > v3.14. I haven't really investigated what caused this, but I assume > it is related with DT node reordering by addresses. Can you explain what kind of issue do you observe? I have just tested the master branch of Linus and (excepted for SATA but Andrew will send a patch set soon), I didn't experiment any issues on Armada 370 and Armada XP based boards. Thanks, Gregory > > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > registered before core clocks driver. Unfortunately, we cannot > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > init order for our drivers is always core clocks before clock gating, > we maintain init order ourselves by hooking CLK_OF_DECLARE to one > init function that will register core clocks before clock gating > driver. > > This patch is based on pre-v3.14-rc1 mainline and should go in as > fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > I suggest Jason picks it up as a topic branch. > > The patches have been boot tested on Dove and compile-tested only > for Kirkwood, Armada 370 and XP. > > Sebastian Hesselbarth (4): > clk: mvebu: armada-370: maintain clock init order > clk: mvebu: armada-xp: maintain clock init order > clk: mvebu: dove: maintain clock init order > clk: mvebu: kirkwood: maintain clock init order > > drivers/clk/mvebu/armada-370.c | 21 ++--- > drivers/clk/mvebu/armada-xp.c | 20 +--- > drivers/clk/mvebu/dove.c | 19 +-- > drivers/clk/mvebu/kirkwood.c | 34 -- > 4 files changed, 44 insertions(+), 50 deletions(-) > > --- > Cc: Mike Turquette > Cc: Jason Cooper > Cc: Andrew Lunn > Cc: Gregory Clement > Cc: Thomas Petazzoni > Cc: Ezequiel Garcia > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
Hi Sebastian, On 25/01/2014 19:19, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Can you explain what kind of issue do you observe? I have just tested the master branch of Linus and (excepted for SATA but Andrew will send a patch set soon), I didn't experiment any issues on Armada 370 and Armada XP based boards. Thanks, Gregory Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. The patches have been boot tested on Dove and compile-tested only for Kirkwood, Armada 370 and XP. Sebastian Hesselbarth (4): clk: mvebu: armada-370: maintain clock init order clk: mvebu: armada-xp: maintain clock init order clk: mvebu: dove: maintain clock init order clk: mvebu: kirkwood: maintain clock init order drivers/clk/mvebu/armada-370.c | 21 ++--- drivers/clk/mvebu/armada-xp.c | 20 +--- drivers/clk/mvebu/dove.c | 19 +-- drivers/clk/mvebu/kirkwood.c | 34 -- 4 files changed, 44 insertions(+), 50 deletions(-) --- Cc: Mike Turquette mturque...@linaro.org Cc: Jason Cooper ja...@lakedaemon.net Cc: Andrew Lunn and...@lunn.ch Cc: Gregory Clement gregory.clem...@free-electrons.com Cc: Thomas Petazzoni thomas.petazz...@free-electrons.com Cc: Ezequiel Garcia ezequiel.gar...@free-electrons.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On 01/30/14 11:24, Gregory CLEMENT wrote: On 25/01/2014 19:19, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Can you explain what kind of issue do you observe? Sure. When probing CLK_OF_DECLAREed clock drivers, clock-gating driver gets registered before core-clocks. It therefore cannot resolve it's parent clock name for tclk and all clock gates will have no parent clock. Usually, you'll see in some drivers (e.g. v643xx_eth) div_by_zero errors poping up, when they calculate a frequency division factors based on clock gate frequency, which should have been tclk but is 0 now. I have just tested the master branch of Linus and (excepted for SATA but Andrew will send a patch set soon), I didn't experiment any issues on Armada 370 and Armada XP based boards. -- 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 0/4] clk: mvebu: fix clk init order
On Mon, Jan 27, 2014 at 07:21:38PM +0100, Sebastian Hesselbarth wrote: > On 01/27/14 15:39, Thomas Petazzoni wrote: > >On Sat, 25 Jan 2014 19:19:06 +0100, Sebastian Hesselbarth wrote: > >>This patch set fixes clk init order that went upside-down with > >>v3.14. I haven't really investigated what caused this, but I assume > >>it is related with DT node reordering by addresses. > >> > >>Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > >>registered before core clocks driver. Unfortunately, we cannot > >>return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > >>init order for our drivers is always core clocks before clock gating, > >>we maintain init order ourselves by hooking CLK_OF_DECLARE to one > >>init function that will register core clocks before clock gating > >>driver. > >> > >>This patch is based on pre-v3.14-rc1 mainline and should go in as > >>fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > >>I suggest Jason picks it up as a topic branch. > > > >I'm not sure I really like the solution you're proposing here. I'd very > >much prefer to keep one CLK_OF_DECLARE() per clock type, associated to > >one function registering only this clock type. > > Have you ever had a look at e.g. clk-imx28.c? Not that I really like > the approach, but it is common practice to do so. > > >Instead, shouldn't the clock framework be improved to *not* register a > >clock until its parent have been registered? If the DT you have the > >gatable clocks that depend on the core clocks, then the gatable clocks > >should not be registered if the core clocks have not yet been > >registered. > > > >Do you think this is possible? Am I missing something here? > > As I said, clk_of_init does not care about return values from the > clock init functions. Without it, it cannot decide if a clock > driver failed horribly, failed because of missing dependencies, or > successfully installed all clocks. Also, it is early stuff and I guess > clk_of_init will have to build its own "defered_list" and loop over > until done. > > BTW, this is a fix not an improvement. We should find an acceptable > solution soon. But I am still open for suggestions, too. fyi: I suspect this may be the problem currently experienced by Kevin on mirabox in the boot farm. He sees it on current master. Adding him to the Cc. thx, Jason. -- 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 0/4] clk: mvebu: fix clk init order
On 01/27/14 15:39, Thomas Petazzoni wrote: On Sat, 25 Jan 2014 19:19:06 +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. I'm not sure I really like the solution you're proposing here. I'd very much prefer to keep one CLK_OF_DECLARE() per clock type, associated to one function registering only this clock type. Have you ever had a look at e.g. clk-imx28.c? Not that I really like the approach, but it is common practice to do so. Instead, shouldn't the clock framework be improved to *not* register a clock until its parent have been registered? If the DT you have the gatable clocks that depend on the core clocks, then the gatable clocks should not be registered if the core clocks have not yet been registered. Do you think this is possible? Am I missing something here? As I said, clk_of_init does not care about return values from the clock init functions. Without it, it cannot decide if a clock driver failed horribly, failed because of missing dependencies, or successfully installed all clocks. Also, it is early stuff and I guess clk_of_init will have to build its own "defered_list" and loop over until done. BTW, this is a fix not an improvement. We should find an acceptable solution soon. But I am still open for suggestions, too. Sebastian -- 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 0/4] clk: mvebu: fix clk init order
Dear Sebastian Hesselbarth, On Sat, 25 Jan 2014 19:19:06 +0100, Sebastian Hesselbarth wrote: > This patch set fixes clk init order that went upside-down with > v3.14. I haven't really investigated what caused this, but I assume > it is related with DT node reordering by addresses. > > Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets > registered before core clocks driver. Unfortunately, we cannot > return -EPROBE_DEFER in drivers initialized by clk_of_init. As the > init order for our drivers is always core clocks before clock gating, > we maintain init order ourselves by hooking CLK_OF_DECLARE to one > init function that will register core clocks before clock gating > driver. > > This patch is based on pre-v3.14-rc1 mainline and should go in as > fixes for it. As we now send MVEBU clk pull-requests to Mike directly, > I suggest Jason picks it up as a topic branch. I'm not sure I really like the solution you're proposing here. I'd very much prefer to keep one CLK_OF_DECLARE() per clock type, associated to one function registering only this clock type. Instead, shouldn't the clock framework be improved to *not* register a clock until its parent have been registered? If the DT you have the gatable clocks that depend on the core clocks, then the gatable clocks should not be registered if the core clocks have not yet been registered. Do you think this is possible? Am I missing something here? Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
Dear Sebastian Hesselbarth, On Sat, 25 Jan 2014 19:19:06 +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. I'm not sure I really like the solution you're proposing here. I'd very much prefer to keep one CLK_OF_DECLARE() per clock type, associated to one function registering only this clock type. Instead, shouldn't the clock framework be improved to *not* register a clock until its parent have been registered? If the DT you have the gatable clocks that depend on the core clocks, then the gatable clocks should not be registered if the core clocks have not yet been registered. Do you think this is possible? Am I missing something here? Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
On 01/27/14 15:39, Thomas Petazzoni wrote: On Sat, 25 Jan 2014 19:19:06 +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. I'm not sure I really like the solution you're proposing here. I'd very much prefer to keep one CLK_OF_DECLARE() per clock type, associated to one function registering only this clock type. Have you ever had a look at e.g. clk-imx28.c? Not that I really like the approach, but it is common practice to do so. Instead, shouldn't the clock framework be improved to *not* register a clock until its parent have been registered? If the DT you have the gatable clocks that depend on the core clocks, then the gatable clocks should not be registered if the core clocks have not yet been registered. Do you think this is possible? Am I missing something here? As I said, clk_of_init does not care about return values from the clock init functions. Without it, it cannot decide if a clock driver failed horribly, failed because of missing dependencies, or successfully installed all clocks. Also, it is early stuff and I guess clk_of_init will have to build its own defered_list and loop over until done. BTW, this is a fix not an improvement. We should find an acceptable solution soon. But I am still open for suggestions, too. Sebastian -- 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 0/4] clk: mvebu: fix clk init order
On Mon, Jan 27, 2014 at 07:21:38PM +0100, Sebastian Hesselbarth wrote: On 01/27/14 15:39, Thomas Petazzoni wrote: On Sat, 25 Jan 2014 19:19:06 +0100, Sebastian Hesselbarth wrote: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. As the init order for our drivers is always core clocks before clock gating, we maintain init order ourselves by hooking CLK_OF_DECLARE to one init function that will register core clocks before clock gating driver. This patch is based on pre-v3.14-rc1 mainline and should go in as fixes for it. As we now send MVEBU clk pull-requests to Mike directly, I suggest Jason picks it up as a topic branch. I'm not sure I really like the solution you're proposing here. I'd very much prefer to keep one CLK_OF_DECLARE() per clock type, associated to one function registering only this clock type. Have you ever had a look at e.g. clk-imx28.c? Not that I really like the approach, but it is common practice to do so. Instead, shouldn't the clock framework be improved to *not* register a clock until its parent have been registered? If the DT you have the gatable clocks that depend on the core clocks, then the gatable clocks should not be registered if the core clocks have not yet been registered. Do you think this is possible? Am I missing something here? As I said, clk_of_init does not care about return values from the clock init functions. Without it, it cannot decide if a clock driver failed horribly, failed because of missing dependencies, or successfully installed all clocks. Also, it is early stuff and I guess clk_of_init will have to build its own defered_list and loop over until done. BTW, this is a fix not an improvement. We should find an acceptable solution soon. But I am still open for suggestions, too. fyi: I suspect this may be the problem currently experienced by Kevin on mirabox in the boot farm. He sees it on current master. Adding him to the Cc. thx, Jason. -- 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 0/4] clk: mvebu: fix clk init order
Hi Emilio, Thanks for your help with this. On Sat, Jan 25, 2014 at 07:11:07PM -0300, Emilio López wrote: [..] > > > > Ok, I'll look if using of_clk_get_parent_name will help here. But again, > > I can see that clk-gating driver gets registered before core-clk driver. > > There may be no code to give you the parent name at that time. > > After looking at some of the armada*.dtsi, I see you don't list the > clock names on the coreclk node, so of_clk_get_parent_name may not be of > much value after all. > IIRC, we faced a similar issue with the Core Divider clock and solved it by specifying the clock names in the DT. I meant to complete the core and gating clocks in a similar way (providing names on the DT), but apparently (as discussed with Gregory Clement) Mike Turquette and others are planning to remove the clock names from the DT entirely. Can you guys explain about this plan a bit further? Or do you think we should specify the names on the DT for all the clocks instead? Notice that the latter would remove lots of strings from the kernel itself (right?) -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- 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 0/4] clk: mvebu: fix clk init order
Sebastian, El 25/01/14 18:44, Sebastian Hesselbarth escribió: On 01/25/2014 10:32 PM, Emilio López wrote: El 25/01/14 15:19, Sebastian Hesselbarth escribió: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. The framework should be able to deal with unordered registration. I am not very familiar with the mvebu driver though, do you have a valid reason to require a specific order? Emilio, I rather think that everthing registered with CLK_OF_DECLARE cannot deal with unordered registration. The callback passed to CLK_OF_DECLARE has to have void as return value, so there is no way to pass errors, e.g. -EPROBE_DEFER, back to of_clk_init. Indeed. What I meant is that the framework works fine if you first register a child clock that refers to a not yet registered parent, and then register the parent. The registration need not be strictly ordered. The reason for this ordering is that the clock gates depend on core clocks. It is always that way, so merging both init functions isn't that odd. If your only dependency is the parent name, and you can use DT or something else to get it, then you don't need to enforce an order. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. Why would you need to do so? After a quick inspection on the code, I see you may have problems on mvebu_clk_gating_setup() when getting the default parent clock name, but I believe you could solve it in an easier way by using of_clk_get_parent_name(). Ok, I'll look if using of_clk_get_parent_name will help here. But again, I can see that clk-gating driver gets registered before core-clk driver. There may be no code to give you the parent name at that time. After looking at some of the armada*.dtsi, I see you don't list the clock names on the coreclk node, so of_clk_get_parent_name may not be of much value after all. Cheers, Emilio -- 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 0/4] clk: mvebu: fix clk init order
Hello Sebastian, El 25/01/14 15:19, Sebastian Hesselbarth escribió: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. The framework should be able to deal with unordered registration. I am not very familiar with the mvebu driver though, do you have a valid reason to require a specific order? Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. Why would you need to do so? After a quick inspection on the code, I see you may have problems on mvebu_clk_gating_setup() when getting the default parent clock name, but I believe you could solve it in an easier way by using of_clk_get_parent_name(). Cheers, Emilio -- 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 0/4] clk: mvebu: fix clk init order
On 01/25/2014 10:32 PM, Emilio López wrote: El 25/01/14 15:19, Sebastian Hesselbarth escribió: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. The framework should be able to deal with unordered registration. I am not very familiar with the mvebu driver though, do you have a valid reason to require a specific order? Emilio, I rather think that everthing registered with CLK_OF_DECLARE cannot deal with unordered registration. The callback passed to CLK_OF_DECLARE has to have void as return value, so there is no way to pass errors, e.g. -EPROBE_DEFER, back to of_clk_init. The reason for this ordering is that the clock gates depend on core clocks. It is always that way, so merging both init functions isn't that odd. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. Why would you need to do so? After a quick inspection on the code, I see you may have problems on mvebu_clk_gating_setup() when getting the default parent clock name, but I believe you could solve it in an easier way by using of_clk_get_parent_name(). Ok, I'll look if using of_clk_get_parent_name will help here. But again, I can see that clk-gating driver gets registered before core-clk driver. There may be no code to give you the parent name at that time. Sebastian -- 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 0/4] clk: mvebu: fix clk init order
On 01/25/2014 10:32 PM, Emilio López wrote: El 25/01/14 15:19, Sebastian Hesselbarth escribió: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. The framework should be able to deal with unordered registration. I am not very familiar with the mvebu driver though, do you have a valid reason to require a specific order? Emilio, I rather think that everthing registered with CLK_OF_DECLARE cannot deal with unordered registration. The callback passed to CLK_OF_DECLARE has to have void as return value, so there is no way to pass errors, e.g. -EPROBE_DEFER, back to of_clk_init. The reason for this ordering is that the clock gates depend on core clocks. It is always that way, so merging both init functions isn't that odd. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. Why would you need to do so? After a quick inspection on the code, I see you may have problems on mvebu_clk_gating_setup() when getting the default parent clock name, but I believe you could solve it in an easier way by using of_clk_get_parent_name(). Ok, I'll look if using of_clk_get_parent_name will help here. But again, I can see that clk-gating driver gets registered before core-clk driver. There may be no code to give you the parent name at that time. Sebastian -- 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 0/4] clk: mvebu: fix clk init order
Hello Sebastian, El 25/01/14 15:19, Sebastian Hesselbarth escribió: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. The framework should be able to deal with unordered registration. I am not very familiar with the mvebu driver though, do you have a valid reason to require a specific order? Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. Why would you need to do so? After a quick inspection on the code, I see you may have problems on mvebu_clk_gating_setup() when getting the default parent clock name, but I believe you could solve it in an easier way by using of_clk_get_parent_name(). Cheers, Emilio -- 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 0/4] clk: mvebu: fix clk init order
Sebastian, El 25/01/14 18:44, Sebastian Hesselbarth escribió: On 01/25/2014 10:32 PM, Emilio López wrote: El 25/01/14 15:19, Sebastian Hesselbarth escribió: This patch set fixes clk init order that went upside-down with v3.14. I haven't really investigated what caused this, but I assume it is related with DT node reordering by addresses. The framework should be able to deal with unordered registration. I am not very familiar with the mvebu driver though, do you have a valid reason to require a specific order? Emilio, I rather think that everthing registered with CLK_OF_DECLARE cannot deal with unordered registration. The callback passed to CLK_OF_DECLARE has to have void as return value, so there is no way to pass errors, e.g. -EPROBE_DEFER, back to of_clk_init. Indeed. What I meant is that the framework works fine if you first register a child clock that refers to a not yet registered parent, and then register the parent. The registration need not be strictly ordered. The reason for this ordering is that the clock gates depend on core clocks. It is always that way, so merging both init functions isn't that odd. If your only dependency is the parent name, and you can use DT or something else to get it, then you don't need to enforce an order. Anyway, with v3.14 for MVEBU SoCs, the clock gating driver gets registered before core clocks driver. Unfortunately, we cannot return -EPROBE_DEFER in drivers initialized by clk_of_init. Why would you need to do so? After a quick inspection on the code, I see you may have problems on mvebu_clk_gating_setup() when getting the default parent clock name, but I believe you could solve it in an easier way by using of_clk_get_parent_name(). Ok, I'll look if using of_clk_get_parent_name will help here. But again, I can see that clk-gating driver gets registered before core-clk driver. There may be no code to give you the parent name at that time. After looking at some of the armada*.dtsi, I see you don't list the clock names on the coreclk node, so of_clk_get_parent_name may not be of much value after all. Cheers, Emilio -- 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 0/4] clk: mvebu: fix clk init order
Hi Emilio, Thanks for your help with this. On Sat, Jan 25, 2014 at 07:11:07PM -0300, Emilio López wrote: [..] Ok, I'll look if using of_clk_get_parent_name will help here. But again, I can see that clk-gating driver gets registered before core-clk driver. There may be no code to give you the parent name at that time. After looking at some of the armada*.dtsi, I see you don't list the clock names on the coreclk node, so of_clk_get_parent_name may not be of much value after all. IIRC, we faced a similar issue with the Core Divider clock and solved it by specifying the clock names in the DT. I meant to complete the core and gating clocks in a similar way (providing names on the DT), but apparently (as discussed with Gregory Clement) Mike Turquette and others are planning to remove the clock names from the DT entirely. Can you guys explain about this plan a bit further? Or do you think we should specify the names on the DT for all the clocks instead? Notice that the latter would remove lots of strings from the kernel itself (right?) -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- 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/
