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

login
register
mail settings
Submitter Jay Agarwal
Date May 8, 2013, 10:57 a.m.
Message ID <1368010660-31465-2-git-send-email-jagarwal@nvidia.com>
Download mbox | patch
Permalink /patch/242554/
State Not Applicable
Headers show

Comments

Jay Agarwal - May 8, 2013, 10:57 a.m.
- 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
- Patch is based on remotes/gitorious_thierryreding_linux/tegra/next
- and should be applied on top of this.

Signed-off-by: Jay Agarwal <jagarwal@nvidia.com>
---
 .../bindings/pci/nvidia,tegra20-pcie.txt           |    1 +
 drivers/pci/host/pci-tegra.c                       |  145 +++++++++++++++++---
 2 files changed, 127 insertions(+), 19 deletions(-)
Stephen Warren - May 8, 2013, 4:53 p.m.
On 05/08/2013 04:57 AM, 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

> - Patch is based on remotes/gitorious_thierryreding_linux/tegra/next
> - and should be applied on top of this.

Again, those two lines would go below the ---. And since that's all one
bullet point, why is the second line indented with a "-"?

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

>  	pcie-controller {
>  		vdd-supply = <&pci_vdd_reg>;
>  		pex-clk-supply = <&pci_clk_reg>;
> +		avdd-supply = <&ldo2_reg>; /* required for tegra30 */

I would simply drop that comment, and perhaps even the whole line; this
is just an example after all, and doesn't need to cover the latest SoC.
Equally, the example SoC DTSI section is an example for Tegra20, so
making the example board DTS section contain a Tegra30 example would be
inconsistent. Tegra30 should be capitalized; it's a name.

Why is there no change to the "Required Properties" section of this
document? It should list the set of clocks and regulators that are
required. That section is the definitive reference, whereas the example
is just than; an example.

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

> +	unsigned int pads_pll_ctl;
> +	unsigned int tx_ref_sel;

Perhaps those 2 values should be u32 to match the readl/writel
parameters? They're HW register values after all.

> @@ -220,6 +240,7 @@ struct tegra_pcie {
>  	struct clk *afi_clk;
>  	struct clk *pcie_xclk;
>  	struct clk *pll_e;
> +	struct clk *cml0_clk;

I think this clock should be called "cml" not "cml0". The clocks within
the driver and DT binding should be named from the perspective of the
PCI HW unit, and not from the perspective of the system that surrounds
it and provides those clocks.

The only exception would be if some future Tegra version might end up
with multiple cml clocks to a single PCIe unit, and we need to number
them to identify them. However, that seems quite unlikely since the PCIe
unit already supports multiple ports, and multiple port support would be
about the only reason that I could think of to have multiple clock
instances.

I think I mentioned this when I reviewed the previous version of the
patch, but you didn't respond to the suggestion.

> @@ -749,6 +778,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->pex_bias_ctrl)
> +		afi_writel(pcie, 0, AFI_PEXBIAS_CTRL_0);

Renaming pex_bias_ctrl to has_pex_bias_ctrl might make it more obvious
what that variable means.

A similar comment might apply to soc->pex_clkreq_en and
soc->intr_prsnt_sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
index 1ebc526..b4c4e42 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
@@ -91,6 +91,7 @@  Board DTS:
 	pcie-controller {
 		vdd-supply = <&pci_vdd_reg>;
 		pex-clk-supply = <&pci_clk_reg>;
+		avdd-supply = <&ldo2_reg>; /* required for tegra30 */
 		status = "okay";
 
 		/* root port 00:01.0 */
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 24085ed..f7fc650 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -51,7 +51,6 @@ 
 #include <mach/powergate.h>
 
 #define INT_PCI_MSI_NR (8 * 32)
-#define TEGRA_MAX_PORTS 2
 
 /* register definitions */
 
@@ -143,14 +142,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)
 
@@ -161,8 +161,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)
 
@@ -176,7 +179,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)
@@ -184,8 +188,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);
@@ -196,6 +203,19 @@  struct tegra_msi {
 	int irq;
 };
 
+/* used to differentiate tegra chips code */
+struct tegra_pcie_soc_data {
+	unsigned int num_max_ports;
+	unsigned int pads_pll_ctl;
+	unsigned int tx_ref_sel;
+	unsigned int msi_base_shift;
+	bool pex_clkreq_en;
+	bool pex_bias_ctrl;
+	bool intr_prsnt_sense;
+	bool has_avdd_supply;
+	bool has_cml0_clk;
+};
+
 static inline struct tegra_msi *to_tegra_msi(struct msi_chip *chip)
 {
 	return container_of(chip, struct tegra_msi, chip);
@@ -220,6 +240,7 @@  struct tegra_pcie {
 	struct clk *afi_clk;
 	struct clk *pcie_xclk;
 	struct clk *pll_e;
+	struct clk *cml0_clk;
 
 	struct tegra_msi msi;
 
@@ -229,6 +250,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 {
@@ -511,12 +535,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->pex_clkreq_en)
+		value |= AFI_PEX_CTRL_CLKREQ_EN;
 	afi_writel(port->pcie, value, ctrl);
 
 	tegra_pcie_port_reset(port);
@@ -569,6 +596,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)
@@ -749,6 +778,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->pex_bias_ctrl)
+		afi_writel(pcie, 0, AFI_PEXBIAS_CTRL_0);
 
 	/* configure mode and disable all ports */
 	value = afi_readl(pcie, AFI_PCIE_CONFIG);
@@ -776,26 +810,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");
@@ -824,6 +858,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->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);
 
@@ -838,6 +874,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? */
@@ -849,19 +886,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);
@@ -885,6 +929,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) {
@@ -902,6 +955,15 @@  static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 		return err;
 	}
 
+	if (soc->has_cml0_clk) {
+		err = clk_prepare_enable(pcie->cml0_clk);
+		if (err < 0) {
+			dev_err(pcie->dev, "failed to enable cml0 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);
@@ -913,6 +975,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);
@@ -929,6 +993,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_cml0_clk) {
+		pcie->cml0_clk = devm_clk_get(pcie->dev, "cml0");
+		if (IS_ERR(pcie->cml0_clk))
+			return PTR_ERR(pcie->cml0_clk);
+	}
 	return 0;
 }
 
@@ -1151,6 +1220,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;
@@ -1187,7 +1257,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);
@@ -1284,6 +1354,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;
 	const __be32 *range = NULL;
 	struct resource res;
 	u32 lanes = 0;
@@ -1297,6 +1368,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);
+	}
+
 	while ((range = of_pci_process_ranges(np, &res, range)) != NULL) {
 		switch (res.flags & IORESOURCE_TYPE_BITS) {
 		case IORESOURCE_IO:
@@ -1341,7 +1418,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;
 		}
@@ -1477,8 +1554,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,
+	.pads_pll_ctl = PADS_PLL_CTL_T20,
+	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
+	.msi_base_shift = 0,
+	.pex_clkreq_en = false,
+	.pex_bias_ctrl = false,
+	.intr_prsnt_sense = false,
+	.has_avdd_supply = false,
+	.has_cml0_clk = false,
+};
+
+static const struct tegra_pcie_soc_data tegra30_pcie_data = {
+	.num_max_ports = 3,
+	.pads_pll_ctl = PADS_PLL_CTL_T30,
+	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
+	.msi_base_shift = 8,
+	.pex_clkreq_en = true,
+	.pex_bias_ctrl = true,
+	.intr_prsnt_sense = true,
+	.has_avdd_supply = true,
+	.has_cml0_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;
 
@@ -1489,6 +1597,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)
@@ -1560,11 +1672,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",