[v2,06/11] ARM: dts: zynq: Add DMAC for Parallella
diff mbox

Message ID 1406242820-20140-7-git-send-email-afaerber@suse.de
State New, archived
Headers show

Commit Message

Andreas Färber July 24, 2014, 11 p.m. UTC
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 v2: New
 
 arch/arm/boot/dts/zynq-7000.dtsi      | 17 +++++++++++++++++
 arch/arm/boot/dts/zynq-parallella.dts |  4 ++++
 2 files changed, 21 insertions(+)

Comments

Soren Brinkmann July 24, 2014, 11:28 p.m. UTC | #1
On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  v2: New
>  
>  arch/arm/boot/dts/zynq-7000.dtsi      | 17 +++++++++++++++++
>  arch/arm/boot/dts/zynq-parallella.dts |  4 ++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index eed3df0..1a70277 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -223,6 +223,23 @@
>  			};
>  		};
>  
> +		dmac_s: dmac@f8003000 {
> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0xf8003000 0x1000>;
> +			status = "disabled";
I think for this IP we can omit the 'status' property since it is always
enabled. I don't see a reason to override it in each board DT.

> +			interrupt-parent = <&intc>;
> +			interrupts = <0 13 4>,
> +			             <0 14 4>, <0 15 4>,
> +			             <0 16 4>, <0 17 4>,
> +			             <0 40 4>, <0 41 4>,
> +			             <0 42 4>, <0 43 4>;
> +			#dma-cells = <1>;
> +			#dma-channels = <8>;
> +			#dma-requests = <4>;
> +			clocks = <&clkc 27>;
> +			clock-names = "apb_pclk";
> +		};
> +

Acked-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

	Sören
Michal Simek July 25, 2014, 8:02 a.m. UTC | #2
On 07/25/2014 01:28 AM, Sören Brinkmann wrote:
> On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  v2: New
>>  
>>  arch/arm/boot/dts/zynq-7000.dtsi      | 17 +++++++++++++++++
>>  arch/arm/boot/dts/zynq-parallella.dts |  4 ++++
>>  2 files changed, 21 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> index eed3df0..1a70277 100644
>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> @@ -223,6 +223,23 @@
>>  			};
>>  		};
>>  
>> +		dmac_s: dmac@f8003000 {
>> +			compatible = "arm,pl330", "arm,primecell";
>> +			reg = <0xf8003000 0x1000>;
>> +			status = "disabled";
> I think for this IP we can omit the 'status' property since it is always
> enabled. I don't see a reason to override it in each board DT.

Done this change myself

> 
>> +			interrupt-parent = <&intc>;
>> +			interrupts = <0 13 4>,
>> +			             <0 14 4>, <0 15 4>,
>> +			             <0 16 4>, <0 17 4>,
>> +			             <0 40 4>, <0 41 4>,
>> +			             <0 42 4>, <0 43 4>;
>> +			#dma-cells = <1>;
>> +			#dma-channels = <8>;
>> +			#dma-requests = <4>;
>> +			clocks = <&clkc 27>;
>> +			clock-names = "apb_pclk";
>> +		};
>> +
> 
> Acked-by: Soren Brinkmann <soren.brinkmann@xilinx.com>


Applied to zynq/dt branch.
https://github.com/Xilinx/linux-xlnx/commits/zynq/dt

Thanks,
Michal
Andreas Färber July 25, 2014, 8:24 a.m. UTC | #3
Am 25.07.2014 10:02, schrieb Michal Simek:
> On 07/25/2014 01:28 AM, Sören Brinkmann wrote:
>> On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>>  v2: New
>>>  
>>>  arch/arm/boot/dts/zynq-7000.dtsi      | 17 +++++++++++++++++
>>>  arch/arm/boot/dts/zynq-parallella.dts |  4 ++++
>>>  2 files changed, 21 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>> index eed3df0..1a70277 100644
>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>> @@ -223,6 +223,23 @@
>>>  			};
>>>  		};
>>>  
>>> +		dmac_s: dmac@f8003000 {
>>> +			compatible = "arm,pl330", "arm,primecell";
>>> +			reg = <0xf8003000 0x1000>;
>>> +			status = "disabled";
>> I think for this IP we can omit the 'status' property since it is always
>> enabled. I don't see a reason to override it in each board DT.
> 
> Done this change myself

Fine with me, but allow me to point out that the TRM documents the DMAC
being mapped as DMAC S at the above address, and as DMAC NS at F800_4000
(secure vs. non-secure, ch. 4.6, p. 116). Not sure how this would be
handled driver-wise if not through alternative dt nodes?

>>> +			interrupt-parent = <&intc>;
>>> +			interrupts = <0 13 4>,
>>> +			             <0 14 4>, <0 15 4>,
>>> +			             <0 16 4>, <0 17 4>,
>>> +			             <0 40 4>, <0 41 4>,
>>> +			             <0 42 4>, <0 43 4>;
>>> +			#dma-cells = <1>;
>>> +			#dma-channels = <8>;
>>> +			#dma-requests = <4>;
>>> +			clocks = <&clkc 27>;
>>> +			clock-names = "apb_pclk";
>>> +		};
>>> +
>>
>> Acked-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> 
> 
> Applied to zynq/dt branch.
> https://github.com/Xilinx/linux-xlnx/commits/zynq/dt

Thanks,
Andreas
Michal Simek July 25, 2014, 8:36 a.m. UTC | #4
On 07/25/2014 10:24 AM, Andreas Färber wrote:
> Am 25.07.2014 10:02, schrieb Michal Simek:
>> On 07/25/2014 01:28 AM, Sören Brinkmann wrote:
>>> On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>> ---
>>>>  v2: New
>>>>  
>>>>  arch/arm/boot/dts/zynq-7000.dtsi      | 17 +++++++++++++++++
>>>>  arch/arm/boot/dts/zynq-parallella.dts |  4 ++++
>>>>  2 files changed, 21 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> index eed3df0..1a70277 100644
>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> @@ -223,6 +223,23 @@
>>>>  			};
>>>>  		};
>>>>  
>>>> +		dmac_s: dmac@f8003000 {
>>>> +			compatible = "arm,pl330", "arm,primecell";
>>>> +			reg = <0xf8003000 0x1000>;
>>>> +			status = "disabled";
>>> I think for this IP we can omit the 'status' property since it is always
>>> enabled. I don't see a reason to override it in each board DT.
>>
>> Done this change myself
> 
> Fine with me, but allow me to point out that the TRM documents the DMAC
> being mapped as DMAC S at the above address, and as DMAC NS at F800_4000
> (secure vs. non-secure, ch. 4.6, p. 116). Not sure how this would be
> handled driver-wise if not through alternative dt nodes?

Kernel on zynq runs in secure world. All the sw on zynq from beginning runs in secure world.
I haven't had any time to look at switching kernel to NS.
That's why having dmac in this dts as default configuration in secure world
is fine.

It is not about how driver will handle - driver shouldn't care.
It is how users will handle that kernel runs in non secure
and how to change DTS to reflect this.
Bootloader can do this change for example. Or if there is an option
to detect at run time by kernel that we are in NS than kernel itself can
patch dmac node and use different address.
Definitely bootloader or setup correct address by user is preferred way.

Thanks,
Michal
Soren Brinkmann July 25, 2014, 2:43 p.m. UTC | #5
On Fri, 2014-07-25 at 10:24AM +0200, Andreas Färber wrote:
> Am 25.07.2014 10:02, schrieb Michal Simek:
> > On 07/25/2014 01:28 AM, Sören Brinkmann wrote:
> >> On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
> >>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> >>> ---
> >>>  v2: New
> >>>  
> >>>  arch/arm/boot/dts/zynq-7000.dtsi      | 17 +++++++++++++++++
> >>>  arch/arm/boot/dts/zynq-parallella.dts |  4 ++++
> >>>  2 files changed, 21 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> >>> index eed3df0..1a70277 100644
> >>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> >>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> >>> @@ -223,6 +223,23 @@
> >>>  			};
> >>>  		};
> >>>  
> >>> +		dmac_s: dmac@f8003000 {
> >>> +			compatible = "arm,pl330", "arm,primecell";
> >>> +			reg = <0xf8003000 0x1000>;
> >>> +			status = "disabled";
> >> I think for this IP we can omit the 'status' property since it is always
> >> enabled. I don't see a reason to override it in each board DT.
> > 
> > Done this change myself
> 
> Fine with me, but allow me to point out that the TRM documents the DMAC
> being mapped as DMAC S at the above address, and as DMAC NS at F800_4000
> (secure vs. non-secure, ch. 4.6, p. 116). Not sure how this would be
> handled driver-wise if not through alternative dt nodes?

The upstream Linux runs in secure state on Zynq, hence I think this is fine. If
somebody wants to run Linux on Zynq non-secure they have to do some work
anyhow. This way the standard configuration can use the DMA engine.

	Sören

Patch
diff mbox

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index eed3df0..1a70277 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -223,6 +223,23 @@ 
 			};
 		};
 
+		dmac_s: dmac@f8003000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0xf8003000 0x1000>;
+			status = "disabled";
+			interrupt-parent = <&intc>;
+			interrupts = <0 13 4>,
+			             <0 14 4>, <0 15 4>,
+			             <0 16 4>, <0 17 4>,
+			             <0 40 4>, <0 41 4>,
+			             <0 42 4>, <0 43 4>;
+			#dma-cells = <1>;
+			#dma-channels = <8>;
+			#dma-requests = <4>;
+			clocks = <&clkc 27>;
+			clock-names = "apb_pclk";
+		};
+
 		devcfg: devcfg@f8007000 {
 			compatible = "xlnx,zynq-devcfg-1.0";
 			reg = <0xf8007000 0x100>;
diff --git a/arch/arm/boot/dts/zynq-parallella.dts b/arch/arm/boot/dts/zynq-parallella.dts
index 56f68ea..e60a0a9 100644
--- a/arch/arm/boot/dts/zynq-parallella.dts
+++ b/arch/arm/boot/dts/zynq-parallella.dts
@@ -34,6 +34,10 @@ 
 	};
 };
 
+&dmac_s {
+	status = "okay";
+};
+
 &gem0 {
 	status = "okay";
 	phy-mode = "rgmii-id";