diff mbox series

[v8,17/23] dt-bindings: crypto: meson: correct clk and remove second interrupt line

Message ID 20240607141242.2616580-18-avromanov@salutedevices.com (mailing list archive)
State New, archived
Headers show
Series Support more Amlogic SoC families in crypto driver | expand

Commit Message

Alexey Romanov June 7, 2024, 2:12 p.m. UTC
GXL and newer SoC's uses the DMA engine (not blkmv) for crypto HW.
Crypto HW doesn't actually use the blkmv clk. At RTL level, crypto
engine is hard weired to a clk81 (CLKID_CLK81).

Also, GXL crypto IP isn't to seconnd interrput line. So we must
remove it from dt-bindings.

Fixes: 7f7d115dfb51 ("dt-bindings: crypto: Add DT bindings
documentation for amlogic-crypto")

Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
---
 .../bindings/crypto/amlogic,gxl-crypto.yaml          | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Rob Herring June 10, 2024, 10:28 p.m. UTC | #1
On Fri, Jun 07, 2024 at 05:12:36PM +0300, Alexey Romanov wrote:
> GXL and newer SoC's uses the DMA engine (not blkmv) for crypto HW.
> Crypto HW doesn't actually use the blkmv clk. At RTL level, crypto
> engine is hard weired to a clk81 (CLKID_CLK81).

typo.

> 
> Also, GXL crypto IP isn't to seconnd interrput line. So we must

2 more typos. Spell checkers exist. Use them instead of me please.

I think you forgot the word 'connected' too.

> remove it from dt-bindings.


So did this binding not work at all? Are there any users? You need a bit 
more justification for an ABI breaking change.

> 
> Fixes: 7f7d115dfb51 ("dt-bindings: crypto: Add DT bindings
> documentation for amlogic-crypto")

This line should not be wrapped.
> 

Drop the blank line.

> Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
> ---
>  .../bindings/crypto/amlogic,gxl-crypto.yaml          | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> index 948e11ebe4ee..aff6f3234dc9 100644
> --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> @@ -20,13 +20,15 @@ properties:
>    interrupts:
>      items:
>        - description: Interrupt for flow 0
> -      - description: Interrupt for flow 1
>  
>    clocks:
>      maxItems: 1
>  
>    clock-names:
> -    const: blkmv
> +    const: clk81

Clocks are supposed be named local to the block like what function or 
part of the block they clock. This sounds like something global. 

With only 1 clock, I'd just drop the name altogether.

> +
> +  power-domains:
> +    maxItems: 1
>  
>  required:
>    - compatible
> @@ -46,7 +48,7 @@ examples:
>      crypto: crypto-engine@c883e000 {
>          compatible = "amlogic,gxl-crypto";
>          reg = <0xc883e000 0x36>;
> -        interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>, <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>;
> -        clocks = <&clkc CLKID_BLKMV>;
> -        clock-names = "blkmv";
> +        interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>;
> +        clocks = <&clkc CLKID_CLK81>;
> +        clock-names = "clk81";
>      };
> -- 
> 2.34.1
>
Alexey Romanov July 8, 2024, 10:33 a.m. UTC | #2
Hi Rob,

thank you for the review.

On Mon, Jun 10, 2024 at 04:28:27PM -0600, Rob Herring wrote:
> On Fri, Jun 07, 2024 at 05:12:36PM +0300, Alexey Romanov wrote:
> > GXL and newer SoC's uses the DMA engine (not blkmv) for crypto HW.
> > Crypto HW doesn't actually use the blkmv clk. At RTL level, crypto
> > engine is hard weired to a clk81 (CLKID_CLK81).
> 
> typo.
> 
> > 
> > Also, GXL crypto IP isn't to seconnd interrput line. So we must
> 
> 2 more typos. Spell checkers exist. Use them instead of me please.
> 
> I think you forgot the word 'connected' too.

Yep, sorry, I will fix all typos.

> 
> > remove it from dt-bindings.
> 
> 
> So did this binding not work at all? 

Yes.

> Are there any users? 

No.

> You need a bit 
> more justification for an ABI breaking change.

I will add more clarification information in commit message in V9.

> 
> > 
> > Fixes: 7f7d115dfb51 ("dt-bindings: crypto: Add DT bindings
> > documentation for amlogic-crypto")
> 
> This line should not be wrapped.
> > 
> 
> Drop the blank line.
> 
> > Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
> > ---
> >  .../bindings/crypto/amlogic,gxl-crypto.yaml          | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> > index 948e11ebe4ee..aff6f3234dc9 100644
> > --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> > +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> > @@ -20,13 +20,15 @@ properties:
> >    interrupts:
> >      items:
> >        - description: Interrupt for flow 0
> > -      - description: Interrupt for flow 1
> >  
> >    clocks:
> >      maxItems: 1
> >  
> >    clock-names:
> > -    const: blkmv
> > +    const: clk81
> 
> Clocks are supposed be named local to the block like what function or 
> part of the block they clock. This sounds like something global. 
> 
> With only 1 clock, I'd just drop the name altogether.

I think I have to remove clock-names field in V9.

> 
> > +
> > +  power-domains:
> > +    maxItems: 1
> >  
> >  required:
> >    - compatible
> > @@ -46,7 +48,7 @@ examples:
> >      crypto: crypto-engine@c883e000 {
> >          compatible = "amlogic,gxl-crypto";
> >          reg = <0xc883e000 0x36>;
> > -        interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>, <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>;
> > -        clocks = <&clkc CLKID_BLKMV>;
> > -        clock-names = "blkmv";
> > +        interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>;
> > +        clocks = <&clkc CLKID_CLK81>;
> > +        clock-names = "clk81";
> >      };
> > -- 
> > 2.34.1
> >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
index 948e11ebe4ee..aff6f3234dc9 100644
--- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
+++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
@@ -20,13 +20,15 @@  properties:
   interrupts:
     items:
       - description: Interrupt for flow 0
-      - description: Interrupt for flow 1
 
   clocks:
     maxItems: 1
 
   clock-names:
-    const: blkmv
+    const: clk81
+
+  power-domains:
+    maxItems: 1
 
 required:
   - compatible
@@ -46,7 +48,7 @@  examples:
     crypto: crypto-engine@c883e000 {
         compatible = "amlogic,gxl-crypto";
         reg = <0xc883e000 0x36>;
-        interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>, <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>;
-        clocks = <&clkc CLKID_BLKMV>;
-        clock-names = "blkmv";
+        interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>;
+        clocks = <&clkc CLKID_CLK81>;
+        clock-names = "clk81";
     };