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

Groups > linux.kernel > #1484547 > unrolled thread

Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam Andy Gross <andy.gross@linaro.org> - 2016-09-15 23:20 +0200
    Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam Iaroslav Gridin <voker57@gmail.com> - 2016-09-16 14:00 +0200
    Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam Iaroslav Gridin <voker57@gmail.com> - 2016-09-16 19:40 +0200
      Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam Andy Gross <andy.gross@linaro.org> - 2016-09-16 21:00 +0200

#1484547 — Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam

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

[toc] | [next] | [standalone]

#1484916

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.

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

#1485204

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.

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

#1485245

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web