>Hi,
>
>Grygorii Strashko <grygorii.stras...@ti.com> writes:
>> On 04/15/2016 02:31 PM, Felipe Balbi wrote:
>>> 
>>> Hi,
>>> 
>>> Catalin Marinas <catalin.mari...@arm.com> writes:
>>>> On Fri, Apr 15, 2016 at 01:30:01PM +0300, Felipe Balbi wrote:
>>>>> Catalin Marinas <catalin.mari...@arm.com> 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.mari...@arm.com> 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.stras...@ti.com>
>> 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.stras...@ti.com>
>> ---
>>  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
>
>David, does this work for your setup ?
>

Yes - works for me. Happy to test other candidate patches given that this seems 
to be work in progress.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to