On Wed, Mar 2, 2016 at 4:59 PM, Li Yang <le...@freescale.com> wrote: > On Mon, Feb 22, 2016 at 4:07 PM, Bjorn Andersson > <bjorn.anders...@linaro.org> wrote: >> 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. > > We also hit the same issue with the dwc3 driver. > >>> > >>> >Signed-off-by: Bjorn Andersson <bjorn.anders...@linaro.org> >>> >--- >>> > 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 <linux/usb/chipidea.h> >>> > #include <linux/usb/of.h> >>> > #include <linux/of.h> >>> >+#include <linux/of_device.h> >>> > #include <linux/phy.h> >>> > #include <linux/regulator/consumer.h> >>> > #include <linux/usb/ehci_def.h> >>> >@@ -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. > > I also agree. We need address it in a more generic way. I did a > search for platform_device_add()/platform_device_register() in the > kernel source code. I found a lot of them and many could be also > doing DMA. Looks like it is still too early to assume every device is > already getting dma_ops set through bus probe. Otherwise, many > drivers are potentially broken by this assumption.
Any further comment on this topic? I added the linux-arm mailing list which was missing from previous discussion. Regards, Leo -- 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