| Message ID | 20230208183755.2907771-12-vladimir.zapolskiy@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | crypto: qcom-qce: Add YAML bindings & support for newer SoCs | expand |
Hi Vladimir, On 2/9/23 12:07 AM, Vladimir Zapolskiy wrote: > Add description of QCE and its corresponding BAM DMA IPs on SM8250 SoC. > > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > --- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi > index e59c16f74d17..d8698d18223e 100644 > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi > @@ -2215,6 +2215,30 @@ ufs_mem_phy_lanes: phy@1d87400 { > }; > }; > > + cryptobam: dma-controller@1dc4000 { > + compatible = "qcom,bam-v1.7.0"; > + reg = <0x0 0x01dc4000 0x0 0x24000>; > + interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>; > + #dma-cells = <1>; > + qcom,ee = <0>; > + qcom,controlled-remotely; > + num-channels = <8>; > + qcom,num-ees = <2>; > + iommus = <&apps_smmu 0x586 0x11>, > + <&apps_smmu 0x596 0x11>; > + }; > + > + crypto: crypto@1dfa000 { > + compatible = "qcom,sm8250-qce", "qcom,sm8150-qce"; > + reg = <0x0 0x01dfa000 0x0 0x6000>; > + dmas = <&cryptobam 6>, <&cryptobam 7>; > + dma-names = "rx", "tx"; > + interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 &mc_virt SLAVE_EBI_CH0>; > + interconnect-names = "memory"; > + iommus = <&apps_smmu 0x586 0x11>, > + <&apps_smmu 0x596 0x11>; > + }; > + > tcsr_mutex: hwlock@1f40000 { > compatible = "qcom,tcsr-mutex"; > reg = <0x0 0x01f40000 0x0 0x40000>; This patch was part of the v7 arm64 dts fixes I sent out - see [1]. Probably you can use it as a base and make the changes (interconnect property for the BAM DMA node and qce-compatible names) directly there and include it in your patch series. [1]. https://lore.kernel.org/linux-arm-msm/20220921045602.1462007-3-bhupesh.sharma@linaro.org/ Thanks, Bhupesh
Hi Bhupesh, On 2/9/23 14:21, Bhupesh Sharma wrote: > Hi Vladimir, > > On 2/9/23 12:07 AM, Vladimir Zapolskiy wrote: >> Add description of QCE and its corresponding BAM DMA IPs on SM8250 SoC. >> >> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/sm8250.dtsi | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi >> index e59c16f74d17..d8698d18223e 100644 >> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi >> @@ -2215,6 +2215,30 @@ ufs_mem_phy_lanes: phy@1d87400 { >> }; >> }; >> >> + cryptobam: dma-controller@1dc4000 { >> + compatible = "qcom,bam-v1.7.0"; >> + reg = <0x0 0x01dc4000 0x0 0x24000>; >> + interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>; >> + #dma-cells = <1>; >> + qcom,ee = <0>; >> + qcom,controlled-remotely; >> + num-channels = <8>; >> + qcom,num-ees = <2>; >> + iommus = <&apps_smmu 0x586 0x11>, >> + <&apps_smmu 0x596 0x11>; >> + }; >> + >> + crypto: crypto@1dfa000 { >> + compatible = "qcom,sm8250-qce", "qcom,sm8150-qce"; >> + reg = <0x0 0x01dfa000 0x0 0x6000>; >> + dmas = <&cryptobam 6>, <&cryptobam 7>; >> + dma-names = "rx", "tx"; >> + interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 &mc_virt SLAVE_EBI_CH0>; >> + interconnect-names = "memory"; >> + iommus = <&apps_smmu 0x586 0x11>, >> + <&apps_smmu 0x596 0x11>; >> + }; >> + >> tcsr_mutex: hwlock@1f40000 { >> compatible = "qcom,tcsr-mutex"; >> reg = <0x0 0x01f40000 0x0 0x40000>; > > This patch was part of the v7 arm64 dts fixes I sent out - see [1]. thank you for pointing it out, so there are two different v7 series of related patches, as it's stated in the cover letter I based my v8 on top of linux-crypto changes [*], and this particular change was developed independently from yours. And so, there are some differences, can you please comment on them? - My change does not have 'interconnects' property under dma-controller@1dc4000 device tree node, I believe it is not needed, but please correct me here. - My change uses DMA channel number taken from the downstream code [**], is there a reason to use different ones like in your change? - My change has a shorter list of IOMMUs also copied from the same downstream code, is there a reason to use a different extended list like in your change? - On my end I have to retest your change without 'num-channels' and 'qcom,num-ees' properties, since this also seems as an important difference between two patches. Nevertheless, thank you again for bringing my attention to a different patch series, I'll reuse the changes from it, and also publish all of them together and in a separate changeset as Krzysztof suggested. > Probably you can use it as a base and make the changes (interconnect > property for the BAM DMA node and qce-compatible names) directly there > and include it in your patch series. [*] https://lore.kernel.org/linux-arm-msm/20220920114051.1116441-1-bhupesh.sharma@linaro.org/ [**] https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/arch/arm64/boot/dts/qcom/kona.dtsi?h=LA.UM.8.12.3.1&id=f3dd4aaeb34438c877ccd42f5a48ccd554dd765a#n2979 > [1]. > https://lore.kernel.org/linux-arm-msm/20220921045602.1462007-3-bhupesh.sharma@linaro.org/ -- Best wishes, Vladimir
Hi Vladimir, On 2/9/23 8:00 PM, Vladimir Zapolskiy wrote: > Hi Bhupesh, > > On 2/9/23 14:21, Bhupesh Sharma wrote: >> Hi Vladimir, >> >> On 2/9/23 12:07 AM, Vladimir Zapolskiy wrote: >>> Add description of QCE and its corresponding BAM DMA IPs on SM8250 SoC. >>> >>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> >>> --- >>> arch/arm64/boot/dts/qcom/sm8250.dtsi | 24 ++++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi >>> b/arch/arm64/boot/dts/qcom/sm8250.dtsi >>> index e59c16f74d17..d8698d18223e 100644 >>> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi >>> @@ -2215,6 +2215,30 @@ ufs_mem_phy_lanes: phy@1d87400 { >>> }; >>> }; >>> + cryptobam: dma-controller@1dc4000 { >>> + compatible = "qcom,bam-v1.7.0"; >>> + reg = <0x0 0x01dc4000 0x0 0x24000>; >>> + interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>; >>> + #dma-cells = <1>; >>> + qcom,ee = <0>; >>> + qcom,controlled-remotely; >>> + num-channels = <8>; >>> + qcom,num-ees = <2>; >>> + iommus = <&apps_smmu 0x586 0x11>, >>> + <&apps_smmu 0x596 0x11>; >>> + }; >>> + >>> + crypto: crypto@1dfa000 { >>> + compatible = "qcom,sm8250-qce", "qcom,sm8150-qce"; >>> + reg = <0x0 0x01dfa000 0x0 0x6000>; >>> + dmas = <&cryptobam 6>, <&cryptobam 7>; >>> + dma-names = "rx", "tx"; >>> + interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 >>> &mc_virt SLAVE_EBI_CH0>; >>> + interconnect-names = "memory"; >>> + iommus = <&apps_smmu 0x586 0x11>, >>> + <&apps_smmu 0x596 0x11>; >>> + }; >>> + >>> tcsr_mutex: hwlock@1f40000 { >>> compatible = "qcom,tcsr-mutex"; >>> reg = <0x0 0x01f40000 0x0 0x40000>; >> >> This patch was part of the v7 arm64 dts fixes I sent out - see [1]. > > thank you for pointing it out, so there are two different v7 series of > related > patches, as it's stated in the cover letter I based my v8 on top of > linux-crypto > changes [*], and this particular change was developed independently from > yours. > > And so, there are some differences, can you please comment on them? > > - My change does not have 'interconnects' property under > dma-controller@1dc4000 > device tree node, I believe it is not needed, but please correct me > here. > - My change uses DMA channel number taken from the downstream code [**], is > there a reason to use different ones like in your change? > - My change has a shorter list of IOMMUs also copied from the same > downstream > code, is there a reason to use a different extended list like in your > change? > - On my end I have to retest your change without 'num-channels' and > 'qcom,num-ees' > properties, since this also seems as an important difference between > two patches. > > Nevertheless, thank you again for bringing my attention to a different > patch > series, I'll reuse the changes from it, and also publish all of them > together and > in a separate changeset as Krzysztof suggested. > >> Probably you can use it as a base and make the changes (interconnect >> property for the BAM DMA node and qce-compatible names) directly there >> and include it in your patch series. > > [*] > https://lore.kernel.org/linux-arm-msm/20220920114051.1116441-1-bhupesh.sharma@linaro.org/ > [**] > https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/arch/arm64/boot/dts/qcom/kona.dtsi?h=LA.UM.8.12.3.1&id=f3dd4aaeb34438c877ccd42f5a48ccd554dd765a#n2979 > >> [1]. >> https://lore.kernel.org/linux-arm-msm/20220921045602.1462007-3-bhupesh.sharma@linaro.org/ Now that you mention it, I am a bit confused. My v7 series had 3 sub-series (one each for the arm64 dts + defconfig, crypto & dma tree): - https://lore.kernel.org/linux-arm-msm/20220921045602.1462007-1-bhupesh.sharma@linaro.org/ - https://lore.kernel.org/linux-arm-msm/20220920114051.1116441-1-bhupesh.sharma@linaro.org/ - https://lore.kernel.org/linux-arm-msm/20220921030649.1436434-1-bhupesh.sharma@linaro.org/ Where do you propose the v9 of the dma tree patch? I think you have already clubbed the v9 arm64 dts patchset with this series (you have left out the defconfig patch from there as it is already merged and the sm8150 dts change, so that I can send it later), right? Having said that, with only the v9 crypto series you shared, I am not able to bring up crypto on either sm8250-mtp or (with one dts patch added) on sa8155p-adp board(s). So, do I need to make other changes (include patches from my earlier series) to get the same working? Thanks, Bhupesh
Hi Bhupesh, On 2/13/23 19:12, Bhupesh Sharma wrote: > Hi Vladimir, > > On 2/9/23 8:00 PM, Vladimir Zapolskiy wrote: >> Hi Bhupesh, >> >> On 2/9/23 14:21, Bhupesh Sharma wrote: >>> Hi Vladimir, >>> >>> On 2/9/23 12:07 AM, Vladimir Zapolskiy wrote: >>>> Add description of QCE and its corresponding BAM DMA IPs on SM8250 SoC. >>>> >>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> >>>> --- >>>> arch/arm64/boot/dts/qcom/sm8250.dtsi | 24 ++++++++++++++++++++++++ >>>> 1 file changed, 24 insertions(+) >>>> >>>> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi >>>> b/arch/arm64/boot/dts/qcom/sm8250.dtsi >>>> index e59c16f74d17..d8698d18223e 100644 >>>> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi >>>> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi >>>> @@ -2215,6 +2215,30 @@ ufs_mem_phy_lanes: phy@1d87400 { >>>> }; >>>> }; >>>> + cryptobam: dma-controller@1dc4000 { >>>> + compatible = "qcom,bam-v1.7.0"; >>>> + reg = <0x0 0x01dc4000 0x0 0x24000>; >>>> + interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>; >>>> + #dma-cells = <1>; >>>> + qcom,ee = <0>; >>>> + qcom,controlled-remotely; >>>> + num-channels = <8>; >>>> + qcom,num-ees = <2>; >>>> + iommus = <&apps_smmu 0x586 0x11>, >>>> + <&apps_smmu 0x596 0x11>; >>>> + }; >>>> + >>>> + crypto: crypto@1dfa000 { >>>> + compatible = "qcom,sm8250-qce", "qcom,sm8150-qce"; >>>> + reg = <0x0 0x01dfa000 0x0 0x6000>; >>>> + dmas = <&cryptobam 6>, <&cryptobam 7>; >>>> + dma-names = "rx", "tx"; >>>> + interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 >>>> &mc_virt SLAVE_EBI_CH0>; >>>> + interconnect-names = "memory"; >>>> + iommus = <&apps_smmu 0x586 0x11>, >>>> + <&apps_smmu 0x596 0x11>; >>>> + }; >>>> + >>>> tcsr_mutex: hwlock@1f40000 { >>>> compatible = "qcom,tcsr-mutex"; >>>> reg = <0x0 0x01f40000 0x0 0x40000>; >>> >>> This patch was part of the v7 arm64 dts fixes I sent out - see [1]. >> >> thank you for pointing it out, so there are two different v7 series of >> related >> patches, as it's stated in the cover letter I based my v8 on top of >> linux-crypto >> changes [*], and this particular change was developed independently from >> yours. >> >> And so, there are some differences, can you please comment on them? >> >> - My change does not have 'interconnects' property under >> dma-controller@1dc4000 >> device tree node, I believe it is not needed, but please correct me >> here. >> - My change uses DMA channel number taken from the downstream code [**], is >> there a reason to use different ones like in your change? >> - My change has a shorter list of IOMMUs also copied from the same >> downstream >> code, is there a reason to use a different extended list like in your >> change? >> - On my end I have to retest your change without 'num-channels' and >> 'qcom,num-ees' >> properties, since this also seems as an important difference between >> two patches. >> >> Nevertheless, thank you again for bringing my attention to a different >> patch >> series, I'll reuse the changes from it, and also publish all of them >> together and >> in a separate changeset as Krzysztof suggested. >> >>> Probably you can use it as a base and make the changes (interconnect >>> property for the BAM DMA node and qce-compatible names) directly there >>> and include it in your patch series. >> >> [*] >> https://lore.kernel.org/linux-arm-msm/20220920114051.1116441-1-bhupesh.sharma@linaro.org/ >> [**] >> https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/arch/arm64/boot/dts/qcom/kona.dtsi?h=LA.UM.8.12.3.1&id=f3dd4aaeb34438c877ccd42f5a48ccd554dd765a#n2979 >> >>> [1]. >>> https://lore.kernel.org/linux-arm-msm/20220921045602.1462007-3-bhupesh.sharma@linaro.org/ > > Now that you mention it, I am a bit confused. My v7 series had 3 > sub-series (one each for the arm64 dts + defconfig, crypto & dma tree): > > - > https://lore.kernel.org/linux-arm-msm/20220921045602.1462007-1-bhupesh.sharma@linaro.org/ > - > https://lore.kernel.org/linux-arm-msm/20220920114051.1116441-1-bhupesh.sharma@linaro.org/ > - > https://lore.kernel.org/linux-arm-msm/20220921030649.1436434-1-bhupesh.sharma@linaro.org/ Thank you, I will follow these series, the interconnects on DMA BAM series is slipped from my attention, however I am able to run QCE without adding interconnects to DMA BAM on available to me hardware. > Where do you propose the v9 of the dma tree patch? I think you have > already clubbed the v9 arm64 dts patchset with this series > (you have left out the defconfig patch from there as it is already > merged and the sm8150 dts change, so that I can send it later), right? > > Having said that, with only the v9 crypto series you shared, I am not > able to bring up crypto on either sm8250-mtp or (with one dts patch > added) on sa8155p-adp board(s). So, do I need to make other changes > (include patches from my earlier series) to get the same working? No, the shared v9 is complete, and I test it on top of the linux-next. I'm able to successfully test QCE by running 'cryptsetup benchmark' on RB5 hardware, the involvement of QCE and DMA BAM IPs can be checked either by looking at bam_dma interrupt numbers from /proc/interrupts, by adding qcrypto.dyndbg=+p into the kernel command line or by registered algos and priorities found at /proc/crypto: [ 9.515729] qcrypto 1dfa000.crypto: Crypto device found, version 5.5.0 I will be glad to know your opinion about the technical difference between our two different sm8250.dtsi changes, please find my queries in the previous email. -- Best wishes, Vladimir
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi index e59c16f74d17..d8698d18223e 100644 --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi @@ -2215,6 +2215,30 @@ ufs_mem_phy_lanes: phy@1d87400 { }; }; + cryptobam: dma-controller@1dc4000 { + compatible = "qcom,bam-v1.7.0"; + reg = <0x0 0x01dc4000 0x0 0x24000>; + interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>; + #dma-cells = <1>; + qcom,ee = <0>; + qcom,controlled-remotely; + num-channels = <8>; + qcom,num-ees = <2>; + iommus = <&apps_smmu 0x586 0x11>, + <&apps_smmu 0x596 0x11>; + }; + + crypto: crypto@1dfa000 { + compatible = "qcom,sm8250-qce", "qcom,sm8150-qce"; + reg = <0x0 0x01dfa000 0x0 0x6000>; + dmas = <&cryptobam 6>, <&cryptobam 7>; + dma-names = "rx", "tx"; + interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 &mc_virt SLAVE_EBI_CH0>; + interconnect-names = "memory"; + iommus = <&apps_smmu 0x586 0x11>, + <&apps_smmu 0x596 0x11>; + }; + tcsr_mutex: hwlock@1f40000 { compatible = "qcom,tcsr-mutex"; reg = <0x0 0x01f40000 0x0 0x40000>;
Add description of QCE and its corresponding BAM DMA IPs on SM8250 SoC. Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> --- arch/arm64/boot/dts/qcom/sm8250.dtsi | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)