linux-kernel - RE: [PATCH] imx6 PCI Host initialisation order

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 9 Sep 2014 02:47:03 +0000
From:	"Hong-Xing.Zhu@...escale.com" <Hong-Xing.Zhu@...escale.com>
To:	Bjorn Helgaas <bhelgaas@...gle.com>,
	Shawn Guo <Shawn.Guo@...escale.com>
CC:	Raymond van der Rots <raymond@...ctrocompaniet.no>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Lucas Stach <l.stach@...gutronix.de>,
	Tim Harvey <tharvey@...eworks.com>
Subject: RE: [PATCH] imx6 PCI Host initialisation order


> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@...gle.com]
> Sent: Tuesday, September 09, 2014 12:51 AM
> To: Guo Shawn-R65073
> Cc: Raymond van der Rots; Zhu Richard-R65037; linux-kernel@...r.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@...escale.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@...escale.com> and Shawn Guo
> <shawn.guo@...escale.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@...ctrocompaniet.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