| Message ID | 1359399397-29729-12-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/28/2013 11:56 AM, Thomas Petazzoni wrote: > The Armada 370 has two gatable clocks for each PCIe interface, and we > want both of them to be enabled. We therefore make one of the two > clocks a child of the other, as we did for the sataX and sataXlnk > clocks on Armada XP. > diff --git a/drivers/clk/mvebu/clk-gating-ctrl.c b/drivers/clk/mvebu/clk-gating-ctrl.c > @@ -119,8 +119,8 @@ static const struct mvebu_soc_descr __initconst armada_370_gating_descr[] = { > { "pex1_en", NULL, 2 }, > { "ge1", NULL, 3 }, > { "ge0", NULL, 4 }, > - { "pex0", NULL, 5 }, > - { "pex1", NULL, 9 }, > + { "pex0", "pex0_en", 5 }, > + { "pex1", "pex1_en", 9 }, I must admit, I know nothing about struct mvebu_soc_descr, but I'm having a hard time seeing how that code change makes one of those clock a parent of the other, since the pex0 entry doesn't reference anything "pex1"-related, nor vice-versa. Is more explanation in the commit message warranted here?
Dear Stephen Warren, On Mon, 28 Jan 2013 15:08:46 -0700, Stephen Warren wrote: > I must admit, I know nothing about struct mvebu_soc_descr, but I'm > having a hard time seeing how that code change makes one of those clock > a parent of the other, since the pex0 entry doesn't reference anything > "pex1"-related, nor vice-versa. Is more explanation in the commit > message warranted here? See the definition of mvebu_soc_descr: struct mvebu_soc_descr { const char *name; const char *parent; int bit_idx; }; It simply registers the pex0 clock with the pex0_en clock as its parents. Those clocks are normal gatable clocks, registered with clk_register_gate(). This ensures that whenever the pex0 clock is enabled, its parent clock pex0_en gets enabled as well. We do the same for SATA clocks on Armada XP, for example: static const struct mvebu_soc_descr __initconst armada_xp_gating_descr[] = { { "audio", NULL, 0 }, [...] { "sata0lnk", NULL, 14 }, { "sata0", "sata0lnk", 15 }, [...] { "sata1lnk", NULL, 29 }, { "sata1", "sata1lnk", 30 }, { } }; Best regards, Thomas
On 01/28/2013 03:21 PM, Thomas Petazzoni wrote: > Dear Stephen Warren, > > On Mon, 28 Jan 2013 15:08:46 -0700, Stephen Warren wrote: > >> I must admit, I know nothing about struct mvebu_soc_descr, but I'm >> having a hard time seeing how that code change makes one of those clock >> a parent of the other, since the pex0 entry doesn't reference anything >> "pex1"-related, nor vice-versa. Is more explanation in the commit >> message warranted here? > > See the definition of mvebu_soc_descr: > > struct mvebu_soc_descr { > const char *name; > const char *parent; > int bit_idx; > }; > > It simply registers the pex0 clock with the pex0_en clock as its > parents. Those clocks are normal gatable clocks, registered with > clk_register_gate(). This ensures that whenever the pex0 clock is > enabled, its parent clock pex0_en gets enabled as well. Oh I see; I was confused by the patch description. The two clocks being made child/parent are the two clocks for a port, and this relationship is set up for each port; for some reason I thought there was a requirement to make one port's clock a child of the other port's clock.
Dear Stephen Warren, On Mon, 28 Jan 2013 15:27:19 -0700, Stephen Warren wrote: > Oh I see; I was confused by the patch description. The two clocks being > made child/parent are the two clocks for a port, and this relationship > is set up for each port; for some reason I thought there was a > requirement to make one port's clock a child of the other port's clock. Aah, ok, I understand the confusion now. Re-reading my commit log, I'm not sure where the confusion comes from, but english is not my native language, so maybe something that sounds clear to me is not clear in reality. I'll rephrase the commit log to make sure this confusion is clarified. Thanks! Thomas
diff --git a/drivers/clk/mvebu/clk-gating-ctrl.c b/drivers/clk/mvebu/clk-gating-ctrl.c index 8fa5408..fd52b5f 100644 --- a/drivers/clk/mvebu/clk-gating-ctrl.c +++ b/drivers/clk/mvebu/clk-gating-ctrl.c @@ -119,8 +119,8 @@ static const struct mvebu_soc_descr __initconst armada_370_gating_descr[] = { { "pex1_en", NULL, 2 }, { "ge1", NULL, 3 }, { "ge0", NULL, 4 }, - { "pex0", NULL, 5 }, - { "pex1", NULL, 9 }, + { "pex0", "pex0_en", 5 }, + { "pex1", "pex1_en", 9 }, { "sata0", NULL, 15 }, { "sdio", NULL, 17 }, { "tdm", NULL, 25 },
The Armada 370 has two gatable clocks for each PCIe interface, and we want both of them to be enabled. We therefore make one of the two clocks a child of the other, as we did for the sataX and sataXlnk clocks on Armada XP. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Mike Turquette <mturquette@linaro.org> --- drivers/clk/mvebu/clk-gating-ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)