Hi Kishon,

Thank you for the review.

On Fri, Feb 21, 2020 at 11:36 AM Kishon Vijay Abraham I [off-list ref] wrote:

Hi Prabhakar,

On 09/02/20 12:06 am, Lad Prabhakar wrote:

quoted

R-Car PCIe controller has support to map multiple memory regions for
mapping the outbound memory in local system also the controller limits
single allocation for each region (that is, once a chunk is used from the
region it cannot be used to allocate a new one). This features inspires to
add support for handling multiple memory bases in endpoint framework.

With this patch pci_epc_mem_init() now accepts multiple regions, also
page_size for each memory region is passed during initialization so as
to handle single allocation for each region by setting the page_size to
window_size.

This patch looks much better now except for one comment below..

quoted

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pci/controller/cadence/pcie-cadence-ep.c |   7 +-
 drivers/pci/controller/dwc/pcie-designware-ep.c  |  29 ++--
 drivers/pci/controller/pcie-rockchip-ep.c        |   7 +-
 drivers/pci/endpoint/pci-epc-mem.c               | 166 ++++++++++++++++-------
 include/linux/pci-epc.h                          |  39 ++++--
 5 files changed, 168 insertions(+), 80 deletions(-)

.
.
<snip>
.
.

quoted

diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
index d2b174c..b3eedee 100644
--- a/drivers/pci/endpoint/pci-epc-mem.c
+++ b/drivers/pci/endpoint/pci-epc-mem.c

@@ -38,57 +38,76 @@ static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size)
 /**
  * __pci_epc_mem_init() - initialize the pci_epc_mem structure
  * @epc: the EPC device that invoked pci_epc_mem_init
- * @phys_base: the physical address of the base
- * @size: the size of the address space
- * @page_size: size of each page
+ * @windows: pointer to windows supported by the device
+ * @num_windows: number of windows device supports
  *
  * Invoke to initialize the pci_epc_mem structure used by the
  * endpoint functions to allocate mapped PCI address.
  */
-int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_base, size_t size,
-                    size_t page_size)
+int __pci_epc_mem_init(struct pci_epc *epc, struct pci_epc_mem_window *windows,
+                    int num_windows)
 {
-     int ret;
-     struct pci_epc_mem *mem;
-     unsigned long *bitmap;
+     struct pci_epc_mem *mem = NULL;
+     unsigned long *bitmap = NULL;
      unsigned int page_shift;
-     int pages;
+     size_t page_size;
      int bitmap_size;
-
-     if (page_size < PAGE_SIZE)
-             page_size = PAGE_SIZE;
-
-     page_shift = ilog2(page_size);
-     pages = size >> page_shift;
-     bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
-
-     mem = kzalloc(sizeof(*mem), GFP_KERNEL);
-     if (!mem) {
-             ret = -ENOMEM;
-             goto err;
-     }
-
-     bitmap = kzalloc(bitmap_size, GFP_KERNEL);
-     if (!bitmap) {
-             ret = -ENOMEM;
-             goto err_mem;
+     int pages;
+     int ret;
+     int i;
+
+     epc->mem_windows = 0;
+
+     if (!windows)
+             return -EINVAL;
+
+     if (num_windows <= 0)
+             return -EINVAL;
+
+     epc->mem = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL);
+     if (!epc->mem)
+             return -EINVAL;
+
+     for (i = 0; i < num_windows; i++) {
+             page_size = windows[i].page_size;
+             if (page_size < PAGE_SIZE)
+                     page_size = PAGE_SIZE;
+             page_shift = ilog2(page_size);
+             pages = windows[i].size >> page_shift;
+             bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
+
+             mem = kzalloc(sizeof(*mem), GFP_KERNEL);
+             if (!mem) {
+                     ret = -ENOMEM;
+                     goto err_mem;
+             }
+
+             bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+             if (!bitmap) {
+                     ret = -ENOMEM;
+                     goto err_mem;
+             }
+
+             mem->bitmap = bitmap;
+             mem->window.phys_base = windows[i].phys_base;
+             mem->page_size = page_size;
+             mem->pages = pages;
+             mem->window.size = windows[i].size;
+             epc->mem[i] = mem;
      }
-
-     mem->bitmap = bitmap;
-     mem->phys_base = phys_base;
-     mem->page_size = page_size;
-     mem->pages = pages;
-     mem->size = size;
-
-     epc->mem = mem;
+     epc->mem_windows = num_windows;

      return 0;

 err_mem:
-     kfree(mem);
+     for (; i >= 0; i--) {
+             mem = epc->mem[i];
+             kfree(mem->bitmap);
+             kfree(mem);
+     }
+     kfree(epc->mem);

-err:
-return ret;
+     return ret;
 }
 EXPORT_SYMBOL_GPL(__pci_epc_mem_init);

@@ -101,11 +120,21 @@ EXPORT_SYMBOL_GPL(__pci_epc_mem_init);
  */
 void pci_epc_mem_exit(struct pci_epc *epc)
 {
-     struct pci_epc_mem *mem = epc->mem;
+     struct pci_epc_mem *mem;
+     int i;
+
+     if (!epc->mem_windows)
+             return;
+
+     for (i = 0; i <= epc->mem_windows; i++) {
+             mem = epc->mem[i];
+             kfree(mem->bitmap);
+             kfree(mem);
+     }
+     kfree(epc->mem);

      epc->mem = NULL;
-     kfree(mem->bitmap);
-     kfree(mem);
+     epc->mem_windows = 0;
 }
 EXPORT_SYMBOL_GPL(pci_epc_mem_exit);

@@ -121,20 +150,30 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_exit);
 void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
                                   phys_addr_t *phys_addr, size_t size)
 {
-     int pageno;
-     void __iomem *virt_addr;
-     struct pci_epc_mem *mem = epc->mem;
-     unsigned int page_shift = ilog2(mem->page_size);
+     void __iomem *virt_addr = NULL;
+     struct pci_epc_mem *mem;
+     unsigned int page_shift;
+     int pageno = -EINVAL;
      int order;
+     int i;

-     size = ALIGN(size, mem->page_size);
-     order = pci_epc_mem_get_order(mem, size);
+     for (i = 0; i < epc->mem_windows; i++) {
+             mem = epc->mem[i];
+             size = ALIGN(size, mem->page_size);
+             order = pci_epc_mem_get_order(mem, size);
+
+             pageno = bitmap_find_free_region(mem->bitmap, mem->pages,
+                                              order);
+             if (pageno >= 0)
+                     break;
+     }

-     pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order);
      if (pageno < 0)
              return NULL;

-     *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift);
+     page_shift = ilog2(mem->page_size);
+     *phys_addr = mem->window.phys_base +
+                  ((phys_addr_t)pageno << page_shift);
      virt_addr = ioremap(*phys_addr, size);
      if (!virt_addr)
              bitmap_release_region(mem->bitmap, pageno, order);

@@ -143,6 +182,22 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
 }
 EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr);

+struct pci_epc_mem *pci_epc_get_matching_window(struct pci_epc *epc,
+                                             phys_addr_t phys_addr)
+{
+     struct pci_epc_mem *mem;
+     int i;
+
+     for (i = 0; i < epc->mem_windows; i++) {
+             mem = epc->mem[i];
+
+             if (mem->window.phys_base == phys_addr)
+                     return mem;

This will work only if the phys_addr is same as start of windows base.
This need not be true for all the platforms and will fail for all the
allocations except the first allocation.

Agreed, this worked for me because different windows were used for allocation.
If you are OK with below changes Ill post a V5 soon.

for (i = 0; i < epc->mem_windows; i++) {
    mem = epc->mem[i];

    if (phys_addr >= mem->window.phys_base &&
        phys_addr < (mem->window.phys_base + mem->window.size))
        return mem;
....
...
}

Cheers,
--Prabhakar Lad

Thanks
Kishon

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel