From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755870AbcBVWH4 (ORCPT ); Mon, 22 Feb 2016 17:07:56 -0500 Received: from mail-pa0-f49.google.com ([209.85.220.49]:33153 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754469AbcBVWHy (ORCPT ); Mon, 22 Feb 2016 17:07:54 -0500 Date: Mon, 22 Feb 2016 14:07:50 -0800 From: Bjorn Andersson To: Srinivas Kandagatla , Arnd Bergmann Cc: Peter Chen , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-arm-msm , linux-kernel@vger.kernel.org Subject: Re: [PATCH] usb: chipidea: Configure DMA properties and ops from DT Message-ID: <20160222220750.GI21240@tuxbot> References: <1456119133-16114-1-git-send-email-bjorn.andersson@linaro.org> <56CADCE9.9090305@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56CADCE9.9090305@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 22 Feb 02:03 PST 2016, Srinivas Kandagatla wrote: > > > On 22/02/16 05:32, Bjorn Andersson wrote: > >On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly set > >to be able to do DMA allocations, so use the of_dma_configure() helper > >to populate the dma properties and assign an appropriate dma_ops. > > > >Signed-off-by: Bjorn Andersson > >--- > > drivers/usb/chipidea/core.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > >diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > >index 7404064b9bbc..047b9d4e67aa 100644 > >--- a/drivers/usb/chipidea/core.c > >+++ b/drivers/usb/chipidea/core.c > >@@ -62,6 +62,7 @@ > > #include > > #include > > #include > >+#include > > #include > > #include > > #include > >@@ -834,6 +835,9 @@ struct platform_device *ci_hdrc_add_device(struct device *dev, > > pdev->dev.dma_parms = dev->dma_parms; > > dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask); > > > >+ if (IS_ENABLED(CONFIG_OF) && dev->of_node) > >+ of_dma_configure(&pdev->dev, dev->of_node); > >+ > Would we hit the same issue if we are on non Device tree platforms like ACPI > or platform device style itself? > As far as I can see, yes. > > > ret = platform_device_add_resources(pdev, res, nres); > > if (ret) > > goto err; > > > > I think this is the side effect of commit > 1dccb598df549d892b6450c261da54cdd7af44b4(arm64: simplify dma_get_ops) > I agree, before that we would have hit: __generic_dma_ops() { .. else if (acpi_disabled) return dma_ops; ... } with dma_ops being swiotlb_dma_ops from arm64_dma_init(). But this would not have saved us in the ACPI case, i.e. the result would have been as with my suggested patch. Poking Arnd here to see if he has any input. > None of the drivers call of_dma_configure() explicitly, which makes me feel > that we are doing something wrong. TBH, this should be handled in more > generic way rather than driver like this having an explicit call to > of_dma_configure(). > I agree, trying to figure out if it should be inherited or something. > > On the other hand, I think we could also solve the issue by using correct > device(parent device) while allocating dma/dma-pool. > Unfortunately this solves the allocation part, but in udc-core we try to map and unmap buffers based on the gadget's parent pointer (i.e. the chipidea device). I'm still puzzled to why the chipidea lives as a child device of the msm device; but as this is a rather common structure I believe this still needs to be figured out. @Arnd, the Qualcomm (msm) chipidea driver instantiates a chipidea device, which tries to do DMA mapping operations on ARM64 and fails, because we don't have dma_ops specified on the child. Using of_dma_configure() we can populate the child with appropriate settings and ops, but we would be the first driver doing so. Do you have any pointers to follow on what to do here? Regards, Bjorn