Re: [RFC PATCH 1/2] dt-bindings: crypto: Add Mediatek EIP-93 crypto engine

[Christian Marangi <ansuelsmth@gmail.com>]

On Wed, Oct 09, 2024 at 04:24:00PM -0500, Rob Herring wrote:
> On Wed, Oct 09, 2024 at 07:12:20PM +0200, Christian Marangi wrote:
> > Add bindings for the Mediatek EIP-93 crypto engine. The same IP is also
> > present on Airoha SoC.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  .../bindings/crypto/mediatek,mtk-eip93.yaml   | 40 +++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/crypto/mediatek,mtk-eip93.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/crypto/mediatek,mtk-eip93.yaml b/Documentation/devicetree/bindings/crypto/mediatek,mtk-eip93.yaml
> > new file mode 100644
> > index 000000000000..b0173b4da42d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/crypto/mediatek,mtk-eip93.yaml
> > @@ -0,0 +1,40 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/crypto/mediatek,mtk-eip93.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Mediatek EIP93 crypto engine
> > +
> > +maintainers:
> > +  - Christian Marangi <ansuelsmth@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - mediatek, mtk-eip93
> 
> space?
> 
> Why mediatek and mtk? Is eip93 an SoC? 
>

Hi Rob,

totally blind for not noticing this (and not even dt_binding_check
notice it wow)

Anyway the naming of this thing is a bit strange and hope you can give
some hint about what to use.

This is a crypto block included in various product like Mediatek SoC
(mt7621), DSP (ADSP-BF70x Blackfin) and in more recent stuff Airoha SoC
EN7581.

In documentation and in GPL source is called in various name... PKTE,
EIP93, geneirc "Crypto". One common name in drivers is tho EIP93.

Currently upstream it's supported the more recent version of this kind
of HW Crypto block, EIP197.

There the compatible is

"inside-secure,safexcel-eip197"

So from these info IN THEORY, the real produced of all this stuff is
inside-secure and the product is safexcel (confirmed also in other
product)

NOW the real problem.

From what I notice EIP93 HW Crytpo present on the old Mediatek SoC
(mt7621) have some small difference in some registry so maybe a specific
compatible will be needed.

Given this situation would it be acceptable to have

- inside-secure,safexcel-eip93
- inside-secure,safexcel-eip93-mt7621 (or maybe risky but
    more generic -mediatek ?)

The current driver doesn't fully account for the mediatek variant so it
would require some later changes. Maybe a better strategy is to just not
declare mediatek compatible for now? I know it sound stupid to ask a
question for something not entirely supported now but it's really to
understand how to move in the future. (just to prevent case where the
generic compatible is misused and we get mad on handling it in the
driver)

> > +      - airoha,mtk-eip93
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    crypto@1e004000 {
> > +      compatible = "airoha,mtk-eip93";
> > +      reg = <0x1fb70000 0x1000>;
> > +
> > +      interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> > +    };
> > -- 
> > 2.45.2
> > 

-- 
	Ansuel