* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
@ 2016-08-30 15:37 ` Iaroslav Gridin
0 siblings, 0 replies; 18+ messages in thread
From: Iaroslav Gridin @ 2016-08-30 15:37 UTC (permalink / raw)
To: linux-arm-kernel
From: Voker57 <voker57@gmail.com>
Add device tree definitions for Qualcomm Cryptography engine and its BAM
Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 561d4d1..c0da739 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -287,6 +287,48 @@
reg = <0xf9011000 0x1000>;
};
+ cryptobam: dma at fd444000 {
+ compatible = "qcom,bam-v1.4.0";
+ reg = <0xfd444000 0x15000>;
+ interrupts = <0 236 0>;
+ clocks = <&gcc GCC_CE2_AHB_CLK>,
+ <&gcc GCC_CE2_AXI_CLK>,
+ <&gcc GCC_CE2_CLK>;
+ clock-names = "bam_clk", "axi_clk", "core_clk";
+ #dma-cells = <1>;
+ qcom,ee = <1>;
+ qcom,controlled-remotely;
+ };
+
+ qcom,qcrypto at fd440000 {
+ compatible = "qcom,crypto-v5.1";
+ reg = <0xfd45a000 0x6000>;
+ reg-names = "crypto-base";
+ interrupts = <0 236 0>;
+ qcom,bam-pipe-pair = <2>;
+ qcom,ce-hw-instance = <1>;
+ qcom,ce-device = <0>;
+ clocks = <&gcc GCC_CE2_CLK>,
+ <&gcc GCC_CE2_AHB_CLK>,
+ <&gcc GCC_CE2_AXI_CLK>,
+ <&gcc CE2_CLK_SRC>;
+
+ dmas = <&cryptobam 2>, <&cryptobam 3>;
+ dma-names = "rx", "tx";
+ clock-names = "core", "iface", "bus", "core_src";
+ qcom,clk-mgmt-sus-res;
+ qcom,msm-bus,name = "qcrypto-noc";
+
+ qcom,msm-bus,num-cases = <2>;
+ qcom,msm-bus,num-paths = <1>;
+ qcom,use-sw-aes-cbc-ecb-ctr-algo;
+ qcom,use-sw-aes-xts-algo;
+ qcom,use-sw-ahash-algo;
+ qcom,msm-bus,vectors-KBps = <56 512 0 0>,
+ <56 512 3936000 393600>;
+ };
+
+
timer at f9020000 {
#address-cells = <1>;
#size-cells = <1>;
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
2016-08-30 15:37 ` Iaroslav Gridin
@ 2016-09-07 13:09 ` Stanimir Varbanov
-1 siblings, 0 replies; 18+ messages in thread
From: Stanimir Varbanov @ 2016-09-07 13:09 UTC (permalink / raw)
To: Iaroslav Gridin, andy.gross
Cc: david.brown, robh+dt, mark.rutland, linux, linux-arm-msm,
linux-soc, linux-arm-kernel, linux-kernel
Hi Iaroslav,
On 08/30/2016 06:37 PM, Iaroslav Gridin wrote:
> From: Voker57 <voker57@gmail.com>
>
> Add device tree definitions for Qualcomm Cryptography engine and its BAM
> Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 561d4d1..c0da739 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -287,6 +287,48 @@
> reg = <0xf9011000 0x1000>;
> };
>
> + cryptobam: dma@fd444000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xfd444000 0x15000>;
> + interrupts = <0 236 0>;
should be
interrupts = <GIC_SPI 236 IRQ_NONE>;
> + clocks = <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc GCC_CE2_CLK>;
> + clock-names = "bam_clk", "axi_clk", "core_clk";
> + #dma-cells = <1>;
> + qcom,ee = <1>;
> + qcom,controlled-remotely;
> + };
indentation please.
> +
> + qcom,qcrypto@fd440000 {
> + compatible = "qcom,crypto-v5.1";
> + reg = <0xfd45a000 0x6000>;
> + reg-names = "crypto-base";
> + interrupts = <0 236 0>;
> + qcom,bam-pipe-pair = <2>;
> + qcom,ce-hw-instance = <1>;
> + qcom,ce-device = <0>;
You are getting those 3 properties from qcom downstream kernel, so they
are not relevant to qce in mainline kernel, please drop them.
> + clocks = <&gcc GCC_CE2_CLK>,
> + <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc CE2_CLK_SRC>;
> +
> + dmas = <&cryptobam 2>, <&cryptobam 3>;
> + dma-names = "rx", "tx";
> + clock-names = "core", "iface", "bus", "core_src";
from here to ...
> + qcom,clk-mgmt-sus-res;
> + qcom,msm-bus,name = "qcrypto-noc";
> +
> + qcom,msm-bus,num-cases = <2>;
> + qcom,msm-bus,num-paths = <1>;
> + qcom,use-sw-aes-cbc-ecb-ctr-algo;
> + qcom,use-sw-aes-xts-algo;
> + qcom,use-sw-ahash-algo;
> + qcom,msm-bus,vectors-KBps = <56 512 0 0>,
> + <56 512 3936000 393600>;
... here, please drop these properties they are no parsed and useful.
regards,
Stan
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
@ 2016-09-07 13:09 ` Stanimir Varbanov
0 siblings, 0 replies; 18+ messages in thread
From: Stanimir Varbanov @ 2016-09-07 13:09 UTC (permalink / raw)
To: linux-arm-kernel
Hi Iaroslav,
On 08/30/2016 06:37 PM, Iaroslav Gridin wrote:
> From: Voker57 <voker57@gmail.com>
>
> Add device tree definitions for Qualcomm Cryptography engine and its BAM
> Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 561d4d1..c0da739 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -287,6 +287,48 @@
> reg = <0xf9011000 0x1000>;
> };
>
> + cryptobam: dma at fd444000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xfd444000 0x15000>;
> + interrupts = <0 236 0>;
should be
interrupts = <GIC_SPI 236 IRQ_NONE>;
> + clocks = <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc GCC_CE2_CLK>;
> + clock-names = "bam_clk", "axi_clk", "core_clk";
> + #dma-cells = <1>;
> + qcom,ee = <1>;
> + qcom,controlled-remotely;
> + };
indentation please.
> +
> + qcom,qcrypto at fd440000 {
> + compatible = "qcom,crypto-v5.1";
> + reg = <0xfd45a000 0x6000>;
> + reg-names = "crypto-base";
> + interrupts = <0 236 0>;
> + qcom,bam-pipe-pair = <2>;
> + qcom,ce-hw-instance = <1>;
> + qcom,ce-device = <0>;
You are getting those 3 properties from qcom downstream kernel, so they
are not relevant to qce in mainline kernel, please drop them.
> + clocks = <&gcc GCC_CE2_CLK>,
> + <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc CE2_CLK_SRC>;
> +
> + dmas = <&cryptobam 2>, <&cryptobam 3>;
> + dma-names = "rx", "tx";
> + clock-names = "core", "iface", "bus", "core_src";
from here to ...
> + qcom,clk-mgmt-sus-res;
> + qcom,msm-bus,name = "qcrypto-noc";
> +
> + qcom,msm-bus,num-cases = <2>;
> + qcom,msm-bus,num-paths = <1>;
> + qcom,use-sw-aes-cbc-ecb-ctr-algo;
> + qcom,use-sw-aes-xts-algo;
> + qcom,use-sw-ahash-algo;
> + qcom,msm-bus,vectors-KBps = <56 512 0 0>,
> + <56 512 3936000 393600>;
... here, please drop these properties they are no parsed and useful.
regards,
Stan
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
2016-09-07 13:09 ` Stanimir Varbanov
@ 2016-09-07 16:20 ` Iaroslav Gridin
-1 siblings, 0 replies; 18+ messages in thread
From: Iaroslav Gridin @ 2016-09-07 16:20 UTC (permalink / raw)
To: Stanimir Varbanov
Cc: andy.gross, david.brown, robh+dt, mark.rutland, linux,
linux-arm-msm, linux-soc, linux-arm-kernel, linux-kernel
> > + clocks = <&gcc GCC_CE2_AHB_CLK>,
> > + <&gcc GCC_CE2_AXI_CLK>,
> > + <&gcc GCC_CE2_CLK>;
> > + clock-names = "bam_clk", "axi_clk", "core_clk";
> > + #dma-cells = <1>;
> > + qcom,ee = <1>;
> > + qcom,controlled-remotely;
> > + };
>
> indentation please.
Similar indentation (tabs then spaces to align) is used all over dts, am I doing it wrong or is
this bad practice?
Example @ line 334:
> interrupts = <0 8 0x4>,
> <0 7 0x4>;
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
@ 2016-09-07 16:20 ` Iaroslav Gridin
0 siblings, 0 replies; 18+ messages in thread
From: Iaroslav Gridin @ 2016-09-07 16:20 UTC (permalink / raw)
To: linux-arm-kernel
> > + clocks = <&gcc GCC_CE2_AHB_CLK>,
> > + <&gcc GCC_CE2_AXI_CLK>,
> > + <&gcc GCC_CE2_CLK>;
> > + clock-names = "bam_clk", "axi_clk", "core_clk";
> > + #dma-cells = <1>;
> > + qcom,ee = <1>;
> > + qcom,controlled-remotely;
> > + };
>
> indentation please.
Similar indentation (tabs then spaces to align) is used all over dts, am I doing it wrong or is
this bad practice?
Example @ line 334:
> interrupts = <0 8 0x4>,
> <0 7 0x4>;
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
2016-09-07 13:09 ` Stanimir Varbanov
@ 2016-09-17 0:10 ` Stephen Boyd
-1 siblings, 0 replies; 18+ messages in thread
From: Stephen Boyd @ 2016-09-17 0:10 UTC (permalink / raw)
To: Stanimir Varbanov, Iaroslav Gridin, andy.gross
Cc: david.brown, robh+dt, mark.rutland, linux, linux-arm-msm,
linux-soc, linux-arm-kernel, linux-kernel
On 09/07/2016 06:09 AM, Stanimir Varbanov wrote:
> Hi Iaroslav,
>
> On 08/30/2016 06:37 PM, Iaroslav Gridin wrote:
>> From: Voker57 <voker57@gmail.com>
>>
>> Add device tree definitions for Qualcomm Cryptography engine and its BAM
>> Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
>> ---
>> arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
>> index 561d4d1..c0da739 100644
>> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
>> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
>> @@ -287,6 +287,48 @@
>> reg = <0xf9011000 0x1000>;
>> };
>>
>> + cryptobam: dma@fd444000 {
>> + compatible = "qcom,bam-v1.4.0";
>> + reg = <0xfd444000 0x15000>;
>> + interrupts = <0 236 0>;
> should be
>
> interrupts = <GIC_SPI 236 IRQ_NONE>;
Please don't use IRQ_NONE. This one looks to be IRQ_TYPE_EDGE_RISING if
I'm not mistaken.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
@ 2016-09-17 0:10 ` Stephen Boyd
0 siblings, 0 replies; 18+ messages in thread
From: Stephen Boyd @ 2016-09-17 0:10 UTC (permalink / raw)
To: linux-arm-kernel
On 09/07/2016 06:09 AM, Stanimir Varbanov wrote:
> Hi Iaroslav,
>
> On 08/30/2016 06:37 PM, Iaroslav Gridin wrote:
>> From: Voker57 <voker57@gmail.com>
>>
>> Add device tree definitions for Qualcomm Cryptography engine and its BAM
>> Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
>> ---
>> arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
>> index 561d4d1..c0da739 100644
>> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
>> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
>> @@ -287,6 +287,48 @@
>> reg = <0xf9011000 0x1000>;
>> };
>>
>> + cryptobam: dma at fd444000 {
>> + compatible = "qcom,bam-v1.4.0";
>> + reg = <0xfd444000 0x15000>;
>> + interrupts = <0 236 0>;
> should be
>
> interrupts = <GIC_SPI 236 IRQ_NONE>;
Please don't use IRQ_NONE. This one looks to be IRQ_TYPE_EDGE_RISING if
I'm not mistaken.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
2016-08-30 15:37 ` Iaroslav Gridin
@ 2016-09-13 3:51 ` Bjorn Andersson
-1 siblings, 0 replies; 18+ messages in thread
From: Bjorn Andersson @ 2016-09-13 3:51 UTC (permalink / raw)
To: Iaroslav Gridin
Cc: andy.gross, david.brown, robh+dt, mark.rutland, linux,
linux-arm-msm, linux-soc, linux-arm-kernel, linux-kernel
On Tue 30 Aug 08:37 PDT 2016, Iaroslav Gridin wrote:
> From: Voker57 <voker57@gmail.com>
>
> Add device tree definitions for Qualcomm Cryptography engine and its BAM
> Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 561d4d1..c0da739 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -287,6 +287,48 @@
> reg = <0xf9011000 0x1000>;
> };
>
> + cryptobam: dma@fd444000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xfd444000 0x15000>;
> + interrupts = <0 236 0>;
> + clocks = <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc GCC_CE2_CLK>;
> + clock-names = "bam_clk", "axi_clk", "core_clk";
> + #dma-cells = <1>;
> + qcom,ee = <1>;
> + qcom,controlled-remotely;
> + };
As Stan noted, please shift this '}' one step left (the rest looks well
indented.
> +
> + qcom,qcrypto@fd440000 {
Rename this "qcrypto" and make sure the address matches the reg
property.
> + compatible = "qcom,crypto-v5.1";
> + reg = <0xfd45a000 0x6000>;
> + reg-names = "crypto-base";
> + interrupts = <0 236 0>;
> + qcom,bam-pipe-pair = <2>;
> + qcom,ce-hw-instance = <1>;
> + qcom,ce-device = <0>;
> + clocks = <&gcc GCC_CE2_CLK>,
> + <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc CE2_CLK_SRC>;
> +
> + dmas = <&cryptobam 2>, <&cryptobam 3>;
> + dma-names = "rx", "tx";
> + clock-names = "core", "iface", "bus", "core_src";
> + qcom,clk-mgmt-sus-res;
> + qcom,msm-bus,name = "qcrypto-noc";
> +
> + qcom,msm-bus,num-cases = <2>;
> + qcom,msm-bus,num-paths = <1>;
> + qcom,use-sw-aes-cbc-ecb-ctr-algo;
> + qcom,use-sw-aes-xts-algo;
> + qcom,use-sw-ahash-algo;
> + qcom,msm-bus,vectors-KBps = <56 512 0 0>,
> + <56 512 3936000 393600>;
> + };
> +
> +
> timer@f9020000 {
It's nice to keep the nodes within a group ordered by address.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
@ 2016-09-13 3:51 ` Bjorn Andersson
0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Andersson @ 2016-09-13 3:51 UTC (permalink / raw)
To: linux-arm-kernel
On Tue 30 Aug 08:37 PDT 2016, Iaroslav Gridin wrote:
> From: Voker57 <voker57@gmail.com>
>
> Add device tree definitions for Qualcomm Cryptography engine and its BAM
> Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 561d4d1..c0da739 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -287,6 +287,48 @@
> reg = <0xf9011000 0x1000>;
> };
>
> + cryptobam: dma at fd444000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xfd444000 0x15000>;
> + interrupts = <0 236 0>;
> + clocks = <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc GCC_CE2_CLK>;
> + clock-names = "bam_clk", "axi_clk", "core_clk";
> + #dma-cells = <1>;
> + qcom,ee = <1>;
> + qcom,controlled-remotely;
> + };
As Stan noted, please shift this '}' one step left (the rest looks well
indented.
> +
> + qcom,qcrypto at fd440000 {
Rename this "qcrypto" and make sure the address matches the reg
property.
> + compatible = "qcom,crypto-v5.1";
> + reg = <0xfd45a000 0x6000>;
> + reg-names = "crypto-base";
> + interrupts = <0 236 0>;
> + qcom,bam-pipe-pair = <2>;
> + qcom,ce-hw-instance = <1>;
> + qcom,ce-device = <0>;
> + clocks = <&gcc GCC_CE2_CLK>,
> + <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc CE2_CLK_SRC>;
> +
> + dmas = <&cryptobam 2>, <&cryptobam 3>;
> + dma-names = "rx", "tx";
> + clock-names = "core", "iface", "bus", "core_src";
> + qcom,clk-mgmt-sus-res;
> + qcom,msm-bus,name = "qcrypto-noc";
> +
> + qcom,msm-bus,num-cases = <2>;
> + qcom,msm-bus,num-paths = <1>;
> + qcom,use-sw-aes-cbc-ecb-ctr-algo;
> + qcom,use-sw-aes-xts-algo;
> + qcom,use-sw-ahash-algo;
> + qcom,msm-bus,vectors-KBps = <56 512 0 0>,
> + <56 512 3936000 393600>;
> + };
> +
> +
> timer at f9020000 {
It's nice to keep the nodes within a group ordered by address.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
2016-08-30 15:37 ` Iaroslav Gridin
@ 2016-09-15 21:18 ` Andy Gross
-1 siblings, 0 replies; 18+ messages in thread
From: Andy Gross @ 2016-09-15 21:18 UTC (permalink / raw)
To: Iaroslav Gridin
Cc: david.brown, robh+dt, mark.rutland, linux, linux-arm-msm,
linux-soc, linux-arm-kernel, linux-kernel
On Tue, Aug 30, 2016 at 06:37:40PM +0300, Iaroslav Gridin wrote:
> From: Voker57 <voker57@gmail.com>
>
> Add device tree definitions for Qualcomm Cryptography engine and its BAM
> Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 561d4d1..c0da739 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -287,6 +287,48 @@
> reg = <0xf9011000 0x1000>;
> };
>
> + cryptobam: dma@fd444000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xfd444000 0x15000>;
> + interrupts = <0 236 0>;
> + clocks = <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc GCC_CE2_CLK>;
> + clock-names = "bam_clk", "axi_clk", "core_clk";
Actually, on thinking about this more, the bam block itself only requires the
single clock. The peripheral it is attached to has to keep its sanity during
the duration of the transfer (crypto). The crypto requires 3 clocks, one of
which is the same clk the bam requires.
You can access the BAM registers with the bam_clk only, correct?
> + #dma-cells = <1>;
> + qcom,ee = <1>;
> + qcom,controlled-remotely;
> + };
> +
> + qcom,qcrypto@fd440000 {
> + compatible = "qcom,crypto-v5.1";
> + reg = <0xfd45a000 0x6000>;
> + reg-names = "crypto-base";
> + interrupts = <0 236 0>;
> + qcom,bam-pipe-pair = <2>;
> + qcom,ce-hw-instance = <1>;
> + qcom,ce-device = <0>;
> + clocks = <&gcc GCC_CE2_CLK>,
> + <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc CE2_CLK_SRC>;
The CLK_SRC is unnecessary. Or should be at least. That gets turned on by
getting the CE2_CLK. I vaguely remember a parent issue that was fixed.
> +
> + dmas = <&cryptobam 2>, <&cryptobam 3>;
> + dma-names = "rx", "tx";
> + clock-names = "core", "iface", "bus", "core_src";
> + qcom,clk-mgmt-sus-res;
> + qcom,msm-bus,name = "qcrypto-noc";
> +
> + qcom,msm-bus,num-cases = <2>;
> + qcom,msm-bus,num-paths = <1>;
> + qcom,use-sw-aes-cbc-ecb-ctr-algo;
> + qcom,use-sw-aes-xts-algo;
> + qcom,use-sw-ahash-algo;
> + qcom,msm-bus,vectors-KBps = <56 512 0 0>,
> + <56 512 3936000 393600>;
> + };
> +
> +
> timer@f9020000 {
> #address-cells = <1>;
> #size-cells = <1>;
Regards,
Andy
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
@ 2016-09-15 21:18 ` Andy Gross
0 siblings, 0 replies; 18+ messages in thread
From: Andy Gross @ 2016-09-15 21:18 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 30, 2016 at 06:37:40PM +0300, Iaroslav Gridin wrote:
> From: Voker57 <voker57@gmail.com>
>
> Add device tree definitions for Qualcomm Cryptography engine and its BAM
> Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 561d4d1..c0da739 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -287,6 +287,48 @@
> reg = <0xf9011000 0x1000>;
> };
>
> + cryptobam: dma at fd444000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xfd444000 0x15000>;
> + interrupts = <0 236 0>;
> + clocks = <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc GCC_CE2_CLK>;
> + clock-names = "bam_clk", "axi_clk", "core_clk";
Actually, on thinking about this more, the bam block itself only requires the
single clock. The peripheral it is attached to has to keep its sanity during
the duration of the transfer (crypto). The crypto requires 3 clocks, one of
which is the same clk the bam requires.
You can access the BAM registers with the bam_clk only, correct?
> + #dma-cells = <1>;
> + qcom,ee = <1>;
> + qcom,controlled-remotely;
> + };
> +
> + qcom,qcrypto at fd440000 {
> + compatible = "qcom,crypto-v5.1";
> + reg = <0xfd45a000 0x6000>;
> + reg-names = "crypto-base";
> + interrupts = <0 236 0>;
> + qcom,bam-pipe-pair = <2>;
> + qcom,ce-hw-instance = <1>;
> + qcom,ce-device = <0>;
> + clocks = <&gcc GCC_CE2_CLK>,
> + <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc CE2_CLK_SRC>;
The CLK_SRC is unnecessary. Or should be at least. That gets turned on by
getting the CE2_CLK. I vaguely remember a parent issue that was fixed.
> +
> + dmas = <&cryptobam 2>, <&cryptobam 3>;
> + dma-names = "rx", "tx";
> + clock-names = "core", "iface", "bus", "core_src";
> + qcom,clk-mgmt-sus-res;
> + qcom,msm-bus,name = "qcrypto-noc";
> +
> + qcom,msm-bus,num-cases = <2>;
> + qcom,msm-bus,num-paths = <1>;
> + qcom,use-sw-aes-cbc-ecb-ctr-algo;
> + qcom,use-sw-aes-xts-algo;
> + qcom,use-sw-ahash-algo;
> + qcom,msm-bus,vectors-KBps = <56 512 0 0>,
> + <56 512 3936000 393600>;
> + };
> +
> +
> timer at f9020000 {
> #address-cells = <1>;
> #size-cells = <1>;
Regards,
Andy
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
2016-09-15 21:18 ` Andy Gross
@ 2016-09-16 11:59 ` Iaroslav Gridin
-1 siblings, 0 replies; 18+ messages in thread
From: Iaroslav Gridin @ 2016-09-16 11:59 UTC (permalink / raw)
To: Andy Gross
Cc: david.brown, robh+dt, mark.rutland, linux, linux-arm-msm,
linux-soc, linux-arm-kernel, linux-kernel
On Thu, Sep 15, 2016 at 04:18:42PM -0500, Andy Gross wrote:
> Actually, on thinking about this more, the bam block itself only
> requires the
> single clock. The peripheral it is attached to has to keep its sanity
> during
> the duration of the transfer (crypto). The crypto requires 3 clocks,
> one of
> which is the same clk the bam requires.
>
> You can access the BAM registers with the bam_clk only, correct?
No, with only bam_clk board reboots. In fact, core_clk is the only
required one.
> The CLK_SRC is unnecessary. Or should be at least. That gets turned
> on by
> getting the CE2_CLK. I vaguely remember a parent issue that was
> fixed.
Yes, I thought it was required to change its speed to achieve maximum
QCE performance but as it have been pointed out, same adjustment on core
clock does the same.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
@ 2016-09-16 11:59 ` Iaroslav Gridin
0 siblings, 0 replies; 18+ messages in thread
From: Iaroslav Gridin @ 2016-09-16 11:59 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 15, 2016 at 04:18:42PM -0500, Andy Gross wrote:
> Actually, on thinking about this more, the bam block itself only
> requires the
> single clock. The peripheral it is attached to has to keep its sanity
> during
> the duration of the transfer (crypto). The crypto requires 3 clocks,
> one of
> which is the same clk the bam requires.
>
> You can access the BAM registers with the bam_clk only, correct?
No, with only bam_clk board reboots. In fact, core_clk is the only
required one.
> The CLK_SRC is unnecessary. Or should be at least. That gets turned
> on by
> getting the CE2_CLK. I vaguely remember a parent issue that was
> fixed.
Yes, I thought it was required to change its speed to achieve maximum
QCE performance but as it have been pointed out, same adjustment on core
clock does the same.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
2016-09-15 21:18 ` Andy Gross
@ 2016-09-16 17:38 ` Iaroslav Gridin
-1 siblings, 0 replies; 18+ messages in thread
From: Iaroslav Gridin @ 2016-09-16 17:38 UTC (permalink / raw)
To: Andy Gross
Cc: david.brown, robh+dt, mark.rutland, linux, linux-arm-msm,
linux-soc, linux-arm-kernel, linux-kernel
On Thu, Sep 15, 2016 at 04:18:42PM -0500, Andy Gross wrote:
> Actually, on thinking about this more, the bam block itself only requires the
> single clock. The peripheral it is attached to has to keep its sanity during
> the duration of the transfer (crypto). The crypto requires 3 clocks, one of
> which is the same clk the bam requires.
>
> You can access the BAM registers with the bam_clk only, correct?
Not preparing bam_clk degrades QCE performance about 3x, though.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
@ 2016-09-16 17:38 ` Iaroslav Gridin
0 siblings, 0 replies; 18+ messages in thread
From: Iaroslav Gridin @ 2016-09-16 17:38 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 15, 2016 at 04:18:42PM -0500, Andy Gross wrote:
> Actually, on thinking about this more, the bam block itself only requires the
> single clock. The peripheral it is attached to has to keep its sanity during
> the duration of the transfer (crypto). The crypto requires 3 clocks, one of
> which is the same clk the bam requires.
>
> You can access the BAM registers with the bam_clk only, correct?
Not preparing bam_clk degrades QCE performance about 3x, though.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
2016-09-16 17:38 ` Iaroslav Gridin
@ 2016-09-16 18:50 ` Andy Gross
-1 siblings, 0 replies; 18+ messages in thread
From: Andy Gross @ 2016-09-16 18:50 UTC (permalink / raw)
To: Iaroslav Gridin
Cc: david.brown, robh+dt, mark.rutland, linux, linux-arm-msm,
linux-soc, linux-arm-kernel, linux-kernel
On Fri, Sep 16, 2016 at 08:38:01PM +0300, Iaroslav Gridin wrote:
> On Thu, Sep 15, 2016 at 04:18:42PM -0500, Andy Gross wrote:
>
> > Actually, on thinking about this more, the bam block itself only requires the
> > single clock. The peripheral it is attached to has to keep its sanity during
> > the duration of the transfer (crypto). The crypto requires 3 clocks, one of
> > which is the same clk the bam requires.
> >
> > You can access the BAM registers with the bam_clk only, correct?
>
> Not preparing bam_clk degrades QCE performance about 3x, though.
If the CE2_CLK is the only required clk, that makes it the "bam_clk". I see the
crypto requires getting all three clocks: AXI (bus), AHB (iface), and CE2 (core)
If the crypto is active during DMA transfers, which it has to be, then the
performance shouldn't degrade due to the BAM not preparing the AHB.
Regards,
Andy
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
@ 2016-09-16 18:50 ` Andy Gross
0 siblings, 0 replies; 18+ messages in thread
From: Andy Gross @ 2016-09-16 18:50 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 16, 2016 at 08:38:01PM +0300, Iaroslav Gridin wrote:
> On Thu, Sep 15, 2016 at 04:18:42PM -0500, Andy Gross wrote:
>
> > Actually, on thinking about this more, the bam block itself only requires the
> > single clock. The peripheral it is attached to has to keep its sanity during
> > the duration of the transfer (crypto). The crypto requires 3 clocks, one of
> > which is the same clk the bam requires.
> >
> > You can access the BAM registers with the bam_clk only, correct?
>
> Not preparing bam_clk degrades QCE performance about 3x, though.
If the CE2_CLK is the only required clk, that makes it the "bam_clk". I see the
crypto requires getting all three clocks: AXI (bus), AHB (iface), and CE2 (core)
If the crypto is active during DMA transfers, which it has to be, then the
performance shouldn't degrade due to the BAM not preparing the AHB.
Regards,
Andy
^ permalink raw reply [flat|nested] 18+ messages in thread