Hi Bjorn, Thank you for the review. On Thu, Apr 9, 2020 at 12:46 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > On Wed, Apr 08, 2020 at 04:37:56PM +0100, 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 whereas __pci_epc_mem_init() > > now accepts pointer to multiple windows supported by endpoint controller. > > Adding a double underscore prefix usually indicates an internal > function that skips some checking. > > It doesn't seem like quite the right thing for this external interface > that adds functionality. Maybe the name could include something like > "multi"? > Agreed. how about pci_epc_mem_multi_init() ? > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > This needs an ack from Kishon, of course. > Yes waiting for Kishon to review it. > This patch seems like it does several things that could possibly be > split into separate patches? > > - Change pci_epc_mem_init() interface to add page_size argument (the > only one that touches cadence & rockchip; it would be nice if this > were a tiny patch) > Can be done. > - Add struct pci_epc_mem_window > > - Add a pci_epc_multi_mem_init() or similar, implement > pci_epc_mem_init() in terms of it (as you already do) > The above two needs to be single patch. Is that OK with you ? Cheers, --Prabhakar
