| Message ID | 20231214103600.2613988-3-quic_omprsing@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d488f903a8600203dc367985a0a7a742b530adc0 |
| Headers | show |
| Series | Add QCrypto support for SC7280 | expand |
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"; >
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;
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";
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(+)