Hi Prabhakar,

On 10/01/20 11:38 PM, Lad, Prabhakar wrote:

Hi Kishon,

Thank you for the review.

On Thu, Jan 9, 2020 at 6:25 AM Kishon Vijay Abraham I [off-list ref] wrote:

quoted

Hi Prabhakar,

On 08/01/20 9:52 PM, 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.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../pci/controller/cadence/pcie-cadence-ep.c  |  12 +-
 .../pci/controller/dwc/pcie-designware-ep.c   |  31 ++-
 drivers/pci/controller/pcie-rockchip-ep.c     |  14 +-
 drivers/pci/endpoint/functions/pci-epf-test.c |  29 +--
 drivers/pci/endpoint/pci-epc-core.c           |   7 +-
 drivers/pci/endpoint/pci-epc-mem.c            | 199 ++++++++++++++----
 include/linux/pci-epc.h                       |  46 ++--
 7 files changed, 245 insertions(+), 93 deletions(-)

.
.
<snip>
.
.

quoted

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 2091508c1620..289c266c2d90 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c

@@ -358,13 +358,15 @@ EXPORT_SYMBOL_GPL(pci_epc_unmap_addr);
  * @epc: the EPC device on which address is allocated
  * @func_no: the endpoint function number in the EPC device
  * @phys_addr: physical address of the local system
+ * @window: index to the window region where PCI address will be mapped
  * @pci_addr: PCI address to which the physical address should be mapped
  * @size: the size of the allocation
  *
  * Invoke to map CPU address with PCI address.
  */
 int pci_epc_map_addr(struct pci_epc *epc, u8 func_no,
-                  phys_addr_t phys_addr, u64 pci_addr, size_t size)
+                  phys_addr_t phys_addr, int window,
+                  u64 pci_addr, size_t size)
 {
      int ret;
      unsigned long flags;

@@ -376,7 +378,8 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no,
              return 0;

      spin_lock_irqsave(&epc->lock, flags);
-     ret = epc->ops->map_addr(epc, func_no, phys_addr, pci_addr, size);
+     ret = epc->ops->map_addr(epc, func_no, phys_addr,
+                              window, pci_addr, size);
      spin_unlock_irqrestore(&epc->lock, flags);

      return ret;

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

@@ -38,57 +38,77 @@ 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;
+     int pages;
+     int ret;
+     int i;

-     if (page_size < PAGE_SIZE)
-             page_size = PAGE_SIZE;
+     epc->mem_windows = 0;

-     page_shift = ilog2(page_size);
-     pages = size >> page_shift;
-     bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
+     if (!windows)
+             return -EINVAL;

-     mem = kzalloc(sizeof(*mem), GFP_KERNEL);
-     if (!mem) {
-             ret = -ENOMEM;
-             goto err;
-     }
+     if (num_windows <= 0)
+             return -EINVAL;

-     bitmap = kzalloc(bitmap_size, GFP_KERNEL);
-     if (!bitmap) {
-             ret = -ENOMEM;
-             goto err_mem;
-     }
+     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;
+             }

-     mem->bitmap = bitmap;
-     mem->phys_base = phys_base;
-     mem->page_size = page_size;
-     mem->pages = pages;
-     mem->size = size;
+             bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+             if (!bitmap) {
+                     ret = -ENOMEM;
+                     goto err_mem;
+             }

-     epc->mem = 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;
+             mem->window.map_size = 0;
+
+             epc->mem[i] = mem;
+     }
+     epc->mem_windows = num_windows;

      return 0;

 err_mem:
-     kfree(mem);
+     for (; i >= 0; i--) {

mem has to be reinitialized for every iteration of the loop.

not sure what exactly you mean here, could you please elaborate.

You are invoking "kfree(mem->bitmap);" in a loop without re-initializing
mem. Refer pci_epc_mem_exit() where you are doing the free properly.

quoted

quoted

+             kfree(mem->bitmap);
+             kfree(epc->mem[i]);
+     }
+     kfree(epc->mem);

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

@@ -101,48 +121,127 @@ 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];

Missing the above line in the error handling above.

quoted

quoted

+             kfree(mem->bitmap);
+             kfree(epc->mem[i]);
+     }

Thanks
Kishon

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