Patchwork [2/2] PCI: imx6: Fix the clock for PCIe

login
register
mail settings
Submitter Marek Vasut
Date Oct. 11, 2013, 2:12 a.m.
Message ID <1381457552-6575-2-git-send-email-marex@denx.de>
Download mbox | patch
Permalink /patch/282512/
State Superseded
Headers show

Comments

Marek Vasut - Oct. 11, 2013, 2:12 a.m.
The clk #205 are lvds_sel, not lvds_gate , so fix this. Moreover,
the PCIe needs lvds_gate for successful operation, so make the PCIe
driver enable (set as output) the lvds_gate .

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 arch/arm/boot/dts/imx6qdl.dtsi |  6 +++---
 drivers/pci/host/pci-imx6.c    | 19 ++++++++++++++++++-
 2 files changed, 21 insertions(+), 4 deletions(-)
Jingoo Han - Oct. 11, 2013, 7:20 a.m.
On Friday, October 11, 2013 11:13 AM, Marek Vasut wrote:
> 
> The clk #205 are lvds_sel, not lvds_gate , so fix this. Moreover,
> the PCIe needs lvds_gate for successful operation, so make the PCIe
> driver enable (set as output) the lvds_gate .
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Yinghai Lu <yinghai@kernel.org>
> ---
>  arch/arm/boot/dts/imx6qdl.dtsi |  6 +++---
>  drivers/pci/host/pci-imx6.c    | 19 ++++++++++++++++++-
>  2 files changed, 21 insertions(+), 4 deletions(-)

Hi Marek Vasut,

How about splitting this patch into two patches such as
arch part and driver part?

It is because driver part was merged into PCI tree by Bjorn Helgaas,[1]
and arch part was merged into i.MX tree by Shawn Guo.[2]
Thus, your patch can make the conflict issue.

For example,
  [PATCH 1/2] ARM: dts: imx6qdl: Fix the clock for PCIe
  [PATCH 2/2] PCI: imx6: Fix the clock for PCIe

Then, 1st patch can be merged to i.MX tree, and 2nd patch can be merged
to PCI tree.

If I am wrong, please let me know kindly. :-)

[1] http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=next&id=bb38919ec56e0758c3ae56dfc091dcde1391353e
[2] https://git.linaro.org/gitweb?p=people/shawnguo/linux-2.6.git;a=commit;h=3a57291fa4ca7f7647d826f5b47082ef306d839f

Best regards,
Jingoo Han

--
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
Marek Vasut - Oct. 11, 2013, 11:55 a.m.
Dear Jingoo Han,

> On Friday, October 11, 2013 11:13 AM, Marek Vasut wrote:
> > The clk #205 are lvds_sel, not lvds_gate , so fix this. Moreover,
> > the PCIe needs lvds_gate for successful operation, so make the PCIe
> > driver enable (set as output) the lvds_gate .
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Frank Li <lznuaa@gmail.com>
> > Cc: Richard Zhu <r65037@freescale.com>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Sean Cross <xobs@kosagi.com>
> > Cc: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Tim Harvey <tharvey@gateworks.com>
> > Cc: Yinghai Lu <yinghai@kernel.org>
> > ---
> > 
> >  arch/arm/boot/dts/imx6qdl.dtsi |  6 +++---
> >  drivers/pci/host/pci-imx6.c    | 19 ++++++++++++++++++-
> >  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> Hi Marek Vasut,
> 
> How about splitting this patch into two patches such as
> arch part and driver part?
> 
> It is because driver part was merged into PCI tree by Bjorn Helgaas,[1]
> and arch part was merged into i.MX tree by Shawn Guo.[2]
> Thus, your patch can make the conflict issue.
> 
> For example,
>   [PATCH 1/2] ARM: dts: imx6qdl: Fix the clock for PCIe
>   [PATCH 2/2] PCI: imx6: Fix the clock for PCIe
> 
> Then, 1st patch can be merged to i.MX tree, and 2nd patch can be merged
> to PCI tree.
> 
> If I am wrong, please let me know kindly. :-)

By all means, you're right. Is the patch addressing the issue correctly (if we 
ignore that it's not split)? Shawn?

Best regards,
Marek Vasut
--
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
Shawn Guo - Oct. 12, 2013, 7:13 a.m.
On Fri, Oct 11, 2013 at 01:55:04PM +0200, Marek Vasut wrote:
> > It is because driver part was merged into PCI tree by Bjorn Helgaas,[1]
> > and arch part was merged into i.MX tree by Shawn Guo.[2]
> > Thus, your patch can make the conflict issue.
> > 
> > For example,
> >   [PATCH 1/2] ARM: dts: imx6qdl: Fix the clock for PCIe
> >   [PATCH 2/2] PCI: imx6: Fix the clock for PCIe
> > 
> > Then, 1st patch can be merged to i.MX tree, and 2nd patch can be merged
> > to PCI tree.
> > 
> > If I am wrong, please let me know kindly. :-)
> 
> By all means, you're right. Is the patch addressing the issue correctly (if we 
> ignore that it's not split)? Shawn?

I need to first understand if you are seeing the issue with the latest
linux-next like next-20131010.  Note, linux-next tree goes to
git://gitorious.org/thierryreding/linux-next [1] these days.

Shawn

[1] https://lkml.org/lkml/2013/9/30/179

--
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

Patch

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 3d8874a..b8601ea 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -127,9 +127,9 @@ 
 				  0x82000000 0 0x01000000 0x01000000 0 0x00f00000>; /* non-prefetchable memory */
 			num-lanes = <1>;
 			interrupts = <0 123 0x04>;
-			clocks = <&clks 189>, <&clks 187>, <&clks 205>, <&clks 144>;
-			clock-names = "pcie_ref_125m", "sata_ref_100m", "lvds_gate", "pcie_axi";
-			status = "disabled";
+			clocks = <&clks 189>, <&clks 187>, <&clks 205>, <&clks 144>, <&clks 207>;
+			clock-names = "pcie_ref_125m", "sata_ref_100m", "lvds_sel", "pcie_axi", "lvds_gate";
+			status = "okay";
 		};
 
 		pmu {
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 8e7adce..32b30ca 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -35,6 +35,7 @@  struct imx6_pcie {
 	int			power_on_gpio;
 	int			wake_up_gpio;
 	int			disable_gpio;
+	struct clk		*lvds_sel;
 	struct clk		*lvds_gate;
 	struct clk		*sata_ref_100m;
 	struct clk		*pcie_ref_125m;
@@ -255,6 +256,12 @@  static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 		goto err_pcie_ref;
 	}
 
+	ret = clk_prepare_enable(imx6_pcie->lvds_sel);
+	if (ret) {
+		dev_err(pp->dev, "unable to enable lvds_sel\n");
+		goto err_lvds_sel;
+	}
+
 	ret = clk_prepare_enable(imx6_pcie->lvds_gate);
 	if (ret) {
 		dev_err(pp->dev, "unable to enable lvds_gate\n");
@@ -273,8 +280,10 @@  static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 	return 0;
 
 err_pcie_axi:
-	clk_disable_unprepare(imx6_pcie->lvds_gate);
+	clk_disable_unprepare(imx6_pcie->lvds_sel);
 err_lvds_gate:
+	clk_disable_unprepare(imx6_pcie->lvds_gate);
+err_lvds_sel:
 	clk_disable_unprepare(imx6_pcie->pcie_ref_125m);
 err_pcie_ref:
 	clk_disable_unprepare(imx6_pcie->sata_ref_100m);
@@ -498,6 +507,14 @@  static int __init imx6_pcie_probe(struct platform_device *pdev)
 	}
 
 	/* Fetch clocks */
+	imx6_pcie->lvds_sel = devm_clk_get(&pdev->dev, "lvds_sel");
+	if (IS_ERR(imx6_pcie->lvds_sel)) {
+		dev_err(&pdev->dev,
+			"lvds_sel clock select missing or invalid\n");
+		ret = PTR_ERR(imx6_pcie->lvds_sel);
+		goto err;
+	}
+
 	imx6_pcie->lvds_gate = devm_clk_get(&pdev->dev, "lvds_gate");
 	if (IS_ERR(imx6_pcie->lvds_gate)) {
 		dev_err(&pdev->dev,