Re: dwc3 initiated xhci probe problem in arm64 4.4 kernel due to DMA setup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Grygorii Strashko <grygorii.strashko@xxxxxx> writes:
> On 04/15/2016 02:31 PM, Felipe Balbi wrote:
>> Catalin Marinas <catalin.marinas@xxxxxxx> writes:
>>> On Fri, Apr 15, 2016 at 01:30:01PM +0300, Felipe Balbi wrote:
>>>> Catalin Marinas <catalin.marinas@xxxxxxx> writes:
>>>>> On Fri, Apr 15, 2016 at 11:01:08AM +0100, Catalin Marinas wrote:
>>>>>> On Fri, Apr 15, 2016 at 12:49:15PM +0300, Felipe Balbi wrote:
>>>>>>> Catalin Marinas <catalin.marinas@xxxxxxx> writes:
>>>>>>>> On Thu, Apr 14, 2016 at 12:46:47PM +0000, David Fisher wrote:
>>>>>>>>> dwc3 is in dual-role, with "synopsys,dwc3" specified in DT.
>>>>>>>>>
>>>>>>>>> When xhci is probed, initiated from dwc3/host.c (not DT), we get :
>>>>>>>>> xhci-hcd: probe of xhci-hcd.7.auto failed with error -5
>>>>>>>>> This -EIO error originated from inside dma_set_mask() down in include/asm-generic/dma-mapping-common.h
>>>>>>>>>
>>>>>>>>> If "generic-xhci" is specified in DT instead, it probes fine as a host-only dwc3
>>>>>>>>> The difference between DT initiated probe and dwc3 initiated probe is that
>>>>>>>>> when DT initiated probe gets to dma_supported, dma_supported is
>>>>>>>>> implemented by swiotlb_dma_supported (previously set up by a DT call to arch_dma_setup_ops).
>>>>>>>>> Whereas when dwc3 initiated xhci probe gets to dma_supported, arch_dma_setup_ops has not been called
>>>>>>>>> and dma_supported is only implemented by __dummy_dma_supported, returning 0.
>>>>>>>>>
>>>>>>>>> Bisecting finds the "bad" commit as
>>>>>>>>> 1dccb598df549d892b6450c261da54cdd7af44b4  (inbetween 4.4-rc1 and 4.4-rc2)
>>>>>>>>> --- a/arch/arm64/include/asm/dma-mapping.h
>>>>>>>>> --- a/arch/arm64/mm/dma-mapping.c
>>>>>>>>>
>>>>>>>>> Previous to this commit, dma_ops = &swiotlb_dma_ops was done in arm64_dma_init
>>>>>>>>> After this commit, the assignment is only done in arch_dma_setup_ops.
>>>>>>>>
>>>>>>>> This restriction was added on purpose and the arm64 __generic_dma_ops()
>>>>>>>> now has a comment:
>>>>>>>>
>>>>>>>> 	/*
>>>>>>>> 	 * We expect no ISA devices, and all other DMA masters are expected to
>>>>>>>> 	 * have someone call arch_setup_dma_ops at device creation time.
>>>>>>>> 	 */
>>>>>>>
>>>>>>> how ?
>>>>>>
>>>>>> Usually by calling arch_setup_dma_ops().
>>>>>
>>>>> Or of_dma_configure(), I forgot to mention this (see the
>>>>> pci_dma_configure() function as an example).
>>>>
>>>> the device is manually created, there's not OF/DT for it ;-)
>>>
>>> As for PCI, the created device doesn't have a node but the bridge does.
>>> Something like below, completely untested:
>>>
>>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
>>> index c679f63783ae..96d8babd7f23 100644
>>> --- a/drivers/usb/dwc3/host.c
>>> +++ b/drivers/usb/dwc3/host.c
>>> @@ -32,7 +32,10 @@ int dwc3_host_init(struct dwc3 *dwc)
>>>   		return -ENOMEM;
>>>   	}
>>>   
>>> -	dma_set_coherent_mask(&xhci->dev, dwc->dev->coherent_dma_mask);
>>> +	if (IS_ENABLED(CONFIG_OF) && dwc->dev->of_node)
>>> +		of_dma_configure(&xhci->dev, dwc->dev->of_node);
>>> +	else
> {
>>> +		dma_set_coherent_mask(&xhci->dev, dwc->dev->coherent_dma_mask);
>
> 	 xhci->dev.dma_mask      = dwc->dev->dma_mask;
>          xhci->dev.dma_parms     = dwc->dev->dma_parms;
> }
>
>> 
>> assuming this works fine for all users, I don't have an issue with
>> it. Grygorii has just been working on DMA setup for k2 and dwc3, so I
>> want to make sure this won't regress those devices.
>> 
>
> And this is the same approach I've proposed here:
> https://lkml.org/lkml/2016/3/31/609
>
> :)
>
> So, final change could be:
>
> ---
> From c68225e97e8c9505aca4ceab19a0d8e4dde31b73 Mon Sep 17 00:00:00 2001
> From: Grygorii Strashko <grygorii.strashko@xxxxxx>
> Date: Thu, 31 Mar 2016 19:40:52 +0300
> Subject: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev
>
> Now not all DMA paremters configured properly for "xhci-hcd" platform
> device which is created manually. For example: dma_pfn_offset, dam_ops
> and iommu configuration will not corresponds "dwc3" devices
> configuration. As result, this will cause problems like wrong DMA
> addresses translation on platforms with LPAE enabled like Keystone 2.
>
> When platform is using DT boot mode the DMA configuration will be
> parsed and applied from DT, so, to fix this issue, reuse
> of_dma_configure() API and retrieve DMA configuartion for "xhci-hcd"
> from DWC3 device node.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@xxxxxx>
> ---
>  drivers/usb/dwc3/host.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index c679f63..93c8ef9 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -17,6 +17,7 @@
>  
>  #include <linux/platform_device.h>
>  #include <linux/usb/xhci_pdriver.h>
> +#include <linux/of_device.h>
>  
>  #include "core.h"
>  
> @@ -32,12 +33,7 @@ int dwc3_host_init(struct dwc3 *dwc)
>  		return -ENOMEM;
>  	}
>  
> -	dma_set_coherent_mask(&xhci->dev, dwc->dev->coherent_dma_mask);
> -
>  	xhci->dev.parent	= dwc->dev;
> -	xhci->dev.dma_mask	= dwc->dev->dma_mask;
> -	xhci->dev.dma_parms	= dwc->dev->dma_parms;
> -
>  	dwc->xhci = xhci;
>  
>  	ret = platform_device_add_resources(xhci, dwc->xhci_resources,
> @@ -62,6 +58,15 @@ int dwc3_host_init(struct dwc3 *dwc)
>  	phy_create_lookup(dwc->usb3_generic_phy, "usb3-phy",
>  			  dev_name(&xhci->dev));
>  
> +	if (IS_ENABLED(CONFIG_OF) && dwc->dev->of_node) {
> +		of_dma_configure(&xhci->dev, dwc->dev->of_node);
> +	} else {
> +		dma_set_coherent_mask(&xhci->dev, dwc->dev->coherent_dma_mask);
> +
> +		xhci->dev.dma_mask	= dwc->dev->dma_mask;
> +		xhci->dev.dma_parms	= dwc->dev->dma_parms;
> +	}
> +
>  	ret = platform_device_add(xhci);
>  	if (ret) {
>  		dev_err(dwc->dev, "failed to register xHCI device\n");
> -- 
> 2.8.1

Are you sending this as a formal patch ?

-- 
balbi

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux