diff mbox series

[v12,11/19] PCI: dwc: Add support for triggering legacy IRQs

Message ID 20230414061622.2930995-12-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series PCI: rcar-gen4: Add R-Car Gen4 PCIe support | expand

Commit Message

Yoshihiro Shimoda April 14, 2023, 6:16 a.m. UTC
Add support for triggering legacy IRQs by using outbound iATU.
Outbound iATU is utilized to send assert and de-assert INTx TLPs.
The message is generated based on the payloadless Msg TLP with type
0x14, where 0x4 is the routing code implying the terminated at
Receiver message. The message code is specified as b1000xx for
the INTx assertion and b1001xx for the INTx de-assertion.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 .../pci/controller/dwc/pcie-designware-ep.c   | 65 +++++++++++++++++--
 drivers/pci/controller/dwc/pcie-designware.h  |  2 +
 2 files changed, 63 insertions(+), 4 deletions(-)

Comments

Bjorn Helgaas April 14, 2023, 7:42 p.m. UTC | #1
On Fri, Apr 14, 2023 at 03:16:14PM +0900, Yoshihiro Shimoda wrote:
> Add support for triggering legacy IRQs by using outbound iATU.
> Outbound iATU is utilized to send assert and de-assert INTx TLPs.
> The message is generated based on the payloadless Msg TLP with type
> 0x14, where 0x4 is the routing code implying the terminated at
> Receiver message. The message code is specified as b1000xx for
> the INTx assertion and b1001xx for the INTx de-assertion.

s/terminated at Receiver/Terminate at Receiver/, since I assume this
refers to the Message Routing mechanisms in Table 2-20 in sec 2.2.8.

I have a slight preference for using "INTx" instead of "legacy IRQ" in
subject, commit log, function names, etc because it's more specific.
"Legacy" is clear now, but tends to become obscure over time as more
and more features are added.  Eventually it just means "something old
that we don't like anymore."

> +static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
> +					 int intx)
> +{
> +	int ret;
> +
> +	ret = dw_pcie_ep_send_msg(ep, func_no, PCI_CODE_ASSERT_INTA + intx,
> +				  PCI_MSG_ROUTING_LOCAL);
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(1000, 2000);

Where do these values (1000, 2000) come from?  Spec reference would be
good if there is one.

Bjorn
Yoshihiro Shimoda April 17, 2023, 8:09 a.m. UTC | #2
Hi Bjorn,

> From: Bjorn Helgaas, Sent: Saturday, April 15, 2023 4:43 AM
> 
> On Fri, Apr 14, 2023 at 03:16:14PM +0900, Yoshihiro Shimoda wrote:
> > Add support for triggering legacy IRQs by using outbound iATU.
> > Outbound iATU is utilized to send assert and de-assert INTx TLPs.
> > The message is generated based on the payloadless Msg TLP with type
> > 0x14, where 0x4 is the routing code implying the terminated at
> > Receiver message. The message code is specified as b1000xx for
> > the INTx assertion and b1001xx for the INTx de-assertion.
> 
> s/terminated at Receiver/Terminate at Receiver/, since I assume this
> refers to the Message Routing mechanisms in Table 2-20 in sec 2.2.8.

I'll revise the description on v13.

> I have a slight preference for using "INTx" instead of "legacy IRQ" in
> subject, commit log, function names, etc because it's more specific.
> "Legacy" is clear now, but tends to become obscure over time as more
> and more features are added.  Eventually it just means "something old
> that we don't like anymore."

I got it. I'll use "INTx" instead of "legacy" on v13.

> > +static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
> > +					 int intx)
> > +{
> > +	int ret;
> > +
> > +	ret = dw_pcie_ep_send_msg(ep, func_no, PCI_CODE_ASSERT_INTA + intx,
> > +				  PCI_MSG_ROUTING_LOCAL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	usleep_range(1000, 2000);
> 
> Where do these values (1000, 2000) come from?  Spec reference would be
> good if there is one.

I referred the values from drivers/pci/controller/pcie-rcar-ep.c, but modified
the second value:
	usleep_range(1000, 1001);

Today I checked the documents of PCIe and this controller, but I could not
find any specification about the period. So, I tried some cases a little.

 No sleep: Always "NOT OKAY"
 udelay(10): Sometimes "NOT OKAY"
 usleep_range(50, 100): Always "OKAY"
 usleep_range(100, 200): Always "OKAY"
 usleep_range(1000, 2000): Always "OKAY"

So, using (1000, 2000) seems too long. So, I'll change the values
as (50, 100) and add comment like below:

	/*
	 * The documents of PCIe and the controller don't mention
	 * how long the INTx should be asserted. If 10 usec, sometimes
	 * it failed. So, asserted for 50 usec.
	 */
	usleep_range(50, 100);

Best regards,
Yoshihiro Shimoda

> Bjorn
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index cd6721594e43..2f34843272bd 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -6,6 +6,7 @@ 
  * Author: Kishon Vijay Abraham I <kishon@ti.com>
  */
 
+#include <linux/delay.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 
@@ -484,14 +485,57 @@  static const struct pci_epc_ops epc_ops = {
 	.get_features		= dw_pcie_ep_get_features,
 };
 
+static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
+			       u8 routing)
+{
+	struct dw_pcie_outbound_atu atu = { 0 };
+	struct pci_epc *epc = ep->epc;
+	int ret;
+
+	atu.func_no = func_no;
+	atu.code = code;
+	atu.routing = routing;
+	atu.type = PCIE_ATU_TYPE_MSG;
+	atu.cpu_addr = ep->intx_mem_phys;
+	atu.size = epc->mem->window.page_size;
+	ret = dw_pcie_ep_outbound_atu(ep, &atu);
+	if (ret)
+		return ret;
+
+	writel(0, ep->intx_mem);
+
+	dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
+
+	return 0;
+}
+
+static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
+					 int intx)
+{
+	int ret;
+
+	ret = dw_pcie_ep_send_msg(ep, func_no, PCI_CODE_ASSERT_INTA + intx,
+				  PCI_MSG_ROUTING_LOCAL);
+	if (ret)
+		return ret;
+
+	usleep_range(1000, 2000);
+
+	return dw_pcie_ep_send_msg(ep, func_no, PCI_CODE_DEASSERT_INTA + intx,
+				   PCI_MSG_ROUTING_LOCAL);
+}
+
 int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 	struct device *dev = pci->dev;
 
-	dev_err(dev, "EP cannot trigger legacy IRQs\n");
+	if (!ep->intx_mem) {
+		dev_err(dev, "EP cannot trigger legacy IRQs\n");
+		return -EINVAL;
+	}
 
-	return -EINVAL;
+	return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_legacy_irq);
 
@@ -622,6 +666,10 @@  void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 
 	dw_pcie_edma_remove(pci);
 
+	if (ep->intx_mem)
+		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
+				      epc->mem->window.page_size);
+
 	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
 			      epc->mem->window.page_size);
 
@@ -794,9 +842,14 @@  int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 		goto err_exit_epc_mem;
 	}
 
+	ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
+					      epc->mem->window.page_size);
+	if (!ep->intx_mem)
+		dev_warn(dev, "Failed to reserve memory for INTx\n");
+
 	ret = dw_pcie_edma_detect(pci);
 	if (ret)
-		goto err_free_epc_mem;
+		goto err_free_epc_mem_intx;
 
 	if (ep->ops->get_features) {
 		epc_features = ep->ops->get_features(ep);
@@ -813,7 +866,11 @@  int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 err_remove_edma:
 	dw_pcie_edma_remove(pci);
 
-err_free_epc_mem:
+err_free_epc_mem_intx:
+	if (ep->intx_mem)
+		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
+				      epc->mem->window.page_size);
+
 	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
 			      epc->mem->window.page_size);
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ef264809f7ce..5ec046744bb6 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -369,6 +369,8 @@  struct dw_pcie_ep {
 	unsigned long		*ob_window_map;
 	void __iomem		*msi_mem;
 	phys_addr_t		msi_mem_phys;
+	void __iomem		*intx_mem;
+	phys_addr_t		intx_mem_phys;
 	struct pci_epf_bar	*epf_bar[PCI_STD_NUM_BARS];
 };