[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 <jaga...@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[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 <jaga...@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 <mi...@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",

[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 <jaga...@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 {

[Jay Agarwal]

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

Signed-off-by: Jay Agarwal <jaga...@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 */

[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 <jaga...@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?

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

[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 {

Yes you are right, it is not required. I will revert it along with taking care of other comments.

[Jay Agarwal]

- Make private data structure for each SoC
- Add required Tegra30 clocks and regulators
- Add Tegra30 specific code in enable controller

- Added Tegra30 specific properties in pci binding document

Signed-off-by: Jay Agarwal <jaga...@nvidia.com>
---
Patch is based on remotes/gitorious_thierryreding_linux/tegra/next and should be applied on top of this.

Changes in V4:
As per review comment
- Corrected pci document
- Removed fix in read/write config space
- modified num_max_ports -> num_ports
- Avoided changes in tegra_pcie_remove
---
.../bindings/pci/nvidia,tegra20-pcie.txt | 4 +-
drivers/pci/host/pci-tegra.c | 147 +++++++++++++++++---
2 files changed, 130 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
index 90c112f..db673c0 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
@@ -1,7 +1,7 @@
NVIDIA Tegra PCIe controller

Required properties:
-- compatible: "nvidia,tegra20-pcie"
+- compatible: "nvidia,tegra20-pcie" "nvidia,tegra30-pcie"
- device_type: Must be "pci"
- reg: A list of physical base address and length for each set of controller
registers. Must contain an entry for each entry in the reg-names property.

@@ -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) (not required for tegra20)

- 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 (not required for tegra20)

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..6a992ef 100644

+/* used to differentiate tegra chips code */

+struct tegra_pcie_soc_data {
+ unsigned int num_ports;

@@ -510,12 +534,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 +595,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 +777,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 +809,26 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)

@@ -823,6 +857,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 +873,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 +885,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 +928,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 +954,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 +974,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 +992,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 +1219,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 +1256,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 +1353,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 +1373,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 +1425,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_ports) {

dev_err(pcie->dev, "invalid port number: %d\n", index);
return -EINVAL;
}

@@ -1484,8 +1561,39 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)

return 0;
}

+static const struct tegra_pcie_soc_data tegra20_pcie_data = {

+ .num_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_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 +1604,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)

@@ -1567,11 +1679,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

[Stephen Warren]

On 06/05/2013 09:13 AM, Jay Agarwal wrote:
> - Make private data structure for each SoC
> - Add required Tegra30 clocks and regulators
> - Add Tegra30 specific code in enable controller
> - Added Tegra30 specific properties in pci binding document

It's a bit odd to send an updated version of just some of the patches;
reposting a whole series is much easier for the person applying the
patches. However, hopefully Thierry can handle it in this case.

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

> Required properties:
> -- compatible: "nvidia,tegra20-pcie"
> +- compatible: "nvidia,tegra20-pcie" "nvidia,tegra30-pcie"

perhaps add "or" between those two entries?

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

..

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

Tegra20 should be capitalized in both those two lines.

I assume that Thierry can fix up these nits when he applies the patches,
so you don't need to repost. Thierry can correct me if I'm wrong.

[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

[Jay Agarwal]

> * PGP Signed by an unknown key
>

> On Wed, Jun 05, 2013 at 10:36:01AM -0600, Stephen Warren wrote:
> > On 06/05/2013 09:13 AM, Jay Agarwal wrote:
> > > - Make private data structure for each SoC
> > > - Add required Tegra30 clocks and regulators
> > > - Add Tegra30 specific code in enable controller
> > > - Added Tegra30 specific properties in pci binding document
> >
> > It's a bit odd to send an updated version of just some of the patches;
> > reposting a whole series is much easier for the person applying the
> > patches. However, hopefully Thierry can handle it in this case.
>

> Yes, I think I can handle it. It might be good to have Jay look at the final
> patches when I'm done, though, to make sure I haven't forgotten anything.
>
Yes, please let me know whenever you have integrated my patches to your repo, so that I can sync, review & verify again.

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

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

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

[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 <jaga...@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

[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 <jaga...@nvidia.com>
---
Patch is based on remotes/gitorious_thierryreding_linux/tegra/next and should be applied on top of this.

Changes in V4:
- Avoid modifying tegra pcie driver name for duplicate clocks as per review comments

drivers/clk/tegra/clk-tegra30.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index c6921f5..ba99e38 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)

--
1.7.0.4

[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 <jaga...@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.
｢鴈ｹｻ ｮ&ﾞ~ｺ&ｶ ｬ�-ｱ鰡ｶ ･学ｮ寨岾ﾊ穃饕樌dzｹﾞ�奛跖w* jgｬｱｨ ｶ凹至ﾝ｢j/�艘ｹﾞ槙�巌勣隴ﾚ&｢)ﾟ｡ｫaｶﾚ �ｮGｫ晞hｮ 詼:+v鴎学閹ﾙ･>W坡�iﾛaxP jﾘmｶ�ﾃ-ｻ +�d喟

[Stephen Warren]

On 06/12/2013 01:13 AM, 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 <jaga...@nvidia.com>
> ---
> Patch is based on remotes/gitorious_thierryreding_linux/tegra/next and should be applied on top of this.

In this case, it would make more sense to apply this to the clock tree,
since it's a standalone bug-fix.

Acked-by: Stephen Warren <swa...@nvidia.com>

[Mike Turquette]

Quoting Stephen Warren (2013-06-12 08:36:11)

> On 06/12/2013 01:13 AM, 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 <jaga...@nvidia.com>
> > ---
> > Patch is based on remotes/gitorious_thierryreding_linux/tegra/next and should be applied on top of this.
>
> In this case, it would make more sense to apply this to the clock tree,
> since it's a standalone bug-fix.
>
> Acked-by: Stephen Warren <swa...@nvidia.com>

Pulled into clk-fixes for -rc7.

Regards,
Mike

[Jonghwan Choi]

This patch looks like it should be in the 3.9-stable tree, should we apply
it?

------------------

From: "Jay Agarwal <jaga...@nvidia.com>"

commit ff49fad1d9bf2c49f52817b04cde8e4412434637 upstream

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 <jaga...@nvidia.com>

Acked-by: Stephen Warren <swa...@nvidia.com>
Signed-off-by: Mike Turquette <mturq...@linaro.org>
Signed-off-by: Jonghwan Choi <jhbir...@samsung.com>
---
drivers/clk/tegra/clk-tegra30.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra30.c
b/drivers/clk/tegra/clk-tegra30.c
index ba6f51b..1f8595b 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1592,6 +1592,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,

@@ -1710,11 +1716,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)
--

1.7.9.5

[Jay Agarwal]

+Stephen to suggest

[Stephen Warren]

On 06/18/2013 06:19 PM, Jonghwan Choi wrote:
> This patch looks like it should be in the 3.9-stable tree, should we apply
> it?

The PCIe driver doesn't work usefully in that release, so there's not
much point back-porting this fix; alone, it won't make PCIe work there.