[PATCH 0/2] A couple of DMA teardown fixes for 3.19

[PATCH 0/2] A couple of DMA teardown fixes for 3.19

[Will Deacon]

Hi all,

Here are a couple of DMA teardown fixes for 3.19. The first patch adds
a call to of_dma_deconfigure on device destruction and the second patch
fixes a redundant "Not attached" message on the console when a device
without an IOMMU is destroyed on ARM.

I guess the second patch can go independently via rmk?

Will

Will Deacon (2):
  of/platform: teardown DMA mappings on device destruction
  ARM: dma-mapping: don't detach devices without an IOMMU during
    teardown

 arch/arm/mm/dma-mapping.c | 5 ++++-
 drivers/of/platform.c     | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.1.4

[PATCH 1/2] of/platform: teardown DMA mappings on device destruction

[Will Deacon]

Now that we can create and attach to IOMMU domains via of_dma_configure,
make sure we give the architecture a chance to tear them down when a
platform or amba device is destroyed.

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/of/platform.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 5b33c6a21807..3f61d668ded7 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -526,6 +526,7 @@ static int of_platform_device_destroy(struct device *dev, void *data)
 		amba_device_unregister(to_amba_device(dev));
 #endif
 
+	of_dma_deconfigure(dev);
 	of_node_clear_flag(dev->of_node, OF_POPULATED);
 	of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
 	return 0;
-- 
2.1.4

[PATCH 2/2] ARM: dma-mapping: don't detach devices without an IOMMU during teardown

[Will Deacon]

When tearing down the DMA ops for a device via of_dma_deconfigure, we
unconditionally detach the device from its IOMMU domain. For devices
that aren't actually behind an IOMMU, this produces a "Not attached"
warning message on the console.

This patch changes the teardown code so that we don't detach from the
IOMMU domain when there isn't an IOMMU dma mapping to start with.

Repoerted-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mm/dma-mapping.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7864797609b3..711c3d2802fb 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2023,7 +2023,10 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
 
 static void arm_teardown_iommu_dma_ops(struct device *dev)
 {
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
+
+	if (!mapping)
+		return;
 
 	arm_iommu_detach_device(dev);
 	arm_iommu_release_mapping(mapping);
-- 
2.1.4

[PATCH 2/2] ARM: dma-mapping: don't detach devices without an IOMMU during teardown

[Laurent Pinchart]

Hi Will,

Thank you for the patch.

On Monday 12 January 2015 17:23:33 Will Deacon wrote:
> When tearing down the DMA ops for a device via of_dma_deconfigure, we
> unconditionally detach the device from its IOMMU domain. For devices
> that aren't actually behind an IOMMU, this produces a "Not attached"
> warning message on the console.
> 
> This patch changes the teardown code so that we don't detach from the
> IOMMU domain when there isn't an IOMMU dma mapping to start with.
> 
> Repoerted-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I might have reported the problem, but I haven't repoerted it :-)

> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/mm/dma-mapping.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 7864797609b3..711c3d2802fb 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2023,7 +2023,10 @@ static bool arm_setup_iommu_dma_ops(struct device
> *dev, u64 dma_base, u64 size,
> 
>  static void arm_teardown_iommu_dma_ops(struct device *dev)
>  {
> -	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
> +	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);

As the function is already protected by an #ifdef CONFIG_ARM_DMA_USE_IOMMU is 
there a specific reason for this change ?

> +
> +	if (!mapping)
> +		return;
> 
>  	arm_iommu_detach_device(dev);
>  	arm_iommu_release_mapping(mapping);

-- 
Regards,

Laurent Pinchart

[PATCH 2/2] ARM: dma-mapping: don't detach devices without an IOMMU during teardown

[Will Deacon]

On Tue, Jan 13, 2015 at 02:53:44PM +0000, Laurent Pinchart wrote:
> On Monday 12 January 2015 17:23:33 Will Deacon wrote:
> > When tearing down the DMA ops for a device via of_dma_deconfigure, we
> > unconditionally detach the device from its IOMMU domain. For devices
> > that aren't actually behind an IOMMU, this produces a "Not attached"
> > warning message on the console.
> > 
> > This patch changes the teardown code so that we don't detach from the
> > IOMMU domain when there isn't an IOMMU dma mapping to start with.
> > 
> > Repoerted-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> I might have reported the problem, but I haven't repoerted it :-)

D'oh, fat fingers. Sorry!

> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm/mm/dma-mapping.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index 7864797609b3..711c3d2802fb 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -2023,7 +2023,10 @@ static bool arm_setup_iommu_dma_ops(struct device
> > *dev, u64 dma_base, u64 size,
> > 
> >  static void arm_teardown_iommu_dma_ops(struct device *dev)
> >  {
> > -	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
> > +	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
> 
> As the function is already protected by an #ifdef CONFIG_ARM_DMA_USE_IOMMU is 
> there a specific reason for this change ?

I just wanted to hide the archdata access, since we shouldn't really care
where it's stored. I could do it as a seperate patch, but I was in the
area...

Will

[PATCH 2/2] ARM: dma-mapping: don't detach devices without an IOMMU during teardown

[Laurent Pinchart]

Hi Will,

On Tuesday 13 January 2015 15:24:45 Will Deacon wrote:
> On Tue, Jan 13, 2015 at 02:53:44PM +0000, Laurent Pinchart wrote:
> > On Monday 12 January 2015 17:23:33 Will Deacon wrote:
> > > When tearing down the DMA ops for a device via of_dma_deconfigure, we
> > > unconditionally detach the device from its IOMMU domain. For devices
> > > that aren't actually behind an IOMMU, this produces a "Not attached"
> > > warning message on the console.
> > > 
> > > This patch changes the teardown code so that we don't detach from the
> > > IOMMU domain when there isn't an IOMMU dma mapping to start with.
> > > 
> > > Repoerted-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > I might have reported the problem, but I haven't repoerted it :-)
> 
> D'oh, fat fingers. Sorry!
> 
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > ---
> > > 
> > >  arch/arm/mm/dma-mapping.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > > index 7864797609b3..711c3d2802fb 100644
> > > --- a/arch/arm/mm/dma-mapping.c
> > > +++ b/arch/arm/mm/dma-mapping.c
> > > @@ -2023,7 +2023,10 @@ static bool arm_setup_iommu_dma_ops(struct device
> > > *dev, u64 dma_base, u64 size,
> > > 
> > >  static void arm_teardown_iommu_dma_ops(struct device *dev)
> > >  {
> > > 
> > > -	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
> > > +	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
> > 
> > As the function is already protected by an #ifdef CONFIG_ARM_DMA_USE_IOMMU
> > is there a specific reason for this change ?
> 
> I just wanted to hide the archdata access, since we shouldn't really care
> where it's stored. I could do it as a seperate patch, but I was in the
> area...

No issue about that, but you could then send another patch to replace all 
direct references to dev->archdata.mapping in the file.

-- 
Regards,

Laurent Pinchart