All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
To: Kishon Vijay Abraham I <kishon@ti.com>
Cc: devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org,
	linux-pci@vger.kernel.org, Marek Vasut <marex@denx.de>,
	balajitk@ti.com, Mohit Kumar <mohit.kumar@st.com>,
	Jingoo Han <jg1.han@samsung.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	rogerq@ti.com
Subject: Re: [PATCH 05/17] pci: host: pcie-dra7xx: add support for pcie-dra7xx controller
Date: Tue, 6 May 2014 10:35:08 -0600	[thread overview]
Message-ID: <20140506163507.GA15542@obsidianresearch.com> (raw)
In-Reply-To: <1399383244-14556-6-git-send-email-kishon@ti.com>

On Tue, May 06, 2014 at 07:03:51PM +0530, Kishon Vijay Abraham I wrote:
> +Example:
> +pcie@51000000 {
> +	compatible = "ti,dra7xx-pcie";
> +	reg = <0x51002000 0x14c>, <0x51000000 0x2000>;
> +	reg-names = "ti_conf", "rc_dbics";
> +	interrupts = <0 232 0x4>, <0 233 0x4>;
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	device_type = "pci";
> +	ti,device_type = <3>;
> +	ranges = <0x00000800 0 0x20001000 0x20001000 0 0x00002000  /* Configuration Space */

Configuration space should not show up in the ranges, please don't
copy that mistake from other drivers, put it in reg.

> +	interrupt-map-mask = <0 0 0 0>;
> +	interrupt-map = <0x0 0 &gic 134>;

The HW cannot decode INTA/B/C/D?

> +#define	PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI		0x0034
> +#define	PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI		0x0038
> +#define	INTA						BIT(0)
> +#define	INTB						BIT(1)
> +#define	INTC						BIT(2)
> +#define	INTD						BIT(3)
> +#define	MSI						BIT(4)
> +#define	LEG_EP_INTERRUPTS (INTA | INTB | INTC | INTD)

Oh, it can, it would be wise to export this from the driver. Look at
the latest patches from Srikanth Thokala for the Xilinx PCI driver to
see how this should look

> +static int dra7xx_pcie_establish_link(struct pcie_port *pp)
> +{
> +	u32 reg;
> +	int retries = 1000;
> +	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
> +
> +	if (dw_pcie_link_up(pp)) {
> +		dev_err(pp->dev, "link is already up\n");
> +		return 0;
> +	}
> +
> +	reg = dra7xx_pcie_readl(dra7xx->base, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
> +	reg |= LTSSM_EN;
> +	dra7xx_pcie_writel(dra7xx->base, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
> +
> +	while (--retries) {
> +		reg = dra7xx_pcie_readl(dra7xx->base,
> +					PCIECTRL_DRA7XX_CONF_PHY_CS);
> +		if (reg & LINK_UP)
> +			break;
> +		usleep_range(10, 20);
> +	}
> +
> +	if (retries <= 0) {
> +		dev_err(pp->dev, "link is not up\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}

It would be really nice to see the link bring up process live in the
PCI core, every driver seems to have its own take on this.

The PCI-E spec requires a 100ms delay after link bring up (aka hot
reset) before sending any configuration TLPs.

Jason

WARNING: multiple messages have this Message-ID (diff)
From: jgunthorpe@obsidianresearch.com (Jason Gunthorpe)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 05/17] pci: host: pcie-dra7xx: add support for pcie-dra7xx controller
Date: Tue, 6 May 2014 10:35:08 -0600	[thread overview]
Message-ID: <20140506163507.GA15542@obsidianresearch.com> (raw)
In-Reply-To: <1399383244-14556-6-git-send-email-kishon@ti.com>

On Tue, May 06, 2014 at 07:03:51PM +0530, Kishon Vijay Abraham I wrote:
> +Example:
> +pcie at 51000000 {
> +	compatible = "ti,dra7xx-pcie";
> +	reg = <0x51002000 0x14c>, <0x51000000 0x2000>;
> +	reg-names = "ti_conf", "rc_dbics";
> +	interrupts = <0 232 0x4>, <0 233 0x4>;
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	device_type = "pci";
> +	ti,device_type = <3>;
> +	ranges = <0x00000800 0 0x20001000 0x20001000 0 0x00002000  /* Configuration Space */

Configuration space should not show up in the ranges, please don't
copy that mistake from other drivers, put it in reg.

> +	interrupt-map-mask = <0 0 0 0>;
> +	interrupt-map = <0x0 0 &gic 134>;

The HW cannot decode INTA/B/C/D?

> +#define	PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI		0x0034
> +#define	PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI		0x0038
> +#define	INTA						BIT(0)
> +#define	INTB						BIT(1)
> +#define	INTC						BIT(2)
> +#define	INTD						BIT(3)
> +#define	MSI						BIT(4)
> +#define	LEG_EP_INTERRUPTS (INTA | INTB | INTC | INTD)

Oh, it can, it would be wise to export this from the driver. Look at
the latest patches from Srikanth Thokala for the Xilinx PCI driver to
see how this should look

> +static int dra7xx_pcie_establish_link(struct pcie_port *pp)
> +{
> +	u32 reg;
> +	int retries = 1000;
> +	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
> +
> +	if (dw_pcie_link_up(pp)) {
> +		dev_err(pp->dev, "link is already up\n");
> +		return 0;
> +	}
> +
> +	reg = dra7xx_pcie_readl(dra7xx->base, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
> +	reg |= LTSSM_EN;
> +	dra7xx_pcie_writel(dra7xx->base, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
> +
> +	while (--retries) {
> +		reg = dra7xx_pcie_readl(dra7xx->base,
> +					PCIECTRL_DRA7XX_CONF_PHY_CS);
> +		if (reg & LINK_UP)
> +			break;
> +		usleep_range(10, 20);
> +	}
> +
> +	if (retries <= 0) {
> +		dev_err(pp->dev, "link is not up\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}

It would be really nice to see the link bring up process live in the
PCI core, every driver seems to have its own take on this.

The PCI-E spec requires a 100ms delay after link bring up (aka hot
reset) before sending any configuration TLPs.

Jason

  parent reply	other threads:[~2014-05-06 16:35 UTC|newest]

Thread overview: 195+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-06 13:33 [PATCH 00/17] PCIe support for DRA7xx Kishon Vijay Abraham I
2014-05-06 13:33 ` Kishon Vijay Abraham I
2014-05-06 13:33 ` Kishon Vijay Abraham I
2014-05-06 13:33 ` [PATCH 01/17] phy: phy-omap-pipe3: Add support for PCIe PHY Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-14 12:57   ` Roger Quadros
2014-05-14 12:57     ` Roger Quadros
2014-05-14 12:57     ` Roger Quadros
2014-05-06 13:33 ` [PATCH 02/17] phy: omap-control: add external clock " Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-14 13:02   ` Roger Quadros
2014-05-14 13:02     ` Roger Quadros
2014-05-14 13:02     ` Roger Quadros
2014-05-06 13:33 ` [PATCH 03/17] phy: ti-pipe3: " Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-14 13:16   ` Roger Quadros
2014-05-14 13:16     ` Roger Quadros
2014-05-14 13:16     ` Roger Quadros
2014-05-14 15:19     ` Kishon Vijay Abraham I
2014-05-14 15:19       ` Kishon Vijay Abraham I
2014-05-14 15:19       ` Kishon Vijay Abraham I
2014-05-14 15:34       ` Nishanth Menon
2014-05-14 15:34         ` Nishanth Menon
2014-05-14 15:34         ` Nishanth Menon
2014-05-15  9:15         ` Kishon Vijay Abraham I
2014-05-15  9:15           ` Kishon Vijay Abraham I
2014-05-15  9:15           ` Kishon Vijay Abraham I
2014-05-15  9:15           ` Kishon Vijay Abraham I
2014-05-15  9:25           ` Roger Quadros
2014-05-15  9:25             ` Roger Quadros
2014-05-15  9:25             ` Roger Quadros
2014-05-15  9:25             ` Roger Quadros
2014-05-15 11:46             ` Nishanth Menon
2014-05-15 11:46               ` Nishanth Menon
2014-05-15 11:46               ` Nishanth Menon
2014-05-15 11:59               ` Kishon Vijay Abraham I
2014-05-15 11:59                 ` Kishon Vijay Abraham I
2014-05-15 11:59                 ` Kishon Vijay Abraham I
2014-05-15 11:59                 ` Kishon Vijay Abraham I
2014-05-15 12:12                 ` Nishanth Menon
2014-05-15 12:12                   ` Nishanth Menon
2014-05-15 12:12                   ` Nishanth Menon
2014-05-15 12:18                   ` Kishon Vijay Abraham I
2014-05-15 12:18                     ` Kishon Vijay Abraham I
2014-05-15 12:18                     ` Kishon Vijay Abraham I
2014-05-15 12:18                     ` Kishon Vijay Abraham I
2014-05-15 12:33                     ` Nishanth Menon
2014-05-15 12:33                       ` Nishanth Menon
2014-05-15 12:33                       ` Nishanth Menon
2014-05-15 12:33                       ` Nishanth Menon
2014-05-15 12:42                       ` Kishon Vijay Abraham I
2014-05-15 12:42                         ` Kishon Vijay Abraham I
2014-05-15 12:42                         ` Kishon Vijay Abraham I
2014-05-15 12:42                         ` Kishon Vijay Abraham I
2014-05-27  6:11                       ` Kishon Vijay Abraham I
2014-05-27  6:11                         ` Kishon Vijay Abraham I
2014-05-27  6:11                         ` Kishon Vijay Abraham I
2014-05-27  6:11                         ` Kishon Vijay Abraham I
2014-05-28  1:54                       ` Mike Turquette
2014-05-28  1:54                         ` Mike Turquette
2014-05-28  1:54                         ` Mike Turquette
2014-05-28  1:54                         ` Mike Turquette
2014-05-28 15:52                         ` Nishanth Menon
2014-05-28 15:52                           ` Nishanth Menon
2014-05-28 15:52                           ` Nishanth Menon
2014-05-06 13:33 ` [PATCH 04/17] phy: pipe3: insert delay to enumerate in GEN2 mode Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-14 13:20   ` Roger Quadros
2014-05-14 13:20     ` Roger Quadros
2014-05-14 13:20     ` Roger Quadros
2014-05-06 13:33 ` [PATCH 05/17] pci: host: pcie-dra7xx: add support for pcie-dra7xx controller Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:44   ` Marek Vasut
2014-05-06 13:44     ` Marek Vasut
2014-05-07  8:21     ` Kishon Vijay Abraham I
2014-05-07  8:21       ` Kishon Vijay Abraham I
2014-05-07  8:21       ` Kishon Vijay Abraham I
2014-05-09  9:43     ` Pavel Machek
2014-05-09  9:43       ` Pavel Machek
2014-05-09  9:43       ` Pavel Machek
2014-05-06 13:54   ` Arnd Bergmann
2014-05-06 13:54     ` Arnd Bergmann
2014-05-06 13:54     ` Arnd Bergmann
2014-05-07  8:44     ` Kishon Vijay Abraham I
2014-05-07  8:44       ` Kishon Vijay Abraham I
2014-05-07  8:44       ` Kishon Vijay Abraham I
2014-05-07  9:30       ` Arnd Bergmann
2014-05-07  9:30         ` Arnd Bergmann
2014-05-09 11:29         ` Kishon Vijay Abraham I
2014-05-09 11:29           ` Kishon Vijay Abraham I
2014-05-09 11:29           ` Kishon Vijay Abraham I
2014-05-06 16:35   ` Jason Gunthorpe [this message]
2014-05-06 16:35     ` Jason Gunthorpe
2014-05-07  9:22     ` Kishon Vijay Abraham I
2014-05-07  9:22       ` Kishon Vijay Abraham I
2014-05-07  9:22       ` Kishon Vijay Abraham I
2014-05-07  9:25       ` Arnd Bergmann
2014-05-07  9:25         ` Arnd Bergmann
2014-05-08  8:56         ` Jingoo Han
2014-05-08  8:56           ` Jingoo Han
2014-05-08  9:16           ` Arnd Bergmann
2014-05-08  9:16             ` Arnd Bergmann
2014-05-06 13:33 ` [PATCH 06/17] pci: host: pcie-designware: Use *base-mask* for configuring the iATU Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:59   ` Arnd Bergmann
2014-05-06 13:59     ` Arnd Bergmann
2014-05-08  9:05     ` Jingoo Han
2014-05-08  9:05       ` Jingoo Han
2014-05-08  9:05       ` Jingoo Han
2014-05-08  9:18       ` Arnd Bergmann
2014-05-08  9:18         ` Arnd Bergmann
2014-05-09 11:50         ` Kishon Vijay Abraham I
2014-05-09 11:50           ` Kishon Vijay Abraham I
2014-05-09 11:50           ` Kishon Vijay Abraham I
2014-05-12  1:44           ` Jingoo Han
2014-05-12  1:44             ` Jingoo Han
2014-05-13 12:31         ` Kishon Vijay Abraham I
2014-05-13 12:31           ` Kishon Vijay Abraham I
2014-05-13 12:31           ` Kishon Vijay Abraham I
2014-05-13 12:47           ` Arnd Bergmann
2014-05-13 12:47             ` Arnd Bergmann
2014-05-13 12:47             ` Arnd Bergmann
2014-05-13 13:26             ` Kishon Vijay Abraham I
2014-05-13 13:26               ` Kishon Vijay Abraham I
2014-05-13 13:26               ` Kishon Vijay Abraham I
2014-05-13 13:27               ` Arnd Bergmann
2014-05-13 13:27                 ` Arnd Bergmann
2014-05-13 13:27                 ` Arnd Bergmann
2014-05-13 13:34                 ` Arnd Bergmann
2014-05-13 13:34                   ` Arnd Bergmann
2014-05-13 13:34                   ` Arnd Bergmann
2014-05-14  5:44                   ` Kishon Vijay Abraham I
2014-05-14  5:44                     ` Kishon Vijay Abraham I
2014-05-14  5:44                     ` Kishon Vijay Abraham I
2014-05-14 12:45                     ` Arnd Bergmann
2014-05-14 12:45                       ` Arnd Bergmann
2014-05-14 15:04                       ` Kishon Vijay Abraham I
2014-05-14 15:04                         ` Kishon Vijay Abraham I
2014-05-14 15:04                         ` Kishon Vijay Abraham I
2014-05-16  9:00                       ` Kishon Vijay Abraham I
2014-05-16  9:00                         ` Kishon Vijay Abraham I
2014-05-16  9:00                         ` Kishon Vijay Abraham I
2014-05-19 12:45                         ` Arnd Bergmann
2014-05-19 12:45                           ` Arnd Bergmann
2014-05-06 13:33 ` [PATCH 07/17] ARM: dts: DRA7: Add divider table to optfclk_pciephy_div clock Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33 ` [PATCH 08/17] ARM: dts: DRA7: Change the parent of apll_pcie_in_clk_mux to dpll_pcie_ref_m2ldo_ck Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33 ` [PATCH 09/17] arm: dra7xx: Add hwmod data for pcie1 phy and pcie2 phy Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33 ` [PATCH 10/17] arm: dra7xx: Add hwmod data for pcie1 and pcie2 subsystems Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33 ` [PATCH 11/17] ARM: dts: dra7xx-clocks: Add missing 32khz clocks used for PHY Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-14 13:23   ` Roger Quadros
2014-05-14 13:23     ` Roger Quadros
2014-05-14 13:23     ` Roger Quadros
2014-05-14 15:19     ` Kishon Vijay Abraham I
2014-05-14 15:19       ` Kishon Vijay Abraham I
2014-05-14 15:19       ` Kishon Vijay Abraham I
2014-05-06 13:33 ` [PATCH 12/17] ARM: dts: dra7: Add dt data for PCIe PHY control module Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33 ` [PATCH 13/17] ARM: dts: dra7: Add dt data for PCIe PHY Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:33   ` Kishon Vijay Abraham I
2014-05-06 13:34 ` [PATCH 14/17] ARM: dts: dra7: Add dt data for PCIe controller Kishon Vijay Abraham I
2014-05-06 13:34   ` Kishon Vijay Abraham I
2014-05-06 13:34   ` Kishon Vijay Abraham I
2014-05-06 13:34 ` [PATCH 15/17] ARM: OMAP: Enable PCI for DRA7 Kishon Vijay Abraham I
2014-05-06 13:34   ` Kishon Vijay Abraham I
2014-05-06 13:34   ` Kishon Vijay Abraham I
2014-05-06 13:34 ` [TEMP PATCH 16/17] pci: host: pcie-dra7xx: use reset framework APIs to reset PCIe Kishon Vijay Abraham I
2014-05-06 13:34   ` Kishon Vijay Abraham I
2014-05-06 13:34   ` Kishon Vijay Abraham I
2014-05-06 13:41   ` Dan Murphy
2014-05-06 13:41     ` Dan Murphy
2014-05-06 13:41     ` Dan Murphy
2014-05-06 13:34 ` [TEMP PATCH 17/17] ARM: dts: dra7: Add *resets* property for PCIe dt node Kishon Vijay Abraham I
2014-05-06 13:34   ` Kishon Vijay Abraham I
2014-05-06 13:34   ` Kishon Vijay Abraham I
2014-05-06 13:40   ` Dan Murphy
2014-05-06 13:40     ` Dan Murphy
2014-05-06 13:40     ` Dan Murphy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140506163507.GA15542@obsidianresearch.com \
    --to=jgunthorpe@obsidianresearch.com \
    --cc=balajitk@ti.com \
    --cc=bhelgaas@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jg1.han@samsung.com \
    --cc=kishon@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=marex@denx.de \
    --cc=mohit.kumar@st.com \
    --cc=rogerq@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.