Le Tue, Mar 22, 2022 at 12:00:06PM +0000, Robin Murphy a écrit :

On 2022-03-21 20:07, Corentin Labbe wrote:

quoted

The rk3399 has a crypto IP handled by the rk3288 crypto driver so adds a
node for it.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
  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.

Yes rk3399 has 2 instances of the same IP (Exception: crypto1 does not have RSA).

The problem is that only one drivername (like rk-md5) could exists.
If crypto0 and crypto1 register with different drivername (rk-md5-0/rk-md5-1), only one will be used anyway.
So I merged them into only one instance.
I think this way will be easier, but you are right, this is not pretty.

I found another way with 2 nodes:
You could preview it at https://github.com/montjoie/linux/tree/cryptorockchipv4
Basicly the crypto0 is a normal instance, and crypto1 "registers" itself against crypto0.
So if crypto0 know another instance exists it will load balance requests.

Regards

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel