diff mbox series

[RFC,v2,1/3] spinlock: extend guard with spinlock_bh variants

Message ID 20241017004335.27471-1-ansuelsmth@gmail.com
State Superseded
Headers show
Series [RFC,v2,1/3] spinlock: extend guard with spinlock_bh variants | expand

Commit Message

Christian Marangi Oct. 17, 2024, 12:43 a.m. UTC
Extend guard APIs with missing raw/spinlock_bh variants.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 include/linux/spinlock.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Christian Marangi Oct. 17, 2024, 12:29 p.m. UTC | #1
On Thu, Oct 17, 2024 at 10:23:54AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Oct 17, 2024 at 02:43:18AM +0200, Christian Marangi wrote:
>  +
> > +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.
> 
> You write it is licensed to vendors, so are you sure these could be
> used alone, without vendor customizations/hookups etc? I think you
> should have a dedicated, SoC-specific compatible in the front. I am not
> sure if this was discussed already, though.

Yes in v1, Rob asked some info about the compatible that was
mediatek,mtk-eip93 or airoha,mtk-eip93.

The thing is that from what I checked from different documentation
around, the register map and how the thing works doesn't change across
vendors, what I have seen is at max specific register added that are
outside of the crypto module usage (example debug register for BUS
bandwidth)

Any suggestion on what could be a good compatible? Honestly I'm more
tempted of using this similar to how is done by the new model EIP197.

> > +
> > +  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
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - inside-secure,safexcel-eip93i
> > +      - inside-secure,safexcel-eip93ie
> > +      - inside-secure,safexcel-eip93is
> > +      - inside-secure,safexcel-eip93ies
> > +      - inside-secure,safexcel-eip93iw
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    crypto@1e004000 {
> > +      compatible = "inside-secure,safexcel-eip93ies";
> > +      reg = <0x1fb70000 0x1000>;
> 
> Looks like not matching unit address.
>

Yes sorry a typo, will fix in v3.
Rob Herring (Arm) Oct. 17, 2024, 12:48 p.m. UTC | #2
On Thu, Oct 17, 2024 at 10:23:54AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Oct 17, 2024 at 02:43:18AM +0200, Christian Marangi wrote:
>  +
> > +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.
> 
> You write it is licensed to vendors, so are you sure these could be
> used alone, without vendor customizations/hookups etc? I think you
> should have a dedicated, SoC-specific compatible in the front. I am not
> sure if this was discussed already, though.

Probably should, but some reason we haven't on other Inside Secure IP. 
Perhaps they are just simple enough from a DT perspective to get away 
without. Also, there may not be any SoC associated with some of these. 
If there is an SoC, then better to add a compatible to help avoid any 
future DT changes.

Rob
Christian Marangi Oct. 17, 2024, 12:58 p.m. UTC | #3
On Thu, Oct 17, 2024 at 07:38:23AM -0500, Rob Herring wrote:
> On Thu, Oct 17, 2024 at 02:43:18AM +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@gmail.com>
> > ---
> > Changes v2:
> > - Change to better compatible
> > - Add description for EIP93 models
> > 
> >  .../crypto/inside-secure,safexcel-eip93.yaml  | 61 +++++++++++++++++++
> >  1 file changed, 61 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..fc0877d93514
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/crypto/inside-secure,safexcel-eip93.yaml
> > @@ -0,0 +1,61 @@
> > +# 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@gmail.com>
> > +
> > +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
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - inside-secure,safexcel-eip93i
> > +      - inside-secure,safexcel-eip93ie
> > +      - inside-secure,safexcel-eip93is
> > +      - inside-secure,safexcel-eip93ies
> > +      - inside-secure,safexcel-eip93iw
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> 
> 
> No clocks? All their other IP has clocks.
>

They are handled internally for each submodule. From the IP side, they
can be disabled with some register but in Autonomous mode (the one the
driver supports) they are handled automatically.

In short, yes for this "old" IP, no clock.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    crypto@1e004000 {
> > +      compatible = "inside-secure,safexcel-eip93ies";
> > +      reg = <0x1fb70000 0x1000>;
> > +
> > +      interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> > +    };
> > -- 
> > 2.45.2
> >
diff mbox series

Patch

diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 3fcd20de6ca8..e016b27f64c4 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -550,6 +550,12 @@  DEFINE_LOCK_GUARD_1(raw_spinlock_irq, raw_spinlock_t,
 
 DEFINE_LOCK_GUARD_1_COND(raw_spinlock_irq, _try, raw_spin_trylock_irq(_T->lock))
 
+DEFINE_LOCK_GUARD_1(raw_spinlock_bh, raw_spinlock_t,
+		    raw_spin_lock_bh(_T->lock),
+		    raw_spin_unlock_bh(_T->lock))
+
+DEFINE_LOCK_GUARD_1_COND(raw_spinlock_bh, _try, raw_spin_trylock_bh(_T->lock))
+
 DEFINE_LOCK_GUARD_1(raw_spinlock_irqsave, raw_spinlock_t,
 		    raw_spin_lock_irqsave(_T->lock, _T->flags),
 		    raw_spin_unlock_irqrestore(_T->lock, _T->flags),
@@ -571,6 +577,13 @@  DEFINE_LOCK_GUARD_1(spinlock_irq, spinlock_t,
 DEFINE_LOCK_GUARD_1_COND(spinlock_irq, _try,
 			 spin_trylock_irq(_T->lock))
 
+DEFINE_LOCK_GUARD_1(spinlock_bh, spinlock_t,
+		    spin_lock_bh(_T->lock),
+		    spin_unlock_bh(_T->lock))
+
+DEFINE_LOCK_GUARD_1_COND(spinlock_bh, _try,
+			 spin_trylock_bh(_T->lock))
+
 DEFINE_LOCK_GUARD_1(spinlock_irqsave, spinlock_t,
 		    spin_lock_irqsave(_T->lock, _T->flags),
 		    spin_unlock_irqrestore(_T->lock, _T->flags),