diff mbox

[V3] pci: exynos: split into two parts such as Synopsys part and Exynos part

Message ID 002801ce8376$a807dbf0$f81793d0$@samsung.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Jingoo Han July 18, 2013, 5:21 a.m. UTC
Exynos PCIe IP consists of Synopsys specific part and Exynos
specific part. Only core block is a Synopsys designware part;
other parts are Exynos specific.
Also, the Synopsys designware part can be shared with other
platforms; thus, it can be split two parts such as Synopsys
designware part and Exynos specific part.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
---
Changes since v2:
- replaced 'pcie-' prefix with 'pci-' prefix
- made the number of lanes configurable
- added 'num-lanes' property
- added a 'struct exynos_pcie'

 .../devicetree/bindings/pci/designware-pcie.txt    |    3 +
 arch/arm/boot/dts/exynos5440.dtsi                  |    2 +
 drivers/pci/host/Makefile                          |    1 +
 drivers/pci/host/pci-exynos.c                      |  552 +++++++++++
 drivers/pci/host/pcie-designware.c                 |  998 +++++---------------
 drivers/pci/host/pcie-designware.h                 |   65 ++
 6 files changed, 882 insertions(+), 739 deletions(-)
 create mode 100644 drivers/pci/host/pci-exynos.c
 create mode 100644 drivers/pci/host/pcie-designware.h

Comments

Kishon Vijay Abraham I July 22, 2013, 3:03 p.m. UTC | #1
Hi,

On Thursday 18 July 2013 10:51 AM, Jingoo Han wrote:
> Exynos PCIe IP consists of Synopsys specific part and Exynos
> specific part. Only core block is a Synopsys designware part;
> other parts are Exynos specific.
> Also, the Synopsys designware part can be shared with other
> platforms; thus, it can be split two parts such as Synopsys
> designware part and Exynos specific part.

some more queries and comments..
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> ---
> Changes since v2:
.
.
<snip>
.
.
> +
> +static struct pcie_host_ops exynos_pcie_host_ops = {
> +	.readl_rc = exynos_pcie_readl_rc,
> +	.writel_rc = exynos_pcie_writel_rc,
> +	.rd_own_conf = exynos_pcie_rd_own_conf,
> +	.wr_own_conf = exynos_pcie_wr_own_conf,
> +	.link_up = exynos_pcie_link_up,
> +	.host_init = exynos_pcie_host_init,
> +};
> +
> +static int add_pcie_port(struct pcie_port *pp, struct platform_device *pdev)
> +{
> +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);

We can move the exynos_pcie specific initialization to probe and leave only
pcie_port initialization here.
> +	struct resource *elbi_base;
> +	struct resource *phy_base;
> +	struct resource *block_base;
> +	int ret;
> +
> +	elbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!elbi_base) {
> +		dev_err(&pdev->dev, "couldn't get elbi base resource\n");
> +		return -EINVAL;
> +	}
> +	exynos_pcie->elbi_base = devm_ioremap_resource(&pdev->dev, elbi_base);
> +	if (IS_ERR(exynos_pcie->elbi_base))
> +		return PTR_ERR(exynos_pcie->elbi_base);
> +
> +	phy_base = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!phy_base) {
> +		dev_err(&pdev->dev, "couldn't get phy base resource\n");
> +		return -EINVAL;
> +	}
> +	exynos_pcie->phy_base = devm_ioremap_resource(&pdev->dev, phy_base);
> +	if (IS_ERR(exynos_pcie->phy_base))
> +		return PTR_ERR(exynos_pcie->phy_base);
> +
> +	block_base = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +	if (!block_base) {
> +		dev_err(&pdev->dev, "couldn't get block base resource\n");
> +		return -EINVAL;
> +	}
> +	exynos_pcie->block_base = devm_ioremap_resource(&pdev->dev, block_base);
> +	if (IS_ERR(exynos_pcie->block_base))
> +		return PTR_ERR(exynos_pcie->block_base);

So all this till here can be moved to probe.
> +
> +	pp->irq = platform_get_irq(pdev, 1);
> +	if (!pp->irq) {
> +		dev_err(&pdev->dev, "failed to get irq\n");
> +		return -ENODEV;
> +	}
> +	ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler,
> +				IRQF_SHARED, "exynos-pcie", pp);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request irq\n");
> +		return ret;
> +	}
> +
> +	pp->root_bus_nr = -1;
> +	pp->ops = &exynos_pcie_host_ops;
> +
> +	spin_lock_init(&pp->conf_lock);
> +	ret = dw_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to initialize host\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int __init exynos_pcie_probe(struct platform_device *pdev)
> +{
> +	struct exynos_pcie *exynos_pcie;
> +	struct pcie_port *pp;
> +	struct device_node *np = pdev->dev.of_node;
> +	int ret;
> +
> +	exynos_pcie = devm_kzalloc(&pdev->dev, sizeof(*exynos_pcie),
> +				GFP_KERNEL);
> +	if (!exynos_pcie) {
> +		dev_err(&pdev->dev, "no memory for exynos pcie\n");
> +		return -ENOMEM;
> +	}
> +
> +	pp = &exynos_pcie->pp;
> +
> +	pp->dev = &pdev->dev;
> +
> +	exynos_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> +
> +	exynos_pcie->clk = devm_clk_get(&pdev->dev, "pcie");
> +	if (IS_ERR(exynos_pcie->clk)) {
> +		dev_err(&pdev->dev, "Failed to get pcie rc clock\n");
> +		return PTR_ERR(exynos_pcie->clk);
> +	}
> +	ret = clk_prepare_enable(exynos_pcie->clk);
> +	if (ret)
> +		return ret;
> +
> +	exynos_pcie->bus_clk = devm_clk_get(&pdev->dev, "pcie_bus");
> +	if (IS_ERR(exynos_pcie->bus_clk)) {
> +		dev_err(&pdev->dev, "Failed to get pcie bus clock\n");
> +		ret = PTR_ERR(exynos_pcie->bus_clk);
> +		goto fail_clk;
> +	}
> +	ret = clk_prepare_enable(exynos_pcie->bus_clk);
> +	if (ret)
> +		goto fail_clk;
> +
> +	ret = add_pcie_port(pp, pdev);
> +	if (ret < 0)
> +		goto fail_bus_clk;

I think we should move all the below code to designware core file. IMO it
should be common everyone who use designware core.
> +
> +	dw_pci.nr_controllers = 1;
> +	dw_pci.private_data = (void **)&pp;
> +
> +	pci_common_init(&dw_pci);
> +	pci_assign_unassigned_resources();
> +#ifdef CONFIG_PCI_DOMAINS
> +	dw_pci.domain++;
> +#endif
> +
> +	platform_set_drvdata(pdev, exynos_pcie);
> +	return 0;
> +
> +fail_bus_clk:
> +	clk_disable_unprepare(exynos_pcie->bus_clk);
> +fail_clk:
> +	clk_disable_unprepare(exynos_pcie->clk);
> +	return ret;
> +}
> +
> +static int __exit exynos_pcie_remove(struct platform_device *pdev)
> +{
> +	struct exynos_pcie *exynos_pcie = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(exynos_pcie->bus_clk);
> +	clk_disable_unprepare(exynos_pcie->clk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id exynos_pcie_of_match[] = {
> +	{ .compatible = "samsung,exynos5440-pcie", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_pcie_of_match);
> +
> +static struct platform_driver exynos_pcie_driver = {
> +	.remove		= __exit_p(exynos_pcie_remove),
> +	.driver = {
> +		.name	= "exynos-pcie",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(exynos_pcie_of_match),
> +	},
> +};
> +
> +/* Exynos PCIe driver does not allow module unload */
> +
> +static int __init pcie_init(void)
> +{
> +	return platform_driver_probe(&exynos_pcie_driver, exynos_pcie_probe);
> +}
> +subsys_initcall(pcie_init);
> +
> +MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
> +MODULE_DESCRIPTION("Samsung PCIe host controller driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 26bdbda..f097eff 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -1,5 +1,5 @@
>  /*
> - * PCIe host controller driver for Samsung EXYNOS SoCs
> + * Synopsys Designware PCIe host controller driver
>   *
>   * Copyright (C) 2013 Samsung Electronics Co., Ltd.
>   *		http://www.samsung.com
> @@ -11,74 +11,28 @@
>   * published by the Free Software Foundation.
>   */
>  
> -#include <linux/clk.h>
> -#include <linux/delay.h>
> -#include <linux/gpio.h>
> -#include <linux/interrupt.h>
>  #include <linux/kernel.h>
> -#include <linux/list.h>
>  #include <linux/module.h>
> -#include <linux/of.h>
>  #include <linux/of_address.h>
> -#include <linux/of_gpio.h>
> -#include <linux/of_pci.h>
>  #include <linux/pci.h>
>  #include <linux/pci_regs.h>
> -#include <linux/platform_device.h>
> -#include <linux/resource.h>
> -#include <linux/signal.h>
> -#include <linux/slab.h>
>  #include <linux/types.h>
>  
> -struct pcie_port_info {
> -	u32		cfg0_size;
> -	u32		cfg1_size;
> -	u32		io_size;
> -	u32		mem_size;
> -	phys_addr_t	io_bus_addr;
> -	phys_addr_t	mem_bus_addr;
> -};
> -
> -struct pcie_port {
> -	struct device		*dev;
> -	u8			controller;
> -	u8			root_bus_nr;
> -	void __iomem		*dbi_base;
> -	void __iomem		*elbi_base;
> -	void __iomem		*phy_base;
> -	void __iomem		*purple_base;
> -	u64			cfg0_base;
> -	void __iomem		*va_cfg0_base;
> -	u64			cfg1_base;
> -	void __iomem		*va_cfg1_base;
> -	u64			io_base;
> -	u64			mem_base;
> -	spinlock_t		conf_lock;
> -	struct resource		cfg;
> -	struct resource		io;
> -	struct resource		mem;
> -	struct pcie_port_info	config;
> -	struct clk		*clk;
> -	struct clk		*bus_clk;
> -	int			irq;
> -	int			reset_gpio;
> -};
> -
> -/*
> - * Exynos PCIe IP consists of Synopsys specific part and Exynos
> - * specific part. Only core block is a Synopsys designware part;
> - * other parts are Exynos specific.
> - */
> +#include "pcie-designware.h"
>  
>  /* Synopsis specific PCIE configuration registers */
>  #define PCIE_PORT_LINK_CONTROL		0x710
>  #define PORT_LINK_MODE_MASK		(0x3f << 16)
> +#define PORT_LINK_MODE_1_LANES		(0x1 << 16)
> +#define PORT_LINK_MODE_2_LANES		(0x3 << 16)
>  #define PORT_LINK_MODE_4_LANES		(0x7 << 16)
>  
>  #define PCIE_LINK_WIDTH_SPEED_CONTROL	0x80C
>  #define PORT_LOGIC_SPEED_CHANGE		(0x1 << 17)
>  #define PORT_LOGIC_LINK_WIDTH_MASK	(0x1ff << 8)
> -#define PORT_LOGIC_LINK_WIDTH_4_LANES	(0x7 << 8)
> +#define PORT_LOGIC_LINK_WIDTH_1_LANES	(0x1 << 8)
> +#define PORT_LOGIC_LINK_WIDTH_2_LANES	(0x2 << 8)
> +#define PORT_LOGIC_LINK_WIDTH_4_LANES	(0x4 << 8)
>  
>  #define PCIE_MSI_ADDR_LO		0x820
>  #define PCIE_MSI_ADDR_HI		0x824
> @@ -108,69 +62,14 @@ struct pcie_port {
>  #define PCIE_ATU_FUNC(x)		(((x) & 0x7) << 16)
>  #define PCIE_ATU_UPPER_TARGET		0x91C
>  
> -/* Exynos specific PCIE configuration registers */
> -
> -/* PCIe ELBI registers */
> -#define PCIE_IRQ_PULSE			0x000
> -#define IRQ_INTA_ASSERT			(0x1 << 0)
> -#define IRQ_INTB_ASSERT			(0x1 << 2)
> -#define IRQ_INTC_ASSERT			(0x1 << 4)
> -#define IRQ_INTD_ASSERT			(0x1 << 6)
> -#define PCIE_IRQ_LEVEL			0x004
> -#define PCIE_IRQ_SPECIAL		0x008
> -#define PCIE_IRQ_EN_PULSE		0x00c
> -#define PCIE_IRQ_EN_LEVEL		0x010
> -#define PCIE_IRQ_EN_SPECIAL		0x014
> -#define PCIE_PWR_RESET			0x018
> -#define PCIE_CORE_RESET			0x01c
> -#define PCIE_CORE_RESET_ENABLE		(0x1 << 0)
> -#define PCIE_STICKY_RESET		0x020
> -#define PCIE_NONSTICKY_RESET		0x024
> -#define PCIE_APP_INIT_RESET		0x028
> -#define PCIE_APP_LTSSM_ENABLE		0x02c
> -#define PCIE_ELBI_RDLH_LINKUP		0x064
> -#define PCIE_ELBI_LTSSM_ENABLE		0x1
> -#define PCIE_ELBI_SLV_AWMISC		0x11c
> -#define PCIE_ELBI_SLV_ARMISC		0x120
> -#define PCIE_ELBI_SLV_DBI_ENABLE	(0x1 << 21)
> -
> -/* PCIe Purple registers */
> -#define PCIE_PHY_GLOBAL_RESET		0x000
> -#define PCIE_PHY_COMMON_RESET		0x004
> -#define PCIE_PHY_CMN_REG		0x008
> -#define PCIE_PHY_MAC_RESET		0x00c
> -#define PCIE_PHY_PLL_LOCKED		0x010
> -#define PCIE_PHY_TRSVREG_RESET		0x020
> -#define PCIE_PHY_TRSV_RESET		0x024
> -
> -/* PCIe PHY registers */
> -#define PCIE_PHY_IMPEDANCE		0x004
> -#define PCIE_PHY_PLL_DIV_0		0x008
> -#define PCIE_PHY_PLL_BIAS		0x00c
> -#define PCIE_PHY_DCC_FEEDBACK		0x014
> -#define PCIE_PHY_PLL_DIV_1		0x05c
> -#define PCIE_PHY_TRSV0_EMP_LVL		0x084
> -#define PCIE_PHY_TRSV0_DRV_LVL		0x088
> -#define PCIE_PHY_TRSV0_RXCDR		0x0ac
> -#define PCIE_PHY_TRSV0_LVCC		0x0dc
> -#define PCIE_PHY_TRSV1_EMP_LVL		0x144
> -#define PCIE_PHY_TRSV1_RXCDR		0x16c
> -#define PCIE_PHY_TRSV1_LVCC		0x19c
> -#define PCIE_PHY_TRSV2_EMP_LVL		0x204
> -#define PCIE_PHY_TRSV2_RXCDR		0x22c
> -#define PCIE_PHY_TRSV2_LVCC		0x25c
> -#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
> -#define PCIE_PHY_TRSV3_RXCDR		0x2ec
> -#define PCIE_PHY_TRSV3_LVCC		0x31c
> -
> -static struct hw_pci exynos_pci;
> +unsigned long global_io_offset;
>  
>  static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
>  {
>  	return sys->private_data;
>  }
>  
> -static inline int cfg_read(void *addr, int where, int size, u32 *val)
> +int cfg_read(void *addr, int where, int size, u32 *val)
>  {
>  	*val = readl(addr);
>  
> @@ -184,7 +83,7 @@ static inline int cfg_read(void *addr, int where, int size, u32 *val)
>  	return PCIBIOS_SUCCESSFUL;
>  }
>  
> -static inline int cfg_write(void *addr, int where, int size, u32 val)
> +int cfg_write(void *addr, int where, int size, u32 val)
>  {
>  	if (size == 4)
>  		writel(val, addr);
> @@ -198,155 +97,230 @@ static inline int cfg_write(void *addr, int where, int size, u32 val)
>  	return PCIBIOS_SUCCESSFUL;
>  }
>  
> -static void exynos_pcie_sideband_dbi_w_mode(struct pcie_port *pp, bool on)
> -{
> -	u32 val;
> -
> -	if (on) {
> -		val = readl(pp->elbi_base + PCIE_ELBI_SLV_AWMISC);
> -		val |= PCIE_ELBI_SLV_DBI_ENABLE;
> -		writel(val, pp->elbi_base + PCIE_ELBI_SLV_AWMISC);
> -	} else {
> -		val = readl(pp->elbi_base + PCIE_ELBI_SLV_AWMISC);
> -		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
> -		writel(val, pp->elbi_base + PCIE_ELBI_SLV_AWMISC);
> -	}
> -}
> -
> -static void exynos_pcie_sideband_dbi_r_mode(struct pcie_port *pp, bool on)
> +static inline void dw_pcie_readl_rc(struct pcie_port *pp,
> +				void __iomem *dbi_addr, u32 *val)
>  {
> -	u32 val;
> -
> -	if (on) {
> -		val = readl(pp->elbi_base + PCIE_ELBI_SLV_ARMISC);
> -		val |= PCIE_ELBI_SLV_DBI_ENABLE;
> -		writel(val, pp->elbi_base + PCIE_ELBI_SLV_ARMISC);
> -	} else {
> -		val = readl(pp->elbi_base + PCIE_ELBI_SLV_ARMISC);
> -		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
> -		writel(val, pp->elbi_base + PCIE_ELBI_SLV_ARMISC);
> -	}
> -}
> -
> -static inline void readl_rc(struct pcie_port *pp, void *dbi_base, u32 *val)
> -{
> -	exynos_pcie_sideband_dbi_r_mode(pp, true);
> -	*val = readl(dbi_base);
> -	exynos_pcie_sideband_dbi_r_mode(pp, false);
> -	return;
> +	if (pp->ops->readl_rc)
> +		pp->ops->readl_rc(pp, dbi_addr, val);
> +	else
> +		*val = readl(dbi_addr);
>  }
>  
> -static inline void writel_rc(struct pcie_port *pp, u32 val, void *dbi_base)
> +static inline void dw_pcie_writel_rc(struct pcie_port *pp,
> +				u32 val, void __iomem *dbi_addr)
>  {
> -	exynos_pcie_sideband_dbi_w_mode(pp, true);
> -	writel(val, dbi_base);
> -	exynos_pcie_sideband_dbi_w_mode(pp, false);
> -	return;
> +	if (pp->ops->writel_rc)
> +		pp->ops->writel_rc(pp, val, dbi_addr);
> +	else
> +		writel(val, dbi_addr);
>  }
>  
> -static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> +int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
>  				u32 *val)
>  {
>  	int ret;
>  
> -	exynos_pcie_sideband_dbi_r_mode(pp, true);
> -	ret = cfg_read(pp->dbi_base + (where & ~0x3), where, size, val);
> -	exynos_pcie_sideband_dbi_r_mode(pp, false);
> +	if (pp->ops->rd_own_conf)
> +		ret = pp->ops->rd_own_conf(pp, where, size, val);
> +	else
> +		ret = cfg_read(pp->dbi_base + (where & ~0x3), where, size, val);
> +
>  	return ret;
>  }
>  
> -static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> +int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
>  				u32 val)
>  {
>  	int ret;
>  
> -	exynos_pcie_sideband_dbi_w_mode(pp, true);
> -	ret = cfg_write(pp->dbi_base + (where & ~0x3), where, size, val);
> -	exynos_pcie_sideband_dbi_w_mode(pp, false);
> +	if (pp->ops->wr_own_conf)
> +		ret = pp->ops->wr_own_conf(pp, where, size, val);
> +	else
> +		ret = cfg_write(pp->dbi_base + (where & ~0x3), where, size,
> +				val);
> +
>  	return ret;
>  }
>  
> -static void exynos_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
> +int dw_pcie_link_up(struct pcie_port *pp)
> +{
> +	if (pp->ops->link_up)
> +		return pp->ops->link_up(pp);
> +	else
> +		return 0;
> +}
> +
> +int dw_pcie_host_init(struct pcie_port *pp)
> +{
> +	struct device_node *np = pp->dev->of_node;
> +	struct of_pci_range range;
> +	struct of_pci_range_parser parser;
> +	u32 val;
> +
> +	if (of_pci_range_parser_init(&parser, np)) {
> +		dev_err(pp->dev, "missing ranges property\n");
> +		return -EINVAL;
> +	}

I have some confusion here w.r.t address space :-s
> +
> +	/* Get the I/O and memory ranges from DT */
> +	for_each_of_pci_range(&parser, &range) {
> +		unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
> +		if (restype == IORESOURCE_IO) {
> +			of_pci_range_to_resource(&range, np, &pp->io);
> +			pp->io.name = "I/O";
> +			pp->io.start = max_t(resource_size_t,
> +					     PCIBIOS_MIN_IO,
> +					     range.pci_addr + global_io_offset);
> +			pp->io.end = min_t(resource_size_t,
> +					   IO_SPACE_LIMIT,
> +					   range.pci_addr + range.size
> +					   + global_io_offset);
> +			pp->config.io_size = resource_size(&pp->io);
> +			pp->config.io_bus_addr = range.pci_addr;
> +		}
> +		if (restype == IORESOURCE_MEM) {
> +			of_pci_range_to_resource(&range, np, &pp->mem);
> +			pp->mem.name = "MEM";
> +			pp->config.mem_size = resource_size(&pp->mem);
> +			pp->config.mem_bus_addr = range.pci_addr;
> +		}
> +		if (restype == 0) {
> +			of_pci_range_to_resource(&range, np, &pp->cfg);
> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> +		}
> +	}
> +
> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> +				resource_size(&pp->cfg));

Why is configuraion space divided into two? Why should it be same as dbi_base?
AFAIK, jacinto6 has a dedicated configuration/io/memory space that is entirely
different from dbi_base.

Thanks
Kishon
--
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
Jingoo Han July 23, 2013, 1:14 a.m. UTC | #2
On Tuesday, July 23, 2013 12:04 AM, Kishon Vijay Abraham I wrote:
> On Thursday 18 July 2013 10:51 AM, Jingoo Han wrote:
> > Exynos PCIe IP consists of Synopsys specific part and Exynos
> > specific part. Only core block is a Synopsys designware part;
> > other parts are Exynos specific.
> > Also, the Synopsys designware part can be shared with other
> > platforms; thus, it can be split two parts such as Synopsys
> > designware part and Exynos specific part.
> 
> some more queries and comments..

Hi Kishon,
Thank you for your comments. :)

Hi Pratyush Anand,
Please, answer one question mentioned below. :)

> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > Cc: Pratyush Anand <pratyush.anand@st.com>
> > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > ---
> > Changes since v2:
> .
> .
> <snip>
> .
> .
> > +
> > +static struct pcie_host_ops exynos_pcie_host_ops = {
> > +	.readl_rc = exynos_pcie_readl_rc,
> > +	.writel_rc = exynos_pcie_writel_rc,
> > +	.rd_own_conf = exynos_pcie_rd_own_conf,
> > +	.wr_own_conf = exynos_pcie_wr_own_conf,
> > +	.link_up = exynos_pcie_link_up,
> > +	.host_init = exynos_pcie_host_init,
> > +};
> > +
> > +static int add_pcie_port(struct pcie_port *pp, struct platform_device *pdev)
> > +{
> > +	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
> 
> We can move the exynos_pcie specific initialization to probe and leave only
> pcie_port initialization here.

OK, I will move the exynos_pcie specific initialization to probe
as you mentioned.

> > +	struct resource *elbi_base;
> > +	struct resource *phy_base;
> > +	struct resource *block_base;
> > +	int ret;
> > +
> > +	elbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!elbi_base) {
> > +		dev_err(&pdev->dev, "couldn't get elbi base resource\n");
> > +		return -EINVAL;
> > +	}
> > +	exynos_pcie->elbi_base = devm_ioremap_resource(&pdev->dev, elbi_base);
> > +	if (IS_ERR(exynos_pcie->elbi_base))
> > +		return PTR_ERR(exynos_pcie->elbi_base);
> > +
> > +	phy_base = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > +	if (!phy_base) {
> > +		dev_err(&pdev->dev, "couldn't get phy base resource\n");
> > +		return -EINVAL;
> > +	}
> > +	exynos_pcie->phy_base = devm_ioremap_resource(&pdev->dev, phy_base);
> > +	if (IS_ERR(exynos_pcie->phy_base))
> > +		return PTR_ERR(exynos_pcie->phy_base);
> > +
> > +	block_base = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> > +	if (!block_base) {
> > +		dev_err(&pdev->dev, "couldn't get block base resource\n");
> > +		return -EINVAL;
> > +	}
> > +	exynos_pcie->block_base = devm_ioremap_resource(&pdev->dev, block_base);
> > +	if (IS_ERR(exynos_pcie->block_base))
> > +		return PTR_ERR(exynos_pcie->block_base);
> 
> So all this till here can be moved to probe.

As above mentioned, I will move it to probe.

> > +
> > +	pp->irq = platform_get_irq(pdev, 1);
> > +	if (!pp->irq) {
> > +		dev_err(&pdev->dev, "failed to get irq\n");
> > +		return -ENODEV;
> > +	}
> > +	ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler,
> > +				IRQF_SHARED, "exynos-pcie", pp);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to request irq\n");
> > +		return ret;
> > +	}
> > +
> > +	pp->root_bus_nr = -1;
> > +	pp->ops = &exynos_pcie_host_ops;
> > +
> > +	spin_lock_init(&pp->conf_lock);
> > +	ret = dw_pcie_host_init(pp);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to initialize host\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int __init exynos_pcie_probe(struct platform_device *pdev)
> > +{
> > +	struct exynos_pcie *exynos_pcie;
> > +	struct pcie_port *pp;
> > +	struct device_node *np = pdev->dev.of_node;
> > +	int ret;
> > +
> > +	exynos_pcie = devm_kzalloc(&pdev->dev, sizeof(*exynos_pcie),
> > +				GFP_KERNEL);
> > +	if (!exynos_pcie) {
> > +		dev_err(&pdev->dev, "no memory for exynos pcie\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	pp = &exynos_pcie->pp;
> > +
> > +	pp->dev = &pdev->dev;
> > +
> > +	exynos_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> > +
> > +	exynos_pcie->clk = devm_clk_get(&pdev->dev, "pcie");
> > +	if (IS_ERR(exynos_pcie->clk)) {
> > +		dev_err(&pdev->dev, "Failed to get pcie rc clock\n");
> > +		return PTR_ERR(exynos_pcie->clk);
> > +	}
> > +	ret = clk_prepare_enable(exynos_pcie->clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	exynos_pcie->bus_clk = devm_clk_get(&pdev->dev, "pcie_bus");
> > +	if (IS_ERR(exynos_pcie->bus_clk)) {
> > +		dev_err(&pdev->dev, "Failed to get pcie bus clock\n");
> > +		ret = PTR_ERR(exynos_pcie->bus_clk);
> > +		goto fail_clk;
> > +	}
> > +	ret = clk_prepare_enable(exynos_pcie->bus_clk);
> > +	if (ret)
> > +		goto fail_clk;
> > +
> > +	ret = add_pcie_port(pp, pdev);
> > +	if (ret < 0)
> > +		goto fail_bus_clk;
> 
> I think we should move all the below code to designware core file. IMO it
> should be common everyone who use designware core.

OK, I will move the below code to dw_pcie_host_init() in pcie-designware.c


> > +
> > +	dw_pci.nr_controllers = 1;
> > +	dw_pci.private_data = (void **)&pp;
> > +
> > +	pci_common_init(&dw_pci);
> > +	pci_assign_unassigned_resources();
> > +#ifdef CONFIG_PCI_DOMAINS
> > +	dw_pci.domain++;
> > +#endif
> > +
> > +	platform_set_drvdata(pdev, exynos_pcie);
> > +	return 0;
> > +
> > +fail_bus_clk:
> > +	clk_disable_unprepare(exynos_pcie->bus_clk);
> > +fail_clk:
> > +	clk_disable_unprepare(exynos_pcie->clk);
> > +	return ret;
> > +}
> > +

[.....]

> > +int dw_pcie_host_init(struct pcie_port *pp)
> > +{
> > +	struct device_node *np = pp->dev->of_node;
> > +	struct of_pci_range range;
> > +	struct of_pci_range_parser parser;
> > +	u32 val;
> > +
> > +	if (of_pci_range_parser_init(&parser, np)) {
> > +		dev_err(pp->dev, "missing ranges property\n");
> > +		return -EINVAL;
> > +	}
> 
> I have some confusion here w.r.t address space :-s

This scheme has been confirmed for last 6 months.
Previous mail threads on Marvell PCIe, Tegra PCIe will be helpful
to catch up this.

> > +
> > +	/* Get the I/O and memory ranges from DT */
> > +	for_each_of_pci_range(&parser, &range) {
> > +		unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
> > +		if (restype == IORESOURCE_IO) {
> > +			of_pci_range_to_resource(&range, np, &pp->io);
> > +			pp->io.name = "I/O";
> > +			pp->io.start = max_t(resource_size_t,
> > +					     PCIBIOS_MIN_IO,
> > +					     range.pci_addr + global_io_offset);
> > +			pp->io.end = min_t(resource_size_t,
> > +					   IO_SPACE_LIMIT,
> > +					   range.pci_addr + range.size
> > +					   + global_io_offset);
> > +			pp->config.io_size = resource_size(&pp->io);
> > +			pp->config.io_bus_addr = range.pci_addr;
> > +		}
> > +		if (restype == IORESOURCE_MEM) {
> > +			of_pci_range_to_resource(&range, np, &pp->mem);
> > +			pp->mem.name = "MEM";
> > +			pp->config.mem_size = resource_size(&pp->mem);
> > +			pp->config.mem_bus_addr = range.pci_addr;
> > +		}
> > +		if (restype == 0) {
> > +			of_pci_range_to_resource(&range, np, &pp->cfg);
> > +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
> > +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> > +		}
> > +	}
> > +
> > +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> > +				resource_size(&pp->cfg));
> 
> Why is configuraion space divided into two?

Sorry, I don't know the exact reason. :(
Pratyush Anand may know about this.
Pratyush Anand, could you answer the question?

Also, if you find some problems, please let me know.


> Why should it be same as dbi_base?
> AFAIK, jacinto6 has a dedicated configuration/io/memory space that is entirely
> different from dbi_base.

According to the Synopsys designware PCIe datasheet,
in chapter 5.1.1 Register Space Layout,
'Port Logic Registers' are placed between (config space 0x0 + 0x700)
and (config space 0x0 + 0xFFF).
'dbi_base' is used for reading/writing 'Port Logic Registers'.
Exynos are using 'dbi_base' like this. Thus, the base addresses of
both 'dbi_base' and configuration/io/memory space are same.

Just now, I looked at Spear PCIe driver.
However, in the case of Spear, the base address of configuration/io/memory
space is defined as 0x80000000. The base address of 'Port Logic Registers'
is defined as 0xb1000000.
I think that the situation of 'jacinto6' is similar with Spear, right?

Then, I will move pp->dbi_base mapping code from pcie-designware.c
to pci-exynos.c.


Best regards,
Jingoo Han

--
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
Pratyush ANAND July 23, 2013, 4:49 a.m. UTC | #3
On 7/23/2013 6:44 AM, Jingoo Han wrote:
>>> +           if (restype == IORESOURCE_MEM) {
>>> > >+                   of_pci_range_to_resource(&range, np, &pp->mem);
>>> > >+                   pp->mem.name = "MEM";
>>> > >+                   pp->config.mem_size = resource_size(&pp->mem);
>>> > >+                   pp->config.mem_bus_addr = range.pci_addr;
>>> > >+           }
>>> > >+           if (restype == 0) {
>>> > >+                   of_pci_range_to_resource(&range, np, &pp->cfg);
>>> > >+                   pp->config.cfg0_size = resource_size(&pp->cfg)/2;
>>> > >+                   pp->config.cfg1_size = resource_size(&pp->cfg)/2;
>>> > >+           }
>>> > >+   }
>>> > >+
>>> > >+   pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>>> > >+                           resource_size(&pp->cfg));
>> >
>> >Why is configuraion space divided into two?
> Sorry, I don't know the exact reason.:(
> Pratyush Anand may know about this.
> Pratyush Anand, could you answer the question?
>

CfgRd1 and CfgWr1 transactions are needed when an EP is not directly 
connected to RC, rather connected through a bridge. For more detail see 
PCIe specs.
Now, if we wish to generate a CfgRd/Wr1 transaction using SNPS 
controller then we can not use same programmed viewport as that for 
CfgRd/Wr0. However, a viewport can be reprogrammed using same physical 
cfg address to generate either cfg0 or cfg1. So, in that case we can do 
with only one cfg area and no reason to divide it into two.
Current code assumes that there are only 4 viewports available (2 in 
outbound and 2 in inbound direction) and it always programs viewport 
dynamically for cfg0/1 as per need. But same can not be true for all 
implementations. There can be a case where hardware has sufficient 
number of viewports and software does not need to reprogram it 
dynamically. In that situation different physical memory area for each 
type of TLP would make the execution faster.

> Also, if you find some problems, please let me know.
>
>
>> >Why should it be same as dbi_base?
>> >AFAIK, jacinto6 has a dedicated configuration/io/memory space that is entirely
>> >different from dbi_base.
> According to the Synopsys designware PCIe datasheet,
> in chapter 5.1.1 Register Space Layout,
> 'Port Logic Registers' are placed between (config space 0x0 + 0x700)
> and (config space 0x0 + 0xFFF).
> 'dbi_base' is used for reading/writing 'Port Logic Registers'.
> Exynos are using 'dbi_base' like this. Thus, the base addresses of
> both 'dbi_base' and configuration/io/memory space are same.

Yes address space are same, but at any moment they work either as dbi 
space or as configuration/io/memory space depending on the dbi_enable 
bit of application register. Similar functionality was there in one 
older SPEAr implementation.

>
> Just now, I looked at Spear PCIe driver.
> However, in the case of Spear, the base address of configuration/io/memory
> space is defined as 0x80000000. The base address of 'Port Logic Registers'
> is defined as 0xb1000000.
> I think that the situation of 'jacinto6' is similar with Spear, right?

In SPEAr1340 PCIe memory layout is as follows:

DBI base (0xB1000000 to 0xB1001FFF): This space is used to read/write 
own PCI Configuration Header, Capability and Port Logic(PL) Registers.

ELBI space (0xB1002000 to  0xB17FFFFF): These are completely SPEAr 
specific application registers.

configuration/io/memory space(0x80000000 to 0x8FFFFFFF): These can be 
used in viewport programming to generate different type of outbound 
transaction.

Regards
Pratyush


>
> Then, I will move pp->dbi_base mapping code from pcie-designware.c
> to pci-exynos.c.


--
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
Kishon Vijay Abraham I July 23, 2013, 6:29 a.m. UTC | #4
Hi,

On Tuesday 23 July 2013 06:44 AM, Jingoo Han wrote:
> On Tuesday, July 23, 2013 12:04 AM, Kishon Vijay Abraham I wrote:
>> On Thursday 18 July 2013 10:51 AM, Jingoo Han wrote:
>>> Exynos PCIe IP consists of Synopsys specific part and Exynos
>>> specific part. Only core block is a Synopsys designware part;
>>> other parts are Exynos specific.
>>> Also, the Synopsys designware part can be shared with other
>>> platforms; thus, it can be split two parts such as Synopsys
>>> designware part and Exynos specific part.
>>
>> some more queries and comments..
> 
.
.
<snip>
.
.
>>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
>>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
>>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
>>> +		}
>>> +	}
>>> +
>>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>>> +				resource_size(&pp->cfg));
>>
>> Why is configuraion space divided into two?
> 
> Sorry, I don't know the exact reason. :(
> Pratyush Anand may know about this.
> Pratyush Anand, could you answer the question?
> 
> Also, if you find some problems, please let me know.
> 
> 
>> Why should it be same as dbi_base?
>> AFAIK, jacinto6 has a dedicated configuration/io/memory space that is entirely
>> different from dbi_base.
> 
> According to the Synopsys designware PCIe datasheet,
> in chapter 5.1.1 Register Space Layout,
> 'Port Logic Registers' are placed between (config space 0x0 + 0x700)
> and (config space 0x0 + 0xFFF).
> 'dbi_base' is used for reading/writing 'Port Logic Registers'.
> Exynos are using 'dbi_base' like this. Thus, the base addresses of
> both 'dbi_base' and configuration/io/memory space are same.
> 
> Just now, I looked at Spear PCIe driver.
> However, in the case of Spear, the base address of configuration/io/memory
> space is defined as 0x80000000. The base address of 'Port Logic Registers'
> is defined as 0xb1000000.
> I think that the situation of 'jacinto6' is similar with Spear, right?
> 
> Then, I will move pp->dbi_base mapping code from pcie-designware.c
> to pci-exynos.c.

I think you need not move this to exynos (since registers in dbi_base is common
for all platforms) but modify the pcie-designware.c to use different address
space for dbi_base. In your case, you'll duplicate the address space for
dbi_base and configuration space.

Also I have one more query.
In your dt binding, your pci address and cpu address is the same. But the pci
address should start at 0x00000000 and end at 0xffffffff (for 32bit). Shouldn't
the cpu address map to something within this range of pci address?

Thanks
Kishon
--
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
Jingoo Han July 23, 2013, 7 a.m. UTC | #5
On Tuesday, July 23, 2013 3:30 PM, Kishon Vijay Abraham I wrote:
> On Tuesday 23 July 2013 06:44 AM, Jingoo Han wrote:
> > On Tuesday, July 23, 2013 12:04 AM, Kishon Vijay Abraham I wrote:
> >> On Thursday 18 July 2013 10:51 AM, Jingoo Han wrote:
> >>> Exynos PCIe IP consists of Synopsys specific part and Exynos
> >>> specific part. Only core block is a Synopsys designware part;
> >>> other parts are Exynos specific.
> >>> Also, the Synopsys designware part can be shared with other
> >>> platforms; thus, it can be split two parts such as Synopsys
> >>> designware part and Exynos specific part.
> >>
> >> some more queries and comments..
> >
> .
> .
> <snip>
> .
> .
> >>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
> >>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
> >>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> >>> +		}
> >>> +	}
> >>> +
> >>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> >>> +				resource_size(&pp->cfg));
> >>
> >> Why is configuraion space divided into two?
> >
> > Sorry, I don't know the exact reason. :(
> > Pratyush Anand may know about this.
> > Pratyush Anand, could you answer the question?
> >
> > Also, if you find some problems, please let me know.
> >
> >
> >> Why should it be same as dbi_base?
> >> AFAIK, jacinto6 has a dedicated configuration/io/memory space that is entirely
> >> different from dbi_base.
> >
> > According to the Synopsys designware PCIe datasheet,
> > in chapter 5.1.1 Register Space Layout,
> > 'Port Logic Registers' are placed between (config space 0x0 + 0x700)
> > and (config space 0x0 + 0xFFF).
> > 'dbi_base' is used for reading/writing 'Port Logic Registers'.
> > Exynos are using 'dbi_base' like this. Thus, the base addresses of
> > both 'dbi_base' and configuration/io/memory space are same.
> >
> > Just now, I looked at Spear PCIe driver.
> > However, in the case of Spear, the base address of configuration/io/memory
> > space is defined as 0x80000000. The base address of 'Port Logic Registers'
> > is defined as 0xb1000000.
> > I think that the situation of 'jacinto6' is similar with Spear, right?
> >
> > Then, I will move pp->dbi_base mapping code from pcie-designware.c
> > to pci-exynos.c.
> 
> I think you need not move this to exynos (since registers in dbi_base is common
> for all platforms) but modify the pcie-designware.c to use different address
> space for dbi_base. In your case, you'll duplicate the address space for
> dbi_base and configuration space.

I cannot understand fully what you said.
Please, give a pseudo code.

> 
> Also I have one more query.
> In your dt binding, your pci address and cpu address is the same. But the pci
> address should start at 0x00000000 and end at 0xffffffff (for 32bit). Shouldn't
> the cpu address map to something within this range of pci address?
> 

Sorry, I cannot answer it exactly.
DT binding was confirmed by Arnd Bergmann.
He will answer it exactly.

Best regards,
Jingoo Han


--
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
Jingoo Han July 23, 2013, 8:42 a.m. UTC | #6
On Tuesday, July 23, 2013 3:30 PM, Kishon Vijay Abraham I wrote:
> On Tuesday 23 July 2013 06:44 AM, Jingoo Han wrote:
> > On Tuesday, July 23, 2013 12:04 AM, Kishon Vijay Abraham I wrote:
> >> On Thursday 18 July 2013 10:51 AM, Jingoo Han wrote:
> >>> Exynos PCIe IP consists of Synopsys specific part and Exynos
> >>> specific part. Only core block is a Synopsys designware part;
> >>> other parts are Exynos specific.
> >>> Also, the Synopsys designware part can be shared with other
> >>> platforms; thus, it can be split two parts such as Synopsys
> >>> designware part and Exynos specific part.
> >>
> >> some more queries and comments..
> >
> .
> .
> <snip>
> .
> .
> >>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
> >>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
> >>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> >>> +		}
> >>> +	}
> >>> +
> >>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> >>> +				resource_size(&pp->cfg));

[.....]

> >
> >> Why should it be same as dbi_base?
> >> AFAIK, jacinto6 has a dedicated configuration/io/memory space that is entirely
> >> different from dbi_base.
> >
> > According to the Synopsys designware PCIe datasheet,
> > in chapter 5.1.1 Register Space Layout,
> > 'Port Logic Registers' are placed between (config space 0x0 + 0x700)
> > and (config space 0x0 + 0xFFF).
> > 'dbi_base' is used for reading/writing 'Port Logic Registers'.
> > Exynos are using 'dbi_base' like this. Thus, the base addresses of
> > both 'dbi_base' and configuration/io/memory space are same.
> >
> > Just now, I looked at Spear PCIe driver.
> > However, in the case of Spear, the base address of configuration/io/memory
> > space is defined as 0x80000000. The base address of 'Port Logic Registers'
> > is defined as 0xb1000000.
> > I think that the situation of 'jacinto6' is similar with Spear, right?
> >
> > Then, I will move pp->dbi_base mapping code from pcie-designware.c
> > to pci-exynos.c.
> 
> I think you need not move this to exynos (since registers in dbi_base is common
> for all platforms) but modify the pcie-designware.c to use different address
> space for dbi_base. In your case, you'll duplicate the address space for
> dbi_base and configuration space.
> 

I will map 'pp->dbi_base' as below:
Also, I referenced SPEAr PCIe patches made by Pratyush Anand.
(http://permalink.gmane.org/gmane.linux.kernel.pci/18400)
(http://permalink.gmane.org/gmane.linux.kernel.pci/18403)


1. The different addresses between dbi_base and configuration space
    : SPEAr PCIe, OMAP PCIe
      'pp->dbi_base' value is come from DT binding, then, 'pp->dbi_base' will
       be mapped in spear_pcie_probe() in pci-spear.c

(arch/arm/boot/dts/spear13xx.dtsi)
		pcie0@b1000000 {
			reg = <0xb1000000 0x2000      <-- dbi register
				 0xb1002000 0x7fdfff   <-- elbi register

(drivers/pci/host/pci-spear.c)
static int __init spear_pcie_probe(struct platform_device *pdev)
{
	struct resource *dbi_base;
	struct resource *elbi_base;

	dbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	pp->dbi_base = devm_ioremap_resource(&pdev->dev, dbi_base);

	elbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 1);
	spear_pcie->elbi_base = devm_ioremap_resource(&pdev->dev, elbi_base);



2. The same addresses between dbi_base and configuration space
    : Exynos PCIe
      'pp->dbi_base' will be mapped in dw_pcie_host_init() of pcie-designware.c.

(drivers/pci/host/pcie-exynos.c)
static int __init exynos_pcie_probe(struct platform_device *pdev)
{
	struct resource *elbi_base;

	elbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	exynos_pcie->elbi_base = devm_ioremap_resource(&pdev->dev, elbi_base);

(drivers/pci/host/pcie-designware.c)
int dw_pcie_host_init(struct pcie_port *pp)
{
	if (!pp->dbi_base) {
		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
						resource_size(&pp->cfg));
		if (!pp->dbi_base) {
			dev_err(pp->dev, "error with ioremap\n");
			return -ENOMEM;
		}
	}


Best regards,
Jingoo Han


--
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
Arnd Bergmann July 24, 2013, 9:02 p.m. UTC | #7
On Tuesday 23 July 2013, Jingoo Han wrote:
> > 
> > Also I have one more query.
> > In your dt binding, your pci address and cpu address is the same. But the pci
> > address should start at 0x00000000 and end at 0xffffffff (for 32bit). Shouldn't
> > the cpu address map to something within this range of pci address?

The size is limited by the window available (e.g. 0x80000000-0x8FFFFFFF), it can
never be the full 4GB on a 32 bit non-LPAE system. If you only care about memory
space here (in practice you want at least also config space) that means you could
either have an identity map

	<0x82000000 0 0x80000000 0x80000000 0 0x1fffffff>;

or use bus address 0

	<0x82000000 0 0 0x80000000 0 0x1fffffff>;

but the length is always limited by the upstream bus.

> Sorry, I cannot answer it exactly.
> DT binding was confirmed by Arnd Bergmann.
> He will answer it exactly.

Normally you want the pci and cpu addresses to be the same, i.e. identity mapped.
This simplifies PCI bus master DMA as it ensures that there is no aliasing between
PCI memory space and RAM addresses visible to the host.

If you know that there is never any RAM at CPU address 0, you can also make the
PCI memory space be mapped from bus address 0, which has the advantage of allowing
access to low PCI addresses, e.g. for legacy VGA output using the 0xa0000-0xbffff
range, but it's less common. In particular on x86 there is always an identity
mapping.

The driver should be able to handle any mapping that can be described by the binding
and is physically possible to be programmed into the translation windows.

	Arnd
--
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
Kishon Vijay Abraham I Sept. 12, 2013, 6:34 a.m. UTC | #8
Hi Jingoo,

On Tuesday 23 July 2013 12:30 PM, Jingoo Han wrote:
> On Tuesday, July 23, 2013 3:30 PM, Kishon Vijay Abraham I wrote:
>> On Tuesday 23 July 2013 06:44 AM, Jingoo Han wrote:
>>> On Tuesday, July 23, 2013 12:04 AM, Kishon Vijay Abraham I wrote:
>>>> On Thursday 18 July 2013 10:51 AM, Jingoo Han wrote:
>>>>> Exynos PCIe IP consists of Synopsys specific part and Exynos
>>>>> specific part. Only core block is a Synopsys designware part;
>>>>> other parts are Exynos specific.
>>>>> Also, the Synopsys designware part can be shared with other
>>>>> platforms; thus, it can be split two parts such as Synopsys
>>>>> designware part and Exynos specific part.
>>>>
>>>> some more queries and comments..
>>>
>> .
>> .
>> <snip>
>> .
>> .
>>>>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
>>>>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
>>>>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
>>>>> +		}
>>>>> +	}
>>>>> +
>>>>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>>>>> +				resource_size(&pp->cfg));
>>>>
>>>> Why is configuraion space divided into two?
>>>
>>> Sorry, I don't know the exact reason. :(
>>> Pratyush Anand may know about this.
>>> Pratyush Anand, could you answer the question?
>>>
>>> Also, if you find some problems, please let me know.

One more query..

Where is inbound translation configuration done in your driver? how should it
be done?

Thanks
Kishon
--
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
Jingoo Han Sept. 12, 2013, 7:15 a.m. UTC | #9
On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
> >> .
> >> .
> >>>>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
> >>>>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
> >>>>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> >>>>> +		}
> >>>>> +	}
> >>>>> +
> >>>>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> >>>>> +				resource_size(&pp->cfg));
> >>>>
> >>>> Why is configuraion space divided into two?
> >>>
> >>> Sorry, I don't know the exact reason. :(
> >>> Pratyush Anand may know about this.
> >>> Pratyush Anand, could you answer the question?
> >>>
> >>> Also, if you find some problems, please let me know.
> 
> One more query..
> 
> Where is inbound translation configuration done in your driver? how should it
> be done?

Hi Kishon,

Sorry, I cannot understand your question exactly.
However, the following thread would be helpful.

http://www.spinics.net/lists/arm-kernel/msg252078.html
https://lkml.org/lkml/2013/6/17/890

Best regards.
Jingoo Han

--
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
Pratyush ANAND Sept. 12, 2013, 9:30 a.m. UTC | #10
Hi Jingoo,


On Thu, Sep 12, 2013 at 03:15:04PM +0800, Jingoo Han wrote:
> On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
> > >> .
> > >> .
> > >>>>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
> > >>>>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
> > >>>>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> > >>>>> +		}
> > >>>>> +	}
> > >>>>> +
> > >>>>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> > >>>>> +				resource_size(&pp->cfg));
> > >>>>
> > >>>> Why is configuraion space divided into two?
> > >>>
> > >>> Sorry, I don't know the exact reason. :(
> > >>> Pratyush Anand may know about this.
> > >>> Pratyush Anand, could you answer the question?
> > >>>
> > >>> Also, if you find some problems, please let me know.
> > 
> > One more query..
> > 
> > Where is inbound translation configuration done in your driver? how should it
> > be done?
> 

Yes, Kishon is right. Inbound translation configuration is missing in
your code and I think it should be implemented.

> Hi Kishon,
> 
> Sorry, I cannot understand your question exactly.
> However, the following thread would be helpful.
> 
> http://www.spinics.net/lists/arm-kernel/msg252078.html
> https://lkml.org/lkml/2013/6/17/890

From this conversation, It seems that you
have tested this driver and it works fine without inbound translation
function. I am sure that you would have tested a PCIe card with DMA
capability such as PCIe2USB or PCIe2Ethernet. Since it worked, it
means that by default your controller is supporting one to one mapping
in case of inbound transaction even when address translation is enabled.

In my opinion you should call a function like as follows from
dw_pcie_host_init in pcie-designware.c.  It will insure one to one
mapping for any inbound request in memory range 0 to (in_mem_size -
1) for all dw implementation.

static void dw_pcie_prog_viewport_mem_inbound(struct pcie_port *pp)
{        
	u32 val;
	void __iomem *dbi_base = pp->dbi_base;

	/* Program viewport 0 : INBOUND : MEMORY*/
	val = PCIE_ATU_REGION_INBOUND | (0 & 0xF);
	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT));
	dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, dbi_base + PCIE_ATU_CR1));
	val = PCIE_ATU_ENABLE | PCIE_ATU_BAR_MODE_ENABLE;
	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_CR2));
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_LOWER_BASE));
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_BASE));
	/* in_mem_size must be in power of 2 */
	dw_pcie_writel_rc(pp, pp->config.in_mem_size - 1, dbi_base + PCIE_ATU_LIMIT));
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_LOWER_TARGET));
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET));
}

Regards
Pratyush

> 
> Best regards.
> Jingoo Han
--
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
Kishon Vijay Abraham I Sept. 12, 2013, 9:43 a.m. UTC | #11
Hi,

On Thursday 12 September 2013 03:00 PM, Pratyush Anand wrote:
> Hi Jingoo,
> 
> 
> On Thu, Sep 12, 2013 at 03:15:04PM +0800, Jingoo Han wrote:
>> On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
>>>>> .
>>>>> .
>>>>>>>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
>>>>>>>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
>>>>>>>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
>>>>>>>> +		}
>>>>>>>> +	}
>>>>>>>> +
>>>>>>>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>>>>>>>> +				resource_size(&pp->cfg));
>>>>>>>
>>>>>>> Why is configuraion space divided into two?
>>>>>>
>>>>>> Sorry, I don't know the exact reason. :(
>>>>>> Pratyush Anand may know about this.
>>>>>> Pratyush Anand, could you answer the question?
>>>>>>
>>>>>> Also, if you find some problems, please let me know.
>>>
>>> One more query..
>>>
>>> Where is inbound translation configuration done in your driver? how should it
>>> be done?
>>
> 
> Yes, Kishon is right. Inbound translation configuration is missing in
> your code and I think it should be implemented.
> 
>> Hi Kishon,
>>
>> Sorry, I cannot understand your question exactly.
>> However, the following thread would be helpful.
>>
>> http://www.spinics.net/lists/arm-kernel/msg252078.html
>> https://lkml.org/lkml/2013/6/17/890
> 
> From this conversation, It seems that you
> have tested this driver and it works fine without inbound translation
> function. I am sure that you would have tested a PCIe card with DMA
> capability such as PCIe2USB or PCIe2Ethernet. Since it worked, it
> means that by default your controller is supporting one to one mapping
> in case of inbound transaction even when address translation is enabled.

btw, I'm testing Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8111/8168B PCI Express Gigabit Ethernet controller.

when I do ifconfig eth0 up, I get *r8169 0000:01:00.0 eth0: link up.*
But I dont receive any packets and ping also fails and the tx and rx packet
count is also 0. Could it be related to inbound translation?

Thanks
Kishon
--
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
Pratyush ANAND Sept. 12, 2013, 9:52 a.m. UTC | #12
Hi Kishon,

On Thu, Sep 12, 2013 at 05:43:40PM +0800, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 12 September 2013 03:00 PM, Pratyush Anand wrote:
> > Hi Jingoo,
> > 
> > 
> > On Thu, Sep 12, 2013 at 03:15:04PM +0800, Jingoo Han wrote:
> >> On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
> >>>>> .
> >>>>> .
> >>>>>>>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
> >>>>>>>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
> >>>>>>>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> >>>>>>>> +		}
> >>>>>>>> +	}
> >>>>>>>> +
> >>>>>>>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> >>>>>>>> +				resource_size(&pp->cfg));
> >>>>>>>
> >>>>>>> Why is configuraion space divided into two?
> >>>>>>
> >>>>>> Sorry, I don't know the exact reason. :(
> >>>>>> Pratyush Anand may know about this.
> >>>>>> Pratyush Anand, could you answer the question?
> >>>>>>
> >>>>>> Also, if you find some problems, please let me know.
> >>>
> >>> One more query..
> >>>
> >>> Where is inbound translation configuration done in your driver? how should it
> >>> be done?
> >>
> > 
> > Yes, Kishon is right. Inbound translation configuration is missing in
> > your code and I think it should be implemented.
> > 
> >> Hi Kishon,
> >>
> >> Sorry, I cannot understand your question exactly.
> >> However, the following thread would be helpful.
> >>
> >> http://www.spinics.net/lists/arm-kernel/msg252078.html
> >> https://lkml.org/lkml/2013/6/17/890
> > 
> > From this conversation, It seems that you
> > have tested this driver and it works fine without inbound translation
> > function. I am sure that you would have tested a PCIe card with DMA
> > capability such as PCIe2USB or PCIe2Ethernet. Since it worked, it
> > means that by default your controller is supporting one to one mapping
> > in case of inbound transaction even when address translation is enabled.
> 
> btw, I'm testing Ethernet controller: Realtek Semiconductor Co., Ltd.
> RTL8111/8168B PCI Express Gigabit Ethernet controller.
> 
> when I do ifconfig eth0 up, I get *r8169 0000:01:00.0 eth0: link up.*
> But I dont receive any packets and ping also fails and the tx and rx packet
> count is also 0. Could it be related to inbound translation?

A PCIe analyser log would tell a definite cause. Most likely either
inbound translation is not working or INTx/MSI is not working.

Regards
Pratyush

> 
> Thanks
> Kishon
--
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
Kishon Vijay Abraham I Sept. 12, 2013, 10:07 a.m. UTC | #13
Hi,

On Thursday 12 September 2013 03:22 PM, Pratyush Anand wrote:
> Hi Kishon,
> 
> On Thu, Sep 12, 2013 at 05:43:40PM +0800, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 12 September 2013 03:00 PM, Pratyush Anand wrote:
>>> Hi Jingoo,
>>>
>>>
>>> On Thu, Sep 12, 2013 at 03:15:04PM +0800, Jingoo Han wrote:
>>>> On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
>>>>>>> .
>>>>>>> .
>>>>>>>>>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
>>>>>>>>>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
>>>>>>>>>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
>>>>>>>>>> +		}
>>>>>>>>>> +	}
>>>>>>>>>> +
>>>>>>>>>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>>>>>>>>>> +				resource_size(&pp->cfg));
>>>>>>>>>
>>>>>>>>> Why is configuraion space divided into two?
>>>>>>>>
>>>>>>>> Sorry, I don't know the exact reason. :(
>>>>>>>> Pratyush Anand may know about this.
>>>>>>>> Pratyush Anand, could you answer the question?
>>>>>>>>
>>>>>>>> Also, if you find some problems, please let me know.
>>>>>
>>>>> One more query..
>>>>>
>>>>> Where is inbound translation configuration done in your driver? how should it
>>>>> be done?
>>>>
>>>
>>> Yes, Kishon is right. Inbound translation configuration is missing in
>>> your code and I think it should be implemented.
>>>
>>>> Hi Kishon,
>>>>
>>>> Sorry, I cannot understand your question exactly.
>>>> However, the following thread would be helpful.
>>>>
>>>> http://www.spinics.net/lists/arm-kernel/msg252078.html
>>>> https://lkml.org/lkml/2013/6/17/890
>>>
>>> From this conversation, It seems that you
>>> have tested this driver and it works fine without inbound translation
>>> function. I am sure that you would have tested a PCIe card with DMA
>>> capability such as PCIe2USB or PCIe2Ethernet. Since it worked, it
>>> means that by default your controller is supporting one to one mapping
>>> in case of inbound transaction even when address translation is enabled.
>>
>> btw, I'm testing Ethernet controller: Realtek Semiconductor Co., Ltd.
>> RTL8111/8168B PCI Express Gigabit Ethernet controller.
>>
>> when I do ifconfig eth0 up, I get *r8169 0000:01:00.0 eth0: link up.*
>> But I dont receive any packets and ping also fails and the tx and rx packet
>> count is also 0. Could it be related to inbound translation?
> 
> A PCIe analyser log would tell a definite cause. Most likely either
> inbound translation is not working or INTx/MSI is not working.

I have enabled only legacy interrupts. Whenever I connect or disconnect
ethernet cable I get link up/link down message and also the interrupt count for
eth0 increases. So I'm not doubting INTx interrupts as such.

btw configuring inbound translation once in dw_pcie_host_init enough is it? I
mean we use the same registers for configuring outbound translation also no? So
doesn't the inbound configuration gets lost?

Thanks
Kishon
--
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
Pratyush ANAND Sept. 12, 2013, 10:18 a.m. UTC | #14
On Thu, Sep 12, 2013 at 06:07:23PM +0800, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 12 September 2013 03:22 PM, Pratyush Anand wrote:
> > Hi Kishon,
> > 
> > On Thu, Sep 12, 2013 at 05:43:40PM +0800, Kishon Vijay Abraham I wrote:
> >> Hi,
> >>
> >> On Thursday 12 September 2013 03:00 PM, Pratyush Anand wrote:
> >>> Hi Jingoo,
> >>>
> >>>
> >>> On Thu, Sep 12, 2013 at 03:15:04PM +0800, Jingoo Han wrote:
> >>>> On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
> >>>>>>> .
> >>>>>>> .

[...]

> >> when I do ifconfig eth0 up, I get *r8169 0000:01:00.0 eth0: link up.*
> >> But I dont receive any packets and ping also fails and the tx and rx packet
> >> count is also 0. Could it be related to inbound translation?
> > 
> > A PCIe analyser log would tell a definite cause. Most likely either
> > inbound translation is not working or INTx/MSI is not working.
> 
> I have enabled only legacy interrupts. Whenever I connect or disconnect
> ethernet cable I get link up/link down message and also the interrupt count for
> eth0 increases. So I'm not doubting INTx interrupts as such.
> 
> btw configuring inbound translation once in dw_pcie_host_init enough is it? I
> mean we use the same registers for configuring outbound translation also no? So
> doesn't the inbound configuration gets lost?

No, you write at the same register, but you program direction as
inbound. There are different resources for each viewport in inbound
and outbound direction.

Regards
Pratyush

> 
> Thanks
> Kishon
--
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
Jingoo Han Sept. 12, 2013, 10:25 a.m. UTC | #15
On Thursday, September 12, 2013 6:44 PM, Kishon Vijay Abraham I wrote:
> On Thursday 12 September 2013 03:00 PM, Pratyush Anand wrote:
> >
> > From this conversation, It seems that you
> > have tested this driver and it works fine without inbound translation
> > function. I am sure that you would have tested a PCIe card with DMA
> > capability such as PCIe2USB or PCIe2Ethernet. Since it worked, it
> > means that by default your controller is supporting one to one mapping
> > in case of inbound transaction even when address translation is enabled.
> 
> btw, I'm testing Ethernet controller: Realtek Semiconductor Co., Ltd.
> RTL8111/8168B PCI Express Gigabit Ethernet controller.
> 
> when I do ifconfig eth0 up, I get *r8169 0000:01:00.0 eth0: link up.*
> But I dont receive any packets and ping also fails and the tx and rx packet
> count is also 0. Could it be related to inbound translation?
> 

Hi Kishon,

I have tested Ethernet controller: Intel Corporation 82574L PCI Express
Gigabit Ethernet controller.

Without inbound translation, it works properly with both legacy interrupt
mode and MSI mode.

Best regards,
Jingoo Han


--
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
Pratyush ANAND Sept. 12, 2013, 10:46 a.m. UTC | #16
On Thu, Sep 12, 2013 at 03:48:03PM +0530, Pratyush Anand wrote:
> On Thu, Sep 12, 2013 at 06:07:23PM +0800, Kishon Vijay Abraham I wrote:
> > Hi,
> > 
> > On Thursday 12 September 2013 03:22 PM, Pratyush Anand wrote:
> > > Hi Kishon,
> > > 
> > > On Thu, Sep 12, 2013 at 05:43:40PM +0800, Kishon Vijay Abraham I wrote:
> > >> Hi,
> > >>
> > >> On Thursday 12 September 2013 03:00 PM, Pratyush Anand wrote:
> > >>> Hi Jingoo,
> > >>>
> > >>>
> > >>> On Thu, Sep 12, 2013 at 03:15:04PM +0800, Jingoo Han wrote:
> > >>>> On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
> > >>>>>>> .
> > >>>>>>> .
> 
> [...]
> 
> > >> when I do ifconfig eth0 up, I get *r8169 0000:01:00.0 eth0: link up.*
> > >> But I dont receive any packets and ping also fails and the tx and rx packet
> > >> count is also 0. Could it be related to inbound translation?
> > > 
> > > A PCIe analyser log would tell a definite cause. Most likely either
> > > inbound translation is not working or INTx/MSI is not working.
> > 
> > I have enabled only legacy interrupts. Whenever I connect or disconnect
> > ethernet cable I get link up/link down message and also the interrupt count for
> > eth0 increases. So I'm not doubting INTx interrupts as such.

Just a question, what is the MRRS of your RC? if it is 128, can you
try with passing pci=pcie_bus_peer2peer  in your bootargs.

Again, analyser will help a lot in diagnosing such issues.

Regards
Pratyush
> > 
> > btw configuring inbound translation once in dw_pcie_host_init enough is it? I
> > mean we use the same registers for configuring outbound translation also no? So
> > doesn't the inbound configuration gets lost?
> 
> No, you write at the same register, but you program direction as
> inbound. There are different resources for each viewport in inbound
> and outbound direction.
> 
> Regards
> Pratyush
> 
> > 
> > Thanks
> > Kishon
--
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
Kishon Vijay Abraham I Sept. 12, 2013, 11:14 a.m. UTC | #17
On Thursday 12 September 2013 04:16 PM, Pratyush Anand wrote:
> On Thu, Sep 12, 2013 at 03:48:03PM +0530, Pratyush Anand wrote:
>> On Thu, Sep 12, 2013 at 06:07:23PM +0800, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Thursday 12 September 2013 03:22 PM, Pratyush Anand wrote:
>>>> Hi Kishon,
>>>>
>>>> On Thu, Sep 12, 2013 at 05:43:40PM +0800, Kishon Vijay Abraham I wrote:
>>>>> Hi,
>>>>>
>>>>> On Thursday 12 September 2013 03:00 PM, Pratyush Anand wrote:
>>>>>> Hi Jingoo,
>>>>>>
>>>>>>
>>>>>> On Thu, Sep 12, 2013 at 03:15:04PM +0800, Jingoo Han wrote:
>>>>>>> On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
>>>>>>>>>> .
>>>>>>>>>> .
>>
>> [...]
>>
>>>>> when I do ifconfig eth0 up, I get *r8169 0000:01:00.0 eth0: link up.*
>>>>> But I dont receive any packets and ping also fails and the tx and rx packet
>>>>> count is also 0. Could it be related to inbound translation?
>>>>
>>>> A PCIe analyser log would tell a definite cause. Most likely either
>>>> inbound translation is not working or INTx/MSI is not working.
>>>
>>> I have enabled only legacy interrupts. Whenever I connect or disconnect
>>> ethernet cable I get link up/link down message and also the interrupt count for
>>> eth0 increases. So I'm not doubting INTx interrupts as such.
> 
> Just a question, what is the MRRS of your RC? if it is 128, can you

it's 512.
> try with passing pci=pcie_bus_peer2peer  in your bootargs.
> 
> Again, analyser will help a lot in diagnosing such issues.

dont have a analyser here :-(

Thanks
Kishon
--
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
Kishon Vijay Abraham I Sept. 21, 2013, 2:56 p.m. UTC | #18
Hi,

On Thursday 12 September 2013 04:16 PM, Pratyush Anand wrote:
> On Thu, Sep 12, 2013 at 03:48:03PM +0530, Pratyush Anand wrote:
>> On Thu, Sep 12, 2013 at 06:07:23PM +0800, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Thursday 12 September 2013 03:22 PM, Pratyush Anand wrote:
>>>> Hi Kishon,
>>>>
>>>> On Thu, Sep 12, 2013 at 05:43:40PM +0800, Kishon Vijay Abraham I wrote:
>>>>> Hi,
>>>>>
>>>>> On Thursday 12 September 2013 03:00 PM, Pratyush Anand wrote:
>>>>>> Hi Jingoo,
>>>>>>
>>>>>>
>>>>>> On Thu, Sep 12, 2013 at 03:15:04PM +0800, Jingoo Han wrote:
>>>>>>> On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
>>>>>>>>>> .
>>>>>>>>>> .
>>
>> [...]
>>
>>>>> when I do ifconfig eth0 up, I get *r8169 0000:01:00.0 eth0: link up.*
>>>>> But I dont receive any packets and ping also fails and the tx and rx packet
>>>>> count is also 0. Could it be related to inbound translation?
>>>>
>>>> A PCIe analyser log would tell a definite cause. Most likely either
>>>> inbound translation is not working or INTx/MSI is not working.
>>>
>>> I have enabled only legacy interrupts. Whenever I connect or disconnect
>>> ethernet cable I get link up/link down message and also the interrupt count for
>>> eth0 increases. So I'm not doubting INTx interrupts as such.
> 
> Just a question, what is the MRRS of your RC? if it is 128, can you
> try with passing pci=pcie_bus_peer2peer  in your bootargs.
> 
> Again, analyser will help a lot in diagnosing such issues.
> 
> Regards
> Pratyush
>>>
>>> btw configuring inbound translation once in dw_pcie_host_init enough is it? I
>>> mean we use the same registers for configuring outbound translation also no? So
>>> doesn't the inbound configuration gets lost?
>>
>> No, you write at the same register, but you program direction as
>> inbound. There are different resources for each viewport in inbound
>> and outbound direction.

I did inbound translation programming like
static void dw_pcie_prog_viewport_mem_inbound(struct pcie_port *pp)
{
	u32 val;
	void __iomem *val1;
	void __iomem *dbi_base = pp->dbi_base;

	/* Program viewport 0 : INBOUND : MEMORY*/
	val = PCIE_ATU_REGION_INBOUND | (0 & 0xF);
	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
	val1 = ioremap(0x80000000, 0x5fffffff);
	dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_BASE);
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_BASE);
	/* in_mem_size must be in power of 2 */
	dw_pcie_writel_rc(pp, 0x5FFFFFFF, dbi_base + PCIE_ATU_LIMIT);
	dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_TARGET);
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);
	dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, dbi_base + PCIE_ATU_CR1);
	val = PCIE_ATU_ENABLE;
	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_CR2);
}

I'm using Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express
Gigabit Ethernet controller (TP-LINK TG-3468) (it uses r8169 driver).

whenever I do a ifconfig ethp up, I get a non fatal error interrupt (after
Transmitter/Receiver Enable in r8169 driver).

I somehow starting to doubt the DMA address programmed in the ethernet card
which is in my RAM address range (0x80000000 to 0xBFFFFFFF). Should this
address be programmed in the BAR of the ethernet card? How should it be done?

My pcie dt data looks like this.
ranges = <0x00000800 0 0x20000000 0x20000000 0 0x00001000 /* configuration space */
	  0x81000000 0 0	 0x20001000 0 0x00010000 /* io */
	  0x82000000 0 0x20011000 0x20011000 0 0x0ffef000>; /* memory */

Thanks
Kishon
--
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
Arnd Bergmann Sept. 21, 2013, 10:03 p.m. UTC | #19
On Saturday 21 September 2013, Kishon Vijay Abraham I wrote:
> {
>         u32 val;
>         void __iomem *val1;
>         void __iomem *dbi_base = pp->dbi_base;
> 
>         /* Program viewport 0 : INBOUND : MEMORY*/
>         val = PCIE_ATU_REGION_INBOUND | (0 & 0xF);
>         dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
>         val1 = ioremap(0x80000000, 0x5fffffff);

The ioremap here makes no sense at all, and I suspect it will fail anyway,
because you exhaust the vmalloc area size, but since the value is not
used anywhere, it won't matter.

>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_BASE);
>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_BASE);
>         /* in_mem_size must be in power of 2 */
>         dw_pcie_writel_rc(pp, 0x5FFFFFFF, dbi_base + PCIE_ATU_LIMIT);
>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_TARGET);
>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);

These numbers need to come from somewhere, you shouldn't just hardcode them, 

I guess you should either program an inbound window covering the entire 64-bit
address space, or you should look at the top-level "memory" nodes to find
the location of physical RAM.

I can't see anything wrong with the way it's set up though, unless you have
an IOMMU. Can you confirm that there is no IOMMU (aka SMMU) in your system
that handles the PCIe root complex?

> I somehow starting to doubt the DMA address programmed in the ethernet card
> which is in my RAM address range (0x80000000 to 0xBFFFFFFF). Should this
> address be programmed in the BAR of the ethernet card? How should it be done?

No, it should not be in the BAR. The ethernet device driver calls dma_map_*
or pci_map_* interfaces to get a valid token that can be passed into the
device registers that are starting the DMA. You have to ensure that the
dma_map_ops for the device return the value that is set up in the translation.

The normal case is an identity mapping between device DMA space and host
memory space, i.e. PCIE_ATU_LOWER_TARGET == PCIE_ATU_LOWER_BASE, so
in the dma_map_single implementation, phys_addr_t == dma_addr_t.

If you set up the dma_addr_t space to start at 0 instead, you have to add
the offset in the dma_map_ops.

	Arnd
--
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
Kishon Vijay Abraham I Sept. 22, 2013, 11:16 a.m. UTC | #20
Hi Arnd,

Thanks for replying :-)

On Sunday 22 September 2013 03:33 AM, Arnd Bergmann wrote:
> On Saturday 21 September 2013, Kishon Vijay Abraham I wrote:
>> {
>>         u32 val;
>>         void __iomem *val1;
>>         void __iomem *dbi_base = pp->dbi_base;
>>
>>         /* Program viewport 0 : INBOUND : MEMORY*/
>>         val = PCIE_ATU_REGION_INBOUND | (0 & 0xF);
>>         dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
>>         val1 = ioremap(0x80000000, 0x5fffffff);
> 
> The ioremap here makes no sense at all, and I suspect it will fail anyway,
> because you exhaust the vmalloc area size, but since the value is not
> used anywhere, it won't matter.
> 
>>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_BASE);
>>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_BASE);
>>         /* in_mem_size must be in power of 2 */
>>         dw_pcie_writel_rc(pp, 0x5FFFFFFF, dbi_base + PCIE_ATU_LIMIT);
>>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_TARGET);
>>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);
> 
> These numbers need to come from somewhere, you shouldn't just hardcode them, 

right. I'm still in the process of getting it work ;-)
> 
> I guess you should either program an inbound window covering the entire 64-bit
> address space, or you should look at the top-level "memory" nodes to find
> the location of physical RAM.
> 
> I can't see anything wrong with the way it's set up though, unless you have
> an IOMMU. Can you confirm that there is no IOMMU (aka SMMU) in your system
> that handles the PCIe root complex?

There is a MMU for PCIe root complex but that's disabled.
> 
>> I somehow starting to doubt the DMA address programmed in the ethernet card
>> which is in my RAM address range (0x80000000 to 0xBFFFFFFF). Should this
>> address be programmed in the BAR of the ethernet card? How should it be done?
> 
> No, it should not be in the BAR. The ethernet device driver calls dma_map_*
> or pci_map_* interfaces to get a valid token that can be passed into the
> device registers that are starting the DMA. You have to ensure that the
> dma_map_ops for the device return the value that is set up in the translation.
> 
> The normal case is an identity mapping between device DMA space and host
> memory space, i.e. PCIE_ATU_LOWER_TARGET == PCIE_ATU_LOWER_BASE, so
> in the dma_map_single implementation, phys_addr_t == dma_addr_t.
> 
> If you set up the dma_addr_t space to start at 0 instead, you have to add
> the offset in the dma_map_ops.

My DMA address is in 0x80000000 to 0xBFFFFFFF range and I program my inbound
translation for this range. Not sure what is missing still :-(

Thanks
Kishon
--
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
Pratyush ANAND Sept. 23, 2013, 4:14 a.m. UTC | #21
Hi Kishon,


On Sun, Sep 22, 2013 at 07:16:34PM +0800, Kishon Vijay Abraham I wrote:
> Hi Arnd,
> 
> Thanks for replying :-)
> 
> On Sunday 22 September 2013 03:33 AM, Arnd Bergmann wrote:
> > On Saturday 21 September 2013, Kishon Vijay Abraham I wrote:
> >> {
> >>         u32 val;
> >>         void __iomem *val1;
> >>         void __iomem *dbi_base = pp->dbi_base;
> >>
> >>         /* Program viewport 0 : INBOUND : MEMORY*/
> >>         val = PCIE_ATU_REGION_INBOUND | (0 & 0xF);
> >>         dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
> >>         val1 = ioremap(0x80000000, 0x5fffffff);
> > 
> > The ioremap here makes no sense at all, and I suspect it will fail anyway,
> > because you exhaust the vmalloc area size, but since the value is not
> > used anywhere, it won't matter.
> > 
> >>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_BASE);
> >>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_BASE);
> >>         /* in_mem_size must be in power of 2 */
> >>         dw_pcie_writel_rc(pp, 0x5FFFFFFF, dbi_base + PCIE_ATU_LIMIT);

This is wrong. You should program here 0xBFFFFFFF.

Translation rule is as follows:

Region between "Start Address" and "End Address" is translated to
"Target Address" with region size = "End Address" - "Start Address".
Where: Start Address = (PCIE_ATU_UPPER_BASE | PCIE_ATU_LOWER_BASE)
        End Address = (PCIE_ATU_UPPER_BASE | PCIE_ATU_LIMIT)
        Target Address = (PCIE_ATU_UPPER_TARGET | PCIE_ATU_LOWER_TARGET) 

> >>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_TARGET);
> >>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);
> > 
> > These numbers need to come from somewhere, you shouldn't just hardcode them, 
> 
> right. I'm still in the process of getting it work ;-)
> > 
> > I guess you should either program an inbound window covering the entire 64-bit
> > address space, or you should look at the top-level "memory" nodes to find
> > the location of physical RAM.
> > 
> > I can't see anything wrong with the way it's set up though, unless you have
> > an IOMMU. Can you confirm that there is no IOMMU (aka SMMU) in your system
> > that handles the PCIe root complex?
> 
> There is a MMU for PCIe root complex but that's disabled.
> > 
> >> I somehow starting to doubt the DMA address programmed in the ethernet card
> >> which is in my RAM address range (0x80000000 to 0xBFFFFFFF). Should this
> >> address be programmed in the BAR of the ethernet card? How should it be done?
> > 
> > No, it should not be in the BAR. The ethernet device driver calls dma_map_*
> > or pci_map_* interfaces to get a valid token that can be passed into the
> > device registers that are starting the DMA. You have to ensure that the
> > dma_map_ops for the device return the value that is set up in the translation.
> > 
> > The normal case is an identity mapping between device DMA space and host
> > memory space, i.e. PCIE_ATU_LOWER_TARGET == PCIE_ATU_LOWER_BASE, so
> > in the dma_map_single implementation, phys_addr_t == dma_addr_t.
> > 
> > If you set up the dma_addr_t space to start at 0 instead, you have to add
> > the offset in the dma_map_ops.
> 
> My DMA address is in 0x80000000 to 0xBFFFFFFF range and I program my inbound
> translation for this range. Not sure what is missing still :-(

Hope, above modification helps. Let me know.

Regards
Pratyush
> 
> Thanks
> Kishon
--
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
Kishon Vijay Abraham I Sept. 23, 2013, 5:32 a.m. UTC | #22
Hi Pratyush,

On Monday 23 September 2013 09:44 AM, Pratyush Anand wrote:
> Hi Kishon,
> 
> 
> On Sun, Sep 22, 2013 at 07:16:34PM +0800, Kishon Vijay Abraham I wrote:
>> Hi Arnd,
>>
>> Thanks for replying :-)
>>
>> On Sunday 22 September 2013 03:33 AM, Arnd Bergmann wrote:
>>> On Saturday 21 September 2013, Kishon Vijay Abraham I wrote:
>>>> {
>>>>         u32 val;
>>>>         void __iomem *val1;
>>>>         void __iomem *dbi_base = pp->dbi_base;
>>>>
>>>>         /* Program viewport 0 : INBOUND : MEMORY*/
>>>>         val = PCIE_ATU_REGION_INBOUND | (0 & 0xF);
>>>>         dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
>>>>         val1 = ioremap(0x80000000, 0x5fffffff);
>>>
>>> The ioremap here makes no sense at all, and I suspect it will fail anyway,
>>> because you exhaust the vmalloc area size, but since the value is not
>>> used anywhere, it won't matter.
>>>
>>>>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_BASE);
>>>>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_BASE);
>>>>         /* in_mem_size must be in power of 2 */
>>>>         dw_pcie_writel_rc(pp, 0x5FFFFFFF, dbi_base + PCIE_ATU_LIMIT);
> 
> This is wrong. You should program here 0xBFFFFFFF.

That dint help :-(

Btw if we hadn't programmed inbound translation table, the address will go
untranslated (according to the data book). I guess that's how it was working
for Jingoo Han.

**
3.10.4
Inbound iATU Operation

When there is no match, then the address is untranslated
**

> 
> Translation rule is as follows:
> 
> Region between "Start Address" and "End Address" is translated to
> "Target Address" with region size = "End Address" - "Start Address".
> Where: Start Address = (PCIE_ATU_UPPER_BASE | PCIE_ATU_LOWER_BASE)
>         End Address = (PCIE_ATU_UPPER_BASE | PCIE_ATU_LIMIT)
>         Target Address = (PCIE_ATU_UPPER_TARGET | PCIE_ATU_LOWER_TARGET) 
> 
>>>>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_TARGET);
>>>>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);
>>>
>>> These numbers need to come from somewhere, you shouldn't just hardcode them, 
>>
>> right. I'm still in the process of getting it work ;-)
>>>
>>> I guess you should either program an inbound window covering the entire 64-bit
>>> address space, or you should look at the top-level "memory" nodes to find
>>> the location of physical RAM.
>>>
>>> I can't see anything wrong with the way it's set up though, unless you have
>>> an IOMMU. Can you confirm that there is no IOMMU (aka SMMU) in your system
>>> that handles the PCIe root complex?
>>
>> There is a MMU for PCIe root complex but that's disabled.
>>>
>>>> I somehow starting to doubt the DMA address programmed in the ethernet card
>>>> which is in my RAM address range (0x80000000 to 0xBFFFFFFF). Should this
>>>> address be programmed in the BAR of the ethernet card? How should it be done?
>>>
>>> No, it should not be in the BAR. The ethernet device driver calls dma_map_*
>>> or pci_map_* interfaces to get a valid token that can be passed into the
>>> device registers that are starting the DMA. You have to ensure that the
>>> dma_map_ops for the device return the value that is set up in the translation.
>>>
>>> The normal case is an identity mapping between device DMA space and host
>>> memory space, i.e. PCIE_ATU_LOWER_TARGET == PCIE_ATU_LOWER_BASE, so
>>> in the dma_map_single implementation, phys_addr_t == dma_addr_t.
>>>
>>> If you set up the dma_addr_t space to start at 0 instead, you have to add
>>> the offset in the dma_map_ops.
>>
>> My DMA address is in 0x80000000 to 0xBFFFFFFF range and I program my inbound
>> translation for this range. Not sure what is missing still :-(
> 
> Hope, above modification helps. Let me know.
> 
> Regards
> Pratyush
>>
>> Thanks
>> Kishon

Thanks
Kishon

--
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
Pratyush ANAND Sept. 23, 2013, 6:50 a.m. UTC | #23
On Mon, Sep 23, 2013 at 01:32:54PM +0800, Kishon Vijay Abraham I wrote:
> Hi Pratyush,
> 
> On Monday 23 September 2013 09:44 AM, Pratyush Anand wrote:
> > Hi Kishon,
> > 
> > 
> > On Sun, Sep 22, 2013 at 07:16:34PM +0800, Kishon Vijay Abraham I wrote:
> >> Hi Arnd,
> >>
> >> Thanks for replying :-)
> >>
> >> On Sunday 22 September 2013 03:33 AM, Arnd Bergmann wrote:
> >>> On Saturday 21 September 2013, Kishon Vijay Abraham I wrote:
> >>>> {
> >>>>         u32 val;
> >>>>         void __iomem *val1;
> >>>>         void __iomem *dbi_base = pp->dbi_base;
> >>>>
> >>>>         /* Program viewport 0 : INBOUND : MEMORY*/
> >>>>         val = PCIE_ATU_REGION_INBOUND | (0 & 0xF);
> >>>>         dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
> >>>>         val1 = ioremap(0x80000000, 0x5fffffff);
> >>>
> >>> The ioremap here makes no sense at all, and I suspect it will fail anyway,
> >>> because you exhaust the vmalloc area size, but since the value is not
> >>> used anywhere, it won't matter.
> >>>
> >>>>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_BASE);
> >>>>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_BASE);
> >>>>         /* in_mem_size must be in power of 2 */
> >>>>         dw_pcie_writel_rc(pp, 0x5FFFFFFF, dbi_base + PCIE_ATU_LIMIT);
> > 
> > This is wrong. You should program here 0xBFFFFFFF.
> 
> That dint help :-(
> 
> Btw if we hadn't programmed inbound translation table, the address will go
> untranslated (according to the data book). I guess that's how it was working
> for Jingoo Han.
> 
> **
> 3.10.4
> Inbound iATU Operation
> 
> When there is no match, then the address is untranslated
> **
> 

ok.. so it seems that you might not be getting non-fatal error because
of address translation. I am not working with PCIe for a long time,
but if I remember correctly other reason for getting this error were
unsupported read/write request size. Although, you have said that MRRS
is 512 for your RC. Can you cross check both programmed MPS and MRRS
in your RC as well as your EP (ethernet card). May be you can share
Content of PCIe Capability Device Control Register (Offset 08h) for
both RC and EP.

Regards
Pratyush



> > 
> > Translation rule is as follows:
> > 
> > Region between "Start Address" and "End Address" is translated to
> > "Target Address" with region size = "End Address" - "Start Address".
> > Where: Start Address = (PCIE_ATU_UPPER_BASE | PCIE_ATU_LOWER_BASE)
> >         End Address = (PCIE_ATU_UPPER_BASE | PCIE_ATU_LIMIT)
> >         Target Address = (PCIE_ATU_UPPER_TARGET | PCIE_ATU_LOWER_TARGET) 
> > 
> >>>>         dw_pcie_writel_rc(pp, 0x80000000, dbi_base + PCIE_ATU_LOWER_TARGET);
> >>>>         dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);
> >>>
> >>> These numbers need to come from somewhere, you shouldn't just hardcode them, 
> >>
> >> right. I'm still in the process of getting it work ;-)
> >>>
> >>> I guess you should either program an inbound window covering the entire 64-bit
> >>> address space, or you should look at the top-level "memory" nodes to find
> >>> the location of physical RAM.
> >>>
> >>> I can't see anything wrong with the way it's set up though, unless you have
> >>> an IOMMU. Can you confirm that there is no IOMMU (aka SMMU) in your system
> >>> that handles the PCIe root complex?
> >>
> >> There is a MMU for PCIe root complex but that's disabled.
> >>>
> >>>> I somehow starting to doubt the DMA address programmed in the ethernet card
> >>>> which is in my RAM address range (0x80000000 to 0xBFFFFFFF). Should this
> >>>> address be programmed in the BAR of the ethernet card? How should it be done?
> >>>
> >>> No, it should not be in the BAR. The ethernet device driver calls dma_map_*
> >>> or pci_map_* interfaces to get a valid token that can be passed into the
> >>> device registers that are starting the DMA. You have to ensure that the
> >>> dma_map_ops for the device return the value that is set up in the translation.
> >>>
> >>> The normal case is an identity mapping between device DMA space and host
> >>> memory space, i.e. PCIE_ATU_LOWER_TARGET == PCIE_ATU_LOWER_BASE, so
> >>> in the dma_map_single implementation, phys_addr_t == dma_addr_t.
> >>>
> >>> If you set up the dma_addr_t space to start at 0 instead, you have to add
> >>> the offset in the dma_map_ops.
> >>
> >> My DMA address is in 0x80000000 to 0xBFFFFFFF range and I program my inbound
> >> translation for this range. Not sure what is missing still :-(
> > 
> > Hope, above modification helps. Let me know.
> > 
> > Regards
> > Pratyush
> >>
> >> Thanks
> >> Kishon
> 
> Thanks
> Kishon
--
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
Arnd Bergmann Sept. 24, 2013, 9:23 p.m. UTC | #24
On Monday 23 September 2013, Kishon Vijay Abraham I wrote:
> Btw if we hadn't programmed inbound translation table, the address will go
> untranslated (according to the data book). I guess that's how it was working
> for Jingoo Han.
> 
> **
> 3.10.4
> Inbound iATU Operation
> 
> When there is no match, then the address is untranslated
> **
> 

Well, that should work just as well, since you have a 1:1 translation anyway.
Do you get the same error without the translation?

	Arnd
--
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
Kishon Vijay Abraham I Sept. 26, 2013, 5:06 a.m. UTC | #25
On Wednesday 25 September 2013 02:53 AM, Arnd Bergmann wrote:
> On Monday 23 September 2013, Kishon Vijay Abraham I wrote:
>> Btw if we hadn't programmed inbound translation table, the address will go
>> untranslated (according to the data book). I guess that's how it was working
>> for Jingoo Han.
>>
>> **
>> 3.10.4
>> Inbound iATU Operation
>>
>> When there is no match, then the address is untranslated
>> **
>>
> 
> Well, that should work just as well, since you have a 1:1 translation anyway.
> Do you get the same error without the translation?

Yes. I get the same non-fatal error interrupt in RC.

Thanks
Kishon
--
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
Arnd Bergmann Sept. 26, 2013, 9:51 a.m. UTC | #26
On Thursday 26 September 2013, Kishon Vijay Abraham I wrote:
> On Wednesday 25 September 2013 02:53 AM, Arnd Bergmann wrote:
> > On Monday 23 September 2013, Kishon Vijay Abraham I wrote:
> >> Btw if we hadn't programmed inbound translation table, the address will go
> >> untranslated (according to the data book). I guess that's how it was working
> >> for Jingoo Han.
> >>
> >> **
> >> 3.10.4
> >> Inbound iATU Operation
> >>
> >> When there is no match, then the address is untranslated
> >> **
> >>
> > 
> > Well, that should work just as well, since you have a 1:1 translation anyway.
> > Do you get the same error without the translation?
> 
> Yes. I get the same non-fatal error interrupt in RC.

Ok, then I guess the translation is actually not at fault here but something
else. I would recommend looking at the IOMMU as the potential culprit. Maybe
having it disabled means that no DMA is going through, rather than all DMA
going through untranslated. Another possibility is that the IOMMU is set up
so that when disabled, it maps DMA address 0 to the start of RAM, rather
than identity mapping DMA address 0x80000000 there. If that's the case,
you either have to use the IOMMU, or set up the mapping in the root
complex to revert it.

	Arnd
--
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
Kishon Vijay Abraham I Sept. 26, 2013, 10:12 a.m. UTC | #27
Hi Arnd,

On Thursday 26 September 2013 03:21 PM, Arnd Bergmann wrote:
> On Thursday 26 September 2013, Kishon Vijay Abraham I wrote:
>> On Wednesday 25 September 2013 02:53 AM, Arnd Bergmann wrote:
>>> On Monday 23 September 2013, Kishon Vijay Abraham I wrote:
>>>> Btw if we hadn't programmed inbound translation table, the address will go
>>>> untranslated (according to the data book). I guess that's how it was working
>>>> for Jingoo Han.
>>>>
>>>> **
>>>> 3.10.4
>>>> Inbound iATU Operation
>>>>
>>>> When there is no match, then the address is untranslated
>>>> **
>>>>
>>>
>>> Well, that should work just as well, since you have a 1:1 translation anyway.
>>> Do you get the same error without the translation?
>>
>> Yes. I get the same non-fatal error interrupt in RC.
> 
> Ok, then I guess the translation is actually not at fault here but something
> else. I would recommend looking at the IOMMU as the potential culprit. Maybe
> having it disabled means that no DMA is going through, rather than all DMA
> going through untranslated. Another possibility is that the IOMMU is set up
> so that when disabled, it maps DMA address 0 to the start of RAM, rather
> than identity mapping DMA address 0x80000000 there. If that's the case,
> you either have to use the IOMMU, or set up the mapping in the root
> complex to revert it.

Thanks for your inputs. I'll check if that's the problem.

Thanks
Kishon
--
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
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index e2371f5..eabcb4b 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -18,6 +18,7 @@  Required properties:
 - interrupt-map-mask and interrupt-map: standard PCI properties
 	to define the mapping of the PCIe interface to interrupt
 	numbers.
+- num-lanes: number of lanes to use
 - reset-gpio: gpio pin number of power good signal
 
 Example:
@@ -41,6 +42,7 @@  SoC specific DT Entry:
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 0>;
 		interrupt-map = <0x0 0 &gic 53>;
+		num-lanes = <4>;
 	};
 
 	pcie@2a0000 {
@@ -60,6 +62,7 @@  SoC specific DT Entry:
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 0>;
 		interrupt-map = <0x0 0 &gic 56>;
+		num-lanes = <4>;
 	};
 
 Board specific DT Entry:
diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
index ff7f5d8..586134e 100644
--- a/arch/arm/boot/dts/exynos5440.dtsi
+++ b/arch/arm/boot/dts/exynos5440.dtsi
@@ -248,6 +248,7 @@ 
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 0>;
 		interrupt-map = <0x0 0 &gic 53>;
+		num-lanes = <4>;
 	};
 
 	pcie@2a0000 {
@@ -267,5 +268,6 @@ 
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 0>;
 		interrupt-map = <0x0 0 &gic 56>;
+		num-lanes = <4>;
 	};
 };
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 086d850..701acd1 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -1,2 +1,3 @@ 
 obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
 obj-$(CONFIG_PCIE_DW) += pcie-designware.o
+obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
new file mode 100644
index 0000000..3fe7ce4
--- /dev/null
+++ b/drivers/pci/host/pci-exynos.c
@@ -0,0 +1,552 @@ 
+/*
+ * PCIe host controller driver for Samsung EXYNOS SoCs
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Author: Jingoo Han <jg1.han@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/signal.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+#define to_exynos_pcie(x)	container_of(x, struct exynos_pcie, pp)
+
+struct exynos_pcie {
+	void __iomem		*elbi_base;
+	void __iomem		*phy_base;
+	void __iomem		*block_base;
+	int			reset_gpio;
+	struct clk		*clk;
+	struct clk		*bus_clk;
+	struct pcie_port	pp;
+};
+
+/* PCIe ELBI registers */
+#define PCIE_IRQ_PULSE			0x000
+#define IRQ_INTA_ASSERT			(0x1 << 0)
+#define IRQ_INTB_ASSERT			(0x1 << 2)
+#define IRQ_INTC_ASSERT			(0x1 << 4)
+#define IRQ_INTD_ASSERT			(0x1 << 6)
+#define PCIE_IRQ_LEVEL			0x004
+#define PCIE_IRQ_SPECIAL		0x008
+#define PCIE_IRQ_EN_PULSE		0x00c
+#define PCIE_IRQ_EN_LEVEL		0x010
+#define PCIE_IRQ_EN_SPECIAL		0x014
+#define PCIE_PWR_RESET			0x018
+#define PCIE_CORE_RESET			0x01c
+#define PCIE_CORE_RESET_ENABLE		(0x1 << 0)
+#define PCIE_STICKY_RESET		0x020
+#define PCIE_NONSTICKY_RESET		0x024
+#define PCIE_APP_INIT_RESET		0x028
+#define PCIE_APP_LTSSM_ENABLE		0x02c
+#define PCIE_ELBI_RDLH_LINKUP		0x064
+#define PCIE_ELBI_LTSSM_ENABLE		0x1
+#define PCIE_ELBI_SLV_AWMISC		0x11c
+#define PCIE_ELBI_SLV_ARMISC		0x120
+#define PCIE_ELBI_SLV_DBI_ENABLE	(0x1 << 21)
+
+/* PCIe Purple registers */
+#define PCIE_PHY_GLOBAL_RESET		0x000
+#define PCIE_PHY_COMMON_RESET		0x004
+#define PCIE_PHY_CMN_REG		0x008
+#define PCIE_PHY_MAC_RESET		0x00c
+#define PCIE_PHY_PLL_LOCKED		0x010
+#define PCIE_PHY_TRSVREG_RESET		0x020
+#define PCIE_PHY_TRSV_RESET		0x024
+
+/* PCIe PHY registers */
+#define PCIE_PHY_IMPEDANCE		0x004
+#define PCIE_PHY_PLL_DIV_0		0x008
+#define PCIE_PHY_PLL_BIAS		0x00c
+#define PCIE_PHY_DCC_FEEDBACK		0x014
+#define PCIE_PHY_PLL_DIV_1		0x05c
+#define PCIE_PHY_TRSV0_EMP_LVL		0x084
+#define PCIE_PHY_TRSV0_DRV_LVL		0x088
+#define PCIE_PHY_TRSV0_RXCDR		0x0ac
+#define PCIE_PHY_TRSV0_LVCC		0x0dc
+#define PCIE_PHY_TRSV1_EMP_LVL		0x144
+#define PCIE_PHY_TRSV1_RXCDR		0x16c
+#define PCIE_PHY_TRSV1_LVCC		0x19c
+#define PCIE_PHY_TRSV2_EMP_LVL		0x204
+#define PCIE_PHY_TRSV2_RXCDR		0x22c
+#define PCIE_PHY_TRSV2_LVCC		0x25c
+#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
+#define PCIE_PHY_TRSV3_RXCDR		0x2ec
+#define PCIE_PHY_TRSV3_LVCC		0x31c
+
+static void exynos_pcie_sideband_dbi_w_mode(struct pcie_port *pp, bool on)
+{
+	u32 val;
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+
+	if (on) {
+		val = readl(exynos_pcie->elbi_base + PCIE_ELBI_SLV_AWMISC);
+		val |= PCIE_ELBI_SLV_DBI_ENABLE;
+		writel(val, exynos_pcie->elbi_base + PCIE_ELBI_SLV_AWMISC);
+	} else {
+		val = readl(exynos_pcie->elbi_base + PCIE_ELBI_SLV_AWMISC);
+		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
+		writel(val, exynos_pcie->elbi_base + PCIE_ELBI_SLV_AWMISC);
+	}
+}
+
+static void exynos_pcie_sideband_dbi_r_mode(struct pcie_port *pp, bool on)
+{
+	u32 val;
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+
+	if (on) {
+		val = readl(exynos_pcie->elbi_base + PCIE_ELBI_SLV_ARMISC);
+		val |= PCIE_ELBI_SLV_DBI_ENABLE;
+		writel(val, exynos_pcie->elbi_base + PCIE_ELBI_SLV_ARMISC);
+	} else {
+		val = readl(exynos_pcie->elbi_base + PCIE_ELBI_SLV_ARMISC);
+		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
+		writel(val, exynos_pcie->elbi_base + PCIE_ELBI_SLV_ARMISC);
+	}
+}
+
+static void exynos_pcie_assert_core_reset(struct pcie_port *pp)
+{
+	u32 val;
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	void __iomem *elbi_base = exynos_pcie->elbi_base;
+
+	val = readl(elbi_base + PCIE_CORE_RESET);
+	val &= ~PCIE_CORE_RESET_ENABLE;
+	writel(val, elbi_base + PCIE_CORE_RESET);
+	writel(0, elbi_base + PCIE_PWR_RESET);
+	writel(0, elbi_base + PCIE_STICKY_RESET);
+	writel(0, elbi_base + PCIE_NONSTICKY_RESET);
+}
+
+static void exynos_pcie_deassert_core_reset(struct pcie_port *pp)
+{
+	u32 val;
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	void __iomem *elbi_base = exynos_pcie->elbi_base;
+	void __iomem *block_base = exynos_pcie->block_base;
+
+	val = readl(elbi_base + PCIE_CORE_RESET);
+	val |= PCIE_CORE_RESET_ENABLE;
+	writel(val, elbi_base + PCIE_CORE_RESET);
+	writel(1, elbi_base + PCIE_STICKY_RESET);
+	writel(1, elbi_base + PCIE_NONSTICKY_RESET);
+	writel(1, elbi_base + PCIE_APP_INIT_RESET);
+	writel(0, elbi_base + PCIE_APP_INIT_RESET);
+	writel(1, block_base + PCIE_PHY_MAC_RESET);
+}
+
+static void exynos_pcie_assert_phy_reset(struct pcie_port *pp)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	void __iomem *block_base = exynos_pcie->block_base;
+
+	writel(0, block_base + PCIE_PHY_MAC_RESET);
+	writel(1, block_base + PCIE_PHY_GLOBAL_RESET);
+}
+
+static void exynos_pcie_deassert_phy_reset(struct pcie_port *pp)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	void __iomem *elbi_base = exynos_pcie->elbi_base;
+	void __iomem *block_base = exynos_pcie->block_base;
+
+	writel(0, block_base + PCIE_PHY_GLOBAL_RESET);
+	writel(1, elbi_base + PCIE_PWR_RESET);
+	writel(0, block_base + PCIE_PHY_COMMON_RESET);
+	writel(0, block_base + PCIE_PHY_CMN_REG);
+	writel(0, block_base + PCIE_PHY_TRSVREG_RESET);
+	writel(0, block_base + PCIE_PHY_TRSV_RESET);
+}
+
+static void exynos_pcie_init_phy(struct pcie_port *pp)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	void __iomem *phy_base = exynos_pcie->phy_base;
+
+	/* DCC feedback control off */
+	writel(0x29, phy_base + PCIE_PHY_DCC_FEEDBACK);
+
+	/* set TX/RX impedance */
+	writel(0xd5, phy_base + PCIE_PHY_IMPEDANCE);
+
+	/* set 50Mhz PHY clock */
+	writel(0x14, phy_base + PCIE_PHY_PLL_DIV_0);
+	writel(0x12, phy_base + PCIE_PHY_PLL_DIV_1);
+
+	/* set TX Differential output for lane 0 */
+	writel(0x7f, phy_base + PCIE_PHY_TRSV0_DRV_LVL);
+
+	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
+	writel(0x0, phy_base + PCIE_PHY_TRSV0_EMP_LVL);
+
+	/* set RX clock and data recovery bandwidth */
+	writel(0xe7, phy_base + PCIE_PHY_PLL_BIAS);
+	writel(0x82, phy_base + PCIE_PHY_TRSV0_RXCDR);
+	writel(0x82, phy_base + PCIE_PHY_TRSV1_RXCDR);
+	writel(0x82, phy_base + PCIE_PHY_TRSV2_RXCDR);
+	writel(0x82, phy_base + PCIE_PHY_TRSV3_RXCDR);
+
+	/* change TX Pre-emphasis Level Control for lanes */
+	writel(0x39, phy_base + PCIE_PHY_TRSV0_EMP_LVL);
+	writel(0x39, phy_base + PCIE_PHY_TRSV1_EMP_LVL);
+	writel(0x39, phy_base + PCIE_PHY_TRSV2_EMP_LVL);
+	writel(0x39, phy_base + PCIE_PHY_TRSV3_EMP_LVL);
+
+	/* set LVCC */
+	writel(0x20, phy_base + PCIE_PHY_TRSV0_LVCC);
+	writel(0xa0, phy_base + PCIE_PHY_TRSV1_LVCC);
+	writel(0xa0, phy_base + PCIE_PHY_TRSV2_LVCC);
+	writel(0xa0, phy_base + PCIE_PHY_TRSV3_LVCC);
+}
+
+static void exynos_pcie_assert_reset(struct pcie_port *pp)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+
+	if (exynos_pcie->reset_gpio >= 0)
+		devm_gpio_request_one(pp->dev, exynos_pcie->reset_gpio,
+				GPIOF_OUT_INIT_HIGH, "RESET");
+	return;
+}
+
+static int exynos_pcie_establish_link(struct pcie_port *pp)
+{
+	u32 val;
+	int count = 0;
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	void __iomem *elbi_base = exynos_pcie->elbi_base;
+	void __iomem *block_base = exynos_pcie->block_base;
+	void __iomem *phy_base = exynos_pcie->phy_base;
+
+	if (dw_pcie_link_up(pp)) {
+		dev_err(pp->dev, "Link already up\n");
+		return 0;
+	}
+
+	/* assert reset signals */
+	exynos_pcie_assert_core_reset(pp);
+	exynos_pcie_assert_phy_reset(pp);
+
+	/* de-assert phy reset */
+	exynos_pcie_deassert_phy_reset(pp);
+
+	/* initialize phy */
+	exynos_pcie_init_phy(pp);
+
+	/* pulse for common reset */
+	writel(1, block_base + PCIE_PHY_COMMON_RESET);
+	udelay(500);
+	writel(0, block_base + PCIE_PHY_COMMON_RESET);
+
+	/* de-assert core reset */
+	exynos_pcie_deassert_core_reset(pp);
+
+	/* setup root complex */
+	dw_pcie_setup_rc(pp);
+
+	/* assert reset signal */
+	exynos_pcie_assert_reset(pp);
+
+	/* assert LTSSM enable */
+	writel(PCIE_ELBI_LTSSM_ENABLE, elbi_base + PCIE_APP_LTSSM_ENABLE);
+
+	/* check if the link is up or not */
+	while (!dw_pcie_link_up(pp)) {
+		mdelay(100);
+		count++;
+		if (count == 10) {
+			while (readl(phy_base + PCIE_PHY_PLL_LOCKED) == 0) {
+				val = readl(block_base + PCIE_PHY_PLL_LOCKED);
+				dev_info(pp->dev, "PLL Locked: 0x%x\n", val);
+			}
+			dev_err(pp->dev, "PCIe Link Fail\n");
+			return -EINVAL;
+		}
+	}
+
+	dev_info(pp->dev, "Link up\n");
+
+	return 0;
+}
+
+static void exynos_pcie_clear_irq_pulse(struct pcie_port *pp)
+{
+	u32 val;
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	void __iomem *elbi_base = exynos_pcie->elbi_base;
+
+	val = readl(elbi_base + PCIE_IRQ_PULSE);
+	writel(val, elbi_base + PCIE_IRQ_PULSE);
+	return;
+}
+
+static void exynos_pcie_enable_irq_pulse(struct pcie_port *pp)
+{
+	u32 val;
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	void __iomem *elbi_base = exynos_pcie->elbi_base;
+
+	/* enable INTX interrupt */
+	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
+		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT,
+	writel(val, elbi_base + PCIE_IRQ_EN_PULSE);
+	return;
+}
+
+static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
+{
+	struct pcie_port *pp = arg;
+
+	exynos_pcie_clear_irq_pulse(pp);
+	return IRQ_HANDLED;
+}
+
+static void exynos_pcie_enable_interrupts(struct pcie_port *pp)
+{
+	exynos_pcie_enable_irq_pulse(pp);
+	return;
+}
+
+static inline void exynos_pcie_readl_rc(struct pcie_port *pp, void *dbi_base,
+					u32 *val)
+{
+	exynos_pcie_sideband_dbi_r_mode(pp, true);
+	*val = readl(dbi_base);
+	exynos_pcie_sideband_dbi_r_mode(pp, false);
+	return;
+}
+
+static inline void exynos_pcie_writel_rc(struct pcie_port *pp, u32 val,
+					void *dbi_base)
+{
+	exynos_pcie_sideband_dbi_w_mode(pp, true);
+	writel(val, dbi_base);
+	exynos_pcie_sideband_dbi_w_mode(pp, false);
+	return;
+}
+
+static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
+				u32 *val)
+{
+	int ret;
+
+	exynos_pcie_sideband_dbi_r_mode(pp, true);
+	ret = cfg_read(pp->dbi_base + (where & ~0x3), where, size, val);
+	exynos_pcie_sideband_dbi_r_mode(pp, false);
+	return ret;
+}
+
+static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
+				u32 val)
+{
+	int ret;
+
+	exynos_pcie_sideband_dbi_w_mode(pp, true);
+	ret = cfg_write(pp->dbi_base + (where & ~0x3), where, size, val);
+	exynos_pcie_sideband_dbi_w_mode(pp, false);
+	return ret;
+}
+
+static int exynos_pcie_link_up(struct pcie_port *pp)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	u32 val = readl(exynos_pcie->elbi_base + PCIE_ELBI_RDLH_LINKUP);
+
+	if (val == PCIE_ELBI_LTSSM_ENABLE)
+		return 1;
+
+	return 0;
+}
+
+static void exynos_pcie_host_init(struct pcie_port *pp)
+{
+	exynos_pcie_establish_link(pp);
+	exynos_pcie_enable_interrupts(pp);
+}
+
+static struct pcie_host_ops exynos_pcie_host_ops = {
+	.readl_rc = exynos_pcie_readl_rc,
+	.writel_rc = exynos_pcie_writel_rc,
+	.rd_own_conf = exynos_pcie_rd_own_conf,
+	.wr_own_conf = exynos_pcie_wr_own_conf,
+	.link_up = exynos_pcie_link_up,
+	.host_init = exynos_pcie_host_init,
+};
+
+static int add_pcie_port(struct pcie_port *pp, struct platform_device *pdev)
+{
+	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	struct resource *elbi_base;
+	struct resource *phy_base;
+	struct resource *block_base;
+	int ret;
+
+	elbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!elbi_base) {
+		dev_err(&pdev->dev, "couldn't get elbi base resource\n");
+		return -EINVAL;
+	}
+	exynos_pcie->elbi_base = devm_ioremap_resource(&pdev->dev, elbi_base);
+	if (IS_ERR(exynos_pcie->elbi_base))
+		return PTR_ERR(exynos_pcie->elbi_base);
+
+	phy_base = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!phy_base) {
+		dev_err(&pdev->dev, "couldn't get phy base resource\n");
+		return -EINVAL;
+	}
+	exynos_pcie->phy_base = devm_ioremap_resource(&pdev->dev, phy_base);
+	if (IS_ERR(exynos_pcie->phy_base))
+		return PTR_ERR(exynos_pcie->phy_base);
+
+	block_base = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (!block_base) {
+		dev_err(&pdev->dev, "couldn't get block base resource\n");
+		return -EINVAL;
+	}
+	exynos_pcie->block_base = devm_ioremap_resource(&pdev->dev, block_base);
+	if (IS_ERR(exynos_pcie->block_base))
+		return PTR_ERR(exynos_pcie->block_base);
+
+	pp->irq = platform_get_irq(pdev, 1);
+	if (!pp->irq) {
+		dev_err(&pdev->dev, "failed to get irq\n");
+		return -ENODEV;
+	}
+	ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler,
+				IRQF_SHARED, "exynos-pcie", pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request irq\n");
+		return ret;
+	}
+
+	pp->root_bus_nr = -1;
+	pp->ops = &exynos_pcie_host_ops;
+
+	spin_lock_init(&pp->conf_lock);
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize host\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __init exynos_pcie_probe(struct platform_device *pdev)
+{
+	struct exynos_pcie *exynos_pcie;
+	struct pcie_port *pp;
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	exynos_pcie = devm_kzalloc(&pdev->dev, sizeof(*exynos_pcie),
+				GFP_KERNEL);
+	if (!exynos_pcie) {
+		dev_err(&pdev->dev, "no memory for exynos pcie\n");
+		return -ENOMEM;
+	}
+
+	pp = &exynos_pcie->pp;
+
+	pp->dev = &pdev->dev;
+
+	exynos_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
+
+	exynos_pcie->clk = devm_clk_get(&pdev->dev, "pcie");
+	if (IS_ERR(exynos_pcie->clk)) {
+		dev_err(&pdev->dev, "Failed to get pcie rc clock\n");
+		return PTR_ERR(exynos_pcie->clk);
+	}
+	ret = clk_prepare_enable(exynos_pcie->clk);
+	if (ret)
+		return ret;
+
+	exynos_pcie->bus_clk = devm_clk_get(&pdev->dev, "pcie_bus");
+	if (IS_ERR(exynos_pcie->bus_clk)) {
+		dev_err(&pdev->dev, "Failed to get pcie bus clock\n");
+		ret = PTR_ERR(exynos_pcie->bus_clk);
+		goto fail_clk;
+	}
+	ret = clk_prepare_enable(exynos_pcie->bus_clk);
+	if (ret)
+		goto fail_clk;
+
+	ret = add_pcie_port(pp, pdev);
+	if (ret < 0)
+		goto fail_bus_clk;
+
+	dw_pci.nr_controllers = 1;
+	dw_pci.private_data = (void **)&pp;
+
+	pci_common_init(&dw_pci);
+	pci_assign_unassigned_resources();
+#ifdef CONFIG_PCI_DOMAINS
+	dw_pci.domain++;
+#endif
+
+	platform_set_drvdata(pdev, exynos_pcie);
+	return 0;
+
+fail_bus_clk:
+	clk_disable_unprepare(exynos_pcie->bus_clk);
+fail_clk:
+	clk_disable_unprepare(exynos_pcie->clk);
+	return ret;
+}
+
+static int __exit exynos_pcie_remove(struct platform_device *pdev)
+{
+	struct exynos_pcie *exynos_pcie = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(exynos_pcie->bus_clk);
+	clk_disable_unprepare(exynos_pcie->clk);
+
+	return 0;
+}
+
+static const struct of_device_id exynos_pcie_of_match[] = {
+	{ .compatible = "samsung,exynos5440-pcie", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, exynos_pcie_of_match);
+
+static struct platform_driver exynos_pcie_driver = {
+	.remove		= __exit_p(exynos_pcie_remove),
+	.driver = {
+		.name	= "exynos-pcie",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(exynos_pcie_of_match),
+	},
+};
+
+/* Exynos PCIe driver does not allow module unload */
+
+static int __init pcie_init(void)
+{
+	return platform_driver_probe(&exynos_pcie_driver, exynos_pcie_probe);
+}
+subsys_initcall(pcie_init);
+
+MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
+MODULE_DESCRIPTION("Samsung PCIe host controller driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 26bdbda..f097eff 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -1,5 +1,5 @@ 
 /*
- * PCIe host controller driver for Samsung EXYNOS SoCs
+ * Synopsys Designware PCIe host controller driver
  *
  * Copyright (C) 2013 Samsung Electronics Co., Ltd.
  *		http://www.samsung.com
@@ -11,74 +11,28 @@ 
  * published by the Free Software Foundation.
  */
 
-#include <linux/clk.h>
-#include <linux/delay.h>
-#include <linux/gpio.h>
-#include <linux/interrupt.h>
 #include <linux/kernel.h>
-#include <linux/list.h>
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/of_address.h>
-#include <linux/of_gpio.h>
-#include <linux/of_pci.h>
 #include <linux/pci.h>
 #include <linux/pci_regs.h>
-#include <linux/platform_device.h>
-#include <linux/resource.h>
-#include <linux/signal.h>
-#include <linux/slab.h>
 #include <linux/types.h>
 
-struct pcie_port_info {
-	u32		cfg0_size;
-	u32		cfg1_size;
-	u32		io_size;
-	u32		mem_size;
-	phys_addr_t	io_bus_addr;
-	phys_addr_t	mem_bus_addr;
-};
-
-struct pcie_port {
-	struct device		*dev;
-	u8			controller;
-	u8			root_bus_nr;
-	void __iomem		*dbi_base;
-	void __iomem		*elbi_base;
-	void __iomem		*phy_base;
-	void __iomem		*purple_base;
-	u64			cfg0_base;
-	void __iomem		*va_cfg0_base;
-	u64			cfg1_base;
-	void __iomem		*va_cfg1_base;
-	u64			io_base;
-	u64			mem_base;
-	spinlock_t		conf_lock;
-	struct resource		cfg;
-	struct resource		io;
-	struct resource		mem;
-	struct pcie_port_info	config;
-	struct clk		*clk;
-	struct clk		*bus_clk;
-	int			irq;
-	int			reset_gpio;
-};
-
-/*
- * Exynos PCIe IP consists of Synopsys specific part and Exynos
- * specific part. Only core block is a Synopsys designware part;
- * other parts are Exynos specific.
- */
+#include "pcie-designware.h"
 
 /* Synopsis specific PCIE configuration registers */
 #define PCIE_PORT_LINK_CONTROL		0x710
 #define PORT_LINK_MODE_MASK		(0x3f << 16)
+#define PORT_LINK_MODE_1_LANES		(0x1 << 16)
+#define PORT_LINK_MODE_2_LANES		(0x3 << 16)
 #define PORT_LINK_MODE_4_LANES		(0x7 << 16)
 
 #define PCIE_LINK_WIDTH_SPEED_CONTROL	0x80C
 #define PORT_LOGIC_SPEED_CHANGE		(0x1 << 17)
 #define PORT_LOGIC_LINK_WIDTH_MASK	(0x1ff << 8)
-#define PORT_LOGIC_LINK_WIDTH_4_LANES	(0x7 << 8)
+#define PORT_LOGIC_LINK_WIDTH_1_LANES	(0x1 << 8)
+#define PORT_LOGIC_LINK_WIDTH_2_LANES	(0x2 << 8)
+#define PORT_LOGIC_LINK_WIDTH_4_LANES	(0x4 << 8)
 
 #define PCIE_MSI_ADDR_LO		0x820
 #define PCIE_MSI_ADDR_HI		0x824
@@ -108,69 +62,14 @@  struct pcie_port {
 #define PCIE_ATU_FUNC(x)		(((x) & 0x7) << 16)
 #define PCIE_ATU_UPPER_TARGET		0x91C
 
-/* Exynos specific PCIE configuration registers */
-
-/* PCIe ELBI registers */
-#define PCIE_IRQ_PULSE			0x000
-#define IRQ_INTA_ASSERT			(0x1 << 0)
-#define IRQ_INTB_ASSERT			(0x1 << 2)
-#define IRQ_INTC_ASSERT			(0x1 << 4)
-#define IRQ_INTD_ASSERT			(0x1 << 6)
-#define PCIE_IRQ_LEVEL			0x004
-#define PCIE_IRQ_SPECIAL		0x008
-#define PCIE_IRQ_EN_PULSE		0x00c
-#define PCIE_IRQ_EN_LEVEL		0x010
-#define PCIE_IRQ_EN_SPECIAL		0x014
-#define PCIE_PWR_RESET			0x018
-#define PCIE_CORE_RESET			0x01c
-#define PCIE_CORE_RESET_ENABLE		(0x1 << 0)
-#define PCIE_STICKY_RESET		0x020
-#define PCIE_NONSTICKY_RESET		0x024
-#define PCIE_APP_INIT_RESET		0x028
-#define PCIE_APP_LTSSM_ENABLE		0x02c
-#define PCIE_ELBI_RDLH_LINKUP		0x064
-#define PCIE_ELBI_LTSSM_ENABLE		0x1
-#define PCIE_ELBI_SLV_AWMISC		0x11c
-#define PCIE_ELBI_SLV_ARMISC		0x120
-#define PCIE_ELBI_SLV_DBI_ENABLE	(0x1 << 21)
-
-/* PCIe Purple registers */
-#define PCIE_PHY_GLOBAL_RESET		0x000
-#define PCIE_PHY_COMMON_RESET		0x004
-#define PCIE_PHY_CMN_REG		0x008
-#define PCIE_PHY_MAC_RESET		0x00c
-#define PCIE_PHY_PLL_LOCKED		0x010
-#define PCIE_PHY_TRSVREG_RESET		0x020
-#define PCIE_PHY_TRSV_RESET		0x024
-
-/* PCIe PHY registers */
-#define PCIE_PHY_IMPEDANCE		0x004
-#define PCIE_PHY_PLL_DIV_0		0x008
-#define PCIE_PHY_PLL_BIAS		0x00c
-#define PCIE_PHY_DCC_FEEDBACK		0x014
-#define PCIE_PHY_PLL_DIV_1		0x05c
-#define PCIE_PHY_TRSV0_EMP_LVL		0x084
-#define PCIE_PHY_TRSV0_DRV_LVL		0x088
-#define PCIE_PHY_TRSV0_RXCDR		0x0ac
-#define PCIE_PHY_TRSV0_LVCC		0x0dc
-#define PCIE_PHY_TRSV1_EMP_LVL		0x144
-#define PCIE_PHY_TRSV1_RXCDR		0x16c
-#define PCIE_PHY_TRSV1_LVCC		0x19c
-#define PCIE_PHY_TRSV2_EMP_LVL		0x204
-#define PCIE_PHY_TRSV2_RXCDR		0x22c
-#define PCIE_PHY_TRSV2_LVCC		0x25c
-#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
-#define PCIE_PHY_TRSV3_RXCDR		0x2ec
-#define PCIE_PHY_TRSV3_LVCC		0x31c
-
-static struct hw_pci exynos_pci;
+unsigned long global_io_offset;
 
 static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
 {
 	return sys->private_data;
 }
 
-static inline int cfg_read(void *addr, int where, int size, u32 *val)
+int cfg_read(void *addr, int where, int size, u32 *val)
 {
 	*val = readl(addr);
 
@@ -184,7 +83,7 @@  static inline int cfg_read(void *addr, int where, int size, u32 *val)
 	return PCIBIOS_SUCCESSFUL;
 }
 
-static inline int cfg_write(void *addr, int where, int size, u32 val)
+int cfg_write(void *addr, int where, int size, u32 val)
 {
 	if (size == 4)
 		writel(val, addr);
@@ -198,155 +97,230 @@  static inline int cfg_write(void *addr, int where, int size, u32 val)
 	return PCIBIOS_SUCCESSFUL;
 }
 
-static void exynos_pcie_sideband_dbi_w_mode(struct pcie_port *pp, bool on)
-{
-	u32 val;
-
-	if (on) {
-		val = readl(pp->elbi_base + PCIE_ELBI_SLV_AWMISC);
-		val |= PCIE_ELBI_SLV_DBI_ENABLE;
-		writel(val, pp->elbi_base + PCIE_ELBI_SLV_AWMISC);
-	} else {
-		val = readl(pp->elbi_base + PCIE_ELBI_SLV_AWMISC);
-		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
-		writel(val, pp->elbi_base + PCIE_ELBI_SLV_AWMISC);
-	}
-}
-
-static void exynos_pcie_sideband_dbi_r_mode(struct pcie_port *pp, bool on)
+static inline void dw_pcie_readl_rc(struct pcie_port *pp,
+				void __iomem *dbi_addr, u32 *val)
 {
-	u32 val;
-
-	if (on) {
-		val = readl(pp->elbi_base + PCIE_ELBI_SLV_ARMISC);
-		val |= PCIE_ELBI_SLV_DBI_ENABLE;
-		writel(val, pp->elbi_base + PCIE_ELBI_SLV_ARMISC);
-	} else {
-		val = readl(pp->elbi_base + PCIE_ELBI_SLV_ARMISC);
-		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
-		writel(val, pp->elbi_base + PCIE_ELBI_SLV_ARMISC);
-	}
-}
-
-static inline void readl_rc(struct pcie_port *pp, void *dbi_base, u32 *val)
-{
-	exynos_pcie_sideband_dbi_r_mode(pp, true);
-	*val = readl(dbi_base);
-	exynos_pcie_sideband_dbi_r_mode(pp, false);
-	return;
+	if (pp->ops->readl_rc)
+		pp->ops->readl_rc(pp, dbi_addr, val);
+	else
+		*val = readl(dbi_addr);
 }
 
-static inline void writel_rc(struct pcie_port *pp, u32 val, void *dbi_base)
+static inline void dw_pcie_writel_rc(struct pcie_port *pp,
+				u32 val, void __iomem *dbi_addr)
 {
-	exynos_pcie_sideband_dbi_w_mode(pp, true);
-	writel(val, dbi_base);
-	exynos_pcie_sideband_dbi_w_mode(pp, false);
-	return;
+	if (pp->ops->writel_rc)
+		pp->ops->writel_rc(pp, val, dbi_addr);
+	else
+		writel(val, dbi_addr);
 }
 
-static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
+int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
 				u32 *val)
 {
 	int ret;
 
-	exynos_pcie_sideband_dbi_r_mode(pp, true);
-	ret = cfg_read(pp->dbi_base + (where & ~0x3), where, size, val);
-	exynos_pcie_sideband_dbi_r_mode(pp, false);
+	if (pp->ops->rd_own_conf)
+		ret = pp->ops->rd_own_conf(pp, where, size, val);
+	else
+		ret = cfg_read(pp->dbi_base + (where & ~0x3), where, size, val);
+
 	return ret;
 }
 
-static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
+int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
 				u32 val)
 {
 	int ret;
 
-	exynos_pcie_sideband_dbi_w_mode(pp, true);
-	ret = cfg_write(pp->dbi_base + (where & ~0x3), where, size, val);
-	exynos_pcie_sideband_dbi_w_mode(pp, false);
+	if (pp->ops->wr_own_conf)
+		ret = pp->ops->wr_own_conf(pp, where, size, val);
+	else
+		ret = cfg_write(pp->dbi_base + (where & ~0x3), where, size,
+				val);
+
 	return ret;
 }
 
-static void exynos_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
+int dw_pcie_link_up(struct pcie_port *pp)
+{
+	if (pp->ops->link_up)
+		return pp->ops->link_up(pp);
+	else
+		return 0;
+}
+
+int dw_pcie_host_init(struct pcie_port *pp)
+{
+	struct device_node *np = pp->dev->of_node;
+	struct of_pci_range range;
+	struct of_pci_range_parser parser;
+	u32 val;
+
+	if (of_pci_range_parser_init(&parser, np)) {
+		dev_err(pp->dev, "missing ranges property\n");
+		return -EINVAL;
+	}
+
+	/* Get the I/O and memory ranges from DT */
+	for_each_of_pci_range(&parser, &range) {
+		unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
+		if (restype == IORESOURCE_IO) {
+			of_pci_range_to_resource(&range, np, &pp->io);
+			pp->io.name = "I/O";
+			pp->io.start = max_t(resource_size_t,
+					     PCIBIOS_MIN_IO,
+					     range.pci_addr + global_io_offset);
+			pp->io.end = min_t(resource_size_t,
+					   IO_SPACE_LIMIT,
+					   range.pci_addr + range.size
+					   + global_io_offset);
+			pp->config.io_size = resource_size(&pp->io);
+			pp->config.io_bus_addr = range.pci_addr;
+		}
+		if (restype == IORESOURCE_MEM) {
+			of_pci_range_to_resource(&range, np, &pp->mem);
+			pp->mem.name = "MEM";
+			pp->config.mem_size = resource_size(&pp->mem);
+			pp->config.mem_bus_addr = range.pci_addr;
+		}
+		if (restype == 0) {
+			of_pci_range_to_resource(&range, np, &pp->cfg);
+			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
+			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
+		}
+	}
+
+	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
+				resource_size(&pp->cfg));
+	if (!pp->dbi_base) {
+		dev_err(pp->dev, "error with ioremap\n");
+		return -ENOMEM;
+	}
+
+	pp->cfg0_base = pp->cfg.start;
+	pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
+	pp->io_base = pp->io.start;
+	pp->mem_base = pp->mem.start;
+
+	pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
+					pp->config.cfg0_size);
+	if (!pp->va_cfg0_base) {
+		dev_err(pp->dev, "error with ioremap in function\n");
+		return -ENOMEM;
+	}
+	pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
+					pp->config.cfg1_size);
+	if (!pp->va_cfg1_base) {
+		dev_err(pp->dev, "error with ioremap\n");
+		return -ENOMEM;
+	}
+
+	if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
+		dev_err(pp->dev, "Failed to parse the number of lanes\n");
+		return -EINVAL;
+	}
+
+	if (pp->ops->host_init)
+		pp->ops->host_init(pp);
+
+	dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);
+
+	/* program correct class for RC */
+	dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI);
+
+	dw_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val);
+	val |= PORT_LOGIC_SPEED_CHANGE;
+	dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
+
+	return 0;
+}
+
+static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
 {
 	u32 val;
 	void __iomem *dbi_base = pp->dbi_base;
 
 	/* Program viewport 0 : OUTBOUND : CFG0 */
 	val = PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0;
-	writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
-	writel_rc(pp, pp->cfg0_base, dbi_base + PCIE_ATU_LOWER_BASE);
-	writel_rc(pp, (pp->cfg0_base >> 32), dbi_base + PCIE_ATU_UPPER_BASE);
-	writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1,
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
+	dw_pcie_writel_rc(pp, pp->cfg0_base, dbi_base + PCIE_ATU_LOWER_BASE);
+	dw_pcie_writel_rc(pp, (pp->cfg0_base >> 32),
+			dbi_base + PCIE_ATU_UPPER_BASE);
+	dw_pcie_writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1,
 			dbi_base + PCIE_ATU_LIMIT);
-	writel_rc(pp, busdev, dbi_base + PCIE_ATU_LOWER_TARGET);
-	writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);
-	writel_rc(pp, PCIE_ATU_TYPE_CFG0, dbi_base + PCIE_ATU_CR1);
+	dw_pcie_writel_rc(pp, busdev, dbi_base + PCIE_ATU_LOWER_TARGET);
+	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);
+	dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG0, dbi_base + PCIE_ATU_CR1);
 	val = PCIE_ATU_ENABLE;
-	writel_rc(pp, val, dbi_base + PCIE_ATU_CR2);
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_CR2);
 }
 
-static void exynos_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev)
+static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev)
 {
 	u32 val;
 	void __iomem *dbi_base = pp->dbi_base;
 
 	/* Program viewport 1 : OUTBOUND : CFG1 */
 	val = PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX1;
-	writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
-	writel_rc(pp, PCIE_ATU_TYPE_CFG1, dbi_base + PCIE_ATU_CR1);
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
+	dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, dbi_base + PCIE_ATU_CR1);
 	val = PCIE_ATU_ENABLE;
-	writel_rc(pp, val, dbi_base + PCIE_ATU_CR2);
-	writel_rc(pp, pp->cfg1_base, dbi_base + PCIE_ATU_LOWER_BASE);
-	writel_rc(pp, (pp->cfg1_base >> 32), dbi_base + PCIE_ATU_UPPER_BASE);
-	writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1,
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_CR2);
+	dw_pcie_writel_rc(pp, pp->cfg1_base, dbi_base + PCIE_ATU_LOWER_BASE);
+	dw_pcie_writel_rc(pp, (pp->cfg1_base >> 32),
+			dbi_base + PCIE_ATU_UPPER_BASE);
+	dw_pcie_writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1,
 			dbi_base + PCIE_ATU_LIMIT);
-	writel_rc(pp, busdev, dbi_base + PCIE_ATU_LOWER_TARGET);
-	writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);
+	dw_pcie_writel_rc(pp, busdev, dbi_base + PCIE_ATU_LOWER_TARGET);
+	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET);
 }
 
-static void exynos_pcie_prog_viewport_mem_outbound(struct pcie_port *pp)
+static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp)
 {
 	u32 val;
 	void __iomem *dbi_base = pp->dbi_base;
 
 	/* Program viewport 0 : OUTBOUND : MEM */
 	val = PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0;
-	writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
-	writel_rc(pp, PCIE_ATU_TYPE_MEM, dbi_base + PCIE_ATU_CR1);
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
+	dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, dbi_base + PCIE_ATU_CR1);
 	val = PCIE_ATU_ENABLE;
-	writel_rc(pp, val, dbi_base + PCIE_ATU_CR2);
-	writel_rc(pp, pp->mem_base, dbi_base + PCIE_ATU_LOWER_BASE);
-	writel_rc(pp, (pp->mem_base >> 32), dbi_base + PCIE_ATU_UPPER_BASE);
-	writel_rc(pp, pp->mem_base + pp->config.mem_size - 1,
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_CR2);
+	dw_pcie_writel_rc(pp, pp->mem_base, dbi_base + PCIE_ATU_LOWER_BASE);
+	dw_pcie_writel_rc(pp, (pp->mem_base >> 32),
+			dbi_base + PCIE_ATU_UPPER_BASE);
+	dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1,
 			dbi_base + PCIE_ATU_LIMIT);
-	writel_rc(pp, pp->config.mem_bus_addr,
+	dw_pcie_writel_rc(pp, pp->config.mem_bus_addr,
 			dbi_base + PCIE_ATU_LOWER_TARGET);
-	writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr),
+	dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr),
 			dbi_base + PCIE_ATU_UPPER_TARGET);
 }
 
-static void exynos_pcie_prog_viewport_io_outbound(struct pcie_port *pp)
+static void dw_pcie_prog_viewport_io_outbound(struct pcie_port *pp)
 {
 	u32 val;
 	void __iomem *dbi_base = pp->dbi_base;
 
 	/* Program viewport 1 : OUTBOUND : IO */
 	val = PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX1;
-	writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
-	writel_rc(pp, PCIE_ATU_TYPE_IO, dbi_base + PCIE_ATU_CR1);
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT);
+	dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_IO, dbi_base + PCIE_ATU_CR1);
 	val = PCIE_ATU_ENABLE;
-	writel_rc(pp, val, dbi_base + PCIE_ATU_CR2);
-	writel_rc(pp, pp->io_base, dbi_base + PCIE_ATU_LOWER_BASE);
-	writel_rc(pp, (pp->io_base >> 32), dbi_base + PCIE_ATU_UPPER_BASE);
-	writel_rc(pp, pp->io_base + pp->config.io_size - 1,
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_CR2);
+	dw_pcie_writel_rc(pp, pp->io_base, dbi_base + PCIE_ATU_LOWER_BASE);
+	dw_pcie_writel_rc(pp, (pp->io_base >> 32),
+			dbi_base + PCIE_ATU_UPPER_BASE);
+	dw_pcie_writel_rc(pp, pp->io_base + pp->config.io_size - 1,
 			dbi_base + PCIE_ATU_LIMIT);
-	writel_rc(pp, pp->config.io_bus_addr,
+	dw_pcie_writel_rc(pp, pp->config.io_bus_addr,
 			dbi_base + PCIE_ATU_LOWER_TARGET);
-	writel_rc(pp, upper_32_bits(pp->config.io_bus_addr),
+	dw_pcie_writel_rc(pp, upper_32_bits(pp->config.io_bus_addr),
 			dbi_base + PCIE_ATU_UPPER_TARGET);
 }
 
-static int exynos_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 		u32 devfn, int where, int size, u32 *val)
 {
 	int ret = PCIBIOS_SUCCESSFUL;
@@ -357,19 +331,19 @@  static int exynos_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 	address = where & ~0x3;
 
 	if (bus->parent->number == pp->root_bus_nr) {
-		exynos_pcie_prog_viewport_cfg0(pp, busdev);
+		dw_pcie_prog_viewport_cfg0(pp, busdev);
 		ret = cfg_read(pp->va_cfg0_base + address, where, size, val);
-		exynos_pcie_prog_viewport_mem_outbound(pp);
+		dw_pcie_prog_viewport_mem_outbound(pp);
 	} else {
-		exynos_pcie_prog_viewport_cfg1(pp, busdev);
+		dw_pcie_prog_viewport_cfg1(pp, busdev);
 		ret = cfg_read(pp->va_cfg1_base + address, where, size, val);
-		exynos_pcie_prog_viewport_io_outbound(pp);
+		dw_pcie_prog_viewport_io_outbound(pp);
 	}
 
 	return ret;
 }
 
-static int exynos_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 		u32 devfn, int where, int size, u32 val)
 {
 	int ret = PCIBIOS_SUCCESSFUL;
@@ -380,59 +354,25 @@  static int exynos_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 	address = where & ~0x3;
 
 	if (bus->parent->number == pp->root_bus_nr) {
-		exynos_pcie_prog_viewport_cfg0(pp, busdev);
+		dw_pcie_prog_viewport_cfg0(pp, busdev);
 		ret = cfg_write(pp->va_cfg0_base + address, where, size, val);
-		exynos_pcie_prog_viewport_mem_outbound(pp);
+		dw_pcie_prog_viewport_mem_outbound(pp);
 	} else {
-		exynos_pcie_prog_viewport_cfg1(pp, busdev);
+		dw_pcie_prog_viewport_cfg1(pp, busdev);
 		ret = cfg_write(pp->va_cfg1_base + address, where, size, val);
-		exynos_pcie_prog_viewport_io_outbound(pp);
+		dw_pcie_prog_viewport_io_outbound(pp);
 	}
 
 	return ret;
 }
 
-static unsigned long global_io_offset;
-
-static int exynos_pcie_setup(int nr, struct pci_sys_data *sys)
-{
-	struct pcie_port *pp;
-
-	pp = sys_to_pcie(sys);
-
-	if (!pp)
-		return 0;
-
-	if (global_io_offset < SZ_1M && pp->config.io_size > 0) {
-		sys->io_offset = global_io_offset - pp->config.io_bus_addr;
-		pci_ioremap_io(sys->io_offset, pp->io.start);
-		global_io_offset += SZ_64K;
-		pci_add_resource_offset(&sys->resources, &pp->io,
-					sys->io_offset);
-	}
-
-	sys->mem_offset = pp->mem.start - pp->config.mem_bus_addr;
-	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
-
-	return 1;
-}
-
-static int exynos_pcie_link_up(struct pcie_port *pp)
-{
-	u32 val = readl(pp->elbi_base + PCIE_ELBI_RDLH_LINKUP);
-
-	if (val == PCIE_ELBI_LTSSM_ENABLE)
-		return 1;
 
-	return 0;
-}
-
-static int exynos_pcie_valid_config(struct pcie_port *pp,
+static int dw_pcie_valid_config(struct pcie_port *pp,
 				struct pci_bus *bus, int dev)
 {
 	/* If there is no link, then there is no device */
 	if (bus->number != pp->root_bus_nr) {
-		if (!exynos_pcie_link_up(pp))
+		if (!dw_pcie_link_up(pp))
 			return 0;
 	}
 
@@ -450,7 +390,7 @@  static int exynos_pcie_valid_config(struct pcie_port *pp,
 	return 1;
 }
 
-static int exynos_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
+static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 			int size, u32 *val)
 {
 	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
@@ -462,23 +402,23 @@  static int exynos_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 		return -EINVAL;
 	}
 
-	if (exynos_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
+	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
 		*val = 0xffffffff;
 		return PCIBIOS_DEVICE_NOT_FOUND;
 	}
 
 	spin_lock_irqsave(&pp->conf_lock, flags);
 	if (bus->number != pp->root_bus_nr)
-		ret = exynos_pcie_rd_other_conf(pp, bus, devfn,
+		ret = dw_pcie_rd_other_conf(pp, bus, devfn,
 						where, size, val);
 	else
-		ret = exynos_pcie_rd_own_conf(pp, where, size, val);
+		ret = dw_pcie_rd_own_conf(pp, where, size, val);
 	spin_unlock_irqrestore(&pp->conf_lock, flags);
 
 	return ret;
 }
 
-static int exynos_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
+static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 			int where, int size, u32 val)
 {
 	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
@@ -490,34 +430,56 @@  static int exynos_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 		return -EINVAL;
 	}
 
-	if (exynos_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
+	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
 	spin_lock_irqsave(&pp->conf_lock, flags);
 	if (bus->number != pp->root_bus_nr)
-		ret = exynos_pcie_wr_other_conf(pp, bus, devfn,
+		ret = dw_pcie_wr_other_conf(pp, bus, devfn,
 						where, size, val);
 	else
-		ret = exynos_pcie_wr_own_conf(pp, where, size, val);
+		ret = dw_pcie_wr_own_conf(pp, where, size, val);
 	spin_unlock_irqrestore(&pp->conf_lock, flags);
 
 	return ret;
 }
 
-static struct pci_ops exynos_pcie_ops = {
-	.read = exynos_pcie_rd_conf,
-	.write = exynos_pcie_wr_conf,
+static struct pci_ops dw_pcie_ops = {
+	.read = dw_pcie_rd_conf,
+	.write = dw_pcie_wr_conf,
 };
 
-static struct pci_bus *exynos_pcie_scan_bus(int nr,
-					struct pci_sys_data *sys)
+int dw_pcie_setup(int nr, struct pci_sys_data *sys)
+{
+	struct pcie_port *pp;
+
+	pp = sys_to_pcie(sys);
+
+	if (!pp)
+		return 0;
+
+	if (global_io_offset < SZ_1M && pp->config.io_size > 0) {
+		sys->io_offset = global_io_offset - pp->config.io_bus_addr;
+		pci_ioremap_io(sys->io_offset, pp->io.start);
+		global_io_offset += SZ_64K;
+		pci_add_resource_offset(&sys->resources, &pp->io,
+					sys->io_offset);
+	}
+
+	sys->mem_offset = pp->mem.start - pp->config.mem_bus_addr;
+	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
+
+	return 1;
+}
+
+struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 {
 	struct pci_bus *bus;
 	struct pcie_port *pp = sys_to_pcie(sys);
 
 	if (pp) {
 		pp->root_bus_nr = sys->busnr;
-		bus = pci_scan_root_bus(NULL, sys->busnr, &exynos_pcie_ops,
+		bus = pci_scan_root_bus(NULL, sys->busnr, &dw_pcie_ops,
 					sys, &sys->resources);
 	} else {
 		bus = NULL;
@@ -527,20 +489,20 @@  static struct pci_bus *exynos_pcie_scan_bus(int nr,
 	return bus;
 }
 
-static int exynos_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 {
 	struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
 
 	return pp->irq;
 }
 
-static struct hw_pci exynos_pci = {
-	.setup		= exynos_pcie_setup,
-	.scan		= exynos_pcie_scan_bus,
-	.map_irq	= exynos_pcie_map_irq,
+struct hw_pci dw_pci = {
+	.setup		= dw_pcie_setup,
+	.scan		= dw_pcie_scan_bus,
+	.map_irq	= dw_pcie_map_irq,
 };
 
-static void exynos_pcie_setup_rc(struct pcie_port *pp)
+void dw_pcie_setup_rc(struct pcie_port *pp)
 {
 	struct pcie_port_info *config = &pp->config;
 	void __iomem *dbi_base = pp->dbi_base;
@@ -549,509 +511,67 @@  static void exynos_pcie_setup_rc(struct pcie_port *pp)
 	u32 memlimit;
 
 	/* set the number of lines as 4 */
-	readl_rc(pp, dbi_base + PCIE_PORT_LINK_CONTROL, &val);
+	dw_pcie_readl_rc(pp, dbi_base + PCIE_PORT_LINK_CONTROL, &val);
 	val &= ~PORT_LINK_MODE_MASK;
-	val |= PORT_LINK_MODE_4_LANES;
-	writel_rc(pp, val, dbi_base + PCIE_PORT_LINK_CONTROL);
+	switch (pp->lanes) {
+	case 1:
+		val |= PORT_LINK_MODE_1_LANES;
+		break;
+	case 2:
+		val |= PORT_LINK_MODE_2_LANES;
+		break;
+	case 4:
+		val |= PORT_LINK_MODE_4_LANES;
+		break;
+	}
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_PORT_LINK_CONTROL);
 
 	/* set link width speed control register */
-	readl_rc(pp, dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL, &val);
+	dw_pcie_readl_rc(pp, dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL, &val);
 	val &= ~PORT_LOGIC_LINK_WIDTH_MASK;
-	val |= PORT_LOGIC_LINK_WIDTH_4_LANES;
-	writel_rc(pp, val, dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
+	switch (pp->lanes) {
+	case 1:
+		val |= PORT_LOGIC_LINK_WIDTH_1_LANES;
+		break;
+	case 2:
+		val |= PORT_LOGIC_LINK_WIDTH_2_LANES;
+		break;
+	case 4:
+		val |= PORT_LOGIC_LINK_WIDTH_4_LANES;
+		break;
+	}
+	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
 
 	/* setup RC BARs */
-	writel_rc(pp, 0x00000004, dbi_base + PCI_BASE_ADDRESS_0);
-	writel_rc(pp, 0x00000004, dbi_base + PCI_BASE_ADDRESS_1);
+	dw_pcie_writel_rc(pp, 0x00000004, dbi_base + PCI_BASE_ADDRESS_0);
+	dw_pcie_writel_rc(pp, 0x00000004, dbi_base + PCI_BASE_ADDRESS_1);
 
 	/* setup interrupt pins */
-	readl_rc(pp, dbi_base + PCI_INTERRUPT_LINE, &val);
+	dw_pcie_readl_rc(pp, dbi_base + PCI_INTERRUPT_LINE, &val);
 	val &= 0xffff00ff;
 	val |= 0x00000100;
-	writel_rc(pp, val, dbi_base + PCI_INTERRUPT_LINE);
+	dw_pcie_writel_rc(pp, val, dbi_base + PCI_INTERRUPT_LINE);
 
 	/* setup bus numbers */
-	readl_rc(pp, dbi_base + PCI_PRIMARY_BUS, &val);
+	dw_pcie_readl_rc(pp, dbi_base + PCI_PRIMARY_BUS, &val);
 	val &= 0xff000000;
 	val |= 0x00010100;
-	writel_rc(pp, val, dbi_base + PCI_PRIMARY_BUS);
+	dw_pcie_writel_rc(pp, val, dbi_base + PCI_PRIMARY_BUS);
 
 	/* setup memory base, memory limit */
 	membase = ((u32)pp->mem_base & 0xfff00000) >> 16;
 	memlimit = (config->mem_size + (u32)pp->mem_base) & 0xfff00000;
 	val = memlimit | membase;
-	writel_rc(pp, val, dbi_base + PCI_MEMORY_BASE);
+	dw_pcie_writel_rc(pp, val, dbi_base + PCI_MEMORY_BASE);
 
 	/* setup command register */
-	readl_rc(pp, dbi_base + PCI_COMMAND, &val);
+	dw_pcie_readl_rc(pp, dbi_base + PCI_COMMAND, &val);
 	val &= 0xffff0000;
 	val |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
 		PCI_COMMAND_MASTER | PCI_COMMAND_SERR;
-	writel_rc(pp, val, dbi_base + PCI_COMMAND);
-}
-
-static void exynos_pcie_assert_core_reset(struct pcie_port *pp)
-{
-	u32 val;
-	void __iomem *elbi_base = pp->elbi_base;
-
-	val = readl(elbi_base + PCIE_CORE_RESET);
-	val &= ~PCIE_CORE_RESET_ENABLE;
-	writel(val, elbi_base + PCIE_CORE_RESET);
-	writel(0, elbi_base + PCIE_PWR_RESET);
-	writel(0, elbi_base + PCIE_STICKY_RESET);
-	writel(0, elbi_base + PCIE_NONSTICKY_RESET);
-}
-
-static void exynos_pcie_deassert_core_reset(struct pcie_port *pp)
-{
-	u32 val;
-	void __iomem *elbi_base = pp->elbi_base;
-	void __iomem *purple_base = pp->purple_base;
-
-	val = readl(elbi_base + PCIE_CORE_RESET);
-	val |= PCIE_CORE_RESET_ENABLE;
-	writel(val, elbi_base + PCIE_CORE_RESET);
-	writel(1, elbi_base + PCIE_STICKY_RESET);
-	writel(1, elbi_base + PCIE_NONSTICKY_RESET);
-	writel(1, elbi_base + PCIE_APP_INIT_RESET);
-	writel(0, elbi_base + PCIE_APP_INIT_RESET);
-	writel(1, purple_base + PCIE_PHY_MAC_RESET);
-}
-
-static void exynos_pcie_assert_phy_reset(struct pcie_port *pp)
-{
-	void __iomem *purple_base = pp->purple_base;
-
-	writel(0, purple_base + PCIE_PHY_MAC_RESET);
-	writel(1, purple_base + PCIE_PHY_GLOBAL_RESET);
-}
-
-static void exynos_pcie_deassert_phy_reset(struct pcie_port *pp)
-{
-	void __iomem *elbi_base = pp->elbi_base;
-	void __iomem *purple_base = pp->purple_base;
-
-	writel(0, purple_base + PCIE_PHY_GLOBAL_RESET);
-	writel(1, elbi_base + PCIE_PWR_RESET);
-	writel(0, purple_base + PCIE_PHY_COMMON_RESET);
-	writel(0, purple_base + PCIE_PHY_CMN_REG);
-	writel(0, purple_base + PCIE_PHY_TRSVREG_RESET);
-	writel(0, purple_base + PCIE_PHY_TRSV_RESET);
-}
-
-static void exynos_pcie_init_phy(struct pcie_port *pp)
-{
-	void __iomem *phy_base = pp->phy_base;
-
-	/* DCC feedback control off */
-	writel(0x29, phy_base + PCIE_PHY_DCC_FEEDBACK);
-
-	/* set TX/RX impedance */
-	writel(0xd5, phy_base + PCIE_PHY_IMPEDANCE);
-
-	/* set 50Mhz PHY clock */
-	writel(0x14, phy_base + PCIE_PHY_PLL_DIV_0);
-	writel(0x12, phy_base + PCIE_PHY_PLL_DIV_1);
-
-	/* set TX Differential output for lane 0 */
-	writel(0x7f, phy_base + PCIE_PHY_TRSV0_DRV_LVL);
-
-	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
-	writel(0x0, phy_base + PCIE_PHY_TRSV0_EMP_LVL);
-
-	/* set RX clock and data recovery bandwidth */
-	writel(0xe7, phy_base + PCIE_PHY_PLL_BIAS);
-	writel(0x82, phy_base + PCIE_PHY_TRSV0_RXCDR);
-	writel(0x82, phy_base + PCIE_PHY_TRSV1_RXCDR);
-	writel(0x82, phy_base + PCIE_PHY_TRSV2_RXCDR);
-	writel(0x82, phy_base + PCIE_PHY_TRSV3_RXCDR);
-
-	/* change TX Pre-emphasis Level Control for lanes */
-	writel(0x39, phy_base + PCIE_PHY_TRSV0_EMP_LVL);
-	writel(0x39, phy_base + PCIE_PHY_TRSV1_EMP_LVL);
-	writel(0x39, phy_base + PCIE_PHY_TRSV2_EMP_LVL);
-	writel(0x39, phy_base + PCIE_PHY_TRSV3_EMP_LVL);
-
-	/* set LVCC */
-	writel(0x20, phy_base + PCIE_PHY_TRSV0_LVCC);
-	writel(0xa0, phy_base + PCIE_PHY_TRSV1_LVCC);
-	writel(0xa0, phy_base + PCIE_PHY_TRSV2_LVCC);
-	writel(0xa0, phy_base + PCIE_PHY_TRSV3_LVCC);
-}
-
-static void exynos_pcie_assert_reset(struct pcie_port *pp)
-{
-	if (pp->reset_gpio >= 0)
-		devm_gpio_request_one(pp->dev, pp->reset_gpio,
-				GPIOF_OUT_INIT_HIGH, "RESET");
-	return;
-}
-
-static int exynos_pcie_establish_link(struct pcie_port *pp)
-{
-	u32 val;
-	int count = 0;
-	void __iomem *elbi_base = pp->elbi_base;
-	void __iomem *purple_base = pp->purple_base;
-	void __iomem *phy_base = pp->phy_base;
-
-	if (exynos_pcie_link_up(pp)) {
-		dev_err(pp->dev, "Link already up\n");
-		return 0;
-	}
-
-	/* assert reset signals */
-	exynos_pcie_assert_core_reset(pp);
-	exynos_pcie_assert_phy_reset(pp);
-
-	/* de-assert phy reset */
-	exynos_pcie_deassert_phy_reset(pp);
-
-	/* initialize phy */
-	exynos_pcie_init_phy(pp);
-
-	/* pulse for common reset */
-	writel(1, purple_base + PCIE_PHY_COMMON_RESET);
-	udelay(500);
-	writel(0, purple_base + PCIE_PHY_COMMON_RESET);
-
-	/* de-assert core reset */
-	exynos_pcie_deassert_core_reset(pp);
-
-	/* setup root complex */
-	exynos_pcie_setup_rc(pp);
-
-	/* assert reset signal */
-	exynos_pcie_assert_reset(pp);
-
-	/* assert LTSSM enable */
-	writel(PCIE_ELBI_LTSSM_ENABLE, elbi_base + PCIE_APP_LTSSM_ENABLE);
-
-	/* check if the link is up or not */
-	while (!exynos_pcie_link_up(pp)) {
-		mdelay(100);
-		count++;
-		if (count == 10) {
-			while (readl(phy_base + PCIE_PHY_PLL_LOCKED) == 0) {
-				val = readl(purple_base + PCIE_PHY_PLL_LOCKED);
-				dev_info(pp->dev, "PLL Locked: 0x%x\n", val);
-			}
-			dev_err(pp->dev, "PCIe Link Fail\n");
-			return -EINVAL;
-		}
-	}
-
-	dev_info(pp->dev, "Link up\n");
-
-	return 0;
-}
-
-static void exynos_pcie_clear_irq_pulse(struct pcie_port *pp)
-{
-	u32 val;
-	void __iomem *elbi_base = pp->elbi_base;
-
-	val = readl(elbi_base + PCIE_IRQ_PULSE);
-	writel(val, elbi_base + PCIE_IRQ_PULSE);
-	return;
-}
-
-static void exynos_pcie_enable_irq_pulse(struct pcie_port *pp)
-{
-	u32 val;
-	void __iomem *elbi_base = pp->elbi_base;
-
-	/* enable INTX interrupt */
-	val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
-		IRQ_INTC_ASSERT | IRQ_INTD_ASSERT,
-	writel(val, elbi_base + PCIE_IRQ_EN_PULSE);
-	return;
-}
-
-static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
-{
-	struct pcie_port *pp = arg;
-
-	exynos_pcie_clear_irq_pulse(pp);
-	return IRQ_HANDLED;
-}
-
-static void exynos_pcie_enable_interrupts(struct pcie_port *pp)
-{
-	exynos_pcie_enable_irq_pulse(pp);
-	return;
-}
-
-static void exynos_pcie_host_init(struct pcie_port *pp)
-{
-	struct pcie_port_info *config = &pp->config;
-	u32 val;
-
-	/* Keep first 64K for IO */
-	pp->cfg0_base = pp->cfg.start;
-	pp->cfg1_base = pp->cfg.start + config->cfg0_size;
-	pp->io_base = pp->io.start;
-	pp->mem_base = pp->mem.start;
-
-	/* enable link */
-	exynos_pcie_establish_link(pp);
-
-	exynos_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);
-
-	/* program correct class for RC */
-	exynos_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI);
-
-	exynos_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val);
-	val |= PORT_LOGIC_SPEED_CHANGE;
-	exynos_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
-
-	exynos_pcie_enable_interrupts(pp);
-}
-
-static int add_pcie_port(struct pcie_port *pp, struct platform_device *pdev)
-{
-	struct resource *elbi_base;
-	struct resource *phy_base;
-	struct resource *purple_base;
-	int ret;
-
-	elbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!elbi_base) {
-		dev_err(&pdev->dev, "couldn't get elbi base resource\n");
-		return -EINVAL;
-	}
-	pp->elbi_base = devm_ioremap_resource(&pdev->dev, elbi_base);
-	if (IS_ERR(pp->elbi_base))
-		return PTR_ERR(pp->elbi_base);
-
-	phy_base = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!phy_base) {
-		dev_err(&pdev->dev, "couldn't get phy base resource\n");
-		return -EINVAL;
-	}
-	pp->phy_base = devm_ioremap_resource(&pdev->dev, phy_base);
-	if (IS_ERR(pp->phy_base))
-		return PTR_ERR(pp->phy_base);
-
-	purple_base = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-	if (!purple_base) {
-		dev_err(&pdev->dev, "couldn't get purple base resource\n");
-		return -EINVAL;
-	}
-	pp->purple_base = devm_ioremap_resource(&pdev->dev, purple_base);
-	if (IS_ERR(pp->purple_base))
-		return PTR_ERR(pp->purple_base);
-
-	pp->irq = platform_get_irq(pdev, 1);
-	if (!pp->irq) {
-		dev_err(&pdev->dev, "failed to get irq\n");
-		return -ENODEV;
-	}
-	ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler,
-				IRQF_SHARED, "exynos-pcie", pp);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to request irq\n");
-		return ret;
-	}
-
-	pp->dbi_base = devm_ioremap(&pdev->dev, pp->cfg.start,
-				resource_size(&pp->cfg));
-	if (!pp->dbi_base) {
-		dev_err(&pdev->dev, "error with ioremap\n");
-		return -ENOMEM;
-	}
-
-	pp->root_bus_nr = -1;
-
-	spin_lock_init(&pp->conf_lock);
-	exynos_pcie_host_init(pp);
-	pp->va_cfg0_base = devm_ioremap(&pdev->dev, pp->cfg0_base,
-					pp->config.cfg0_size);
-	if (!pp->va_cfg0_base) {
-		dev_err(pp->dev, "error with ioremap in function\n");
-		return -ENOMEM;
-	}
-	pp->va_cfg1_base = devm_ioremap(&pdev->dev, pp->cfg1_base,
-					pp->config.cfg1_size);
-	if (!pp->va_cfg1_base) {
-		dev_err(pp->dev, "error with ioremap\n");
-		return -ENOMEM;
-	}
-
-	return 0;
-}
-
-static int __init exynos_pcie_probe(struct platform_device *pdev)
-{
-	struct pcie_port *pp;
-	struct device_node *np = pdev->dev.of_node;
-	struct of_pci_range range;
-	struct of_pci_range_parser parser;
-	int ret;
-
-	pp = devm_kzalloc(&pdev->dev, sizeof(*pp), GFP_KERNEL);
-	if (!pp) {
-		dev_err(&pdev->dev, "no memory for pcie port\n");
-		return -ENOMEM;
-	}
-
-	pp->dev = &pdev->dev;
-
-	if (of_pci_range_parser_init(&parser, np)) {
-		dev_err(&pdev->dev, "missing ranges property\n");
-		return -EINVAL;
-	}
-
-	/* Get the I/O and memory ranges from DT */
-	for_each_of_pci_range(&parser, &range) {
-		unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
-		if (restype == IORESOURCE_IO) {
-			of_pci_range_to_resource(&range, np, &pp->io);
-			pp->io.name = "I/O";
-			pp->io.start = max_t(resource_size_t,
-					     PCIBIOS_MIN_IO,
-					     range.pci_addr + global_io_offset);
-			pp->io.end = min_t(resource_size_t,
-					   IO_SPACE_LIMIT,
-					   range.pci_addr + range.size
-					   + global_io_offset);
-			pp->config.io_size = resource_size(&pp->io);
-			pp->config.io_bus_addr = range.pci_addr;
-		}
-		if (restype == IORESOURCE_MEM) {
-			of_pci_range_to_resource(&range, np, &pp->mem);
-			pp->mem.name = "MEM";
-			pp->config.mem_size = resource_size(&pp->mem);
-			pp->config.mem_bus_addr = range.pci_addr;
-		}
-		if (restype == 0) {
-			of_pci_range_to_resource(&range, np, &pp->cfg);
-			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
-			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
-		}
-	}
-
-	pp->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
-
-	pp->clk = devm_clk_get(&pdev->dev, "pcie");
-	if (IS_ERR(pp->clk)) {
-		dev_err(&pdev->dev, "Failed to get pcie rc clock\n");
-		return PTR_ERR(pp->clk);
-	}
-	ret = clk_prepare_enable(pp->clk);
-	if (ret)
-		return ret;
-
-	pp->bus_clk = devm_clk_get(&pdev->dev, "pcie_bus");
-	if (IS_ERR(pp->bus_clk)) {
-		dev_err(&pdev->dev, "Failed to get pcie bus clock\n");
-		ret = PTR_ERR(pp->bus_clk);
-		goto fail_clk;
-	}
-	ret = clk_prepare_enable(pp->bus_clk);
-	if (ret)
-		goto fail_clk;
-
-	ret = add_pcie_port(pp, pdev);
-	if (ret < 0)
-		goto fail_bus_clk;
-
-	pp->controller = exynos_pci.nr_controllers;
-	exynos_pci.nr_controllers = 1;
-	exynos_pci.private_data = (void **)&pp;
-
-	pci_common_init(&exynos_pci);
-	pci_assign_unassigned_resources();
-#ifdef CONFIG_PCI_DOMAINS
-	exynos_pci.domain++;
-#endif
-
-	platform_set_drvdata(pdev, pp);
-	return 0;
-
-fail_bus_clk:
-	clk_disable_unprepare(pp->bus_clk);
-fail_clk:
-	clk_disable_unprepare(pp->clk);
-	return ret;
-}
-
-static int __exit exynos_pcie_remove(struct platform_device *pdev)
-{
-	struct pcie_port *pp = platform_get_drvdata(pdev);
-
-	clk_disable_unprepare(pp->bus_clk);
-	clk_disable_unprepare(pp->clk);
-
-	return 0;
-}
-
-static const struct of_device_id exynos_pcie_of_match[] = {
-	{ .compatible = "samsung,exynos5440-pcie", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, exynos_pcie_of_match);
-
-static struct platform_driver exynos_pcie_driver = {
-	.remove		= __exit_p(exynos_pcie_remove),
-	.driver = {
-		.name	= "exynos-pcie",
-		.owner	= THIS_MODULE,
-		.of_match_table = of_match_ptr(exynos_pcie_of_match),
-	},
-};
-
-static int exynos_pcie_abort(unsigned long addr, unsigned int fsr,
-			struct pt_regs *regs)
-{
-	unsigned long pc = instruction_pointer(regs);
-	unsigned long instr = *(unsigned long *)pc;
-
-	WARN_ONCE(1, "pcie abort\n");
-
-	/*
-	 * If the instruction being executed was a read,
-	 * make it look like it read all-ones.
-	 */
-	if ((instr & 0x0c100000) == 0x04100000) {
-		int reg = (instr >> 12) & 15;
-		unsigned long val;
-
-		if (instr & 0x00400000)
-			val = 255;
-		else
-			val = -1;
-
-		regs->uregs[reg] = val;
-		regs->ARM_pc += 4;
-		return 0;
-	}
-
-	if ((instr & 0x0e100090) == 0x00100090) {
-		int reg = (instr >> 12) & 15;
-
-		regs->uregs[reg] = -1;
-		regs->ARM_pc += 4;
-		return 0;
-	}
-
-	return 1;
-}
-
-/* Exynos PCIe driver does not allow module unload */
-
-static int __init pcie_init(void)
-{
-	hook_fault_code(16 + 6, exynos_pcie_abort, SIGBUS, 0,
-			"imprecise external abort");
-
-	platform_driver_probe(&exynos_pcie_driver, exynos_pcie_probe);
-
-	return 0;
+	dw_pcie_writel_rc(pp, val, dbi_base + PCI_COMMAND);
 }
-subsys_initcall(pcie_init);
 
 MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
-MODULE_DESCRIPTION("Samsung PCIe host controller driver");
+MODULE_DESCRIPTION("Designware PCIe host controller driver");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
new file mode 100644
index 0000000..4ed10e2
--- /dev/null
+++ b/drivers/pci/host/pcie-designware.h
@@ -0,0 +1,65 @@ 
+/*
+ * Synopsys Designware PCIe host controller driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Author: Jingoo Han <jg1.han@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+struct pcie_port_info {
+	u32		cfg0_size;
+	u32		cfg1_size;
+	u32		io_size;
+	u32		mem_size;
+	phys_addr_t	io_bus_addr;
+	phys_addr_t	mem_bus_addr;
+};
+
+struct pcie_port {
+	struct device		*dev;
+	u8			root_bus_nr;
+	void __iomem		*dbi_base;
+	u64			cfg0_base;
+	void __iomem		*va_cfg0_base;
+	u64			cfg1_base;
+	void __iomem		*va_cfg1_base;
+	u64			io_base;
+	u64			mem_base;
+	spinlock_t		conf_lock;
+	struct resource		cfg;
+	struct resource		io;
+	struct resource		mem;
+	struct pcie_port_info	config;
+	int			irq;
+	u32			lanes;
+	struct pcie_host_ops	*ops;
+};
+
+struct pcie_host_ops {
+	void (*readl_rc)(struct pcie_port *pp, void *dbi_base, u32 *val);
+	void (*writel_rc)(struct pcie_port *pp, u32 val, void *dbi_base);
+	int (*rd_own_conf)(struct pcie_port *pp, int where, int size, u32 *val);
+	int (*wr_own_conf)(struct pcie_port *pp, int where, int size, u32 val);
+	int (*link_up)(struct pcie_port *pp);
+	void (*host_init)(struct pcie_port *pp);
+};
+
+extern struct hw_pci dw_pci;
+
+extern unsigned long global_io_offset;
+
+int cfg_read(void *addr, int where, int size, u32 *val);
+int cfg_write(void *addr, int where, int size, u32 val);
+int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size, u32 val);
+int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size, u32 *val);
+int dw_pcie_link_up(struct pcie_port *pp);
+void dw_pcie_setup_rc(struct pcie_port *pp);
+int dw_pcie_host_init(struct pcie_port *pp);
+int dw_pcie_setup(int nr, struct pci_sys_data *sys);
+struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys);
+int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin);