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

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

[Jay Agarwal]

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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
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(-)

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 */
 };
-- 
1.7.0.4

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

[Jay Agarwal]

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

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 */
 };
-- 
1.7.0.4

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

[Jay Agarwal]

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

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 */
 };
-- 
1.7.0.4

[PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

- Enable PCIe root port 2 for Cardhu
- Make private data structure for each SoC
- Add required Tegra30 clocks and regulators
- Add Tegra30 specific code in enable controller
- Corrected logic in read/write config space to
  display right device number on bus 0
- Fixed unnecessary freeing of tegra_pcie_bus structure in tegra_pcie_remove
- Added Tegra30 specific properties in pci binding document

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:
- Added entry in required properties in pci document as per review comment
- Corrected logic in read/write config space to display right device number on bus 0
- Fixed unnecessary freeing of tegra_pcie_bus structure in tegra_pcie_remove
- Took care of other review comments to change data types and names of some members of tegra_pcie_soc_data structure

 .../bindings/pci/nvidia,tegra20-pcie.txt           |    2 +
 drivers/pci/host/pci-tegra.c                       |  157 ++++++++++++++++----
 2 files changed, 133 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
index 90c112f..4ac22ab 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
@@ -16,6 +16,7 @@ Required properties:
   "msi": The Tegra interrupt that is asserted when an MSI is received
 - pex-clk-supply: Supply voltage for internal reference clock
 - vdd-supply: Power supply for controller (1.05V)
+- avdd-supply: Power supply for controller (1.05V)
 - bus-range: Range of bus numbers associated with this controller
 - #address-cells: Address representation for root ports (must be 3)
   - cell 0 specifies the bus and device numbers of the root port:
@@ -48,6 +49,7 @@ Required properties:
   "afi": The Tegra clock of that name
   "pcie_xclk": The Tegra clock of that name
   "pll_e": The Tegra clock of that name
+  "cml": The Tegra clock of that name
 
 Root ports are defined as subnodes of the PCIe controller node.
 
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index c2d0b22..27c39d8 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -1,5 +1,5 @@
 /*
- * PCIe host controller driver for TEGRA(2) SOCs
+ * PCIe host controller driver for TEGRA SOCs
  *
  * Copyright (c) 2010, CompuLab, Ltd.
  * Author: Mike Rapoport <mike@compulab.co.il>
@@ -50,7 +50,6 @@
 #include <asm/mach/pci.h>
 
 #define INT_PCI_MSI_NR (8 * 32)
-#define TEGRA_MAX_PORTS 2
 
 /* register definitions */
 
@@ -142,14 +141,15 @@
 #define  AFI_INTR_EN_DFPCI_DECERR	(1 << 5)
 #define  AFI_INTR_EN_AXI_DECERR		(1 << 6)
 #define  AFI_INTR_EN_FPCI_TIMEOUT	(1 << 7)
+#define AFI_INTR_EN_PRSNT_SENSE		(1 << 8)
 
 #define AFI_PCIE_CONFIG					0x0f8
 #define  AFI_PCIE_CONFIG_PCIE_DISABLE(x)		(1 << ((x) + 1))
 #define  AFI_PCIE_CONFIG_PCIE_DISABLE_ALL		0xe
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_MASK	(0xf << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_SINGLE	(0x0 << 20)
-#define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_420	(0x0 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_DUAL	(0x1 << 20)
+#define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_420	(0x0 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_222	(0x1 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_411	(0x2 << 20)
 
@@ -160,8 +160,11 @@
 #define AFI_PEX1_CTRL			0x118
 #define AFI_PEX2_CTRL			0x128
 #define  AFI_PEX_CTRL_RST		(1 << 0)
+#define  AFI_PEX_CTRL_CLKREQ_EN		(1 << 1)
 #define  AFI_PEX_CTRL_REFCLK_EN		(1 << 3)
 
+#define  AFI_PEXBIAS_CTRL_0		0x168
+
 #define RP_VEND_XP	0x00000F00
 #define  RP_VEND_XP_DL_UP	(1 << 30)
 
@@ -175,7 +178,8 @@
 #define  PADS_CTL_TX_DATA_EN_1L	(1 << 6)
 #define  PADS_CTL_RX_DATA_EN_1L	(1 << 10)
 
-#define PADS_PLL_CTL				0x000000B8
+#define  PADS_PLL_CTL_T20			0x000000B8
+#define  PADS_PLL_CTL_T30			0x000000B4
 #define  PADS_PLL_CTL_RST_B4SM			(1 << 1)
 #define  PADS_PLL_CTL_LOCKDET			(1 << 8)
 #define  PADS_PLL_CTL_REFCLK_MASK		(0x3 << 16)
@@ -183,8 +187,11 @@
 #define  PADS_PLL_CTL_REFCLK_INTERNAL_CMOS	(1 << 16)
 #define  PADS_PLL_CTL_REFCLK_EXTERNAL		(2 << 16)
 #define  PADS_PLL_CTL_TXCLKREF_MASK		(0x1 << 20)
+#define  PADS_PLL_CTL_TXCLKREF_BUF_EN		(1 << 22)
 #define  PADS_PLL_CTL_TXCLKREF_DIV10		(0 << 20)
 #define  PADS_PLL_CTL_TXCLKREF_DIV5		(1 << 20)
+#define  PADS_REFCLK_CFG0			0x000000C8
+#define  PADS_REFCLK_CFG1			0x000000CC
 
 struct tegra_msi {
 	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
@@ -195,6 +202,19 @@ struct tegra_msi {
 	int irq;
 };
 
+/* used to differentiate tegra chips code */
+struct tegra_pcie_soc_data {
+	unsigned int num_max_ports;
+	unsigned int msi_base_shift;
+	u32 pads_pll_ctl;
+	u32 tx_ref_sel;
+	bool has_pex_clkreq_en;
+	bool has_pex_bias_ctrl;
+	bool has_intr_prsnt_sense;
+	bool has_avdd_supply;
+	bool has_cml_clk;
+};
+
 static inline struct tegra_msi *to_tegra_msi(struct msi_chip *chip)
 {
 	return container_of(chip, struct tegra_msi, chip);
@@ -219,6 +239,7 @@ struct tegra_pcie {
 	struct clk *afi_clk;
 	struct clk *pcie_xclk;
 	struct clk *pll_e;
+	struct clk *cml_clk;
 
 	struct tegra_msi msi;
 
@@ -228,6 +249,9 @@ struct tegra_pcie {
 
 	struct regulator *pex_clk_supply;
 	struct regulator *vdd_supply;
+	struct regulator *avdd_supply;
+
+	struct tegra_pcie_soc_data *soc_data;
 };
 
 struct tegra_pcie_port {
@@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
 		struct tegra_pcie_port *port;
 
 		list_for_each_entry(port, &pcie->ports, list) {
-			if (port->index + 1 == slot) {
+			if (port->index == slot) {
 				addr = port->base + (where & ~3);
 				break;
 			}
@@ -398,7 +422,6 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
 			*value = 0xffffffff;
 			return PCIBIOS_DEVICE_NOT_FOUND;
 		}
-
 		addr += tegra_pcie_conf_offset(devfn, where);
 	}
 
@@ -429,7 +452,7 @@ static int tegra_pcie_write_conf(struct pci_bus *bus, unsigned int devfn,
 		struct tegra_pcie_port *port;
 
 		list_for_each_entry(port, &pcie->ports, list) {
-			if (port->index + 1 == slot) {
+			if (port->index == slot) {
 				addr = port->base + (where & ~3);
 				break;
 			}
@@ -510,12 +533,15 @@ static void tegra_pcie_port_reset(struct tegra_pcie_port *port)
 
 static void tegra_pcie_port_enable(struct tegra_pcie_port *port)
 {
+	struct tegra_pcie_soc_data *soc = port->pcie->soc_data;
 	unsigned long ctrl = tegra_pcie_port_get_pex_ctrl(port);
 	unsigned long value;
 
 	/* enable reference clock */
 	value = afi_readl(port->pcie, ctrl);
 	value |= AFI_PEX_CTRL_REFCLK_EN;
+	if (soc->has_pex_clkreq_en)
+		value |= AFI_PEX_CTRL_CLKREQ_EN;
 	afi_writel(port->pcie, value, ctrl);
 
 	tegra_pcie_port_reset(port);
@@ -568,6 +594,8 @@ static void tegra_pcie_fixup_class(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf0, tegra_pcie_fixup_class);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class);
 
 /* Tegra PCIE requires relaxed ordering */
 static void tegra_pcie_relax_enable(struct pci_dev *dev)
@@ -748,6 +776,11 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	struct tegra_pcie_port *port;
 	unsigned int timeout;
 	unsigned long value;
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
+
+	/* power down to PCIe slot clock bias pad */
+	if (soc->has_pex_bias_ctrl)
+		afi_writel(pcie, 0, AFI_PEXBIAS_CTRL_0);
 
 	/* configure mode and disable all ports */
 	value = afi_readl(pcie, AFI_PCIE_CONFIG);
@@ -775,26 +808,26 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	 * Set up PHY PLL inputs select PLLE output as refclock,
 	 * set TX ref sel to div10 (not div5).
 	 */
-	value = pads_readl(pcie, PADS_PLL_CTL);
+	value = pads_readl(pcie, soc->pads_pll_ctl);
 	value &= ~(PADS_PLL_CTL_REFCLK_MASK | PADS_PLL_CTL_TXCLKREF_MASK);
-	value |= (PADS_PLL_CTL_REFCLK_INTERNAL_CML | PADS_PLL_CTL_TXCLKREF_DIV10);
-	pads_writel(pcie, value, PADS_PLL_CTL);
+	value |= PADS_PLL_CTL_REFCLK_INTERNAL_CML | soc->tx_ref_sel;
+	pads_writel(pcie, value, soc->pads_pll_ctl);
 
 	/* take PLL out of reset  */
-	value = pads_readl(pcie, PADS_PLL_CTL);
+	value = pads_readl(pcie, soc->pads_pll_ctl);
 	value |= PADS_PLL_CTL_RST_B4SM;
-	pads_writel(pcie, value, PADS_PLL_CTL);
+	pads_writel(pcie, value, soc->pads_pll_ctl);
 
 	/*
 	 * Hack, set the clock voltage to the DEFAULT provided by hw folks.
 	 * This doesn't exist in the documentation.
 	 */
-	pads_writel(pcie, 0xfa5cfa5c, 0xc8);
+	pads_writel(pcie, 0xfa5cfa5c, PADS_REFCLK_CFG0);
 
 	/* wait for the PLL to lock */
 	timeout = 300;
 	do {
-		value = pads_readl(pcie, PADS_PLL_CTL);
+		value = pads_readl(pcie, soc->pads_pll_ctl);
 		usleep_range(1000, 1000);
 		if (--timeout == 0) {
 			pr_err("Tegra PCIe error: timeout waiting for PLL\n");
@@ -823,6 +856,8 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	value = AFI_INTR_EN_INI_SLVERR | AFI_INTR_EN_INI_DECERR |
 		AFI_INTR_EN_TGT_SLVERR | AFI_INTR_EN_TGT_DECERR |
 		AFI_INTR_EN_TGT_WRERR | AFI_INTR_EN_DFPCI_DECERR;
+	if (soc->has_intr_prsnt_sense)
+		value |= AFI_INTR_EN_PRSNT_SENSE;
 	afi_writel(pcie, value, AFI_AFI_INTR_ENABLE);
 	afi_writel(pcie, 0xffffffff, AFI_SM_INTR_ENABLE);
 
@@ -837,6 +872,7 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 
 static void tegra_pcie_power_off(struct tegra_pcie *pcie)
 {
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	int err;
 
 	/* TODO: disable and unprepare clocks? */
@@ -848,19 +884,26 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie)
 	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
 	tegra_pmc_pcie_xclk_clamp(true);
 
+	if (soc->has_avdd_supply) {
+		err = regulator_disable(pcie->avdd_supply);
+		if (err < 0)
+			dev_warn(pcie->dev, "failed to disable AVDD regulator: %d\n",
+			err);
+	}
 	err = regulator_disable(pcie->pex_clk_supply);
 	if (err < 0)
-		dev_err(pcie->dev, "failed to disable pex-clk regulator: %d\n",
+		dev_warn(pcie->dev, "failed to disable pex-clk regulator: %d\n",
 			err);
 
 	err = regulator_disable(pcie->vdd_supply);
 	if (err < 0)
-		dev_err(pcie->dev, "failed to disable VDD regulator: %d\n",
+		dev_warn(pcie->dev, "failed to disable VDD regulator: %d\n",
 			err);
 }
 
 static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 {
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	int err;
 
 	tegra_periph_reset_assert(pcie->pcie_xclk);
@@ -884,6 +927,15 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 		return err;
 	}
 
+	if (soc->has_avdd_supply) {
+		err = regulator_enable(pcie->avdd_supply);
+		if (err < 0) {
+			dev_err(pcie->dev, "failed to enable AVDD regulator: %d\n",
+				err);
+			return err;
+		}
+	}
+
 	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_PCIE,
 						pcie->pex_clk);
 	if (err) {
@@ -901,6 +953,15 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 		return err;
 	}
 
+	if (soc->has_cml_clk) {
+		err = clk_prepare_enable(pcie->cml_clk);
+		if (err < 0) {
+			dev_err(pcie->dev, "failed to enable cml clock: %d\n",
+				err);
+			return err;
+		}
+	}
+
 	err = clk_prepare_enable(pcie->pll_e);
 	if (err < 0) {
 		dev_err(pcie->dev, "failed to enable PLLE clock: %d\n", err);
@@ -912,6 +973,8 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 
 static int tegra_pcie_clocks_get(struct tegra_pcie *pcie)
 {
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
+
 	pcie->pex_clk = devm_clk_get(pcie->dev, "pex");
 	if (IS_ERR(pcie->pex_clk))
 		return PTR_ERR(pcie->pex_clk);
@@ -928,6 +991,11 @@ static int tegra_pcie_clocks_get(struct tegra_pcie *pcie)
 	if (IS_ERR(pcie->pll_e))
 		return PTR_ERR(pcie->pll_e);
 
+	if (soc->has_cml_clk) {
+		pcie->cml_clk = devm_clk_get(pcie->dev, "cml");
+		if (IS_ERR(pcie->cml_clk))
+			return PTR_ERR(pcie->cml_clk);
+	}
 	return 0;
 }
 
@@ -1150,6 +1218,7 @@ static int tegra_pcie_enable_msi(struct tegra_pcie *pcie)
 {
 	struct platform_device *pdev = to_platform_device(pcie->dev);
 	struct tegra_msi *msi = &pcie->msi;
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	unsigned long base;
 	int err;
 	u32 reg;
@@ -1186,7 +1255,7 @@ static int tegra_pcie_enable_msi(struct tegra_pcie *pcie)
 	msi->pages = __get_free_pages(GFP_KERNEL, 3);
 	base = virt_to_phys((void *)msi->pages);
 
-	afi_writel(pcie, base, AFI_MSI_FPCI_BAR_ST);
+	afi_writel(pcie, base >> soc->msi_base_shift, AFI_MSI_FPCI_BAR_ST);
 	afi_writel(pcie, base, AFI_MSI_AXI_BAR_ST);
 	/* this register is in 4K increments */
 	afi_writel(pcie, 1, AFI_MSI_BAR_SZ);
@@ -1283,6 +1352,7 @@ static u32 tegra_pcie_get_xbar_config(struct tegra_pcie *pcie, u32 lanes)
 static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 {
 	struct device_node *np = pcie->dev->of_node, *port;
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	struct of_pci_range_parser parser;
 	struct of_pci_range range;
 	struct resource res;
@@ -1302,6 +1372,12 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 	if (IS_ERR(pcie->pex_clk_supply))
 		return PTR_ERR(pcie->pex_clk_supply);
 
+	if (soc->has_avdd_supply) {
+		pcie->avdd_supply = devm_regulator_get(pcie->dev, "avdd");
+		if (IS_ERR(pcie->avdd_supply))
+			return PTR_ERR(pcie->avdd_supply);
+	}
+
 	for_each_of_pci_range(&parser, &range) {
 		of_pci_range_to_resource(&range, np, &res);
 
@@ -1348,7 +1424,7 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 
 		index = PCI_SLOT(err);
 
-		if (index < 1 || index > TEGRA_MAX_PORTS) {
+		if (index < 1 || index > pcie->soc_data->num_max_ports) {
 			dev_err(pcie->dev, "invalid port number: %d\n", index);
 			return -EINVAL;
 		}
@@ -1484,8 +1560,39 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
 	return 0;
 }
 
+static const struct tegra_pcie_soc_data tegra20_pcie_data = {
+	.num_max_ports = 2,
+	.msi_base_shift = 0,
+	.pads_pll_ctl = PADS_PLL_CTL_T20,
+	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
+	.has_pex_clkreq_en = false,
+	.has_pex_bias_ctrl = false,
+	.has_intr_prsnt_sense = false,
+	.has_avdd_supply = false,
+	.has_cml_clk = false,
+};
+
+static const struct tegra_pcie_soc_data tegra30_pcie_data = {
+	.num_max_ports = 3,
+	.msi_base_shift = 8,
+	.pads_pll_ctl = PADS_PLL_CTL_T30,
+	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
+	.has_pex_clkreq_en = true,
+	.has_pex_bias_ctrl = true,
+	.has_intr_prsnt_sense = true,
+	.has_avdd_supply = true,
+	.has_cml_clk = true,
+};
+
+static const struct of_device_id tegra_pcie_of_match[] = {
+	{ .compatible = "nvidia,tegra30-pcie", .data = &tegra30_pcie_data},
+	{ .compatible = "nvidia,tegra20-pcie", .data = &tegra20_pcie_data},
+	{ },
+};
+
 static int tegra_pcie_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *match;
 	struct tegra_pcie *pcie;
 	int err;
 
@@ -1496,6 +1603,10 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&pcie->busses);
 	INIT_LIST_HEAD(&pcie->ports);
 	pcie->dev = &pdev->dev;
+	match = of_match_device(tegra_pcie_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+	pcie->soc_data = (struct tegra_pcie_soc_data *)match->data;
 
 	err = tegra_pcie_parse_dt(pcie);
 	if (err < 0)
@@ -1549,10 +1660,9 @@ static int tegra_pcie_remove(struct platform_device *pdev)
 	struct tegra_pcie_bus *bus;
 	int err;
 
-	list_for_each_entry(bus, &pcie->busses, list) {
+	list_for_each_entry(bus, &pcie->busses, list)
 		vunmap(bus->area->addr);
-		kfree(bus);
-	}
+	kfree(bus);
 
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		err = tegra_pcie_disable_msi(pcie);
@@ -1567,11 +1677,6 @@ static int tegra_pcie_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id tegra_pcie_of_match[] = {
-	{ .compatible = "nvidia,tegra20-pcie", },
-	{ },
-};
-
 static struct platform_driver tegra_pcie_driver = {
 	.driver = {
 		.name = "tegra-pcie",
-- 
1.7.0.4

[PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

- Enable PCIe root port 2 for Cardhu
- Make private data structure for each SoC
- Add required Tegra30 clocks and regulators
- Add Tegra30 specific code in enable controller
- Corrected logic in read/write config space to
  display right device number on bus 0
- Fixed unnecessary freeing of tegra_pcie_bus structure in tegra_pcie_remove
- Added Tegra30 specific properties in pci binding document

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:
- Added entry in required properties in pci document as per review comment
- Corrected logic in read/write config space to display right device number on bus 0
- Fixed unnecessary freeing of tegra_pcie_bus structure in tegra_pcie_remove
- Took care of other review comments to change data types and names of some members of tegra_pcie_soc_data structure

 .../bindings/pci/nvidia,tegra20-pcie.txt           |    2 +
 drivers/pci/host/pci-tegra.c                       |  157 ++++++++++++++++----
 2 files changed, 133 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
index 90c112f..4ac22ab 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
@@ -16,6 +16,7 @@ Required properties:
   "msi": The Tegra interrupt that is asserted when an MSI is received
 - pex-clk-supply: Supply voltage for internal reference clock
 - vdd-supply: Power supply for controller (1.05V)
+- avdd-supply: Power supply for controller (1.05V)
 - bus-range: Range of bus numbers associated with this controller
 - #address-cells: Address representation for root ports (must be 3)
   - cell 0 specifies the bus and device numbers of the root port:
@@ -48,6 +49,7 @@ Required properties:
   "afi": The Tegra clock of that name
   "pcie_xclk": The Tegra clock of that name
   "pll_e": The Tegra clock of that name
+  "cml": The Tegra clock of that name
 
 Root ports are defined as subnodes of the PCIe controller node.
 
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index c2d0b22..27c39d8 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -1,5 +1,5 @@
 /*
- * PCIe host controller driver for TEGRA(2) SOCs
+ * PCIe host controller driver for TEGRA SOCs
  *
  * Copyright (c) 2010, CompuLab, Ltd.
  * Author: Mike Rapoport <mike@compulab.co.il>
@@ -50,7 +50,6 @@
 #include <asm/mach/pci.h>
 
 #define INT_PCI_MSI_NR (8 * 32)
-#define TEGRA_MAX_PORTS 2
 
 /* register definitions */
 
@@ -142,14 +141,15 @@
 #define  AFI_INTR_EN_DFPCI_DECERR	(1 << 5)
 #define  AFI_INTR_EN_AXI_DECERR		(1 << 6)
 #define  AFI_INTR_EN_FPCI_TIMEOUT	(1 << 7)
+#define AFI_INTR_EN_PRSNT_SENSE		(1 << 8)
 
 #define AFI_PCIE_CONFIG					0x0f8
 #define  AFI_PCIE_CONFIG_PCIE_DISABLE(x)		(1 << ((x) + 1))
 #define  AFI_PCIE_CONFIG_PCIE_DISABLE_ALL		0xe
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_MASK	(0xf << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_SINGLE	(0x0 << 20)
-#define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_420	(0x0 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_DUAL	(0x1 << 20)
+#define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_420	(0x0 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_222	(0x1 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_411	(0x2 << 20)
 
@@ -160,8 +160,11 @@
 #define AFI_PEX1_CTRL			0x118
 #define AFI_PEX2_CTRL			0x128
 #define  AFI_PEX_CTRL_RST		(1 << 0)
+#define  AFI_PEX_CTRL_CLKREQ_EN		(1 << 1)
 #define  AFI_PEX_CTRL_REFCLK_EN		(1 << 3)
 
+#define  AFI_PEXBIAS_CTRL_0		0x168
+
 #define RP_VEND_XP	0x00000F00
 #define  RP_VEND_XP_DL_UP	(1 << 30)
 
@@ -175,7 +178,8 @@
 #define  PADS_CTL_TX_DATA_EN_1L	(1 << 6)
 #define  PADS_CTL_RX_DATA_EN_1L	(1 << 10)
 
-#define PADS_PLL_CTL				0x000000B8
+#define  PADS_PLL_CTL_T20			0x000000B8
+#define  PADS_PLL_CTL_T30			0x000000B4
 #define  PADS_PLL_CTL_RST_B4SM			(1 << 1)
 #define  PADS_PLL_CTL_LOCKDET			(1 << 8)
 #define  PADS_PLL_CTL_REFCLK_MASK		(0x3 << 16)
@@ -183,8 +187,11 @@
 #define  PADS_PLL_CTL_REFCLK_INTERNAL_CMOS	(1 << 16)
 #define  PADS_PLL_CTL_REFCLK_EXTERNAL		(2 << 16)
 #define  PADS_PLL_CTL_TXCLKREF_MASK		(0x1 << 20)
+#define  PADS_PLL_CTL_TXCLKREF_BUF_EN		(1 << 22)
 #define  PADS_PLL_CTL_TXCLKREF_DIV10		(0 << 20)
 #define  PADS_PLL_CTL_TXCLKREF_DIV5		(1 << 20)
+#define  PADS_REFCLK_CFG0			0x000000C8
+#define  PADS_REFCLK_CFG1			0x000000CC
 
 struct tegra_msi {
 	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
@@ -195,6 +202,19 @@ struct tegra_msi {
 	int irq;
 };
 
+/* used to differentiate tegra chips code */
+struct tegra_pcie_soc_data {
+	unsigned int num_max_ports;
+	unsigned int msi_base_shift;
+	u32 pads_pll_ctl;
+	u32 tx_ref_sel;
+	bool has_pex_clkreq_en;
+	bool has_pex_bias_ctrl;
+	bool has_intr_prsnt_sense;
+	bool has_avdd_supply;
+	bool has_cml_clk;
+};
+
 static inline struct tegra_msi *to_tegra_msi(struct msi_chip *chip)
 {
 	return container_of(chip, struct tegra_msi, chip);
@@ -219,6 +239,7 @@ struct tegra_pcie {
 	struct clk *afi_clk;
 	struct clk *pcie_xclk;
 	struct clk *pll_e;
+	struct clk *cml_clk;
 
 	struct tegra_msi msi;
 
@@ -228,6 +249,9 @@ struct tegra_pcie {
 
 	struct regulator *pex_clk_supply;
 	struct regulator *vdd_supply;
+	struct regulator *avdd_supply;
+
+	struct tegra_pcie_soc_data *soc_data;
 };
 
 struct tegra_pcie_port {
@@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
 		struct tegra_pcie_port *port;
 
 		list_for_each_entry(port, &pcie->ports, list) {
-			if (port->index + 1 == slot) {
+			if (port->index == slot) {
 				addr = port->base + (where & ~3);
 				break;
 			}
@@ -398,7 +422,6 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
 			*value = 0xffffffff;
 			return PCIBIOS_DEVICE_NOT_FOUND;
 		}
-
 		addr += tegra_pcie_conf_offset(devfn, where);
 	}
 
@@ -429,7 +452,7 @@ static int tegra_pcie_write_conf(struct pci_bus *bus, unsigned int devfn,
 		struct tegra_pcie_port *port;
 
 		list_for_each_entry(port, &pcie->ports, list) {
-			if (port->index + 1 == slot) {
+			if (port->index == slot) {
 				addr = port->base + (where & ~3);
 				break;
 			}
@@ -510,12 +533,15 @@ static void tegra_pcie_port_reset(struct tegra_pcie_port *port)
 
 static void tegra_pcie_port_enable(struct tegra_pcie_port *port)
 {
+	struct tegra_pcie_soc_data *soc = port->pcie->soc_data;
 	unsigned long ctrl = tegra_pcie_port_get_pex_ctrl(port);
 	unsigned long value;
 
 	/* enable reference clock */
 	value = afi_readl(port->pcie, ctrl);
 	value |= AFI_PEX_CTRL_REFCLK_EN;
+	if (soc->has_pex_clkreq_en)
+		value |= AFI_PEX_CTRL_CLKREQ_EN;
 	afi_writel(port->pcie, value, ctrl);
 
 	tegra_pcie_port_reset(port);
@@ -568,6 +594,8 @@ static void tegra_pcie_fixup_class(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf0, tegra_pcie_fixup_class);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class);
 
 /* Tegra PCIE requires relaxed ordering */
 static void tegra_pcie_relax_enable(struct pci_dev *dev)
@@ -748,6 +776,11 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	struct tegra_pcie_port *port;
 	unsigned int timeout;
 	unsigned long value;
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
+
+	/* power down to PCIe slot clock bias pad */
+	if (soc->has_pex_bias_ctrl)
+		afi_writel(pcie, 0, AFI_PEXBIAS_CTRL_0);
 
 	/* configure mode and disable all ports */
 	value = afi_readl(pcie, AFI_PCIE_CONFIG);
@@ -775,26 +808,26 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	 * Set up PHY PLL inputs select PLLE output as refclock,
 	 * set TX ref sel to div10 (not div5).
 	 */
-	value = pads_readl(pcie, PADS_PLL_CTL);
+	value = pads_readl(pcie, soc->pads_pll_ctl);
 	value &= ~(PADS_PLL_CTL_REFCLK_MASK | PADS_PLL_CTL_TXCLKREF_MASK);
-	value |= (PADS_PLL_CTL_REFCLK_INTERNAL_CML | PADS_PLL_CTL_TXCLKREF_DIV10);
-	pads_writel(pcie, value, PADS_PLL_CTL);
+	value |= PADS_PLL_CTL_REFCLK_INTERNAL_CML | soc->tx_ref_sel;
+	pads_writel(pcie, value, soc->pads_pll_ctl);
 
 	/* take PLL out of reset  */
-	value = pads_readl(pcie, PADS_PLL_CTL);
+	value = pads_readl(pcie, soc->pads_pll_ctl);
 	value |= PADS_PLL_CTL_RST_B4SM;
-	pads_writel(pcie, value, PADS_PLL_CTL);
+	pads_writel(pcie, value, soc->pads_pll_ctl);
 
 	/*
 	 * Hack, set the clock voltage to the DEFAULT provided by hw folks.
 	 * This doesn't exist in the documentation.
 	 */
-	pads_writel(pcie, 0xfa5cfa5c, 0xc8);
+	pads_writel(pcie, 0xfa5cfa5c, PADS_REFCLK_CFG0);
 
 	/* wait for the PLL to lock */
 	timeout = 300;
 	do {
-		value = pads_readl(pcie, PADS_PLL_CTL);
+		value = pads_readl(pcie, soc->pads_pll_ctl);
 		usleep_range(1000, 1000);
 		if (--timeout == 0) {
 			pr_err("Tegra PCIe error: timeout waiting for PLL\n");
@@ -823,6 +856,8 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	value = AFI_INTR_EN_INI_SLVERR | AFI_INTR_EN_INI_DECERR |
 		AFI_INTR_EN_TGT_SLVERR | AFI_INTR_EN_TGT_DECERR |
 		AFI_INTR_EN_TGT_WRERR | AFI_INTR_EN_DFPCI_DECERR;
+	if (soc->has_intr_prsnt_sense)
+		value |= AFI_INTR_EN_PRSNT_SENSE;
 	afi_writel(pcie, value, AFI_AFI_INTR_ENABLE);
 	afi_writel(pcie, 0xffffffff, AFI_SM_INTR_ENABLE);
 
@@ -837,6 +872,7 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 
 static void tegra_pcie_power_off(struct tegra_pcie *pcie)
 {
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	int err;
 
 	/* TODO: disable and unprepare clocks? */
@@ -848,19 +884,26 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie)
 	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
 	tegra_pmc_pcie_xclk_clamp(true);
 
+	if (soc->has_avdd_supply) {
+		err = regulator_disable(pcie->avdd_supply);
+		if (err < 0)
+			dev_warn(pcie->dev, "failed to disable AVDD regulator: %d\n",
+			err);
+	}
 	err = regulator_disable(pcie->pex_clk_supply);
 	if (err < 0)
-		dev_err(pcie->dev, "failed to disable pex-clk regulator: %d\n",
+		dev_warn(pcie->dev, "failed to disable pex-clk regulator: %d\n",
 			err);
 
 	err = regulator_disable(pcie->vdd_supply);
 	if (err < 0)
-		dev_err(pcie->dev, "failed to disable VDD regulator: %d\n",
+		dev_warn(pcie->dev, "failed to disable VDD regulator: %d\n",
 			err);
 }
 
 static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 {
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	int err;
 
 	tegra_periph_reset_assert(pcie->pcie_xclk);
@@ -884,6 +927,15 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 		return err;
 	}
 
+	if (soc->has_avdd_supply) {
+		err = regulator_enable(pcie->avdd_supply);
+		if (err < 0) {
+			dev_err(pcie->dev, "failed to enable AVDD regulator: %d\n",
+				err);
+			return err;
+		}
+	}
+
 	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_PCIE,
 						pcie->pex_clk);
 	if (err) {
@@ -901,6 +953,15 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 		return err;
 	}
 
+	if (soc->has_cml_clk) {
+		err = clk_prepare_enable(pcie->cml_clk);
+		if (err < 0) {
+			dev_err(pcie->dev, "failed to enable cml clock: %d\n",
+				err);
+			return err;
+		}
+	}
+
 	err = clk_prepare_enable(pcie->pll_e);
 	if (err < 0) {
 		dev_err(pcie->dev, "failed to enable PLLE clock: %d\n", err);
@@ -912,6 +973,8 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 
 static int tegra_pcie_clocks_get(struct tegra_pcie *pcie)
 {
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
+
 	pcie->pex_clk = devm_clk_get(pcie->dev, "pex");
 	if (IS_ERR(pcie->pex_clk))
 		return PTR_ERR(pcie->pex_clk);
@@ -928,6 +991,11 @@ static int tegra_pcie_clocks_get(struct tegra_pcie *pcie)
 	if (IS_ERR(pcie->pll_e))
 		return PTR_ERR(pcie->pll_e);
 
+	if (soc->has_cml_clk) {
+		pcie->cml_clk = devm_clk_get(pcie->dev, "cml");
+		if (IS_ERR(pcie->cml_clk))
+			return PTR_ERR(pcie->cml_clk);
+	}
 	return 0;
 }
 
@@ -1150,6 +1218,7 @@ static int tegra_pcie_enable_msi(struct tegra_pcie *pcie)
 {
 	struct platform_device *pdev = to_platform_device(pcie->dev);
 	struct tegra_msi *msi = &pcie->msi;
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	unsigned long base;
 	int err;
 	u32 reg;
@@ -1186,7 +1255,7 @@ static int tegra_pcie_enable_msi(struct tegra_pcie *pcie)
 	msi->pages = __get_free_pages(GFP_KERNEL, 3);
 	base = virt_to_phys((void *)msi->pages);
 
-	afi_writel(pcie, base, AFI_MSI_FPCI_BAR_ST);
+	afi_writel(pcie, base >> soc->msi_base_shift, AFI_MSI_FPCI_BAR_ST);
 	afi_writel(pcie, base, AFI_MSI_AXI_BAR_ST);
 	/* this register is in 4K increments */
 	afi_writel(pcie, 1, AFI_MSI_BAR_SZ);
@@ -1283,6 +1352,7 @@ static u32 tegra_pcie_get_xbar_config(struct tegra_pcie *pcie, u32 lanes)
 static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 {
 	struct device_node *np = pcie->dev->of_node, *port;
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	struct of_pci_range_parser parser;
 	struct of_pci_range range;
 	struct resource res;
@@ -1302,6 +1372,12 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 	if (IS_ERR(pcie->pex_clk_supply))
 		return PTR_ERR(pcie->pex_clk_supply);
 
+	if (soc->has_avdd_supply) {
+		pcie->avdd_supply = devm_regulator_get(pcie->dev, "avdd");
+		if (IS_ERR(pcie->avdd_supply))
+			return PTR_ERR(pcie->avdd_supply);
+	}
+
 	for_each_of_pci_range(&parser, &range) {
 		of_pci_range_to_resource(&range, np, &res);
 
@@ -1348,7 +1424,7 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 
 		index = PCI_SLOT(err);
 
-		if (index < 1 || index > TEGRA_MAX_PORTS) {
+		if (index < 1 || index > pcie->soc_data->num_max_ports) {
 			dev_err(pcie->dev, "invalid port number: %d\n", index);
 			return -EINVAL;
 		}
@@ -1484,8 +1560,39 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
 	return 0;
 }
 
+static const struct tegra_pcie_soc_data tegra20_pcie_data = {
+	.num_max_ports = 2,
+	.msi_base_shift = 0,
+	.pads_pll_ctl = PADS_PLL_CTL_T20,
+	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
+	.has_pex_clkreq_en = false,
+	.has_pex_bias_ctrl = false,
+	.has_intr_prsnt_sense = false,
+	.has_avdd_supply = false,
+	.has_cml_clk = false,
+};
+
+static const struct tegra_pcie_soc_data tegra30_pcie_data = {
+	.num_max_ports = 3,
+	.msi_base_shift = 8,
+	.pads_pll_ctl = PADS_PLL_CTL_T30,
+	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
+	.has_pex_clkreq_en = true,
+	.has_pex_bias_ctrl = true,
+	.has_intr_prsnt_sense = true,
+	.has_avdd_supply = true,
+	.has_cml_clk = true,
+};
+
+static const struct of_device_id tegra_pcie_of_match[] = {
+	{ .compatible = "nvidia,tegra30-pcie", .data = &tegra30_pcie_data},
+	{ .compatible = "nvidia,tegra20-pcie", .data = &tegra20_pcie_data},
+	{ },
+};
+
 static int tegra_pcie_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *match;
 	struct tegra_pcie *pcie;
 	int err;
 
@@ -1496,6 +1603,10 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&pcie->busses);
 	INIT_LIST_HEAD(&pcie->ports);
 	pcie->dev = &pdev->dev;
+	match = of_match_device(tegra_pcie_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+	pcie->soc_data = (struct tegra_pcie_soc_data *)match->data;
 
 	err = tegra_pcie_parse_dt(pcie);
 	if (err < 0)
@@ -1549,10 +1660,9 @@ static int tegra_pcie_remove(struct platform_device *pdev)
 	struct tegra_pcie_bus *bus;
 	int err;
 
-	list_for_each_entry(bus, &pcie->busses, list) {
+	list_for_each_entry(bus, &pcie->busses, list)
 		vunmap(bus->area->addr);
-		kfree(bus);
-	}
+	kfree(bus);
 
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		err = tegra_pcie_disable_msi(pcie);
@@ -1567,11 +1677,6 @@ static int tegra_pcie_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id tegra_pcie_of_match[] = {
-	{ .compatible = "nvidia,tegra20-pcie", },
-	{ },
-};
-
 static struct platform_driver tegra_pcie_driver = {
 	.driver = {
 		.name = "tegra-pcie",
-- 
1.7.0.4

[PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

- Enable PCIe root port 2 for Cardhu
- Make private data structure for each SoC
- Add required Tegra30 clocks and regulators
- Add Tegra30 specific code in enable controller
- Corrected logic in read/write config space to
  display right device number on bus 0
- Fixed unnecessary freeing of tegra_pcie_bus structure in tegra_pcie_remove
- Added Tegra30 specific properties in pci binding document

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:
- Added entry in required properties in pci document as per review comment
- Corrected logic in read/write config space to display right device number on bus 0
- Fixed unnecessary freeing of tegra_pcie_bus structure in tegra_pcie_remove
- Took care of other review comments to change data types and names of some members of tegra_pcie_soc_data structure

 .../bindings/pci/nvidia,tegra20-pcie.txt           |    2 +
 drivers/pci/host/pci-tegra.c                       |  157 ++++++++++++++++----
 2 files changed, 133 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
index 90c112f..4ac22ab 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
@@ -16,6 +16,7 @@ Required properties:
   "msi": The Tegra interrupt that is asserted when an MSI is received
 - pex-clk-supply: Supply voltage for internal reference clock
 - vdd-supply: Power supply for controller (1.05V)
+- avdd-supply: Power supply for controller (1.05V)
 - bus-range: Range of bus numbers associated with this controller
 - #address-cells: Address representation for root ports (must be 3)
   - cell 0 specifies the bus and device numbers of the root port:
@@ -48,6 +49,7 @@ Required properties:
   "afi": The Tegra clock of that name
   "pcie_xclk": The Tegra clock of that name
   "pll_e": The Tegra clock of that name
+  "cml": The Tegra clock of that name
 
 Root ports are defined as subnodes of the PCIe controller node.
 
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index c2d0b22..27c39d8 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -1,5 +1,5 @@
 /*
- * PCIe host controller driver for TEGRA(2) SOCs
+ * PCIe host controller driver for TEGRA SOCs
  *
  * Copyright (c) 2010, CompuLab, Ltd.
  * Author: Mike Rapoport <mike@compulab.co.il>
@@ -50,7 +50,6 @@
 #include <asm/mach/pci.h>
 
 #define INT_PCI_MSI_NR (8 * 32)
-#define TEGRA_MAX_PORTS 2
 
 /* register definitions */
 
@@ -142,14 +141,15 @@
 #define  AFI_INTR_EN_DFPCI_DECERR	(1 << 5)
 #define  AFI_INTR_EN_AXI_DECERR		(1 << 6)
 #define  AFI_INTR_EN_FPCI_TIMEOUT	(1 << 7)
+#define AFI_INTR_EN_PRSNT_SENSE		(1 << 8)
 
 #define AFI_PCIE_CONFIG					0x0f8
 #define  AFI_PCIE_CONFIG_PCIE_DISABLE(x)		(1 << ((x) + 1))
 #define  AFI_PCIE_CONFIG_PCIE_DISABLE_ALL		0xe
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_MASK	(0xf << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_SINGLE	(0x0 << 20)
-#define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_420	(0x0 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_DUAL	(0x1 << 20)
+#define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_420	(0x0 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_222	(0x1 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_411	(0x2 << 20)
 
@@ -160,8 +160,11 @@
 #define AFI_PEX1_CTRL			0x118
 #define AFI_PEX2_CTRL			0x128
 #define  AFI_PEX_CTRL_RST		(1 << 0)
+#define  AFI_PEX_CTRL_CLKREQ_EN		(1 << 1)
 #define  AFI_PEX_CTRL_REFCLK_EN		(1 << 3)
 
+#define  AFI_PEXBIAS_CTRL_0		0x168
+
 #define RP_VEND_XP	0x00000F00
 #define  RP_VEND_XP_DL_UP	(1 << 30)
 
@@ -175,7 +178,8 @@
 #define  PADS_CTL_TX_DATA_EN_1L	(1 << 6)
 #define  PADS_CTL_RX_DATA_EN_1L	(1 << 10)
 
-#define PADS_PLL_CTL				0x000000B8
+#define  PADS_PLL_CTL_T20			0x000000B8
+#define  PADS_PLL_CTL_T30			0x000000B4
 #define  PADS_PLL_CTL_RST_B4SM			(1 << 1)
 #define  PADS_PLL_CTL_LOCKDET			(1 << 8)
 #define  PADS_PLL_CTL_REFCLK_MASK		(0x3 << 16)
@@ -183,8 +187,11 @@
 #define  PADS_PLL_CTL_REFCLK_INTERNAL_CMOS	(1 << 16)
 #define  PADS_PLL_CTL_REFCLK_EXTERNAL		(2 << 16)
 #define  PADS_PLL_CTL_TXCLKREF_MASK		(0x1 << 20)
+#define  PADS_PLL_CTL_TXCLKREF_BUF_EN		(1 << 22)
 #define  PADS_PLL_CTL_TXCLKREF_DIV10		(0 << 20)
 #define  PADS_PLL_CTL_TXCLKREF_DIV5		(1 << 20)
+#define  PADS_REFCLK_CFG0			0x000000C8
+#define  PADS_REFCLK_CFG1			0x000000CC
 
 struct tegra_msi {
 	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
@@ -195,6 +202,19 @@ struct tegra_msi {
 	int irq;
 };
 
+/* used to differentiate tegra chips code */
+struct tegra_pcie_soc_data {
+	unsigned int num_max_ports;
+	unsigned int msi_base_shift;
+	u32 pads_pll_ctl;
+	u32 tx_ref_sel;
+	bool has_pex_clkreq_en;
+	bool has_pex_bias_ctrl;
+	bool has_intr_prsnt_sense;
+	bool has_avdd_supply;
+	bool has_cml_clk;
+};
+
 static inline struct tegra_msi *to_tegra_msi(struct msi_chip *chip)
 {
 	return container_of(chip, struct tegra_msi, chip);
@@ -219,6 +239,7 @@ struct tegra_pcie {
 	struct clk *afi_clk;
 	struct clk *pcie_xclk;
 	struct clk *pll_e;
+	struct clk *cml_clk;
 
 	struct tegra_msi msi;
 
@@ -228,6 +249,9 @@ struct tegra_pcie {
 
 	struct regulator *pex_clk_supply;
 	struct regulator *vdd_supply;
+	struct regulator *avdd_supply;
+
+	struct tegra_pcie_soc_data *soc_data;
 };
 
 struct tegra_pcie_port {
@@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
 		struct tegra_pcie_port *port;
 
 		list_for_each_entry(port, &pcie->ports, list) {
-			if (port->index + 1 == slot) {
+			if (port->index == slot) {
 				addr = port->base + (where & ~3);
 				break;
 			}
@@ -398,7 +422,6 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
 			*value = 0xffffffff;
 			return PCIBIOS_DEVICE_NOT_FOUND;
 		}
-
 		addr += tegra_pcie_conf_offset(devfn, where);
 	}
 
@@ -429,7 +452,7 @@ static int tegra_pcie_write_conf(struct pci_bus *bus, unsigned int devfn,
 		struct tegra_pcie_port *port;
 
 		list_for_each_entry(port, &pcie->ports, list) {
-			if (port->index + 1 == slot) {
+			if (port->index == slot) {
 				addr = port->base + (where & ~3);
 				break;
 			}
@@ -510,12 +533,15 @@ static void tegra_pcie_port_reset(struct tegra_pcie_port *port)
 
 static void tegra_pcie_port_enable(struct tegra_pcie_port *port)
 {
+	struct tegra_pcie_soc_data *soc = port->pcie->soc_data;
 	unsigned long ctrl = tegra_pcie_port_get_pex_ctrl(port);
 	unsigned long value;
 
 	/* enable reference clock */
 	value = afi_readl(port->pcie, ctrl);
 	value |= AFI_PEX_CTRL_REFCLK_EN;
+	if (soc->has_pex_clkreq_en)
+		value |= AFI_PEX_CTRL_CLKREQ_EN;
 	afi_writel(port->pcie, value, ctrl);
 
 	tegra_pcie_port_reset(port);
@@ -568,6 +594,8 @@ static void tegra_pcie_fixup_class(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf0, tegra_pcie_fixup_class);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class);
 
 /* Tegra PCIE requires relaxed ordering */
 static void tegra_pcie_relax_enable(struct pci_dev *dev)
@@ -748,6 +776,11 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	struct tegra_pcie_port *port;
 	unsigned int timeout;
 	unsigned long value;
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
+
+	/* power down to PCIe slot clock bias pad */
+	if (soc->has_pex_bias_ctrl)
+		afi_writel(pcie, 0, AFI_PEXBIAS_CTRL_0);
 
 	/* configure mode and disable all ports */
 	value = afi_readl(pcie, AFI_PCIE_CONFIG);
@@ -775,26 +808,26 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	 * Set up PHY PLL inputs select PLLE output as refclock,
 	 * set TX ref sel to div10 (not div5).
 	 */
-	value = pads_readl(pcie, PADS_PLL_CTL);
+	value = pads_readl(pcie, soc->pads_pll_ctl);
 	value &= ~(PADS_PLL_CTL_REFCLK_MASK | PADS_PLL_CTL_TXCLKREF_MASK);
-	value |= (PADS_PLL_CTL_REFCLK_INTERNAL_CML | PADS_PLL_CTL_TXCLKREF_DIV10);
-	pads_writel(pcie, value, PADS_PLL_CTL);
+	value |= PADS_PLL_CTL_REFCLK_INTERNAL_CML | soc->tx_ref_sel;
+	pads_writel(pcie, value, soc->pads_pll_ctl);
 
 	/* take PLL out of reset  */
-	value = pads_readl(pcie, PADS_PLL_CTL);
+	value = pads_readl(pcie, soc->pads_pll_ctl);
 	value |= PADS_PLL_CTL_RST_B4SM;
-	pads_writel(pcie, value, PADS_PLL_CTL);
+	pads_writel(pcie, value, soc->pads_pll_ctl);
 
 	/*
 	 * Hack, set the clock voltage to the DEFAULT provided by hw folks.
 	 * This doesn't exist in the documentation.
 	 */
-	pads_writel(pcie, 0xfa5cfa5c, 0xc8);
+	pads_writel(pcie, 0xfa5cfa5c, PADS_REFCLK_CFG0);
 
 	/* wait for the PLL to lock */
 	timeout = 300;
 	do {
-		value = pads_readl(pcie, PADS_PLL_CTL);
+		value = pads_readl(pcie, soc->pads_pll_ctl);
 		usleep_range(1000, 1000);
 		if (--timeout == 0) {
 			pr_err("Tegra PCIe error: timeout waiting for PLL\n");
@@ -823,6 +856,8 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 	value = AFI_INTR_EN_INI_SLVERR | AFI_INTR_EN_INI_DECERR |
 		AFI_INTR_EN_TGT_SLVERR | AFI_INTR_EN_TGT_DECERR |
 		AFI_INTR_EN_TGT_WRERR | AFI_INTR_EN_DFPCI_DECERR;
+	if (soc->has_intr_prsnt_sense)
+		value |= AFI_INTR_EN_PRSNT_SENSE;
 	afi_writel(pcie, value, AFI_AFI_INTR_ENABLE);
 	afi_writel(pcie, 0xffffffff, AFI_SM_INTR_ENABLE);
 
@@ -837,6 +872,7 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 
 static void tegra_pcie_power_off(struct tegra_pcie *pcie)
 {
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	int err;
 
 	/* TODO: disable and unprepare clocks? */
@@ -848,19 +884,26 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie)
 	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
 	tegra_pmc_pcie_xclk_clamp(true);
 
+	if (soc->has_avdd_supply) {
+		err = regulator_disable(pcie->avdd_supply);
+		if (err < 0)
+			dev_warn(pcie->dev, "failed to disable AVDD regulator: %d\n",
+			err);
+	}
 	err = regulator_disable(pcie->pex_clk_supply);
 	if (err < 0)
-		dev_err(pcie->dev, "failed to disable pex-clk regulator: %d\n",
+		dev_warn(pcie->dev, "failed to disable pex-clk regulator: %d\n",
 			err);
 
 	err = regulator_disable(pcie->vdd_supply);
 	if (err < 0)
-		dev_err(pcie->dev, "failed to disable VDD regulator: %d\n",
+		dev_warn(pcie->dev, "failed to disable VDD regulator: %d\n",
 			err);
 }
 
 static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 {
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	int err;
 
 	tegra_periph_reset_assert(pcie->pcie_xclk);
@@ -884,6 +927,15 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 		return err;
 	}
 
+	if (soc->has_avdd_supply) {
+		err = regulator_enable(pcie->avdd_supply);
+		if (err < 0) {
+			dev_err(pcie->dev, "failed to enable AVDD regulator: %d\n",
+				err);
+			return err;
+		}
+	}
+
 	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_PCIE,
 						pcie->pex_clk);
 	if (err) {
@@ -901,6 +953,15 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 		return err;
 	}
 
+	if (soc->has_cml_clk) {
+		err = clk_prepare_enable(pcie->cml_clk);
+		if (err < 0) {
+			dev_err(pcie->dev, "failed to enable cml clock: %d\n",
+				err);
+			return err;
+		}
+	}
+
 	err = clk_prepare_enable(pcie->pll_e);
 	if (err < 0) {
 		dev_err(pcie->dev, "failed to enable PLLE clock: %d\n", err);
@@ -912,6 +973,8 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 
 static int tegra_pcie_clocks_get(struct tegra_pcie *pcie)
 {
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
+
 	pcie->pex_clk = devm_clk_get(pcie->dev, "pex");
 	if (IS_ERR(pcie->pex_clk))
 		return PTR_ERR(pcie->pex_clk);
@@ -928,6 +991,11 @@ static int tegra_pcie_clocks_get(struct tegra_pcie *pcie)
 	if (IS_ERR(pcie->pll_e))
 		return PTR_ERR(pcie->pll_e);
 
+	if (soc->has_cml_clk) {
+		pcie->cml_clk = devm_clk_get(pcie->dev, "cml");
+		if (IS_ERR(pcie->cml_clk))
+			return PTR_ERR(pcie->cml_clk);
+	}
 	return 0;
 }
 
@@ -1150,6 +1218,7 @@ static int tegra_pcie_enable_msi(struct tegra_pcie *pcie)
 {
 	struct platform_device *pdev = to_platform_device(pcie->dev);
 	struct tegra_msi *msi = &pcie->msi;
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	unsigned long base;
 	int err;
 	u32 reg;
@@ -1186,7 +1255,7 @@ static int tegra_pcie_enable_msi(struct tegra_pcie *pcie)
 	msi->pages = __get_free_pages(GFP_KERNEL, 3);
 	base = virt_to_phys((void *)msi->pages);
 
-	afi_writel(pcie, base, AFI_MSI_FPCI_BAR_ST);
+	afi_writel(pcie, base >> soc->msi_base_shift, AFI_MSI_FPCI_BAR_ST);
 	afi_writel(pcie, base, AFI_MSI_AXI_BAR_ST);
 	/* this register is in 4K increments */
 	afi_writel(pcie, 1, AFI_MSI_BAR_SZ);
@@ -1283,6 +1352,7 @@ static u32 tegra_pcie_get_xbar_config(struct tegra_pcie *pcie, u32 lanes)
 static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 {
 	struct device_node *np = pcie->dev->of_node, *port;
+	struct tegra_pcie_soc_data *soc = pcie->soc_data;
 	struct of_pci_range_parser parser;
 	struct of_pci_range range;
 	struct resource res;
@@ -1302,6 +1372,12 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 	if (IS_ERR(pcie->pex_clk_supply))
 		return PTR_ERR(pcie->pex_clk_supply);
 
+	if (soc->has_avdd_supply) {
+		pcie->avdd_supply = devm_regulator_get(pcie->dev, "avdd");
+		if (IS_ERR(pcie->avdd_supply))
+			return PTR_ERR(pcie->avdd_supply);
+	}
+
 	for_each_of_pci_range(&parser, &range) {
 		of_pci_range_to_resource(&range, np, &res);
 
@@ -1348,7 +1424,7 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 
 		index = PCI_SLOT(err);
 
-		if (index < 1 || index > TEGRA_MAX_PORTS) {
+		if (index < 1 || index > pcie->soc_data->num_max_ports) {
 			dev_err(pcie->dev, "invalid port number: %d\n", index);
 			return -EINVAL;
 		}
@@ -1484,8 +1560,39 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
 	return 0;
 }
 
+static const struct tegra_pcie_soc_data tegra20_pcie_data = {
+	.num_max_ports = 2,
+	.msi_base_shift = 0,
+	.pads_pll_ctl = PADS_PLL_CTL_T20,
+	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
+	.has_pex_clkreq_en = false,
+	.has_pex_bias_ctrl = false,
+	.has_intr_prsnt_sense = false,
+	.has_avdd_supply = false,
+	.has_cml_clk = false,
+};
+
+static const struct tegra_pcie_soc_data tegra30_pcie_data = {
+	.num_max_ports = 3,
+	.msi_base_shift = 8,
+	.pads_pll_ctl = PADS_PLL_CTL_T30,
+	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
+	.has_pex_clkreq_en = true,
+	.has_pex_bias_ctrl = true,
+	.has_intr_prsnt_sense = true,
+	.has_avdd_supply = true,
+	.has_cml_clk = true,
+};
+
+static const struct of_device_id tegra_pcie_of_match[] = {
+	{ .compatible = "nvidia,tegra30-pcie", .data = &tegra30_pcie_data},
+	{ .compatible = "nvidia,tegra20-pcie", .data = &tegra20_pcie_data},
+	{ },
+};
+
 static int tegra_pcie_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *match;
 	struct tegra_pcie *pcie;
 	int err;
 
@@ -1496,6 +1603,10 @@ static int tegra_pcie_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&pcie->busses);
 	INIT_LIST_HEAD(&pcie->ports);
 	pcie->dev = &pdev->dev;
+	match = of_match_device(tegra_pcie_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+	pcie->soc_data = (struct tegra_pcie_soc_data *)match->data;
 
 	err = tegra_pcie_parse_dt(pcie);
 	if (err < 0)
@@ -1549,10 +1660,9 @@ static int tegra_pcie_remove(struct platform_device *pdev)
 	struct tegra_pcie_bus *bus;
 	int err;
 
-	list_for_each_entry(bus, &pcie->busses, list) {
+	list_for_each_entry(bus, &pcie->busses, list)
 		vunmap(bus->area->addr);
-		kfree(bus);
-	}
+	kfree(bus);
 
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		err = tegra_pcie_disable_msi(pcie);
@@ -1567,11 +1677,6 @@ static int tegra_pcie_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id tegra_pcie_of_match[] = {
-	{ .compatible = "nvidia,tegra20-pcie", },
-	{ },
-};
-
 static struct platform_driver tegra_pcie_driver = {
 	.driver = {
 		.name = "tegra-pcie",
-- 
1.7.0.4

[PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Jay Agarwal]

- Add interrupt-names property
- Correct downstream I/O size

Signed-off-by: Jay Agarwal <jagarwal-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Patch is based on remotes/gitorious_thierryreding_linux/tegra/next and should be applied on top of this.

Changes in V3:
- Avoided changes in cml clock as per review comment

 arch/arm/boot/dts/tegra30.dtsi |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index a242b2e..a301389 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -21,7 +21,7 @@
 		reg-names = "pads", "afi", "cs";
 		interrupts = <0 98 0x04   /* controller interrupt */
 		              0 99 0x04>; /* MSI interrupt */
-
+		interrupt-names = "intr", "msi";
 		bus-range = <0x00 0xff>;
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -29,7 +29,7 @@
 		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
 			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
 			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
-			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
+			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
 			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
 			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
 
-- 
1.7.0.4

[PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Jay Agarwal]

- Add interrupt-names property
- Correct downstream I/O size

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:
- Avoided changes in cml clock as per review comment

 arch/arm/boot/dts/tegra30.dtsi |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index a242b2e..a301389 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -21,7 +21,7 @@
 		reg-names = "pads", "afi", "cs";
 		interrupts = <0 98 0x04   /* controller interrupt */
 		              0 99 0x04>; /* MSI interrupt */
-
+		interrupt-names = "intr", "msi";
 		bus-range = <0x00 0xff>;
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -29,7 +29,7 @@
 		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
 			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
 			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
-			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
+			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
 			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
 			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
 
-- 
1.7.0.4

[PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Jay Agarwal]

- Add interrupt-names property
- Correct downstream I/O size

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:
- Avoided changes in cml clock as per review comment

 arch/arm/boot/dts/tegra30.dtsi |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index a242b2e..a301389 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -21,7 +21,7 @@
 		reg-names = "pads", "afi", "cs";
 		interrupts = <0 98 0x04   /* controller interrupt */
 		              0 99 0x04>; /* MSI interrupt */
-
+		interrupt-names = "intr", "msi";
 		bus-range = <0x00 0xff>;
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -29,7 +29,7 @@
 		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
 			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
 			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
-			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
+			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
 			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
 			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
 
-- 
1.7.0.4

[PATCH V3 4/4] ARM: tegra: pcie: Enable PCIe controller on Cardhu

[Jay Agarwal]

- Enable PCIe controller on Cardhu
- Only port 2 is connected on this board
- Add regulators required for Tegra30

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:
- Added num-lanes property for cardhu as per review comments

 arch/arm/boot/dts/tegra30-cardhu.dtsi |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
index 8ad4841..b6270e3 100644
--- a/arch/arm/boot/dts/tegra30-cardhu.dtsi
+++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi
@@ -27,6 +27,24 @@
 	model = "NVIDIA Tegra30 Cardhu evaluation board";
 	compatible = "nvidia,cardhu", "nvidia,tegra30";
 
+	pcie-controller {
+		status = "okay";
+		pex-clk-supply = <&pex_hvdd_3v3_reg>;
+		vdd-supply = <&ldo1_reg>;
+		avdd-supply = <&ldo2_reg>;
+
+		pci@1,0 {
+			nvidia,num-lanes = <4>;
+		};
+		pci@2,0 {
+			nvidia,num-lanes = <1>;
+		};
+		pci@3,0 {
+			status = "okay";
+			nvidia,num-lanes = <1>;
+		};
+	};
+
 	host1x {
 		dc@54200000 {
 			rgb {
-- 
1.7.0.4

[PATCH V3 4/4] ARM: tegra: pcie: Enable PCIe controller on Cardhu

[Jay Agarwal]

- Enable PCIe controller on Cardhu
- Only port 2 is connected on this board
- Add regulators required for Tegra30

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:
- Added num-lanes property for cardhu as per review comments

 arch/arm/boot/dts/tegra30-cardhu.dtsi |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
index 8ad4841..b6270e3 100644
--- a/arch/arm/boot/dts/tegra30-cardhu.dtsi
+++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi
@@ -27,6 +27,24 @@
 	model = "NVIDIA Tegra30 Cardhu evaluation board";
 	compatible = "nvidia,cardhu", "nvidia,tegra30";
 
+	pcie-controller {
+		status = "okay";
+		pex-clk-supply = <&pex_hvdd_3v3_reg>;
+		vdd-supply = <&ldo1_reg>;
+		avdd-supply = <&ldo2_reg>;
+
+		pci@1,0 {
+			nvidia,num-lanes = <4>;
+		};
+		pci@2,0 {
+			nvidia,num-lanes = <1>;
+		};
+		pci@3,0 {
+			status = "okay";
+			nvidia,num-lanes = <1>;
+		};
+	};
+
 	host1x {
 		dc@54200000 {
 			rgb {
-- 
1.7.0.4

[PATCH V3 4/4] ARM: tegra: pcie: Enable PCIe controller on Cardhu

[Jay Agarwal]

- Enable PCIe controller on Cardhu
- Only port 2 is connected on this board
- Add regulators required for Tegra30

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:
- Added num-lanes property for cardhu as per review comments

 arch/arm/boot/dts/tegra30-cardhu.dtsi |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
index 8ad4841..b6270e3 100644
--- a/arch/arm/boot/dts/tegra30-cardhu.dtsi
+++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi
@@ -27,6 +27,24 @@
 	model = "NVIDIA Tegra30 Cardhu evaluation board";
 	compatible = "nvidia,cardhu", "nvidia,tegra30";
 
+	pcie-controller {
+		status = "okay";
+		pex-clk-supply = <&pex_hvdd_3v3_reg>;
+		vdd-supply = <&ldo1_reg>;
+		avdd-supply = <&ldo2_reg>;
+
+		pci at 1,0 {
+			nvidia,num-lanes = <4>;
+		};
+		pci at 2,0 {
+			nvidia,num-lanes = <1>;
+		};
+		pci at 3,0 {
+			status = "okay";
+			nvidia,num-lanes = <1>;
+		};
+	};
+
 	host1x {
 		dc at 54200000 {
 			rgb {
-- 
1.7.0.4

Re: [PATCH V3 1/4] ARM: tegra30: clocks: Fix pciex clock registration

[Stephen Warren]

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?

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

[Stephen Warren]

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?

Re: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Stephen Warren]

On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> - Enable PCIe root port 2 for Cardhu
> - Make private data structure for each SoC
> - Add required Tegra30 clocks and regulators
> - Add Tegra30 specific code in enable controller
> - Corrected logic in read/write config space to
>   display right device number on bus 0
> - Fixed unnecessary freeing of tegra_pcie_bus structure in tegra_pcie_remove
> - Added Tegra30 specific properties in pci binding document

> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt

> +- avdd-supply: Power supply for controller (1.05V)

> +  "cml": The Tegra clock of that name

Both those changes need to mention that those additions are only
required for Tegra30, not Tegra20. In other words,

+- avdd-supply: Power supply for controller (1.05V) (not required for
Tegra20)

+  "cml": The Tegra clock of that name (not required for Tegra20)

You probably also want to explicitly mention nvidia,tegra30-pcie as a
legal compatible value.

> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c

> +/* used to differentiate tegra chips code */
> +struct tegra_pcie_soc_data {
> +	unsigned int num_max_ports;

nit: "num max" seems redundant. max_ports or num_ports would be better.

>  struct tegra_pcie_port {
> @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
>  		struct tegra_pcie_port *port;
>  
>  		list_for_each_entry(port, &pcie->ports, list) {
> -			if (port->index + 1 == slot) {
> +			if (port->index == slot) {

This and the equivalent change in tegra_pcie_write_conf() seem like a
bug-fix unrelated to the addition of Tegra30 support. Hence, they should
be a separate patch.


> @@ -398,7 +422,6 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
>  			*value = 0xffffffff;
>  			return PCIBIOS_DEVICE_NOT_FOUND;
>  		}
> -
>  		addr += tegra_pcie_conf_offset(devfn, where);

Unnecessary white-space change.

> @@ -1549,10 +1660,9 @@ static int tegra_pcie_remove(struct platform_device *pdev)
>  	struct tegra_pcie_bus *bus;
>  	int err;
>  
> -	list_for_each_entry(bus, &pcie->busses, list) {
> +	list_for_each_entry(bus, &pcie->busses, list)
>  		vunmap(bus->area->addr);
> -		kfree(bus);
> -	}
> +	kfree(bus);

This doesn't look right. Can you please explain it further? This is
looping over every bus in a dynamic list, so surely each entry needs to
be freed? Did you do this to avoid a crash? If so, the issue is likely
that the loop should use list_for_each_entry_safe() rather than
list_for_each_entry().

[PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Stephen Warren]

On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> - Enable PCIe root port 2 for Cardhu
> - Make private data structure for each SoC
> - Add required Tegra30 clocks and regulators
> - Add Tegra30 specific code in enable controller
> - Corrected logic in read/write config space to
>   display right device number on bus 0
> - Fixed unnecessary freeing of tegra_pcie_bus structure in tegra_pcie_remove
> - Added Tegra30 specific properties in pci binding document

> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt

> +- avdd-supply: Power supply for controller (1.05V)

> +  "cml": The Tegra clock of that name

Both those changes need to mention that those additions are only
required for Tegra30, not Tegra20. In other words,

+- avdd-supply: Power supply for controller (1.05V) (not required for
Tegra20)

+  "cml": The Tegra clock of that name (not required for Tegra20)

You probably also want to explicitly mention nvidia,tegra30-pcie as a
legal compatible value.

> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c

> +/* used to differentiate tegra chips code */
> +struct tegra_pcie_soc_data {
> +	unsigned int num_max_ports;

nit: "num max" seems redundant. max_ports or num_ports would be better.

>  struct tegra_pcie_port {
> @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
>  		struct tegra_pcie_port *port;
>  
>  		list_for_each_entry(port, &pcie->ports, list) {
> -			if (port->index + 1 == slot) {
> +			if (port->index == slot) {

This and the equivalent change in tegra_pcie_write_conf() seem like a
bug-fix unrelated to the addition of Tegra30 support. Hence, they should
be a separate patch.


> @@ -398,7 +422,6 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
>  			*value = 0xffffffff;
>  			return PCIBIOS_DEVICE_NOT_FOUND;
>  		}
> -
>  		addr += tegra_pcie_conf_offset(devfn, where);

Unnecessary white-space change.

> @@ -1549,10 +1660,9 @@ static int tegra_pcie_remove(struct platform_device *pdev)
>  	struct tegra_pcie_bus *bus;
>  	int err;
>  
> -	list_for_each_entry(bus, &pcie->busses, list) {
> +	list_for_each_entry(bus, &pcie->busses, list)
>  		vunmap(bus->area->addr);
> -		kfree(bus);
> -	}
> +	kfree(bus);

This doesn't look right. Can you please explain it further? This is
looping over every bus in a dynamic list, so surely each entry needs to
be freed? Did you do this to avoid a crash? If so, the issue is likely
that the loop should use list_for_each_entry_safe() rather than
list_for_each_entry().

RE: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

> > diff --git
> > a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> > b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> 
> > +- avdd-supply: Power supply for controller (1.05V)
> 
> > +  "cml": The Tegra clock of that name
> 
> Both those changes need to mention that those additions are only required
> for Tegra30, not Tegra20. In other words,
> 
> +- avdd-supply: Power supply for controller (1.05V) (not required for
> Tegra20)
> 
> +  "cml": The Tegra clock of that name (not required for Tegra20)
> 
> You probably also want to explicitly mention nvidia,tegra30-pcie as a legal
> compatible value.
> 
> > diff --git a/drivers/pci/host/pci-tegra.c
> > b/drivers/pci/host/pci-tegra.c
> 
> > +/* used to differentiate tegra chips code */ struct
> > +tegra_pcie_soc_data {
> > +	unsigned int num_max_ports;
> 
> nit: "num max" seems redundant. max_ports or num_ports would be better.
> 
> >  struct tegra_pcie_port {
> > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus,
> unsigned int devfn,
> >  		struct tegra_pcie_port *port;
> >
> >  		list_for_each_entry(port, &pcie->ports, list) {
> > -			if (port->index + 1 == slot) {
> > +			if (port->index == slot) {
> 
> This and the equivalent change in tegra_pcie_write_conf() seem like a bug-
> fix unrelated to the addition of Tegra30 support. Hence, they should be a
> separate patch.
> 
> 
> > @@ -398,7 +422,6 @@ static int tegra_pcie_read_conf(struct pci_bus *bus,
> unsigned int devfn,
> >  			*value = 0xffffffff;
> >  			return PCIBIOS_DEVICE_NOT_FOUND;
> >  		}
> > -
> >  		addr += tegra_pcie_conf_offset(devfn, where);
> 
> Unnecessary white-space change.
> 
> > @@ -1549,10 +1660,9 @@ static int tegra_pcie_remove(struct
> platform_device *pdev)
> >  	struct tegra_pcie_bus *bus;
> >  	int err;
> >
> > -	list_for_each_entry(bus, &pcie->busses, list) {
> > +	list_for_each_entry(bus, &pcie->busses, list)
> >  		vunmap(bus->area->addr);
> > -		kfree(bus);
> > -	}
> > +	kfree(bus);
> 
> This doesn't look right. Can you please explain it further? This is looping
> over every bus in a dynamic list, so surely each entry needs to be freed? Did
> you do this to avoid a crash? If so, the issue is likely that the loop should use
> list_for_each_entry_safe() rather than list_for_each_entry().
Yes you are right, it is not required. I will revert it along with taking care of other comments.

RE: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

> > diff --git
> > a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> > b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> 
> > +- avdd-supply: Power supply for controller (1.05V)
> 
> > +  "cml": The Tegra clock of that name
> 
> Both those changes need to mention that those additions are only required
> for Tegra30, not Tegra20. In other words,
> 
> +- avdd-supply: Power supply for controller (1.05V) (not required for
> Tegra20)
> 
> +  "cml": The Tegra clock of that name (not required for Tegra20)
> 
> You probably also want to explicitly mention nvidia,tegra30-pcie as a legal
> compatible value.
> 
> > diff --git a/drivers/pci/host/pci-tegra.c
> > b/drivers/pci/host/pci-tegra.c
> 
> > +/* used to differentiate tegra chips code */ struct
> > +tegra_pcie_soc_data {
> > +	unsigned int num_max_ports;
> 
> nit: "num max" seems redundant. max_ports or num_ports would be better.
> 
> >  struct tegra_pcie_port {
> > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus,
> unsigned int devfn,
> >  		struct tegra_pcie_port *port;
> >
> >  		list_for_each_entry(port, &pcie->ports, list) {
> > -			if (port->index + 1 == slot) {
> > +			if (port->index == slot) {
> 
> This and the equivalent change in tegra_pcie_write_conf() seem like a bug-
> fix unrelated to the addition of Tegra30 support. Hence, they should be a
> separate patch.
> 
> 
> > @@ -398,7 +422,6 @@ static int tegra_pcie_read_conf(struct pci_bus *bus,
> unsigned int devfn,
> >  			*value = 0xffffffff;
> >  			return PCIBIOS_DEVICE_NOT_FOUND;
> >  		}
> > -
> >  		addr += tegra_pcie_conf_offset(devfn, where);
> 
> Unnecessary white-space change.
> 
> > @@ -1549,10 +1660,9 @@ static int tegra_pcie_remove(struct
> platform_device *pdev)
> >  	struct tegra_pcie_bus *bus;
> >  	int err;
> >
> > -	list_for_each_entry(bus, &pcie->busses, list) {
> > +	list_for_each_entry(bus, &pcie->busses, list)
> >  		vunmap(bus->area->addr);
> > -		kfree(bus);
> > -	}
> > +	kfree(bus);
> 
> This doesn't look right. Can you please explain it further? This is looping
> over every bus in a dynamic list, so surely each entry needs to be freed? Did
> you do this to avoid a crash? If so, the issue is likely that the loop should use
> list_for_each_entry_safe() rather than list_for_each_entry().
Yes you are right, it is not required. I will revert it along with taking care of other comments.

[PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

> > diff --git
> > a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> > b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> 
> > +- avdd-supply: Power supply for controller (1.05V)
> 
> > +  "cml": The Tegra clock of that name
> 
> Both those changes need to mention that those additions are only required
> for Tegra30, not Tegra20. In other words,
> 
> +- avdd-supply: Power supply for controller (1.05V) (not required for
> Tegra20)
> 
> +  "cml": The Tegra clock of that name (not required for Tegra20)
> 
> You probably also want to explicitly mention nvidia,tegra30-pcie as a legal
> compatible value.
> 
> > diff --git a/drivers/pci/host/pci-tegra.c
> > b/drivers/pci/host/pci-tegra.c
> 
> > +/* used to differentiate tegra chips code */ struct
> > +tegra_pcie_soc_data {
> > +	unsigned int num_max_ports;
> 
> nit: "num max" seems redundant. max_ports or num_ports would be better.
> 
> >  struct tegra_pcie_port {
> > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus,
> unsigned int devfn,
> >  		struct tegra_pcie_port *port;
> >
> >  		list_for_each_entry(port, &pcie->ports, list) {
> > -			if (port->index + 1 == slot) {
> > +			if (port->index == slot) {
> 
> This and the equivalent change in tegra_pcie_write_conf() seem like a bug-
> fix unrelated to the addition of Tegra30 support. Hence, they should be a
> separate patch.
> 
> 
> > @@ -398,7 +422,6 @@ static int tegra_pcie_read_conf(struct pci_bus *bus,
> unsigned int devfn,
> >  			*value = 0xffffffff;
> >  			return PCIBIOS_DEVICE_NOT_FOUND;
> >  		}
> > -
> >  		addr += tegra_pcie_conf_offset(devfn, where);
> 
> Unnecessary white-space change.
> 
> > @@ -1549,10 +1660,9 @@ static int tegra_pcie_remove(struct
> platform_device *pdev)
> >  	struct tegra_pcie_bus *bus;
> >  	int err;
> >
> > -	list_for_each_entry(bus, &pcie->busses, list) {
> > +	list_for_each_entry(bus, &pcie->busses, list)
> >  		vunmap(bus->area->addr);
> > -		kfree(bus);
> > -	}
> > +	kfree(bus);
> 
> This doesn't look right. Can you please explain it further? This is looping
> over every bus in a dynamic list, so surely each entry needs to be freed? Did
> you do this to avoid a crash? If so, the issue is likely that the loop should use
> list_for_each_entry_safe() rather than list_for_each_entry().
Yes you are right, it is not required. I will revert it along with taking care of other comments.

Re: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]

On Tue, Jun 04, 2013 at 01:17:15PM -0600, Stephen Warren wrote:
> On 06/04/2013 12:57 PM, Jay Agarwal wrote:
[...]
> >  struct tegra_pcie_port {
> > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
> >  		struct tegra_pcie_port *port;
> >  
> >  		list_for_each_entry(port, &pcie->ports, list) {
> > -			if (port->index + 1 == slot) {
> > +			if (port->index == slot) {
> 
> This and the equivalent change in tegra_pcie_write_conf() seem like a
> bug-fix unrelated to the addition of Tegra30 support. Hence, they should
> be a separate patch.

What exactly is this change supposed to fix? The description doesn't
provide any details about why this is required. Furthermore this was
done on purpose to model the Tegra PCIe controller according to what
typical Linux systems provide.

Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0
etc are the root ports. The change proposed above makes 0:00.0 the
first root port, therefore breaking what systems usually expect.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Thierry Reding]

On Tue, Jun 04, 2013 at 01:17:15PM -0600, Stephen Warren wrote:
> On 06/04/2013 12:57 PM, Jay Agarwal wrote:
[...]
> >  struct tegra_pcie_port {
> > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
> >  		struct tegra_pcie_port *port;
> >  
> >  		list_for_each_entry(port, &pcie->ports, list) {
> > -			if (port->index + 1 == slot) {
> > +			if (port->index == slot) {
> 
> This and the equivalent change in tegra_pcie_write_conf() seem like a
> bug-fix unrelated to the addition of Tegra30 support. Hence, they should
> be a separate patch.

What exactly is this change supposed to fix? The description doesn't
provide any details about why this is required. Furthermore this was
done on purpose to model the Tegra PCIe controller according to what
typical Linux systems provide.

Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0
etc are the root ports. The change proposed above makes 0:00.0 the
first root port, therefore breaking what systems usually expect.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130610/1cbe8463/attachment.sig>

Re: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]

On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
[...]
> @@ -29,7 +29,7 @@
>  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
>  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
>  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
>  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
>  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */

That increases the I/O region size from 64 KiB to 1 MiB. Why is that
necessary? I/O operations can only address 64 KiB, so I don't think
adding more makes any sense.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Thierry Reding]

On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
[...]
> @@ -29,7 +29,7 @@
>  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
>  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
>  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
>  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
>  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */

That increases the I/O region size from 64 KiB to 1 MiB. Why is that
necessary? I/O operations can only address 64 KiB, so I don't think
adding more makes any sense.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130610/3878be8d/attachment.sig>

RE: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

> * PGP Signed by an unknown key
> 
> On Tue, Jun 04, 2013 at 01:17:15PM -0600, Stephen Warren wrote:
> > On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> [...]
> > >  struct tegra_pcie_port {
> > > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus
> *bus, unsigned int devfn,
> > >  		struct tegra_pcie_port *port;
> > >
> > >  		list_for_each_entry(port, &pcie->ports, list) {
> > > -			if (port->index + 1 == slot) {
> > > +			if (port->index == slot) {
> >
> > This and the equivalent change in tegra_pcie_write_conf() seem like a
> > bug-fix unrelated to the addition of Tegra30 support. Hence, they
> > should be a separate patch.
> 
> What exactly is this change supposed to fix? The description doesn't provide
> any details about why this is required. Furthermore this was done on
> purpose to model the Tegra PCIe controller according to what typical Linux
> systems provide.

I have mentioned it in description as -> "Corrected logic in read/write config space to display right device number on bus 0"

> Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0 etc are
> the root ports. The change proposed above makes 0:00.0 the first root port,
> therefore breaking what systems usually expect.
> 
I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03, which I thought should be pci_bus 0000:02, so made this change.

> Thierry
> 
> * Unknown Key
> * 0x7F3EB3A1

RE: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

> * PGP Signed by an unknown key
> 
> On Tue, Jun 04, 2013 at 01:17:15PM -0600, Stephen Warren wrote:
> > On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> [...]
> > >  struct tegra_pcie_port {
> > > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus
> *bus, unsigned int devfn,
> > >  		struct tegra_pcie_port *port;
> > >
> > >  		list_for_each_entry(port, &pcie->ports, list) {
> > > -			if (port->index + 1 == slot) {
> > > +			if (port->index == slot) {
> >
> > This and the equivalent change in tegra_pcie_write_conf() seem like a
> > bug-fix unrelated to the addition of Tegra30 support. Hence, they
> > should be a separate patch.
> 
> What exactly is this change supposed to fix? The description doesn't provide
> any details about why this is required. Furthermore this was done on
> purpose to model the Tegra PCIe controller according to what typical Linux
> systems provide.

I have mentioned it in description as -> "Corrected logic in read/write config space to display right device number on bus 0"

> Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0 etc are
> the root ports. The change proposed above makes 0:00.0 the first root port,
> therefore breaking what systems usually expect.
> 
I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03, which I thought should be pci_bus 0000:02, so made this change.

> Thierry
> 
> * Unknown Key
> * 0x7F3EB3A1

[PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

> * PGP Signed by an unknown key
> 
> On Tue, Jun 04, 2013 at 01:17:15PM -0600, Stephen Warren wrote:
> > On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> [...]
> > >  struct tegra_pcie_port {
> > > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus
> *bus, unsigned int devfn,
> > >  		struct tegra_pcie_port *port;
> > >
> > >  		list_for_each_entry(port, &pcie->ports, list) {
> > > -			if (port->index + 1 == slot) {
> > > +			if (port->index == slot) {
> >
> > This and the equivalent change in tegra_pcie_write_conf() seem like a
> > bug-fix unrelated to the addition of Tegra30 support. Hence, they
> > should be a separate patch.
> 
> What exactly is this change supposed to fix? The description doesn't provide
> any details about why this is required. Furthermore this was done on
> purpose to model the Tegra PCIe controller according to what typical Linux
> systems provide.

I have mentioned it in description as -> "Corrected logic in read/write config space to display right device number on bus 0"

> Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0 etc are
> the root ports. The change proposed above makes 0:00.0 the first root port,
> therefore breaking what systems usually expect.
> 
I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03, which I thought should be pci_bus 0000:02, so made this change.

> Thierry
> 
> * Unknown Key
> * 0x7F3EB3A1

RE: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Jay Agarwal]

> On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> [...]
> > @@ -29,7 +29,7 @@
> >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000
> /* port 0 configuration space */
> >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000
> /* port 1 configuration space */
> >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000
> /* port 2 configuration space */
> > -			  0x81000000 0 0          0x02000000 0 0x00010000   /*
> downstream I/O */
> > +			  0x81000000 0 0          0x02000000 0 0x00100000   /*
> downstream I/O */
> >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000
> /* non-prefetchable memory */
> >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>;
> /*
> > prefetchable memory */
> 
> That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> necessary? I/O operations can only address 64 KiB, so I don't think adding
> more makes any sense.
> 
Okay, you can keep it 64KiB then, I did it to match with downstream. 
Please let me know if you want me to upload new patch for this or you will take care while integrating.

RE: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Jay Agarwal]

> On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> [...]
> > @@ -29,7 +29,7 @@
> >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000
> /* port 0 configuration space */
> >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000
> /* port 1 configuration space */
> >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000
> /* port 2 configuration space */
> > -			  0x81000000 0 0          0x02000000 0 0x00010000   /*
> downstream I/O */
> > +			  0x81000000 0 0          0x02000000 0 0x00100000   /*
> downstream I/O */
> >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000
> /* non-prefetchable memory */
> >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>;
> /*
> > prefetchable memory */
> 
> That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> necessary? I/O operations can only address 64 KiB, so I don't think adding
> more makes any sense.
> 
Okay, you can keep it 64KiB then, I did it to match with downstream. 
Please let me know if you want me to upload new patch for this or you will take care while integrating.

RE: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Jay Agarwal]

> On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> [...]
> > @@ -29,7 +29,7 @@
> >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000
> /* port 0 configuration space */
> >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000
> /* port 1 configuration space */
> >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000
> /* port 2 configuration space */
> > -			  0x81000000 0 0          0x02000000 0 0x00010000   /*
> downstream I/O */
> > +			  0x81000000 0 0          0x02000000 0 0x00100000   /*
> downstream I/O */
> >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000
> /* non-prefetchable memory */
> >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>;
> /*
> > prefetchable memory */
> 
> That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> necessary? I/O operations can only address 64 KiB, so I don't think adding
> more makes any sense.
> 
Okay, you can keep it 64KiB then, I did it to match with downstream. 
Please let me know if you want me to upload new patch for this or you will take care while integrating.

[PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Jay Agarwal]

> On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> [...]
> > @@ -29,7 +29,7 @@
> >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000
> /* port 0 configuration space */
> >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000
> /* port 1 configuration space */
> >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000
> /* port 2 configuration space */
> > -			  0x81000000 0 0          0x02000000 0 0x00010000   /*
> downstream I/O */
> > +			  0x81000000 0 0          0x02000000 0 0x00100000   /*
> downstream I/O */
> >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000
> /* non-prefetchable memory */
> >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>;
> /*
> > prefetchable memory */
> 
> That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> necessary? I/O operations can only address 64 KiB, so I don't think adding
> more makes any sense.
> 
Okay, you can keep it 64KiB then, I did it to match with downstream. 
Please let me know if you want me to upload new patch for this or you will take care while integrating.

Re: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Peter De Schrijver]

On Mon, Jun 10, 2013 at 09:55:12PM +0200, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> [...]
> > @@ -29,7 +29,7 @@
> >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
> >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
> >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> > -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> > +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
> >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
> >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
> 
> That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> necessary? I/O operations can only address 64 KiB, so I don't think
> adding more makes any sense.

At least PCI allows 32bit I/O addresses. No idea if anyone uses them though.

Cheers,

Peter.

Re: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Peter De Schrijver]

On Mon, Jun 10, 2013 at 09:55:12PM +0200, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> [...]
> > @@ -29,7 +29,7 @@
> >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
> >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
> >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> > -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> > +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
> >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
> >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
> 
> That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> necessary? I/O operations can only address 64 KiB, so I don't think
> adding more makes any sense.

At least PCI allows 32bit I/O addresses. No idea if anyone uses them though.

Cheers,

Peter.

Re: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Peter De Schrijver]

On Mon, Jun 10, 2013 at 09:55:12PM +0200, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> [...]
> > @@ -29,7 +29,7 @@
> >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
> >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
> >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> > -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> > +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
> >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
> >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
> 
> That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> necessary? I/O operations can only address 64 KiB, so I don't think
> adding more makes any sense.

At least PCI allows 32bit I/O addresses. No idea if anyone uses them though.

Cheers,

Peter.

[PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Peter De Schrijver]

On Mon, Jun 10, 2013 at 09:55:12PM +0200, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> [...]
> > @@ -29,7 +29,7 @@
> >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
> >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
> >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> > -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> > +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
> >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
> >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
> 
> That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> necessary? I/O operations can only address 64 KiB, so I don't think
> adding more makes any sense.

At least PCI allows 32bit I/O addresses. No idea if anyone uses them though.

Cheers,

Peter.

Re: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

On Tue, Jun 11, 2013 at 10:13:38AM +0530, Jay Agarwal wrote:
> > * PGP Signed by an unknown key
> > 
> > On Tue, Jun 04, 2013 at 01:17:15PM -0600, Stephen Warren wrote:
> > > On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> > [...]
> > > >  struct tegra_pcie_port {
> > > > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus
> > *bus, unsigned int devfn,
> > > >  		struct tegra_pcie_port *port;
> > > >
> > > >  		list_for_each_entry(port, &pcie->ports, list) {
> > > > -			if (port->index + 1 == slot) {
> > > > +			if (port->index == slot) {
> > >
> > > This and the equivalent change in tegra_pcie_write_conf() seem like a
> > > bug-fix unrelated to the addition of Tegra30 support. Hence, they
> > > should be a separate patch.
> > 
> > What exactly is this change supposed to fix? The description doesn't provide
> > any details about why this is required. Furthermore this was done on
> > purpose to model the Tegra PCIe controller according to what typical Linux
> > systems provide.
> 
> I have mentioned it in description as -> "Corrected logic in read/write config space to display right device number on bus 0"
> 
> > Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0 etc are
> > the root ports. The change proposed above makes 0:00.0 the first root port,
> > therefore breaking what systems usually expect.
> > 
> I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03, which I thought should be pci_bus 0000:02, so made this change.

Yes, that's done on purpose to mirror what a typical PCI tree looks
like, as I already explained. So unless this fixes a real bug I'll just
drop it while applying.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

On Tue, Jun 11, 2013 at 10:13:38AM +0530, Jay Agarwal wrote:
> > * PGP Signed by an unknown key
> > 
> > On Tue, Jun 04, 2013 at 01:17:15PM -0600, Stephen Warren wrote:
> > > On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> > [...]
> > > >  struct tegra_pcie_port {
> > > > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus
> > *bus, unsigned int devfn,
> > > >  		struct tegra_pcie_port *port;
> > > >
> > > >  		list_for_each_entry(port, &pcie->ports, list) {
> > > > -			if (port->index + 1 == slot) {
> > > > +			if (port->index == slot) {
> > >
> > > This and the equivalent change in tegra_pcie_write_conf() seem like a
> > > bug-fix unrelated to the addition of Tegra30 support. Hence, they
> > > should be a separate patch.
> > 
> > What exactly is this change supposed to fix? The description doesn't provide
> > any details about why this is required. Furthermore this was done on
> > purpose to model the Tegra PCIe controller according to what typical Linux
> > systems provide.
> 
> I have mentioned it in description as -> "Corrected logic in read/write config space to display right device number on bus 0"
> 
> > Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0 etc are
> > the root ports. The change proposed above makes 0:00.0 the first root port,
> > therefore breaking what systems usually expect.
> > 
> I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03, which I thought should be pci_bus 0000:02, so made this change.

Yes, that's done on purpose to mirror what a typical PCI tree looks
like, as I already explained. So unless this fixes a real bug I'll just
drop it while applying.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

On Tue, Jun 11, 2013 at 10:13:38AM +0530, Jay Agarwal wrote:
> > * PGP Signed by an unknown key
> > 
> > On Tue, Jun 04, 2013 at 01:17:15PM -0600, Stephen Warren wrote:
> > > On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> > [...]
> > > >  struct tegra_pcie_port {
> > > > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus
> > *bus, unsigned int devfn,
> > > >  		struct tegra_pcie_port *port;
> > > >
> > > >  		list_for_each_entry(port, &pcie->ports, list) {
> > > > -			if (port->index + 1 == slot) {
> > > > +			if (port->index == slot) {
> > >
> > > This and the equivalent change in tegra_pcie_write_conf() seem like a
> > > bug-fix unrelated to the addition of Tegra30 support. Hence, they
> > > should be a separate patch.
> > 
> > What exactly is this change supposed to fix? The description doesn't provide
> > any details about why this is required. Furthermore this was done on
> > purpose to model the Tegra PCIe controller according to what typical Linux
> > systems provide.
> 
> I have mentioned it in description as -> "Corrected logic in read/write config space to display right device number on bus 0"
> 
> > Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0 etc are
> > the root ports. The change proposed above makes 0:00.0 the first root port,
> > therefore breaking what systems usually expect.
> > 
> I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03, which I thought should be pci_bus 0000:02, so made this change.

Yes, that's done on purpose to mirror what a typical PCI tree looks
like, as I already explained. So unless this fixes a real bug I'll just
drop it while applying.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Thierry Reding]

On Tue, Jun 11, 2013 at 10:13:38AM +0530, Jay Agarwal wrote:
> > * PGP Signed by an unknown key
> > 
> > On Tue, Jun 04, 2013 at 01:17:15PM -0600, Stephen Warren wrote:
> > > On 06/04/2013 12:57 PM, Jay Agarwal wrote:
> > [...]
> > > >  struct tegra_pcie_port {
> > > > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus
> > *bus, unsigned int devfn,
> > > >  		struct tegra_pcie_port *port;
> > > >
> > > >  		list_for_each_entry(port, &pcie->ports, list) {
> > > > -			if (port->index + 1 == slot) {
> > > > +			if (port->index == slot) {
> > >
> > > This and the equivalent change in tegra_pcie_write_conf() seem like a
> > > bug-fix unrelated to the addition of Tegra30 support. Hence, they
> > > should be a separate patch.
> > 
> > What exactly is this change supposed to fix? The description doesn't provide
> > any details about why this is required. Furthermore this was done on
> > purpose to model the Tegra PCIe controller according to what typical Linux
> > systems provide.
> 
> I have mentioned it in description as -> "Corrected logic in read/write config space to display right device number on bus 0"
> 
> > Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0 etc are
> > the root ports. The change proposed above makes 0:00.0 the first root port,
> > therefore breaking what systems usually expect.
> > 
> I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03, which I thought should be pci_bus 0000:02, so made this change.

Yes, that's done on purpose to mirror what a typical PCI tree looks
like, as I already explained. So unless this fixes a real bug I'll just
drop it while applying.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130611/e28e7f75/attachment-0001.sig>

RE: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

> > I have mentioned it in description as -> "Corrected logic in read/write
> config space to display right device number on bus 0"
> >
> > > Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0
> > > etc are the root ports. The change proposed above makes 0:00.0 the
> > > first root port, therefore breaking what systems usually expect.
> > >
> > I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03,
> which I thought should be pci_bus 0000:02, so made this change.
> 
> Yes, that's done on purpose to mirror what a typical PCI tree looks like, as I
> already explained. So unless this fixes a real bug I'll just drop it while
> applying.

Please apply V4 patch instead which does not contain this change along with taking care of other review comments.

RE: [PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

> > I have mentioned it in description as -> "Corrected logic in read/write
> config space to display right device number on bus 0"
> >
> > > Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0
> > > etc are the root ports. The change proposed above makes 0:00.0 the
> > > first root port, therefore breaking what systems usually expect.
> > >
> > I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03,
> which I thought should be pci_bus 0000:02, so made this change.
> 
> Yes, that's done on purpose to mirror what a typical PCI tree looks like, as I
> already explained. So unless this fixes a real bug I'll just drop it while
> applying.

Please apply V4 patch instead which does not contain this change along with taking care of other review comments.

[PATCH V3 2/4] ARM: tegra: pcie: Add tegra3 support

[Jay Agarwal]

> > I have mentioned it in description as -> "Corrected logic in read/write
> config space to display right device number on bus 0"
> >
> > > Device 0:00.0 is usually the root complex, and device 0:01.0, 0:02.0
> > > etc are the root ports. The change proposed above makes 0:00.0 the
> > > first root port, therefore breaking what systems usually expect.
> > >
> > I was seeing root port 2 in cardhu being enumerated as pci_bus 0000:03,
> which I thought should be pci_bus 0000:02, so made this change.
> 
> Yes, that's done on purpose to mirror what a typical PCI tree looks like, as I
> already explained. So unless this fixes a real bug I'll just drop it while
> applying.

Please apply V4 patch instead which does not contain this change along with taking care of other review comments.

Re: [PATCH V3 1/4] ARM: tegra30: clocks: Fix pciex clock registration

[Mike Turquette]

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

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

[Mike Turquette]

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

RE: [PATCH V3 1/4] ARM: tegra30: clocks: Fix pciex clock registration

[Jay Agarwal]

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

RE: [PATCH V3 1/4] ARM: tegra30: clocks: Fix pciex clock registration

[Jay Agarwal]

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

> 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.
ÿôèº{.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¥

RE: [PATCH V3 1/4] ARM: tegra30: clocks: Fix pciex clock registration

[Jay Agarwal]

PiBRdW90aW5nIFN0ZXBoZW4gV2FycmVuICgyMDEzLTA2LTA0IDEyOjA4OjA4KQ0KPiA+IE9uIDA2
LzA0LzIwMTMgMTI6NTcgUE0sIEpheSBBZ2Fyd2FsIHdyb3RlOg0KPiA+ID4gUmVnaXN0ZXJpbmcg
cGNpZXggYXMgcGVyaXBoZXJhbCBjbG9jayBpbnN0ZWFkIG9mIGZpeGVkIGNsb2NrIGFzDQo+ID4g
PiB0ZWdyYV9wZXJpaF9yZXNldF9hc3NlcnQoZGVhc3NlcnQpIGFwaSBvZiB0aGlzIGNsb2NrIGFw
aSBnaXZlcw0KPiA+ID4gd2FybmluZyBhbmQgdWx0aW1hdGVseSBkb2VzIG5vdCBzdWNjZWVkIHRv
IGFzc2VydChkZWFzc2VydCkuDQo+ID4gPg0KPiA+ID4gU2lnbmVkLW9mZi1ieTogSmF5IEFnYXJ3
YWwgPGphZ2Fyd2FsQG52aWRpYS5jb20+DQo+ID4gPiAtLS0NCj4gPiA+IFBhdGNoIGlzIGJhc2Vk
IG9uIHJlbW90ZXMvZ2l0b3Jpb3VzX3RoaWVycnlyZWRpbmdfbGludXgvdGVncmEvbmV4dCBhbmQN
Cj4gc2hvdWxkIGJlIGFwcGxpZWQgb24gdG9wIG9mIHRoaXMuDQo+ID4NCj4gPiBGb3IgdGhpcyBj
aGFuZ2UsIE1pa2UgbWF5IGFzIHdlbGwgYXBwbHkgaXQgZGlyZWN0bHkgdG8gdGhlIGNsb2NrIHRy
ZWUuDQo+ID4gVGhpZXJyeSBjYW4gdGhlbiBwaWNrIGl0IHVwIHdoZW4gaGUgcmViYXNlcyBoaXMg
dGVncmEvbmV4dCB0cmVlLg0KPiA+DQo+ID4gVGhhdCBzYWlkLCBJIGRvbid0IHRoaW5rIHlvdSBz
aG91bGQgbmVlZCBhbnkgb2YgdGhlDQo+ID4gVEVHUkFfQ0xLX0RVUExJQ0FURSgpIGVudHJpZXM7
IHRoZSBQQ0llIGRyaXZlciBzaG91bGQgZ2V0IGl0cyBjbG9ja3MNCj4gPiBmcm9tIGRldmljZSB0
cmVlIG5vdywgYW5kIGhlbmNlIHRoZSBkcml2ZXIgbmFtZSBpbiB0aGUgY2xvY2sNCj4gPiByZWdp
c3RyYXRpb24gc2hvdWxkbid0IGJlIG5lY2Vzc2FyeS4gQWxsIG9mIHRoZXNlDQo+ID4gVEVHUkFf
Q0xLX0RVUExJQ0FURSgpIGVudHJpZXMgc2hvdWxkIGJlIHJlbW92ZWQgZW4gbWFzcyBzb21ldGlt
ZQ0KPiBzb29uDQo+ID4gd2l0aCBsdWNrLiBTbywgY2FuIHlvdSBzaW1wbHkgbGVhdmUgdGhlIHR3
byBURUdSQV9DTEtfRFVQTElDQVRFKCkNCj4gPiBlbnRyaWVzIHVudG91Y2hlZCwgcmF0aGVyIHRo
YW4gY2hhbmdpbmcgdGhlbT8NCj4gDQo+IFBpbmcgb24gdGhpcyBwYXRjaC4gIEkgY2FuIHRha2Ug
aXQgdGhyb3VnaCBteSB0cmVlLCBidXQgaXMgdGhlcmUgZ29pbmcgdG8gYmUNCj4gcmV3b3JrIGJh
c2VkIG9uIFN0ZXBoZW4ncyBjb21tZW50cz8NCg0KSGkgTWlrZSwNCkkgaGF2ZSB1cGxvYWRlZCBu
ZXh0IHZlcnNpb24gVjQgb2YgdGhpcyBwYXRjaCBhZnRlciB0YWtpbmcgY2FyZSBvZiB0aGlzIGNv
bW1lbnQuDQo=

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

[Jay Agarwal]

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

Re: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1493 bytes --]

On Tue, Jun 11, 2013 at 10:30:48AM +0300, Peter De Schrijver wrote:
> On Mon, Jun 10, 2013 at 09:55:12PM +0200, Thierry Reding wrote:
> > * PGP Signed by an unknown key
> > 
> > On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> > [...]
> > > @@ -29,7 +29,7 @@
> > >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
> > >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
> > >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> > > -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> > > +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
> > >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
> > >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
> > 
> > That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> > necessary? I/O operations can only address 64 KiB, so I don't think
> > adding more makes any sense.
> 
> At least PCI allows 32bit I/O addresses. No idea if anyone uses them though.

I just realized that we are constrained to 64 KiB by the implementation
of pci_ioremap_io(), which assumes each mapping is 64 KiB. Not that it
couldn't be changed, but unless there actually is a use-case where more
than 64 KiB are required I don't think we should worry about it.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1493 bytes --]

On Tue, Jun 11, 2013 at 10:30:48AM +0300, Peter De Schrijver wrote:
> On Mon, Jun 10, 2013 at 09:55:12PM +0200, Thierry Reding wrote:
> > * PGP Signed by an unknown key
> > 
> > On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> > [...]
> > > @@ -29,7 +29,7 @@
> > >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
> > >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
> > >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> > > -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> > > +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
> > >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
> > >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
> > 
> > That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> > necessary? I/O operations can only address 64 KiB, so I don't think
> > adding more makes any sense.
> 
> At least PCI allows 32bit I/O addresses. No idea if anyone uses them though.

I just realized that we are constrained to 64 KiB by the implementation
of pci_ioremap_io(), which assumes each mapping is 64 KiB. Not that it
couldn't be changed, but unless there actually is a use-case where more
than 64 KiB are required I don't think we should worry about it.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1493 bytes --]

On Tue, Jun 11, 2013 at 10:30:48AM +0300, Peter De Schrijver wrote:
> On Mon, Jun 10, 2013 at 09:55:12PM +0200, Thierry Reding wrote:
> > * PGP Signed by an unknown key
> > 
> > On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> > [...]
> > > @@ -29,7 +29,7 @@
> > >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
> > >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
> > >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> > > -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> > > +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
> > >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
> > >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
> > 
> > That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> > necessary? I/O operations can only address 64 KiB, so I don't think
> > adding more makes any sense.
> 
> At least PCI allows 32bit I/O addresses. No idea if anyone uses them though.

I just realized that we are constrained to 64 KiB by the implementation
of pci_ioremap_io(), which assumes each mapping is 64 KiB. Not that it
couldn't be changed, but unless there actually is a use-case where more
than 64 KiB are required I don't think we should worry about it.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH V3 3/4] ARM: dts: tegra: Correct PCIe entry

[Thierry Reding]

On Tue, Jun 11, 2013 at 10:30:48AM +0300, Peter De Schrijver wrote:
> On Mon, Jun 10, 2013 at 09:55:12PM +0200, Thierry Reding wrote:
> > * PGP Signed by an unknown key
> > 
> > On Wed, Jun 05, 2013 at 12:27:31AM +0530, Jay Agarwal wrote:
> > [...]
> > > @@ -29,7 +29,7 @@
> > >  		ranges = <0x82000000 0 0x00000000 0x00000000 0 0x00001000   /* port 0 configuration space */
> > >  			  0x82000000 0 0x00001000 0x00001000 0 0x00001000   /* port 1 configuration space */
> > >  			  0x82000000 0 0x00004000 0x00004000 0 0x00001000   /* port 2 configuration space */
> > > -			  0x81000000 0 0          0x02000000 0 0x00010000   /* downstream I/O */
> > > +			  0x81000000 0 0          0x02000000 0 0x00100000   /* downstream I/O */
> > >  			  0x82000000 0 0x20000000 0x20000000 0 0x10000000   /* non-prefetchable memory */
> > >  			  0xc2000000 0 0x30000000 0x30000000 0 0x10000000>; /* prefetchable memory */
> > 
> > That increases the I/O region size from 64 KiB to 1 MiB. Why is that
> > necessary? I/O operations can only address 64 KiB, so I don't think
> > adding more makes any sense.
> 
> At least PCI allows 32bit I/O addresses. No idea if anyone uses them though.

I just realized that we are constrained to 64 KiB by the implementation
of pci_ioremap_io(), which assumes each mapping is 64 KiB. Not that it
couldn't be changed, but unless there actually is a use-case where more
than 64 KiB are required I don't think we should worry about it.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130717/db9411c6/attachment.sig>