dma-mapping: skip USB devices when configuring DMA during probe

PATCH Superseded Johan Hovold 4 participants 9 messages Aug 3, 2017 – Aug 6, 2017

Subsystems (3)

Stable branch USB Driver core

lore

Share & tools

lore

Johan Hovold

author maintainer

2017-08-03 13:15 UTC #0 lore raw

PATCH v2 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 simply skipping USB devices when dma_configure() is called during probe.

Fixes 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")

Cc stable <[email protected]> # 4.12

Cc 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]>

---

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 | 8 ++++++++
 include/linux/usb.h        | 5 +++++
 2 files changed, 13 insertions(+)

drivers/base/dma-mapping.c +8 -0

index b555ff9dd8fc..604e99e6660a 100644
@@ -13,6 +13,7 @@
 #include <linux/gfp.h>
 #include <linux/of_device.h>
 #include <linux/slab.h>
+#include <linux/usb.h>
 #include <linux/vmalloc.h>
 
 /*

@@ -345,6 +346,10 @@ int dma_configure(struct device *dev)
 	enum dev_dma_attr attr;
 	int ret = 0;
 
+	/* USB devices share the controller's mask. */
+	if (dev_is_usb(dev))
+		return 0;
+
 	if (dev_is_pci(dev)) {
 		bridge = pci_get_host_bridge_device(to_pci_dev(dev));
 		dma_dev = bridge;

@@ -369,6 +374,9 @@ int dma_configure(struct device *dev)
 
 void dma_deconfigure(struct device *dev)
 {
+	if (dev_is_usb(dev))
+		return;
+
 	of_dma_deconfigure(dev);
 	acpi_dma_deconfigure(dev);
 }

include/linux/usb.h +5 -0

index cb9fbd54386e..f86ad9d8c756 100644
@@ -1222,6 +1222,11 @@ struct usb_device_driver {
 
 extern struct bus_type usb_bus_type;
 
+static inline bool dev_is_usb(struct device *dev)
+{
+	return dev->bus == &usb_bus_type;
+}
+
 /**
  * struct usb_class_driver - identifies a USB driver that wants to use the USB major number
  * @name: the usb class device name for this driver.  Will show up in sysfs.

Signature · 1 line

2.13.3

Robin Murphy

Reviewed-by on v2 1/1

2017-08-03 13:20 UTC #1 lore raw

Quoted 24 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 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 simply skipping USB devices when dma_configure() is called during probe.

Quoted 7 lines

Fixes 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")
Cc stable <[email protected]> # 4.12
Cc 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]>

Reviewed-by Robin Murphy <[email protected]>

Quoted 62 lines

---

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 | 8 ++++++++
 include/linux/usb.h        | 5 +++++
 2 files changed, 13 insertions(+)

drivers/base/dma-mapping.c +8 -0

index b555ff9dd8fc..604e99e6660a 100644
@@ -13,6 +13,7 @@
 #include <linux/gfp.h>
 #include <linux/of_device.h>
 #include <linux/slab.h>
+#include <linux/usb.h>
 #include <linux/vmalloc.h>
 
 /*

@@ -345,6 +346,10 @@ int dma_configure(struct device *dev)
 	enum dev_dma_attr attr;
 	int ret = 0;
 
+	/* USB devices share the controller's mask. */
+	if (dev_is_usb(dev))
+		return 0;
+
 	if (dev_is_pci(dev)) {
 		bridge = pci_get_host_bridge_device(to_pci_dev(dev));
 		dma_dev = bridge;

@@ -369,6 +374,9 @@ int dma_configure(struct device *dev)
 
 void dma_deconfigure(struct device *dev)
 {
+	if (dev_is_usb(dev))
+		return;
+
 	of_dma_deconfigure(dev);
 	acpi_dma_deconfigure(dev);
 }

include/linux/usb.h +5 -0

index cb9fbd54386e..f86ad9d8c756 100644
@@ -1222,6 +1222,11 @@ struct usb_device_driver {
 
 extern struct bus_type usb_bus_type;
 
+static inline bool dev_is_usb(struct device *dev)
+{
+	return dev->bus == &usb_bus_type;
+}
+
 /**
  * struct usb_class_driver - identifies a USB driver that wants to use the USB major number
  * @name: the usb class device name for this driver.  Will show up in sysfs.

Alan Stern

2017-08-03 14:04 UTC #2 lore raw

In reply to Johan Hovold

Quoted 44 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 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 simply skipping USB devices when dma_configure() is called during probe.
Fixes 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")
Cc stable <[email protected]> # 4.12
Cc 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]>
---
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)
include/linux/usb.h +0 -0
index cb9fbd54386e..f86ad9d8c756 100644

include/linux/usb.h struct usb_device_driver { · line 1229

Quoted 8 lines

include/linux/usb.h +5 -0

@@ -1222,6 +1222,11 @@ struct usb_device_driver {
 
 extern struct bus_type usb_bus_type;
 
+static inline bool dev_is_usb(struct device *dev)
+{
+	return dev->bus == &usb_bus_type;
+}
+

Will this work if the USB subsystem is built as a module?

Alan Stern

Johan Hovold

author maintainer

2017-08-03 14:23 UTC #3 lore raw

Quoted 43 lines · Alan Stern

Quoted 41 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 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 simply skipping USB devices when dma_configure() is called during probe.
Fixes 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")
Cc stable <[email protected]> # 4.12
Cc 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]>
---
include/linux/usb.h +0 -0
index cb9fbd54386e..f86ad9d8c756 100644
@@ -1222,6 +1222,11 @@ struct usb_device_driver {
 
 extern struct bus_type usb_bus_type;
 

Quoted 7 lines

include/linux/usb.h struct usb_device_driver { · line 1229
Quoted 5 lines
include/linux/usb.h +5 -0
+static inline bool dev_is_usb(struct device *dev)
+{
+	return dev->bus == &usb_bus_type;
+}
+
Will this work if the USB subsystem is built as a module?

Nope. :-/

Add another flag (e.g. skip_dma_configure) to struct device for now?

Thanks,
Johan

Robin Murphy

2017-08-03 14:48 UTC #4 lore raw

Quoted 52 lines · Johan Hovold

Quoted 43 lines · Alan Stern
Quoted 41 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 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 simply skipping USB devices when dma_configure() is called during probe.
Fixes 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")
Cc stable <[email protected]> # 4.12
Cc 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]>
---
include/linux/usb.h +0 -0
index cb9fbd54386e..f86ad9d8c756 100644
@@ -1222,6 +1222,11 @@ struct usb_device_driver {
 
 extern struct bus_type usb_bus_type;
 
Quoted 7 lines
include/linux/usb.h struct usb_device_driver { · line 1229
Quoted 5 lines
include/linux/usb.h +5 -0
+static inline bool dev_is_usb(struct device *dev)
+{
+	return dev->bus == &usb_bus_type;
+}
+
Will this work if the USB subsystem is built as a module?

Quoted 1 line

Nope. :-/

Oh bum, I hadn't even realised usb_bus_type could be modular.

Quoted 1 line

Add another flag (e.g. skip_dma_configure) to struct device for now?

Would it be sufficient to look for dev->of_node_reused, or is it also possible for USB devices to have OF nodes of their own such that of_dma_configure() would still blat the mask with a 32-bit default? (Although that would still un-break Rpi3, even if strictly wrong)

Robin.

Johan Hovold

author maintainer

2017-08-03 14:53 UTC #5 lore raw

Quoted 22 lines · Robin Murphy

Quoted 19 lines · Johan Hovold
Quoted 10 lines · Alan Stern
Quoted 9 lines · Johan Hovold
include/linux/usb.h +0 -0
index cb9fbd54386e..f86ad9d8c756 100644
include/linux/usb.h +0 -0
@@ -1222,6 +1222,11 @@ struct usb_device_driver {
 
 extern struct bus_type usb_bus_type;
 
Quoted 7 lines
include/linux/usb.h struct usb_device_driver { · line 1229
Quoted 5 lines
include/linux/usb.h +5 -0
+static inline bool dev_is_usb(struct device *dev)
+{
+	return dev->bus == &usb_bus_type;
+}
+
Will this work if the USB subsystem is built as a module?
Quoted 1 line
Nope. :-/

Quoted 8 lines

Oh bum, I hadn't even realised usb_bus_type could be modular.
Quoted 1 line
Add another flag (e.g. skip_dma_configure) to struct device for now?
Would it be sufficient to look for dev->of_node_reused, or is it also possible for USB devices to have OF nodes of their own such that of_dma_configure() would still blat the mask with a 32-bit default? (Although that would still un-break Rpi3, even if strictly wrong)

Yes, USB devices can have their own OF nodes so of_node_reused would not prevent of_dma_configure() from being called for them.

But testing against the platform bus instead of not-USB should do the trick.

Johan

Johan Hovold

author maintainer

2017-08-03 14:49 UTC #6 lore raw

In reply to Johan Hovold

Quoted 52 lines · Johan Hovold

Quoted 43 lines · Alan Stern
Quoted 41 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 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 simply skipping USB devices when dma_configure() is called during probe.
Fixes 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")
Cc stable <[email protected]> # 4.12
Cc 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]>
---
include/linux/usb.h +0 -0
index cb9fbd54386e..f86ad9d8c756 100644
@@ -1222,6 +1222,11 @@ struct usb_device_driver {
 
 extern struct bus_type usb_bus_type;
 
Quoted 7 lines
include/linux/usb.h struct usb_device_driver { · line 1229
Quoted 5 lines
include/linux/usb.h +5 -0
+static inline bool dev_is_usb(struct device *dev)
+{
+	return dev->bus == &usb_bus_type;
+}
+
Will this work if the USB subsystem is built as a module?

Quoted 3 lines

Nope. :-/
Add another flag (e.g. skip_dma_configure) to struct device for now?

Or we can make sure that of_dma_configure() is only ever called for platform_devices by checking against platform_bus_type instead.

I guess this was the intention of 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices") too.

Thanks,
Johan

Johan Hovold

author maintainer

2017-08-03 15:57 UTC #7 lore raw

Quoted 55 lines · Johan Hovold

Quoted 54 lines · Johan Hovold
Quoted 43 lines · Alan Stern
Quoted 41 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 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 simply skipping USB devices when dma_configure() is called during probe.
Fixes 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")
Cc stable <[email protected]> # 4.12
Cc 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]>
---
include/linux/usb.h +0 -0
index cb9fbd54386e..f86ad9d8c756 100644
@@ -1222,6 +1222,11 @@ struct usb_device_driver {
 
 extern struct bus_type usb_bus_type;
 
Quoted 7 lines
include/linux/usb.h struct usb_device_driver { · line 1229
Quoted 5 lines
include/linux/usb.h +5 -0
+static inline bool dev_is_usb(struct device *dev)
+{
+	return dev->bus == &usb_bus_type;
+}
+
Will this work if the USB subsystem is built as a module?
Nope. :-/

Quoted 7 lines

Quoted 1 line
Add another flag (e.g. skip_dma_configure) to struct device for now?
Or we can make sure that of_dma_configure() is only ever called for platform_devices by checking against platform_bus_type instead.
I guess this was the intention of 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices") too.

Or perhaps not, as of_dma_configure() should also be called for pci devices.

I suggest adding a new flag to struct device.

Johan

kbuild test robot

build bot

2017-08-06 11:27 UTC #8 lore raw

In reply to Johan Hovold

url: https://github.com/0day-ci/linux/commits/Johan-Hovold/dma-mapping-skip-USB-devices-when-configuring-DMA-during-probe/20170804-155414 config: x86_64-randconfig-a0-08061415 (attached as .config)…

Hi Johan,

[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on v4.13-rc3 next-20170804]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Johan-Hovold/dma-mapping-skip-USB-devices-when-configuring-DMA-during-probe/20170804-155414
config: x86_64-randconfig-a0-08061415 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

drivers/base/dma-mapping.o: In function `dma_configure':

>> (.text+0x7de): undefined reference to `usb_bus_type'

drivers/base/dma-mapping.o: In function `dma_deconfigure':
(.text+0x8ca): undefined reference to `usb_bus_type'

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation