Add PCIe support to TI's J721E SoC

commit bd22885aa188f135fd9 ("PCI: cadence: Refactor driver to use as a core library") while refactoring the Cadence PCIe driver to be used as library, removed pm_runtime_get_sync() from cdns_pcie_ep_setup() and cdns_pcie_host_setup() but missed to remove the corresponding pm_runtime_put_sync() in the error path. Fix it here.

Fixes commit bd22885aa188f135fd9 ("PCI: cadence: Refactor driver to use

as a core library")

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence-ep.c   | 2 --
 drivers/pci/controller/cadence/pcie-cadence-host.c | 2 --
 2 files changed, 4 deletions(-)

drivers/pci/controller/cadence/pcie-cadence-ep.c +0 -2

index 1c173dad67d1..560f22b4d165 100644
@@ -473,7 +473,5 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
 	pci_epc_mem_exit(epc);
 
  err_init:
-	pm_runtime_put_sync(dev);
-
 	return ret;
 }

drivers/pci/controller/cadence/pcie-cadence-host.c +0 -0

index 9b1c3966414b..ccf55e143e1d 100644

drivers/pci/controller/cadence/pcie-cadence-host.c +0 -0

 	return ret;
 }

Signature · 1 line

2.17.1

Quoted 6 lines · Kishon Vijay Abraham I

commit bd22885aa188f135fd9 ("PCI: cadence: Refactor driver to use as a core library") while refactoring the Cadence PCIe driver to be used as library, removed pm_runtime_get_sync() from cdns_pcie_ep_setup() and cdns_pcie_host_setup() but missed to remove the corresponding pm_runtime_put_sync() in the error path. Fix it here.

Quoted 1 line

Fixes commit bd22885aa188f135fd9 ("PCI: cadence: Refactor driver to use

As this is a fix, a commit subject starting with PCI: cadence: Fix ... may be more obvious.

I'd suggest you use the shorter form of this, i.e. Fixes: %h (\"%s\"))

Fixes bd22885aa188 ("PCI: cadence: Refactor driver to use as a core library")

Quoted 1 line

as a core library")

Quoted 26 lines

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
  drivers/pci/controller/cadence/pcie-cadence-ep.c   | 2 --
  drivers/pci/controller/cadence/pcie-cadence-host.c | 2 --
  2 files changed, 4 deletions(-)

drivers/pci/controller/cadence/pcie-cadence-ep.c +0 -2

index 1c173dad67d1..560f22b4d165 100644
@@ -473,7 +473,5 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
  	pci_epc_mem_exit(epc);
 
   err_init:
-	pm_runtime_put_sync(dev);
-
  	return ret;
  }

drivers/pci/controller/cadence/pcie-cadence-host.c +0 -0

index 9b1c3966414b..ccf55e143e1d 100644
@@ -275,7 +275,5 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
  	pci_free_resource_list(&resources);
 

drivers/pci/controller/cadence/pcie-cadence-host.c int cdns_pcie_host_setup(struct cdns_pcie_rc *rc) · line 279

Quoted 3 lines

drivers/pci/controller/cadence/pcie-cadence-host.c +0 -2

   err_init:
-	pm_runtime_put_sync(dev);
-

There is probably more you can do here for both -host and -ep:

- Remove the possibly now unused <linux/pm_runtime.h> include
- Remove the err_init label and return directly from source.

Add support to use custom read and write accessors. Platforms that doesn't support half word or byte access or any other constraint

while accessing registers can use this feature to populate custom read and write accessors. These custom accessors are used for both standard register access and configuration space register access.

This is in preparation for adding PCIe support in TI's J721E SoC which uses Cadence PCIe core.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence.h | 99 +++++++++++++++++--
 1 file changed, 90 insertions(+), 9 deletions(-)

drivers/pci/controller/cadence/pcie-cadence.h +5 -0

index a2b28b912ca4..d0d91c69fa1d 100644
@@ -223,6 +223,11 @@ enum cdns_pcie_msg_routing {
 	MSG_ROUTING_GATHER,
 };
 
+struct cdns_pcie_ops {
+	u32	(*read)(void __iomem *addr, int size);
+	void	(*write)(void __iomem *addr, int size, u32 value);
+};
+
 /**
  * struct cdns_pcie - private data for Cadence PCIe controller drivers
  * @reg_base: IO mapped register base

drivers/pci/controller/cadence/pcie-cadence.h +1 -0

+	const struct cdns_pcie_ops *ops;
 };
 
 /**

@@ -301,21 +306,47 @@ struct cdns_pcie_ep {
 /* Register access */

drivers/pci/controller/cadence/pcie-cadence.h +77 -8

 }
 
 static inline void cdns_pcie_writew(struct cdns_pcie *pcie, u32 reg, u16 value)
 {
+	void __iomem *addr = pcie->reg_base + reg;
+
+	if (pcie->ops && pcie->ops->write) {
+		pcie->ops->write(addr, 0x2, value);
+		return;
+	}
+
 	writew(value, pcie->reg_base + reg);
 }
 
 static inline void cdns_pcie_writel(struct cdns_pcie *pcie, u32 reg, u32 value)
 {
+	void __iomem *addr = pcie->reg_base + reg;
+
+	if (pcie->ops && pcie->ops->write) {
+		pcie->ops->write(addr, 0x4, value);
+		return;
+	}
+
 	writel(value, pcie->reg_base + reg);
 }
 
 static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg)
 {
+	void __iomem *addr = pcie->reg_base + reg;
+
+	if (pcie->ops && pcie->ops->read)
+		return pcie->ops->read(addr, 0x4);
+
 	return readl(pcie->reg_base + reg);
 }
 

@@ -323,47 +354,97 @@ static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg)
 static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie,
 				       u32 reg, u8 value)
 {
-	writeb(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
+	void __iomem *addr = pcie->reg_base + CDNS_PCIE_RP_BASE + reg;
+
+	if (pcie->ops && pcie->ops->write) {
+		pcie->ops->write(addr, 0x1, value);
+		return;
+	}
+
+	writeb(value, addr);
 }
 
 static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
 				       u32 reg, u16 value)
 {
-	writew(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
+	void __iomem *addr = pcie->reg_base + CDNS_PCIE_RP_BASE + reg;
+
+	if (pcie->ops && pcie->ops->write) {
+		pcie->ops->write(addr, 0x2, value);
+		return;
+	}
+
+	writew(value, addr);
 }
 
 /* Endpoint Function register access */
 static inline void cdns_pcie_ep_fn_writeb(struct cdns_pcie *pcie, u8 fn,
 					  u32 reg, u8 value)
 {
-	writeb(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+	void __iomem *addr = pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg;
+
+	if (pcie->ops && pcie->ops->write) {
+		pcie->ops->write(addr, 0x1, value);
+		return;
+	}
+
+	writeb(value, addr);
 }
 
 static inline void cdns_pcie_ep_fn_writew(struct cdns_pcie *pcie, u8 fn,
 					  u32 reg, u16 value)
 {
-	writew(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+	void __iomem *addr = pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg;
+
+	if (pcie->ops && pcie->ops->write) {
+		pcie->ops->write(addr, 0x2, value);
+		return;
+	}
+
+	writew(value, addr);
 }
 
 static inline void cdns_pcie_ep_fn_writel(struct cdns_pcie *pcie, u8 fn,
 					  u32 reg, u32 value)
 {
-	writel(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+	void __iomem *addr = pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg;
+
+	if (pcie->ops && pcie->ops->write) {
+		pcie->ops->write(addr, 0x4, value);
+		return;
+	}
+
+	writel(value, addr);
 }
 
 static inline u8 cdns_pcie_ep_fn_readb(struct cdns_pcie *pcie, u8 fn, u32 reg)
 {
-	return readb(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+	void __iomem *addr = pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg;
+
+	if (pcie->ops && pcie->ops->read)
+		return pcie->ops->read(addr, 0x1);
+
+	return readb(addr);
 }
 
 static inline u16 cdns_pcie_ep_fn_readw(struct cdns_pcie *pcie, u8 fn, u32 reg)
 {
-	return readw(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+	void __iomem *addr = pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg;
+
+	if (pcie->ops && pcie->ops->read)
+		return pcie->ops->read(addr, 0x2);
+
+	return readw(addr);
 }
 
 static inline u32 cdns_pcie_ep_fn_readl(struct cdns_pcie *pcie, u8 fn, u32 reg)
 {
-	return readl(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
+	void __iomem *addr = pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg;
+
+	if (pcie->ops && pcie->ops->read)
+		return pcie->ops->read(addr, 0x4);
+
+	return readl(addr);
 }
 
 #ifdef CONFIG_PCIE_CADENCE_HOST

Signature · 1 line

2.17.1

Quoted 2 lines · Kishon Vijay Abraham I

Add support to use custom read and write accessors. Platforms that doesn't support half word or byte access or any other constraint

s/doesn't/don't/

Quoted 3 lines

while accessing registers can use this feature to populate custom read and write accessors. These custom accessors are used for both standard register access and configuration space register access.

You can put the following sentence underneath a --- as it's not needed in the commit message (but may be helpful to reviewers).

Quoted 16 lines

This is in preparation for adding PCIe support in TI's J721E SoC which uses Cadence PCIe core.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence.h | 99 +++++++++++++++++--
 1 file changed, 90 insertions(+), 9 deletions(-)

drivers/pci/controller/cadence/pcie-cadence.h +0 -0

index a2b28b912ca4..d0d91c69fa1d 100644
@@ -223,6 +223,11 @@ enum cdns_pcie_msg_routing {
 	MSG_ROUTING_GATHER,
 };
 

drivers/pci/controller/cadence/pcie-cadence.h struct cdns_pcie { · line 242

Quoted 13 lines

drivers/pci/controller/cadence/pcie-cadence.h +5 -1

+struct cdns_pcie_ops {
+	u32	(*read)(void __iomem *addr, int size);
+	void	(*write)(void __iomem *addr, int size, u32 value);
+};
+
 /**
  * struct cdns_pcie - private data for Cadence PCIe controller drivers
  * @reg_base: IO mapped register base

@@ -239,7 +244,7 @@ struct cdns_pcie {
 	int			phy_count;
 	struct phy		**phy;
 	struct device_link	**link;
-	const struct cdns_pcie_common_ops *ops;

What was cdns_pcie_common_ops? It's not defined in the current tree is it?

Quoted 4 lines

drivers/pci/controller/cadence/pcie-cadence.h +1 -0

+	const struct cdns_pcie_ops *ops;
 };
 

drivers/pci/controller/cadence/pcie-cadence.h struct cdns_pcie_ep { · line 316

Quoted 13 lines

drivers/pci/controller/cadence/pcie-cadence.h +7 -0

 /**

@@ -301,21 +306,47 @@ struct cdns_pcie_ep {
 /* Register access */
 static inline void cdns_pcie_writeb(struct cdns_pcie *pcie, u32 reg, u8 value)
 {
+	void __iomem *addr = pcie->reg_base + reg;
+
+	if (pcie->ops && pcie->ops->write) {
+		pcie->ops->write(addr, 0x1, value);
+		return;
+	}
+
 	writeb(value, pcie->reg_base + reg);

Can you use 'addr' here instead of 'pcie->reg_base + reg'? (And similar for the rest of them).

Add cdns_pcie_ops to start link and verify link status. The registers to start link and to check link status is in Platform specific PCIe wrapper. Add support for platform specific drivers to add callback functions for the PCIe Cadence core to start link and verify link status.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 .../pci/controller/cadence/pcie-cadence-ep.c  |  8 ++++++
 .../controller/cadence/pcie-cadence-host.c    | 28 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h | 23 +++++++++++++++
 3 files changed, 59 insertions(+)

drivers/pci/controller/cadence/pcie-cadence-ep.c +8 -0

index 560f22b4d165..088394b6be04 100644
@@ -355,8 +355,10 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
 	struct cdns_pcie *pcie = &ep->pcie;
+	struct device *dev = pcie->dev;
 	struct pci_epf *epf;
 	u32 cfg;
+	int ret;
 
 	/*
 	 * BIT(0) is hardwired to 1, hence function 0 is always enabled

@@ -367,6 +369,12 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
 		cfg |= BIT(epf->func_no);
 	cdns_pcie_writel(pcie, CDNS_PCIE_LM_EP_FUNC_CFG, cfg);
 
+	ret = cdns_pcie_start_link(pcie, true);
+	if (ret) {
+		dev_err(dev, "Failed to start link\n");
+		return ret;
+	}
+
 	return 0;
 }
 

drivers/pci/controller/cadence/pcie-cadence-host.c +7 -0

index ccf55e143e1d..0929554f5a81 100644
@@ -3,6 +3,7 @@
 // Cadence PCIe host controller driver.
 // Author: Cyrille Pitchen <[email protected]>
 
+#include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>

@@ -201,6 +202,23 @@ static int cdns_pcie_host_init(struct device *dev,
 	return err;
 }
 
+static int cdns_pcie_host_wait_for_link(struct cdns_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	int retries;
+
+	/* Check if the link is up or not */

drivers/pci/controller/cadence/pcie-cadence-host.c +11 -0

+
 int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 {
 	struct device *dev = rc->pcie.dev;

@@ -254,6 +272,16 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 
 	pcie->mem_res = res;
 
+	ret = cdns_pcie_start_link(pcie, true);
+	if (ret) {
+		dev_err(dev, "Failed to start link\n");
+		return ret;
+	}
+
+	ret = cdns_pcie_host_wait_for_link(pcie);
+	if (ret)
+		dev_dbg(dev, "PCIe link never came up\n");
+
 	ret = cdns_pcie_host_init(dev, &resources, rc);
 	if (ret)
 		goto err_init;

drivers/pci/controller/cadence/pcie-cadence.h +23 -0

index d0d91c69fa1d..f0395eaf9df5 100644
@@ -10,6 +10,11 @@
 #include <linux/pci.h>
 #include <linux/phy/phy.h>
 
+/* Parameters for the waiting for link up routine */
+#define LINK_WAIT_MAX_RETRIES	10
+#define LINK_WAIT_USLEEP_MIN	90000
+#define LINK_WAIT_USLEEP_MAX	100000
+
 /*
  * Local Management Registers
  */

@@ -226,6 +231,8 @@ enum cdns_pcie_msg_routing {
 struct cdns_pcie_ops {
 	u32	(*read)(void __iomem *addr, int size);
 	void	(*write)(void __iomem *addr, int size, u32 value);
+	int	(*start_link)(struct cdns_pcie *pcie, bool start);
+	bool	(*is_link_up)(struct cdns_pcie *pcie);
 };
 
 /**

@@ -447,6 +454,22 @@ static inline u32 cdns_pcie_ep_fn_readl(struct cdns_pcie *pcie, u8 fn, u32 reg)
 	return readl(addr);
 }
 
+static inline int cdns_pcie_start_link(struct cdns_pcie *pcie, bool start)
+{
+	if (pcie->ops->start_link)
+		return pcie->ops->start_link(pcie, start);
+
+	return 0;
+}
+
+static inline bool cdns_pcie_is_link_up(struct cdns_pcie *pcie)
+{
+	if (pcie->ops->is_link_up)
+		return pcie->ops->is_link_up(pcie);
+
+	return true;
+}
+
 #ifdef CONFIG_PCIE_CADENCE_HOST
 int cdns_pcie_host_setup(struct cdns_pcie_rc *rc);
 #else

Signature · 1 line

2.17.1

Quoted 62 lines · Kishon Vijay Abraham I

Add cdns_pcie_ops to start link and verify link status. The registers to start link and to check link status is in Platform specific PCIe wrapper. Add support for platform specific drivers to add callback functions for the PCIe Cadence core to start link and verify link status.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 .../pci/controller/cadence/pcie-cadence-ep.c  |  8 ++++++
 .../controller/cadence/pcie-cadence-host.c    | 28 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h | 23 +++++++++++++++
 3 files changed, 59 insertions(+)

drivers/pci/controller/cadence/pcie-cadence-ep.c +8 -0

index 560f22b4d165..088394b6be04 100644
@@ -355,8 +355,10 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
 	struct cdns_pcie *pcie = &ep->pcie;
+	struct device *dev = pcie->dev;
 	struct pci_epf *epf;
 	u32 cfg;
+	int ret;
 
 	/*
 	 * BIT(0) is hardwired to 1, hence function 0 is always enabled

@@ -367,6 +369,12 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
 		cfg |= BIT(epf->func_no);
 	cdns_pcie_writel(pcie, CDNS_PCIE_LM_EP_FUNC_CFG, cfg);
 
+	ret = cdns_pcie_start_link(pcie, true);
+	if (ret) {
+		dev_err(dev, "Failed to start link\n");
+		return ret;
+	}
+
 	return 0;
 }
 

drivers/pci/controller/cadence/pcie-cadence-host.c +7 -0

index ccf55e143e1d..0929554f5a81 100644
@@ -3,6 +3,7 @@
 // Cadence PCIe host controller driver.
 // Author: Cyrille Pitchen <[email protected]>
 
+#include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>

@@ -201,6 +202,23 @@ static int cdns_pcie_host_init(struct device *dev,
 	return err;
 }
 
+static int cdns_pcie_host_wait_for_link(struct cdns_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	int retries;
+
+	/* Check if the link is up or not */

drivers/pci/controller/cadence/pcie-cadence-host.c static int cdns_pcie_host_init(struct device *dev, · line 220

Quoted 10 lines

drivers/pci/controller/cadence/pcie-cadence-host.c +10 -0

+	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+		if (cdns_pcie_is_link_up(pcie)) {
+			dev_info(dev, "Link up\n");
+			return 0;
+		}
+		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+	}
+
+	return -ETIMEDOUT;
+}

This patch looks fine, except this function (above) is identical to dw_pcie_wait_for_link, advk_pcie_wait_for_link and nwl_wait_for_link. Even the definitions of LINK_WAIT_USLEEP_xx are the same.

I don't see any justification to duplicating this again - can you consolidate these functions to something that all controller drivers can use?

Certain platforms like TI's J721E allow only 32-bit register accesses. Add read and write accessors to perform only 32-bit accesses in order to support platfroms like TI's J721E.

Certain platforms like TI's J721E allow only 32-bit register accesses.

Add read and write accessors to perform only 32-bit accesses in order to support platfroms like TI's J721E.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence.c | 40 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h |  2 +
 2 files changed, 42 insertions(+)

drivers/pci/controller/cadence/pcie-cadence.c +0 -0

index cd795f6fc1e2..de5b3b06f2d0 100644

drivers/pci/controller/cadence/pcie-cadence.c +21 -0

+	void __iomem *aligned_addr = PTR_ALIGN_DOWN(addr, 0x4);
+	unsigned int offset = (unsigned long)addr & 0x3;
+	u32 mask;
+	u32 val;
+
+	if (!IS_ALIGNED((uintptr_t)addr, size)) {
+		pr_err("Invalid Address in function:%s\n", __func__);
+		return;
+	}
+
+	if (size > 2) {
+		writel(value, addr);
+		return;
+	}
+
+	mask = ~(((1 << (size * 8)) - 1) << (offset * 8));
+	val = readl(aligned_addr) & mask;
+	val |= value << (offset * 8);
+	writel(val, aligned_addr);
+}
+
 void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 fn,
 				   u32 r, bool is_io,
 				   u64 cpu_addr, u64 pci_addr, size_t size)

drivers/pci/controller/cadence/pcie-cadence.h +2 -0

index f0395eaf9df5..5171d0da37da 100644
@@ -498,6 +498,8 @@ void cdns_pcie_reset_outbound_region(struct cdns_pcie *pcie, u32 r);
 void cdns_pcie_disable_phy(struct cdns_pcie *pcie);
 int cdns_pcie_enable_phy(struct cdns_pcie *pcie);
 int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie);
+u32 cdns_pcie_read32(void __iomem *addr, int size);
+void cdns_pcie_write32(void __iomem *addr, int size, u32 value);
 extern const struct dev_pm_ops cdns_pcie_pm_ops;
 
 #endif /* _PCIE_CADENCE_H */

Signature · 1 line

2.17.1

Quoted 1 line · Kishon Vijay Abraham I

Certain platforms like TI's J721E allow only 32-bit register accesses.

When I first read this I thought you meant only 32-bit accesses are allowed and not other sizes (such as 64-bit). However the limitation you address here is that the J721E allows only 32-bit *aligned* register accesses.

It would be helpful to make this clearer in the commit message.

You can also shorten the commit subject to 'PCI: cadence: Add read/write accessors for 32-bit aligned accesses' or similar.

Quoted 14 lines

Add read and write accessors to perform only 32-bit accesses in order to support platfroms like TI's J721E.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence.c | 40 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h |  2 +
 2 files changed, 42 insertions(+)

drivers/pci/controller/cadence/pcie-cadence.c +0 -0

index cd795f6fc1e2..de5b3b06f2d0 100644

drivers/pci/controller/cadence/pcie-cadence.c line 10

Quoted 4 lines

drivers/pci/controller/cadence/pcie-cadence.c +1 -0

@@ -7,6 +7,46 @@
 
 #include "pcie-cadence.h"
 
+u32 cdns_pcie_read32(void __iomem *addr, int size)

Given there is already a cdns_pcie_readl in pcie-cadence.h it may help to name this in a way that doesn't cause confusion. Here 32 is perhaps being used to suggest the size of the actual read performed, the maximum size of 'size' or the alignment.

drivers/pci/controller/cadence/pcie-cadence.c line 17

Quoted 7 lines

drivers/pci/controller/cadence/pcie-cadence.c +7 -0

+{
+	void __iomem *aligned_addr = PTR_ALIGN_DOWN(addr, 0x4);
+	unsigned int offset = (unsigned long)addr & 0x3;
+	u32 val = readl(aligned_addr);
+
+	if (!IS_ALIGNED((uintptr_t)addr, size)) {
+		pr_err("Invalid Address in function:%s\n", __func__);

Would this be better as a BUG? Without a BUG this error could get ignored and yet the device may not behave as expected.

drivers/pci/controller/cadence/pcie-cadence.c line 22

Quoted 7 lines

drivers/pci/controller/cadence/pcie-cadence.c +5 -0

+		return 0;
+	}
+
+	if (size > 2)
+		return val;

I think you make the assumption here that if size > 2 then it's 4. It could be 3 (though unlikely) in which case you'd want to fall through to the next line.

Hi Andrew,

Quoted 1 line · Andrew Murray

On Mon, Dec 09, 2019 at 02:51:39PM +0530, Kishon Vijay Abraham I wrote:

Quoted 5 lines

Quoted 1 line

Certain platforms like TI's J721E allow only 32-bit register accesses.

When I first read this I thought you meant only 32-bit accesses are allowed and not other sizes (such as 64-bit). However the limitation you address here is that the J721E allows only 32-bit *aligned* register accesses.

It's both, it allows only 32-bit aligned accesses and the size should be only 32 bits. That's why I always use "readl" in the APIs below.

Quoted 11 lines · Kishon Vijay Abraham I

Hi Andrew,

Quoted 6 lines · Andrew Murray

Quoted 1 line · Kishon Vijay Abraham I

Certain platforms like TI's J721E allow only 32-bit register accesses.

When I first read this I thought you meant only 32-bit accesses are allowed and not other sizes (such as 64-bit). However the limitation you address here is that the J721E allows only 32-bit *aligned* register accesses.

Quoted 2 lines

It's both, it allows only 32-bit aligned accesses and the size should be only 32 bits. That's why I always use "readl" in the APIs below.

In that case, can't you use the pci_generic_config_read32/write32 functions with a cadence specific .map_bus() function?

Quoted 12 lines · Arnd Bergmann

Quoted 11 lines · Kishon Vijay Abraham I

Hi Andrew,

Quoted 6 lines · Andrew Murray

Quoted 1 line · Kishon Vijay Abraham I

Certain platforms like TI's J721E allow only 32-bit register accesses.

When I first read this I thought you meant only 32-bit accesses are allowed and not other sizes (such as 64-bit). However the limitation you address here is that the J721E allows only 32-bit *aligned* register accesses.

Quoted 5 lines

Quoted 2 lines

It's both, it allows only 32-bit aligned accesses and the size should be only 32 bits. That's why I always use "readl" in the APIs below.

In that case, can't you use the pci_generic_config_read32/write32 functions with a cadence specific .map_bus() function?

pci_generic_config_read32() is for reading configuration space registers only. The accessors I added here are for the controller IP configuration.

For the configuration space access I use pci_generic_config_read32/write32()([PATCH 11/13] PCI: j721e: Add TI J721E PCIe driver).

Certain platforms like TI's J721E allow only 32-bit register accesses. Add read and write accessors to perform only 32-bit accesses in order to support platfroms like TI's J721E.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence.c | 40 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h |  2 +
 2 files changed, 42 insertions(+)

drivers/pci/controller/cadence/pcie-cadence.c +0 -0

index cd795f6fc1e2..de5b3b06f2d0 100644
@@ -7,6 +7,46 @@
 

Quoted 3 lines · Kishon Vijay Abraham I

Certain platforms like TI's J721E allow only 32-bit register accesses. Add read and write accessors to perform only 32-bit accesses in order to support platfroms like TI's J721E.

s/platfroms/platforms/

Quoted 14 lines

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence.c | 40 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h |  2 +
 2 files changed, 42 insertions(+)

drivers/pci/controller/cadence/pcie-cadence.c +0 -0

index cd795f6fc1e2..de5b3b06f2d0 100644
@@ -7,6 +7,46 @@
 
 #include "pcie-cadence.h"
 

drivers/pci/controller/cadence/pcie-cadence.c line 17

Quoted 8 lines

drivers/pci/controller/cadence/pcie-cadence.c +8 -0

+u32 cdns_pcie_read32(void __iomem *addr, int size)
+{
+	void __iomem *aligned_addr = PTR_ALIGN_DOWN(addr, 0x4);
+	unsigned int offset = (unsigned long)addr & 0x3;
+	u32 val = readl(aligned_addr);
+
+	if (!IS_ALIGNED((uintptr_t)addr, size)) {
+		pr_err("Invalid Address in function:%s\n", __func__);

It might be nice to have a hint about *why* it's invalid, e.g., the addr and size values.

Certain platforms like TI's J721E allows only 32-bit configuration space access. In such cases pci_generic_config_read and pci_generic_config_write cannot be used. Add support in Cadence core to let pci_host_bridge have custom pci_ops.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence-host.c | 7 ++++---
 drivers/pci/controller/cadence/pcie-cadence.h      | 8 ++++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

drivers/pci/controller/cadence/pcie-cadence-host.c +4 -3

index 0929554f5a81..2efc33b1cade 100644
@@ -12,8 +12,8 @@
 
 #include "pcie-cadence.h"
 
-static void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
-				      int where)
+void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
+			       int where)
 {
 	struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
 	struct cdns_pcie_rc *rc = pci_host_bridge_priv(bridge);

@@ -289,7 +289,8 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 	list_splice_init(&resources, &bridge->windows);
 	bridge->dev.parent = dev;
 	bridge->busnr = pcie->bus;
-	bridge->ops = &cdns_pcie_host_ops;
+	if (!bridge->ops)
+		bridge->ops = &cdns_pcie_host_ops;
 	bridge->map_irq = of_irq_parse_and_map_pci;
 	bridge->swizzle_irq = pci_common_swizzle;
 

drivers/pci/controller/cadence/pcie-cadence.h +0 -0

index 5171d0da37da..c879dd3d2893 100644

drivers/pci/controller/cadence/pcie-cadence.h +1 -0

+}
 #endif
 
 #ifdef CONFIG_PCIE_CADENCE_EP

Signature · 1 line

2.17.1

Quoted 42 lines · Kishon Vijay Abraham I

Certain platforms like TI's J721E allows only 32-bit configuration space access. In such cases pci_generic_config_read and pci_generic_config_write cannot be used. Add support in Cadence core to let pci_host_bridge have custom pci_ops.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence-host.c | 7 ++++---
 drivers/pci/controller/cadence/pcie-cadence.h      | 8 ++++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

drivers/pci/controller/cadence/pcie-cadence-host.c +4 -3

index 0929554f5a81..2efc33b1cade 100644
@@ -12,8 +12,8 @@
 
 #include "pcie-cadence.h"
 
-static void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
-				      int where)
+void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
+			       int where)
 {
 	struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
 	struct cdns_pcie_rc *rc = pci_host_bridge_priv(bridge);

@@ -289,7 +289,8 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 	list_splice_init(&resources, &bridge->windows);
 	bridge->dev.parent = dev;
 	bridge->busnr = pcie->bus;
-	bridge->ops = &cdns_pcie_host_ops;
+	if (!bridge->ops)
+		bridge->ops = &cdns_pcie_host_ops;
 	bridge->map_irq = of_irq_parse_and_map_pci;
 	bridge->swizzle_irq = pci_common_swizzle;
 

drivers/pci/controller/cadence/pcie-cadence.h +0 -0

index 5171d0da37da..c879dd3d2893 100644
@@ -472,11 +472,19 @@ static inline bool cdns_pcie_is_link_up(struct cdns_pcie *pcie)
 

drivers/pci/controller/cadence/pcie-cadence.h static inline bool cdns_pcie_is_link_up(struct cdns_pcie *pcie) · line 476

Quoted 4 lines

drivers/pci/controller/cadence/pcie-cadence.h +2 -0

 #ifdef CONFIG_PCIE_CADENCE_HOST
 int cdns_pcie_host_setup(struct cdns_pcie_rc *rc);
+void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
+			       int where);

The commit message doesn't explain why this change in visibility is needed).

drivers/pci/controller/cadence/pcie-cadence.h static inline bool cdns_pcie_is_link_up(struct cdns_pcie *pcie) · line 486

Quoted 11 lines

drivers/pci/controller/cadence/pcie-cadence.h +5 -0

 #else
 static inline int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 {
 	return 0;
 }
+
+static inline void __iomem *cdns_pci_map_bus(struct pci_bus *bus,
+					     unsigned int devfn,
+					     int where)
+{

This still needs to return something right?

Cadence driver uses "mem" memory resource to obtain the offset of configuration space address region, memory space address region and message space address region. The obtained offset is used to program the Address Translation Unit (ATU). However certain platforms like TI's J721E SoC require the absolute address to be programmed in the ATU and not just the offset.

The same problem was solved in designware driver using a platform specific ops for CPU addr fixup in commit a660083eb06c5bb0 ("PCI: dwc: designware:

Add new *ops* for CPU addr fixup"). Follow a similar mechanism in Cadence too instead of directly using "mem" memory resource in Cadence PCIe core.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 .../pci/controller/cadence/pcie-cadence-host.c    | 15 ++++-----------
 drivers/pci/controller/cadence/pcie-cadence.c     |  8 ++++++--
 drivers/pci/controller/cadence/pcie-cadence.h     |  1 +
 3 files changed, 11 insertions(+), 13 deletions(-)

drivers/pci/controller/cadence/pcie-cadence-host.c +1 -2

index 2efc33b1cade..cf817be237af 100644
@@ -105,15 +105,14 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
 static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
 {
 	struct cdns_pcie *pcie = &rc->pcie;
-	struct resource *mem_res = pcie->mem_res;
 	struct resource *bus_range = rc->bus_range;
 	struct resource *cfg_res = rc->cfg_res;
 	struct device *dev = pcie->dev;
 	struct device_node *np = dev->of_node;
 	struct of_pci_range_parser parser;
+	u64 cpu_addr = cfg_res->start;
 	struct of_pci_range range;
 	u32 addr0, addr1, desc1;
-	u64 cpu_addr;
 	int r, err;
 
 	/*

drivers/pci/controller/cadence/pcie-cadence-host.c +0 -8

 	addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(12) |
 		(lower_32_bits(cpu_addr) & GENMASK(31, 8));
 	addr1 = upper_32_bits(cpu_addr);

@@ -264,14 +265,6 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 	}
 	rc->cfg_res = res;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mem");
-	if (!res) {
-		dev_err(dev, "missing \"mem\"\n");
-		return -EINVAL;
-	}
-
-	pcie->mem_res = res;
-
 	ret = cdns_pcie_start_link(pcie, true);
 	if (ret) {
 		dev_err(dev, "Failed to start link\n");

drivers/pci/controller/cadence/pcie-cadence.c +6 -2

index de5b3b06f2d0..bd93d0f92f55 100644
@@ -113,7 +113,9 @@ void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 fn,
 	cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(r), desc1);
 
 	/* Set the CPU address */
-	cpu_addr -= pcie->mem_res->start;
+	if (pcie->ops->cpu_addr_fixup)
+		cpu_addr = pcie->ops->cpu_addr_fixup(pcie, cpu_addr);
+
 	addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(nbits) |
 		(lower_32_bits(cpu_addr) & GENMASK(31, 8));
 	addr1 = upper_32_bits(cpu_addr);

@@ -140,7 +142,9 @@ void cdns_pcie_set_outbound_region_for_normal_msg(struct cdns_pcie *pcie, u8 fn,
 	}
 
 	/* Set the CPU address */
-	cpu_addr -= pcie->mem_res->start;
+	if (pcie->ops->cpu_addr_fixup)
+		cpu_addr = pcie->ops->cpu_addr_fixup(pcie, cpu_addr);
+
 	addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(17) |
 		(lower_32_bits(cpu_addr) & GENMASK(31, 8));
 	addr1 = upper_32_bits(cpu_addr);

drivers/pci/controller/cadence/pcie-cadence.h +1 -0

index c879dd3d2893..ffa8b9f78ff8 100644
@@ -233,6 +233,7 @@ struct cdns_pcie_ops {
 	void	(*write)(void __iomem *addr, int size, u32 value);
 	int	(*start_link)(struct cdns_pcie *pcie, bool start);
 	bool	(*is_link_up)(struct cdns_pcie *pcie);
+	u64     (*cpu_addr_fixup)(struct cdns_pcie *pcie, u64 cpu_addr);
 };
 
 /**

Signature · 1 line

2.17.1

Quoted 9 lines · Kishon Vijay Abraham I

Cadence driver uses "mem" memory resource to obtain the offset of configuration space address region, memory space address region and message space address region. The obtained offset is used to program the Address Translation Unit (ATU). However certain platforms like TI's J721E SoC require the absolute address to be programmed in the ATU and not just the offset.

The same problem was solved in designware driver using a platform specific ops for CPU addr fixup in commit a660083eb06c5bb0 ("PCI: dwc: designware:

Thanks for this reference, though this doesn't need to be in the commit log, please put such comments underneath a ---.

Quoted 36 lines

Add new *ops* for CPU addr fixup"). Follow a similar mechanism in Cadence too instead of directly using "mem" memory resource in Cadence PCIe core.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 .../pci/controller/cadence/pcie-cadence-host.c    | 15 ++++-----------
 drivers/pci/controller/cadence/pcie-cadence.c     |  8 ++++++--
 drivers/pci/controller/cadence/pcie-cadence.h     |  1 +
 3 files changed, 11 insertions(+), 13 deletions(-)

drivers/pci/controller/cadence/pcie-cadence-host.c +1 -2

index 2efc33b1cade..cf817be237af 100644
@@ -105,15 +105,14 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
 static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
 {
 	struct cdns_pcie *pcie = &rc->pcie;
-	struct resource *mem_res = pcie->mem_res;
 	struct resource *bus_range = rc->bus_range;
 	struct resource *cfg_res = rc->cfg_res;
 	struct device *dev = pcie->dev;
 	struct device_node *np = dev->of_node;
 	struct of_pci_range_parser parser;
+	u64 cpu_addr = cfg_res->start;
 	struct of_pci_range range;
 	u32 addr0, addr1, desc1;
-	u64 cpu_addr;
 	int r, err;
 
 	/*

@@ -126,7 +125,9 @@ static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
 	cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(0), addr1);
 	cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(0), desc1);
 

drivers/pci/controller/cadence/pcie-cadence-host.c static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc) · line 130

Quoted 4 lines

drivers/pci/controller/cadence/pcie-cadence-host.c +3 -1

-	cpu_addr = cfg_res->start - mem_res->start;
+	if (pcie->ops->cpu_addr_fixup)
+		cpu_addr = pcie->ops->cpu_addr_fixup(pcie, cpu_addr);
+

Won't this patch cause a breakage for existing users that won't have defined a cpu_addr_fixup? The offset isn't being calculated and so cpu_addr will be wrong?

PCI_VENDOR_ID in root port configuration space is read-only register and writing to it will have no effect. Use local management register to configure Vendor ID and Subsystem Vendor ID.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence-host.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

drivers/pci/controller/cadence/pcie-cadence-host.c +7 -2

index cf817be237af..afb2c96a6538 100644
@@ -71,6 +71,7 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
 {
 	struct cdns_pcie *pcie = &rc->pcie;
 	u32 value, ctrl;
+	u32 id;
 
 	/*
 	 * Set the root complex BAR configuration register:

@@ -90,8 +91,12 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
 	cdns_pcie_writel(pcie, CDNS_PCIE_LM_RC_BAR_CFG, value);
 
 	/* Set root port configuration space */
-	if (rc->vendor_id != 0xffff)
-		cdns_pcie_rp_writew(pcie, PCI_VENDOR_ID, rc->vendor_id);
+	if (rc->vendor_id != 0xffff) {
+		id = CDNS_PCIE_LM_ID_VENDOR(rc->vendor_id) |
+			CDNS_PCIE_LM_ID_SUBSYS(rc->vendor_id);
+		cdns_pcie_writel(pcie, CDNS_PCIE_LM_ID, id);
+	}
+
 	if (rc->device_id != 0xffff)
 		cdns_pcie_rp_writew(pcie, PCI_DEVICE_ID, rc->device_id);
 

Signature · 1 line

2.17.1

Quoted 3 lines · Kishon Vijay Abraham I

PCI_VENDOR_ID in root port configuration space is read-only register and writing to it will have no effect. Use local management register to configure Vendor ID and Subsystem Vendor ID.

Is this a bug fix? Can you add a Fixes tag and make that clearer?

Thanks,

Andrew Murray

Quoted 36 lines

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/pcie-cadence-host.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

drivers/pci/controller/cadence/pcie-cadence-host.c +7 -2

index cf817be237af..afb2c96a6538 100644
@@ -71,6 +71,7 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
 {
 	struct cdns_pcie *pcie = &rc->pcie;
 	u32 value, ctrl;
+	u32 id;
 
 	/*
 	 * Set the root complex BAR configuration register:

@@ -90,8 +91,12 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
 	cdns_pcie_writel(pcie, CDNS_PCIE_LM_RC_BAR_CFG, value);
 
 	/* Set root port configuration space */
-	if (rc->vendor_id != 0xffff)
-		cdns_pcie_rp_writew(pcie, PCI_VENDOR_ID, rc->vendor_id);
+	if (rc->vendor_id != 0xffff) {
+		id = CDNS_PCIE_LM_ID_VENDOR(rc->vendor_id) |
+			CDNS_PCIE_LM_ID_SUBSYS(rc->vendor_id);
+		cdns_pcie_writel(pcie, CDNS_PCIE_LM_ID, id);
+	}
+
 	if (rc->device_id != 0xffff)
 		cdns_pcie_rp_writew(pcie, PCI_DEVICE_ID, rc->device_id);
 

Signature · 1 line

2.17.1

Add host mode dt-bindings for TI's J721E SoC.

Cc Rob Herring <[email protected]>

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 .../bindings/pci/ti,j721e-pci-host.yaml       | 161 ++++++++++++++++++
 1 file changed, 161 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +1 -0

new file mode 100644
index 000000000000..96184e1f419f
@@ -0,0 +1,161 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +6 -0

+
+  reg:
+    maxItems: 4
+
+  reg-names:
+    items:

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +19 -0

+
+  max-link-speed:
+    minimum: 1
+    maximum: 3
+
+  num-lanes:
+    minimum: 1
+    maximum: 2
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description: clock-specifier to represent input to the PCIe
+
+  clock-names:
+    items:
+      - const: fck

+ - enum: [16]

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +15 -0

+
+  reset-gpios:
+    description: GPIO specifier for the PERST# signal
+
+  phys:
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
+
+  phy-names:
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
+
+required:
+  - compatible
+  - reg

+ - reset-gpios

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +37 -0

+  - phys
+  - phy-names
+
+examples:
+  - |
+    #include <dt-bindings/soc/ti,sci_pm_domain.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+
+    pcie0_rc: pcie@2900000 {
+            compatible = "ti,j721e-pcie-host";
+            reg = <0x00 0x02900000 0x00 0x1000>,
+                  <0x00 0x02907000 0x00 0x400>,
+                  <0x00 0x0d000000 0x00 0x00800000>,
+                  <0x00 0x10000000 0x00 0x00001000>;
+            reg-names = "intd_cfg", "user_cfg", "reg", "cfg";
+            ti,syscon-pcie-ctrl = <&pcie0_ctrl>;
+            max-link-speed = <3>;
+            num-lanes = <2>;
+            power-domains = <&k3_pds 239 TI_SCI_PD_EXCLUSIVE>;
+            clocks = <&k3_clks 239 1>;
+            clock-names = "fck";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            bus-range = <0x0 0xf>;
+            cdns,max-outbound-regions = <16>;
+            cdns,no-bar-match-nbits = <64>;
+            vendor-id = /bits/ 16 <0x104c>;
+            device-id = /bits/ 16 <0xb00d>;
+            msi-map = <0x0 &gic_its 0x0 0x10000>;
+            dma-coherent;
+            reset-gpios = <&exp1 6 GPIO_ACTIVE_HIGH>;
+            phys = <&serdes0_pcie_link>;
+            phy-names = "pcie_phy";
+            ranges = <0x01000000 0x0 0x10001000  0x00 0x10001000  0x0 0x0010000>,
+                     <0x02000000 0x0 0x10011000  0x00 0x10011000  0x0 0x7fef000>;
+    };

Signature · 1 line

2.17.1

Quoted 16 lines · Kishon Vijay Abraham I

Add host mode dt-bindings for TI's J721E SoC.

Cc Rob Herring <[email protected]>

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 .../bindings/pci/ti,j721e-pci-host.yaml       | 161 ++++++++++++++++++
 1 file changed, 161 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +1 -0

new file mode 100644
index 000000000000..96184e1f419f
@@ -0,0 +1,161 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 11

Quoted 10 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +10 -0

+# Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/pci/ti,j721e-pci-host.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: TI J721E PCI Host (PCIe Wrapper)
+
+maintainers:
+  - Kishon Vijay Abraham I <[email protected]>

There's now a PCI bus schema. Reference it here:

allOf:
- $ref: "/schemas/pci/pci-bus.yaml#"

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 16

Quoted 5 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +5 -0

+
+properties:
+  compatible:
+      enum:
+          - ti,j721e-pcie-host

Indentation.

Quoted 6 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +6 -0

+
+  reg:
+    maxItems: 4
+
+  reg-names:
+    items:

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 32

Quoted 10 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +10 -0

+      - const: intd_cfg
+      - const: user_cfg
+      - const: reg
+      - const: cfg
+
+  ti,syscon-pcie-ctrl:
+    description: Phandle to the SYSCON entry required for configuring PCIe mode
+                 and link speed.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/phandle

You can drop the 'allOf' here if there aren't more constraints.

Quoted 19 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +19 -0

+
+  max-link-speed:
+    minimum: 1
+    maximum: 3
+
+  num-lanes:
+    minimum: 1
+    maximum: 2
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description: clock-specifier to represent input to the PCIe
+
+  clock-names:
+    items:
+      - const: fck

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 60

Quoted 10 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +9 -0

+
 
+  "#address-cells":
+    const: 3
+
+  "#size-cells":
+    const: 2
+
+  bus-range:
+    description: Range of bus numbers associated with this controller.

Drop these 3 as they are all standard.

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 66

Quoted 6 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +6 -0

+
+  cdns,max-outbound-regions:
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/int32

Can be negative? Use uint32.

The int* definitions are kind of broken until dtc is fixed to maintain sign.

Quoted 1 line

+ - enum: [16]

const: 16

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 79

Quoted 12 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +12 -0

+
+  cdns,no-bar-match-nbits:
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/int32
+      - enum: [64]
+
+  vendor-id:
+    const: 0x104c
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt

And elsewhere. Drop the description.

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 91

Quoted 12 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +12 -0

+
+  device-id:
+    const: 0xb00d
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
+
+  msi-map: true
+
+  dma-coherent:
+    description: Indicates that the PCIe IP block can ensure the coherency
+
+  ranges: true

Don't you know how many?

Quoted 15 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +15 -0

+
+  reset-gpios:
+    description: GPIO specifier for the PERST# signal
+
+  phys:
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
+
+  phy-names:
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
+
+required:
+  - compatible
+  - reg

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 115

Quoted 10 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +9 -0

+  - reg-names
+  - ti,syscon-pcie-ctrl
+  - max-link-speed
+  - num-lanes
+  - power-domains
+  - clocks
+  - clock-names
 
+  - "#address-cells"
+  - "#size-cells"

Can drop these 2. The bus schema requires them.

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 123

Quoted 8 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +8 -0

+  - bus-range
+  - cdns,max-outbound-regions
+  - cdns,no-bar-match-nbits
+  - vendor-id
+  - device-id
+  - msi-map
+  - dma-coherent
+  - ranges

Can drop, too.

Quoted 1 line

+ - reset-gpios

Isn't having this board dependent?

Quoted 16 lines · Kishon Vijay Abraham I

Add host mode dt-bindings for TI's J721E SoC.

Cc Rob Herring <[email protected]>

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 .../bindings/pci/ti,j721e-pci-host.yaml       | 161 ++++++++++++++++++
 1 file changed, 161 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +1 -0

new file mode 100644
index 000000000000..96184e1f419f
@@ -0,0 +1,161 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 11

Quoted 10 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +10 -0

+# Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/pci/ti,j721e-pci-host.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: TI J721E PCI Host (PCIe Wrapper)
+
+maintainers:
+  - Kishon Vijay Abraham I <[email protected]>

There's now a PCI bus schema. Reference it here:

allOf:
- $ref: "/schemas/pci/pci-bus.yaml#"

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 16

Quoted 5 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +5 -0

+
+properties:
+  compatible:
+      enum:
+          - ti,j721e-pcie-host

Indentation.

Quoted 10 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +10 -0

+
+  reg:
+    maxItems: 4
+
+  reg-names:
+    items:
+      - const: intd_cfg
+      - const: user_cfg
+      - const: reg
+      - const: cfg

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml line 32

Quoted 6 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml new file +6 -0

+
+  ti,syscon-pcie-ctrl:
+    description: Phandle to the SYSCON entry required for configuring PCIe mode
+                 and link speed.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/phandle

You can drop the 'allOf' here if there aren't more constraints.

Add PCIe EP mode dt-bindings for TI's J721E SoC.

Cc Rob Herring <[email protected]>

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 .../bindings/pci/ti,j721e-pci-ep.yaml         | 113 ++++++++++++++++++
 1 file changed, 113 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +6 -0

new file mode 100644
index 000000000000..4e2af4733998
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/pci/ti,j721e-pci-ep.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +31 -0

+
+  reg:
+    maxItems: 4
+
+  reg-names:
+    items:
+      - const: intd_cfg
+      - const: user_cfg
+      - const: reg
+      - const: mem
+
+  ti,syscon-pcie-ctrl:
+    description: Phandle to the SYSCON entry required for configuring PCIe mode
+                 and link speed.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/phandle
+
+  max-link-speed:
+    minimum: 1
+    maximum: 3
+
+  num-lanes:
+    minimum: 1
+    maximum: 2
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description: clock-specifier to represent input to the PCIe

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +40 -0

+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - ti,syscon-pcie-ctrl
+  - max-link-speed
+  - num-lanes
+  - power-domains
+  - clocks
+  - clock-names
+  - cdns,max-outbound-regions
+  - dma-coherent
+  - max-functions
+  - phys
+  - phy-names
+
+examples:
+  - |
+    #include <dt-bindings/soc/ti,sci_pm_domain.h>
+
+     pcie0_ep: pcie-ep@d000000 {
+            compatible = "ti,j721e-pcie-ep";
+            reg = <0x00 0x02900000 0x00 0x1000>,
+                  <0x00 0x02907000 0x00 0x400>,
+                  <0x00 0x0d000000 0x00 0x00800000>,
+                  <0x00 0x10000000 0x00 0x08000000>;
+            reg-names = "intd_cfg", "user_cfg", "reg", "mem";
+            ti,syscon-pcie-ctrl = <&pcie0_ctrl>;
+            max-link-speed = <3>;
+            num-lanes = <2>;
+            power-domains = <&k3_pds 239 TI_SCI_PD_EXCLUSIVE>;
+            clocks = <&k3_clks 239 1>;
+            clock-names = "fck";
+            cdns,max-outbound-regions = <16>;
+            max-functions = /bits/ 8 <6>;
+            dma-coherent;
+            phys = <&serdes0_pcie_link>;
+            phy-names = "pcie_phy";
+    };

Signature · 1 line

2.17.1

Quoted 21 lines · Kishon Vijay Abraham I

Add PCIe EP mode dt-bindings for TI's J721E SoC.

Cc Rob Herring <[email protected]>

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 .../bindings/pci/ti,j721e-pci-ep.yaml         | 113 ++++++++++++++++++
 1 file changed, 113 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +6 -0

new file mode 100644
index 000000000000..4e2af4733998
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/pci/ti,j721e-pci-ep.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml line 16

Quoted 10 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +10 -0

+
+title: TI J721E PCI EP (PCIe Wrapper)
+
+maintainers:
+  - Kishon Vijay Abraham I <[email protected]>
+
+properties:
+  compatible:
+      enum:
+          - ti,j721e-pcie-ep

Indentation.

Quoted 31 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +31 -0

+
+  reg:
+    maxItems: 4
+
+  reg-names:
+    items:
+      - const: intd_cfg
+      - const: user_cfg
+      - const: reg
+      - const: mem
+
+  ti,syscon-pcie-ctrl:
+    description: Phandle to the SYSCON entry required for configuring PCIe mode
+                 and link speed.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/phandle
+
+  max-link-speed:
+    minimum: 1
+    maximum: 3
+
+  num-lanes:
+    minimum: 1
+    maximum: 2
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description: clock-specifier to represent input to the PCIe

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml line 57

Quoted 10 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +10 -0

+
+  clock-names:
+    items:
+      - const: fck
+
+  cdns,max-outbound-regions:
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/int32

uint32

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml line 62

Quoted 5 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +5 -0

+      - enum: [16]
+
+  max-functions:
+    minimum: 1
+    maximum: 6

Needs a type ref. Or a common definition.

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml line 67

Quoted 5 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +5 -0

+
+  dma-coherent:
+    description: Indicates that the PCIe IP block can ensure the coherency
+
+  phys:

How many? Need to convert cdns,cdns-pcie-host.txt...

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml line 73

Quoted 7 lines

Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml new file +6 -0

+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
+
+  phy-names:
+    description: As defined in
+                 Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt

For all the properties shared with host mode, it might make sense to define a common schema with all those properties and then include it in the host and endpoint schemas.

Add support for PCIe controller in J721E SoC. The controller uses the Cadence PCIe core programmed by pcie-cadence*.c. The PCIe controller will work in both host mode and device mode.
Some of the features of the controller are:
*) Supports both RC mode and EP mode
*) Supports MSI and MSI-X support
*) Supports upto GEN3 speed mode
*) Supports SR-IOV capability
*) Ability to route all transactions via SMMU (support will be added
in a later patch).

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/Kconfig     |  23 ++
 drivers/pci/controller/cadence/Makefile    |   1 +
 drivers/pci/controller/cadence/pci-j721e.c | 430 +++++++++++++++++++++
 3 files changed, 454 insertions(+)
 create mode 100644 drivers/pci/controller/cadence/pci-j721e.c

drivers/pci/controller/cadence/Kconfig +23 -0

index b76b3cf55ce5..5d30564190e1 100644
@@ -42,4 +42,27 @@ config PCIE_CADENCE_PLAT_EP
 	  endpoint mode. This PCIe controller may be embedded into many
 	  different vendors SoCs.
 
+config PCI_J721E
+	bool
+
+config PCI_J721E_HOST
+	bool "TI J721E PCIe platform host controller"
+	depends on OF
+	select PCIE_CADENCE_HOST
+	select PCI_J721E
+	help
+	  Say Y here if you want to support the TI J721E PCIe platform
+	  controller in host mode. TI J721E PCIe controller uses Cadence PCIe
+	  core.
+
+config PCI_J721E_EP
+	bool "TI J721E PCIe platform endpoint controller"
+	depends on OF
+	depends on PCI_ENDPOINT
+	select PCIE_CADENCE_EP
+	select PCI_J721E
+	help
+	  Say Y here if you want to support the TI J721E PCIe platform
+	  controller in endpoint mode. TI J721E PCIe controller uses Cadence PCIe
+	  core.
 endmenu

drivers/pci/controller/cadence/Makefile +0 -0

index 232a3f20876a..9bac5fb2f13d 100644

drivers/pci/controller/cadence/pci-j721e.c new file +264 -0

new file mode 100644
index 000000000000..9ffb7e88c739
@@ -0,0 +1,430 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * pci-j721e - PCIe controller driver for TI's J721E SoCs
+ *
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Kishon Vijay Abraham I <[email protected]>
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/io.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include "../../pci.h"
+#include "pcie-cadence.h"
+
+#define J721E_PCIE_USER_CMD_STATUS	0x4
+#define LINK_TRAINING_ENABLE		BIT(0)
+
+#define J721E_PCIE_USER_LINKSTATUS	0x14
+#define LINK_STATUS			GENMASK(1, 0)
+
+enum link_status {
+	NO_RECIEVERS_DETECTED,
+	LINK_TRAINING_IN_PROGRESS,
+	LINK_UP_DL_IN_PROGRESS,
+	LINK_UP_DL_COMPLETED,
+};
+
+#define J721E_MODE_RC			BIT(7)
+#define LANE_COUNT_MASK			BIT(8)
+#define LANE_COUNT(n)			((n) << 8)
+
+#define GENERATION_SEL_MASK		GENMASK(1, 0)
+
+#define MAX_LANES			2
+
+struct j721e_pcie {
+	struct device		*dev;
+	struct device_node	*node;
+	u32			mode;
+	u32			num_lanes;
+	struct cdns_pcie	*cdns_pcie;
+	void __iomem		*user_cfg_base;
+};
+
+enum j721e_pcie_mode {
+	PCI_MODE_RC,
+	PCI_MODE_EP,
+};
+
+struct j721e_pcie_data {
+	enum j721e_pcie_mode	mode;
+};
+
+static inline u32 j721e_pcie_user_readl(struct j721e_pcie *pcie, u32 offset)
+{
+	return readl(pcie->user_cfg_base + offset);
+}
+
+static inline void j721e_pcie_user_writel(struct j721e_pcie *pcie, u32 offset,
+					  u32 value)
+{
+	writel(value, pcie->user_cfg_base + offset);
+}
+
+static int j721e_pcie_start_link(struct cdns_pcie *cdns_pcie, bool start)
+{
+	struct j721e_pcie *pcie = dev_get_drvdata(cdns_pcie->dev);
+	u32 reg;
+
+	reg = j721e_pcie_user_readl(pcie, J721E_PCIE_USER_CMD_STATUS);
+	if (start)
+		reg |= LINK_TRAINING_ENABLE;
+	else
+		reg &= ~LINK_TRAINING_ENABLE;
+	j721e_pcie_user_writel(pcie, J721E_PCIE_USER_CMD_STATUS, reg);
+
+	return 0;
+}
+
+static bool j721e_pcie_is_link_up(struct cdns_pcie *cdns_pcie)
+{
+	struct j721e_pcie *pcie = dev_get_drvdata(cdns_pcie->dev);
+	u32 reg;
+
+	reg = j721e_pcie_user_readl(pcie, J721E_PCIE_USER_LINKSTATUS);
+	reg &= LINK_STATUS;
+	if (reg == LINK_UP_DL_COMPLETED)
+		return true;
+
+	return false;
+}
+
+static const struct cdns_pcie_ops j721e_ops_ops = {
+	.read = cdns_pcie_read32,
+	.write = cdns_pcie_write32,
+	.start_link = j721e_pcie_start_link,
+	.is_link_up = j721e_pcie_is_link_up,
+};
+
+static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon)
+{
+	struct device *dev = pcie->dev;
+	u32 mask = J721E_MODE_RC;
+	u32 mode = pcie->mode;
+	u32 val = 0;
+	int ret = 0;
+
+	if (mode == PCI_MODE_RC)
+		val = J721E_MODE_RC;
+
+	ret = regmap_update_bits(syscon, 0, mask, val);
+	if (ret)
+		dev_err(dev, "failed to set pcie mode\n");
+
+	return ret;
+}
+
+static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
+				     struct regmap *syscon)
+{
+	struct device *dev = pcie->dev;
+	struct device_node *np = dev->of_node;
+	int link_speed;
+	u32 val = 0;
+	int ret;
+
+	link_speed = of_pci_get_max_link_speed(np);
+	if (link_speed < 2)
+		link_speed = 2;
+
+	val = link_speed - 1;
+	ret = regmap_update_bits(syscon, 0, GENERATION_SEL_MASK, val);
+	if (ret)
+		dev_err(dev, "failed to set link speed\n");
+
+	return ret;
+}
+
+static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie,
+				     struct regmap *syscon)
+{
+	struct device *dev = pcie->dev;
+	u32 lanes = pcie->num_lanes;
+	u32 val = 0;
+	int ret;
+
+	val = LANE_COUNT(lanes - 1);
+	ret = regmap_update_bits(syscon, 0, LANE_COUNT_MASK, val);
+	if (ret)
+		dev_err(dev, "failed to set link count\n");
+
+	return ret;
+}
+
+static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	struct device_node *node = dev->of_node;
+	struct regmap *syscon;
+	int ret;
+
+	syscon = syscon_regmap_lookup_by_phandle(node, "ti,syscon-pcie-ctrl");
+	if (IS_ERR(syscon)) {
+		dev_err(dev, "Unable to get ti,syscon-pcie-ctrl regmap\n");
+		return PTR_ERR(syscon);
+	}
+
+	ret = j721e_pcie_set_mode(pcie, syscon);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set pci mode\n");
+		return ret;
+	}
+
+	ret = j721e_pcie_set_link_speed(pcie, syscon);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set link speed\n");
+		return ret;
+	}
+
+	ret = j721e_pcie_set_lane_count(pcie, syscon);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set num-lanes\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int cdns_ti_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
+				    int where, int size, u32 *value)
+{
+	struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
+	struct cdns_pcie_rc *rc = pci_host_bridge_priv(bridge);
+	unsigned int busn = bus->number;
+
+	if (busn == rc->bus_range->start)
+		return pci_generic_config_read32(bus, devfn, where, size,
+						 value);
+
+	return pci_generic_config_read(bus, devfn, where, size, value);
+}
+
+static int cdns_ti_pcie_config_write(struct pci_bus *bus, unsigned int devfn,
+				     int where, int size, u32 value)
+{
+	struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
+	struct cdns_pcie_rc *rc = pci_host_bridge_priv(bridge);
+	unsigned int busn = bus->number;
+
+	if (busn == rc->bus_range->start)
+		return pci_generic_config_write32(bus, devfn, where, size,
+						  value);
+
+	return pci_generic_config_write(bus, devfn, where, size, value);
+}
+
+static struct pci_ops cdns_ti_pcie_host_ops = {
+	.map_bus	= cdns_pci_map_bus,
+	.read		= cdns_ti_pcie_config_read,
+	.write		= cdns_ti_pcie_config_write,
+};
+
+static const struct j721e_pcie_data j721e_pcie_rc_data = {
+	.mode = PCI_MODE_RC,
+};
+
+static const struct j721e_pcie_data j721e_pcie_ep_data = {
+	.mode = PCI_MODE_EP,
+};
+
+static const struct of_device_id of_j721e_pcie_match[] = {
+	{
+		.compatible = "ti,j721e-pcie-host",
+		.data = &j721e_pcie_rc_data,
+	},
+	{
+		.compatible = "ti,j721e-pcie-ep",
+		.data = &j721e_pcie_ep_data,
+	},
+	{},
+};
+
+static int j721e_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	const struct of_device_id *match;
+	struct pci_host_bridge *bridge;
+	struct j721e_pcie_data *data;
+	struct cdns_pcie *cdns_pcie;
+	struct j721e_pcie *pcie;
+	struct cdns_pcie_rc *rc;
+	struct cdns_pcie_ep *ep;
+	struct gpio_desc *gpiod;
+	struct resource *res;

drivers/pci/controller/cadence/pci-j721e.c new file +17 -0

+	mode = (u32)data->mode;
+
+	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->dev = dev;
+	pcie->node = node;
+	pcie->mode = mode;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "user_cfg");
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+	pcie->user_cfg_base = base;
+
+	ret = of_property_read_u32(node, "num-lanes", &num_lanes);

drivers/pci/controller/cadence/pci-j721e.c new file +129 -0

+	}
+
+	ret = j721e_pcie_ctrl_init(pcie);
+	if (ret < 0) {
+		dev_err(dev, "pm_runtime_get_sync failed\n");
+		goto err_get_sync;
+	}
+
+	switch (mode) {
+	case PCI_MODE_RC:
+		if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST)) {
+			ret = -ENODEV;
+			goto err_get_sync;
+		}
+
+		bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
+		if (!bridge) {
+			ret = -ENOMEM;
+			goto err_get_sync;
+		}
+
+		bridge->ops = &cdns_ti_pcie_host_ops;
+		rc = pci_host_bridge_priv(bridge);
+
+		cdns_pcie = &rc->pcie;
+		cdns_pcie->dev = dev;
+		cdns_pcie->ops = &j721e_ops_ops;
+		pcie->cdns_pcie = cdns_pcie;
+
+		gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+		if (IS_ERR(gpiod)) {
+			ret = PTR_ERR(gpiod);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Failed to get reset GPIO\n");
+			goto err_get_sync;
+		}
+
+		ret = cdns_pcie_init_phy(dev, cdns_pcie);
+		if (ret) {
+			dev_err(dev, "Failed to init phy\n");
+			goto err_get_sync;
+		}
+
+		/*
+		 * "Power Sequencing and Reset Signal Timings" table in
+		 * PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, REV. 3.0
+		 * indicates PERST# should be deasserted after minimum of 100us
+		 * once REFCLK is stable. The REFCLK to the connector in RC
+		 * mode is selected while enabling the PHY. So deassert PERST#
+		 * after 100 us.
+		 */
+		if (gpiod) {
+			usleep_range(100, 200);
+			gpiod_set_value_cansleep(gpiod, 1);
+		}
+
+		ret = cdns_pcie_host_setup(rc);
+		if (ret < 0)
+			goto err_pcie_setup;
+
+		break;
+	case PCI_MODE_EP:
+		if (!IS_ENABLED(CONFIG_PCIE_CADENCE_EP)) {
+			ret = -ENODEV;
+			goto err_get_sync;
+		}
+
+		ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
+		if (!ep) {
+			ret = -ENOMEM;
+			goto err_get_sync;
+		}
+
+		cdns_pcie = &ep->pcie;
+		cdns_pcie->dev = dev;
+		cdns_pcie->ops = &j721e_ops_ops;
+		pcie->cdns_pcie = cdns_pcie;
+
+		ret = cdns_pcie_init_phy(dev, cdns_pcie);
+		if (ret) {
+			dev_err(dev, "Failed to init phy\n");
+			goto err_get_sync;
+		}
+
+		ret = cdns_pcie_ep_setup(ep);
+		if (ret < 0)
+			goto err_pcie_setup;
+
+		break;
+	default:
+		dev_err(dev, "INVALID device type %d\n", mode);
+	}
+
+	return 0;
+
+err_pcie_setup:
+	cdns_pcie_disable_phy(cdns_pcie);
+
+err_get_sync:
+	pm_runtime_put(dev);
+	pm_runtime_disable(dev);
+
+	return ret;
+}
+
+static int j721e_pcie_remove(struct platform_device *pdev)
+{
+	struct j721e_pcie *pcie = platform_get_drvdata(pdev);
+	struct cdns_pcie *cdns_pcie = pcie->cdns_pcie;
+	struct device *dev = &pdev->dev;
+
+	cdns_pcie_disable_phy(cdns_pcie);
+	pm_runtime_put(dev);
+	pm_runtime_disable(dev);
+	of_platform_depopulate(dev);
+
+	return 0;
+}
+
+static struct platform_driver j721e_pcie_driver = {
+	.probe  = j721e_pcie_probe,
+	.remove = j721e_pcie_remove,
+	.driver = {
+		.name	= "j721e-pcie",
+		.of_match_table = of_j721e_pcie_match,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(j721e_pcie_driver);

Signature · 1 line

2.17.1

Add support for PCIe controller in J721E SoC. The controller uses the Cadence PCIe core programmed by pcie-cadence*.c. The PCIe controller will work in both host mode and device mode.
Some of the features of the controller are:
*) Supports both RC mode and EP mode
*) Supports MSI and MSI-X support
*) Supports upto GEN3 speed mode
*) Supports SR-IOV capability
*) Ability to route all transactions via SMMU (support will be added
in a later patch).

Signed-off-by Kishon Vijay Abraham I <[email protected]>

---
 drivers/pci/controller/cadence/Kconfig     |  23 ++
 drivers/pci/controller/cadence/Makefile    |   1 +
 drivers/pci/controller/cadence/pci-j721e.c | 430 +++++++++++++++++++++
 3 files changed, 454 insertions(+)
 create mode 100644 drivers/pci/controller/cadence/pci-j721e.c

drivers/pci/controller/cadence/Kconfig +5 -0

index b76b3cf55ce5..5d30564190e1 100644
@@ -42,4 +42,27 @@ config PCIE_CADENCE_PLAT_EP
 	  endpoint mode. This PCIe controller may be embedded into many
 	  different vendors SoCs.
 
+config PCI_J721E
+	bool
+
+config PCI_J721E_HOST
+	bool "TI J721E PCIe platform host controller"

drivers/pci/controller/cadence/Kconfig +8 -0

+	depends on OF
+	depends on PCI_ENDPOINT
+	select PCIE_CADENCE_EP
+	select PCI_J721E
+	help
+	  Say Y here if you want to support the TI J721E PCIe platform
+	  controller in endpoint mode. TI J721E PCIe controller uses Cadence PCIe
+	  core.
 endmenu

drivers/pci/controller/cadence/Makefile +1 -0

index 232a3f20876a..9bac5fb2f13d 100644
@@ -3,3 +3,4 @@ obj-$(CONFIG_PCIE_CADENCE) += pcie-cadence.o
 obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o
 obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
 obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o
+obj-$(CONFIG_PCI_J721E) += pci-j721e.o

drivers/pci/controller/cadence/pci-j721e.c new file +21 -0

new file mode 100644
index 000000000000..9ffb7e88c739
@@ -0,0 +1,430 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * pci-j721e - PCIe controller driver for TI's J721E SoCs
+ *
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Kishon Vijay Abraham I <[email protected]>
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/io.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include "../../pci.h"