| Message ID | 20220919221509.1057574-2-bhupesh.sharma@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: dts + defconfig: Add support for Qualcomm QCE block on new SoCs and in defconfig | expand |
On 20/09/2022 00:15, Bhupesh Sharma wrote: > Since we are using soc specific qce crypto IP compatibles > in the bindings now, use the same in the device tree files > which include the crypto nodes. > > Cc: Bjorn Andersson <andersson@kernel.org> > Cc: Rob Herring <robh@kernel.org> > Tested-by: Jordan Crouse <jorcrous@amazon.com> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > arch/arm/boot/dts/qcom-ipq4019.dtsi | 2 +- > arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +- > arch/arm64/boot/dts/qcom/ipq8074.dtsi | 2 +- > arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +- > 5 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi > index b23591110bd2..9c40714562d5 100644 > --- a/arch/arm/boot/dts/qcom-ipq4019.dtsi > +++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi > @@ -314,7 +314,7 @@ cryptobam: dma-controller@8e04000 { > }; > > crypto: crypto@8e3a000 { > - compatible = "qcom,crypto-v5.1"; > + compatible = "qcom,ipq4019-qce"; There are few issues here: 1. Compatible is not documented. 2. Compatible is not supported by old kernel - ABI break. 3. Everything won't be bisectable... The same in other places. Best regards, Krzysztof
On 9/20/22 12:55 PM, Krzysztof Kozlowski wrote: > On 20/09/2022 00:15, Bhupesh Sharma wrote: >> Since we are using soc specific qce crypto IP compatibles >> in the bindings now, use the same in the device tree files >> which include the crypto nodes. >> >> Cc: Bjorn Andersson <andersson@kernel.org> >> Cc: Rob Herring <robh@kernel.org> >> Tested-by: Jordan Crouse <jorcrous@amazon.com> >> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> >> --- >> arch/arm/boot/dts/qcom-ipq4019.dtsi | 2 +- >> arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +- >> arch/arm64/boot/dts/qcom/ipq8074.dtsi | 2 +- >> arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +- >> arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +- >> 5 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi >> index b23591110bd2..9c40714562d5 100644 >> --- a/arch/arm/boot/dts/qcom-ipq4019.dtsi >> +++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi >> @@ -314,7 +314,7 @@ cryptobam: dma-controller@8e04000 { >> }; >> >> crypto: crypto@8e3a000 { >> - compatible = "qcom,crypto-v5.1"; >> + compatible = "qcom,ipq4019-qce"; > > There are few issues here: > 1. Compatible is not documented. Its documented here: https://lore.kernel.org/linux-arm-msm/30756e6f-952f-ccf2-b493-e515ba4f0a64@linaro.org/ [as mentioned in the dependency section in the cover letter :)] > 2. Compatible is not supported by old kernel - ABI break. > 3. Everything won't be bisectable... I think its a question of dependencies b/w the patchsets intended for separate areas. Let me think more on how, I can resolve it in newer versions. Thanks, Bhupesh > The same in other places. > > Best regards, > Krzysztof
On 20/09/2022 10:57, Bhupesh Sharma wrote: >>> crypto: crypto@8e3a000 { >>> - compatible = "qcom,crypto-v5.1"; >>> + compatible = "qcom,ipq4019-qce"; >> >> There are few issues here: >> 1. Compatible is not documented. > > Its documented here: > https://lore.kernel.org/linux-arm-msm/30756e6f-952f-ccf2-b493-e515ba4f0a64@linaro.org/ > > [as mentioned in the dependency section in the cover letter :)] > >> 2. Compatible is not supported by old kernel - ABI break. You cannot fix this with dependencies/ordering. >> 3. Everything won't be bisectable... > > I think its a question of dependencies b/w the patchsets intended for > separate areas. Let me think more on how, I can resolve it in newer > versions. DTS always goes separately so this also cannot be fixed with ordering or dependencies. However if Bjorn is fine with it, it's good. Best regards, Krzysztof
On Tue, 20 Sept 2022 at 15:09, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 20/09/2022 10:57, Bhupesh Sharma wrote: > >>> crypto: crypto@8e3a000 { > >>> - compatible = "qcom,crypto-v5.1"; > >>> + compatible = "qcom,ipq4019-qce"; > >> > >> There are few issues here: > >> 1. Compatible is not documented. > > > > Its documented here: > > https://lore.kernel.org/linux-arm-msm/30756e6f-952f-ccf2-b493-e515ba4f0a64@linaro.org/ > > > > [as mentioned in the dependency section in the cover letter :)] > > > >> 2. Compatible is not supported by old kernel - ABI break. > > You cannot fix this with dependencies/ordering. > > >> 3. Everything won't be bisectable... > > > > I think its a question of dependencies b/w the patchsets intended for > > separate areas. Let me think more on how, I can resolve it in newer > > versions. > > DTS always goes separately so this also cannot be fixed with ordering or > dependencies. However if Bjorn is fine with it, it's good. Sure, I get your point. SInce I haven't sent out the crypto driver and DMA driver subsets yet, let me stop and respin the series with the dt-bindings changes clubbed with the crypto driver patches in a single patchset. I can keep the DMA and dts patchsets separate and send them out separately. I think that should help maintain the ABI and backward compatibility. Please let me know if you think otherwise. Thanks, Bhupesh
On 20/09/2022 12:48, Bhupesh Sharma wrote: > On Tue, 20 Sept 2022 at 15:09, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 20/09/2022 10:57, Bhupesh Sharma wrote: >>>>> crypto: crypto@8e3a000 { >>>>> - compatible = "qcom,crypto-v5.1"; >>>>> + compatible = "qcom,ipq4019-qce"; >>>> >>>> There are few issues here: >>>> 1. Compatible is not documented. >>> >>> Its documented here: >>> https://lore.kernel.org/linux-arm-msm/30756e6f-952f-ccf2-b493-e515ba4f0a64@linaro.org/ >>> >>> [as mentioned in the dependency section in the cover letter :)] >>> >>>> 2. Compatible is not supported by old kernel - ABI break. >> >> You cannot fix this with dependencies/ordering. >> >>>> 3. Everything won't be bisectable... >>> >>> I think its a question of dependencies b/w the patchsets intended for >>> separate areas. Let me think more on how, I can resolve it in newer >>> versions. >> >> DTS always goes separately so this also cannot be fixed with ordering or >> dependencies. However if Bjorn is fine with it, it's good. > > Sure, I get your point. SInce I haven't sent out the crypto driver and > DMA driver subsets yet, let me stop and respin the series with the > dt-bindings changes clubbed with the crypto driver patches in a single > patchset. I can keep the DMA and dts patchsets separate and send them > out separately. > > I think that should help maintain the ABI and backward compatibility. > Please let me know if you think otherwise. I actually don't know what's in the drivers, so maybe there is no ABI break by kernel... but you are changing the compatibles in DTS thus any other project using them will be still broken. Best regards, Krzysztof
On Tue, 20 Sept 2022 at 16:46, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 20/09/2022 12:48, Bhupesh Sharma wrote: > > On Tue, 20 Sept 2022 at 15:09, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 20/09/2022 10:57, Bhupesh Sharma wrote: > >>>>> crypto: crypto@8e3a000 { > >>>>> - compatible = "qcom,crypto-v5.1"; > >>>>> + compatible = "qcom,ipq4019-qce"; > >>>> > >>>> There are few issues here: > >>>> 1. Compatible is not documented. > >>> > >>> Its documented here: > >>> https://lore.kernel.org/linux-arm-msm/30756e6f-952f-ccf2-b493-e515ba4f0a64@linaro.org/ > >>> > >>> [as mentioned in the dependency section in the cover letter :)] > >>> > >>>> 2. Compatible is not supported by old kernel - ABI break. > >> > >> You cannot fix this with dependencies/ordering. > >> > >>>> 3. Everything won't be bisectable... > >>> > >>> I think its a question of dependencies b/w the patchsets intended for > >>> separate areas. Let me think more on how, I can resolve it in newer > >>> versions. > >> > >> DTS always goes separately so this also cannot be fixed with ordering or > >> dependencies. However if Bjorn is fine with it, it's good. > > > > Sure, I get your point. SInce I haven't sent out the crypto driver and > > DMA driver subsets yet, let me stop and respin the series with the > > dt-bindings changes clubbed with the crypto driver patches in a single > > patchset. I can keep the DMA and dts patchsets separate and send them > > out separately. > > > > I think that should help maintain the ABI and backward compatibility. > > Please let me know if you think otherwise. > > I actually don't know what's in the drivers, so maybe there is no ABI > break by kernel... but you are changing the compatibles in DTS thus any > other project using them will be still broken. I have sent out the crypto and dt-bindings clubbed together as one patchset in the v7 version (see [1]). [1]. https://lore.kernel.org/linux-arm-msm/20220920114051.1116441-1-bhupesh.sharma@linaro.org/ Thanks, Bhupesh
diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi index b23591110bd2..9c40714562d5 100644 --- a/arch/arm/boot/dts/qcom-ipq4019.dtsi +++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi @@ -314,7 +314,7 @@ cryptobam: dma-controller@8e04000 { }; crypto: crypto@8e3a000 { - compatible = "qcom,crypto-v5.1"; + compatible = "qcom,ipq4019-qce"; reg = <0x08e3a000 0x6000>; clocks = <&gcc GCC_CRYPTO_AHB_CLK>, <&gcc GCC_CRYPTO_AXI_CLK>, diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi index a7c7ca980a71..0ae3c601b279 100644 --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi @@ -198,7 +198,7 @@ cryptobam: dma-controller@704000 { }; crypto: crypto@73a000 { - compatible = "qcom,crypto-v5.1"; + compatible = "qcom,ipq6018-qce"; reg = <0x0 0x0073a000 0x0 0x6000>; clocks = <&gcc GCC_CRYPTO_AHB_CLK>, <&gcc GCC_CRYPTO_AXI_CLK>, diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi index a47acf9bdf24..0683ef931413 100644 --- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi +++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi @@ -286,7 +286,7 @@ cryptobam: dma-controller@704000 { }; crypto: crypto@73a000 { - compatible = "qcom,crypto-v5.1"; + compatible = "qcom,ipq8074-qce"; reg = <0x0073a000 0x6000>; clocks = <&gcc GCC_CRYPTO_AHB_CLK>, <&gcc GCC_CRYPTO_AXI_CLK>, diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi index c0a2baffa49d..0dd6e1fea99c 100644 --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi @@ -755,7 +755,7 @@ cryptobam: dma-controller@644000 { }; crypto: crypto@67a000 { - compatible = "qcom,crypto-v5.4"; + compatible = "qcom,msm8996-qce"; reg = <0x0067a000 0x6000>; clocks = <&gcc GCC_CE1_AHB_CLK>, <&gcc GCC_CE1_AXI_CLK>, diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index d761da47220d..4aa5a82bd265 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -2566,7 +2566,7 @@ cryptobam: dma-controller@1dc4000 { }; crypto: crypto@1dfa000 { - compatible = "qcom,crypto-v5.4"; + compatible = "qcom,sdm845-qce"; reg = <0 0x01dfa000 0 0x6000>; clocks = <&gcc GCC_CE1_AHB_CLK>, <&gcc GCC_CE1_AXI_CLK>,