linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Fredrik Noring <noring@nocrew.org>
To: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Cc: Robin Murphy <robin.murphy@arm.com>, "hch@lst.de" <hch@lst.de>,
	"stern@rowland.harvard.edu" <stern@rowland.harvard.edu>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"marex@denx.de" <marex@denx.de>,
	"JuergenUrban@gmx.de" <JuergenUrban@gmx.de>,
	Leo Li <leoyang.li@nxp.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework
Date: Wed, 15 May 2019 18:28:58 +0200	[thread overview]
Message-ID: <20190515162858.GB17162@sx9> (raw)
In-Reply-To: <0e5f3b86-7a80-eec7-691b-34a123194208@nxp.com>

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.

Fredrik

diff --git a/drivers/usb/host/ohci-ps2.c b/drivers/usb/host/ohci-ps2.c
--- 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,12 @@ 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);
+	int err;
 
 	if (ps2priv->iop_dma_addr != 0)
 		return 0;
@@ -112,28 +113,37 @@ 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;
+	}
+
+	err = gen_pool_add_virt(hcd->localmem_pool, ps2priv->iop_dma_addr,
+		iop_bus_to_phys(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 +187,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;
 

  reply	other threads:[~2019-05-15 16:29 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-14 14:38 [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework laurentiu.tudor
2019-05-14 14:38 ` [RFC PATCH v2 1/3] USB: use genalloc for USB HCs with local memory laurentiu.tudor
2019-05-14 14:42   ` Christoph Hellwig
2019-05-15  9:57     ` Laurentiu Tudor
2019-05-15 12:42       ` Christoph Hellwig
2019-05-14 14:38 ` [RFC PATCH v2 2/3] usb: host: ohci-sm501: init genalloc for " laurentiu.tudor
2019-05-14 14:38 ` [RFC PATCH v2 3/3] usb: host: ohci-tmio: " laurentiu.tudor
2019-05-14 15:17 ` [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework Robin Murphy
2019-05-14 18:29   ` Fredrik Noring
2019-05-15 10:37     ` Laurentiu Tudor
2019-05-15 16:28       ` Fredrik Noring [this message]
2019-05-16  9:35         ` Laurentiu Tudor
2019-05-16 13:47           ` Fredrik Noring
2019-05-16 11:49         ` Laurentiu Tudor
2019-05-16 15:15           ` Fredrik Noring
2019-05-17 10:52             ` Laurentiu Tudor
2019-05-17 17:41               ` Fredrik Noring
2019-05-20 11:34                 ` Laurentiu Tudor
2019-05-20 15:41                   ` Fredrik Noring
2019-05-21 10:32                     ` hch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190515162858.GB17162@sx9 \
    --to=noring@nocrew.org \
    --cc=JuergenUrban@gmx.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=laurentiu.tudor@nxp.com \
    --cc=leoyang.li@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=marex@denx.de \
    --cc=robin.murphy@arm.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).