Re: [PATCH v8] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

From: Ray Jui
Date: Tue Nov 17 2015 - 11:24:54 EST




On 11/17/2015 5:55 AM, Marc Zyngier wrote:
On 17/11/15 13:27, Bharat Kumar Gogada wrote:

On Tue, 17 Nov 2015 04:59:39 +0000
Bharat Kumar Gogada <bharat.kumar.gogada@xxxxxxxxxx> wrote:

On 11/16/2015 7:14 AM, Marc Zyngier wrote:
On 11/11/15 06:33, Bharat Kumar Gogada wrote:
Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP.

Signed-off-by: Bharat Kumar Gogada <bharatku@xxxxxxxxxx>
Signed-off-by: Ravi Kiran Gummaluri <rgummal@xxxxxxxxxx>
---
Added logic to allocate contiguous hwirq in nwl_irq_domain_alloc
function.
Moved MSI functionality to separate functions.
Changed error return values.
---
.../devicetree/bindings/pci/xilinx-nwl-pcie.txt | 68 ++
drivers/pci/host/Kconfig | 16 +-
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pcie-xilinx-nwl.c | 1062
++++++++++++++++++++
4 files changed, 1144 insertions(+), 3 deletions(-)
create mode 100644
Documentation/devicetree/bindings/pci/xilinx-nwl-
pcie.txt
create mode 100644 drivers/pci/host/pcie-xilinx-nwl.c


[...]

+static int nwl_pcie_enable_msi(struct nwl_pcie *pcie, struct
+pci_bus
+*bus) {
+ struct platform_device *pdev = to_platform_device(pcie-
dev);
+ struct nwl_msi *msi = &pcie->msi;
+ unsigned long base;
+ int ret;
+
+ mutex_init(&msi->lock);
+
+ /* Check for msii_present bit */
+ ret = nwl_bridge_readl(pcie, I_MSII_CAPABILITIES) &
MSII_PRESENT;
+ if (!ret) {
+ dev_err(pcie->dev, "MSI not present\n");
+ ret = -EIO;
+ goto err;
+ }
+
+ /* Enable MSII */
+ nwl_bridge_writel(pcie, nwl_bridge_readl(pcie,
I_MSII_CONTROL) |
+ MSII_ENABLE, I_MSII_CONTROL);
+
+ /* Enable MSII status */
+ nwl_bridge_writel(pcie, nwl_bridge_readl(pcie,
I_MSII_CONTROL) |
+ MSII_STATUS_ENABLE, I_MSII_CONTROL);
+
+ /* setup AFI/FPCI range */
+ msi->pages = __get_free_pages(GFP_KERNEL, 0);
+ base = virt_to_phys((void *)msi->pages);
+ nwl_bridge_writel(pcie, lower_32_bits(base),
I_MSII_BASE_LO);
+ nwl_bridge_writel(pcie, upper_32_bits(base),
I_MSII_BASE_HI);

BTW, you still haven't answered my question as to why you need to
waste a page of memory here, and why putting a device address
doesn't
work.

As this is (to the best of my knowledge) the only driver doing so,
I'd really like you to explain the rational behind this.

Might not be the only driver doing so after I start sending out
patches for the iProc MSI support (soon), :)

I'm not sure how it works for the Xilinx NWL controller, which
Bharat should be able to help to explain. But for the iProc MSI
controller, there's no device I/O memory reserved for MSI posted writes
in the ASIC.
Therefore one needs to reserve host memory for these writes.


Our SoC doesn't reserve any memory for MSI, hence we need to assign a
memory space for it out of RAM.

Question to both of you: Does the write make it to memory? Or is it sampled
by the bridge and dropped?

No, write will not do any modification in memory, it is consumed by bridge.

Then you do not need to allocate memory at all. Use whatever memory you
already have. CC-ing Robin, as this may have interaction with the SMMU.


What happens if you replace the page in RAM with a dummy address?
What do you mean by dummy address ?

Any random (and suitably aligned) address. 0x00000deadbeef000 for example.

In our case, I'm pretty sure the writes make it to memory (RAM). I can try replacing it with a dummy address, but I'm pretty sure that will not work.

Thanks,

Ray


Thanks,

M.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/