[RFC] DMA initialization for manually created devices

[Felipe Balbi]

Hi folks,

Today I noticed two issues with dwc3 on PCI-based systems which, while
debugging, I uncovered some details which we might want to change,
however I need a little guidance here.

The first problem is that when running with intel-iommu disable and
falling back to swiotlb, I can easily run out of space for DMA
mapping/unmapping:

[  574.862949] DMA: Out of SW-IOMMU space for 217 bytes at device dwc3.0.auto
[  574.870820] dwc3 dwc3.0.auto: failed to map buffer

I checked that I'm not leaking any of the mapped buffers and they're all
balanced with a matching unmap call.

The second problem is that when enabling intel-iommu then I can't
allocate from coherent:

[   81.797657] DMAR: Allocating domain for dwc3.0.auto failed
[   81.803980] dwc3 dwc3.0.auto: can't allocate event buffer
[   81.810221] dwc3 dwc3.0.auto: failed to allocate event buffers

The reason for that I'm using a manually created platform_device and
that misses dev->archdata which the underlying/parent PCI device has.

Here I have two options:

1) continue to use my manually allocated platform_device pointer for DMA
operations and just copy necessary bits from the parent PCI device:

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index adc1e8a624cb..011d0055abd0 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -174,6 +174,14 @@ static int dwc3_pci_probe(struct pci_dev *pci,
 	if (ret)
 		goto err;
 
+	dwc3->dev.dma_mask = &pci->dma_mask;
+	dwc3->dev.dma_parms  = &pci->dma_parms;
+
+	/* is there a better way ?? */
+	memcpy(&dwc3->dev.archdata, &dev->archdata, sizeof(dev->archdata));
+
+	dma_set_coherent_mask(&dwc3->dev, pci->dma_mask);
+
 	dwc3->dev.parent = dev;
 	ACPI_COMPANION_SET(&dwc3->dev, ACPI_COMPANION(dev));
 
This works fine with intel-iommu, I just tested.

2) map/unmap using the parent PCI device. IOW:

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 143deb420481..a4e4b0417bf3 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -967,7 +967,7 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 		u32	transfer_size = 0;
 		u32	maxpacket;
 
-		ret = usb_gadget_map_request(&dwc->gadget, &req->request,
+		ret = usb_gadget_map_request_by_dev(dwc->dev, parent, &req->request,
 				dep->number);
 		if (ret) {
 			dwc3_trace(trace_dwc3_ep0, "failed to map request\n");
@@ -995,7 +995,7 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 				dwc->ep0_bounce_addr, transfer_size,
 				DWC3_TRBCTL_CONTROL_DATA, false);
 	} else {
-		ret = usb_gadget_map_request(&dwc->gadget, &req->request,
+		ret = usb_gadget_map_request_by_dev(dwc->dev, parent, &req->request,
 				dep->number);
 		if (ret) {
 			dwc3_trace(trace_dwc3_ep0, "failed to map request\n");
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index e39f29bd2fff..0732d14d2687 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -191,7 +191,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
 	if (dwc->ep0_bounced && dep->number == 0)
 		dwc->ep0_bounced = false;
 	else
-		usb_gadget_unmap_request(&dwc->gadget, &req->request,
+		usb_gadget_unmap_request_by_dev(dwc->dev->parent, &req->request,
 				req->direction);
 
 	trace_dwc3_gadget_giveback(req);


This I haven't tested, but it should work.

Anyway, the question is: which of the two approaches is preferred ?

cheers

ps: I haven't debugged why I'm swiotlb error, that's in my list for
after $subject gets solved.

-- 
balbi

Attachment: signature.asc
Description: PGP signature