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
