[PATCH v2 00/17] dwc MSI fixes, ARTPEC-6 EP mode support, ARTPEC-7 SoC support

[PATCH v2 00/17] dwc MSI fixes, ARTPEC-6 EP mode support, ARTPEC-7 SoC support

[Niklas Cassel]

This is a series that adds:
- PCI endpoint mode support in the ARTPEC-6 driver.
- ARTPEC-7 SoC support in the ARTPEC-6 driver (the SoCs are very similar).
- Small fixes for MSI in designware-ep and designware-host,
  needed to get endpoint mode support working for ARTPEC-6.

New in V2:
Capitalized first letter in summary for all patches.
Moved DT binding changes to self contained patches.
Fixed all review comments, for detailed changes, see the note for in patch.

Niklas Cassel (17):
  PCI: dwc: Use DMA-API for allocating MSI data
  PCI: designware-ep: dw_pcie_ep_set_msi() should only set MMC bits
  PCI: designware-ep: Read-only registers need DBI_RO_WR_EN to be
    writable
  PCI: designware-ep: Pre-allocate memory for MSI in dw_pcie_ep_init
  PCI: designware-ep: Remove static keyword from dw_pcie_ep_reset_bar()
  PCI: designware-ep: Add generic function for raising MSI irq
  PCI: dwc: dra7xx: Refactor Kconfig and Makefile handling for host/ep
    mode
  PCI: dwc: dra7xx: Assign pp->ops in dra7xx_add_pcie_port() rather than
    in probe
  PCI: dwc: dra7xx: Add ifdefs for host/ep specific code
  PCI: dwc: artpec6: Remove unused defines
  PCI: dwc: artpec6: Use BIT and GENMASK macros
  PCI: dwc: artpec6: Split artpec6_pcie_establish_link to smaller
    functions
  bindings: PCI: artpec: Add support for endpoint mode
  PCI: dwc: artpec6: Add support for endpoint mode
  PCI: dwc: Make cpu_addr_fixup take struct dw_pcie as argument
  bindings: PCI: artpec: Add support for the ARTPEC-7 SoC
  PCI: dwc: artpec6: Add support for the ARTPEC-7 SoC

 .../devicetree/bindings/pci/axis,artpec6-pcie.txt  |   5 +-
 drivers/pci/dwc/Kconfig                            |  68 ++--
 drivers/pci/dwc/Makefile                           |   4 +-
 drivers/pci/dwc/pci-dra7xx.c                       |  53 +--
 drivers/pci/dwc/pcie-artpec6.c                     | 416 ++++++++++++++++++---
 drivers/pci/dwc/pcie-designware-ep.c               |  56 ++-
 drivers/pci/dwc/pcie-designware-host.c             |  15 +-
 drivers/pci/dwc/pcie-designware.c                  |   2 +-
 drivers/pci/dwc/pcie-designware.h                  |  22 +-
 9 files changed, 530 insertions(+), 111 deletions(-)

-- 
2.14.2

[PATCH v2 00/17] dwc MSI fixes, ARTPEC-6 EP mode support, ARTPEC-7 SoC support

[Niklas Cassel]

This is a series that adds:
- PCI endpoint mode support in the ARTPEC-6 driver.
- ARTPEC-7 SoC support in the ARTPEC-6 driver (the SoCs are very similar).
- Small fixes for MSI in designware-ep and designware-host,
  needed to get endpoint mode support working for ARTPEC-6.

New in V2:
Capitalized first letter in summary for all patches.
Moved DT binding changes to self contained patches.
Fixed all review comments, for detailed changes, see the note for in patch.

Niklas Cassel (17):
  PCI: dwc: Use DMA-API for allocating MSI data
  PCI: designware-ep: dw_pcie_ep_set_msi() should only set MMC bits
  PCI: designware-ep: Read-only registers need DBI_RO_WR_EN to be
    writable
  PCI: designware-ep: Pre-allocate memory for MSI in dw_pcie_ep_init
  PCI: designware-ep: Remove static keyword from dw_pcie_ep_reset_bar()
  PCI: designware-ep: Add generic function for raising MSI irq
  PCI: dwc: dra7xx: Refactor Kconfig and Makefile handling for host/ep
    mode
  PCI: dwc: dra7xx: Assign pp->ops in dra7xx_add_pcie_port() rather than
    in probe
  PCI: dwc: dra7xx: Add ifdefs for host/ep specific code
  PCI: dwc: artpec6: Remove unused defines
  PCI: dwc: artpec6: Use BIT and GENMASK macros
  PCI: dwc: artpec6: Split artpec6_pcie_establish_link to smaller
    functions
  bindings: PCI: artpec: Add support for endpoint mode
  PCI: dwc: artpec6: Add support for endpoint mode
  PCI: dwc: Make cpu_addr_fixup take struct dw_pcie as argument
  bindings: PCI: artpec: Add support for the ARTPEC-7 SoC
  PCI: dwc: artpec6: Add support for the ARTPEC-7 SoC

 .../devicetree/bindings/pci/axis,artpec6-pcie.txt  |   5 +-
 drivers/pci/dwc/Kconfig                            |  68 ++--
 drivers/pci/dwc/Makefile                           |   4 +-
 drivers/pci/dwc/pci-dra7xx.c                       |  53 +--
 drivers/pci/dwc/pcie-artpec6.c                     | 416 ++++++++++++++++++---
 drivers/pci/dwc/pcie-designware-ep.c               |  56 ++-
 drivers/pci/dwc/pcie-designware-host.c             |  15 +-
 drivers/pci/dwc/pcie-designware.c                  |   2 +-
 drivers/pci/dwc/pcie-designware.h                  |  22 +-
 9 files changed, 530 insertions(+), 111 deletions(-)

-- 
2.14.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 01/17] PCI: dwc: Use DMA-API for allocating MSI data

[Niklas Cassel]

Since it is a PCIe endpoint device, rather than the CPU, that is supposed
to write to this location, the proper way to get the address to this this
location is really to use the DMA API, rather than virt_to_phys.

Using virt_to_phys might work on some systems, but by using the DMA API,
we know that it will work on all systems.

This is essentially the same thing as allocating a buffer in a driver,
to which the endpoint will write to. To do this, we use the DMA API.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* Sort headers.
* MSI with captial letters in error print.
* Don't change to lower_32_bits/upper_32_bits, since that should in that
  case be its own patch.

drivers/pci/dwc/pcie-designware-host.c | 15 ++++++++++++---
 drivers/pci/dwc/pcie-designware.h      |  3 ++-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 81e2157a7cfb..33b52fe98a01 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -83,10 +83,19 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 
 void dw_pcie_msi_init(struct pcie_port *pp)
 {
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct device *dev = pci->dev;
+	struct page *page;
 	u64 msi_target;
 
-	pp->msi_data = __get_free_pages(GFP_KERNEL, 0);
-	msi_target = virt_to_phys((void *)pp->msi_data);
+	page = alloc_page(GFP_KERNEL | GFP_DMA32);
+	pp->msi_data = dma_map_page(dev, page, 0, PAGE_SIZE, DMA_FROM_DEVICE);
+	if (dma_mapping_error(dev, pp->msi_data)) {
+		dev_err(dev, "failed to map MSI data\n");
+		__free_page(page);
+		return;
+	}
+	msi_target = (u64)pp->msi_data;
 
 	/* program the msi_data */
 	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
@@ -187,7 +196,7 @@ static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
 	if (pp->ops->get_msi_addr)
 		msi_target = pp->ops->get_msi_addr(pp);
 	else
-		msi_target = virt_to_phys((void *)pp->msi_data);
+		msi_target = (u64)pp->msi_data;
 
 	msg.address_lo = (u32)(msi_target & 0xffffffff);
 	msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff);
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index e5d9d77b778e..ecdede68522a 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -14,6 +14,7 @@
 #ifndef _PCIE_DESIGNWARE_H
 #define _PCIE_DESIGNWARE_H
 
+#include <linux/dma-mapping.h>
 #include <linux/irq.h>
 #include <linux/msi.h>
 #include <linux/pci.h>
@@ -168,7 +169,7 @@ struct pcie_port {
 	const struct dw_pcie_host_ops *ops;
 	int			msi_irq;
 	struct irq_domain	*irq_domain;
-	unsigned long		msi_data;
+	dma_addr_t		msi_data;
 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
 };
 
-- 
2.14.2

[PATCH v2 02/17] PCI: designware-ep: dw_pcie_ep_set_msi() should only set MMC bits

[Niklas Cassel]

Previously, dw_pcie_ep_set_msi() wrote all bits in the Message Control
register, thus overwriting the PCI_MSI_FLAGS_64BIT bit.
By clearing the PCI_MSI_FLAGS_64BIT bit, we break MSI
on systems where the RC has set a 64 bit MSI address.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* Clarified commit message.

 drivers/pci/dwc/pcie-designware-ep.c | 4 +++-
 drivers/pci/dwc/pcie-designware.h    | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index d53d5f168363..c92ab87fd660 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -220,7 +220,9 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 encode_int)
 	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 
-	val = (encode_int << MSI_CAP_MMC_SHIFT);
+	val = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
+	val &= ~MSI_CAP_MMC_MASK;
+	val |= (encode_int << MSI_CAP_MMC_SHIFT) & MSI_CAP_MMC_MASK;
 	dw_pcie_writew_dbi(pci, MSI_MESSAGE_CONTROL, val);
 
 	return 0;
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index ecdede68522a..9aaf0cd04dd6 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -101,6 +101,7 @@
 
 #define MSI_MESSAGE_CONTROL		0x52
 #define MSI_CAP_MMC_SHIFT		1
+#define MSI_CAP_MMC_MASK		(7 << MSI_CAP_MMC_SHIFT)
 #define MSI_CAP_MME_SHIFT		4
 #define MSI_CAP_MME_MASK		(7 << MSI_CAP_MME_SHIFT)
 #define MSI_MESSAGE_ADDR_L32		0x54
-- 
2.14.2

[PATCH v2 03/17] PCI: designware-ep: Read-only registers need DBI_RO_WR_EN to be writable

[Niklas Cassel]

Certain registers that pcie-designware-ep tries to write are read-only
registers. However, these registers can become read/write if we first
enable the DBI_RO_WR_EN bit.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* No change.

 drivers/pci/dwc/pcie-designware-ep.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index c92ab87fd660..3fb34be99715 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -35,8 +35,10 @@ static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
 	u32 reg;
 
 	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
+	dw_pcie_dbi_ro_wr_en(pci);
 	dw_pcie_writel_dbi2(pci, reg, 0x0);
 	dw_pcie_writel_dbi(pci, reg, 0x0);
+	dw_pcie_dbi_ro_wr_dis(pci);
 }
 
 static int dw_pcie_ep_write_header(struct pci_epc *epc,
@@ -45,6 +47,7 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc,
 	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 
+	dw_pcie_dbi_ro_wr_en(pci);
 	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
 	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
 	dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
@@ -58,6 +61,7 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc,
 	dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_ID, hdr->subsys_id);
 	dw_pcie_writeb_dbi(pci, PCI_INTERRUPT_PIN,
 			   hdr->interrupt_pin);
+	dw_pcie_dbi_ro_wr_dis(pci);
 
 	return 0;
 }
@@ -142,8 +146,10 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, enum pci_barno bar,
 	if (ret)
 		return ret;
 
+	dw_pcie_dbi_ro_wr_en(pci);
 	dw_pcie_writel_dbi2(pci, reg, size - 1);
 	dw_pcie_writel_dbi(pci, reg, flags);
+	dw_pcie_dbi_ro_wr_dis(pci);
 
 	return 0;
 }
@@ -223,7 +229,9 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 encode_int)
 	val = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
 	val &= ~MSI_CAP_MMC_MASK;
 	val |= (encode_int << MSI_CAP_MMC_SHIFT) & MSI_CAP_MMC_MASK;
+	dw_pcie_dbi_ro_wr_en(pci);
 	dw_pcie_writew_dbi(pci, MSI_MESSAGE_CONTROL, val);
+	dw_pcie_dbi_ro_wr_dis(pci);
 
 	return 0;
 }
-- 
2.14.2

[PATCH v2 04/17] PCI: designware-ep: Pre-allocate memory for MSI in dw_pcie_ep_init

[Niklas Cassel]

Certain SoCs need to map the MSI address in raise_irq.
To map an address, you first need to call pci_epc_mem_alloc_addr,
however, pci_epc_mem_alloc_addr calls ioremap (which can sleep).

Since raise_irq is only called from atomic context, we can't call
pci_epc_mem_alloc_addr from raise_irq, instead we pre-allocate
a page in dw_pcie_ep_init, so this page can later be used to map/unmap
the MSI address in raise_irq.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* No change.

 drivers/pci/dwc/pcie-designware-ep.c | 8 ++++++++
 drivers/pci/dwc/pcie-designware.h    | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index 3fb34be99715..c291da2a10ba 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -286,6 +286,8 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 {
 	struct pci_epc *epc = ep->epc;
 
+	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem, PAGE_SIZE);
+
 	pci_epc_mem_exit(epc);
 }
 
@@ -341,6 +343,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 		return ret;
 	}
 
+	ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys, PAGE_SIZE);
+	if (!ep->msi_mem) {
+		dev_err(dev, "Failed to reserve memory for MSI\n");
+		return -ENOMEM;
+	}
+
 	ep->epc = epc;
 	epc_set_drvdata(epc, ep);
 	dw_pcie_setup(pci);
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index 9aaf0cd04dd6..5a1da459eda5 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -198,6 +198,8 @@ struct dw_pcie_ep {
 	unsigned long		ob_window_map;
 	u32			num_ib_windows;
 	u32			num_ob_windows;
+	void __iomem		*msi_mem;
+	phys_addr_t		msi_mem_phys;
 };
 
 struct dw_pcie_ops {
-- 
2.14.2

Re: [PATCH v2 04/17] PCI: designware-ep: Pre-allocate memory for MSI in dw_pcie_ep_init

[Kishon Vijay Abraham I]

Hi Niklas,

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> Certain SoCs need to map the MSI address in raise_irq.
> To map an address, you first need to call pci_epc_mem_alloc_addr,
> however, pci_epc_mem_alloc_addr calls ioremap (which can sleep).
> 
> Since raise_irq is only called from atomic context, we can't call
> pci_epc_mem_alloc_addr from raise_irq, instead we pre-allocate
> a page in dw_pcie_ep_init, so this page can later be used to map/unmap
> the MSI address in raise_irq.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> V2:
> * No change.
> 
>  drivers/pci/dwc/pcie-designware-ep.c | 8 ++++++++
>  drivers/pci/dwc/pcie-designware.h    | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
> index 3fb34be99715..c291da2a10ba 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -286,6 +286,8 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>  {
>  	struct pci_epc *epc = ep->epc;
>  
> +	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem, PAGE_SIZE);
> +
>  	pci_epc_mem_exit(epc);
>  }
>  
> @@ -341,6 +343,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  		return ret;
>  	}
>  
> +	ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys, PAGE_SIZE);

ep->page_size instead of PAGE_SIZE?

Thanks
Kishon

Re: [PATCH v2 04/17] PCI: designware-ep: Pre-allocate memory for MSI in dw_pcie_ep_init

[Niklas Cassel]

On 10/31/2017 07:01 AM, Kishon Vijay Abraham I wrote:
> Hi Niklas,
> 
> On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
>> Certain SoCs need to map the MSI address in raise_irq.
>> To map an address, you first need to call pci_epc_mem_alloc_addr,
>> however, pci_epc_mem_alloc_addr calls ioremap (which can sleep).
>>
>> Since raise_irq is only called from atomic context, we can't call
>> pci_epc_mem_alloc_addr from raise_irq, instead we pre-allocate
>> a page in dw_pcie_ep_init, so this page can later be used to map/unmap
>> the MSI address in raise_irq.
>>
>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>> ---
>> V2:
>> * No change.
>>
>>  drivers/pci/dwc/pcie-designware-ep.c | 8 ++++++++
>>  drivers/pci/dwc/pcie-designware.h    | 2 ++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
>> index 3fb34be99715..c291da2a10ba 100644
>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>> @@ -286,6 +286,8 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>>  {
>>  	struct pci_epc *epc = ep->epc;
>>  
>> +	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem, PAGE_SIZE);
>> +
>>  	pci_epc_mem_exit(epc);
>>  }
>>  
>> @@ -341,6 +343,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>>  		return ret;
>>  	}
>>  
>> +	ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys, PAGE_SIZE);
> 
> ep->page_size instead of PAGE_SIZE?

Hello Kishon,

Sure thing, will fix in V3.

Regards,
Niklas

Re: [PATCH v2 04/17] PCI: designware-ep: Pre-allocate memory for MSI in dw_pcie_ep_init

[Niklas Cassel]

On 10/31/2017 07:01 AM, Kishon Vijay Abraham I wrote:
> Hi Niklas,
> 
> On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
>> Certain SoCs need to map the MSI address in raise_irq.
>> To map an address, you first need to call pci_epc_mem_alloc_addr,
>> however, pci_epc_mem_alloc_addr calls ioremap (which can sleep).
>>
>> Since raise_irq is only called from atomic context, we can't call
>> pci_epc_mem_alloc_addr from raise_irq, instead we pre-allocate
>> a page in dw_pcie_ep_init, so this page can later be used to map/unmap
>> the MSI address in raise_irq.
>>
>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>> ---
>> V2:
>> * No change.
>>
>>  drivers/pci/dwc/pcie-designware-ep.c | 8 ++++++++
>>  drivers/pci/dwc/pcie-designware.h    | 2 ++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
>> index 3fb34be99715..c291da2a10ba 100644
>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>> @@ -286,6 +286,8 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>>  {
>>  	struct pci_epc *epc = ep->epc;
>>  
>> +	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem, PAGE_SIZE);
>> +
>>  	pci_epc_mem_exit(epc);
>>  }
>>  
>> @@ -341,6 +343,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>>  		return ret;
>>  	}
>>  
>> +	ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys, PAGE_SIZE);
> 
> ep->page_size instead of PAGE_SIZE?
> 

It was a really good call of Bjorn not to stress this series,
since I apparently did not properly test this... I'm sorry,
it will not happen again.


ep->page_size is by default set to 0.

__pci_epc_mem_init basically does this:
if ep->page_size < PAGE_SIZE:
    epc->mem->page_size = PAGE_SIZE
else
    epc->mem->page_size = ep->page_size

So __pci_epc_mem_init is safe to call with ep->page_size 0,
however, the same thing is not true for pci_epc_mem_alloc_addr.

I will need to do a v5 of this patch series.

Kishon, what do you think about doing this instead?

+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -321,7 +321,7 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
        struct pci_epc *epc = ep->epc;
 
        pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
-                             ep->page_size);
+                             epc->mem->page_size);
 
        pci_epc_mem_exit(epc);
 }
@@ -379,7 +379,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
        }
 
        ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
-                                            ep->page_size);
+                                            epc->mem->page_size);
        if (!ep->msi_mem) {
                dev_err(dev, "Failed to reserve memory for MSI\n");
                return -ENOMEM;


Regards,
Niklas

[PATCH v2 05/17] PCI: designware-ep: Remove static keyword from dw_pcie_ep_reset_bar()

[Niklas Cassel]

This way pci-dra7xx.c does not need its own copy of dw_pcie_ep_reset_bar().

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* New patch in this series.

 drivers/pci/dwc/pci-dra7xx.c         | 9 ---------
 drivers/pci/dwc/pcie-designware-ep.c | 2 +-
 drivers/pci/dwc/pcie-designware.h    | 5 +++++
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
index d0848006945a..a743545da4d4 100644
--- a/drivers/pci/dwc/pci-dra7xx.c
+++ b/drivers/pci/dwc/pci-dra7xx.c
@@ -336,15 +336,6 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
-{
-	u32 reg;
-
-	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
-	dw_pcie_writel_dbi2(pci, reg, 0x0);
-	dw_pcie_writel_dbi(pci, reg, 0x0);
-}
-
 static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index c291da2a10ba..47134a85a342 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -30,7 +30,7 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
 	pci_epc_linkup(epc);
 }
 
-static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
+void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
 {
 	u32 reg;
 
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index 5a1da459eda5..37dfad8d003f 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -338,6 +338,7 @@ static inline int dw_pcie_host_init(struct pcie_port *pp)
 void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
 int dw_pcie_ep_init(struct dw_pcie_ep *ep);
 void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
+void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
 #else
 static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
 {
@@ -351,5 +352,9 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 {
 }
+
+static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
+{
+}
 #endif
 #endif /* _PCIE_DESIGNWARE_H */
-- 
2.14.2

Re: [PATCH v2 05/17] PCI: designware-ep: Remove static keyword from dw_pcie_ep_reset_bar()

[Kishon Vijay Abraham I]

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> This way pci-dra7xx.c does not need its own copy of dw_pcie_ep_reset_bar().
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> V2:
> * New patch in this series.
> 
>  drivers/pci/dwc/pci-dra7xx.c         | 9 ---------
>  drivers/pci/dwc/pcie-designware-ep.c | 2 +-
>  drivers/pci/dwc/pcie-designware.h    | 5 +++++
>  3 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index d0848006945a..a743545da4d4 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -336,15 +336,6 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> -static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> -{
> -	u32 reg;
> -
> -	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> -	dw_pcie_writel_dbi2(pci, reg, 0x0);
> -	dw_pcie_writel_dbi(pci, reg, 0x0);
> -}
> -
>  static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
> index c291da2a10ba..47134a85a342 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -30,7 +30,7 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>  	pci_epc_linkup(epc);
>  }
>  
> -static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> +void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>  {
>  	u32 reg;
>  
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index 5a1da459eda5..37dfad8d003f 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -338,6 +338,7 @@ static inline int dw_pcie_host_init(struct pcie_port *pp)
>  void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
>  int dw_pcie_ep_init(struct dw_pcie_ep *ep);
>  void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
> +void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
>  #else
>  static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>  {
> @@ -351,5 +352,9 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>  {
>  }
> +
> +static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> +{
> +}
>  #endif
>  #endif /* _PCIE_DESIGNWARE_H */
> 

Re: [PATCH v2 05/17] PCI: designware-ep: Remove static keyword from dw_pcie_ep_reset_bar()

[Kishon Vijay Abraham I]

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> This way pci-dra7xx.c does not need its own copy of dw_pcie_ep_reset_bar().
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> V2:
> * New patch in this series.
> 
>  drivers/pci/dwc/pci-dra7xx.c         | 9 ---------
>  drivers/pci/dwc/pcie-designware-ep.c | 2 +-
>  drivers/pci/dwc/pcie-designware.h    | 5 +++++
>  3 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index d0848006945a..a743545da4d4 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -336,15 +336,6 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> -static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> -{
> -	u32 reg;
> -
> -	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> -	dw_pcie_writel_dbi2(pci, reg, 0x0);
> -	dw_pcie_writel_dbi(pci, reg, 0x0);
> -}
> -
>  static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
> index c291da2a10ba..47134a85a342 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -30,7 +30,7 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>  	pci_epc_linkup(epc);
>  }
>  
> -static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> +void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>  {
>  	u32 reg;
>  
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index 5a1da459eda5..37dfad8d003f 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -338,6 +338,7 @@ static inline int dw_pcie_host_init(struct pcie_port *pp)
>  void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
>  int dw_pcie_ep_init(struct dw_pcie_ep *ep);
>  void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
> +void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
>  #else
>  static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>  {
> @@ -351,5 +352,9 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>  {
>  }
> +
> +static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> +{
> +}
>  #endif
>  #endif /* _PCIE_DESIGNWARE_H */
> 

[PATCH v2 06/17] PCI: designware-ep: Add generic function for raising MSI irq

[Niklas Cassel]

This function can be used by all DWC based controllers to raise a MSI
irq. However, certain controllers, like DRA7xx, has a special
convenience register for raising MSI irqs that doesn't require you to
explicitly map the MSI address.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* New patch in this series. The other changes in the changelog are against
  the version that was included in pcie-artpec6.c in V1.
* raise_msi_irq now uses readw/writew instead of masking.
* raise_msi_irq now uses fixed offset for MSI cap in raise_msi, since it is
  always at the same offset for this IP.
* raise_msi_irq now uses PCI_MSI_FLAGS_64BIT rather than BIT(7).

 drivers/pci/dwc/pcie-designware-ep.c | 34 ++++++++++++++++++++++++++++++++++
 drivers/pci/dwc/pcie-designware.h    |  9 +++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index 47134a85a342..92b7ca36bae4 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -282,6 +282,40 @@ static const struct pci_epc_ops epc_ops = {
 	.stop			= dw_pcie_ep_stop,
 };
 
+int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep,
+			     u8 interrupt_num)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	struct pci_epc *epc = ep->epc;
+	u16 msg_ctrl, msg_data;
+	u32 msg_addr_lower, msg_addr_upper;
+	u64 msg_addr;
+	bool has_upper;
+	int ret;
+
+	/* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
+	msg_ctrl = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
+	has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
+	msg_addr_lower = dw_pcie_readl_dbi(pci, MSI_MESSAGE_ADDR_L32);
+	if (has_upper) {
+		msg_addr_upper = dw_pcie_readl_dbi(pci, MSI_MESSAGE_ADDR_U32);
+		msg_data = dw_pcie_readw_dbi(pci, MSI_MESSAGE_DATA_64);
+	} else {
+		msg_addr_upper = 0;
+		msg_data = dw_pcie_readw_dbi(pci, MSI_MESSAGE_DATA_32);
+	}
+	msg_addr = ((u64) msg_addr_upper) << 32 | msg_addr_lower;
+	ret = dw_pcie_ep_map_addr(epc, ep->msi_mem_phys, msg_addr, PAGE_SIZE);
+	if (ret)
+		return ret;
+
+	writel(msg_data | (interrupt_num - 1), ep->msi_mem);
+
+	dw_pcie_ep_unmap_addr(epc, ep->msi_mem_phys);
+
+	return 0;
+}
+
 void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 {
 	struct pci_epc *epc = ep->epc;
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index 37dfad8d003f..24edac035160 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -106,6 +106,8 @@
 #define MSI_CAP_MME_MASK		(7 << MSI_CAP_MME_SHIFT)
 #define MSI_MESSAGE_ADDR_L32		0x54
 #define MSI_MESSAGE_ADDR_U32		0x58
+#define MSI_MESSAGE_DATA_32		0x58
+#define MSI_MESSAGE_DATA_64		0x5C
 
 /*
  * Maximum number of MSI IRQs can be 256 per controller. But keep
@@ -338,6 +340,7 @@ static inline int dw_pcie_host_init(struct pcie_port *pp)
 void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
 int dw_pcie_ep_init(struct dw_pcie_ep *ep);
 void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
+int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 interrupt_num);
 void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
 #else
 static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
@@ -353,6 +356,12 @@ static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 {
 }
 
+static inline int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep,
+					   u8 interrupt_num)
+{
+	return 0;
+}
+
 static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
 {
 }
-- 
2.14.2

Re: [PATCH v2 06/17] PCI: designware-ep: Add generic function for raising MSI irq

[Kishon Vijay Abraham I]

Hi,

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> This function can be used by all DWC based controllers to raise a MSI
> irq. However, certain controllers, like DRA7xx, has a special
> convenience register for raising MSI irqs that doesn't require you to
> explicitly map the MSI address.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> V2:
> * New patch in this series. The other changes in the changelog are against
>   the version that was included in pcie-artpec6.c in V1.
> * raise_msi_irq now uses readw/writew instead of masking.
> * raise_msi_irq now uses fixed offset for MSI cap in raise_msi, since it is
>   always at the same offset for this IP.
> * raise_msi_irq now uses PCI_MSI_FLAGS_64BIT rather than BIT(7).
> 
>  drivers/pci/dwc/pcie-designware-ep.c | 34 ++++++++++++++++++++++++++++++++++
>  drivers/pci/dwc/pcie-designware.h    |  9 +++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
> index 47134a85a342..92b7ca36bae4 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -282,6 +282,40 @@ static const struct pci_epc_ops epc_ops = {
>  	.stop			= dw_pcie_ep_stop,
>  };
>  
> +int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep,
> +			     u8 interrupt_num)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	struct pci_epc *epc = ep->epc;
> +	u16 msg_ctrl, msg_data;
> +	u32 msg_addr_lower, msg_addr_upper;
> +	u64 msg_addr;
> +	bool has_upper;
> +	int ret;
> +
> +	/* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> +	msg_ctrl = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
> +	has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> +	msg_addr_lower = dw_pcie_readl_dbi(pci, MSI_MESSAGE_ADDR_L32);
> +	if (has_upper) {
> +		msg_addr_upper = dw_pcie_readl_dbi(pci, MSI_MESSAGE_ADDR_U32);
> +		msg_data = dw_pcie_readw_dbi(pci, MSI_MESSAGE_DATA_64);
> +	} else {
> +		msg_addr_upper = 0;
> +		msg_data = dw_pcie_readw_dbi(pci, MSI_MESSAGE_DATA_32);
> +	}
> +	msg_addr = ((u64) msg_addr_upper) << 32 | msg_addr_lower;
> +	ret = dw_pcie_ep_map_addr(epc, ep->msi_mem_phys, msg_addr, PAGE_SIZE);
> +	if (ret)
> +		return ret;
> +
> +	writel(msg_data | (interrupt_num - 1), ep->msi_mem);
> +
> +	dw_pcie_ep_unmap_addr(epc, ep->msi_mem_phys);
> +
> +	return 0;
> +}
> +
>  void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>  {
>  	struct pci_epc *epc = ep->epc;
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index 37dfad8d003f..24edac035160 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -106,6 +106,8 @@
>  #define MSI_CAP_MME_MASK		(7 << MSI_CAP_MME_SHIFT)
>  #define MSI_MESSAGE_ADDR_L32		0x54
>  #define MSI_MESSAGE_ADDR_U32		0x58
> +#define MSI_MESSAGE_DATA_32		0x58

Does ADDR_U32 and DATA_32 share the same offset? (in dra7xx MSI_64_EN is
hardwired to 1 so I can't know this for sure.)

Thanks
Kishon

Re: [PATCH v2 06/17] PCI: designware-ep: Add generic function for raising MSI irq

[Niklas Cassel]

On 10/31/2017 07:22 AM, Kishon Vijay Abraham I wrote:

(snip)

>> --- a/drivers/pci/dwc/pcie-designware.h
>> +++ b/drivers/pci/dwc/pcie-designware.h
>> @@ -106,6 +106,8 @@
>>  #define MSI_CAP_MME_MASK		(7 << MSI_CAP_MME_SHIFT)
>>  #define MSI_MESSAGE_ADDR_L32		0x54
>>  #define MSI_MESSAGE_ADDR_U32		0x58
>> +#define MSI_MESSAGE_DATA_32		0x58
> 
> Does ADDR_U32 and DATA_32 share the same offset? (in dra7xx MSI_64_EN is
> hardwired to 1 so I can't know this for sure.)
> 

Hello Kishon,

Yes, they do.
The information in the register depends on if the bit MSI_64_EN
is set or not.

There exists defines for this also in the generic uapi header pci_regs.h,
however, utilizing that header would be a separate patch.

$ grep -E "MSI_DATA|MSI_MASK" include/uapi/linux/pci_regs.h
#define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit devices */
#define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit devices */
#define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit devices */
#define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit devices */

Regards,
Niklas

[PATCH v2 07/17] PCI: dwc: dra7xx: Refactor Kconfig and Makefile handling for host/ep mode

[Niklas Cassel]

The previous handling was a bit unorthodox and would have been a bit
bloated once more DWC based controllers added support for ep mode.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* New patch in this series.

 drivers/pci/dwc/Kconfig  | 45 ++++++++++++++++++++++-----------------------
 drivers/pci/dwc/Makefile |  4 +---
 2 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
index 113e09440f85..3954353e3e2e 100644
--- a/drivers/pci/dwc/Kconfig
+++ b/drivers/pci/dwc/Kconfig
@@ -15,39 +15,38 @@ config PCIE_DW_EP
 	select PCIE_DW
 
 config PCI_DRA7XX
-	bool "TI DRA7xx PCIe controller"
-	depends on SOC_DRA7XX || COMPILE_TEST
-	depends on (PCI && PCI_MSI_IRQ_DOMAIN) || PCI_ENDPOINT
-	depends on OF && HAS_IOMEM && TI_PIPE3
-	help
-	 Enables support for the PCIe controller in the DRA7xx SoC. There
-	 are two instances of PCIe controller in DRA7xx. This controller can
-	 work either as EP or RC. In order to enable host-specific features
-	 PCI_DRA7XX_HOST must be selected and in order to enable device-
-	 specific features PCI_DRA7XX_EP must be selected. This uses
-	 the DesignWare core.
-
-if PCI_DRA7XX
+	bool
 
 config PCI_DRA7XX_HOST
-	bool "PCI DRA7xx Host Mode"
-	depends on PCI
-	depends on PCI_MSI_IRQ_DOMAIN
+	bool "TI DRA7xx PCIe controller Host Mode"
+	depends on SOC_DRA7XX || COMPILE_TEST
+	depends on PCI && PCI_MSI_IRQ_DOMAIN
+	depends on OF && HAS_IOMEM && TI_PIPE3
 	select PCIE_DW_HOST
+	select PCI_DRA7XX
 	default y
 	help
-	 Enables support for the PCIe controller in the DRA7xx SoC to work in
-	 host mode.
+	  Enables support for the PCIe controller in the DRA7xx SoC to work in
+	  host mode. There are two instances of PCIe controller in DRA7xx.
+	  This controller can work either as EP or RC. In order to enable
+	  host-specific features PCI_DRA7XX_HOST must be selected and in order
+	  to enable device-specific features PCI_DRA7XX_EP must be selected.
+	  This uses the DesignWare core.
 
 config PCI_DRA7XX_EP
-	bool "PCI DRA7xx Endpoint Mode"
+	bool "TI DRA7xx PCIe controller Endpoint Mode"
+	depends on SOC_DRA7XX || COMPILE_TEST
 	depends on PCI_ENDPOINT
+	depends on OF && HAS_IOMEM && TI_PIPE3
 	select PCIE_DW_EP
+	select PCI_DRA7XX
 	help
-	 Enables support for the PCIe controller in the DRA7xx SoC to work in
-	 endpoint mode.
-
-endif
+	  Enables support for the PCIe controller in the DRA7xx SoC to work in
+	  endpoint mode. There are two instances of PCIe controller in DRA7xx.
+	  This controller can work either as EP or RC. In order to enable
+	  host-specific features PCI_DRA7XX_HOST must be selected and in order
+	  to enable device-specific features PCI_DRA7XX_EP must be selected.
+	  This uses the DesignWare core.
 
 config PCIE_DW_PLAT
 	bool "Platform bus based DesignWare PCIe Controller"
diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
index 54f56f6e9236..260589bd4e1d 100644
--- a/drivers/pci/dwc/Makefile
+++ b/drivers/pci/dwc/Makefile
@@ -2,9 +2,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
 obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
 obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
 obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
-ifneq ($(filter y,$(CONFIG_PCI_DRA7XX_HOST) $(CONFIG_PCI_DRA7XX_EP)),)
-        obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
-endif
+obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
 obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
 obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
-- 
2.14.2

Re: [PATCH v2 07/17] PCI: dwc: dra7xx: Refactor Kconfig and Makefile handling for host/ep mode

[Kishon Vijay Abraham I]

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> The previous handling was a bit unorthodox and would have been a bit
> bloated once more DWC based controllers added support for ep mode.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> V2:
> * New patch in this series.
> 
>  drivers/pci/dwc/Kconfig  | 45 ++++++++++++++++++++++-----------------------
>  drivers/pci/dwc/Makefile |  4 +---
>  2 files changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
> index 113e09440f85..3954353e3e2e 100644
> --- a/drivers/pci/dwc/Kconfig
> +++ b/drivers/pci/dwc/Kconfig
> @@ -15,39 +15,38 @@ config PCIE_DW_EP
>  	select PCIE_DW
>  
>  config PCI_DRA7XX
> -	bool "TI DRA7xx PCIe controller"
> -	depends on SOC_DRA7XX || COMPILE_TEST
> -	depends on (PCI && PCI_MSI_IRQ_DOMAIN) || PCI_ENDPOINT
> -	depends on OF && HAS_IOMEM && TI_PIPE3
> -	help
> -	 Enables support for the PCIe controller in the DRA7xx SoC. There
> -	 are two instances of PCIe controller in DRA7xx. This controller can
> -	 work either as EP or RC. In order to enable host-specific features
> -	 PCI_DRA7XX_HOST must be selected and in order to enable device-
> -	 specific features PCI_DRA7XX_EP must be selected. This uses
> -	 the DesignWare core.
> -
> -if PCI_DRA7XX
> +	bool
>  
>  config PCI_DRA7XX_HOST
> -	bool "PCI DRA7xx Host Mode"
> -	depends on PCI
> -	depends on PCI_MSI_IRQ_DOMAIN
> +	bool "TI DRA7xx PCIe controller Host Mode"
> +	depends on SOC_DRA7XX || COMPILE_TEST
> +	depends on PCI && PCI_MSI_IRQ_DOMAIN
> +	depends on OF && HAS_IOMEM && TI_PIPE3
>  	select PCIE_DW_HOST
> +	select PCI_DRA7XX
>  	default y
>  	help
> -	 Enables support for the PCIe controller in the DRA7xx SoC to work in
> -	 host mode.
> +	  Enables support for the PCIe controller in the DRA7xx SoC to work in
> +	  host mode. There are two instances of PCIe controller in DRA7xx.
> +	  This controller can work either as EP or RC. In order to enable
> +	  host-specific features PCI_DRA7XX_HOST must be selected and in order
> +	  to enable device-specific features PCI_DRA7XX_EP must be selected.
> +	  This uses the DesignWare core.
>  
>  config PCI_DRA7XX_EP
> -	bool "PCI DRA7xx Endpoint Mode"
> +	bool "TI DRA7xx PCIe controller Endpoint Mode"
> +	depends on SOC_DRA7XX || COMPILE_TEST
>  	depends on PCI_ENDPOINT
> +	depends on OF && HAS_IOMEM && TI_PIPE3
>  	select PCIE_DW_EP
> +	select PCI_DRA7XX
>  	help
> -	 Enables support for the PCIe controller in the DRA7xx SoC to work in
> -	 endpoint mode.
> -
> -endif
> +	  Enables support for the PCIe controller in the DRA7xx SoC to work in
> +	  endpoint mode. There are two instances of PCIe controller in DRA7xx.
> +	  This controller can work either as EP or RC. In order to enable
> +	  host-specific features PCI_DRA7XX_HOST must be selected and in order
> +	  to enable device-specific features PCI_DRA7XX_EP must be selected.
> +	  This uses the DesignWare core.
>  
>  config PCIE_DW_PLAT
>  	bool "Platform bus based DesignWare PCIe Controller"
> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
> index 54f56f6e9236..260589bd4e1d 100644
> --- a/drivers/pci/dwc/Makefile
> +++ b/drivers/pci/dwc/Makefile
> @@ -2,9 +2,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>  obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>  obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> -ifneq ($(filter y,$(CONFIG_PCI_DRA7XX_HOST) $(CONFIG_PCI_DRA7XX_EP)),)
> -        obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> -endif
> +obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>  obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>  obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> 

[PATCH v2 08/17] PCI: dwc: dra7xx: Assign pp->ops in dra7xx_add_pcie_port() rather than in probe

[Niklas Cassel]

This matches how other drivers like exynos, imx7, keystone, armada8k,
artpec6, designware-plat, hisi, kirin and spear13xx does it.

This is probably a remainder since when dev and ops were assigned as
members to pp. Since we now assign them as members to struct dw_pcie,
the pp->ops assignment should definitely be in dra7xx_add_pcie_port().

This is also needed to be able to compile host/ep mode specific code
independently.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* New patch in this series.

 drivers/pci/dwc/pci-dra7xx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
index a743545da4d4..009f6aeeee1c 100644
--- a/drivers/pci/dwc/pci-dra7xx.c
+++ b/drivers/pci/dwc/pci-dra7xx.c
@@ -460,6 +460,8 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
 	if (!pci->dbi_base)
 		return -ENOMEM;
 
+	pp->ops = &dra7xx_pcie_host_ops;
+
 	ret = dw_pcie_host_init(pp);
 	if (ret) {
 		dev_err(dev, "failed to initialize host\n");
@@ -588,7 +590,6 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 	void __iomem *base;
 	struct resource *res;
 	struct dw_pcie *pci;
-	struct pcie_port *pp;
 	struct dra7xx_pcie *dra7xx;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
@@ -616,9 +617,6 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 	pci->dev = dev;
 	pci->ops = &dw_pcie_ops;
 
-	pp = &pci->pp;
-	pp->ops = &dra7xx_pcie_host_ops;
-
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		dev_err(dev, "missing IRQ resource: %d\n", irq);
-- 
2.14.2

Re: [PATCH v2 08/17] PCI: dwc: dra7xx: Assign pp->ops in dra7xx_add_pcie_port() rather than in probe

[Kishon Vijay Abraham I]

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> This matches how other drivers like exynos, imx7, keystone, armada8k,
> artpec6, designware-plat, hisi, kirin and spear13xx does it.
> 
> This is probably a remainder since when dev and ops were assigned as
> members to pp. Since we now assign them as members to struct dw_pcie,
> the pp->ops assignment should definitely be in dra7xx_add_pcie_port().
> 
> This is also needed to be able to compile host/ep mode specific code
> independently.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> V2:
> * New patch in this series.
> 
>  drivers/pci/dwc/pci-dra7xx.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index a743545da4d4..009f6aeeee1c 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -460,6 +460,8 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  	if (!pci->dbi_base)
>  		return -ENOMEM;
>  
> +	pp->ops = &dra7xx_pcie_host_ops;
> +
>  	ret = dw_pcie_host_init(pp);
>  	if (ret) {
>  		dev_err(dev, "failed to initialize host\n");
> @@ -588,7 +590,6 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  	void __iomem *base;
>  	struct resource *res;
>  	struct dw_pcie *pci;
> -	struct pcie_port *pp;
>  	struct dra7xx_pcie *dra7xx;
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node;
> @@ -616,9 +617,6 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  	pci->dev = dev;
>  	pci->ops = &dw_pcie_ops;
>  
> -	pp = &pci->pp;
> -	pp->ops = &dra7xx_pcie_host_ops;
> -
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
>  		dev_err(dev, "missing IRQ resource: %d\n", irq);
> 

Re: [PATCH v2 08/17] PCI: dwc: dra7xx: Assign pp->ops in dra7xx_add_pcie_port() rather than in probe

[Kishon Vijay Abraham I]

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> This matches how other drivers like exynos, imx7, keystone, armada8k,
> artpec6, designware-plat, hisi, kirin and spear13xx does it.
> 
> This is probably a remainder since when dev and ops were assigned as
> members to pp. Since we now assign them as members to struct dw_pcie,
> the pp->ops assignment should definitely be in dra7xx_add_pcie_port().
> 
> This is also needed to be able to compile host/ep mode specific code
> independently.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> V2:
> * New patch in this series.
> 
>  drivers/pci/dwc/pci-dra7xx.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index a743545da4d4..009f6aeeee1c 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -460,6 +460,8 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  	if (!pci->dbi_base)
>  		return -ENOMEM;
>  
> +	pp->ops = &dra7xx_pcie_host_ops;
> +
>  	ret = dw_pcie_host_init(pp);
>  	if (ret) {
>  		dev_err(dev, "failed to initialize host\n");
> @@ -588,7 +590,6 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  	void __iomem *base;
>  	struct resource *res;
>  	struct dw_pcie *pci;
> -	struct pcie_port *pp;
>  	struct dra7xx_pcie *dra7xx;
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node;
> @@ -616,9 +617,6 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  	pci->dev = dev;
>  	pci->ops = &dw_pcie_ops;
>  
> -	pp = &pci->pp;
> -	pp->ops = &dra7xx_pcie_host_ops;
> -
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
>  		dev_err(dev, "missing IRQ resource: %d\n", irq);
> 

[PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

[Niklas Cassel]

This way you will not build and include unused code
when only building for only one mode.

Moved dra7xx_pcie_enable_wrapper_interrupts() in order
to avoid adding an extra ifdef block.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* New patch in this series.

 drivers/pci/dwc/pci-dra7xx.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
index 009f6aeeee1c..175544d6c3ab 100644
--- a/drivers/pci/dwc/pci-dra7xx.c
+++ b/drivers/pci/dwc/pci-dra7xx.c
@@ -171,6 +171,15 @@ static int dra7xx_pcie_establish_link(struct dw_pcie *pci)
 	return 0;
 }
 
+static void dra7xx_pcie_enable_wrapper_interrupts(struct dra7xx_pcie *dra7xx)
+{
+	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN,
+			   INTERRUPTS);
+	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN,
+			   INTERRUPTS);
+}
+
+#ifdef CONFIG_PCI_DRA7XX_HOST
 static void dra7xx_pcie_enable_msi_interrupts(struct dra7xx_pcie *dra7xx)
 {
 	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI,
@@ -181,14 +190,6 @@ static void dra7xx_pcie_enable_msi_interrupts(struct dra7xx_pcie *dra7xx)
 			   MSI | LEG_EP_INTERRUPTS);
 }
 
-static void dra7xx_pcie_enable_wrapper_interrupts(struct dra7xx_pcie *dra7xx)
-{
-	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN,
-			   INTERRUPTS);
-	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN,
-			   INTERRUPTS);
-}
-
 static void dra7xx_pcie_enable_interrupts(struct dra7xx_pcie *dra7xx)
 {
 	dra7xx_pcie_enable_wrapper_interrupts(dra7xx);
@@ -276,6 +277,7 @@ static irqreturn_t dra7xx_pcie_msi_irq_handler(int irq, void *arg)
 
 	return IRQ_HANDLED;
 }
+#endif
 
 static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
 {
@@ -336,6 +338,7 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_PCI_DRA7XX_EP
 static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -427,7 +430,9 @@ static int __init dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
 
 	return 0;
 }
+#endif
 
+#ifdef CONFIG_PCI_DRA7XX_HOST
 static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
 				       struct platform_device *pdev)
 {
@@ -470,6 +475,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
 
 	return 0;
 }
+#endif
 
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.cpu_addr_fixup = dra7xx_pcie_cpu_addr_fixup,
@@ -517,23 +523,31 @@ static int dra7xx_pcie_enable_phy(struct dra7xx_pcie *dra7xx)
 	return ret;
 }
 
+#ifdef CONFIG_PCI_DRA7XX_HOST
 static const struct dra7xx_pcie_of_data dra7xx_pcie_rc_of_data = {
 	.mode = DW_PCIE_RC_TYPE,
 };
+#endif
 
+#ifdef CONFIG_PCI_DRA7XX_EP
 static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = {
 	.mode = DW_PCIE_EP_TYPE,
 };
+#endif
 
 static const struct of_device_id of_dra7xx_pcie_match[] = {
+#ifdef CONFIG_PCI_DRA7XX_HOST
 	{
 		.compatible = "ti,dra7-pcie",
 		.data = &dra7xx_pcie_rc_of_data,
 	},
+#endif
+#ifdef CONFIG_PCI_DRA7XX_EP
 	{
 		.compatible = "ti,dra7-pcie-ep",
 		.data = &dra7xx_pcie_ep_of_data,
 	},
+#endif
 	{},
 };
 
@@ -548,6 +562,7 @@ static const struct of_device_id of_dra7xx_pcie_match[] = {
  *
  * To avoid this issue set PCIE_SS1_AXI2OCP_LEGACY_MODE_ENABLE to 1.
  */
+#ifdef CONFIG_PCI_DRA7XX_EP
 static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
 {
 	int ret;
@@ -578,6 +593,7 @@ static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
 
 	return ret;
 }
+#endif
 
 static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 {
@@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 		dra7xx->link_gen = 2;
 
 	switch (mode) {
+#ifdef CONFIG_PCI_DRA7XX_HOST
 	case DW_PCIE_RC_TYPE:
 		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
 				   DEVICE_TYPE_RC);
@@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 		if (ret < 0)
 			goto err_gpio;
 		break;
+#endif
+#ifdef CONFIG_PCI_DRA7XX_EP
 	case DW_PCIE_EP_TYPE:
 		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
 				   DEVICE_TYPE_EP);
@@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 		if (ret < 0)
 			goto err_gpio;
 		break;
+#endif
 	default:
 		dev_err(dev, "INVALID device type %d\n", mode);
 	}
-- 
2.14.2

Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

[Kishon Vijay Abraham I]

Hi,

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> This way you will not build and include unused code
> when only building for only one mode.
> 
> Moved dra7xx_pcie_enable_wrapper_interrupts() in order
> to avoid adding an extra ifdef block.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> V2:
> * New patch in this series.
> 
>  drivers/pci/dwc/pci-dra7xx.c | 36 ++++++++++++++++++++++++++++--------
>  1 file changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index 009f6aeeee1c..175544d6c3ab 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -171,6 +171,15 @@ static int dra7xx_pcie_establish_link(struct dw_pcie *pci)
>  	return 0;
>  }
>  
> +static void dra7xx_pcie_enable_wrapper_interrupts(struct dra7xx_pcie *dra7xx)
> +{
> +	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN,
> +			   INTERRUPTS);
> +	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN,
> +			   INTERRUPTS);
> +}
> +
> +#ifdef CONFIG_PCI_DRA7XX_HOST
>  static void dra7xx_pcie_enable_msi_interrupts(struct dra7xx_pcie *dra7xx)
>  {
>  	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI,
> @@ -181,14 +190,6 @@ static void dra7xx_pcie_enable_msi_interrupts(struct dra7xx_pcie *dra7xx)
>  			   MSI | LEG_EP_INTERRUPTS);
>  }
>  
> -static void dra7xx_pcie_enable_wrapper_interrupts(struct dra7xx_pcie *dra7xx)
> -{
> -	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN,
> -			   INTERRUPTS);
> -	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN,
> -			   INTERRUPTS);
> -}
> -
>  static void dra7xx_pcie_enable_interrupts(struct dra7xx_pcie *dra7xx)
>  {
>  	dra7xx_pcie_enable_wrapper_interrupts(dra7xx);
> @@ -276,6 +277,7 @@ static irqreturn_t dra7xx_pcie_msi_irq_handler(int irq, void *arg)
>  
>  	return IRQ_HANDLED;
>  }
> +#endif
>  
>  static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
>  {
> @@ -336,6 +338,7 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -427,7 +430,9 @@ static int __init dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
>  
>  	return 0;
>  }
> +#endif
>  
> +#ifdef CONFIG_PCI_DRA7XX_HOST

This block can also be moved around the file so that there is a single +#ifdef
CONFIG_PCI_DRA7XX_HOST in this file.
>  static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  				       struct platform_device *pdev)
>  {
> @@ -470,6 +475,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  
>  	return 0;
>  }
> +#endif
>  
>  static const struct dw_pcie_ops dw_pcie_ops = {
>  	.cpu_addr_fixup = dra7xx_pcie_cpu_addr_fixup,
> @@ -517,23 +523,31 @@ static int dra7xx_pcie_enable_phy(struct dra7xx_pcie *dra7xx)
>  	return ret;
>  }

IMO all the #ifdef's after this point can be removed.
>  
> +#ifdef CONFIG_PCI_DRA7XX_HOST
>  static const struct dra7xx_pcie_of_data dra7xx_pcie_rc_of_data = {
>  	.mode = DW_PCIE_RC_TYPE,
>  };
> +#endif
>  
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = {
>  	.mode = DW_PCIE_EP_TYPE,
>  };
> +#endif
>  
>  static const struct of_device_id of_dra7xx_pcie_match[] = {
> +#ifdef CONFIG_PCI_DRA7XX_HOST
>  	{
>  		.compatible = "ti,dra7-pcie",
>  		.data = &dra7xx_pcie_rc_of_data,
>  	},
> +#endif
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  	{
>  		.compatible = "ti,dra7-pcie-ep",
>  		.data = &dra7xx_pcie_ep_of_data,
>  	},
> +#endif
>  	{},
>  };
>  
> @@ -548,6 +562,7 @@ static const struct of_device_id of_dra7xx_pcie_match[] = {
>   *
>   * To avoid this issue set PCIE_SS1_AXI2OCP_LEGACY_MODE_ENABLE to 1.
>   */
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
>  {
>  	int ret;
> @@ -578,6 +593,7 @@ static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
>  
>  	return ret;
>  }
> +#endif
>  
>  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  {
> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  		dra7xx->link_gen = 2;
>  
>  	switch (mode) {
> +#ifdef CONFIG_PCI_DRA7XX_HOST
>  	case DW_PCIE_RC_TYPE:
>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>  				   DEVICE_TYPE_RC);
> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  		if (ret < 0)
>  			goto err_gpio;
>  		break;
> +#endif
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  	case DW_PCIE_EP_TYPE:
>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>  				   DEVICE_TYPE_EP);
> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  		if (ret < 0)
>  			goto err_gpio;
>  		break;
> +#endif
>  	default:
>  		dev_err(dev, "INVALID device type %d\n", mode);
>  	}
> 

Thanks
Kishon

Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

[Kishon Vijay Abraham I]

Hi,

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> This way you will not build and include unused code
> when only building for only one mode.
> 
> Moved dra7xx_pcie_enable_wrapper_interrupts() in order
> to avoid adding an extra ifdef block.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> V2:
> * New patch in this series.
> 
>  drivers/pci/dwc/pci-dra7xx.c | 36 ++++++++++++++++++++++++++++--------
>  1 file changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index 009f6aeeee1c..175544d6c3ab 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -171,6 +171,15 @@ static int dra7xx_pcie_establish_link(struct dw_pcie *pci)
>  	return 0;
>  }
>  
> +static void dra7xx_pcie_enable_wrapper_interrupts(struct dra7xx_pcie *dra7xx)
> +{
> +	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN,
> +			   INTERRUPTS);
> +	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN,
> +			   INTERRUPTS);
> +}
> +
> +#ifdef CONFIG_PCI_DRA7XX_HOST
>  static void dra7xx_pcie_enable_msi_interrupts(struct dra7xx_pcie *dra7xx)
>  {
>  	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI,
> @@ -181,14 +190,6 @@ static void dra7xx_pcie_enable_msi_interrupts(struct dra7xx_pcie *dra7xx)
>  			   MSI | LEG_EP_INTERRUPTS);
>  }
>  
> -static void dra7xx_pcie_enable_wrapper_interrupts(struct dra7xx_pcie *dra7xx)
> -{
> -	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN,
> -			   INTERRUPTS);
> -	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN,
> -			   INTERRUPTS);
> -}
> -
>  static void dra7xx_pcie_enable_interrupts(struct dra7xx_pcie *dra7xx)
>  {
>  	dra7xx_pcie_enable_wrapper_interrupts(dra7xx);
> @@ -276,6 +277,7 @@ static irqreturn_t dra7xx_pcie_msi_irq_handler(int irq, void *arg)
>  
>  	return IRQ_HANDLED;
>  }
> +#endif
>  
>  static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
>  {
> @@ -336,6 +338,7 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -427,7 +430,9 @@ static int __init dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
>  
>  	return 0;
>  }
> +#endif
>  
> +#ifdef CONFIG_PCI_DRA7XX_HOST

This block can also be moved around the file so that there is a single +#ifdef
CONFIG_PCI_DRA7XX_HOST in this file.
>  static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  				       struct platform_device *pdev)
>  {
> @@ -470,6 +475,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  
>  	return 0;
>  }
> +#endif
>  
>  static const struct dw_pcie_ops dw_pcie_ops = {
>  	.cpu_addr_fixup = dra7xx_pcie_cpu_addr_fixup,
> @@ -517,23 +523,31 @@ static int dra7xx_pcie_enable_phy(struct dra7xx_pcie *dra7xx)
>  	return ret;
>  }

IMO all the #ifdef's after this point can be removed.
>  
> +#ifdef CONFIG_PCI_DRA7XX_HOST
>  static const struct dra7xx_pcie_of_data dra7xx_pcie_rc_of_data = {
>  	.mode = DW_PCIE_RC_TYPE,
>  };
> +#endif
>  
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = {
>  	.mode = DW_PCIE_EP_TYPE,
>  };
> +#endif
>  
>  static const struct of_device_id of_dra7xx_pcie_match[] = {
> +#ifdef CONFIG_PCI_DRA7XX_HOST
>  	{
>  		.compatible = "ti,dra7-pcie",
>  		.data = &dra7xx_pcie_rc_of_data,
>  	},
> +#endif
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  	{
>  		.compatible = "ti,dra7-pcie-ep",
>  		.data = &dra7xx_pcie_ep_of_data,
>  	},
> +#endif
>  	{},
>  };
>  
> @@ -548,6 +562,7 @@ static const struct of_device_id of_dra7xx_pcie_match[] = {
>   *
>   * To avoid this issue set PCIE_SS1_AXI2OCP_LEGACY_MODE_ENABLE to 1.
>   */
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
>  {
>  	int ret;
> @@ -578,6 +593,7 @@ static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
>  
>  	return ret;
>  }
> +#endif
>  
>  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  {
> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  		dra7xx->link_gen = 2;
>  
>  	switch (mode) {
> +#ifdef CONFIG_PCI_DRA7XX_HOST
>  	case DW_PCIE_RC_TYPE:
>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>  				   DEVICE_TYPE_RC);
> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  		if (ret < 0)
>  			goto err_gpio;
>  		break;
> +#endif
> +#ifdef CONFIG_PCI_DRA7XX_EP
>  	case DW_PCIE_EP_TYPE:
>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>  				   DEVICE_TYPE_EP);
> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  		if (ret < 0)
>  			goto err_gpio;
>  		break;
> +#endif
>  	default:
>  		dev_err(dev, "INVALID device type %d\n", mode);
>  	}
> 

Thanks
Kishon

Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

[Niklas Cassel]

On 10/31/2017 09:29 AM, Kishon Vijay Abraham I wrote:

(snip)

>>  
>> +#ifdef CONFIG_PCI_DRA7XX_HOST
> 
> This block can also be moved around the file so that there is a single +#ifdef
> CONFIG_PCI_DRA7XX_HOST in this file.

Hello Kishon,

Sure, will fix in V3.

>>  static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>>  				       struct platform_device *pdev)
>>  {
>> @@ -470,6 +475,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>>  
>>  	return 0;
>>  }
>> +#endif
>>  
>>  static const struct dw_pcie_ops dw_pcie_ops = {
>>  	.cpu_addr_fixup = dra7xx_pcie_cpu_addr_fixup,
>> @@ -517,23 +523,31 @@ static int dra7xx_pcie_enable_phy(struct dra7xx_pcie *dra7xx)
>>  	return ret;
>>  }
> 
> IMO all the #ifdef's after this point can be removed.

I will remove the ifdefs in the match table and the ifdefs surrounding
the of match table data.

>>  
>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>  static const struct dra7xx_pcie_of_data dra7xx_pcie_rc_of_data = {
>>  	.mode = DW_PCIE_RC_TYPE,
>>  };
>> +#endif
>>  
>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>  static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = {
>>  	.mode = DW_PCIE_EP_TYPE,
>>  };
>> +#endif
>>  
>>  static const struct of_device_id of_dra7xx_pcie_match[] = {
>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>  	{
>>  		.compatible = "ti,dra7-pcie",
>>  		.data = &dra7xx_pcie_rc_of_data,
>>  	},
>> +#endif
>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>  	{
>>  		.compatible = "ti,dra7-pcie-ep",
>>  		.data = &dra7xx_pcie_ep_of_data,
>>  	},
>> +#endif
>>  	{},
>>  };
>>  
>> @@ -548,6 +562,7 @@ static const struct of_device_id of_dra7xx_pcie_match[] = {
>>   *
>>   * To avoid this issue set PCIE_SS1_AXI2OCP_LEGACY_MODE_ENABLE to 1.
>>   */
>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>  static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
>>  {
>>  	int ret;
>> @@ -578,6 +593,7 @@ static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
>>  
>>  	return ret;
>>  }
>> +#endif

I will move this to the CONFIG_PCI_DRA7XX_EP block, so that there will only
be a single CONFIG_PCI_DRA7XX_EP ifdef surrounding EP specific functions.

>>  
>>  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>  {
>> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>  		dra7xx->link_gen = 2;
>>  
>>  	switch (mode) {
>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>  	case DW_PCIE_RC_TYPE:
>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>  				   DEVICE_TYPE_RC);
>> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>  		if (ret < 0)
>>  			goto err_gpio;
>>  		break;
>> +#endif
>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>  	case DW_PCIE_EP_TYPE:
>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>  				   DEVICE_TYPE_EP);
>> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>  		if (ret < 0)
>>  			goto err_gpio;
>>  		break;
>> +#endif

Actually, these ifdefs has to stay, otherwise we get build warnings, since we
are calling functions that aren't defined (dra7xx_pcie_ep_unaligned_memaccess,
dra7xx_add_pcie_ep, dra7xx_add_pcie_port).
We could add dummy implementations for these inside an #else block following
the ifdef blocks. However, I think that adding dummy implementations in the
#else block is uglier and more verbose than keeping the ifdefs around the
two cases.


Regards,
Niklas

Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

[Niklas Cassel]

On 10/31/2017 09:29 AM, Kishon Vijay Abraham I wrote:

(snip)

>>  
>> +#ifdef CONFIG_PCI_DRA7XX_HOST
> 
> This block can also be moved around the file so that there is a single +#ifdef
> CONFIG_PCI_DRA7XX_HOST in this file.

Hello Kishon,

Sure, will fix in V3.

>>  static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>>  				       struct platform_device *pdev)
>>  {
>> @@ -470,6 +475,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>>  
>>  	return 0;
>>  }
>> +#endif
>>  
>>  static const struct dw_pcie_ops dw_pcie_ops = {
>>  	.cpu_addr_fixup = dra7xx_pcie_cpu_addr_fixup,
>> @@ -517,23 +523,31 @@ static int dra7xx_pcie_enable_phy(struct dra7xx_pcie *dra7xx)
>>  	return ret;
>>  }
> 
> IMO all the #ifdef's after this point can be removed.

I will remove the ifdefs in the match table and the ifdefs surrounding
the of match table data.

>>  
>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>  static const struct dra7xx_pcie_of_data dra7xx_pcie_rc_of_data = {
>>  	.mode = DW_PCIE_RC_TYPE,
>>  };
>> +#endif
>>  
>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>  static const struct dra7xx_pcie_of_data dra7xx_pcie_ep_of_data = {
>>  	.mode = DW_PCIE_EP_TYPE,
>>  };
>> +#endif
>>  
>>  static const struct of_device_id of_dra7xx_pcie_match[] = {
>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>  	{
>>  		.compatible = "ti,dra7-pcie",
>>  		.data = &dra7xx_pcie_rc_of_data,
>>  	},
>> +#endif
>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>  	{
>>  		.compatible = "ti,dra7-pcie-ep",
>>  		.data = &dra7xx_pcie_ep_of_data,
>>  	},
>> +#endif
>>  	{},
>>  };
>>  
>> @@ -548,6 +562,7 @@ static const struct of_device_id of_dra7xx_pcie_match[] = {
>>   *
>>   * To avoid this issue set PCIE_SS1_AXI2OCP_LEGACY_MODE_ENABLE to 1.
>>   */
>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>  static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
>>  {
>>  	int ret;
>> @@ -578,6 +593,7 @@ static int dra7xx_pcie_ep_unaligned_memaccess(struct device *dev)
>>  
>>  	return ret;
>>  }
>> +#endif

I will move this to the CONFIG_PCI_DRA7XX_EP block, so that there will only
be a single CONFIG_PCI_DRA7XX_EP ifdef surrounding EP specific functions.

>>  
>>  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>  {
>> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>  		dra7xx->link_gen = 2;
>>  
>>  	switch (mode) {
>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>  	case DW_PCIE_RC_TYPE:
>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>  				   DEVICE_TYPE_RC);
>> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>  		if (ret < 0)
>>  			goto err_gpio;
>>  		break;
>> +#endif
>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>  	case DW_PCIE_EP_TYPE:
>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>  				   DEVICE_TYPE_EP);
>> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>  		if (ret < 0)
>>  			goto err_gpio;
>>  		break;
>> +#endif

Actually, these ifdefs has to stay, otherwise we get build warnings, since we
are calling functions that aren't defined (dra7xx_pcie_ep_unaligned_memaccess,
dra7xx_add_pcie_ep, dra7xx_add_pcie_port).
We could add dummy implementations for these inside an #else block following
the ifdef blocks. However, I think that adding dummy implementations in the
#else block is uglier and more verbose than keeping the ifdefs around the
two cases.


Regards,
Niklas

Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

[Niklas Cassel]

>>>  
>>>  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>  {
>>> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>  		dra7xx->link_gen = 2;
>>>  
>>>  	switch (mode) {
>>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>>  	case DW_PCIE_RC_TYPE:
>>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>>  				   DEVICE_TYPE_RC);
>>> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>  		if (ret < 0)
>>>  			goto err_gpio;
>>>  		break;
>>> +#endif
>>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>>  	case DW_PCIE_EP_TYPE:
>>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>>  				   DEVICE_TYPE_EP);
>>> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>  		if (ret < 0)
>>>  			goto err_gpio;
>>>  		break;
>>> +#endif
> 
> Actually, these ifdefs has to stay, otherwise we get build warnings, since we
> are calling functions that aren't defined (dra7xx_pcie_ep_unaligned_memaccess,
> dra7xx_add_pcie_ep, dra7xx_add_pcie_port).
> We could add dummy implementations for these inside an #else block following
> the ifdef blocks. However, I think that adding dummy implementations in the
> #else block is uglier and more verbose than keeping the ifdefs around the
> two cases.
> 

..however, if you prefer dummy implementations inside the #else blocks,
I will of course do that.


Regards,
Niklas

Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

[Niklas Cassel]

>>>  
>>>  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>  {
>>> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>  		dra7xx->link_gen = 2;
>>>  
>>>  	switch (mode) {
>>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>>  	case DW_PCIE_RC_TYPE:
>>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>>  				   DEVICE_TYPE_RC);
>>> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>  		if (ret < 0)
>>>  			goto err_gpio;
>>>  		break;
>>> +#endif
>>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>>  	case DW_PCIE_EP_TYPE:
>>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>>  				   DEVICE_TYPE_EP);
>>> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>  		if (ret < 0)
>>>  			goto err_gpio;
>>>  		break;
>>> +#endif
> 
> Actually, these ifdefs has to stay, otherwise we get build warnings, since we
> are calling functions that aren't defined (dra7xx_pcie_ep_unaligned_memaccess,
> dra7xx_add_pcie_ep, dra7xx_add_pcie_port).
> We could add dummy implementations for these inside an #else block following
> the ifdef blocks. However, I think that adding dummy implementations in the
> #else block is uglier and more verbose than keeping the ifdefs around the
> two cases.
> 

..however, if you prefer dummy implementations inside the #else blocks,
I will of course do that.


Regards,
Niklas

Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

[Niklas Cassel]

On 10/31/2017 10:38 PM, Niklas Cassel wrote:
>>>>  
>>>>  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>>  {
>>>> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>>  		dra7xx->link_gen = 2;
>>>>  
>>>>  	switch (mode) {
>>>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>>>  	case DW_PCIE_RC_TYPE:
>>>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>>>  				   DEVICE_TYPE_RC);
>>>> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>>  		if (ret < 0)
>>>>  			goto err_gpio;
>>>>  		break;
>>>> +#endif
>>>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>>>  	case DW_PCIE_EP_TYPE:
>>>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>>>  				   DEVICE_TYPE_EP);
>>>> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>>  		if (ret < 0)
>>>>  			goto err_gpio;
>>>>  		break;
>>>> +#endif
>>
>> Actually, these ifdefs has to stay, otherwise we get build warnings, since we
>> are calling functions that aren't defined (dra7xx_pcie_ep_unaligned_memaccess,
>> dra7xx_add_pcie_ep, dra7xx_add_pcie_port).
>> We could add dummy implementations for these inside an #else block following
>> the ifdef blocks. However, I think that adding dummy implementations in the
>> #else block is uglier and more verbose than keeping the ifdefs around the
>> two cases.
>>
> 
> ..however, if you prefer dummy implementations inside the #else blocks,
> I will of course do that.
> 


And after thinking about it for a while a decided to go with dummy
implementations in the #else blocks anyway :)

This is how we usually do it in kernel code, we have one implementation
if a certain Kconfig is set, and a dummy one if the Kconfig is not set.
This way we avoid seeing ifdefs sprinkled all over the code, which makes
it easier when trying to follow the flow of the code.


Regards,
Niklas

Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

[Niklas Cassel]

On 10/31/2017 10:38 PM, Niklas Cassel wrote:
>>>>  
>>>>  static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>>  {
>>>> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>>  		dra7xx->link_gen = 2;
>>>>  
>>>>  	switch (mode) {
>>>> +#ifdef CONFIG_PCI_DRA7XX_HOST
>>>>  	case DW_PCIE_RC_TYPE:
>>>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>>>  				   DEVICE_TYPE_RC);
>>>> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>>  		if (ret < 0)
>>>>  			goto err_gpio;
>>>>  		break;
>>>> +#endif
>>>> +#ifdef CONFIG_PCI_DRA7XX_EP
>>>>  	case DW_PCIE_EP_TYPE:
>>>>  		dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>>>>  				   DEVICE_TYPE_EP);
>>>> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>>>  		if (ret < 0)
>>>>  			goto err_gpio;
>>>>  		break;
>>>> +#endif
>>
>> Actually, these ifdefs has to stay, otherwise we get build warnings, since we
>> are calling functions that aren't defined (dra7xx_pcie_ep_unaligned_memaccess,
>> dra7xx_add_pcie_ep, dra7xx_add_pcie_port).
>> We could add dummy implementations for these inside an #else block following
>> the ifdef blocks. However, I think that adding dummy implementations in the
>> #else block is uglier and more verbose than keeping the ifdefs around the
>> two cases.
>>
> 
> ..however, if you prefer dummy implementations inside the #else blocks,
> I will of course do that.
> 


And after thinking about it for a while a decided to go with dummy
implementations in the #else blocks anyway :)

This is how we usually do it in kernel code, we have one implementation
if a certain Kconfig is set, and a dummy one if the Kconfig is not set.
This way we avoid seeing ifdefs sprinkled all over the code, which makes
it easier when trying to follow the flow of the code.


Regards,
Niklas

[PATCH v2 10/17] PCI: dwc: artpec6: Remove unused defines

[Niklas Cassel]

Commit b015b37e6693 ("PCI: artpec6: Stop enabling writes to
DBI read-only registers") removed the only write using these
defines, but it did not remove the defines.
Remove the defines since they are now unused.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* Changed commit message to include correct prefix.

 drivers/pci/dwc/pcie-artpec6.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
index 6653619db6a1..4b8ef266dc2f 100644
--- a/drivers/pci/dwc/pcie-artpec6.c
+++ b/drivers/pci/dwc/pcie-artpec6.c
@@ -37,9 +37,6 @@ struct artpec6_pcie {
 #define PCIE_PHY_DEBUG_R0		(PL_OFFSET + 0x28)
 #define PCIE_PHY_DEBUG_R1		(PL_OFFSET + 0x2c)
 
-#define MISC_CONTROL_1_OFF		(PL_OFFSET + 0x1bc)
-#define  DBI_RO_WR_EN			1
-
 /* ARTPEC-6 specific registers */
 #define PCIECFG				0x18
 #define  PCIECFG_DBG_OEN		(1 << 24)
-- 
2.14.2

[PATCH v2 11/17] PCI: dwc: artpec6: Use BIT and GENMASK macros

[Niklas Cassel]

This greatly improves readability.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* No change.

 drivers/pci/dwc/pcie-artpec6.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
index 4b8ef266dc2f..18075e0fab80 100644
--- a/drivers/pci/dwc/pcie-artpec6.c
+++ b/drivers/pci/dwc/pcie-artpec6.c
@@ -39,28 +39,28 @@ struct artpec6_pcie {
 
 /* ARTPEC-6 specific registers */
 #define PCIECFG				0x18
-#define  PCIECFG_DBG_OEN		(1 << 24)
-#define  PCIECFG_CORE_RESET_REQ		(1 << 21)
-#define  PCIECFG_LTSSM_ENABLE		(1 << 20)
-#define  PCIECFG_CLKREQ_B		(1 << 11)
-#define  PCIECFG_REFCLK_ENABLE		(1 << 10)
-#define  PCIECFG_PLL_ENABLE		(1 << 9)
-#define  PCIECFG_PCLK_ENABLE		(1 << 8)
-#define  PCIECFG_RISRCREN		(1 << 4)
-#define  PCIECFG_MODE_TX_DRV_EN		(1 << 3)
-#define  PCIECFG_CISRREN		(1 << 2)
-#define  PCIECFG_MACRO_ENABLE		(1 << 0)
+#define  PCIECFG_DBG_OEN		BIT(24)
+#define  PCIECFG_CORE_RESET_REQ		BIT(21)
+#define  PCIECFG_LTSSM_ENABLE		BIT(20)
+#define  PCIECFG_CLKREQ_B		BIT(11)
+#define  PCIECFG_REFCLK_ENABLE		BIT(10)
+#define  PCIECFG_PLL_ENABLE		BIT(9)
+#define  PCIECFG_PCLK_ENABLE		BIT(8)
+#define  PCIECFG_RISRCREN		BIT(4)
+#define  PCIECFG_MODE_TX_DRV_EN		BIT(3)
+#define  PCIECFG_CISRREN		BIT(2)
+#define  PCIECFG_MACRO_ENABLE		BIT(0)
 
 #define NOCCFG				0x40
-#define NOCCFG_ENABLE_CLK_PCIE		(1 << 4)
-#define NOCCFG_POWER_PCIE_IDLEACK	(1 << 3)
-#define NOCCFG_POWER_PCIE_IDLE		(1 << 2)
-#define NOCCFG_POWER_PCIE_IDLEREQ	(1 << 1)
+#define  NOCCFG_ENABLE_CLK_PCIE		BIT(4)
+#define  NOCCFG_POWER_PCIE_IDLEACK	BIT(3)
+#define  NOCCFG_POWER_PCIE_IDLE		BIT(2)
+#define  NOCCFG_POWER_PCIE_IDLEREQ	BIT(1)
 
 #define PHY_STATUS			0x118
-#define PHY_COSPLLLOCK			(1 << 0)
+#define  PHY_COSPLLLOCK			BIT(0)
 
-#define ARTPEC6_CPU_TO_BUS_ADDR		0x0fffffff
+#define ARTPEC6_CPU_TO_BUS_ADDR		GENMASK(27, 0)
 
 static u32 artpec6_pcie_readl(struct artpec6_pcie *artpec6_pcie, u32 offset)
 {
-- 
2.14.2

[PATCH v2 12/17] PCI: dwc: artpec6: Split artpec6_pcie_establish_link to smaller functions

[Niklas Cassel]

This is done to better match other drivers such as dra7xx and imx6,
but also to prepare for endpoint mode support.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* Changed the order of the functions to be more logical.

 drivers/pci/dwc/pcie-artpec6.c | 53 +++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
index 18075e0fab80..3b635e745d25 100644
--- a/drivers/pci/dwc/pcie-artpec6.c
+++ b/drivers/pci/dwc/pcie-artpec6.c
@@ -80,18 +80,23 @@ static u64 artpec6_pcie_cpu_addr_fixup(u64 pci_addr)
 	return pci_addr & ARTPEC6_CPU_TO_BUS_ADDR;
 }
 
-static int artpec6_pcie_establish_link(struct artpec6_pcie *artpec6_pcie)
+static int artpec6_pcie_establish_link(struct dw_pcie *pci)
 {
-	struct dw_pcie *pci = artpec6_pcie->pci;
-	struct pcie_port *pp = &pci->pp;
+	struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pci);
 	u32 val;
-	unsigned int retries;
 
-	/* Hold DW core in reset */
 	val = artpec6_pcie_readl(artpec6_pcie, PCIECFG);
-	val |= PCIECFG_CORE_RESET_REQ;
+	val |= PCIECFG_LTSSM_ENABLE;
 	artpec6_pcie_writel(artpec6_pcie, PCIECFG, val);
 
+	return 0;
+}
+
+static void artpec6_pcie_init_phy(struct artpec6_pcie *artpec6_pcie)
+{
+	u32 val;
+	unsigned int retries;
+
 	val = artpec6_pcie_readl(artpec6_pcie, PCIECFG);
 	val |=  PCIECFG_RISRCREN |	/* Receiver term. 50 Ohm */
 		PCIECFG_MODE_TX_DRV_EN |
@@ -131,30 +136,25 @@ static int artpec6_pcie_establish_link(struct artpec6_pcie *artpec6_pcie)
 		val = readl(artpec6_pcie->phy_base + PHY_STATUS);
 		retries--;
 	} while (retries && !(val & PHY_COSPLLLOCK));
+}
+
+static void artpec6_pcie_assert_core_reset(struct artpec6_pcie *artpec6_pcie)
+{
+	u32 val;
 
-	/* Take DW core out of reset */
 	val = artpec6_pcie_readl(artpec6_pcie, PCIECFG);
-	val &= ~PCIECFG_CORE_RESET_REQ;
+	val |= PCIECFG_CORE_RESET_REQ;
 	artpec6_pcie_writel(artpec6_pcie, PCIECFG, val);
-	usleep_range(100, 200);
+}
 
-	/* setup root complex */
-	dw_pcie_setup_rc(pp);
+static void artpec6_pcie_deassert_core_reset(struct artpec6_pcie *artpec6_pcie)
+{
+	u32 val;
 
-	/* assert LTSSM enable */
 	val = artpec6_pcie_readl(artpec6_pcie, PCIECFG);
-	val |= PCIECFG_LTSSM_ENABLE;
+	val &= ~PCIECFG_CORE_RESET_REQ;
 	artpec6_pcie_writel(artpec6_pcie, PCIECFG, val);
-
-	/* check if the link is up or not */
-	if (!dw_pcie_wait_for_link(pci))
-		return 0;
-
-	dev_dbg(pci->dev, "DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
-		dw_pcie_readl_dbi(pci, PCIE_PHY_DEBUG_R0),
-		dw_pcie_readl_dbi(pci, PCIE_PHY_DEBUG_R1));
-
-	return -ETIMEDOUT;
+	usleep_range(100, 200);
 }
 
 static void artpec6_pcie_enable_interrupts(struct artpec6_pcie *artpec6_pcie)
@@ -171,7 +171,12 @@ static int artpec6_pcie_host_init(struct pcie_port *pp)
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pci);
 
-	artpec6_pcie_establish_link(artpec6_pcie);
+	artpec6_pcie_assert_core_reset(artpec6_pcie);
+	artpec6_pcie_init_phy(artpec6_pcie);
+	artpec6_pcie_deassert_core_reset(artpec6_pcie);
+	dw_pcie_setup_rc(pp);
+	artpec6_pcie_establish_link(pci);
+	dw_pcie_wait_for_link(pci);
 	artpec6_pcie_enable_interrupts(artpec6_pcie);
 
 	return 0;
-- 
2.14.2

[PATCH v2 13/17] bindings: PCI: artpec: Add support for endpoint mode

[Niklas Cassel]

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
Acked-by: Rob Herring <robh@kernel.org>
---
V2:
* Split out the DT binding change to a self contained patch.
* Added Rob's ack (from V1). No change to this file since V1.

 Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
index 4e4aee4439ea..33eef7ae5a23 100644
--- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
@@ -4,7 +4,8 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
 and thus inherits all the common properties defined in designware-pcie.txt.
 
 Required properties:
-- compatible: "axis,artpec6-pcie", "snps,dw-pcie"
+- compatible: "axis,artpec6-pcie", "snps,dw-pcie" for ARTPEC-6 in RC mode;
+	      "axis,artpec6-pcie-ep", "snps,dw-pcie" for ARTPEC-6 in EP mode;
 - reg: base addresses and lengths of the PCIe controller (DBI),
 	the PHY controller, and configuration address space.
 - reg-names: Must include the following entries:
-- 
2.14.2

[PATCH v2 14/17] PCI: dwc: artpec6: Add support for endpoint mode

[Niklas Cassel]

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* DT binding change is now in a separate commit.
* Removed local copy of dw_pcie_ep_reset_bar, it is now part of
  pcie-designware-ep.c.
* Removed raise_msi_irq, it is now part of pcie-designware-ep.c.
* Refactored Kconfig and Makefile handling.
* Added ifdefs on host/ep mode specific code.
* Changed the order of the functions to be more logical.

 drivers/pci/dwc/Kconfig        |  23 ++++--
 drivers/pci/dwc/pcie-artpec6.c | 162 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 174 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
index 3954353e3e2e..0fb96c7754de 100644
--- a/drivers/pci/dwc/Kconfig
+++ b/drivers/pci/dwc/Kconfig
@@ -148,15 +148,28 @@ config PCIE_ARMADA_8K
 	  DesignWare core functions to implement the driver.
 
 config PCIE_ARTPEC6
-	bool "Axis ARTPEC-6 PCIe controller"
-	depends on PCI
+	bool
+
+config PCIE_ARTPEC6_HOST
+	bool "Axis ARTPEC-6 PCIe controller Host Mode"
 	depends on MACH_ARTPEC6
-	depends on PCI_MSI_IRQ_DOMAIN
+	depends on PCI && PCI_MSI_IRQ_DOMAIN
 	select PCIEPORTBUS
 	select PCIE_DW_HOST
+	select PCIE_ARTPEC6
+	help
+	  Enables support for the PCIe controller in the ARTPEC-6 SoC to work in
+	  host mode. This uses the DesignWare core.
+
+config PCIE_ARTPEC6_EP
+	bool "Axis ARTPEC-6 PCIe controller Endpoint Mode"
+	depends on MACH_ARTPEC6
+	depends on PCI_ENDPOINT
+	select PCIE_DW_EP
+	select PCIE_ARTPEC6
 	help
-	  Say Y here to enable PCIe controller support on Axis ARTPEC-6
-	  SoCs.  This PCIe controller uses the DesignWare core.
+	  Enables support for the PCIe controller in the ARTPEC-6 SoC to work in
+	  endpoint mode. This uses the DesignWare core.
 
 config PCIE_KIRIN
 	depends on OF && ARM64
diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
index 3b635e745d25..d47da02b08fe 100644
--- a/drivers/pci/dwc/pcie-artpec6.c
+++ b/drivers/pci/dwc/pcie-artpec6.c
@@ -13,6 +13,7 @@
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/of_device.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/resource.h>
@@ -30,8 +31,15 @@ struct artpec6_pcie {
 	struct dw_pcie		*pci;
 	struct regmap		*regmap;	/* DT axis,syscon-pcie */
 	void __iomem		*phy_base;	/* DT phy */
+	enum dw_pcie_device_mode mode;
 };
 
+struct artpec_pcie_of_data {
+	enum dw_pcie_device_mode mode;
+};
+
+static const struct of_device_id artpec6_pcie_of_match[];
+
 /* PCIe Port Logic registers (memory-mapped) */
 #define PL_OFFSET			0x700
 #define PCIE_PHY_DEBUG_R0		(PL_OFFSET + 0x28)
@@ -42,6 +50,7 @@ struct artpec6_pcie {
 #define  PCIECFG_DBG_OEN		BIT(24)
 #define  PCIECFG_CORE_RESET_REQ		BIT(21)
 #define  PCIECFG_LTSSM_ENABLE		BIT(20)
+#define  PCIECFG_DEVICE_TYPE_MASK	GENMASK(19, 16)
 #define  PCIECFG_CLKREQ_B		BIT(11)
 #define  PCIECFG_REFCLK_ENABLE		BIT(10)
 #define  PCIECFG_PLL_ENABLE		BIT(9)
@@ -92,6 +101,22 @@ static int artpec6_pcie_establish_link(struct dw_pcie *pci)
 	return 0;
 }
 
+static void artpec6_pcie_stop_link(struct dw_pcie *pci)
+{
+	struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pci);
+	u32 val;
+
+	val = artpec6_pcie_readl(artpec6_pcie, PCIECFG);
+	val &= ~PCIECFG_LTSSM_ENABLE;
+	artpec6_pcie_writel(artpec6_pcie, PCIECFG, val);
+}
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+	.cpu_addr_fixup = artpec6_pcie_cpu_addr_fixup,
+	.start_link = artpec6_pcie_establish_link,
+	.stop_link = artpec6_pcie_stop_link,
+};
+
 static void artpec6_pcie_init_phy(struct artpec6_pcie *artpec6_pcie)
 {
 	u32 val;
@@ -157,6 +182,7 @@ static void artpec6_pcie_deassert_core_reset(struct artpec6_pcie *artpec6_pcie)
 	usleep_range(100, 200);
 }
 
+#ifdef CONFIG_PCIE_ARTPEC6_HOST
 static void artpec6_pcie_enable_interrupts(struct artpec6_pcie *artpec6_pcie)
 {
 	struct dw_pcie *pci = artpec6_pcie->pci;
@@ -231,11 +257,80 @@ static int artpec6_add_pcie_port(struct artpec6_pcie *artpec6_pcie,
 
 	return 0;
 }
+#endif
 
-static const struct dw_pcie_ops dw_pcie_ops = {
-	.cpu_addr_fixup = artpec6_pcie_cpu_addr_fixup,
+#ifdef CONFIG_PCIE_ARTPEC6_EP
+static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pci);
+	enum pci_barno bar;
+
+	artpec6_pcie_assert_core_reset(artpec6_pcie);
+	artpec6_pcie_init_phy(artpec6_pcie);
+	artpec6_pcie_deassert_core_reset(artpec6_pcie);
+
+	for (bar = BAR_0; bar <= BAR_5; bar++)
+		dw_pcie_ep_reset_bar(pci, bar);
+}
+
+static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep,
+				  enum pci_epc_irq_type type, u8 interrupt_num)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+
+	switch (type) {
+	case PCI_EPC_IRQ_LEGACY:
+		dev_err(pci->dev, "EP cannot trigger legacy IRQs\n");
+		return -EINVAL;
+	case PCI_EPC_IRQ_MSI:
+		return dw_pcie_ep_raise_msi_irq(ep, interrupt_num);
+	default:
+		dev_err(pci->dev, "UNKNOWN IRQ type\n");
+	}
+
+	return 0;
+}
+
+static struct dw_pcie_ep_ops pcie_ep_ops = {
+	.ep_init = artpec6_pcie_ep_init,
+	.raise_irq = artpec6_pcie_raise_irq,
 };
 
+static int artpec6_add_pcie_ep(struct artpec6_pcie *artpec6_pcie,
+			       struct platform_device *pdev)
+{
+	int ret;
+	struct dw_pcie_ep *ep;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	struct dw_pcie *pci = artpec6_pcie->pci;
+
+	ep = &pci->ep;
+	ep->ops = &pcie_ep_ops;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2");
+	pci->dbi_base2 = devm_ioremap(dev, res->start, resource_size(res));
+	if (IS_ERR(pci->dbi_base2))
+		return PTR_ERR(pci->dbi_base2);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
+	if (!res)
+		return -EINVAL;
+
+	ep->phys_base = res->start;
+	ep->addr_size = resource_size(res);
+
+	ret = dw_pcie_ep_init(ep);
+	if (ret) {
+		dev_err(dev, "failed to initialize endpoint\n");
+		return ret;
+	}
+
+	return 0;
+}
+#endif
+
 static int artpec6_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -244,6 +339,16 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
 	struct resource *dbi_base;
 	struct resource *phy_base;
 	int ret;
+	const struct of_device_id *match;
+	const struct artpec_pcie_of_data *data;
+	enum dw_pcie_device_mode mode;
+
+	match = of_match_device(artpec6_pcie_of_match, dev);
+	if (!match)
+		return -EINVAL;
+
+	data = (struct artpec_pcie_of_data *)match->data;
+	mode = (enum dw_pcie_device_mode)data->mode;
 
 	artpec6_pcie = devm_kzalloc(dev, sizeof(*artpec6_pcie), GFP_KERNEL);
 	if (!artpec6_pcie)
@@ -257,6 +362,7 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
 	pci->ops = &dw_pcie_ops;
 
 	artpec6_pcie->pci = pci;
+	artpec6_pcie->mode = mode;
 
 	dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
 	pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
@@ -276,15 +382,59 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, artpec6_pcie);
 
-	ret = artpec6_add_pcie_port(artpec6_pcie, pdev);
-	if (ret < 0)
-		return ret;
+	switch (artpec6_pcie->mode) {
+#ifdef CONFIG_PCIE_ARTPEC6_HOST
+	case DW_PCIE_RC_TYPE:
+		ret = artpec6_add_pcie_port(artpec6_pcie, pdev);
+		if (ret < 0)
+			return ret;
+		break;
+#endif
+#ifdef CONFIG_PCIE_ARTPEC6_EP
+	case DW_PCIE_EP_TYPE: {
+		u32 val;
+
+		val = artpec6_pcie_readl(artpec6_pcie, PCIECFG);
+		val &= ~PCIECFG_DEVICE_TYPE_MASK;
+		artpec6_pcie_writel(artpec6_pcie, PCIECFG, val);
+		ret = artpec6_add_pcie_ep(artpec6_pcie, pdev);
+		if (ret < 0)
+			return ret;
+		break;
+	}
+#endif
+	default:
+		dev_err(dev, "INVALID device type %d\n", artpec6_pcie->mode);
+	}
 
 	return 0;
 }
 
+#ifdef CONFIG_PCIE_ARTPEC6_HOST
+static const struct artpec_pcie_of_data artpec6_pcie_rc_of_data = {
+	.mode = DW_PCIE_RC_TYPE,
+};
+#endif
+
+#ifdef CONFIG_PCIE_ARTPEC6_EP
+static const struct artpec_pcie_of_data artpec6_pcie_ep_of_data = {
+	.mode = DW_PCIE_EP_TYPE,
+};
+#endif
+
 static const struct of_device_id artpec6_pcie_of_match[] = {
-	{ .compatible = "axis,artpec6-pcie", },
+#ifdef CONFIG_PCIE_ARTPEC6_HOST
+	{
+		.compatible = "axis,artpec6-pcie",
+		.data = &artpec6_pcie_rc_of_data,
+	},
+#endif
+#ifdef CONFIG_PCIE_ARTPEC6_EP
+	{
+		.compatible = "axis,artpec6-pcie-ep",
+		.data = &artpec6_pcie_ep_of_data,
+	},
+#endif
 	{},
 };
 
-- 
2.14.2

[PATCH v2 15/17] PCI: dwc: Make cpu_addr_fixup take struct dw_pcie as argument

[Niklas Cassel]

There is no need to hard code the cpu to bus address fixup mask.
The PCIe controller has a global address on the AXI bus, however,
from the perspective of the PCIe controller, its base starts at 0x0,
so the local address is 0x0. To get the bus address, simply subtract
the global address from the cpu address. The global address is taken
from device tree.

Also for ARTPEC-7, hard coding the cpu to bus address fixup mask is
not possible, since it uses a High Address Bits Look Up Table, which
means that it can, at runtime, map the PCIe window to an arbitrary
address in the 32-bit address space.

This also fixes a bug for ARTPEC-6, where the cpu to bus address
fixup mask previously was off by one (GENMASK(27, 0), rather than
GENMASK(28, 0)), which is another reason why the it is a good idea
to calculate the mask by using values from device tree.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* Reworked cpu_addr_fixup for pcie-artpec6, since the previous logic did
  not handle the case where the size of the pcie window was greater than
  the global address of the pcie controller.

 drivers/pci/dwc/pci-dra7xx.c      |  2 +-
 drivers/pci/dwc/pcie-artpec6.c    | 18 ++++++++++++++----
 drivers/pci/dwc/pcie-designware.c |  2 +-
 drivers/pci/dwc/pcie-designware.h |  2 +-
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
index 175544d6c3ab..c01277811c8e 100644
--- a/drivers/pci/dwc/pci-dra7xx.c
+++ b/drivers/pci/dwc/pci-dra7xx.c
@@ -109,7 +109,7 @@ static inline void dra7xx_pcie_writel(struct dra7xx_pcie *pcie, u32 offset,
 	writel(value, pcie->base + offset);
 }
 
-static u64 dra7xx_pcie_cpu_addr_fixup(u64 pci_addr)
+static u64 dra7xx_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 pci_addr)
 {
 	return pci_addr & DRA7XX_CPU_TO_BUS_ADDR;
 }
diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
index d47da02b08fe..ef8815ea4776 100644
--- a/drivers/pci/dwc/pcie-artpec6.c
+++ b/drivers/pci/dwc/pcie-artpec6.c
@@ -69,8 +69,6 @@ static const struct of_device_id artpec6_pcie_of_match[];
 #define PHY_STATUS			0x118
 #define  PHY_COSPLLLOCK			BIT(0)
 
-#define ARTPEC6_CPU_TO_BUS_ADDR		GENMASK(27, 0)
-
 static u32 artpec6_pcie_readl(struct artpec6_pcie *artpec6_pcie, u32 offset)
 {
 	u32 val;
@@ -84,9 +82,21 @@ static void artpec6_pcie_writel(struct artpec6_pcie *artpec6_pcie, u32 offset, u
 	regmap_write(artpec6_pcie->regmap, offset, val);
 }
 
-static u64 artpec6_pcie_cpu_addr_fixup(u64 pci_addr)
+static u64 artpec6_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 pci_addr)
 {
-	return pci_addr & ARTPEC6_CPU_TO_BUS_ADDR;
+	struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pci);
+	struct pcie_port *pp = &pci->pp;
+	struct dw_pcie_ep *ep = &pci->ep;
+
+	switch (artpec6_pcie->mode) {
+	case DW_PCIE_RC_TYPE:
+		return pci_addr - pp->cfg0_base;
+	case DW_PCIE_EP_TYPE:
+		return pci_addr - ep->phys_base;
+	default:
+		dev_err(pci->dev, "UNKNOWN device type\n");
+	}
+	return pci_addr;
 }
 
 static int artpec6_pcie_establish_link(struct dw_pcie *pci)
diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
index 88abdddee2ad..800be7a4f087 100644
--- a/drivers/pci/dwc/pcie-designware.c
+++ b/drivers/pci/dwc/pcie-designware.c
@@ -149,7 +149,7 @@ void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
 	u32 retries, val;
 
 	if (pci->ops->cpu_addr_fixup)
-		cpu_addr = pci->ops->cpu_addr_fixup(cpu_addr);
+		cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
 
 	if (pci->iatu_unroll_enabled) {
 		dw_pcie_prog_outbound_atu_unroll(pci, index, type, cpu_addr,
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index 24edac035160..cca5a81c1c74 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -205,7 +205,7 @@ struct dw_pcie_ep {
 };
 
 struct dw_pcie_ops {
-	u64	(*cpu_addr_fixup)(u64 cpu_addr);
+	u64	(*cpu_addr_fixup)(struct dw_pcie *pcie, u64 cpu_addr);
 	u32	(*read_dbi)(struct dw_pcie *pcie, void __iomem *base, u32 reg,
 			    size_t size);
 	void	(*write_dbi)(struct dw_pcie *pcie, void __iomem *base, u32 reg,
-- 
2.14.2

[PATCH v2 16/17] bindings: PCI: artpec: Add support for the ARTPEC-7 SoC

[Niklas Cassel]

The ARTPEC-6 SoC and the ARTPEC-7 SoC are very similar.
Unfortunately, some fields in the PCIECFG and PCIESTAT
register have changed.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
Acked-by: Rob Herring <robh@kernel.org>
---
V2:
* Split out the DT binding change to a self contained patch.
* Added Rob's ack (from V1). No change to this file since V1.

 Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
index 33eef7ae5a23..979dc7b6cfe8 100644
--- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
@@ -6,6 +6,8 @@ and thus inherits all the common properties defined in designware-pcie.txt.
 Required properties:
 - compatible: "axis,artpec6-pcie", "snps,dw-pcie" for ARTPEC-6 in RC mode;
 	      "axis,artpec6-pcie-ep", "snps,dw-pcie" for ARTPEC-6 in EP mode;
+	      "axis,artpec7-pcie", "snps,dw-pcie" for ARTPEC-7 in RC mode;
+	      "axis,artpec7-pcie-ep", "snps,dw-pcie" for ARTPEC-7 in EP mode;
 - reg: base addresses and lengths of the PCIe controller (DBI),
 	the PHY controller, and configuration address space.
 - reg-names: Must include the following entries:
-- 
2.14.2

[PATCH v2 17/17] PCI: dwc: artpec6: Add support for the ARTPEC-7 SoC

[Niklas Cassel]

The ARTPEC-6 SoC and the ARTPEC-7 SoC are very similar.
Unfortunately, some fields in the PCIECFG and PCIESTAT
register have changed.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
V2:
* DT binding change is now in a separate commit.
* Changed the order of the functions to be more logical.

 drivers/pci/dwc/pcie-artpec6.c | 162 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 159 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
index ef8815ea4776..679874794a51 100644
--- a/drivers/pci/dwc/pcie-artpec6.c
+++ b/drivers/pci/dwc/pcie-artpec6.c
@@ -27,14 +27,21 @@
 
 #define to_artpec6_pcie(x)	dev_get_drvdata((x)->dev)
 
+enum artpec_pcie_variants {
+	ARTPEC6,
+	ARTPEC7,
+};
+
 struct artpec6_pcie {
 	struct dw_pcie		*pci;
 	struct regmap		*regmap;	/* DT axis,syscon-pcie */
 	void __iomem		*phy_base;	/* DT phy */
+	enum artpec_pcie_variants variant;
 	enum dw_pcie_device_mode mode;
 };
 
 struct artpec_pcie_of_data {
+	enum artpec_pcie_variants variant;
 	enum dw_pcie_device_mode mode;
 };
 
@@ -45,6 +52,13 @@ static const struct of_device_id artpec6_pcie_of_match[];
 #define PCIE_PHY_DEBUG_R0		(PL_OFFSET + 0x28)
 #define PCIE_PHY_DEBUG_R1		(PL_OFFSET + 0x2c)
 
+#define ACK_F_ASPM_CTRL_OFF		(PL_OFFSET + 0xc)
+#define ACK_N_FTS_MASK			GENMASK(15, 8)
+#define ACK_N_FTS(x)			(((x) << 8) & ACK_N_FTS_MASK)
+
+#define FAST_TRAINING_SEQ_MASK		GENMASK(7, 0)
+#define FAST_TRAINING_SEQ(x)		(((x) << 0) & FAST_TRAINING_SEQ_MASK)
+
 /* ARTPEC-6 specific registers */
 #define PCIECFG				0x18
 #define  PCIECFG_DBG_OEN		BIT(24)
@@ -59,6 +73,13 @@ static const struct of_device_id artpec6_pcie_of_match[];
 #define  PCIECFG_MODE_TX_DRV_EN		BIT(3)
 #define  PCIECFG_CISRREN		BIT(2)
 #define  PCIECFG_MACRO_ENABLE		BIT(0)
+/* ARTPEC-7 specific fields */
+#define  PCIECFG_REFCLKSEL		BIT(23)
+#define  PCIECFG_NOC_RESET		BIT(3)
+
+#define PCIESTAT			0x1c
+/* ARTPEC-7 specific fields */
+#define  PCIESTAT_EXTREFCLK		BIT(3)
 
 #define NOCCFG				0x40
 #define  NOCCFG_ENABLE_CLK_PCIE		BIT(4)
@@ -69,6 +90,12 @@ static const struct of_device_id artpec6_pcie_of_match[];
 #define PHY_STATUS			0x118
 #define  PHY_COSPLLLOCK			BIT(0)
 
+#define PHY_TX_ASIC_OUT			0x1014
+#define  PHY_TX_ASIC_OUT_TX_ACK		BIT(0)
+
+#define PHY_RX_ASIC_OUT			0x101b
+#define  PHY_RX_ASIC_OUT_ACK		BIT(0)
+
 static u32 artpec6_pcie_readl(struct artpec6_pcie *artpec6_pcie, u32 offset)
 {
 	u32 val;
@@ -127,7 +154,7 @@ static const struct dw_pcie_ops dw_pcie_ops = {
 	.stop_link = artpec6_pcie_stop_link,
 };
 
-static void artpec6_pcie_init_phy(struct artpec6_pcie *artpec6_pcie)
+static void artpec6_pcie_init_phy_a6(struct artpec6_pcie *artpec6_pcie)
 {
 	u32 val;
 	unsigned int retries;
@@ -173,12 +200,109 @@ static void artpec6_pcie_init_phy(struct artpec6_pcie *artpec6_pcie)
 	} while (retries && !(val & PHY_COSPLLLOCK));
 }
 
+static void artpec6_pcie_init_phy_a7(struct artpec6_pcie *artpec6_pcie)
+{
+	struct dw_pcie *pci = artpec6_pcie->pci;
+	u32 val;
+	u16 phy_status_tx, phy_status_rx;
+	unsigned int retries;
+	bool extrefclk;
+
+	/* Check if external reference clock is connected */
+	val = artpec6_pcie_readl(artpec6_pcie, PCIESTAT);
+	extrefclk = !!(val & PCIESTAT_EXTREFCLK);
+	dev_dbg(pci->dev, "Using reference clock: %s\n",
+		extrefclk ? "external" : "internal");
+
+	val = artpec6_pcie_readl(artpec6_pcie, PCIECFG);
+	val |=  PCIECFG_RISRCREN |	/* Receiver term. 50 Ohm */
+		PCIECFG_PCLK_ENABLE;
+	if (extrefclk)
+		val |= PCIECFG_REFCLKSEL;
+	else
+		val &= ~PCIECFG_REFCLKSEL;
+	artpec6_pcie_writel(artpec6_pcie, PCIECFG, val);
+	usleep_range(10, 20);
+
+	val = artpec6_pcie_readl(artpec6_pcie, NOCCFG);
+	val |= NOCCFG_ENABLE_CLK_PCIE;
+	artpec6_pcie_writel(artpec6_pcie, NOCCFG, val);
+	usleep_range(20, 30);
+
+	val = artpec6_pcie_readl(artpec6_pcie, NOCCFG);
+	val &= ~NOCCFG_POWER_PCIE_IDLEREQ;
+	artpec6_pcie_writel(artpec6_pcie, NOCCFG, val);
+
+	retries = 50;
+	do {
+		usleep_range(1000, 2000);
+		val = artpec6_pcie_readl(artpec6_pcie, NOCCFG);
+		retries--;
+	} while (retries &&
+		(val & (NOCCFG_POWER_PCIE_IDLEACK | NOCCFG_POWER_PCIE_IDLE)));
+
+	retries = 50;
+	do {
+		usleep_range(1000, 2000);
+		phy_status_tx = readw(artpec6_pcie->phy_base + PHY_TX_ASIC_OUT);
+		phy_status_rx = readw(artpec6_pcie->phy_base + PHY_RX_ASIC_OUT);
+		retries--;
+	} while (retries && ((phy_status_tx & PHY_TX_ASIC_OUT_TX_ACK) ||
+				(phy_status_rx & PHY_RX_ASIC_OUT_ACK)));
+}
+
+static void artpec6_pcie_init_phy(struct artpec6_pcie *artpec6_pcie)
+{
+	switch (artpec6_pcie->variant) {
+	case ARTPEC6:
+		artpec6_pcie_init_phy_a6(artpec6_pcie);
+		break;
+	case ARTPEC7:
+		artpec6_pcie_init_phy_a7(artpec6_pcie);
+		break;
+	}
+}
+
+static void artpec6_pcie_set_nfts(struct artpec6_pcie *artpec6_pcie)
+{
+	struct dw_pcie *pci = artpec6_pcie->pci;
+	u32 val;
+
+	if (artpec6_pcie->variant != ARTPEC7)
+		return;
+
+	/*
+	 * Increase the N_FTS (Number of Fast Training Sequences)
+	 * to be transmitted when transitioning from L0s to L0.
+	 */
+	val = dw_pcie_readl_dbi(pci, ACK_F_ASPM_CTRL_OFF);
+	val &= ~ACK_N_FTS_MASK;
+	val |= ACK_N_FTS(180);
+	dw_pcie_writel_dbi(pci, ACK_F_ASPM_CTRL_OFF, val);
+
+	/*
+	 * Set the Number of Fast Training Sequences that the core
+	 * advertises as its N_FTS during Gen2 or Gen3 link training.
+	 */
+	val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
+	val &= ~FAST_TRAINING_SEQ_MASK;
+	val |= FAST_TRAINING_SEQ(180);
+	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
+}
+
 static void artpec6_pcie_assert_core_reset(struct artpec6_pcie *artpec6_pcie)
 {
 	u32 val;
 
 	val = artpec6_pcie_readl(artpec6_pcie, PCIECFG);
-	val |= PCIECFG_CORE_RESET_REQ;
+	switch (artpec6_pcie->variant) {
+	case ARTPEC6:
+		val |= PCIECFG_CORE_RESET_REQ;
+		break;
+	case ARTPEC7:
+		val &= ~PCIECFG_NOC_RESET;
+		break;
+	}
 	artpec6_pcie_writel(artpec6_pcie, PCIECFG, val);
 }
 
@@ -187,7 +311,14 @@ static void artpec6_pcie_deassert_core_reset(struct artpec6_pcie *artpec6_pcie)
 	u32 val;
 
 	val = artpec6_pcie_readl(artpec6_pcie, PCIECFG);
-	val &= ~PCIECFG_CORE_RESET_REQ;
+	switch (artpec6_pcie->variant) {
+	case ARTPEC6:
+		val &= ~PCIECFG_CORE_RESET_REQ;
+		break;
+	case ARTPEC7:
+		val |= PCIECFG_NOC_RESET;
+		break;
+	}
 	artpec6_pcie_writel(artpec6_pcie, PCIECFG, val);
 	usleep_range(100, 200);
 }
@@ -210,6 +341,7 @@ static int artpec6_pcie_host_init(struct pcie_port *pp)
 	artpec6_pcie_assert_core_reset(artpec6_pcie);
 	artpec6_pcie_init_phy(artpec6_pcie);
 	artpec6_pcie_deassert_core_reset(artpec6_pcie);
+	artpec6_pcie_set_nfts(artpec6_pcie);
 	dw_pcie_setup_rc(pp);
 	artpec6_pcie_establish_link(pci);
 	dw_pcie_wait_for_link(pci);
@@ -279,6 +411,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
 	artpec6_pcie_assert_core_reset(artpec6_pcie);
 	artpec6_pcie_init_phy(artpec6_pcie);
 	artpec6_pcie_deassert_core_reset(artpec6_pcie);
+	artpec6_pcie_set_nfts(artpec6_pcie);
 
 	for (bar = BAR_0; bar <= BAR_5; bar++)
 		dw_pcie_ep_reset_bar(pci, bar);
@@ -351,6 +484,7 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
 	int ret;
 	const struct of_device_id *match;
 	const struct artpec_pcie_of_data *data;
+	enum artpec_pcie_variants variant;
 	enum dw_pcie_device_mode mode;
 
 	match = of_match_device(artpec6_pcie_of_match, dev);
@@ -358,6 +492,7 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	data = (struct artpec_pcie_of_data *)match->data;
+	variant = (enum artpec_pcie_variants)data->variant;
 	mode = (enum dw_pcie_device_mode)data->mode;
 
 	artpec6_pcie = devm_kzalloc(dev, sizeof(*artpec6_pcie), GFP_KERNEL);
@@ -372,6 +507,7 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
 	pci->ops = &dw_pcie_ops;
 
 	artpec6_pcie->pci = pci;
+	artpec6_pcie->variant = variant;
 	artpec6_pcie->mode = mode;
 
 	dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
@@ -422,12 +558,24 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_PCIE_ARTPEC6_HOST
 static const struct artpec_pcie_of_data artpec6_pcie_rc_of_data = {
+	.variant = ARTPEC6,
+	.mode = DW_PCIE_RC_TYPE,
+};
+
+static const struct artpec_pcie_of_data artpec7_pcie_rc_of_data = {
+	.variant = ARTPEC7,
 	.mode = DW_PCIE_RC_TYPE,
 };
 #endif
 
 #ifdef CONFIG_PCIE_ARTPEC6_EP
 static const struct artpec_pcie_of_data artpec6_pcie_ep_of_data = {
+	.variant = ARTPEC6,
+	.mode = DW_PCIE_EP_TYPE,
+};
+
+static const struct artpec_pcie_of_data artpec7_pcie_ep_of_data = {
+	.variant = ARTPEC7,
 	.mode = DW_PCIE_EP_TYPE,
 };
 #endif
@@ -438,12 +586,20 @@ static const struct of_device_id artpec6_pcie_of_match[] = {
 		.compatible = "axis,artpec6-pcie",
 		.data = &artpec6_pcie_rc_of_data,
 	},
+	{
+		.compatible = "axis,artpec7-pcie",
+		.data = &artpec7_pcie_rc_of_data,
+	},
 #endif
 #ifdef CONFIG_PCIE_ARTPEC6_EP
 	{
 		.compatible = "axis,artpec6-pcie-ep",
 		.data = &artpec6_pcie_ep_of_data,
 	},
+	{
+		.compatible = "axis,artpec7-pcie-ep",
+		.data = &artpec7_pcie_ep_of_data,
+	},
 #endif
 	{},
 };
-- 
2.14.2