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 is also available on sm8150 and sm8250 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).
[1]. https://linux.die.net/man/8/cryptsetup
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Bhupesh Sharma (19):
arm64: dts: qcom: msm8996: Fix qcom,controlled-remotely property
arm64: dts: qcom: msm8996: Fix 'dma' nodes in dts
dt-bindings: qcom-bam: Convert binding to YAML
dt-bindings: qcom-bam: Add 'interconnects' & 'interconnect-names' to
optional properties
dt-bindings: qcom-bam: Add 'iommus' to optional properties
dt-bindings: qcom-bam: Add "powered remotely" mode
dt-bindings: qcom-qce: Convert bindings to yaml
dt-bindings: qcom-qce: Add 'interconnects' and 'interconnect-names'
dt-bindings: qcom-qce: Move 'clocks' to optional properties
dt-bindings: qcom-qce: Add 'iommus' to optional properties
dt-bindings: crypto : Add new compatible strings for qcom-qce
arm64/dts: qcom: Use new compatibles for crypto nodes
crypto: qce: Add new compatibles for qce crypto driver
crypto: qce: Print a failure msg in case probe() fails
crypto: qce: Defer probing if BAM dma channel is not yet initialized
crypto: qce: Add 'sm8250-qce' compatible string check
crypto: qce: Add 'sm8150-qce' compatible string check
arm64/dts: qcom: sm8250: Add dt entries to support crypto engine.
arm64/dts: qcom: sm8150: Add dt entries to support crypto engine.
Thara Gopinath (3):
dma: qcom: bam_dma: Add support to initialize interconnect path
crypto: qce: core: Add support to initialize interconnect path
crypto: qce: core: Make clocks optional
.../devicetree/bindings/crypto/qcom-qce.txt | 25 ----
.../devicetree/bindings/crypto/qcom-qce.yaml | 90 ++++++++++++++
.../devicetree/bindings/dma/qcom_bam_dma.txt | 50 --------
.../devicetree/bindings/dma/qcom_bam_dma.yaml | 115 ++++++++++++++++++
arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sm8150.dtsi | 28 +++++
arch/arm64/boot/dts/qcom/sm8250.dtsi | 28 +++++
drivers/crypto/qce/core.c | 66 +++++++---
drivers/crypto/qce/core.h | 1 +
drivers/dma/qcom/bam_dma.c | 11 ++
12 files changed, 326 insertions(+), 96 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/crypto/qcom-qce.txt
create mode 100644 Documentation/devicetree/bindings/crypto/qcom-qce.yaml
delete mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
--
2.31.1
Property qcom,controlled-remotely should be boolean. Fix it.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index bccc2d0b35a8..27683d7fdfe6 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -713,7 +713,7 @@ cryptobam: dma@644000 {
clock-names = "bam_clk";
#dma-cells = <1>;
qcom,ee = <0>;
- qcom,controlled-remotely = <1>;
+ qcom,controlled-remotely;
};
crypto: crypto@67a000 {
--
2.31.1
Convert Qualcomm BAM DMA devicetree binding to YAML.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
.../devicetree/bindings/dma/qcom_bam_dma.txt | 50 ----------
.../devicetree/bindings/dma/qcom_bam_dma.yaml | 91 +++++++++++++++++++
2 files changed, 91 insertions(+), 50 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
deleted file mode 100644
index cf5b9e44432c..000000000000
--- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
+++ /dev/null
@@ -1,50 +0,0 @@
-QCOM BAM DMA controller
-
-Required properties:
-- compatible: must be one of the following:
- * "qcom,bam-v1.4.0" for MSM8974, APQ8074 and APQ8084
- * "qcom,bam-v1.3.0" for APQ8064, IPQ8064 and MSM8960
- * "qcom,bam-v1.7.0" for MSM8916
-- reg: Address range for DMA registers
-- interrupts: Should contain the one interrupt shared by all channels
-- #dma-cells: must be <1>, the cell in the dmas property of the client device
- represents the channel number
-- clocks: required clock
-- clock-names: must contain "bam_clk" entry
-- qcom,ee : indicates the active Execution Environment identifier (0-7) used in
- the secure world.
-- qcom,controlled-remotely : optional, indicates that the bam is controlled by
- remote proccessor i.e. execution environment.
-- num-channels : optional, indicates supported number of DMA channels in a
- remotely controlled bam.
-- qcom,num-ees : optional, indicates supported number of Execution Environments
- in a remotely controlled bam.
-
-Example:
-
- uart-bam: dma@f9984000 = {
- compatible = "qcom,bam-v1.4.0";
- reg = <0xf9984000 0x15000>;
- interrupts = <0 94 0>;
- clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
- clock-names = "bam_clk";
- #dma-cells = <1>;
- qcom,ee = <0>;
- };
-
-DMA clients must use the format described in the dma.txt file, using a two cell
-specifier for each channel.
-
-Example:
- serial@f991e000 {
- compatible = "qcom,msm-uart";
- reg = <0xf991e000 0x1000>
- <0xf9944000 0x19000>;
- interrupts = <0 108 0>;
- clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>,
- <&gcc GCC_BLSP1_AHB_CLK>;
- clock-names = "core", "iface";
-
- dmas = <&uart-bam 0>, <&uart-bam 1>;
- dma-names = "rx", "tx";
- };
diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
new file mode 100644
index 000000000000..3ca222bd10bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/qcom_bam_dma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: QCOM BAM DMA controller binding
+
+maintainers:
+ - Bhupesh Sharma <[email protected]>
+
+description: |
+ This document defines the binding for the BAM DMA controller
+ found on Qualcomm parts.
+
+allOf:
+ - $ref: "dma-controller.yaml#"
+
+properties:
+ compatible:
+ enum:
+ - qcom,bam-v1.3.0 # for APQ8064, IPQ8064 and MSM8960
+ - qcom,bam-v1.4.0 # for MSM8974, APQ8074 and APQ8084
+ - qcom,bam-v1.7.0 # for MSM8916
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ const: bam_clk
+
+ interrupts:
+ minItems: 1
+ maxItems: 31
+
+ num-channels:
+ maximum: 31
+ description:
+ Indicates supported number of DMA channels in a remotely controlled bam.
+
+ "#dma-cells":
+ const: 1
+ description: The single cell represents the channel index.
+
+ qcom,ee:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 7
+ description:
+ Indicates the active Execution Environment identifier (0-7)
+ used in the secure world.
+
+ qcom,controlled-remotely:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Indicates that the bam is controlled by remote proccessor i.e.
+ execution environment.
+
+ qcom,num-ees:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 31
+ default: 2
+ description:
+ Indicates supported number of Execution Environments in a
+ remotely controlled bam.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - "#dma-cells"
+ - qcom,ee
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,gcc-msm8974.h>
+ dma-controller@f9984000 {
+ compatible = "qcom,bam-v1.4.0";
+ reg = <0xf9984000 0x15000>;
+ interrupts = <0 94 0>;
+ clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
+ clock-names = "bam_clk";
+ #dma-cells = <1>;
+ qcom,ee = <0>;
+ };
--
2.31.1
Add 'optional' property - 'iommus' to the
device-tree binding documentation for qcom-bam DMA IP.
This property describes the phandle(s) to apps_smmu node
with sid mask.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
.../devicetree/bindings/dma/qcom_bam_dma.yaml | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
index 20e734448c1f..cfff3a2286fb 100644
--- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
+++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
@@ -53,6 +53,12 @@ properties:
interconnect-names:
const: memory
+ iommus:
+ minItems: 1
+ maxItems: 8
+ description:
+ phandle(s) to apps_smmu node with sid mask.
+
qcom,ee:
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
@@ -96,4 +102,8 @@ examples:
clock-names = "bam_clk";
#dma-cells = <1>;
qcom,ee = <0>;
+ iommus = <&apps_smmu 0x584 0x0011>,
+ <&apps_smmu 0x586 0x0011>,
+ <&apps_smmu 0x594 0x0011>,
+ <&apps_smmu 0x596 0x0011>;
};
--
2.31.1
Preparatory patch for subsequent patch in this series which
converts the qcom_bam_dma device-tree binding into YAML format.
Correct dma-controller node inside msm8996 dts, which
leads to following errors with 'make dtbs_check':
dma@164400: $nodename:0: 'dma@164400' does not match
'^dma-controller(@.*)?$'
Fix the same.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 27683d7fdfe6..508cd9d06350 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -705,7 +705,7 @@ tsens1: thermal-sensor@4ad000 {
#thermal-sensor-cells = <1>;
};
- cryptobam: dma@644000 {
+ cryptobam: dma-controller@644000 {
compatible = "qcom,bam-v1.7.0";
reg = <0x00644000 0x24000>;
interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>;
--
2.31.1
In some configurations, the BAM DMA controller is set up by a remote
processor and the local processor can simply start making use of it
without setting up the BAM. This is already supported using the
"qcom,controlled-remotely" property.
However, for some reason another possible configuration is that the
remote processor is responsible for powering up the BAM, but we are
still responsible for initializing it (e.g. resetting it etc). Add
a "qcom,powered-remotely" property to describe that configuration.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
[moved Stephan's change to the YAML dt-binding format]
Signed-off-by: Stephan Gerhold <[email protected]>
---
Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
index cfff3a2286fb..bf0a59e8a2bf 100644
--- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
+++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
@@ -73,6 +73,12 @@ properties:
Indicates that the bam is controlled by remote proccessor i.e.
execution environment.
+ qcom,powered-remotely:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Indicates that the bam is powered up by a remote processor
+ but must be initialized by the local processor.
+
qcom,num-ees:
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
--
2.31.1
Add 'interconnects' and 'interconnect-names' as optional properties
to the device-tree binding documentation for qcom crypto IP.
These properties describe the interconnect path between crypto and main
memory and the interconnect type respectively.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
index 3a839c159d92..30deaa0fa93d 100644
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -32,6 +32,14 @@ properties:
- const: bus
- const: core
+ interconnects:
+ maxItems: 1
+ description:
+ Interconnect path between qce crypto and main memory.
+
+ interconnect-names:
+ const: memory
+
dmas:
items:
- description: DMA specifiers for rx dma channel.
--
2.31.1
Add the missing optional property - 'iommus' to the
device-tree binding documentation for qcom-qce crypto IP.
This property describes the phandle(s) to apps_smmu node with sid mask.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
.../devicetree/bindings/crypto/qcom-qce.yaml | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
index f35bdb9ee7a8..efe349e66ae7 100644
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -32,6 +32,12 @@ properties:
- const: bus
- const: core
+ iommus:
+ minItems: 1
+ maxItems: 8
+ description: |
+ phandle to apps_smmu node with sid mask.
+
interconnects:
maxItems: 1
description:
@@ -70,4 +76,9 @@ examples:
clock-names = "iface", "bus", "core";
dmas = <&cryptobam 2>, <&cryptobam 3>;
dma-names = "rx", "tx";
+ iommus = <&apps_smmu 0x584 0x0011>,
+ <&apps_smmu 0x586 0x0011>,
+ <&apps_smmu 0x594 0x0011>,
+ <&apps_smmu 0x596 0x0011>;
+
};
--
2.31.1
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: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 933b56103a46..f477d026c949 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -204,7 +204,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/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 526087586ba4..8e7cbadff25a 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2329,7 +2329,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.31.1
Newer qcom chips support newer versions of the qce crypto IP, so add
soc specific compatible strings for qcom-qce instead of using crypto
IP version specific ones.
Keep the old strings for backward-compatibility, but mark them as
deprecated.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
index efe349e66ae7..77b9f544f32f 100644
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -15,7 +15,13 @@ description: |
properties:
compatible:
- const: qcom,crypto-v5.1
+ enum:
+ - qcom,crypto-v5.1 # Deprecated. Kept only for backward compatibility
+ - qcom,ipq6018-qce
+ - qcom,sdm845-qce
+ - qcom,sm8150-qce
+ - qcom,sm8250-qce
+ - qcom,sm8350-qce
reg:
maxItems: 1
@@ -68,7 +74,7 @@ examples:
- |
#include <dt-bindings/clock/qcom,gcc-apq8084.h>
crypto-engine@fd45a000 {
- compatible = "qcom,crypto-v5.1";
+ compatible = "qcom,ipq6018-qce";
reg = <0xfd45a000 0x6000>;
clocks = <&gcc GCC_CE2_AHB_CLK>,
<&gcc GCC_CE2_AXI_CLK>,
--
2.31.1
QCom QCE block on some SoCs like ipq6018 don't
require clock as the required property, so the properties
'clocks' and 'clock-names' can be moved instead in the dt-bindings
to the 'optional' properties section.
Otherwise, running 'make dtbs_check' leads to the following
errors:
dma-controller@7984000: clock-names:0: 'bam_clk' was expected
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
dma-controller@7984000: clock-names: Additional items are not allowed ('bam_clk' was unexpected)
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
dma-controller@7984000: clock-names: ['iface_clk', 'bam_clk'] is too long
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
dma-controller@7984000: clocks: [[9, 138], [9, 137]] is too long
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 2 --
1 file changed, 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
index 30deaa0fa93d..f35bdb9ee7a8 100644
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -53,8 +53,6 @@ properties:
required:
- compatible
- reg
- - clocks
- - clock-names
- dmas
- dma-names
--
2.31.1
From: Thara Gopinath <[email protected]>
BAM dma engine associated with certain hardware blocks could require
relevant interconnect pieces be initialized prior to the dma engine
initialization. For e.g. crypto bam dma engine on sm8250. Such requirement
is passed on to the bam dma driver from dt via the "interconnects"
property. Add support in bam_dma driver to check whether the interconnect
path is accessible/enabled prior to attempting driver intializations.
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
[Make header file inclusion alphabetical and use 'devm_of_icc_get()']
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/dma/qcom/bam_dma.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index c8a77b428b52..19fb17db467f 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -26,6 +26,7 @@
#include <linux/kernel.h>
#include <linux/io.h>
#include <linux/init.h>
+#include <linux/interconnect.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/interrupt.h>
@@ -392,6 +393,7 @@ struct bam_device {
const struct reg_offset_data *layout;
struct clk *bamclk;
+ struct icc_path *mem_path;
int irq;
/* dma start transaction tasklet */
@@ -1284,6 +1286,15 @@ static int bam_dma_probe(struct platform_device *pdev)
return ret;
}
+ /* Ensure that interconnects are initialized */
+ bdev->mem_path = devm_of_icc_get(bdev->dev, "memory");
+
+ if (IS_ERR(bdev->mem_path)) {
+ ret = PTR_ERR(bdev->mem_path);
+ dev_err(bdev->dev, "failed to acquire icc path %d\n", ret);
+ goto err_disable_clk;
+ }
+
ret = bam_init(bdev);
if (ret)
goto err_disable_clk;
--
2.31.1
Since we decided to use soc specific compatibles for describing
the qce crypto IP nodes in the device-trees, adapt the driver
now to handle the same.
Keep the old deprecated compatible strings still in the driver,
to ensure backward compatibility.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
drivers/crypto/qce/core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index 89d9c01ba009..dd2604f5ce6a 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -297,8 +297,12 @@ static int qce_crypto_remove(struct platform_device *pdev)
}
static const struct of_device_id qce_crypto_of_match[] = {
+ /* Following two entries are deprecated (kept only for backward compatibility) */
{ .compatible = "qcom,crypto-v5.1", },
{ .compatible = "qcom,crypto-v5.4", },
+ /* Add compatible strings as per updated dt-bindings, here: */
+ { .compatible = "qcom,ipq6018-qce", },
+ { .compatible = "qcom,sdm845-qce", },
{}
};
MODULE_DEVICE_TABLE(of, qce_crypto_of_match);
--
2.31.1
Add crypto engine (CE) and CE BAM related nodes and definitions to
"sm8250.dtsi".
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
Signed-off-by: Thara Gopinath <[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 6f6129b39c9c..691c28066cec 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -4104,6 +4104,34 @@ cpufreq_hw: cpufreq@18591000 {
#freq-domain-cells = <1>;
};
+
+ 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";
+ };
};
timer {
--
2.31.1
Print a failure message (dev_err) in case the qcom qce crypto
driver probe() fails.
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Reviewed-by: Thara Gopinath <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
drivers/crypto/qce/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index 98784d63d78c..7c90401a2ef1 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -280,6 +280,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
err_mem_path_disable:
icc_set_bw(qce->mem_path, 0, 0);
err:
+ dev_err(dev, "%s failed : %d\n", __func__, ret);
return ret;
}
--
2.31.1
Add crypto engine (CE) and CE BAM related nodes and definitions to
"sm8150.dtsi".
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[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 81b4ff2cc4cd..2af74a11da69 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -3672,6 +3672,34 @@ wifi: wifi@18800000 {
iommus = <&apps_smmu 0x0640 0x1>;
status = "disabled";
};
+
+ 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";
+ };
};
timer {
--
2.31.1
Add 'sm8150-qce' compatible string check in qce crypto
driver as we add support for sm8150 crypto device in the
device-tree in the subsequent patch.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
drivers/crypto/qce/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index a7d7d7d5d02f..0a11ffacc2de 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -309,6 +309,7 @@ static const struct of_device_id qce_crypto_of_match[] = {
/* Add compatible strings as per updated dt-bindings, here: */
{ .compatible = "qcom,ipq6018-qce", },
{ .compatible = "qcom,sdm845-qce", },
+ { .compatible = "qcom,sm8150-qce", },
{ .compatible = "qcom,sm8250-qce", },
{}
};
--
2.31.1
From: Thara Gopinath <[email protected]>
Crypto engine on certain Snapdragon processors like sm8150, sm8250, sm8350
etc. requires interconnect path between the engine and memory to be
explicitly enabled and bandwidth set prior to any operations. Add support
in the qce core to enable the interconnect path appropriately.
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
[Make header file inclusion alphabetical and use devm_of_icc_get()]
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/crypto/qce/core.c | 34 +++++++++++++++++++++++++++-------
drivers/crypto/qce/core.h | 1 +
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index d3780be44a76..89d9c01ba009 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -5,6 +5,7 @@
#include <linux/clk.h>
#include <linux/dma-mapping.h>
+#include <linux/interconnect.h>
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
@@ -22,6 +23,8 @@
#define QCE_MAJOR_VERSION5 0x05
#define QCE_QUEUE_LENGTH 1
+#define QCE_DEFAULT_MEM_BANDWIDTH 393600
+
static const struct qce_algo_ops *qce_ops[] = {
#ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
&skcipher_ops,
@@ -206,21 +209,35 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
+ qce->mem_path = devm_of_icc_get(qce->dev, "memory");
+ if (IS_ERR(qce->mem_path))
+ return PTR_ERR(qce->mem_path);
+
qce->core = devm_clk_get(qce->dev, "core");
- if (IS_ERR(qce->core))
- return PTR_ERR(qce->core);
+ if (IS_ERR(qce->core)) {
+ ret = PTR_ERR(qce->core);
+ goto err;
+ }
qce->iface = devm_clk_get(qce->dev, "iface");
- if (IS_ERR(qce->iface))
- return PTR_ERR(qce->iface);
+ if (IS_ERR(qce->iface)) {
+ ret = PTR_ERR(qce->iface);
+ goto err;
+ }
qce->bus = devm_clk_get(qce->dev, "bus");
- if (IS_ERR(qce->bus))
- return PTR_ERR(qce->bus);
+ if (IS_ERR(qce->bus)) {
+ ret = PTR_ERR(qce->bus);
+ goto err;
+ }
+
+ ret = icc_set_bw(qce->mem_path, QCE_DEFAULT_MEM_BANDWIDTH, QCE_DEFAULT_MEM_BANDWIDTH);
+ if (ret)
+ goto err;
ret = clk_prepare_enable(qce->core);
if (ret)
- return ret;
+ goto err_mem_path_disable;
ret = clk_prepare_enable(qce->iface);
if (ret)
@@ -260,6 +277,9 @@ static int qce_crypto_probe(struct platform_device *pdev)
clk_disable_unprepare(qce->iface);
err_clks_core:
clk_disable_unprepare(qce->core);
+err_mem_path_disable:
+ icc_set_bw(qce->mem_path, 0, 0);
+err:
return ret;
}
diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
index 085774cdf641..228fcd69ec51 100644
--- a/drivers/crypto/qce/core.h
+++ b/drivers/crypto/qce/core.h
@@ -35,6 +35,7 @@ struct qce_device {
void __iomem *base;
struct device *dev;
struct clk *core, *iface, *bus;
+ struct icc_path *mem_path;
struct qce_dma_data dma;
int burst_size;
unsigned int pipe_pair_id;
--
2.31.1
Since the Qualcomm qce crypto driver needs the BAM dma driver to be
setup first (to allow crypto operations), it makes sense to defer
the qce crypto driver probing in case the BAM dma driver is not yet
probed.
Move the code leg requesting dma channels earlier in the
probe() flow. This fixes the qce probe failure issues when both qce
and BMA dma are compiled as static part of the kernel.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
drivers/crypto/qce/core.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index 7c90401a2ef1..84ed9e253d5d 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -209,9 +209,19 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
+ /* qce driver requires BAM dma driver to be setup first.
+ * In case the dma channel are not set yet, this check
+ * helps use to return -EPROBE_DEFER earlier.
+ */
+ ret = qce_dma_request(qce->dev, &qce->dma);
+ if (ret)
+ return ret;
+
qce->mem_path = devm_of_icc_get(qce->dev, "memory");
- if (IS_ERR(qce->mem_path))
- return PTR_ERR(qce->mem_path);
+ if (IS_ERR(qce->mem_path)) {
+ ret = PTR_ERR(qce->mem_path);
+ goto err;
+ }
qce->core = devm_clk_get_optional(qce->dev, "core");
if (IS_ERR(qce->core)) {
@@ -247,10 +257,6 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (ret)
goto err_clks_iface;
- ret = qce_dma_request(qce->dev, &qce->dma);
- if (ret)
- goto err_clks;
-
ret = qce_check_version(qce);
if (ret)
goto err_clks;
@@ -265,12 +271,10 @@ static int qce_crypto_probe(struct platform_device *pdev)
ret = qce_register_algs(qce);
if (ret)
- goto err_dma;
+ goto err_clks;
return 0;
-err_dma:
- qce_dma_release(&qce->dma);
err_clks:
clk_disable_unprepare(qce->bus);
err_clks_iface:
@@ -280,6 +284,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
err_mem_path_disable:
icc_set_bw(qce->mem_path, 0, 0);
err:
+ qce_dma_release(&qce->dma);
dev_err(dev, "%s failed : %d\n", __func__, ret);
return ret;
}
--
2.31.1
Add 'sm8250-qce' compatible string check in qce crypto
driver as we add support for sm8250 crypto device in the
device-tree in the subsequent patch.
Cc: Bjorn Andersson <[email protected]>
Reviewed-by: Thara Gopinath <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
drivers/crypto/qce/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index 84ed9e253d5d..a7d7d7d5d02f 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -309,6 +309,7 @@ static const struct of_device_id qce_crypto_of_match[] = {
/* Add compatible strings as per updated dt-bindings, here: */
{ .compatible = "qcom,ipq6018-qce", },
{ .compatible = "qcom,sdm845-qce", },
+ { .compatible = "qcom,sm8250-qce", },
{}
};
MODULE_DEVICE_TABLE(of, qce_crypto_of_match);
--
2.31.1
Add new optional properties - 'interconnects' and
'interconnect-names' to the device-tree binding documentation for
qcom-bam DMA IP.
These properties describe the interconnect path between bam and main
memory and the interconnect type respectively.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
index 3ca222bd10bd..20e734448c1f 100644
--- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
+++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
@@ -45,6 +45,14 @@ properties:
const: 1
description: The single cell represents the channel index.
+ interconnects:
+ maxItems: 1
+ description:
+ Interconnect path between bam and main memory.
+
+ interconnect-names:
+ const: memory
+
qcom,ee:
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
--
2.31.1
Convert Qualcomm QCE crypto devicetree binding to YAML.
Cc: Thara Gopinath <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
.../devicetree/bindings/crypto/qcom-qce.txt | 25 -------
.../devicetree/bindings/crypto/qcom-qce.yaml | 67 +++++++++++++++++++
2 files changed, 67 insertions(+), 25 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/crypto/qcom-qce.txt
create mode 100644 Documentation/devicetree/bindings/crypto/qcom-qce.yaml
diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.txt b/Documentation/devicetree/bindings/crypto/qcom-qce.txt
deleted file mode 100644
index fdd53b184ba8..000000000000
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-Qualcomm crypto engine driver
-
-Required properties:
-
-- compatible : should be "qcom,crypto-v5.1"
-- reg : specifies base physical address and size of the registers map
-- clocks : phandle to clock-controller plus clock-specifier pair
-- clock-names : "iface" clocks register interface
- "bus" clocks data transfer interface
- "core" clocks rest of the crypto block
-- dmas : DMA specifiers for tx and rx dma channels. For more see
- Documentation/devicetree/bindings/dma/dma.txt
-- dma-names : DMA request names should be "rx" and "tx"
-
-Example:
- crypto@fd45a000 {
- compatible = "qcom,crypto-v5.1";
- reg = <0xfd45a000 0x6000>;
- clocks = <&gcc GCC_CE2_AHB_CLK>,
- <&gcc GCC_CE2_AXI_CLK>,
- <&gcc GCC_CE2_CLK>;
- clock-names = "iface", "bus", "core";
- dmas = <&cryptobam 2>, <&cryptobam 3>;
- dma-names = "rx", "tx";
- };
diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
new file mode 100644
index 000000000000..3a839c159d92
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/crypto/qcom-qce.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm crypto engine driver
+
+maintainers:
+ - Bhupesh Sharma <[email protected]>
+
+description: |
+ This document defines the binding for the QCE crypto
+ controller found on Qualcomm parts.
+
+properties:
+ compatible:
+ const: qcom,crypto-v5.1
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: iface clocks register interface.
+ - description: bus clocks data transfer interface.
+ - description: core clocks rest of the crypto block.
+
+ clock-names:
+ items:
+ - const: iface
+ - const: bus
+ - const: core
+
+ dmas:
+ items:
+ - description: DMA specifiers for rx dma channel.
+ - description: DMA specifiers for tx dma channel.
+
+ dma-names:
+ items:
+ - const: rx
+ - const: tx
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - dmas
+ - dma-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,gcc-apq8084.h>
+ crypto-engine@fd45a000 {
+ compatible = "qcom,crypto-v5.1";
+ reg = <0xfd45a000 0x6000>;
+ clocks = <&gcc GCC_CE2_AHB_CLK>,
+ <&gcc GCC_CE2_AXI_CLK>,
+ <&gcc GCC_CE2_CLK>;
+ clock-names = "iface", "bus", "core";
+ dmas = <&cryptobam 2>, <&cryptobam 3>;
+ dma-names = "rx", "tx";
+ };
--
2.31.1
From: Thara Gopinath <[email protected]>
On certain Snapdragon processors, the crypto engine clocks are enabled by
default by security firmware and the driver need not/ should not handle the
clocks. Make acquiring of all the clocks optional in crypto engine driver
so that the driver initializes properly even if no clocks are specified in
the dt.
Cc: Bjorn Andersson <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
[Massage the commit log]
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/crypto/qce/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index dd2604f5ce6a..98784d63d78c 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -213,19 +213,19 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (IS_ERR(qce->mem_path))
return PTR_ERR(qce->mem_path);
- qce->core = devm_clk_get(qce->dev, "core");
+ qce->core = devm_clk_get_optional(qce->dev, "core");
if (IS_ERR(qce->core)) {
ret = PTR_ERR(qce->core);
goto err;
}
- qce->iface = devm_clk_get(qce->dev, "iface");
+ qce->iface = devm_clk_get_optional(qce->dev, "iface");
if (IS_ERR(qce->iface)) {
ret = PTR_ERR(qce->iface);
goto err;
}
- qce->bus = devm_clk_get(qce->dev, "bus");
+ qce->bus = devm_clk_get_optional(qce->dev, "bus");
if (IS_ERR(qce->bus)) {
ret = PTR_ERR(qce->bus);
goto err;
--
2.31.1
On Wed, 10 Nov 2021 16:29:03 +0530, Bhupesh Sharma wrote:
> Convert Qualcomm BAM DMA devicetree binding to YAML.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../devicetree/bindings/dma/qcom_bam_dma.txt | 50 ----------
> .../devicetree/bindings/dma/qcom_bam_dma.yaml | 91 +++++++++++++++++++
> 2 files changed, 91 insertions(+), 50 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
>
Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.
Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.
Full log is available here: https://patchwork.ozlabs.org/patch/1553369
dma@12142000: $nodename:0: 'dma@12142000' does not match '^dma-controller(@.*)?$'
arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dt.yaml
dma@12182000: $nodename:0: 'dma@12182000' does not match '^dma-controller(@.*)?$'
arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dt.yaml
arch/arm/boot/dts/qcom-apq8064-cm-qs600.dt.yaml
arch/arm/boot/dts/qcom-apq8064-ifc6410.dt.yaml
arch/arm/boot/dts/qcom-apq8064-sony-xperia-yuga.dt.yaml
arch/arm/boot/dts/qcom-ipq8064-ap148.dt.yaml
arch/arm/boot/dts/qcom-ipq8064-rb3011.dt.yaml
arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dt.yaml
dma@121c2000: $nodename:0: 'dma@121c2000' does not match '^dma-controller(@.*)?$'
arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dt.yaml
arch/arm/boot/dts/qcom-apq8064-cm-qs600.dt.yaml
arch/arm/boot/dts/qcom-apq8064-ifc6410.dt.yaml
arch/arm/boot/dts/qcom-apq8064-sony-xperia-yuga.dt.yaml
dma@12402000: $nodename:0: 'dma@12402000' does not match '^dma-controller(@.*)?$'
arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dt.yaml
arch/arm/boot/dts/qcom-apq8064-cm-qs600.dt.yaml
arch/arm/boot/dts/qcom-apq8064-ifc6410.dt.yaml
arch/arm/boot/dts/qcom-apq8064-sony-xperia-yuga.dt.yaml
arch/arm/boot/dts/qcom-ipq8064-ap148.dt.yaml
arch/arm/boot/dts/qcom-ipq8064-rb3011.dt.yaml
dma@1dc4000: $nodename:0: 'dma@1dc4000' does not match '^dma-controller(@.*)?$'
arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-db845c.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-mtp.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dt.yaml
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dt.yaml
dma@1dc4000: 'iommus' does not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-db845c.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-mtp.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dt.yaml
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dt.yaml
dma@1dc4000: qcom,controlled-remotely: 'oneOf' conditional failed, one must be fixed:
arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-db845c.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-mtp.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dt.yaml
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dt.yaml
dma@704000: $nodename:0: 'dma@704000' does not match '^dma-controller(@.*)?$'
arch/arm64/boot/dts/qcom/ipq8074-hk01.dt.yaml
arch/arm64/boot/dts/qcom/ipq8074-hk10-c1.dt.yaml
arch/arm64/boot/dts/qcom/ipq8074-hk10-c2.dt.yaml
dma@704000: qcom,controlled-remotely: 'oneOf' conditional failed, one must be fixed:
arch/arm64/boot/dts/qcom/ipq8074-hk01.dt.yaml
arch/arm64/boot/dts/qcom/ipq8074-hk10-c1.dt.yaml
arch/arm64/boot/dts/qcom/ipq8074-hk10-c2.dt.yaml
dma@7544000: $nodename:0: 'dma@7544000' does not match '^dma-controller(@.*)?$'
arch/arm64/boot/dts/qcom/apq8096-db820c.dt.yaml
arch/arm64/boot/dts/qcom/apq8096-ifc6640.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-mtp.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-dora.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-kagura.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-keyaki.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-dora.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-kagura.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-keyaki.dt.yaml
dma@7584000: $nodename:0: 'dma@7584000' does not match '^dma-controller(@.*)?$'
arch/arm64/boot/dts/qcom/apq8096-db820c.dt.yaml
arch/arm64/boot/dts/qcom/apq8096-ifc6640.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-mtp.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-dora.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-kagura.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-keyaki.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-dora.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-kagura.dt.yaml
arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-keyaki.dt.yaml
dma@7884000: $nodename:0: 'dma@7884000' does not match '^dma-controller(@.*)?$'
arch/arm/boot/dts/qcom-ipq4018-ap120c-ac-bit.dt.yaml
arch/arm/boot/dts/qcom-ipq4018-ap120c-ac.dt.yaml
arch/arm/boot/dts/qcom-ipq4018-jalapeno.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1-c1.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c1.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c3.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c2.dt.yaml
dma@7984000: $nodename:0: 'dma@7984000' does not match '^dma-controller(@.*)?$'
arch/arm/boot/dts/qcom-ipq4018-ap120c-ac-bit.dt.yaml
arch/arm/boot/dts/qcom-ipq4018-ap120c-ac.dt.yaml
arch/arm/boot/dts/qcom-ipq4018-jalapeno.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1-c1.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c1.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c3.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c2.dt.yaml
dma@8e04000: $nodename:0: 'dma@8e04000' does not match '^dma-controller(@.*)?$'
arch/arm/boot/dts/qcom-ipq4018-ap120c-ac-bit.dt.yaml
arch/arm/boot/dts/qcom-ipq4018-ap120c-ac.dt.yaml
arch/arm/boot/dts/qcom-ipq4018-jalapeno.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1-c1.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c1.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c3.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dt.yaml
arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c2.dt.yaml
dma@c184000: $nodename:0: 'dma@c184000' does not match '^dma-controller(@.*)?$'
arch/arm64/boot/dts/qcom/msm8998-asus-novago-tp370ql.dt.yaml
arch/arm64/boot/dts/qcom/msm8998-hp-envy-x2.dt.yaml
arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dt.yaml
arch/arm64/boot/dts/qcom/msm8998-mtp.dt.yaml
arch/arm64/boot/dts/qcom/msm8998-oneplus-cheeseburger.dt.yaml
arch/arm64/boot/dts/qcom/msm8998-oneplus-dumpling.dt.yaml
dma-controller@17184000: 'iommus' does not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-db845c.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-mtp.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dt.yaml
arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dt.yaml
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dt.yaml
dma-controller@704000: 'qcom,config-pipe-trust-reg' does not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
dma-controller@704000: qcom,controlled-remotely: 'oneOf' conditional failed, one must be fixed:
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
dma-controller@7984000: clock-names:0: 'bam_clk' was expected
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
dma-controller@7984000: clock-names: Additional items are not allowed ('bam_clk' was unexpected)
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
dma-controller@7984000: clock-names: ['iface_clk', 'bam_clk'] is too long
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
dma-controller@7984000: clocks: [[9, 138], [9, 137]] is too long
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
Hi Bhupesh,
On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> Convert Qualcomm BAM DMA devicetree binding to YAML.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../devicetree/bindings/dma/qcom_bam_dma.txt | 50 ----------
> .../devicetree/bindings/dma/qcom_bam_dma.yaml | 91 +++++++++++++++++++
> 2 files changed, 91 insertions(+), 50 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
>
> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> deleted file mode 100644
> index cf5b9e44432c..000000000000
> --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -QCOM BAM DMA controller
> -
> -Required properties:
> -- compatible: must be one of the following:
> - * "qcom,bam-v1.4.0" for MSM8974, APQ8074 and APQ8084
> - * "qcom,bam-v1.3.0" for APQ8064, IPQ8064 and MSM8960
> - * "qcom,bam-v1.7.0" for MSM8916
> -- reg: Address range for DMA registers
> -- interrupts: Should contain the one interrupt shared by all channels
> -- #dma-cells: must be <1>, the cell in the dmas property of the client device
> - represents the channel number
> -- clocks: required clock
> -- clock-names: must contain "bam_clk" entry
> -- qcom,ee : indicates the active Execution Environment identifier (0-7) used in
> - the secure world.
> -- qcom,controlled-remotely : optional, indicates that the bam is controlled by
> - remote proccessor i.e. execution environment.
> -- num-channels : optional, indicates supported number of DMA channels in a
> - remotely controlled bam.
> -- qcom,num-ees : optional, indicates supported number of Execution Environments
> - in a remotely controlled bam.
> -
> -Example:
> -
> - uart-bam: dma@f9984000 = {
> - compatible = "qcom,bam-v1.4.0";
> - reg = <0xf9984000 0x15000>;
> - interrupts = <0 94 0>;
> - clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
> - clock-names = "bam_clk";
> - #dma-cells = <1>;
> - qcom,ee = <0>;
> - };
> -
> -DMA clients must use the format described in the dma.txt file, using a two cell
> -specifier for each channel.
> -
> -Example:
> - serial@f991e000 {
> - compatible = "qcom,msm-uart";
> - reg = <0xf991e000 0x1000>
> - <0xf9944000 0x19000>;
> - interrupts = <0 108 0>;
> - clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>,
> - <&gcc GCC_BLSP1_AHB_CLK>;
> - clock-names = "core", "iface";
> -
> - dmas = <&uart-bam 0>, <&uart-bam 1>;
> - dma-names = "rx", "tx";
> - };
> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> new file mode 100644
> index 000000000000..3ca222bd10bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/qcom_bam_dma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: QCOM BAM DMA controller binding
> +
> +maintainers:
> + - Bhupesh Sharma <[email protected]>
> +
> +description: |
> + This document defines the binding for the BAM DMA controller
> + found on Qualcomm parts.
> +
> +allOf:
> + - $ref: "dma-controller.yaml#"
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,bam-v1.3.0 # for APQ8064, IPQ8064 and MSM8960
> + - qcom,bam-v1.4.0 # for MSM8974, APQ8074 and APQ8084
> + - qcom,bam-v1.7.0 # for MSM8916
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: bam_clk
> +
> + interrupts:
> + minItems: 1
> + maxItems: 31
> +
> + num-channels:
> + maximum: 31
> + description:
> + Indicates supported number of DMA channels in a remotely controlled bam.
> +
> + "#dma-cells":
> + const: 1
> + description: The single cell represents the channel index.
> +
> + qcom,ee:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 7
> + description:
> + Indicates the active Execution Environment identifier (0-7)
> + used in the secure world.
> +
> + qcom,controlled-remotely:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Indicates that the bam is controlled by remote proccessor i.e.
> + execution environment.
> +
> + qcom,num-ees:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 31
> + default: 2
> + description:
> + Indicates supported number of Execution Environments in a
> + remotely controlled bam.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - "#dma-cells"
> + - qcom,ee
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,gcc-msm8974.h>
> + dma-controller@f9984000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xf9984000 0x15000>;
> + interrupts = <0 94 0>;
> + clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
> + clock-names = "bam_clk";
> + #dma-cells = <1>;
> + qcom,ee = <0>;
> + };
>
this change should be rebased on top of the upstream commit 37aef53f5cc ("dt-bindings:
dmaengine: bam_dma: Add "powered remotely" mode"), which adds 'qcom,powered-remotely'
property description.
--
Best wishes,
Vladimir
Hi Bhupesh,
On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> In some configurations, the BAM DMA controller is set up by a remote
> processor and the local processor can simply start making use of it
> without setting up the BAM. This is already supported using the
> "qcom,controlled-remotely" property.
>
> However, for some reason another possible configuration is that the
> remote processor is responsible for powering up the BAM, but we are
> still responsible for initializing it (e.g. resetting it etc). Add
> a "qcom,powered-remotely" property to describe that configuration.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> [moved Stephan's change to the YAML dt-binding format]
> Signed-off-by: Stephan Gerhold <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> index cfff3a2286fb..bf0a59e8a2bf 100644
> --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> @@ -73,6 +73,12 @@ properties:
> Indicates that the bam is controlled by remote proccessor i.e.
> execution environment.
>
> + qcom,powered-remotely:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Indicates that the bam is powered up by a remote processor
> + but must be initialized by the local processor.
> +
> qcom,num-ees:
> $ref: /schemas/types.yaml#/definitions/uint32
> minimum: 0
>
after rebasing the change on top of master this particular patch won't be
needed anymore. See my review comment to v5 03/22.
--
Best wishes,
Vladimir
Hi Bhupesh,
On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> Newer qcom chips support newer versions of the qce crypto IP, so add
> soc specific compatible strings for qcom-qce instead of using crypto
> IP version specific ones.
>
> Keep the old strings for backward-compatibility, but mark them as
> deprecated.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> index efe349e66ae7..77b9f544f32f 100644
> --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> @@ -15,7 +15,13 @@ description: |
>
> properties:
> compatible:
> - const: qcom,crypto-v5.1
> + enum:
> + - qcom,crypto-v5.1 # Deprecated. Kept only for backward compatibility
> + - qcom,ipq6018-qce
> + - qcom,sdm845-qce
> + - qcom,sm8150-qce
> + - qcom,sm8250-qce
> + - qcom,sm8350-qce
let me ask you to add at least two more compatibles to the list: qcom,ipq8074-qce
and qcom,msm8996-qce.
>
> reg:
> maxItems: 1
> @@ -68,7 +74,7 @@ examples:
> - |
> #include <dt-bindings/clock/qcom,gcc-apq8084.h>
> crypto-engine@fd45a000 {
> - compatible = "qcom,crypto-v5.1";
> + compatible = "qcom,ipq6018-qce";
> reg = <0xfd45a000 0x6000>;
> clocks = <&gcc GCC_CE2_AHB_CLK>,
> <&gcc GCC_CE2_AXI_CLK>,
>
--
Best wishes,
Vladimir
Hi Bhupesh,
On 11/10/21 12:59 PM, 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: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 933b56103a46..f477d026c949 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -204,7 +204,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/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 526087586ba4..8e7cbadff25a 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -2329,7 +2329,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>,
>
and in connection to my review comment on v5 11/22 there should be done
similar changes for ipq8074.dtsi and msm8996.dtsi.
--
Best wishes,
Vladimir
Hi Bhupesh,
On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> From: Thara Gopinath <[email protected]>
>
> BAM dma engine associated with certain hardware blocks could require
> relevant interconnect pieces be initialized prior to the dma engine
> initialization. For e.g. crypto bam dma engine on sm8250. Such requirement
> is passed on to the bam dma driver from dt via the "interconnects"
> property. Add support in bam_dma driver to check whether the interconnect
> path is accessible/enabled prior to attempting driver intializations.
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> [Make header file inclusion alphabetical and use 'devm_of_icc_get()']
> Signed-off-by: Thara Gopinath <[email protected]>
please let me ask you to swap your and Thara's sob tags above, there is
a rule applicable to all cases dealing with someone's else changes:
From Documentation/process/submitting-patches.rst:
Any further SoBs (Signed-off-by:'s) following the author's SoB are from
people handling and transporting the patch, but were not involved in its
development. SoB chains should reflect the **real** route a patch took
as it was propagated to the maintainers and ultimately to Linus, with
the first SoB entry signalling primary authorship of a single author.
--
Best wishes,
Vladimir
Hi Bhupesh,
On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> Since we decided to use soc specific compatibles for describing
> the qce crypto IP nodes in the device-trees, adapt the driver
> now to handle the same.
>
> Keep the old deprecated compatible strings still in the driver,
> to ensure backward compatibility.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> drivers/crypto/qce/core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index 89d9c01ba009..dd2604f5ce6a 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -297,8 +297,12 @@ static int qce_crypto_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id qce_crypto_of_match[] = {
> + /* Following two entries are deprecated (kept only for backward compatibility) */
> { .compatible = "qcom,crypto-v5.1", },
> { .compatible = "qcom,crypto-v5.4", },
> + /* Add compatible strings as per updated dt-bindings, here: */
> + { .compatible = "qcom,ipq6018-qce", },
> + { .compatible = "qcom,sdm845-qce", },
> {}
> };
> MODULE_DEVICE_TABLE(of, qce_crypto_of_match);
>
and two more compatibles should be added to the list, see my review
comment on v5 11/22.
--
Best wishes,
Vladimir
Hi Bhupesh,
On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> From: Thara Gopinath <[email protected]>
>
> Crypto engine on certain Snapdragon processors like sm8150, sm8250, sm8350
> etc. requires interconnect path between the engine and memory to be
> explicitly enabled and bandwidth set prior to any operations. Add support
> in the qce core to enable the interconnect path appropriately.
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> [Make header file inclusion alphabetical and use devm_of_icc_get()]
> Signed-off-by: Thara Gopinath <[email protected]>
similar SoB swap is expected above.
> ---
> drivers/crypto/qce/core.c | 34 +++++++++++++++++++++++++++-------
> drivers/crypto/qce/core.h | 1 +
> 2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index d3780be44a76..89d9c01ba009 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -5,6 +5,7 @@
>
> #include <linux/clk.h>
> #include <linux/dma-mapping.h>
> +#include <linux/interconnect.h>
> #include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> @@ -22,6 +23,8 @@
> #define QCE_MAJOR_VERSION5 0x05
> #define QCE_QUEUE_LENGTH 1
>
> +#define QCE_DEFAULT_MEM_BANDWIDTH 393600
> +
> static const struct qce_algo_ops *qce_ops[] = {
> #ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> &skcipher_ops,
> @@ -206,21 +209,35 @@ static int qce_crypto_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> + qce->mem_path = devm_of_icc_get(qce->dev, "memory");
> + if (IS_ERR(qce->mem_path))
> + return PTR_ERR(qce->mem_path);
> +
> qce->core = devm_clk_get(qce->dev, "core");
> - if (IS_ERR(qce->core))
> - return PTR_ERR(qce->core);
> + if (IS_ERR(qce->core)) {
> + ret = PTR_ERR(qce->core);
> + goto err;
> + }
>
> qce->iface = devm_clk_get(qce->dev, "iface");
> - if (IS_ERR(qce->iface))
> - return PTR_ERR(qce->iface);
> + if (IS_ERR(qce->iface)) {
> + ret = PTR_ERR(qce->iface);
> + goto err;
> + }
>
> qce->bus = devm_clk_get(qce->dev, "bus");
> - if (IS_ERR(qce->bus))
> - return PTR_ERR(qce->bus);
> + if (IS_ERR(qce->bus)) {
> + ret = PTR_ERR(qce->bus);
> + goto err;
formally all these changes from 'return' to 'goto err' are not needed,
the necessity of such a transition will be required in a later change.
Please consider to move addition of 'err' goto label directly into
patch v5 18/22 -- since I still think that v17/22 is not needed...
> + }
> +
> + ret = icc_set_bw(qce->mem_path, QCE_DEFAULT_MEM_BANDWIDTH, QCE_DEFAULT_MEM_BANDWIDTH);
> + if (ret)
> + goto err;
>
> ret = clk_prepare_enable(qce->core);
> if (ret)
> - return ret;
> + goto err_mem_path_disable;
>
> ret = clk_prepare_enable(qce->iface);
> if (ret)
> @@ -260,6 +277,9 @@ static int qce_crypto_probe(struct platform_device *pdev)
> clk_disable_unprepare(qce->iface);
> err_clks_core:
> clk_disable_unprepare(qce->core);
> +err_mem_path_disable:
> + icc_set_bw(qce->mem_path, 0, 0);
> +err:
> return ret;
See my comment above.
> }
>
> diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
> index 085774cdf641..228fcd69ec51 100644
> --- a/drivers/crypto/qce/core.h
> +++ b/drivers/crypto/qce/core.h
> @@ -35,6 +35,7 @@ struct qce_device {
> void __iomem *base;
> struct device *dev;
> struct clk *core, *iface, *bus;
> + struct icc_path *mem_path;
> struct qce_dma_data dma;
> int burst_size;
> unsigned int pipe_pair_id;
>
--
Best wishes,
Vladimir
Hi Bhupesh,
On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> Print a failure message (dev_err) in case the qcom qce crypto
> driver probe() fails.
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Reviewed-by: Thara Gopinath <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> drivers/crypto/qce/core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index 98784d63d78c..7c90401a2ef1 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -280,6 +280,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
> err_mem_path_disable:
> icc_set_bw(qce->mem_path, 0, 0);
> err:
> + dev_err(dev, "%s failed : %d\n", __func__, ret);
> return ret;
> }
>
>
in my opinion expressed earlier this change is not needed, but I'll recede,
if somebody thinks that the change is useful in any way.
--
Best wishes,
Vladimir
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> Convert Qualcomm BAM DMA devicetree binding to YAML.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../devicetree/bindings/dma/qcom_bam_dma.txt | 50 ----------
> .../devicetree/bindings/dma/qcom_bam_dma.yaml | 91 +++++++++++++++++++
> 2 files changed, 91 insertions(+), 50 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
>
> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> deleted file mode 100644
> index cf5b9e44432c..000000000000
> --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -QCOM BAM DMA controller
> -
> -Required properties:
> -- compatible: must be one of the following:
> - * "qcom,bam-v1.4.0" for MSM8974, APQ8074 and APQ8084
> - * "qcom,bam-v1.3.0" for APQ8064, IPQ8064 and MSM8960
> - * "qcom,bam-v1.7.0" for MSM8916
> -- reg: Address range for DMA registers
> -- interrupts: Should contain the one interrupt shared by all channels
> -- #dma-cells: must be <1>, the cell in the dmas property of the client device
> - represents the channel number
> -- clocks: required clock
> -- clock-names: must contain "bam_clk" entry
> -- qcom,ee : indicates the active Execution Environment identifier (0-7) used in
> - the secure world.
> -- qcom,controlled-remotely : optional, indicates that the bam is controlled by
> - remote proccessor i.e. execution environment.
> -- num-channels : optional, indicates supported number of DMA channels in a
> - remotely controlled bam.
> -- qcom,num-ees : optional, indicates supported number of Execution Environments
> - in a remotely controlled bam.
> -
> -Example:
> -
> - uart-bam: dma@f9984000 = {
> - compatible = "qcom,bam-v1.4.0";
> - reg = <0xf9984000 0x15000>;
> - interrupts = <0 94 0>;
> - clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
> - clock-names = "bam_clk";
> - #dma-cells = <1>;
> - qcom,ee = <0>;
> - };
> -
> -DMA clients must use the format described in the dma.txt file, using a two cell
> -specifier for each channel.
> -
> -Example:
> - serial@f991e000 {
> - compatible = "qcom,msm-uart";
> - reg = <0xf991e000 0x1000>
> - <0xf9944000 0x19000>;
> - interrupts = <0 108 0>;
> - clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>,
> - <&gcc GCC_BLSP1_AHB_CLK>;
> - clock-names = "core", "iface";
> -
> - dmas = <&uart-bam 0>, <&uart-bam 1>;
> - dma-names = "rx", "tx";
> - };
> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> new file mode 100644
> index 000000000000..3ca222bd10bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/qcom_bam_dma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: QCOM BAM DMA controller binding
> +
> +maintainers:
> + - Bhupesh Sharma <[email protected]>
> +
> +description: |
> + This document defines the binding for the BAM DMA controller
> + found on Qualcomm parts.
> +
> +allOf:
> + - $ref: "dma-controller.yaml#"
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,bam-v1.3.0 # for APQ8064, IPQ8064 and MSM8960
> + - qcom,bam-v1.4.0 # for MSM8974, APQ8074 and APQ8084
> + - qcom,bam-v1.7.0 # for MSM8916
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: bam_clk
> +
> + interrupts:
> + minItems: 1
> + maxItems: 31
The old binding uses the wording "the one interrupt" and at least the
Linux implementation indicates that there's only a single interrupt.
So I think this should just be maxItems: 1
> +
> + num-channels:
> + maximum: 31
> + description:
> + Indicates supported number of DMA channels in a remotely controlled bam.
> +
> + "#dma-cells":
> + const: 1
> + description: The single cell represents the channel index.
> +
> + qcom,ee:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 7
> + description:
> + Indicates the active Execution Environment identifier (0-7)
> + used in the secure world.
> +
> + qcom,controlled-remotely:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Indicates that the bam is controlled by remote proccessor i.e.
> + execution environment.
> +
> + qcom,num-ees:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 31
> + default: 2
> + description:
> + Indicates supported number of Execution Environments in a
> + remotely controlled bam.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - "#dma-cells"
> + - qcom,ee
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,gcc-msm8974.h>
> + dma-controller@f9984000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xf9984000 0x15000>;
> + interrupts = <0 94 0>;
While the txt->yaml conversion should retain the original content, I
think it's okay to fix this line up while you're at it; and make it:
interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
Regards,
Bjorn
> + clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
> + clock-names = "bam_clk";
> + #dma-cells = <1>;
> + qcom,ee = <0>;
> + };
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> Convert Qualcomm QCE crypto devicetree binding to YAML.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../devicetree/bindings/crypto/qcom-qce.txt | 25 -------
> .../devicetree/bindings/crypto/qcom-qce.yaml | 67 +++++++++++++++++++
> 2 files changed, 67 insertions(+), 25 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/crypto/qcom-qce.txt
> create mode 100644 Documentation/devicetree/bindings/crypto/qcom-qce.yaml
>
> diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.txt b/Documentation/devicetree/bindings/crypto/qcom-qce.txt
> deleted file mode 100644
> index fdd53b184ba8..000000000000
> --- a/Documentation/devicetree/bindings/crypto/qcom-qce.txt
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -Qualcomm crypto engine driver
> -
> -Required properties:
> -
> -- compatible : should be "qcom,crypto-v5.1"
> -- reg : specifies base physical address and size of the registers map
> -- clocks : phandle to clock-controller plus clock-specifier pair
> -- clock-names : "iface" clocks register interface
> - "bus" clocks data transfer interface
> - "core" clocks rest of the crypto block
> -- dmas : DMA specifiers for tx and rx dma channels. For more see
> - Documentation/devicetree/bindings/dma/dma.txt
> -- dma-names : DMA request names should be "rx" and "tx"
> -
> -Example:
> - crypto@fd45a000 {
> - compatible = "qcom,crypto-v5.1";
> - reg = <0xfd45a000 0x6000>;
> - clocks = <&gcc GCC_CE2_AHB_CLK>,
> - <&gcc GCC_CE2_AXI_CLK>,
> - <&gcc GCC_CE2_CLK>;
> - clock-names = "iface", "bus", "core";
> - dmas = <&cryptobam 2>, <&cryptobam 3>;
> - dma-names = "rx", "tx";
> - };
> diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> new file mode 100644
> index 000000000000..3a839c159d92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/qcom-qce.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm crypto engine driver
> +
> +maintainers:
> + - Bhupesh Sharma <[email protected]>
> +
> +description: |
Nit. The '|' retains the formatting of the description, there's no need
for that.
Reviewed-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
> + This document defines the binding for the QCE crypto
> + controller found on Qualcomm parts.
> +
> +properties:
> + compatible:
> + const: qcom,crypto-v5.1
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: iface clocks register interface.
> + - description: bus clocks data transfer interface.
> + - description: core clocks rest of the crypto block.
> +
> + clock-names:
> + items:
> + - const: iface
> + - const: bus
> + - const: core
> +
> + dmas:
> + items:
> + - description: DMA specifiers for rx dma channel.
> + - description: DMA specifiers for tx dma channel.
> +
> + dma-names:
> + items:
> + - const: rx
> + - const: tx
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - dmas
> + - dma-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,gcc-apq8084.h>
> + crypto-engine@fd45a000 {
> + compatible = "qcom,crypto-v5.1";
> + reg = <0xfd45a000 0x6000>;
> + clocks = <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc GCC_CE2_CLK>;
> + clock-names = "iface", "bus", "core";
> + dmas = <&cryptobam 2>, <&cryptobam 3>;
> + dma-names = "rx", "tx";
> + };
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> QCom QCE block on some SoCs like ipq6018 don't
> require clock as the required property, so the properties
> 'clocks' and 'clock-names' can be moved instead in the dt-bindings
> to the 'optional' properties section.
>
> Otherwise, running 'make dtbs_check' leads to the following
> errors:
>
> dma-controller@7984000: clock-names:0: 'bam_clk' was expected
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> dma-controller@7984000: clock-names: Additional items are not allowed ('bam_clk' was unexpected)
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> dma-controller@7984000: clock-names: ['iface_clk', 'bam_clk'] is too long
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> dma-controller@7984000: clocks: [[9, 138], [9, 137]] is too long
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> index 30deaa0fa93d..f35bdb9ee7a8 100644
> --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> @@ -53,8 +53,6 @@ properties:
> required:
> - compatible
> - reg
> - - clocks
> - - clock-names
I would prefer that we make this conditional on the compatible. That
said, if this only applies to ipq6018 I think we should double check the
fact that there's no clock there...
For the sake of making progress on the series, I think you should omit
this patch from the next version.
Regards,
Bjorn
> - dmas
> - dma-names
>
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> Add the missing optional property - 'iommus' to the
> device-tree binding documentation for qcom-qce crypto IP.
>
> This property describes the phandle(s) to apps_smmu node with sid mask.
"This property describes the iommu streams for crypto pipes" or
something along those lines - depending on what those streams actually
represent.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../devicetree/bindings/crypto/qcom-qce.yaml | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> index f35bdb9ee7a8..efe349e66ae7 100644
> --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> @@ -32,6 +32,12 @@ properties:
> - const: bus
> - const: core
>
> + iommus:
> + minItems: 1
> + maxItems: 8
> + description: |
No need for the '|' here...
> + phandle to apps_smmu node with sid mask.
> +
> interconnects:
> maxItems: 1
> description:
> @@ -70,4 +76,9 @@ examples:
> clock-names = "iface", "bus", "core";
> dmas = <&cryptobam 2>, <&cryptobam 3>;
> dma-names = "rx", "tx";
> + iommus = <&apps_smmu 0x584 0x0011>,
> + <&apps_smmu 0x586 0x0011>,
> + <&apps_smmu 0x594 0x0011>,
> + <&apps_smmu 0x596 0x0011>;
> +
Extra empty line here.
Regards,
Bjorn
> };
> --
> 2.31.1
>
Hi Vladimir,
On Fri, 12 Nov 2021 at 15:56, Vladimir Zapolskiy
<[email protected]> wrote:
>
> Hi Bhupesh,
>
> On 11/10/21 12:59 PM, 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: Thara Gopinath <[email protected]>
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
> > arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> > index 933b56103a46..f477d026c949 100644
> > --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> > @@ -204,7 +204,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/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 526087586ba4..8e7cbadff25a 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -2329,7 +2329,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>,
> >
>
> and in connection to my review comment on v5 11/22 there should be done
> similar changes for ipq8074.dtsi and msm8996.dtsi.
Ok, I will fix this in v6.
Thanks,
Bhupesh
Hi Vladimir,
On Fri, 12 Nov 2021 at 16:06, Vladimir Zapolskiy
<[email protected]> wrote:
>
> Hi Bhupesh,
>
> On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> > Since we decided to use soc specific compatibles for describing
> > the qce crypto IP nodes in the device-trees, adapt the driver
> > now to handle the same.
> >
> > Keep the old deprecated compatible strings still in the driver,
> > to ensure backward compatibility.
> >
> > Cc: Thara Gopinath <[email protected]>
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > drivers/crypto/qce/core.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> > index 89d9c01ba009..dd2604f5ce6a 100644
> > --- a/drivers/crypto/qce/core.c
> > +++ b/drivers/crypto/qce/core.c
> > @@ -297,8 +297,12 @@ static int qce_crypto_remove(struct platform_device *pdev)
> > }
> >
> > static const struct of_device_id qce_crypto_of_match[] = {
> > + /* Following two entries are deprecated (kept only for backward compatibility) */
> > { .compatible = "qcom,crypto-v5.1", },
> > { .compatible = "qcom,crypto-v5.4", },
> > + /* Add compatible strings as per updated dt-bindings, here: */
> > + { .compatible = "qcom,ipq6018-qce", },
> > + { .compatible = "qcom,sdm845-qce", },
> > {}
> > };
> > MODULE_DEVICE_TABLE(of, qce_crypto_of_match);
> >
>
> and two more compatibles should be added to the list, see my review
> comment on v5 11/22.
Ok, I will fix this in v6.
Regards,
Bhupesh
Hi Vladimir,
On Fri, 12 Nov 2021 at 16:12, Vladimir Zapolskiy
<[email protected]> wrote:
>
> Hi Bhupesh,
>
> On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> > Print a failure message (dev_err) in case the qcom qce crypto
> > driver probe() fails.
> >
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Reviewed-by: Thara Gopinath <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > drivers/crypto/qce/core.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> > index 98784d63d78c..7c90401a2ef1 100644
> > --- a/drivers/crypto/qce/core.c
> > +++ b/drivers/crypto/qce/core.c
> > @@ -280,6 +280,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
> > err_mem_path_disable:
> > icc_set_bw(qce->mem_path, 0, 0);
> > err:
> > + dev_err(dev, "%s failed : %d\n", __func__, ret);
> > return ret;
> > }
> >
> >
>
> in my opinion expressed earlier this change is not needed, but I'll recede,
> if somebody thinks that the change is useful in any way.
As I mentioned in the reply to the review comments to the previous
series, the need for this failure message was actually felt to address
failures seen with boot-on crypto tests via
'CRYPTO_MANAGER_EXTRA_TESTS'.
So, I would suggest keeping this patch in, unless there are some major
concerns with the change.
Regards,
Bhupesh
Hi Vladimir,
On Fri, 12 Nov 2021 at 16:02, Vladimir Zapolskiy
<[email protected]> wrote:
>
> Hi Bhupesh,
>
> On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> > From: Thara Gopinath <[email protected]>
> >
> > BAM dma engine associated with certain hardware blocks could require
> > relevant interconnect pieces be initialized prior to the dma engine
> > initialization. For e.g. crypto bam dma engine on sm8250. Such requirement
> > is passed on to the bam dma driver from dt via the "interconnects"
> > property. Add support in bam_dma driver to check whether the interconnect
> > path is accessible/enabled prior to attempting driver intializations.
> >
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > [Make header file inclusion alphabetical and use 'devm_of_icc_get()']
> > Signed-off-by: Thara Gopinath <[email protected]>
>
> please let me ask you to swap your and Thara's sob tags above, there is
> a rule applicable to all cases dealing with someone's else changes:
>
> From Documentation/process/submitting-patches.rst:
>
> Any further SoBs (Signed-off-by:'s) following the author's SoB are from
> people handling and transporting the patch, but were not involved in its
> development. SoB chains should reflect the **real** route a patch took
> as it was propagated to the maintainers and ultimately to Linus, with
> the first SoB entry signalling primary authorship of a single author.
Sure, I will fix it in v6.
Regards,
Bhupesh
Hi Vladimir,
On Fri, 12 Nov 2021 at 15:55, Vladimir Zapolskiy
<[email protected]> wrote:
>
> Hi Bhupesh,
>
> On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> > Newer qcom chips support newer versions of the qce crypto IP, so add
> > soc specific compatible strings for qcom-qce instead of using crypto
> > IP version specific ones.
> >
> > Keep the old strings for backward-compatibility, but mark them as
> > deprecated.
> >
> > Cc: Thara Gopinath <[email protected]>
> > Cc: Bjorn Andersson <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > index efe349e66ae7..77b9f544f32f 100644
> > --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > @@ -15,7 +15,13 @@ description: |
> >
> > properties:
> > compatible:
> > - const: qcom,crypto-v5.1
> > + enum:
> > + - qcom,crypto-v5.1 # Deprecated. Kept only for backward compatibility
> > + - qcom,ipq6018-qce
> > + - qcom,sdm845-qce
> > + - qcom,sm8150-qce
> > + - qcom,sm8250-qce
> > + - qcom,sm8350-qce
>
> let me ask you to add at least two more compatibles to the list: qcom,ipq8074-qce
> and qcom,msm8996-qce.
Sure, I will fix it in v6.
Regards,
Bhupesh
> >
> > reg:
> > maxItems: 1
> > @@ -68,7 +74,7 @@ examples:
> > - |
> > #include <dt-bindings/clock/qcom,gcc-apq8084.h>
> > crypto-engine@fd45a000 {
> > - compatible = "qcom,crypto-v5.1";
> > + compatible = "qcom,ipq6018-qce";
> > reg = <0xfd45a000 0x6000>;
> > clocks = <&gcc GCC_CE2_AHB_CLK>,
> > <&gcc GCC_CE2_AXI_CLK>,
> >
>
> --
> Best wishes,
> Vladimir
Hi Bjorn,
On Sun, 14 Nov 2021 at 01:32, Bjorn Andersson
<[email protected]> wrote:
>
> On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
>
> > QCom QCE block on some SoCs like ipq6018 don't
> > require clock as the required property, so the properties
> > 'clocks' and 'clock-names' can be moved instead in the dt-bindings
> > to the 'optional' properties section.
> >
> > Otherwise, running 'make dtbs_check' leads to the following
> > errors:
> >
> > dma-controller@7984000: clock-names:0: 'bam_clk' was expected
> > arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> >
> > dma-controller@7984000: clock-names: Additional items are not allowed ('bam_clk' was unexpected)
> > arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> >
> > dma-controller@7984000: clock-names: ['iface_clk', 'bam_clk'] is too long
> > arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> >
> > dma-controller@7984000: clocks: [[9, 138], [9, 137]] is too long
> > arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> >
> > Cc: Thara Gopinath <[email protected]>
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > index 30deaa0fa93d..f35bdb9ee7a8 100644
> > --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > @@ -53,8 +53,6 @@ properties:
> > required:
> > - compatible
> > - reg
> > - - clocks
> > - - clock-names
>
> I would prefer that we make this conditional on the compatible. That
> said, if this only applies to ipq6018 I think we should double check the
> fact that there's no clock there...
>
> For the sake of making progress on the series, I think you should omit
> this patch from the next version.
Without this patch, 'make dtbs_check' fails with the following error:
dma-controller@7984000: clock-names:0: 'bam_clk' was expected
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
dma-controller@7984000: clock-names: Additional items are not allowed
('bam_clk' was unexpected)
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
which I think is making Rob bot-check fail.
So, I think instead of dropping the patch, let's try and understand
from the 'ipq6018 qce' documentation if the clocks are really
'optional' there for the qce block (as clock properties are not
mentioned in the dts from the very first upstream version). If not, we
can try and fix the 'ipq6018 qce' dts node itself.
Regards,
Bhupesh
Hi Vladimir,
On Fri, 12 Nov 2021 at 14:11, Vladimir Zapolskiy
<[email protected]> wrote:
>
> Hi Bhupesh,
>
> On 11/10/21 12:59 PM, Bhupesh Sharma wrote:
> > Convert Qualcomm BAM DMA devicetree binding to YAML.
> >
> > Cc: Thara Gopinath <[email protected]>
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > .../devicetree/bindings/dma/qcom_bam_dma.txt | 50 ----------
> > .../devicetree/bindings/dma/qcom_bam_dma.yaml | 91 +++++++++++++++++++
> > 2 files changed, 91 insertions(+), 50 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> > create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> > deleted file mode 100644
> > index cf5b9e44432c..000000000000
> > --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> > +++ /dev/null
> > @@ -1,50 +0,0 @@
> > -QCOM BAM DMA controller
> > -
> > -Required properties:
> > -- compatible: must be one of the following:
> > - * "qcom,bam-v1.4.0" for MSM8974, APQ8074 and APQ8084
> > - * "qcom,bam-v1.3.0" for APQ8064, IPQ8064 and MSM8960
> > - * "qcom,bam-v1.7.0" for MSM8916
> > -- reg: Address range for DMA registers
> > -- interrupts: Should contain the one interrupt shared by all channels
> > -- #dma-cells: must be <1>, the cell in the dmas property of the client device
> > - represents the channel number
> > -- clocks: required clock
> > -- clock-names: must contain "bam_clk" entry
> > -- qcom,ee : indicates the active Execution Environment identifier (0-7) used in
> > - the secure world.
> > -- qcom,controlled-remotely : optional, indicates that the bam is controlled by
> > - remote proccessor i.e. execution environment.
> > -- num-channels : optional, indicates supported number of DMA channels in a
> > - remotely controlled bam.
> > -- qcom,num-ees : optional, indicates supported number of Execution Environments
> > - in a remotely controlled bam.
> > -
> > -Example:
> > -
> > - uart-bam: dma@f9984000 = {
> > - compatible = "qcom,bam-v1.4.0";
> > - reg = <0xf9984000 0x15000>;
> > - interrupts = <0 94 0>;
> > - clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
> > - clock-names = "bam_clk";
> > - #dma-cells = <1>;
> > - qcom,ee = <0>;
> > - };
> > -
> > -DMA clients must use the format described in the dma.txt file, using a two cell
> > -specifier for each channel.
> > -
> > -Example:
> > - serial@f991e000 {
> > - compatible = "qcom,msm-uart";
> > - reg = <0xf991e000 0x1000>
> > - <0xf9944000 0x19000>;
> > - interrupts = <0 108 0>;
> > - clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>,
> > - <&gcc GCC_BLSP1_AHB_CLK>;
> > - clock-names = "core", "iface";
> > -
> > - dmas = <&uart-bam 0>, <&uart-bam 1>;
> > - dma-names = "rx", "tx";
> > - };
> > diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> > new file mode 100644
> > index 000000000000..3ca222bd10bd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> > @@ -0,0 +1,91 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/dma/qcom_bam_dma.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: QCOM BAM DMA controller binding
> > +
> > +maintainers:
> > + - Bhupesh Sharma <[email protected]>
> > +
> > +description: |
> > + This document defines the binding for the BAM DMA controller
> > + found on Qualcomm parts.
> > +
> > +allOf:
> > + - $ref: "dma-controller.yaml#"
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - qcom,bam-v1.3.0 # for APQ8064, IPQ8064 and MSM8960
> > + - qcom,bam-v1.4.0 # for MSM8974, APQ8074 and APQ8084
> > + - qcom,bam-v1.7.0 # for MSM8916
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + const: bam_clk
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 31
> > +
> > + num-channels:
> > + maximum: 31
> > + description:
> > + Indicates supported number of DMA channels in a remotely controlled bam.
> > +
> > + "#dma-cells":
> > + const: 1
> > + description: The single cell represents the channel index.
> > +
> > + qcom,ee:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 7
> > + description:
> > + Indicates the active Execution Environment identifier (0-7)
> > + used in the secure world.
> > +
> > + qcom,controlled-remotely:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + Indicates that the bam is controlled by remote proccessor i.e.
> > + execution environment.
> > +
> > + qcom,num-ees:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 31
> > + default: 2
> > + description:
> > + Indicates supported number of Execution Environments in a
> > + remotely controlled bam.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - "#dma-cells"
> > + - qcom,ee
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/qcom,gcc-msm8974.h>
> > + dma-controller@f9984000 {
> > + compatible = "qcom,bam-v1.4.0";
> > + reg = <0xf9984000 0x15000>;
> > + interrupts = <0 94 0>;
> > + clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
> > + clock-names = "bam_clk";
> > + #dma-cells = <1>;
> > + qcom,ee = <0>;
> > + };
> >
>
> this change should be rebased on top of the upstream commit 37aef53f5cc ("dt-bindings:
> dmaengine: bam_dma: Add "powered remotely" mode"), which adds 'qcom,powered-remotely'
> property description.
Indeed. Seems there was some confusion, as we had earlier agreed that
Stephan's change would be part of my v5 series (see [1]), but I see
that the change has anyway being picked up for Linus's tree (in the
.txt version of the bindings).
No problem, I will fix the same in v6.
[1]. https://www.spinics.net/lists/linux-arm-msm/msg97143.html
Regards,
Bhupesh
Hi Rob,
On Thu, 11 Nov 2021 at 01:14, Rob Herring <[email protected]> wrote:
>
> On Wed, 10 Nov 2021 16:29:03 +0530, Bhupesh Sharma wrote:
> > Convert Qualcomm BAM DMA devicetree binding to YAML.
> >
> > Cc: Thara Gopinath <[email protected]>
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > .../devicetree/bindings/dma/qcom_bam_dma.txt | 50 ----------
> > .../devicetree/bindings/dma/qcom_bam_dma.yaml | 91 +++++++++++++++++++
> > 2 files changed, 91 insertions(+), 50 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> > create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> >
>
> Running 'make dtbs_check' with the schema in this patch gives the
> following warnings. Consider if they are expected or the schema is
> incorrect. These may not be new warnings.
>
> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> This will change in the future.
>
> Full log is available here: https://patchwork.ozlabs.org/patch/1553369
>
>
> dma@12142000: $nodename:0: 'dma@12142000' does not match '^dma-controller(@.*)?$'
> arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dt.yaml
>
> dma@12182000: $nodename:0: 'dma@12182000' does not match '^dma-controller(@.*)?$'
> arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dt.yaml
> arch/arm/boot/dts/qcom-apq8064-cm-qs600.dt.yaml
> arch/arm/boot/dts/qcom-apq8064-ifc6410.dt.yaml
> arch/arm/boot/dts/qcom-apq8064-sony-xperia-yuga.dt.yaml
> arch/arm/boot/dts/qcom-ipq8064-ap148.dt.yaml
> arch/arm/boot/dts/qcom-ipq8064-rb3011.dt.yaml
> arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dt.yaml
>
> dma@121c2000: $nodename:0: 'dma@121c2000' does not match '^dma-controller(@.*)?$'
> arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dt.yaml
> arch/arm/boot/dts/qcom-apq8064-cm-qs600.dt.yaml
> arch/arm/boot/dts/qcom-apq8064-ifc6410.dt.yaml
> arch/arm/boot/dts/qcom-apq8064-sony-xperia-yuga.dt.yaml
>
> dma@12402000: $nodename:0: 'dma@12402000' does not match '^dma-controller(@.*)?$'
> arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dt.yaml
> arch/arm/boot/dts/qcom-apq8064-cm-qs600.dt.yaml
> arch/arm/boot/dts/qcom-apq8064-ifc6410.dt.yaml
> arch/arm/boot/dts/qcom-apq8064-sony-xperia-yuga.dt.yaml
> arch/arm/boot/dts/qcom-ipq8064-ap148.dt.yaml
> arch/arm/boot/dts/qcom-ipq8064-rb3011.dt.yaml
>
> dma@1dc4000: $nodename:0: 'dma@1dc4000' does not match '^dma-controller(@.*)?$'
> arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-db845c.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-mtp.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dt.yaml
> arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dt.yaml
>
> dma@1dc4000: 'iommus' does not match any of the regexes: 'pinctrl-[0-9]+'
> arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-db845c.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-mtp.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dt.yaml
> arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dt.yaml
>
> dma@1dc4000: qcom,controlled-remotely: 'oneOf' conditional failed, one must be fixed:
> arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-db845c.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-mtp.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dt.yaml
> arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dt.yaml
>
> dma@704000: $nodename:0: 'dma@704000' does not match '^dma-controller(@.*)?$'
> arch/arm64/boot/dts/qcom/ipq8074-hk01.dt.yaml
> arch/arm64/boot/dts/qcom/ipq8074-hk10-c1.dt.yaml
> arch/arm64/boot/dts/qcom/ipq8074-hk10-c2.dt.yaml
>
> dma@704000: qcom,controlled-remotely: 'oneOf' conditional failed, one must be fixed:
> arch/arm64/boot/dts/qcom/ipq8074-hk01.dt.yaml
> arch/arm64/boot/dts/qcom/ipq8074-hk10-c1.dt.yaml
> arch/arm64/boot/dts/qcom/ipq8074-hk10-c2.dt.yaml
>
> dma@7544000: $nodename:0: 'dma@7544000' does not match '^dma-controller(@.*)?$'
> arch/arm64/boot/dts/qcom/apq8096-db820c.dt.yaml
> arch/arm64/boot/dts/qcom/apq8096-ifc6640.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-mtp.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-dora.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-kagura.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-keyaki.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-dora.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-kagura.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-keyaki.dt.yaml
>
> dma@7584000: $nodename:0: 'dma@7584000' does not match '^dma-controller(@.*)?$'
> arch/arm64/boot/dts/qcom/apq8096-db820c.dt.yaml
> arch/arm64/boot/dts/qcom/apq8096-ifc6640.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-mtp.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-dora.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-kagura.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-keyaki.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-dora.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-kagura.dt.yaml
> arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-keyaki.dt.yaml
>
> dma@7884000: $nodename:0: 'dma@7884000' does not match '^dma-controller(@.*)?$'
> arch/arm/boot/dts/qcom-ipq4018-ap120c-ac-bit.dt.yaml
> arch/arm/boot/dts/qcom-ipq4018-ap120c-ac.dt.yaml
> arch/arm/boot/dts/qcom-ipq4018-jalapeno.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1-c1.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c1.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c3.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c2.dt.yaml
>
> dma@7984000: $nodename:0: 'dma@7984000' does not match '^dma-controller(@.*)?$'
> arch/arm/boot/dts/qcom-ipq4018-ap120c-ac-bit.dt.yaml
> arch/arm/boot/dts/qcom-ipq4018-ap120c-ac.dt.yaml
> arch/arm/boot/dts/qcom-ipq4018-jalapeno.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1-c1.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c1.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c3.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c2.dt.yaml
>
> dma@8e04000: $nodename:0: 'dma@8e04000' does not match '^dma-controller(@.*)?$'
> arch/arm/boot/dts/qcom-ipq4018-ap120c-ac-bit.dt.yaml
> arch/arm/boot/dts/qcom-ipq4018-ap120c-ac.dt.yaml
> arch/arm/boot/dts/qcom-ipq4018-jalapeno.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1-c1.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c1.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1-c3.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dt.yaml
> arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c2.dt.yaml
>
> dma@c184000: $nodename:0: 'dma@c184000' does not match '^dma-controller(@.*)?$'
> arch/arm64/boot/dts/qcom/msm8998-asus-novago-tp370ql.dt.yaml
> arch/arm64/boot/dts/qcom/msm8998-hp-envy-x2.dt.yaml
> arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dt.yaml
> arch/arm64/boot/dts/qcom/msm8998-mtp.dt.yaml
> arch/arm64/boot/dts/qcom/msm8998-oneplus-cheeseburger.dt.yaml
> arch/arm64/boot/dts/qcom/msm8998-oneplus-dumpling.dt.yaml
>
> dma-controller@17184000: 'iommus' does not match any of the regexes: 'pinctrl-[0-9]+'
> arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-db845c.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-mtp.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dt.yaml
> arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dt.yaml
> arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dt.yaml
>
> dma-controller@704000: 'qcom,config-pipe-trust-reg' does not match any of the regexes: 'pinctrl-[0-9]+'
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> dma-controller@704000: qcom,controlled-remotely: 'oneOf' conditional failed, one must be fixed:
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> dma-controller@7984000: clock-names:0: 'bam_clk' was expected
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> dma-controller@7984000: clock-names: Additional items are not allowed ('bam_clk' was unexpected)
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> dma-controller@7984000: clock-names: ['iface_clk', 'bam_clk'] is too long
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> dma-controller@7984000: clocks: [[9, 138], [9, 137]] is too long
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
As noted with v4 review as well, all the errors reported above are
fixed via other patches in this patchset, for e.g.:
[PATCH v5 01/22] arm64: dts: qcom: msm8996: Fix
qcom,controlled-remotely property
[PATCH v5 02/22] arm64: dts: qcom: msm8996: Fix 'dma' nodes in dts
[PATCH v5 04/22] dt-bindings: qcom-bam: Add 'interconnects' &
'interconnect-names' to optional properties
[PATCH v5 05/22] dt-bindings: qcom-bam: Add 'iommus' to optional properties
Regards,
Bhupesh
Hi Bjorn,
On Sun, 14 Nov 2021 at 00:43, Bjorn Andersson
<[email protected]> wrote:
>
> On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
>
> > Convert Qualcomm BAM DMA devicetree binding to YAML.
> >
> > Cc: Thara Gopinath <[email protected]>
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > .../devicetree/bindings/dma/qcom_bam_dma.txt | 50 ----------
> > .../devicetree/bindings/dma/qcom_bam_dma.yaml | 91 +++++++++++++++++++
> > 2 files changed, 91 insertions(+), 50 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> > create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> > deleted file mode 100644
> > index cf5b9e44432c..000000000000
> > --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> > +++ /dev/null
> > @@ -1,50 +0,0 @@
> > -QCOM BAM DMA controller
> > -
> > -Required properties:
> > -- compatible: must be one of the following:
> > - * "qcom,bam-v1.4.0" for MSM8974, APQ8074 and APQ8084
> > - * "qcom,bam-v1.3.0" for APQ8064, IPQ8064 and MSM8960
> > - * "qcom,bam-v1.7.0" for MSM8916
> > -- reg: Address range for DMA registers
> > -- interrupts: Should contain the one interrupt shared by all channels
> > -- #dma-cells: must be <1>, the cell in the dmas property of the client device
> > - represents the channel number
> > -- clocks: required clock
> > -- clock-names: must contain "bam_clk" entry
> > -- qcom,ee : indicates the active Execution Environment identifier (0-7) used in
> > - the secure world.
> > -- qcom,controlled-remotely : optional, indicates that the bam is controlled by
> > - remote proccessor i.e. execution environment.
> > -- num-channels : optional, indicates supported number of DMA channels in a
> > - remotely controlled bam.
> > -- qcom,num-ees : optional, indicates supported number of Execution Environments
> > - in a remotely controlled bam.
> > -
> > -Example:
> > -
> > - uart-bam: dma@f9984000 = {
> > - compatible = "qcom,bam-v1.4.0";
> > - reg = <0xf9984000 0x15000>;
> > - interrupts = <0 94 0>;
> > - clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
> > - clock-names = "bam_clk";
> > - #dma-cells = <1>;
> > - qcom,ee = <0>;
> > - };
> > -
> > -DMA clients must use the format described in the dma.txt file, using a two cell
> > -specifier for each channel.
> > -
> > -Example:
> > - serial@f991e000 {
> > - compatible = "qcom,msm-uart";
> > - reg = <0xf991e000 0x1000>
> > - <0xf9944000 0x19000>;
> > - interrupts = <0 108 0>;
> > - clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>,
> > - <&gcc GCC_BLSP1_AHB_CLK>;
> > - clock-names = "core", "iface";
> > -
> > - dmas = <&uart-bam 0>, <&uart-bam 1>;
> > - dma-names = "rx", "tx";
> > - };
> > diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> > new file mode 100644
> > index 000000000000..3ca222bd10bd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml
> > @@ -0,0 +1,91 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/dma/qcom_bam_dma.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: QCOM BAM DMA controller binding
> > +
> > +maintainers:
> > + - Bhupesh Sharma <[email protected]>
> > +
> > +description: |
> > + This document defines the binding for the BAM DMA controller
> > + found on Qualcomm parts.
> > +
> > +allOf:
> > + - $ref: "dma-controller.yaml#"
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - qcom,bam-v1.3.0 # for APQ8064, IPQ8064 and MSM8960
> > + - qcom,bam-v1.4.0 # for MSM8974, APQ8074 and APQ8084
> > + - qcom,bam-v1.7.0 # for MSM8916
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + const: bam_clk
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 31
>
> The old binding uses the wording "the one interrupt" and at least the
> Linux implementation indicates that there's only a single interrupt.
>
> So I think this should just be maxItems: 1
>
> > +
> > + num-channels:
> > + maximum: 31
> > + description:
> > + Indicates supported number of DMA channels in a remotely controlled bam.
> > +
> > + "#dma-cells":
> > + const: 1
> > + description: The single cell represents the channel index.
> > +
> > + qcom,ee:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 7
> > + description:
> > + Indicates the active Execution Environment identifier (0-7)
> > + used in the secure world.
> > +
> > + qcom,controlled-remotely:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + Indicates that the bam is controlled by remote proccessor i.e.
> > + execution environment.
> > +
> > + qcom,num-ees:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 31
> > + default: 2
> > + description:
> > + Indicates supported number of Execution Environments in a
> > + remotely controlled bam.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - "#dma-cells"
> > + - qcom,ee
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/qcom,gcc-msm8974.h>
> > + dma-controller@f9984000 {
> > + compatible = "qcom,bam-v1.4.0";
> > + reg = <0xf9984000 0x15000>;
> > + interrupts = <0 94 0>;
>
> While the txt->yaml conversion should retain the original content, I
> think it's okay to fix this line up while you're at it; and make it:
>
> interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
Sure, will do this in v6.
Regards,
Bhupesh
> > + clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
> > + clock-names = "bam_clk";
> > + #dma-cells = <1>;
> > + qcom,ee = <0>;
> > + };
> > --
> > 2.31.1
> >
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> From: Thara Gopinath <[email protected]>
>
> On certain Snapdragon processors, the crypto engine clocks are enabled by
> default by security firmware and the driver need not/ should not handle the
> clocks. Make acquiring of all the clocks optional in crypto engine driver
> so that the driver initializes properly even if no clocks are specified in
> the dt.
>
So on some Snapdragons we don't have clocks, on others failing to
specify the clock leads to hard-to-debug issues. Can we make the clk_get
calls conditional based on the compatible instead?
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> [Massage the commit log]
It's a good habit to prefix these messages with your name, i.e.
[Bhupesh: Massaged the commit log]
Regards,
Bjorn
> Signed-off-by: Thara Gopinath <[email protected]>
> ---
> drivers/crypto/qce/core.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index dd2604f5ce6a..98784d63d78c 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -213,19 +213,19 @@ static int qce_crypto_probe(struct platform_device *pdev)
> if (IS_ERR(qce->mem_path))
> return PTR_ERR(qce->mem_path);
>
> - qce->core = devm_clk_get(qce->dev, "core");
> + qce->core = devm_clk_get_optional(qce->dev, "core");
> if (IS_ERR(qce->core)) {
> ret = PTR_ERR(qce->core);
> goto err;
> }
>
> - qce->iface = devm_clk_get(qce->dev, "iface");
> + qce->iface = devm_clk_get_optional(qce->dev, "iface");
> if (IS_ERR(qce->iface)) {
> ret = PTR_ERR(qce->iface);
> goto err;
> }
>
> - qce->bus = devm_clk_get(qce->dev, "bus");
> + qce->bus = devm_clk_get_optional(qce->dev, "bus");
> if (IS_ERR(qce->bus)) {
> ret = PTR_ERR(qce->bus);
> goto err;
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> From: Thara Gopinath <[email protected]>
>
> Crypto engine on certain Snapdragon processors like sm8150, sm8250, sm8350
> etc. requires interconnect path between the engine and memory to be
> explicitly enabled and bandwidth set prior to any operations. Add support
> in the qce core to enable the interconnect path appropriately.
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> [Make header file inclusion alphabetical and use devm_of_icc_get()]
> Signed-off-by: Thara Gopinath <[email protected]>
> ---
> drivers/crypto/qce/core.c | 34 +++++++++++++++++++++++++++-------
> drivers/crypto/qce/core.h | 1 +
> 2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index d3780be44a76..89d9c01ba009 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -5,6 +5,7 @@
>
> #include <linux/clk.h>
> #include <linux/dma-mapping.h>
> +#include <linux/interconnect.h>
> #include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> @@ -22,6 +23,8 @@
> #define QCE_MAJOR_VERSION5 0x05
> #define QCE_QUEUE_LENGTH 1
>
> +#define QCE_DEFAULT_MEM_BANDWIDTH 393600
> +
> static const struct qce_algo_ops *qce_ops[] = {
> #ifdef CONFIG_CRYPTO_DEV_QCE_SKCIPHER
> &skcipher_ops,
> @@ -206,21 +209,35 @@ static int qce_crypto_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> + qce->mem_path = devm_of_icc_get(qce->dev, "memory");
> + if (IS_ERR(qce->mem_path))
> + return PTR_ERR(qce->mem_path);
> +
> qce->core = devm_clk_get(qce->dev, "core");
> - if (IS_ERR(qce->core))
> - return PTR_ERR(qce->core);
> + if (IS_ERR(qce->core)) {
> + ret = PTR_ERR(qce->core);
> + goto err;
I don't see a reason here, or in the following patches to move from
return to goto here.
Regards,
Bjorn
> + }
>
> qce->iface = devm_clk_get(qce->dev, "iface");
> - if (IS_ERR(qce->iface))
> - return PTR_ERR(qce->iface);
> + if (IS_ERR(qce->iface)) {
> + ret = PTR_ERR(qce->iface);
> + goto err;
> + }
>
> qce->bus = devm_clk_get(qce->dev, "bus");
> - if (IS_ERR(qce->bus))
> - return PTR_ERR(qce->bus);
> + if (IS_ERR(qce->bus)) {
> + ret = PTR_ERR(qce->bus);
> + goto err;
> + }
> +
> + ret = icc_set_bw(qce->mem_path, QCE_DEFAULT_MEM_BANDWIDTH, QCE_DEFAULT_MEM_BANDWIDTH);
> + if (ret)
> + goto err;
>
> ret = clk_prepare_enable(qce->core);
> if (ret)
> - return ret;
> + goto err_mem_path_disable;
>
> ret = clk_prepare_enable(qce->iface);
> if (ret)
> @@ -260,6 +277,9 @@ static int qce_crypto_probe(struct platform_device *pdev)
> clk_disable_unprepare(qce->iface);
> err_clks_core:
> clk_disable_unprepare(qce->core);
> +err_mem_path_disable:
> + icc_set_bw(qce->mem_path, 0, 0);
> +err:
> return ret;
> }
>
> diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
> index 085774cdf641..228fcd69ec51 100644
> --- a/drivers/crypto/qce/core.h
> +++ b/drivers/crypto/qce/core.h
> @@ -35,6 +35,7 @@ struct qce_device {
> void __iomem *base;
> struct device *dev;
> struct clk *core, *iface, *bus;
> + struct icc_path *mem_path;
> struct qce_dma_data dma;
> int burst_size;
> unsigned int pipe_pair_id;
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> Add 'sm8250-qce' compatible string check in qce crypto
> driver as we add support for sm8250 crypto device in the
> device-tree in the subsequent patch.
>
> Cc: Bjorn Andersson <[email protected]>
> Reviewed-by: Thara Gopinath <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
Both patch 19 and 20 can be squashed with the previous patch adding
sdm845 & ipq6018.
Regards,
Bjorn
> ---
> drivers/crypto/qce/core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index 84ed9e253d5d..a7d7d7d5d02f 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -309,6 +309,7 @@ static const struct of_device_id qce_crypto_of_match[] = {
> /* Add compatible strings as per updated dt-bindings, here: */
> { .compatible = "qcom,ipq6018-qce", },
> { .compatible = "qcom,sdm845-qce", },
> + { .compatible = "qcom,sm8250-qce", },
> {}
> };
> MODULE_DEVICE_TABLE(of, qce_crypto_of_match);
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> Since the Qualcomm qce crypto driver needs the BAM dma driver to be
> setup first (to allow crypto operations), it makes sense to defer
> the qce crypto driver probing in case the BAM dma driver is not yet
> probed.
>
To me this sentence implies that qce_crypto_probe() doesn't return
-EPROBE_DEFER from qce_dma_request(), but looking at the code I don't
see why this would be...
> Move the code leg requesting dma channels earlier in the
> probe() flow. This fixes the qce probe failure issues when both qce
> and BMA dma are compiled as static part of the kernel.
>
As far as I can tell the only actual difference is that you're moving
the qce_dma_request() above the icc_set_bw() call and the three
clk_prepare_enable() calls.
This is a very valid optimization, but where does qce_crypto_probe()
fail and why does it need the BAM driver to have probed before we try to
turn on the clocks and (our) interconnect vote?
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> drivers/crypto/qce/core.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index 7c90401a2ef1..84ed9e253d5d 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -209,9 +209,19 @@ static int qce_crypto_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> + /* qce driver requires BAM dma driver to be setup first.
> + * In case the dma channel are not set yet, this check
> + * helps use to return -EPROBE_DEFER earlier.
> + */
This comment warrants the change, but I don't see that it will add value
in the code once the patch is merged. Please drop it.
> + ret = qce_dma_request(qce->dev, &qce->dma);
> + if (ret)
I presume this is where your added dev_err() in err: comes in handy. I
definitely think this warrants an error print; so return dev_err_probe()
here would be the right thing to do (in the previous patch).
> + return ret;
> +
> qce->mem_path = devm_of_icc_get(qce->dev, "memory");
> - if (IS_ERR(qce->mem_path))
> - return PTR_ERR(qce->mem_path);
But I see no reason for moving it above devm_of_icc_get() and the
devm_clk_get*() calls - as they don't actually do anything, but moving
qce_dma_request() above them forces the introduction of goto here.
Regards,
Bjorn
> + if (IS_ERR(qce->mem_path)) {
> + ret = PTR_ERR(qce->mem_path);
> + goto err;
> + }
>
> qce->core = devm_clk_get_optional(qce->dev, "core");
> if (IS_ERR(qce->core)) {
> @@ -247,10 +257,6 @@ static int qce_crypto_probe(struct platform_device *pdev)
> if (ret)
> goto err_clks_iface;
>
> - ret = qce_dma_request(qce->dev, &qce->dma);
> - if (ret)
> - goto err_clks;
> -
> ret = qce_check_version(qce);
> if (ret)
> goto err_clks;
> @@ -265,12 +271,10 @@ static int qce_crypto_probe(struct platform_device *pdev)
>
> ret = qce_register_algs(qce);
> if (ret)
> - goto err_dma;
> + goto err_clks;
>
> return 0;
>
> -err_dma:
> - qce_dma_release(&qce->dma);
> err_clks:
> clk_disable_unprepare(qce->bus);
> err_clks_iface:
> @@ -280,6 +284,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
> err_mem_path_disable:
> icc_set_bw(qce->mem_path, 0, 0);
> err:
> + qce_dma_release(&qce->dma);
> dev_err(dev, "%s failed : %d\n", __func__, ret);
> return ret;
> }
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> Add crypto engine (CE) and CE BAM related nodes and definitions to
> "sm8250.dtsi".
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> Signed-off-by: Thara Gopinath <[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 6f6129b39c9c..691c28066cec 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4104,6 +4104,34 @@ cpufreq_hw: cpufreq@18591000 {
>
> #freq-domain-cells = <1>;
> };
> +
> + cryptobam: dma-controller@1dc4000 {
> + compatible = "qcom,bam-v1.7.0";
> + reg = <0 0x01dc4000 0 0x24000>;
Please keep nodes under /soc sorted by address.
Thanks,
Bjorn
> + 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";
> + };
> };
>
> timer {
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
Forgot to mention, please double check that the $subject prefix matches
other patches to the file.
Regards,
Bjorn
> Add crypto engine (CE) and CE BAM related nodes and definitions to
> "sm8250.dtsi".
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> Signed-off-by: Thara Gopinath <[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 6f6129b39c9c..691c28066cec 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4104,6 +4104,34 @@ cpufreq_hw: cpufreq@18591000 {
>
> #freq-domain-cells = <1>;
> };
> +
> + 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";
> + };
> };
>
> timer {
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> Print a failure message (dev_err) in case the qcom qce crypto
> driver probe() fails.
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Reviewed-by: Thara Gopinath <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> drivers/crypto/qce/core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index 98784d63d78c..7c90401a2ef1 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -280,6 +280,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
> err_mem_path_disable:
> icc_set_bw(qce->mem_path, 0, 0);
> err:
> + dev_err(dev, "%s failed : %d\n", __func__, ret);
There's two possible outcomes of this style of error logging:
1) You came through a code path with a specific error message, so you
will have something that will say:
qce: Some useful error text
qce: qce_crypto_probe failed: -22
2) You came through a code path without a specific error message:
qce: qce_crypto_probe failed: -22
In the first case the second line is just pure spam, in the second case
the bare -22 is typically completely useless - given that there tend to
be just a few commonly used errno values coming from multiple possible
error sources.
As such, no thanks. If you have an error case in qce_crypto_probe() that
doesn't have a good, useful, error message, please fix that.
Regards,
Bjorn
> return ret;
> }
>
> --
> 2.31.1
>
On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> From: Thara Gopinath <[email protected]>
>
> BAM dma engine associated with certain hardware blocks could require
> relevant interconnect pieces be initialized prior to the dma engine
> initialization. For e.g. crypto bam dma engine on sm8250. Such requirement
> is passed on to the bam dma driver from dt via the "interconnects"
> property. Add support in bam_dma driver to check whether the interconnect
> path is accessible/enabled prior to attempting driver intializations.
>
This patch acquires the path, presumably between BAM and DDR, but I
don't see that it actually do anything with it.
So if this makes sm8250 work I presume it's because you'll get probe
deferred until the interconnect provider is in place and it will hold
buses at max speed until sync_state - and then in runtime perhaps the
crypto driver hides the fact that BAM doesn't vote for its bandwidth?
I'm sceptical about hiding behind such circumstances, but at least the
commit message should be clear on what's going on - or you need cast
some bandwidth votes when the block is expected to transfer data.
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> [Make header file inclusion alphabetical and use 'devm_of_icc_get()']
> Signed-off-by: Thara Gopinath <[email protected]>
As Vladimir pointed out, the S-o-b of a patch should be read in
chronological order.
Please read the section about Developer's Certificate of Origin (link
below), it should make it clear why the specific order is important.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1
> ---
> drivers/dma/qcom/bam_dma.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index c8a77b428b52..19fb17db467f 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -26,6 +26,7 @@
> #include <linux/kernel.h>
> #include <linux/io.h>
> #include <linux/init.h>
> +#include <linux/interconnect.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> #include <linux/interrupt.h>
> @@ -392,6 +393,7 @@ struct bam_device {
> const struct reg_offset_data *layout;
>
> struct clk *bamclk;
> + struct icc_path *mem_path;
> int irq;
>
> /* dma start transaction tasklet */
> @@ -1284,6 +1286,15 @@ static int bam_dma_probe(struct platform_device *pdev)
> return ret;
> }
>
> + /* Ensure that interconnects are initialized */
> + bdev->mem_path = devm_of_icc_get(bdev->dev, "memory");
> +
Please drop this empty line.
> + if (IS_ERR(bdev->mem_path)) {
> + ret = PTR_ERR(bdev->mem_path);
> + dev_err(bdev->dev, "failed to acquire icc path %d\n", ret);
We typically want to avoid printing error messages when ret is
-EPROBE_DEFER. The best way around this is to utilize the
dev_err_probe() helper function.
Replace the two lines above with:
ret = dev_err_probe(bdev->dev, PTR_ERR(bdev->mem_path),
"failed to acquire icc path\n")
> + goto err_disable_clk;
If you move the devm_of_icc_get() right before clk_prepare_enable()
(still after devm_clk_get*()) you can simply return dev_err_probe() -
and will avoid toggling the CE clock unnecessarily on EPROBE_DEFER.
Regards,
Bjorn
> + }
> +
> ret = bam_init(bdev);
> if (ret)
> goto err_disable_clk;
> --
> 2.31.1
>
On 10-11-21, 16:29, Bhupesh Sharma wrote:
> Preparatory patch for subsequent patch in this series which
> converts the qcom_bam_dma device-tree binding into YAML format.
>
> Correct dma-controller node inside msm8996 dts, which
> leads to following errors with 'make dtbs_check':
>
> dma@164400: $nodename:0: 'dma@164400' does not match
> '^dma-controller(@.*)?$'
>
> Fix the same.
Looks like one more crept in, this is the only one.. I did fix a bunch
previously...
Reviewed-by: Vinod Koul <[email protected]>
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 27683d7fdfe6..508cd9d06350 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -705,7 +705,7 @@ tsens1: thermal-sensor@4ad000 {
> #thermal-sensor-cells = <1>;
> };
>
> - cryptobam: dma@644000 {
> + cryptobam: dma-controller@644000 {
> compatible = "qcom,bam-v1.7.0";
> reg = <0x00644000 0x24000>;
> interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>;
> --
> 2.31.1
--
~Vinod
On Wed, 10 Nov 2021 16:29:04 +0530, Bhupesh Sharma wrote:
> Add new optional properties - 'interconnects' and
> 'interconnect-names' to the device-tree binding documentation for
> qcom-bam DMA IP.
>
> These properties describe the interconnect path between bam and main
> memory and the interconnect type respectively.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/qcom_bam_dma.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Wed, 10 Nov 2021 16:29:05 +0530, Bhupesh Sharma wrote:
> Add 'optional' property - 'iommus' to the
> device-tree binding documentation for qcom-bam DMA IP.
>
> This property describes the phandle(s) to apps_smmu node
> with sid mask.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../devicetree/bindings/dma/qcom_bam_dma.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Wed, 10 Nov 2021 16:29:08 +0530, Bhupesh Sharma wrote:
> Add 'interconnects' and 'interconnect-names' as optional properties
> to the device-tree binding documentation for qcom crypto IP.
>
> These properties describe the interconnect path between crypto and main
> memory and the interconnect type respectively.
>
> Cc: Thara Gopinath <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Mon, Nov 15, 2021 at 11:04:31AM +0530, Bhupesh Sharma wrote:
> Hi Bjorn,
>
> On Sun, 14 Nov 2021 at 01:32, Bjorn Andersson
> <[email protected]> wrote:
> >
> > On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> >
> > > QCom QCE block on some SoCs like ipq6018 don't
> > > require clock as the required property, so the properties
> > > 'clocks' and 'clock-names' can be moved instead in the dt-bindings
> > > to the 'optional' properties section.
> > >
> > > Otherwise, running 'make dtbs_check' leads to the following
> > > errors:
> > >
> > > dma-controller@7984000: clock-names:0: 'bam_clk' was expected
> > > arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > dma-controller@7984000: clock-names: Additional items are not allowed ('bam_clk' was unexpected)
> > > arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > dma-controller@7984000: clock-names: ['iface_clk', 'bam_clk'] is too long
> > > arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > dma-controller@7984000: clocks: [[9, 138], [9, 137]] is too long
> > > arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > Cc: Thara Gopinath <[email protected]>
> > > Cc: Bjorn Andersson <[email protected]>
> > > Cc: Rob Herring <[email protected]>
> > > Signed-off-by: Bhupesh Sharma <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 2 --
> > > 1 file changed, 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > > index 30deaa0fa93d..f35bdb9ee7a8 100644
> > > --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > > +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > > @@ -53,8 +53,6 @@ properties:
> > > required:
> > > - compatible
> > > - reg
> > > - - clocks
> > > - - clock-names
> >
> > I would prefer that we make this conditional on the compatible. That
> > said, if this only applies to ipq6018 I think we should double check the
> > fact that there's no clock there...
> >
> > For the sake of making progress on the series, I think you should omit
> > this patch from the next version.
>
> Without this patch, 'make dtbs_check' fails with the following error:
> dma-controller@7984000: clock-names:0: 'bam_clk' was expected
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
>
> dma-controller@7984000: clock-names: Additional items are not allowed
> ('bam_clk' was unexpected)
> arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
Those errors do not correspond to the change here. Adding something to
'required' would never solve any error (other than a driver requires a
property to function).
> which I think is making Rob bot-check fail.
dtbs_check don't have to be fixed as the message says.
> So, I think instead of dropping the patch, let's try and understand
> from the 'ipq6018 qce' documentation if the clocks are really
> 'optional' there for the qce block (as clock properties are not
> mentioned in the dts from the very first upstream version). If not, we
> can try and fix the 'ipq6018 qce' dts node itself.
>
> Regards,
> Bhupesh
>
Some nitpicks:
- `description:` -> `description: >`
- you dropped part of example, wouldn't be better to keep it there?
- remove `binding` from the title
Feel free to put:
Reviewed-by: David Heidelberg <[email protected]>
Thank you
David Heidelberg
On 10/11/2021 13:59, Bhupesh Sharma wrote:
> From: Thara Gopinath <[email protected]>
>
> BAM dma engine associated with certain hardware blocks could require
> relevant interconnect pieces be initialized prior to the dma engine
> initialization. For e.g. crypto bam dma engine on sm8250. Such requirement
> is passed on to the bam dma driver from dt via the "interconnects"
> property. Add support in bam_dma driver to check whether the interconnect
> path is accessible/enabled prior to attempting driver intializations.
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> [Make header file inclusion alphabetical and use 'devm_of_icc_get()']
> Signed-off-by: Thara Gopinath <[email protected]>
> ---
> drivers/dma/qcom/bam_dma.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index c8a77b428b52..19fb17db467f 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -26,6 +26,7 @@
> #include <linux/kernel.h>
> #include <linux/io.h>
> #include <linux/init.h>
> +#include <linux/interconnect.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> #include <linux/interrupt.h>
> @@ -392,6 +393,7 @@ struct bam_device {
> const struct reg_offset_data *layout;
>
> struct clk *bamclk;
> + struct icc_path *mem_path;
> int irq;
>
> /* dma start transaction tasklet */
> @@ -1284,6 +1286,15 @@ static int bam_dma_probe(struct platform_device *pdev)
> return ret;
> }
>
> + /* Ensure that interconnects are initialized */
> + bdev->mem_path = devm_of_icc_get(bdev->dev, "memory");
Also, as a note, the "memory" is not a good name for the ICC path.
Usually they take the form of "src-dst". However in this case you can
probably use NULL for the first and the only icc path.
> +
Extra newline, not necessary.
> + if (IS_ERR(bdev->mem_path)) {
> + ret = PTR_ERR(bdev->mem_path);
> + dev_err(bdev->dev, "failed to acquire icc path %d\n", ret);
> + goto err_disable_clk;
> + }
> +
> ret = bam_init(bdev);
> if (ret)
> goto err_disable_clk;
--
With best wishes
Dmitry