| Message ID | 20230426045557.3613826-9-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: rcar-gen4: Add R-Car Gen4 PCIe support | expand |
On Wed, Apr 26, 2023 at 01:55:44PM +0900, Yoshihiro Shimoda wrote: > Add support for triggering INTx 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 Terminate at > Receiver message. The message code is specified as b1000xx for > the INTx assertion and b1001xx for the INTx de-assertion. [PATCH v14 08/21] PCI: dwc: Add support for triggering INTx IRQs from endpoint drivers What about shortening the subject out a bit: "PCI: designware-ep: Add INTx IRQs support" ? > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > .../pci/controller/dwc/pcie-designware-ep.c | 71 +++++++++++++++++-- > drivers/pci/controller/dwc/pcie-designware.h | 2 + > 2 files changed, 69 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 96375b0aba82..b35ed2b06193 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> > > @@ -485,14 +486,63 @@ 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_intx_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; > + > + /* > + * 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); > + > + return dw_pcie_ep_send_msg(ep, func_no, PCI_CODE_DEASSERT_INTA + intx, > + PCI_MSG_ROUTING_LOCAL); > +} Why do you need the underscored version of the method? I don't see it being utilized anywhere but in the dw_pcie_ep_raise_intx_irq() function. Thus its body can be completely moved to dw_pcie_ep_raise_intx_irq(). -Serge(y) > + > int dw_pcie_ep_raise_intx_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 INTx IRQs\n"); > + if (!ep->intx_mem) { > + dev_err(dev, "INTx not supported\n"); > + return -EOPNOTSUPP; > + } > > - return -EINVAL; > + return __dw_pcie_ep_raise_intx_irq(ep, func_no, 0); > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_intx_irq); > > @@ -623,6 +673,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 +848,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 +872,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 954d504890a1..8c08159ea08e 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]; > }; > > -- > 2.25.1 >
> -----Original Message----- > From: Serge Semin <fancer.lancer@gmail.com> > Sent: Monday, May 1, 2023 2:25 PM > To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > Cc: jingoohan1@gmail.com; mani@kernel.org; > gustavo.pimentel@synopsys.com; lpieralisi@kernel.org; > robh+dt@kernel.org; kw@linux.com; bhelgaas@google.com; > kishon@kernel.org; marek.vasut+renesas@gmail.com; linux- > pci@vger.kernel.org; devicetree@vger.kernel.org; linux-renesas- > soc@vger.kernel.org > Subject: [EXT] Re: [PATCH v14 08/21] PCI: dwc: Add support for triggering > INTx IRQs from endpoint drivers > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On Wed, Apr 26, 2023 at 01:55:44PM +0900, Yoshihiro Shimoda wrote: > > Add support for triggering INTx 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 Terminate at > > Receiver message. The message code is specified as b1000xx for > > the INTx assertion and b1001xx for the INTx de-assertion. > > [PATCH v14 08/21] PCI: dwc: Add support for triggering INTx IRQs from > endpoint drivers > > What about shortening the subject out a bit: > "PCI: designware-ep: Add INTx IRQs support" > ? > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > .../pci/controller/dwc/pcie-designware-ep.c | 71 +++++++++++++++++-- > > drivers/pci/controller/dwc/pcie-designware.h | 2 + > > 2 files changed, 69 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c > b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index 96375b0aba82..b35ed2b06193 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> > > > > @@ -485,14 +486,63 @@ 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_intx_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; > > + > > + /* > > + * 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); It is good method to implement legacy irq support. But there is problem which need be considered. According to PCI spec, section 6.2.1 PCI-compatible INTx Emulation PCI Express emulates the PCI interrupt mechanism including the Interrupt Pin and Interrupt Line registers of the PCI Configuration Space for PCI device Functions. PCI Express non-Switch devices may optionally support these registers for backwards compatibility. Switch devices are required to support them. Actual interrupt signaling uses in-band Messages rather than being signaled using physical pins. Two types of Messages are defined, Assert_INTx and Deassert_INTx, for emulation of PCI INTx signaling, where x is A, B, C, and D for respective PCI interrupt signals. These Messages are used to provide "virtual wires" for signaling interrupts across a Link. Switches collect these virtual wires and present a combined set at the Switch's Upstream Port. Ultimately, the virtual wires are routed to the Root Complex which maps the virtual wires to system interrupt resources. Devices must use assert/deassert Messages in pairs to emulate PCI interrupt **level-triggered** signaling. Actual mapping of PCI Express INTx emulation to system interrupts is implementation specific as is mapping of physical interrupt signals in conventional PCI. It should be level triggered. When call __dw_pcie_ep_raise_intx_irq, should be just assert INTx, then after PCI Host driver's irq handler to clear irq, EP side can desert INTx. So it should be two functions, one function raise INTx, and another one desert INTx. But I don't know How to avoid host side possible flood irq happen if EP can't desert INTx in time. > > + > > + return dw_pcie_ep_send_msg(ep, func_no, > PCI_CODE_DEASSERT_INTA + intx, > > + PCI_MSG_ROUTING_LOCAL); > > +} > > Why do you need the underscored version of the method? I don't see it > being utilized anywhere but in the dw_pcie_ep_raise_intx_irq() > function. Thus its body can be completely moved to > dw_pcie_ep_raise_intx_irq(). > > -Serge(y) > > > + > > int dw_pcie_ep_raise_intx_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 INTx IRQs\n"); > > + if (!ep->intx_mem) { > > + dev_err(dev, "INTx not supported\n"); > > + return -EOPNOTSUPP; > > + } > > > > - return -EINVAL; > > + return __dw_pcie_ep_raise_intx_irq(ep, func_no, 0); > > } > > EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_intx_irq); > > > > @@ -623,6 +673,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 +848,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 +872,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 954d504890a1..8c08159ea08e 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]; > > }; > > > > -- > > 2.25.1 > >
Hi Serge, > From: Serge Semin, Sent: Tuesday, May 2, 2023 4:25 AM > > On Wed, Apr 26, 2023 at 01:55:44PM +0900, Yoshihiro Shimoda wrote: > > Add support for triggering INTx 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 Terminate at > > Receiver message. The message code is specified as b1000xx for > > the INTx assertion and b1001xx for the INTx de-assertion. > > [PATCH v14 08/21] PCI: dwc: Add support for triggering INTx IRQs from endpoint drivers > > What about shortening the subject out a bit: > "PCI: designware-ep: Add INTx IRQs support" > ? Thank you for the suggestion. I'll modify the subject. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > .../pci/controller/dwc/pcie-designware-ep.c | 71 +++++++++++++++++-- > > drivers/pci/controller/dwc/pcie-designware.h | 2 + > > 2 files changed, 69 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index 96375b0aba82..b35ed2b06193 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> > > > > @@ -485,14 +486,63 @@ 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_intx_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; > > + > > + /* > > + * 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); > > + > > + return dw_pcie_ep_send_msg(ep, func_no, PCI_CODE_DEASSERT_INTA + intx, > > + PCI_MSG_ROUTING_LOCAL); > > +} > > Why do you need the underscored version of the method? I don't see it > being utilized anywhere but in the dw_pcie_ep_raise_intx_irq() > function. I thought that readability can be improved a bit if a function was separated. But, it seemed wrong. > Thus its body can be completely moved to > dw_pcie_ep_raise_intx_irq(). I got it. Best regards, Yoshihiro Shimoda > -Serge(y) > > > + > > int dw_pcie_ep_raise_intx_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 INTx IRQs\n"); > > + if (!ep->intx_mem) { > > + dev_err(dev, "INTx not supported\n"); > > + return -EOPNOTSUPP; > > + } > > > > - return -EINVAL; > > + return __dw_pcie_ep_raise_intx_irq(ep, func_no, 0); > > } > > EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_intx_irq); > > > > @@ -623,6 +673,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 +848,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 +872,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 954d504890a1..8c08159ea08e 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]; > > }; > > > > -- > > 2.25.1 > >
Hi Frank, > From: Frank Li, Sent: Wednesday, May 3, 2023 2:04 AM > > > > > On Wed, Apr 26, 2023 at 01:55:44PM +0900, Yoshihiro Shimoda wrote: > > > Add support for triggering INTx 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 Terminate at > > > Receiver message. The message code is specified as b1000xx for > > > the INTx assertion and b1001xx for the INTx de-assertion. > > > > [PATCH v14 08/21] PCI: dwc: Add support for triggering INTx IRQs from > > endpoint drivers > > > > What about shortening the subject out a bit: > > "PCI: designware-ep: Add INTx IRQs support" > > ? > > > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > --- > > > .../pci/controller/dwc/pcie-designware-ep.c | 71 +++++++++++++++++-- > > > drivers/pci/controller/dwc/pcie-designware.h | 2 + > > > 2 files changed, 69 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c > > b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > index 96375b0aba82..b35ed2b06193 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> > > > > > > @@ -485,14 +486,63 @@ 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_intx_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; > > > + > > > + /* > > > + * 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); > > It is good method to implement legacy irq support. But there is problem which need be considered. > > According to PCI spec, section 6.2.1 PCI-compatible INTx Emulation > > PCI Express emulates the PCI interrupt mechanism including the Interrupt Pin and Interrupt Line registers of the PCI > Configuration Space for PCI device Functions. PCI Express non-Switch devices may optionally support these registers for > backwards compatibility. Switch devices are required to support them. Actual interrupt signaling uses in-band Messages > rather than being signaled using physical pins. > Two types of Messages are defined, Assert_INTx and Deassert_INTx, for emulation of PCI INTx signaling, where x is A, B, > C, and D for respective PCI interrupt signals. These Messages are used to provide "virtual wires" for signaling interrupts > across a Link. Switches collect these virtual wires and present a combined set at the Switch's Upstream Port. Ultimately, > the virtual wires are routed to the Root Complex which maps the virtual wires to system interrupt resources. Devices > must use assert/deassert Messages in pairs to emulate PCI interrupt **level-triggered** signaling. Actual mapping of PCI > Express INTx emulation to system interrupts is implementation specific as is mapping of physical interrupt signals in > conventional PCI. > > It should be level triggered. When call __dw_pcie_ep_raise_intx_irq, should be just assert INTx, then after PCI > Host driver's irq handler to clear irq, EP side can desert INTx. > > So it should be two functions, one function raise INTx, and another one desert INTx. But I don't know > How to avoid host side possible flood irq happen if EP can't desert INTx in time. I also don't know how to design dessert INTx... To Kishon, Do you have any comments about supporting deassert INTx on PCIe EP framework? I checked the Documentation/PCI/endpoint/pci-endpoint.rst, but it doesn't mention about it. Should I modify the PCIe EP framework while I submit this patch series? If possible, after this patch series have been merged into upstream, I'll try to consider this problem somehow. Best regards, Yoshihiro Shimoda > > > + > > > + return dw_pcie_ep_send_msg(ep, func_no, > > PCI_CODE_DEASSERT_INTA + intx, > > > + PCI_MSG_ROUTING_LOCAL); > > > +} > > > > Why do you need the underscored version of the method? I don't see it > > being utilized anywhere but in the dw_pcie_ep_raise_intx_irq() > > function. Thus its body can be completely moved to > > dw_pcie_ep_raise_intx_irq(). > > > > -Serge(y) > > > > > + > > > int dw_pcie_ep_raise_intx_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 INTx IRQs\n"); > > > + if (!ep->intx_mem) { > > > + dev_err(dev, "INTx not supported\n"); > > > + return -EOPNOTSUPP; > > > + } > > > > > > - return -EINVAL; > > > + return __dw_pcie_ep_raise_intx_irq(ep, func_no, 0); > > > } > > > EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_intx_irq); > > > > > > @@ -623,6 +673,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 +848,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 +872,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 954d504890a1..8c08159ea08e 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]; > > > }; > > > > > > -- > > > 2.25.1 > > >
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 96375b0aba82..b35ed2b06193 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> @@ -485,14 +486,63 @@ 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_intx_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; + + /* + * 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); + + return dw_pcie_ep_send_msg(ep, func_no, PCI_CODE_DEASSERT_INTA + intx, + PCI_MSG_ROUTING_LOCAL); +} + int dw_pcie_ep_raise_intx_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 INTx IRQs\n"); + if (!ep->intx_mem) { + dev_err(dev, "INTx not supported\n"); + return -EOPNOTSUPP; + } - return -EINVAL; + return __dw_pcie_ep_raise_intx_irq(ep, func_no, 0); } EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_intx_irq); @@ -623,6 +673,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 +848,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 +872,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 954d504890a1..8c08159ea08e 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]; };
Add support for triggering INTx 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 Terminate 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 | 71 +++++++++++++++++-- drivers/pci/controller/dwc/pcie-designware.h | 2 + 2 files changed, 69 insertions(+), 4 deletions(-)