2022-09-19 22:21:48

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v6 0/4 RESEND] ARM: dts + defconfig: Add support for Qualcomm QCE block on new SoCs and in defconfig

Fixed the typos in the cover letter while resending this version.

Changes since v5:
=================
- v5 can be seen here: https://lore.kernel.org/lkml/[email protected]/
- As per Bjorn's suggestion on irc, broke down the patchset into 4
separate patchsets, one each for the following areas to allow easier
review and handling from the respective maintainer(s):
'arm-msm', 'crypto', 'dma' and 'devicetree'
This patchset is directed for the 'arm-msm' tree / area.
- Addressed Rob's, Vladimir's and Bjorn's review comments on v5.
- Added Tested-by from Jordan received on the v5.
- Also added a 'defconfig' change where I enabled the QCE block as a module.

Changes since v4:
=================
- v4 for sm8250 can be seen here: https://lore.kernel.org/linux-arm-msm/[email protected]/
- v1 for sm8150 qce enablement can be seen here: https://lore.kernel.org/linux-arm-msm/[email protected]/
- Merged the sm8150 and sm8250 enablement patches in the same patchset,
as per suggestions from Bjorn.
- Dropped a couple of patches from v4, as these have been picked by
Bjorn already via his tree.
- Addressed review comments from Vladimir, Thara and Rob.
- Collect Reviewed-by from Rob and Thara on some of the patches from the
v4 patchset.

Changes since v3:
=================
- v3 can be seen here: https://lore.kernel.org/linux-arm-msm/[email protected]/
- Dropped a couple of patches from v3, on basis of the review comments:
~ [PATCH 13/17] crypto: qce: core: Make clocks optional
~ [PATCH 15/17] crypto: qce: Convert the device found dev_dbg() to dev_info()
- Addressed review comments from Thara, Rob and Stephan Gerhold.
- Collect Reviewed-by from Rob and Thara on some of the patches from the
v3 patchset.

Changes since v2:
=================
- v2 can be seen here: https://lore.kernel.org/dmaengine/[email protected]/
- Drop a couple of patches from v1, which tried to address the defered
probing of qce driver in case bam dma driver is not yet probed.
Replace it instead with a single (simpler) patch [PATCH 16/17].
- Convert bam dma and qce crypto dt-bindings to YAML.
- Addressed review comments from Thara, Bjorn, Vinod and Rob.

Changes since v1:
=================
- v1 can be seen here: https://lore.kernel.org/linux-arm-msm/[email protected]/
- v1 did not work well as reported earlier by Dmitry, so v2 contains the following
changes/fixes:
~ Enable the interconnect path b/w BAM DMA and main memory first
before trying to access the BAM DMA registers.
~ Enable the interconnect path b/w qce crytpo and main memory first
before trying to access the qce crypto registers.
~ Make sure to document the required and optional properties for both
BAM DMA and qce crypto drivers.
~ Add a few debug related print messages in case the qce crypto driver
passes or fails to probe.
~ Convert the qce crypto driver probe to a defered one in case the BAM DMA
or the interconnect driver(s) (needed on specific Qualcomm parts) are not
yet probed.

Qualcomm crypto engine (qce) is available on several Snapdragon SoCs.
The qce block supports hardware accelerated algorithms for encryption
and authentication. It also provides support for aes, des, 3des
encryption algorithms and sha1, sha256, hmac(sha1), hmac(sha256)
authentication algorithms.

Tested the enabled crypto algorithms with cryptsetup test utilities
on sm8150-mtp, sa8155p-adp, sm8250-mtp and RB5 boards (see [1]) and
also with crypto self-tests, including the fuzz tests
(CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y).

Note that this patchset is dependent on the dt-bindings patchset (see [2]) sent to devicetree list.

[1]. https://linux.die.net/man/8/cryptsetup
[2]. https://lore.kernel.org/linux-arm-msm/[email protected]/

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Tested-by: Jordan Crouse <[email protected]>

Bhupesh Sharma (4):
ARM: dts: qcom: Use new compatibles for crypto nodes
arm64: dts: qcom: sm8250: Add dt entries to support crypto engine.
arm64: dts: qcom: sm8150: Add dt entries to support crypto engine.
arm64: defconfig: Enable Qualcomm QCE crypto

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 +-
arch/arm64/boot/dts/qcom/sm8150.dtsi | 28 +++++++++++++++++++++++++++
arch/arm64/boot/dts/qcom/sm8250.dtsi | 28 +++++++++++++++++++++++++++
arch/arm64/configs/defconfig | 1 +
8 files changed, 62 insertions(+), 5 deletions(-)

--
2.37.1


2022-09-19 22:35:03

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v6 3/4 RESEND] arm64: dts: qcom: sm8150: Add dt entries to support crypto engine.

Add crypto engine (CE) and CE BAM related nodes and definitions to
'sm8150.dtsi'.

Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Tested-by: Jordan Crouse <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8150.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index cef8c4f4f0ff..6e21352a158c 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -2041,6 +2041,34 @@ ufs_mem_phy_lanes: phy@1d87400 {
};
};

+ cryptobam: dma-controller@1dc4000 {
+ compatible = "qcom,bam-v1.7.0";
+ reg = <0 0x01dc4000 0 0x24000>;
+ interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
+ #dma-cells = <1>;
+ qcom,ee = <0>;
+ qcom,controlled-remotely;
+ iommus = <&apps_smmu 0x504 0x0011>,
+ <&apps_smmu 0x506 0x0011>,
+ <&apps_smmu 0x514 0x0011>,
+ <&apps_smmu 0x516 0x0011>;
+ interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 &mc_virt SLAVE_EBI_CH0>;
+ interconnect-names = "memory";
+ };
+
+ crypto: crypto@1dfa000 {
+ compatible = "qcom,sm8150-qce";
+ reg = <0 0x01dfa000 0 0x6000>;
+ dmas = <&cryptobam 4>, <&cryptobam 5>;
+ dma-names = "rx", "tx";
+ iommus = <&apps_smmu 0x504 0x0011>,
+ <&apps_smmu 0x506 0x0011>,
+ <&apps_smmu 0x514 0x0011>,
+ <&apps_smmu 0x516 0x0011>;
+ interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 &mc_virt SLAVE_EBI_CH0>;
+ interconnect-names = "memory";
+ };
+
ipa_virt: interconnect@1e00000 {
compatible = "qcom,sm8150-ipa-virt";
reg = <0 0x01e00000 0 0x1000>;
--
2.37.1

2022-09-19 23:11:18

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v6 2/4 RESEND] arm64: dts: qcom: sm8250: Add dt entries to support crypto engine.

Add crypto engine (CE) and CE BAM related nodes and definitions to
'sm8250.dtsi'.

Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Tested-by: Jordan Crouse <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index a5b62cadb129..7b3af34f8486 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -2188,6 +2188,34 @@ ufs_mem_phy_lanes: phy@1d87400 {
};
};

+ cryptobam: dma-controller@1dc4000 {
+ compatible = "qcom,bam-v1.7.0";
+ reg = <0 0x01dc4000 0 0x24000>;
+ interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
+ #dma-cells = <1>;
+ qcom,ee = <0>;
+ qcom,controlled-remotely;
+ iommus = <&apps_smmu 0x584 0x0011>,
+ <&apps_smmu 0x586 0x0011>,
+ <&apps_smmu 0x594 0x0011>,
+ <&apps_smmu 0x596 0x0011>;
+ interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 &mc_virt SLAVE_EBI_CH0>;
+ interconnect-names = "memory";
+ };
+
+ crypto: crypto@1dfa000 {
+ compatible = "qcom,sm8250-qce";
+ reg = <0 0x01dfa000 0 0x6000>;
+ dmas = <&cryptobam 4>, <&cryptobam 5>;
+ dma-names = "rx", "tx";
+ iommus = <&apps_smmu 0x584 0x0011>,
+ <&apps_smmu 0x586 0x0011>,
+ <&apps_smmu 0x594 0x0011>,
+ <&apps_smmu 0x596 0x0011>;
+ interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 &mc_virt SLAVE_EBI_CH0>;
+ interconnect-names = "memory";
+ };
+
ipa_virt: interconnect@1e00000 {
compatible = "qcom,sm8250-ipa-virt";
reg = <0 0x01e00000 0 0x1000>;
--
2.37.1

2022-09-19 23:19:38

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v6 4/4 RESEND] arm64: defconfig: Enable Qualcomm QCE crypto

Now that the QCE crypto block is supported on several
Qualcomm SoCs upstream, enable the same as a module in the
arm64 defconfig.

Cc: Bjorn Andersson <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 5a4ba141d15c..46d6c95b8d25 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1321,6 +1321,7 @@ CONFIG_CRYPTO_USER_API_RNG=m
CONFIG_CRYPTO_DEV_SUN8I_CE=m
CONFIG_CRYPTO_DEV_FSL_CAAM=m
CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM=m
+CONFIG_CRYPTO_DEV_QCE=m
CONFIG_CRYPTO_DEV_QCOM_RNG=m
CONFIG_CRYPTO_DEV_CCREE=m
CONFIG_CRYPTO_DEV_HISI_SEC2=m
--
2.37.1

2022-09-19 23:28:17

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v6 1/4 RESEND] ARM: dts: qcom: Use new compatibles for crypto nodes

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 <[email protected]>
Cc: Rob Herring <[email protected]>
Tested-by: Jordan Crouse <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
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";
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>,
--
2.37.1

2022-09-20 07:33:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v6 1/4 RESEND] ARM: dts: qcom: Use new compatibles for crypto nodes

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 <[email protected]>
> Cc: Rob Herring <[email protected]>
> Tested-by: Jordan Crouse <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> 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

2022-09-20 09:49:55

by Bhupesh Sharma

[permalink] [raw]
Subject: Re: [PATCH v6 1/4 RESEND] ARM: dts: qcom: Use new compatibles for crypto nodes



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 <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Tested-by: Jordan Crouse <[email protected]>
>> Signed-off-by: Bhupesh Sharma <[email protected]>
>> ---
>> 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/[email protected]/

[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

2022-09-20 09:53:28

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v6 1/4 RESEND] ARM: dts: qcom: Use new compatibles for crypto nodes

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/[email protected]/
>
> [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

2022-09-20 11:01:04

by Bhupesh Sharma

[permalink] [raw]
Subject: Re: [PATCH v6 1/4 RESEND] ARM: dts: qcom: Use new compatibles for crypto nodes

On Tue, 20 Sept 2022 at 15:09, Krzysztof Kozlowski
<[email protected]> 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/[email protected]/
> >
> > [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

2022-09-20 11:29:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v6 1/4 RESEND] ARM: dts: qcom: Use new compatibles for crypto nodes

On 20/09/2022 12:48, Bhupesh Sharma wrote:
> On Tue, 20 Sept 2022 at 15:09, Krzysztof Kozlowski
> <[email protected]> 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/[email protected]/
>>>
>>> [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

2022-09-20 12:13:19

by Bhupesh Sharma

[permalink] [raw]
Subject: Re: [PATCH v6 1/4 RESEND] ARM: dts: qcom: Use new compatibles for crypto nodes

On Tue, 20 Sept 2022 at 16:46, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 20/09/2022 12:48, Bhupesh Sharma wrote:
> > On Tue, 20 Sept 2022 at 15:09, Krzysztof Kozlowski
> > <[email protected]> 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/[email protected]/
> >>>
> >>> [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/[email protected]/

Thanks,
Bhupesh