Re: [PATCH v3] dma-mapping: skip USB devices when configuring DMA during probe

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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@vger.kernel.org>	# 4.12
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Sricharan R <sricharan@codeaurora.org>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
> 
> 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