From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0A6ADC433EF for ; Wed, 23 Mar 2022 16:30:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=kFQeb+6h7RBgzfUexBegLRGID8kb2aMXujK74R6aAOM=; b=uBXZrfG6oi1Ay9 yQhnjAVJHT1EXB0wmJ7ujf2hwk2nokARbT4+QSG3BA7HHJR4gOnnHpQf4Pbp+EFB+xNpeMw8r3vM6 a88K18hYgCsGC5qSvrn6y4rv2V4Ly7kjxa5cpnQAshQPZaqZKrbr6npDHHJEzU2iV/xPiTYI7WBZV +OP/F+JtG87PoXHFgIekWfuOt0uFcqfx7ZQRBK/jMvlNdAG74Isr+bm6bwqn3j/IuL7uG3peUZ98B IMsMlebsi5oDwlU0q98F16iPLKg8OR2d7p3Q+oMI+EAjpnbYNKpeV5wi10YFdMnyhH6coYbcmsgce VOaKdJrTV2Kd9fhzkz+Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nX3ql-00EFLT-Lz; Wed, 23 Mar 2022 16:28:43 +0000 Received: from gloria.sntech.de ([185.11.138.130]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nX3qh-00EFIz-Ro; Wed, 23 Mar 2022 16:28:41 +0000 Received: from ip5b412258.dynamic.kabel-deutschland.de ([91.65.34.88] helo=diego.localnet) by gloria.sntech.de with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nX3qT-00030m-16; Wed, 23 Mar 2022 17:28:25 +0100 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: Robin Murphy , LABBE Corentin Cc: herbert@gondor.apana.org.au, krzk+dt@kernel.org, mturquette@baylibre.com, robh+dt@kernel.org, sboyd@kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org Subject: Re: [PATCH v3 18/26] arm64: dts: rockchip: rk3399: add crypto node Date: Wed, 23 Mar 2022 17:28:24 +0100 Message-ID: <1898921.yKVeVyVuyW@diego> In-Reply-To: References: <20220321200739.3572792-1-clabbe@baylibre.com> <70422777-a3f9-b2f1-5faa-94d24fe200ac@arm.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220323_092839_911836_93A88213 X-CRM114-Status: GOOD ( 39.28 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Am Mittwoch, 23. M=E4rz 2022, 14:22:41 CET schrieb LABBE Corentin: > Le Tue, Mar 22, 2022 at 12:00:06PM +0000, Robin Murphy a =E9crit : > > On 2022-03-21 20:07, Corentin Labbe wrote: > > > The rk3399 has a crypto IP handled by the rk3288 crypto driver so add= s a > > > node for it. > > > = > > > Signed-off-by: Corentin Labbe > > > --- > > > 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/bo= ot/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 =3D "disabled"; > > > }; > > > = > > > + crypto0: crypto@ff8b0000 { > > > + compatible =3D "rockchip,rk3399-crypto"; > > > + reg =3D <0x0 0xff8b0000 0x0 0x4000>, > > > + <0x0 0xff8b8000 0x0 0x4000>; > > > + interrupts =3D , > > > + ; > > > + clocks =3D <&cru SCLK_CRYPTO0>, <&cru HCLK_M_CRYPTO0>, <&cru HCLK_= S_CRYPTO0>, > > > + <&cru SCLK_CRYPTO1>, <&cru HCLK_M_CRYPTO1>, <&cru HCLK_S_CRYPTO1= >; > > > + resets =3D <&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 wa= s = > > 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 shoul= d = > > 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 certainl= y = > > 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 t= o = > > select between distinct devices at that point, and actually end up a lo= t = > > simpler and cleaner overall. > = > Yes rk3399 has 2 instances of the same IP (Exception: crypto1 does not ha= ve 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-md= 5-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/cryptorock= chipv4 > 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. The DT-nodes in that branch are @@ -573,6 +573,22 @@ status =3D "disabled"; }; = + crypto0: crypto@ff8b0000 { + compatible =3D "rockchip,rk3399-crypto0"; + reg =3D <0x0 0xff8b0000 0x0 0x4000>; + interrupts =3D ; + clocks =3D <&cru SCLK_CRYPTO0>, <&cru HCLK_M_CRYPTO0>, <&cru HCLK_S_CRYP= TO0>; + resets =3D <&cru SRST_CRYPTO0>, <&cru SRST_CRYPTO0_S>, <&cru SRST_CRYPTO= 0_M>; + }; + + crypto1: crypto@ff8b8000 { + compatible =3D "rockchip,rk3399-crypto1"; + reg =3D <0x0 0xff8b8000 0x0 0x4000>; + interrupts =3D ; + clocks =3D <&cru SCLK_CRYPTO1>, <&cru HCLK_M_CRYPTO1>, <&cru HCLK_S_CRYP= TO1>; + resets =3D <&cru SRST_CRYPTO1>, <&cru SRST_CRYPTO1_S>, <&cru SRST_CRYPTO= 1_M>; + }; + i2c1: i2c@ff110000 { compatible =3D "rockchip,rk3399-i2c"; reg =3D <0x0 0xff110000 0x0 0x1000>; which looks at lot better :-) . I'm not sure about the different compatibles yet, but as the blocks are really _not_ the same implementation that actually does make sense [i.e. one not having RSA] Though I think you'll need to update the binding for them. Heiko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel