Joao,
On 05/05/17 15:11, Joao Pinto wrote:
> Hello,
> I am currently adding the support for both MSI and MSI-x in pcie-designware and
> I am now facing a dificulty.
>
> My test endpoint is a USB 3.1 MSI / MSI-X capable and I tested that with
> the changes introduced by this patch we are able to enable MSI and MSI-X
> in this endpoint (depends on the usage of the MSI_FLAG_PCI_MSIX flag).
>
> The problem I am facing now is that Intc for the USB 3.1 Endpoint is completely
> bogus (524288) when it should be 1, and so I am not receiving any interrupts
> from the endpoint.
It is not bogus at all. It is computed from the PCI requester ID in
pci_msi_domain_calc_hwirq. What you're seeing is the PCI/MDI domain
view of that interrupt, which is completely virtual.
The real thing happens in your own irqdomain, where the hwirq for IRQ46
is probably 1 (only you can know that). As for why it doesn't work, see
below:
>
> I would apreciate that more experienced people in this interrupt subject could
> give me an hint.
>
> I send you the "lspci -v" results and /proc/interrupts.
>
> Thank you so much for your help.
>
> # cat /proc/interrupts
> CPU0
> 3: 755 ARC In-core Intc 3 Timer0 (per-cpu-tick)
> 4: 0 dw-apb-ictl 4 eth0
> 8: 1 dw-apb-ictl 8 ehci_hcd:usb1, ohci_hcd:usb2
> 9: 37 dw-apb-ictl 7 dw-mci
> 14: 0 dw-apb-ictl 14 e001d000.i2c
> 16: 0 dw-apb-ictl 16 e001f000.i2c
> 19: 145 dw-apb-ictl 19 serial
> 45: 0 PCI-MSI 0 aerdrv
> 46: 0 PCI-MSI 524288 xhci_hcd
>
> # lspci -v
> 00:00.0 PCI bridge: Synopsys, Inc. Device eddc (rev 01) (prog-if 00 [Normal decode])
> Flags: bus master, fast devsel, latency 0, IRQ 45
> Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> Memory behind bridge: d0400000-d04fffff
> [virtual] Expansion ROM at d0500000 [disabled] [size=64K]
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+
> Capabilities: [70] Express Root Port (Slot-), MSI 00
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [148] #19
> Capabilities: [158] Vendor Specific Information: ID=0002 Rev=4 Len=100 <?>
> Kernel driver in use: pcieport
>
> 01:00.0 USB controller: ASMedia Technology Inc. ASM1142 USB 3.1 Host Controller (prog-if 30 [XHCI])
> Subsystem: ASMedia Technology Inc. ASM1142 USB 3.1 Host Controller
> Flags: bus master, fast devsel, latency 0, IRQ 46
> Memory at d0400000 (64-bit, non-prefetchable) [size=32K]
> Capabilities: [50] MSI: Enable+ Count=1/8 Maskable- 64bit+
> Capabilities: [68] MSI-X: Enable- Count=8 Masked-
> Capabilities: [78] Power Management version 3
> Capabilities: [80] Express Endpoint, MSI 00
> Capabilities: [100] Virtual Channel
> Capabilities: [200] Advanced Error Reporting
> Capabilities: [280] #19
> Capabilities: [300] Latency Tolerance Reporting
> Kernel driver in use: xhci_hcd
>
> Signed-off-by: Joao Pinto <jpinto@xxxxxxxxxxxx>
> ---
> .../devicetree/bindings/pci/designware-pcie.txt | 2 +
> drivers/pci/dwc/pcie-designware-host.c | 292 ++++++++-------------
> drivers/pci/dwc/pcie-designware-plat.c | 35 +--
> drivers/pci/dwc/pcie-designware.h | 7 +-
> 4 files changed, 143 insertions(+), 193 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index b2480dd..41803ea 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -34,6 +34,8 @@ Optional properties:
> RC mode:
> - num-viewport: number of view ports configured in
> hardware. If a platform does not specify it, the driver assumes 2.
> +- num-vectors: number of available interrupt vectors. If a platform does not
> + specify it, the driver assumes 1.
> - bus-range: PCI bus numbers covered (it is recommended
> for new devicetrees to specify this property, to keep backwards
> compatibility a range of 0x00-0xff is assumed if not present)
> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> index 28ed32b..96a5fb3 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -11,6 +11,9 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/interrupt.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/msi.h>
> #include <linux/irqdomain.h>
> #include <linux/of_address.h>
> #include <linux/of_pci.h>
> @@ -53,32 +56,43 @@ static struct irq_chip dw_msi_irq_chip = {
> .irq_unmask = pci_msi_unmask_irq,
> };
It looks to me like the first mistake is just above. You don't seem to
propagate the mask/unmask operations into the parent domain, so your
interrupts are never enabled... You'd need something like this:
diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 28ed32ba4f1b..d42b8b12f168 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -45,12 +45,22 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
return dw_pcie_write(pci->dbi_base + where, size, val);
}
+static void dw_msi_mask_irq(struct irq_data *d)
+{
+ pci_msi_mask_irq(d);
+ irq_chip_mask_parent(d);
+}
+
+static void dw_msi_unmask_irq(struct irq_data *d)
+{
+ pci_msi_unmask_irq(d);
+ irq_chip_unmask_parent(d);
+}
+
static struct irq_chip dw_msi_irq_chip = {
.name = "PCI-MSI",
- .irq_enable = pci_msi_unmask_irq,
- .irq_disable = pci_msi_mask_irq,
- .irq_mask = pci_msi_mask_irq,
- .irq_unmask = pci_msi_unmask_irq,
+ .irq_mask = dw_msi_mask_irq,
+ .irq_unmask = dw_msi_unmask_irq,
};
I haven't dug any further, but this should be fixed first.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
