Mohit,Hello Murali,-----Original Message----- From: Murali Karicheri [mailto:m-karicheri2@ti.com] Sent: Wednesday, June 11, 2014 12:21 AM To: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux- doc@vger.kernel.org Cc: Murali Karicheri; Santosh Shilimkar; Russell King; Grant Likely; Rob Herring; Mohit KUMAR DCG; Jingoo Han; Bjorn Helgaas; Pratyush ANAND; Richard Zhu; Kishon Vijay Abraham I; Marek Vasut; Arnd Bergmann; Pawel Moll; Mark Rutland; Ian Campbell; Kumar Gala; Randy Dunlap Subject: [PATCH v2 0/8] Add Keystone PCIe controller driver This patch adds a PCIe controller driver for Keystone SoCs. This is based on v1 of the series posted to the mailing list.1. I think your first patch is OK which handles platform specific ATU implementation. 2. For MSI part, I think you just need to add two new callbacks with pp-ops, something similar to: pp->ops->msi_set pp->ops->msi_clear With these two platform specific callbacks you should be able to manage MSI handling. So idea is that dw_msi code uses pp->ops->msi_set/clear if platform define these, otherwise use dw_msi_set/clear (which you need to refactor from existing code) So other than your keystone changes we expect 3 patches: -- 1st same as you sent 1/8: for ATU handeling -- 2nd to refactor dw_msi_set/clear: refactor from existing code
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index e4bd19a..f985811 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -277,11 +277,15 @@ static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
��������������� }
��������������� set_bit(pos0 + i, pp->msi_irq_in_use);
��������������� /*Enable corresponding interrupt in MSI interrupt controller */
-�������������� res = ((pos0 + i) / 32) * 12;
-�������������� bit = (pos0 + i) % 32;
-�������������� dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
-�������������� val |= 1 << bit;
-�������������� dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
+������������� if (!(pp->version & DW_V3_65)) {
+��������������������� res = ((pos0 + i) / 32) * 12;
+��������������������� bit = (pos0 + i) % 32;
+��������������������� dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
+������������������������������������� ��� 4, &val);
+��������������������� val |= 1 << bit;
+��������������������� dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
+������������������������������������� ��� 4, val);
+������������� }
������� }
������� *pos = pos0;
@@ -349,7 +353,10 @@ static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,
������� �*/
������� desc->msi_attrib.multiple = msgvec;
�
-������ msg.address_lo = virt_to_phys((void *)pp->msi_data);
+����� if (pp->ops->get_msi_data)
+������������� msg.address_lo = pp->ops->get_msi_data(pp);
+����� else
+������������� msg.address_lo = virt_to_phys((void *)pp->msi_data);
������� msg.address_hi = 0x0;
������� msg.data = pos;
Why not API enhancement and refactor the code in a single patch?-- 3rd to use pp->ops->msi_set/clear if defined.
Pls let us know for any issue or have different opinion. Regards Mohit-- 1.7.9.5