Hi all,
this series cleans up a bit how we call ->dma_configure and how
we clean up after tearing down a device that the method did set
up.
We can just use the default implementation.
Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/arm/include/asm/dma-mapping.h | 2 ++
arch/arm/mm/dma-mapping-nommu.c | 4 ----
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index 8436f6ade57d..965b7c846ecb 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -100,8 +100,10 @@ static inline unsigned long dma_max_pfn(struct device *dev)
extern void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
const struct iommu_ops *iommu, bool coherent);
+#ifdef CONFIG_MMU
#define arch_teardown_dma_ops arch_teardown_dma_ops
extern void arch_teardown_dma_ops(struct device *dev);
+#endif
/* do not use this function in a driver */
static inline bool is_device_dma_coherent(struct device *dev)
diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
index f448a0663b10..aa7aba302e76 100644
--- a/arch/arm/mm/dma-mapping-nommu.c
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -237,7 +237,3 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
set_dma_ops(dev, dma_ops);
}
-
-void arch_teardown_dma_ops(struct device *dev)
-{
-}
--
2.18.0
This goes through a lot of hooks just to call arch_teardown_dma_ops.
Replace it with a direct call instead.
Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/acpi/arm64/iort.c | 2 +-
drivers/acpi/scan.c | 10 ----------
drivers/base/dd.c | 4 ++--
drivers/of/device.c | 12 ------------
include/acpi/acpi_bus.h | 1 -
include/linux/acpi.h | 2 --
include/linux/dma-mapping.h | 6 ------
include/linux/of_device.h | 3 ---
kernel/dma/mapping.c | 6 ------
9 files changed, 3 insertions(+), 43 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 08f26db2da7e..2a361e22d38d 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1428,7 +1428,7 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node,
return 0;
dma_deconfigure:
- acpi_dma_deconfigure(&pdev->dev);
+ arch_teardown_dma_ops(&pdev->dev);
dev_put:
platform_device_put(pdev);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index e1b6231cfa1c..56676a56b3e3 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1469,16 +1469,6 @@ int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
}
EXPORT_SYMBOL_GPL(acpi_dma_configure);
-/**
- * acpi_dma_deconfigure - Tear-down DMA configuration for the device.
- * @dev: The pointer to the device
- */
-void acpi_dma_deconfigure(struct device *dev)
-{
- arch_teardown_dma_ops(dev);
-}
-EXPORT_SYMBOL_GPL(acpi_dma_deconfigure);
-
static void acpi_init_coherency(struct acpi_device *adev)
{
unsigned long long cca = 0;
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 65128cf8427c..169412ee4ae8 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -539,7 +539,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)
goto done;
probe_failed:
- dma_deconfigure(dev);
+ arch_teardown_dma_ops(dev);
dma_failed:
if (dev->bus)
blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
@@ -968,7 +968,7 @@ static void __device_release_driver(struct device *dev, struct device *parent)
drv->remove(dev);
device_links_driver_cleanup(dev);
- dma_deconfigure(dev);
+ arch_teardown_dma_ops(dev);
devres_release_all(dev);
dev->driver = NULL;
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 5957cd4fa262..c7fa5a9697c9 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -170,18 +170,6 @@ int of_dma_configure(struct device *dev, struct device_node *np, bool force_dma)
}
EXPORT_SYMBOL_GPL(of_dma_configure);
-/**
- * of_dma_deconfigure - Clean up DMA configuration
- * @dev: Device for which to clean up DMA configuration
- *
- * Clean up all configuration performed by of_dma_configure_ops() and free all
- * resources that have been allocated.
- */
-void of_dma_deconfigure(struct device *dev)
-{
- arch_teardown_dma_ops(dev);
-}
-
int of_device_register(struct platform_device *pdev)
{
device_initialize(&pdev->dev);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index ba4dd54f2c82..53600f527a70 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -595,7 +595,6 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
u64 *size);
int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
-void acpi_dma_deconfigure(struct device *dev);
struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
u64 address, bool check_children);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index de8d3d3fa651..af4628979d13 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -831,8 +831,6 @@ static inline int acpi_dma_configure(struct device *dev,
return 0;
}
-static inline void acpi_dma_deconfigure(struct device *dev) { }
-
#define ACPI_PTR(_ptr) (NULL)
static inline void acpi_device_set_enumerated(struct acpi_device *adev)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 1c6c7c09bcf2..1423b69f3cc9 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -753,12 +753,6 @@ dma_mark_declared_memory_occupied(struct device *dev,
}
#endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */
-#ifdef CONFIG_HAS_DMA
-void dma_deconfigure(struct device *dev);
-#else
-static inline void dma_deconfigure(struct device *dev) {}
-#endif
-
/*
* Managed DMA API
*/
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 165fd302b442..8d31e39dd564 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -58,7 +58,6 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
int of_dma_configure(struct device *dev,
struct device_node *np,
bool force_dma);
-void of_dma_deconfigure(struct device *dev);
#else /* CONFIG_OF */
static inline int of_driver_match_device(struct device *dev,
@@ -113,8 +112,6 @@ static inline int of_dma_configure(struct device *dev,
{
return 0;
}
-static inline void of_dma_deconfigure(struct device *dev)
-{}
#endif /* CONFIG_OF */
#endif /* _LINUX_OF_DEVICE_H */
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 25607ceb4a50..3540cb399bd2 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -327,9 +327,3 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
vunmap(cpu_addr);
}
#endif
-
-void dma_deconfigure(struct device *dev)
-{
- of_dma_deconfigure(dev);
- acpi_dma_deconfigure(dev);
-}
--
2.18.0
There is no good reason for this indirection given that the method
always exists.
Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/base/dd.c | 8 +++++---
include/linux/dma-mapping.h | 6 ------
kernel/dma/mapping.c | 10 ----------
3 files changed, 5 insertions(+), 19 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index edfc9f0b1180..65128cf8427c 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -480,9 +480,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",
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 1db6a6b46d0d..1c6c7c09bcf2 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -754,14 +754,8 @@ 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
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index d2a92ddaac4d..25607ceb4a50 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -328,16 +328,6 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
}
#endif
-/*
- * enables DMA API use for a device
- */
-int dma_configure(struct device *dev)
-{
- if (dev->bus->dma_configure)
- return dev->bus->dma_configure(dev);
- return 0;
-}
-
void dma_deconfigure(struct device *dev)
{
of_dma_deconfigure(dev);
--
2.18.0
There is no reason to leave the per-device dma_ops around when
deconfiguring a device, so move this code from arm64 into the
common code.
Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/arm64/include/asm/dma-mapping.h | 5 -----
arch/arm64/mm/dma-mapping.c | 5 -----
include/linux/dma-mapping.h | 5 ++++-
3 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index b7847eb8a7bb..0a2d13332545 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -39,11 +39,6 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
const struct iommu_ops *iommu, bool coherent);
#define arch_setup_dma_ops arch_setup_dma_ops
-#ifdef CONFIG_IOMMU_DMA
-void arch_teardown_dma_ops(struct device *dev);
-#define arch_teardown_dma_ops arch_teardown_dma_ops
-#endif
-
/* do not use this function in a driver */
static inline bool is_device_dma_coherent(struct device *dev)
{
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 072c51fb07d7..cdcb73db9ea2 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -862,11 +862,6 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
dev_name(dev));
}
-void arch_teardown_dma_ops(struct device *dev)
-{
- dev->dma_ops = NULL;
-}
-
#else
static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 1423b69f3cc9..eafd6f318e78 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -664,7 +664,10 @@ static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
#endif
#ifndef arch_teardown_dma_ops
-static inline void arch_teardown_dma_ops(struct device *dev) { }
+static inline void arch_teardown_dma_ops(struct device *dev)
+{
+ dev->dma_ops = NULL;
+}
#endif
static inline unsigned int dma_get_max_seg_size(struct device *dev)
--
2.18.0
On 27/08/18 09:47, Christoph Hellwig wrote:
> We can just use the default implementation.
Reviewed-by: Robin Murphy <[email protected]>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> arch/arm/include/asm/dma-mapping.h | 2 ++
> arch/arm/mm/dma-mapping-nommu.c | 4 ----
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
> index 8436f6ade57d..965b7c846ecb 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -100,8 +100,10 @@ static inline unsigned long dma_max_pfn(struct device *dev)
> extern void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> const struct iommu_ops *iommu, bool coherent);
>
> +#ifdef CONFIG_MMU
> #define arch_teardown_dma_ops arch_teardown_dma_ops
> extern void arch_teardown_dma_ops(struct device *dev);
> +#endif
>
> /* do not use this function in a driver */
> static inline bool is_device_dma_coherent(struct device *dev)
> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
> index f448a0663b10..aa7aba302e76 100644
> --- a/arch/arm/mm/dma-mapping-nommu.c
> +++ b/arch/arm/mm/dma-mapping-nommu.c
> @@ -237,7 +237,3 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
>
> set_dma_ops(dev, dma_ops);
> }
> -
> -void arch_teardown_dma_ops(struct device *dev)
> -{
> -}
>
On 27/08/18 09:47, Christoph Hellwig wrote:
> There is no good reason for this indirection given that the method
> always exists.
And indeed with the way it works these days we don't really want
dma_configure() to look like something which can be called from anywhere.
Reviewed-by: Robin Murphy <[email protected]>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/base/dd.c | 8 +++++---
> include/linux/dma-mapping.h | 6 ------
> kernel/dma/mapping.c | 10 ----------
> 3 files changed, 5 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index edfc9f0b1180..65128cf8427c 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -480,9 +480,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",
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 1db6a6b46d0d..1c6c7c09bcf2 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -754,14 +754,8 @@ 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
>
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index d2a92ddaac4d..25607ceb4a50 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -328,16 +328,6 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
> }
> #endif
>
> -/*
> - * enables DMA API use for a device
> - */
> -int dma_configure(struct device *dev)
> -{
> - if (dev->bus->dma_configure)
> - return dev->bus->dma_configure(dev);
> - return 0;
> -}
> -
> void dma_deconfigure(struct device *dev)
> {
> of_dma_deconfigure(dev);
>
On 27/08/18 09:47, Christoph Hellwig wrote:
> This goes through a lot of hooks just to call arch_teardown_dma_ops.
> Replace it with a direct call instead.
Agreed. We originally had the deconfigure() hooks for symmetry in case
there might need to be some firmware-specific state to tear down, but
with the benefit of a couple of years' hindsight now it really doesn't
look like that's ever likely to be necessary.
Reviewed-by: Robin Murphy <[email protected]>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/acpi/arm64/iort.c | 2 +-
> drivers/acpi/scan.c | 10 ----------
> drivers/base/dd.c | 4 ++--
> drivers/of/device.c | 12 ------------
> include/acpi/acpi_bus.h | 1 -
> include/linux/acpi.h | 2 --
> include/linux/dma-mapping.h | 6 ------
> include/linux/of_device.h | 3 ---
> kernel/dma/mapping.c | 6 ------
> 9 files changed, 3 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 08f26db2da7e..2a361e22d38d 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1428,7 +1428,7 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node,
> return 0;
>
> dma_deconfigure:
> - acpi_dma_deconfigure(&pdev->dev);
> + arch_teardown_dma_ops(&pdev->dev);
> dev_put:
> platform_device_put(pdev);
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index e1b6231cfa1c..56676a56b3e3 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1469,16 +1469,6 @@ int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
> }
> EXPORT_SYMBOL_GPL(acpi_dma_configure);
>
> -/**
> - * acpi_dma_deconfigure - Tear-down DMA configuration for the device.
> - * @dev: The pointer to the device
> - */
> -void acpi_dma_deconfigure(struct device *dev)
> -{
> - arch_teardown_dma_ops(dev);
> -}
> -EXPORT_SYMBOL_GPL(acpi_dma_deconfigure);
> -
> static void acpi_init_coherency(struct acpi_device *adev)
> {
> unsigned long long cca = 0;
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 65128cf8427c..169412ee4ae8 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -539,7 +539,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> goto done;
>
> probe_failed:
> - dma_deconfigure(dev);
> + arch_teardown_dma_ops(dev);
> dma_failed:
> if (dev->bus)
> blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
> @@ -968,7 +968,7 @@ static void __device_release_driver(struct device *dev, struct device *parent)
> drv->remove(dev);
>
> device_links_driver_cleanup(dev);
> - dma_deconfigure(dev);
> + arch_teardown_dma_ops(dev);
>
> devres_release_all(dev);
> dev->driver = NULL;
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 5957cd4fa262..c7fa5a9697c9 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -170,18 +170,6 @@ int of_dma_configure(struct device *dev, struct device_node *np, bool force_dma)
> }
> EXPORT_SYMBOL_GPL(of_dma_configure);
>
> -/**
> - * of_dma_deconfigure - Clean up DMA configuration
> - * @dev: Device for which to clean up DMA configuration
> - *
> - * Clean up all configuration performed by of_dma_configure_ops() and free all
> - * resources that have been allocated.
> - */
> -void of_dma_deconfigure(struct device *dev)
> -{
> - arch_teardown_dma_ops(dev);
> -}
> -
> int of_device_register(struct platform_device *pdev)
> {
> device_initialize(&pdev->dev);
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index ba4dd54f2c82..53600f527a70 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -595,7 +595,6 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
> int acpi_dma_get_range(struct device *dev, u64 *dma_addr, u64 *offset,
> u64 *size);
> int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
> -void acpi_dma_deconfigure(struct device *dev);
>
> struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
> u64 address, bool check_children);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index de8d3d3fa651..af4628979d13 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -831,8 +831,6 @@ static inline int acpi_dma_configure(struct device *dev,
> return 0;
> }
>
> -static inline void acpi_dma_deconfigure(struct device *dev) { }
> -
> #define ACPI_PTR(_ptr) (NULL)
>
> static inline void acpi_device_set_enumerated(struct acpi_device *adev)
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 1c6c7c09bcf2..1423b69f3cc9 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -753,12 +753,6 @@ dma_mark_declared_memory_occupied(struct device *dev,
> }
> #endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */
>
> -#ifdef CONFIG_HAS_DMA
> -void dma_deconfigure(struct device *dev);
> -#else
> -static inline void dma_deconfigure(struct device *dev) {}
> -#endif
> -
> /*
> * Managed DMA API
> */
> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
> index 165fd302b442..8d31e39dd564 100644
> --- a/include/linux/of_device.h
> +++ b/include/linux/of_device.h
> @@ -58,7 +58,6 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
> int of_dma_configure(struct device *dev,
> struct device_node *np,
> bool force_dma);
> -void of_dma_deconfigure(struct device *dev);
> #else /* CONFIG_OF */
>
> static inline int of_driver_match_device(struct device *dev,
> @@ -113,8 +112,6 @@ static inline int of_dma_configure(struct device *dev,
> {
> return 0;
> }
> -static inline void of_dma_deconfigure(struct device *dev)
> -{}
> #endif /* CONFIG_OF */
>
> #endif /* _LINUX_OF_DEVICE_H */
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index 25607ceb4a50..3540cb399bd2 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -327,9 +327,3 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
> vunmap(cpu_addr);
> }
> #endif
> -
> -void dma_deconfigure(struct device *dev)
> -{
> - of_dma_deconfigure(dev);
> - acpi_dma_deconfigure(dev);
> -}
>
On 27/08/18 09:47, Christoph Hellwig wrote:
> There is no reason to leave the per-device dma_ops around when
> deconfiguring a device, so move this code from arm64 into the
> common code.
Reviewed-by: Robin Murphy <[email protected]>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> arch/arm64/include/asm/dma-mapping.h | 5 -----
> arch/arm64/mm/dma-mapping.c | 5 -----
> include/linux/dma-mapping.h | 5 ++++-
> 3 files changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
> index b7847eb8a7bb..0a2d13332545 100644
> --- a/arch/arm64/include/asm/dma-mapping.h
> +++ b/arch/arm64/include/asm/dma-mapping.h
> @@ -39,11 +39,6 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> const struct iommu_ops *iommu, bool coherent);
> #define arch_setup_dma_ops arch_setup_dma_ops
>
> -#ifdef CONFIG_IOMMU_DMA
> -void arch_teardown_dma_ops(struct device *dev);
> -#define arch_teardown_dma_ops arch_teardown_dma_ops
> -#endif
> -
> /* do not use this function in a driver */
> static inline bool is_device_dma_coherent(struct device *dev)
> {
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 072c51fb07d7..cdcb73db9ea2 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -862,11 +862,6 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> dev_name(dev));
> }
>
> -void arch_teardown_dma_ops(struct device *dev)
> -{
> - dev->dma_ops = NULL;
> -}
> -
> #else
>
> static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 1423b69f3cc9..eafd6f318e78 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -664,7 +664,10 @@ static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
> #endif
>
> #ifndef arch_teardown_dma_ops
> -static inline void arch_teardown_dma_ops(struct device *dev) { }
> +static inline void arch_teardown_dma_ops(struct device *dev)
> +{
> + dev->dma_ops = NULL;
> +}
> #endif
>
> static inline unsigned int dma_get_max_seg_size(struct device *dev)
>
On 27/08/18 09:47, Christoph Hellwig wrote:
> We can just use the default implementation.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> arch/arm/include/asm/dma-mapping.h | 2 ++
> arch/arm/mm/dma-mapping-nommu.c | 4 ----
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
> index 8436f6ade57d..965b7c846ecb 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -100,8 +100,10 @@ static inline unsigned long dma_max_pfn(struct device *dev)
> extern void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> const struct iommu_ops *iommu, bool coherent);
>
> +#ifdef CONFIG_MMU
> #define arch_teardown_dma_ops arch_teardown_dma_ops
> extern void arch_teardown_dma_ops(struct device *dev);
> +#endif
>
> /* do not use this function in a driver */
> static inline bool is_device_dma_coherent(struct device *dev)
> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
> index f448a0663b10..aa7aba302e76 100644
> --- a/arch/arm/mm/dma-mapping-nommu.c
> +++ b/arch/arm/mm/dma-mapping-nommu.c
> @@ -237,7 +237,3 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
>
> set_dma_ops(dev, dma_ops);
> }
> -
> -void arch_teardown_dma_ops(struct device *dev)
> -{
> -}
>
FWIW:
Reviewed-by: Vladimir Murzin <[email protected]>
Thanks
Vladimir
Hi,
On Mon, Aug 27, 2018 at 10:47:11AM +0200, Christoph Hellwig wrote:
> There is no reason to leave the per-device dma_ops around when
> deconfiguring a device, so move this code from arm64 into the
> common code.
> Signed-off-by: Christoph Hellwig <[email protected]>
> Reviewed-by: Robin Murphy <[email protected]>
This patch causes various ppc images to crash in -next due to NULL
DMA ops in dma_alloc_attrs().
Looking into the code, the macio driver tries to instantiate on
the 0000:f0:0d.0 PCI address (the driver maps to all Apple PCI IDs)
and fails. This results in a call to arch_teardown_dma_ops(). Later,
the same device pointer is used to instantiate ohci-pci, which
crashes in probe because the dma_ops pointer has been cleared.
I don't claim to fully understand the code, but to me it looks like
the pci device is allocated and waiting for a driver to attach to.
See arch/powerpc/kernel/pci_of_scan.c:of_create_pci_dev().
If attaching a driver (here: macio) fails, the same device pointer
is then reused for the next matching driver until a match is found
and the device is successfully instantiated. Of course this fails
quite badly if the device pointer has been scrubbed after the first
failure.
I don't know if this is generic PCI behavior or ppc specific.
I am copying pci and ppc maintainers for additional input.
Either case, reverting the patch fixes the problem.
Guenter
---
ohci-pci 0000:f0:0d.0: OHCI PCI host controller
ohci-pci 0000:f0:0d.0: new USB bus registered, assigned bus number 1
------------[ cut here ]------------
kernel BUG at ./include/linux/dma-mapping.h:516!
Oops: Exception in kernel mode, sig: 5 [#1]
BE PREEMPT SMP NR_CPUS=4 NUMA PowerMac
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W
4.19.0-rc4-next-20180921-dirty #1
NIP: c00000000086b824 LR: c00000000086b5dc CTR: c00000000086dd70
REGS: c00000003d30f0b0 TRAP: 0700 Tainted: G W
(4.19.0-rc4-next-20180921-dirty)
MSR: 800000000002b032 <SF,EE,FP,ME,IR,DR,RI> CR: 28008842 XER: 00000000
IRQMASK: 0
GPR00: c00000000086b5dc c00000003d30f330 c000000001199900 c00000003d3ce898
GPR04: c00000000115b798 c00000003d8c3a48 0000000000000000 0000000000000000
GPR08: 0000000000000000 ffffffffffffff00 0000000000000000 0000000000000020
GPR12: 0000000024008442 c000000001299000 c00000000000f720 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000000 c0000000010452e8 c000000001045420 0000000000000080
GPR28: 000000000000001c c000000001045408 c00000003d3ce898 c00000003d8c3a30
NIP [c00000000086b824] .ohci_init+0x564/0x570
LR [c00000000086b5dc] .ohci_init+0x31c/0x570
Call Trace:
[c00000003d30f330] [c00000000086b5dc] .ohci_init+0x31c/0x570 (unreliable)
[c00000003d30f3c0] [c00000000086de58] .ohci_pci_reset+0xa8/0xb0
[c00000003d30f440] [c0000000008335ec] .usb_add_hcd+0x35c/0x9b0
[c00000003d30f510] [c00000000084ea90] .usb_hcd_pci_probe+0x320/0x510
[c00000003d30f5c0] [c0000000006c7df8] .local_pci_probe+0x68/0x140
[c00000003d30f660] [c0000000006c92a4] .pci_device_probe+0x144/0x210
[c00000003d30f710] [c00000000074cd48] .really_probe+0x2a8/0x3c0
[c00000003d30f7b0] [c00000000074d100] .driver_probe_device+0x80/0x170
[c00000003d30f840] [c00000000074d33c] .__driver_attach+0x14c/0x150
[c00000003d30f8d0] [c000000000749c6c] .bus_for_each_dev+0xac/0x100
[c00000003d30f970] [c00000000074c334] .driver_attach+0x34/0x50
[c00000003d30f9f0] [c00000000074b9b8] .bus_add_driver+0x178/0x2f0
[c00000003d30fa90] [c00000000074e560] .driver_register+0x90/0x1a0
[c00000003d30fb10] [c0000000006c707c] .__pci_register_driver+0x6c/0x90
[c00000003d30fba0] [c000000000f39f14] .ohci_pci_init+0x90/0xac
[c00000003d30fc10] [c00000000000f380] .do_one_initcall+0x70/0x2d0
[c00000003d30fce0] [c000000000edfca4] .kernel_init_freeable+0x3b8/0x4b0
[c00000003d30fdb0] [c00000000000f744] .kernel_init+0x24/0x160
[c00000003d30fe30] [c00000000000b7a4] .ret_from_kernel_thread+0x58/0x74
---
# bad: [46c163a036b41a29b0ec3c475bf97515d755ff41] Add linux-next specific files for 20180921
# good: [7876320f88802b22d4e2daf7eb027dd14175a0f8] Linux 4.19-rc4
git bisect start 'HEAD' 'v4.19-rc4'
# bad: [03b5533c4d89cc558063a98fa4201a5d2b4eb1f7] Merge remote-tracking branch 'crypto/master'
git bisect bad 03b5533c4d89cc558063a98fa4201a5d2b4eb1f7
# bad: [62c54071a46255d59e26e95528b80bf432796cb4] Merge remote-tracking branch 'v9fs/9p-next'
git bisect bad 62c54071a46255d59e26e95528b80bf432796cb4
# bad: [1eee72bfcf0977daca74e9f902956adbb4f38847] Merge remote-tracking branch 'realtek/for-next'
git bisect bad 1eee72bfcf0977daca74e9f902956adbb4f38847
# good: [30d3220045f49c707bbeec1d35423bd60488c433] Merge remote-tracking branch 'scsi-fixes/fixes'
git bisect good 30d3220045f49c707bbeec1d35423bd60488c433
# bad: [a31a9772a2aa569dc279468da4be555b737e51f8] Merge remote-tracking branch 'at91/at91-next'
git bisect bad a31a9772a2aa569dc279468da4be555b737e51f8
# bad: [3f52a0ad91857e78a5feed28327eafa11c44412c] Merge remote-tracking branch 'arm-soc/for-next'
git bisect bad 3f52a0ad91857e78a5feed28327eafa11c44412c
# good: [92c76bf9685778b2b7e5d6b4c93d74d9ef5d54a7] Merge remote-tracking branch 'leaks/leaks-next'
git bisect good 92c76bf9685778b2b7e5d6b4c93d74d9ef5d54a7
# good: [4e7afff85160ffaa236785591126cf52e11f077c] Merge branch 'fixes' into for-next
git bisect good 4e7afff85160ffaa236785591126cf52e11f077c
# bad: [5748e1b35ba28368515d850e8087929a3a65e055] MIPS: don't select DMA_MAYBE_COHERENT from DMA_PERDEV_COHERENT
git bisect bad 5748e1b35ba28368515d850e8087929a3a65e055
# good: [ccf640f4c9988653ef884672381b03b9be247bec] dma-mapping: remove dma_configure
git bisect good ccf640f4c9988653ef884672381b03b9be247bec
# bad: [46053c73685411915d3de50c5a0045beef32806b] dma-mapping: clear dev->dma_ops in arch_teardown_dma_ops
git bisect bad 46053c73685411915d3de50c5a0045beef32806b
# good: [dc3c05504d38849f77149cb962caeaedd1efa127] dma-mapping: remove dma_deconfigure
git bisect good dc3c05504d38849f77149cb962caeaedd1efa127
# first bad commit: [46053c73685411915d3de50c5a0045beef32806b] dma-mapping: clear dev->dma_ops in arch_teardown_dma_ops
Guenter Roeck <[email protected]> writes:
> Hi,
>
> On Mon, Aug 27, 2018 at 10:47:11AM +0200, Christoph Hellwig wrote:
>> There is no reason to leave the per-device dma_ops around when
>> deconfiguring a device, so move this code from arm64 into the
>> common code.
>> Signed-off-by: Christoph Hellwig <[email protected]>
>> Reviewed-by: Robin Murphy <[email protected]>
>
> This patch causes various ppc images to crash in -next due to NULL
> DMA ops in dma_alloc_attrs().
I finally remembered where you autobuilder is :)
https://kerneltests.org/builders/qemu-ppc-next/builds/970/steps/qemubuildcommand_1/logs/stdio
Looks like mac99 at least is failing.
Just reproduced it on my setup.
> Looking into the code, the macio driver tries to instantiate on
> the 0000:f0:0d.0 PCI address (the driver maps to all Apple PCI IDs)
> and fails. This results in a call to arch_teardown_dma_ops(). Later,
> the same device pointer is used to instantiate ohci-pci, which
> crashes in probe because the dma_ops pointer has been cleared.
>
> I don't claim to fully understand the code, but to me it looks like
> the pci device is allocated and waiting for a driver to attach to.
> See arch/powerpc/kernel/pci_of_scan.c:of_create_pci_dev().
> If attaching a driver (here: macio) fails, the same device pointer
> is then reused for the next matching driver until a match is found
> and the device is successfully instantiated. Of course this fails
> quite badly if the device pointer has been scrubbed after the first
> failure.
>
> I don't know if this is generic PCI behavior or ppc specific.
> I am copying pci and ppc maintainers for additional input.
I would guess this is some PPC special sauce O_o
Will have a look.
cheers
Looking at the code I think this commit is simply broken for
architectures not using arch_setup_dma_ops, but instead setting up
the dma ops through arch specific magic.
I'll revert the patch.
Christoph Hellwig <[email protected]> writes:
> Looking at the code I think this commit is simply broken for
> architectures not using arch_setup_dma_ops, but instead setting up
> the dma ops through arch specific magic.
I arch_setup_dma_ops() called from of_dma_configure(), but
pci_dma_configure() doesn't call that for this device:
static int pci_dma_configure(struct device *dev)
{
...
if (IS_ENABLED(CONFIG_OF) && bridge->parent &&
bridge->parent->of_node) {
ret = of_dma_configure(dev, bridge->parent->of_node, true);
bridge->parent is NULL.
So I don't think arch_setup_dma_ops() would help here?
> I'll revert the patch.
Thanks.
cheers
On 25/09/18 21:16, Christoph Hellwig wrote:
> Looking at the code I think this commit is simply broken for
> architectures not using arch_setup_dma_ops, but instead setting up
> the dma ops through arch specific magic.
>
> I'll revert the patch.
Ugh, sorry about missing that too. Ack to a revert - thinking about
those PPC symptoms, it might actually be that other architectures could
also get into the same pickle just by unbinding and rebinding a driver
(e.g. switching to VFIO then back again).
Robin.