Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1330817 > unrolled thread

[PATCH v3 0/8] dmaengine: rcar-dmac: add iommu support for slave transfers

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/8] dmaengine: rcar-dmac: add iommu support for slave transfers Niklas Söderlund   <niklas.soderlund+renesas@ragnatech.se> - 2016-02-10 02:10 +0100
    [PATCH v3 1/8] iommu: Add MMIO mapping type Niklas Söderlund   <niklas.soderlund+renesas@ragnatech.se> - 2016-02-10 02:10 +0100
      Re: [PATCH v3 1/8] iommu: Add MMIO mapping type Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-02-11 01:10 +0100
        Re: [PATCH v3 1/8] iommu: Add MMIO mapping type Robin Murphy <robin.murphy@arm.com> - 2016-02-11 17:00 +0100
          Re: [PATCH v3 1/8] iommu: Add MMIO mapping type "Niklas Söderlund" <niklas.soderlund@ragnatech.se> - 2016-02-16 13:10 +0100
            Re: [PATCH v3 1/8] iommu: Add MMIO mapping type Robin Murphy <robin.murphy@arm.com> - 2016-02-16 13:50 +0100
              Re: [PATCH v3 1/8] iommu: Add MMIO mapping type Niklas Söderlund   <niklas.soderlund@ragnatech.se> - 2016-02-16 14:40 +0100
    [PATCH v3 5/8] dmaengine: rcar-dmac: group slave configuration Niklas Söderlund   <niklas.soderlund+renesas@ragnatech.se> - 2016-02-10 02:10 +0100
      Re: [PATCH v3 5/8] dmaengine: rcar-dmac: group slave configuration Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-02-11 01:20 +0100
    [PATCH v3 6/8] dmaengine: rcar-dmac: add iommu support for slave transfers Niklas Söderlund   <niklas.soderlund+renesas@ragnatech.se> - 2016-02-10 02:10 +0100
      Re: [PATCH v3 6/8] dmaengine: rcar-dmac: add iommu support for slave  transfers Robin Murphy <robin.murphy@arm.com> - 2016-02-10 11:50 +0100
      Re: [PATCH v3 6/8] dmaengine: rcar-dmac: add iommu support for slave transfers Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-02-11 01:40 +0100
    [PATCH v3 3/8] dma-mapping: add dma_{map,unmap}_resource Niklas Söderlund   <niklas.soderlund+renesas@ragnatech.se> - 2016-02-10 02:10 +0100
      Re: [PATCH v3 3/8] dma-mapping: add dma_{map,unmap}_resource Robin Murphy <robin.murphy@arm.com> - 2016-02-10 11:30 +0100
    [PATCH v3 7/8] ARM: dts: r8a7790: add iommus to dmac0 and dmac1 Niklas Söderlund   <niklas.soderlund+renesas@ragnatech.se> - 2016-02-10 02:10 +0100
      Re: [PATCH v3 7/8] ARM: dts: r8a7790: add iommus to dmac0 and dmac1 Simon Horman <horms@verge.net.au> - 2016-02-10 19:00 +0100
        Re: [PATCH v3 7/8] ARM: dts: r8a7790: add iommus to dmac0 and dmac1 "Niklas Söderlund" <niklas.soderlund@ragnatech.se> - 2016-02-11 02:00 +0100
      Re: [PATCH v3 7/8] ARM: dts: r8a7790: add iommus to dmac0 and dmac1 Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-02-11 01:50 +0100

#1330817 — [PATCH v3 0/8] dmaengine: rcar-dmac: add iommu support for slave transfers

Hi,

This series add iommu support to rcar-dmac. It's tested on koelsch with
CONFIG_IPMMU_VMSA and by enabling the ipmmu_ds node in r8a7791.dtsi. I
verified operation by interacting with /dev/mmcblk1  which is a device
behind the iommu.

The series depends on out of tree patch '[PATCH] dmaengine: use
phys_addr_t for slave configuration' which currently is under review.

* Changes since v2
- Drop patch to add dma_{map,unmap}_page_attrs.
- Add dma_{map,unmap}_resource to handle the mapping without involving a
  'struct page'. Thanks Laurent and Robin for pointing this out.
- Use size instead of address to keep track of if a mapping exist or not
  since addr == 0 is valid. Thanks Laurent.
- Pick up patch from Robin with Laurents ack (hope it's OK for me to
  attach the ack?) to add IOMMU_MMIO.
- Fix bug in rcar_dmac_device_config where the error check where
  inverted.
- Use DMA_BIDIRECTIONAL in rcar_dmac_device_config since we at that
  point can't be sure what direction the mapping is going to be used.

* Changes since v1
- Add and use a dma_{map,unmap}_page_attrs to be able to map the page
  using attributes DMA_ATTR_NO_KERNEL_MAPPING and
  DMA_ATTR_SKIP_CPU_SYNC. Thanks Laurent.
- Drop check if dmac is part of a iommu group or not, let the DMA
  mapping api handle it.
- Move slave configuration data around in rcar-dmac to avoid code
  duplication.
- Fix build issue reported by 'kbuild test robot' regarding phys_to_page
  not availability on some configurations.
- Add DT information for r8a7791.

* Changes since RFC
- Switch to use the dma-mapping api instead of using the iommu_map()
  directly. Turns out the dma-mapper is much smarter then me...
- Dropped the patch to expose domain->ops->pgsize_bitmap from within the
  iommu api.
- Dropped the patch showing how I tested the RFC.

Niklas Söderlund (7):
  dma-mapping: add {map,unmap}_resource to dma_map_ops
  dma-mapping: add dma_{map,unmap}_resource
  arm: dma-mapping: add {map,unmap}_resource for iommu ops
  dmaengine: rcar-dmac: group slave configuration
  dmaengine: rcar-dmac: add iommu support for slave transfers
  ARM: dts: r8a7790: add iommus to dmac0 and dmac1
  ARM: dts: r8a7791: add iommus to dmac0 and dmac1

Robin Murphy (1):
  iommu: Add MMIO mapping type

 arch/arm/boot/dts/r8a7790.dtsi | 30 +++++++++++++++
 arch/arm/boot/dts/r8a7791.dtsi | 30 +++++++++++++++
 arch/arm/mm/dma-mapping.c      | 63 +++++++++++++++++++++++++++++++
 drivers/dma/sh/rcar-dmac.c     | 86 +++++++++++++++++++++++++++++++++---------
 drivers/iommu/io-pgtable-arm.c |  4 +-
 include/linux/dma-mapping.h    | 33 ++++++++++++++++
 include/linux/iommu.h          |  1 +
 7 files changed, 229 insertions(+), 18 deletions(-)

--
2.7.1

[toc] | [next] | [standalone]

#1330818 — [PATCH v3 1/8] iommu: Add MMIO mapping type

From: Robin Murphy <robin.murphy@arm.com>

On some platforms, MMIO regions might need slightly different treatment
compared to mapping regular memory; add the notion of MMIO mappings to
the IOMMU API's memory type flags, so that callers can let the IOMMU
drivers know to do the right thing.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/iommu/io-pgtable-arm.c | 4 +++-
 include/linux/iommu.h          | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 381ca5a..3ff4f87 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data,
 			pte |= ARM_LPAE_PTE_HAP_READ;
 		if (prot & IOMMU_WRITE)
 			pte |= ARM_LPAE_PTE_HAP_WRITE;
-		if (prot & IOMMU_CACHE)
+		if (prot & IOMMU_MMIO)
+			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
+		else if (prot & IOMMU_CACHE)
 			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
 		else
 			pte |= ARM_LPAE_PTE_MEMATTR_NC;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a5c539f..34b6432 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -30,6 +30,7 @@
 #define IOMMU_WRITE	(1 << 1)
 #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
 #define IOMMU_NOEXEC	(1 << 3)
+#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */
 
 struct iommu_ops;
 struct iommu_group;
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1331593 — Re: [PATCH v3 1/8] iommu: Add MMIO mapping type

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:51 Niklas Söderlund wrote:
> From: Robin Murphy <robin.murphy@arm.com>
> 
> On some platforms, MMIO regions might need slightly different treatment
> compared to mapping regular memory; add the notion of MMIO mappings to
> the IOMMU API's memory type flags, so that callers can let the IOMMU
> drivers know to do the right thing.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Answering the question from the cover letter, yes, it's totally fine to pick 
the ack, that's actually expected.

> ---
>  drivers/iommu/io-pgtable-arm.c | 4 +++-
>  include/linux/iommu.h          | 1 +

You might be asked to split this patch in two.

>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 381ca5a..3ff4f87 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
> arm_lpae_io_pgtable *data, pte |= ARM_LPAE_PTE_HAP_READ;
>  		if (prot & IOMMU_WRITE)
>  			pte |= ARM_LPAE_PTE_HAP_WRITE;
> -		if (prot & IOMMU_CACHE)
> +		if (prot & IOMMU_MMIO)
> +			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
> +		else if (prot & IOMMU_CACHE)
>  			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
>  		else
>  			pte |= ARM_LPAE_PTE_MEMATTR_NC;
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index a5c539f..34b6432 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -30,6 +30,7 @@
>  #define IOMMU_WRITE	(1 << 1)
>  #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
>  #define IOMMU_NOEXEC	(1 << 3)
> +#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */
> 
>  struct iommu_ops;
>  struct iommu_group;

-- 
Regards,

Laurent Pinchart

[toc] | [prev] | [next] | [standalone]

#1332175 — Re: [PATCH v3 1/8] iommu: Add MMIO mapping type

On 11/02/16 00:02, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Wednesday 10 February 2016 01:57:51 Niklas Söderlund wrote:
>> From: Robin Murphy <robin.murphy@arm.com>
>>
>> On some platforms, MMIO regions might need slightly different treatment
>> compared to mapping regular memory; add the notion of MMIO mappings to
>> the IOMMU API's memory type flags, so that callers can let the IOMMU
>> drivers know to do the right thing.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Answering the question from the cover letter, yes, it's totally fine to pick
> the ack, that's actually expected.
>
>> ---
>>   drivers/iommu/io-pgtable-arm.c | 4 +++-
>>   include/linux/iommu.h          | 1 +
>
> You might be asked to split this patch in two.

Worse than that, you might also be asked to fix it up when the silly 
author remembers that he did this on a stage-2-only ARM SMMU, and the 
attributes for the stage 1 tables that the IPMMU uses are in a different 
code path:

--->8---
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 5b5c299..7622c6e 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -354,7 +354,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct 
arm_lpae_io_pgtable *data,
                 if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
                         pte |= ARM_LPAE_PTE_AP_RDONLY;

-               if (prot & IOMMU_CACHE)
+               if (prot & IOMMU_MMIO)
+                       pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
+                               << ARM_LPAE_PTE_ATTRINDX_SHIFT);
+               else if (prot & IOMMU_CACHE)
                         pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
                                 << ARM_LPAE_PTE_ATTRINDX_SHIFT);
         } else {
--->8---

Sorry for the bother,
Robin.

>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
>> index 381ca5a..3ff4f87 100644
>> --- a/drivers/iommu/io-pgtable-arm.c
>> +++ b/drivers/iommu/io-pgtable-arm.c
>> @@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
>> arm_lpae_io_pgtable *data, pte |= ARM_LPAE_PTE_HAP_READ;
>>   		if (prot & IOMMU_WRITE)
>>   			pte |= ARM_LPAE_PTE_HAP_WRITE;
>> -		if (prot & IOMMU_CACHE)
>> +		if (prot & IOMMU_MMIO)
>> +			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
>> +		else if (prot & IOMMU_CACHE)
>>   			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
>>   		else
>>   			pte |= ARM_LPAE_PTE_MEMATTR_NC;
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index a5c539f..34b6432 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -30,6 +30,7 @@
>>   #define IOMMU_WRITE	(1 << 1)
>>   #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
>>   #define IOMMU_NOEXEC	(1 << 3)
>> +#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */
>>
>>   struct iommu_ops;
>>   struct iommu_group;
>

[toc] | [prev] | [next] | [standalone]

#1335312 — Re: [PATCH v3 1/8] iommu: Add MMIO mapping type

Hi Robin,

Thanks for your update patch I will include it in my next version. But
I'm sorry I do not understand, is your modification an addition or a
substitution to your original patch?

* Robin Murphy <robin.murphy@arm.com> [2016-02-11 15:57:26 +0000]:

> On 11/02/16 00:02, Laurent Pinchart wrote:
> >Hi Niklas,
> >
> >Thank you for the patch.
> >
> >On Wednesday 10 February 2016 01:57:51 Niklas Söderlund wrote:
> >>From: Robin Murphy <robin.murphy@arm.com>
> >>
> >>On some platforms, MMIO regions might need slightly different treatment
> >>compared to mapping regular memory; add the notion of MMIO mappings to
> >>the IOMMU API's memory type flags, so that callers can let the IOMMU
> >>drivers know to do the right thing.
> >>
> >>Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> >>Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> >Answering the question from the cover letter, yes, it's totally fine to pick
> >the ack, that's actually expected.
> >
> >>---
> >>  drivers/iommu/io-pgtable-arm.c | 4 +++-
> >>  include/linux/iommu.h          | 1 +
> >
> >You might be asked to split this patch in two.
>
> Worse than that, you might also be asked to fix it up when the silly author
> remembers that he did this on a stage-2-only ARM SMMU, and the attributes
> for the stage 1 tables that the IPMMU uses are in a different code path:
>
> --->8---
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 5b5c299..7622c6e 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -354,7 +354,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
> arm_lpae_io_pgtable *data,
>                 if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
>                         pte |= ARM_LPAE_PTE_AP_RDONLY;
>
> -               if (prot & IOMMU_CACHE)
> +               if (prot & IOMMU_MMIO)
> +                       pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
> +                               << ARM_LPAE_PTE_ATTRINDX_SHIFT);
> +               else if (prot & IOMMU_CACHE)
>                         pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
>                                 << ARM_LPAE_PTE_ATTRINDX_SHIFT);
>         } else {
> --->8---
>
> Sorry for the bother,
> Robin.
>
> >>  2 files changed, 4 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> >>index 381ca5a..3ff4f87 100644
> >>--- a/drivers/iommu/io-pgtable-arm.c
> >>+++ b/drivers/iommu/io-pgtable-arm.c
> >>@@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
> >>arm_lpae_io_pgtable *data, pte |= ARM_LPAE_PTE_HAP_READ;
> >>  		if (prot & IOMMU_WRITE)
> >>  			pte |= ARM_LPAE_PTE_HAP_WRITE;
> >>-		if (prot & IOMMU_CACHE)
> >>+		if (prot & IOMMU_MMIO)
> >>+			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
> >>+		else if (prot & IOMMU_CACHE)
> >>  			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
> >>  		else
> >>  			pte |= ARM_LPAE_PTE_MEMATTR_NC;
> >>diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> >>index a5c539f..34b6432 100644
> >>--- a/include/linux/iommu.h
> >>+++ b/include/linux/iommu.h
> >>@@ -30,6 +30,7 @@
> >>  #define IOMMU_WRITE	(1 << 1)
> >>  #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
> >>  #define IOMMU_NOEXEC	(1 << 3)
> >>+#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */
> >>
> >>  struct iommu_ops;
> >>  struct iommu_group;
> >
>

[toc] | [prev] | [next] | [standalone]

#1335331 — Re: [PATCH v3 1/8] iommu: Add MMIO mapping type

On 16/02/16 12:06, Niklas Söderlund wrote:
> Hi Robin,
>
> Thanks for your update patch I will include it in my next version. But
> I'm sorry I do not understand, is your modification an addition or a
> substitution to your original patch?

Apologies for being confusing - that was a diff on top of the existing 
patch, to be folded in. My original patch was only handling IOMMU_MMIO 
for stage 2 PTEs, so we also need the extra code to handle the different 
way of setting the appropriate memory type in stage 1 PTEs.

Robin.

> * Robin Murphy <robin.murphy@arm.com> [2016-02-11 15:57:26 +0000]:
>
>> On 11/02/16 00:02, Laurent Pinchart wrote:
>>> Hi Niklas,
>>>
>>> Thank you for the patch.
>>>
>>> On Wednesday 10 February 2016 01:57:51 Niklas Söderlund wrote:
>>>> From: Robin Murphy <robin.murphy@arm.com>
>>>>
>>>> On some platforms, MMIO regions might need slightly different treatment
>>>> compared to mapping regular memory; add the notion of MMIO mappings to
>>>> the IOMMU API's memory type flags, so that callers can let the IOMMU
>>>> drivers know to do the right thing.
>>>>
>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>>> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>> Answering the question from the cover letter, yes, it's totally fine to pick
>>> the ack, that's actually expected.
>>>
>>>> ---
>>>>   drivers/iommu/io-pgtable-arm.c | 4 +++-
>>>>   include/linux/iommu.h          | 1 +
>>>
>>> You might be asked to split this patch in two.
>>
>> Worse than that, you might also be asked to fix it up when the silly author
>> remembers that he did this on a stage-2-only ARM SMMU, and the attributes
>> for the stage 1 tables that the IPMMU uses are in a different code path:
>>
>> --->8---
>> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
>> index 5b5c299..7622c6e 100644
>> --- a/drivers/iommu/io-pgtable-arm.c
>> +++ b/drivers/iommu/io-pgtable-arm.c
>> @@ -354,7 +354,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
>> arm_lpae_io_pgtable *data,
>>                  if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
>>                          pte |= ARM_LPAE_PTE_AP_RDONLY;
>>
>> -               if (prot & IOMMU_CACHE)
>> +               if (prot & IOMMU_MMIO)
>> +                       pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
>> +                               << ARM_LPAE_PTE_ATTRINDX_SHIFT);
>> +               else if (prot & IOMMU_CACHE)
>>                          pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
>>                                  << ARM_LPAE_PTE_ATTRINDX_SHIFT);
>>          } else {
>> --->8---
>>
>> Sorry for the bother,
>> Robin.
>>
>>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
>>>> index 381ca5a..3ff4f87 100644
>>>> --- a/drivers/iommu/io-pgtable-arm.c
>>>> +++ b/drivers/iommu/io-pgtable-arm.c
>>>> @@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
>>>> arm_lpae_io_pgtable *data, pte |= ARM_LPAE_PTE_HAP_READ;
>>>>   		if (prot & IOMMU_WRITE)
>>>>   			pte |= ARM_LPAE_PTE_HAP_WRITE;
>>>> -		if (prot & IOMMU_CACHE)
>>>> +		if (prot & IOMMU_MMIO)
>>>> +			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
>>>> +		else if (prot & IOMMU_CACHE)
>>>>   			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
>>>>   		else
>>>>   			pte |= ARM_LPAE_PTE_MEMATTR_NC;
>>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>>> index a5c539f..34b6432 100644
>>>> --- a/include/linux/iommu.h
>>>> +++ b/include/linux/iommu.h
>>>> @@ -30,6 +30,7 @@
>>>>   #define IOMMU_WRITE	(1 << 1)
>>>>   #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
>>>>   #define IOMMU_NOEXEC	(1 << 3)
>>>> +#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */
>>>>
>>>>   struct iommu_ops;
>>>>   struct iommu_group;
>>>
>>
>

[toc] | [prev] | [next] | [standalone]

#1335385 — Re: [PATCH v3 1/8] iommu: Add MMIO mapping type

* Robin Murphy <robin.murphy@arm.com> [2016-02-16 12:43:40 +0000]:

> On 16/02/16 12:06, Niklas Söderlund wrote:
> >Hi Robin,
> >
> >Thanks for your update patch I will include it in my next version. But
> >I'm sorry I do not understand, is your modification an addition or a
> >substitution to your original patch?
>
> Apologies for being confusing - that was a diff on top of the existing
> patch, to be folded in. My original patch was only handling IOMMU_MMIO for
> stage 2 PTEs, so we also need the extra code to handle the different way of
> setting the appropriate memory type in stage 1 PTEs.

That's what I though but wanted to be clear, thanks for clarifying. I
will fold the diff into your patch and keep your SoB line and send it
out with my series, hope that's a OK way for me to handle it.

Once more thanks for your patch and feedback.

>
> Robin.
>
> >* Robin Murphy <robin.murphy@arm.com> [2016-02-11 15:57:26 +0000]:
> >
> >>On 11/02/16 00:02, Laurent Pinchart wrote:
> >>>Hi Niklas,
> >>>
> >>>Thank you for the patch.
> >>>
> >>>On Wednesday 10 February 2016 01:57:51 Niklas Söderlund wrote:
> >>>>From: Robin Murphy <robin.murphy@arm.com>
> >>>>
> >>>>On some platforms, MMIO regions might need slightly different treatment
> >>>>compared to mapping regular memory; add the notion of MMIO mappings to
> >>>>the IOMMU API's memory type flags, so that callers can let the IOMMU
> >>>>drivers know to do the right thing.
> >>>>
> >>>>Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> >>>>Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>
> >>>Answering the question from the cover letter, yes, it's totally fine to pick
> >>>the ack, that's actually expected.
> >>>
> >>>>---
> >>>>  drivers/iommu/io-pgtable-arm.c | 4 +++-
> >>>>  include/linux/iommu.h          | 1 +
> >>>
> >>>You might be asked to split this patch in two.
> >>
> >>Worse than that, you might also be asked to fix it up when the silly author
> >>remembers that he did this on a stage-2-only ARM SMMU, and the attributes
> >>for the stage 1 tables that the IPMMU uses are in a different code path:
> >>
> >>--->8---
> >>diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> >>index 5b5c299..7622c6e 100644
> >>--- a/drivers/iommu/io-pgtable-arm.c
> >>+++ b/drivers/iommu/io-pgtable-arm.c
> >>@@ -354,7 +354,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
> >>arm_lpae_io_pgtable *data,
> >>                 if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
> >>                         pte |= ARM_LPAE_PTE_AP_RDONLY;
> >>
> >>-               if (prot & IOMMU_CACHE)
> >>+               if (prot & IOMMU_MMIO)
> >>+                       pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
> >>+                               << ARM_LPAE_PTE_ATTRINDX_SHIFT);
> >>+               else if (prot & IOMMU_CACHE)
> >>                         pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
> >>                                 << ARM_LPAE_PTE_ATTRINDX_SHIFT);
> >>         } else {
> >>--->8---
> >>
> >>Sorry for the bother,
> >>Robin.
> >>
> >>>>  2 files changed, 4 insertions(+), 1 deletion(-)
> >>>>
> >>>>diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> >>>>index 381ca5a..3ff4f87 100644
> >>>>--- a/drivers/iommu/io-pgtable-arm.c
> >>>>+++ b/drivers/iommu/io-pgtable-arm.c
> >>>>@@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
> >>>>arm_lpae_io_pgtable *data, pte |= ARM_LPAE_PTE_HAP_READ;
> >>>>  		if (prot & IOMMU_WRITE)
> >>>>  			pte |= ARM_LPAE_PTE_HAP_WRITE;
> >>>>-		if (prot & IOMMU_CACHE)
> >>>>+		if (prot & IOMMU_MMIO)
> >>>>+			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
> >>>>+		else if (prot & IOMMU_CACHE)
> >>>>  			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
> >>>>  		else
> >>>>  			pte |= ARM_LPAE_PTE_MEMATTR_NC;
> >>>>diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> >>>>index a5c539f..34b6432 100644
> >>>>--- a/include/linux/iommu.h
> >>>>+++ b/include/linux/iommu.h
> >>>>@@ -30,6 +30,7 @@
> >>>>  #define IOMMU_WRITE	(1 << 1)
> >>>>  #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
> >>>>  #define IOMMU_NOEXEC	(1 << 3)
> >>>>+#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */
> >>>>
> >>>>  struct iommu_ops;
> >>>>  struct iommu_group;
> >>>
> >>
> >
>

[toc] | [prev] | [next] | [standalone]

#1330819 — [PATCH v3 5/8] dmaengine: rcar-dmac: group slave configuration

Group slave address and transfer size in own structs for source and
destination. This is in preparation for hooking up the dma-mapping API
to the slave addresses.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/dma/sh/rcar-dmac.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 7820d07..743873c 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -118,14 +118,21 @@ struct rcar_dmac_desc_page {
 	sizeof(struct rcar_dmac_xfer_chunk))
 
 /*
+ * @slave_addr: slave memory address
+ * @xfer_size: size (in bytes) of hardware transfers
+ */
+struct rcar_dmac_chan_slave {
+	dma_addr_t slave_addr;
+	unsigned int xfer_size;
+};
+
+/*
  * struct rcar_dmac_chan - R-Car Gen2 DMA Controller Channel
  * @chan: base DMA channel object
  * @iomem: channel I/O memory base
  * @index: index of this channel in the controller
- * @src_xfer_size: size (in bytes) of hardware transfers on the source side
- * @dst_xfer_size: size (in bytes) of hardware transfers on the destination side
- * @src_slave_addr: slave source memory address
- * @dst_slave_addr: slave destination memory address
+ * @src: slave memory address and size on the source side
+ * @dst: slave memory address and size on the destination side
  * @mid_rid: hardware MID/RID for the DMA client using this channel
  * @lock: protects the channel CHCR register and the desc members
  * @desc.free: list of free descriptors
@@ -142,10 +149,8 @@ struct rcar_dmac_chan {
 	void __iomem *iomem;
 	unsigned int index;
 
-	unsigned int src_xfer_size;
-	unsigned int dst_xfer_size;
-	dma_addr_t src_slave_addr;
-	dma_addr_t dst_slave_addr;
+	struct rcar_dmac_chan_slave src;
+	struct rcar_dmac_chan_slave dst;
 	int mid_rid;
 
 	spinlock_t lock;
@@ -793,13 +798,13 @@ static void rcar_dmac_chan_configure_desc(struct rcar_dmac_chan *chan,
 	case DMA_DEV_TO_MEM:
 		chcr = RCAR_DMACHCR_DM_INC | RCAR_DMACHCR_SM_FIXED
 		     | RCAR_DMACHCR_RS_DMARS;
-		xfer_size = chan->src_xfer_size;
+		xfer_size = chan->src.xfer_size;
 		break;
 
 	case DMA_MEM_TO_DEV:
 		chcr = RCAR_DMACHCR_DM_FIXED | RCAR_DMACHCR_SM_INC
 		     | RCAR_DMACHCR_RS_DMARS;
-		xfer_size = chan->dst_xfer_size;
+		xfer_size = chan->dst.xfer_size;
 		break;
 
 	case DMA_MEM_TO_MEM:
@@ -1038,7 +1043,7 @@ rcar_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	}
 
 	dev_addr = dir == DMA_DEV_TO_MEM
-		 ? rchan->src_slave_addr : rchan->dst_slave_addr;
+		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
 	return rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
 				      dir, flags, false);
 }
@@ -1093,7 +1098,7 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
 	}
 
 	dev_addr = dir == DMA_DEV_TO_MEM
-		 ? rchan->src_slave_addr : rchan->dst_slave_addr;
+		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
 	desc = rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
 				      dir, flags, true);
 
@@ -1110,10 +1115,10 @@ static int rcar_dmac_device_config(struct dma_chan *chan,
 	 * We could lock this, but you shouldn't be configuring the
 	 * channel, while using it...
 	 */
-	rchan->src_slave_addr = cfg->src_addr;
-	rchan->dst_slave_addr = cfg->dst_addr;
-	rchan->src_xfer_size = cfg->src_addr_width;
-	rchan->dst_xfer_size = cfg->dst_addr_width;
+	rchan->src.slave_addr = cfg->src_addr;
+	rchan->dst.slave_addr = cfg->dst_addr;
+	rchan->src.xfer_size = cfg->src_addr_width;
+	rchan->dst.xfer_size = cfg->dst_addr_width;
 
 	return 0;
 }
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1331599 — Re: [PATCH v3 5/8] dmaengine: rcar-dmac: group slave configuration

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:55 Niklas Söderlund wrote:
> Group slave address and transfer size in own structs for source and
> destination. This is in preparation for hooking up the dma-mapping API
> to the slave addresses.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/dma/sh/rcar-dmac.c | 37 +++++++++++++++++++++----------------
>  1 file changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 7820d07..743873c 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -118,14 +118,21 @@ struct rcar_dmac_desc_page {
>  	sizeof(struct rcar_dmac_xfer_chunk))
> 
>  /*
> + * @slave_addr: slave memory address
> + * @xfer_size: size (in bytes) of hardware transfers
> + */
> +struct rcar_dmac_chan_slave {
> +	dma_addr_t slave_addr;
> +	unsigned int xfer_size;
> +};
> +
> +/*
>   * struct rcar_dmac_chan - R-Car Gen2 DMA Controller Channel
>   * @chan: base DMA channel object
>   * @iomem: channel I/O memory base
>   * @index: index of this channel in the controller
> - * @src_xfer_size: size (in bytes) of hardware transfers on the source side
> - * @dst_xfer_size: size (in bytes) of hardware transfers on the
> destination side - * @src_slave_addr: slave source memory address
> - * @dst_slave_addr: slave destination memory address
> + * @src: slave memory address and size on the source side
> + * @dst: slave memory address and size on the destination side
>   * @mid_rid: hardware MID/RID for the DMA client using this channel
>   * @lock: protects the channel CHCR register and the desc members
>   * @desc.free: list of free descriptors
> @@ -142,10 +149,8 @@ struct rcar_dmac_chan {
>  	void __iomem *iomem;
>  	unsigned int index;
> 
> -	unsigned int src_xfer_size;
> -	unsigned int dst_xfer_size;
> -	dma_addr_t src_slave_addr;
> -	dma_addr_t dst_slave_addr;
> +	struct rcar_dmac_chan_slave src;
> +	struct rcar_dmac_chan_slave dst;
>  	int mid_rid;
> 
>  	spinlock_t lock;
> @@ -793,13 +798,13 @@ static void rcar_dmac_chan_configure_desc(struct
> rcar_dmac_chan *chan, case DMA_DEV_TO_MEM:
>  		chcr = RCAR_DMACHCR_DM_INC | RCAR_DMACHCR_SM_FIXED
> 
>  		     | RCAR_DMACHCR_RS_DMARS;
> 
> -		xfer_size = chan->src_xfer_size;
> +		xfer_size = chan->src.xfer_size;
>  		break;
> 
>  	case DMA_MEM_TO_DEV:
>  		chcr = RCAR_DMACHCR_DM_FIXED | RCAR_DMACHCR_SM_INC
> 
>  		     | RCAR_DMACHCR_RS_DMARS;
> 
> -		xfer_size = chan->dst_xfer_size;
> +		xfer_size = chan->dst.xfer_size;
>  		break;
> 
>  	case DMA_MEM_TO_MEM:
> @@ -1038,7 +1043,7 @@ rcar_dmac_prep_slave_sg(struct dma_chan *chan, struct
> scatterlist *sgl, }
> 
>  	dev_addr = dir == DMA_DEV_TO_MEM
> -		 ? rchan->src_slave_addr : rchan->dst_slave_addr;
> +		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
>  	return rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
>  				      dir, flags, false);
>  }
> @@ -1093,7 +1098,7 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan,
> dma_addr_t buf_addr, }
> 
>  	dev_addr = dir == DMA_DEV_TO_MEM
> -		 ? rchan->src_slave_addr : rchan->dst_slave_addr;
> +		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
>  	desc = rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
>  				      dir, flags, true);
> 
> @@ -1110,10 +1115,10 @@ static int rcar_dmac_device_config(struct dma_chan
> *chan, * We could lock this, but you shouldn't be configuring the
>  	 * channel, while using it...
>  	 */
> -	rchan->src_slave_addr = cfg->src_addr;
> -	rchan->dst_slave_addr = cfg->dst_addr;
> -	rchan->src_xfer_size = cfg->src_addr_width;
> -	rchan->dst_xfer_size = cfg->dst_addr_width;
> +	rchan->src.slave_addr = cfg->src_addr;
> +	rchan->dst.slave_addr = cfg->dst_addr;
> +	rchan->src.xfer_size = cfg->src_addr_width;
> +	rchan->dst.xfer_size = cfg->dst_addr_width;
> 
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart

[toc] | [prev] | [next] | [standalone]

#1330820 — [PATCH v3 6/8] dmaengine: rcar-dmac: add iommu support for slave transfers

Enable slave transfers to devices behind IPMMU:s by mapping the slave
addresses using the dma-mapping API.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/dma/sh/rcar-dmac.c | 57 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 743873c..268407c 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1106,21 +1106,68 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
 	return desc;
 }
 
+static int rcar_dmac_set_slave_addr(struct dma_chan *chan,
+				     struct rcar_dmac_chan_slave *slave,
+				     phys_addr_t addr, size_t size)
+{
+	struct dma_attrs attrs;
+	enum dma_data_direction dir;
+
+	init_dma_attrs(&attrs);
+	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &attrs);
+	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
+
+	/*
+	 * We can't know the direction at this time, see documentation for
+	 * 'direction' in struct dma_slave_config.
+	 */
+	dir = DMA_BIDIRECTIONAL;
+
+	if (slave->xfer_size) {
+		dma_unmap_resource(chan->device->dev, slave->slave_addr,
+				slave->xfer_size, dir, &attrs);
+		slave->slave_addr = 0;
+		slave->xfer_size = 0;
+	}
+
+	if (size) {
+		slave->slave_addr = dma_map_resource(chan->device->dev, addr,
+				size, dir, &attrs);
+
+		if (dma_mapping_error(chan->device->dev, slave->slave_addr)) {
+			struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+
+			dev_err(chan->device->dev,
+					"chan%u: failed to map %zx@%pap",
+					rchan->index, size, &addr);
+			return -EIO;
+		}
+
+		slave->xfer_size = size;
+	}
+
+	return 0;
+}
+
 static int rcar_dmac_device_config(struct dma_chan *chan,
 				   struct dma_slave_config *cfg)
 {
 	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+	int ret;
 
 	/*
 	 * We could lock this, but you shouldn't be configuring the
 	 * channel, while using it...
 	 */
-	rchan->src.slave_addr = cfg->src_addr;
-	rchan->dst.slave_addr = cfg->dst_addr;
-	rchan->src.xfer_size = cfg->src_addr_width;
-	rchan->dst.xfer_size = cfg->dst_addr_width;
 
-	return 0;
+	ret = rcar_dmac_set_slave_addr(chan, &rchan->src, cfg->src_addr,
+			cfg->src_addr_width);
+	if (ret)
+		return ret;
+
+	ret = rcar_dmac_set_slave_addr(chan, &rchan->dst, cfg->dst_addr,
+			cfg->dst_addr_width);
+	return ret;
 }
 
 static int rcar_dmac_chan_terminate_all(struct dma_chan *chan)
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1331060 — Re: [PATCH v3 6/8] dmaengine: rcar-dmac: add iommu support for slave transfers

On 10/02/16 00:57, Niklas Söderlund wrote:
> Enable slave transfers to devices behind IPMMU:s by mapping the slave
> addresses using the dma-mapping API.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>   drivers/dma/sh/rcar-dmac.c | 57 ++++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 52 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 743873c..268407c 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1106,21 +1106,68 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
>   	return desc;
>   }
>
> +static int rcar_dmac_set_slave_addr(struct dma_chan *chan,
> +				     struct rcar_dmac_chan_slave *slave,
> +				     phys_addr_t addr, size_t size)
> +{
> +	struct dma_attrs attrs;
> +	enum dma_data_direction dir;
> +
> +	init_dma_attrs(&attrs);
> +	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &attrs);
> +	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);

Now that we have a way to deal with MMIO addresses properly, we don't 
need these any more.

Robin.

> +
> +	/*
> +	 * We can't know the direction at this time, see documentation for
> +	 * 'direction' in struct dma_slave_config.
> +	 */
> +	dir = DMA_BIDIRECTIONAL;
> +
> +	if (slave->xfer_size) {
> +		dma_unmap_resource(chan->device->dev, slave->slave_addr,
> +				slave->xfer_size, dir, &attrs);
> +		slave->slave_addr = 0;
> +		slave->xfer_size = 0;
> +	}
> +
> +	if (size) {
> +		slave->slave_addr = dma_map_resource(chan->device->dev, addr,
> +				size, dir, &attrs);
> +
> +		if (dma_mapping_error(chan->device->dev, slave->slave_addr)) {
> +			struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
> +
> +			dev_err(chan->device->dev,
> +					"chan%u: failed to map %zx@%pap",
> +					rchan->index, size, &addr);
> +			return -EIO;
> +		}
> +
> +		slave->xfer_size = size;
> +	}
> +
> +	return 0;
> +}
> +
>   static int rcar_dmac_device_config(struct dma_chan *chan,
>   				   struct dma_slave_config *cfg)
>   {
>   	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
> +	int ret;
>
>   	/*
>   	 * We could lock this, but you shouldn't be configuring the
>   	 * channel, while using it...
>   	 */
> -	rchan->src.slave_addr = cfg->src_addr;
> -	rchan->dst.slave_addr = cfg->dst_addr;
> -	rchan->src.xfer_size = cfg->src_addr_width;
> -	rchan->dst.xfer_size = cfg->dst_addr_width;
>
> -	return 0;
> +	ret = rcar_dmac_set_slave_addr(chan, &rchan->src, cfg->src_addr,
> +			cfg->src_addr_width);
> +	if (ret)
> +		return ret;
> +
> +	ret = rcar_dmac_set_slave_addr(chan, &rchan->dst, cfg->dst_addr,
> +			cfg->dst_addr_width);
> +	return ret;
>   }
>
>   static int rcar_dmac_chan_terminate_all(struct dma_chan *chan)
>

[toc] | [prev] | [next] | [standalone]

#1331609 — Re: [PATCH v3 6/8] dmaengine: rcar-dmac: add iommu support for slave transfers

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:56 Niklas Söderlund wrote:
> Enable slave transfers to devices behind IPMMU:s by mapping the slave
> addresses using the dma-mapping API.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/dma/sh/rcar-dmac.c | 57 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 52 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 743873c..268407c 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1106,21 +1106,68 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan,
> dma_addr_t buf_addr, return desc;
>  }
> 
> +static int rcar_dmac_set_slave_addr(struct dma_chan *chan,
> +				     struct rcar_dmac_chan_slave *slave,
> +				     phys_addr_t addr, size_t size)
> +{
> +	struct dma_attrs attrs;
> +	enum dma_data_direction dir;
> +
> +	init_dma_attrs(&attrs);
> +	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &attrs);
> +	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
> +
> +	/*
> +	 * We can't know the direction at this time, see documentation for
> +	 * 'direction' in struct dma_slave_config.
> +	 */
> +	dir = DMA_BIDIRECTIONAL;
> +
> +	if (slave->xfer_size) {
> +		dma_unmap_resource(chan->device->dev, slave->slave_addr,
> +				slave->xfer_size, dir, &attrs);

Nitpicking, you can align slave with chan on the previous line.

> +		slave->slave_addr = 0;
> +		slave->xfer_size = 0;
> +	}
> +
> +	if (size) {
> +		slave->slave_addr = dma_map_resource(chan->device->dev, addr,
> +				size, dir, &attrs);
> +
> +		if (dma_mapping_error(chan->device->dev, slave->slave_addr)) {
> +			struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
> +
> +			dev_err(chan->device->dev,
> +					"chan%u: failed to map %zx@%pap",
> +					rchan->index, size, &addr);

Indentation looks weird to me.

> +			return -EIO;
> +		}
> +
> +		slave->xfer_size = size;
> +	}
> +
> +	return 0;
> +}
> +
>  static int rcar_dmac_device_config(struct dma_chan *chan,
>  				   struct dma_slave_config *cfg)
>  {
>  	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
> +	int ret;
> 
>  	/*
>  	 * We could lock this, but you shouldn't be configuring the
>  	 * channel, while using it...
>  	 */
> -	rchan->src.slave_addr = cfg->src_addr;
> -	rchan->dst.slave_addr = cfg->dst_addr;
> -	rchan->src.xfer_size = cfg->src_addr_width;
> -	rchan->dst.xfer_size = cfg->dst_addr_width;
> 
> -	return 0;
> +	ret = rcar_dmac_set_slave_addr(chan, &rchan->src, cfg->src_addr,
> +			cfg->src_addr_width);
> +	if (ret)
> +		return ret;
> +
> +	ret = rcar_dmac_set_slave_addr(chan, &rchan->dst, cfg->dst_addr,
> +			cfg->dst_addr_width);

You could align cfg with chan on the previous line (twice).

With this fixed and the attributes removed as explained by Robin,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	return ret;
>  }
> 
>  static int rcar_dmac_chan_terminate_all(struct dma_chan *chan)

-- 
Regards,

Laurent Pinchart

[toc] | [prev] | [next] | [standalone]

#1330821 — [PATCH v3 3/8] dma-mapping: add dma_{map,unmap}_resource

Map/Unmap a device resource from a physical address. If no dma_map_ops
method is available the operation is a no-op.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 include/linux/dma-mapping.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index e3aba4e..21bf986 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -216,6 +216,33 @@ static inline void dma_unmap_page(struct device *dev, dma_addr_t addr,
 	debug_dma_unmap_page(dev, addr, size, dir, false);
 }
 
+static inline dma_addr_t dma_map_resource(struct device *dev,
+					  phys_addr_t phys_addr,
+					  size_t size,
+					  enum dma_data_direction dir,
+					  struct dma_attrs *attrs)
+{
+	struct dma_map_ops *ops = get_dma_ops(dev);
+
+	BUG_ON(!valid_dma_direction(dir));
+	if (ops->map_resource)
+		return ops->map_resource(dev, phys_addr, size, dir, attrs);
+
+	return phys_addr;
+}
+
+static inline void dma_unmap_resource(struct device *dev, dma_addr_t addr,
+				      size_t size, enum dma_data_direction dir,
+				      struct dma_attrs *attrs)
+{
+	struct dma_map_ops *ops = get_dma_ops(dev);
+
+	BUG_ON(!valid_dma_direction(dir));
+	if (ops->unmap_resource)
+		ops->unmap_resource(dev, addr, size, dir, attrs);
+
+}
+
 static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
 					   size_t size,
 					   enum dma_data_direction dir)
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1331039 — Re: [PATCH v3 3/8] dma-mapping: add dma_{map,unmap}_resource

Hi Niklas,

Thanks for doing this, it looks good. Just a couple of minor comments on 
this and the next patch...

On 10/02/16 00:57, Niklas Söderlund wrote:
> Map/Unmap a device resource from a physical address. If no dma_map_ops
> method is available the operation is a no-op.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>   include/linux/dma-mapping.h | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index e3aba4e..21bf986 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -216,6 +216,33 @@ static inline void dma_unmap_page(struct device *dev, dma_addr_t addr,
>   	debug_dma_unmap_page(dev, addr, size, dir, false);
>   }
>
> +static inline dma_addr_t dma_map_resource(struct device *dev,
> +					  phys_addr_t phys_addr,
> +					  size_t size,
> +					  enum dma_data_direction dir,
> +					  struct dma_attrs *attrs)
> +{
> +	struct dma_map_ops *ops = get_dma_ops(dev);
> +
> +	BUG_ON(!valid_dma_direction(dir));

I think it would be worth also having the same inverse pfn_valid() check 
as ioremap() here, to make sure this is similarly hard to misuse.

Robin.

> +	if (ops->map_resource)
> +		return ops->map_resource(dev, phys_addr, size, dir, attrs);
> +
> +	return phys_addr;
> +}
> +
> +static inline void dma_unmap_resource(struct device *dev, dma_addr_t addr,
> +				      size_t size, enum dma_data_direction dir,
> +				      struct dma_attrs *attrs)
> +{
> +	struct dma_map_ops *ops = get_dma_ops(dev);
> +
> +	BUG_ON(!valid_dma_direction(dir));
> +	if (ops->unmap_resource)
> +		ops->unmap_resource(dev, addr, size, dir, attrs);
> +
> +}
> +
>   static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
>   					   size_t size,
>   					   enum dma_data_direction dir)
>

[toc] | [prev] | [next] | [standalone]

#1330823 — [PATCH v3 7/8] ARM: dts: r8a7790: add iommus to dmac0 and dmac1

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 arch/arm/boot/dts/r8a7790.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 7dfd393..048bbf8 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -294,6 +294,21 @@
 		power-domains = <&cpg_clocks>;
 		#dma-cells = <1>;
 		dma-channels = <15>;
+		iommus = <&ipmmu_ds 0>,
+			 <&ipmmu_ds 1>,
+			 <&ipmmu_ds 2>,
+			 <&ipmmu_ds 3>,
+			 <&ipmmu_ds 4>,
+			 <&ipmmu_ds 5>,
+			 <&ipmmu_ds 6>,
+			 <&ipmmu_ds 7>,
+			 <&ipmmu_ds 8>,
+			 <&ipmmu_ds 9>,
+			 <&ipmmu_ds 10>,
+			 <&ipmmu_ds 11>,
+			 <&ipmmu_ds 12>,
+			 <&ipmmu_ds 13>,
+			 <&ipmmu_ds 14>;
 	};
 
 	dmac1: dma-controller@e6720000 {
@@ -325,6 +340,21 @@
 		power-domains = <&cpg_clocks>;
 		#dma-cells = <1>;
 		dma-channels = <15>;
+		iommus = <&ipmmu_ds 15>,
+			 <&ipmmu_ds 16>,
+			 <&ipmmu_ds 17>,
+			 <&ipmmu_ds 18>,
+			 <&ipmmu_ds 19>,
+			 <&ipmmu_ds 20>,
+			 <&ipmmu_ds 21>,
+			 <&ipmmu_ds 22>,
+			 <&ipmmu_ds 23>,
+			 <&ipmmu_ds 24>,
+			 <&ipmmu_ds 25>,
+			 <&ipmmu_ds 26>,
+			 <&ipmmu_ds 27>,
+			 <&ipmmu_ds 28>,
+			 <&ipmmu_ds 29>;
 	};
 
 	audma0: dma-controller@ec700000 {
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1331383 — Re: [PATCH v3 7/8] ARM: dts: r8a7790: add iommus to dmac0 and dmac1

Hi Niklas,

I am deferring accepting this and the similar patch for the r8a7791 pending
acceptance of the driver changes earlier in this series. Please let me know
if you prefer a different course of action.

I notice that the devel branch of there renesas tree there are
dmac nodes for the r8a7793, r8a7794 and r8a7795. Is this change,
also suitable for those SoCs? If so, do you plan to update them?
If not I'll add it to my todo list.

[toc] | [prev] | [next] | [standalone]

#1331619 — Re: [PATCH v3 7/8] ARM: dts: r8a7790: add iommus to dmac0 and dmac1

Hi Simon,

* Simon Horman <horms@verge.net.au> [2016-02-10 18:55:59 +0100]:

> Hi Niklas,
>
> I am deferring accepting this and the similar patch for the r8a7791 pending
> acceptance of the driver changes earlier in this series. Please let me know
> if you prefer a different course of action.

That sounds good, thanks.

>
> I notice that the devel branch of there renesas tree there are
> dmac nodes for the r8a7793, r8a7794 and r8a7795. Is this change,
> also suitable for those SoCs? If so, do you plan to update them?
> If not I'll add it to my todo list.

I planed to update all effected SoCs once the dependencies for this
series where accepted. But if you want to keep track of this I'm happy.

[toc] | [prev] | [next] | [standalone]

#1331612 — Re: [PATCH v3 7/8] ARM: dts: r8a7790: add iommus to dmac0 and dmac1

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:57 Niklas Söderlund wrote:

No commit message ? I'd at least mention that as a side effect of this patch 
channel 0 and 15 are disabled, reducing the effective number of channels to 14 
per DMAC.

> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Same comment and ack for patch 8/8.

Note that we should still try to find a way to selectively enable the IOMMU in 
a per-device fashion, as system integrators might want it to be disabled for 
some devices. There's no urgency though.

> ---
>  arch/arm/boot/dts/r8a7790.dtsi | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index 7dfd393..048bbf8 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -294,6 +294,21 @@
>  		power-domains = <&cpg_clocks>;
>  		#dma-cells = <1>;
>  		dma-channels = <15>;
> +		iommus = <&ipmmu_ds 0>,
> +			 <&ipmmu_ds 1>,
> +			 <&ipmmu_ds 2>,
> +			 <&ipmmu_ds 3>,
> +			 <&ipmmu_ds 4>,
> +			 <&ipmmu_ds 5>,
> +			 <&ipmmu_ds 6>,
> +			 <&ipmmu_ds 7>,
> +			 <&ipmmu_ds 8>,
> +			 <&ipmmu_ds 9>,
> +			 <&ipmmu_ds 10>,
> +			 <&ipmmu_ds 11>,
> +			 <&ipmmu_ds 12>,
> +			 <&ipmmu_ds 13>,
> +			 <&ipmmu_ds 14>;
>  	};
> 
>  	dmac1: dma-controller@e6720000 {
> @@ -325,6 +340,21 @@
>  		power-domains = <&cpg_clocks>;
>  		#dma-cells = <1>;
>  		dma-channels = <15>;
> +		iommus = <&ipmmu_ds 15>,
> +			 <&ipmmu_ds 16>,
> +			 <&ipmmu_ds 17>,
> +			 <&ipmmu_ds 18>,
> +			 <&ipmmu_ds 19>,
> +			 <&ipmmu_ds 20>,
> +			 <&ipmmu_ds 21>,
> +			 <&ipmmu_ds 22>,
> +			 <&ipmmu_ds 23>,
> +			 <&ipmmu_ds 24>,
> +			 <&ipmmu_ds 25>,
> +			 <&ipmmu_ds 26>,
> +			 <&ipmmu_ds 27>,
> +			 <&ipmmu_ds 28>,
> +			 <&ipmmu_ds 29>;
>  	};
> 
>  	audma0: dma-controller@ec700000 {

-- 
Regards,

Laurent Pinchart

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web