[RFC PATCH 0/3] prerequisites for device reserved local mem rework

[RFC PATCH 0/3] prerequisites for device reserved local mem rework

[laurentiu.tudor]

From: Laurentiu Tudor <laurentiu.tudor@nxp.com>

For HCs that have local memory, replace the current DMA API usage
with a genalloc generic allocator to manage the mappings for these
devices.
This is in preparation for dropping the existing "coherent" dma
mem declaration APIs. Current implementation was relying on a short
circuit in the DMA API that in the end, was acting as an allocator
for these type of devices.

Only compiled tested, so any volunteers willing to test are most welcome.

Thank you!

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Laurentiu Tudor (3):
  ohci-hcd: use genalloc for USB HCs with local memory
  usb: host: ohci-sm501: init genalloc for local memory
  usb: host: ohci-tmio: init genalloc for local memory

 drivers/usb/host/ohci-hcd.c   | 21 +++++++++---
 drivers/usb/host/ohci-sm501.c | 63 +++++++++++++++++++----------------
 drivers/usb/host/ohci-tmio.c  | 23 ++++++++-----
 drivers/usb/host/ohci.h       |  3 ++
 4 files changed, 69 insertions(+), 41 deletions(-)

-- 
2.17.1

[RFC PATCH 1/3] ohci-hcd: use genalloc for USB HCs with local memory

[laurentiu.tudor]

From: Laurentiu Tudor <laurentiu.tudor@nxp.com>

For HCs that have local memory, replace the current DMA API usage
with a genalloc generic allocator to manage the mappings for these
devices.
This is in preparation for dropping the existing "coherent" dma
mem declaration APIs. Current implementation was relying on a short
circuit in the DMA API that in the end, was acting as an allocator
for these type of devices.

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 drivers/usb/host/ohci-hcd.c | 23 ++++++++++++++++++-----
 drivers/usb/host/ohci.h     |  3 +++
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 210181fd98d2..c739d55901b3 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -40,6 +40,7 @@
 #include <linux/dmapool.h>
 #include <linux/workqueue.h>
 #include <linux/debugfs.h>
+#include <linux/genalloc.h>
 
 #include <asm/io.h>
 #include <asm/irq.h>
@@ -505,8 +506,15 @@ static int ohci_init (struct ohci_hcd *ohci)
 	timer_setup(&ohci->io_watchdog, io_watchdog_func, 0);
 	ohci->prev_frame_no = IO_WATCHDOG_OFF;
 
-	ohci->hcca = dma_alloc_coherent (hcd->self.controller,
-			sizeof(*ohci->hcca), &ohci->hcca_dma, GFP_KERNEL);
+	if (hcd->driver->flags & HCD_LOCAL_MEM)
+		ohci->hcca = gen_pool_dma_alloc(ohci->localmem_pool,
+						sizeof(*ohci->hcca),
+						&ohci->hcca_dma);
+	else
+		ohci->hcca = dma_alloc_coherent(hcd->self.controller,
+						sizeof(*ohci->hcca),
+						&ohci->hcca_dma,
+						GFP_KERNEL);
 	if (!ohci->hcca)
 		return -ENOMEM;
 
@@ -990,9 +998,14 @@ static void ohci_stop (struct usb_hcd *hcd)
 	remove_debug_files (ohci);
 	ohci_mem_cleanup (ohci);
 	if (ohci->hcca) {
-		dma_free_coherent (hcd->self.controller,
-				sizeof *ohci->hcca,
-				ohci->hcca, ohci->hcca_dma);
+		if (hcd->driver->flags & HCD_LOCAL_MEM)
+			gen_pool_free(ohci->localmem_pool,
+				      (unsigned long)ohci->hcca,
+				      sizeof(*ohci->hcca));
+		else
+			dma_free_coherent(hcd->self.controller,
+					  sizeof(*ohci->hcca),
+					  ohci->hcca, ohci->hcca_dma);
 		ohci->hcca = NULL;
 		ohci->hcca_dma = 0;
 	}
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index ef4813bfc5bf..cd196172c6fd 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -367,6 +367,9 @@ struct ohci_hcd {
 	 */
 	struct ohci_regs __iomem *regs;
 
+	/* allocator for HCs having local memory */
+	struct gen_pool         *localmem_pool;
+
 	/*
 	 * main memory used to communicate with the HC (dma-consistent).
 	 * hcd adds to schedule for a live hc any time, but removals finish
-- 
2.17.1

[RFC PATCH 2/3] usb: host: ohci-sm501: init genalloc for local memory

[laurentiu.tudor]

From: Laurentiu Tudor <laurentiu.tudor@nxp.com>

In preparation for dropping the existing "coherent" dma mem declaration
APIs, replace the current dma_declare_coherent_memory() based mechanism
with the creation of a genalloc pool that will be used in the OHCI
subsystem as replacement for the DMA APIs.

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 drivers/usb/host/ohci-sm501.c | 65 ++++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/host/ohci-sm501.c b/drivers/usb/host/ohci-sm501.c
index c26228c25f99..4aa18bfc5f2a 100644
--- a/drivers/usb/host/ohci-sm501.c
+++ b/drivers/usb/host/ohci-sm501.c
@@ -16,6 +16,7 @@
 #include <linux/jiffies.h>
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
+#include <linux/genalloc.h>
 #include <linux/sm501.h>
 #include <linux/sm501-regs.h>
 
@@ -92,6 +93,7 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device *pdev)
 	struct resource	*res, *mem;
 	int retval, irq;
 	struct usb_hcd *hcd = NULL;
+	struct ohci_hcd *ohci = NULL;
 
 	irq = retval = platform_get_irq(pdev, 0);
 	if (retval < 0)
@@ -110,40 +112,18 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device *pdev)
 		goto err0;
 	}
 
-	/* The sm501 chip is equipped with local memory that may be used
-	 * by on-chip devices such as the video controller and the usb host.
-	 * This driver uses dma_declare_coherent_memory() to make sure
-	 * usb allocations with dma_alloc_coherent() allocate from
-	 * this local memory. The dma_handle returned by dma_alloc_coherent()
-	 * will be an offset starting from 0 for the first local memory byte.
-	 *
-	 * So as long as data is allocated using dma_alloc_coherent() all is
-	 * fine. This is however not always the case - buffers may be allocated
-	 * using kmalloc() - so the usb core needs to be told that it must copy
-	 * data into our local memory if the buffers happen to be placed in
-	 * regular memory. The HCD_LOCAL_MEM flag does just that.
-	 */
-
-	retval = dma_declare_coherent_memory(dev, mem->start,
-					 mem->start - mem->parent->start,
-					 resource_size(mem));
-	if (retval) {
-		dev_err(dev, "cannot declare coherent memory\n");
-		goto err1;
-	}
-
 	/* allocate, reserve and remap resources for registers */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL) {
 		dev_err(dev, "no resource definition for registers\n");
 		retval = -ENOENT;
-		goto err2;
+		goto err1;
 	}
 
 	hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
 	if (!hcd) {
 		retval = -ENOMEM;
-		goto err2;
+		goto err1;
 	}
 
 	hcd->rsrc_start = res->start;
@@ -162,8 +142,40 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device *pdev)
 		goto err4;
 	}
 
-	ohci_hcd_init(hcd_to_ohci(hcd));
 
+	ohci = hcd_to_ohci(hcd);
+	ohci_hcd_init(ohci);
+
+	/* The sm501 chip is equipped with local memory that may be used
+	 * by on-chip devices such as the video controller and the usb host.
+	 * This driver uses genalloc so that usb allocations with
+	 * gen_pool_dma_alloc() allocate from this local memory. The dma_handle
+	 * returned by gen_pool_dma_alloc() will be an offset starting from 0
+	 * for the first local memory byte.
+	 *
+	 * So as long as data is allocated using gen_pool_dma_alloc() all is
+	 * fine. This is however not always the case - buffers may be allocated
+	 * using kmalloc() - so the usb core needs to be told that it must copy
+	 * data into our local memory if the buffers happen to be placed in
+	 * regular memory. The HCD_LOCAL_MEM flag does just that.
+	 */
+
+	ohci->localmem_pool = devm_gen_pool_create(dev, PAGE_SHIFT,
+						   dev_to_node(dev),
+						   "ohci-sm501");
+	if (IS_ERR(ohci->localmem_pool)) {
+		retval = PTR_ERR(ohci->localmem_pool);
+		goto err5;
+	}
+	retval = gen_pool_add_virt(ohci->localmem_pool,
+				   (unsigned long)mem->start -
+					mem->parent->start,
+				   mem->start, resource_size(mem),
+				   dev_to_node(dev));
+	if (retval < 0) {
+		dev_err(dev, "failed to add to pool: %d\n", retval);
+		goto err5;
+	}
 	retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (retval)
 		goto err5;
@@ -181,8 +193,6 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device *pdev)
 	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 err3:
 	usb_put_hcd(hcd);
-err2:
-	dma_release_declared_memory(dev);
 err1:
 	release_mem_region(mem->start, resource_size(mem));
 err0:
@@ -197,7 +207,6 @@ static int ohci_hcd_sm501_drv_remove(struct platform_device *pdev)
 	usb_remove_hcd(hcd);
 	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 	usb_put_hcd(hcd);
-	dma_release_declared_memory(&pdev->dev);
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	if (mem)
 		release_mem_region(mem->start, resource_size(mem));
-- 
2.17.1

[RFC PATCH 3/3] usb: host: ohci-tmio: init genalloc for local memory

[laurentiu.tudor]

From: Laurentiu Tudor <laurentiu.tudor@nxp.com>

In preparation for dropping the existing "coherent" dma mem declaration
APIs, replace the current dma_declare_coherent_memory() based mechanism
with the creation of a genalloc pool that will be used in the OHCI
subsystem as replacement for the DMA APIs.

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 drivers/usb/host/ohci-tmio.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ohci-tmio.c b/drivers/usb/host/ohci-tmio.c
index f88a0370659f..16ff99b56fd3 100644
--- a/drivers/usb/host/ohci-tmio.c
+++ b/drivers/usb/host/ohci-tmio.c
@@ -30,6 +30,7 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/tmio.h>
 #include <linux/dma-mapping.h>
+#include <linux/genalloc.h>
 
 /*-------------------------------------------------------------------------*/
 
@@ -224,11 +225,6 @@ static int ohci_hcd_tmio_drv_probe(struct platform_device *dev)
 		goto err_ioremap_regs;
 	}
 
-	ret = dma_declare_coherent_memory(&dev->dev, sram->start, sram->start,
-				resource_size(sram));
-	if (ret)
-		goto err_dma_declare;
-
 	if (cell->enable) {
 		ret = cell->enable(dev);
 		if (ret)
@@ -239,6 +235,20 @@ static int ohci_hcd_tmio_drv_probe(struct platform_device *dev)
 	ohci = hcd_to_ohci(hcd);
 	ohci_hcd_init(ohci);
 
+	ohci->localmem_pool = devm_gen_pool_create(&dev->dev, PAGE_SHIFT,
+						   dev_to_node(&dev->dev),
+						   "ohci-sm501");
+	if (IS_ERR(ohci->localmem_pool)) {
+		ret = PTR_ERR(ohci->localmem_pool);
+		goto err_enable;
+	}
+	ret = gen_pool_add_virt(ohci->localmem_pool, sram->start, sram->start,
+				resource_size(sram), dev_to_node(&dev->dev));
+	if (ret < 0) {
+		dev_err(&dev->dev, "failed to add to pool: %d\n", ret);
+		goto err_enable;
+	}
+
 	ret = usb_add_hcd(hcd, irq, 0);
 	if (ret)
 		goto err_add_hcd;
@@ -254,8 +264,6 @@ static int ohci_hcd_tmio_drv_probe(struct platform_device *dev)
 	if (cell->disable)
 		cell->disable(dev);
 err_enable:
-	dma_release_declared_memory(&dev->dev);
-err_dma_declare:
 	iounmap(hcd->regs);
 err_ioremap_regs:
 	iounmap(tmio->ccr);
@@ -276,7 +284,6 @@ static int ohci_hcd_tmio_drv_remove(struct platform_device *dev)
 	tmio_stop_hc(dev);
 	if (cell->disable)
 		cell->disable(dev);
-	dma_release_declared_memory(&dev->dev);
 	iounmap(hcd->regs);
 	iounmap(tmio->ccr);
 	usb_put_hcd(hcd);
-- 
2.17.1

Re: [RFC PATCH 0/3] prerequisites for device reserved local mem rework

[Robin Murphy]

Hi Laurentiu,

On 10/05/2019 15:56,  wrote:
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> 
> For HCs that have local memory, replace the current DMA API usage
> with a genalloc generic allocator to manage the mappings for these
> devices.
> This is in preparation for dropping the existing "coherent" dma
> mem declaration APIs. Current implementation was relying on a short
> circuit in the DMA API that in the end, was acting as an allocator
> for these type of devices.
> 
> Only compiled tested, so any volunteers willing to test are most welcome.

Based on my diggings into this in the past, I would expect that you need 
to do something about hcd_alloc_coherent() as well.

Robin.

> 
> Thank you!
> 
> For context, see thread here: https://lkml.org/lkml/2019/4/22/357
> 
> Laurentiu Tudor (3):
>    ohci-hcd: use genalloc for USB HCs with local memory
>    usb: host: ohci-sm501: init genalloc for local memory
>    usb: host: ohci-tmio: init genalloc for local memory
> 
>   drivers/usb/host/ohci-hcd.c   | 21 +++++++++---
>   drivers/usb/host/ohci-sm501.c | 63 +++++++++++++++++++----------------
>   drivers/usb/host/ohci-tmio.c  | 23 ++++++++-----
>   drivers/usb/host/ohci.h       |  3 ++
>   4 files changed, 69 insertions(+), 41 deletions(-)
> 

RE: [RFC PATCH 0/3] prerequisites for device reserved local mem rework

[Laurentiu Tudor]

Hi Robin,

> -----Original Message-----
> From: Robin Murphy <robin.murphy@arm.com>
> Sent: Friday, May 10, 2019 6:04 PM
> 
> Hi Laurentiu,
> 
> On 10/05/2019 15:56,  wrote:
> > From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >
> > For HCs that have local memory, replace the current DMA API usage
> > with a genalloc generic allocator to manage the mappings for these
> > devices.
> > This is in preparation for dropping the existing "coherent" dma
> > mem declaration APIs. Current implementation was relying on a short
> > circuit in the DMA API that in the end, was acting as an allocator
> > for these type of devices.
> >
> > Only compiled tested, so any volunteers willing to test are most welcome.
> 
> Based on my diggings into this in the past, I would expect that you need
> to do something about hcd_alloc_coherent() as well.

Indeed looks like it. Thanks a lot for the pointer, I'll dig into it.

---
Best Regards, Laurentiu

> 
> >
> > Thank you!
> >
> > For context, see thread here:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org
> %2Flkml%2F2019%2F4%2F22%2F357&amp;data=02%7C01%7Claurentiu.tudor%40nxp.com
> %7C1ad4c377d109419121b808d6d558b0b0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636930974257581614&amp;sdata=pNRPPIoBmZ7UBfip%2FjRBYiMgGT3pf1riWLn2
> DS%2Fotl4%3D&amp;reserved=0
> >
> > Laurentiu Tudor (3):
> >    ohci-hcd: use genalloc for USB HCs with local memory
> >    usb: host: ohci-sm501: init genalloc for local memory
> >    usb: host: ohci-tmio: init genalloc for local memory
> >
> >   drivers/usb/host/ohci-hcd.c   | 21 +++++++++---
> >   drivers/usb/host/ohci-sm501.c | 63 +++++++++++++++++++----------------
> >   drivers/usb/host/ohci-tmio.c  | 23 ++++++++-----
> >   drivers/usb/host/ohci.h       |  3 ++
> >   4 files changed, 69 insertions(+), 41 deletions(-)
> >

Re: [RFC PATCH 0/3] prerequisites for device reserved local mem rework

[Christoph Hellwig]

On Fri, May 10, 2019 at 04:03:37PM +0100, Robin Murphy wrote:
> Hi Laurentiu,
>
> On 10/05/2019 15:56,  wrote:
>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>
>> For HCs that have local memory, replace the current DMA API usage
>> with a genalloc generic allocator to manage the mappings for these
>> devices.
>> This is in preparation for dropping the existing "coherent" dma
>> mem declaration APIs. Current implementation was relying on a short
>> circuit in the DMA API that in the end, was acting as an allocator
>> for these type of devices.
>>
>> Only compiled tested, so any volunteers willing to test are most welcome.
>
> Based on my diggings into this in the past, I would expect that you need to 
> do something about hcd_alloc_coherent() as well.

Yep.  And it might make sense to share the code for that and the
ohci internal allocations with a helper.