On Fri, Oct 25, 2024 at 05:44:39PM +0100, Conor Dooley wrote: > On Fri, Oct 25, 2024 at 11:47:23AM +0200, Christian Marangi wrote: > > Add bindings for the Inside Secure SafeXcel EIP-93 crypto engine. > > > > The IP is present on Airoha SoC and on various Mediatek devices and > > other SoC under different names like mtk-eip93 or PKTE. > > > > Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx> > > --- > > Changes v4: > > - Out of RFC > > I left comments on v3, that I do not see addressed here. > I missed them sorry, I was confused with the other reply about RFC not asking for comments. Let me copy the comments here so we can continue here. > > Changes v3: > > - Add SoC compatible with generic one > > Changes v2: > > - Change to better compatible > > - Add description for EIP93 models > > > > .../crypto/inside-secure,safexcel-eip93.yaml | 63 +++++++++++++++++++ > > 1 file changed, 63 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/crypto/inside-secure,safexcel-eip93.yaml > > > > diff --git a/Documentation/devicetree/bindings/crypto/inside-secure,safexcel-eip93.yaml b/Documentation/devicetree/bindings/crypto/inside-secure,safexcel-eip93.yaml > > new file mode 100644 > > index 000000000000..13341710ee31 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/crypto/inside-secure,safexcel-eip93.yaml > > @@ -0,0 +1,63 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/crypto/inside-secure,safexcel-eip93.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Inside Secure SafeXcel EIP-93 cryptographic engine > > + > > +maintainers: > > + - Christian Marangi <ansuelsmth@xxxxxxxxx> > > + > > +description: | > > + The Inside Secure SafeXcel EIP-93 is a cryptographic engine IP block > > + integrated in varios devices with very different and generic name from > > + PKTE to simply vendor+EIP93. The real IP under the hood is actually > > + developed by Inside Secure and given to license to vendors. > > + > > + The IP block is sold with different model based on what feature are > > + needed and are identified with the final letter. Each letter correspond > > + to a specific set of feature and multiple letter reflect the sum of the > > + feature set. > > + > > + EIP-93 models: > > + - EIP-93i: (basic) DES/Triple DES, AES, PRNG, IPsec ESP, SRTP, SHA1 > > + - EIP-93ie: i + SHA224/256, AES-192/256 > > + - EIP-93is: i + SSL/DTLS/DTLS, MD5, ARC4 > > + - EIP-93ies: i + e + s > > + - EIP-93iw: i + AES-XCB-MAC, AES-CCM > > > This implies that you should have a non-trivial set of fallbacks, with > the "i" model as the base for that. eg: > > "ie", "i" > "is", "i" > "iw", "i" > "ies", "ie, "is", "i" (I dunno which would be a better order here) > These info are what I found around since informations about models are very scarce. The driver itself makes use of a bitmap in the IP to detect the supported stuff so the meaning of this is really to comunicate the set of feature mounted on the system. Any hint on how to describe this better? I assume you refer to some kind of yaml logic structure to put in the compatible? > > + > > +properties: > > + compatible: > > + items: > > + - const: airoha,crypto-eip93 > > + - enum: > > + - inside-secure,safexcel-eip93i > > + - inside-secure,safexcel-eip93ie > > + - inside-secure,safexcel-eip93is > > + - inside-secure,safexcel-eip93ies > > + - inside-secure,safexcel-eip93iw > > + > > I don't really get what's going on here. Why is the first compatible the > generic one? That seems suspect to me, as I doubt the crypto block on a > particular SoC varies? I'd expect to see some soc-specific compatibles > with a fallback to the inside-secure IP version that it integrates. > This was already discussed and hoped this solution was accepted (I didn't get any reply in the other revision, so I'm probably wrong) Everything started with: - airoha,mtk-eip93 Was wrong as the compatible wasn't clear on what mtk was and if the IP was from airoha (it's not, it's licensed to...) Then only the inside-secure ones, following how it's done for the newer inside-secure eip197. Krzysztof then suggested that, since it's licensed but OEM can make modification, it should be sensible to put a compatible of the SoC where the thing is mounted at the front of the other compatible. Eip197 should have received the same treatement but for some reason it didn't. So here in v3/v4 with this proposed solution. First compatible is SoC name, useful if the Vendor made modification to the IP. Then the generic model that describe the set of feature supported. I checked the register of 3 different device where EIP93 is implemented and they ALL match them. ONLY additional register are added for debug purpose and never conflicting bits are introduced. Hence why IMHO it's OK to use the combo of Vendor + second compatible for the generic implementation.
