[1/2] arm64: amd-seattle: Fix dma-ranges property

Commit Message

Suravee Suthikulpanit Dec. 1, 2014, 3:46 a.m. UTC

From: Suravee Suthikulpanit <[e-mail]>

AMD Seattle should support 40-bit DMA.

Signed-off-by: Suravee Suthikulpanit <[e-mail]>
---
 arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Arnd Bergmann Dec. 1, 2014, 1:33 p.m. UTC | #1

On Sunday 30 November 2014 21:46:39 [e-mail] wrote:
> From: Suravee Suthikulpanit <[e-mail]>
> 
> AMD Seattle should support 40-bit DMA.
> 
> Signed-off-by: Suravee Suthikulpanit <[e-mail]>
> 

The change looks good, but two things about the patch description:

- You should be a little more verbose, especially after we had discussed
  this to some detail on IRC, explain why you use the specific values.

- You should configure git-send-email to know your full email address
  including the name. Right now it gets sent from
  "[e-mail]" instead of "Suravee Suthikulpanit
  <[e-mail]>", and that causes the extra From
  line at the start of the mail, besides looking a bit strange.

Thanks,

	Arnd

Suravee Suthikulpanit Dec. 2, 2014, 2:12 p.m. UTC | #2

Hi Arnd,

On 12/1/14, 20:33, "Arnd Bergmann" <[e-mail]> wrote:

>On Sunday 30 November 2014 21:46:39 [e-mail] wrote:
>> From: Suravee Suthikulpanit <[e-mail]>
>> 
>> AMD Seattle should support 40-bit DMA.
>> 
>> Signed-off-by: Suravee Suthikulpanit <[e-mail]>
>> 
>
>The change looks good, but two things about the patch description:
>
>- You should be a little more verbose, especially after we had discussed
>  this to some detail on IRC, explain why you use the specific values.

Would you minding adding the following detail in the commit message?

- BEGIN -
AMD Seattle supports 40-bit DMA, which includes two ranges:
    1. GICv2m MSI register frame at 0xe0080000
    2. DRAM range [0x8000000000 to 0xffffffffff]

Since the current parsing logic handles only single range, this patch
specifies the range from zero to 0x10000000000 to cover the whole
40-bit range.

However, it expects DMA allocation/mapping logic to check the specified
DMA range against the reported DRAM range, which starts from
0x8000000000 in this case.
- END -

>
>- You should configure git-send-email to know your full email address
>  including the name. Right now it gets sent from
>  "[e-mail]" instead of "Suravee Suthikulpanit
>  <[e-mail]>", and that causes the extra From
>  line at the start of the mail, besides looking a bit strange.

Thanks for the pointer. I didn¹t notice that. I¹ll fix that for the future.

Suravee
>
>Thanks,
>
>	Arnd

Arnd Bergmann Dec. 4, 2014, 4:17 p.m. UTC | #3

On Tuesday 02 December 2014 14:12:48 Suthikulpanit, Suravee wrote:
> 
> Would you minding adding the following detail in the commit message?

I was going to, but then decided not to. It's usually better if you
just send a new patch when requested to update the changelog, so
please do that.
 
> - BEGIN -
> AMD Seattle supports 40-bit DMA, which includes two ranges:
>     1. GICv2m MSI register frame at 0xe0080000
>     2. DRAM range [0x8000000000 to 0xffffffffff]
> 
> Since the current parsing logic handles only single range, this patch
> specifies the range from zero to 0x10000000000 to cover the whole
> 40-bit range.
> 
> However, it expects DMA allocation/mapping logic to check the specified
> DMA range against the reported DRAM range, which starts from
> 0x8000000000 in this case.

Unfortunately the description is (for the most part) wrong:

- It doesn't matter whatsoever what the target of the DMA is or
  how large the DRAM can be. The only thing that matters is the
  transformation that the bus physically performs. If the AXI bus
  is (as you said earlier, though I'm no longer sure about it
  after your text above) limited to 40 bit address, you should
  use the 40-bit mask you gave above. If the 40-bit address is not
  a result of a limitation of the AXI bus but of the memory
  controller, you should just list a full 64-bit mask here.
  In that case, you actually have to use '0xffffffff 0xffffffff'
  as the length, which is one byte short as before, but we can't
  express that otherwise.

- Furthermore it does not matter at all what the Linux implementation
  does. If your hardware does a transformation that the current
  code can't handle, we need to fix the code rather than work around
  the implementation bug by putting incorrect data into the DT.

	Arnd

Patch

diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
index 0c8e7ae..f370f03 100644
--- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
+++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
@@ -55,8 +55,12 @@ 
 		#size-cells = <2>;
 		ranges;
 
-		/* DDR range is 40-bit addressing */
-		dma-ranges = <0x80 0x0 0x80 0x0 0x7f 0xffffffff>;
+		/*
+		 * dma-ranges is 40-bit address space containing:
+		 * - GICv2m MSI register is at 0xe0080000
+		 * - DRAM range [0x8000000000 to 0xffffffffff]
+		 */
+		dma-ranges = <0x0 0x0 0x0 0x0 0x100 0x0>;
 
 		/include/ "amd-seattle-clks.dtsi"
 
@@ -159,7 +163,7 @@ 
 				<0x1000 0x0 0x0 0x4 &gic0 0x0 0x0 0x0 0x123 0x1>;
 
 			dma-coherent;
-			dma-ranges = <0x43000000 0x80 0x0 0x80 0x0 0x7f 0xffffffff>;
+			dma-ranges = <0x43000000 0x0 0x0 0x0 0x0 0x100 0x0>;
 			ranges =
 				/* I/O Memory (size=64K) */
 				<0x01000000 0x00 0x00000000 0x00 0xefff0000 0x00 0x00010000>,