On Mon, Feb 22, 2016 at 4:07 PM, Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> 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.andersson@xxxxxxxxxx> >> >--- >> > 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. Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html