RE: [PATCH] imx6 PCI Host initialisation order

["Hong-Xing.Zhu@freescale.com" <Hong-Xing.Zhu@freescale.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3631 bytes --]


> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Tuesday, September 09, 2014 12:51 AM
> To: Guo Shawn-R65073
> Cc: Raymond van der Rots; Zhu Richard-R65037; linux-kernel@vger.kernel.org;
> Lucas Stach; Tim Harvey
> Subject: Re: [PATCH] imx6 PCI Host initialisation order
> 
> On Mon, Sep 8, 2014 at 5:28 AM, Shawn Guo <shawn.guo@freescale.com> wrote:
> > Hi Raymond,
> >
> > It seems that there is already a similar patch [1] from Tim floating
> > on the list.
> >
> > Shawn
> >
> > [1] http://www.spinics.net/lists/linux-pci/msg33520.html
> 
> Yes.  MAINTAINERS lists Richard Zhu <r65037@freescale.com> and Shawn Guo
> <shawn.guo@freescale.com> (hi Shawn :)) as the caretakers for pci-imx6.c.
> Neither has acked it yet, and Richard asked for a comment update, which I
> haven't seen yet.  So I haven't applied it yet.
> 
[Richard] Yes it is. I recommended to add one comment used to describe why
the modification is required.

> > On Mon, Sep 08, 2014 at 11:10:37AM +0200, Raymond van der Rots wrote:
> >> Hi,
> >>
> >> The imx6dl on our hardware board frequently had problems bringing up the
> PCI link, with or without peripherals connected. I found these issues to be
> due to the initialisation order of the PCI Host.
> >>
> >> The host driver first enables the phy, and then enables its clocks. However,
> according to the reference manual (IMX6SDLRM, page 2033):
> >> > The phy_ref_ssp_en signal must remain deasserted until the reference
> clock is running at the appropriate frequency, at which point phy_ref_ssp_en
> can be asserted.
> >>
> >>
> >> Which implies that the clocks should be brought up first, after which the
> peripheral should be enabled.
> >> This patch changes that initialisation order.
> >>
> >> I do not have other hardware with an imx6dl, so this patch has only been
> tested on our board. Could someone confirm that this is more technically
> correct or improves behaviour?
> >>
> >> Signed-off-by: Raymond van der Rots <raymond@electrocompaniet.no>
> >> ---
> >> diff --git a/drivers/pci/host/pci-imx6.c
> >> b/drivers/pci/host/pci-imx6.c index a568efa..17c35b4 100644
> >> --- a/drivers/pci/host/pci-imx6.c
> >> +++ b/drivers/pci/host/pci-imx6.c
> >> @@ -228,11 +228,6 @@ static int imx6_pcie_deassert_core_reset(struct
> pcie_port *pp)
> >>       struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
> >>       int ret;
> >>
> >> -     regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> >> -                     IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
> >> -     regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> >> -                     IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
> >> -
> >>       ret = clk_prepare_enable(imx6_pcie->pcie_phy);
> >>       if (ret) {
> >>               dev_err(pp->dev, "unable to enable pcie_phy clock\n");
> >> @@ -254,6 +249,11 @@ static int imx6_pcie_deassert_core_reset(struct
> pcie_port *pp)
> >>       /* allow the clocks to stabilize */
> >>       usleep_range(200, 500);
> >>
> >> +     regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> >> +                     IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
> >> +     regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> >> +                     IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
> >> +
> >>       /* Some boards don't have PCIe reset GPIO. */
> >>       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> >>               gpio_set_value(imx6_pcie->reset_gpio, 0);
> >>
> >>

Best Regards
Richard Zhu

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥