Skip to contentSkip to conversation

Port mxs-dcp to imx6ull and imx6sll

PATCH Superseded Leonard Crestez 2 participants 8 messages Oct 16, 2018

Subsystems (4)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v3 cover Port mxs-dcp to imx6ull and imx6sll

The DCP block is present on 6sll and 6ull but not enabled. The hardware is mostly compatible with 6sl, the only important difference is that explicit clock enabling is required.

There were several issues with the functionality of this driver (it didn't even probe properly) but they are fixed in cryptodev/master by this series:

    https://lore.kernel.org/patchwork/cover/994874/

---
Changes since v2:
* Rename 6ull dcp node to crypto@*
* Use clk_prepare_enable on probe
* Fix not unpreparing on remove
* Realign dcp_clk struct member with rest of attributes (using tabs)
* Link to v2: https://lore.kernel.org/patchwork/cover/999795/

Changes since v1:
* Add devicetree maintainers for dt-bindings
* Add a patch enabling in imx_v6_v7_defconfig. Since tcrypt now passes this
shouldn't cause any issues
* Link to v1: https://lore.kernel.org/patchwork/cover/994893/

Leonard Crestez (4):
dt-bindings: crypto: Mention clocks for mxs-dcp
crypto: mxs-dcp - Add support for dcp clk
ARM: dts: imx6ull: Add dcp node
ARM: imx_v6_v7_defconfig: Enable CRYPTO_DEV_MXS_DCP

 .../devicetree/bindings/crypto/fsl-dcp.txt        |  2 ++
 arch/arm/boot/dts/imx6ull.dtsi                    | 10 ++++++++++
 arch/arm/configs/imx_v6_v7_defconfig              |  1 +
 drivers/crypto/mxs-dcp.c                          | 15 +++++++++++++++
 4 files changed, 28 insertions(+)
Signature · 1 line
2.17.1
#1 lore raw

PATCH v3 1/4 dt-bindings: crypto: Mention clocks for mxs-dcp

Explicit clock enabling is required on 6sll and 6ull so mention that standard clock bindings are used.

---
 Documentation/devicetree/bindings/crypto/fsl-dcp.txt | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/crypto/fsl-dcp.txt +2 -0
index 76a0b4e80e83..4e4d387e38a5 100644
@@ -4,10 +4,12 @@ Required properties:
 - compatible : Should be "fsl,<soc>-dcp"
 - reg : Should contain MXS DCP registers location and length
 - interrupts : Should contain MXS DCP interrupt numbers, VMI IRQ and DCP IRQ
                must be supplied, optionally Secure IRQ can be present, but
 	       is currently not implemented and not used.
+- clocks : Clock reference (only required on some SOCs: 6ull and 6sll).
+- clock-names : Must be "dcp".
 
 Example:
 
 dcp at 80028000 {
 	compatible = "fsl,imx28-dcp", "fsl,imx23-dcp";
Signature · 1 line
2.17.1
#2 lore raw

PATCH v3 2/4 crypto: mxs-dcp - Add support for dcp clk

In reply to Leonard Crestez

On 6ull and 6sll the DCP block has a clock which needs to be explicitly enabled.

Add minimal handling for this at probe/remove time.

---
 drivers/crypto/mxs-dcp.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
drivers/crypto/mxs-dcp.c +15 -0
index 4e6ff32f8a7e..a9855f622e05 100644
@@ -18,10 +18,11 @@
 #include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/stmp_device.h>
+#include <linux/clk.h>
 
 #include <crypto/aes.h>
 #include <crypto/sha.h>
 #include <crypto/internal/hash.h>
 #include <crypto/internal/skcipher.h>
@@ -80,10 +81,11 @@ struct dcp {
 
 	struct completion		completion[DCP_MAX_CHANS];
 	spinlock_t			lock[DCP_MAX_CHANS];
 	struct task_struct		*thread[DCP_MAX_CHANS];
 	struct crypto_queue		queue[DCP_MAX_CHANS];
+	struct clk			*dcp_clk;
 };
 
 enum dcp_chan {
 	DCP_CHAN_HASH_SHA	= 0,
 	DCP_CHAN_CRYPTO		= 2,
@@ -1027,10 +1029,21 @@ static int mxs_dcp_probe(struct platform_device *pdev)
 	sdcp->dev = dev;
 	sdcp->base = devm_ioremap_resource(dev, iores);
 	if (IS_ERR(sdcp->base))
 		return PTR_ERR(sdcp->base);
 
+	/* DCP clock is optional, only used on some SOCs */
+	sdcp->dcp_clk = devm_clk_get(dev, "dcp");
+	if (IS_ERR(sdcp->dcp_clk)) {
+		if (sdcp->dcp_clk != ERR_PTR(-ENOENT))
+			return PTR_ERR(sdcp->dcp_clk);
+		sdcp->dcp_clk = NULL;
+	}
+
+	ret = clk_prepare_enable(sdcp->dcp_clk);
+	if (ret)
+		return ret;
 
 	ret = devm_request_irq(dev, dcp_vmi_irq, mxs_dcp_irq, 0,
 			       "dcp-vmi-irq", sdcp);
 	if (ret) {
 		dev_err(dev, "Failed to claim DCP VMI IRQ!\n");
@@ -1168,10 +1181,12 @@ static int mxs_dcp_remove(struct platform_device *pdev)
 		crypto_unregister_algs(dcp_aes_algs, ARRAY_SIZE(dcp_aes_algs));
 
 	kthread_stop(sdcp->thread[DCP_CHAN_HASH_SHA]);
 	kthread_stop(sdcp->thread[DCP_CHAN_CRYPTO]);
 
+	clk_disable_unprepare(sdcp->dcp_clk);
+
 	platform_set_drvdata(pdev, NULL);
 
 	global_sdcp = NULL;
 
 	return 0;
Signature · 1 line
2.17.1
#5 lore raw

PATCH v3 3/4 ARM: dts: imx6ull: Add dcp node

In reply to Leonard Crestez

The DCP block on 6ull has no major differences other than requiring explicit clock enabling.

---
 arch/arm/boot/dts/imx6ull.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)
arch/arm/boot/dts/imx6ull.dtsi +10 -0
index 796ed35d4ac9..f3668fe69eac 100644
@@ -37,10 +37,20 @@
 			#address-cells = <1>;
 			#size-cells = <1>;
 			reg = <0x02200000 0x100000>;
 			ranges;
 
+			dcp: crypto at 2280000 {
+				compatible = "fsl,imx6ull-dcp", "fsl,imx28-dcp";
+				reg = <0x02280000 0x4000>;
+				interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6ULL_CLK_DCP_CLK>;
+				clock-names = "dcp";
+			};
+
 			iomuxc_snvs: iomuxc-snvs at 2290000 {
 				compatible = "fsl,imx6ull-iomuxc-snvs";
 				reg = <0x02290000 0x4000>;
 			};
 
Signature · 1 line
2.17.1
#7 lore raw

PATCH v3 4/4 ARM: imx_v6_v7_defconfig: Enable CRYPTO_DEV_MXS_DCP

In reply to Leonard Crestez

This block is present in 6sl, 6sll and 6ull so it should be enabled in the default imx kernel config.

---
 arch/arm/configs/imx_v6_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm/configs/imx_v6_v7_defconfig +1 -0
index 1ad5736c8fa6..57928dff9bce 100644
@@ -428,10 +428,11 @@ CONFIG_NLS_ISO8859_1=y
 CONFIG_NLS_ISO8859_15=m
 CONFIG_NLS_UTF8=y
 CONFIG_SECURITYFS=y
 CONFIG_CRYPTO_DEV_FSL_CAAM=y
 CONFIG_CRYPTO_DEV_SAHARA=y
+CONFIG_CRYPTO_DEV_MXS_DCP=y
 CONFIG_CRC_CCITT=m
 CONFIG_CRC_T10DIF=y
 CONFIG_CRC7=m
 CONFIG_LIBCRC32C=m
 CONFIG_FONTS=y
Signature · 1 line
2.17.1
reviewer
#3 lore raw

Re: [PATCH v3 2/4] crypto: mxs-dcp - Add support for dcp clk

In reply to Leonard Crestez

Hi Leonard,

drivers/crypto/mxs-dcp.c static int mxs_dcp_probe(struct platform_device *pdev) · line 1039

Quoted 6 lines · Leonard Crestez
drivers/crypto/mxs-dcp.c +6 -0
+       /* DCP clock is optional, only used on some SOCs */
+       sdcp->dcp_clk = devm_clk_get(dev, "dcp");
+       if (IS_ERR(sdcp->dcp_clk)) {
+               if (sdcp->dcp_clk != ERR_PTR(-ENOENT))
+                       return PTR_ERR(sdcp->dcp_clk);
+               sdcp->dcp_clk = NULL;

This dcp_clk assignment to NULL does not seem to be necessary.

drivers/crypto/mxs-dcp.c static int mxs_dcp_probe(struct platform_device *pdev) · line 1049

Quoted 9 lines
drivers/crypto/mxs-dcp.c +4 -0
+
+       ret = clk_prepare_enable(sdcp->dcp_clk);
+       if (ret)
+               return ret;
 
        ret = devm_request_irq(dev, dcp_vmi_irq, mxs_dcp_irq, 0,
                               "dcp-vmi-irq", sdcp);
        if (ret) {
                dev_err(dev, "Failed to claim DCP VMI IRQ!\n");

In case of subsequent errors you should call
clk_disable_unprepare(sdcp->dcp_clk).

#4 lore raw

Re: [PATCH v3 2/4] crypto: mxs-dcp - Add support for dcp clk

In reply to Fabio Estevam
Quoted 8 lines · Fabio Estevam

drivers/crypto/mxs-dcp.c static int mxs_dcp_probe(struct platform_device *pdev) · line 1039

Quoted 6 lines
drivers/crypto/mxs-dcp.c +6 -0
+       /* DCP clock is optional, only used on some SOCs */
+       sdcp->dcp_clk = devm_clk_get(dev, "dcp");
+       if (IS_ERR(sdcp->dcp_clk)) {
+               if (sdcp->dcp_clk != ERR_PTR(-ENOENT))
+                       return PTR_ERR(sdcp->dcp_clk);
+               sdcp->dcp_clk = NULL;

This dcp_clk assignment to NULL does not seem to be necessary.

The clk API ignores all calls if clk == NULL (which is very nice) but not if IS_ERR(clk). Setting dcp_clk to NULL avoids sprinkling other checks.

Quoted 11 lines

drivers/crypto/mxs-dcp.c static int mxs_dcp_probe(struct platform_device *pdev) · line 1049

Quoted 8 lines
drivers/crypto/mxs-dcp.c +3 -0
+       ret = clk_prepare_enable(sdcp->dcp_clk);
+       if (ret)
+               return ret;
 
        ret = devm_request_irq(dev, dcp_vmi_irq, mxs_dcp_irq, 0,
                               "dcp-vmi-irq", sdcp);
        if (ret) {
                dev_err(dev, "Failed to claim DCP VMI IRQ!\n");

In case of subsequent errors you should call
clk_disable_unprepare(sdcp->dcp_clk).

Yes, will resend. Maybe some day clk consumer will automatically undo pending prepare/enables on release?