[PATCH V3 0/2] Add QCrypto support for SC7280

[PATCH V3 0/2] Add QCrypto support for SC7280

[Om Prakash Singh]

Document SC7280 support for QCrypto driver and add QCE and Crypto BAM DMA nodes

Om Prakash Singh (2):
  dt-bindings: crypto: qcom-qce: document the SC7280 crypto engine
  arm64: dts: qcom: sc7280: add QCrypto nodes

 .../devicetree/bindings/crypto/qcom-qce.yaml  |  1 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi          | 22 +++++++++++++++++++
 2 files changed, 23 insertions(+)

-- 
2.25.1

[PATCH V3 1/2] dt-bindings: crypto: qcom-qce: document the SC7280 crypto engine

[Om Prakash Singh]

Document the crypto engine on the SM7280 Platform.

Signed-off-by: Om Prakash Singh <quic_omprsing@quicinc.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in V3:
	No change in this patch

Changes in V2:
	No change in this patch

 Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
index eeb8a956d7cb..1797699cf454 100644
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -44,6 +44,7 @@ properties:
 
       - items:
           - enum:
+              - qcom,sc7280-qce
               - qcom,sm8250-qce
               - qcom,sm8350-qce
               - qcom,sm8450-qce
-- 
2.25.1

Re: [PATCH V3 1/2] dt-bindings: crypto: qcom-qce: document the SC7280 crypto engine

[Herbert Xu]

On Thu, Dec 14, 2023 at 04:05:59PM +0530, Om Prakash Singh wrote:
> Document the crypto engine on the SM7280 Platform.
> 
> Signed-off-by: Om Prakash Singh <quic_omprsing@quicinc.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> Changes in V3:
> 	No change in this patch
> 
> Changes in V2:
> 	No change in this patch
> 
>  Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 1 +
>  1 file changed, 1 insertion(+)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH V3 2/2] arm64: dts: qcom: sc7280: add QCrypto nodes

[Om Prakash Singh]

Add the QCE and Crypto BAM DMA nodes.

Signed-off-by: Om Prakash Singh <quic_omprsing@quicinc.com>
---

Changes in V3:
  - V2 patch was sent without actual modification. Resending the patch with modified file.

Changes in V2:
  - Update DT node sequence as per register ascending order.
  - Fix DT node properties as per convention.

 arch/arm64/boot/dts/qcom/sc7280.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 66f1eb83cca7..b819724c1255 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2233,6 +2233,28 @@ pcie1_phy: phy@1c0e000 {
 			status = "disabled";
 		};
 
+		cryptobam: dma-controller@1dc4000 {
+			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
+			reg = <0x0 0x01dc4000 0x0 0x28000>;
+			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			iommus = <&apps_smmu 0x4e4 0x0011>,
+				 <&apps_smmu 0x4e6 0x0011>;
+			qcom,ee = <0>;
+			qcom,controlled-remotely;
+		};
+
+		crypto: crypto@1dfa000 {
+			compatible = "qcom,sc7280-qce", "qcom,sm8150-qce", "qcom,qce";
+			reg = <0x0 0x01dfa000 0x0 0x6000>;
+			dmas = <&cryptobam 4>, <&cryptobam 5>;
+			dma-names = "rx", "tx";
+			iommus = <&apps_smmu 0x4e4 0x0011>,
+				 <&apps_smmu 0x4e4 0x0011>;
+			interconnects = <&aggre2_noc MASTER_CRYPTO 0 &mc_virt SLAVE_EBI1 0>;
+			interconnect-names = "memory";
+		};
+
 		ipa: ipa@1e40000 {
 			compatible = "qcom,sc7280-ipa";
 
-- 
2.25.1

Re: [PATCH V3 2/2] arm64: dts: qcom: sc7280: add QCrypto nodes

[Luca Weiss]

On Thu Dec 14, 2023 at 11:36 AM CET, Om Prakash Singh wrote:
> Add the QCE and Crypto BAM DMA nodes.
>
> Signed-off-by: Om Prakash Singh <quic_omprsing@quicinc.com>
> ---
>
> Changes in V3:
>   - V2 patch was sent without actual modification. Resending the patch with modified file.
>
> Changes in V2:
>   - Update DT node sequence as per register ascending order.
>   - Fix DT node properties as per convention.
>
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 66f1eb83cca7..b819724c1255 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -2233,6 +2233,28 @@ pcie1_phy: phy@1c0e000 {
>  			status = "disabled";
>  		};
>  
> +		cryptobam: dma-controller@1dc4000 {
> +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> +			reg = <0x0 0x01dc4000 0x0 0x28000>;
> +			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			iommus = <&apps_smmu 0x4e4 0x0011>,
> +				 <&apps_smmu 0x4e6 0x0011>;
> +			qcom,ee = <0>;
> +			qcom,controlled-remotely;
> +		};

Hi,

Unfortunately I seem to have boot failure / device crash with cryptobam
enabled on my qcm6490-fairphone-fp5. Are you aware of any firmware
differences that could cause this with QCM6490 LA firmware?

Looking at downstream msm-5.4 dmesg I do see this BAM being used so it
should generally be accessible from Linux.

[    5.217214] qce 1de0000.qcedev: Adding to iommu group 18
[    5.223741] QCE50: __qce_get_device_tree_data: CE operating frequency is not defined, setting to default 100MHZ
[    5.234986] qce 1de0000.qcedev: QTI Crypto 5.6.0 device found @0x1de0000
[    5.242981] sps_register_bam_device: sps:BAM 0x0000000001dc4000 is registered
[    5.251124] sps_bam_enable: sps:BAM 0x0000000001dc4000 (va:0x000000001db63156) enabled: ver:0x27, number of pipes:16
[    5.262783] QCE50: qce_sps_init:  QTI MSM CE-BAM at 0x0000000001dc4000 irq 9
[    5.271820] qce 1de0000.qcedev:qcom_cedev_ns_cb: Adding to iommu group 19
[    5.281083] qce 1de0000.qcedev:qcom_cedev_s_cb: Adding to iommu group 20
[    5.289376] qcrypto 1de0000.qcrypto: Adding to iommu group 21
[    5.296326] QCE50: __qce_get_device_tree_data: CE operating frequency is not defined, setting to default 100MHZ
[    5.307675] qcrypto 1de0000.qcrypto: QTI Crypto 5.6.0 device found @0x1de0000
[    5.315867] QCE50: qce_sps_init:  QTI MSM CE-BAM at 0x0000000001dc4000 irq 9

Any idea?

Regards
Luca

> +
> +		crypto: crypto@1dfa000 {
> +			compatible = "qcom,sc7280-qce", "qcom,sm8150-qce", "qcom,qce";
> +			reg = <0x0 0x01dfa000 0x0 0x6000>;
> +			dmas = <&cryptobam 4>, <&cryptobam 5>;
> +			dma-names = "rx", "tx";
> +			iommus = <&apps_smmu 0x4e4 0x0011>,
> +				 <&apps_smmu 0x4e4 0x0011>;
> +			interconnects = <&aggre2_noc MASTER_CRYPTO 0 &mc_virt SLAVE_EBI1 0>;
> +			interconnect-names = "memory";
> +		};
> +
>  		ipa: ipa@1e40000 {
>  			compatible = "qcom,sc7280-ipa";
>  

Re: [PATCH V3 2/2] arm64: dts: qcom: sc7280: add QCrypto nodes

[Luca Weiss]

On Thu Dec 28, 2023 at 3:29 PM CET, Luca Weiss wrote:
> On Thu Dec 14, 2023 at 11:36 AM CET, Om Prakash Singh wrote:
> > Add the QCE and Crypto BAM DMA nodes.
> >
> > Signed-off-by: Om Prakash Singh <quic_omprsing@quicinc.com>
> > ---
> >
> > Changes in V3:
> >   - V2 patch was sent without actual modification. Resending the patch with modified file.
> >
> > Changes in V2:
> >   - Update DT node sequence as per register ascending order.
> >   - Fix DT node properties as per convention.
> >
> >  arch/arm64/boot/dts/qcom/sc7280.dtsi | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > index 66f1eb83cca7..b819724c1255 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -2233,6 +2233,28 @@ pcie1_phy: phy@1c0e000 {
> >  			status = "disabled";
> >  		};
> >  
> > +		cryptobam: dma-controller@1dc4000 {
> > +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> > +			reg = <0x0 0x01dc4000 0x0 0x28000>;
> > +			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> > +			#dma-cells = <1>;
> > +			iommus = <&apps_smmu 0x4e4 0x0011>,
> > +				 <&apps_smmu 0x4e6 0x0011>;
> > +			qcom,ee = <0>;
> > +			qcom,controlled-remotely;
> > +		};
>
> Hi,
>
> Unfortunately I seem to have boot failure / device crash with cryptobam
> enabled on my qcm6490-fairphone-fp5. Are you aware of any firmware
> differences that could cause this with QCM6490 LA firmware?
>
> Looking at downstream msm-5.4 dmesg I do see this BAM being used so it
> should generally be accessible from Linux.
>
> [    5.217214] qce 1de0000.qcedev: Adding to iommu group 18
> [    5.223741] QCE50: __qce_get_device_tree_data: CE operating frequency is not defined, setting to default 100MHZ
> [    5.234986] qce 1de0000.qcedev: QTI Crypto 5.6.0 device found @0x1de0000
> [    5.242981] sps_register_bam_device: sps:BAM 0x0000000001dc4000 is registered
> [    5.251124] sps_bam_enable: sps:BAM 0x0000000001dc4000 (va:0x000000001db63156) enabled: ver:0x27, number of pipes:16
> [    5.262783] QCE50: qce_sps_init:  QTI MSM CE-BAM at 0x0000000001dc4000 irq 9
> [    5.271820] qce 1de0000.qcedev:qcom_cedev_ns_cb: Adding to iommu group 19
> [    5.281083] qce 1de0000.qcedev:qcom_cedev_s_cb: Adding to iommu group 20
> [    5.289376] qcrypto 1de0000.qcrypto: Adding to iommu group 21
> [    5.296326] QCE50: __qce_get_device_tree_data: CE operating frequency is not defined, setting to default 100MHZ
> [    5.307675] qcrypto 1de0000.qcrypto: QTI Crypto 5.6.0 device found @0x1de0000
> [    5.315867] QCE50: qce_sps_init:  QTI MSM CE-BAM at 0x0000000001dc4000 irq 9

After some hints from Stephan Gerhold, it looks like when the bam driver
tries to read the registers for getting num-ees and num-channels we
need to have the interconnect up, otherwise the register read fails and
resets the device into EDL mode.

After applying the patch I've attached below the registers can be read
successfully.

The patch I've actually sent now[0] adds the values read from these
registers as static properties so that during cryptobam probe we don't
need to have the interconnect up because we don't read any registers.
Stephan Gerhold mentioned off-list that this is an adequate solution
because all the other times the cryptobam is used by the &crypto node
the interconnect will be up.

[0] https://lore.kernel.org/linux-arm-msm/20231229-sc7280-cryptobam-fixup-v1-1-bd8f68589b80@fairphone.com/T/#u

I imagine the cause why cryptobam & crypto are disabled in sm8350.dtsi
is the same, so the solution there probably also is just hardcoding
those properties and then using &crypto should work as expected.

Regards
Luca

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 83b5b76ba179..b2517631e884 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2345,6 +2345,9 @@ cryptobam: dma-controller@1dc4000 {
 				 <&apps_smmu 0x4e6 0x0011>;
 			qcom,ee = <0>;
 			qcom,controlled-remotely;
+			interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+			interconnect-names = "memory";
 		};
 
 		crypto: crypto@1dfa000 {
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 5e7d332731e0..b2f85c6bcbea 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -40,6 +40,7 @@
 #include <linux/circ_buf.h>
 #include <linux/clk.h>
 #include <linux/dmaengine.h>
+#include <linux/interconnect.h>
 #include <linux/pm_runtime.h>
 
 #include "../dmaengine.h"
@@ -394,6 +395,7 @@ struct bam_device {
 	const struct reg_offset_data *layout;
 
 	struct clk *bamclk;
+	struct icc_path *mem_path;
 	int irq;
 
 	/* dma start transaction tasklet */
@@ -1298,6 +1300,14 @@ static int bam_dma_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	bdev->mem_path = devm_of_icc_get(bdev->dev, "memory");
+	if (IS_ERR(bdev->mem_path))
+		return PTR_ERR(bdev->mem_path);
+
+	ret = icc_set_bw(bdev->mem_path, 1, 1);
+	if (ret)
+		return ret;
+
 	ret = bam_init(bdev);
 	if (ret)
 		goto err_disable_clk;

Re: (subset) [PATCH V3 0/2] Add QCrypto support for SC7280

[Bjorn Andersson]

On Thu, 14 Dec 2023 16:05:58 +0530, Om Prakash Singh wrote:
> Document SC7280 support for QCrypto driver and add QCE and Crypto BAM DMA nodes
> 
> Om Prakash Singh (2):
>   dt-bindings: crypto: qcom-qce: document the SC7280 crypto engine
>   arm64: dts: qcom: sc7280: add QCrypto nodes
> 
> .../devicetree/bindings/crypto/qcom-qce.yaml  |  1 +
>  arch/arm64/boot/dts/qcom/sc7280.dtsi          | 22 +++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> [...]

Applied, thanks!

[2/2] arm64: dts: qcom: sc7280: add QCrypto nodes
      commit: d488f903a8600203dc367985a0a7a742b530adc0

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>