On 06/20/2017 12:28 PM, Joao Pinto wrote: > Às 4:50 PM de 6/20/2017, Murali Karicheri escreveu: >> On 06/19/2017 12:31 PM, Joao Pinto wrote: >>> >>> Hi Murali, >>> >>> Às 10:18 PM de 6/12/2017, Murali Karicheri escreveu: >>>> Joao, >>>> >>>> On 06/05/2017 12:19 PM, Joao Pinto wrote: >>>>> This patch series adds the new interrupt api to pcie-designware >>>>> make it possible to use features like MSIX. >>>>> >>>>> The work consisted of adapting the pcie-designware-host and each SoC >>>>> specific driver. >>>>> >>>>> The patch set was made against the Bjorn' next branch. >>>>> >>>>> Joao Pinto (9): >>>>> pci: adding new irq api to pci-designware >>>>> pci: exynos SoC driver adapted to new irq API >>>>> pci: imx6 SoC driver adapted to new irq API >>>>> pci: artpec6 SoC driver adapted to new irq API >>>>> pci: generic PCIe DW driver adapted to new irq API >>>>> pci: qcom SoC driver adapted to new irq API >>>>> pci: keystone SoC driver adapted to new irq API >>>>> pci: removing old irq api from pcie-designware >>>>> pci: remove limitation of the number of the available IRQs >>>>> >>>>> drivers/pci/dwc/pci-exynos.c | 18 -- >>>>> drivers/pci/dwc/pci-imx6.c | 18 -- >>>>> drivers/pci/dwc/pci-keystone-dw.c | 96 +------- >>>>> drivers/pci/dwc/pci-keystone.c | 1 + >>>>> drivers/pci/dwc/pci-keystone.h | 4 +- >>>>> drivers/pci/dwc/pci-layerscape.c | 4 +- >>>>> drivers/pci/dwc/pcie-artpec6.c | 18 -- >>>>> drivers/pci/dwc/pcie-designware-host.c | 395 +++++++++++++++++++-------------- >>>>> drivers/pci/dwc/pcie-designware-plat.c | 15 -- >>>>> drivers/pci/dwc/pcie-designware.h | 30 ++- >>>>> drivers/pci/dwc/pcie-qcom.c | 15 -- >>>>> 11 files changed, 255 insertions(+), 359 deletions(-) >>>>> >>>> >>>> I gave this a try today and it failed. Logs at >>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.ubuntu.com_24843963_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=FR_WRctJN74GGjjHco1VUqNfINiGA7lGruObX4bplAY&s=8QYJU-SCW5JEmMwLUqbOEBJA5hio9vIyeNyotVRos4Q&e= >>>> >>>> The first part of the log is with your patch series. >>>> The second part is before applying the patch. You will see >>>> that there is qc timeout log that shows the issue >>>> >>>> [ 12.791852] ata2.00: qc timeout (cmd 0xec) >>>> [ 12.795947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) >>>> [ 12.802061] ata2: limiting SATA link speed to 3.0 Gbps >>>> [ 13.301853] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320) >>>> [ 23.511852] ata2.00: qc timeout (cmd 0xec) >>>> [ 23.515947] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) >>>> [ 24.021850] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320) >>>> >>>> Is there any DT change I need to make for this? I didn't have a >>>> chance to review your patch w.r.t Keystone. But I see you have change >>>> the MSI IRq handling. >>>> >>> >>> There is no need to change your DT. I think I will need to debug this issue >>> further. Could you please advice a board containing this SoC? >>> >> I think I may be able to work with you on this. Keystone has different >> register set for MSI IRQ. It is in application space. Also there are >> 8 interrupts to ARM GIC that multiplexes the 32 MSI interrupts. So >> please review the original code vs the changed one w.r.t to this. I haven't >> had a chance to review the code as I am in the middle of a release. >> But will be able to spend some time after two weeks. Meanwhile please >> spend some time reviewing the code and exchange email to identify >> the issue. I can add specific debug prints for you as well and provide >> you the log for analysis. > > Ok, I agree! Thanks! > >> Look at this logic in the patch where a potential issue exists for Keystone. +static void dw_pci_setup_msi_msg(struct irq_data *data, struct msi_msg *msg) +{ + struct dw_pcie *pci = irq_data_get_irq_chip_data(data); + struct pcie_port *pp = &pci->pp; + u64 msi_target; + + if (pp->ops->get_msi_addr) + msi_target = pp->ops->get_msi_addr(pp); + else + msi_target = virt_to_phys((void *)pp->msi_data); + + msg->address_lo = lower_32_bits(msi_target); + msg->address_hi = upper_32_bits(msi_target); + + if (pp->ops->get_msi_data) + msg->data = pp->ops->get_msi_data(pp, data->hwirq); + else + msg->data = data->hwirq; + + dev_dbg(pci->dev, "msi#%d address_hi %#x address_lo %#x\n", + (int)data->hwirq, msg->address_hi, msg->address_lo); +} Keystone has 8 hw irqs, 4 multiplexed over each since the register set is different than other DWC cores. So will not work for Keystone, right? Murali >> Let me know. >> >> Murali >>> Thanks. >>> >>> >>> >>> >> >> > -- Murali Karicheri Linux Kernel, Keystone