Patchwork [V3,1/4] ARM: tegra30: clocks: Fix pciex clock registration

login
register
mail settings
Submitter Jay Agarwal
Date June 4, 2013, 6:57 p.m.
Message ID <1370372252-4332-1-git-send-email-jagarwal@nvidia.com>
Download mbox | patch
Permalink /patch/248810/
State Not Applicable
Headers show

Comments

Jay Agarwal - June 4, 2013, 6:57 p.m.
Registering pciex as peripheral clock instead of fixed clock
as tegra_perih_reset_assert(deassert) api of this clock api
gives warning and ultimately does not succeed to assert(deassert).

Signed-off-by: Jay Agarwal <jagarwal@nvidia.com>
---
Patch is based on remotes/gitorious_thierryreding_linux/tegra/next and should be applied on top of this.

Changes in V3:
- Avoid removing pciex duplicate clock as per review comment
- Corrected tegra pcie driver name for duplicate clocks

 drivers/clk/tegra/clk-tegra30.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)
Stephen Warren - June 4, 2013, 7:08 p.m.
On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> Registering pciex as peripheral clock instead of fixed clock
> as tegra_perih_reset_assert(deassert) api of this clock api
> gives warning and ultimately does not succeed to assert(deassert).
> 
> Signed-off-by: Jay Agarwal <jagarwal@nvidia.com>
> ---
> Patch is based on remotes/gitorious_thierryreding_linux/tegra/next and should be applied on top of this.

For this change, Mike may as well apply it directly to the clock tree.
Thierry can then pick it up when he rebases his tegra/next tree.

That said, I don't think you should need any of the
TEGRA_CLK_DUPLICATE() entries; the PCIe driver should get its clocks
from device tree now, and hence the driver name in the clock
registration shouldn't be necessary. All of these TEGRA_CLK_DUPLICATE()
entries should be removed en mass sometime soon with luck. So, can you
simply leave the two TEGRA_CLK_DUPLICATE() entries untouched, rather
than changing them?
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Turquette - June 11, 2013, 10:17 p.m.
Quoting Stephen Warren (2013-06-04 12:08:08)
> On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> > Registering pciex as peripheral clock instead of fixed clock
> > as tegra_perih_reset_assert(deassert) api of this clock api
> > gives warning and ultimately does not succeed to assert(deassert).
> > 
> > Signed-off-by: Jay Agarwal <jagarwal@nvidia.com>
> > ---
> > Patch is based on remotes/gitorious_thierryreding_linux/tegra/next and should be applied on top of this.
> 
> For this change, Mike may as well apply it directly to the clock tree.
> Thierry can then pick it up when he rebases his tegra/next tree.
> 
> That said, I don't think you should need any of the
> TEGRA_CLK_DUPLICATE() entries; the PCIe driver should get its clocks
> from device tree now, and hence the driver name in the clock
> registration shouldn't be necessary. All of these TEGRA_CLK_DUPLICATE()
> entries should be removed en mass sometime soon with luck. So, can you
> simply leave the two TEGRA_CLK_DUPLICATE() entries untouched, rather
> than changing them?

Ping on this patch.  I can take it through my tree, but is there going
to be rework based on Stephen's comments?

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jay Agarwal - June 12, 2013, 7:11 a.m.
> Quoting Stephen Warren (2013-06-04 12:08:08)

> > On 06/04/2013 12:57 PM, Jay Agarwal wrote:

> > > Registering pciex as peripheral clock instead of fixed clock as

> > > tegra_perih_reset_assert(deassert) api of this clock api gives

> > > warning and ultimately does not succeed to assert(deassert).

> > >

> > > Signed-off-by: Jay Agarwal <jagarwal@nvidia.com>

> > > ---

> > > Patch is based on remotes/gitorious_thierryreding_linux/tegra/next and

> should be applied on top of this.

> >

> > For this change, Mike may as well apply it directly to the clock tree.

> > Thierry can then pick it up when he rebases his tegra/next tree.

> >

> > That said, I don't think you should need any of the

> > TEGRA_CLK_DUPLICATE() entries; the PCIe driver should get its clocks

> > from device tree now, and hence the driver name in the clock

> > registration shouldn't be necessary. All of these

> > TEGRA_CLK_DUPLICATE() entries should be removed en mass sometime

> soon

> > with luck. So, can you simply leave the two TEGRA_CLK_DUPLICATE()

> > entries untouched, rather than changing them?

> 

> Ping on this patch.  I can take it through my tree, but is there going to be

> rework based on Stephen's comments?


Hi Mike,
I have uploaded next version V4 of this patch after taking care of this comment.

Patch

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index c6921f5..edb2b9b 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1598,6 +1598,12 @@  static void __init tegra30_periph_clk_init(void)
 	clk_register_clkdev(clk, "afi", "tegra-pcie");
 	clks[afi] = clk;
 
+	/* pciex */
+	clk = tegra_clk_register_periph_gate("pciex", "pll_e", 0, clk_base, 0,
+				    74, &periph_u_regs, periph_clk_enb_refcnt);
+	clk_register_clkdev(clk, "pciex", "tegra-pcie");
+	clks[pciex] = clk;
+
 	/* kfuse */
 	clk = tegra_clk_register_periph_gate("kfuse", "clk_m",
 				    TEGRA_PERIPH_ON_APB,
@@ -1716,11 +1722,6 @@  static void __init tegra30_fixed_clk_init(void)
 				1, 0, &cml_lock);
 	clk_register_clkdev(clk, "cml1", NULL);
 	clks[cml1] = clk;
-
-	/* pciex */
-	clk = clk_register_fixed_rate(NULL, "pciex", "pll_e", 0, 100000000);
-	clk_register_clkdev(clk, "pciex", NULL);
-	clks[pciex] = clk;
 }
 
 static void __init tegra30_osc_clk_init(void)
@@ -1942,8 +1943,8 @@  static struct tegra_clk_duplicate tegra_clk_duplicates[] = {
 	TEGRA_CLK_DUPLICATE(bsea, "tegra-aes", "bsea"),
 	TEGRA_CLK_DUPLICATE(bsea, "nvavp", "bsea"),
 	TEGRA_CLK_DUPLICATE(cml1, "tegra_sata_cml", NULL),
-	TEGRA_CLK_DUPLICATE(cml0, "tegra_pcie", "cml"),
-	TEGRA_CLK_DUPLICATE(pciex, "tegra_pcie", "pciex"),
+	TEGRA_CLK_DUPLICATE(cml0, "tegra-pcie", "cml"),
+	TEGRA_CLK_DUPLICATE(pciex, "tegra-pcie", "pciex"),
 	TEGRA_CLK_DUPLICATE(vcp, "nvavp", "vcp"),
 	TEGRA_CLK_DUPLICATE(clk_max, NULL, NULL), /* MUST be the last entry */
 };