dma-mapping: move dma configuration to bus infrastructure

Message ID 1520868292-2479-1-git-send-email-nipun.gupta@nxp.com
State Not Applicable
Delegated to: Bjorn Helgaas
Headers show
Series
  • dma-mapping: move dma configuration to bus infrastructure
Related show

Commit Message

Nipun Gupta March 12, 2018, 3:24 p.m.
The change introduces 'dma_configure' & 'dma_deconfigure'as
bus callback functions so each bus can choose to implement
its own dma configuration function.
This eases the addition of new busses w.r.t. adding the dma
configuration functionality.

The change also updates the PCI, Platform and ACPI bus to use
new introduced callbacks.

Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
---
 - This patch is based on the comments on:
   https://patchwork.kernel.org/patch/10259087/
 - I have validated for PCI and platform, but not for AMBA as I
   do not have infrastructure to validate it.
   Can anyone please validate them on AMBA?

 drivers/amba/bus.c          | 38 ++++++++++++++++++++++++-----
 drivers/base/dd.c           | 14 +++++++----
 drivers/base/dma-mapping.c  | 41 -------------------------------
 drivers/base/platform.c     | 36 ++++++++++++++++++++++-----
 drivers/pci/pci-driver.c    | 59 ++++++++++++++++++++++++++++++++++++---------
 include/linux/device.h      |  6 +++++
 include/linux/dma-mapping.h | 12 ---------
 7 files changed, 124 insertions(+), 82 deletions(-)

Comments

Sinan Kaya March 12, 2018, 4:44 p.m. | #1
On 3/12/2018 11:24 AM, Nipun Gupta wrote:
> +	if (dma_dev->of_node) {
> +		ret = of_dma_configure(dev, dma_dev->of_node);
> +	} else if (has_acpi_companion(dma_dev)) {
> +		attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));
> +		if (attr != DEV_DMA_NOT_SUPPORTED)
> +			ret = acpi_dma_configure(dev, attr);
> +	}
> +
> +	pci_put_host_bridge_device(bridge);
> +
> +	return ret;
> +}
> +
> +void pci_dma_deconfigure(struct device *dev)
> +{
> +	of_dma_deconfigure(dev);
> +	acpi_dma_deconfigure(dev);
> +}

Isn't this one or the other one but not both?

Something like:

if (dev->of_node)
	of_dma_deconfigure(dev);
else
	acpi_dma_deconfigure(dev);

should work.
Nipun Gupta March 13, 2018, 4:22 a.m. | #2
> -----Original Message-----

> From: Sinan Kaya [mailto:okaya@codeaurora.org]

> Sent: Monday, March 12, 2018 22:14

> To: Nipun Gupta <nipun.gupta@nxp.com>; hch@lst.de;

> robin.murphy@arm.com; linux@armlinux.org.uk; gregkh@linuxfoundation.org;

> m.szyprowski@samsung.com; bhelgaas@google.com

> Cc: dmitry.torokhov@gmail.com; rafael.j.wysocki@intel.com;

> jarkko.sakkinen@linux.intel.com; linus.walleij@linaro.org; johan@kernel.org;

> msuchanek@suse.de; linux-kernel@vger.kernel.org; iommu@lists.linux-

> foundation.org; linux-pci@vger.kernel.org

> Subject: Re: [PATCH] dma-mapping: move dma configuration to bus

> infrastructure

> 

> On 3/12/2018 11:24 AM, Nipun Gupta wrote:

> > +	if (dma_dev->of_node) {

> > +		ret = of_dma_configure(dev, dma_dev->of_node);

> > +	} else if (has_acpi_companion(dma_dev)) {

> > +		attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev-

> >fwnode));

> > +		if (attr != DEV_DMA_NOT_SUPPORTED)

> > +			ret = acpi_dma_configure(dev, attr);

> > +	}

> > +

> > +	pci_put_host_bridge_device(bridge);

> > +

> > +	return ret;

> > +}

> > +

> > +void pci_dma_deconfigure(struct device *dev)

> > +{

> > +	of_dma_deconfigure(dev);

> > +	acpi_dma_deconfigure(dev);

> > +}

> 

> Isn't this one or the other one but not both?

> 

> Something like:

> 

> if (dev->of_node)

> 	of_dma_deconfigure(dev);

> else

> 	acpi_dma_deconfigure(dev);

> 

> should work.


I understand your point. Seems reasonable as we should not expect
the 'of/acpi DMA deconfigure' API to not fail when they are not configured.

But, here we would also need to get dma_device (just as we get in
'pci_dma_configure') and need a check on it as for PCI there 'of_node'
is present in the dma_dev.

Ill update this in v2, and also make similar changes for platform and AMBA bus.

Thanks,
Nipun

> 

> --

> Sinan Kaya

> Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm

> Technologies, Inc.

> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux

> Foundation Collaborative Project.
Christoph Hellwig March 13, 2018, 7:27 a.m. | #3
On Tue, Mar 13, 2018 at 04:22:53AM +0000, Nipun Gupta wrote:
> > Isn't this one or the other one but not both?
> > 
> > Something like:
> > 
> > if (dev->of_node)
> > 	of_dma_deconfigure(dev);
> > else
> > 	acpi_dma_deconfigure(dev);
> > 
> > should work.
> 
> I understand your point. Seems reasonable as we should not expect
> the 'of/acpi DMA deconfigure' API to not fail when they are not configured.
> 
> But, here we would also need to get dma_device (just as we get in
> 'pci_dma_configure') and need a check on it as for PCI there 'of_node'
> is present in the dma_dev.

Both of_dma_deconfigure and acpi_dma_deconfigure just end up calling
arch_teardown_dma_ops.  So my preference would be to just remove
of_dma_deconfigure and acpi_dma_deconfigure and call arch_teardown_dma_ops
as a prep patch before this one.
Christoph Hellwig March 13, 2018, 7:34 a.m. | #4
> +int amba_dma_configure(struct device *dev)
> +{
> +	enum dev_dma_attr attr;
> +	int ret = 0;
> +
> +	if (dev->of_node) {
> +		ret = of_dma_configure(dev, dev->of_node);
> +	} else if (has_acpi_companion(dev)) {
> +		attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));
> +		if (attr != DEV_DMA_NOT_SUPPORTED)
> +			ret = acpi_dma_configure(dev, attr);
> +	}
> +
> +	return ret;

This code sniplet is duplicated so many times that I think we should
just have some sort of dma_common_configure() for it that the various
busses can use.

> +void amba_dma_deconfigure(struct device *dev)
> +{
> +	of_dma_deconfigure(dev);
> +	acpi_dma_deconfigure(dev);
> +}

As mention in my previous reply I think we don't even need a deconfigure
callback at this point - just remove the ACPI and OF wrappers and
clear the dma ops.

Also in this series we should replace the force_dma flag by use of the
proper method, e.g. give a force parameter to of_dma_configure and the
new dma_common_configure helper that the busses that want it can set.
Robin Murphy March 13, 2018, 11:35 a.m. | #5
On 12/03/18 15:24, Nipun Gupta wrote:
> The change introduces 'dma_configure' & 'dma_deconfigure'as
> bus callback functions so each bus can choose to implement
> its own dma configuration function.
> This eases the addition of new busses w.r.t. adding the dma
> configuration functionality.

It's probably worth clarifying - either in the commit message, the 
kerneldoc, or both - that the bus-specific aspect is that of mapping 
between a given device on the bus and the relevant firmware description 
of its DMA configuration.

> The change also updates the PCI, Platform and ACPI bus to use
> new introduced callbacks.
> 
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
>   - This patch is based on the comments on:
>     https://patchwork.kernel.org/patch/10259087/
>   - I have validated for PCI and platform, but not for AMBA as I
>     do not have infrastructure to validate it.
>     Can anyone please validate them on AMBA?
> 
>   drivers/amba/bus.c          | 38 ++++++++++++++++++++++++-----
>   drivers/base/dd.c           | 14 +++++++----
>   drivers/base/dma-mapping.c  | 41 -------------------------------
>   drivers/base/platform.c     | 36 ++++++++++++++++++++++-----
>   drivers/pci/pci-driver.c    | 59 ++++++++++++++++++++++++++++++++++++---------
>   include/linux/device.h      |  6 +++++
>   include/linux/dma-mapping.h | 12 ---------
>   7 files changed, 124 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index 594c228..58241d2 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -20,6 +20,8 @@
>   #include <linux/sizes.h>
>   #include <linux/limits.h>
>   #include <linux/clk/clk-conf.h>
> +#include <linux/acpi.h>
> +#include <linux/of_device.h>
>   
>   #include <asm/irq.h>
>   
> @@ -171,6 +173,28 @@ static int amba_pm_runtime_resume(struct device *dev)
>   }
>   #endif /* CONFIG_PM */
>   
> +int amba_dma_configure(struct device *dev)
> +{
> +	enum dev_dma_attr attr;
> +	int ret = 0;
> +
> +	if (dev->of_node) {
> +		ret = of_dma_configure(dev, dev->of_node);
> +	} else if (has_acpi_companion(dev)) {
> +		attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));
> +		if (attr != DEV_DMA_NOT_SUPPORTED)
> +			ret = acpi_dma_configure(dev, attr);
> +	}
> +
> +	return ret;
> +}

I would be inclined to have amba_bustype just reference 
platform_dma_configure() directly rather than duplicate it like this, 
since there's no sensible reason for them to ever differ.

> +
> +void amba_dma_deconfigure(struct device *dev)
> +{
> +	of_dma_deconfigure(dev);
> +	acpi_dma_deconfigure(dev);
> +}
> +
>   static const struct dev_pm_ops amba_pm = {
>   	.suspend	= pm_generic_suspend,
>   	.resume		= pm_generic_resume,
> @@ -190,12 +214,14 @@ static int amba_pm_runtime_resume(struct device *dev)
>    * so we call the bus "amba".
>    */
>   struct bus_type amba_bustype = {
> -	.name		= "amba",
> -	.dev_groups	= amba_dev_groups,
> -	.match		= amba_match,
> -	.uevent		= amba_uevent,
> -	.pm		= &amba_pm,
> -	.force_dma	= true,
> +	.name			= "amba",
> +	.dev_groups		= amba_dev_groups,
> +	.match			= amba_match,
> +	.uevent			= amba_uevent,
> +	.pm			= &amba_pm,
> +	.dma_configure		= amba_dma_configure,
> +	.dma_deconfigure	= amba_dma_deconfigure,
> +	.force_dma		= true,

This patch should also be removing force_dma because it no longer makes 
sense. If DMA configuration is now done by a bus-level callback, then a 
bus which wants its children to get DMA configuration needs to implement 
that callback; there's nowhere to force a "default" global behaviour any 
more.

>   };
>   
>   static int __init amba_init(void)
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index de6fd09..f124f3f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -421,9 +421,11 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>   	if (ret)
>   		goto pinctrl_bind_failed;
>   
> -	ret = dma_configure(dev);
> -	if (ret)
> -		goto dma_failed;
> +	if (dev->bus->dma_configure) {
> +		ret = dev->bus->dma_configure(dev);
> +		if (ret)
> +			goto dma_failed;
> +	}
>   
>   	if (driver_sysfs_add(dev)) {
>   		printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n",
> @@ -486,7 +488,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>   	goto done;
>   
>   probe_failed:
> -	dma_deconfigure(dev);
> +	if (dev->bus->dma_deconfigure)
> +		dev->bus->dma_deconfigure(dev);
>   dma_failed:
>   	if (dev->bus)
>   		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
> @@ -895,7 +898,8 @@ static void __device_release_driver(struct device *dev, struct device *parent)
>   			drv->remove(dev);
>   
>   		device_links_driver_cleanup(dev);
> -		dma_deconfigure(dev);
> +		if (dev->bus->dma_deconfigure)
> +			dev->bus->dma_deconfigure(dev);
>   
>   		devres_release_all(dev);
>   		dev->driver = NULL;
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index 3b11835..f16bd49 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -6,11 +6,9 @@
>    * Copyright (c) 2006  Tejun Heo <teheo@suse.de>
>    */
>   
> -#include <linux/acpi.h>
>   #include <linux/dma-mapping.h>
>   #include <linux/export.h>
>   #include <linux/gfp.h>
> -#include <linux/of_device.h>
>   #include <linux/slab.h>
>   #include <linux/vmalloc.h>
>   
> @@ -329,42 +327,3 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
>   	vunmap(cpu_addr);
>   }
>   #endif
> -
> -/*
> - * Common configuration to enable DMA API use for a device
> - */
> -#include <linux/pci.h>
> -
> -int dma_configure(struct device *dev)
> -{
> -	struct device *bridge = NULL, *dma_dev = dev;
> -	enum dev_dma_attr attr;
> -	int ret = 0;
> -
> -	if (dev_is_pci(dev)) {
> -		bridge = pci_get_host_bridge_device(to_pci_dev(dev));
> -		dma_dev = bridge;
> -		if (IS_ENABLED(CONFIG_OF) && dma_dev->parent &&
> -		    dma_dev->parent->of_node)
> -			dma_dev = dma_dev->parent;
> -	}
> -
> -	if (dma_dev->of_node) {
> -		ret = of_dma_configure(dev, dma_dev->of_node);
> -	} else if (has_acpi_companion(dma_dev)) {
> -		attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));
> -		if (attr != DEV_DMA_NOT_SUPPORTED)
> -			ret = acpi_dma_configure(dev, attr);
> -	}
> -
> -	if (bridge)
> -		pci_put_host_bridge_device(bridge);
> -
> -	return ret;
> -}
> -
> -void dma_deconfigure(struct device *dev)
> -{
> -	of_dma_deconfigure(dev);
> -	acpi_dma_deconfigure(dev);
> -}
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index f1bf7b3..adf94eb 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1130,6 +1130,28 @@ int platform_pm_restore(struct device *dev)
>   
>   #endif /* CONFIG_HIBERNATE_CALLBACKS */
>   
> +int platform_dma_configure(struct device *dev)
> +{
> +	enum dev_dma_attr attr;
> +	int ret = 0;
> +
> +	if (dev->of_node) {
> +		ret = of_dma_configure(dev, dev->of_node);
> +	} else if (has_acpi_companion(dev)) {
> +		attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));
> +		if (attr != DEV_DMA_NOT_SUPPORTED)
> +			ret = acpi_dma_configure(dev, attr);
> +	}
> +
> +	return ret;
> +}
> +
> +void platform_dma_deconfigure(struct device *dev)
> +{
> +	of_dma_deconfigure(dev);
> +	acpi_dma_deconfigure(dev);
> +}
> +
>   static const struct dev_pm_ops platform_dev_pm_ops = {
>   	.runtime_suspend = pm_generic_runtime_suspend,
>   	.runtime_resume = pm_generic_runtime_resume,
> @@ -1137,12 +1159,14 @@ int platform_pm_restore(struct device *dev)
>   };
>   
>   struct bus_type platform_bus_type = {
> -	.name		= "platform",
> -	.dev_groups	= platform_dev_groups,
> -	.match		= platform_match,
> -	.uevent		= platform_uevent,
> -	.pm		= &platform_dev_pm_ops,
> -	.force_dma	= true,
> +	.name			= "platform",
> +	.dev_groups		= platform_dev_groups,
> +	.match			= platform_match,
> +	.uevent			= platform_uevent,
> +	.pm			= &platform_dev_pm_ops,
> +	.dma_configure		= platform_dma_configure,
> +	.dma_deconfigure	= platform_dma_deconfigure,
> +	.force_dma		= true,
>   };
>   EXPORT_SYMBOL_GPL(platform_bus_type);
>   
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 3bed6be..4a77814 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -18,6 +18,8 @@
>   #include <linux/pm_runtime.h>
>   #include <linux/suspend.h>
>   #include <linux/kexec.h>
> +#include <linux/acpi.h>
> +#include <linux/of_device.h>
>   #include "pci.h"
>   
>   struct pci_dynid {
> @@ -1522,19 +1524,52 @@ static int pci_bus_num_vf(struct device *dev)
>   	return pci_num_vf(to_pci_dev(dev));
>   }
>   
> +int pci_dma_configure(struct device *dev)
> +{
> +	struct device *bridge, *dma_dev;

You don't need dma_dev here; see the code removed in 09515ef5ddad for 
how the logic originally worked.

> +	enum dev_dma_attr attr;
> +	int ret = 0;
> +
> +	bridge = pci_get_host_bridge_device(to_pci_dev(dev));
> +	dma_dev = bridge;
> +	if (IS_ENABLED(CONFIG_OF) && dma_dev->parent &&
> +	    dma_dev->parent->of_node)
> +		dma_dev = dma_dev->parent;
> +
> +	if (dma_dev->of_node) {
> +		ret = of_dma_configure(dev, dma_dev->of_node);
> +	} else if (has_acpi_companion(dma_dev)) {
> +		attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));
> +		if (attr != DEV_DMA_NOT_SUPPORTED)
> +			ret = acpi_dma_configure(dev, attr);
> +	}
> +
> +	pci_put_host_bridge_device(bridge);
> +
> +	return ret;
> +}
> +
> +void pci_dma_deconfigure(struct device *dev)
> +{
> +	of_dma_deconfigure(dev);
> +	acpi_dma_deconfigure(dev);
> +}
> +
>   struct bus_type pci_bus_type = {
> -	.name		= "pci",
> -	.match		= pci_bus_match,
> -	.uevent		= pci_uevent,
> -	.probe		= pci_device_probe,
> -	.remove		= pci_device_remove,
> -	.shutdown	= pci_device_shutdown,
> -	.dev_groups	= pci_dev_groups,
> -	.bus_groups	= pci_bus_groups,
> -	.drv_groups	= pci_drv_groups,
> -	.pm		= PCI_PM_OPS_PTR,
> -	.num_vf		= pci_bus_num_vf,
> -	.force_dma	= true,
> +	.name			= "pci",
> +	.match			= pci_bus_match,
> +	.uevent			= pci_uevent,
> +	.probe			= pci_device_probe,
> +	.remove			= pci_device_remove,
> +	.shutdown		= pci_device_shutdown,
> +	.dev_groups		= pci_dev_groups,
> +	.bus_groups		= pci_bus_groups,
> +	.drv_groups		= pci_drv_groups,
> +	.pm			= PCI_PM_OPS_PTR,
> +	.num_vf			= pci_bus_num_vf,
> +	.dma_configure		= pci_dma_configure,
> +	.dma_deconfigure	= pci_dma_deconfigure,
> +	.force_dma		= true,
>   };
>   EXPORT_SYMBOL(pci_bus_type);
>   
> diff --git a/include/linux/device.h b/include/linux/device.h
> index b093405..9b2dcf6 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -88,6 +88,9 @@ extern int __must_check bus_create_file(struct bus_type *,
>    * @resume:	Called to bring a device on this bus out of sleep mode.
>    * @num_vf:	Called to find out how many virtual functions a device on this
>    *		bus supports.
> + * @dma_configure:	Called to setup DMA configuration on a device on
> +			this bus.
> + * @dma_deconfigure:	Called to tear down the DMA configuration.
>    * @pm:		Power management operations of this bus, callback the specific
>    *		device driver's pm-ops.
>    * @iommu_ops:  IOMMU specific operations for this bus, used to attach IOMMU
> @@ -130,6 +133,9 @@ struct bus_type {
>   
>   	int (*num_vf)(struct device *dev);
>   
> +	int (*dma_configure)(struct device *dev);
> +	void (*dma_deconfigure)(struct device *dev);

Seeing it laid out in the patch, I really don't think we need a 
deconfigure callback like this - the fact that we're just copy-pasting 
the existing implementation everywhere is a big hint, but more 
conceptually I can't see a good reason for it to ever need bus-specific 
behaviour in the same way that configure does.

Maybe that means we keep dma_configure() around for the sake of 
symmetry, but just reduce it to:

int dma_configure(struct device *dev)
{
	if (dev->bus->dma_configure)
		return dev->bus->dma_configure(dev);
	return 0;
}

Realistically though, dma_deconfigure() only exists for the sake of 
calling arch_teardown_dma_ops(), and that only really exists for the 
sake of the old ARM IOMMU code, so I'm not inclined to pretend it's 
anywhere near as important as the dma_configure() path in design terms.

Robin.

> +
>   	const struct dev_pm_ops *pm;
>   
>   	const struct iommu_ops *iommu_ops;
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index eb9eab4..039224b 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -761,18 +761,6 @@ void *dma_mark_declared_memory_occupied(struct device *dev,
>   }
>   #endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */
>   
> -#ifdef CONFIG_HAS_DMA
> -int dma_configure(struct device *dev);
> -void dma_deconfigure(struct device *dev);
> -#else
> -static inline int dma_configure(struct device *dev)
> -{
> -	return 0;
> -}
> -
> -static inline void dma_deconfigure(struct device *dev) {}
> -#endif
> -
>   /*
>    * Managed DMA API
>    */
>
Nipun Gupta March 13, 2018, 3:59 p.m. | #6
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@lst.de]
> Sent: Tuesday, March 13, 2018 13:05
> > +int amba_dma_configure(struct device *dev)
> > +{
> > +	enum dev_dma_attr attr;
> > +	int ret = 0;
> > +
> > +	if (dev->of_node) {
> > +		ret = of_dma_configure(dev, dev->of_node);
> > +	} else if (has_acpi_companion(dev)) {
> > +		attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));
> > +		if (attr != DEV_DMA_NOT_SUPPORTED)
> > +			ret = acpi_dma_configure(dev, attr);
> > +	}
> > +
> > +	return ret;
> 
> This code sniplet is duplicated so many times that I think we should
> just have some sort of dma_common_configure() for it that the various
> busses can use.

Agree. There is no good point in duplicating the code.
So this new API will be part of 'drivers/base/dma-mapping.c' file?

> 
> > +void amba_dma_deconfigure(struct device *dev)
> > +{
> > +	of_dma_deconfigure(dev);
> > +	acpi_dma_deconfigure(dev);
> > +}
> 
> As mention in my previous reply I think we don't even need a deconfigure
> callback at this point - just remove the ACPI and OF wrappers and
> clear the dma ops.
> 
> Also in this series we should replace the force_dma flag by use of the
> proper method, e.g. give a force parameter to of_dma_configure and the
> new dma_common_configure helper that the busses that want it can set.

I am more inclined to what Robin states in other mail to keep symmetry.
i.e. to keep dma_configure() and dma_deconfigure() and call
dev->bus->dma_configure from dma_configure(). Is this okay?

Thanks,
Nipun
Nipun Gupta March 13, 2018, 4:11 p.m. | #7
> -----Original Message-----

> From: Robin Murphy [mailto:robin.murphy@arm.com]

> Sent: Tuesday, March 13, 2018 17:06

> 

> On 12/03/18 15:24, Nipun Gupta wrote:

> > The change introduces 'dma_configure' & 'dma_deconfigure'as

> > bus callback functions so each bus can choose to implement

> > its own dma configuration function.

> > This eases the addition of new busses w.r.t. adding the dma

> > configuration functionality.

> 

> It's probably worth clarifying - either in the commit message, the

> kerneldoc, or both - that the bus-specific aspect is that of mapping

> between a given device on the bus and the relevant firmware description

> of its DMA configuration.


Okay.

>

> > The change also updates the PCI, Platform and ACPI bus to use

> > new introduced callbacks.

> >

> > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>

> > ---

> >   - This patch is based on the comments on:

> >

> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwo

> rk.kernel.org%2Fpatch%2F10259087%2F&data=02%7C01%7Cnipun.gupta%40nxp.com%7

> Cc541100ecb944e7650a408d588d69ab0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7

> C0%7C636565377665676631&sdata=k2Xjn5B1GECx4UjCg9tChOpOrD3NPM7BkzIXLLSv3rI%

> 3D&reserved=0

> >   - I have validated for PCI and platform, but not for AMBA as I

> >     do not have infrastructure to validate it.

> >     Can anyone please validate them on AMBA?

> >

> >   drivers/amba/bus.c          | 38 ++++++++++++++++++++++++-----

> >   drivers/base/dd.c           | 14 +++++++----

> >   drivers/base/dma-mapping.c  | 41 -------------------------------

> >   drivers/base/platform.c     | 36 ++++++++++++++++++++++-----

> >   drivers/pci/pci-driver.c    | 59 ++++++++++++++++++++++++++++++++++++-

> --------

> >   include/linux/device.h      |  6 +++++

> >   include/linux/dma-mapping.h | 12 ---------

> >   7 files changed, 124 insertions(+), 82 deletions(-)

> >

> > diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c

> > index 594c228..58241d2 100644

> > --- a/drivers/amba/bus.c

> > +++ b/drivers/amba/bus.c

> > @@ -20,6 +20,8 @@

> >   #include <linux/sizes.h>

> >   #include <linux/limits.h>

> >   #include <linux/clk/clk-conf.h>

> > +#include <linux/acpi.h>

> > +#include <linux/of_device.h>

> >

> >   #include <asm/irq.h>

> >

> > @@ -171,6 +173,28 @@ static int amba_pm_runtime_resume(struct device

> *dev)

> >   }

> >   #endif /* CONFIG_PM */

> >

> > +int amba_dma_configure(struct device *dev)

> > +{

> > +	enum dev_dma_attr attr;

> > +	int ret = 0;

> > +

> > +	if (dev->of_node) {

> > +		ret = of_dma_configure(dev, dev->of_node);

> > +	} else if (has_acpi_companion(dev)) {

> > +		attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));

> > +		if (attr != DEV_DMA_NOT_SUPPORTED)

> > +			ret = acpi_dma_configure(dev, attr);

> > +	}

> > +

> > +	return ret;

> > +}

> 

> I would be inclined to have amba_bustype just reference

> platform_dma_configure() directly rather than duplicate it like this,

> since there's no sensible reason for them to ever differ.


I think dma_common_configure() having this as the common code seems pretty
Decent. All the busses will probably call this API.

> 

> > +

> > +void amba_dma_deconfigure(struct device *dev)

> > +{

> > +	of_dma_deconfigure(dev);

> > +	acpi_dma_deconfigure(dev);

> > +}

> > +

> >   static const struct dev_pm_ops amba_pm = {

> >   	.suspend	= pm_generic_suspend,

> >   	.resume		= pm_generic_resume,

> > @@ -190,12 +214,14 @@ static int amba_pm_runtime_resume(struct device

> *dev)

> >    * so we call the bus "amba".

> >    */

> >   struct bus_type amba_bustype = {

> > -	.name		= "amba",

> > -	.dev_groups	= amba_dev_groups,

> > -	.match		= amba_match,

> > -	.uevent		= amba_uevent,

> > -	.pm		= &amba_pm,

> > -	.force_dma	= true,

> > +	.name			= "amba",

> > +	.dev_groups		= amba_dev_groups,

> > +	.match			= amba_match,

> > +	.uevent			= amba_uevent,

> > +	.pm			= &amba_pm,

> > +	.dma_configure		= amba_dma_configure,

> > +	.dma_deconfigure	= amba_dma_deconfigure,

> > +	.force_dma		= true,

> 

> This patch should also be removing force_dma because it no longer makes

> sense. If DMA configuration is now done by a bus-level callback, then a

> bus which wants its children to get DMA configuration needs to implement

> that callback; there's nowhere to force a "default" global behaviour any

> more.


Agree. We will also need to pass a force_dma flag in of_dma_configure() as
Christoph suggests. Ill update this.

> 

> >   };

> >

> >   static int __init amba_init(void)

> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c

> > index de6fd09..f124f3f 100644

> > --- a/drivers/base/dd.c

> > +++ b/drivers/base/dd.c

> > @@ -421,9 +421,11 @@ static int really_probe(struct device *dev, struct

> device_driver *drv)

> >   	if (ret)

> >   		goto pinctrl_bind_failed;

> >

> > -	ret = dma_configure(dev);

> > -	if (ret)

> > -		goto dma_failed;

> > +	if (dev->bus->dma_configure) {

> > +		ret = dev->bus->dma_configure(dev);

> > +		if (ret)

> > +			goto dma_failed;

> > +	}

> >

> >   	if (driver_sysfs_add(dev)) {

> >   		printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n",

> > @@ -486,7 +488,8 @@ static int really_probe(struct device *dev, struct

> device_driver *drv)

> >   	goto done;

> >

> >   probe_failed:

> > -	dma_deconfigure(dev);

> > +	if (dev->bus->dma_deconfigure)

> > +		dev->bus->dma_deconfigure(dev);

> >   dma_failed:

> >   	if (dev->bus)

> >   		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,

> > @@ -895,7 +898,8 @@ static void __device_release_driver(struct device

> *dev, struct device *parent)

> >   			drv->remove(dev);

> >

> >   		device_links_driver_cleanup(dev);

> > -		dma_deconfigure(dev);

> > +		if (dev->bus->dma_deconfigure)

> > +			dev->bus->dma_deconfigure(dev);

> >

> >   		devres_release_all(dev);

> >   		dev->driver = NULL;

> > diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c

> > index 3b11835..f16bd49 100644

> > --- a/drivers/base/dma-mapping.c

> > +++ b/drivers/base/dma-mapping.c

> > @@ -6,11 +6,9 @@

> >    * Copyright (c) 2006  Tejun Heo <teheo@suse.de>

> >    */

> >

> > -#include <linux/acpi.h>

> >   #include <linux/dma-mapping.h>

> >   #include <linux/export.h>

> >   #include <linux/gfp.h>

> > -#include <linux/of_device.h>

> >   #include <linux/slab.h>

> >   #include <linux/vmalloc.h>

> >

> > @@ -329,42 +327,3 @@ void dma_common_free_remap(void *cpu_addr, size_t

> size, unsigned long vm_flags)

> >   	vunmap(cpu_addr);

> >   }

> >   #endif

> > -

> > -/*

> > - * Common configuration to enable DMA API use for a device

> > - */

> > -#include <linux/pci.h>

> > -

> > -int dma_configure(struct device *dev)

> > -{

> > -	struct device *bridge = NULL, *dma_dev = dev;

> > -	enum dev_dma_attr attr;

> > -	int ret = 0;

> > -

> > -	if (dev_is_pci(dev)) {

> > -		bridge = pci_get_host_bridge_device(to_pci_dev(dev));

> > -		dma_dev = bridge;

> > -		if (IS_ENABLED(CONFIG_OF) && dma_dev->parent &&

> > -		    dma_dev->parent->of_node)

> > -			dma_dev = dma_dev->parent;

> > -	}

> > -

> > -	if (dma_dev->of_node) {

> > -		ret = of_dma_configure(dev, dma_dev->of_node);

> > -	} else if (has_acpi_companion(dma_dev)) {

> > -		attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));

> > -		if (attr != DEV_DMA_NOT_SUPPORTED)

> > -			ret = acpi_dma_configure(dev, attr);

> > -	}

> > -

> > -	if (bridge)

> > -		pci_put_host_bridge_device(bridge);

> > -

> > -	return ret;

> > -}

> > -

> > -void dma_deconfigure(struct device *dev)

> > -{

> > -	of_dma_deconfigure(dev);

> > -	acpi_dma_deconfigure(dev);

> > -}

> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c

> > index f1bf7b3..adf94eb 100644

> > --- a/drivers/base/platform.c

> > +++ b/drivers/base/platform.c

> > @@ -1130,6 +1130,28 @@ int platform_pm_restore(struct device *dev)

> >

> >   #endif /* CONFIG_HIBERNATE_CALLBACKS */

> >

> > +int platform_dma_configure(struct device *dev)

> > +{

> > +	enum dev_dma_attr attr;

> > +	int ret = 0;

> > +

> > +	if (dev->of_node) {

> > +		ret = of_dma_configure(dev, dev->of_node);

> > +	} else if (has_acpi_companion(dev)) {

> > +		attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));

> > +		if (attr != DEV_DMA_NOT_SUPPORTED)

> > +			ret = acpi_dma_configure(dev, attr);

> > +	}

> > +

> > +	return ret;

> > +}

> > +

> > +void platform_dma_deconfigure(struct device *dev)

> > +{

> > +	of_dma_deconfigure(dev);

> > +	acpi_dma_deconfigure(dev);

> > +}

> > +

> >   static const struct dev_pm_ops platform_dev_pm_ops = {

> >   	.runtime_suspend = pm_generic_runtime_suspend,

> >   	.runtime_resume = pm_generic_runtime_resume,

> > @@ -1137,12 +1159,14 @@ int platform_pm_restore(struct device *dev)

> >   };

> >

> >   struct bus_type platform_bus_type = {

> > -	.name		= "platform",

> > -	.dev_groups	= platform_dev_groups,

> > -	.match		= platform_match,

> > -	.uevent		= platform_uevent,

> > -	.pm		= &platform_dev_pm_ops,

> > -	.force_dma	= true,

> > +	.name			= "platform",

> > +	.dev_groups		= platform_dev_groups,

> > +	.match			= platform_match,

> > +	.uevent			= platform_uevent,

> > +	.pm			= &platform_dev_pm_ops,

> > +	.dma_configure		= platform_dma_configure,

> > +	.dma_deconfigure	= platform_dma_deconfigure,

> > +	.force_dma		= true,

> >   };

> >   EXPORT_SYMBOL_GPL(platform_bus_type);

> >

> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c

> > index 3bed6be..4a77814 100644

> > --- a/drivers/pci/pci-driver.c

> > +++ b/drivers/pci/pci-driver.c

> > @@ -18,6 +18,8 @@

> >   #include <linux/pm_runtime.h>

> >   #include <linux/suspend.h>

> >   #include <linux/kexec.h>

> > +#include <linux/acpi.h>

> > +#include <linux/of_device.h>

> >   #include "pci.h"

> >

> >   struct pci_dynid {

> > @@ -1522,19 +1524,52 @@ static int pci_bus_num_vf(struct device *dev)

> >   	return pci_num_vf(to_pci_dev(dev));

> >   }

> >

> > +int pci_dma_configure(struct device *dev)

> > +{

> > +	struct device *bridge, *dma_dev;

> 

> You don't need dma_dev here; see the code removed in 09515ef5ddad for

> how the logic originally worked.


Okay. I will have a look in this commit id.

> 

> > +	enum dev_dma_attr attr;

> > +	int ret = 0;

> > +

> > +	bridge = pci_get_host_bridge_device(to_pci_dev(dev));

> > +	dma_dev = bridge;

> > +	if (IS_ENABLED(CONFIG_OF) && dma_dev->parent &&

> > +	    dma_dev->parent->of_node)

> > +		dma_dev = dma_dev->parent;

> > +

> > +	if (dma_dev->of_node) {

> > +		ret = of_dma_configure(dev, dma_dev->of_node);

> > +	} else if (has_acpi_companion(dma_dev)) {

> > +		attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));

> > +		if (attr != DEV_DMA_NOT_SUPPORTED)

> > +			ret = acpi_dma_configure(dev, attr);

> > +	}

> > +

> > +	pci_put_host_bridge_device(bridge);

> > +

> > +	return ret;

> > +}

> > +

> > +void pci_dma_deconfigure(struct device *dev)

> > +{

> > +	of_dma_deconfigure(dev);

> > +	acpi_dma_deconfigure(dev);

> > +}

> > +

> >   struct bus_type pci_bus_type = {

> > -	.name		= "pci",

> > -	.match		= pci_bus_match,

> > -	.uevent		= pci_uevent,

> > -	.probe		= pci_device_probe,

> > -	.remove		= pci_device_remove,

> > -	.shutdown	= pci_device_shutdown,

> > -	.dev_groups	= pci_dev_groups,

> > -	.bus_groups	= pci_bus_groups,

> > -	.drv_groups	= pci_drv_groups,

> > -	.pm		= PCI_PM_OPS_PTR,

> > -	.num_vf		= pci_bus_num_vf,

> > -	.force_dma	= true,

> > +	.name			= "pci",

> > +	.match			= pci_bus_match,

> > +	.uevent			= pci_uevent,

> > +	.probe			= pci_device_probe,

> > +	.remove			= pci_device_remove,

> > +	.shutdown		= pci_device_shutdown,

> > +	.dev_groups		= pci_dev_groups,

> > +	.bus_groups		= pci_bus_groups,

> > +	.drv_groups		= pci_drv_groups,

> > +	.pm			= PCI_PM_OPS_PTR,

> > +	.num_vf			= pci_bus_num_vf,

> > +	.dma_configure		= pci_dma_configure,

> > +	.dma_deconfigure	= pci_dma_deconfigure,

> > +	.force_dma		= true,

> >   };

> >   EXPORT_SYMBOL(pci_bus_type);

> >

> > diff --git a/include/linux/device.h b/include/linux/device.h

> > index b093405..9b2dcf6 100644

> > --- a/include/linux/device.h

> > +++ b/include/linux/device.h

> > @@ -88,6 +88,9 @@ extern int __must_check bus_create_file(struct

> bus_type *,

> >    * @resume:	Called to bring a device on this bus out of sleep mode.

> >    * @num_vf:	Called to find out how many virtual functions a device on

> this

> >    *		bus supports.

> > + * @dma_configure:	Called to setup DMA configuration on a device on

> > +			this bus.

> > + * @dma_deconfigure:	Called to tear down the DMA configuration.

> >    * @pm:		Power management operations of this bus, callback

> the specific

> >    *		device driver's pm-ops.

> >    * @iommu_ops:  IOMMU specific operations for this bus, used to attach

> IOMMU

> > @@ -130,6 +133,9 @@ struct bus_type {

> >

> >   	int (*num_vf)(struct device *dev);

> >

> > +	int (*dma_configure)(struct device *dev);

> > +	void (*dma_deconfigure)(struct device *dev);

> 

> Seeing it laid out in the patch, I really don't think we need a

> deconfigure callback like this - the fact that we're just copy-pasting

> the existing implementation everywhere is a big hint, but more

> conceptually I can't see a good reason for it to ever need bus-specific

> behaviour in the same way that configure does.

> 

> Maybe that means we keep dma_configure() around for the sake of

> symmetry, but just reduce it to:

> 

> int dma_configure(struct device *dev)

> {

> 	if (dev->bus->dma_configure)

> 		return dev->bus->dma_configure(dev);

> 	return 0;

> }

> 

> Realistically though, dma_deconfigure() only exists for the sake of

> calling arch_teardown_dma_ops(), and that only really exists for the

> sake of the old ARM IOMMU code, so I'm not inclined to pretend it's

> anywhere near as important as the dma_configure() path in design terms.


Yes, I will remove dma_deconfigure callback.

Thanks,
Nipun

> 

> Robin.

> 

> > +

> >   	const struct dev_pm_ops *pm;

> >

> >   	const struct iommu_ops *iommu_ops;

> > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h

> > index eb9eab4..039224b 100644

> > --- a/include/linux/dma-mapping.h

> > +++ b/include/linux/dma-mapping.h

> > @@ -761,18 +761,6 @@ void *dma_mark_declared_memory_occupied(struct

> device *dev,

> >   }

> >   #endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */

> >

> > -#ifdef CONFIG_HAS_DMA

> > -int dma_configure(struct device *dev);

> > -void dma_deconfigure(struct device *dev);

> > -#else

> > -static inline int dma_configure(struct device *dev)

> > -{

> > -	return 0;

> > -}

> > -

> > -static inline void dma_deconfigure(struct device *dev) {}

> > -#endif

> > -

> >   /*

> >    * Managed DMA API

> >    */

> >
kbuild test robot March 13, 2018, 9:53 p.m. | #8
Hi Nipun,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc5 next-20180313]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nipun-Gupta/dma-mapping-move-dma-configuration-to-bus-infrastructure/20180313-225250
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/base/platform.c:1133:5: sparse: symbol 'platform_dma_configure' was not declared. Should it be static?
>> drivers/base/platform.c:1149:6: sparse: symbol 'platform_dma_deconfigure' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Christoph Hellwig March 14, 2018, 9:02 a.m. | #9
>> +	.dev_groups		= amba_dev_groups,
>> +	.match			= amba_match,
>> +	.uevent			= amba_uevent,
>> +	.pm			= &amba_pm,
>> +	.dma_configure		= amba_dma_configure,
>> +	.dma_deconfigure	= amba_dma_deconfigure,
>> +	.force_dma		= true,
>
> This patch should also be removing force_dma because it no longer makes 
> sense. If DMA configuration is now done by a bus-level callback, then a bus 
> which wants its children to get DMA configuration needs to implement that 
> callback; there's nowhere to force a "default" global behaviour any more.

Btw, we don't really know how many busses currently rely on OF or ACPI
configuration.  So maybe we need to keep those as a default?
Christoph Hellwig March 14, 2018, 9:03 a.m. | #10
> Agree. There is no good point in duplicating the code.
> So this new API will be part of 'drivers/base/dma-mapping.c' file?

Yes.

> > As mention in my previous reply I think we don't even need a deconfigure
> > callback at this point - just remove the ACPI and OF wrappers and
> > clear the dma ops.
> > 
> > Also in this series we should replace the force_dma flag by use of the
> > proper method, e.g. give a force parameter to of_dma_configure and the
> > new dma_common_configure helper that the busses that want it can set.
> 
> I am more inclined to what Robin states in other mail to keep symmetry.
> i.e. to keep dma_configure() and dma_deconfigure() and call
> dev->bus->dma_configure from dma_configure(). Is this okay?

Sure.
Christoph Hellwig March 30, 2018, 7:15 a.m. | #11
Can you resend the current state of affairs so we can get it in for
4.17?
Nipun Gupta March 30, 2018, 7:17 a.m. | #12
I am just going to send it within an hour :)

Thanks,
Nipun

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@lst.de]
> Sent: Friday, March 30, 2018 12:46
> To: Nipun Gupta <nipun.gupta@nxp.com>
> Cc: hch@lst.de; robin.murphy@arm.com; linux@armlinux.org.uk;
> gregkh@linuxfoundation.org; m.szyprowski@samsung.com; bhelgaas@google.com;
> dmitry.torokhov@gmail.com; rafael.j.wysocki@intel.com;
> jarkko.sakkinen@linux.intel.com; linus.walleij@linaro.org;
> johan@kernel.org; msuchanek@suse.de; linux-kernel@vger.kernel.org;
> iommu@lists.linux-foundation.org; linux-pci@vger.kernel.org
> Subject: Re: [PATCH] dma-mapping: move dma configuration to bus
> infrastructure
> 
> Can you resend the current state of affairs so we can get it in for
> 4.17?
Greg Kroah-Hartman March 30, 2018, 9:05 a.m. | #13
On Fri, Mar 30, 2018 at 09:15:30AM +0200, Christoph Hellwig wrote:
> Can you resend the current state of affairs so we can get it in for
> 4.17?

Changes to the driver core and 3 different busses 2 days before 4.16 is
out, preventing any testing at all in linux-next?  This needs to wait
for the next merge window, sorry.  I'm away all this weekend, and can't
test on my end, sorry.

greg k-h

Patch

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 594c228..58241d2 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -20,6 +20,8 @@ 
 #include <linux/sizes.h>
 #include <linux/limits.h>
 #include <linux/clk/clk-conf.h>
+#include <linux/acpi.h>
+#include <linux/of_device.h>
 
 #include <asm/irq.h>
 
@@ -171,6 +173,28 @@  static int amba_pm_runtime_resume(struct device *dev)
 }
 #endif /* CONFIG_PM */
 
+int amba_dma_configure(struct device *dev)
+{
+	enum dev_dma_attr attr;
+	int ret = 0;
+
+	if (dev->of_node) {
+		ret = of_dma_configure(dev, dev->of_node);
+	} else if (has_acpi_companion(dev)) {
+		attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));
+		if (attr != DEV_DMA_NOT_SUPPORTED)
+			ret = acpi_dma_configure(dev, attr);
+	}
+
+	return ret;
+}
+
+void amba_dma_deconfigure(struct device *dev)
+{
+	of_dma_deconfigure(dev);
+	acpi_dma_deconfigure(dev);
+}
+
 static const struct dev_pm_ops amba_pm = {
 	.suspend	= pm_generic_suspend,
 	.resume		= pm_generic_resume,
@@ -190,12 +214,14 @@  static int amba_pm_runtime_resume(struct device *dev)
  * so we call the bus "amba".
  */
 struct bus_type amba_bustype = {
-	.name		= "amba",
-	.dev_groups	= amba_dev_groups,
-	.match		= amba_match,
-	.uevent		= amba_uevent,
-	.pm		= &amba_pm,
-	.force_dma	= true,
+	.name			= "amba",
+	.dev_groups		= amba_dev_groups,
+	.match			= amba_match,
+	.uevent			= amba_uevent,
+	.pm			= &amba_pm,
+	.dma_configure		= amba_dma_configure,
+	.dma_deconfigure	= amba_dma_deconfigure,
+	.force_dma		= true,
 };
 
 static int __init amba_init(void)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index de6fd09..f124f3f 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -421,9 +421,11 @@  static int really_probe(struct device *dev, struct device_driver *drv)
 	if (ret)
 		goto pinctrl_bind_failed;
 
-	ret = dma_configure(dev);
-	if (ret)
-		goto dma_failed;
+	if (dev->bus->dma_configure) {
+		ret = dev->bus->dma_configure(dev);
+		if (ret)
+			goto dma_failed;
+	}
 
 	if (driver_sysfs_add(dev)) {
 		printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n",
@@ -486,7 +488,8 @@  static int really_probe(struct device *dev, struct device_driver *drv)
 	goto done;
 
 probe_failed:
-	dma_deconfigure(dev);
+	if (dev->bus->dma_deconfigure)
+		dev->bus->dma_deconfigure(dev);
 dma_failed:
 	if (dev->bus)
 		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
@@ -895,7 +898,8 @@  static void __device_release_driver(struct device *dev, struct device *parent)
 			drv->remove(dev);
 
 		device_links_driver_cleanup(dev);
-		dma_deconfigure(dev);
+		if (dev->bus->dma_deconfigure)
+			dev->bus->dma_deconfigure(dev);
 
 		devres_release_all(dev);
 		dev->driver = NULL;
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 3b11835..f16bd49 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -6,11 +6,9 @@ 
  * Copyright (c) 2006  Tejun Heo <teheo@suse.de>
  */
 
-#include <linux/acpi.h>
 #include <linux/dma-mapping.h>
 #include <linux/export.h>
 #include <linux/gfp.h>
-#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 
@@ -329,42 +327,3 @@  void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
 	vunmap(cpu_addr);
 }
 #endif
-
-/*
- * Common configuration to enable DMA API use for a device
- */
-#include <linux/pci.h>
-
-int dma_configure(struct device *dev)
-{
-	struct device *bridge = NULL, *dma_dev = dev;
-	enum dev_dma_attr attr;
-	int ret = 0;
-
-	if (dev_is_pci(dev)) {
-		bridge = pci_get_host_bridge_device(to_pci_dev(dev));
-		dma_dev = bridge;
-		if (IS_ENABLED(CONFIG_OF) && dma_dev->parent &&
-		    dma_dev->parent->of_node)
-			dma_dev = dma_dev->parent;
-	}
-
-	if (dma_dev->of_node) {
-		ret = of_dma_configure(dev, dma_dev->of_node);
-	} else if (has_acpi_companion(dma_dev)) {
-		attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));
-		if (attr != DEV_DMA_NOT_SUPPORTED)
-			ret = acpi_dma_configure(dev, attr);
-	}
-
-	if (bridge)
-		pci_put_host_bridge_device(bridge);
-
-	return ret;
-}
-
-void dma_deconfigure(struct device *dev)
-{
-	of_dma_deconfigure(dev);
-	acpi_dma_deconfigure(dev);
-}
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index f1bf7b3..adf94eb 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1130,6 +1130,28 @@  int platform_pm_restore(struct device *dev)
 
 #endif /* CONFIG_HIBERNATE_CALLBACKS */
 
+int platform_dma_configure(struct device *dev)
+{
+	enum dev_dma_attr attr;
+	int ret = 0;
+
+	if (dev->of_node) {
+		ret = of_dma_configure(dev, dev->of_node);
+	} else if (has_acpi_companion(dev)) {
+		attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));
+		if (attr != DEV_DMA_NOT_SUPPORTED)
+			ret = acpi_dma_configure(dev, attr);
+	}
+
+	return ret;
+}
+
+void platform_dma_deconfigure(struct device *dev)
+{
+	of_dma_deconfigure(dev);
+	acpi_dma_deconfigure(dev);
+}
+
 static const struct dev_pm_ops platform_dev_pm_ops = {
 	.runtime_suspend = pm_generic_runtime_suspend,
 	.runtime_resume = pm_generic_runtime_resume,
@@ -1137,12 +1159,14 @@  int platform_pm_restore(struct device *dev)
 };
 
 struct bus_type platform_bus_type = {
-	.name		= "platform",
-	.dev_groups	= platform_dev_groups,
-	.match		= platform_match,
-	.uevent		= platform_uevent,
-	.pm		= &platform_dev_pm_ops,
-	.force_dma	= true,
+	.name			= "platform",
+	.dev_groups		= platform_dev_groups,
+	.match			= platform_match,
+	.uevent			= platform_uevent,
+	.pm			= &platform_dev_pm_ops,
+	.dma_configure		= platform_dma_configure,
+	.dma_deconfigure	= platform_dma_deconfigure,
+	.force_dma		= true,
 };
 EXPORT_SYMBOL_GPL(platform_bus_type);
 
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 3bed6be..4a77814 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -18,6 +18,8 @@ 
 #include <linux/pm_runtime.h>
 #include <linux/suspend.h>
 #include <linux/kexec.h>
+#include <linux/acpi.h>
+#include <linux/of_device.h>
 #include "pci.h"
 
 struct pci_dynid {
@@ -1522,19 +1524,52 @@  static int pci_bus_num_vf(struct device *dev)
 	return pci_num_vf(to_pci_dev(dev));
 }
 
+int pci_dma_configure(struct device *dev)
+{
+	struct device *bridge, *dma_dev;
+	enum dev_dma_attr attr;
+	int ret = 0;
+
+	bridge = pci_get_host_bridge_device(to_pci_dev(dev));
+	dma_dev = bridge;
+	if (IS_ENABLED(CONFIG_OF) && dma_dev->parent &&
+	    dma_dev->parent->of_node)
+		dma_dev = dma_dev->parent;
+
+	if (dma_dev->of_node) {
+		ret = of_dma_configure(dev, dma_dev->of_node);
+	} else if (has_acpi_companion(dma_dev)) {
+		attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));
+		if (attr != DEV_DMA_NOT_SUPPORTED)
+			ret = acpi_dma_configure(dev, attr);
+	}
+
+	pci_put_host_bridge_device(bridge);
+
+	return ret;
+}
+
+void pci_dma_deconfigure(struct device *dev)
+{
+	of_dma_deconfigure(dev);
+	acpi_dma_deconfigure(dev);
+}
+
 struct bus_type pci_bus_type = {
-	.name		= "pci",
-	.match		= pci_bus_match,
-	.uevent		= pci_uevent,
-	.probe		= pci_device_probe,
-	.remove		= pci_device_remove,
-	.shutdown	= pci_device_shutdown,
-	.dev_groups	= pci_dev_groups,
-	.bus_groups	= pci_bus_groups,
-	.drv_groups	= pci_drv_groups,
-	.pm		= PCI_PM_OPS_PTR,
-	.num_vf		= pci_bus_num_vf,
-	.force_dma	= true,
+	.name			= "pci",
+	.match			= pci_bus_match,
+	.uevent			= pci_uevent,
+	.probe			= pci_device_probe,
+	.remove			= pci_device_remove,
+	.shutdown		= pci_device_shutdown,
+	.dev_groups		= pci_dev_groups,
+	.bus_groups		= pci_bus_groups,
+	.drv_groups		= pci_drv_groups,
+	.pm			= PCI_PM_OPS_PTR,
+	.num_vf			= pci_bus_num_vf,
+	.dma_configure		= pci_dma_configure,
+	.dma_deconfigure	= pci_dma_deconfigure,
+	.force_dma		= true,
 };
 EXPORT_SYMBOL(pci_bus_type);
 
diff --git a/include/linux/device.h b/include/linux/device.h
index b093405..9b2dcf6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -88,6 +88,9 @@  extern int __must_check bus_create_file(struct bus_type *,
  * @resume:	Called to bring a device on this bus out of sleep mode.
  * @num_vf:	Called to find out how many virtual functions a device on this
  *		bus supports.
+ * @dma_configure:	Called to setup DMA configuration on a device on
+			this bus.
+ * @dma_deconfigure:	Called to tear down the DMA configuration.
  * @pm:		Power management operations of this bus, callback the specific
  *		device driver's pm-ops.
  * @iommu_ops:  IOMMU specific operations for this bus, used to attach IOMMU
@@ -130,6 +133,9 @@  struct bus_type {
 
 	int (*num_vf)(struct device *dev);
 
+	int (*dma_configure)(struct device *dev);
+	void (*dma_deconfigure)(struct device *dev);
+
 	const struct dev_pm_ops *pm;
 
 	const struct iommu_ops *iommu_ops;
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index eb9eab4..039224b 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -761,18 +761,6 @@  void *dma_mark_declared_memory_occupied(struct device *dev,
 }
 #endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */
 
-#ifdef CONFIG_HAS_DMA
-int dma_configure(struct device *dev);
-void dma_deconfigure(struct device *dev);
-#else
-static inline int dma_configure(struct device *dev)
-{
-	return 0;
-}
-
-static inline void dma_deconfigure(struct device *dev) {}
-#endif
-
 /*
  * Managed DMA API
  */