On Thu, Aug 03, 2017 at 05:52:08PM +0200, Johan Hovold wrote:
> 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.
>
> Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices")
> Cc: stable <stable@xxxxxxxxxxxxxxx> # 4.12
> Cc: Robin Murphy <robin.murphy@xxxxxxx>
> Cc: Sricharan R <sricharan@xxxxxxxxxxxxxx>
> Cc: Stefan Wahren <stefan.wahren@xxxxxxxx>
> Reported-by: Hans Verkuil <hverkuil@xxxxxxxxx>
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---
>
> 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(+)
>
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index b555ff9dd8fc..f9f703be0ad1 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -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);
> }
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 17681d5638ac..2a85d905b539 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -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