Re: [PATCH v3 18/26] arm64: dts: rockchip: rk3399: add crypto node

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022-03-21 20:07, Corentin Labbe wrote:
The rk3399 has a crypto IP handled by the rk3288 crypto driver so adds a
node for it.

Signed-off-by: Corentin Labbe <clabbe@xxxxxxxxxxxx>
---
  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 88f26d89eea1..ca2c658371a5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -573,6 +573,18 @@ saradc: saradc@ff100000 {
  		status = "disabled";
  	};
+ crypto0: crypto@ff8b0000 {
+		compatible = "rockchip,rk3399-crypto";
+		reg = <0x0 0xff8b0000 0x0 0x4000>,
+		      <0x0 0xff8b8000 0x0 0x4000>;
+		interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru SCLK_CRYPTO0>, <&cru HCLK_M_CRYPTO0>, <&cru HCLK_S_CRYPTO0>,
+			 <&cru SCLK_CRYPTO1>, <&cru HCLK_M_CRYPTO1>, <&cru HCLK_S_CRYPTO1>;
+		resets = <&cru SRST_CRYPTO0>, <&cru SRST_CRYPTO0_S>, <&cru SRST_CRYPTO0_M>,
+			 <&cru SRST_CRYPTO1>, <&cru SRST_CRYPTO1_S>, <&cru SRST_CRYPTO1_M>;
+	};

What's going on here? If these are simply two instances of the same IP block as the evidence suggests, why are they crammed into a single DT node rather than simply being described as two separate instances? I was rather wondering what all the confusing mess in patch #16 was about, until I got here.

If there's something in the crypto API that means the driver can't simply naively register itself multiple times, there should be any number of ways for the probe routine to keep track of whether it's already registered something and associate any subsequent devices with the first one internally if need be. Linux implementation details should not leak out as non-standard DT weirdness.

I know the Rockchip IOMMU driver does this, but in that case the two IOMMU instances are closely coupled and sharing work such that they effectively need to be programmed identically at all times, so it was a bit more justifiable. I don't know the full story here, but it certainly looks like rk_get_engine_number() is just a means to schedule work on any available unit independently, so looks like it wouldn't take much to select between distinct devices at that point, and actually end up a lot simpler and cleaner overall.

Robin.

+
  	i2c1: i2c@ff110000 {
  		compatible = "rockchip,rk3399-i2c";
  		reg = <0x0 0xff110000 0x0 0x1000>;



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux