dma-mapping: skip USB devices when configuring DMA during probe
Subsystems (3)
Johan Hovold
maintainerPATCH v3 1/1 dma-mapping: skip USB devices when configuring DMA during probe
USB devices use the DMA mask and offset of the controller, which have already been setup when a device is probed. Note that modifying the DMA mask of a USB device would change the mask for the controller (and all devices on the bus) as the mask is literally shared.
Since commit 2bf698671205 ("USB: of: fix root-hub device-tree node handling"), of_dma_configure() would be called also for root hubs, which use the device node of the controller. A separate, long-standing bug that makes of_dma_configure() generate a 30-bit DMA mask from the RPI3's "dma-ranges" would thus set a broken mask also for the controller. This in turn prevents USB devices from enumerating when control transfers fail:
dwc2 3f980000.usb: Cannot do DMA to address 0x000000003a166a00
Note that the aforementioned DMA-mask bug was benign for the HCD itself as the dwc2 driver overwrites the mask previously set by of_dma_configure() for the platform device in its probe callback. The mask would only later get corrupted when the root-hub child device was probed.
Fix this, and similar future problems, by adding a flag to struct device which prevents driver core from calling dma_configure() during probe and making sure it is set for USB devices.
---
v3
- add flag to struct device to prevent DMA configuration during probe instead of checking for the USB bus type, which is not available when USB is built as a module as noted by Alan
- drop moderated rpi list from CC
v2
- amend commit message and point out that the long-standing 30-bit DMA-mask bug was benign to the dwc2 HCD itself (Robin)
- add and use a new dev_is_usb() helper (Robin)
drivers/base/dma-mapping.c | 6 ++++++ drivers/usb/core/usb.c | 1 + include/linux/device.h | 3 +++ 3 files changed, 10 insertions(+)
drivers/base/dma-mapping.c +6 -0
@@ -345,6 +345,9 @@ int dma_configure(struct device *dev)enum dev_dma_attr attr;int ret =0 ; +if (dev->skip_dma_configure) +return 0 ; +if (dev_is_pci (dev)) { bridge =pci_get_host_bridge_device (to_pci_dev (dev)); dma_dev = bridge;
@@ -369,6 +372,9 @@ int dma_configure(struct device *dev)void dma_deconfigure (struct device *dev) { +if (dev->skip_dma_configure) +return ; +of_dma_deconfigure (dev);acpi_dma_deconfigure (dev); }
drivers/usb/core/usb.c +1 -0
@@ -588,6 +588,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,* Note: calling dma_set_mask() on a USB device would set the * mask for the entire HCD, so don't do that. */ + dev->dev.skip_dma_configure = true; dev->dev.dma_mask = bus->sysdev->dma_mask; dev->dev.dma_pfn_offset = bus->sysdev->dma_pfn_offset;set_dev_node (&dev->dev,dev_to_node (bus->sysdev));
include/linux/device.h +3 -0
@@ -877,6 +877,8 @@ struct dev_links_info {* @offline: Set after successful invocation of bus type's .offline(). * @of_node_reused: Set if the device-tree node is shared with an ancestor * device. +* @skip_dma_configure: Set if driver core should not configure DMA for this +* device during probe. * * At the lowest level, every device in a Linux system is represented by an * instance of struct device. The device structure contains the information
@@ -965,6 +967,7 @@ struct device { bool offline_disabled:1; bool offline:1; bool of_node_reused:1; + bool skip_dma_configure:1; };static inline struct device *kobj_to_dev (struct kobject *kobj)
Signature · 1 line
2.13.3
Greg Kroah-Hartman
maintainerQuoted 80 lines · Johan Hovold
USB devices use the DMA mask and offset of the controller, which have already been setup when a device is probed. Note that modifying the DMA mask of a USB device would change the mask for the controller (and all devices on the bus) as the mask is literally shared.
Since commit
2bf698671205("USB: of: fix root-hub device-tree node handling"),of_dma_configure()would be called also for root hubs, which use the device node of the controller. A separate, long-standing bug that makesof_dma_configure()generate a 30-bit DMA mask from the RPI3's "dma-ranges" would thus set a broken mask also for the controller. This in turn prevents USB devices from enumerating when control transfers fail:dwc2 3f980000.usb: Cannot do DMA to address 0x000000003a166a00Note that the aforementioned DMA-mask bug was benign for the HCD itself as the dwc2 driver overwrites the mask previously set by
of_dma_configure()for the platform device in its probe callback. The mask would only later get corrupted when the root-hub child device was probed.Fix this, and similar future problems, by adding a flag to struct device which prevents driver core from calling
dma_configure()during probe and making sure it is set for USB devices.Fixes 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")Cc stable <[email protected]> # 4.12Cc Robin Murphy <[email protected]>Cc Sricharan R <[email protected]>Cc Stefan Wahren <[email protected]>Reported-by Hans Verkuil <[email protected]>Signed-off-by Johan Hovold <[email protected]>---
v3
- add flag to struct device to prevent DMA configuration during probe instead of checking for the USB bus type, which is not available when USB is built as a module as noted by Alan
- drop moderated rpi list from CCv2
- amend commit message and point out that the long-standing 30-bit DMA-mask bug was benign to the dwc2 HCD itself (Robin)
- add and use a newdev_is_usb()helper (Robin)drivers/base/dma-mapping.c | 6 ++++++ drivers/usb/core/usb.c | 1 + include/linux/device.h | 3 +++ 3 files changed, 10 insertions(+)drivers/base/dma-mapping.c +6
-0@@ -345,6 +345,9 @@ int dma_configure(struct device *dev)enum dev_dma_attr attr;int ret =0 ; +if (dev->skip_dma_configure) +return 0 ; +if (dev_is_pci (dev)) { bridge =pci_get_host_bridge_device (to_pci_dev (dev)); dma_dev = bridge;@@ -369,6 +372,9 @@ int dma_configure(struct device *dev)void dma_deconfigure (struct device *dev) { +if (dev->skip_dma_configure) +return ; +of_dma_deconfigure (dev);acpi_dma_deconfigure (dev); }drivers/usb/core/usb.c +0
-0
drivers/usb/core/usb.c struct usb_device *usb_alloc_dev(struct usb_device *parent, · line 593
Quoted 6 lines
drivers/usb/core/usb.c +1
-0@@ -588,6 +588,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,* Note: calling dma_set_mask() on a USB device would set the * mask for the entire HCD, so don't do that. */ + dev->dev.skip_dma_configure = true; dev->dev.dma_mask = bus->sysdev->dma_mask; dev->dev.dma_pfn_offset = bus->sysdev->dma_pfn_offset;
Why do we need to set the mask and offset if we are not going to configure the DMA for this device?
I feel like this is a random device quirk that you are trying to work around in a generic way when we haven't seen any other broken hardware like this :)
thanks,
greg k-h
Johan Hovold
maintainerQuoted 82 lines · Greg Kroah-Hartman
Quoted 81 lines · Johan Hovold
USB devices use the DMA mask and offset of the controller, which have already been setup when a device is probed. Note that modifying the DMA mask of a USB device would change the mask for the controller (and all devices on the bus) as the mask is literally shared.
Since commit
2bf698671205("USB: of: fix root-hub device-tree node handling"),of_dma_configure()would be called also for root hubs, which use the device node of the controller. A separate, long-standing bug that makesof_dma_configure()generate a 30-bit DMA mask from the RPI3's "dma-ranges" would thus set a broken mask also for the controller. This in turn prevents USB devices from enumerating when control transfers fail:dwc2 3f980000.usb: Cannot do DMA to address 0x000000003a166a00Note that the aforementioned DMA-mask bug was benign for the HCD itself as the dwc2 driver overwrites the mask previously set by
of_dma_configure()for the platform device in its probe callback. The mask would only later get corrupted when the root-hub child device was probed.Fix this, and similar future problems, by adding a flag to struct device which prevents driver core from calling
dma_configure()during probe and making sure it is set for USB devices.Fixes 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")Cc stable <[email protected]> # 4.12Cc Robin Murphy <[email protected]>Cc Sricharan R <[email protected]>Cc Stefan Wahren <[email protected]>Reported-by Hans Verkuil <[email protected]>Signed-off-by Johan Hovold <[email protected]>---
v3
- add flag to struct device to prevent DMA configuration during probe instead of checking for the USB bus type, which is not available when USB is built as a module as noted by Alan
- drop moderated rpi list from CCv2
- amend commit message and point out that the long-standing 30-bit DMA-mask bug was benign to the dwc2 HCD itself (Robin)
- add and use a newdev_is_usb()helper (Robin)drivers/base/dma-mapping.c | 6 ++++++ drivers/usb/core/usb.c | 1 + include/linux/device.h | 3 +++ 3 files changed, 10 insertions(+)drivers/base/dma-mapping.c +6
-0@@ -345,6 +345,9 @@ int dma_configure(struct device *dev)enum dev_dma_attr attr;int ret =0 ; +if (dev->skip_dma_configure) +return 0 ; +if (dev_is_pci (dev)) { bridge =pci_get_host_bridge_device (to_pci_dev (dev)); dma_dev = bridge;@@ -369,6 +372,9 @@ int dma_configure(struct device *dev)void dma_deconfigure (struct device *dev) { +if (dev->skip_dma_configure) +return ; +of_dma_deconfigure (dev);acpi_dma_deconfigure (dev); }drivers/usb/core/usb.c +0
-0@@ -588,6 +588,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,* Note: calling dma_set_mask() on a USB device would set the
Quoted 8 lines
drivers/usb/core/usb.c struct usb_device *usb_alloc_dev(struct usb_device *parent, · line 593
Quoted 5 lines
drivers/usb/core/usb.c +1
-0* mask for the entire HCD, so don't do that. */ + dev->dev.skip_dma_configure = true; dev->dev.dma_mask = bus->sysdev->dma_mask; dev->dev.dma_pfn_offset = bus->sysdev->dma_pfn_offset;Why do we need to set the mask and offset if we are not going to configure the DMA for this device?
Take a look at the comment just above in usb_alloc_dev() that was added by commit b44bbc46a8bb ("usb: core: setup dma_pfn_offset for USB devices and, interfaces").
We've been setting the dma_mask like this since before git, and the offset since the above mentioned commit, and other subsystems apparently expect them to be set.
What this patch does is just to prevent driver core from doing what the comment explicitly says must not be done, namely to set the dma_mask for a USB device, something which is clearly broken.
Long term we should probably fix this up in some way, but this regression should be addressed first.
Quoted 3 lines
I feel like this is a random device quirk that you are trying to work around in a generic way when we haven't seen any other broken hardware like this :)
This device, and that other DMA-mask bug, only allowed us to detect this broken dma_mask manipulation, which should not have been done in the first place.
In fact, I guess since commit 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices") all controllers would have their masks reset to DMA_BIT_MASK(32) whenever we have an OF node defined for any of its ports. I should probably have mentioned that in the commit message as well.
Thanks,
Johan
I think the root problem is that the code added by
" of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices"
is completely bogus and needs to be reverted. We can't simply iterate over all devices in the system and set up dma for them. We'll need to ask the firmware / OF what root port this applies to and only apply it to those buses.
Note that the current code checks for a OF or ACPI node already, so we should start by refining these checks.
Johan Hovold
maintainerQuoted 8 lines · Christoph Hellwig
I think the root problem is that the code added by
" of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices"is completely bogus and needs to be reverted. We can't simply iterate over all devices in the system and set up dma for them. We'll need to ask the firmware / OF what root port this applies to and only apply it to those buses.
I'm afraid I haven't had time to look any more at this, and now I'll be offline for the next two weeks.
It sounded like Robin was working on a fix for the broken DMA-mask on RPI3 and that should address Hans's ethernet regression too even if we ultimately need to fix dma_configure() as well.
Thanks,
Johan
Hi Robin,
Am 08.08.2017 um 20:03 schrieb Johan Hovold:
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines
I think the root problem is that the code added by
" of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices"is completely bogus and needs to be reverted. We can't simply iterate over all devices in the system and set up dma for them. We'll need to ask the firmware / OF what root port this applies to and only apply it to those buses.
Quoted 6 lines
I'm afraid I haven't had time to look any more at this, and now I'll be offline for the next two weeks.
It sounded like Robin was working on a fix for the broken DMA-mask on RPI3 and that should address Hans's ethernet regression too even if we ultimately need to fix
dma_configure()as well.
can you confirm that DMA-mask on RPI3 is broken? Why?
Quoted 3 lines
Thanks,
Johan