On 04/15/2016 03:55 PM, Felipe Balbi wrote:
> 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
>
> David, does this work for your setup ?
>
Felipe, Seems the problem might be deeper than on first look :(
Lets see what grep says below. Even if we descope all SoC's drivers,
there still will be few USB core components which manipulate with DMA parameters:
---
./drivers/usb/core/hcd.c: hcd->self.uses_dma = (dev->dma_mask != NULL);
usb_create_shared_hcd()
hcd->self.uses_dma = (dev->dma_mask != NULL);
---
./drivers/usb/core/usb.c: dev->dev.dma_mask = bus->controller->dma_mask;
usb_alloc_dev()
dev->dev.dma_mask = bus->controller->dma_mask;
---
./drivers/usb/core/message.c: intf->dev.dma_mask = dev->dev.dma_mask;
usb_set_configuration()
intf->dev.dma_mask = dev->dev.dma_mask;
^ why it is here is total mystery :(
---
./drivers/usb/dwc3/core.c: dev->dma_mask = dev->parent->dma_mask;
---
./drivers/usb/dwc2/hcd.c: /* Check if the bus driver or platform code has setup a dma_mask */
dwc2_hcd_init()
dma_set_mask(hsotg->dev, DMA_BIT_MASK(32)) < 0)
---
./drivers/usb/host/xhci.c:
xhci_gen_setup()
!dma_set_mask(dev, DMA_BIT_MASK(64)
./drivers/usb/host/xhci-plat.c: if (WARN_ON(!pdev->dev.dma_mask))
---
./drivers/usb/musb/musb_core.c: if (use_dma && dev->dma_mask) {
---
./drivers/usb/storage/scsiglue.c: if (!us->pusb_dev->bus->controller->dma_mask)
It is big secret as for me (I'm USB noob:) which device is used for DMA transfers and when :(
-------------------------------------------------------------------------------------
grep -Ir -w dma_mask ./drivers/usb/*
./drivers/usb/chipidea/core.c: pdev->dev.dma_mask = dev->dma_mask;
./drivers/usb/core/hcd.c: hcd->self.uses_dma = (dev->dma_mask != NULL);
./drivers/usb/core/usb.c: dev->dev.dma_mask = bus->controller->dma_mask;
./drivers/usb/core/usb.c: if (controller->dma_mask) {
./drivers/usb/core/usb.c: if (controller->dma_mask) {
./drivers/usb/core/usb.c: if (controller->dma_mask) {
./drivers/usb/core/usb.c: || !controller->dma_mask)
./drivers/usb/core/usb.c: || !controller->dma_mask)
./drivers/usb/core/usb.c: || !controller->dma_mask)
./drivers/usb/core/buffer.c: (!hcd->self.controller->dma_mask &&
./drivers/usb/core/buffer.c: (!bus->controller->dma_mask &&
./drivers/usb/core/buffer.c: (!bus->controller->dma_mask &&
./drivers/usb/core/message.c: intf->dev.dma_mask = dev->dev.dma_mask;
./drivers/usb/dwc2/hcd.c: /* Check if the bus driver or platform code has setup a dma_mask */
./drivers/usb/dwc2/hcd.c: hsotg->dev->dma_mask == NULL) {
./drivers/usb/dwc2/hcd.c: "dma_mask not set, disabling DMA\n");
./drivers/usb/dwc2/platform.c: if (!dev->dev.dma_mask)
./drivers/usb/dwc2/platform.c: dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
./drivers/usb/dwc3/dwc3-exynos.c: * Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/dwc3/host.c: xhci->dev.dma_mask = dwc->dev->dma_mask;
./drivers/usb/dwc3/core.c: if (!dev->dma_mask) {
./drivers/usb/dwc3/core.c: dev->dma_mask = dev->parent->dma_mask;
./drivers/usb/gadget/udc/fotg210-udc.c: fotg210->gadget.dev.dma_mask = pdev->dev.dma_mask;
./drivers/usb/gadget/udc/lpc32xx_udc.c: pdev->dev.dma_mask = &lpc32xx_usbd_dmamask;
./drivers/usb/gadget/udc/bdc/bdc_pci.c: bdc->dev.dma_mask = pci->dev.dma_mask;
./drivers/usb/gadget/udc/pxa25x_udc.h: u64 dma_mask;
./drivers/usb/host/isp116x-hcd.c: if (pdev->dev.dma_mask) {
./drivers/usb/host/ehci-grlib.c: /* usb_create_hcd requires dma_mask != NULL */
./drivers/usb/host/ehci-grlib.c: op->dev.dma_mask = &op->dev.coherent_dma_mask;
./drivers/usb/host/ehci-orion.c: * Right now device-tree probed devices don't get dma_mask
./drivers/usb/host/ohci-at91.c: /* Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/uhci-grlib.c: /* usb_create_hcd requires dma_mask != NULL */
./drivers/usb/host/uhci-grlib.c: op->dev.dma_mask = &op->dev.coherent_dma_mask;
./drivers/usb/host/isp1362-hcd.c: if (pdev->dev.dma_mask) {
./drivers/usb/host/ehci-spear.c: * Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/ehci-hcd.c: * NOTE: the dma mask is visible through dev->dma_mask, so
./drivers/usb/host/uhci-platform.c: * Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/ohci-spear.c: * Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/bcma-hcd.c: hci_dev->dev.dma_mask = &hci_dev->dev.coherent_dma_mask;
./drivers/usb/host/oxu210hp-hcd.c: hcd->self.controller->dma_mask = NULL;
./drivers/usb/host/oxu210hp-hcd.c: * NOTE: the dma mask is visible through dev->dma_mask, so
./drivers/usb/host/ehci-ps3.c: dev->core.dma_mask = &dummy_mask; /* FIXME: for improper usb code */
./drivers/usb/host/ohci-ps3.c: dev->core.dma_mask = &dummy_mask; /* FIXME: for improper usb code */
./drivers/usb/host/xhci.c: /* Set dma_mask and coherent_dma_mask to 64-bits,
./drivers/usb/host/ohci-exynos.c: * Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/ohci-omap3.c: * Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/ehci-omap.c: * Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/ssb-hcd.c: hci_dev->dev.dma_mask = &hci_dev->dev.coherent_dma_mask;
./drivers/usb/host/xhci-plat.c: if (WARN_ON(!pdev->dev.dma_mask))
./drivers/usb/host/xhci-plat.c: /* Platform did not initialize dma_mask */
./drivers/usb/host/u132-hcd.c: if (pdev->dev.dma_mask)
./drivers/usb/host/ehci-tegra.c: /* Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/r8a66597-hcd.c: if (pdev->dev.dma_mask) {
./drivers/usb/host/ohci-pxa27x.c: /* Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/ehci-exynos.c: * Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/fotg210-hcd.c: * NOTE: the dma mask is visible through dev->dma_mask, so
./drivers/usb/host/fsl-mph-dr-of.c: if (!pdev->dev.dma_mask)
./drivers/usb/host/fsl-mph-dr-of.c: pdev->dev.dma_mask = &ofdev->dev.coherent_dma_mask;
./drivers/usb/host/xhci-mtk.c: /* Initialize dma_mask and coherent_dma_mask to 32-bits */
./drivers/usb/host/xhci-mtk.c: if (!dev->dma_mask)
./drivers/usb/host/xhci-mtk.c: dev->dma_mask = &dev->coherent_dma_mask;
./drivers/usb/host/ehci-atmel.c: /* Right now device-tree probed devices don't get dma_mask set.
./drivers/usb/host/sl811-hcd.c: if (dev->dev.dma_mask) {
./drivers/usb/isp1760/isp1760-core.c: dev->dma_mask = NULL;
./drivers/usb/isp1760/isp1760-if.c: dev->dev.dma_mask = NULL;
./drivers/usb/misc/ftdi-elan.c: ftdi->platform_dev.dev.dma_mask = NULL;
./drivers/usb/musb/musb_core.c: if (use_dma && dev->dma_mask) {
./drivers/usb/musb/davinci.c: .dma_mask = DMA_BIT_MASK(32),
./drivers/usb/musb/omap2430.c: musb->dev.dma_mask = &omap2430_dmamask;
./drivers/usb/musb/da8xx.c: .dma_mask = DMA_BIT_MASK(32),
./drivers/usb/musb/tusb6010.c: .dma_mask = DMA_BIT_MASK(32),
./drivers/usb/musb/blackfin.c: musb->dev.dma_mask = &bfin_dmamask;
./drivers/usb/musb/musb_dsps.c: musb->dev.dma_mask = &musb_dmamask;
./drivers/usb/musb/am35x.c: .dma_mask = DMA_BIT_MASK(32),
./drivers/usb/musb/ux500.c: musb->dev.dma_mask = &pdev->dev.coherent_dma_mask;
./drivers/usb/storage/scsiglue.c: * They indicate this by setting their dma_mask to NULL. For
./drivers/usb/storage/scsiglue.c: if (!us->pusb_dev->bus->controller->dma_mask)
--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
