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

Laurentiu Tudor
2019-05-16T11:49:39Z

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

Hi Fredrick,

On 15.05.2019 19:28, Fredrik Noring wrote:
> Hi Lauretniu,
>
>> I think that any recent kernel will do, so I'd say your current branch
>> should be fine.
>
> The kernel oopses with "unable to handle kernel paging request at virtual
> address 000aba0b" in hcd_alloc_coherent via usb_hcd_map_urb_for_dma. This
> relates to patch 2/3 that I didn't quite understand, where
>
> - retval = dma_declare_coherent_memory(dev, mem->start,
> - mem->start - mem->parent->start,
> - resource_size(mem));
>
> becomes
>
> + retval = gen_pool_add_virt(hcd->localmem_pool,
> + (unsigned long)mem->start -
> + mem->parent->start,
> + mem->start, resource_size(mem),
>
> so that arguments two and three switch places. Given
>
> int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
> dma_addr_t device_addr, size_t size);
>
> and
>
> int gen_pool_add_virt(struct gen_pool *pool, unsigned long virt, phys_addr_t phys
> size_t size, int nid)
>
> it seems that the device address (dma_addr_t device_addr) now becomes a
> virtual address (unsigned long virt). Is that intended?
>
> My corresponding patch is below for reference. I applied your 1/3 patch
> to test it.

I took your code, added the missing mapping and placed it in a patch.
Please see attached (compile tested only).

---
Thanks & Best Regards, Laurentiu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-usb-host-ohci-ps2-init-genalloc-for-local-memory.patch --]
[-- Type: text/x-patch; name="0001-usb-host-ohci-ps2-init-genalloc-for-local-memory.patch", Size: 3428 bytes --]

From 510c34990bbe899a2e4f43f3b904183f126f81de Mon Sep 17 00:00:00 2001
From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Date: Thu, 16 May 2019 14:35:06 +0300
Subject: [PATCH] usb: host: ohci-ps2: init genalloc for local memory

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-ps2.c | 44 ++++++++++++++++++++++++++-----------
1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/ohci-ps2.c b/drivers/usb/host/ohci-ps2.c
index 7669b7358bc3..454e5e3ac96e 100755
--- a/drivers/usb/host/ohci-ps2.c
+++ b/drivers/usb/host/ohci-ps2.c
@@ -7,6 +7,7 @@
*/

#include <linux/dma-mapping.h>
+#include <linux/genalloc.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/usb.h>
@@ -92,12 +93,13 @@ static irqreturn_t ohci_ps2_irq(struct usb_hcd *hcd)
return ohci_irq(hcd); /* Call normal IRQ handler. */
}

-static int iopheap_alloc_coherent(struct platform_device *pdev,
- size_t size, int flags)
+static int iopheap_alloc_coherent(struct platform_device *pdev, size_t size)
{
struct device *dev = &pdev->dev;
struct usb_hcd *hcd = platform_get_drvdata(pdev);
struct ps2_hcd *ps2priv = hcd_to_priv(hcd);
+ void __iomem *local_mem;
+ int err;

if (ps2priv->iop_dma_addr != 0)
return 0;
@@ -112,28 +114,45 @@ static int iopheap_alloc_coherent(struct platform_device *pdev,
return -ENOMEM;
}

- if (dma_declare_coherent_memory(dev,
- iop_bus_to_phys(ps2priv->iop_dma_addr),
- ps2priv->iop_dma_addr, size, flags)) {
- dev_err(dev, "dma_declare_coherent_memory failed\n");
- iop_free(ps2priv->iop_dma_addr);
- ps2priv->iop_dma_addr = 0;
- return -ENOMEM;
+ hcd->localmem_pool = devm_gen_pool_create(dev, PAGE_SHIFT,
+ dev_to_node(dev), DRV_NAME);
+ if (IS_ERR(hcd->localmem_pool)) {
+ err = PTR_ERR(hcd->localmem_pool);
+ goto out_err;
+ }
+
+ local_mem = devm_ioremap(dev,
+ iop_bus_to_phys(ps2priv->iop_dma_addr),
+ size);
+ if (!local_mem) {
+ err = -ENOMEM;
+ goto out_err;
+ }
+
+ err = gen_pool_add_virt(hcd->localmem_pool, (unsigned long)local_mem,
+ ps2priv->iop_dma_addr, size, dev_to_node(dev));
+ if (err < 0) {
+ dev_err(dev, "gen_pool_add_virt failed with %d\n", err);
+ goto out_err;
}

return 0;
+
+out_err:
+ iop_free(ps2priv->iop_dma_addr);
+ ps2priv->iop_dma_addr = 0;
+
+ return err;
}

static void iopheap_free_coherent(struct platform_device *pdev)
{
- struct device *dev = &pdev->dev;
struct usb_hcd *hcd = platform_get_drvdata(pdev);
struct ps2_hcd *ps2priv = hcd_to_priv(hcd);

if (ps2priv->iop_dma_addr == 0)
return;

- dma_release_declared_memory(dev);
iop_free(ps2priv->iop_dma_addr);
ps2priv->iop_dma_addr = 0;
}
@@ -177,8 +196,7 @@ static int ohci_hcd_ps2_probe(struct platform_device *pdev)
goto err;
}

- ret = iopheap_alloc_coherent(pdev,
- DMA_BUFFER_SIZE, DMA_MEMORY_EXCLUSIVE);
+ ret = iopheap_alloc_coherent(pdev, DMA_BUFFER_SIZE);
if (ret != 0)
goto err_alloc_coherent;

--
2.17.1