Message ID | 1587666159-6035-1-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com |
---|---|
Headers | show |
Series | Add endpoint driver for R-Car PCIe controller | expand |
Hi Prabhakar-san, > From: Lad Prabhakar, Sent: Friday, April 24, 2020 3:23 AM > > pci_epc_mem_init() internally used page size equal to *PAGE_SIZE* to > manage the address space so instead just pass the page size as a > argument to pci_epc_mem_init(). > > Also make pci_epc_mem_init() as a C function instead of a macro function > in preparation for adding support for pci-epc-mem core to handle multiple > windows. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thank you for the patch! Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda
Hi Prabhakar-san, > From: Lad Prabhakar, Sent: Friday, April 24, 2020 3:23 AM > > 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() initializes address space for endpoint > controller which support single window and pci_epc_multi_mem_init() > initializes multiple windows supported by endpoint controller. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thank you for the patch! Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda
Hi Prabhakar-san, > From: Lad Prabhakar, Sent: Friday, April 24, 2020 3:23 AM > > Add support for R-Car PCIe controller to work in endpoint mode. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thank you for the patch! Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda
Hi Prabhakar, On 4/23/2020 11:52 PM, Lad Prabhakar wrote: > 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() initializes address space for endpoint > controller which support single window and pci_epc_multi_mem_init() > initializes multiple windows supported by endpoint controller. Have a couple of clean-up comments. See below. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > .../pci/controller/dwc/pcie-designware-ep.c | 16 +- > drivers/pci/endpoint/pci-epc-mem.c | 199 ++++++++++++------ > include/linux/pci-epc.h | 33 ++- > 3 files changed, 170 insertions(+), 78 deletions(-) > . . <snip> . . > diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c > index cdd1d3821249..a3466da2a16f 100644 > --- a/drivers/pci/endpoint/pci-epc-mem.c > +++ b/drivers/pci/endpoint/pci-epc-mem.c > @@ -23,7 +23,7 @@ > static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size) > { > int order; > - unsigned int page_shift = ilog2(mem->page_size); > + unsigned int page_shift = ilog2(mem->window.page_size); > > size--; > size >>= page_shift; > @@ -36,67 +36,95 @@ 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 > + * pci_epc_multi_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_multi_mem_init(struct pci_epc *epc, > + struct pci_epc_mem_window *windows, > + unsigned 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->num_windows = 0; > > - page_shift = ilog2(page_size); > - pages = size >> page_shift; > - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long); > + if (!windows || !num_windows) > + return -EINVAL; > > - mem = kzalloc(sizeof(*mem), GFP_KERNEL); > - if (!mem) { > - ret = -ENOMEM; > - goto err; > - } > + epc->windows = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL); > + if (!epc->windows) > + return -ENOMEM; > > - bitmap = kzalloc(bitmap_size, GFP_KERNEL); > - if (!bitmap) { > - ret = -ENOMEM; > - goto err_mem; > - } > + 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->bitmap = bitmap; > - mem->phys_base = phys_base; > - mem->page_size = page_size; > - mem->pages = pages; > - mem->size = size; > - mutex_init(&mem->lock); > + mem = kzalloc(sizeof(*mem), GFP_KERNEL); > + if (!mem) { > + ret = -ENOMEM; > + i--; > + goto err_mem; > + } > > - epc->mem = mem; > + bitmap = kzalloc(bitmap_size, GFP_KERNEL); > + if (!bitmap) { > + ret = -ENOMEM; > + kfree(mem); > + i--; > + goto err_mem; > + } > + > + mem->window.phys_base = windows[i].phys_base; > + mem->window.size = windows[i].size; > + mem->window.page_size = page_size; > + mem->bitmap = bitmap; > + mem->pages = pages; > + mutex_init(&mem->lock); > + epc->windows[i] = mem; > + } > + > + epc->mem = epc->windows[0]; "mem" member of EPC looks unnecessary since that value is available at epc->windows[0]. > + epc->num_windows = num_windows; > > return 0; > > err_mem: > - kfree(mem); > + for (; i >= 0; i--) { > + mem = epc->windows[i]; > + kfree(mem->bitmap); > + kfree(mem); > + } > + kfree(epc->windows); > > -err: > -return ret; > + return ret; > } > -EXPORT_SYMBOL_GPL(__pci_epc_mem_init); > +EXPORT_SYMBOL_GPL(pci_epc_multi_mem_init); > > int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base, > size_t size, size_t page_size) > { > - return __pci_epc_mem_init(epc, base, size, page_size); > + struct pci_epc_mem_window mem_window; > + > + mem_window.phys_base = base; > + mem_window.size = size; > + mem_window.page_size = page_size; > + > + return pci_epc_multi_mem_init(epc, &mem_window, 1); > } > EXPORT_SYMBOL_GPL(pci_epc_mem_init); > > @@ -109,11 +137,22 @@ 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->num_windows) > + return; > + > + for (i = 0; i < epc->num_windows; i++) { > + mem = epc->windows[i]; > + kfree(mem->bitmap); > + kfree(mem); > + } > + kfree(epc->windows); > + > + epc->windows = NULL; > epc->mem = NULL; > - kfree(mem->bitmap); > - kfree(mem); > + epc->num_windows = 0; > } > EXPORT_SYMBOL_GPL(pci_epc_mem_exit); > > @@ -129,31 +168,60 @@ 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 = NULL; > - struct pci_epc_mem *mem = epc->mem; > - unsigned int page_shift = ilog2(mem->page_size); > + struct pci_epc_mem *mem; > + unsigned int page_shift; > + size_t align_size; > + int pageno; > int order; > + int i; > > - size = ALIGN(size, mem->page_size); > - order = pci_epc_mem_get_order(mem, size); > - > - mutex_lock(&mem->lock); > - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order); > - if (pageno < 0) > - goto ret; > + for (i = 0; i < epc->num_windows; i++) { > + mem = epc->windows[i]; > + mutex_lock(&mem->lock); > + align_size = ALIGN(size, mem->window.page_size); > + order = pci_epc_mem_get_order(mem, align_size); > > - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift); > - virt_addr = ioremap(*phys_addr, size); > - if (!virt_addr) > - bitmap_release_region(mem->bitmap, pageno, order); > + pageno = bitmap_find_free_region(mem->bitmap, mem->pages, > + order); > + if (pageno >= 0) { > + page_shift = ilog2(mem->window.page_size); > + *phys_addr = mem->window.phys_base + > + ((phys_addr_t)pageno << page_shift); > + virt_addr = ioremap(*phys_addr, align_size); > + if (!virt_addr) { > + bitmap_release_region(mem->bitmap, > + pageno, order); > + mutex_unlock(&mem->lock); > + continue; > + } > + mutex_unlock(&mem->lock); > + return virt_addr; > + } > + mutex_unlock(&mem->lock); > + } > > -ret: > - mutex_unlock(&mem->lock); > return virt_addr; > } > 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->num_windows; i++) { > + mem = epc->windows[i]; > + > + if (phys_addr >= mem->window.phys_base && > + phys_addr < (mem->window.phys_base + mem->window.size)) > + return mem; > + } > + > + return NULL; > +} > + > /** > * pci_epc_mem_free_addr() - free the allocated memory address > * @epc: the EPC device on which memory was allocated > @@ -166,14 +234,23 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr); > void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr, > void __iomem *virt_addr, size_t size) > { > + struct pci_epc_mem *mem; > + unsigned int page_shift; > + size_t page_size; > int pageno; > - struct pci_epc_mem *mem = epc->mem; > - unsigned int page_shift = ilog2(mem->page_size); > int order; > > + mem = pci_epc_get_matching_window(epc, phys_addr); > + if (!mem) { > + pr_err("failed to get matching window\n"); > + return; > + } > + > + page_size = mem->window.page_size; > + page_shift = ilog2(page_size); > iounmap(virt_addr); > - pageno = (phys_addr - mem->phys_base) >> page_shift; > - size = ALIGN(size, mem->page_size); > + pageno = (phys_addr - mem->window.phys_base) >> page_shift; > + size = ALIGN(size, page_size); > order = pci_epc_mem_get_order(mem, size); > mutex_lock(&mem->lock); > bitmap_release_region(mem->bitmap, pageno, order); > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > index 5bc1de65849e..cc66bec8be90 100644 > --- a/include/linux/pci-epc.h > +++ b/include/linux/pci-epc.h > @@ -65,20 +65,28 @@ struct pci_epc_ops { > struct module *owner; > }; > > +/** > + * struct pci_epc_mem_window - address window of the endpoint controller > + * @phys_base: physical base address of the PCI address window > + * @size: the size of the PCI address window > + * @page_size: size of each page > + */ > +struct pci_epc_mem_window { > + phys_addr_t phys_base; > + size_t size; > + size_t page_size; > +}; > + > /** > * struct pci_epc_mem - address space of the endpoint controller > - * @phys_base: physical base address of the PCI address space > - * @size: the size of the PCI address space > + * @window: address window of the endpoint controller > * @bitmap: bitmap to manage the PCI address space > * @pages: number of bits representing the address region > - * @page_size: size of each page > * @lock: mutex to protect bitmap > */ > struct pci_epc_mem { > - phys_addr_t phys_base; > - size_t size; > + struct pci_epc_mem_window window; Don't see any additional value in moving phys_base, size, page_size to a new structure and again including it here. Thanks Kishon
Hi Kishon, Thank you for the review. On Fri, Apr 24, 2020 at 7:13 AM Kishon Vijay Abraham I <kishon@ti.com> wrote: > > Hi Prabhakar, > > On 4/23/2020 11:52 PM, Lad Prabhakar wrote: > > 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() initializes address space for endpoint > > controller which support single window and pci_epc_multi_mem_init() > > initializes multiple windows supported by endpoint controller. > > Have a couple of clean-up comments. See below. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > .../pci/controller/dwc/pcie-designware-ep.c | 16 +- > > drivers/pci/endpoint/pci-epc-mem.c | 199 ++++++++++++------ > > include/linux/pci-epc.h | 33 ++- > > 3 files changed, 170 insertions(+), 78 deletions(-) > > > . > . > <snip> > . > . > > diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c > > index cdd1d3821249..a3466da2a16f 100644 > > --- a/drivers/pci/endpoint/pci-epc-mem.c > > +++ b/drivers/pci/endpoint/pci-epc-mem.c > > @@ -23,7 +23,7 @@ > > static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size) > > { > > int order; > > - unsigned int page_shift = ilog2(mem->page_size); > > + unsigned int page_shift = ilog2(mem->window.page_size); > > > > size--; > > size >>= page_shift; > > @@ -36,67 +36,95 @@ 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 > > + * pci_epc_multi_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_multi_mem_init(struct pci_epc *epc, > > + struct pci_epc_mem_window *windows, > > + unsigned 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->num_windows = 0; > > > > - page_shift = ilog2(page_size); > > - pages = size >> page_shift; > > - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long); > > + if (!windows || !num_windows) > > + return -EINVAL; > > > > - mem = kzalloc(sizeof(*mem), GFP_KERNEL); > > - if (!mem) { > > - ret = -ENOMEM; > > - goto err; > > - } > > + epc->windows = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL); > > + if (!epc->windows) > > + return -ENOMEM; > > > > - bitmap = kzalloc(bitmap_size, GFP_KERNEL); > > - if (!bitmap) { > > - ret = -ENOMEM; > > - goto err_mem; > > - } > > + 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->bitmap = bitmap; > > - mem->phys_base = phys_base; > > - mem->page_size = page_size; > > - mem->pages = pages; > > - mem->size = size; > > - mutex_init(&mem->lock); > > + mem = kzalloc(sizeof(*mem), GFP_KERNEL); > > + if (!mem) { > > + ret = -ENOMEM; > > + i--; > > + goto err_mem; > > + } > > > > - epc->mem = mem; > > + bitmap = kzalloc(bitmap_size, GFP_KERNEL); > > + if (!bitmap) { > > + ret = -ENOMEM; > > + kfree(mem); > > + i--; > > + goto err_mem; > > + } > > + > > + mem->window.phys_base = windows[i].phys_base; > > + mem->window.size = windows[i].size; > > + mem->window.page_size = page_size; > > + mem->bitmap = bitmap; > > + mem->pages = pages; > > + mutex_init(&mem->lock); > > + epc->windows[i] = mem; > > + } > > + > > + epc->mem = epc->windows[0]; > > "mem" member of EPC looks unnecessary since that value is available at > epc->windows[0]. This was suggested by Shimoda-san, as most of the current controller drivers support single region this pointer would be easier to access the region instead of adding #define EPC_DEFAULT_WINDOW 0 and accessing as epc->windows[EPC_DEFAULT_WINDOW]; > > + epc->num_windows = num_windows; > > > > return 0; > > > > err_mem: > > - kfree(mem); > > + for (; i >= 0; i--) { > > + mem = epc->windows[i]; > > + kfree(mem->bitmap); > > + kfree(mem); > > + } > > + kfree(epc->windows); > > > > -err: > > -return ret; > > + return ret; > > } > > -EXPORT_SYMBOL_GPL(__pci_epc_mem_init); > > +EXPORT_SYMBOL_GPL(pci_epc_multi_mem_init); > > > > int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base, > > size_t size, size_t page_size) > > { > > - return __pci_epc_mem_init(epc, base, size, page_size); > > + struct pci_epc_mem_window mem_window; > > + > > + mem_window.phys_base = base; > > + mem_window.size = size; > > + mem_window.page_size = page_size; > > + > > + return pci_epc_multi_mem_init(epc, &mem_window, 1); > > } > > EXPORT_SYMBOL_GPL(pci_epc_mem_init); > > > > @@ -109,11 +137,22 @@ 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->num_windows) > > + return; > > + > > + for (i = 0; i < epc->num_windows; i++) { > > + mem = epc->windows[i]; > > + kfree(mem->bitmap); > > + kfree(mem); > > + } > > + kfree(epc->windows); > > + > > + epc->windows = NULL; > > epc->mem = NULL; > > - kfree(mem->bitmap); > > - kfree(mem); > > + epc->num_windows = 0; > > } > > EXPORT_SYMBOL_GPL(pci_epc_mem_exit); > > > > @@ -129,31 +168,60 @@ 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 = NULL; > > - struct pci_epc_mem *mem = epc->mem; > > - unsigned int page_shift = ilog2(mem->page_size); > > + struct pci_epc_mem *mem; > > + unsigned int page_shift; > > + size_t align_size; > > + int pageno; > > int order; > > + int i; > > > > - size = ALIGN(size, mem->page_size); > > - order = pci_epc_mem_get_order(mem, size); > > - > > - mutex_lock(&mem->lock); > > - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order); > > - if (pageno < 0) > > - goto ret; > > + for (i = 0; i < epc->num_windows; i++) { > > + mem = epc->windows[i]; > > + mutex_lock(&mem->lock); > > + align_size = ALIGN(size, mem->window.page_size); > > + order = pci_epc_mem_get_order(mem, align_size); > > > > - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift); > > - virt_addr = ioremap(*phys_addr, size); > > - if (!virt_addr) > > - bitmap_release_region(mem->bitmap, pageno, order); > > + pageno = bitmap_find_free_region(mem->bitmap, mem->pages, > > + order); > > + if (pageno >= 0) { > > + page_shift = ilog2(mem->window.page_size); > > + *phys_addr = mem->window.phys_base + > > + ((phys_addr_t)pageno << page_shift); > > + virt_addr = ioremap(*phys_addr, align_size); > > + if (!virt_addr) { > > + bitmap_release_region(mem->bitmap, > > + pageno, order); > > + mutex_unlock(&mem->lock); > > + continue; > > + } > > + mutex_unlock(&mem->lock); > > + return virt_addr; > > + } > > + mutex_unlock(&mem->lock); > > + } > > > > -ret: > > - mutex_unlock(&mem->lock); > > return virt_addr; > > } > > 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->num_windows; i++) { > > + mem = epc->windows[i]; > > + > > + if (phys_addr >= mem->window.phys_base && > > + phys_addr < (mem->window.phys_base + mem->window.size)) > > + return mem; > > + } > > + > > + return NULL; > > +} > > + > > /** > > * pci_epc_mem_free_addr() - free the allocated memory address > > * @epc: the EPC device on which memory was allocated > > @@ -166,14 +234,23 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr); > > void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr, > > void __iomem *virt_addr, size_t size) > > { > > + struct pci_epc_mem *mem; > > + unsigned int page_shift; > > + size_t page_size; > > int pageno; > > - struct pci_epc_mem *mem = epc->mem; > > - unsigned int page_shift = ilog2(mem->page_size); > > int order; > > > > + mem = pci_epc_get_matching_window(epc, phys_addr); > > + if (!mem) { > > + pr_err("failed to get matching window\n"); > > + return; > > + } > > + > > + page_size = mem->window.page_size; > > + page_shift = ilog2(page_size); > > iounmap(virt_addr); > > - pageno = (phys_addr - mem->phys_base) >> page_shift; > > - size = ALIGN(size, mem->page_size); > > + pageno = (phys_addr - mem->window.phys_base) >> page_shift; > > + size = ALIGN(size, page_size); > > order = pci_epc_mem_get_order(mem, size); > > mutex_lock(&mem->lock); > > bitmap_release_region(mem->bitmap, pageno, order); > > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > > index 5bc1de65849e..cc66bec8be90 100644 > > --- a/include/linux/pci-epc.h > > +++ b/include/linux/pci-epc.h > > @@ -65,20 +65,28 @@ struct pci_epc_ops { > > struct module *owner; > > }; > > > > +/** > > + * struct pci_epc_mem_window - address window of the endpoint controller > > + * @phys_base: physical base address of the PCI address window > > + * @size: the size of the PCI address window > > + * @page_size: size of each page > > + */ > > +struct pci_epc_mem_window { > > + phys_addr_t phys_base; > > + size_t size; > > + size_t page_size; > > +}; > > + > > /** > > * struct pci_epc_mem - address space of the endpoint controller > > - * @phys_base: physical base address of the PCI address space > > - * @size: the size of the PCI address space > > + * @window: address window of the endpoint controller > > * @bitmap: bitmap to manage the PCI address space > > * @pages: number of bits representing the address region > > - * @page_size: size of each page > > * @lock: mutex to protect bitmap > > */ > > struct pci_epc_mem { > > - phys_addr_t phys_base; > > - size_t size; > > + struct pci_epc_mem_window window; > > Don't see any additional value in moving phys_base, size, page_size to a new > structure and again including it here. > Controllers supporting multiple windows create a list of supported regions (struct pci_epc_mem_window ) and pass a pointer to pci_epc_multi_mem_init(), hence this split. Cheers, --Prabhakar
Hi Prabhakar, On 4/24/2020 1:16 PM, Lad, Prabhakar wrote: > Hi Kishon, > > Thank you for the review. > > On Fri, Apr 24, 2020 at 7:13 AM Kishon Vijay Abraham I <kishon@ti.com> wrote: >> >> Hi Prabhakar, >> >> On 4/23/2020 11:52 PM, Lad Prabhakar wrote: >>> 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() initializes address space for endpoint >>> controller which support single window and pci_epc_multi_mem_init() >>> initializes multiple windows supported by endpoint controller. >> >> Have a couple of clean-up comments. See below. >>> >>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >>> --- >>> .../pci/controller/dwc/pcie-designware-ep.c | 16 +- >>> drivers/pci/endpoint/pci-epc-mem.c | 199 ++++++++++++------ >>> include/linux/pci-epc.h | 33 ++- >>> 3 files changed, 170 insertions(+), 78 deletions(-) >>> >> . >> . >> <snip> >> . >> . >>> diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c >>> index cdd1d3821249..a3466da2a16f 100644 >>> --- a/drivers/pci/endpoint/pci-epc-mem.c >>> +++ b/drivers/pci/endpoint/pci-epc-mem.c >>> @@ -23,7 +23,7 @@ >>> static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size) >>> { >>> int order; >>> - unsigned int page_shift = ilog2(mem->page_size); >>> + unsigned int page_shift = ilog2(mem->window.page_size); >>> >>> size--; >>> size >>= page_shift; >>> @@ -36,67 +36,95 @@ 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 >>> + * pci_epc_multi_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_multi_mem_init(struct pci_epc *epc, >>> + struct pci_epc_mem_window *windows, >>> + unsigned 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->num_windows = 0; >>> >>> - page_shift = ilog2(page_size); >>> - pages = size >> page_shift; >>> - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long); >>> + if (!windows || !num_windows) >>> + return -EINVAL; >>> >>> - mem = kzalloc(sizeof(*mem), GFP_KERNEL); >>> - if (!mem) { >>> - ret = -ENOMEM; >>> - goto err; >>> - } >>> + epc->windows = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL); >>> + if (!epc->windows) >>> + return -ENOMEM; >>> >>> - bitmap = kzalloc(bitmap_size, GFP_KERNEL); >>> - if (!bitmap) { >>> - ret = -ENOMEM; >>> - goto err_mem; >>> - } >>> + 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->bitmap = bitmap; >>> - mem->phys_base = phys_base; >>> - mem->page_size = page_size; >>> - mem->pages = pages; >>> - mem->size = size; >>> - mutex_init(&mem->lock); >>> + mem = kzalloc(sizeof(*mem), GFP_KERNEL); >>> + if (!mem) { >>> + ret = -ENOMEM; >>> + i--; >>> + goto err_mem; >>> + } >>> >>> - epc->mem = mem; >>> + bitmap = kzalloc(bitmap_size, GFP_KERNEL); >>> + if (!bitmap) { >>> + ret = -ENOMEM; >>> + kfree(mem); >>> + i--; >>> + goto err_mem; >>> + } >>> + >>> + mem->window.phys_base = windows[i].phys_base; >>> + mem->window.size = windows[i].size; >>> + mem->window.page_size = page_size; >>> + mem->bitmap = bitmap; >>> + mem->pages = pages; >>> + mutex_init(&mem->lock); >>> + epc->windows[i] = mem; >>> + } >>> + >>> + epc->mem = epc->windows[0]; >> >> "mem" member of EPC looks unnecessary since that value is available at >> epc->windows[0]. > This was suggested by Shimoda-san, as most of the current controller > drivers support single region this pointer would be easier to access > the region instead of adding #define EPC_DEFAULT_WINDOW 0 and > accessing as epc->windows[EPC_DEFAULT_WINDOW]; > >>> + epc->num_windows = num_windows; >>> >>> return 0; >>> >>> err_mem: >>> - kfree(mem); >>> + for (; i >= 0; i--) { >>> + mem = epc->windows[i]; >>> + kfree(mem->bitmap); >>> + kfree(mem); >>> + } >>> + kfree(epc->windows); >>> >>> -err: >>> -return ret; >>> + return ret; >>> } >>> -EXPORT_SYMBOL_GPL(__pci_epc_mem_init); >>> +EXPORT_SYMBOL_GPL(pci_epc_multi_mem_init); >>> >>> int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base, >>> size_t size, size_t page_size) >>> { >>> - return __pci_epc_mem_init(epc, base, size, page_size); >>> + struct pci_epc_mem_window mem_window; >>> + >>> + mem_window.phys_base = base; >>> + mem_window.size = size; >>> + mem_window.page_size = page_size; >>> + >>> + return pci_epc_multi_mem_init(epc, &mem_window, 1); >>> } >>> EXPORT_SYMBOL_GPL(pci_epc_mem_init); >>> >>> @@ -109,11 +137,22 @@ 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->num_windows) >>> + return; >>> + >>> + for (i = 0; i < epc->num_windows; i++) { >>> + mem = epc->windows[i]; >>> + kfree(mem->bitmap); >>> + kfree(mem); >>> + } >>> + kfree(epc->windows); >>> + >>> + epc->windows = NULL; >>> epc->mem = NULL; >>> - kfree(mem->bitmap); >>> - kfree(mem); >>> + epc->num_windows = 0; >>> } >>> EXPORT_SYMBOL_GPL(pci_epc_mem_exit); >>> >>> @@ -129,31 +168,60 @@ 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 = NULL; >>> - struct pci_epc_mem *mem = epc->mem; >>> - unsigned int page_shift = ilog2(mem->page_size); >>> + struct pci_epc_mem *mem; >>> + unsigned int page_shift; >>> + size_t align_size; >>> + int pageno; >>> int order; >>> + int i; >>> >>> - size = ALIGN(size, mem->page_size); >>> - order = pci_epc_mem_get_order(mem, size); >>> - >>> - mutex_lock(&mem->lock); >>> - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order); >>> - if (pageno < 0) >>> - goto ret; >>> + for (i = 0; i < epc->num_windows; i++) { >>> + mem = epc->windows[i]; >>> + mutex_lock(&mem->lock); >>> + align_size = ALIGN(size, mem->window.page_size); >>> + order = pci_epc_mem_get_order(mem, align_size); >>> >>> - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift); >>> - virt_addr = ioremap(*phys_addr, size); >>> - if (!virt_addr) >>> - bitmap_release_region(mem->bitmap, pageno, order); >>> + pageno = bitmap_find_free_region(mem->bitmap, mem->pages, >>> + order); >>> + if (pageno >= 0) { >>> + page_shift = ilog2(mem->window.page_size); >>> + *phys_addr = mem->window.phys_base + >>> + ((phys_addr_t)pageno << page_shift); >>> + virt_addr = ioremap(*phys_addr, align_size); >>> + if (!virt_addr) { >>> + bitmap_release_region(mem->bitmap, >>> + pageno, order); >>> + mutex_unlock(&mem->lock); >>> + continue; >>> + } >>> + mutex_unlock(&mem->lock); >>> + return virt_addr; >>> + } >>> + mutex_unlock(&mem->lock); >>> + } >>> >>> -ret: >>> - mutex_unlock(&mem->lock); >>> return virt_addr; >>> } >>> 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->num_windows; i++) { >>> + mem = epc->windows[i]; >>> + >>> + if (phys_addr >= mem->window.phys_base && >>> + phys_addr < (mem->window.phys_base + mem->window.size)) >>> + return mem; >>> + } >>> + >>> + return NULL; >>> +} >>> + >>> /** >>> * pci_epc_mem_free_addr() - free the allocated memory address >>> * @epc: the EPC device on which memory was allocated >>> @@ -166,14 +234,23 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr); >>> void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr, >>> void __iomem *virt_addr, size_t size) >>> { >>> + struct pci_epc_mem *mem; >>> + unsigned int page_shift; >>> + size_t page_size; >>> int pageno; >>> - struct pci_epc_mem *mem = epc->mem; >>> - unsigned int page_shift = ilog2(mem->page_size); >>> int order; >>> >>> + mem = pci_epc_get_matching_window(epc, phys_addr); >>> + if (!mem) { >>> + pr_err("failed to get matching window\n"); >>> + return; >>> + } >>> + >>> + page_size = mem->window.page_size; >>> + page_shift = ilog2(page_size); >>> iounmap(virt_addr); >>> - pageno = (phys_addr - mem->phys_base) >> page_shift; >>> - size = ALIGN(size, mem->page_size); >>> + pageno = (phys_addr - mem->window.phys_base) >> page_shift; >>> + size = ALIGN(size, page_size); >>> order = pci_epc_mem_get_order(mem, size); >>> mutex_lock(&mem->lock); >>> bitmap_release_region(mem->bitmap, pageno, order); >>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h >>> index 5bc1de65849e..cc66bec8be90 100644 >>> --- a/include/linux/pci-epc.h >>> +++ b/include/linux/pci-epc.h >>> @@ -65,20 +65,28 @@ struct pci_epc_ops { >>> struct module *owner; >>> }; >>> >>> +/** >>> + * struct pci_epc_mem_window - address window of the endpoint controller >>> + * @phys_base: physical base address of the PCI address window >>> + * @size: the size of the PCI address window >>> + * @page_size: size of each page >>> + */ >>> +struct pci_epc_mem_window { >>> + phys_addr_t phys_base; >>> + size_t size; >>> + size_t page_size; >>> +}; >>> + >>> /** >>> * struct pci_epc_mem - address space of the endpoint controller >>> - * @phys_base: physical base address of the PCI address space >>> - * @size: the size of the PCI address space >>> + * @window: address window of the endpoint controller >>> * @bitmap: bitmap to manage the PCI address space >>> * @pages: number of bits representing the address region >>> - * @page_size: size of each page >>> * @lock: mutex to protect bitmap >>> */ >>> struct pci_epc_mem { >>> - phys_addr_t phys_base; >>> - size_t size; >>> + struct pci_epc_mem_window window; >> >> Don't see any additional value in moving phys_base, size, page_size to a new >> structure and again including it here. >> > Controllers supporting multiple windows create a list of supported > regions (struct pci_epc_mem_window ) and pass a pointer to > pci_epc_multi_mem_init(), hence this split. Okay, thanks for clarifying. Regards Kishon
Hi Kishon, On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Hi All, > > This patch series adds support for endpoint driver for R-Car PCIe controller on > R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows > supported by the controller for mapping PCI address locally. > > Note: > The cadence/rockchip/designware endpoint drivers are build tested only. > > Changes for v9 (Re-spun this series as there were minimal changes requested): > * Rebased patches on top of v5.7.rc1 > * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx() > * Added a check for max_functions read from DT to restrict with > RCAR_EPC_MAX_FUNCTIONS > * Replaced MSICAP0_MMENUM with MSICAP0_MMESE > * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr() > * Fixed looping for number windows in pci_epc_mem_exit() > * Set maximum to 1 for max-functions in DT binding (I have restored the acks > from Rob and Shimoda-san) > * Sorted the entry in MAINTAINERS > > Changes for v8: > * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h > * Fixed typo in commit message for patch 2/8 > * Reworded commit message for patch 5/8 as suggested by Bjorn > * Split up patch to add pci_epc_mem_init() interface to add page_size argument > as suggested by Bjorn. > > Changes for v7: > * Fixed review comments pointed by Shimoda-san > 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed > the example as its built with #{address,size}-cells = <1>. I have still > restored the Ack from Rob and Shimoda-san with these changes. > 2] Split up the patches so that they can be picked up by respective subsystem > patches 1/4-9/11 are now part of this series. > 3] Dropped altering a comment in pci-epc.h > 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size > variable doesn't get overwritten in the loop. > 5] Replaced i-=1 with i-- > 6] Replaced rcar with R-Car in patch subject and description. > 7] Set MACCTLR in init() callback > > Changes for v6: > 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/ > scm/linux/kernel/git/lpieralisi/pci.git/ > 2] Fixed review comments from Shimoda-san > a] Made sure defconfig changes were in separate patch > b] Created rcar_pcie_host/rcar_pcie_ep structures > c] Added pci-id for R8A774C0 > d] Added entry in MAINTAINERS for dt-binding > e] Dropped unnecessary braces > 3] Added support for msi. > > Changes for v5: > 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/ > linux/kernel/git/helgaas/pci.git > 2] Fixed review comments reported by Kishon while fetching the matching > window in function pci_epc_get_matching_window() > 3] Fixed review comments reported by Bjorn > a] Split patch up first patch so that its easier to review and incremental > b] Fixed typos > 4] Included Reviewed tag from Rob for the dt-binding patch > 5] Fixed issue reported by Nathan for assigning variable to itself > > Changes for v4: > 1] Fixed dtb_check error reported by Rob > 2] Fixed review comments reported by Kishon > a] Dropped pci_epc_find_best_fit_window() > b] Fixed initializing mem ptr in __pci_epc_mem_init() > c] Dropped map_size from pci_epc_mem_window structure > > Changes for v3: > 1] Fixed review comments from Bjorn and Kishon. > 3] Converted to DT schema > > Changes for v2: > 1] Fixed review comments from Biju for dt-bindings to include an example > for a tested platform. > 2] Fixed review comments from Kishon to extend the features of outbound > regions in epf framework. > 3] Added support to parse outbound-ranges in OF. > > Lad Prabhakar (8): > PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c > PCI: rcar: Move shareable code to a common file > PCI: rcar: Fix calculating mask for PCIEPAMR register > PCI: endpoint: Pass page size as argument to pci_epc_mem_init() > PCI: endpoint: Add support to handle multiple base for mapping > outbound memory Could you please do the needy for the above two patches, so that this can be picked up by Lorenzo. Cheers, --Prabhakar > dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint > controller > PCI: rcar: Add endpoint mode support > MAINTAINERS: Add file patterns for rcar PCI device tree bindings > > .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++ > MAINTAINERS | 1 + > drivers/pci/controller/Kconfig | 18 + > drivers/pci/controller/Makefile | 3 +- > .../pci/controller/cadence/pcie-cadence-ep.c | 2 +- > .../pci/controller/dwc/pcie-designware-ep.c | 16 +- > drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++ > drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++ > drivers/pci/controller/pcie-rcar.c | 1206 +---------------- > drivers/pci/controller/pcie-rcar.h | 140 ++ > drivers/pci/controller/pcie-rockchip-ep.c | 2 +- > drivers/pci/endpoint/pci-epc-mem.c | 204 ++- > include/linux/pci-epc.h | 38 +- > 13 files changed, 2078 insertions(+), 1251 deletions(-) > create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml > create mode 100644 drivers/pci/controller/pcie-rcar-ep.c > create mode 100644 drivers/pci/controller/pcie-rcar-host.c > create mode 100644 drivers/pci/controller/pcie-rcar.h > > -- > 2.17.1 >
On Thu, Apr 30, 2020 at 09:43:20AM +0100, Lad, Prabhakar wrote: > Hi Kishon, > > On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > Hi All, > > > > This patch series adds support for endpoint driver for R-Car PCIe controller on > > R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows > > supported by the controller for mapping PCI address locally. > > > > Note: > > The cadence/rockchip/designware endpoint drivers are build tested only. > > > > Changes for v9 (Re-spun this series as there were minimal changes requested): > > * Rebased patches on top of v5.7.rc1 > > * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx() > > * Added a check for max_functions read from DT to restrict with > > RCAR_EPC_MAX_FUNCTIONS > > * Replaced MSICAP0_MMENUM with MSICAP0_MMESE > > * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr() > > * Fixed looping for number windows in pci_epc_mem_exit() > > * Set maximum to 1 for max-functions in DT binding (I have restored the acks > > from Rob and Shimoda-san) > > * Sorted the entry in MAINTAINERS > > > > Changes for v8: > > * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h > > * Fixed typo in commit message for patch 2/8 > > * Reworded commit message for patch 5/8 as suggested by Bjorn > > * Split up patch to add pci_epc_mem_init() interface to add page_size argument > > as suggested by Bjorn. > > > > Changes for v7: > > * Fixed review comments pointed by Shimoda-san > > 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed > > the example as its built with #{address,size}-cells = <1>. I have still > > restored the Ack from Rob and Shimoda-san with these changes. > > 2] Split up the patches so that they can be picked up by respective subsystem > > patches 1/4-9/11 are now part of this series. > > 3] Dropped altering a comment in pci-epc.h > > 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size > > variable doesn't get overwritten in the loop. > > 5] Replaced i-=1 with i-- > > 6] Replaced rcar with R-Car in patch subject and description. > > 7] Set MACCTLR in init() callback > > > > Changes for v6: > > 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/ > > scm/linux/kernel/git/lpieralisi/pci.git/ > > 2] Fixed review comments from Shimoda-san > > a] Made sure defconfig changes were in separate patch > > b] Created rcar_pcie_host/rcar_pcie_ep structures > > c] Added pci-id for R8A774C0 > > d] Added entry in MAINTAINERS for dt-binding > > e] Dropped unnecessary braces > > 3] Added support for msi. > > > > Changes for v5: > > 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/ > > linux/kernel/git/helgaas/pci.git > > 2] Fixed review comments reported by Kishon while fetching the matching > > window in function pci_epc_get_matching_window() > > 3] Fixed review comments reported by Bjorn > > a] Split patch up first patch so that its easier to review and incremental > > b] Fixed typos > > 4] Included Reviewed tag from Rob for the dt-binding patch > > 5] Fixed issue reported by Nathan for assigning variable to itself > > > > Changes for v4: > > 1] Fixed dtb_check error reported by Rob > > 2] Fixed review comments reported by Kishon > > a] Dropped pci_epc_find_best_fit_window() > > b] Fixed initializing mem ptr in __pci_epc_mem_init() > > c] Dropped map_size from pci_epc_mem_window structure > > > > Changes for v3: > > 1] Fixed review comments from Bjorn and Kishon. > > 3] Converted to DT schema > > > > Changes for v2: > > 1] Fixed review comments from Biju for dt-bindings to include an example > > for a tested platform. > > 2] Fixed review comments from Kishon to extend the features of outbound > > regions in epf framework. > > 3] Added support to parse outbound-ranges in OF. > > > > Lad Prabhakar (8): > > PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c > > PCI: rcar: Move shareable code to a common file > > PCI: rcar: Fix calculating mask for PCIEPAMR register > > PCI: endpoint: Pass page size as argument to pci_epc_mem_init() > > PCI: endpoint: Add support to handle multiple base for mapping > > outbound memory > Could you please do the needy for the above two patches, so that this > can be picked up by Lorenzo. Yes please. I would kindly ask you to rebase it on top of my pci/rcar branch - with Kishon's ACK when provided. Thanks, Lorenzo > Cheers, > --Prabhakar > > > dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint > > controller > > PCI: rcar: Add endpoint mode support > > MAINTAINERS: Add file patterns for rcar PCI device tree bindings > > > > .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++ > > MAINTAINERS | 1 + > > drivers/pci/controller/Kconfig | 18 + > > drivers/pci/controller/Makefile | 3 +- > > .../pci/controller/cadence/pcie-cadence-ep.c | 2 +- > > .../pci/controller/dwc/pcie-designware-ep.c | 16 +- > > drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++ > > drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++ > > drivers/pci/controller/pcie-rcar.c | 1206 +---------------- > > drivers/pci/controller/pcie-rcar.h | 140 ++ > > drivers/pci/controller/pcie-rockchip-ep.c | 2 +- > > drivers/pci/endpoint/pci-epc-mem.c | 204 ++- > > include/linux/pci-epc.h | 38 +- > > 13 files changed, 2078 insertions(+), 1251 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml > > create mode 100644 drivers/pci/controller/pcie-rcar-ep.c > > create mode 100644 drivers/pci/controller/pcie-rcar-host.c > > create mode 100644 drivers/pci/controller/pcie-rcar.h > > > > -- > > 2.17.1 > >
Hi Lorenzo, On Tue, May 5, 2020 at 10:44 AM Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > On Thu, Apr 30, 2020 at 09:43:20AM +0100, Lad, Prabhakar wrote: > > Hi Kishon, > > > > On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > > > Hi All, > > > > > > This patch series adds support for endpoint driver for R-Car PCIe controller on > > > R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows > > > supported by the controller for mapping PCI address locally. > > > > > > Note: > > > The cadence/rockchip/designware endpoint drivers are build tested only. > > > > > > Changes for v9 (Re-spun this series as there were minimal changes requested): > > > * Rebased patches on top of v5.7.rc1 > > > * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx() > > > * Added a check for max_functions read from DT to restrict with > > > RCAR_EPC_MAX_FUNCTIONS > > > * Replaced MSICAP0_MMENUM with MSICAP0_MMESE > > > * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr() > > > * Fixed looping for number windows in pci_epc_mem_exit() > > > * Set maximum to 1 for max-functions in DT binding (I have restored the acks > > > from Rob and Shimoda-san) > > > * Sorted the entry in MAINTAINERS > > > > > > Changes for v8: > > > * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h > > > * Fixed typo in commit message for patch 2/8 > > > * Reworded commit message for patch 5/8 as suggested by Bjorn > > > * Split up patch to add pci_epc_mem_init() interface to add page_size argument > > > as suggested by Bjorn. > > > > > > Changes for v7: > > > * Fixed review comments pointed by Shimoda-san > > > 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed > > > the example as its built with #{address,size}-cells = <1>. I have still > > > restored the Ack from Rob and Shimoda-san with these changes. > > > 2] Split up the patches so that they can be picked up by respective subsystem > > > patches 1/4-9/11 are now part of this series. > > > 3] Dropped altering a comment in pci-epc.h > > > 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size > > > variable doesn't get overwritten in the loop. > > > 5] Replaced i-=1 with i-- > > > 6] Replaced rcar with R-Car in patch subject and description. > > > 7] Set MACCTLR in init() callback > > > > > > Changes for v6: > > > 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/ > > > scm/linux/kernel/git/lpieralisi/pci.git/ > > > 2] Fixed review comments from Shimoda-san > > > a] Made sure defconfig changes were in separate patch > > > b] Created rcar_pcie_host/rcar_pcie_ep structures > > > c] Added pci-id for R8A774C0 > > > d] Added entry in MAINTAINERS for dt-binding > > > e] Dropped unnecessary braces > > > 3] Added support for msi. > > > > > > Changes for v5: > > > 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/ > > > linux/kernel/git/helgaas/pci.git > > > 2] Fixed review comments reported by Kishon while fetching the matching > > > window in function pci_epc_get_matching_window() > > > 3] Fixed review comments reported by Bjorn > > > a] Split patch up first patch so that its easier to review and incremental > > > b] Fixed typos > > > 4] Included Reviewed tag from Rob for the dt-binding patch > > > 5] Fixed issue reported by Nathan for assigning variable to itself > > > > > > Changes for v4: > > > 1] Fixed dtb_check error reported by Rob > > > 2] Fixed review comments reported by Kishon > > > a] Dropped pci_epc_find_best_fit_window() > > > b] Fixed initializing mem ptr in __pci_epc_mem_init() > > > c] Dropped map_size from pci_epc_mem_window structure > > > > > > Changes for v3: > > > 1] Fixed review comments from Bjorn and Kishon. > > > 3] Converted to DT schema > > > > > > Changes for v2: > > > 1] Fixed review comments from Biju for dt-bindings to include an example > > > for a tested platform. > > > 2] Fixed review comments from Kishon to extend the features of outbound > > > regions in epf framework. > > > 3] Added support to parse outbound-ranges in OF. > > > > > > Lad Prabhakar (8): > > > PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c > > > PCI: rcar: Move shareable code to a common file > > > PCI: rcar: Fix calculating mask for PCIEPAMR register > > > PCI: endpoint: Pass page size as argument to pci_epc_mem_init() > > > PCI: endpoint: Add support to handle multiple base for mapping > > > outbound memory > > Could you please do the needy for the above two patches, so that this > > can be picked up by Lorenzo. > > Yes please. I would kindly ask you to rebase it on top of my > pci/rcar branch - with Kishon's ACK when provided. > Sure will do that as soon as I get Kishon's Ack. Cheers, --Prabhakar > Thanks, > Lorenzo > > > Cheers, > > --Prabhakar > > > > > dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint > > > controller > > > PCI: rcar: Add endpoint mode support > > > MAINTAINERS: Add file patterns for rcar PCI device tree bindings > > > > > > .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++ > > > MAINTAINERS | 1 + > > > drivers/pci/controller/Kconfig | 18 + > > > drivers/pci/controller/Makefile | 3 +- > > > .../pci/controller/cadence/pcie-cadence-ep.c | 2 +- > > > .../pci/controller/dwc/pcie-designware-ep.c | 16 +- > > > drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++ > > > drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++ > > > drivers/pci/controller/pcie-rcar.c | 1206 +---------------- > > > drivers/pci/controller/pcie-rcar.h | 140 ++ > > > drivers/pci/controller/pcie-rockchip-ep.c | 2 +- > > > drivers/pci/endpoint/pci-epc-mem.c | 204 ++- > > > include/linux/pci-epc.h | 38 +- > > > 13 files changed, 2078 insertions(+), 1251 deletions(-) > > > create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml > > > create mode 100644 drivers/pci/controller/pcie-rcar-ep.c > > > create mode 100644 drivers/pci/controller/pcie-rcar-host.c > > > create mode 100644 drivers/pci/controller/pcie-rcar.h > > > > > > -- > > > 2.17.1 > > >
On 4/23/2020 11:52 PM, Lad Prabhakar wrote: > pci_epc_mem_init() internally used page size equal to *PAGE_SIZE* to > manage the address space so instead just pass the page size as a > argument to pci_epc_mem_init(). > > Also make pci_epc_mem_init() as a C function instead of a macro function > in preparation for adding support for pci-epc-mem core to handle multiple > windows. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Acked-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/controller/cadence/pcie-cadence-ep.c | 2 +- > drivers/pci/controller/pcie-rockchip-ep.c | 2 +- > drivers/pci/endpoint/pci-epc-mem.c | 7 +++++++ > include/linux/pci-epc.h | 5 ++--- > 4 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c > index 1c173dad67d1..1c15c8352125 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c > +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c > @@ -450,7 +450,7 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep) > epc->max_functions = 1; > > ret = pci_epc_mem_init(epc, pcie->mem_res->start, > - resource_size(pcie->mem_res)); > + resource_size(pcie->mem_res), PAGE_SIZE); > if (ret < 0) { > dev_err(dev, "failed to initialize the memory space\n"); > goto err_init; > diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c > index d743b0a48988..5eaf36629a75 100644 > --- a/drivers/pci/controller/pcie-rockchip-ep.c > +++ b/drivers/pci/controller/pcie-rockchip-ep.c > @@ -615,7 +615,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev) > rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG); > > err = pci_epc_mem_init(epc, rockchip->mem_res->start, > - resource_size(rockchip->mem_res)); > + resource_size(rockchip->mem_res), PAGE_SIZE); > if (err < 0) { > dev_err(dev, "failed to initialize the memory space\n"); > goto err_uninit_port; > diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c > index abfac1109a13..cdd1d3821249 100644 > --- a/drivers/pci/endpoint/pci-epc-mem.c > +++ b/drivers/pci/endpoint/pci-epc-mem.c > @@ -93,6 +93,13 @@ return ret; > } > EXPORT_SYMBOL_GPL(__pci_epc_mem_init); > > +int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base, > + size_t size, size_t page_size) > +{ > + return __pci_epc_mem_init(epc, base, size, page_size); > +} > +EXPORT_SYMBOL_GPL(pci_epc_mem_init); > + > /** > * pci_epc_mem_exit() - cleanup the pci_epc_mem structure > * @epc: the EPC device that invoked pci_epc_mem_exit > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > index e0ed9d01f6e5..5bc1de65849e 100644 > --- a/include/linux/pci-epc.h > +++ b/include/linux/pci-epc.h > @@ -137,9 +137,6 @@ struct pci_epc_features { > #define devm_pci_epc_create(dev, ops) \ > __devm_pci_epc_create((dev), (ops), THIS_MODULE) > > -#define pci_epc_mem_init(epc, phys_addr, size) \ > - __pci_epc_mem_init((epc), (phys_addr), (size), PAGE_SIZE) > - > static inline void epc_set_drvdata(struct pci_epc *epc, void *data) > { > dev_set_drvdata(&epc->dev, data); > @@ -195,6 +192,8 @@ unsigned int pci_epc_get_first_free_bar(const struct pci_epc_features > struct pci_epc *pci_epc_get(const char *epc_name); > void pci_epc_put(struct pci_epc *epc); > > +int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base, > + size_t size, size_t page_size); > int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_addr, size_t size, > size_t page_size); > void pci_epc_mem_exit(struct pci_epc *epc); >
On 4/23/2020 11:52 PM, Lad Prabhakar wrote: > 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() initializes address space for endpoint > controller which support single window and pci_epc_multi_mem_init() > initializes multiple windows supported by endpoint controller. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Acked-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > .../pci/controller/dwc/pcie-designware-ep.c | 16 +- > drivers/pci/endpoint/pci-epc-mem.c | 199 ++++++++++++------ > include/linux/pci-epc.h | 33 ++- > 3 files changed, 170 insertions(+), 78 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 1cdcbd102ce8..a78902cbf2f0 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -412,11 +412,11 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, > reg = ep->msi_cap + PCI_MSI_DATA_32; > msg_data = dw_pcie_readw_dbi(pci, reg); > } > - aligned_offset = msg_addr_lower & (epc->mem->page_size - 1); > + aligned_offset = msg_addr_lower & (epc->mem->window.page_size - 1); > msg_addr = ((u64)msg_addr_upper) << 32 | > (msg_addr_lower & ~aligned_offset); > ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr, > - epc->mem->page_size); > + epc->mem->window.page_size); > if (ret) > return ret; > > @@ -459,9 +459,9 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > return -EPERM; > } > > - aligned_offset = msg_addr & (epc->mem->page_size - 1); > + aligned_offset = msg_addr & (epc->mem->window.page_size - 1); > ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr, > - epc->mem->page_size); > + epc->mem->window.page_size); > if (ret) > return ret; > > @@ -477,7 +477,7 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep) > struct pci_epc *epc = ep->epc; > > pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem, > - epc->mem->page_size); > + epc->mem->window.page_size); > > pci_epc_mem_exit(epc); > } > @@ -610,15 +610,15 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > if (ret < 0) > epc->max_functions = 1; > > - ret = __pci_epc_mem_init(epc, ep->phys_base, ep->addr_size, > - ep->page_size); > + ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size, > + ep->page_size); > if (ret < 0) { > dev_err(dev, "Failed to initialize address space\n"); > return ret; > } > > ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys, > - epc->mem->page_size); > + epc->mem->window.page_size); > if (!ep->msi_mem) { > dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n"); > return -ENOMEM; > diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c > index cdd1d3821249..a3466da2a16f 100644 > --- a/drivers/pci/endpoint/pci-epc-mem.c > +++ b/drivers/pci/endpoint/pci-epc-mem.c > @@ -23,7 +23,7 @@ > static int pci_epc_mem_get_order(struct pci_epc_mem *mem, size_t size) > { > int order; > - unsigned int page_shift = ilog2(mem->page_size); > + unsigned int page_shift = ilog2(mem->window.page_size); > > size--; > size >>= page_shift; > @@ -36,67 +36,95 @@ 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 > + * pci_epc_multi_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_multi_mem_init(struct pci_epc *epc, > + struct pci_epc_mem_window *windows, > + unsigned 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->num_windows = 0; > > - page_shift = ilog2(page_size); > - pages = size >> page_shift; > - bitmap_size = BITS_TO_LONGS(pages) * sizeof(long); > + if (!windows || !num_windows) > + return -EINVAL; > > - mem = kzalloc(sizeof(*mem), GFP_KERNEL); > - if (!mem) { > - ret = -ENOMEM; > - goto err; > - } > + epc->windows = kcalloc(num_windows, sizeof(*mem), GFP_KERNEL); > + if (!epc->windows) > + return -ENOMEM; > > - bitmap = kzalloc(bitmap_size, GFP_KERNEL); > - if (!bitmap) { > - ret = -ENOMEM; > - goto err_mem; > - } > + 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->bitmap = bitmap; > - mem->phys_base = phys_base; > - mem->page_size = page_size; > - mem->pages = pages; > - mem->size = size; > - mutex_init(&mem->lock); > + mem = kzalloc(sizeof(*mem), GFP_KERNEL); > + if (!mem) { > + ret = -ENOMEM; > + i--; > + goto err_mem; > + } > > - epc->mem = mem; > + bitmap = kzalloc(bitmap_size, GFP_KERNEL); > + if (!bitmap) { > + ret = -ENOMEM; > + kfree(mem); > + i--; > + goto err_mem; > + } > + > + mem->window.phys_base = windows[i].phys_base; > + mem->window.size = windows[i].size; > + mem->window.page_size = page_size; > + mem->bitmap = bitmap; > + mem->pages = pages; > + mutex_init(&mem->lock); > + epc->windows[i] = mem; > + } > + > + epc->mem = epc->windows[0]; > + epc->num_windows = num_windows; > > return 0; > > err_mem: > - kfree(mem); > + for (; i >= 0; i--) { > + mem = epc->windows[i]; > + kfree(mem->bitmap); > + kfree(mem); > + } > + kfree(epc->windows); > > -err: > -return ret; > + return ret; > } > -EXPORT_SYMBOL_GPL(__pci_epc_mem_init); > +EXPORT_SYMBOL_GPL(pci_epc_multi_mem_init); > > int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base, > size_t size, size_t page_size) > { > - return __pci_epc_mem_init(epc, base, size, page_size); > + struct pci_epc_mem_window mem_window; > + > + mem_window.phys_base = base; > + mem_window.size = size; > + mem_window.page_size = page_size; > + > + return pci_epc_multi_mem_init(epc, &mem_window, 1); > } > EXPORT_SYMBOL_GPL(pci_epc_mem_init); > > @@ -109,11 +137,22 @@ 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->num_windows) > + return; > + > + for (i = 0; i < epc->num_windows; i++) { > + mem = epc->windows[i]; > + kfree(mem->bitmap); > + kfree(mem); > + } > + kfree(epc->windows); > + > + epc->windows = NULL; > epc->mem = NULL; > - kfree(mem->bitmap); > - kfree(mem); > + epc->num_windows = 0; > } > EXPORT_SYMBOL_GPL(pci_epc_mem_exit); > > @@ -129,31 +168,60 @@ 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 = NULL; > - struct pci_epc_mem *mem = epc->mem; > - unsigned int page_shift = ilog2(mem->page_size); > + struct pci_epc_mem *mem; > + unsigned int page_shift; > + size_t align_size; > + int pageno; > int order; > + int i; > > - size = ALIGN(size, mem->page_size); > - order = pci_epc_mem_get_order(mem, size); > - > - mutex_lock(&mem->lock); > - pageno = bitmap_find_free_region(mem->bitmap, mem->pages, order); > - if (pageno < 0) > - goto ret; > + for (i = 0; i < epc->num_windows; i++) { > + mem = epc->windows[i]; > + mutex_lock(&mem->lock); > + align_size = ALIGN(size, mem->window.page_size); > + order = pci_epc_mem_get_order(mem, align_size); > > - *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift); > - virt_addr = ioremap(*phys_addr, size); > - if (!virt_addr) > - bitmap_release_region(mem->bitmap, pageno, order); > + pageno = bitmap_find_free_region(mem->bitmap, mem->pages, > + order); > + if (pageno >= 0) { > + page_shift = ilog2(mem->window.page_size); > + *phys_addr = mem->window.phys_base + > + ((phys_addr_t)pageno << page_shift); > + virt_addr = ioremap(*phys_addr, align_size); > + if (!virt_addr) { > + bitmap_release_region(mem->bitmap, > + pageno, order); > + mutex_unlock(&mem->lock); > + continue; > + } > + mutex_unlock(&mem->lock); > + return virt_addr; > + } > + mutex_unlock(&mem->lock); > + } > > -ret: > - mutex_unlock(&mem->lock); > return virt_addr; > } > 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->num_windows; i++) { > + mem = epc->windows[i]; > + > + if (phys_addr >= mem->window.phys_base && > + phys_addr < (mem->window.phys_base + mem->window.size)) > + return mem; > + } > + > + return NULL; > +} > + > /** > * pci_epc_mem_free_addr() - free the allocated memory address > * @epc: the EPC device on which memory was allocated > @@ -166,14 +234,23 @@ EXPORT_SYMBOL_GPL(pci_epc_mem_alloc_addr); > void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr, > void __iomem *virt_addr, size_t size) > { > + struct pci_epc_mem *mem; > + unsigned int page_shift; > + size_t page_size; > int pageno; > - struct pci_epc_mem *mem = epc->mem; > - unsigned int page_shift = ilog2(mem->page_size); > int order; > > + mem = pci_epc_get_matching_window(epc, phys_addr); > + if (!mem) { > + pr_err("failed to get matching window\n"); > + return; > + } > + > + page_size = mem->window.page_size; > + page_shift = ilog2(page_size); > iounmap(virt_addr); > - pageno = (phys_addr - mem->phys_base) >> page_shift; > - size = ALIGN(size, mem->page_size); > + pageno = (phys_addr - mem->window.phys_base) >> page_shift; > + size = ALIGN(size, page_size); > order = pci_epc_mem_get_order(mem, size); > mutex_lock(&mem->lock); > bitmap_release_region(mem->bitmap, pageno, order); > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > index 5bc1de65849e..cc66bec8be90 100644 > --- a/include/linux/pci-epc.h > +++ b/include/linux/pci-epc.h > @@ -65,20 +65,28 @@ struct pci_epc_ops { > struct module *owner; > }; > > +/** > + * struct pci_epc_mem_window - address window of the endpoint controller > + * @phys_base: physical base address of the PCI address window > + * @size: the size of the PCI address window > + * @page_size: size of each page > + */ > +struct pci_epc_mem_window { > + phys_addr_t phys_base; > + size_t size; > + size_t page_size; > +}; > + > /** > * struct pci_epc_mem - address space of the endpoint controller > - * @phys_base: physical base address of the PCI address space > - * @size: the size of the PCI address space > + * @window: address window of the endpoint controller > * @bitmap: bitmap to manage the PCI address space > * @pages: number of bits representing the address region > - * @page_size: size of each page > * @lock: mutex to protect bitmap > */ > struct pci_epc_mem { > - phys_addr_t phys_base; > - size_t size; > + struct pci_epc_mem_window window; > unsigned long *bitmap; > - size_t page_size; > int pages; > /* mutex to protect against concurrent access for memory allocation*/ > struct mutex lock; > @@ -89,7 +97,11 @@ struct pci_epc_mem { > * @dev: PCI EPC device > * @pci_epf: list of endpoint functions present in this EPC device > * @ops: function pointers for performing endpoint operations > - * @mem: address space of the endpoint controller > + * @windows: array of address space of the endpoint controller > + * @mem: first window of the endpoint controller, which corresponds to > + * default address space of the endpoint controller supporting > + * single window. > + * @num_windows: number of windows supported by device > * @max_functions: max number of functions that can be configured in this EPC > * @group: configfs group representing the PCI EPC device > * @lock: mutex to protect pci_epc ops > @@ -100,7 +112,9 @@ struct pci_epc { > struct device dev; > struct list_head pci_epf; > const struct pci_epc_ops *ops; > + struct pci_epc_mem **windows; > struct pci_epc_mem *mem; > + unsigned int num_windows; > u8 max_functions; > struct config_group *group; > /* mutex to protect against concurrent access of EP controller */ > @@ -194,8 +208,9 @@ void pci_epc_put(struct pci_epc *epc); > > int pci_epc_mem_init(struct pci_epc *epc, phys_addr_t base, > size_t size, size_t page_size); > -int __pci_epc_mem_init(struct pci_epc *epc, phys_addr_t phys_addr, size_t size, > - size_t page_size); > +int pci_epc_multi_mem_init(struct pci_epc *epc, > + struct pci_epc_mem_window *window, > + unsigned int num_windows); > void pci_epc_mem_exit(struct pci_epc *epc); > void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc, > phys_addr_t *phys_addr, size_t size); >
Hi Prabhakar, On 5/5/2020 3:17 PM, Lad, Prabhakar wrote: > Hi Lorenzo, > > On Tue, May 5, 2020 at 10:44 AM Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: >> >> On Thu, Apr 30, 2020 at 09:43:20AM +0100, Lad, Prabhakar wrote: >>> Hi Kishon, >>> >>> On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar >>> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: >>>> >>>> Hi All, >>>> >>>> This patch series adds support for endpoint driver for R-Car PCIe controller on >>>> R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows >>>> supported by the controller for mapping PCI address locally. >>>> >>>> Note: >>>> The cadence/rockchip/designware endpoint drivers are build tested only. >>>> >>>> Changes for v9 (Re-spun this series as there were minimal changes requested): >>>> * Rebased patches on top of v5.7.rc1 >>>> * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx() >>>> * Added a check for max_functions read from DT to restrict with >>>> RCAR_EPC_MAX_FUNCTIONS >>>> * Replaced MSICAP0_MMENUM with MSICAP0_MMESE >>>> * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr() >>>> * Fixed looping for number windows in pci_epc_mem_exit() >>>> * Set maximum to 1 for max-functions in DT binding (I have restored the acks >>>> from Rob and Shimoda-san) >>>> * Sorted the entry in MAINTAINERS >>>> >>>> Changes for v8: >>>> * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h >>>> * Fixed typo in commit message for patch 2/8 >>>> * Reworded commit message for patch 5/8 as suggested by Bjorn >>>> * Split up patch to add pci_epc_mem_init() interface to add page_size argument >>>> as suggested by Bjorn. >>>> >>>> Changes for v7: >>>> * Fixed review comments pointed by Shimoda-san >>>> 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed >>>> the example as its built with #{address,size}-cells = <1>. I have still >>>> restored the Ack from Rob and Shimoda-san with these changes. >>>> 2] Split up the patches so that they can be picked up by respective subsystem >>>> patches 1/4-9/11 are now part of this series. >>>> 3] Dropped altering a comment in pci-epc.h >>>> 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size >>>> variable doesn't get overwritten in the loop. >>>> 5] Replaced i-=1 with i-- >>>> 6] Replaced rcar with R-Car in patch subject and description. >>>> 7] Set MACCTLR in init() callback >>>> >>>> Changes for v6: >>>> 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/ >>>> scm/linux/kernel/git/lpieralisi/pci.git/ >>>> 2] Fixed review comments from Shimoda-san >>>> a] Made sure defconfig changes were in separate patch >>>> b] Created rcar_pcie_host/rcar_pcie_ep structures >>>> c] Added pci-id for R8A774C0 >>>> d] Added entry in MAINTAINERS for dt-binding >>>> e] Dropped unnecessary braces >>>> 3] Added support for msi. >>>> >>>> Changes for v5: >>>> 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/ >>>> linux/kernel/git/helgaas/pci.git >>>> 2] Fixed review comments reported by Kishon while fetching the matching >>>> window in function pci_epc_get_matching_window() >>>> 3] Fixed review comments reported by Bjorn >>>> a] Split patch up first patch so that its easier to review and incremental >>>> b] Fixed typos >>>> 4] Included Reviewed tag from Rob for the dt-binding patch >>>> 5] Fixed issue reported by Nathan for assigning variable to itself >>>> >>>> Changes for v4: >>>> 1] Fixed dtb_check error reported by Rob >>>> 2] Fixed review comments reported by Kishon >>>> a] Dropped pci_epc_find_best_fit_window() >>>> b] Fixed initializing mem ptr in __pci_epc_mem_init() >>>> c] Dropped map_size from pci_epc_mem_window structure >>>> >>>> Changes for v3: >>>> 1] Fixed review comments from Bjorn and Kishon. >>>> 3] Converted to DT schema >>>> >>>> Changes for v2: >>>> 1] Fixed review comments from Biju for dt-bindings to include an example >>>> for a tested platform. >>>> 2] Fixed review comments from Kishon to extend the features of outbound >>>> regions in epf framework. >>>> 3] Added support to parse outbound-ranges in OF. >>>> >>>> Lad Prabhakar (8): >>>> PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c >>>> PCI: rcar: Move shareable code to a common file >>>> PCI: rcar: Fix calculating mask for PCIEPAMR register >>>> PCI: endpoint: Pass page size as argument to pci_epc_mem_init() >>>> PCI: endpoint: Add support to handle multiple base for mapping >>>> outbound memory >>> Could you please do the needy for the above two patches, so that this >>> can be picked up by Lorenzo. >> >> Yes please. I would kindly ask you to rebase it on top of my >> pci/rcar branch - with Kishon's ACK when provided. >> > Sure will do that as soon as I get Kishon's Ack. I've given my Acked by on the two endpoint core patches. I've also tested my endpoint series [1] after applying this series. Thanks Kishon [1] -> http://lore.kernel.org/r/20200506151429.12255-1-kishon@ti.com > > Cheers, > --Prabhakar > >> Thanks, >> Lorenzo >> >>> Cheers, >>> --Prabhakar >>> >>>> dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint >>>> controller >>>> PCI: rcar: Add endpoint mode support >>>> MAINTAINERS: Add file patterns for rcar PCI device tree bindings >>>> >>>> .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++ >>>> MAINTAINERS | 1 + >>>> drivers/pci/controller/Kconfig | 18 + >>>> drivers/pci/controller/Makefile | 3 +- >>>> .../pci/controller/cadence/pcie-cadence-ep.c | 2 +- >>>> .../pci/controller/dwc/pcie-designware-ep.c | 16 +- >>>> drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++ >>>> drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++ >>>> drivers/pci/controller/pcie-rcar.c | 1206 +---------------- >>>> drivers/pci/controller/pcie-rcar.h | 140 ++ >>>> drivers/pci/controller/pcie-rockchip-ep.c | 2 +- >>>> drivers/pci/endpoint/pci-epc-mem.c | 204 ++- >>>> include/linux/pci-epc.h | 38 +- >>>> 13 files changed, 2078 insertions(+), 1251 deletions(-) >>>> create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml >>>> create mode 100644 drivers/pci/controller/pcie-rcar-ep.c >>>> create mode 100644 drivers/pci/controller/pcie-rcar-host.c >>>> create mode 100644 drivers/pci/controller/pcie-rcar.h >>>> >>>> -- >>>> 2.17.1 >>>>
Hi Kishon, On Thu, May 7, 2020 at 4:13 AM Kishon Vijay Abraham I <kishon@ti.com> wrote: > > Hi Prabhakar, > > On 5/5/2020 3:17 PM, Lad, Prabhakar wrote: > > Hi Lorenzo, > > > > On Tue, May 5, 2020 at 10:44 AM Lorenzo Pieralisi > > <lorenzo.pieralisi@arm.com> wrote: > >> > >> On Thu, Apr 30, 2020 at 09:43:20AM +0100, Lad, Prabhakar wrote: > >>> Hi Kishon, > >>> > >>> On Thu, Apr 23, 2020 at 7:23 PM Lad Prabhakar > >>> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > >>>> > >>>> Hi All, > >>>> > >>>> This patch series adds support for endpoint driver for R-Car PCIe controller on > >>>> R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows > >>>> supported by the controller for mapping PCI address locally. > >>>> > >>>> Note: > >>>> The cadence/rockchip/designware endpoint drivers are build tested only. > >>>> > >>>> Changes for v9 (Re-spun this series as there were minimal changes requested): > >>>> * Rebased patches on top of v5.7.rc1 > >>>> * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx() > >>>> * Added a check for max_functions read from DT to restrict with > >>>> RCAR_EPC_MAX_FUNCTIONS > >>>> * Replaced MSICAP0_MMENUM with MSICAP0_MMESE > >>>> * Retry ioremap for other windows on failure in pci_epc_mem_alloc_addr() > >>>> * Fixed looping for number windows in pci_epc_mem_exit() > >>>> * Set maximum to 1 for max-functions in DT binding (I have restored the acks > >>>> from Rob and Shimoda-san) > >>>> * Sorted the entry in MAINTAINERS > >>>> > >>>> Changes for v8: > >>>> * Dropped adding R8A774C0 (0x002d) pci-id in pci_ids.h > >>>> * Fixed typo in commit message for patch 2/8 > >>>> * Reworded commit message for patch 5/8 as suggested by Bjorn > >>>> * Split up patch to add pci_epc_mem_init() interface to add page_size argument > >>>> as suggested by Bjorn. > >>>> > >>>> Changes for v7: > >>>> * Fixed review comments pointed by Shimoda-san > >>>> 1] Made DT bindings dual licensed, added Shimoda-san as maintainer and fixed > >>>> the example as its built with #{address,size}-cells = <1>. I have still > >>>> restored the Ack from Rob and Shimoda-san with these changes. > >>>> 2] Split up the patches so that they can be picked up by respective subsystem > >>>> patches 1/4-9/11 are now part of this series. > >>>> 3] Dropped altering a comment in pci-epc.h > >>>> 4] Used a local variable align_size in pci_epc_mem_alloc_addr() so that size > >>>> variable doesn't get overwritten in the loop. > >>>> 5] Replaced i-=1 with i-- > >>>> 6] Replaced rcar with R-Car in patch subject and description. > >>>> 7] Set MACCTLR in init() callback > >>>> > >>>> Changes for v6: > >>>> 1] Rebased patches on endpoint branch of https://git.kernel.org/pub/ > >>>> scm/linux/kernel/git/lpieralisi/pci.git/ > >>>> 2] Fixed review comments from Shimoda-san > >>>> a] Made sure defconfig changes were in separate patch > >>>> b] Created rcar_pcie_host/rcar_pcie_ep structures > >>>> c] Added pci-id for R8A774C0 > >>>> d] Added entry in MAINTAINERS for dt-binding > >>>> e] Dropped unnecessary braces > >>>> 3] Added support for msi. > >>>> > >>>> Changes for v5: > >>>> 1] Rebased patches on next branch of https://git.kernel.org/pub/scm/ > >>>> linux/kernel/git/helgaas/pci.git > >>>> 2] Fixed review comments reported by Kishon while fetching the matching > >>>> window in function pci_epc_get_matching_window() > >>>> 3] Fixed review comments reported by Bjorn > >>>> a] Split patch up first patch so that its easier to review and incremental > >>>> b] Fixed typos > >>>> 4] Included Reviewed tag from Rob for the dt-binding patch > >>>> 5] Fixed issue reported by Nathan for assigning variable to itself > >>>> > >>>> Changes for v4: > >>>> 1] Fixed dtb_check error reported by Rob > >>>> 2] Fixed review comments reported by Kishon > >>>> a] Dropped pci_epc_find_best_fit_window() > >>>> b] Fixed initializing mem ptr in __pci_epc_mem_init() > >>>> c] Dropped map_size from pci_epc_mem_window structure > >>>> > >>>> Changes for v3: > >>>> 1] Fixed review comments from Bjorn and Kishon. > >>>> 3] Converted to DT schema > >>>> > >>>> Changes for v2: > >>>> 1] Fixed review comments from Biju for dt-bindings to include an example > >>>> for a tested platform. > >>>> 2] Fixed review comments from Kishon to extend the features of outbound > >>>> regions in epf framework. > >>>> 3] Added support to parse outbound-ranges in OF. > >>>> > >>>> Lad Prabhakar (8): > >>>> PCI: rcar: Rename pcie-rcar.c to pcie-rcar-host.c > >>>> PCI: rcar: Move shareable code to a common file > >>>> PCI: rcar: Fix calculating mask for PCIEPAMR register > >>>> PCI: endpoint: Pass page size as argument to pci_epc_mem_init() > >>>> PCI: endpoint: Add support to handle multiple base for mapping > >>>> outbound memory > >>> Could you please do the needy for the above two patches, so that this > >>> can be picked up by Lorenzo. > >> > >> Yes please. I would kindly ask you to rebase it on top of my > >> pci/rcar branch - with Kishon's ACK when provided. > >> > > Sure will do that as soon as I get Kishon's Ack. > > I've given my Acked by on the two endpoint core patches. I've also tested my > endpoint series [1] after applying this series. > Thank you for testing and the Ack's. Cheers, --Prabhakar > Thanks > Kishon > > [1] -> http://lore.kernel.org/r/20200506151429.12255-1-kishon@ti.com > > > > Cheers, > > --Prabhakar > > > >> Thanks, > >> Lorenzo > >> > >>> Cheers, > >>> --Prabhakar > >>> > >>>> dt-bindings: PCI: rcar: Add bindings for R-Car PCIe endpoint > >>>> controller > >>>> PCI: rcar: Add endpoint mode support > >>>> MAINTAINERS: Add file patterns for rcar PCI device tree bindings > >>>> > >>>> .../devicetree/bindings/pci/rcar-pci-ep.yaml | 77 ++ > >>>> MAINTAINERS | 1 + > >>>> drivers/pci/controller/Kconfig | 18 + > >>>> drivers/pci/controller/Makefile | 3 +- > >>>> .../pci/controller/cadence/pcie-cadence-ep.c | 2 +- > >>>> .../pci/controller/dwc/pcie-designware-ep.c | 16 +- > >>>> drivers/pci/controller/pcie-rcar-ep.c | 557 ++++++++ > >>>> drivers/pci/controller/pcie-rcar-host.c | 1065 +++++++++++++++ > >>>> drivers/pci/controller/pcie-rcar.c | 1206 +---------------- > >>>> drivers/pci/controller/pcie-rcar.h | 140 ++ > >>>> drivers/pci/controller/pcie-rockchip-ep.c | 2 +- > >>>> drivers/pci/endpoint/pci-epc-mem.c | 204 ++- > >>>> include/linux/pci-epc.h | 38 +- > >>>> 13 files changed, 2078 insertions(+), 1251 deletions(-) > >>>> create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci-ep.yaml > >>>> create mode 100644 drivers/pci/controller/pcie-rcar-ep.c > >>>> create mode 100644 drivers/pci/controller/pcie-rcar-host.c > >>>> create mode 100644 drivers/pci/controller/pcie-rcar.h > >>>> > >>>> -- > >>>> 2.17.1 > >>>>
Hi! > R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows > supported by the controller for mapping PCI address locally. > > Note: > The cadence/rockchip/designware endpoint drivers are build tested only. > > Changes for v9 (Re-spun this series as there were minimal changes requested): ... > * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx() Are you sure that is good idea? You are requesting 1ms sleep time with 1us tolerance, I dont believe common systems can do that. Best regards, Pavel
Hi Pavel, On Thu, May 7, 2020 at 6:44 PM Pavel Machek <pavel@ucw.cz> wrote: > > Hi! > > > > R-Car/RZ-G2x SoC's, this also extends the epf framework to handle multiple windows > > supported by the controller for mapping PCI address locally. > > > > Note: > > The cadence/rockchip/designware endpoint drivers are build tested only. > > > > Changes for v9 (Re-spun this series as there were minimal changes requested): > ... > > * Replaced mdelay(1) with usleep_range(1000, 1001) in rcar_pcie_ep_assert_intx() > > Are you sure that is good idea? You are requesting 1ms sleep time with 1us tolerance, > I dont believe common systems can do that. > Agreed the systems cannot do that, but the main reason of replacing mdelay(1) with usleep_range(1000, 1001) was since pci_epc_raise_irq() calls mutex_lock() and then this function rcar_pcie_ep_assert_intx(), so we can assume this function also can sleep. And, according to Documentation/timers/timers-howto.rst, we should use usleep_range() instead of mdelay(). Cheers, --Prabhakar > Best regards, > Pavel > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html